Page MenuHomePhabricator

Improve initial implementations of Workboard triggers and groups
Open, NormalPublic

Description

Followup to T5474. See also T13074.

Trigger Types: These trigger types are probably worth considering:

  • Assign to <user>.
  • Require assignment.
  • Close as <status> (only acts if task is not already closed).
  • Reopen as <status>.
  • Set priority to <priority>.
  • Raise/lower priority to <priority> (enforces a minimum/maximum).
  • Require triage.
  • Add/remove projects.
  • Prompt for comment.
  • Reject drag.
  • Enforce point limit.

These are a little more out there, probably:

  • Custom field stuff.
  • Start / stop tracking time.
  • Send an email / notification.

Sounds: Currently, there are only a couple of hard-coded sounds. All applications which can play UI sounds (Workboards, Conpherence, Notifications) could benefit from customizable sounds.


Groups: A couple of possible followups for workboard groups:

  • Add count/point information to headers.
  • Some kind of header filtering buttons to expand/collapse particular headers?
  • Ability to mark statuses and priorities as hidden by default unless some tasks belong to the group ("Duplicate", "Spite", "Unbreak Now").

Copies/ References: Currently, each column has its own trigger.

I plan to change this a bit: when you create a workboard from another board, I'd like to copy the triggers. The edit operations will become "edit this trigger, edit a copy, reference another trigger, remove trigger".

This is how the database works already and "just" missing UI, but it makes the UI a bit more complex so I'm hoping to delay it a bit and focus feedback on the simpler initial implementation, make sure that's on the right path, and then add all the extra complexity. Presumably, this feature isn't terribly critical out of the gate.


Errata

  • Maniphest hovercards are using some of the same code as workboard cards and have gone slightly nutty as a consequence.
  • On a board, breadcrumbs should link to parent boards (if they exist?) not default pages.
  • I'd like to make the header "trigger" and "dropdown" buttons look more like buttons.
  • (Maybe) If a trigger is trivial (close tasks, assign to X) maybe show an icon hinting at what it does instead of the generic icon.

Event Timeline

epriestley triaged this task as Normal priority.Mar 25 2019, 8:20 PM
epriestley created this task.
epriestley updated the task description. (Show Details)Mar 25 2019, 9:15 PM
epriestley updated the task description. (Show Details)Mar 25 2019, 9:26 PM
epriestley updated the task description. (Show Details)Mar 26 2019, 4:10 PM
epriestley updated the task description. (Show Details)Mar 26 2019, 7:52 PM
shandor added a subscriber: shandor.
azurasean added a subscriber: azurasean.
amckinley updated the task description. (Show Details)Apr 10 2019, 6:14 PM
  • Close as <status> (only acts if task is not already closed).
  • Reopen as <status>.

Are these different from the existing implementation in PhabricatorProjectTriggerManiphestStatusRule?

  • Require assignment.

What does this mean? "Reject the drag unless the task is assigned"? "Prompt the user to enter a username on unassigned tasks when dragged"?

  • Raise/lower priority to <priority> (enforces a minimum/maximum).

Is this "increment the current priority by x, up to a maximum of y"? Not sure what this is supposed to do.

  • Require triage.

Similar to "require assignment"? "Reject the task if it's still in state Needs Triage"?

  • Enforce point limit.

Is this "Define a point limit of x for open tasks in this column, and reject a drag if it would exceed that point limit"? (Also, sounds like we should similarly have a "require non-zero point limit" trigger rule).

  • Prompt for comment.

As in, pop an inline editor to attach a comment to explain the reason for the drag?

  • Reject drag.

Just unconditionally? If I remember the implementation correctly, we gather all the transactions from all the column triggers before applying any of them, so it's not too silly to let users setup nonsensical collections of trigger rules that do a bunch of actions only to at some point reject the drag. We could also explicitly require that triggers containing the "Reject Drag" rule can only contain that rule.

Are these different from the existing implementation in PhabricatorProjectTriggerManiphestStatusRule?

My thinking here is that you might want a column which closes open tasks, but doesn't touch already-closed tasks. So you use this rule:

  • [ Close task as ][ resolved ]

