From 28cec2f8a2bf96df572c04360c412c8436356421 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 27 Oct 2017 08:55:56 -0700 Subject: [PATCH] Allow revisions to be held as drafts, even after builds finish Summary: Ref T2543. Instead of autosubmitting revisions to "Needs Review" when builds finish, allow them to be held in "Draft" indefinitely. There's currently no UI for this. I plan to just expose it as `arc diff --draft` for now, in a followup change. Test Plan: - Created a revision (via Conduit) with "hold as draft", saw it hold as draft after builds finished. - Created a revision (normally), saw it autosubmit after builds finished. - Requested review of a "hold as draft" revision to kick it out of draft state. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T2543 Differential Revision: https://secure.phabricator.com/D18737 --- src/__phutil_library_map__.php | 2 + .../customfield/DifferentialDraftField.php | 6 +++ .../editor/DifferentialRevisionEditEngine.php | 16 +++++++ .../editor/DifferentialTransactionEditor.php | 2 +- .../storage/DifferentialRevision.php | 9 ++++ ...fferentialRevisionHoldDraftTransaction.php | 47 +++++++++++++++++++ 6 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 src/applications/differential/xaction/DifferentialRevisionHoldDraftTransaction.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index ff1f00c821..7180a7ae04 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -550,6 +550,7 @@ phutil_register_library_map(array( 'DifferentialRevisionHasTaskRelationship' => 'applications/differential/relationships/DifferentialRevisionHasTaskRelationship.php', 'DifferentialRevisionHeraldField' => 'applications/differential/herald/DifferentialRevisionHeraldField.php', 'DifferentialRevisionHeraldFieldGroup' => 'applications/differential/herald/DifferentialRevisionHeraldFieldGroup.php', + 'DifferentialRevisionHoldDraftTransaction' => 'applications/differential/xaction/DifferentialRevisionHoldDraftTransaction.php', 'DifferentialRevisionIDCommitMessageField' => 'applications/differential/field/DifferentialRevisionIDCommitMessageField.php', 'DifferentialRevisionInlineTransaction' => 'applications/differential/xaction/DifferentialRevisionInlineTransaction.php', 'DifferentialRevisionInlinesController' => 'applications/differential/controller/DifferentialRevisionInlinesController.php', @@ -5592,6 +5593,7 @@ phutil_register_library_map(array( 'DifferentialRevisionHasTaskRelationship' => 'DifferentialRevisionRelationship', 'DifferentialRevisionHeraldField' => 'HeraldField', 'DifferentialRevisionHeraldFieldGroup' => 'HeraldFieldGroup', + 'DifferentialRevisionHoldDraftTransaction' => 'DifferentialRevisionTransactionType', 'DifferentialRevisionIDCommitMessageField' => 'DifferentialCommitMessageField', 'DifferentialRevisionInlineTransaction' => 'PhabricatorModularTransactionType', 'DifferentialRevisionInlinesController' => 'DifferentialController', diff --git a/src/applications/differential/customfield/DifferentialDraftField.php b/src/applications/differential/customfield/DifferentialDraftField.php index e7ed2bedb2..5d625e3ce9 100644 --- a/src/applications/differential/customfield/DifferentialDraftField.php +++ b/src/applications/differential/customfield/DifferentialDraftField.php @@ -36,6 +36,12 @@ final class DifferentialDraftField return array(); } + // If the author has held this revision as a draft explicitly, don't + // show any misleading messages about it autosubmitting later. + if ($revision->getHoldAsDraft()) { + return array(); + } + $warnings = array(); $blocking_map = array( diff --git a/src/applications/differential/editor/DifferentialRevisionEditEngine.php b/src/applications/differential/editor/DifferentialRevisionEditEngine.php index 9aad031a7c..2bd3a5cdc3 100644 --- a/src/applications/differential/editor/DifferentialRevisionEditEngine.php +++ b/src/applications/differential/editor/DifferentialRevisionEditEngine.php @@ -235,6 +235,22 @@ final class DifferentialRevisionEditEngine $fields[] = $action->newEditField($object, $viewer); } + $fields[] = id(new PhabricatorBoolEditField()) + ->setKey('draft') + ->setLabel(pht('Hold as Draft')) + ->setIsConduitOnly(true) + ->setOptions( + pht('Autosubmit Once Builds Finish'), + pht('Hold as Draft')) + ->setTransactionType( + DifferentialRevisionHoldDraftTransaction::TRANSACTIONTYPE) + ->setDescription(pht('Hold revision as as draft.')) + ->setConduitDescription( + pht( + 'Change autosubmission from draft state after builds finish.')) + ->setConduitTypeDescription(pht('New "Hold as Draft" setting.')) + ->setValue($object->getHoldAsDraft()); + return $fields; } diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index 9ab30c3f06..edc8fd0ad3 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -1540,7 +1540,7 @@ final class DifferentialTransactionEditor protected function didApplyTransactions($object, array $xactions) { // If a draft revision has no outstanding builds and we're automatically // making drafts public after builds finish, make the revision public. - $auto_undraft = true; + $auto_undraft = !$object->getHoldAsDraft(); if ($object->isDraft() && $auto_undraft) { $active_builds = $this->hasActiveBuilds($object); diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index da7e18ce05..467e503f6f 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -57,6 +57,7 @@ final class DifferentialRevision extends DifferentialDAO const RELATION_SUBSCRIBED = 'subd'; const PROPERTY_CLOSED_FROM_ACCEPTED = 'wasAcceptedBeforeClose'; + const PROPERTY_DRAFT_HOLD = 'draft.hold'; public static function initializeNewRevision(PhabricatorUser $actor) { $app = id(new PhabricatorApplicationQuery()) @@ -708,6 +709,14 @@ final class DifferentialRevision extends DifferentialDAO return false; } + public function getHoldAsDraft() { + return $this->getProperty(self::PROPERTY_DRAFT_HOLD, false); + } + + public function setHoldAsDraft($hold) { + return $this->setProperty(self::PROPERTY_DRAFT_HOLD, $hold); + } + public function loadActiveBuilds(PhabricatorUser $viewer) { $diff = $this->getActiveDiff(); diff --git a/src/applications/differential/xaction/DifferentialRevisionHoldDraftTransaction.php b/src/applications/differential/xaction/DifferentialRevisionHoldDraftTransaction.php new file mode 100644 index 0000000000..5bc257ab62 --- /dev/null +++ b/src/applications/differential/xaction/DifferentialRevisionHoldDraftTransaction.php @@ -0,0 +1,47 @@ +getHoldAsDraft(); + } + + public function generateNewValue($object, $value) { + return (bool)$value; + } + + public function applyInternalEffects($object, $value) { + $object->setHoldAsDraft($value); + } + + public function getTitle() { + if ($this->getNewValue()) { + return pht( + '%s held this revision as a draft.', + $this->renderAuthor()); + } else { + return pht( + '%s set this revision to automatically submit once builds complete.', + $this->renderAuthor()); + } + } + + public function getTitleForFeed() { + if ($this->getNewValue()) { + return pht( + '%s held %s as a draft.', + $this->renderAuthor(), + $this->renderObject()); + } else { + return pht( + '%s set %s to automatically submit once builds complete.', + $this->renderAuthor(), + $this->renderObject()); + } + } + +}