From bc41c3f5a5b95da3d2cae75d701fadae3eb6b680 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 30 Jan 2017 09:51:21 -0800
Subject: [PATCH] Use DifferentialCommitMessageParser and Modular Transactions
 to implement "Auditors: ..."

Summary:
Ref T10978. Updates how we implement "Auditors: ..." in commit messages:

  - Use the same parsing code as everything else.
    - (Also: parse package names.)
  - Use the new transaction code.

Also, fix some UI strings.

Test Plan: Used `bin/repository reparse --herald <commit>` to re-run this code on commits with various messages (valid Auditors, invalid Auditors, no Auditors). Saw appropriate auditors added in the UI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10978

Differential Revision: https://secure.phabricator.com/D17262
---
 .../audit/editor/PhabricatorAuditEditor.php   | 31 +++++++------------
 ...DifferentialAuditorsCommitMessageField.php |  1 +
 .../PhabricatorUSEnglishTranslation.php       | 20 ++++++++++++
 3 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php
index 952cf50a77..ff1e69d8cb 100644
--- a/src/applications/audit/editor/PhabricatorAuditEditor.php
+++ b/src/applications/audit/editor/PhabricatorAuditEditor.php
@@ -375,35 +375,26 @@ final class PhabricatorAuditEditor
   private function createAuditRequestTransactionFromCommitMessage(
     PhabricatorRepositoryCommit $commit) {
 
+    $actor = $this->getActor();
     $data = $commit->getCommitData();
     $message = $data->getCommitMessage();
 
-    $matches = null;
-    if (!preg_match('/^Auditors?:\s*(.*)$/im', $message, $matches)) {
-      return array();
-    }
-
-    $phids = id(new PhabricatorObjectListQuery())
-      ->setViewer($this->getActor())
-      ->setAllowPartialResults(true)
-      ->setAllowedTypes(
-        array(
-          PhabricatorPeopleUserPHIDType::TYPECONST,
-          PhabricatorProjectProjectPHIDType::TYPECONST,
-        ))
-      ->setObjectList($matches[1])
-      ->execute();
+    $result = DifferentialCommitMessageParser::newStandardParser($actor)
+      ->setRaiseMissingFieldErrors(false)
+      ->parseFields($message);
 
+    $field_key = DifferentialAuditorsCommitMessageField::FIELDKEY;
+    $phids = idx($result, $field_key, null);
     if (!$phids) {
       return array();
     }
 
-    foreach ($phids as $phid) {
-      $this->addAuditReason($phid, pht('Requested by Author'));
-    }
     return id(new PhabricatorAuditTransaction())
-      ->setTransactionType(PhabricatorAuditActionConstants::ADD_AUDITORS)
-      ->setNewValue(array_fuse($phids));
+      ->setTransactionType(DiffusionCommitAuditorsTransaction::TRANSACTIONTYPE)
+      ->setNewValue(
+        array(
+          '+' => array_fuse($phids),
+        ));
   }
 
   protected function sortTransactions(array $xactions) {
diff --git a/src/applications/differential/field/DifferentialAuditorsCommitMessageField.php b/src/applications/differential/field/DifferentialAuditorsCommitMessageField.php
index 94f629e557..42e6b5ecd5 100644
--- a/src/applications/differential/field/DifferentialAuditorsCommitMessageField.php
+++ b/src/applications/differential/field/DifferentialAuditorsCommitMessageField.php
@@ -15,6 +15,7 @@ final class DifferentialAuditorsCommitMessageField
       array(
         PhabricatorPeopleUserPHIDType::TYPECONST,
         PhabricatorProjectProjectPHIDType::TYPECONST,
+        PhabricatorOwnersPackagePHIDType::TYPECONST,
       ));
   }
 
diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
index 291097a6c0..428f3955cf 100644
--- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
+++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
@@ -1588,6 +1588,26 @@ final class PhabricatorUSEnglishTranslation
         ),
       ),
 
+      '%s added %s auditor(s): %s.' => array(
+        array(
+          '%s added an auditor: %3$s.',
+          '%s added auditors: %3$s.',
+        ),
+      ),
+
+      '%s removed %s auditor(s): %s.' => array(
+        array(
+          '%s removed an auditor: %3$s.',
+          '%s removed auditors: %3$s.',
+        ),
+      ),
+
+      '%s edited %s auditor(s), removed %s: %s; added %s: %s.' => array(
+        array(
+          '%s edited auditors, removed: %4$s; added: %6$s.',
+        ),
+      ),
+
     );
   }