From bf62badfdaeccb8be2d28243d39521661e9dde55 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 22 Jun 2016 05:35:40 -0700 Subject: [PATCH] Modularize "related objects" menu items in Maniphest Summary: Ref T11179. This generates the Maniphest menu items in a modular way. It doesn't change any of the underlying code yet. Searching for commits doesn't work particularly well so I've just hidden that for now, but the item itself works fine. Test Plan: {F1696849} Reviewers: chad Reviewed By: chad Maniphest Tasks: T11179 Differential Revision: https://secure.phabricator.com/D16162 --- src/__phutil_library_map__.php | 16 +++- .../PhabricatorDifferentialApplication.php | 1 - .../DifferentialActionMenuEventListener.php | 50 ---------- .../ManiphestTaskDetailController.php | 9 ++ .../ManiphestTaskHasCommitRelationship.php | 27 ++++++ .../ManiphestTaskHasMockRelationship.php | 20 ++++ .../ManiphestTaskHasRevisionRelationship.php | 20 ++++ .../ManiphestTaskRelationship.php | 19 ++++ .../PhabricatorPholioApplication.php | 6 -- .../event/PholioActionMenuEventListener.php | 50 ---------- .../PhabricatorObjectRelationship.php | 81 ++++++++++++++++ .../PhabricatorObjectRelationshipList.php | 95 +++++++++++++++++++ src/view/layout/PhabricatorActionView.php | 4 + 13 files changed, 287 insertions(+), 111 deletions(-) delete mode 100644 src/applications/differential/event/DifferentialActionMenuEventListener.php create mode 100644 src/applications/maniphest/relationship/ManiphestTaskHasCommitRelationship.php create mode 100644 src/applications/maniphest/relationship/ManiphestTaskHasMockRelationship.php create mode 100644 src/applications/maniphest/relationship/ManiphestTaskHasRevisionRelationship.php create mode 100644 src/applications/maniphest/relationship/ManiphestTaskRelationship.php delete mode 100644 src/applications/pholio/event/PholioActionMenuEventListener.php create mode 100644 src/applications/search/relationship/PhabricatorObjectRelationship.php create mode 100644 src/applications/search/relationship/PhabricatorObjectRelationshipList.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 6a518c2244..c0e6166baf 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -350,7 +350,6 @@ phutil_register_library_map(array( 'DefaultDatabaseConfigurationProvider' => 'infrastructure/storage/configuration/DefaultDatabaseConfigurationProvider.php', 'DifferentialAction' => 'applications/differential/constants/DifferentialAction.php', 'DifferentialActionEmailCommand' => 'applications/differential/command/DifferentialActionEmailCommand.php', - 'DifferentialActionMenuEventListener' => 'applications/differential/event/DifferentialActionMenuEventListener.php', 'DifferentialAddCommentView' => 'applications/differential/view/DifferentialAddCommentView.php', 'DifferentialAdjustmentMapTestCase' => 'applications/differential/storage/__tests__/DifferentialAdjustmentMapTestCase.php', 'DifferentialAffectedPath' => 'applications/differential/storage/DifferentialAffectedPath.php', @@ -1420,8 +1419,11 @@ phutil_register_library_map(array( 'ManiphestTaskEditController' => 'applications/maniphest/controller/ManiphestTaskEditController.php', 'ManiphestTaskFulltextEngine' => 'applications/maniphest/search/ManiphestTaskFulltextEngine.php', 'ManiphestTaskHasCommitEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasCommitEdgeType.php', + 'ManiphestTaskHasCommitRelationship' => 'applications/maniphest/relationship/ManiphestTaskHasCommitRelationship.php', 'ManiphestTaskHasMockEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasMockEdgeType.php', + 'ManiphestTaskHasMockRelationship' => 'applications/maniphest/relationship/ManiphestTaskHasMockRelationship.php', 'ManiphestTaskHasRevisionEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasRevisionEdgeType.php', + 'ManiphestTaskHasRevisionRelationship' => 'applications/maniphest/relationship/ManiphestTaskHasRevisionRelationship.php', 'ManiphestTaskHeraldField' => 'applications/maniphest/herald/ManiphestTaskHeraldField.php', 'ManiphestTaskHeraldFieldGroup' => 'applications/maniphest/herald/ManiphestTaskHeraldFieldGroup.php', 'ManiphestTaskListController' => 'applications/maniphest/controller/ManiphestTaskListController.php', @@ -1437,6 +1439,7 @@ phutil_register_library_map(array( 'ManiphestTaskPriorityHeraldAction' => 'applications/maniphest/herald/ManiphestTaskPriorityHeraldAction.php', 'ManiphestTaskPriorityHeraldField' => 'applications/maniphest/herald/ManiphestTaskPriorityHeraldField.php', 'ManiphestTaskQuery' => 'applications/maniphest/query/ManiphestTaskQuery.php', + 'ManiphestTaskRelationship' => 'applications/maniphest/relationship/ManiphestTaskRelationship.php', 'ManiphestTaskResultListView' => 'applications/maniphest/view/ManiphestTaskResultListView.php', 'ManiphestTaskSearchEngine' => 'applications/maniphest/query/ManiphestTaskSearchEngine.php', 'ManiphestTaskStatus' => 'applications/maniphest/constants/ManiphestTaskStatus.php', @@ -2860,6 +2863,8 @@ phutil_register_library_map(array( 'PhabricatorObjectMentionedByObjectEdgeType' => 'applications/transactions/edges/PhabricatorObjectMentionedByObjectEdgeType.php', 'PhabricatorObjectMentionsObjectEdgeType' => 'applications/transactions/edges/PhabricatorObjectMentionsObjectEdgeType.php', 'PhabricatorObjectQuery' => 'applications/phid/query/PhabricatorObjectQuery.php', + 'PhabricatorObjectRelationship' => 'applications/search/relationship/PhabricatorObjectRelationship.php', + 'PhabricatorObjectRelationshipList' => 'applications/search/relationship/PhabricatorObjectRelationshipList.php', 'PhabricatorObjectRemarkupRule' => 'infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php', 'PhabricatorObjectSelectorDialog' => 'view/control/PhabricatorObjectSelectorDialog.php', 'PhabricatorOffsetPagedQuery' => 'infrastructure/query/PhabricatorOffsetPagedQuery.php', @@ -3834,7 +3839,6 @@ phutil_register_library_map(array( 'PhluxVariablePHIDType' => 'applications/phlux/phid/PhluxVariablePHIDType.php', 'PhluxVariableQuery' => 'applications/phlux/query/PhluxVariableQuery.php', 'PhluxViewController' => 'applications/phlux/controller/PhluxViewController.php', - 'PholioActionMenuEventListener' => 'applications/pholio/event/PholioActionMenuEventListener.php', 'PholioController' => 'applications/pholio/controller/PholioController.php', 'PholioDAO' => 'applications/pholio/storage/PholioDAO.php', 'PholioDefaultEditCapability' => 'applications/pholio/capability/PholioDefaultEditCapability.php', @@ -4662,7 +4666,6 @@ phutil_register_library_map(array( ), 'DifferentialAction' => 'Phobject', 'DifferentialActionEmailCommand' => 'MetaMTAEmailTransactionCommand', - 'DifferentialActionMenuEventListener' => 'PhabricatorEventListener', 'DifferentialAddCommentView' => 'AphrontView', 'DifferentialAdjustmentMapTestCase' => 'PhutilTestCase', 'DifferentialAffectedPath' => 'DifferentialDAO', @@ -5906,8 +5909,11 @@ phutil_register_library_map(array( 'ManiphestTaskEditController' => 'ManiphestController', 'ManiphestTaskFulltextEngine' => 'PhabricatorFulltextEngine', 'ManiphestTaskHasCommitEdgeType' => 'PhabricatorEdgeType', + 'ManiphestTaskHasCommitRelationship' => 'ManiphestTaskRelationship', 'ManiphestTaskHasMockEdgeType' => 'PhabricatorEdgeType', + 'ManiphestTaskHasMockRelationship' => 'ManiphestTaskRelationship', 'ManiphestTaskHasRevisionEdgeType' => 'PhabricatorEdgeType', + 'ManiphestTaskHasRevisionRelationship' => 'ManiphestTaskRelationship', 'ManiphestTaskHeraldField' => 'HeraldField', 'ManiphestTaskHeraldFieldGroup' => 'HeraldFieldGroup', 'ManiphestTaskListController' => 'ManiphestController', @@ -5923,6 +5929,7 @@ phutil_register_library_map(array( 'ManiphestTaskPriorityHeraldAction' => 'HeraldAction', 'ManiphestTaskPriorityHeraldField' => 'ManiphestTaskHeraldField', 'ManiphestTaskQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'ManiphestTaskRelationship' => 'PhabricatorObjectRelationship', 'ManiphestTaskResultListView' => 'ManiphestView', 'ManiphestTaskSearchEngine' => 'PhabricatorApplicationSearchEngine', 'ManiphestTaskStatus' => 'ManiphestConstants', @@ -7547,6 +7554,8 @@ phutil_register_library_map(array( 'PhabricatorObjectMentionedByObjectEdgeType' => 'PhabricatorEdgeType', 'PhabricatorObjectMentionsObjectEdgeType' => 'PhabricatorEdgeType', 'PhabricatorObjectQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorObjectRelationship' => 'Phobject', + 'PhabricatorObjectRelationshipList' => 'Phobject', 'PhabricatorObjectRemarkupRule' => 'PhutilRemarkupRule', 'PhabricatorObjectSelectorDialog' => 'Phobject', 'PhabricatorOffsetPagedQuery' => 'PhabricatorQuery', @@ -8730,7 +8739,6 @@ phutil_register_library_map(array( 'PhluxVariablePHIDType' => 'PhabricatorPHIDType', 'PhluxVariableQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhluxViewController' => 'PhluxController', - 'PholioActionMenuEventListener' => 'PhabricatorEventListener', 'PholioController' => 'PhabricatorController', 'PholioDAO' => 'PhabricatorLiskDAO', 'PholioDefaultEditCapability' => 'PhabricatorPolicyCapability', diff --git a/src/applications/differential/application/PhabricatorDifferentialApplication.php b/src/applications/differential/application/PhabricatorDifferentialApplication.php index ed0053061b..589dfe239d 100644 --- a/src/applications/differential/application/PhabricatorDifferentialApplication.php +++ b/src/applications/differential/application/PhabricatorDifferentialApplication.php @@ -43,7 +43,6 @@ final class PhabricatorDifferentialApplication extends PhabricatorApplication { public function getEventListeners() { return array( - new DifferentialActionMenuEventListener(), new DifferentialLandingActionMenuEventListener(), ); } diff --git a/src/applications/differential/event/DifferentialActionMenuEventListener.php b/src/applications/differential/event/DifferentialActionMenuEventListener.php deleted file mode 100644 index a35f834136..0000000000 --- a/src/applications/differential/event/DifferentialActionMenuEventListener.php +++ /dev/null @@ -1,50 +0,0 @@ -listen(PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS); - } - - public function handleEvent(PhutilEvent $event) { - switch ($event->getType()) { - case PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS: - $this->handleActionsEvent($event); - break; - } - } - - private function handleActionsEvent(PhutilEvent $event) { - $object = $event->getValue('object'); - - $actions = null; - if ($object instanceof ManiphestTask) { - $actions = $this->renderTaskItems($event); - $this->addActionMenuItems($event, $actions); - } - - } - - private function renderTaskItems(PhutilEvent $event) { - if (!$this->canUseApplication($event->getUser())) { - return null; - } - - $task = $event->getValue('object'); - $phid = $task->getPHID(); - - $can_edit = PhabricatorPolicyFilter::hasCapability( - $event->getUser(), - $task, - PhabricatorPolicyCapability::CAN_EDIT); - - return id(new PhabricatorActionView()) - ->setName(pht('Edit Differential Revisions')) - ->setHref("/search/attach/{$phid}/DREV/") - ->setIcon('fa-cog') - ->setDisabled(!$can_edit) - ->setWorkflow(true); - } - -} diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php index c29c7ee9a1..a2de17c341 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -215,6 +215,15 @@ final class ManiphestTaskDetailController extends ManiphestController { ->setIcon('fa-anchor') ->setSubmenu($task_submenu)); + $relationship_list = PhabricatorObjectRelationshipList::newForObject( + $viewer, + $task); + + $relationship_submenu = $relationship_list->newActionMenu(); + if ($relationship_submenu) { + $curtain->addAction($relationship_submenu); + } + $owner_phid = $task->getOwnerPHID(); $author_phid = $task->getAuthorPHID(); $handles = $viewer->loadHandles(array($owner_phid, $author_phid)); diff --git a/src/applications/maniphest/relationship/ManiphestTaskHasCommitRelationship.php b/src/applications/maniphest/relationship/ManiphestTaskHasCommitRelationship.php new file mode 100644 index 0000000000..e4195e3626 --- /dev/null +++ b/src/applications/maniphest/relationship/ManiphestTaskHasCommitRelationship.php @@ -0,0 +1,27 @@ +getViewer(); + + $has_app = PhabricatorApplication::isClassInstalledForViewer( + 'PhabricatorManiphestApplication', + $viewer); + if (!$has_app) { + return false; + } + + return ($object instanceof ManiphestTask); + } + +} diff --git a/src/applications/pholio/application/PhabricatorPholioApplication.php b/src/applications/pholio/application/PhabricatorPholioApplication.php index e7c841b68a..4d80dd12ae 100644 --- a/src/applications/pholio/application/PhabricatorPholioApplication.php +++ b/src/applications/pholio/application/PhabricatorPholioApplication.php @@ -26,12 +26,6 @@ final class PhabricatorPholioApplication extends PhabricatorApplication { return pht('Things before they were cool.'); } - public function getEventListeners() { - return array( - new PholioActionMenuEventListener(), - ); - } - public function getRemarkupRules() { return array( new PholioRemarkupRule(), diff --git a/src/applications/pholio/event/PholioActionMenuEventListener.php b/src/applications/pholio/event/PholioActionMenuEventListener.php deleted file mode 100644 index 0b563537fa..0000000000 --- a/src/applications/pholio/event/PholioActionMenuEventListener.php +++ /dev/null @@ -1,50 +0,0 @@ -listen(PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS); - } - - public function handleEvent(PhutilEvent $event) { - switch ($event->getType()) { - case PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS: - $this->handleActionsEvent($event); - break; - } - } - - private function handleActionsEvent(PhutilEvent $event) { - $object = $event->getValue('object'); - - $actions = null; - if ($object instanceof ManiphestTask) { - $actions = $this->renderTaskItems($event); - } - - $this->addActionMenuItems($event, $actions); - } - - private function renderTaskItems(PhutilEvent $event) { - if (!$this->canUseApplication($event->getUser())) { - return; - } - - $task = $event->getValue('object'); - $phid = $task->getPHID(); - - $can_edit = PhabricatorPolicyFilter::hasCapability( - $event->getUser(), - $task, - PhabricatorPolicyCapability::CAN_EDIT); - - return id(new PhabricatorActionView()) - ->setName(pht('Edit Pholio Mocks')) - ->setHref("/search/attach/{$phid}/MOCK/edge/") - ->setIcon('fa-camera-retro') - ->setDisabled(!$can_edit) - ->setWorkflow(true); - } - -} diff --git a/src/applications/search/relationship/PhabricatorObjectRelationship.php b/src/applications/search/relationship/PhabricatorObjectRelationship.php new file mode 100644 index 0000000000..1cc28e6a1d --- /dev/null +++ b/src/applications/search/relationship/PhabricatorObjectRelationship.php @@ -0,0 +1,81 @@ +viewer = $viewer; + return $this; + } + + public function getViewer() { + return $this->viewer; + } + + final public function getRelationshipConstant() { + return $this->getPhobjectClassConstant('RELATIONSHIPKEY'); + } + + abstract public function isEnabledForObject($object); + + abstract public function getEdgeConstant(); + + abstract protected function getActionName(); + abstract protected function getActionIcon(); + + public function shouldAppearInActionMenu() { + return true; + } + + protected function isActionEnabled($object) { + $viewer = $this->getViewer(); + + return PhabricatorPolicyFilter::hasCapability( + $viewer, + $object, + PhabricatorPolicyCapability::CAN_EDIT); + } + + final public function newAction($object) { + $is_enabled = $this->isActionEnabled($object); + $action_uri = $this->getActionURI($object); + + return id(new PhabricatorActionView()) + ->setName($this->getActionName()) + ->setHref($action_uri) + ->setIcon($this->getActionIcon()) + ->setDisabled(!$is_enabled) + ->setWorkflow(true); + } + + final public static function getAllRelationships() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass(__CLASS__) + ->setUniqueMethod('getRelationshipConstant') + ->execute(); + } + + private function getActionURI($object) { + $phid = $object->getPHID(); + + // TODO: Remove this, this is just legacy support for the current + // controller until a new one gets built. + $legacy_kinds = array( + ManiphestTaskHasCommitEdgeType::EDGECONST => 'CMIT', + ManiphestTaskHasMockEdgeType::EDGECONST => 'MOCK', + ManiphestTaskHasRevisionEdgeType::EDGECONST => 'DREV', + ); + + $edge_type = $this->getEdgeConstant(); + $legacy_kind = idx($legacy_kinds, $edge_type); + if (!$legacy_kind) { + throw new Exception( + pht( + 'Only specific legacy relationships are supported!')); + } + + return "/search/attach/{$phid}/{$legacy_kind}/"; + } + +} diff --git a/src/applications/search/relationship/PhabricatorObjectRelationshipList.php b/src/applications/search/relationship/PhabricatorObjectRelationshipList.php new file mode 100644 index 0000000000..36d2ab2d1b --- /dev/null +++ b/src/applications/search/relationship/PhabricatorObjectRelationshipList.php @@ -0,0 +1,95 @@ +viewer = $viewer; + return $this; + } + + public function getViewer() { + if ($this->viewer === null) { + throw new PhutilInvalidStateException('setViewer'); + } + + return $this->viewer; + } + + public function setObject($object) { + $this->object = $object; + return $this; + } + + public function getObject() { + if ($this->object === null) { + throw new PhutilInvalidStateException('setObject'); + } + + return $this->object; + } + + public function setRelationships(array $relationships) { + assert_instances_of($relationships, 'PhabricatorObjectRelationship'); + $this->relationships = $relationships; + return $this; + } + + public function getRelationships() { + if ($this->relationships === null) { + throw new PhutilInvalidStateException('setRelationships'); + } + + return $this->relationships; + } + + public function newActionMenu() { + $relationships = $this->getRelationships(); + $object = $this->getObject(); + + $actions = array(); + foreach ($relationships as $key => $relationship) { + if (!$relationship->shouldAppearInActionMenu()) { + continue; + } + + $actions[$key] = $relationship->newAction($object); + } + + if (!$actions) { + return null; + } + + $actions = msort($actions, 'getName'); + + return id(new PhabricatorActionView()) + ->setName(pht('Edit Related Objects...')) + ->setIcon('fa-link') + ->setSubmenu($actions); + } + + public static function newForObject(PhabricatorUser $viewer, $object) { + $relationships = PhabricatorObjectRelationship::getAllRelationships(); + + $results = array(); + foreach ($relationships as $key => $relationship) { + $relationship = clone $relationship; + + $relationship->setViewer($viewer); + if (!$relationship->isEnabledForObject($object)) { + continue; + } + + $results[$key] = $relationship; + } + + return id(new self()) + ->setViewer($viewer) + ->setObject($object) + ->setRelationships($results); + } + +} diff --git a/src/view/layout/PhabricatorActionView.php b/src/view/layout/PhabricatorActionView.php index a936b36e47..7a7cc64c3b 100644 --- a/src/view/layout/PhabricatorActionView.php +++ b/src/view/layout/PhabricatorActionView.php @@ -70,6 +70,10 @@ final class PhabricatorActionView extends AphrontView { return $this; } + public function getName() { + return $this->name; + } + public function setLabel($label) { $this->label = $label; return $this;