Page MenuHomePhabricator
Create Task
Maniphest T9206

Herald unable to place task up for grabs
Closed, ResolvedPublic
Actions

Assigned To
None
Authored By
dpaola2
Aug 17 2015, 4:13 PM
Tags
Referenced Files
F725372: Screenshot 2015-08-17 09.30.46.png
Aug 17 2015, 4:30 PM
F725367: Screenshot 2015-08-17 09.29.56.png
Aug 17 2015, 4:30 PM
F725361: Screenshot 2015-08-17 09.29.09.png
Aug 17 2015, 4:29 PM
F725298: Screenshot 2015-08-17 09.12.53.png
Aug 17 2015, 4:13 PM
Subscribers
dpaola2
epriestley
joshuaspence

Description

At the moment, assigning a task to an empty user doesn't place the task up for grabs, as it does when assigning to an empty user from the maniphest task UI.

Steps to reproduce:

  1. Create new herald rule that assigns to an empty user based on association with a project
  2. Create new maniphest task assigned to yourself
  3. Associate new task with the project, triggering the herald rule
  4. Observe task remains assigned to you

Here's a screenshot of an example:

Screenshot 2015-08-17 09.12.53.png (601×1 px, 85 KB)

Then, on the task:

Screenshot 2015-08-17 09.29.09.png (122×681 px, 28 KB)

Running a3393c3ecb92753277cd7bf447631ebb510f59ce

Revisions and Commits

rP Phabricator
D14684rPbc331f0fbf54 Allow "Assign task" Herald Action in Maniphest to accept "None" to unassign

Event Timeline

dpaola2 created this task.Aug 17 2015, 4:13 PM
dpaola2 updated the task description. (Show Details)
dpaola2 added a project: Herald.
dpaola2 added a subscriber: dpaola2.
dpaola2 updated the task description. (Show Details)EditedAug 17 2015, 4:29 PM

Here's the rule transcript:

Screenshot 2015-08-17 09.30.46.png (258×435 px, 50 KB)

joshuaspence added a subscriber: joshuaspence.Aug 17 2015, 8:34 PM
epriestley triaged this task as Low priority.Aug 18 2015, 5:56 PM
epriestley added a subscriber: epriestley.

This behavior is currently expected: this construction means "no effect", not "place up for grabs".

(We interpreted something similar in the batch editor as "place up for grabs" a while ago and got a couple of reports that it was confusing/unintuitive/not discoverable. To avoid that, I want to make "unassign" explicit and discoverable going forward.)

The capability seems reasonable to add. I think there are two general approaches:

  1. Add a separate "Unassign Task" action.
  2. Let the user enter the "None" token into this typeahead (use ManiphestAssigneeDatasource instead of PhabricatorPeopleDatasource).

(1) is a little more straightforward technically, but not consistent with "Batch Edit" and not necessarily as intuitive as making this action available from "Assign Task".

(2) is relatively involved because the "None" token doesn't have a real PHID and parts of the editing and rendering pipelines don't fully pass through the Datasource stuff that they'd need to in order to pick up the rendering. There's also currently no way for a datasource to render plan text flavors of tokens (e.g., the text "No Owner" for the summary line "Assign task to: No Owner"). This is more flexible, general and consistent, but also relatively involved (I poked at it for a few minutes without making much headway).

At some point, we should also:

  • Limit the tokenizer to a single input (there are some TODOs about this in the code).
  • Raise an error in the UI when you try to save a rule with a meaningless action or field (e.g., assign to no targets).
epriestley added a project: Typeahead.Aug 18 2015, 5:57 PM
dpaola2 added a comment.Aug 18 2015, 6:18 PM

I'd be curious how important it is to the project to maintain UI consistency between Herald and Maniphest. Currently leaving the field blank in Maniphest places the task up for grabs -- wouldn't it be preferable to maintain the same expectations in Herald?

(I may be dense here)

epriestley added a comment.Aug 18 2015, 6:41 PM
This comment was removed by epriestley.
epriestley added a comment.Aug 18 2015, 6:41 PM

In the past, the bulk editor worked like this under a similar theory (and or laziness, I don't remember which contributed more), but we had at least one (and maybe a few? I don't recall offhand) user who failed to guess it and didn't even try the interaction in the bulk editor.

I think these contexts aren't quite the same. On the task, all the fields are "set" fields: removing the assignee sets assignee to none. Removing all the subscribers deletes all the subscribers. Removing all the projects deletes all the projects. It's common for fields to be empty when they have no value.

On the bulk editor and Herald, most fields are "add" (or "remove") fields. "Add subscribers: X" means to just add that subscriber, not set the subscribers to that value. There is no "Subscribers:" field, and removing all subscribers wouldn't make much sense. Empty actions generally don't make much sense here. I think it's reasonable for users to come into these UIs with a slightly different mindset, and understandable that "Assign task to: [(blank)]" isn't obvious as an unassign action.

The new behavior of the bulk editor is:

Assign to: [ Type a username or "none"... ]

...which I think is clear, unambiguous, and discoverable.

dpaola2 added a comment.Aug 18 2015, 6:50 PM

Excellent point, thanks for clarifying.

epriestley added a revision: D14684: Allow "Assign task" Herald Action in Maniphest to accept "None" to unassign.Dec 5 2015, 8:38 PM
epriestley closed this task as Resolved by committing rPbc331f0fbf54: Allow "Assign task" Herald Action in Maniphest to accept "None" to unassign.Dec 5 2015, 8:39 PM
epriestley added a commit: rPbc331f0fbf54: Allow "Assign task" Herald Action in Maniphest to accept "None" to unassign.
Phabricator · Built by Phacility