1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 00:42:41 +01:00

Create revisions into "Draft", publish them when builds finish

Summary:
Ref T2543. This doesn't stand alone since mail still goes out normally, but gets this piece working: new revisions start as "Draft", then after updates if there are no builds they go into "Needs Review".

This should work in general because builds update revisions when they complete, to publish a "Harbormaster finished build yada yada" transaction. So either we'll un-draft immediately, or un-draft after the last build finishes.

I'll hold this until the mail and some other stuff (like UI hints) are in slightly better shape since I think it's probably too rough on its own.

Test Plan: Created revisions locally, saw them un-draft after builds.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18628
This commit is contained in:
epriestley 2017-09-18 15:28:18 -07:00
parent fca553f142
commit 36df39761e
3 changed files with 104 additions and 4 deletions

View file

@ -107,9 +107,11 @@ final class PhabricatorConfigEditor
return parent::transactionHasEffect($object, $xaction);
}
protected function didApplyTransactions(array $xactions) {
protected function didApplyTransactions($object, array $xactions) {
// Force all the setup checks to run on the next page load.
PhabricatorSetupCheck::deleteSetupCheckCache();
return $xactions;
}
public static function storeNewValue(

View file

@ -1528,4 +1528,102 @@ final class DifferentialTransactionEditor
return array_reverse($xactions);
}
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;
if ($object->isDraft() && $auto_undraft) {
$active_builds = $this->hasActiveBuilds($object);
if (!$active_builds) {
$xaction = $object->getApplicationTransactionTemplate()
->setTransactionType(
DifferentialRevisionRequestReviewTransaction::TRANSACTIONTYPE)
->setOldValue(false)
->setNewValue(true);
$xaction = $this->populateTransaction($object, $xaction);
// If we're creating this revision and immediately moving it out of
// the draft state, mark this as a create transaction so it gets
// hidden in the timeline and mail, since it isn't interesting: it
// is as though the draft phase never happened.
if ($this->getIsNewObject()) {
$xaction->setIsCreateTransaction(true);
}
$object->openTransaction();
$object
->setStatus(DifferentialRevisionStatus::NEEDS_REVIEW)
->save();
$xaction->save();
$object->saveTransaction();
$xactions[] = $xaction;
}
}
return $xactions;
}
private function hasActiveBuilds($object) {
$viewer = $this->requireActor();
$diff = $object->getActiveDiff();
$buildables = id(new HarbormasterBuildableQuery())
->setViewer($viewer)
->withContainerPHIDs(array($object->getPHID()))
->withBuildablePHIDs(array($diff->getPHID()))
->withManualBuildables(false)
->execute();
if (!$buildables) {
return false;
}
$builds = id(new HarbormasterBuildQuery())
->setViewer($viewer)
->withBuildablePHIDs(mpull($buildables, 'getPHID'))
->withBuildStatuses(
array(
HarbormasterBuildStatus::STATUS_INACTIVE,
HarbormasterBuildStatus::STATUS_PENDING,
HarbormasterBuildStatus::STATUS_BUILDING,
HarbormasterBuildStatus::STATUS_FAILED,
HarbormasterBuildStatus::STATUS_ABORTED,
HarbormasterBuildStatus::STATUS_ERROR,
HarbormasterBuildStatus::STATUS_PAUSED,
HarbormasterBuildStatus::STATUS_DEADLOCKED,
))
->needBuildTargets(true)
->execute();
if (!$builds) {
return false;
}
$active = array();
foreach ($builds as $key => $build) {
foreach ($build->getBuildTargets() as $target) {
if ($target->isAutotarget()) {
// Ignore autotargets when looking for active of failed builds. If
// local tests fail and you continue anyway, you don't need to
// double-confirm them.
continue;
}
// This build has at least one real target that's doing something.
$active[$key] = $build;
break;
}
}
if (!$active) {
return false;
}
return true;
}
}

View file

@ -1105,7 +1105,7 @@ abstract class PhabricatorApplicationTransactionEditor
$this->heraldForcedEmailPHIDs = $adapter->getForcedEmailPHIDs();
}
$this->didApplyTransactions($xactions);
$xactions = $this->didApplyTransactions($object, $xactions);
if ($object instanceof PhabricatorCustomFieldInterface) {
// Maybe this makes more sense to move into the search index itself? For
@ -1234,9 +1234,9 @@ abstract class PhabricatorApplicationTransactionEditor
return $xactions;
}
protected function didApplyTransactions(array $xactions) {
protected function didApplyTransactions($object, array $xactions) {
// Hook for subclasses.
return;
return $xactions;
}