Skip to content

Commit 28cec2f

Browse files
author
epriestley
committed
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
1 parent f5336cd commit 28cec2f

File tree

6 files changed

+81
-1
lines changed

6 files changed

+81
-1
lines changed

src/__phutil_library_map__.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,7 @@
550550
'DifferentialRevisionHasTaskRelationship' => 'applications/differential/relationships/DifferentialRevisionHasTaskRelationship.php',
551551
'DifferentialRevisionHeraldField' => 'applications/differential/herald/DifferentialRevisionHeraldField.php',
552552
'DifferentialRevisionHeraldFieldGroup' => 'applications/differential/herald/DifferentialRevisionHeraldFieldGroup.php',
553+
'DifferentialRevisionHoldDraftTransaction' => 'applications/differential/xaction/DifferentialRevisionHoldDraftTransaction.php',
553554
'DifferentialRevisionIDCommitMessageField' => 'applications/differential/field/DifferentialRevisionIDCommitMessageField.php',
554555
'DifferentialRevisionInlineTransaction' => 'applications/differential/xaction/DifferentialRevisionInlineTransaction.php',
555556
'DifferentialRevisionInlinesController' => 'applications/differential/controller/DifferentialRevisionInlinesController.php',
@@ -5592,6 +5593,7 @@
55925593
'DifferentialRevisionHasTaskRelationship' => 'DifferentialRevisionRelationship',
55935594
'DifferentialRevisionHeraldField' => 'HeraldField',
55945595
'DifferentialRevisionHeraldFieldGroup' => 'HeraldFieldGroup',
5596+
'DifferentialRevisionHoldDraftTransaction' => 'DifferentialRevisionTransactionType',
55955597
'DifferentialRevisionIDCommitMessageField' => 'DifferentialCommitMessageField',
55965598
'DifferentialRevisionInlineTransaction' => 'PhabricatorModularTransactionType',
55975599
'DifferentialRevisionInlinesController' => 'DifferentialController',

src/applications/differential/customfield/DifferentialDraftField.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ public function getWarningsForRevisionHeader(array $handles) {
3636
return array();
3737
}
3838

39+
// If the author has held this revision as a draft explicitly, don't
40+
// show any misleading messages about it autosubmitting later.
41+
if ($revision->getHoldAsDraft()) {
42+
return array();
43+
}
44+
3945
$warnings = array();
4046

4147
$blocking_map = array(

src/applications/differential/editor/DifferentialRevisionEditEngine.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,22 @@ protected function buildCustomEditFields($object) {
235235
$fields[] = $action->newEditField($object, $viewer);
236236
}
237237

238+
$fields[] = id(new PhabricatorBoolEditField())
239+
->setKey('draft')
240+
->setLabel(pht('Hold as Draft'))
241+
->setIsConduitOnly(true)
242+
->setOptions(
243+
pht('Autosubmit Once Builds Finish'),
244+
pht('Hold as Draft'))
245+
->setTransactionType(
246+
DifferentialRevisionHoldDraftTransaction::TRANSACTIONTYPE)
247+
->setDescription(pht('Hold revision as as draft.'))
248+
->setConduitDescription(
249+
pht(
250+
'Change autosubmission from draft state after builds finish.'))
251+
->setConduitTypeDescription(pht('New "Hold as Draft" setting.'))
252+
->setValue($object->getHoldAsDraft());
253+
238254
return $fields;
239255
}
240256

src/applications/differential/editor/DifferentialTransactionEditor.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1540,7 +1540,7 @@ private function loadUnbroadcastTransactions($object) {
15401540
protected function didApplyTransactions($object, array $xactions) {
15411541
// If a draft revision has no outstanding builds and we're automatically
15421542
// making drafts public after builds finish, make the revision public.
1543-
$auto_undraft = true;
1543+
$auto_undraft = !$object->getHoldAsDraft();
15441544

15451545
if ($object->isDraft() && $auto_undraft) {
15461546
$active_builds = $this->hasActiveBuilds($object);

src/applications/differential/storage/DifferentialRevision.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ final class DifferentialRevision extends DifferentialDAO
5757
const RELATION_SUBSCRIBED = 'subd';
5858

5959
const PROPERTY_CLOSED_FROM_ACCEPTED = 'wasAcceptedBeforeClose';
60+
const PROPERTY_DRAFT_HOLD = 'draft.hold';
6061

6162
public static function initializeNewRevision(PhabricatorUser $actor) {
6263
$app = id(new PhabricatorApplicationQuery())
@@ -708,6 +709,14 @@ public function shouldBroadcast() {
708709
return false;
709710
}
710711

712+
public function getHoldAsDraft() {
713+
return $this->getProperty(self::PROPERTY_DRAFT_HOLD, false);
714+
}
715+
716+
public function setHoldAsDraft($hold) {
717+
return $this->setProperty(self::PROPERTY_DRAFT_HOLD, $hold);
718+
}
719+
711720
public function loadActiveBuilds(PhabricatorUser $viewer) {
712721
$diff = $this->getActiveDiff();
713722

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
final class DifferentialRevisionHoldDraftTransaction
4+
extends DifferentialRevisionTransactionType {
5+
6+
const TRANSACTIONTYPE = 'draft';
7+
const EDITKEY = 'draft';
8+
9+
public function generateOldValue($object) {
10+
return (bool)$object->getHoldAsDraft();
11+
}
12+
13+
public function generateNewValue($object, $value) {
14+
return (bool)$value;
15+
}
16+
17+
public function applyInternalEffects($object, $value) {
18+
$object->setHoldAsDraft($value);
19+
}
20+
21+
public function getTitle() {
22+
if ($this->getNewValue()) {
23+
return pht(
24+
'%s held this revision as a draft.',
25+
$this->renderAuthor());
26+
} else {
27+
return pht(
28+
'%s set this revision to automatically submit once builds complete.',
29+
$this->renderAuthor());
30+
}
31+
}
32+
33+
public function getTitleForFeed() {
34+
if ($this->getNewValue()) {
35+
return pht(
36+
'%s held %s as a draft.',
37+
$this->renderAuthor(),
38+
$this->renderObject());
39+
} else {
40+
return pht(
41+
'%s set %s to automatically submit once builds complete.',
42+
$this->renderAuthor(),
43+
$this->renderObject());
44+
}
45+
}
46+
47+
}

0 commit comments

Comments
 (0)