D19933.id47572.diff
No OneTemporary
Actions

Size

32 KB

Referenced Files

None

Subscribers

None

D19933.id47572.diff
View Options

	diff --git a/resources/sql/autopatches/20181222.banish.01.user.sql b/resources/sql/autopatches/20181222.banish.01.user.sql
	new file mode 100644
	--- /dev/null
	+++ b/resources/sql/autopatches/20181222.banish.01.user.sql
	@@ -0,0 +1,2 @@
	+ALTER TABLE {$NAMESPACE}_user.user
	+ ADD isBanished BOOL NOT NULL;
	diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
	--- a/src/__phutil_library_map__.php
	+++ b/src/__phutil_library_map__.php
	@@ -4608,6 +4608,7 @@
	'PhabricatorUser' => 'applications/people/storage/PhabricatorUser.php',
	'PhabricatorUserApproveTransaction' => 'applications/people/xaction/PhabricatorUserApproveTransaction.php',
	'PhabricatorUserBadgesCacheType' => 'applications/people/cache/PhabricatorUserBadgesCacheType.php',
	+ 'PhabricatorUserBanishTransaction' => 'applications/people/xaction/PhabricatorUserBanishTransaction.php',
	'PhabricatorUserBlurbField' => 'applications/people/customfield/PhabricatorUserBlurbField.php',
	'PhabricatorUserCache' => 'applications/people/storage/PhabricatorUserCache.php',
	'PhabricatorUserCachePurger' => 'applications/cache/purger/PhabricatorUserCachePurger.php',
	@@ -10673,6 +10674,7 @@
	),
	'PhabricatorUserApproveTransaction' => 'PhabricatorUserTransactionType',
	'PhabricatorUserBadgesCacheType' => 'PhabricatorUserCacheType',
	+ 'PhabricatorUserBanishTransaction' => 'PhabricatorUserTransactionType',
	'PhabricatorUserBlurbField' => 'PhabricatorUserCustomField',
	'PhabricatorUserCache' => 'PhabricatorUserDAO',
	'PhabricatorUserCachePurger' => 'PhabricatorCachePurger',
	diff --git a/src/applications/auth/controller/PhabricatorDisabledUserController.php b/src/applications/auth/controller/PhabricatorDisabledUserController.php
	--- a/src/applications/auth/controller/PhabricatorDisabledUserController.php
	+++ b/src/applications/auth/controller/PhabricatorDisabledUserController.php
	@@ -15,10 +15,18 @@
	return new Aphront404Response();
	}

	+ if ($viewer->getIsBanished()) {
	+ $title = pht('Account Banished');
	+ $body = pht('Your account has been banished.');
	+ } else {
	+ $title = pht('Account Disabled');
	+ $body = pht('Your account has been disabled.');
	+ }
	+
	return $this->newDialog()
	- ->setTitle(pht('Account Disabled'))
	- ->addCancelButton('/logout/', pht('Okay'))
	- ->appendParagraph(pht('Your account has been disabled.'));
	+ ->setTitle($title)
	+ ->appendParagraph($body)
	+ ->addCancelButton('/logout/', pht('Okay'));
	}

	}
	diff --git a/src/applications/people/application/PhabricatorPeopleApplication.php b/src/applications/people/application/PhabricatorPeopleApplication.php
	--- a/src/applications/people/application/PhabricatorPeopleApplication.php
	+++ b/src/applications/people/application/PhabricatorPeopleApplication.php
	@@ -52,9 +52,7 @@
	=> 'PhabricatorPeopleInviteSendController',
	),
	'approve/(?P<id>[1-9]\d*)/' => 'PhabricatorPeopleApproveController',
	- '(?P<via>disapprove)/(?P<id>[1-9]\d*)/'
	- => 'PhabricatorPeopleDisableController',
	- '(?P<via>disable)/(?P<id>[1-9]\d*)/'
	+ '(?P<via>disapprove\|disable\|banish)/(?P<id>[1-9]\d*)/'
	=> 'PhabricatorPeopleDisableController',
	'empower/(?P<id>[1-9]\d*)/' => 'PhabricatorPeopleEmpowerController',
	'delete/(?P<id>[1-9]\d*)/' => 'PhabricatorPeopleDeleteController',
	diff --git a/src/applications/people/controller/PhabricatorPeopleDisableController.php b/src/applications/people/controller/PhabricatorPeopleDisableController.php
	--- a/src/applications/people/controller/PhabricatorPeopleDisableController.php
	+++ b/src/applications/people/controller/PhabricatorPeopleDisableController.php
	@@ -29,6 +29,8 @@
	// Disabling via "Disable" requires the permission only.

	$is_disapprove = ($via == 'disapprove');
	+ $is_banish = ($via == 'banish');
	+
	if ($is_disapprove) {
	$done_uri = $this->getApplicationURI('query/approval/');

	@@ -63,15 +65,28 @@
	->setTitle(pht('Something Stays Your Hand'))
	->appendParagraph(
	pht(
	- 'Try as you might, you find you can not disable your own account.'))
	+ 'Try as you might, you find you can not disable or banish '.
	+ 'your own account.'))
	->addCancelButton($done_uri, pht('Curses!'));
	}

	+ // Make the "undisable/unbanish" action always apply the appropriate
	+ // type of change, regardless of how the controller was invoked.
	+ if (!$should_disable) {
	+ $is_banish = $user->getIsBanished();
	+ }
	+
	if ($request->isFormPost()) {
	$xactions = array();

	+ if ($is_banish) {
	+ $xaction_type = PhabricatorUserBanishTransaction::TRANSACTIONTYPE;
	+ } else {
	+ $xaction_type = PhabricatorUserDisableTransaction::TRANSACTIONTYPE;
	+ }
	+
	$xactions[] = id(new PhabricatorUserTransaction())
	- ->setTransactionType(PhabricatorUserDisableTransaction::TRANSACTIONTYPE)
	+ ->setTransactionType($xaction_type)
	->setNewValue($should_disable);

	id(new PhabricatorUserTransactionEditor())
	@@ -85,26 +100,52 @@
	return id(new AphrontRedirectResponse())->setURI($done_uri);
	}

	- if ($should_disable) {
	- $title = pht('Disable User?');
	- $short_title = pht('Disable User');
	-
	- $body = pht(
	- 'Disable %s? They will no longer be able to access Phabricator or '.
	- 'receive email.',
	- phutil_tag('strong', array(), $user->getUsername()));
	+ $display_username = phutil_tag('strong', array(), $user->getUsername());

	- $submit = pht('Disable User');
	+ if ($should_disable) {
	+ if ($is_banish) {
	+ $title = pht('Banish User?');
	+ $short_title = pht('Banish User');
	+
	+ $body = pht(
	+ 'Banish %s? They will no longer be able to access Phabricator or '.
	+ 'receive email, and their profile and comments will be hidden.',
	+ $display_username);
	+
	+ $submit = pht('Banish User');
	+ } else {
	+ $title = pht('Disable User?');
	+ $short_title = pht('Disable User');
	+
	+ $body = pht(
	+ 'Disable %s? They will no longer be able to access Phabricator or '.
	+ 'receive email.',
	+ $display_username);
	+
	+ $submit = pht('Disable User');
	+ }
	} else {
	- $title = pht('Enable User?');
	- $short_title = pht('Enable User');
	-
	- $body = pht(
	- 'Enable %s? They will be able to access Phabricator and receive '.
	- 'email again.',
	- phutil_tag('strong', array(), $user->getUsername()));
	-
	- $submit = pht('Enable User');
	+ if ($is_banish) {
	+ $title = pht('Unbanish User?');
	+ $short_title = pht('Unbanish User');
	+
	+ $body = pht(
	+ 'Unbanish %s? They will be able to access Phabricator and receive '.
	+ 'email again, and their profile and comments will become visible.',
	+ $display_username);
	+
	+ $submit = pht('Unbanish User');
	+ } else {
	+ $title = pht('Enable User?');
	+ $short_title = pht('Enable User');
	+
	+ $body = pht(
	+ 'Enable %s? They will be able to access Phabricator and receive '.
	+ 'email again.',
	+ $display_username);
	+
	+ $submit = pht('Enable User');
	+ }
	}

	return $this->newDialog()
	diff --git a/src/applications/people/controller/PhabricatorPeopleProfileController.php b/src/applications/people/controller/PhabricatorPeopleProfileController.php
	--- a/src/applications/people/controller/PhabricatorPeopleProfileController.php
	+++ b/src/applications/people/controller/PhabricatorPeopleProfileController.php
	@@ -74,6 +74,9 @@
	if ($user->getIsAdmin()) {
	$roles[] = pht('Administrator');
	}
	+ if ($user->getIsBanished()) {
	+ $roles[] = pht('Banished');
	+ }
	if ($user->getIsDisabled()) {
	$roles[] = pht('Disabled');
	}
	@@ -106,7 +109,9 @@

	require_celerity_resource('project-view-css');

	- if ($user->getIsDisabled()) {
	+ if ($user->getIsBanished()) {
	+ $header->setStatus('fa-gavel', 'red', pht('Banished'));
	+ } else if ($user->getIsDisabled()) {
	$header->setStatus('fa-ban', 'red', pht('Disabled'));
	} else {
	$header->setStatus($profile_icon, 'bluegrey', $profile_title);
	@@ -125,4 +130,23 @@
	return $header;
	}

	+ protected function newProfileAccessResponse(PhabricatorUser $user) {
	+ if ($user->getIsBanished()) {
	+ $has_disable = $this->hasApplicationCapability(
	+ PeopleDisableUsersCapability::CAPABILITY);
	+
	+ if (!$has_disable) {
	+ return $this->newDialog()
	+ ->setTitle(pht('User Banished'))
	+ ->appendParagraph(
	+ pht(
	+ 'This user has been banished. You must have the '.
	+ '"Can Disable Users" permission to view their profile.'))
	+ ->addCancelButton('/', pht('Done'));
	+ }
	+ }
	+
	+ return null;
	+ }
	+
	}
	diff --git a/src/applications/people/controller/PhabricatorPeopleProfileManageController.php b/src/applications/people/controller/PhabricatorPeopleProfileManageController.php
	--- a/src/applications/people/controller/PhabricatorPeopleProfileManageController.php
	+++ b/src/applications/people/controller/PhabricatorPeopleProfileManageController.php
	@@ -22,6 +22,11 @@
	return new Aphront404Response();
	}

	+ $response = $this->newProfileAccessResponse($user);
	+ if ($response) {
	+ return $response;
	+ }
	+
	$this->setUser($user);
	$header = $this->buildProfileHeader();

	@@ -144,12 +149,29 @@
	->setWorkflow(true)
	->setHref($this->getApplicationURI('rename/'.$user->getID().'/')));

	- if ($user->getIsDisabled()) {
	+ $curtain->addAction(
	+ id(new PhabricatorActionView())
	+ ->setIcon('fa-envelope')
	+ ->setName(pht('Send Welcome Email'))
	+ ->setWorkflow(true)
	+ ->setDisabled(!$can_welcome)
	+ ->setHref($this->getApplicationURI('welcome/'.$user->getID().'/')));
	+
	+ $curtain->addAction(
	+ id(new PhabricatorActionView())
	+ ->setType(PhabricatorActionView::TYPE_DIVIDER));
	+
	+ $show_banish = false;
	+ if ($user->getIsBanished()) {
	+ $disable_icon = 'fa-check-circle-o';
	+ $disable_name = pht('Unbanish User');
	+ } else if ($user->getIsDisabled()) {
	$disable_icon = 'fa-check-circle-o';
	$disable_name = pht('Enable User');
	} else {
	$disable_icon = 'fa-ban';
	$disable_name = pht('Disable User');
	+ $show_banish = true;
	}

	$curtain->addAction(
	@@ -160,6 +182,16 @@
	->setWorkflow(true)
	->setHref($this->getApplicationURI('disable/'.$user->getID().'/')));

	+ if ($show_banish) {
	+ $curtain->addAction(
	+ id(new PhabricatorActionView())
	+ ->setIcon('fa-gavel')
	+ ->setName(pht('Banish User'))
	+ ->setDisabled(!$can_disable)
	+ ->setWorkflow(true)
	+ ->setHref($this->getApplicationURI('banish/'.$user->getID().'/')));
	+ }
	+
	$curtain->addAction(
	id(new PhabricatorActionView())
	->setIcon('fa-times')
	@@ -170,11 +202,7 @@

	$curtain->addAction(
	id(new PhabricatorActionView())
	- ->setIcon('fa-envelope')
	- ->setName(pht('Send Welcome Email'))
	- ->setWorkflow(true)
	- ->setDisabled(!$can_welcome)
	- ->setHref($this->getApplicationURI('welcome/'.$user->getID().'/')));
	+ ->setType(PhabricatorActionView::TYPE_DIVIDER));

	return $curtain;
	}
	diff --git a/src/applications/people/phid/PhabricatorPeopleUserPHIDType.php b/src/applications/people/phid/PhabricatorPeopleUserPHIDType.php
	--- a/src/applications/people/phid/PhabricatorPeopleUserPHIDType.php
	+++ b/src/applications/people/phid/PhabricatorPeopleUserPHIDType.php
	@@ -38,14 +38,21 @@

	foreach ($handles as $phid => $handle) {
	$user = $objects[$phid];
	- $realname = $user->getRealName();
	$username = $user->getUsername();

	+ if ($user->getIsBanished()) {
	+ $profile_image = celerity_get_resource_uri('/rsrc/image/avatar.png');
	+ $full_name = $username;
	+ } else {
	+ $profile_image = $user->getProfileImageURI();
	+ $full_name = $user->getFullName();
	+ }
	+
	$handle
	->setName($username)
	->setURI('/p/'.$username.'/')
	- ->setFullName($user->getFullName())
	- ->setImageURI($user->getProfileImageURI())
	+ ->setFullName($full_name)
	+ ->setImageURI($profile_image)
	->setMailStampName('@'.$username);

	if ($user->getIsMailingList()) {
	diff --git a/src/applications/people/query/PhabricatorPeopleQuery.php b/src/applications/people/query/PhabricatorPeopleQuery.php
	--- a/src/applications/people/query/PhabricatorPeopleQuery.php
	+++ b/src/applications/people/query/PhabricatorPeopleQuery.php
	@@ -19,6 +19,7 @@
	private $nameTokens;
	private $namePrefixes;
	private $isEnrolledInMultiFactor;
	+ private $isBanished;

	private $needPrimaryEmail;
	private $needProfile;
	@@ -87,6 +88,11 @@
	return $this;
	}

	+ public function withIsBanished($banished) {
	+ $this->isBanished = $banished;
	+ return $this;
	+ }
	+
	public function withNameLike($like) {
	$this->nameLike = $like;
	return $this;
	@@ -356,6 +362,13 @@
	(int)$this->isEnrolledInMultiFactor);
	}

	+ if ($this->isBanished !== null) {
	+ $where[] = qsprintf(
	+ $conn,
	+ 'user.isBanished = %d',
	+ (int)$this->isBanished);
	+ }
	+
	return $where;
	}

	diff --git a/src/applications/people/query/PhabricatorPeopleSearchEngine.php b/src/applications/people/query/PhabricatorPeopleSearchEngine.php
	--- a/src/applications/people/query/PhabricatorPeopleSearchEngine.php
	+++ b/src/applications/people/query/PhabricatorPeopleSearchEngine.php
	@@ -84,6 +84,17 @@
	pht(
	'Pass true to find only users awaiting administrative approval, '.
	'or false to omit these users.')),
	+ id(new PhabricatorSearchThreeStateField())
	+ ->setLabel(pht('Banished'))
	+ ->setKey('banished')
	+ ->setOptions(
	+ pht('(Show All)'),
	+ pht('Show Only Banished Users'),
	+ pht('Hide Banished Users'))
	+ ->setDescription(
	+ pht(
	+ 'Pass true to find only users who have been banished, '.
	+ 'or false to omit these users.')),
	);

	$viewer = $this->requireViewer();
	@@ -169,6 +180,10 @@
	$query->withIsApproved(!$map['needsApproval']);
	}

	+ if ($map['banished'] !== null) {
	+ $query->withIsBanished($map['banished']);
	+ }
	+
	if (idx($map, 'mfa') !== null) {
	$viewer = $this->requireViewer();
	if (!$viewer->getIsAdmin()) {
	@@ -263,7 +278,10 @@
	$item->addAttribute($primary_email->getAddress());
	}

	- if ($user->getIsDisabled()) {
	+ if ($user->getIsBanished()) {
	+ $item->addIcon('fa-gavel', pht('Banished'));
	+ $item->setDisabled(true);
	+ } else if ($user->getIsDisabled()) {
	$item->addIcon('fa-ban', pht('Disabled'));
	$item->setDisabled(true);
	}
	@@ -336,9 +354,15 @@

	$export = array();
	foreach ($users as $user) {
	+ if ($user->getIsBanished()) {
	+ $real_name = null;
	+ } else {
	+ $real_name = $user->getRealName();
	+ }
	+
	$export[] = array(
	'username' => $user->getUsername(),
	- 'realName' => $user->getRealName(),
	+ 'realName' => $real_name,
	);
	}

	diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php
	--- a/src/applications/people/storage/PhabricatorUser.php
	+++ b/src/applications/people/storage/PhabricatorUser.php
	@@ -44,6 +44,7 @@
	protected $isEmailVerified = 0;
	protected $isApproved = 0;
	protected $isEnrolledInMultiFactor = 0;
	+ protected $isBanished = 0;

	protected $accountSecret;

	@@ -113,6 +114,10 @@
	return false;
	}

	+ if ($this->getIsBanished()) {
	+ return false;
	+ }
	+
	if (PhabricatorUserEmail::isEmailVerificationRequired()) {
	if (!$this->getIsEmailVerified()) {
	return false;
	@@ -229,6 +234,7 @@
	'availabilityCacheTTL' => 'uint32?',
	'defaultProfileImagePHID' => 'phid?',
	'defaultProfileImageVersion' => 'text64?',
	+ 'isBanished' => 'bool',
	),
	self::CONFIG_KEY_SCHEMA => array(
	'key_phid' => null,
	@@ -246,6 +252,9 @@
	'key_approved' => array(
	'columns' => array('isApproved'),
	),
	+ 'key_banished' => array(
	+ 'columns' => array('isBanished'),
	+ ),
	),
	self::CONFIG_NO_MUTATE => array(
	'availabilityCache' => true,
	@@ -1443,9 +1452,19 @@
	$roles[] = 'activated';
	}

	+ if ($this->getIsBanished()) {
	+ $roles[] = 'banished';
	+ }
	+
	+ if ($this->getIsBanished()) {
	+ $real_name = null;
	+ } else {
	+ $real_name = $this->getRealName();
	+ }
	+
	return array(
	'username' => $this->getUsername(),
	- 'realName' => $this->getRealName(),
	+ 'realName' => $real_name,
	'roles' => $roles,
	);
	}
	diff --git a/src/applications/people/storage/PhabricatorUserLog.php b/src/applications/people/storage/PhabricatorUserLog.php
	--- a/src/applications/people/storage/PhabricatorUserLog.php
	+++ b/src/applications/people/storage/PhabricatorUserLog.php
	@@ -18,6 +18,7 @@
	const ACTION_SYSTEM_AGENT = 'system-agent';
	const ACTION_MAILING_LIST = 'mailing-list';
	const ACTION_DISABLE = 'disable';
	+ const ACTION_BANISH = 'banish';
	const ACTION_APPROVE = 'approve';
	const ACTION_DELETE = 'delete';

	@@ -83,6 +84,7 @@
	self::ACTION_FAIL_HISEC => pht('Hisec: Failed Attempt'),
	self::ACTION_MULTI_ADD => pht('Multi-Factor: Add Factor'),
	self::ACTION_MULTI_REMOVE => pht('Multi-Factor: Remove Factor'),
	+ self::ACTION_BANISH => pht('Banish'),
	);
	}

	diff --git a/src/applications/people/xaction/PhabricatorUserDisableTransaction.php b/src/applications/people/xaction/PhabricatorUserBanishTransaction.php
	copy from src/applications/people/xaction/PhabricatorUserDisableTransaction.php
	copy to src/applications/people/xaction/PhabricatorUserBanishTransaction.php
	--- a/src/applications/people/xaction/PhabricatorUserDisableTransaction.php
	+++ b/src/applications/people/xaction/PhabricatorUserBanishTransaction.php
	@@ -1,12 +1,12 @@
	<?php

	-final class PhabricatorUserDisableTransaction
	+final class PhabricatorUserBanishTransaction
	extends PhabricatorUserTransactionType {

	- const TRANSACTIONTYPE = 'user.disable';
	+ const TRANSACTIONTYPE = 'user.banish';

	public function generateOldValue($object) {
	- return (bool)$object->getIsDisabled();
	+ return (bool)$object->getIsBanished();
	}

	public function generateNewValue($object, $value) {
	@@ -14,12 +14,14 @@
	}

	public function applyInternalEffects($object, $value) {
	- $object->setIsDisabled((int)$value);
	+ $object
	+ ->setIsDisabled((int)$value)
	+ ->setIsBanished((int)$value);
	}

	public function applyExternalEffects($object, $value) {
	$this->newUserLog(PhabricatorUserLog::ACTION_DISABLE)
	- ->setOldValue((bool)$object->getIsDisabled())
	+ ->setOldValue((bool)$object->getIsBanished())
	->setNewValue((bool)$value)
	->save();
	}
	@@ -28,18 +30,16 @@
	$new = $this->getNewValue();
	if ($new) {
	return pht(
	- '%s disabled this user.',
	+ '%s banished this user to the shadow realm.',
	$this->renderAuthor());
	} else {
	return pht(
	- '%s enabled this user.',
	+ '%s unbanished this user.',
	$this->renderAuthor());
	}
	}

	public function shouldHideForFeed() {
	- // Don't publish feed stories about disabling users, since this can be
	- // a sensitive action.
	return true;
	}

	@@ -47,19 +47,19 @@
	$errors = array();

	foreach ($xactions as $xaction) {
	- $is_disabled = (bool)$object->getIsDisabled();
	+ $is_banished = (bool)$object->getIsBanished();

	- if ((bool)$xaction->getNewValue() === $is_disabled) {
	+ if ((bool)$xaction->getNewValue() === $is_banished) {
	continue;
	}

	- // You must have the "Can Disable Users" permission to disable a user.
	+ // You must have the "Can Disable Users" permission to banish a user.
	$this->requireApplicationCapability(
	PeopleDisableUsersCapability::CAPABILITY);

	if ($this->getActingAsPHID() === $object->getPHID()) {
	$errors[] = $this->newInvalidError(
	- pht('You can not enable or disable your own account.'));
	+ pht('You can not banish or unbanish your own account.'));
	}
	}

	@@ -69,11 +69,6 @@
	public function getRequiredCapabilities(
	$object,
	PhabricatorApplicationTransaction $xaction) {
	-
	- // You do not need to be able to edit users to disable them. Instead, this
	- // requirement is replaced with a requirement that you have the "Can
	- // Disable Users" permission.
	-
	return null;
	}
	}
	diff --git a/src/applications/people/xaction/PhabricatorUserDisableTransaction.php b/src/applications/people/xaction/PhabricatorUserDisableTransaction.php
	--- a/src/applications/people/xaction/PhabricatorUserDisableTransaction.php
	+++ b/src/applications/people/xaction/PhabricatorUserDisableTransaction.php
	@@ -53,13 +53,23 @@
	continue;
	}

	+ if ($object->getIsBanished()) {
	+ $errors[] = $this->newInvalidError(
	+ pht(
	+ 'This user has been banished. To restore their account, you '.
	+ 'must unbanish them.'),
	+ $xaction);
	+ continue;
	+ }
	+
	// You must have the "Can Disable Users" permission to disable a user.
	$this->requireApplicationCapability(
	PeopleDisableUsersCapability::CAPABILITY);

	if ($this->getActingAsPHID() === $object->getPHID()) {
	$errors[] = $this->newInvalidError(
	- pht('You can not enable or disable your own account.'));
	+ pht('You can not enable or disable your own account.'),
	+ $xaction);
	}
	}

	diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php
	--- a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php
	+++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php
	@@ -9,6 +9,11 @@
	$xaction = id(new PhabricatorObjectQuery())
	->setViewer($viewer)
	->withPHIDs(array($request->getURIData('phid')))
	+ ->requireCapabilities(
	+ array(
	+ PhabricatorPolicyCapability::CAN_VIEW,
	+ PhabricatorPolicyCapability::CAN_EDIT,
	+ ))
	->executeOne();
	if (!$xaction) {
	return new Aphront404Response();
	diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentHistoryController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentHistoryController.php
	--- a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentHistoryController.php
	+++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentHistoryController.php
	@@ -15,25 +15,20 @@
	->withPHIDs(array($phid))
	->setViewer($viewer)
	->executeOne();
	-
	if (!$xaction) {
	return new Aphront404Response();
	}

	- if (!$xaction->getComment()) {
	- // You can't view history of a transaction with no comments.
	- return new Aphront404Response();
	- }
	-
	- if ($xaction->getComment()->getIsRemoved()) {
	- // You can't view history of a transaction with a removed comment.
	- return new Aphront400Response();
	+ $response = $this->newAccessResponse($xaction);
	+ if ($response) {
	+ return $response;
	}

	$comments = id(new PhabricatorApplicationTransactionTemplatedCommentQuery())
	->setViewer($viewer)
	->setTemplate($xaction->getApplicationTransactionCommentObject())
	->withTransactionPHIDs(array($xaction->getPHID()))
	+ ->needBanishments(true)
	->execute();

	if (!$comments) {
	diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentQuoteController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentQuoteController.php
	--- a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentQuoteController.php
	+++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentQuoteController.php
	@@ -15,16 +15,9 @@
	return new Aphront404Response();
	}

	- if (!$xaction->getComment()) {
	- return new Aphront404Response();
	- }
	-
	- if ($xaction->getComment()->getIsRemoved()) {
	- return new Aphront400Response();
	- }
	-
	- if (!$xaction->hasComment()) {
	- return new Aphront404Response();
	+ $response = $this->newAccessResponse($xaction);
	+ if ($response) {
	+ return $response;
	}

	$content = $xaction->getComment()->getContent();
	diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentRawController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentRawController.php
	--- a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentRawController.php
	+++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentRawController.php
	@@ -3,6 +3,10 @@
	final class PhabricatorApplicationTransactionCommentRawController
	extends PhabricatorApplicationTransactionController {

	+ public function shouldAllowPublic() {
	+ return true;
	+ }
	+
	public function handleRequest(AphrontRequest $request) {
	$viewer = $this->getViewer();
	$phid = $request->getURIData('phid');
	@@ -11,19 +15,13 @@
	->withPHIDs(array($phid))
	->setViewer($viewer)
	->executeOne();
	-
	if (!$xaction) {
	return new Aphront404Response();
	}

	- if (!$xaction->getComment()) {
	- // You can't view a raw comment if there is no comment.
	- return new Aphront404Response();
	- }
	-
	- if ($xaction->getComment()->getIsRemoved()) {
	- // You can't view a raw comment if the comment is deleted.
	- return new Aphront400Response();
	+ $response = $this->newAccessResponse($xaction);
	+ if ($response) {
	+ return $response;
	}

	$obj_phid = $xaction->getObjectPHID();
	@@ -32,6 +30,8 @@
	->withPHIDs(array($obj_phid))
	->executeOne();

	+ $done_uri = $obj_handle->getURI();
	+
	$title = pht('Raw Comment');
	$body = $xaction->getComment()->getContent();
	$addendum = null;
	@@ -55,9 +55,8 @@
	}
	}
	}
	- $dialog = id(new AphrontDialogView())
	- ->setUser($viewer)
	- ->addCancelButton($obj_handle->getURI())
	+ $dialog = $this->newDialog()
	+ ->addCancelButton($done_uri)
	->setTitle($title);

	$dialog
	@@ -73,11 +72,7 @@
	$dialog->appendParagraph($addendum);
	}

	- return id(new AphrontDialogResponse())->setDialog($dialog);
	- }
	-
	- public function shouldAllowPublic() {
	- return true;
	+ return $dialog;
	}

	}
	diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionController.php
	--- a/src/applications/transactions/controller/PhabricatorApplicationTransactionController.php
	+++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionController.php
	@@ -24,4 +24,49 @@
	return $cancel_uri;
	}

	+ protected function newAccessResponse(
	+ PhabricatorApplicationTransaction $xaction) {
	+ $viewer = $this->getViewer();
	+
	+ $comment = $xaction->getComment();
	+ if (!$comment) {
	+ return new Aphront404Response();
	+ }
	+
	+ $obj_phid = $xaction->getObjectPHID();
	+ $obj_handle = id(new PhabricatorHandleQuery())
	+ ->setViewer($viewer)
	+ ->withPHIDs(array($obj_phid))
	+ ->executeOne();
	+
	+ $done_uri = $obj_handle->getURI();
	+
	+ if ($comment->getIsRemoved()) {
	+ return $this->newDialog()
	+ ->setTitle(pht('Comment Removed'))
	+ ->appendParagraph(
	+ pht('This comment has been removed.'))
	+ ->addCancelButton($done_uri);
	+ }
	+
	+ if ($comment->getIsDeleted()) {
	+ return $this->newDialog()
	+ ->setTitle(pht('Comment Deleted'))
	+ ->appendParagraph(
	+ pht('This comment has been deleted.'))
	+ ->addCancelButton($done_uri);
	+ }
	+
	+ if ($comment->getBanishment()) {
	+ return $this->newDialog()
	+ ->setTitle(pht('Author Banished'))
	+ ->appendParagraph(
	+ pht('The user who made this comment has been banished.'))
	+ ->addCancelButton($done_uri);
	+ }
	+
	+ return null;
	+ }
	+
	+
	}
	diff --git a/src/applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php b/src/applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php
	--- a/src/applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php
	+++ b/src/applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php
	@@ -9,6 +9,7 @@
	private $transactionPHIDs;
	private $isDeleted;
	private $hasTransaction;
	+ private $needBanishments;

	abstract protected function getTemplate();

	@@ -42,6 +43,11 @@
	return $this;
	}

	+ public function needBanishments($banishments) {
	+ $this->needBanishments = $banishments;
	+ return $this;
	+ }
	+
	protected function loadPage() {
	$table = $this->getTemplate();
	$conn_r = $table->establishConnection('r');
	@@ -118,6 +124,44 @@
	return $where;
	}

	+ protected function didFilterPage(array $comments) {
	+ if ($this->needBanishments) {
	+ $type_user = PhabricatorPeopleUserPHIDType::TYPECONST;
	+
	+ $author_phids = array();
	+ foreach ($comments as $comment) {
	+ $author_phid = $comment->getAuthorPHID();
	+ if (!$author_phid) {
	+ continue;
	+ }
	+
	+ if (phid_get_type($author_phid) !== $type_user) {
	+ continue;
	+ }
	+
	+ $author_phids[$author_phid] = $author_phid;
	+ }
	+
	+ if ($author_phids) {
	+ $authors = id(new PhabricatorPeopleQuery())
	+ ->setViewer($this->getViewer())
	+ ->withPHIDs($author_phids)
	+ ->withIsBanished(true)
	+ ->execute();
	+ $authors = mpull($authors, null, 'getPHID');
	+ } else {
	+ $authors = array();
	+ }
	+
	+ foreach ($comments as $comment) {
	+ $author_phid = $comment->getAuthorPHID();
	+ $comment->attachBanishment((bool)idx($authors, $author_phid));
	+ }
	+ }
	+
	+ return $comments;
	+ }
	+
	public function getQueryApplicationClass() {
	// TODO: Figure out the app via the template?
	return null;
	diff --git a/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php b/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php
	--- a/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php
	+++ b/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php
	@@ -89,6 +89,7 @@
	->setTemplate($table->getApplicationTransactionCommentObject())
	->setViewer($this->getViewer())
	->withPHIDs($comment_phids)
	+ ->needBanishments(true)
	->execute();
	$comments = mpull($comments, null, 'getPHID');
	}
	diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php b/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php
	--- a/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php
	+++ b/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php
	@@ -19,6 +19,7 @@
	protected $isDeleted = 0;

	private $oldComment = self::ATTACHABLE;
	+ private $banishment = self::ATTACHABLE;

	abstract public function getApplicationTransactionObject();

	@@ -101,6 +102,15 @@
	return ($this->oldComment !== self::ATTACHABLE);
	}

	+ public function attachBanishment($banishment) {
	+ $this->banishment = $banishment;
	+ return $this;
	+ }
	+
	+ public function getBanishment() {
	+ return $this->assertAttached($this->banishment);
	+ }
	+

	/* -( PhabricatorMarkupInterface )----------------------------------------- */

	diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php
	--- a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php
	+++ b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php
	@@ -294,7 +294,10 @@
	$comment = $xaction->getComment();

	if ($comment) {
	- if ($comment->getIsRemoved()) {
	+ if ($comment->getBanishment()) {
	+ // If the comment author has been banished, don't render anything.
	+ return null;
	+ } else if ($comment->getIsRemoved()) {
	return javelin_tag(
	'span',
	array(

File Metadata

Mime Type: text/plain
Expires: Mon, Mar 10, 5:56 PM (13 h, 50 m)
Storage Engine: amazon-s3
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: phabricator/secure/xv/sp/rczfa4ysljv4pahx
Default Alt Text: D19933.id47572.diff (32 KB)

D19933.id47572.diffNo OneTemporaryActions

D19933.id47572.diffView Options

File Metadata

Event Timeline

D19933.id47572.diff
No OneTemporary
Actions

D19933.id47572.diff
View Options