Remove a bunch of reundant checks for transactions with no effect from Maniphest

Summary: Ref T2217. These checks are no longer necessary, ApplicationTransactions handle them for us.

Test Plan:
  - Made a no-effect edit, verified no new transactions showed up.
  - Made real edits, saw them happen and leave transactions.
  - Made an edit which just reorders CCs, saw it detected as no-effect.
  - As above, with projects.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2217

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

src/applications/maniphest/controller/ManiphestTaskEditController.php

@@ -104,22 +104,16 @@ final class ManiphestTaskEditController extends ManiphestController {
       $new_desc = $request->getStr('description');
       $new_status = $request->getStr('status');
 
-      $workflow = '';
-
-      if ($new_title != $task->getTitle()) {
-        $changes[ManiphestTransaction::TYPE_TITLE] = $new_title;
-      }
-      if ($new_desc != $task->getDescription()) {
-        $changes[ManiphestTransaction::TYPE_DESCRIPTION] = $new_desc;
-      }
-      if ($new_status != $task->getStatus()) {
-        $changes[ManiphestTransaction::TYPE_STATUS] = $new_status;
-      }
-
       if (!$task->getID()) {
         $workflow = 'create';
+      } else {
+        $workflow = '';
       }
 
+      $changes[ManiphestTransaction::TYPE_TITLE] = $new_title;
+      $changes[ManiphestTransaction::TYPE_DESCRIPTION] = $new_desc;
+      $changes[ManiphestTransaction::TYPE_STATUS] = $new_status;
+
       $owner_tokenizer = $request->getArr('assigned_to');
       $owner_phid = reset($owner_tokenizer);
 
@@ -168,30 +162,13 @@ final class ManiphestTaskEditController extends ManiphestController {
         $task->setCCPHIDs($request->getArr('cc'));
         $task->setProjectPHIDs($request->getArr('projects'));
       } else {
-        if ($request->getInt('priority') != $task->getPriority()) {
+
         $changes[ManiphestTransaction::TYPE_PRIORITY] =
           $request->getInt('priority');
-        }
-
-        if ($owner_phid != $task->getOwnerPHID()) {
         $changes[ManiphestTransaction::TYPE_OWNER] = $owner_phid;
-        }
-
-        if ($request->getArr('cc') != $task->getCCPHIDs()) {
         $changes[ManiphestTransaction::TYPE_CCS] = $request->getArr('cc');
-        }
+        $changes[ManiphestTransaction::TYPE_PROJECTS] =
-
+          $request->getArr('projects');
-        $new_proj_arr = $request->getArr('projects');
-        $new_proj_arr = array_values($new_proj_arr);
-        sort($new_proj_arr);
-
-        $cur_proj_arr = $task->getProjectPHIDs();
-        $cur_proj_arr = array_values($cur_proj_arr);
-        sort($cur_proj_arr);
-
-        if ($new_proj_arr != $cur_proj_arr) {
-          $changes[ManiphestTransaction::TYPE_PROJECTS] = $new_proj_arr;
-        }
 
         if ($files) {
           $file_map = mpull($files, 'getPHID');
@@ -201,12 +178,6 @@ final class ManiphestTaskEditController extends ManiphestController {
           );
         }
 
-        $content_source = PhabricatorContentSource::newForSource(
-          PhabricatorContentSource::SOURCE_WEB,
-          array(
-            'ip' => $request->getRemoteAddr(),
-          ));
-
         $template = new ManiphestTransaction();
         $transactions = array();
 
@@ -227,15 +198,6 @@ final class ManiphestTaskEditController extends ManiphestController {
             $old = idx($old_values, $aux_key);
             $new = $aux_field->getNewValueForApplicationTransactions();
 
-            // TODO: This is a ghetto check for transactions with no effect.
-            if (!is_array($old) && !is_array($new)) {
-              if ((string)$old === (string)$new) {
-                continue;
-              }
-            } else if ($old == $new) {
-              continue;
-            }
-
             $transaction->setOldValue($old);
             $transaction->setNewValue($new);
 

src/applications/maniphest/editor/ManiphestTransactionEditorPro.php

@@ -34,11 +34,11 @@ final class ManiphestTransactionEditorPro
       case ManiphestTransaction::TYPE_DESCRIPTION:
         return $object->getDescription();
       case ManiphestTransaction::TYPE_OWNER:
-        return $object->getOwnerPHID();
+        return nonempty($object->getOwnerPHID(), null);
       case ManiphestTransaction::TYPE_CCS:
         return array_values(array_unique($object->getCCPHIDs()));
       case ManiphestTransaction::TYPE_PROJECTS:
-        return $object->getProjectPHIDs();
+        return array_values(array_unique($object->getProjectPHIDs()));
       case ManiphestTransaction::TYPE_ATTACH:
         return $object->getAttached();
       case ManiphestTransaction::TYPE_EDGE:
@@ -57,11 +57,12 @@ final class ManiphestTransactionEditorPro
       case ManiphestTransaction::TYPE_STATUS:
         return (int)$xaction->getNewValue();
       case ManiphestTransaction::TYPE_CCS:
+      case ManiphestTransaction::TYPE_PROJECTS:
         return array_values(array_unique($xaction->getNewValue()));
+      case ManiphestTransaction::TYPE_OWNER:
+        return nonempty($xaction->getNewValue(), null);
       case ManiphestTransaction::TYPE_TITLE:
       case ManiphestTransaction::TYPE_DESCRIPTION:
-      case ManiphestTransaction::TYPE_OWNER:
-      case ManiphestTransaction::TYPE_PROJECTS:
       case ManiphestTransaction::TYPE_ATTACH:
       case ManiphestTransaction::TYPE_EDGE:
         return $xaction->getNewValue();
@@ -69,6 +70,25 @@ final class ManiphestTransactionEditorPro
 
   }
 
+  protected function transactionHasEffect(
+    PhabricatorLiskDAO $object,
+    PhabricatorApplicationTransaction $xaction) {
+
+    $old = $xaction->getOldValue();
+    $new = $xaction->getNewValue();
+
+    switch ($xaction->getTransactionType()) {
+      case ManiphestTransaction::TYPE_PROJECTS:
+      case ManiphestTransaction::TYPE_CCS:
+        sort($old);
+        sort($new);
+        return ($old !== $new);
+    }
+
+    return parent::transactionHasEffect($object, $xaction);
+  }
+
+
   protected function applyCustomInternalTransaction(
     PhabricatorLiskDAO $object,
     PhabricatorApplicationTransaction $xaction) {