From 37829926702bd7bc3e1fd8b97411e9aee4cf1f02 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 24 Jul 2015 10:56:52 -0700 Subject: [PATCH] Modularize "add projects" and "remove projects" Herald actions 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 --- .../sql/autopatches/20150724.herald.3.sql | 11 ++ src/__phutil_library_map__.php | 6 + .../herald/adapter/HeraldAdapter.php | 58 ------ .../PhabricatorProjectAddHeraldAction.php | 28 +++ .../herald/PhabricatorProjectHeraldAction.php | 180 ++++++++++++++++++ .../PhabricatorProjectRemoveHeraldAction.php | 28 +++ .../PhabricatorUSEnglishTranslation.php | 20 ++ 7 files changed, 273 insertions(+), 58 deletions(-) create mode 100644 resources/sql/autopatches/20150724.herald.3.sql create mode 100644 src/applications/project/herald/PhabricatorProjectAddHeraldAction.php create mode 100644 src/applications/project/herald/PhabricatorProjectHeraldAction.php create mode 100644 src/applications/project/herald/PhabricatorProjectRemoveHeraldAction.php diff --git a/resources/sql/autopatches/20150724.herald.3.sql b/resources/sql/autopatches/20150724.herald.3.sql new file mode 100644 index 0000000000..58caa58407 --- /dev/null +++ b/resources/sql/autopatches/20150724.herald.3.sql @@ -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'; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index f46f0538b1..1ddf479fa5 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2540,6 +2540,7 @@ phutil_register_library_map(array( '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', @@ -2572,6 +2573,7 @@ phutil_register_library_map(array( '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', @@ -2593,6 +2595,7 @@ phutil_register_library_map(array( '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', @@ -6496,6 +6499,7 @@ phutil_register_library_map(array( 'PhabricatorCustomFieldInterface', 'PhabricatorDestructibleInterface', ), + 'PhabricatorProjectAddHeraldAction' => 'PhabricatorProjectHeraldAction', 'PhabricatorProjectApplication' => 'PhabricatorApplication', 'PhabricatorProjectArchiveController' => 'PhabricatorProjectController', 'PhabricatorProjectBoardController' => 'PhabricatorProjectController', @@ -6539,6 +6543,7 @@ phutil_register_library_map(array( 'PhabricatorProjectEditPictureController' => 'PhabricatorProjectController', 'PhabricatorProjectEditorTestCase' => 'PhabricatorTestCase', 'PhabricatorProjectFeedController' => 'PhabricatorProjectController', + 'PhabricatorProjectHeraldAction' => 'HeraldAction', 'PhabricatorProjectIcon' => 'Phobject', 'PhabricatorProjectListController' => 'PhabricatorProjectController', 'PhabricatorProjectLogicalAndDatasource' => 'PhabricatorTypeaheadCompositeDatasource', @@ -6559,6 +6564,7 @@ phutil_register_library_map(array( 'PhabricatorProjectProjectHasObjectEdgeType' => 'PhabricatorEdgeType', 'PhabricatorProjectProjectPHIDType' => 'PhabricatorPHIDType', 'PhabricatorProjectQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorProjectRemoveHeraldAction' => 'PhabricatorProjectHeraldAction', 'PhabricatorProjectSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'PhabricatorProjectSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorProjectSearchField' => 'PhabricatorSearchTokenizerField', diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index 3de06e63d6..f2466d94f2 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -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'; @@ -707,15 +705,6 @@ abstract class HeraldAdapter extends Phobject { $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; } @@ -730,8 +719,6 @@ abstract class HeraldAdapter extends Phobject { $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'), @@ -829,17 +816,9 @@ abstract class HeraldAdapter extends Phobject { 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()); @@ -1200,14 +1179,6 @@ abstract class HeraldAdapter extends Phobject { $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) { @@ -1222,35 +1193,6 @@ abstract class HeraldAdapter extends Phobject { 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( diff --git a/src/applications/project/herald/PhabricatorProjectAddHeraldAction.php b/src/applications/project/herald/PhabricatorProjectAddHeraldAction.php new file mode 100644 index 0000000000..5d2a348eca --- /dev/null +++ b/src/applications/project/herald/PhabricatorProjectAddHeraldAction.php @@ -0,0 +1,28 @@ +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)); + } + +} diff --git a/src/applications/project/herald/PhabricatorProjectHeraldAction.php b/src/applications/project/herald/PhabricatorProjectHeraldAction.php new file mode 100644 index 0000000000..599a218213 --- /dev/null +++ b/src/applications/project/herald/PhabricatorProjectHeraldAction.php @@ -0,0 +1,180 @@ +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)); + } + } + +} diff --git a/src/applications/project/herald/PhabricatorProjectRemoveHeraldAction.php b/src/applications/project/herald/PhabricatorProjectRemoveHeraldAction.php new file mode 100644 index 0000000000..77e7071b13 --- /dev/null +++ b/src/applications/project/herald/PhabricatorProjectRemoveHeraldAction.php @@ -0,0 +1,28 @@ +applyProjects($effect->getTarget(), $is_add = false); + } + + public function getHeraldActionStandardType() { + return self::STANDARD_PHID_LIST; + } + + protected function getDatasource() { + return new PhabricatorProjectDatasource(); + } + + public function renderActionDescription($value) { + return pht('Remove projects: %s.', $this->renderHandleList($value)); + } + +} diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php index a23ce1293c..d4343b2ec9 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -1303,6 +1303,26 @@ final class PhabricatorUSEnglishTranslation 'preferences: %2$s.', ), + '%s project(s) are not associated: %s.' => array( + 'A project is not associated: %2$s.', + 'Projects are not associated: %2$s.', + ), + + '%s project(s) are already associated: %s.' => array( + 'A project is already associated: %2$s.', + 'Projects are already associated: %2$s.', + ), + + 'Added %s project(s): %s.' => array( + 'Added a project: %2$s.', + 'Added projects: %2$s.', + ), + + 'Removed %s project(s): %s.' => array( + 'Removed a project: %2$s.', + 'Removed projects: %2$s.', + ), + ); }