From b5d90b4714b2c92fd3d834da25f1ea2b11e9a284 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 22 Jun 2016 08:04:14 -0700 Subject: [PATCH] Drive modular task relationships through a new "relationships" controller Summary: Ref T11179. This is basically a "pro" controller to replace the SearchAttach controller. It does basically the same stuff, just in a (mostly) more modern and modular way. Test Plan: - Added and removed mocks. - Added and removed revisions. - Everything worked just like it did before. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11179 Differential Revision: https://secure.phabricator.com/D16163 --- src/__phutil_library_map__.php | 2 + .../ManiphestTaskHasCommitRelationship.php | 16 ++ .../ManiphestTaskHasMockRelationship.php | 16 ++ .../ManiphestTaskHasRevisionRelationship.php | 16 ++ .../PhabricatorSearchApplication.php | 2 + ...habricatorSearchRelationshipController.php | 198 ++++++++++++++++++ .../PhabricatorObjectRelationship.php | 30 ++- .../PhabricatorObjectRelationshipList.php | 4 + 8 files changed, 266 insertions(+), 18 deletions(-) create mode 100644 src/applications/search/controller/PhabricatorSearchRelationshipController.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index c0e6166baf..9f2625db8f 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3394,6 +3394,7 @@ phutil_register_library_map(array( 'PhabricatorSearchOrderController' => 'applications/search/controller/PhabricatorSearchOrderController.php', 'PhabricatorSearchOrderField' => 'applications/search/field/PhabricatorSearchOrderField.php', 'PhabricatorSearchRelationship' => 'applications/search/constants/PhabricatorSearchRelationship.php', + 'PhabricatorSearchRelationshipController' => 'applications/search/controller/PhabricatorSearchRelationshipController.php', 'PhabricatorSearchResultBucket' => 'applications/search/buckets/PhabricatorSearchResultBucket.php', 'PhabricatorSearchResultBucketGroup' => 'applications/search/buckets/PhabricatorSearchResultBucketGroup.php', 'PhabricatorSearchResultView' => 'applications/search/view/PhabricatorSearchResultView.php', @@ -8213,6 +8214,7 @@ phutil_register_library_map(array( 'PhabricatorSearchOrderController' => 'PhabricatorSearchBaseController', 'PhabricatorSearchOrderField' => 'PhabricatorSearchField', 'PhabricatorSearchRelationship' => 'Phobject', + 'PhabricatorSearchRelationshipController' => 'PhabricatorSearchBaseController', 'PhabricatorSearchResultBucket' => 'Phobject', 'PhabricatorSearchResultBucketGroup' => 'Phobject', 'PhabricatorSearchResultView' => 'AphrontView', diff --git a/src/applications/maniphest/relationship/ManiphestTaskHasCommitRelationship.php b/src/applications/maniphest/relationship/ManiphestTaskHasCommitRelationship.php index e4195e3626..4fb35e46c9 100644 --- a/src/applications/maniphest/relationship/ManiphestTaskHasCommitRelationship.php +++ b/src/applications/maniphest/relationship/ManiphestTaskHasCommitRelationship.php @@ -24,4 +24,20 @@ final class ManiphestTaskHasCommitRelationship return false; } + public function canRelateObjects($src, $dst) { + return ($dst instanceof PhabricatorRepositoryCommit); + } + + public function getDialogTitleText() { + return pht('Edit Related Commits'); + } + + public function getDialogHeaderText() { + return pht('Current Commits'); + } + + public function getDialogButtonText() { + return pht('Save Related Commits'); + } + } diff --git a/src/applications/maniphest/relationship/ManiphestTaskHasMockRelationship.php b/src/applications/maniphest/relationship/ManiphestTaskHasMockRelationship.php index d391e5c486..4425d1c3e9 100644 --- a/src/applications/maniphest/relationship/ManiphestTaskHasMockRelationship.php +++ b/src/applications/maniphest/relationship/ManiphestTaskHasMockRelationship.php @@ -17,4 +17,20 @@ final class ManiphestTaskHasMockRelationship return 'fa-camera-retro'; } + public function canRelateObjects($src, $dst) { + return ($dst instanceof PholioMock); + } + + public function getDialogTitleText() { + return pht('Edit Related Mocks'); + } + + public function getDialogHeaderText() { + return pht('Current Mocks'); + } + + public function getDialogButtonText() { + return pht('Save Related Mocks'); + } + } diff --git a/src/applications/maniphest/relationship/ManiphestTaskHasRevisionRelationship.php b/src/applications/maniphest/relationship/ManiphestTaskHasRevisionRelationship.php index eee0325aff..4530940191 100644 --- a/src/applications/maniphest/relationship/ManiphestTaskHasRevisionRelationship.php +++ b/src/applications/maniphest/relationship/ManiphestTaskHasRevisionRelationship.php @@ -17,4 +17,20 @@ final class ManiphestTaskHasRevisionRelationship return 'fa-cog'; } + public function canRelateObjects($src, $dst) { + return ($dst instanceof DifferentialRevision); + } + + public function getDialogTitleText() { + return pht('Edit Related Revisions'); + } + + public function getDialogHeaderText() { + return pht('Current Revisions'); + } + + public function getDialogButtonText() { + return pht('Save Related Revisions'); + } + } diff --git a/src/applications/search/application/PhabricatorSearchApplication.php b/src/applications/search/application/PhabricatorSearchApplication.php index bd3f78359e..36ad59764c 100644 --- a/src/applications/search/application/PhabricatorSearchApplication.php +++ b/src/applications/search/application/PhabricatorSearchApplication.php @@ -41,6 +41,8 @@ final class PhabricatorSearchApplication extends PhabricatorApplication { 'delete/(?P[^/]+)/(?P[^/]+)/' => 'PhabricatorSearchDeleteController', 'order/(?P[^/]+)/' => 'PhabricatorSearchOrderController', + 'rel/(?P[^/]+)/(?P[^/]+)/' + => 'PhabricatorSearchRelationshipController', ), ); } diff --git a/src/applications/search/controller/PhabricatorSearchRelationshipController.php b/src/applications/search/controller/PhabricatorSearchRelationshipController.php new file mode 100644 index 0000000000..12e4d16216 --- /dev/null +++ b/src/applications/search/controller/PhabricatorSearchRelationshipController.php @@ -0,0 +1,198 @@ +getViewer(); + + $phid = $request->getURIData('sourcePHID'); + $object = id(new PhabricatorObjectQuery()) + ->setViewer($viewer) + ->withPHIDs(array($phid)) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->executeOne(); + if (!$object) { + return new Aphront404Response(); + } + + $list = PhabricatorObjectRelationshipList::newForObject( + $viewer, + $object); + + $relationship_key = $request->getURIData('relationshipKey'); + $relationship = $list->getRelationship($relationship_key); + if (!$relationship) { + return new Aphront404Response(); + } + + $src_phid = $object->getPHID(); + $edge_type = $relationship->getEdgeConstant(); + + $dst_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( + $src_phid, + $edge_type); + + $all_phids = $dst_phids; + $all_phids[] = $src_phid; + + $handles = $viewer->loadHandles($all_phids); + $src_handle = $handles[$src_phid]; + + $done_uri = $src_handle->getURI(); + + if ($request->isFormPost()) { + $phids = explode(';', $request->getStr('phids')); + $phids = array_filter($phids); + $phids = array_values($phids); + + // TODO: Embed these in the form instead, to gracefully resolve + // concurrent edits like we do for subscribers and projects. + $old_phids = $dst_phids; + + $add_phids = $phids; + $rem_phids = array_diff($old_phids, $add_phids); + + if ($add_phids) { + $dst_objects = id(new PhabricatorObjectQuery()) + ->setViewer($viewer) + ->withPHIDs($phids) + ->setRaisePolicyExceptions(true) + ->execute(); + $dst_objects = mpull($dst_objects, null, 'getPHID'); + } else { + $dst_objects = array(); + } + + try { + foreach ($add_phids as $add_phid) { + $dst_object = idx($dst_objects, $add_phid); + if (!$dst_object) { + throw new Exception( + pht( + 'You can not create a relationship to object "%s" because '. + 'the object does not exist or could not be loaded.', + $add_phid)); + } + + if (!$relationship->canRelateObjects($object, $dst_object)) { + throw new Exception( + pht( + 'You can not create a relationship (of type "%s") to object '. + '"%s" because it is not the right type of object for this '. + 'relationship.', + $relationship->getRelationshipConstant(), + $add_phid)); + } + } + } catch (Exception $ex) { + return $this->newUnrelatableObjectResponse($ex, $done_uri); + } + + $editor = $object->getApplicationTransactionEditor() + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnMissingFields(true) + ->setContinueOnNoEffect(true); + + $xactions = array(); + $xactions[] = $object->getApplicationTransactionTemplate() + ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) + ->setMetadataValue('edge:type', $edge_type) + ->setNewValue(array( + '+' => array_fuse($add_phids), + '-' => array_fuse($rem_phids), + )); + + try { + $editor->applyTransactions($object, $xactions); + + return id(new AphrontRedirectResponse())->setURI($done_uri); + } catch (PhabricatorEdgeCycleException $ex) { + return $this->newGraphCycleResponse($ex, $done_uri); + } + } + + $handles = iterator_to_array($handles); + $handles = array_select_keys($handles, $dst_phids); + + // TODO: These are hard-coded for now. + $filters = array( + 'assigned' => pht('Assigned to Me'), + 'created' => pht('Created By Me'), + 'open' => pht('All Open Objects'), + 'all' => pht('All Objects'), + ); + + $dialog_title = $relationship->getDialogTitleText(); + $dialog_header = $relationship->getDialogHeaderText(); + $dialog_button = $relationship->getDialogButtonText(); + $dialog_instructions = $relationship->getDialogInstructionsText(); + + // TODO: Remove this, this is just legacy support. + $legacy_kinds = array( + ManiphestTaskHasCommitEdgeType::EDGECONST => 'CMIT', + ManiphestTaskHasMockEdgeType::EDGECONST => 'MOCK', + ManiphestTaskHasRevisionEdgeType::EDGECONST => 'DREV', + ); + + $edge_type = $relationship->getEdgeConstant(); + $legacy_kind = idx($legacy_kinds, $edge_type); + if (!$legacy_kind) { + throw new Exception( + pht('Only specific legacy relationships are supported!')); + } + + return id(new PhabricatorObjectSelectorDialog()) + ->setUser($viewer) + ->setHandles($handles) + ->setFilters($filters) + ->setSelectedFilter('created') + ->setExcluded($phid) + ->setCancelURI($done_uri) + ->setSearchURI("/search/select/{$legacy_kind}/edge/") + ->setTitle($dialog_title) + ->setHeader($dialog_header) + ->setButtonText($dialog_button) + ->setInstructions($dialog_instructions) + ->buildDialog(); + } + + private function newGraphCycleResponse( + PhabricatorEdgeCycleException $ex, + $done_uri) { + + $viewer = $this->getViewer(); + $cycle = $ex->getCycle(); + + $handles = $this->loadViewerHandles($cycle); + $names = array(); + foreach ($cycle as $cycle_phid) { + $names[] = $handles[$cycle_phid]->getFullName(); + } + + $message = pht( + 'You can not create that relationship because it would create a '. + 'circular dependency: %s.', + implode(" \xE2\x86\x92 ", $names)); + + return $this->newDialog() + ->setTitle(pht('Circular Dependency')) + ->appendParagraph($message) + ->addCancelButton($done_uri); + } + + private function newUnrelatableObjectResponse(Exception $ex, $done_uri) { + $message = $ex->getMessage(); + + return $this->newDialog() + ->setTitle(pht('Invalid Relationship')) + ->appendParagraph($message) + ->addCancelButton($done_uri); + } + +} diff --git a/src/applications/search/relationship/PhabricatorObjectRelationship.php b/src/applications/search/relationship/PhabricatorObjectRelationship.php index 1cc28e6a1d..9742a35adf 100644 --- a/src/applications/search/relationship/PhabricatorObjectRelationship.php +++ b/src/applications/search/relationship/PhabricatorObjectRelationship.php @@ -24,6 +24,16 @@ abstract class PhabricatorObjectRelationship extends Phobject { abstract protected function getActionName(); abstract protected function getActionIcon(); + abstract public function canRelateObjects($src, $dst); + + abstract public function getDialogTitleText(); + abstract public function getDialogHeaderText(); + abstract public function getDialogButtonText(); + + public function getDialogInstructionsText() { + return null; + } + public function shouldAppearInActionMenu() { return true; } @@ -58,24 +68,8 @@ abstract class PhabricatorObjectRelationship extends Phobject { 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}/"; + $type = $this->getRelationshipConstant(); + return "/search/rel/{$type}/{$phid}/"; } } diff --git a/src/applications/search/relationship/PhabricatorObjectRelationshipList.php b/src/applications/search/relationship/PhabricatorObjectRelationshipList.php index 36d2ab2d1b..a43bbaa571 100644 --- a/src/applications/search/relationship/PhabricatorObjectRelationshipList.php +++ b/src/applications/search/relationship/PhabricatorObjectRelationshipList.php @@ -71,6 +71,10 @@ final class PhabricatorObjectRelationshipList extends Phobject { ->setSubmenu($actions); } + public function getRelationship($key) { + return idx($this->relationships, $key); + } + public static function newForObject(PhabricatorUser $viewer, $object) { $relationships = PhabricatorObjectRelationship::getAllRelationships();