Give Audit an informational "This commit now requires (something)..." transaction

Summary: Ref T2393. This adds a state-change transaction hint to Audit, like we have in Differential. This is partly for consistency and partly to make it more clear what should happen next.

Test Plan: {F2477848}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2393

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

src/__phutil_library_map__.php

@@ -671,6 +671,7 @@ phutil_register_library_map(array(
     'DiffusionCommitRevisionReviewersHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionReviewersHeraldField.php',
     'DiffusionCommitRevisionSubscribersHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionSubscribersHeraldField.php',
     'DiffusionCommitSearchConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionCommitSearchConduitAPIMethod.php',
+    'DiffusionCommitStateTransaction' => 'applications/diffusion/xaction/DiffusionCommitStateTransaction.php',
     'DiffusionCommitTagsController' => 'applications/diffusion/controller/DiffusionCommitTagsController.php',
     'DiffusionCommitTransactionType' => 'applications/diffusion/xaction/DiffusionCommitTransactionType.php',
     'DiffusionCompareController' => 'applications/diffusion/controller/DiffusionCompareController.php',
@@ -5378,6 +5379,7 @@ phutil_register_library_map(array(
     'DiffusionCommitRevisionReviewersHeraldField' => 'DiffusionCommitHeraldField',
     'DiffusionCommitRevisionSubscribersHeraldField' => 'DiffusionCommitHeraldField',
     'DiffusionCommitSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod',
+    'DiffusionCommitStateTransaction' => 'DiffusionCommitTransactionType',
     'DiffusionCommitTagsController' => 'DiffusionController',
     'DiffusionCommitTransactionType' => 'PhabricatorModularTransactionType',
     'DiffusionCompareController' => 'DiffusionController',

src/applications/audit/editor/PhabricatorAuditEditor.php

@@ -361,14 +361,41 @@ final class PhabricatorAuditEditor
       }
     }
 
+    $old_status = $object->getAuditStatus();
+
     $requests = $object->getAudits();
     $object->updateAuditStatus($requests);
+
+    $new_status = $object->getAuditStatus();
+
     $object->save();
 
     if ($import_status_flag) {
       $object->writeImportStatusFlag($import_status_flag);
     }
 
+    $partial_status = PhabricatorAuditCommitStatusConstants::PARTIALLY_AUDITED;
+
+    // If the commit has changed state after this edit, add an informational
+    // transaction about the state change.
+    if ($old_status !== $new_status) {
+      if ($new_status === $partial_status) {
+        // This state isn't interesting enough to get a transaction. The
+        // best way we could lead the user forward is something like "This
+        // commit still requires additional audits." but that's redundant and
+        // probably not very useful.
+      } else {
+        $xaction = $object->getApplicationTransactionTemplate()
+          ->setTransactionType(DiffusionCommitStateTransaction::TRANSACTIONTYPE)
+          ->setOldValue($old_status)
+          ->setNewValue($new_status);
+
+        $xaction = $this->populateTransaction($object, $xaction);
+
+        $xaction->save();
+      }
+    }
+
     // Collect auditor PHIDs for building mail.
     $this->auditorPHIDs = mpull($object->getAudits(), 'getAuditorPHID');
 

src/applications/audit/storage/PhabricatorAuditTransaction.php

@@ -498,4 +498,22 @@ final class PhabricatorAuditTransaction
     }
     return $tags;
   }
+
+  public function shouldDisplayGroupWith(array $group) {
+    // Make the "This commit now requires audit." state message stand alone.
+    $type_state = DiffusionCommitStateTransaction::TRANSACTIONTYPE;
+
+    if ($this->getTransactionType() == $type_state) {
+      return false;
+    }
+
+    foreach ($group as $xaction) {
+      if ($xaction->getTransactionType() == $type_state) {
+        return false;
+      }
+    }
+
+    return parent::shouldDisplayGroupWith($group);
+  }
+
 }

src/applications/diffusion/xaction/DiffusionCommitStateTransaction.php

@@ -0,0 +1,66 @@
+<?php
+
+final class DiffusionCommitStateTransaction
+  extends DiffusionCommitTransactionType {
+
+  const TRANSACTIONTYPE = 'diffusion.commit.state';
+
+  public function generateNewValue($object, $value) {
+    // NOTE: This transaction can not be generated or applied normally. It is
+    // written to the transaction log as a side effect of a state change.
+    throw new PhutilMethodNotImplementedException();
+  }
+
+  public function getIcon() {
+    $new = $this->getNewValue();
+    return PhabricatorAuditCommitStatusConstants::getStatusIcon($new);
+  }
+
+  public function getColor() {
+    $new = $this->getNewValue();
+    return PhabricatorAuditCommitStatusConstants::getStatusColor($new);
+  }
+
+  public function getTitle() {
+    $new = $this->getNewValue();
+
+    switch ($new) {
+      case PhabricatorAuditCommitStatusConstants::NONE:
+        return pht('This commit no longer requires audit.');
+      case PhabricatorAuditCommitStatusConstants::NEEDS_AUDIT:
+        return pht('This commit now requires audit.');
+      case PhabricatorAuditCommitStatusConstants::CONCERN_RAISED:
+        return pht('This commit now has outstanding concerns.');
+      case PhabricatorAuditCommitStatusConstants::FULLY_AUDITED:
+        return pht('All concerns with this commit have now been addressed.');
+    }
+
+    return null;
+  }
+
+  public function getTitleForFeed() {
+    $new = $this->getNewValue();
+
+    switch ($new) {
+      case PhabricatorAuditCommitStatusConstants::NONE:
+        return pht(
+          '%s no longer requires audit.',
+          $this->renderObject());
+      case PhabricatorAuditCommitStatusConstants::NEEDS_AUDIT:
+        return pht(
+          '%s now requires audit.',
+          $this->renderObject());
+      case PhabricatorAuditCommitStatusConstants::CONCERN_RAISED:
+        return pht(
+          '%s now has outstanding concerns.',
+          $this->renderObject());
+      case PhabricatorAuditCommitStatusConstants::FULLY_AUDITED:
+        return pht(
+          'All concerns with %s have now been addressed.',
+          $this->renderObject());
+    }
+
+    return null;
+  }
+
+}