Page MenuHomePhabricator
Files F15532074

D19933.diff
No OneTemporary
Actions

D19933.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/PhabricatorPeopleProfileBadgesController.php b/src/applications/people/controller/PhabricatorPeopleProfileBadgesController.php
--- a/src/applications/people/controller/PhabricatorPeopleProfileBadgesController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfileBadgesController.php
@@ -16,6 +16,11 @@
return new Aphront404Response();
}
+ $response = $this->newProfileAccessResponse($user);
+ if ($response) {
+ return $response;
+ }
+
$class = 'PhabricatorBadgesApplication';
if (!PhabricatorApplication::isClassInstalledForViewer($class, $viewer)) {
return new Aphront404Response();
diff --git a/src/applications/people/controller/PhabricatorPeopleProfileCommitsController.php b/src/applications/people/controller/PhabricatorPeopleProfileCommitsController.php
--- a/src/applications/people/controller/PhabricatorPeopleProfileCommitsController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfileCommitsController.php
@@ -18,6 +18,11 @@
return new Aphront404Response();
}
+ $response = $this->newProfileAccessResponse($user);
+ if ($response) {
+ return $response;
+ }
+
$class = 'PhabricatorDiffusionApplication';
if (!PhabricatorApplication::isClassInstalledForViewer($class, $viewer)) {
return new Aphront404Response();
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/controller/PhabricatorPeopleProfileRevisionsController.php b/src/applications/people/controller/PhabricatorPeopleProfileRevisionsController.php
--- a/src/applications/people/controller/PhabricatorPeopleProfileRevisionsController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfileRevisionsController.php
@@ -18,6 +18,11 @@
return new Aphront404Response();
}
+ $response = $this->newProfileAccessResponse($user);
+ if ($response) {
+ return $response;
+ }
+
$class = 'PhabricatorDifferentialApplication';
if (!PhabricatorApplication::isClassInstalledForViewer($class, $viewer)) {
return new Aphront404Response();
diff --git a/src/applications/people/controller/PhabricatorPeopleProfileTasksController.php b/src/applications/people/controller/PhabricatorPeopleProfileTasksController.php
--- a/src/applications/people/controller/PhabricatorPeopleProfileTasksController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfileTasksController.php
@@ -18,6 +18,11 @@
return new Aphront404Response();
}
+ $response = $this->newProfileAccessResponse($user);
+ if ($response) {
+ return $response;
+ }
+
$class = 'PhabricatorManiphestApplication';
if (!PhabricatorApplication::isClassInstalledForViewer($class, $viewer)) {
return new Aphront404Response();
diff --git a/src/applications/people/controller/PhabricatorPeopleProfileViewController.php b/src/applications/people/controller/PhabricatorPeopleProfileViewController.php
--- a/src/applications/people/controller/PhabricatorPeopleProfileViewController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfileViewController.php
@@ -21,6 +21,11 @@
return new Aphront404Response();
}
+ $response = $this->newProfileAccessResponse($user);
+ if ($response) {
+ return $response;
+ }
+
$this->setUser($user);
$header = $this->buildProfileHeader();
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/conduit/TransactionSearchConduitAPIMethod.php b/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php
--- a/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php
+++ b/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php
@@ -104,6 +104,7 @@
->setViewer($viewer)
->setTemplate($template)
->withTransactionPHIDs(mpull($xactions, 'getPHID'))
+ ->needBanishments(true)
->execute();
$comment_map = msort($comment_map, 'getCommentVersion');
@@ -160,9 +161,16 @@
$comment_data = array();
if ($comments) {
$removed = head($comments)->getIsDeleted();
+ $banishment = head($comments)->getBanishment();
foreach ($comments as $comment) {
- if ($removed) {
+ if ($banishment) {
+ // For consistency with the web UI, don't show any of the text if
+ // the author has been banished.
+ $content = array(
+ 'raw' => '',
+ );
+ } else if ($removed) {
// If the most recent version of the comment has been removed,
// don't show the history. This is for consistency with the web
// UI, which also prevents users from retrieving the content of
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
Thu, Apr 24, 2:24 PM (20 h, 30 m ago)
Storage Engine
amazon-s3
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
phabricator/secure/xc/u5/kqsjv6tktkfvantk
Default Alt Text
D19933.diff (37 KB)

Event Timeline

Phabricator · Built by Phacility