Allow auditors to be added and removed from commits in a modern way

Summary: Ref T10978. Ref T7676. Make auditors work more like reviewers, so they can be freely added or removed.

Test Plan:
  - Interacted with auditors via "Edit Commit" and API.
  - Comment area is still oldschool and doesn't work yet.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10978, T7676

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

src/__phutil_library_map__.php

@@ -613,6 +613,7 @@ phutil_register_library_map(array(
     'DiffusionCommandEngine' => 'applications/diffusion/protocol/DiffusionCommandEngine.php',
     'DiffusionCommandEngineTestCase' => 'applications/diffusion/protocol/__tests__/DiffusionCommandEngineTestCase.php',
     'DiffusionCommitAffectedFilesHeraldField' => 'applications/diffusion/herald/DiffusionCommitAffectedFilesHeraldField.php',
+    'DiffusionCommitAuditorsTransaction' => 'applications/diffusion/xaction/DiffusionCommitAuditorsTransaction.php',
     'DiffusionCommitAuthorHeraldField' => 'applications/diffusion/herald/DiffusionCommitAuthorHeraldField.php',
     'DiffusionCommitAutocloseHeraldField' => 'applications/diffusion/herald/DiffusionCommitAutocloseHeraldField.php',
     'DiffusionCommitBranchesController' => 'applications/diffusion/controller/DiffusionCommitBranchesController.php',
@@ -659,6 +660,7 @@ phutil_register_library_map(array(
     'DiffusionCommitRevisionReviewersHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionReviewersHeraldField.php',
     'DiffusionCommitRevisionSubscribersHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionSubscribersHeraldField.php',
     'DiffusionCommitTagsController' => 'applications/diffusion/controller/DiffusionCommitTagsController.php',
+    'DiffusionCommitTransactionType' => 'applications/diffusion/xaction/DiffusionCommitTransactionType.php',
     'DiffusionCompareController' => 'applications/diffusion/controller/DiffusionCompareController.php',
     'DiffusionConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionConduitAPIMethod.php',
     'DiffusionController' => 'applications/diffusion/controller/DiffusionController.php',
@@ -5312,6 +5314,7 @@ phutil_register_library_map(array(
     'DiffusionCommandEngine' => 'Phobject',
     'DiffusionCommandEngineTestCase' => 'PhabricatorTestCase',
     'DiffusionCommitAffectedFilesHeraldField' => 'DiffusionCommitHeraldField',
+    'DiffusionCommitAuditorsTransaction' => 'DiffusionCommitTransactionType',
     'DiffusionCommitAuthorHeraldField' => 'DiffusionCommitHeraldField',
     'DiffusionCommitAutocloseHeraldField' => 'DiffusionCommitHeraldField',
     'DiffusionCommitBranchesController' => 'DiffusionController',
@@ -5358,6 +5361,7 @@ phutil_register_library_map(array(
     'DiffusionCommitRevisionReviewersHeraldField' => 'DiffusionCommitHeraldField',
     'DiffusionCommitRevisionSubscribersHeraldField' => 'DiffusionCommitHeraldField',
     'DiffusionCommitTagsController' => 'DiffusionController',
+    'DiffusionCommitTransactionType' => 'PhabricatorModularTransactionType',
     'DiffusionCompareController' => 'DiffusionController',
     'DiffusionConduitAPIMethod' => 'ConduitAPIMethod',
     'DiffusionController' => 'PhabricatorController',
@@ -6761,7 +6765,7 @@ phutil_register_library_map(array(
     'PhabricatorAuditPreviewController' => 'PhabricatorAuditController',
     'PhabricatorAuditReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler',
     'PhabricatorAuditStatusConstants' => 'Phobject',
-    'PhabricatorAuditTransaction' => 'PhabricatorApplicationTransaction',
+    'PhabricatorAuditTransaction' => 'PhabricatorModularTransaction',
     'PhabricatorAuditTransactionComment' => 'PhabricatorApplicationTransactionComment',
     'PhabricatorAuditTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
     'PhabricatorAuditTransactionView' => 'PhabricatorApplicationTransactionView',

src/applications/audit/storage/PhabricatorAuditTransaction.php

@@ -1,7 +1,7 @@
 <?php
 
 final class PhabricatorAuditTransaction
-  extends PhabricatorApplicationTransaction {
+  extends PhabricatorModularTransaction {
 
   const TYPE_COMMIT = 'audit:commit';
 
@@ -20,6 +20,10 @@ final class PhabricatorAuditTransaction
     return 'audit';
   }
 
+  public function getBaseTransactionClass() {
+    return 'DiffusionCommitTransactionType';
+  }
+
   public function getApplicationTransactionType() {
     return PhabricatorRepositoryCommitPHIDType::TYPECONST;
   }

src/applications/diffusion/editor/DiffusionCommitEditEngine.php

@@ -35,12 +35,14 @@ final class DiffusionCommitEditEngine
 
     return id(new PhabricatorRepositoryCommit())
       ->attachRepository($repository)
-      ->attachCommitData($data);
+      ->attachCommitData($data)
+      ->attachAudits(array());
   }
 
   protected function newObjectQuery() {
     return id(new DiffusionCommitQuery())
-      ->needCommitData(true);
+      ->needCommitData(true)
+      ->needAuditRequests(true);
   }
 
   protected function getObjectCreateTitleText($object) {
@@ -77,6 +79,19 @@ final class DiffusionCommitEditEngine
 
     $fields = array();
 
+    $fields[] = id(new PhabricatorDatasourceEditField())
+      ->setKey('auditors')
+      ->setLabel(pht('Auditors'))
+      ->setDatasource(new DiffusionAuditorDatasource())
+      ->setUseEdgeTransactions(true)
+      ->setTransactionType(
+        DiffusionCommitAuditorsTransaction::TRANSACTIONTYPE)
+      ->setCommentActionLabel(pht('Change Auditors'))
+      ->setDescription(pht('Auditors for this commit.'))
+      ->setConduitDescription(pht('Change the auditors for this commit.'))
+      ->setConduitTypeDescription(pht('New auditors.'))
+      ->setValue($object->getAuditorPHIDsForEdit());
+
     $reason = $data->getCommitDetail('autocloseReason', false);
     $reason = PhabricatorRepository::BECAUSE_AUTOCLOSE_FORCED;
     if ($reason !== false) {

src/applications/diffusion/xaction/DiffusionCommitAuditorsTransaction.php

@@ -0,0 +1,231 @@
+<?php
+
+final class DiffusionCommitAuditorsTransaction
+  extends DiffusionCommitTransactionType {
+
+  const TRANSACTIONTYPE = 'diffusion.commit.auditors';
+
+  public function generateOldValue($object) {
+    $auditors = $object->getAudits();
+    return mpull($auditors, 'getAuditStatus', 'getAuditorPHID');
+  }
+
+  public function generateNewValue($object, $value) {
+    $actor = $this->getActor();
+
+    $auditors = $this->generateOldValue($object);
+    $old_auditors = $auditors;
+
+    $request_status = PhabricatorAuditStatusConstants::AUDIT_REQUESTED;
+
+    $rem = idx($value, '-', array());
+    foreach ($rem as $phid) {
+      unset($auditors[$phid]);
+    }
+
+    $add = idx($value, '+', array());
+    $add_map = array();
+    foreach ($add as $phid) {
+      $add_map[$phid] = $request_status;
+    }
+
+    $set = idx($value, '=', null);
+    if ($set !== null) {
+      foreach ($set as $phid) {
+        $add_map[$phid] = $request_status;
+      }
+
+      $auditors = array();
+    }
+
+    foreach ($add_map as $phid => $new_status) {
+      $old_status = idx($old_auditors, $phid);
+
+      if ($old_status) {
+        $auditors[$phid] = $old_status;
+        continue;
+      }
+
+      $auditors[$phid] = $new_status;
+    }
+
+    return $auditors;
+  }
+
+  public function getTransactionHasEffect($object, $old, $new) {
+    ksort($old);
+    ksort($new);
+    return ($old !== $new);
+  }
+
+  public function applyExternalEffects($object, $value) {
+    $src_phid = $object->getPHID();
+
+    $old = $this->generateOldValue($object);
+    $new = $value;
+
+    $auditors = $object->getAudits();
+    $auditors = mpull($auditors, null, 'getAuditorPHID');
+
+    $rem = array_diff_key($old, $new);
+    foreach ($rem as $phid => $status) {
+      $auditor = idx($auditors, $phid);
+      if ($auditor) {
+        $auditor->delete();
+      }
+    }
+
+    foreach ($new as $phid => $status) {
+      $auditor = idx($auditors, $phid);
+      if (!$auditor) {
+        $auditor = id(new PhabricatorRepositoryAuditRequest())
+          ->setAuditorPHID($phid)
+          ->setCommitPHID($object->getPHID());
+      } else {
+        if ($auditor->getAuditStatus() === $status) {
+          continue;
+        }
+      }
+
+      $auditor
+        ->setAuditStatus($status)
+        ->save();
+    }
+  }
+
+  public function getTitle() {
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    $rem = array_diff_key($old, $new);
+    $add = array_diff_key($new, $old);
+    $rem_phids = array_keys($rem);
+    $add_phids = array_keys($add);
+    $total_count = count($rem) + count($add);
+
+    if ($rem && $add) {
+      return pht(
+        '%s edited %s auditor(s), removed %s: %s; added %s: %s.',
+        $this->renderAuthor(),
+        new PhutilNumber($total_count),
+        phutil_count($rem_phids),
+        $this->renderHandleList($rem_phids),
+        phutil_count($add_phids),
+        $this->renderHandleList($add_phids));
+    } else if ($add) {
+      return pht(
+        '%s added %s auditor(s): %s.',
+        $this->renderAuthor(),
+        phutil_count($add_phids),
+        $this->renderHandleList($add_phids));
+    } else {
+      return pht(
+        '%s removed %s auditor(s): %s.',
+        $this->renderAuthor(),
+        phutil_count($rem_phids),
+        $this->renderHandleList($rem_phids));
+    }
+  }
+
+  public function getTitleForFeed() {
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    $rem = array_diff_key($old, $new);
+    $add = array_diff_key($new, $old);
+    $rem_phids = array_keys($rem);
+    $add_phids = array_keys($add);
+    $total_count = count($rem) + count($add);
+
+    if ($rem && $add) {
+      return pht(
+        '%s edited %s auditor(s) for %s, removed %s: %s; added %s: %s.',
+        $this->renderAuthor(),
+        new PhutilNumber($total_count),
+        $this->renderObject(),
+        phutil_count($rem_phids),
+        $this->renderHandleList($rem_phids),
+        phutil_count($add_phids),
+        $this->renderHandleList($add_phids));
+    } else if ($add) {
+      return pht(
+        '%s added %s auditor(s) for %s: %s.',
+        $this->renderAuthor(),
+        phutil_count($add_phids),
+        $this->renderObject(),
+        $this->renderHandleList($add_phids));
+    } else {
+      return pht(
+        '%s removed %s auditor(s) for %s: %s.',
+        $this->renderAuthor(),
+        phutil_count($rem_phids),
+        $this->renderObject(),
+        $this->renderHandleList($rem_phids));
+    }
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $actor = $this->getActor();
+    $errors = array();
+
+    if (!$xactions) {
+      return $errors;
+    }
+
+    $author_phid = $object->getAuthorPHID();
+    $can_author_close_key = 'audit.can-author-close-audit';
+    $can_author_close = PhabricatorEnv::getEnvConfig($can_author_close_key);
+
+    $old = $this->generateOldValue($object);
+    foreach ($xactions as $xaction) {
+      $new = $this->generateNewValue($object, $xaction->getNewValue());
+
+      $add = array_diff_key($new, $old);
+      if (!$add) {
+        continue;
+      }
+
+      $objects = id(new PhabricatorObjectQuery())
+        ->setViewer($actor)
+        ->withPHIDs(array_keys($add))
+        ->execute();
+      $objects = mpull($objects, null, 'getPHID');
+
+      foreach ($add as $phid => $status) {
+        if (!isset($objects[$phid])) {
+          $errors[] = $this->newInvalidError(
+            pht(
+              'Auditor "%s" is not a valid object.',
+              $phid),
+            $xaction);
+          continue;
+        }
+
+        switch (phid_get_type($phid)) {
+          case PhabricatorPeopleUserPHIDType::TYPECONST:
+          case PhabricatorOwnersPackagePHIDType::TYPECONST:
+          case PhabricatorProjectProjectPHIDType::TYPECONST:
+            break;
+          default:
+            $errors[] = $this->newInvalidError(
+              pht(
+                'Auditor "%s" must be a user, a package, or a project.',
+                $phid),
+              $xaction);
+            continue 2;
+        }
+
+        $is_self = ($phid === $author_phid);
+        if ($is_self && !$can_author_close) {
+          $errors[] = $this->newInvalidError(
+            pht('The author of a commit can not be an auditor.'),
+            $xaction);
+          continue;
+        }
+      }
+    }
+
+    return $errors;
+  }
+
+}

src/applications/diffusion/xaction/DiffusionCommitTransactionType.php

@@ -0,0 +1,4 @@
+<?php
+
+abstract class DiffusionCommitTransactionType
+  extends PhabricatorModularTransactionType {}

src/applications/repository/storage/PhabricatorRepositoryCommit.php

@@ -203,6 +203,11 @@ final class PhabricatorRepositoryCommit
     return $authority_audits;
   }
 
+  public function getAuditorPHIDsForEdit() {
+    $audits = $this->getAudits();
+    return mpull($audits, 'getAuditorPHID');
+  }
+
   public function save() {
     if (!$this->mailKey) {
       $this->mailKey = Filesystem::readRandomCharacters(20);