Actually what happens is that Phabricator doesn't want that a task is closed without anybody assigned to it. If the task wasn't assigned while it was open, then the last resort is to assign it to the person that is closing it.

Possible alternatives would be:

• Nobody can close a task until it is assigned. This would upset users a lot more than the current behavior.
• Tasks can be closed without being assigned to anybody. Si nobody is accountable? I don't think this is a good idea.

A simple solution (and what I do with the tasks I close) is to try to assign it to someone more appropriate than myself before closing it. If I can't find any good choice, having a closed task assign to me is not a big deal.

I habitually did that on Bugzilla too, usually just finding the person who submitted the patch last, but Phabricator makes filling in this data a lot more awkward than Bugzilla.

One way to make this less awkward is to have the "Action"->"Change Status" dropdown trigger the display of the "Assign To" field when the current assignee is "None", in a manner similar to how it shows up when you select "Action"->"Reassign / Claim".

That would be neat!

Hypothesis: the person who closes a task as invalid or stalled takes the responsibility of this status, and so is the correct assignee of the task.

In T84833#933373, @Dereckson wrote:

Hypothesis: the person who closes a task as invalid or stalled takes the responsibility of this status, and so is the correct assignee of the task.

this is pretty much my thinking as well

It would seem, at a minimum, that any action taken like this should show up in the preview area, per TTO's original report. That would open up another possibility from a user interface perspective, which would be to have any of the automatic actions like this have an "x" beside them in the preview area, which allows them to optionally not be done. That would also make it so that if someone doesn't want to perform some of the other automatic actions (such as subscribing to an issue as an artifact of commenting) could also stop that from happening.

Hypothesis: the person who closes a task as invalid or stalled takes the responsibility of this status, and so is the correct assignee of the task.

"Stalled" tasks are not closed and don't cause this behavior. It is reasonable for "Invalid" and "Declined" (definitely surprising when you come from Bugzilla, but reasonable). It is not always reasonable when closing as "Resolved".

That would also make it so that if someone doesn't want to perform some of the other automatic actions (such as subscribing to an issue as an artifact of commenting) could also stop that from happening.

That would also be insanely neat. Another major use case is not adding the task to the projects mentioned in the comment.

This causes assignee drift when people close and reopen a task. Some sort of "automatically assigned" flag, which would make the assignment auto-clear on reopen, and which would change slightly how the assigned field and the history line are phrased, would be an improvement.

In T84833#934094, @Tgr wrote:

This causes assignee drift when people close and reopen a task.

IMO if you closed a task you were responsible for its closing, so if it got reopened you are also responsible to potentially move the task "into the right hands" (may that be some other assignee, or no assignee at all).
If you like it or not when it comes to your time management, but you definitely have to deal with the task again and cannot simply completely ignore the reopen action (because the task shows up in your "Assigned" query). That doesn't sound like a bad thing to me in a FOSS project.

This automatic behavior breaks a lot of team workflows, and clutters the task list . Frequently, a team will go through and do a triage, with a single member doing the mechanical work. The automatic assignment provides a huge disincentive to be the person to do this work, and makes it less likely that teams will do triages (leaving more work for @Aklapper to shoulder).

Assignment should be a deliberate action. When we look at the list of bugs assigned to someone, we should be able to know that's actually what they are working on, or at least something they've agreed to take on. It's toxic if we allow the sloppy state of "oh, that was automatically assigned to me, I'm not actually working on it".

Current behavior was implemented in https://secure.phabricator.com/T25 but no reasons given.

If we wanted to change the current behavior and not set yourself as assignee when closing a task without assignee:

