Page MenuHomePhabricator

Change/drop/reconcile some miscellaneous edit behaviors in Maniphest
ClosedPublic

Authored by epriestley on Dec 4 2015, 6:59 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 24, 11:43 AM
Unknown Object (File)
Thu, Mar 21, 9:54 AM
Unknown Object (File)
Thu, Mar 14, 6:18 PM
Unknown Object (File)
Thu, Mar 14, 6:02 PM
Unknown Object (File)
Thu, Mar 14, 5:52 PM
Unknown Object (File)
Thu, Mar 14, 4:19 PM
Unknown Object (File)
Thu, Mar 14, 3:51 PM
Unknown Object (File)
Thu, Mar 14, 3:41 PM
Subscribers
None

Details

Summary

Ref T9132. Open to discussion here since it's mostly product stuff, but here's my gut on this:

  • Change Maniphest behavior to stop assigning tasks if they're unassigned when closed. I think this behavior often doesn't make much sense. We'll probably separately track "who closed this" in T4434 eventually.
  • Only add the actor as a subscriber if they comment, like in other applications. Previously, we added them as a subscriber for other types of changes (like priority and status changes). This is more consistent, but open to retaining the old behavior or some compromise between the two.
  • Retain the "when changing owner, subscribe the old owner" behavior.
Test Plan
  • Added a comment, got CC'd.
  • Changed owners, saw old owner get CC'd.

Diff Detail

Repository
rP Phabricator
Branch
ee41
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 9292
Build 11030: Run Core Tests
Build 11029: arc lint + arc unit

Event Timeline

epriestley retitled this revision from to Change/drop/reconcile some miscellaneous edit behaviors in Maniphest.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.

eh, I think it's kind of accurate that I own a ticket that I resolve. It's a nice convenience and I think down the road with Facts, will make it less of a burden on a user to claim it as well when closing.

Currently, we claim it for all closed statuses (e.g., wontfix, invalid). Do you want to retain that behavior too?

Yeah, mostly from a Facts point of view. In the future if a manager is generating reports for tasks the team handled, it'd be nice that these were accurate reflections. I can't see basically someone not wanting to claim the task when they reply and WONTFIX it. Or do you think maybe Global Herald is the answer here?

I guess I feel like this creates inaccurate results by destroying data -- we lose the fact that the task was never claimed/assigned from a Facts point of view.

The most common case of close-without-claim I see is general triage (close invalid, close wontfix, merge) where I'm usually doing no actual work and resolving the task doesn't represent anything meaningful from a reporting point of view -- I'm basically just undoing the fact that the task was filed. T9901 is a recent example where I closed a task and implicitly claimed it, but my claim on it is basically meaningless.

We can store "Closed by" (separate from "Owner") after T4434, and surface that in Facts. That would allow generation of two different reports ("Tasks closed by X", "Owned tasks completed by X"). This distinction seems potentially useful to me?

More than that, I just think the behavior is confusing in cases like T9901. The task ending up "Assigned To: epriestley" doesn't make much sense to me.

Anyway, I don't really feel strongly one way or the other. We can go with the current behavior for now, I'll update the diff.

Yeah, I agree this is a bit grey. I like it personally, but I understand it may not be fully reflective of intent. In any regards, has anyone complained about the current behaviour? I have vague recollections of a discussion on this in the past.

I don't immediately recall any prior discussion about it, but could definitely be forgetting something.

Yeah, I think maybe it was with WM, they wanted it to show the assigned field when someone changed the status of an unassigned ticket for clarification.

epriestley edited edge metadata.
  • When closing a previously open, unassigned task and not making other ownership changes, implicitly claim the task.
chad edited edge metadata.
This revision is now accepted and ready to land.Dec 4 2015, 10:00 PM
This revision was automatically updated to reflect the committed changes.

For completeness, T6905 was the WMF complaint about this.