Page MenuHomePhabricator

Communicate drag policies clearly in the workboard/list UIs
Closed, ResolvedPublic

Description

The current policy for moving tasks (in priority-ordered lists in Maniphest, and in general on Workboards) is that having edit permission on the task is necessary and sufficient to drag it.

However, this is not communicated in the UI: tasks which you can not move because you don't have permission to edit them have no visual distinction from tasks which you can move. Immovable tasks should be clearly marked to make this distinction obvious and help hint policy expectations to users.


Original Report

I think i have seen this reported before but I can't find it:
If you don't have permission to edit a task, and try to move it on a workboard in natural view, you get an error. This in unexpected. You shouldn't be able to edit the priority this way but you should be able to move around tasks on an editable workboard.

Event Timeline

eadler created this task.Apr 4 2016, 9:54 PM
eadler added a subscriber: scode.
chad added a subscriber: chad.Apr 4 2016, 9:57 PM

Trying to understand this, since moving tasks on a board is editing the underlying task.

chad added a comment.Apr 4 2016, 9:59 PM

T6502 is the other task you're likely thinking of.

scode added a comment.Apr 4 2016, 9:59 PM

Definitely open to this being a user expectation bug.

In our case, we have a Twitter workboard where I expect to start moving stuff around more soon (whereas @eadler) has been doing it so far.

The reason I, as a user, expect to be able to do it is: The board is private to me/us and represents a perspective of tickets. Things like categorization on that board, or re-prioritization, is a private concern to the board owner(s).

As a result, I would expect to be able to use boards to triage other people's tickets without having edit permissions to the tickets themselves.

It's possible this isn't the intention of the feature etc. Just explaining why I expected it as a user.

to be clear: the #Twitter project was already added to the project. The expectation was that changing the *natural* state (as opposed to the priority state) would be allowed.

chad added a comment.Apr 4 2016, 10:03 PM

No, we don't want everyone being able to move tasks anywhere, anytime. Projects don't infer policies.

T6502 is related except that even if that were to be permissive it wouldn't be possible to edit the sort since the underlying task lacks edit capability. That said, it is the ticket I was thinking of.

scode added a comment.Apr 4 2016, 10:03 PM

To be clear, the Community work-around is totally fine for us. This ticket was filed just to call attention to it. We don't have a case where this matters directly for us (we're not using Phabricator for ticketing and thus won't have these workboards; this was just us encountering it when working on this installation of Phabricator).

@chad this isn't an everyone-everywhere change. Here the expectation, at least for me, is that the natural sort edit should follow the workboard edit policy, not the task edit policy.

chad added a comment.Apr 4 2016, 10:11 PM

T6502 is workboard edit policy, as far as I understand it. We were planning on building it, but fundamentally couldn't come up with good reasons why you'd want different permissions for editing a task vs. editing a board. There are reasons, but they didn't seem worth the current cost of implementation.

At least, allowing people to move tasks without T6502 in place seems problematic. For instance here, that would allow any logged in user the ability to re-arrange the tasks. I assume this should just maybe merge into T6502 @epriestley ?

My current expectation is that the rule is supposed to be that edit permission on the task is both necessary and sufficient to move it on the workboard, but this is almost certainly not fully reflected in the UI, and possibly not fully reflected in permission checks in some cases (?) although it sounds like we're good from this discussion.

We'll realign the UI and permission checks to this rule eventually if we haven't already. There are some comments in the code about the UI case, at least; I don't remember if it got mentioned in a task anywhere. I don't think we currently have an "undraggable card" UI state, and it needs to be distinct from the "draggable card" UI state since a board may have a mixture of the two. There's a similar issue in flat task lists (Maniphest list view, sorted by priority).

In practice, this UI stuff rarely arises (users usually have all-or-nothing edit permission on a board) and it's adjacent to some usually-more-visible remaining work in T10349 (subproject columns on workboards, behavior when you edit a card and remove the project you're looking at) so it didn't make the cut in the last iteration.

T6502 discusses implementing a separate "who can drag cards" permission, other than "exactly users who can edit the task". We don't currently have actual use cases for that and don't plan to pursue it, although we're open to it if use cases do arise.

Let me look for existing discussion of the UI states: I'll either merge this into that or realign this to cover that.

This rule ("edit permission on a task is necessary + sufficient to move it") will become somewhat more important after T5474, when moving a task may produce a cascade of other effects by activating column triggers. It will basically conflate workboard column moves with all other edit types. This is navigable without requiring "edit task" to move, but everything is simpler if we can retain the simpler permission rule.

With a more complex rule, you might pick up a task and then we'd have to, say, put a "" overlay on some of the columns depending on which triggers could possibly activate, and then make some effort to explain the reasoning to the user if they tried to drop the card on the forbidden column. Not impossible, but not an exciting problem to try to solve.

I think there are also a fairly good set of reasonable cases where users may not have edit permission on a project but still have some business moving things on the workboard. We did require this at one point but removed this check in T5204 (although there's little discussion there; that was early and the use case was just "I want to restrict project editing but let normal users touch the workboard").

Finally, we expect this situation (permission to see but not touch) to be fairly rare outside of open source, since drive-by vandalism is presumably almost nonexistent inside companies.

In the meantime, I added @scode to Community.

epriestley renamed this task from Users without permission to edit a task can't move tasks on workboards they could edit to Communicate drag policies clearly in the workboard/list UIs.Apr 4 2016, 10:34 PM
epriestley triaged this task as Normal priority.
epriestley updated the task description. (Show Details)
epriestley added a project: Workboards (v3).
epriestley added a comment.EditedApr 4 2016, 10:42 PM

Upshot:

  • I'd currently consider this a user-expectation issue, but we're doing ourselves zero favors by not providing any kinds of hints about what expectation you should have. We should fix this first. I've realigned this task to discuss making the expectations/behavior more clear.
  • I believe the "edit permission is necessary" part of this rule is hard to reasonably get away from in light of column triggers (T5474). Not impossible, but hard.
  • The "edit permission is sufficient" part of this rule is easy to get away from, but we don't actually have use cases for requiring an additional project permission component right now (T6502).
chad awarded a token.Apr 4 2016, 11:33 PM
epriestley added a comment.EditedMar 1 2019, 10:46 PM

Currently, if a card requires MFA to edit, we're unable to prompt the user. We likely should be able to do a prompt inline. This should also probably have a warning icon on the card itself.

"Soft Locked" statuses may have a similar issue.