Page MenuHomePhabricator

Restricting modification of tasks when they enter sprints
Closed, DeclinedPublic4 Story Points

Description

Idea

What if tasks would keep being as open as they are, but as soon as they are assigned to a sprint project they would be partially protected? Only the assigned and the members of an invitation-only group (i.e. "Sprinters") would be able to perform these actions:

  • Edit Assigned
  • Edit Priority

Ideally, workboards of sprints and releases would be also editable only by this group.

This means that tasks in backlogs could be highly editable by anybody, but as soon as these tasks would enter a sprint or a release, they would obtain a certain level of protection. Title, CC, Projects, Security, and Descriptions would be still editable at all times.

Let's discuss about the principle first, then we can look at the implementation.

Background

Project managers, product owners and profiles alike might be concerned about the possibility for just anybody to land in their projects and start changing priorities and assigned fields against their will, knowing or not what they are doing, in good faith or as pure vandalism. After some conversations at the Wikimedia Foundation offices I can assess that such concern exists.

Part of this comes from the change from a fully controlled environment in Trello or Mingle to Phabricator's openness. Part of this comes from prioritization wars that we have seen in Bugzilla, with a risk of becoming more explosive since the scope now is not limited to bugs. And well, already in the pre-launch period we are seeing that it is easy that different people have different background and expectations when it comes to play with task fields and boards.

Related Objects

Event Timeline

Qgil created this task.Oct 23 2014, 12:53 AM
Qgil raised the priority of this task from to Normal.
Qgil updated the task description. (Show Details)
Qgil added a project: Project-Management.
Qgil changed Security from none to None.
Qgil added a subscriber: Qgil.
Qgil updated the task description. (Show Details)Oct 23 2014, 4:04 AM

Ideally, workboards of sprints and releases would be also editable only by this group.

This is possible already now. When creating the project for a sprint or a release, the Editable By policy must be set to the team. Then only them will be able to move cards in the workboard, or to modify anything else there.

Qgil added a comment.Nov 7 2014, 9:31 PM

Meh, I was wrong. The columns are not editable, but the cars can be moved. Ok, we'll need to find another solution.

I was wondering whether this task could be technically solved by enforcing a policy for tasks associated with Sprint type of projects. The projects provided by the Sprint extension are essentially different than the stock Maniphest projects, so maybe we can play with restrictions around the Sprint scope.

Qgil renamed this task from Restricting modification of tasks when they enter sprints or releases to Restricting modification of tasks when they enter sprints.Dec 3 2014, 10:46 PM
Qgil updated the task description. (Show Details)
Qgil lowered the priority of this task from Normal to Low.Dec 8 2014, 9:36 AM
Christopher edited a custom field.Dec 11 2014, 8:18 AM

Please make sure this remains optional. Not all users of the sprint extension will want to have this edit protection.

Dzahn added a subscriber: Dzahn.Dec 11 2014, 12:40 PM

"... concerned about the possibility for just anybody to land in their projects and start changing ... knowing or not what they are doing, in good faith or as pure vandalism"

it sounds a bit like concerns people had about general wiki editing by "just about anybody". i think it suits us well as the wiki foundation to be as open as possible about editing. changes could also be reverted easily and if there'd be revert wars we could block people after being warned..

Qgil added a comment.Dec 11 2014, 1:01 PM

... one problem being that MediaWiki pages are born to be reverted to any point in history. Phabricator boards, not so much.

