Move user renames to modular transactions

Summary: Cleaning up more super-old code from `PhabricatorUserEditor`. Also fix user logging in approve transactions. I'm not sure how it worked at all previously.

Test Plan: Created new users, renamed them, checked DB for sanity. Entered invalid names, duplicate names, and empty names, got appropriate error messages.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D19887

src/__phutil_library_map__.php

@@ -4646,6 +4646,7 @@ phutil_register_library_map(array(
     '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 @@ phutil_register_library_map(array(
     'PhabricatorUserTransaction' => 'PhabricatorModularTransaction',
     'PhabricatorUserTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
     'PhabricatorUserTransactionType' => 'PhabricatorModularTransactionType',
+    'PhabricatorUserUsernameTransaction' => 'PhabricatorUserTransactionType',
     'PhabricatorUsersEditField' => 'PhabricatorTokenizerEditField',
     'PhabricatorUsersPolicyRule' => 'PhabricatorPolicyRule',
     'PhabricatorUsersSearchField' => 'PhabricatorSearchTokenizerField',

src/applications/people/controller/PhabricatorPeopleRenameController.php

@@ -22,36 +22,29 @@ final class PhabricatorPeopleRenameController
       $request,
       $done_uri);
 
-    $errors = array();
+    $validation_exception = null;
-
+    $username = $user->getUsername();
-    $v_username = $user->getUsername();
-    $e_username = true;
     if ($request->isFormPost()) {
-      $v_username = $request->getStr('username');
+      $username = $request->getStr('username');
+      $xactions = array();
 
-      if (!strlen($v_username)) {
+      $xactions[] = id(new PhabricatorUserTransaction())
-        $e_username = pht('Required');
+        ->setTransactionType(
-        $errors[] = pht('New username is required.');
+          PhabricatorUserUsernameTransaction::TRANSACTIONTYPE)
-      } else if ($v_username == $user->getUsername()) {
+        ->setNewValue($username);
-        $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();
-      }
 
-      if (!$errors) {
+      $editor = id(new PhabricatorUserTransactionEditor())
-        try {
-          id(new PhabricatorUserEditor())
         ->setActor($viewer)
-            ->changeUsername($user, $v_username);
+        ->setContentSourceFromRequest($request)
+        ->setContinueOnMissingFields(true);
 
+      try {
+        $editor->applyTransactions($user, $xactions);
         return id(new AphrontRedirectResponse())->setURI($done_uri);
-        } catch (AphrontDuplicateKeyQueryException $ex) {
+      } catch (PhabricatorApplicationTransactionValidationException $ex) {
-          $e_username = pht('Not Unique');
+        $validation_exception = $ex;
-          $errors[] = pht('Another user already has that username.');
-        }
       }
+
     }
 
     $inst1 = pht(
@@ -87,18 +80,13 @@ final class PhabricatorPeopleRenameController
       ->appendChild(
         id(new AphrontFormTextControl())
           ->setLabel(pht('New Username'))
-          ->setValue($v_username)
+          ->setValue($username)
-          ->setName('username')
+          ->setName('username'));
-          ->setError($e_username));
-
-    if ($errors) {
-      $errors = id(new PHUIInfoView())->setErrors($errors);
-    }
 
     return $this->newDialog()
       ->setWidth(AphrontDialogView::WIDTH_FORM)
       ->setTitle(pht('Change Username'))
-      ->appendChild($errors)
+      ->setValidationException($validation_exception)
       ->appendParagraph($inst1)
       ->appendParagraph($inst2)
       ->appendParagraph($inst3)

src/applications/people/editor/PhabricatorUserEditor.php

@@ -129,53 +129,6 @@ final class PhabricatorUserEditor extends PhabricatorEditor {
   }
 
 
-  /**
-   * @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  )------------------------------------------------------ */
 
 

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;
+  }
+}