1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-22 23:02:42 +01:00

Give the workboard "default" workflows more modern state handling

Summary:
Depends on D20628. Ref T4900. Currently, the "Save Current Order/Filter As Default" flows on workboards duplicate some state construction, and require parameters to be passed to them explicitly.

Now that state management is separate, they can reuse a bit more code and be made to look more like other similar controllers.

Test Plan:
  - Changed the default order of a workboard.
  - Changed the default filter of a workboard.
  - Changed the order of a board to something non-default, then changed the filter, then saved the new filter as the default. Saw the modified order preserved and the modified filter removed, so I ended up in the right ("most correct") place: on the board, with my custom order in a URI parameter, and no filter URI parameter so I could see my new default filter behavior. This is an edge case that's not terribly important to get right, but we do get it right.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T4900

Differential Revision: https://secure.phabricator.com/D20629
This commit is contained in:
epriestley 2019-06-29 15:23:58 -07:00
parent 9c190d68ed
commit 9e096cd274
7 changed files with 63 additions and 43 deletions

View file

@ -4158,6 +4158,7 @@ phutil_register_library_map(array(
'PhabricatorProjectArchiveController' => 'applications/project/controller/PhabricatorProjectArchiveController.php', 'PhabricatorProjectArchiveController' => 'applications/project/controller/PhabricatorProjectArchiveController.php',
'PhabricatorProjectBoardBackgroundController' => 'applications/project/controller/PhabricatorProjectBoardBackgroundController.php', 'PhabricatorProjectBoardBackgroundController' => 'applications/project/controller/PhabricatorProjectBoardBackgroundController.php',
'PhabricatorProjectBoardController' => 'applications/project/controller/PhabricatorProjectBoardController.php', 'PhabricatorProjectBoardController' => 'applications/project/controller/PhabricatorProjectBoardController.php',
'PhabricatorProjectBoardDefaultController' => 'applications/project/controller/PhabricatorProjectBoardDefaultController.php',
'PhabricatorProjectBoardDisableController' => 'applications/project/controller/PhabricatorProjectBoardDisableController.php', 'PhabricatorProjectBoardDisableController' => 'applications/project/controller/PhabricatorProjectBoardDisableController.php',
'PhabricatorProjectBoardImportController' => 'applications/project/controller/PhabricatorProjectBoardImportController.php', 'PhabricatorProjectBoardImportController' => 'applications/project/controller/PhabricatorProjectBoardImportController.php',
'PhabricatorProjectBoardManageController' => 'applications/project/controller/PhabricatorProjectBoardManageController.php', 'PhabricatorProjectBoardManageController' => 'applications/project/controller/PhabricatorProjectBoardManageController.php',
@ -4207,7 +4208,6 @@ phutil_register_library_map(array(
'PhabricatorProjectCustomFieldStringIndex' => 'applications/project/storage/PhabricatorProjectCustomFieldStringIndex.php', 'PhabricatorProjectCustomFieldStringIndex' => 'applications/project/storage/PhabricatorProjectCustomFieldStringIndex.php',
'PhabricatorProjectDAO' => 'applications/project/storage/PhabricatorProjectDAO.php', 'PhabricatorProjectDAO' => 'applications/project/storage/PhabricatorProjectDAO.php',
'PhabricatorProjectDatasource' => 'applications/project/typeahead/PhabricatorProjectDatasource.php', 'PhabricatorProjectDatasource' => 'applications/project/typeahead/PhabricatorProjectDatasource.php',
'PhabricatorProjectDefaultController' => 'applications/project/controller/PhabricatorProjectDefaultController.php',
'PhabricatorProjectDescriptionField' => 'applications/project/customfield/PhabricatorProjectDescriptionField.php', 'PhabricatorProjectDescriptionField' => 'applications/project/customfield/PhabricatorProjectDescriptionField.php',
'PhabricatorProjectDetailsProfileMenuItem' => 'applications/project/menuitem/PhabricatorProjectDetailsProfileMenuItem.php', 'PhabricatorProjectDetailsProfileMenuItem' => 'applications/project/menuitem/PhabricatorProjectDetailsProfileMenuItem.php',
'PhabricatorProjectDropEffect' => 'applications/project/icon/PhabricatorProjectDropEffect.php', 'PhabricatorProjectDropEffect' => 'applications/project/icon/PhabricatorProjectDropEffect.php',
@ -10422,6 +10422,7 @@ phutil_register_library_map(array(
'PhabricatorProjectArchiveController' => 'PhabricatorProjectController', 'PhabricatorProjectArchiveController' => 'PhabricatorProjectController',
'PhabricatorProjectBoardBackgroundController' => 'PhabricatorProjectBoardController', 'PhabricatorProjectBoardBackgroundController' => 'PhabricatorProjectBoardController',
'PhabricatorProjectBoardController' => 'PhabricatorProjectController', 'PhabricatorProjectBoardController' => 'PhabricatorProjectController',
'PhabricatorProjectBoardDefaultController' => 'PhabricatorProjectBoardController',
'PhabricatorProjectBoardDisableController' => 'PhabricatorProjectBoardController', 'PhabricatorProjectBoardDisableController' => 'PhabricatorProjectBoardController',
'PhabricatorProjectBoardImportController' => 'PhabricatorProjectBoardController', 'PhabricatorProjectBoardImportController' => 'PhabricatorProjectBoardController',
'PhabricatorProjectBoardManageController' => 'PhabricatorProjectBoardController', 'PhabricatorProjectBoardManageController' => 'PhabricatorProjectBoardController',
@ -10484,7 +10485,6 @@ phutil_register_library_map(array(
'PhabricatorProjectCustomFieldStringIndex' => 'PhabricatorCustomFieldStringIndexStorage', 'PhabricatorProjectCustomFieldStringIndex' => 'PhabricatorCustomFieldStringIndexStorage',
'PhabricatorProjectDAO' => 'PhabricatorLiskDAO', 'PhabricatorProjectDAO' => 'PhabricatorLiskDAO',
'PhabricatorProjectDatasource' => 'PhabricatorTypeaheadDatasource', 'PhabricatorProjectDatasource' => 'PhabricatorTypeaheadDatasource',
'PhabricatorProjectDefaultController' => 'PhabricatorProjectBoardController',
'PhabricatorProjectDescriptionField' => 'PhabricatorProjectStandardCustomField', 'PhabricatorProjectDescriptionField' => 'PhabricatorProjectStandardCustomField',
'PhabricatorProjectDetailsProfileMenuItem' => 'PhabricatorProfileMenuItem', 'PhabricatorProjectDetailsProfileMenuItem' => 'PhabricatorProfileMenuItem',
'PhabricatorProjectDropEffect' => 'Phobject', 'PhabricatorProjectDropEffect' => 'Phobject',

View file

@ -90,6 +90,8 @@ final class PhabricatorProjectApplication extends PhabricatorApplication {
=> 'PhabricatorProjectBoardManageController', => 'PhabricatorProjectBoardManageController',
'background/' 'background/'
=> 'PhabricatorProjectBoardBackgroundController', => 'PhabricatorProjectBoardBackgroundController',
'default/(?P<target>[^/]+)/'
=> 'PhabricatorProjectBoardDefaultController',
), ),
'column/' => array( 'column/' => array(
'remove/(?P<id>\d+)/' => 'remove/(?P<id>\d+)/' =>
@ -112,8 +114,6 @@ final class PhabricatorProjectApplication extends PhabricatorApplication {
=> 'PhabricatorProjectSilenceController', => 'PhabricatorProjectSilenceController',
'warning/(?P<id>[1-9]\d*)/' 'warning/(?P<id>[1-9]\d*)/'
=> 'PhabricatorProjectSubprojectWarningController', => 'PhabricatorProjectSubprojectWarningController',
'default/(?P<projectID>[1-9]\d*)/(?P<target>[^/]+)/'
=> 'PhabricatorProjectDefaultController',
), ),
'/tag/' => array( '/tag/' => array(
'(?P<slug>[^/]+)/' => 'PhabricatorProjectViewController', '(?P<slug>[^/]+)/' => 'PhabricatorProjectViewController',

View file

@ -13,7 +13,7 @@ abstract class PhabricatorProjectBoardController
return $this->viewState; return $this->viewState;
} }
final private function newViewState() { private function newViewState() {
$project = $this->getProject(); $project = $this->getProject();
$request = $this->getRequest(); $request = $this->getRequest();
@ -22,4 +22,15 @@ abstract class PhabricatorProjectBoardController
->readFromRequest($request); ->readFromRequest($request);
} }
final protected function newBoardDialog() {
$dialog = $this->newDialog();
$state = $this->getViewState();
foreach ($state->getQueryParameters() as $key => $value) {
$dialog->addHiddenInput($key, $value);
}
return $dialog;
}
} }

View file

@ -1,25 +1,20 @@
<?php <?php
final class PhabricatorProjectDefaultController final class PhabricatorProjectBoardDefaultController
extends PhabricatorProjectBoardController { extends PhabricatorProjectBoardController {
public function handleRequest(AphrontRequest $request) { public function handleRequest(AphrontRequest $request) {
$viewer = $request->getViewer(); $viewer = $request->getViewer();
$project_id = $request->getURIData('projectID');
$project = id(new PhabricatorProjectQuery()) $response = $this->loadProjectForEdit();
->setViewer($viewer) if ($response) {
->requireCapabilities( return $response;
array(
PhabricatorPolicyCapability::CAN_VIEW,
PhabricatorPolicyCapability::CAN_EDIT,
))
->withIDs(array($project_id))
->executeOne();
if (!$project) {
return new Aphront404Response();
} }
$this->setProject($project);
$project = $this->getProject();
$state = $this->getViewState();
$board_uri = $state->newWorkboardURI();
$remove_param = null;
$target = $request->getURIData('target'); $target = $request->getURIData('target');
switch ($target) { switch ($target) {
@ -31,8 +26,10 @@ final class PhabricatorProjectDefaultController
'the board.'); 'the board.');
$button = pht('Save Default Filter'); $button = pht('Save Default Filter');
$xaction_value = $request->getStr('filter'); $xaction_value = $state->getQueryKey();
$xaction_type = PhabricatorProjectFilterTransaction::TRANSACTIONTYPE; $xaction_type = PhabricatorProjectFilterTransaction::TRANSACTIONTYPE;
$remove_param = 'filter';
break; break;
case 'sort': case 'sort':
$title = pht('Set Board Default Order'); $title = pht('Set Board Default Order');
@ -42,8 +39,10 @@ final class PhabricatorProjectDefaultController
'the board.'); 'the board.');
$button = pht('Save Default Order'); $button = pht('Save Default Order');
$xaction_value = $request->getStr('order'); $xaction_value = $state->getOrder();
$xaction_type = PhabricatorProjectSortTransaction::TRANSACTIONTYPE; $xaction_type = PhabricatorProjectSortTransaction::TRANSACTIONTYPE;
$remove_param = 'order';
break; break;
default: default:
return new Aphront404Response(); return new Aphront404Response();
@ -51,12 +50,6 @@ final class PhabricatorProjectDefaultController
$id = $project->getID(); $id = $project->getID();
$view_uri = $this->getApplicationURI("board/{$id}/");
$view_uri = new PhutilURI($view_uri);
foreach ($request->getPassthroughRequestData() as $key => $value) {
$view_uri->replaceQueryParam($key, $value);
}
if ($request->isFormPost()) { if ($request->isFormPost()) {
$xactions = array(); $xactions = array();
@ -71,20 +64,18 @@ final class PhabricatorProjectDefaultController
->setContinueOnMissingFields(true) ->setContinueOnMissingFields(true)
->applyTransactions($project, $xactions); ->applyTransactions($project, $xactions);
return id(new AphrontRedirectResponse())->setURI($view_uri); // If the parameter we just modified is present in the query string,
// throw it away so the user is redirected back to the default view of
// the board, allowing them to see the new default behavior.
$board_uri->removeQueryParam($remove_param);
return id(new AphrontRedirectResponse())->setURI($board_uri);
} }
$dialog = $this->newDialog() return $this->newBoardDialog()
->setTitle($title) ->setTitle($title)
->appendChild($body) ->appendChild($body)
->setDisableWorkflowOnCancel(true) ->addCancelButton($board_uri)
->addCancelButton($view_uri)
->addSubmitButton($title); ->addSubmitButton($title);
foreach ($request->getPassthroughRequestData() as $key => $value) {
$dialog->addHiddenInput($key, $value);
}
return $dialog;
} }
} }

View file

@ -797,9 +797,7 @@ final class PhabricatorProjectBoardViewController
$id = $project->getID(); $id = $project->getID();
$save_uri = "default/{$id}/sort/"; $save_uri = $state->newWorkboardURI('default/sort/');
$save_uri = $this->getApplicationURI($save_uri);
$save_uri = $this->getURIWithState($save_uri, $force = true);
$can_edit = PhabricatorPolicyFilter::hasCapability( $can_edit = PhabricatorPolicyFilter::hasCapability(
$viewer, $viewer,
@ -842,6 +840,8 @@ final class PhabricatorProjectBoardViewController
PhabricatorApplicationSearchEngine $engine, PhabricatorApplicationSearchEngine $engine,
$query_key) { $query_key) {
$state = $this->getViewState();
$named = array( $named = array(
'open' => pht('Open Tasks'), 'open' => pht('Open Tasks'),
'all' => pht('All Tasks'), 'all' => pht('All Tasks'),
@ -898,9 +898,7 @@ final class PhabricatorProjectBoardViewController
->setWorkflow(true) ->setWorkflow(true)
->setName(pht('Advanced Filter...')); ->setName(pht('Advanced Filter...'));
$save_uri = "default/{$id}/filter/"; $save_uri = $state->newWorkboardURI('default/filter/');
$save_uri = $this->getApplicationURI($save_uri);
$save_uri = $this->getURIWithState($save_uri, $force = true);
$can_edit = PhabricatorPolicyFilter::hasCapability( $can_edit = PhabricatorPolicyFilter::hasCapability(
$viewer, $viewer,

View file

@ -16,6 +16,21 @@ abstract class PhabricatorProjectController extends PhabricatorController {
} }
protected function loadProject() { protected function loadProject() {
return $this->loadProjectWithCapabilities(
array(
PhabricatorPolicyCapability::CAN_VIEW,
));
}
protected function loadProjectForEdit() {
return $this->loadProjectWithCapabilities(
array(
PhabricatorPolicyCapability::CAN_VIEW,
PhabricatorPolicyCapability::CAN_EDIT,
));
}
private function loadProjectWithCapabilities(array $capabilities) {
$viewer = $this->getViewer(); $viewer = $this->getViewer();
$request = $this->getRequest(); $request = $this->getRequest();
@ -35,6 +50,7 @@ abstract class PhabricatorProjectController extends PhabricatorController {
$query = id(new PhabricatorProjectQuery()) $query = id(new PhabricatorProjectQuery())
->setViewer($viewer) ->setViewer($viewer)
->requireCapabilities($capabilities)
->needMembers(true) ->needMembers(true)
->needWatchers(true) ->needWatchers(true)
->needImages(true) ->needImages(true)

View file

@ -147,4 +147,8 @@ final class PhabricatorWorkboardViewState
return 'open'; return 'open';
} }
public function getQueryParameters() {
return $this->requestState;
}
} }