Sure they are, the full edit history is there. Each change in board column is recorded as a line in the ticket. (Though can't find an edit history for the board configuration itself, meaning the column names and their order.)

I think it would be fair to say that referenceable history exists, but not restorable history. These are quite different in terms of impact.

Qgil added a comment.Dec 11 2014, 2:22 PM

We should also take this in the context of sprints, which are 2-4 week active projects containing tasks that are supposed to be owned by someone, and that are supposed to be reviewed by the team frequently during the sprint. In this context, the chances that external moves of cards will bring more trouble than good are high.

In the meantime, the same tasks will still exist in their original, non-sprint projects with workboards...

Even if the restriction would be enforced, the real point of control would be in the project membership:

  • If only Sprinters can move tasks in Sprint workboards (as proposed, but I have no strong opinion), then you only need to be accepted in Sprinters.
  • If only the Sprint members can move tasks, then it is up to the original members to define the Can Join policy. If you leave it open, then anybody can move the cards after joining.

In both cases, you can kick out someone misbehaving pretty easily.

Yes, there might not yet be a tool to mass revert a selected set of changes (like everything user x has done in the last day to project x so as to revert vandalism to a board). That is worthwhile to have, just in case, even for things that are not related to sprints.

Qgil added a comment.Dec 23 2014, 6:57 PM

@Christopher, since this task is "Doing", can you share your current ideas / opinions / plans about this feature, please?

There are several possibilities with this that I have worked on so far, though none of them are that attractive. First point that should be made clear is that it is not possible to override any of the default projects or maniphest capabilities as they are used in the final class controllers of these applications.

Thus, it is not possible to restrict Maniphest task editing globally from Sprint. However, the SprintBoardTaskEditController is entirely within Sprint (as a fork) so it can be changed to use Sprint capabilities rather than Maniphest's. So, for a board-centric approach to task management, this may be useful for hiding fields in the form. But, this would not prevent anyone determined to change a task from doing it in Maniphest 'prime'.

Regarding the Sprint Board, this uses the capabilities as defined by the project itself in the details view. Since these (currently limited to edit, view and join) can be set per project, I do not see the merit in developing anything more elaborate just for a Sprint project. It is possible to implement a new custom field and create a new capability if someone really wanted it, however.

I think that the simple solution is to set the project edit policy to a selected project group. As Quim mentioned. the control would therefore be in who gets admitted to the group. There is really no reason to distribute the capability of editing a project (by means of a Sprint Board) that I can see. This should definitely be centrally managed by a few people at the most.

Qgil added a comment.Dec 29 2014, 10:21 AM

I'm not sure if I'm following but is this what you are saying?

  • The permission of moving cards in Sprint generated boards can be restricted.
  • The permission of moving cards in Sprints could be given to a global, invitation-only team.
  • Priority and Assigned fields are global, and they cannot be restricted at a Sprint level.

Although having a global team i.e. "Sprinters" would be a good step, I think each team will prefer to have such permission restricted to their own team only. I have no idea how feasible this is, though.

Qgil moved this task from To Triage to Need discussion on the Phabricator board.Dec 29 2014, 10:21 AM

Right. The discussion here https://secure.phabricator.com/T5204 references the idea of restricting the moving of cards on a workboard. It was said that they loosened the previous policy restriction of needing the capability to edit a project in order to move a card in this commit https://secure.phabricator.com/rPaedb694ad6e0892018937c30bf409d720a7b2ed4.

If we wanted to re-implement this policy for Sprint Boards based on the Project "Can Edit" permission, it is a simple one-line change. I think that it is logical to assume that only the team members of project should have the ability to edit it (and move cards).

I am not sure why ordinary users should be allowed to change the workboard, but if this is a requirement, then waiting for upstream to implement the "Can Edit Workboard" capability for projects is the other alternative.

Qgil added a comment.Jan 13 2015, 7:22 AM

I think it is worth trying this restriction in the context of Sprint projects.

gerritbot added a subscriber: gerritbot.

Change 185829 had a related patch set uploaded (by Christopher Johnson (WMDE)):
adds require PhabricatorPolicyCapability::CAN_EDIT to SprintBoardMoveController

https://gerrit.wikimedia.org/r/185829

Patch-For-Review

Change 185829 merged by Christopher Johnson (WMDE):
adds require PhabricatorPolicyCapability::CAN_EDIT to SprintBoardMoveController

https://gerrit.wikimedia.org/r/185829

Christopher closed this task as Resolved.Feb 19 2015, 9:03 AM

This functionality was merged into the sprint extension code base but for this Wikimedia Phabricator instance I set "Can Edit Task Status: All Users" (see T89843). "Can Assign Tasks: All Users" was already set.

If changing the current (global!) policies in this Phabricator instance is wanted, a dedicated task in Wikimedia Phabricator against the "Phabricator" project is welcome, plus that obviously needs broad discussion and agreement (team-practices input?) as this cannot be set on a per-product level as far as I understand?
Just saying.

Qgil reopened this task as Open.Feb 19 2015, 2:25 PM

Reopening, since we went back to the previous square. What was the custom policy overridden by T89843?

See T819#841621 for my reasoning about why this restriction for Sprint projects is fine. I will add this task to the Phabricator project for further discussion, if needed.

I'm fine with the revert anyway, since https://gerrit.wikimedia.org/r/185829 was self-merged by @Christopher, and we have agreed that self-merges should be avoided here (just like we are avoiding them in any Wikimedia project except for urgencies). But I still think that the restriction for sprints is worth trying.

I am not clear about the relationship between implementing the "capability" as was done in the change to the Sprint extension and the actual policy itself. Since this capability, "Can Edit", is a Project application capability, the global policy restrictions can still occur at the application level. Thus, the fact that this was a "self-merge" seems to be off the subject of the implementation of policy. The code change enabled the implementation of policy for the moving of board cards, but did not in any way enforce that policy.

In T819#1049711, @Qgil wrote:

Reopening, since we went back to the previous square. What was the custom policy overridden by T89843?

Admins and triagers.

I'm fine with the revert anyway, since https://gerrit.wikimedia.org/r/185829 was self-merged by @Christopher, and we have agreed that self-merges should be avoided here (just like we are avoiding them in any Wikimedia project except for urgencies). But I still think that the restriction for sprints is worth trying.

If we put that back in place and if there is agreement (asking on teampractices?) we definitely need to announce and document it as it's not too obvious. Plus there's no opt-out per project.

Okay, I should have reloaded this page to see Christopher's comment before adding my comment. Meh.

I think the next step would be to document the functionality by answering the question "How can I restrict changing the status of a task and the assignee of tasks in my project which actively uses a workboard (= not just one huge default Backlog column and nothing else)"?

Is there an actual problem that's being addressed here or is this task living only in the world of theoretical problems? If there's an actual problem, such as users changing the assignment of tasks maliciously and in bad faith, we should address that, but first we'll need concrete examples of the problem we're trying to solve. As it is, this task talks a lot about adding restrictions but I'm not sure I see any evidence that this is needed.

Qgil added a comment.Feb 20 2015, 8:17 AM

It is marked as a blocker of T84: Make sure anti-vandalism features are up to snuff as a preventive measure, yes. We can agree not to do this now, and wait if we run into these problems.

Some people have been (legitimately, I think) concerned about seeing problems in Phabricator as we say in Bugzilla times, with prioritizations wars and more. As far as I'm aware, we have not seen them yet here, but at this point we cannot be sure whether this is due to a better environment or just the fact that Wikimedia Phabricator's history is very short. I'm honestly hoping for the former, since Phabricator and the sprint model allows everybody to understand better what keeps developers busy and how is the backlog for next sprints looking like.

Therefore, as creator of this task I'm happy to keep this task stalled, waiting to see how much this feature is needed.

Aklapper changed the task status from Open to Stalled.Feb 20 2015, 2:29 PM
In T819#1052452, @Qgil wrote:

It is marked as a blocker of T84: Make sure anti-vandalism features are up to snuff as a preventive measure, yes. We can agree not to do this now, and wait if we run into these problems.
[...]
Therefore, as creator of this task I'm happy to keep this task stalled, waiting to see how much this feature is needed.

Sounds good to me for the time being.
If disruptive behavior is observed we can still change the current settings and fall back to this functionality.

Aklapper lowered the priority of this task from Low to Lowest.Jul 31 2015, 5:05 PM

If disruptive behavior is observed we can still change the current settings and fall back to this functionality.

Since 02/2015 I have not gotten aware of "bigger"/long-term cases, only "smaller" spamming or vandalism of specific and newly-created accounts, hence lowering priority.

If there are problems, please make me aware of them.

Restricted Application added a subscriber: scfc. · View Herald TranscriptJul 31 2015, 5:05 PM
Qgil added a comment.Jul 31 2015, 9:38 PM

Actually, I propose to decline this task.

I created it based on an hypothetical scenario drawn by Trello users back when we were organizing their migration to Trello. Since then, we haven't seen any misbehavior that would justify this development. It can happen any day, but... I think a better prevention of this kind of abuse must be more generic, like throttling the number of actions done by new accounts or something (that I believe we started discussing in another task).

For these reasons, I don't see any reason to keep this task open.

Aklapper closed this task as Declined.Aug 3 2015, 9:07 AM
Aklapper claimed this task.

Closing for the time being as we won't take action. Could be reopened when reconsideration is needed.

Restricted Application added a subscriber: Luke081515. · View Herald TranscriptFeb 12 2016, 8:39 AM

I created it based on an hypothetical scenario.
If there are problems, please make me aware of them.

I've had multiple problems in the last months with ticket priority, making me lose many hours of time on explanations, I made them aware of you, @Aklapper, with no response. However, they were not vandalism or anonymous users.

I've had multiple problems in the last months with ticket priority, making me lose many hours of time on explanations, I made them aware of you, @Aklapper, with no response. However, they were not vandalism or anonymous users.

@jcrespo: I am sorry to hear that. :( Could you share how you tried to make me aware, please?

I checked my mail inbox for emails sent by you and could not find any heads-up about such issues, and I cannot remember a message on my private talk pages either...
For future reference, could you drop me a short private email please, so I could take a look and point to Setting task priorities?

I also might have missed some Phab notifications (I'm subscribed to nearly all tasks but do not always read all notifications as it won't scale).
Looking at those 18 tasks assigned to you and including the word "priority", I found T129432#2210751 where you reverted an unwarranted priority change, T108856#1620180 where the initial priority set by Analytics was changed, and T103345#1408281 by a community member questioning the priority but not changing the field value. I did not spot corrections of cases when the reporter set an inappropriately high initial priority field value (instead of keeping the default "Needs Triage") but that should be harder nowadays anyway, as the initial task creation form does not expose the field anymore and requires a separate step.

If you have some examples handy, could you provide them?

@Aklapper I will send you an email in private, I just wanted to discuss options- but last thing I want is to point a finger in public so thath people could get offended (and it is not my intention to prevent legit ticket editing). We can go back here when (if) we substantiate a generic but real issue.

@Aklapper I will send you an email in private

@jcrespo: That email is still welcome. :)

Dzahn removed a subscriber: Dzahn.Jun 21 2016, 3:41 AM