From 3b6a651b69fe96ce782c24b47308a8d98b781acf Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 23 Feb 2017 14:42:40 -0800
Subject: [PATCH] Merge multiple Auditors transactions from Herald

Summary:
Fixes T12302. Currently, we aren't merging multiple "AddAuditors" transactions correctly.

This can occur when Herald triggers multiple auditor rules.

Instead, merge them.

Test Plan:
  - Wrote two different Herald rules that add auditors.
  - Pushed a commit which triggered them.
  - After the change, saw all the auditors get added correctly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12302

Differential Revision: https://secure.phabricator.com/D17403
---
 .../DiffusionCommitAuditorsTransaction.php    | 25 +++++++++++++++++++
 ...habricatorApplicationTransactionEditor.php |  6 +++++
 .../PhabricatorModularTransactionType.php     |  7 ++++++
 3 files changed, 38 insertions(+)

diff --git a/src/applications/diffusion/xaction/DiffusionCommitAuditorsTransaction.php b/src/applications/diffusion/xaction/DiffusionCommitAuditorsTransaction.php
index a4b1d817b4..a02a34590d 100644
--- a/src/applications/diffusion/xaction/DiffusionCommitAuditorsTransaction.php
+++ b/src/applications/diffusion/xaction/DiffusionCommitAuditorsTransaction.php
@@ -58,6 +58,31 @@ final class DiffusionCommitAuditorsTransaction
     return ($old !== $new);
   }
 
+  public function mergeTransactions(
+    $object,
+    PhabricatorApplicationTransaction $u,
+    PhabricatorApplicationTransaction $v) {
+
+    $u_new = $u->getNewValue();
+    $v_new = $v->getNewValue();
+
+    $result = $v_new;
+    foreach (array('-', '+') as $key) {
+      $u_value = idx($u_new, $key, array());
+      $v_value = idx($v_new, $key, array());
+
+      $merged = $u_value + $v_value;
+
+      if ($merged) {
+        $result[$key] = $merged;
+      }
+    }
+
+    $u->setNewValue($result);
+
+    return $u;
+  }
+
   public function applyExternalEffects($object, $value) {
     $src_phid = $object->getPHID();
 
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
index 5851e0186c..60dcc1d513 100644
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -1462,6 +1462,12 @@ abstract class PhabricatorApplicationTransactionEditor
 
     $type = $u->getTransactionType();
 
+    $xtype = $this->getModularTransactionType($type);
+    if ($xtype) {
+      $object = $this->object;
+      return $xtype->mergeTransactions($object, $u, $v);
+    }
+
     switch ($type) {
       case PhabricatorTransactions::TYPE_SUBSCRIBERS:
         return $this->mergePHIDOrEdgeTransactions($u, $v);
diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php
index f57d91b08f..55ad678539 100644
--- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php
+++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php
@@ -87,6 +87,13 @@ abstract class PhabricatorModularTransactionType
     return array();
   }
 
+  public function mergeTransactions(
+    $object,
+    PhabricatorApplicationTransaction $u,
+    PhabricatorApplicationTransaction $v) {
+    return null;
+  }
+
   final public function setStorage(
     PhabricatorApplicationTransaction $xaction) {
     $this->storage = $xaction;