HomePhabricator

Update overall revision status after reviewers change

Description

Update overall revision status after reviewers change

Summary:
Ref T2222. This doesn't feel super clean, but doesn't feel too bad either.

Basically, Differential transactions can have secondary state-based effects (changing the overall revision status) when reviewers resign, are removed, accept, or reject revisions.

To deal with this in ApplicationTransactions, I did this:

  • applyFinalEffects() can now alter the transaction set (notably, add new ones). This mostly matters for email, notifications and feed.
  • In Differential, check for an overall revision state transition in applyFinalEffects() (e.g., your reject moving the revision to a rejected state).
  • I'm only writing the transaction if the transition is implied and indirect.
    • For example, if you "Plan Changes", that action changes the state on its own so there's no implicit state change transaction added.

The transactions themselves are kind of fluff, but it seems useful to keep a record of when state changes occurred in the transaction log. If people complain we can hide/remove them.

Test Plan: {F118143}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D8339

Event Timeline