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)
Thu, Nov 28, 7:01 PM
Unknown Object (File)
Wed, Nov 27, 2:02 PM
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
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