Page MenuHomePhabricator

Move workboard "filter" workflow to a separate controller
ClosedPublic

Authored by epriestley on Jul 2 2019, 12:30 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Mar 14, 5:22 PM
Unknown Object (File)
Feb 3 2024, 10:47 PM
Unknown Object (File)
Jan 17 2024, 11:32 PM
Unknown Object (File)
Jan 17 2024, 3:06 PM
Unknown Object (File)
Dec 31 2023, 6:24 AM
Unknown Object (File)
Dec 22 2023, 2:34 AM
Unknown Object (File)
Dec 22 2023, 2:34 AM
Unknown Object (File)
Dec 22 2023, 2:33 AM
Subscribers
None

Details

Summary

Depends on D20629. Ref T4900. Currently, the "Advanced Filter..." workflow on workboards (where you build a custom query) is inline in the main board controller.

This is because the filter flow depends on some of the board view state: we want to start with the current filter applied to the board, and preserve other state after you change the filter.

Now that ViewState can handle state management, we can separate this stuff out pretty easily.

Test Plan
  • Changed filters on a board.
  • Applied a custom filter to a board.
  • Changed the ordering of a board, then applied a custom filter. Verified "Cancel" and "Apply Filter" both preserve the order state.
  • Changed the ordering of a board, then applied a custom filter, intentionally making a mistake in configuring the filter by entering an invalid date. Saw a dialog with an error. After correcting the error, saw state preserved properly.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

src/applications/project/controller/PhabricatorProjectBoardViewController.php
30–32

Particularly, I think getting rid of this absolute nonsense is a big step forward in clarity. This is saying:

  • if this is a POST (with CSRF); and
  • doesn't look like it's because the user is trying to do a bulk move; and
  • doesn't look like it's because the user is trying to build a query from a column; and
  • doesn't look like it's because the user is trying to initialize a board; then

...apply a filter.

This is weird, non-obvious, non-local, and the approach ("do every possible action in this controller by guessing request parameters") doesn't scale very well, even ignoring how big the file has become in terms of raw line size.

src/applications/project/state/PhabricatorWorkboardViewState.php
91–93

(This isn't ideal from a correctness standpoint, but doesn't appear to actually cause any problematic behavior.)

This revision is now accepted and ready to land.Jul 2 2019, 8:35 PM