From 81be1cf27b5a86a69e5dc52f56c2981efca9e7d8 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 21 Mar 2012 04:37:01 -0700
Subject: [PATCH] Fix batch editor for edits which both add and remove projects

Summary:
Aggregate multiple add/remove transactions so we don't restore removed projects for a (remove + add) batch edit.

(Possibly we should do this in the TransactionEditor as well / instead, but it's fairly easy here and this is the only possible case currently.)

Test Plan: Performed a remove + add batch edit without issues.

Reviewers: btrahan

Reviewed By: btrahan

CC: cpiro, aran, epriestley

Maniphest Tasks: T985

Differential Revision: https://secure.phabricator.com/D1967
---
 .../batch/ManiphestBatchEditController.php    | 32 ++++++++++++++++---
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/src/applications/maniphest/controller/batch/ManiphestBatchEditController.php b/src/applications/maniphest/controller/batch/ManiphestBatchEditController.php
index 08e0ab6cd9..02ac95b92b 100644
--- a/src/applications/maniphest/controller/batch/ManiphestBatchEditController.php
+++ b/src/applications/maniphest/controller/batch/ManiphestBatchEditController.php
@@ -143,6 +143,8 @@ final class ManiphestBatchEditController extends ManiphestController {
 
     // TODO: Set content source to "batch edit".
 
+    $project_xaction = null;
+
     $xactions = array();
     foreach ($actions as $action) {
       $value = $action['value'];
@@ -152,7 +154,20 @@ final class ManiphestBatchEditController extends ManiphestController {
 
           $is_remove = ($action['action'] == 'remove_project');
 
-          $current = array_fill_keys($task->getProjectPHIDs(), true);
+          // We want to roll up all of the add and remove actions into a single
+          // transaction so we don't restore removed projects or remove added
+          // projects in subsequent transactions. If we've already made some
+          // modification to a task's projects, use that as the starting point.
+          // Otherwise, start with the value on the task.
+
+          if ($project_xaction) {
+            $xaction = $project_xaction;
+            $current = $xaction->getNewValue();
+          } else {
+            $current = $task->getProjectPHIDs();
+          }
+
+          $current = array_fill_keys($current, true);
           $value   = array_fill_keys($value, true);
 
           $new = $current;
@@ -178,11 +193,20 @@ final class ManiphestBatchEditController extends ManiphestController {
             break;
           }
 
+          // If this is the first project-related transaction, create a new
+          // transaction object and populate it with appropriate defaults.
+
+          if (!$project_xaction) {
+            $xaction = clone $template;
+            $xaction->setTransactionType(
+              ManiphestTransactionType::TYPE_PROJECTS);
+
+            $xactions[] = $xaction;
+            $project_xaction = $xaction;
+          }
+
           $new = array_keys($new);
-          $xaction = clone $template;
-          $xaction->setTransactionType(ManiphestTransactionType::TYPE_PROJECTS);
           $xaction->setNewValue($new);
-          $xactions[] = $xaction;
           break;
       }
     }