Use transactions to apply "resign" and "close" Audit actions

Summary: Ref T4896. Hook these up with new stuff.

Test Plan:
  - Closed an audit.
  - Resigned from an audit.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4896

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

src/applications/audit/editor/PhabricatorAuditCommentEditor.php

@@ -69,42 +69,13 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor {
     }
 
     if ($action == PhabricatorAuditActionConstants::CLOSE) {
-      if (!PhabricatorEnv::getEnvConfig('audit.can-author-close-audit')) {
+
-        throw new Exception('Cannot Close Audit without enabling'.
+      // This is now applied by the transaction Editor.
-          'audit.can-author-close-audit');
+
-      }
-      // "Close" means wipe out all the concerns.
-      $concerned_status = PhabricatorAuditStatusConstants::CONCERNED;
-      foreach ($requests as $request) {
-        if ($request->getAuditStatus() == $concerned_status) {
-          $request->setAuditStatus(PhabricatorAuditStatusConstants::CLOSED);
-          $request->save();
-        }
-      }
     } else if ($action == PhabricatorAuditActionConstants::RESIGN) {
-      // "Resign" has unusual rules for writing user rows, only affects the
-      // user row (never package/project rows), and always affects the user
-      // row (other actions don't, if they were able to affect a package/project
-      // row).
-      $actor_request = null;
-      foreach ($requests as $request) {
-        if ($request->getAuditorPHID() == $actor->getPHID()) {
-          $actor_request = $request;
-          break;
-        }
-      }
-      if (!$actor_request) {
-        $actor_request = id(new PhabricatorRepositoryAuditRequest())
-          ->setCommitPHID($commit->getPHID())
-          ->setAuditorPHID($actor->getPHID())
-          ->setAuditReasons(array('Resigned'));
-      }
 
-      $actor_request
+      // This is now applied by the transaction Editor.
-        ->setAuditStatus(PhabricatorAuditStatusConstants::RESIGNED)
-        ->save();
 
-      $requests[] = $actor_request;
     } else {
       $have_any_requests = false;
       foreach ($requests as $request) {

src/applications/audit/editor/PhabricatorAuditEditor.php

@@ -119,15 +119,61 @@ final class PhabricatorAuditEditor
             ->save();
         }
 
-        $object->updateAuditStatus($requests);
         $object->attachAudits($requests);
-        $object->save();
         return;
     }
 
     return parent::applyCustomExternalTransaction($object, $xaction);
   }
 
+  protected function applyFinalEffects(
+    PhabricatorLiskDAO $object,
+    array $xactions) {
+
+    $status_concerned = PhabricatorAuditStatusConstants::CONCERNED;
+    $status_closed = PhabricatorAuditStatusConstants::CLOSED;
+    $status_resigned = PhabricatorAuditStatusConstants::RESIGNED;
+
+    $actor_phid = $this->requireActor()->getPHID();
+
+    foreach ($xactions as $xaction) {
+      switch ($xaction->getTransactionType()) {
+        case PhabricatorAuditActionConstants::ACTION:
+          switch ($xaction->getNewValue()) {
+            case PhabricatorAuditActionConstants::CLOSE:
+              // "Close" means wipe out all the concerns.
+              $requests = $object->getAudits();
+              foreach ($requests as $request) {
+                if ($request->getAuditStatus() == $status_concerned) {
+                  $request
+                    ->setAuditStatus($status_closed)
+                    ->save();
+                }
+              }
+              break;
+            case PhabricatorAuditActionConstants::RESIGN:
+              $requests = $object->getAudits();
+              $requests = mpull($requests, null, 'getAuditorPHID');
+              $actor_request = idx($requests, $actor_phid);
+
+              if ($actor_request) {
+                $actor_request
+                  ->setAuditStatus($status_resigned)
+                  ->save();
+              }
+              break;
+          }
+          break;
+      }
+    }
+
+    $requests = $object->getAudits();
+    $object->updateAuditStatus($requests);
+    $object->save();
+
+    return $xactions;
+  }
+
   protected function sortTransactions(array $xactions) {
     $xactions = parent::sortTransactions($xactions);
 
@@ -146,6 +192,69 @@ final class PhabricatorAuditEditor
     return array_values(array_merge($head, $tail));
   }
 
+  protected function validateTransaction(
+    PhabricatorLiskDAO $object,
+    $type,
+    array $xactions) {
+
+    $errors = parent::validateTransaction($object, $type, $xactions);
+
+    foreach ($xactions as $xaction) {
+      switch ($type) {
+        case PhabricatorAuditActionConstants::ACTION:
+          $error = $this->validateAuditAction(
+            $object,
+            $type,
+            $xaction,
+            $xaction->getNewValue());
+          if ($error) {
+            $errors[] = new PhabricatorApplicationTransactionValidationError(
+              $type,
+              pht('Invalid'),
+              $error,
+              $xaction);
+          }
+          break;
+      }
+    }
+
+    return $errors;
+  }
+
+  private function validateAuditAction(
+    PhabricatorLiskDAO $object,
+    $type,
+    PhabricatorAuditTransaction $xaction,
+    $action) {
+
+    $can_author_close_key = 'audit.can-author-close-audit';
+    $can_author_close = PhabricatorEnv::getEnvConfig($can_author_close_key);
+
+    $actor_is_author = ($object->getAuthorPHID()) &&
+      ($object->getAuthorPHID() == $this->requireActor()->getPHID());
+
+    switch ($action) {
+      case PhabricatorAuditActionConstants::CLOSE:
+        if (!$actor_is_author) {
+          return pht(
+            'You can not close this audit because you are not the author '.
+            'of the commit.');
+        }
+
+        if (!$can_author_close) {
+          return pht(
+            'You can not close this audit because "%s" is disabled in '.
+            'the Phabricator configuration.',
+            $can_author_close_key);
+        }
+
+        break;
+    }
+
+    return null;
+  }
+
+
   protected function supportsSearch() {
     return true;
   }