This is different from [ Set task status to ][ resolved ] if you drop a "Wontfix" task into the column. "Set task status" changes it to "Resolved", "Close task as" leaves it as "Wontfix".

We should probably wait for users to actually hit this, though.

What does [require assignment] mean? "Prompt the user to enter a username on unassigned tasks when dragged"?

I'm imagining that you get a dialog with an "Assign To: ..." prompt, yeah.

Is this "increment the current priority by x, up to a maximum of y"? Not sure what this is supposed to do.

This is sort of similar to "Close task as". Suppose you have a column called "Distant Future" with trigger "Lower Priority To: Low".

If you drop a "Wishlist" task into this column, the priority stays "Wishlist" (since "Wishlist" is lower than "Low", so the "Lower priority..." action has no effect). In contrast, if you drop a "Wishlist" task into a "Set priority to: Low" column, the priority is raised to "Low".

So these rules would let you say "Tasks in this column should be 'Low' or lower, but if they're already lower than 'Low' that's fine". Or, put another way, this is like "Set priority to X, but only if the current priority is higher than X".

Similar to "require assignment"? "Reject the task if it's still in state Needs Triage"?

Probably "Prompt for a non-default priority if the task currently has the default priority".

Is this "Define a point limit of x for open tasks in this column, and reject a drag if it would exceed that point limit"? (Also, sounds like we should similarly have a "require non-zero point limit" trigger rule).

Yeah, I'm imagining this just bounces the task back where it came from if the column would end up with too many points.

As in, pop an inline editor to attach a comment to explain the reason for the drag?

Yep.

Just unconditionally? If I remember the implementation correctly, we gather all the transactions from all the column triggers before applying any of them, so it's not too silly to let users setup nonsensical collections of trigger rules that do a bunch of actions only to at some point reject the drag. We could also explicitly require that triggers containing the "Reject Drag" rule can only contain that rule.

I think for the prompt/reject rules, we need to add a new piece of code to the move controller, so the sequence will become:

  • Run all the prompt/reject rules, possibly bailing out early if we're rejecting or prompting.
  • Once those pass, build the move transaction.
  • Add on the actual trigger transactions.
  • Apply everything.

"Reject Drag" itself is probably not really very useful except for testing, although this combination with another rule arguably serves at least some purpose:

[ Play a sound ][ youve_been_gnomed.mp3 ]
[ Reject drag ]

Broadly, it's not clear to me that any of these rules are actually good/useful/worth building. "Comment" and "Require Assignment" seem the most likely? I could easily imagine that "Lower priority to" might see zero use in the wild if we shipped it. So it might be reasonable to just wait for feedback on most/all of this.

Broadly, it's not clear to me that any of these rules are actually good/useful/worth building.

Okie dokie, I'll finish "add and remove projects" and move on, unless there's anything else you think we need for v1.

Things feel pretty reasonable to me for now. I'm going to fix those bugs you hit and I think I have a couple other things I wanted to tweak, but I'm largely happy with the featureset as a v1.

well, I'd say for example the "Close as <status> (only acts if task is not already closed)" is useful, and definitely needed.
Move the task to "done" column on the workboard, and the task automatically gets closed. That's handy.

well, I'd say for example the "Close as <status> (only acts if task is not already closed)" is useful, and definitely needed.
Move the task to "done" column on the workboard, and the task automatically gets closed. That's handy.

That's already implemented, except that the task's status is changed unconditionally in the current implementation. See @epriestley's comment above:

Are these different from the existing implementation in PhabricatorProjectTriggerManiphestStatusRule?

My thinking here is that you might want a column which closes open tasks, but doesn't touch already-closed tasks. So you use this rule:

  • [ Close task as ][ resolved ]

This is different from [ Set task status to ][ resolved ] if you drop a "Wontfix" task into the column. "Set task status" changes it to "Resolved", "Close task as" leaves it as "Wontfix".
We should probably wait for users to actually hit this, though.

ah, I missed that. I guess I should start testing this stuff soon :)

Pawka added a subscriber: Pawka.Apr 18 2019, 5:48 AM