[acko@canis phabricator]$ diff -pu
--- src/applications/maniphest/controller/ManiphestTransactionSaveController.php.old
+++ src/applications/maniphest/controller/ManiphestTransactionSaveController.php	
@@ -107,17 +107,6 @@ final class ManiphestTransactionSaveCont
 
     if ($action == ManiphestTransaction::TYPE_STATUS) {
       $resolution = $request->getStr('resolution');
-      if (!$task->getOwnerPHID() &&
-          ManiphestTaskStatus::isClosedStatus($resolution)) {
-        // Closing an unassigned task. Assign the user as the owner of
-        // this task.
-        $assign = new ManiphestTransaction();
-        $assign->setTransactionType(ManiphestTransaction::TYPE_OWNER);
-        $assign->setNewValue($user->getPHID());
-        $transactions[] = $assign;
-
-        $implicitly_claimed = true;
-      }
     }
 
     $user_owns_task = false;

Let's discuss our processes first. If I'm resolving an unassigned task and I don't want to be the assigned user, what are the alternatives?

1. Another user
   1. Someone active in the task
   2. The author of the task
   3. A maintainer of a project the task is associated with
   4. @Aklapper as phabmaster phallback
   5. A Nobody user we could consider to create
2. (blank, currently not supported)
   
   I don't think we should deviate from Phabricator upstream to allow (blank) assigned in resolved tasks. There is no lack of options for task resolvers to assigning the task to someone else.

I stand by my earlier comment. This is broken.

In T84833#947963, @Qgil wrote:

1. (blank, currently not supported)

It's supported on a low level: you can unassign yourself from a closed task without reopening it and nothing explodes. (Also, auto-assignment doesn't happen if you use the full edit form to close the ticket.) So there isn't any software-level assumption in Phabricator that closed tasks always have an assignee.

On a UI level, none of the other options are supported either: when you select resolved from the comment box, you cannot set or change the assignee. It does not seem easier, or more meaningful, to implement those behaviors than simply not assigning at all.

While this might be broken for users unaware of the problem, we can find a simple and effective solution to the use cases described here affecting a very few people triaging lots of tasks.

Speaking of brokenness, Bugzilla forced us to have a user assigned to a task since the moment of its creation. This is how "Nobody <wikibugs-l@lists.wikimedia.org>" and other fictitious users became owners of massive amount of tasks, and this is how thousands of bugs were resolved without being associated to any real user.

So in practice, both Bugzilla and Phabricator won't allow anybody to resolve a task without a user assigned. The only difference is that Phabricator reflects better the real (lack of) ownership of a task until its resolution. At that point an assigned must be decided. If it's not the user resolving the task, then that user still can decide who (no less than five sensible options were provided in my previous post). If such user still has no clue, we could open the possibility of ending up in the same situation as in Bugzilla, offering a "Nobody" user as fallback.

In T84833#948047, @Tgr wrote:

In T84833#947963, @Qgil wrote:

1. (blank, currently not supported)

It's supported on a low level: you can unassign yourself from a closed task without reopening it and nothing explodes. (Also, auto-assignment doesn't happen if you use the full edit form to close the ticket.)

