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'); + } + }