Page MenuHomePhabricator
Files F14412426

D19887.diff
No OneTemporary
Actions

D19887.diff
View Options

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
@@ -4646,6 +4646,7 @@
'PhabricatorUserTransaction' => 'applications/people/storage/PhabricatorUserTransaction.php',
'PhabricatorUserTransactionEditor' => 'applications/people/editor/PhabricatorUserTransactionEditor.php',
'PhabricatorUserTransactionType' => 'applications/people/xaction/PhabricatorUserTransactionType.php',
+ 'PhabricatorUserUsernameTransaction' => 'applications/people/xaction/PhabricatorUserUsernameTransaction.php',
'PhabricatorUsersEditField' => 'applications/transactions/editfield/PhabricatorUsersEditField.php',
'PhabricatorUsersPolicyRule' => 'applications/people/policyrule/PhabricatorUsersPolicyRule.php',
'PhabricatorUsersSearchField' => 'applications/people/searchfield/PhabricatorUsersSearchField.php',
@@ -10706,6 +10707,7 @@
'PhabricatorUserTransaction' => 'PhabricatorModularTransaction',
'PhabricatorUserTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
'PhabricatorUserTransactionType' => 'PhabricatorModularTransactionType',
+ 'PhabricatorUserUsernameTransaction' => 'PhabricatorUserTransactionType',
'PhabricatorUsersEditField' => 'PhabricatorTokenizerEditField',
'PhabricatorUsersPolicyRule' => 'PhabricatorPolicyRule',
'PhabricatorUsersSearchField' => 'PhabricatorSearchTokenizerField',
diff --git a/src/applications/people/controller/PhabricatorPeopleRenameController.php b/src/applications/people/controller/PhabricatorPeopleRenameController.php
--- a/src/applications/people/controller/PhabricatorPeopleRenameController.php
+++ b/src/applications/people/controller/PhabricatorPeopleRenameController.php
@@ -22,36 +22,29 @@
$request,
$done_uri);
- $errors = array();
-
- $v_username = $user->getUsername();
- $e_username = true;
+ $validation_exception = null;
+ $username = $user->getUsername();
if ($request->isFormPost()) {
- $v_username = $request->getStr('username');
-
- if (!strlen($v_username)) {
- $e_username = pht('Required');
- $errors[] = pht('New username is required.');
- } else if ($v_username == $user->getUsername()) {
- $e_username = pht('Invalid');
- $errors[] = pht('New username must be different from old username.');
- } else if (!PhabricatorUser::validateUsername($v_username)) {
- $e_username = pht('Invalid');
- $errors[] = PhabricatorUser::describeValidUsername();
+ $username = $request->getStr('username');
+ $xactions = array();
+
+ $xactions[] = id(new PhabricatorUserTransaction())
+ ->setTransactionType(
+ PhabricatorUserUsernameTransaction::TRANSACTIONTYPE)
+ ->setNewValue($username);
+
+ $editor = id(new PhabricatorUserTransactionEditor())
+ ->setActor($viewer)
+ ->setContentSourceFromRequest($request)
+ ->setContinueOnMissingFields(true);
+
+ try {
+ $editor->applyTransactions($user, $xactions);
+ return id(new AphrontRedirectResponse())->setURI($done_uri);
+ } catch (PhabricatorApplicationTransactionValidationException $ex) {
+ $validation_exception = $ex;
}
- if (!$errors) {
- try {
- id(new PhabricatorUserEditor())
- ->setActor($viewer)
- ->changeUsername($user, $v_username);
-
- return id(new AphrontRedirectResponse())->setURI($done_uri);
- } catch (AphrontDuplicateKeyQueryException $ex) {
- $e_username = pht('Not Unique');
- $errors[] = pht('Another user already has that username.');
- }
- }
}
$inst1 = pht(
@@ -87,18 +80,13 @@
->appendChild(
id(new AphrontFormTextControl())
->setLabel(pht('New Username'))
- ->setValue($v_username)
- ->setName('username')
- ->setError($e_username));
-
- if ($errors) {
- $errors = id(new PHUIInfoView())->setErrors($errors);
- }
+ ->setValue($username)
+ ->setName('username'));
return $this->newDialog()
->setWidth(AphrontDialogView::WIDTH_FORM)
->setTitle(pht('Change Username'))
- ->appendChild($errors)
+ ->setValidationException($validation_exception)
->appendParagraph($inst1)
->appendParagraph($inst2)
->appendParagraph($inst3)
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,53 +129,6 @@
}
- /**
- * @task edit
- */
- public function changeUsername(PhabricatorUser $user, $username) {
- $actor = $this->requireActor();
-
- if (!$user->getID()) {
- throw new Exception(pht('User has not been created yet!'));
- }
-
- if (!PhabricatorUser::validateUsername($username)) {
- $valid = PhabricatorUser::describeValidUsername();
- throw new Exception(pht('Username is invalid! %s', $valid));
- }
-
- $old_username = $user->getUsername();
-
- $user->openTransaction();
- $user->reload();
- $user->setUsername($username);
-
- try {
- $user->save();
- } catch (AphrontDuplicateKeyQueryException $ex) {
- $user->setUsername($old_username);
- $user->killTransaction();
- throw $ex;
- }
-
- $log = PhabricatorUserLog::initializeNewLog(
- $actor,
- $user->getPHID(),
- PhabricatorUserLog::ACTION_CHANGE_USERNAME);
- $log->setOldValue($old_username);
- $log->setNewValue($username);
- $log->save();
-
- $user->saveTransaction();
-
- // The SSH key cache currently includes usernames, so dirty it. See T12554
- // for discussion.
- PhabricatorAuthSSHKeyQuery::deleteSSHKeyCache();
-
- $user->sendUsernameChangeEmail($actor, $old_username);
- }
-
-
/* -( Editing Roles )------------------------------------------------------ */
diff --git a/src/applications/people/xaction/PhabricatorUserUsernameTransaction.php b/src/applications/people/xaction/PhabricatorUserUsernameTransaction.php
new file mode 100644
--- /dev/null
+++ b/src/applications/people/xaction/PhabricatorUserUsernameTransaction.php
@@ -0,0 +1,92 @@
+<?php
+
+final class PhabricatorUserUsernameTransaction
+ extends PhabricatorUserTransactionType {
+
+ const TRANSACTIONTYPE = 'user.rename';
+
+ public function generateOldValue($object) {
+ return $object->getUsername();
+ }
+
+ public function generateNewValue($object, $value) {
+ return $value;
+ }
+
+ public function applyInternalEffects($object, $value) {
+ $object->setUsername($value);
+ }
+
+ public function applyExternalEffects($object, $value) {
+ $user = $object;
+
+ $this->newUserLog(PhabricatorUserLog::ACTION_CHANGE_USERNAME)
+ ->setOldValue($this->getOldValue())
+ ->setNewValue($value)
+ ->save();
+
+ // The SSH key cache currently includes usernames, so dirty it. See T12554
+ // for discussion.
+ PhabricatorAuthSSHKeyQuery::deleteSSHKeyCache();
+
+ $user->sendUsernameChangeEmail($this->getActor(), $this->getOldValue());
+ }
+
+ public function getTitle() {
+ return pht(
+ '%s renamed this user from %s to %s.',
+ $this->renderAuthor(),
+ $this->renderOldValue(),
+ $this->renderNewValue());
+ }
+
+ public function validateTransactions($object, array $xactions) {
+ $actor = $this->getActor();
+ $errors = array();
+
+ foreach ($xactions as $xaction) {
+ $new = $xaction->getNewValue();
+ $old = $xaction->getOldValue();
+
+ if ($old === $new) {
+ continue;
+ }
+
+ if (!$actor->getIsAdmin()) {
+ $errors[] = $this->newInvalidError(
+ pht('You must be an administrator to rename users.'));
+ }
+
+ if (!strlen($new)) {
+ $errors[] = $this->newRequiredError(
+ pht('New username is required.'), $xaction);
+ } else if (!PhabricatorUser::validateUsername($new)) {
+ $errors[] = $this->newInvalidError(
+ PhabricatorUser::describeValidUsername(), $xaction);
+ }
+
+ $user = id(new PhabricatorPeopleQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->withUsernames(array($new))
+ ->executeOne();
+
+ if ($user) {
+ $errors[] = $this->newInvalidError(
+ pht('Another user already has that username.'), $xaction);
+ }
+
+ }
+
+ return $errors;
+ }
+
+ public function getRequiredCapabilities(
+ $object,
+ PhabricatorApplicationTransaction $xaction) {
+
+ // Unlike normal user edits, renames require admin permissions, which
+ // is enforced by validateTransactions().
+
+ return null;
+ }
+}

File Metadata

Mime Type
text/plain
Expires
Wed, Dec 25, 1:03 PM (11 h, 26 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6926351
Default Alt Text
D19887.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility