Homec4science

Update overall revision status after reviewers change

Authored by epriestley <git@epriestley.com> on Feb 25 2014, 21:36.

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

Details

Committed
epriestley <git@epriestley.com>Feb 25 2014, 21:36
Pushed
aubortJan 31 2017, 17:16
Parents
rPHca8c2c2d11a3: Implement Accept/Reject in ApplicationTransactions, approximately
Branches
Unknown
Tags
Unknown

Event Timeline

epriestley <git@epriestley.com> committed rPHa69cca9fbb15: Update overall revision status after reviewers change (authored by epriestley <git@epriestley.com>).Feb 25 2014, 21:36