revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-01-10 23:01:04 +01:00
Code Issues Releases Wiki Activity

Make Harbormaster buildable status more of a nice flexible map and less of a bunch of switch statements

Browse source 
Summary: Depends on D19063. Ref T13054. Prepare for the addition of a new `PREPARING` status by getting rid of the "scattered mess of switch statements" pattern of status management.

Test Plan: Searched/browsed buildables. Viewed buildables. Viewed revisions. Grepped for all affected symbols.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13054

Differential Revision: https://secure.phabricator.com/D19064
This commit is contained in:
epriestley 2018-02-12 10:55:19 -08:00
parent c42bbd6f5c
commit f939a2b12e
10 changed files with 133 additions and 89 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/__phutil_library_map__.php
View file

@ -1272,6 +1272,7 @@ phutil_register_library_map(array(
'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 @@ phutil_register_library_map(array(
'HarbormasterBuildablePHIDType' => 'PhabricatorPHIDType',
'HarbormasterBuildableQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'HarbormasterBuildableSearchEngine' => 'PhabricatorApplicationSearchEngine',
'HarbormasterBuildableStatus' => 'Phobject',
'HarbormasterBuildableTransaction' => 'PhabricatorApplicationTransaction',
'HarbormasterBuildableTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
'HarbormasterBuildableTransactionQuery' => 'PhabricatorApplicationTransactionQuery',

4
src/applications/differential/customfield/DifferentialUnitField.php
View file

@ -44,12 +44,12 @@ final class DifferentialUnitField
->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;

7
src/applications/diffusion/view/DiffusionView.php
View file

@ -205,12 +205,11 @@ abstract class DiffusionView extends AphrontView {
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())

2
src/applications/harbormaster/conduit/HarbormasterQueryBuildablesConduitAPIMethod.php
View file

@ -65,7 +65,7 @@ final class HarbormasterQueryBuildablesConduitAPIMethod
$monogram = $buildable->getMonogram();
$status = $buildable->getBuildableStatus();
$status_name = HarbormasterBuildable::getBuildableStatusName($status);
$status_name = $buildable->getStatusDisplayName();
$data[] = array(
'id' => $buildable->getID(),

82
src/applications/harbormaster/constants/HarbormasterBuildableStatus.php Normal file
View file

@ -0,0 +1,82 @@
<?php
final class HarbormasterBuildableStatus extends Phobject {
const STATUS_BUILDING = 'building';
const STATUS_PASSED = 'passed';
const STATUS_FAILED = 'failed';
private $key;
private $properties;
public function __construct($key, array $properties) {
$this->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',
);
}
}

13
src/applications/harbormaster/engine/HarbormasterBuildEngine.php
View file

@ -453,11 +453,11 @@ final class HarbormasterBuildEngine extends Phobject {
}
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 @@ final class HarbormasterBuildEngine extends Phobject {
// 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;

10
src/applications/harbormaster/event/HarbormasterUIEventListener.php
View file

@ -87,13 +87,9 @@ final class HarbormasterUIEventListener
$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',

10
src/applications/harbormaster/query/HarbormasterBuildableSearchEngine.php
View file

@ -33,7 +33,7 @@ final class HarbormasterBuildableSearchEngine
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 @@ final class HarbormasterBuildableSearchEngine
$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);

78
src/applications/harbormaster/storage/HarbormasterBuildable.php
View file

@ -15,65 +15,10 @@ final class HarbormasterBuildable extends HarbormasterDAO
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 @@ final class HarbormasterBuildable extends HarbormasterDAO
}
/* -( 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 )------------------------- */

14
src/applications/transactions/storage/PhabricatorApplicationTransaction.php
View file

@ -512,9 +512,9 @@ abstract class PhabricatorApplicationTransaction
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 @@ abstract class PhabricatorApplicationTransaction
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 @@ abstract class PhabricatorApplicationTransaction
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 @@ abstract class PhabricatorApplicationTransaction
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),