mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-30 02:32:42 +01:00
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
This commit is contained in:
parent
d9bd36039f
commit
adf8fdef0e
3 changed files with 46 additions and 9 deletions
|
@ -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.
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -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) {
|
||||
|
|
|
@ -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');
|
||||
}
|
||||
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue