diff --git a/scripts/user/account_admin.php b/scripts/user/account_admin.php --- a/scripts/user/account_admin.php +++ b/scripts/user/account_admin.php @@ -200,14 +200,17 @@ $editor->updateUser($user, $verify_email); } - $editor->makeSystemAgentUser($user, $set_system_agent); - $xactions = array(); $xactions[] = id(new PhabricatorUserTransaction()) ->setTransactionType( PhabricatorUserEmpowerTransaction::TRANSACTIONTYPE) ->setNewValue($set_admin); + $xactions[] = id(new PhabricatorUserTransaction()) + ->setTransactionType( + PhabricatorUserSystemAgentTransaction::TRANSACTIONTYPE) + ->setNewValue($set_system_agent); + $actor = PhabricatorUser::getOmnipotentUser(); $content_source = PhabricatorContentSource::newForSource( PhabricatorConsoleContentSource::SOURCECONST); 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 @@ -4661,6 +4661,7 @@ 'PhabricatorUserIconField' => 'applications/people/customfield/PhabricatorUserIconField.php', 'PhabricatorUserLog' => 'applications/people/storage/PhabricatorUserLog.php', 'PhabricatorUserLogView' => 'applications/people/view/PhabricatorUserLogView.php', + 'PhabricatorUserMailingListTransaction' => 'applications/people/xaction/PhabricatorUserMailingListTransaction.php', 'PhabricatorUserMessageCountCacheType' => 'applications/people/cache/PhabricatorUserMessageCountCacheType.php', 'PhabricatorUserNotificationCountCacheType' => 'applications/people/cache/PhabricatorUserNotificationCountCacheType.php', 'PhabricatorUserNotifyTransaction' => 'applications/people/xaction/PhabricatorUserNotifyTransaction.php', @@ -4680,6 +4681,7 @@ 'PhabricatorUserSchemaSpec' => 'applications/people/storage/PhabricatorUserSchemaSpec.php', 'PhabricatorUserSinceField' => 'applications/people/customfield/PhabricatorUserSinceField.php', 'PhabricatorUserStatusField' => 'applications/people/customfield/PhabricatorUserStatusField.php', + 'PhabricatorUserSystemAgentTransaction' => 'applications/people/xaction/PhabricatorUserSystemAgentTransaction.php', 'PhabricatorUserTestCase' => 'applications/people/storage/__tests__/PhabricatorUserTestCase.php', 'PhabricatorUserTitleField' => 'applications/people/customfield/PhabricatorUserTitleField.php', 'PhabricatorUserTransaction' => 'applications/people/storage/PhabricatorUserTransaction.php', @@ -10773,6 +10775,7 @@ 'PhabricatorPolicyInterface', ), 'PhabricatorUserLogView' => 'AphrontView', + 'PhabricatorUserMailingListTransaction' => 'PhabricatorUserTransactionType', 'PhabricatorUserMessageCountCacheType' => 'PhabricatorUserCacheType', 'PhabricatorUserNotificationCountCacheType' => 'PhabricatorUserCacheType', 'PhabricatorUserNotifyTransaction' => 'PhabricatorUserTransactionType', @@ -10797,6 +10800,7 @@ 'PhabricatorUserSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'PhabricatorUserSinceField' => 'PhabricatorUserCustomField', 'PhabricatorUserStatusField' => 'PhabricatorUserCustomField', + 'PhabricatorUserSystemAgentTransaction' => 'PhabricatorUserTransactionType', 'PhabricatorUserTestCase' => 'PhabricatorTestCase', 'PhabricatorUserTitleField' => 'PhabricatorUserCustomField', 'PhabricatorUserTransaction' => 'PhabricatorModularTransaction', diff --git a/src/applications/people/controller/PhabricatorPeopleNewController.php b/src/applications/people/controller/PhabricatorPeopleNewController.php --- a/src/applications/people/controller/PhabricatorPeopleNewController.php +++ b/src/applications/people/controller/PhabricatorPeopleNewController.php @@ -35,6 +35,7 @@ $e_username = true; $e_realname = $require_real_name ? true : null; $e_email = true; + $validation_exception = null; $errors = array(); $welcome_checked = true; @@ -78,7 +79,6 @@ if (!$errors) { try { - $email = id(new PhabricatorUserEmail()) ->setAddress($new_email) ->setIsVerified(0); @@ -95,17 +95,6 @@ ->setActor($admin) ->createNewUser($user, $email); - if ($is_bot) { - id(new PhabricatorUserEditor()) - ->setActor($admin) - ->makeSystemAgentUser($user, true); - } - - if ($is_list) { - id(new PhabricatorUserEditor()) - ->setActor($admin) - ->makeMailingListUser($user, true); - } if ($welcome_checked) { $welcome_engine = id(new PhabricatorPeopleWelcomeMailEngine()) @@ -116,9 +105,30 @@ } } - $response = id(new AphrontRedirectResponse()) - ->setURI('/p/'.$user->getUsername().'/'); - return $response; + $xactions = array(); + $xactions[] = id(new PhabricatorUserTransaction()) + ->setTransactionType( + PhabricatorUserSystemAgentTransaction::TRANSACTIONTYPE) + ->setNewValue($is_bot); + + $xactions[] = id(new PhabricatorUserTransaction()) + ->setTransactionType( + PhabricatorUserMailingListTransaction::TRANSACTIONTYPE) + ->setNewValue($is_list); + + $editor = id(new PhabricatorUserTransactionEditor()) + ->setActor($admin) + ->setContentSourceFromRequest($request) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true); + + try { + $editor->applyTransactions($user, $xactions); + return id(new AphrontRedirectResponse()) + ->setURI('/p/'.$user->getUsername().'/'); + } catch (PhabricatorApplicationTransactionValidationException $ex) { + $validation_exception = $ex; + } } catch (AphrontDuplicateKeyQueryException $ex) { $errors[] = pht('Username and email must be unique.'); @@ -220,7 +230,7 @@ $box = id(new PHUIObjectBoxView()) ->setHeaderText($title) - ->setFormErrors($errors) + ->setValidationException($validation_exception) ->setBackground(PHUIObjectBoxView::WHITE_CONFIG) ->setForm($form); 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 @@ -129,84 +129,6 @@ } -/* -( Editing Roles )------------------------------------------------------ */ - - /** - * @task role - */ - public function makeSystemAgentUser(PhabricatorUser $user, $system_agent) { - $actor = $this->requireActor(); - - if (!$user->getID()) { - throw new Exception(pht('User has not been created yet!')); - } - - $user->openTransaction(); - $user->beginWriteLocking(); - - $user->reload(); - if ($user->getIsSystemAgent() == $system_agent) { - $user->endWriteLocking(); - $user->killTransaction(); - return $this; - } - - $log = PhabricatorUserLog::initializeNewLog( - $actor, - $user->getPHID(), - PhabricatorUserLog::ACTION_SYSTEM_AGENT); - $log->setOldValue($user->getIsSystemAgent()); - $log->setNewValue($system_agent); - - $user->setIsSystemAgent((int)$system_agent); - $user->save(); - - $log->save(); - - $user->endWriteLocking(); - $user->saveTransaction(); - - return $this; - } - - /** - * @task role - */ - public function makeMailingListUser(PhabricatorUser $user, $mailing_list) { - $actor = $this->requireActor(); - - if (!$user->getID()) { - throw new Exception(pht('User has not been created yet!')); - } - - $user->openTransaction(); - $user->beginWriteLocking(); - - $user->reload(); - if ($user->getIsMailingList() == $mailing_list) { - $user->endWriteLocking(); - $user->killTransaction(); - return $this; - } - - $log = PhabricatorUserLog::initializeNewLog( - $actor, - $user->getPHID(), - PhabricatorUserLog::ACTION_MAILING_LIST); - $log->setOldValue($user->getIsMailingList()); - $log->setNewValue($mailing_list); - - $user->setIsMailingList((int)$mailing_list); - $user->save(); - - $log->save(); - - $user->endWriteLocking(); - $user->saveTransaction(); - - return $this; - } - /* -( Adding, Removing and Changing Email )-------------------------------- */ diff --git a/src/applications/people/xaction/PhabricatorUserMailingListTransaction.php b/src/applications/people/xaction/PhabricatorUserMailingListTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/people/xaction/PhabricatorUserMailingListTransaction.php @@ -0,0 +1,98 @@ +getIsMailingList(); + } + + public function generateNewValue($object, $value) { + return (bool)$value; + } + + public function applyInternalEffects($object, $value) { + $object->setIsMailingList((int)$value); + } + + public function applyExternalEffects($object, $value) { + $user = $object; + + $this->newUserLog(PhabricatorUserLog::ACTION_MAILING_LIST) + ->setOldValue($this->getOldValue()) + ->setNewValue($value) + ->save(); + } + + public function validateTransactions($object, array $xactions) { + $user = $object; + $actor = $this->getActor(); + + $errors = array(); + + if ($object->getIsMailingList()) { + $errors[] = $this->newInvalidError( + pht('Users cannot be both bots and mailing lists.')); + } + + foreach ($xactions as $xaction) { + $old = $xaction->getOldValue(); + $new = $xaction->getNewValue(); + + if ($old === $new) { + continue; + } + + $is_admin = $actor->getIsAdmin(); + $is_omnipotent = $actor->isOmnipotent(); + + if (!$is_admin && !$is_omnipotent) { + $errors[] = $this->newInvalidError( + pht('You must be an administrator to turn users into mailing lists.'), + $xaction); + } + } + + return $errors; + } + + public function getTitle() { + $new = $this->getNewValue(); + if ($new) { + return pht( + '%s transformed this user into a mailing list.', + $this->renderAuthor()); + } else { + return pht( + '%s transformed this user from a mailing list into a regular user.', + $this->renderAuthor()); + } + } + + public function getTitleForFeed() { + $new = $this->getNewValue(); + if ($new) { + return pht( + '%s transformed user %s into a mailing list.', + $this->renderAuthor(), + $this->renderObject()); + } else { + return pht( + '%s transformed %s from a mailing list into a user.', + $this->renderAuthor(), + $this->renderObject()); + } + } + + public function getRequiredCapabilities( + $object, + PhabricatorApplicationTransaction $xaction) { + + // Unlike normal user edits, making users into mailing lists requires admin + // permissions, which is enforced by validateTransactions(). + + return null; + } +} diff --git a/src/applications/people/xaction/PhabricatorUserSystemAgentTransaction.php b/src/applications/people/xaction/PhabricatorUserSystemAgentTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/people/xaction/PhabricatorUserSystemAgentTransaction.php @@ -0,0 +1,98 @@ +getIsSystemAgent(); + } + + public function generateNewValue($object, $value) { + return (bool)$value; + } + + public function applyInternalEffects($object, $value) { + $object->setIsSystemAgent((int)$value); + } + + public function applyExternalEffects($object, $value) { + $user = $object; + + $this->newUserLog(PhabricatorUserLog::ACTION_SYSTEM_AGENT) + ->setOldValue($this->getOldValue()) + ->setNewValue($value) + ->save(); + } + + public function validateTransactions($object, array $xactions) { + $user = $object; + $actor = $this->getActor(); + + $errors = array(); + + if ($object->getIsMailingList()) { + $errors[] = $this->newInvalidError( + pht('Users cannot be both bots and mailing lists.')); + } + + foreach ($xactions as $xaction) { + $old = $xaction->getOldValue(); + $new = $xaction->getNewValue(); + + if ($old === $new) { + continue; + } + + $is_admin = $actor->getIsAdmin(); + $is_omnipotent = $actor->isOmnipotent(); + + if (!$is_admin && !$is_omnipotent) { + $errors[] = $this->newInvalidError( + pht('You must be an administrator to turn users into bots.'), + $xaction); + } + } + + return $errors; + } + + public function getTitle() { + $new = $this->getNewValue(); + if ($new) { + return pht( + '%s transformed this user into a bot.', + $this->renderAuthor()); + } else { + return pht( + '%s transformed this user from a bot into a regular user.', + $this->renderAuthor()); + } + } + + public function getTitleForFeed() { + $new = $this->getNewValue(); + if ($new) { + return pht( + '%s transformed user %s into a bot.', + $this->renderAuthor(), + $this->renderObject()); + } else { + return pht( + '%s transformed %s from a bot into a regular user.', + $this->renderAuthor(), + $this->renderObject()); + } + } + + public function getRequiredCapabilities( + $object, + PhabricatorApplicationTransaction $xaction) { + + // Unlike normal user edits, making users into system agents requires admin + // permissions, which is enforced by validateTransactions(). + + return null; + } +}