Page MenuHomePhabricator
Files F15383747

D19380.id46360.diff
No OneTemporary
Actions

D19380.id46360.diff
View Options

diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -1555,13 +1555,41 @@
$auto_undraft = false;
}
- if ($object->isDraft() && $auto_undraft) {
+ $can_promote = false;
+ $can_demote = false;
+
+ // "Draft" revisions can promote to "Review Requested" after builds pass,
+ // or demote to "Changes Planned" after builds fail.
+ if ($object->isDraft()) {
+ $can_promote = true;
+ $can_demote = true;
+ }
+
+ // See PHI584. "Changes Planned" revisions which are not yet broadcasting
+ // can promote to "Review Requested" if builds pass.
+
+ // This pass is presumably the result of someone restarting the builds and
+ // having them work this time, perhaps because the builds are not perfectly
+ // reliable or perhaps because someone fixed some issue with build hardware
+ // or some other dependency.
+
+ // Currently, there's no legitimate way to end up in this state except
+ // through automatic demotion, so this behavior should not generate an
+ // undue level of confusion or ambiguity. Also note that these changes can
+ // not demote again since they've already been demoted once.
+ if ($object->isChangePlanned()) {
+ if (!$object->getShouldBroadcast()) {
+ $can_promote = true;
+ }
+ }
+
+ if (($can_promote || $can_demote) && $auto_undraft) {
$status = $this->loadCompletedBuildableStatus($object);
$is_passed = ($status === HarbormasterBuildableStatus::STATUS_PASSED);
$is_failed = ($status === HarbormasterBuildableStatus::STATUS_FAILED);
- if ($is_passed) {
+ if ($is_passed && $can_promote) {
// When Harbormaster moves a revision out of the draft state, we
// attribute the action to the revision author since this is more
// natural and more useful.
@@ -1593,7 +1621,7 @@
// batch of transactions finishes so that Herald can fire on the new
// revision state. See T13027 for discussion.
$this->queueTransaction($xaction);
- } else if ($is_failed) {
+ } else if ($is_failed && $can_demote) {
// When demoting a revision, we act as "Harbormaster" instead of
// the author since this feels a little more natural.
$harbormaster_phid = id(new PhabricatorHarbormasterApplication())
@@ -1607,8 +1635,6 @@
->setNewValue(true);
$this->queueTransaction($xaction);
-
- // TODO: Notify the author (only) that we did this.
}
}

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 15, 5:46 PM (2 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7328581
Default Alt Text
D19380.id46360.diff (2 KB)

Event Timeline

Phabricator · Built by Phacility