From 2a7545a452b0336701b5ce95fd5f2d63da42ec11 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 29 Jun 2016 11:09:27 -0700 Subject: [PATCH] Convert Maniphest merge operations to modern Relationship code Summary: Ref T4788. Fixes T7820. This updates the "Merge Duplicates In" interaction, and adds a "Close as Duplicate" action. These are the last interactions that were using the old code, so it removes that code. Merges are now recorded as real edges, so we can show them in the UI later on (originally from T9390, etc). Also provides more general support for relationships which need EDIT permission, not-undoable relationships like merges, preventing relating an object to itself, and relationship side effects like merges. Finally, fixes a couple of behaviors around typing an exact object name (like `T123`) to find the related object. Test Plan: - Merged tasks into the current task. - Closed the current task as a duplicate of another task. - Edited other relationships. - Searched for tasks, commits, etc., by object monogram. Reviewers: chad Reviewed By: chad Maniphest Tasks: T4788, T7820 Differential Revision: https://secure.phabricator.com/D16196 --- src/__phutil_library_map__.php | 12 +- .../ManiphestTaskDetailController.php | 13 +- .../ManiphestTaskHasDuplicateTaskEdgeType.php | 16 + ...ManiphestTaskIsDuplicateOfTaskEdgeType.php | 16 + ...iphestTaskCloseAsDuplicateRelationship.php | 84 +++++ .../ManiphestTaskMergeInRelationship.php | 75 ++++ .../ManiphestTaskRelationship.php | 48 +++ .../PhabricatorSearchApplication.php | 4 - .../PhabricatorSearchAttachController.php | 330 ------------------ ...habricatorSearchRelationshipController.php | 54 ++- ...atorSearchRelationshipSourceController.php | 14 +- .../PhabricatorSearchSelectController.php | 86 ----- .../PhabricatorObjectRelationship.php | 22 ++ .../PhabricatorApplicationTransaction.php | 2 + 14 files changed, 336 insertions(+), 440 deletions(-) create mode 100644 src/applications/maniphest/edge/ManiphestTaskHasDuplicateTaskEdgeType.php create mode 100644 src/applications/maniphest/edge/ManiphestTaskIsDuplicateOfTaskEdgeType.php create mode 100644 src/applications/maniphest/relationship/ManiphestTaskCloseAsDuplicateRelationship.php create mode 100644 src/applications/maniphest/relationship/ManiphestTaskMergeInRelationship.php delete mode 100644 src/applications/search/controller/PhabricatorSearchAttachController.php delete mode 100644 src/applications/search/controller/PhabricatorSearchSelectController.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 71f520674e..51373419d9 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1420,6 +1420,7 @@ phutil_register_library_map(array( 'ManiphestTaskAssigneeHeraldField' => 'applications/maniphest/herald/ManiphestTaskAssigneeHeraldField.php', 'ManiphestTaskAuthorHeraldField' => 'applications/maniphest/herald/ManiphestTaskAuthorHeraldField.php', 'ManiphestTaskAuthorPolicyRule' => 'applications/maniphest/policyrule/ManiphestTaskAuthorPolicyRule.php', + 'ManiphestTaskCloseAsDuplicateRelationship' => 'applications/maniphest/relationship/ManiphestTaskCloseAsDuplicateRelationship.php', 'ManiphestTaskClosedStatusDatasource' => 'applications/maniphest/typeahead/ManiphestTaskClosedStatusDatasource.php', 'ManiphestTaskDependedOnByTaskEdgeType' => 'applications/maniphest/edge/ManiphestTaskDependedOnByTaskEdgeType.php', 'ManiphestTaskDependsOnTaskEdgeType' => 'applications/maniphest/edge/ManiphestTaskDependsOnTaskEdgeType.php', @@ -1430,6 +1431,7 @@ phutil_register_library_map(array( 'ManiphestTaskFulltextEngine' => 'applications/maniphest/search/ManiphestTaskFulltextEngine.php', 'ManiphestTaskHasCommitEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasCommitEdgeType.php', 'ManiphestTaskHasCommitRelationship' => 'applications/maniphest/relationship/ManiphestTaskHasCommitRelationship.php', + 'ManiphestTaskHasDuplicateTaskEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasDuplicateTaskEdgeType.php', 'ManiphestTaskHasMockEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasMockEdgeType.php', 'ManiphestTaskHasMockRelationship' => 'applications/maniphest/relationship/ManiphestTaskHasMockRelationship.php', 'ManiphestTaskHasParentRelationship' => 'applications/maniphest/relationship/ManiphestTaskHasParentRelationship.php', @@ -1438,10 +1440,12 @@ phutil_register_library_map(array( 'ManiphestTaskHasSubtaskRelationship' => 'applications/maniphest/relationship/ManiphestTaskHasSubtaskRelationship.php', 'ManiphestTaskHeraldField' => 'applications/maniphest/herald/ManiphestTaskHeraldField.php', 'ManiphestTaskHeraldFieldGroup' => 'applications/maniphest/herald/ManiphestTaskHeraldFieldGroup.php', + 'ManiphestTaskIsDuplicateOfTaskEdgeType' => 'applications/maniphest/edge/ManiphestTaskIsDuplicateOfTaskEdgeType.php', 'ManiphestTaskListController' => 'applications/maniphest/controller/ManiphestTaskListController.php', 'ManiphestTaskListHTTPParameterType' => 'applications/maniphest/httpparametertype/ManiphestTaskListHTTPParameterType.php', 'ManiphestTaskListView' => 'applications/maniphest/view/ManiphestTaskListView.php', 'ManiphestTaskMailReceiver' => 'applications/maniphest/mail/ManiphestTaskMailReceiver.php', + 'ManiphestTaskMergeInRelationship' => 'applications/maniphest/relationship/ManiphestTaskMergeInRelationship.php', 'ManiphestTaskOpenStatusDatasource' => 'applications/maniphest/typeahead/ManiphestTaskOpenStatusDatasource.php', 'ManiphestTaskPHIDResolver' => 'applications/maniphest/httpparametertype/ManiphestTaskPHIDResolver.php', 'ManiphestTaskPHIDType' => 'applications/maniphest/phid/ManiphestTaskPHIDType.php', @@ -3372,7 +3376,6 @@ phutil_register_library_map(array( 'PhabricatorSearchApplication' => 'applications/search/application/PhabricatorSearchApplication.php', 'PhabricatorSearchApplicationSearchEngine' => 'applications/search/query/PhabricatorSearchApplicationSearchEngine.php', 'PhabricatorSearchApplicationStorageEnginePanel' => 'applications/search/applicationpanel/PhabricatorSearchApplicationStorageEnginePanel.php', - 'PhabricatorSearchAttachController' => 'applications/search/controller/PhabricatorSearchAttachController.php', 'PhabricatorSearchBaseController' => 'applications/search/controller/PhabricatorSearchBaseController.php', 'PhabricatorSearchCheckboxesField' => 'applications/search/field/PhabricatorSearchCheckboxesField.php', 'PhabricatorSearchConfigOptions' => 'applications/search/config/PhabricatorSearchConfigOptions.php', @@ -3415,7 +3418,6 @@ phutil_register_library_map(array( 'PhabricatorSearchResultView' => 'applications/search/view/PhabricatorSearchResultView.php', 'PhabricatorSearchSchemaSpec' => 'applications/search/storage/PhabricatorSearchSchemaSpec.php', 'PhabricatorSearchScopeSetting' => 'applications/settings/setting/PhabricatorSearchScopeSetting.php', - 'PhabricatorSearchSelectController' => 'applications/search/controller/PhabricatorSearchSelectController.php', 'PhabricatorSearchSelectField' => 'applications/search/field/PhabricatorSearchSelectField.php', 'PhabricatorSearchStringListField' => 'applications/search/field/PhabricatorSearchStringListField.php', 'PhabricatorSearchSubscribersField' => 'applications/search/field/PhabricatorSearchSubscribersField.php', @@ -5929,6 +5931,7 @@ phutil_register_library_map(array( 'ManiphestTaskAssigneeHeraldField' => 'ManiphestTaskHeraldField', 'ManiphestTaskAuthorHeraldField' => 'ManiphestTaskHeraldField', 'ManiphestTaskAuthorPolicyRule' => 'PhabricatorPolicyRule', + 'ManiphestTaskCloseAsDuplicateRelationship' => 'ManiphestTaskRelationship', 'ManiphestTaskClosedStatusDatasource' => 'PhabricatorTypeaheadDatasource', 'ManiphestTaskDependedOnByTaskEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskDependsOnTaskEdgeType' => 'PhabricatorEdgeType', @@ -5939,6 +5942,7 @@ phutil_register_library_map(array( 'ManiphestTaskFulltextEngine' => 'PhabricatorFulltextEngine', 'ManiphestTaskHasCommitEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskHasCommitRelationship' => 'ManiphestTaskRelationship', + 'ManiphestTaskHasDuplicateTaskEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskHasMockEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskHasMockRelationship' => 'ManiphestTaskRelationship', 'ManiphestTaskHasParentRelationship' => 'ManiphestTaskRelationship', @@ -5947,10 +5951,12 @@ phutil_register_library_map(array( 'ManiphestTaskHasSubtaskRelationship' => 'ManiphestTaskRelationship', 'ManiphestTaskHeraldField' => 'HeraldField', 'ManiphestTaskHeraldFieldGroup' => 'HeraldFieldGroup', + 'ManiphestTaskIsDuplicateOfTaskEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskListController' => 'ManiphestController', 'ManiphestTaskListHTTPParameterType' => 'AphrontListHTTPParameterType', 'ManiphestTaskListView' => 'ManiphestView', 'ManiphestTaskMailReceiver' => 'PhabricatorObjectMailReceiver', + 'ManiphestTaskMergeInRelationship' => 'ManiphestTaskRelationship', 'ManiphestTaskOpenStatusDatasource' => 'PhabricatorTypeaheadDatasource', 'ManiphestTaskPHIDResolver' => 'PhabricatorPHIDResolver', 'ManiphestTaskPHIDType' => 'PhabricatorPHIDType', @@ -8210,7 +8216,6 @@ phutil_register_library_map(array( 'PhabricatorSearchApplication' => 'PhabricatorApplication', 'PhabricatorSearchApplicationSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorSearchApplicationStorageEnginePanel' => 'PhabricatorApplicationConfigurationPanel', - 'PhabricatorSearchAttachController' => 'PhabricatorSearchBaseController', 'PhabricatorSearchBaseController' => 'PhabricatorController', 'PhabricatorSearchCheckboxesField' => 'PhabricatorSearchField', 'PhabricatorSearchConfigOptions' => 'PhabricatorApplicationConfigOptions', @@ -8253,7 +8258,6 @@ phutil_register_library_map(array( 'PhabricatorSearchResultView' => 'AphrontView', 'PhabricatorSearchSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'PhabricatorSearchScopeSetting' => 'PhabricatorInternalSetting', - 'PhabricatorSearchSelectController' => 'PhabricatorSearchBaseController', 'PhabricatorSearchSelectField' => 'PhabricatorSearchField', 'PhabricatorSearchStringListField' => 'PhabricatorSearchField', 'PhabricatorSearchSubscribersField' => 'PhabricatorSearchTokenizerField', diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php index bc862a1122..df95d07c12 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -201,6 +201,8 @@ final class ManiphestTaskDetailController extends ManiphestController { $parent_key = ManiphestTaskHasParentRelationship::RELATIONSHIPKEY; $subtask_key = ManiphestTaskHasSubtaskRelationship::RELATIONSHIPKEY; + $merge_key = ManiphestTaskMergeInRelationship::RELATIONSHIPKEY; + $close_key = ManiphestTaskCloseAsDuplicateRelationship::RELATIONSHIPKEY; $task_submenu[] = $relationship_list->getRelationship($parent_key) ->newAction($task); @@ -208,12 +210,11 @@ final class ManiphestTaskDetailController extends ManiphestController { $task_submenu[] = $relationship_list->getRelationship($subtask_key) ->newAction($task); - $task_submenu[] = id(new PhabricatorActionView()) - ->setName(pht('Merge Duplicates In')) - ->setHref("/search/attach/{$phid}/TASK/merge/") - ->setIcon('fa-compress') - ->setDisabled(!$can_edit) - ->setWorkflow(true); + $task_submenu[] = $relationship_list->getRelationship($merge_key) + ->newAction($task); + + $task_submenu[] = $relationship_list->getRelationship($close_key) + ->newAction($task); $curtain->addAction( id(new PhabricatorActionView()) diff --git a/src/applications/maniphest/edge/ManiphestTaskHasDuplicateTaskEdgeType.php b/src/applications/maniphest/edge/ManiphestTaskHasDuplicateTaskEdgeType.php new file mode 100644 index 0000000000..b5f5b44ae3 --- /dev/null +++ b/src/applications/maniphest/edge/ManiphestTaskHasDuplicateTaskEdgeType.php @@ -0,0 +1,16 @@ + 1) { + throw new Exception( + pht( + 'A task can only be closed as a duplicate of exactly one other '. + 'task.')); + } + + $task = head($add); + return $this->newMergeIntoTransactions($task); + } + + public function didUpdateRelationships($object, array $add, array $rem) { + $viewer = $this->getViewer(); + $content_source = $this->getContentSource(); + + $task = head($add); + $xactions = $this->newMergeFromTransactions(array($object)); + + $task->getApplicationTransactionEditor() + ->setActor($viewer) + ->setContentSource($content_source) + ->setContinueOnMissingFields(true) + ->setContinueOnNoEffect(true) + ->applyTransactions($task, $xactions); + } + +} diff --git a/src/applications/maniphest/relationship/ManiphestTaskMergeInRelationship.php b/src/applications/maniphest/relationship/ManiphestTaskMergeInRelationship.php new file mode 100644 index 0000000000..5bf45b7911 --- /dev/null +++ b/src/applications/maniphest/relationship/ManiphestTaskMergeInRelationship.php @@ -0,0 +1,75 @@ +newMergeFromTransactions($add); + } + + public function didUpdateRelationships($object, array $add, array $rem) { + $viewer = $this->getViewer(); + $content_source = $this->getContentSource(); + + foreach ($add as $task) { + $xactions = $this->newMergeIntoTransactions($object); + + $task->getApplicationTransactionEditor() + ->setActor($viewer) + ->setContentSource($content_source) + ->setContinueOnMissingFields(true) + ->setContinueOnNoEffect(true) + ->applyTransactions($task, $xactions); + } + } + +} diff --git a/src/applications/maniphest/relationship/ManiphestTaskRelationship.php b/src/applications/maniphest/relationship/ManiphestTaskRelationship.php index a0383d77f4..928d49e87d 100644 --- a/src/applications/maniphest/relationship/ManiphestTaskRelationship.php +++ b/src/applications/maniphest/relationship/ManiphestTaskRelationship.php @@ -16,4 +16,52 @@ abstract class ManiphestTaskRelationship return ($object instanceof ManiphestTask); } + protected function newMergeIntoTransactions(ManiphestTask $task) { + return array( + id(new ManiphestTransaction()) + ->setTransactionType(ManiphestTransaction::TYPE_MERGED_INTO) + ->setNewValue($task->getPHID()), + ); + } + + protected function newMergeFromTransactions(array $tasks) { + $xactions = array(); + + $subscriber_phids = $this->loadMergeSubscriberPHIDs($tasks); + + $xactions[] = id(new ManiphestTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS) + ->setNewValue(array('+' => $subscriber_phids)); + + $xactions[] = id(new ManiphestTransaction()) + ->setTransactionType(ManiphestTransaction::TYPE_MERGED_FROM) + ->setNewValue(mpull($tasks, 'getPHID')); + + return $xactions; + } + + private function loadMergeSubscriberPHIDs(array $tasks) { + $phids = array(); + + foreach ($tasks as $task) { + $phids[] = $task->getAuthorPHID(); + $phids[] = $task->getOwnerPHID(); + } + + $subscribers = id(new PhabricatorSubscribersQuery()) + ->withObjectPHIDs(mpull($tasks, 'getPHID')) + ->execute(); + + foreach ($subscribers as $phid => $subscriber_list) { + foreach ($subscriber_list as $subscriber) { + $phids[] = $subscriber; + } + } + + $phids = array_unique($phids); + $phids = array_filter($phids); + + return $phids; + } + } diff --git a/src/applications/search/application/PhabricatorSearchApplication.php b/src/applications/search/application/PhabricatorSearchApplication.php index 894e14fe2f..a2a28c7665 100644 --- a/src/applications/search/application/PhabricatorSearchApplication.php +++ b/src/applications/search/application/PhabricatorSearchApplication.php @@ -30,10 +30,6 @@ final class PhabricatorSearchApplication extends PhabricatorApplication { return array( '/search/' => array( '(?:query/(?P[^/]+)/)?' => 'PhabricatorSearchController', - 'attach/(?P[^/]+)/(?P\w+)/(?:(?P\w+)/)?' - => 'PhabricatorSearchAttachController', - 'select/(?P\w+)/(?:(?P\w+)/)?' - => 'PhabricatorSearchSelectController', 'index/(?P[^/]+)/' => 'PhabricatorSearchIndexController', 'hovercard/' => 'PhabricatorSearchHovercardController', diff --git a/src/applications/search/controller/PhabricatorSearchAttachController.php b/src/applications/search/controller/PhabricatorSearchAttachController.php deleted file mode 100644 index a3238cb50d..0000000000 --- a/src/applications/search/controller/PhabricatorSearchAttachController.php +++ /dev/null @@ -1,330 +0,0 @@ -getUser(); - $phid = $request->getURIData('phid'); - $attach_type = $request->getURIData('type'); - $action = $request->getURIData('action', self::ACTION_ATTACH); - - $handle = id(new PhabricatorHandleQuery()) - ->setViewer($user) - ->withPHIDs(array($phid)) - ->executeOne(); - - $object_type = $handle->getType(); - - $object = id(new PhabricatorObjectQuery()) - ->setViewer($user) - ->requireCapabilities( - array( - PhabricatorPolicyCapability::CAN_VIEW, - PhabricatorPolicyCapability::CAN_EDIT, - )) - ->withPHIDs(array($phid)) - ->executeOne(); - - if (!$object) { - return new Aphront404Response(); - } - - $edge_type = null; - switch ($action) { - case self::ACTION_EDGE: - case self::ACTION_DEPENDENCIES: - case self::ACTION_BLOCKS: - case self::ACTION_ATTACH: - $edge_type = $this->getEdgeType($object_type, $attach_type); - break; - } - - if ($request->isFormPost()) { - $phids = explode(';', $request->getStr('phids')); - $phids = array_filter($phids); - $phids = array_values($phids); - - if ($edge_type) { - if (!$object instanceof PhabricatorApplicationTransactionInterface) { - throw new Exception( - pht( - 'Expected object ("%s") to implement interface "%s".', - get_class($object), - 'PhabricatorApplicationTransactionInterface')); - } - - $old_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( - $phid, - $edge_type); - $add_phids = $phids; - $rem_phids = array_diff($old_phids, $add_phids); - - $txn_editor = $object->getApplicationTransactionEditor() - ->setActor($user) - ->setContentSourceFromRequest($request) - ->setContinueOnMissingFields(true) - ->setContinueOnNoEffect(true); - - $txn_template = $object->getApplicationTransactionTemplate() - ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) - ->setMetadataValue('edge:type', $edge_type) - ->setNewValue(array( - '+' => array_fuse($add_phids), - '-' => array_fuse($rem_phids), - )); - - try { - $txn_editor->applyTransactions( - $object->getApplicationTransactionObject(), - array($txn_template)); - } catch (PhabricatorEdgeCycleException $ex) { - $this->raiseGraphCycleException($ex); - } - - return id(new AphrontReloadResponse())->setURI($handle->getURI()); - } else { - return $this->performMerge($object, $handle, $phids); - } - } else { - if ($edge_type) { - $phids = PhabricatorEdgeQuery::loadDestinationPHIDs( - $phid, - $edge_type); - } else { - // This is a merge. - $phids = array(); - } - } - - $strings = $this->getStrings($attach_type, $action); - - $handles = $this->loadViewerHandles($phids); - - $obj_dialog = new PhabricatorObjectSelectorDialog(); - $obj_dialog - ->setUser($user) - ->setHandles($handles) - ->setFilters($this->getFilters($strings, $attach_type)) - ->setSelectedFilter($strings['selected']) - ->setExcluded($phid) - ->setCancelURI($handle->getURI()) - ->setSearchURI('/search/select/'.$attach_type.'/'.$action.'/') - ->setTitle($strings['title']) - ->setHeader($strings['header']) - ->setButtonText($strings['button']) - ->setInstructions($strings['instructions']); - - $dialog = $obj_dialog->buildDialog(); - - return id(new AphrontDialogResponse())->setDialog($dialog); - } - - private function performMerge( - ManiphestTask $task, - PhabricatorObjectHandle $handle, - array $phids) { - - $user = $this->getRequest()->getUser(); - $response = id(new AphrontReloadResponse())->setURI($handle->getURI()); - - $phids = array_fill_keys($phids, true); - unset($phids[$task->getPHID()]); // Prevent merging a task into itself. - - if (!$phids) { - return $response; - } - - $targets = id(new ManiphestTaskQuery()) - ->setViewer($user) - ->requireCapabilities( - array( - PhabricatorPolicyCapability::CAN_VIEW, - PhabricatorPolicyCapability::CAN_EDIT, - )) - ->withPHIDs(array_keys($phids)) - ->needSubscriberPHIDs(true) - ->needProjectPHIDs(true) - ->execute(); - - if (empty($targets)) { - return $response; - } - - $editor = id(new ManiphestTransactionEditor()) - ->setActor($user) - ->setContentSourceFromRequest($this->getRequest()) - ->setContinueOnNoEffect(true) - ->setContinueOnMissingFields(true); - - $cc_vector = array(); - // since we loaded this via a generic object query, go ahead and get the - // attach the subscriber and project phids now - $task->attachSubscriberPHIDs( - PhabricatorSubscribersQuery::loadSubscribersForPHID($task->getPHID())); - $task->attachProjectPHIDs( - PhabricatorEdgeQuery::loadDestinationPHIDs($task->getPHID(), - PhabricatorProjectObjectHasProjectEdgeType::EDGECONST)); - - $cc_vector[] = $task->getSubscriberPHIDs(); - foreach ($targets as $target) { - $cc_vector[] = $target->getSubscriberPHIDs(); - $cc_vector[] = array( - $target->getAuthorPHID(), - $target->getOwnerPHID(), - ); - - $merged_into_txn = id(new ManiphestTransaction()) - ->setTransactionType(ManiphestTransaction::TYPE_MERGED_INTO) - ->setNewValue($task->getPHID()); - - $editor->applyTransactions( - $target, - array($merged_into_txn)); - - } - $all_ccs = array_mergev($cc_vector); - $all_ccs = array_filter($all_ccs); - $all_ccs = array_unique($all_ccs); - - $add_ccs = id(new ManiphestTransaction()) - ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS) - ->setNewValue(array('=' => $all_ccs)); - - $merged_from_txn = id(new ManiphestTransaction()) - ->setTransactionType(ManiphestTransaction::TYPE_MERGED_FROM) - ->setNewValue(mpull($targets, 'getPHID')); - - $editor->applyTransactions( - $task, - array($add_ccs, $merged_from_txn)); - - return $response; - } - - private function getStrings($attach_type, $action) { - switch ($attach_type) { - case DifferentialRevisionPHIDType::TYPECONST: - $noun = pht('Revisions'); - $selected = pht('created'); - break; - case ManiphestTaskPHIDType::TYPECONST: - $noun = pht('Tasks'); - $selected = pht('assigned'); - break; - case PhabricatorRepositoryCommitPHIDType::TYPECONST: - $noun = pht('Commits'); - $selected = pht('created'); - break; - case PholioMockPHIDType::TYPECONST: - $noun = pht('Mocks'); - $selected = pht('created'); - break; - } - - switch ($action) { - case self::ACTION_EDGE: - case self::ACTION_ATTACH: - $dialog_title = pht('Manage Attached %s', $noun); - $header_text = pht('Currently Attached %s', $noun); - $button_text = pht('Save %s', $noun); - $instructions = null; - break; - case self::ACTION_MERGE: - $dialog_title = pht('Merge Duplicate Tasks'); - $header_text = pht('Tasks To Merge'); - $button_text = pht('Merge %s', $noun); - $instructions = pht( - 'These tasks will be merged into the current task and then closed. '. - 'The current task will grow stronger.'); - break; - case self::ACTION_DEPENDENCIES: - $dialog_title = pht('Edit Dependencies'); - $header_text = pht('Current Dependencies'); - $button_text = pht('Save Dependencies'); - $instructions = null; - break; - case self::ACTION_BLOCKS: - $dialog_title = pht('Edit Blocking Tasks'); - $header_text = pht('Current Blocking Tasks'); - $button_text = pht('Save Blocking Tasks'); - $instructions = null; - break; - } - - return array( - 'target_plural_noun' => $noun, - 'selected' => $selected, - 'title' => $dialog_title, - 'header' => $header_text, - 'button' => $button_text, - 'instructions' => $instructions, - ); - } - - private function getFilters(array $strings, $attach_type) { - if ($attach_type == PholioMockPHIDType::TYPECONST) { - $filters = array( - 'created' => pht('Created By Me'), - 'all' => pht('All %s', $strings['target_plural_noun']), - ); - } else { - $filters = array( - 'assigned' => pht('Assigned to Me'), - 'created' => pht('Created By Me'), - 'open' => pht('All Open %s', $strings['target_plural_noun']), - 'all' => pht('All %s', $strings['target_plural_noun']), - ); - } - - return $filters; - } - - private function getEdgeType($src_type, $dst_type) { - $t_cmit = PhabricatorRepositoryCommitPHIDType::TYPECONST; - $t_task = ManiphestTaskPHIDType::TYPECONST; - $t_drev = DifferentialRevisionPHIDType::TYPECONST; - $t_mock = PholioMockPHIDType::TYPECONST; - - $map = array( - $t_cmit => array( - $t_task => DiffusionCommitHasTaskEdgeType::EDGECONST, - ), - $t_task => array( - $t_cmit => ManiphestTaskHasCommitEdgeType::EDGECONST, - $t_task => ManiphestTaskDependsOnTaskEdgeType::EDGECONST, - $t_drev => ManiphestTaskHasRevisionEdgeType::EDGECONST, - $t_mock => ManiphestTaskHasMockEdgeType::EDGECONST, - ), - $t_drev => array( - $t_drev => DifferentialRevisionDependsOnRevisionEdgeType::EDGECONST, - $t_task => DifferentialRevisionHasTaskEdgeType::EDGECONST, - ), - $t_mock => array( - $t_task => PholioMockHasTaskEdgeType::EDGECONST, - ), - ); - - if (empty($map[$src_type][$dst_type])) { - return null; - } - - return $map[$src_type][$dst_type]; - } - - private function raiseGraphCycleException(PhabricatorEdgeCycleException $ex) { - $cycle = $ex->getCycle(); - - $handles = $this->loadViewerHandles($cycle); - $names = array(); - foreach ($cycle as $cycle_phid) { - $names[] = $handles[$cycle_phid]->getFullName(); - } - throw new Exception( - pht( - 'You can not create that dependency, because it would create a '. - 'circular dependency: %s.', - implode(" \xE2\x86\x92 ", $names))); - } - -} diff --git a/src/applications/search/controller/PhabricatorSearchRelationshipController.php b/src/applications/search/controller/PhabricatorSearchRelationshipController.php index 2b2fac9bb7..767c756813 100644 --- a/src/applications/search/controller/PhabricatorSearchRelationshipController.php +++ b/src/applications/search/controller/PhabricatorSearchRelationshipController.php @@ -19,9 +19,16 @@ final class PhabricatorSearchRelationshipController $src_phid = $object->getPHID(); $edge_type = $relationship->getEdgeConstant(); - $dst_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( - $src_phid, - $edge_type); + // If this is a normal relationship, users can remove related objects. If + // it's a special relationship like a merge, we can't undo it, so we won't + // prefill the current related objects. + if ($relationship->canUndoRelationship()) { + $dst_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( + $src_phid, + $edge_type); + } else { + $dst_phids = array(); + } $all_phids = $dst_phids; $all_phids[] = $src_phid; @@ -44,12 +51,16 @@ final class PhabricatorSearchRelationshipController // relationships at the same time don't race and overwrite one another. $add_phids = array_diff($phids, $initial_phids); $rem_phids = array_diff($initial_phids, $phids); + $all_phids = array_merge($add_phids, $rem_phids); - if ($add_phids) { + $capabilities = $relationship->getRequiredRelationshipCapabilities(); + + if ($all_phids) { $dst_objects = id(new PhabricatorObjectQuery()) ->setViewer($viewer) - ->withPHIDs($phids) + ->withPHIDs($all_phids) ->setRaisePolicyExceptions(true) + ->requireCapabilities($capabilities) ->execute(); $dst_objects = mpull($dst_objects, null, 'getPHID'); } else { @@ -67,6 +78,14 @@ final class PhabricatorSearchRelationshipController $add_phid)); } + if ($add_phid == $src_phid) { + throw new Exception( + pht( + 'You can not create a relationship to object "%s" because '. + 'objects can not be related to themselves.', + $add_phid)); + } + if (!$relationship->canRelateObjects($object, $dst_object)) { throw new Exception( pht( @@ -81,9 +100,12 @@ final class PhabricatorSearchRelationshipController return $this->newUnrelatableObjectResponse($ex, $done_uri); } + $content_source = PhabricatorContentSource::newFromRequest($request); + $relationship->setContentSource($content_source); + $editor = $object->getApplicationTransactionEditor() ->setActor($viewer) - ->setContentSourceFromRequest($request) + ->setContentSource($content_source) ->setContinueOnMissingFields(true) ->setContinueOnNoEffect(true); @@ -96,9 +118,29 @@ final class PhabricatorSearchRelationshipController '-' => array_fuse($rem_phids), )); + $add_objects = array_select_keys($dst_objects, $add_phids); + $rem_objects = array_select_keys($dst_objects, $rem_phids); + + if ($add_objects || $rem_objects) { + $more_xactions = $relationship->willUpdateRelationships( + $object, + $add_objects, + $rem_objects); + foreach ($more_xactions as $xaction) { + $xactions[] = $xaction; + } + } + try { $editor->applyTransactions($object, $xactions); + if ($add_objects || $rem_objects) { + $relationship->didUpdateRelationships( + $object, + $add_objects, + $rem_objects); + } + return id(new AphrontRedirectResponse())->setURI($done_uri); } catch (PhabricatorEdgeCycleException $ex) { return $this->newGraphCycleResponse($ex, $done_uri); diff --git a/src/applications/search/controller/PhabricatorSearchRelationshipSourceController.php b/src/applications/search/controller/PhabricatorSearchRelationshipSourceController.php index e783eae7f4..4a96727a29 100644 --- a/src/applications/search/controller/PhabricatorSearchRelationshipSourceController.php +++ b/src/applications/search/controller/PhabricatorSearchRelationshipSourceController.php @@ -58,7 +58,7 @@ final class PhabricatorSearchRelationshipSourceController ->execute(); $phids = array_fill_keys(mpull($results, 'getPHID'), true); - $phids += $this->queryObjectNames($query_str, $capabilities); + $phids = $this->queryObjectNames($query, $capabilities) + $phids; $phids = array_keys($phids); $handles = $viewer->loadHandles($phids); @@ -72,15 +72,21 @@ final class PhabricatorSearchRelationshipSourceController return id(new AphrontAjaxResponse())->setContent($data); } - private function queryObjectNames($query, $capabilities) { + private function queryObjectNames( + PhabricatorSavedQuery $query, + array $capabilities) { + $request = $this->getRequest(); $viewer = $request->getUser(); + $types = $query->getParameter('types'); + $match = $query->getParameter('query'); + $objects = id(new PhabricatorObjectQuery()) ->setViewer($viewer) ->requireCapabilities($capabilities) - ->withTypes(array($request->getURIData('type'))) - ->withNames(array($query)) + ->withTypes($query->getParameter('types')) + ->withNames(array($match)) ->execute(); return mpull($objects, 'getPHID'); diff --git a/src/applications/search/controller/PhabricatorSearchSelectController.php b/src/applications/search/controller/PhabricatorSearchSelectController.php deleted file mode 100644 index f663cd03d7..0000000000 --- a/src/applications/search/controller/PhabricatorSearchSelectController.php +++ /dev/null @@ -1,86 +0,0 @@ -getUser(); - $type = $request->getURIData('type'); - $action = $request->getURIData('action'); - - $query = new PhabricatorSavedQuery(); - $query_str = $request->getStr('query'); - - $query->setEngineClassName('PhabricatorSearchApplicationSearchEngine'); - $query->setParameter('query', $query_str); - $query->setParameter('types', array($type)); - - $status_open = PhabricatorSearchRelationship::RELATIONSHIP_OPEN; - - switch ($request->getStr('filter')) { - case 'assigned': - $query->setParameter('ownerPHIDs', array($user->getPHID())); - $query->setParameter('statuses', array($status_open)); - break; - case 'created'; - $query->setParameter('authorPHIDs', array($user->getPHID())); - // TODO - if / when we allow pholio mocks to be archived, etc - // update this - if ($type != PholioMockPHIDType::TYPECONST) { - $query->setParameter('statuses', array($status_open)); - } - break; - case 'open': - $query->setParameter('statuses', array($status_open)); - break; - } - - $query->setParameter('excludePHIDs', array($request->getStr('exclude'))); - - $capabilities = array(PhabricatorPolicyCapability::CAN_VIEW); - switch ($action) { - case self::ACTION_MERGE: - $capabilities[] = PhabricatorPolicyCapability::CAN_EDIT; - break; - default: - break; - } - - $results = id(new PhabricatorSearchDocumentQuery()) - ->setViewer($user) - ->requireObjectCapabilities($capabilities) - ->withSavedQuery($query) - ->setOffset(0) - ->setLimit(100) - ->execute(); - - $phids = array_fill_keys(mpull($results, 'getPHID'), true); - $phids += $this->queryObjectNames($query_str, $capabilities); - - $phids = array_keys($phids); - $handles = $this->loadViewerHandles($phids); - - $data = array(); - foreach ($handles as $handle) { - $view = new PhabricatorHandleObjectSelectorDataView($handle); - $data[] = $view->renderData(); - } - - return id(new AphrontAjaxResponse())->setContent($data); - } - - private function queryObjectNames($query, $capabilities) { - $request = $this->getRequest(); - $viewer = $request->getUser(); - - $objects = id(new PhabricatorObjectQuery()) - ->setViewer($viewer) - ->requireCapabilities($capabilities) - ->withTypes(array($request->getURIData('type'))) - ->withNames(array($query)) - ->execute(); - - return mpull($objects, 'getPHID'); - } - -} diff --git a/src/applications/search/relationship/PhabricatorObjectRelationship.php b/src/applications/search/relationship/PhabricatorObjectRelationship.php index 9711dc1719..50b5299952 100644 --- a/src/applications/search/relationship/PhabricatorObjectRelationship.php +++ b/src/applications/search/relationship/PhabricatorObjectRelationship.php @@ -3,6 +3,7 @@ abstract class PhabricatorObjectRelationship extends Phobject { private $viewer; + private $contentSource; public function setViewer(PhabricatorUser $viewer) { $this->viewer = $viewer; @@ -13,6 +14,15 @@ abstract class PhabricatorObjectRelationship extends Phobject { return $this->viewer; } + public function setContentSource(PhabricatorContentSource $content_source) { + $this->contentSource = $content_source; + return $this; + } + + public function getContentSource() { + return $this->contentSource; + } + final public function getRelationshipConstant() { return $this->getPhobjectClassConstant('RELATIONSHIPKEY'); } @@ -94,4 +104,16 @@ abstract class PhabricatorObjectRelationship extends Phobject { return "/search/rel/{$type}/{$phid}/"; } + public function canUndoRelationship() { + return true; + } + + public function willUpdateRelationships($object, array $add, array $rem) { + return array(); + } + + public function didUpdateRelationships($object, array $add, array $rem) { + return; + } + } diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index 53259984b4..e130522bb2 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -609,6 +609,8 @@ abstract class PhabricatorApplicationTransaction $edge_type = $this->getMetadataValue('edge:type'); switch ($edge_type) { case PhabricatorObjectMentionsObjectEdgeType::EDGECONST: + case ManiphestTaskHasDuplicateTaskEdgeType::EDGECONST: + case ManiphestTaskIsDuplicateOfTaskEdgeType::EDGECONST: return true; break; case PhabricatorObjectMentionedByObjectEdgeType::EDGECONST: