diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1272,6 +1272,7 @@ 'HarbormasterBuildablePHIDType' => 'applications/harbormaster/phid/HarbormasterBuildablePHIDType.php', 'HarbormasterBuildableQuery' => 'applications/harbormaster/query/HarbormasterBuildableQuery.php', 'HarbormasterBuildableSearchEngine' => 'applications/harbormaster/query/HarbormasterBuildableSearchEngine.php', + 'HarbormasterBuildableStatus' => 'applications/harbormaster/constants/HarbormasterBuildableStatus.php', 'HarbormasterBuildableTransaction' => 'applications/harbormaster/storage/HarbormasterBuildableTransaction.php', 'HarbormasterBuildableTransactionEditor' => 'applications/harbormaster/editor/HarbormasterBuildableTransactionEditor.php', 'HarbormasterBuildableTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildableTransactionQuery.php', @@ -6549,6 +6550,7 @@ 'HarbormasterBuildablePHIDType' => 'PhabricatorPHIDType', 'HarbormasterBuildableQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'HarbormasterBuildableSearchEngine' => 'PhabricatorApplicationSearchEngine', + 'HarbormasterBuildableStatus' => 'Phobject', 'HarbormasterBuildableTransaction' => 'PhabricatorApplicationTransaction', 'HarbormasterBuildableTransactionEditor' => 'PhabricatorApplicationTransactionEditor', 'HarbormasterBuildableTransactionQuery' => 'PhabricatorApplicationTransactionQuery', diff --git a/src/applications/differential/customfield/DifferentialUnitField.php b/src/applications/differential/customfield/DifferentialUnitField.php --- a/src/applications/differential/customfield/DifferentialUnitField.php +++ b/src/applications/differential/customfield/DifferentialUnitField.php @@ -44,12 +44,12 @@ ->executeOne(); if ($buildable) { switch ($buildable->getBuildableStatus()) { - case HarbormasterBuildable::STATUS_BUILDING: + case HarbormasterBuildableStatus::STATUS_BUILDING: $warnings[] = pht( 'These changes have not finished building yet and may have build '. 'failures.'); break; - case HarbormasterBuildable::STATUS_FAILED: + case HarbormasterBuildableStatus::STATUS_FAILED: $warnings[] = pht( 'These changes have failed to build.'); break; diff --git a/src/applications/diffusion/view/DiffusionView.php b/src/applications/diffusion/view/DiffusionView.php --- a/src/applications/diffusion/view/DiffusionView.php +++ b/src/applications/diffusion/view/DiffusionView.php @@ -205,12 +205,11 @@ final protected function renderBuildable( HarbormasterBuildable $buildable, $type = null) { - $status = $buildable->getBuildableStatus(); Javelin::initBehavior('phabricator-tooltips'); - $icon = HarbormasterBuildable::getBuildableStatusIcon($status); - $color = HarbormasterBuildable::getBuildableStatusColor($status); - $name = HarbormasterBuildable::getBuildableStatusName($status); + $icon = $buildable->getStatusIcon(); + $color = $buildable->getStatusColor(); + $name = $buildable->getStatusDisplayName(); if ($type == 'button') { return id(new PHUIButtonView()) diff --git a/src/applications/harbormaster/conduit/HarbormasterQueryBuildablesConduitAPIMethod.php b/src/applications/harbormaster/conduit/HarbormasterQueryBuildablesConduitAPIMethod.php --- a/src/applications/harbormaster/conduit/HarbormasterQueryBuildablesConduitAPIMethod.php +++ b/src/applications/harbormaster/conduit/HarbormasterQueryBuildablesConduitAPIMethod.php @@ -65,7 +65,7 @@ $monogram = $buildable->getMonogram(); $status = $buildable->getBuildableStatus(); - $status_name = HarbormasterBuildable::getBuildableStatusName($status); + $status_name = $buildable->getStatusDisplayName(); $data[] = array( 'id' => $buildable->getID(), diff --git a/src/applications/harbormaster/constants/HarbormasterBuildableStatus.php b/src/applications/harbormaster/constants/HarbormasterBuildableStatus.php new file mode 100644 --- /dev/null +++ b/src/applications/harbormaster/constants/HarbormasterBuildableStatus.php @@ -0,0 +1,82 @@ +key = $key; + $this->properties = $properties; + } + + public static function newBuildableStatusObject($status) { + $spec = self::getSpecification($status); + return new self($status, $spec); + } + + private function getProperty($key) { + if (!array_key_exists($key, $this->properties)) { + throw new Exception( + pht( + 'Attempting to access unknown buildable status property ("%s").', + $key)); + } + + return $this->properties[$key]; + } + + public function getIcon() { + return $this->getProperty('icon'); + } + + public function getDisplayName() { + return $this->getProperty('name'); + } + + public function getColor() { + return $this->getProperty('color'); + } + + public static function getOptionMap() { + return ipull(self::getSpecifications(), 'name'); + } + + private static function getSpecifications() { + return array( + self::STATUS_BUILDING => array( + 'name' => pht('Building'), + 'color' => 'blue', + 'icon' => 'fa-chevron-circle-right', + ), + self::STATUS_PASSED => array( + 'name' => pht('Passed'), + 'color' => 'green', + 'icon' => 'fa-check-circle', + ), + self::STATUS_FAILED => array( + 'name' => pht('Failed'), + 'color' => 'red', + 'icon' => 'fa-times-circle', + ), + ); + } + + private static function getSpecification($status) { + $map = self::getSpecifications(); + if (isset($map[$status])) { + return $map[$status]; + } + + return array( + 'name' => pht('Unknown ("%s")', $status), + 'icon' => 'fa-question-circle', + 'color' => 'bluegrey', + ); + } + +} diff --git a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php --- a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php +++ b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php @@ -453,11 +453,11 @@ } if ($any_fail) { - $new_status = HarbormasterBuildable::STATUS_FAILED; + $new_status = HarbormasterBuildableStatus::STATUS_FAILED; } else if ($all_pass) { - $new_status = HarbormasterBuildable::STATUS_PASSED; + $new_status = HarbormasterBuildableStatus::STATUS_PASSED; } else { - $new_status = HarbormasterBuildable::STATUS_BUILDING; + $new_status = HarbormasterBuildableStatus::STATUS_BUILDING; } $old_status = $buildable->getBuildableStatus(); @@ -477,9 +477,10 @@ // can look at the results themselves, and other users generally don't // care about the outcome. - $should_publish = $did_update && - $new_status != HarbormasterBuildable::STATUS_BUILDING && - !$buildable->getIsManualBuildable(); + $should_publish = + ($did_update) && + ($new_status != HarbormasterBuildableStatus::STATUS_BUILDING) && + (!$buildable->getIsManualBuildable()); if (!$should_publish) { return; diff --git a/src/applications/harbormaster/event/HarbormasterUIEventListener.php b/src/applications/harbormaster/event/HarbormasterUIEventListener.php --- a/src/applications/harbormaster/event/HarbormasterUIEventListener.php +++ b/src/applications/harbormaster/event/HarbormasterUIEventListener.php @@ -87,13 +87,9 @@ $status_view = new PHUIStatusListView(); - $buildable_status = $buildable->getBuildableStatus(); - $buildable_icon = HarbormasterBuildable::getBuildableStatusIcon( - $buildable_status); - $buildable_color = HarbormasterBuildable::getBuildableStatusColor( - $buildable_status); - $buildable_name = HarbormasterBuildable::getBuildableStatusName( - $buildable_status); + $buildable_icon = $buildable->getStatusIcon(); + $buildable_color = $buildable->getStatusColor(); + $buildable_name = $buildable->getStatusDisplayName(); $target = phutil_tag( 'a', diff --git a/src/applications/harbormaster/query/HarbormasterBuildableSearchEngine.php b/src/applications/harbormaster/query/HarbormasterBuildableSearchEngine.php --- a/src/applications/harbormaster/query/HarbormasterBuildableSearchEngine.php +++ b/src/applications/harbormaster/query/HarbormasterBuildableSearchEngine.php @@ -33,7 +33,7 @@ id(new PhabricatorSearchCheckboxesField()) ->setKey('statuses') ->setLabel(pht('Statuses')) - ->setOptions(HarbormasterBuildable::getBuildStatusMap()) + ->setOptions(HarbormasterBuildableStatus::getOptionMap()) ->setDescription(pht('Search for builds by buildable status.')), id(new PhabricatorSearchThreeStateField()) ->setLabel(pht('Manual')) @@ -169,11 +169,9 @@ $item->addIcon('fa-wrench grey', pht('Manual')); } - $status = $buildable->getBuildableStatus(); - - $status_icon = HarbormasterBuildable::getBuildableStatusIcon($status); - $status_color = HarbormasterBuildable::getBuildableStatusColor($status); - $status_label = HarbormasterBuildable::getBuildableStatusName($status); + $status_icon = $buildable->getStatusIcon(); + $status_color = $buildable->getStatusColor(); + $status_label = $buildable->getStatusDisplayName(); $item->setStatusIcon("{$status_icon} {$status_color}", $status_label); diff --git a/src/applications/harbormaster/storage/HarbormasterBuildable.php b/src/applications/harbormaster/storage/HarbormasterBuildable.php --- a/src/applications/harbormaster/storage/HarbormasterBuildable.php +++ b/src/applications/harbormaster/storage/HarbormasterBuildable.php @@ -15,65 +15,10 @@ private $containerObject = self::ATTACHABLE; private $builds = self::ATTACHABLE; - const STATUS_BUILDING = 'building'; - const STATUS_PASSED = 'passed'; - const STATUS_FAILED = 'failed'; - - public static function getBuildableStatusName($status) { - $map = self::getBuildStatusMap(); - return idx($map, $status, pht('Unknown ("%s")', $status)); - } - - public static function getBuildStatusMap() { - return array( - self::STATUS_BUILDING => pht('Building'), - self::STATUS_PASSED => pht('Passed'), - self::STATUS_FAILED => pht('Failed'), - ); - } - - public static function getBuildableStatusIcon($status) { - switch ($status) { - case self::STATUS_BUILDING: - return PHUIStatusItemView::ICON_RIGHT; - case self::STATUS_PASSED: - return PHUIStatusItemView::ICON_ACCEPT; - case self::STATUS_FAILED: - return PHUIStatusItemView::ICON_REJECT; - default: - return PHUIStatusItemView::ICON_QUESTION; - } - } - - public static function getBuildableStatusColor($status) { - switch ($status) { - case self::STATUS_BUILDING: - return 'blue'; - case self::STATUS_PASSED: - return 'green'; - case self::STATUS_FAILED: - return 'red'; - default: - return 'bluegrey'; - } - } - - public function getStatusIcon() { - return self::getBuildableStatusIcon($this->getBuildableStatus()); - } - - public function getStatusDisplayName() { - return self::getBuildableStatusName($this->getBuildableStatus()); - } - - public function getStatusColor() { - return self::getBuildableStatusColor($this->getBuildableStatus()); - } - public static function initializeNewBuildable(PhabricatorUser $actor) { return id(new HarbormasterBuildable()) ->setIsManualBuildable(0) - ->setBuildableStatus(self::STATUS_BUILDING); + ->setBuildableStatus(HarbormasterBuildableStatus::STATUS_BUILDING); } public function getMonogram() { @@ -262,6 +207,27 @@ } +/* -( Status )------------------------------------------------------------- */ + + + public function getBuildableStatusObject() { + $status = $this->getBuildableStatus(); + return HarbormasterBuildableStatus::newBuildableStatusObject($status); + } + + public function getStatusIcon() { + return $this->getBuildableStatusObject()->getIcon(); + } + + public function getStatusDisplayName() { + return $this->getBuildableStatusObject()->getDisplayName(); + } + + public function getStatusColor() { + return $this->getBuildableStatusObject()->getColor(); + } + + /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -512,9 +512,9 @@ break; case PhabricatorTransactions::TYPE_BUILDABLE: switch ($this->getNewValue()) { - case HarbormasterBuildable::STATUS_PASSED: + case HarbormasterBuildableStatus::STATUS_PASSED: return 'green'; - case HarbormasterBuildable::STATUS_FAILED: + case HarbormasterBuildableStatus::STATUS_FAILED: return 'red'; } break; @@ -676,7 +676,7 @@ return true; case PhabricatorTransactions::TYPE_BUILDABLE: switch ($this->getNewValue()) { - case HarbormasterBuildable::STATUS_FAILED: + case HarbormasterBuildableStatus::STATUS_FAILED: // For now, only ever send mail when builds fail. We might let // you customize this later, but in most cases this is probably // completely uninteresting. @@ -739,7 +739,7 @@ return true; case PhabricatorTransactions::TYPE_BUILDABLE: switch ($this->getNewValue()) { - case HarbormasterBuildable::STATUS_FAILED: + case HarbormasterBuildableStatus::STATUS_FAILED: // For now, don't notify on build passes either. These are pretty // high volume and annoying, with very little present value. We // might want to turn them back on in the specific case of @@ -1024,19 +1024,19 @@ case PhabricatorTransactions::TYPE_BUILDABLE: switch ($this->getNewValue()) { - case HarbormasterBuildable::STATUS_BUILDING: + case HarbormasterBuildableStatus::STATUS_BUILDING: return pht( '%s started building %s.', $this->renderHandleLink($author_phid), $this->renderHandleLink( $this->getMetadataValue('harbormaster:buildablePHID'))); - case HarbormasterBuildable::STATUS_PASSED: + case HarbormasterBuildableStatus::STATUS_PASSED: return pht( '%s completed building %s.', $this->renderHandleLink($author_phid), $this->renderHandleLink( $this->getMetadataValue('harbormaster:buildablePHID'))); - case HarbormasterBuildable::STATUS_FAILED: + case HarbormasterBuildableStatus::STATUS_FAILED: return pht( '%s failed to build %s!', $this->renderHandleLink($author_phid),