From 93cb6e3bdeb07be7b9c6835bcc2d237e2abb708e Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 29 Mar 2018 09:51:18 -0700
Subject: [PATCH] Make updating a revision with the same active diff a no-op

Summary: Ref T13114. See PHI515. Updating a revision with the same, currently active diff became an error at some point (probably D19175). This is inconsistent; make it an allowable no-op instead.

Test Plan:
  - Updated a revision's diff via Conduit.
  - Updated to the same diff, no-op.
  - Tried to update a different revision, error ("already attached elsewhere").
  - Updated with a different diff.
  - Tried to update with the original diff, error ("previously attached version").

Maniphest Tasks: T13114

Differential Revision: https://secure.phabricator.com/D19267
---
 .../DifferentialRevisionUpdateTransaction.php | 27 ++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/applications/differential/xaction/DifferentialRevisionUpdateTransaction.php b/src/applications/differential/xaction/DifferentialRevisionUpdateTransaction.php
index c89c3c79e1..2b74cbfab5 100644
--- a/src/applications/differential/xaction/DifferentialRevisionUpdateTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionUpdateTransaction.php
@@ -137,7 +137,32 @@ final class DifferentialRevisionUpdateTransaction
         continue;
       }
 
-      if ($diff->getRevisionID()) {
+      $is_attached = ($diff->getRevisionID() == $object->getID());
+      if ($is_attached) {
+        $is_active = ($diff_phid == $object->getActiveDiffPHID());
+      } else {
+        $is_active = false;
+      }
+
+      if ($is_attached) {
+        if ($is_active) {
+          // This is a no-op: we're reattaching the current active diff to the
+          // revision it is already attached to. This is valid and will just
+          // be dropped later on in the process.
+        } else {
+          // At least for now, there's no support for "undoing" a diff and
+          // reverting to an older proposed change without just creating a
+          // new diff from whole cloth.
+          $errors[] = $this->newInvalidError(
+            pht(
+              'You can not update this revision with the specified diff '.
+              '("%s") because this diff is already attached to the revision '.
+              'as an older version of the change.',
+              $diff_phid),
+            $xaction);
+          continue;
+        }
+      } else if ($diff->getRevisionID()) {
         $errors[] = $this->newInvalidError(
           pht(
             'You can not update this revision with the specified diff ("%s") '.