Page MenuHomePhabricator

Store "resigned" as an explicit reviewer state
ClosedPublic

Authored by epriestley on Mar 22 2017, 1:49 PM.
Tags
None
Referenced Files
F15390763: D17531.id42173.diff
Sat, Mar 15, 6:33 AM
F15376507: D17531.diff
Thu, Mar 13, 4:11 AM
F15345583: D17531.id42168.diff
Mon, Mar 10, 11:32 AM
F15289750: D17531.id42168.diff
Tue, Mar 4, 10:16 PM
Unknown Object (File)
Sun, Mar 2, 4:26 AM
Unknown Object (File)
Wed, Feb 26, 7:22 PM
Unknown Object (File)
Mon, Feb 17, 11:26 AM
Unknown Object (File)
Sat, Feb 15, 12:02 PM
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
Branch
resign1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 16083
Build 21339: Run Core Tests
Build 21338: arc lint + arc unit