From 8cf56913d8ef1a5c8cdbfbe94057177b00d83f34 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 13 Aug 2018 15:32:30 -0700 Subject: [PATCH] Deprecate "user.enable" and "user.disable" API methods, redefine them in terms of "user.edit" Summary: Depends on D19604. Ref T13189. See PHI642. Deprecates these in favor of "user.edit", redefines them in terms of it, and removes the old `disableUser()` method. I kept the "is admin" permissions check for consistency, since these methods have always said "(admin only)". This check may not be the most tailored check soon, but we can just keep executing it in addition to the real check. For now, this change stops this method from actually disabling non-bot users (since it implicitly adds a CAN_EDIT requirement, and even administrators don't have that). An upcoming change will fix that. Test Plan: Enabled and disabled a (bot) user via these methods. Checked API UI, saw them marked as "disabled". Reviewers: amckinley Maniphest Tasks: T13189 Differential Revision: https://secure.phabricator.com/D19605 --- .../conduit/UserDisableConduitAPIMethod.php | 28 +++++++++++-- .../conduit/UserEnableConduitAPIMethod.php | 28 +++++++++++-- .../people/editor/PhabricatorUserEditor.php | 39 ------------------- 3 files changed, 48 insertions(+), 47 deletions(-) diff --git a/src/applications/people/conduit/UserDisableConduitAPIMethod.php b/src/applications/people/conduit/UserDisableConduitAPIMethod.php index 3512d3f742..a6f2e2c4f1 100644 --- a/src/applications/people/conduit/UserDisableConduitAPIMethod.php +++ b/src/applications/people/conduit/UserDisableConduitAPIMethod.php @@ -10,6 +10,14 @@ final class UserDisableConduitAPIMethod extends UserConduitAPIMethod { return pht('Permanently disable specified users (admin only).'); } + public function getMethodStatus() { + return self::METHOD_STATUS_DEPRECATED; + } + + public function getMethodStatusDescription() { + return pht('Obsoleted by method "user.edit".'); + } + protected function defineParamTypes() { return array( 'phids' => 'required list', @@ -43,11 +51,23 @@ final class UserDisableConduitAPIMethod extends UserConduitAPIMethod { throw new ConduitException('ERR-BAD-PHID'); } - foreach ($users as $user) { - id(new PhabricatorUserEditor()) - ->setActor($actor) - ->disableUser($user, true); + foreach ($phids as $phid) { + $params = array( + 'transactions' => array( + array( + 'type' => 'disabled', + 'value' => true, + ), + ), + 'objectIdentifier' => $phid, + ); + + id(new ConduitCall('user.edit', $params)) + ->setUser($actor) + ->execute(); } + + return null; } } diff --git a/src/applications/people/conduit/UserEnableConduitAPIMethod.php b/src/applications/people/conduit/UserEnableConduitAPIMethod.php index f7cb117df6..3e4e09b778 100644 --- a/src/applications/people/conduit/UserEnableConduitAPIMethod.php +++ b/src/applications/people/conduit/UserEnableConduitAPIMethod.php @@ -10,6 +10,14 @@ final class UserEnableConduitAPIMethod extends UserConduitAPIMethod { return pht('Re-enable specified users (admin only).'); } + public function getMethodStatus() { + return self::METHOD_STATUS_DEPRECATED; + } + + public function getMethodStatusDescription() { + return pht('Obsoleted by method "user.edit".'); + } + protected function defineParamTypes() { return array( 'phids' => 'required list', @@ -43,11 +51,23 @@ final class UserEnableConduitAPIMethod extends UserConduitAPIMethod { throw new ConduitException('ERR-BAD-PHID'); } - foreach ($users as $user) { - id(new PhabricatorUserEditor()) - ->setActor($actor) - ->disableUser($user, false); + foreach ($phids as $phid) { + $params = array( + 'transactions' => array( + array( + 'type' => 'disabled', + 'value' => false, + ), + ), + 'objectIdentifier' => $phid, + ); + + id(new ConduitCall('user.edit', $params)) + ->setUser($actor) + ->execute(); } + + return null; } } diff --git a/src/applications/people/editor/PhabricatorUserEditor.php b/src/applications/people/editor/PhabricatorUserEditor.php index 786ead79c3..58c2ba1ff1 100644 --- a/src/applications/people/editor/PhabricatorUserEditor.php +++ b/src/applications/people/editor/PhabricatorUserEditor.php @@ -293,45 +293,6 @@ final class PhabricatorUserEditor extends PhabricatorEditor { return $this; } - /** - * @task role - */ - public function disableUser(PhabricatorUser $user, $disable) { - $actor = $this->requireActor(); - - if (!$user->getID()) { - throw new Exception(pht('User has not been created yet!')); - } - - $user->openTransaction(); - $user->beginWriteLocking(); - - $user->reload(); - if ($user->getIsDisabled() == $disable) { - $user->endWriteLocking(); - $user->killTransaction(); - return $this; - } - - $log = PhabricatorUserLog::initializeNewLog( - $actor, - $user->getPHID(), - PhabricatorUserLog::ACTION_DISABLE); - $log->setOldValue($user->getIsDisabled()); - $log->setNewValue($disable); - - $user->setIsDisabled((int)$disable); - $user->save(); - - $log->save(); - - $user->endWriteLocking(); - $user->saveTransaction(); - - return $this; - } - - /** * @task role */