From adf8fdef0e892d4039049495268ce58c2b0b7356 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 3 Apr 2018 09:12:08 -0700
Subject: [PATCH] When remote builds fail, demote revisions to "Changes Planned
 + But, Still A Draft"

Summary:
Depends on D19286. Ref T13110. After builds fail remote builds, put revisions back in the author's queue.

This doesn't actually notify the author quite yet.

Test Plan: Made a failing build plan run on revisions, created a revision, saw it demote after builds failed.

Maniphest Tasks: T13110

Differential Revision: https://secure.phabricator.com/D19287
---
 .../editor/DifferentialTransactionEditor.php  | 17 +++++++++++++--
 .../DifferentialRevisionActionTransaction.php | 21 +++++++++++++++----
 ...erentialRevisionPlanChangesTransaction.php | 17 ++++++++++++---
 3 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
index 81ba158d20..9b1a4d75db 100644
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -1577,8 +1577,21 @@ final class DifferentialTransactionEditor
         // revision state. See T13027 for discussion.
         $this->queueTransaction($xaction);
       } else if ($is_failed) {
-        // TODO: Change to "Changes Planned + Draft", notify the author (only)
-        // of the build failure.
+        // When demoting a revision, we act as "Harbormaster" instead of
+        // the author since this feels a little more natural.
+        $harbormaster_phid = id(new PhabricatorHarbormasterApplication())
+          ->getPHID();
+
+        $xaction = $object->getApplicationTransactionTemplate()
+          ->setAuthorPHID($harbormaster_phid)
+          ->setMetadataValue('draft.demote', true)
+          ->setTransactionType(
+            DifferentialRevisionPlanChangesTransaction::TRANSACTIONTYPE)
+          ->setNewValue(true);
+
+        $this->queueTransaction($xaction);
+
+        // TODO: Notify the author (only) that we did this.
       }
     }
 
diff --git a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php b/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
index 07809dca59..cf2b557764 100644
--- a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
@@ -147,10 +147,23 @@ abstract class DifferentialRevisionActionTransaction
     $actor = $this->getActor();
 
     $action_exception = null;
-    try {
-      $this->validateAction($object, $actor);
-    } catch (Exception $ex) {
-      $action_exception = $ex;
+    foreach ($xactions as $xaction) {
+      // If this is a draft demotion action, let it skip all the normal
+      // validation. This is a little hacky and should perhaps move down
+      // into the actual action implementations, but currently we can not
+      // apply this rule in validateAction() because it doesn't operate on
+      // the actual transaction.
+      if ($xaction->getMetadataValue('draft.demote')) {
+        continue;
+      }
+
+      try {
+        $this->validateAction($object, $actor);
+      } catch (Exception $ex) {
+        $action_exception = $ex;
+      }
+
+      break;
     }
 
     foreach ($xactions as $xaction) {
diff --git a/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php b/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php
index e53426c8ba..bdbe28d5cb 100644
--- a/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php
@@ -96,9 +96,16 @@ final class DifferentialRevisionPlanChangesTransaction
   }
 
   public function getTitle() {
-    return pht(
-      '%s planned changes to this revision.',
-      $this->renderAuthor());
+    if ($this->isDraftDemotion()) {
+      return pht(
+        '%s returned this revision to the author for changes because remote '.
+        'builds failed.',
+        $this->renderAuthor());
+    } else {
+      return pht(
+        '%s planned changes to this revision.',
+        $this->renderAuthor());
+    }
   }
 
   public function getTitleForFeed() {
@@ -108,4 +115,8 @@ final class DifferentialRevisionPlanChangesTransaction
       $this->renderObject());
   }
 
+  private function isDraftDemotion() {
+    return (bool)$this->getMetadataValue('draft.demote');
+  }
+
 }