From de379c8b61b8a6035aa9cc020b1abdd51e4ad414 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 12 Feb 2016 06:19:26 -0800 Subject: [PATCH] Allow workboard sorting and filtering to be saved as defaults Summary: Fixes T6641. This allows users who have permission to edit a project to use "Save as Default" to save the current order and filter as defaults for the project. These are per-board defaults, and apply to all users. The rationale is that I think the best default ordering/filtering depends mostly on the board, not the viewer. This seems to align with most requests in the task, although rationale is a bit light. But, for example, it seems reasonable you might want to change the default filter to "All Tasks" on a sprint board, so you can see what's in the "Done" column. This also fixes some minor issues I ran into: - Herald could hit an issue while checking permissions if the project was a subproject and a non-member had a triggering rule. - "Advanced filter..." did not prefill with the current filter. Test Plan: - Set default order and filter on a workboard. - Reloaded board, saw settings stick. - Tried to edit a board as an unprivileged user (disabled menu items, error). - Reviewed transaction log. Reviewers: chad Reviewed By: chad Maniphest Tasks: T6641 Differential Revision: https://secure.phabricator.com/D15260 --- resources/sql/autopatches/20160212.proj.1.sql | 2 + resources/sql/autopatches/20160212.proj.2.sql | 2 + src/__phutil_library_map__.php | 2 + .../PhabricatorProjectApplication.php | 2 + .../PhabricatorProjectBoardViewController.php | 123 +++++++++++++++--- .../PhabricatorProjectDefaultController.php | 90 +++++++++++++ .../PhabricatorProjectTransactionEditor.php | 26 +++- .../project/storage/PhabricatorProject.php | 30 +++++ .../storage/PhabricatorProjectTransaction.php | 48 +++++-- 9 files changed, 293 insertions(+), 32 deletions(-) create mode 100644 resources/sql/autopatches/20160212.proj.1.sql create mode 100644 resources/sql/autopatches/20160212.proj.2.sql create mode 100644 src/applications/project/controller/PhabricatorProjectDefaultController.php diff --git a/resources/sql/autopatches/20160212.proj.1.sql b/resources/sql/autopatches/20160212.proj.1.sql new file mode 100644 index 0000000000..7d8c19b0b1 --- /dev/null +++ b/resources/sql/autopatches/20160212.proj.1.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_project.project + ADD properties LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20160212.proj.2.sql b/resources/sql/autopatches/20160212.proj.2.sql new file mode 100644 index 0000000000..f6f793aec4 --- /dev/null +++ b/resources/sql/autopatches/20160212.proj.2.sql @@ -0,0 +1,2 @@ +UPDATE {$NAMESPACE}_project.project + SET properties = '{}' WHERE properties = ''; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index b66d31b3c0..aacf85ac45 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2896,6 +2896,7 @@ phutil_register_library_map(array( 'PhabricatorProjectCustomFieldStringIndex' => 'applications/project/storage/PhabricatorProjectCustomFieldStringIndex.php', 'PhabricatorProjectDAO' => 'applications/project/storage/PhabricatorProjectDAO.php', 'PhabricatorProjectDatasource' => 'applications/project/typeahead/PhabricatorProjectDatasource.php', + 'PhabricatorProjectDefaultController' => 'applications/project/controller/PhabricatorProjectDefaultController.php', 'PhabricatorProjectDescriptionField' => 'applications/project/customfield/PhabricatorProjectDescriptionField.php', 'PhabricatorProjectDetailsProfilePanel' => 'applications/project/profilepanel/PhabricatorProjectDetailsProfilePanel.php', 'PhabricatorProjectEditController' => 'applications/project/controller/PhabricatorProjectEditController.php', @@ -7332,6 +7333,7 @@ phutil_register_library_map(array( 'PhabricatorProjectCustomFieldStringIndex' => 'PhabricatorCustomFieldStringIndexStorage', 'PhabricatorProjectDAO' => 'PhabricatorLiskDAO', 'PhabricatorProjectDatasource' => 'PhabricatorTypeaheadDatasource', + 'PhabricatorProjectDefaultController' => 'PhabricatorProjectBoardController', 'PhabricatorProjectDescriptionField' => 'PhabricatorProjectStandardCustomField', 'PhabricatorProjectDetailsProfilePanel' => 'PhabricatorProfilePanel', 'PhabricatorProjectEditController' => 'PhabricatorProjectController', diff --git a/src/applications/project/application/PhabricatorProjectApplication.php b/src/applications/project/application/PhabricatorProjectApplication.php index 2d00c4c90c..3a39aefa22 100644 --- a/src/applications/project/application/PhabricatorProjectApplication.php +++ b/src/applications/project/application/PhabricatorProjectApplication.php @@ -94,6 +94,8 @@ final class PhabricatorProjectApplication extends PhabricatorApplication { => 'PhabricatorProjectSilenceController', 'warning/(?P[1-9]\d*)/' => 'PhabricatorProjectSubprojectWarningController', + 'default/(?P[1-9]\d*)/(?P[^/]+)/' + => 'PhabricatorProjectDefaultController', ), '/tag/' => array( '(?P[^/]+)/' => 'PhabricatorProjectViewController', diff --git a/src/applications/project/controller/PhabricatorProjectBoardViewController.php b/src/applications/project/controller/PhabricatorProjectBoardViewController.php index 0d977c2aca..e9a5159295 100644 --- a/src/applications/project/controller/PhabricatorProjectBoardViewController.php +++ b/src/applications/project/controller/PhabricatorProjectBoardViewController.php @@ -8,7 +8,6 @@ final class PhabricatorProjectBoardViewController private $id; private $slug; private $queryKey; - private $filter; private $sortKey; private $showHidden; @@ -56,10 +55,18 @@ final class PhabricatorProjectBoardViewController $search_engine->getQueryResultsPageURI($saved->getQueryKey()))); } - $query_key = $request->getURIData('queryKey'); - if (!$query_key) { - $query_key = 'open'; + $query_key = $this->getDefaultFilter($project); + + $request_query = $request->getStr('filter'); + if (strlen($request_query)) { + $query_key = $request_query; } + + $uri_query = $request->getURIData('queryKey'); + if (strlen($uri_query)) { + $query_key = $uri_query; + } + $this->queryKey = $query_key; $custom_query = null; @@ -382,10 +389,12 @@ final class PhabricatorProjectBoardViewController $sort_menu = $this->buildSortMenu( $viewer, + $project, $this->sortKey); $filter_menu = $this->buildFilterMenu( $viewer, + $project, $custom_query, $search_engine, $query_key); @@ -445,20 +454,49 @@ final class PhabricatorProjectBoardViewController $this->showHidden = $request->getBool('hidden'); $this->id = $project->getID(); - $sort_key = $request->getStr('order'); - switch ($sort_key) { + $sort_key = $this->getDefaultSort($project); + + $request_sort = $request->getStr('order'); + if ($this->isValidSort($request_sort)) { + $sort_key = $request_sort; + } + + $this->sortKey = $sort_key; + } + + private function getDefaultSort(PhabricatorProject $project) { + $default_sort = $project->getDefaultWorkboardSort(); + + if ($this->isValidSort($default_sort)) { + return $default_sort; + } + + return PhabricatorProjectColumn::DEFAULT_ORDER; + } + + private function getDefaultFilter(PhabricatorProject $project) { + $default_filter = $project->getDefaultWorkboardFilter(); + + if (strlen($default_filter)) { + return $default_filter; + } + + return 'open'; + } + + private function isValidSort($sort) { + switch ($sort) { case PhabricatorProjectColumn::ORDER_NATURAL: case PhabricatorProjectColumn::ORDER_PRIORITY: - break; - default: - $sort_key = PhabricatorProjectColumn::DEFAULT_ORDER; - break; + return true; } - $this->sortKey = $sort_key; + + return false; } private function buildSortMenu( PhabricatorUser $viewer, + PhabricatorProject $project, $sort_key) { $sort_icon = id(new PHUIIconView()) @@ -489,6 +527,24 @@ final class PhabricatorProjectBoardViewController $items[] = $item; } + $id = $project->getID(); + + $save_uri = "default/{$id}/sort/"; + $save_uri = $this->getApplicationURI($save_uri); + $save_uri = $this->getURIWithState($save_uri, $force = true); + + $can_edit = PhabricatorPolicyFilter::hasCapability( + $viewer, + $project, + PhabricatorPolicyCapability::CAN_EDIT); + + $items[] = id(new PhabricatorActionView()) + ->setIcon('fa-floppy-o') + ->setName(pht('Save as Default')) + ->setHref($save_uri) + ->setWorkflow(true) + ->setDisabled(!$can_edit); + $sort_menu = id(new PhabricatorActionListView()) ->setUser($viewer); foreach ($items as $item) { @@ -507,8 +563,10 @@ final class PhabricatorProjectBoardViewController return $sort_button; } + private function buildFilterMenu( PhabricatorUser $viewer, + PhabricatorProject $project, $custom_query, PhabricatorApplicationSearchEngine $engine, $query_key) { @@ -551,18 +609,40 @@ final class PhabricatorProjectBoardViewController $uri = $engine->getQueryResultsPageURI($key); } - $uri = $this->getURIWithState($uri); + $uri = $this->getURIWithState($uri) + ->setQueryParam('filter', null); $item->setHref($uri); $items[] = $item; } + $id = $project->getID(); + + $filter_uri = $this->getApplicationURI("board/{$id}/filter/"); + $filter_uri = $this->getURIWithState($filter_uri, $force = true); + $items[] = id(new PhabricatorActionView()) ->setIcon('fa-cog') - ->setHref($this->getApplicationURI('board/'.$this->id.'/filter/')) + ->setHref($filter_uri) ->setWorkflow(true) ->setName(pht('Advanced Filter...')); + $save_uri = "default/{$id}/filter/"; + $save_uri = $this->getApplicationURI($save_uri); + $save_uri = $this->getURIWithState($save_uri, $force = true); + + $can_edit = PhabricatorPolicyFilter::hasCapability( + $viewer, + $project, + PhabricatorPolicyCapability::CAN_EDIT); + + $items[] = id(new PhabricatorActionView()) + ->setIcon('fa-floppy-o') + ->setName(pht('Save as Default')) + ->setHref($save_uri) + ->setWorkflow(true) + ->setDisabled(!$can_edit); + $filter_menu = id(new PhabricatorActionListView()) ->setUser($viewer); foreach ($items as $item) { @@ -793,22 +873,31 @@ final class PhabricatorProjectBoardViewController * the rest of the board state persistent. If no URI is provided, this method * starts with the request URI. * - * @param string|null URI to add state parameters to. - * @return PhutilURI URI with state parameters. + * @param string|null URI to add state parameters to. + * @param bool True to explicitly include all state. + * @return PhutilURI URI with state parameters. */ - private function getURIWithState($base = null) { + private function getURIWithState($base = null, $force = false) { + $project = $this->getProject(); + if ($base === null) { $base = $this->getRequest()->getRequestURI(); } $base = new PhutilURI($base); - if ($this->sortKey != PhabricatorProjectColumn::DEFAULT_ORDER) { + if ($force || ($this->sortKey != $this->getDefaultSort($project))) { $base->setQueryParam('order', $this->sortKey); } else { $base->setQueryParam('order', null); } + if ($force || ($this->queryKey != $this->getDefaultFilter($project))) { + $base->setQueryParam('filter', $this->queryKey); + } else { + $base->setQueryParam('filter', null); + } + $base->setQueryParam('hidden', $this->showHidden ? 'true' : null); return $base; diff --git a/src/applications/project/controller/PhabricatorProjectDefaultController.php b/src/applications/project/controller/PhabricatorProjectDefaultController.php new file mode 100644 index 0000000000..0246f33f43 --- /dev/null +++ b/src/applications/project/controller/PhabricatorProjectDefaultController.php @@ -0,0 +1,90 @@ +getViewer(); + $project_id = $request->getURIData('projectID'); + + $project = id(new PhabricatorProjectQuery()) + ->setViewer($viewer) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->withIDs(array($project_id)) + ->executeOne(); + if (!$project) { + return new Aphront404Response(); + } + $this->setProject($project); + + $target = $request->getURIData('target'); + switch ($target) { + case 'filter': + $title = pht('Set Board Default Filter'); + $body = pht( + 'Make the current filter the new default filter for this board? '. + 'All users will see the new filter as the default when they view '. + 'the board.'); + $button = pht('Save Default Filter'); + + $xaction_value = $request->getStr('filter'); + $xaction_type = PhabricatorProjectTransaction::TYPE_DEFAULT_FILTER; + break; + case 'sort': + $title = pht('Set Board Default Order'); + $body = pht( + 'Make the current sort order the new default order for this board? '. + 'All users will see the new order as the default when they view '. + 'the board.'); + $button = pht('Save Default Order'); + + $xaction_value = $request->getStr('order'); + $xaction_type = PhabricatorProjectTransaction::TYPE_DEFAULT_SORT; + break; + default: + return new Aphront404Response(); + } + + $id = $project->getID(); + + $view_uri = $this->getApplicationURI("board/{$id}/"); + $view_uri = new PhutilURI($view_uri); + foreach ($request->getPassthroughRequestData() as $key => $value) { + $view_uri->setQueryParam($key, $value); + } + + if ($request->isFormPost()) { + $xactions = array(); + + $xactions[] = id(new PhabricatorProjectTransaction()) + ->setTransactionType($xaction_type) + ->setNewValue($xaction_value); + + id(new PhabricatorProjectTransactionEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true) + ->applyTransactions($project, $xactions); + + return id(new AphrontRedirectResponse())->setURI($view_uri); + } + + $dialog = $this->newDialog() + ->setTitle($title) + ->appendChild($body) + ->setDisableWorkflowOnCancel(true) + ->addCancelButton($view_uri) + ->addSubmitButton($title); + + foreach ($request->getPassthroughRequestData() as $key => $value) { + $dialog->addHiddenInput($key, $value); + } + + return $dialog; + } +} diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php index 51a120000a..c9594122d8 100644 --- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php +++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php @@ -40,6 +40,8 @@ final class PhabricatorProjectTransactionEditor $types[] = PhabricatorProjectTransaction::TYPE_PARENT; $types[] = PhabricatorProjectTransaction::TYPE_MILESTONE; $types[] = PhabricatorProjectTransaction::TYPE_HASWORKBOARD; + $types[] = PhabricatorProjectTransaction::TYPE_DEFAULT_SORT; + $types[] = PhabricatorProjectTransaction::TYPE_DEFAULT_FILTER; return $types; } @@ -71,6 +73,10 @@ final class PhabricatorProjectTransactionEditor case PhabricatorProjectTransaction::TYPE_PARENT: case PhabricatorProjectTransaction::TYPE_MILESTONE: return null; + case PhabricatorProjectTransaction::TYPE_DEFAULT_SORT: + return $object->getDefaultWorkboardSort(); + case PhabricatorProjectTransaction::TYPE_DEFAULT_FILTER: + return $object->getDefaultWorkboardFilter(); } return parent::getCustomTransactionOldValue($object, $xaction); @@ -89,6 +95,8 @@ final class PhabricatorProjectTransactionEditor case PhabricatorProjectTransaction::TYPE_LOCKED: case PhabricatorProjectTransaction::TYPE_PARENT: case PhabricatorProjectTransaction::TYPE_MILESTONE: + case PhabricatorProjectTransaction::TYPE_DEFAULT_SORT: + case PhabricatorProjectTransaction::TYPE_DEFAULT_FILTER: return $xaction->getNewValue(); case PhabricatorProjectTransaction::TYPE_HASWORKBOARD: return (int)$xaction->getNewValue(); @@ -139,6 +147,12 @@ final class PhabricatorProjectTransactionEditor case PhabricatorProjectTransaction::TYPE_HASWORKBOARD: $object->setHasWorkboard($xaction->getNewValue()); return; + case PhabricatorProjectTransaction::TYPE_DEFAULT_SORT: + $object->setDefaultWorkboardSort($xaction->getNewValue()); + return; + case PhabricatorProjectTransaction::TYPE_DEFAULT_FILTER: + $object->setDefaultWorkboardFilter($xaction->getNewValue()); + return; } return parent::applyCustomInternalTransaction($object, $xaction); @@ -181,6 +195,8 @@ final class PhabricatorProjectTransactionEditor case PhabricatorProjectTransaction::TYPE_PARENT: case PhabricatorProjectTransaction::TYPE_MILESTONE: case PhabricatorProjectTransaction::TYPE_HASWORKBOARD: + case PhabricatorProjectTransaction::TYPE_DEFAULT_SORT: + case PhabricatorProjectTransaction::TYPE_DEFAULT_FILTER: return; } @@ -866,8 +882,16 @@ final class PhabricatorProjectTransactionEditor PhabricatorLiskDAO $object, array $xactions) { + // Herald rules may run on behalf of other users and need to execute + // membership checks against ancestors. + $project = id(new PhabricatorProjectQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withPHIDs(array($object->getPHID())) + ->needAncestorMembers(true) + ->executeOne(); + return id(new PhabricatorProjectHeraldAdapter()) - ->setProject($object); + ->setProject($project); } } diff --git a/src/applications/project/storage/PhabricatorProject.php b/src/applications/project/storage/PhabricatorProject.php index 2d8b3f5218..861fac646b 100644 --- a/src/applications/project/storage/PhabricatorProject.php +++ b/src/applications/project/storage/PhabricatorProject.php @@ -36,6 +36,8 @@ final class PhabricatorProject extends PhabricatorProjectDAO protected $projectDepth; protected $projectPathKey; + protected $properties = array(); + private $memberPHIDs = self::ATTACHABLE; private $watcherPHIDs = self::ATTACHABLE; private $sparseWatchers = self::ATTACHABLE; @@ -198,6 +200,9 @@ final class PhabricatorProject extends PhabricatorProjectDAO protected function getConfiguration() { return array( self::CONFIG_AUX_PHID => true, + self::CONFIG_SERIALIZATION => array( + 'properties' => self::SERIALIZATION_JSON, + ), self::CONFIG_COLUMN_SCHEMA => array( 'name' => 'sort128', 'status' => 'text32', @@ -549,6 +554,31 @@ final class PhabricatorProject extends PhabricatorProjectDAO return idx($map, $color, $color); } + public function getProperty($key, $default = null) { + return idx($this->properties, $key, $default); + } + + public function setProperty($key, $value) { + $this->properties[$key] = $value; + return $this; + } + + public function getDefaultWorkboardSort() { + return $this->getProperty('workboard.sort.default'); + } + + public function setDefaultWorkboardSort($sort) { + return $this->setProperty('workboard.sort.default', $sort); + } + + public function getDefaultWorkboardFilter() { + return $this->getProperty('workboard.filter.default'); + } + + public function setDefaultWorkboardFilter($filter) { + return $this->setProperty('workboard.filter.default', $filter); + } + /* -( PhabricatorCustomFieldInterface )------------------------------------ */ diff --git a/src/applications/project/storage/PhabricatorProjectTransaction.php b/src/applications/project/storage/PhabricatorProjectTransaction.php index f668b0d2ec..e812952630 100644 --- a/src/applications/project/storage/PhabricatorProjectTransaction.php +++ b/src/applications/project/storage/PhabricatorProjectTransaction.php @@ -13,6 +13,8 @@ final class PhabricatorProjectTransaction const TYPE_PARENT = 'project:parent'; const TYPE_MILESTONE = 'project:milestone'; const TYPE_HASWORKBOARD = 'project:hasworkboard'; + const TYPE_DEFAULT_SORT = 'project:sort'; + const TYPE_DEFAULT_FILTER = 'project:filter'; // NOTE: This is deprecated, members are just a normal edge now. const TYPE_MEMBERS = 'project:members'; @@ -66,8 +68,29 @@ final class PhabricatorProjectTransaction return parent::getColor(); } - public function getIcon() { + public function shouldHideForFeed() { + switch ($this->getTransactionType()) { + case self::TYPE_HASWORKBOARD: + case self::TYPE_DEFAULT_SORT: + case self::TYPE_DEFAULT_FILTER: + return true; + } + return parent::shouldHideForFeed(); + } + + public function shouldHideForMail(array $xactions) { + switch ($this->getTransactionType()) { + case self::TYPE_HASWORKBOARD: + case self::TYPE_DEFAULT_SORT: + case self::TYPE_DEFAULT_FILTER: + return true; + } + + return parent::shouldHideForMail($xactions); + } + + public function getIcon() { $old = $this->getOldValue(); $new = $this->getNewValue(); @@ -258,6 +281,16 @@ final class PhabricatorProjectTransaction '%s disabled the workboard for this project.', $author_handle); } + + case self::TYPE_DEFAULT_SORT: + return pht( + '%s changed the default sort order for the project workboard.', + $author_handle); + + case self::TYPE_DEFAULT_FILTER: + return pht( + '%s changed the default filter for the project workboard.', + $author_handle); } return parent::getTitle(); @@ -379,19 +412,6 @@ final class PhabricatorProjectTransaction $this->renderSlugList($rem)); } - case self::TYPE_HASWORKBOARD: - if ($new) { - return pht( - '%s enabled the workboard for %s.', - $author_handle, - $object_handle); - } else { - return pht( - '%s disabled the workboard for %s.', - $author_handle, - $object_handle); - } - } return parent::getTitleForFeed();