Page MenuHomePhabricator

Store "resigned" as an explicit reviewer state
ClosedPublic

Authored by epriestley on Mar 22 2017, 1:49 PM.
Tags
None
Referenced Files
F17769602: D17531.diff
Wed, Jul 23, 8:48 AM
F17712885: D17531.id42168.diff
Thu, Jul 17, 11:35 AM
F17704269: D17531.diff
Wed, Jul 16, 3:29 AM
F17678734: D17531.id42168.diff
Sun, Jul 13, 1:37 PM
Unknown Object (File)
Jun 18 2025, 9:57 PM
Unknown Object (File)
May 8 2025, 7:21 AM
Unknown Object (File)
May 7 2025, 9:09 AM
Unknown Object (File)
May 7 2025, 3:07 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