Page MenuHomePhabricator

Store "resigned" as an explicit reviewer state
ClosedPublic

Authored by epriestley on Mar 22 2017, 1:49 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 24, 6:34 PM
Unknown Object (File)
Sun, Nov 24, 2:01 AM
Unknown Object (File)
Wed, Nov 20, 5:14 PM
Unknown Object (File)
Sat, Nov 16, 4:37 PM
Unknown Object (File)
Thu, Nov 14, 7:12 AM
Unknown Object (File)
Wed, Nov 13, 7:32 AM
Unknown Object (File)
Mon, Nov 11, 6:11 AM
Unknown Object (File)
Thu, Nov 7, 11:10 AM
Subscribers
None

Details

Summary

Fixes T11050. Today, when a user resigns, we just delete the record of them ever being a reviewer.

However, this means you have no way to say "I don't care about this and don't want to see it on my dashboard" if you are a member of any project or package reviewers.

Instead, store "resigned" as a distinct state from "not a reviewer", and treat it a little differently in the UI:

  • On the bucketing screen, discard revisions any responsible user has resigned from.
  • On the main /Dxxx page, show these users as resigned explicitly (we could just hide them, too, but I think this is good to start with).
  • In the query, don't treat a "resigned" state as a real "reviewer" (this change happened earlier, in D17517).
  • When resigning, write a "resigned" state instead of deleting the row.
  • When editing a list of reviewers, I'm still treating this reviewer as a reviewer and not special casing it. I think that's sufficiently clear but we could tailor this behavior later.
Test Plan
  • Resigned from a revision.
  • Saw "Resigned" in reviewers list.
  • Saw revision disappear from my dashboard.
  • Edited revision, saw user still appear as an editable reviewer. Saved revision, saw no weird side effects.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable