Refine available filters and defaults for relationship selection
Summary: Ref T4788. Fixes T10703. In the longer term I want to put this on top of ApplicationSearch, but that's somewhat complex and we're at a fairly good point to pause this feature for feedback. Inch toward that instead: provide more appropriate filters and defaults without rebuilding the underlying engine. Specifically: - No "assigned" for commits (barely makes sense). - No "assigned" for mocks (does not make sense). - Default to "open" for parent tasks, subtasks, close as duplicate, and merge into. Also, add a key to the `search_document` table to improve the performance of the "all open stuff of type X" query. "All Open Tasks" is about 100x faster on my machine with this key. Test Plan: - Clicked all object relationships, saw more sensible filters and defaults. - Saw "open" query about 100x faster locally (300ms to 3ms). Reviewers: chad Reviewed By: chad Maniphest Tasks: T4788, T10703 Differential Revision: https://secure.phabricator.com/D16202
This commit is contained in:
		| @@ -38,7 +38,8 @@ final class ManiphestTaskCloseAsDuplicateRelationship | |||||||
|   } |   } | ||||||
|  |  | ||||||
|   protected function newRelationshipSource() { |   protected function newRelationshipSource() { | ||||||
|     return new ManiphestTaskRelationshipSource(); |     return id(new ManiphestTaskRelationshipSource()) | ||||||
|  |       ->setSelectedFilter('open'); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   public function getRequiredRelationshipCapabilities() { |   public function getRequiredRelationshipCapabilities() { | ||||||
|   | |||||||
| @@ -38,7 +38,8 @@ final class ManiphestTaskHasParentRelationship | |||||||
|   } |   } | ||||||
|  |  | ||||||
|   protected function newRelationshipSource() { |   protected function newRelationshipSource() { | ||||||
|     return new ManiphestTaskRelationshipSource(); |     return id(new ManiphestTaskRelationshipSource()) | ||||||
|  |       ->setSelectedFilter('open'); | ||||||
|   } |   } | ||||||
|  |  | ||||||
| } | } | ||||||
|   | |||||||
| @@ -38,7 +38,8 @@ final class ManiphestTaskHasSubtaskRelationship | |||||||
|   } |   } | ||||||
|  |  | ||||||
|   protected function newRelationshipSource() { |   protected function newRelationshipSource() { | ||||||
|     return new ManiphestTaskRelationshipSource(); |     return id(new ManiphestTaskRelationshipSource()) | ||||||
|  |       ->setSelectedFilter('open'); | ||||||
|   } |   } | ||||||
|  |  | ||||||
| } | } | ||||||
|   | |||||||
| @@ -38,7 +38,8 @@ final class ManiphestTaskMergeInRelationship | |||||||
|   } |   } | ||||||
|  |  | ||||||
|   protected function newRelationshipSource() { |   protected function newRelationshipSource() { | ||||||
|     return new ManiphestTaskRelationshipSource(); |     return id(new ManiphestTaskRelationshipSource()) | ||||||
|  |       ->setSelectedFilter('open'); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   public function getRequiredRelationshipCapabilities() { |   public function getRequiredRelationshipCapabilities() { | ||||||
|   | |||||||
| @@ -150,14 +150,6 @@ final class PhabricatorSearchRelationshipController | |||||||
|     $handles = iterator_to_array($handles); |     $handles = iterator_to_array($handles); | ||||||
|     $handles = array_select_keys($handles, $dst_phids); |     $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_title = $relationship->getDialogTitleText(); | ||||||
|     $dialog_header = $relationship->getDialogHeaderText(); |     $dialog_header = $relationship->getDialogHeaderText(); | ||||||
|     $dialog_button = $relationship->getDialogButtonText(); |     $dialog_button = $relationship->getDialogButtonText(); | ||||||
| @@ -165,12 +157,17 @@ final class PhabricatorSearchRelationshipController | |||||||
|  |  | ||||||
|     $source_uri = $relationship->getSourceURI($object); |     $source_uri = $relationship->getSourceURI($object); | ||||||
|  |  | ||||||
|  |     $source = $relationship->newSource(); | ||||||
|  |  | ||||||
|  |     $filters = $source->getFilters(); | ||||||
|  |     $selected_filter = $source->getSelectedFilter(); | ||||||
|  |  | ||||||
|     return id(new PhabricatorObjectSelectorDialog()) |     return id(new PhabricatorObjectSelectorDialog()) | ||||||
|       ->setUser($viewer) |       ->setUser($viewer) | ||||||
|       ->setInitialPHIDs($initial_phids) |       ->setInitialPHIDs($initial_phids) | ||||||
|       ->setHandles($handles) |       ->setHandles($handles) | ||||||
|       ->setFilters($filters) |       ->setFilters($filters) | ||||||
|       ->setSelectedFilter('created') |       ->setSelectedFilter($selected_filter) | ||||||
|       ->setExcluded($src_phid) |       ->setExcluded($src_phid) | ||||||
|       ->setCancelURI($done_uri) |       ->setCancelURI($done_uri) | ||||||
|       ->setSearchURI($source_uri) |       ->setSearchURI($source_uri) | ||||||
|   | |||||||
| @@ -17,4 +17,10 @@ final class DiffusionCommitRelationshipSource | |||||||
|     ); |     ); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   public function getFilters() { | ||||||
|  |     $filters = parent::getFilters(); | ||||||
|  |     unset($filters['assigned']); | ||||||
|  |     return $filters; | ||||||
|  |   } | ||||||
|  |  | ||||||
| } | } | ||||||
|   | |||||||
| @@ -3,6 +3,7 @@ | |||||||
| abstract class PhabricatorObjectRelationshipSource extends Phobject { | abstract class PhabricatorObjectRelationshipSource extends Phobject { | ||||||
|  |  | ||||||
|   private $viewer; |   private $viewer; | ||||||
|  |   private $selectedFilter; | ||||||
|  |  | ||||||
|   final public function setViewer(PhabricatorUser $viewer) { |   final public function setViewer(PhabricatorUser $viewer) { | ||||||
|     $this->viewer = $viewer; |     $this->viewer = $viewer; | ||||||
| @@ -16,4 +17,32 @@ abstract class PhabricatorObjectRelationshipSource extends Phobject { | |||||||
|   abstract public function isEnabledForObject($object); |   abstract public function isEnabledForObject($object); | ||||||
|   abstract public function getResultPHIDTypes(); |   abstract public function getResultPHIDTypes(); | ||||||
|  |  | ||||||
|  |   protected function getDefaultFilter() { | ||||||
|  |     return 'created'; | ||||||
|  |   } | ||||||
|  |  | ||||||
|  |   final public function setSelectedFilter($selected_filter) { | ||||||
|  |     $this->selectedFilter = $selected_filter; | ||||||
|  |     return $this; | ||||||
|  |   } | ||||||
|  |  | ||||||
|  |   final public function getSelectedFilter() { | ||||||
|  |     if ($this->selectedFilter === null) { | ||||||
|  |       return $this->getDefaultFilter(); | ||||||
|  |     } | ||||||
|  |  | ||||||
|  |     return $this->selectedFilter; | ||||||
|  |   } | ||||||
|  |  | ||||||
|  |   public function getFilters() { | ||||||
|  |     // TODO: These are hard-coded for now, and all of this will probably be | ||||||
|  |     // rewritten when we move to ApplicationSearch. | ||||||
|  |     return array( | ||||||
|  |       'assigned' => pht('Assigned to Me'), | ||||||
|  |       'created' => pht('Created By Me'), | ||||||
|  |       'open' => pht('All Open Objects'), | ||||||
|  |       'all' => pht('All Objects'), | ||||||
|  |     ); | ||||||
|  |   } | ||||||
|  |  | ||||||
| } | } | ||||||
|   | |||||||
| @@ -17,4 +17,10 @@ final class PholioMockRelationshipSource | |||||||
|     ); |     ); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   public function getFilters() { | ||||||
|  |     $filters = parent::getFilters(); | ||||||
|  |     unset($filters['assigned']); | ||||||
|  |     return $filters; | ||||||
|  |   } | ||||||
|  |  | ||||||
| } | } | ||||||
|   | |||||||
| @@ -26,6 +26,9 @@ final class PhabricatorSearchDocument extends PhabricatorSearchDAO { | |||||||
|         'documentCreated' => array( |         'documentCreated' => array( | ||||||
|           'columns' => array('documentCreated'), |           'columns' => array('documentCreated'), | ||||||
|         ), |         ), | ||||||
|  |         'key_type' => array( | ||||||
|  |           'columns' => array('documentType', 'documentCreated'), | ||||||
|  |         ), | ||||||
|       ), |       ), | ||||||
|     ) + parent::getConfiguration(); |     ) + parent::getConfiguration(); | ||||||
|   } |   } | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 epriestley
					epriestley