In Maniphest tasks, only move old owner to CC if owner changed

Summary:
Fixes T10426. When the owner of a task changes, we try to add the old owner to CC so they're kept in the loop.

Currently, we do this unconditionally. This can add the owner as a subscriber when someone didn't change anything, which is confusing.

Instead, only do this if the owner actually changed.

Test Plan:
  - With "A" as owner, edited task and saved.
    - Before patch, A was added as subscriber.
    - After patch, A not added.
  - With "A" as owner, changed owner to "B" and saved.
    - Both before and after patch, "A" is added as a subscriber.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10426

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

src/applications/maniphest/editor/ManiphestTransactionEditor.php

@@ -1033,16 +1033,22 @@ final class ManiphestTransactionEditor
             ));
         break;
       case ManiphestTransaction::TYPE_OWNER:
+        // If this is a no-op update, don't expand it.
+        $old_value = $object->getOwnerPHID();
+        $new_value = $xaction->getNewValue();
+        if ($old_value === $new_value) {
+          continue;
+        }
+
         // When a task is reassigned, move the old owner to the subscriber
         // list so they're still in the loop.
-        $owner_phid = $object->getOwnerPHID();
+        if ($old_value) {
-        if ($owner_phid) {
           $results[] = id(new ManiphestTransaction())
             ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
             ->setIgnoreOnNoEffect(true)
             ->setNewValue(
               array(
-                '+' => array($owner_phid => $owner_phid),
+                '+' => array($old_value => $old_value),
               ));
         }
         break;