Page MenuHomePhabricator
Files F13994143

D19282.id46164.diff
No OneTemporary
Actions

D19282.id46164.diff
View Options

diff --git a/src/applications/differential/customfield/DifferentialDraftField.php b/src/applications/differential/customfield/DifferentialDraftField.php
--- a/src/applications/differential/customfield/DifferentialDraftField.php
+++ b/src/applications/differential/customfield/DifferentialDraftField.php
@@ -58,7 +58,7 @@
);
$blocking_map = array_fuse($blocking_map);
- $builds = $revision->loadActiveBuilds($viewer);
+ $builds = $revision->loadImpactfulBuilds($viewer);
$waiting = array();
$blocking = array();
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
@@ -1539,8 +1539,12 @@
}
if ($object->isDraft() && $auto_undraft) {
- $active_builds = $this->hasActiveBuilds($object);
- if (!$active_builds) {
+ $status = $this->loadCompletedBuildableStatus($object);
+
+ $is_passed = ($status === HarbormasterBuildableStatus::STATUS_PASSED);
+ $is_failed = ($status === HarbormasterBuildableStatus::STATUS_FAILED);
+
+ if ($is_passed) {
// 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.
@@ -1572,6 +1576,9 @@
// 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) {
+ // TODO: Change to "Changes Planned + Draft", notify the author (only)
+ // of the build failure.
}
}
@@ -1604,15 +1611,11 @@
return $xactions;
}
- private function hasActiveBuilds($object) {
+ private function loadCompletedBuildableStatus(
+ DifferentialRevision $revision) {
$viewer = $this->requireActor();
-
- $builds = $object->loadActiveBuilds($viewer);
- if (!$builds) {
- return false;
- }
-
- return true;
+ $builds = $revision->loadImpactfulBuilds($viewer);
+ return $revision->newBuildableStatusForBuilds($builds);
}
private function requireReviewers(DifferentialRevision $revision) {
diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php
--- a/src/applications/differential/storage/DifferentialRevision.php
+++ b/src/applications/differential/storage/DifferentialRevision.php
@@ -779,23 +779,14 @@
// when computing build status. Differential only cares about remote
// builds when making publishing and undrafting decisions.
- $builds = id(new HarbormasterBuildQuery())
- ->setViewer($viewer)
- ->withBuildablePHIDs(array($phid))
- ->withAutobuilds(false)
- ->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,
- ))
- ->execute();
+ $builds = $this->loadImpactfulBuildsForBuildablePHIDs(
+ $viewer,
+ array($phid));
+
+ return $this->newBuildableStatusForBuilds($builds);
+ }
+ public function newBuildableStatusForBuilds(array $builds) {
// If we have nothing but passing builds, the buildable passes.
if (!$builds) {
return HarbormasterBuildableStatus::STATUS_PASSED;
@@ -803,8 +794,7 @@
// If we have any completed, non-passing builds, the buildable fails.
foreach ($builds as $build) {
- $status = $build->getBuildStatusObject();
- if ($status->isComplete()) {
+ if ($build->isComplete()) {
return HarbormasterBuildableStatus::STATUS_FAILED;
}
}
@@ -813,7 +803,7 @@
return null;
}
- public function loadActiveBuilds(PhabricatorUser $viewer) {
+ public function loadImpactfulBuilds(PhabricatorUser $viewer) {
$diff = $this->getActiveDiff();
// NOTE: We can't use `withContainerPHIDs()` here because the container
@@ -827,9 +817,18 @@
return array();
}
+ return $this->loadImpactfulBuildsForBuildablePHIDs(
+ $viewer,
+ mpull($buildables, 'getPHID'));
+ }
+
+ private function loadImpactfulBuildsForBuildablePHIDs(
+ PhabricatorUser $viewer,
+ array $phids) {
+
return id(new HarbormasterBuildQuery())
->setViewer($viewer)
- ->withBuildablePHIDs(mpull($buildables, 'getPHID'))
+ ->withBuildablePHIDs($phids)
->withAutobuilds(false)
->withBuildStatuses(
array(

File Metadata

Mime Type
text/plain
Expires
Thu, Oct 24, 4:11 AM (3 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6733144
Default Alt Text
D19282.id46164.diff (4 KB)

Event Timeline

Phabricator · Built by Phacility