Move user approval to modular transactions

Summary: See https://discourse.phabricator-community.org/t/how-to-approve-user-via-conduit-api/2189. This particular use case doesn't seem very compelling, but moving this logic out of `PhabricatorUserEditor` is a win anyway.

Test Plan: Registered a new user, approved/unapproved them conduit, approved from the UI.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

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

src/__phutil_library_map__.php

@@ -4597,6 +4597,7 @@ phutil_register_library_map(array(
     '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 @@ phutil_register_library_map(array(
       'PhabricatorConduitResultInterface',
       'PhabricatorAuthPasswordHashInterface',
     ),
+    'PhabricatorUserApproveTransaction' => 'PhabricatorUserTransactionType',
     'PhabricatorUserBadgesCacheType' => 'PhabricatorUserCacheType',
     'PhabricatorUserBlurbField' => 'PhabricatorUserCustomField',
     'PhabricatorUserCache' => 'PhabricatorUserDAO',

src/applications/people/controller/PhabricatorPeopleApproveController.php

@@ -24,30 +24,18 @@ final class PhabricatorPeopleApproveController
     }
 
     if ($request->isFormPost()) {
-      id(new PhabricatorUserEditor())
+      $xactions = array();
+
+      $xactions[] = id(new PhabricatorUserTransaction())
+        ->setTransactionType(PhabricatorUserApproveTransaction::TRANSACTIONTYPE)
+        ->setNewValue(true);
+
+      id(new PhabricatorUserTransactionEditor())
         ->setActor($viewer)
-        ->approveUser($user, true);
-
-      $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(),
-          $viewer->getUsername()),
-        PhabricatorEnv::getProductionURI('/'));
-
-      $mail = id(new PhabricatorMetaMTAMail())
-        ->addTos(array($user->getPHID()))
-        ->addCCs(array($viewer->getPHID()))
-        ->setSubject('[Phabricator] '.$title)
-        ->setForceDelivery(true)
-        ->setBody($body)
-        ->saveAndSend();
+        ->setContentSourceFromRequest($request)
+        ->setContinueOnMissingFields(true)
+        ->setContinueOnNoEffect(true)
+        ->applyTransactions($user, $xactions);
 
       return id(new AphrontRedirectResponse())->setURI($done_uri);
     }

src/applications/people/editor/PhabricatorUserEditEngine.php

@@ -75,6 +75,16 @@ final class PhabricatorUserEditEngine
         ->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()),
     );
   }
 

src/applications/people/editor/PhabricatorUserEditor.php

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

src/applications/people/xaction/PhabricatorUserApproveTransaction.php

@@ -0,0 +1,96 @@
+<?php
+
+final class PhabricatorUserApproveTransaction
+  extends PhabricatorUserTransactionType {
+
+  const TRANSACTIONTYPE = 'user.approve';
+
+  public function generateOldValue($object) {
+    return (bool)$object->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;
+  }
+}

src/applications/people/xaction/PhabricatorUserDisableTransaction.php

@@ -15,7 +15,9 @@ final class PhabricatorUserDisableTransaction
 
   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)