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

Provide a modular buildable transaction in Diffusion

Summary:
Depends on D19279. Ref T13110. This implements the existing publishing logic for buildables, but does so via ModularTransactions instead of a core transaction type.

Since each application is implementing build transactions independently, this removes the core type.

Next, Differential will get a similar treatment.

Test Plan: Used `bin/harbormaster publish` (with some commenting-out-guard-clauses) to publish a commit Buildable; saw unchanged feed behavior.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13110

Differential Revision: https://secure.phabricator.com/D19280
This commit is contained in:
epriestley 2018-04-03 06:41:00 -07:00
parent c20b4e365b
commit ada0c9126c
11 changed files with 159 additions and 105 deletions

View file

@ -659,6 +659,7 @@ phutil_register_library_map(array(
'DiffusionCommitAutocloseHeraldField' => 'applications/diffusion/herald/DiffusionCommitAutocloseHeraldField.php',
'DiffusionCommitBranchesController' => 'applications/diffusion/controller/DiffusionCommitBranchesController.php',
'DiffusionCommitBranchesHeraldField' => 'applications/diffusion/herald/DiffusionCommitBranchesHeraldField.php',
'DiffusionCommitBuildableTransaction' => 'applications/diffusion/xaction/DiffusionCommitBuildableTransaction.php',
'DiffusionCommitCommitterHeraldField' => 'applications/diffusion/herald/DiffusionCommitCommitterHeraldField.php',
'DiffusionCommitCommitterProjectsHeraldField' => 'applications/diffusion/herald/DiffusionCommitCommitterProjectsHeraldField.php',
'DiffusionCommitConcernTransaction' => 'applications/diffusion/xaction/DiffusionCommitConcernTransaction.php',
@ -5908,6 +5909,7 @@ phutil_register_library_map(array(
'DiffusionCommitAutocloseHeraldField' => 'DiffusionCommitHeraldField',
'DiffusionCommitBranchesController' => 'DiffusionController',
'DiffusionCommitBranchesHeraldField' => 'DiffusionCommitHeraldField',
'DiffusionCommitBuildableTransaction' => 'DiffusionCommitTransactionType',
'DiffusionCommitCommitterHeraldField' => 'DiffusionCommitHeraldField',
'DiffusionCommitCommitterProjectsHeraldField' => 'DiffusionCommitHeraldField',
'DiffusionCommitConcernTransaction' => 'DiffusionCommitAuditTransaction',

View file

@ -1,4 +1,37 @@
<?php
final class DiffusionBuildableEngine
extends HarbormasterBuildableEngine {}
extends HarbormasterBuildableEngine {
public function publishBuildable(
HarbormasterBuildable $old,
HarbormasterBuildable $new) {
// Don't publish manual buildables.
if ($new->getIsManualBuildable()) {
return;
}
// Don't publish anything if the buildable status has not changed. At
// least for now, Diffusion handles buildable status exactly the same
// way that Harbormaster does.
$old_status = $old->getBuildableStatus();
$new_status = $new->getBuildableStatus();
if ($old_status === $new_status) {
return;
}
// Don't publish anything if the buildable is still building.
if ($new->isBuilding()) {
return;
}
$xaction = $this->newTransaction()
->setMetadataValue('harbormaster:buildablePHID', $new->getPHID())
->setTransactionType(DiffusionCommitBuildableTransaction::TRANSACTIONTYPE)
->setNewValue($new->getBuildableStatus());
$this->applyTransactions(array($xaction));
}
}

View file

@ -0,0 +1,89 @@
<?php
final class DiffusionCommitBuildableTransaction
extends DiffusionCommitTransactionType {
// NOTE: This uses an older constant for compatibility. We should perhaps
// migrate these at some point.
const TRANSACTIONTYPE = 'harbormaster:buildable';
public function generateNewValue($object, $value) {
return $value;
}
public function generateOldValue($object) {
return null;
}
public function getIcon() {
return $this->newBuildableStatus()->getIcon();
}
public function getColor() {
return $this->newBuildableStatus()->getColor();
}
public function getActionName() {
return $this->newBuildableStatus()->getActionName();
}
public function shouldHideForFeed() {
return !$this->newBuildableStatus()->isFailed();
}
public function shouldHideForMail() {
return !$this->newBuildableStatus()->isFailed();
}
public function getTitle() {
$new = $this->getNewValue();
$buildable_phid = $this->getBuildablePHID();
switch ($new) {
case HarbormasterBuildableStatus::STATUS_PASSED:
return pht(
'%s completed building %s.',
$this->renderAuthor(),
$this->renderHandle($buildable_phid));
case HarbormasterBuildableStatus::STATUS_FAILED:
return pht(
'%s failed to build %s!',
$this->renderAuthor(),
$this->renderHandle($buildable_phid));
}
return null;
}
public function getTitleForFeed() {
$new = $this->getNewValue();
$buildable_phid = $this->getBuildablePHID();
switch ($new) {
case HarbormasterBuildableStatus::STATUS_PASSED:
return pht(
'%s completed building %s for %s.',
$this->renderAuthor(),
$this->renderHandle($buildable_phid),
$this->renderObject());
case HarbormasterBuildableStatus::STATUS_FAILED:
return pht(
'%s failed to build %s for %s!',
$this->renderAuthor(),
$this->renderHandle($buildable_phid),
$this->renderObject());
}
return null;
}
private function newBuildableStatus() {
$new = $this->getNewValue();
return HarbormasterBuildableStatus::newBuildableStatusObject($new);
}
private function getBuildablePHID() {
return $this->getMetadataValue('harbormaster:buildablePHID');
}
}

View file

@ -39,6 +39,10 @@ final class HarbormasterBuildableStatus extends Phobject {
return $this->getProperty('name');
}
public function getActionName() {
return $this->getProperty('name.action');
}
public function getColor() {
return $this->getProperty('color');
}
@ -47,6 +51,14 @@ final class HarbormasterBuildableStatus extends Phobject {
return ($this->key === self::STATUS_PREPARING);
}
public function isBuilding() {
return ($this->key === self::STATUS_BUILDING);
}
public function isFailed() {
return ($this->key === self::STATUS_FAILED);
}
public static function getOptionMap() {
return ipull(self::getSpecifications(), 'name');
}
@ -57,21 +69,25 @@ final class HarbormasterBuildableStatus extends Phobject {
'name' => pht('Preparing'),
'color' => 'blue',
'icon' => 'fa-hourglass-o',
'name.action' => pht('Build Preparing'),
),
self::STATUS_BUILDING => array(
'name' => pht('Building'),
'color' => 'blue',
'icon' => 'fa-chevron-circle-right',
'name.action' => pht('Build Started'),
),
self::STATUS_PASSED => array(
'name' => pht('Passed'),
'color' => 'green',
'icon' => 'fa-check-circle',
'name.action' => pht('Build Passed'),
),
self::STATUS_FAILED => array(
'name' => pht('Failed'),
'color' => 'red',
'icon' => 'fa-times-circle',
'name.action' => pht('Build Failed'),
),
);
}
@ -86,6 +102,7 @@ final class HarbormasterBuildableStatus extends Phobject {
'name' => pht('Unknown ("%s")', $status),
'icon' => 'fa-question-circle',
'color' => 'bluegrey',
'name.action' => pht('Build Status'),
);
}

View file

@ -45,7 +45,7 @@ abstract class HarbormasterBuildableEngine
return $this->object;
}
final public function publishBuildable(
public function publishBuildable(
HarbormasterBuildable $old,
HarbormasterBuildable $new) {
return;

View file

@ -233,6 +233,10 @@ final class HarbormasterBuildable
return $this->getBuildableStatusObject()->isPreparing();
}
public function isBuilding() {
return $this->getBuildableStatusObject()->isBuilding();
}
/* -( Messages )----------------------------------------------------------- */

View file

@ -9,7 +9,6 @@ final class PhabricatorTransactions extends Phobject {
const TYPE_JOIN_POLICY = 'core:join-policy';
const TYPE_EDGE = 'core:edge';
const TYPE_CUSTOMFIELD = 'core:customfield';
const TYPE_BUILDABLE = 'harbormaster:buildable';
const TYPE_TOKEN = 'token:give';
const TYPE_INLINESTATE = 'core:inlinestate';
const TYPE_SPACE = 'core:space';

View file

@ -313,10 +313,6 @@ abstract class PhabricatorApplicationTransactionEditor
$types[] = PhabricatorTransactions::TYPE_CUSTOMFIELD;
}
if ($this->object instanceof HarbormasterBuildableInterface) {
$types[] = PhabricatorTransactions::TYPE_BUILDABLE;
}
if ($this->object instanceof PhabricatorTokenReceiverInterface) {
$types[] = PhabricatorTransactions::TYPE_TOKEN;
}
@ -469,7 +465,6 @@ abstract class PhabricatorApplicationTransactionEditor
case PhabricatorTransactions::TYPE_VIEW_POLICY:
case PhabricatorTransactions::TYPE_EDIT_POLICY:
case PhabricatorTransactions::TYPE_JOIN_POLICY:
case PhabricatorTransactions::TYPE_BUILDABLE:
case PhabricatorTransactions::TYPE_TOKEN:
case PhabricatorTransactions::TYPE_INLINESTATE:
case PhabricatorTransactions::TYPE_SUBTYPE:
@ -610,7 +605,6 @@ abstract class PhabricatorApplicationTransactionEditor
return $field->applyApplicationTransactionInternalEffects($xaction);
case PhabricatorTransactions::TYPE_CREATE:
case PhabricatorTransactions::TYPE_SUBTYPE:
case PhabricatorTransactions::TYPE_BUILDABLE:
case PhabricatorTransactions::TYPE_TOKEN:
case PhabricatorTransactions::TYPE_VIEW_POLICY:
case PhabricatorTransactions::TYPE_EDIT_POLICY:
@ -673,7 +667,6 @@ abstract class PhabricatorApplicationTransactionEditor
case PhabricatorTransactions::TYPE_CREATE:
case PhabricatorTransactions::TYPE_SUBTYPE:
case PhabricatorTransactions::TYPE_EDGE:
case PhabricatorTransactions::TYPE_BUILDABLE:
case PhabricatorTransactions::TYPE_TOKEN:
case PhabricatorTransactions::TYPE_VIEW_POLICY:
case PhabricatorTransactions::TYPE_EDIT_POLICY:

View file

@ -54,7 +54,6 @@ abstract class PhabricatorApplicationTransaction
public function shouldGenerateOldValue() {
switch ($this->getTransactionType()) {
case PhabricatorTransactions::TYPE_BUILDABLE:
case PhabricatorTransactions::TYPE_TOKEN:
case PhabricatorTransactions::TYPE_CUSTOMFIELD:
case PhabricatorTransactions::TYPE_INLINESTATE:
@ -339,12 +338,6 @@ abstract class PhabricatorApplicationTransaction
break;
case PhabricatorTransactions::TYPE_TOKEN:
break;
case PhabricatorTransactions::TYPE_BUILDABLE:
$phid = $this->getMetadataValue('harbormaster:buildablePHID');
if ($phid) {
$phids[] = array($phid);
}
break;
}
if ($this->getComment()) {
@ -470,8 +463,6 @@ abstract class PhabricatorApplicationTransaction
return 'fa-ambulance';
}
return 'fa-link';
case PhabricatorTransactions::TYPE_BUILDABLE:
return 'fa-wrench';
case PhabricatorTransactions::TYPE_TOKEN:
return 'fa-trophy';
case PhabricatorTransactions::TYPE_SPACE:
@ -515,14 +506,6 @@ abstract class PhabricatorApplicationTransaction
return 'sky';
}
break;
case PhabricatorTransactions::TYPE_BUILDABLE:
switch ($this->getNewValue()) {
case HarbormasterBuildableStatus::STATUS_PASSED:
return 'green';
case HarbormasterBuildableStatus::STATUS_FAILED:
return 'red';
}
break;
}
return null;
}
@ -679,15 +662,6 @@ abstract class PhabricatorApplicationTransaction
switch ($this->getTransactionType()) {
case PhabricatorTransactions::TYPE_TOKEN:
return true;
case PhabricatorTransactions::TYPE_BUILDABLE:
switch ($this->getNewValue()) {
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.
return false;
}
return true;
case PhabricatorTransactions::TYPE_EDGE:
$edge_type = $this->getMetadataValue('edge:type');
switch ($edge_type) {
@ -742,16 +716,6 @@ abstract class PhabricatorApplicationTransaction
switch ($this->getTransactionType()) {
case PhabricatorTransactions::TYPE_TOKEN:
return true;
case PhabricatorTransactions::TYPE_BUILDABLE:
switch ($this->getNewValue()) {
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
// build successes on the current document?
return false;
}
return true;
case PhabricatorTransactions::TYPE_EDGE:
$edge_type = $this->getMetadataValue('edge:type');
switch ($edge_type) {
@ -1027,30 +991,6 @@ abstract class PhabricatorApplicationTransaction
$this->renderHandleLink($author_phid));
}
case PhabricatorTransactions::TYPE_BUILDABLE:
switch ($this->getNewValue()) {
case HarbormasterBuildableStatus::STATUS_BUILDING:
return pht(
'%s started building %s.',
$this->renderHandleLink($author_phid),
$this->renderHandleLink(
$this->getMetadataValue('harbormaster:buildablePHID')));
case HarbormasterBuildableStatus::STATUS_PASSED:
return pht(
'%s completed building %s.',
$this->renderHandleLink($author_phid),
$this->renderHandleLink(
$this->getMetadataValue('harbormaster:buildablePHID')));
case HarbormasterBuildableStatus::STATUS_FAILED:
return pht(
'%s failed to build %s!',
$this->renderHandleLink($author_phid),
$this->renderHandleLink(
$this->getMetadataValue('harbormaster:buildablePHID')));
default:
return null;
}
case PhabricatorTransactions::TYPE_INLINESTATE:
$done = 0;
$undone = 0;
@ -1239,32 +1179,6 @@ abstract class PhabricatorApplicationTransaction
$this->renderHandleLink($author_phid),
$this->renderHandleLink($object_phid));
}
case PhabricatorTransactions::TYPE_BUILDABLE:
switch ($this->getNewValue()) {
case HarbormasterBuildableStatus::STATUS_BUILDING:
return pht(
'%s started building %s for %s.',
$this->renderHandleLink($author_phid),
$this->renderHandleLink(
$this->getMetadataValue('harbormaster:buildablePHID')),
$this->renderHandleLink($object_phid));
case HarbormasterBuildableStatus::STATUS_PASSED:
return pht(
'%s completed building %s for %s.',
$this->renderHandleLink($author_phid),
$this->renderHandleLink(
$this->getMetadataValue('harbormaster:buildablePHID')),
$this->renderHandleLink($object_phid));
case HarbormasterBuildableStatus::STATUS_FAILED:
return pht(
'%s failed to build %s for %s.',
$this->renderHandleLink($author_phid),
$this->renderHandleLink(
$this->getMetadataValue('harbormaster:buildablePHID')),
$this->renderHandleLink($object_phid));
default:
return null;
}
case PhabricatorTransactions::TYPE_COLUMNS:
$moves = $this->getInterestingMoves($new);
@ -1421,15 +1335,6 @@ abstract class PhabricatorApplicationTransaction
return pht('Changed Policy');
case PhabricatorTransactions::TYPE_SUBSCRIBERS:
return pht('Changed Subscribers');
case PhabricatorTransactions::TYPE_BUILDABLE:
switch ($this->getNewValue()) {
case HarbormasterBuildableStatus::STATUS_PASSED:
return pht('Build Passed');
case HarbormasterBuildableStatus::STATUS_FAILED:
return pht('Build Failed');
default:
return pht('Build Status');
}
default:
return pht('Updated');
}

View file

@ -100,6 +100,14 @@ abstract class PhabricatorModularTransaction
return parent::shouldHideForFeed();
}
/* final */ public function shouldHideForMail(array $xactions) {
if ($this->getTransactionImplementation()->shouldHideForMail()) {
return true;
}
return parent::shouldHideForMail($xactions);
}
/* final */ public function getIcon() {
$icon = $this->getTransactionImplementation()->getIcon();
if ($icon !== null) {

View file

@ -51,6 +51,10 @@ abstract class PhabricatorModularTransactionType
return false;
}
public function shouldHideForMail() {
return false;
}
public function getIcon() {
return null;
}