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 @@ -4597,6 +4597,7 @@ 'PhabricatorUnknownContentSource' => 'infrastructure/contentsource/PhabricatorUnknownContentSource.php', 'PhabricatorUnsubscribedFromObjectEdgeType' => 'applications/transactions/edges/PhabricatorUnsubscribedFromObjectEdgeType.php', 'PhabricatorUser' => 'applications/people/storage/PhabricatorUser.php', + 'PhabricatorUserApproveTransaction' => 'applications/people/xaction/PhabricatorUserApproveTransaction.php', 'PhabricatorUserBadgesCacheType' => 'applications/people/cache/PhabricatorUserBadgesCacheType.php', 'PhabricatorUserBlurbField' => 'applications/people/customfield/PhabricatorUserBlurbField.php', 'PhabricatorUserCache' => 'applications/people/storage/PhabricatorUserCache.php', @@ -10645,6 +10646,7 @@ 'PhabricatorConduitResultInterface', 'PhabricatorAuthPasswordHashInterface', ), + 'PhabricatorUserApproveTransaction' => 'PhabricatorUserTransactionType', 'PhabricatorUserBadgesCacheType' => 'PhabricatorUserCacheType', 'PhabricatorUserBlurbField' => 'PhabricatorUserCustomField', 'PhabricatorUserCache' => 'PhabricatorUserDAO', diff --git a/src/applications/people/controller/PhabricatorPeopleApproveController.php b/src/applications/people/controller/PhabricatorPeopleApproveController.php --- a/src/applications/people/controller/PhabricatorPeopleApproveController.php +++ b/src/applications/people/controller/PhabricatorPeopleApproveController.php @@ -24,30 +24,18 @@ } if ($request->isFormPost()) { - id(new PhabricatorUserEditor()) - ->setActor($viewer) - ->approveUser($user, true); - - $title = pht( - 'Phabricator Account "%s" Approved', - $user->getUsername()); + $xactions = array(); - $body = sprintf( - "%s\n\n %s\n\n", - pht( - 'Your Phabricator account (%s) has been approved by %s. You can '. - 'login here:', - $user->getUsername(), - $viewer->getUsername()), - PhabricatorEnv::getProductionURI('/')); + $xactions[] = id(new PhabricatorUserTransaction()) + ->setTransactionType(PhabricatorUserApproveTransaction::TRANSACTIONTYPE) + ->setNewValue(true); - $mail = id(new PhabricatorMetaMTAMail()) - ->addTos(array($user->getPHID())) - ->addCCs(array($viewer->getPHID())) - ->setSubject('[Phabricator] '.$title) - ->setForceDelivery(true) - ->setBody($body) - ->saveAndSend(); + id(new PhabricatorUserTransactionEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnMissingFields(true) + ->setContinueOnNoEffect(true) + ->applyTransactions($user, $xactions); return id(new AphrontRedirectResponse())->setURI($done_uri); } diff --git a/src/applications/people/editor/PhabricatorUserEditEngine.php b/src/applications/people/editor/PhabricatorUserEditEngine.php --- a/src/applications/people/editor/PhabricatorUserEditEngine.php +++ b/src/applications/people/editor/PhabricatorUserEditEngine.php @@ -75,6 +75,16 @@ ->setConduitDescription(pht('Disable or enable the user.')) ->setConduitTypeDescription(pht('True to disable the user.')) ->setValue($object->getIsDisabled()), + id(new PhabricatorBoolEditField()) + ->setKey('approved') + ->setOptions(pht('Approved'), pht('Unapproved')) + ->setLabel(pht('Approved')) + ->setDescription(pht('Approve the user.')) + ->setTransactionType(PhabricatorUserApproveTransaction::TRANSACTIONTYPE) + ->setIsFormField(false) + ->setConduitDescription(pht('Approve or reject the user.')) + ->setConduitTypeDescription(pht('True to approve the user.')) + ->setValue($object->getIsApproved()), ); } diff --git a/src/applications/people/editor/PhabricatorUserEditor.php b/src/applications/people/editor/PhabricatorUserEditor.php --- a/src/applications/people/editor/PhabricatorUserEditor.php +++ b/src/applications/people/editor/PhabricatorUserEditor.php @@ -293,45 +293,6 @@ return $this; } - /** - * @task role - */ - public function approveUser(PhabricatorUser $user, $approve) { - $actor = $this->requireActor(); - - if (!$user->getID()) { - throw new Exception(pht('User has not been created yet!')); - } - - $user->openTransaction(); - $user->beginWriteLocking(); - - $user->reload(); - if ($user->getIsApproved() == $approve) { - $user->endWriteLocking(); - $user->killTransaction(); - return $this; - } - - $log = PhabricatorUserLog::initializeNewLog( - $actor, - $user->getPHID(), - PhabricatorUserLog::ACTION_APPROVE); - $log->setOldValue($user->getIsApproved()); - $log->setNewValue($approve); - - $user->setIsApproved($approve); - $user->save(); - - $log->save(); - - $user->endWriteLocking(); - $user->saveTransaction(); - - return $this; - } - - /* -( Adding, Removing and Changing Email )-------------------------------- */ diff --git a/src/applications/people/xaction/PhabricatorUserApproveTransaction.php b/src/applications/people/xaction/PhabricatorUserApproveTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/people/xaction/PhabricatorUserApproveTransaction.php @@ -0,0 +1,96 @@ +getIsApproved(); + } + + public function generateNewValue($object, $value) { + return (bool)$value; + } + + public function applyInternalEffects($object, $value) { + $object->setIsApproved((int)$value); + } + + public function applyExternalEffects($object, $value) { + $user = $object; + $this->newUserLog(PhabricatorUserLog::ACTION_APPROVE) + ->setOldValue((bool)$user->getIsApproved()) + ->setNewValue((bool)$value) + ->save(); + + $actor = $this->getActor(); + $title = pht( + 'Phabricator Account "%s" Approved', + $user->getUsername()); + + $body = sprintf( + "%s\n\n %s\n\n", + pht( + 'Your Phabricator account (%s) has been approved by %s. You can '. + 'login here:', + $user->getUsername(), + $actor->getUsername()), + PhabricatorEnv::getProductionURI('/')); + + $mail = id(new PhabricatorMetaMTAMail()) + ->addTos(array($user->getPHID())) + ->addCCs(array($actor->getPHID())) + ->setSubject('[Phabricator] '.$title) + ->setForceDelivery(true) + ->setBody($body) + ->saveAndSend(); + } + + public function getTitle() { + $new = $this->getNewValue(); + if ($new) { + return pht( + '%s approved this user.', + $this->renderAuthor()); + } else { + return pht( + '%s rejected this user.', + $this->renderAuthor()); + } + } + + public function shouldHideForFeed() { + return true; + } + + public function validateTransactions($object, array $xactions) { + $actor = $this->getActor(); + $errors = array(); + + foreach ($xactions as $xaction) { + $is_approved = (bool)$object->getIsApproved(); + + if ((bool)$xaction->getNewValue() === $is_approved) { + continue; + } + + if (!$actor->getIsAdmin()) { + $errors[] = $this->newInvalidError( + pht('You must be an administrator to approve users.')); + } + } + + return $errors; + } + + public function getRequiredCapabilities( + $object, + PhabricatorApplicationTransaction $xaction) { + + // Unlike normal user edits, approvals require admin permissions, which + // is enforced by validateTransactions(). + + 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 @@ -15,7 +15,9 @@ public function applyInternalEffects($object, $value) { $object->setIsDisabled((int)$value); + } + public function applyExternalEffects($object, $value) { $this->newUserLog(PhabricatorUserLog::ACTION_DISABLE) ->setOldValue((bool)$object->getIsDisabled()) ->setNewValue((bool)$value)