Yes, this is what I wanted to say when I edited the title and description (but it's late and then I forgot).

Therefore, if users triaging tasks edit the task instead of using the Comment action, then the tasks can be resolved without them or anybody else being assigned to the task. Initial problem solved, and then we can propose or work on UI improvements upstream. But as said above, I don't think this is a reason strong enough to patch our Phabricator instance locally.

While this may not justify a local patch, this is a pretty significant productivity regression from Bugzilla as we had it configured (and as I believe @Tgr was trying to point out in the totality of his comment). At a minimum, this merits an upstream report.

Here's what I'd recommend reporting upstream:

1. Request to revert https://secure.phabricator.com/T25. For those that actually want this behavior, can't this be implemented with Herald now? I suspect that wasn't possible at the time that this was requested, and it seems like site policy that shouldn't be hardcoded.
2. Any action that is taken (such as assignment) should show up in the preview area. This is likely too difficult to implement for Herald rules (or maybe not?...) but for hardcoded behavior like this, it's a minimum courtesy.
3. Provide an opt-out mechanism for any automatic action (e.g. an "x" that shows up on hover on the action as displayed per request #2)
4. Have "Action"->"Change Status" dropdown trigger the display of the "Assign To" field when the current assignee is "None", , in a manner similar to how it shows up when you select "Action"->"Reassign / Claim".

4. Have "Action"->"Change Status" dropdown trigger the display of the "Assign To" field when the current assignee is "None", , in a manner similar to how it shows up when you select "Action"->"Reassign / Claim".

I spent an hour or so figuring out how task editing works in Phabricator, and this actually looks like it would be very easy to implement.

And if it is implemented, then I think we won't need to ask to have upstream T25 reverted; the field would just display your name by default (which is often sensible and convenient) and would let you delete it (when it isn't sensible). So this would magically resolve your points 1 and 3, too.

Implementing correct preview for all this (point 2) might be less trivial, but definitely looks doable. I haven't quite figured out how previewing works, but I am quite sure it involves magic.

So… if anyone files a coherent report upstream, I'll send in a patch. :)

Wontfixed upstream, you can read the reasoning at https://secure.phabricator.com/T6905

I think the solution is: if you want to close an unassigned task without being it assigned to you, do it via "Edit Task".

Maybe we can change the wording to 'Claim and change status'?

About 'Claim and change status', most complex tools have little surprises that you learn by doing a first time, but then you don't need to learn anymore. This is one of those. I don't think it's worth to change anything. Whoever is unhappy about the current behavior is not because they don't know what closing a task does, but because they know, and they dislike the behavior. A longer label doesn't help imho.

I disagree. As you pointed out in T84833#964319, closing a task does not inherently mean you assign it to yourself, as this doesn't happen via the Edit Task pane. The action is therefore not 'close a task which implies you also assign it to yourself' but 'close a task and assign it to yourself'.

The main reason to change the wording is to clarify this is not the right tool to use when you just want to close the task without also assigning it. This solves the issue of tasks accidentally assigned to the closer, even though this was not the intention.

And you're right, it does not solve the issue that people think it should just close the task, but the Edit Task pane is a workaround for that.

Hmph. I'm going to write that patch anyway.

https://github.com/phacility/phabricator/blob/21dca29c5f2929064cfe969cfd4bd417df5621bf/src/applications/maniphest/controller/ManiphestTransactionSaveController.php#L107

You only get assigned if you /close/ the task, not if you change the status to, say, 'stalled'. In addition, you don't claim the task if the task had already previously been assigned. This means my suggestion of renaming it to 'claim and change status' is not correct.

However, it does mean it might be less of an issue: there's no assignee drift, and you won't get assigned if you close the bug for the assignee. The triaging case still stands, but it might not be too unreasonable to ask whoever closed the bug during that stage to also respond to questions/comments from (probably) the reporter.

After discussing with @matmarex on IRC, some notes for a possible patch to change the behavior to 'show assign to field'

• the assign_to field is actually in the HTML, just hidden. This probably runs through behavior-transaction-controls.js, but i'm not quite sure what that hooks into.
• in ManiphestTransactionSaveController.php, the assignment transaction is created at line 107.
  • the correct user to assign to is in $request->getArr('assign_to'); see above for details on how to access it
  • make sure to check for the edge case where the user is already assigned (this if block)

Here's a patch.

I have yet to test it, but I'm pretty sure it will work, and I'm definitely sure it's the right approach. The patch should be easy to maintain locally if it comes to that.

Thank you! What about deploying it in phab-01 or somewhere where it can be
tested and submitting it upstream for review when we are happy about the
feature?

Seeing how the upstream discussion went, I think they're is still a chance.

I'd be happy to see it deployed, but as I noted, I haven't ran the code yet.

Not sure how upstream would feel about it, I focused on keeping the number of lines in the diff as small as possible (assuming that it would have to be maintained locally), rather than making the code abstract and extensible and such.

If the UI is convincing then the code itself won't be the showstopper, I believe. I there is anything that they dislike they will rewrite it. However, if the UI is not convincing, your code can be magical but it won't go through.

@Qgil: I understand @matmarex's hesitation here, given the upstream thread. I think keeping the patch light is probably the right way of going about it, perhaps just linking to P204 from the upstream bug as an FYI for others who want to patch their installations rather than as something for upstream still needs to explicitly consider.

In any case, the patch needs to be deployed somewhere like phab-01 to be
tested. Also if we want to have it as a local patch only...

I have gotten Phabricator set up locally, tested the patch, fixes the bugs in it and updated the paste P204. How do we get it deployed on phab-01?

I'd like to see @matmarex's patch deployed. It's been over 7 months now.

CC'ing @20after4 as I can't judge the potential additional downstream maintenance costs.

It's not entirely a trivial patch, but it's probably a fairly stable part of the codebase. I'm not sure how much maintenance cost would be involved, I'll look it over more thoroughly and think it over a bit.

Can we please just get this on the test instance and worry about whether it can be maintained in production later? I run into this issue on nearly a daily basis and I want to know whether this patch fixes it.

@Krenair: The patch still applies cleanly so I'll try to get it deployed somewhere

At HEAD, you probably need to do something like this:

diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php
index 922317b..c89a66f 100644
--- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php
+++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php
@@ -970,7 +970,7 @@ final class ManiphestTransactionEditor
 
     // If the task is not assigned, not being assigned, currently open, and
     // being closed, try to assign the actor as the owner.
-    if ($is_unassigned && !$any_assign && $is_open && $is_closing) {
+    if (false) {
       // Don't assign the actor if they aren't a real user.
       if ($actor_phid) {
         $results[] = id(new ManiphestTransaction())

I think that should be stable for a long time.

I vaguely attempted to remove this behavior in https://secure.phabricator.com/D14670, but didn't really push it.

Actually I just found https://secure.phabricator.com/T6905, that one went a bit weird. I'll just push this upstream myself. Upstream is now https://secure.phabricator.com/T10343.

After great deliberation, the upstream task (https://secure.phabricator.com/T10343) is now resolved:

• New claim property for maniphest.statuses.
• Defaults to true if omitted.
• If set to false, closing a task into that status does not trigger the claim behavior.

Thus:

• You can disable this behavior completely by setting "claim": false on all closed statuses.
• Or, you can disable this behavior selectively by setting "claim": false on, say, "Invalid" and "Duplicate", but leaving the behavior active on "Resolved".

In T84833#2021596, @epriestley wrote:

After great deliberation, the upstream task (https://secure.phabricator.com/T10343) is now resolved:

• New claim property for maniphest.statuses.
• Defaults to true if omitted.
• If set to false, closing a task into that status does not trigger the claim behavior.

Thus:

• You can disable this behavior completely by setting "claim": false on all closed statuses.
• Or, you can disable this behavior selectively by setting "claim": false on, say, "Invalid" and "Duplicate", but leaving the behavior active on "Resolved".

This is awesome!

So just to be clear here, will the Wikimedia Phabricator configuration be changed to use the new claim property after the update? If so, what is the proposed configuration?

Also, when I originally filed this issue, I wrote that one of my concerns was that "the preview area underneath the comment box doesn't warn me that I am about to claim the task." Will that also be addressed in the coming update?

In T84833#2037636, @TTO wrote:

I wrote that one of my concerns was that "the preview area underneath the comment box doesn't warn me that I am about to claim the task." Will that also be addressed in the coming update?

Yes.

I'm not sure what the value should be for claim, however, simply having the ability to override it is good enough for me.

In T84833#2037941, @mmodell wrote:

I'm not sure what the value should be for claim, however, simply having the ability to override it is good enough for me.

I think "claim": false for everything is probably the best value. It's a little too easy to accidentally claim something when merely trying to do Phab janitorial work, and it's very easy to claim upon closing an issue. The autoclaiming behavior still has a little bit too much of a mystery-meat flavor to it.

Thanks for getting the new version deployed!

@RobLa-WMF: Thanks, I think I agree with you on this. I'll change it and if people complain they can make a case to change it back ;)