Page MenuHomePhabricator

Maniphest - move subpriority edits to be transaction powered
ClosedPublic

Authored by btrahan on Feb 27 2014, 12:28 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 11, 11:21 PM
Unknown Object (File)
Mon, Mar 11, 11:21 PM
Unknown Object (File)
Mon, Mar 11, 11:21 PM
Unknown Object (File)
Mon, Mar 11, 11:09 PM
Unknown Object (File)
Mon, Mar 11, 11:03 PM
Unknown Object (File)
Tue, Mar 5, 11:52 AM
Unknown Object (File)
Feb 21 2024, 5:22 PM
Unknown Object (File)
Feb 21 2024, 5:22 PM

Details

Summary

...this was nice to do for boards, since this diff also starts calling this code in the board move case. The big trick is to use the new expandTransactions hook to expand the subpriority transaction into a priority transaction if pertinent. The other stuff is just about hiding these new subpriority extractions.

...also removes the "edit" UI from the default board since we can't actually edit anything and it thus is buggy.

Ref T4422. Next step is to move board edits into the editor with their own little transaction.

Test Plan

re-orded tasks on a maniphest query, reloaded, and noted re-order stuck.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

src/applications/project/controller/PhabricatorProjectMoveController.php
80

wasn't sure what do to if we couldn't find the task via the afterPHID.

epriestley added inline comments.
src/applications/maniphest/controller/ManiphestSubpriorityController.php
47–48

(Maybe consider calling these something more substantive since they're being saved now.)

src/applications/maniphest/editor/ManiphestTransactionEditor.php
351–352

We can probably just remove this TODO, I think these loads are fine given that we don't leak any data outside of this method.

src/applications/project/controller/PhabricatorProjectMoveController.php
78–79

We should requireCapabilities(...) with CAN_EDIT here, so you can't reprioritize a task you don't have permission to edit.

(Exactly how edit permissions shake out here are a little funky right now, but I think you'll need VIEW on the project and EDIT on the task in the long run to drag stuff around.)

80

I think 404'ing would be okay, but this isn't bad. Shouldn't ever happen in practice.

btrahan updated this revision to Unknown Object (????).Feb 27 2014, 5:38 PM

edits...

  • "after_pri" -> "newPriority"
  • "after_sub" -> "newSubpriorityBase" -- wasn't super sure on this one
  • deleted no longer necessary comment
  • added requireCapabilities(array(EDIT))
  • return a 404 if we fail to query after_task