Skip to content

Commit

Permalink
Modularize "add projects" and "remove projects" Herald actions
Browse files Browse the repository at this point in the history
Summary: Ref T8726. Convert these to be modular.

Test Plan:
  - Created rules using these actions.
  - Upgraded them.
  - Verified they still work.

{F659266}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: joshuaspence, epriestley

Maniphest Tasks: T8726

Differential Revision: https://secure.phabricator.com/D13705
  • Loading branch information
epriestley committed Aug 3, 2015
1 parent 51fead1 commit 3782992
Show file tree
Hide file tree
Showing 7 changed files with 273 additions and 58 deletions.
11 changes: 11 additions & 0 deletions resources/sql/autopatches/20150724.herald.3.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
UPDATE {$NAMESPACE}_herald.herald_actionrecord a
JOIN {$NAMESPACE}_herald.herald_rule r
ON a.ruleID = r.id
SET a.action = 'projects.add'
WHERE a.action = 'addprojects';

UPDATE {$NAMESPACE}_herald.herald_actionrecord a
JOIN {$NAMESPACE}_herald.herald_rule r
ON a.ruleID = r.id
SET a.action = 'projects.remove'
WHERE a.action = 'removeprojects';
6 changes: 6 additions & 0 deletions src/__phutil_library_map__.php
Original file line number Diff line number Diff line change
Expand Up @@ -2540,6 +2540,7 @@
'PhabricatorPolicyType' => 'applications/policy/constants/PhabricatorPolicyType.php',
'PhabricatorPonderApplication' => 'applications/ponder/application/PhabricatorPonderApplication.php',
'PhabricatorProject' => 'applications/project/storage/PhabricatorProject.php',
'PhabricatorProjectAddHeraldAction' => 'applications/project/herald/PhabricatorProjectAddHeraldAction.php',
'PhabricatorProjectApplication' => 'applications/project/application/PhabricatorProjectApplication.php',
'PhabricatorProjectArchiveController' => 'applications/project/controller/PhabricatorProjectArchiveController.php',
'PhabricatorProjectBoardController' => 'applications/project/controller/PhabricatorProjectBoardController.php',
Expand Down Expand Up @@ -2572,6 +2573,7 @@
'PhabricatorProjectEditPictureController' => 'applications/project/controller/PhabricatorProjectEditPictureController.php',
'PhabricatorProjectEditorTestCase' => 'applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php',
'PhabricatorProjectFeedController' => 'applications/project/controller/PhabricatorProjectFeedController.php',
'PhabricatorProjectHeraldAction' => 'applications/project/herald/PhabricatorProjectHeraldAction.php',
'PhabricatorProjectIcon' => 'applications/project/icon/PhabricatorProjectIcon.php',
'PhabricatorProjectInterface' => 'applications/project/interface/PhabricatorProjectInterface.php',
'PhabricatorProjectListController' => 'applications/project/controller/PhabricatorProjectListController.php',
Expand All @@ -2593,6 +2595,7 @@
'PhabricatorProjectProjectHasObjectEdgeType' => 'applications/project/edge/PhabricatorProjectProjectHasObjectEdgeType.php',
'PhabricatorProjectProjectPHIDType' => 'applications/project/phid/PhabricatorProjectProjectPHIDType.php',
'PhabricatorProjectQuery' => 'applications/project/query/PhabricatorProjectQuery.php',
'PhabricatorProjectRemoveHeraldAction' => 'applications/project/herald/PhabricatorProjectRemoveHeraldAction.php',
'PhabricatorProjectSchemaSpec' => 'applications/project/storage/PhabricatorProjectSchemaSpec.php',
'PhabricatorProjectSearchEngine' => 'applications/project/query/PhabricatorProjectSearchEngine.php',
'PhabricatorProjectSearchField' => 'applications/project/searchfield/PhabricatorProjectSearchField.php',
Expand Down Expand Up @@ -6496,6 +6499,7 @@
'PhabricatorCustomFieldInterface',
'PhabricatorDestructibleInterface',
),
'PhabricatorProjectAddHeraldAction' => 'PhabricatorProjectHeraldAction',
'PhabricatorProjectApplication' => 'PhabricatorApplication',
'PhabricatorProjectArchiveController' => 'PhabricatorProjectController',
'PhabricatorProjectBoardController' => 'PhabricatorProjectController',
Expand Down Expand Up @@ -6539,6 +6543,7 @@
'PhabricatorProjectEditPictureController' => 'PhabricatorProjectController',
'PhabricatorProjectEditorTestCase' => 'PhabricatorTestCase',
'PhabricatorProjectFeedController' => 'PhabricatorProjectController',
'PhabricatorProjectHeraldAction' => 'HeraldAction',
'PhabricatorProjectIcon' => 'Phobject',
'PhabricatorProjectListController' => 'PhabricatorProjectController',
'PhabricatorProjectLogicalAndDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
Expand All @@ -6559,6 +6564,7 @@
'PhabricatorProjectProjectHasObjectEdgeType' => 'PhabricatorEdgeType',
'PhabricatorProjectProjectPHIDType' => 'PhabricatorPHIDType',
'PhabricatorProjectQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorProjectRemoveHeraldAction' => 'PhabricatorProjectHeraldAction',
'PhabricatorProjectSchemaSpec' => 'PhabricatorConfigSchemaSpec',
'PhabricatorProjectSearchEngine' => 'PhabricatorApplicationSearchEngine',
'PhabricatorProjectSearchField' => 'PhabricatorSearchTokenizerField',
Expand Down
58 changes: 0 additions & 58 deletions src/applications/herald/adapter/HeraldAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ abstract class HeraldAdapter extends Phobject {

const ACTION_AUDIT = 'audit';
const ACTION_ASSIGN_TASK = 'assigntask';
const ACTION_ADD_PROJECTS = 'addprojects';
const ACTION_REMOVE_PROJECTS = 'removeprojects';
const ACTION_ADD_REVIEWERS = 'addreviewers';
const ACTION_ADD_BLOCKING_REVIEWERS = 'addblockingreviewers';
const ACTION_APPLY_BUILD_PLANS = 'applybuildplans';
Expand Down Expand Up @@ -707,15 +705,6 @@ public function getActions($rule_type) {

$actions = $custom_actions;

$object = $this->newObject();

if (($object instanceof PhabricatorProjectInterface)) {
if ($rule_type == HeraldRuleTypeConfig::RULE_TYPE_GLOBAL) {
$actions[] = self::ACTION_ADD_PROJECTS;
$actions[] = self::ACTION_REMOVE_PROJECTS;
}
}

foreach ($this->getActionsForRuleType($rule_type) as $key => $action) {
$actions[] = $key;
}
Expand All @@ -730,8 +719,6 @@ public function getActionNameMap($rule_type) {
$standard = array(
self::ACTION_AUDIT => pht('Trigger an Audit by'),
self::ACTION_ASSIGN_TASK => pht('Assign task to'),
self::ACTION_ADD_PROJECTS => pht('Add projects'),
self::ACTION_REMOVE_PROJECTS => pht('Remove projects'),
self::ACTION_ADD_REVIEWERS => pht('Add reviewers'),
self::ACTION_ADD_BLOCKING_REVIEWERS => pht('Add blocking reviewers'),
self::ACTION_APPLY_BUILD_PLANS => pht('Run build plans'),
Expand Down Expand Up @@ -829,17 +816,9 @@ public function getValueTypeForAction($action, $rule_type) {
case self::ACTION_ADD_REVIEWERS:
case self::ACTION_ADD_BLOCKING_REVIEWERS:
return new HeraldEmptyFieldValue();
case self::ACTION_ADD_PROJECTS:
case self::ACTION_REMOVE_PROJECTS:
return $this->buildTokenizerFieldValue(
new PhabricatorProjectDatasource());
}
} else {
switch ($action) {
case self::ACTION_ADD_PROJECTS:
case self::ACTION_REMOVE_PROJECTS:
return $this->buildTokenizerFieldValue(
new PhabricatorProjectDatasource());
case self::ACTION_ASSIGN_TASK:
return $this->buildTokenizerFieldValue(
new PhabricatorPeopleDatasource());
Expand Down Expand Up @@ -1200,14 +1179,6 @@ protected function applyStandardEffect(HeraldEffect $effect) {
$rule_type));
}

switch ($action) {
case self::ACTION_ADD_PROJECTS:
case self::ACTION_REMOVE_PROJECTS:
return $this->applyProjectsEffect($effect);
default:
break;
}

$result = $this->handleCustomHeraldEffect($effect);

if (!$result) {
Expand All @@ -1222,35 +1193,6 @@ protected function applyStandardEffect(HeraldEffect $effect) {
return $result;
}

/**
* @task apply
*/
private function applyProjectsEffect(HeraldEffect $effect) {

if ($effect->getAction() == self::ACTION_ADD_PROJECTS) {
$kind = '+';
} else {
$kind = '-';
}

$project_type = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST;
$project_phids = $effect->getTarget();
$xaction = $this->newTransaction()
->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
->setMetadataValue('edge:type', $project_type)
->setNewValue(
array(
$kind => array_fuse($project_phids),
));

$this->queueTransaction($xaction);

return new HeraldApplyTranscript(
$effect,
true,
pht('Added projects.'));
}

public function loadEdgePHIDs($type) {
if (!isset($this->edgeCache[$type])) {
$phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

final class PhabricatorProjectAddHeraldAction
extends PhabricatorProjectHeraldAction {

const ACTIONCONST = 'projects.add';

public function getHeraldActionName() {
return pht('Add projects');
}

public function applyEffect($object, HeraldEffect $effect) {
return $this->applyProjects($effect->getTarget(), $is_add = true);
}

public function getHeraldActionStandardType() {
return self::STANDARD_PHID_LIST;
}

protected function getDatasource() {
return new PhabricatorProjectDatasource();
}

public function renderActionDescription($value) {
return pht('Add projects: %s.', $this->renderHandleList($value));
}

}
180 changes: 180 additions & 0 deletions src/applications/project/herald/PhabricatorProjectHeraldAction.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
<?php

abstract class PhabricatorProjectHeraldAction
extends HeraldAction {

const DO_NO_TARGETS = 'do.no-targets';
const DO_INVALID = 'do.invalid';
const DO_ALREADY_ASSOCIATED = 'do.already-associated';
const DO_ALREADY_UNASSOCIATED = 'do.already-unassociated';
const DO_ADD_PROJECTS = 'do.add-projects';
const DO_REMOVE_PROJECTS = 'do.remove-projects';

public function getActionGroupKey() {
return HeraldSupportActionGroup::ACTIONGROUPKEY;
}

public function supportsObject($object) {
return ($object instanceof PhabricatorProjectInterface);
}

public function supportsRuleType($rule_type) {
return ($rule_type == HeraldRuleTypeConfig::RULE_TYPE_GLOBAL);
}

protected function applyProjects(array $phids, $is_add) {
$phids = array_fuse($phids);
$adapter = $this->getAdapter();

if (!$phids) {
$this->logEffect(self::DO_NO_TARGETS);
return;
}

$projects = id(new PhabricatorProjectQuery())
->setViewer(PhabricatorUser::getOmnipotentUser())
->withPHIDs($phids)
->execute();
$projects = mpull($projects, null, 'getPHID');

$invalid = array();
foreach ($phids as $phid) {
if (empty($projects[$phid])) {
$invalid[] = $phid;
unset($phids[$phid]);
}
}

if ($invalid) {
$this->logEffect(self::DO_INVALID, $invalid);
}

if (!$phids) {
return;
}

$project_type = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST;

$current = $adapter->loadEdgePHIDs($project_type);

if ($is_add) {
$already = array();
foreach ($phids as $phid) {
if (isset($current[$phid])) {
$already[$phid] = $phid;
unset($phids[$phid]);
}
}

if ($already) {
$this->logEffect(self::DO_ALREADY_ASSOCIATED, $already);
}
} else {
$already = array();
foreach ($phids as $phid) {
if (empty($current[$phid])) {
$already[$phid] = $phid;
unset($phids[$phid]);
}
}

if ($already) {
$this->logEffect(self::DO_ALREADY_UNASSOCIATED, $already);
}
}

if (!$phids) {
return;
}

if ($is_add) {
$kind = '+';
} else {
$kind = '-';
}

$xaction = $adapter->newTransaction()
->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
->setMetadataValue('edge:type', $project_type)
->setNewValue(
array(
$kind => $phids,
));

$adapter->queueTransaction($xaction);

if ($is_add) {
$this->logEffect(self::DO_ADD_PROJECTS, $phids);
} else {
$this->logEffect(self::DO_REMOVE_PROJECTS, $phids);
}
}

protected function getActionEffectMap() {
return array(
self::DO_NO_TARGETS => array(
'icon' => 'fa-ban',
'color' => 'grey',
'name' => pht('No Targets'),
),
self::DO_INVALID => array(
'icon' => 'fa-ban',
'color' => 'red',
'name' => pht('Invalid Targets'),
),
self::DO_ALREADY_ASSOCIATED => array(
'icon' => 'fa-chevron-right',
'color' => 'grey',
'name' => pht('Already Associated'),
),
self::DO_ALREADY_UNASSOCIATED => array(
'icon' => 'fa-chevron-right',
'color' => 'grey',
'name' => pht('Already Unassociated'),
),
self::DO_ADD_PROJECTS => array(
'icon' => 'fa-briefcase',
'color' => 'green',
'name' => pht('Added Projects'),
),
self::DO_REMOVE_PROJECTS => array(
'icon' => 'fa-minus-circle',
'color' => 'green',
'name' => pht('Removed Projects'),
),
);
}

public function renderActionEffectDescription($type, $data) {
switch ($type) {
case self::DO_NO_TARGETS:
return pht('Rule lists no projects.');
case self::DO_INVALID:
return pht(
'Declined to act on %s invalid project(s): %s.',
new PhutilNumber(count($data)),
$this->renderHandleList($data));
case self::DO_ALREADY_ASSOCIATED:
return pht(
'%s project(s) are already associated: %s.',
new PhutilNumber(count($data)),
$this->renderHandleList($data));
case self::DO_ALREADY_UNASSOCIATED:
return pht(
'%s project(s) are not associated: %s.',
new PhutilNumber(count($data)),
$this->renderHandleList($data));
case self::DO_ADD_PROJECTS:
return pht(
'Added %s project(s): %s.',
new PhutilNumber(count($data)),
$this->renderHandleList($data));
case self::DO_REMOVE_PROJECTS:
return pht(
'Removed %s project(s): %s.',
new PhutilNumber(count($data)),
$this->renderHandleList($data));
}
}

}
Loading

0 comments on commit 3782992

Please sign in to comment.