From 79ef667dfdc8ed39e7d417b0736c71dd8a445c4f Mon Sep 17 00:00:00 2001 From: James Rhodes Date: Sat, 9 Nov 2013 15:02:07 -0800 Subject: [PATCH] Render build status on revisions and commits Summary: This uses an event listener to render the status of builds on their buildables. The revision and commit view now renders out the status of each of the builds. Currently the revision controller has the results for the latest diff rendered out. We might want to show the status of previous diffs in the future, but for now I think the latest diff should do fine. There's also a number of bug fixes in this diff, including a particularly nasty one where builds would have a build plan PHID generated for them, which resulted in handle lookups always returning invalid objects. Test Plan: Ran builds against diffs and commits, saw them appear on the revision and commit view controllers. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley CC: Korvin, epriestley, aran Maniphest Tasks: T1049 Differential Revision: https://secure.phabricator.com/D7544 --- src/__phutil_library_map__.php | 2 + .../PhabricatorApplicationHarbormaster.php | 6 + .../HarbormasterBuildViewController.php | 15 +++ .../HarbormasterBuildableViewController.php | 1 - .../event/HarbormasterUIEventListener.php | 106 ++++++++++++++++++ .../phid/HarbormasterPHIDTypeBuild.php | 8 +- .../query/HarbormasterBuildQuery.php | 35 +++--- .../step/VariableBuildStepImplementation.php | 7 +- .../storage/build/HarbormasterBuild.php | 2 +- .../worker/HarbormasterBuildWorker.php | 1 - 10 files changed, 154 insertions(+), 29 deletions(-) create mode 100644 src/applications/harbormaster/event/HarbormasterUIEventListener.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4eecc8f456..a20af3b96d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -700,6 +700,7 @@ phutil_register_library_map(array( 'HarbormasterStepAddController' => 'applications/harbormaster/controller/HarbormasterStepAddController.php', 'HarbormasterStepDeleteController' => 'applications/harbormaster/controller/HarbormasterStepDeleteController.php', 'HarbormasterStepEditController' => 'applications/harbormaster/controller/HarbormasterStepEditController.php', + 'HarbormasterUIEventListener' => 'applications/harbormaster/event/HarbormasterUIEventListener.php', 'HeraldAction' => 'applications/herald/storage/HeraldAction.php', 'HeraldAdapter' => 'applications/herald/adapter/HeraldAdapter.php', 'HeraldApplyTranscript' => 'applications/herald/storage/transcript/HeraldApplyTranscript.php', @@ -3002,6 +3003,7 @@ phutil_register_library_map(array( 'HarbormasterStepAddController' => 'HarbormasterController', 'HarbormasterStepDeleteController' => 'HarbormasterController', 'HarbormasterStepEditController' => 'HarbormasterController', + 'HarbormasterUIEventListener' => 'PhabricatorEventListener', 'HeraldAction' => 'HeraldDAO', 'HeraldApplyTranscript' => 'HeraldDAO', 'HeraldCapabilityManageGlobalRules' => 'PhabricatorPolicyCapability', diff --git a/src/applications/harbormaster/application/PhabricatorApplicationHarbormaster.php b/src/applications/harbormaster/application/PhabricatorApplicationHarbormaster.php index cbf9ed908e..122f7ca067 100644 --- a/src/applications/harbormaster/application/PhabricatorApplicationHarbormaster.php +++ b/src/applications/harbormaster/application/PhabricatorApplicationHarbormaster.php @@ -26,6 +26,12 @@ final class PhabricatorApplicationHarbormaster extends PhabricatorApplication { return self::GROUP_UTILITIES; } + public function getEventListeners() { + return array( + new HarbormasterUIEventListener(), + ); + } + public function isBeta() { return true; } diff --git a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php index 4eb5d39d3d..b5f39cbf76 100644 --- a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php @@ -187,6 +187,21 @@ final class HarbormasterBuildViewController pht('Status'), $this->getStatus($build)); + $handles = id(new PhabricatorHandleQuery()) + ->setViewer($viewer) + ->withPHIDs(array( + $build->getBuildablePHID(), + $build->getBuildPlanPHID())) + ->execute(); + + $properties->addProperty( + pht('Buildable'), + $handles[$build->getBuildablePHID()]->renderLink()); + + $properties->addProperty( + pht('Build Plan'), + $handles[$build->getBuildPlanPHID()]->renderLink()); + } private function getStatus(HarbormasterBuild $build) { diff --git a/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php index adae866dfe..c4f5291f37 100644 --- a/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php @@ -28,7 +28,6 @@ final class HarbormasterBuildableViewController $builds = id(new HarbormasterBuildQuery()) ->setViewer($viewer) ->withBuildablePHIDs(array($buildable->getPHID())) - ->needBuildPlans(true) ->execute(); $build_list = id(new PHUIObjectItemListView()) diff --git a/src/applications/harbormaster/event/HarbormasterUIEventListener.php b/src/applications/harbormaster/event/HarbormasterUIEventListener.php new file mode 100644 index 0000000000..9af5f9e0fa --- /dev/null +++ b/src/applications/harbormaster/event/HarbormasterUIEventListener.php @@ -0,0 +1,106 @@ +listen(PhabricatorEventType::TYPE_UI_WILLRENDERPROPERTIES); + } + + public function handleEvent(PhutilEvent $event) { + switch ($event->getType()) { + case PhabricatorEventType::TYPE_UI_WILLRENDERPROPERTIES: + $this->handlePropertyEvent($event); + break; + } + } + + private function handlePropertyEvent($ui_event) { + $user = $ui_event->getUser(); + $object = $ui_event->getValue('object'); + + if (!$object || !$object->getPHID()) { + // No object, or the object has no PHID yet.. + return; + } + + $target = null; + if ($object instanceof PhabricatorRepositoryCommit) { + $target = $object; + } elseif ($object instanceof DifferentialRevision) { + $target = $object->loadActiveDiff(); + } else { + return; + } + + if (!$this->canUseApplication($ui_event->getUser())) { + return; + } + + $buildables = id(new HarbormasterBuildableQuery()) + ->setViewer($user) + ->withBuildablePHIDs(array($target->getPHID())) + ->execute(); + if (!$buildables) { + return; + } + + $builds = id(new HarbormasterBuildQuery()) + ->setViewer($user) + ->withBuildablePHIDs(mpull($buildables, 'getPHID')) + ->execute(); + if (!$builds) { + return; + } + + $build_handles = id(new PhabricatorHandleQuery()) + ->setViewer($user) + ->withPHIDs(mpull($builds, 'getPHID')) + ->execute(); + + $status_view = new PHUIStatusListView(); + + foreach ($builds as $build) { + $item = new PHUIStatusItemView(); + $item->setTarget( + $build_handles[$build->getPHID()]->renderLink()); + + switch ($build->getBuildStatus()) { + case HarbormasterBuild::STATUS_INACTIVE: + $item->setIcon('open-dark', pht('Inactive')); + break; + case HarbormasterBuild::STATUS_PENDING: + $item->setIcon('open-blue', pht('Pending')); + break; + case HarbormasterBuild::STATUS_WAITING: + $item->setIcon('up-blue', pht('Waiting on Resource')); + break; + case HarbormasterBuild::STATUS_BUILDING: + $item->setIcon('right-blue', pht('Building')); + break; + case HarbormasterBuild::STATUS_PASSED: + $item->setIcon('accept-green', pht('Passed')); + break; + case HarbormasterBuild::STATUS_FAILED: + $item->setIcon('reject-red', pht('Failed')); + break; + case HarbormasterBuild::STATUS_ERROR: + $item->setIcon('minus-red', pht('Unexpected Error')); + break; + case HarbormasterBuild::STATUS_CANCELLED: + $item->setIcon('minus-dark', pht('Cancelled')); + break; + default: + $item->setIcon('question', pht('Unknown')); + break; + } + + + $status_view->addItem($item); + } + + $view = $ui_event->getValue('view'); + $view->addProperty(pht('Build Status'), $status_view); + } + +} diff --git a/src/applications/harbormaster/phid/HarbormasterPHIDTypeBuild.php b/src/applications/harbormaster/phid/HarbormasterPHIDTypeBuild.php index 1cfec404fc..22f0c8bb62 100644 --- a/src/applications/harbormaster/phid/HarbormasterPHIDTypeBuild.php +++ b/src/applications/harbormaster/phid/HarbormasterPHIDTypeBuild.php @@ -30,7 +30,13 @@ final class HarbormasterPHIDTypeBuild extends PhabricatorPHIDType { array $objects) { foreach ($handles as $phid => $handle) { - $build_plan = $objects[$phid]; + $build = $objects[$phid]; + $handles[$phid]->setName(pht( + 'Build %d: %s', + $build->getID(), + $build->getName())); + $handles[$phid]->setURI( + '/harbormaster/build/'.$build->getID()); } } diff --git a/src/applications/harbormaster/query/HarbormasterBuildQuery.php b/src/applications/harbormaster/query/HarbormasterBuildQuery.php index cc4fe92bcf..5c52e9418e 100644 --- a/src/applications/harbormaster/query/HarbormasterBuildQuery.php +++ b/src/applications/harbormaster/query/HarbormasterBuildQuery.php @@ -9,8 +9,6 @@ final class HarbormasterBuildQuery private $buildablePHIDs; private $buildPlanPHIDs; - private $needBuildPlans; - public function withIDs(array $ids) { $this->ids = $ids; return $this; @@ -36,11 +34,6 @@ final class HarbormasterBuildQuery return $this; } - public function needBuildPlans($need_plans) { - $this->needBuildPlans = $need_plans; - return $this; - } - protected function loadPage() { $table = new HarbormasterBuild(); $conn_r = $table->establishConnection('r'); @@ -82,23 +75,21 @@ final class HarbormasterBuildQuery } protected function didFilterPage(array $page) { - if ($this->needBuildPlans) { - $plans = array(); + $plans = array(); - $plan_phids = array_filter(mpull($page, 'getBuildPlanPHID')); - if ($plan_phids) { - $plans = id(new PhabricatorObjectQuery()) - ->setViewer($this->getViewer()) - ->withPHIDs($plan_phids) - ->setParentQuery($this) - ->execute(); - $plans = mpull($plans, null, 'getPHID'); - } + $plan_phids = array_filter(mpull($page, 'getBuildPlanPHID')); + if ($plan_phids) { + $plans = id(new PhabricatorObjectQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($plan_phids) + ->setParentQuery($this) + ->execute(); + $plans = mpull($plans, null, 'getPHID'); + } - foreach ($page as $key => $build) { - $plan_phid = $build->getBuildPlanPHID(); - $build->attachBuildPlan(idx($plans, $plan_phid)); - } + foreach ($page as $key => $build) { + $plan_phid = $build->getBuildPlanPHID(); + $build->attachBuildPlan(idx($plans, $plan_phid)); } return $page; diff --git a/src/applications/harbormaster/step/VariableBuildStepImplementation.php b/src/applications/harbormaster/step/VariableBuildStepImplementation.php index 5013266094..dfd85ee6fe 100644 --- a/src/applications/harbormaster/step/VariableBuildStepImplementation.php +++ b/src/applications/harbormaster/step/VariableBuildStepImplementation.php @@ -16,9 +16,10 @@ abstract class VariableBuildStepImplementation extends BuildStepImplementation { $object = $buildable->getBuildableObject(); $repo = null; - if ($object instanceof DifferentialRevision) { - $results['buildable.revision'] = $object->getID(); - $repo = $object->getRepository(); + if ($object instanceof DifferentialDiff) { + $revision = $object->getRevision(); + $results['buildable.revision'] = $revision->getID(); + $repo = $revision->getRepository(); } else if ($object instanceof PhabricatorRepositoryCommit) { $results['buildable.commit'] = $object->getCommitIdentifier(); $repo = $object->getRepository(); diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuild.php b/src/applications/harbormaster/storage/build/HarbormasterBuild.php index 8bbb64c7bb..c6d1393a05 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuild.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuild.php @@ -65,7 +65,7 @@ final class HarbormasterBuild extends HarbormasterDAO public function generatePHID() { return PhabricatorPHID::generateNewPHID( - HarbormasterPHIDTypeBuildPlan::TYPECONST); + HarbormasterPHIDTypeBuild::TYPECONST); } public function attachBuildable(HarbormasterBuildable $buildable) { diff --git a/src/applications/harbormaster/worker/HarbormasterBuildWorker.php b/src/applications/harbormaster/worker/HarbormasterBuildWorker.php index 2f8a31605d..f2c3cce6cc 100644 --- a/src/applications/harbormaster/worker/HarbormasterBuildWorker.php +++ b/src/applications/harbormaster/worker/HarbormasterBuildWorker.php @@ -18,7 +18,6 @@ final class HarbormasterBuildWorker extends PhabricatorWorker { ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withBuildStatuses(array(HarbormasterBuild::STATUS_PENDING)) ->withIDs(array($id)) - ->needBuildPlans(true) ->executeOne(); if (!$build) { throw new PhabricatorWorkerPermanentFailureException(