1538800 - Phabricator shouldn't clear accepted reviews from a blocking reviewer when that reviewer is added as blocking again

Status: NEW

(Conduit :: Phabricator, defect, P2)

Description

[Ian Moody [:Kwan] (UTC+0)]

Same as for non-blocking reviews.

Compare https://phabricator-dev.allizom.org/D1136 and https://phabricator-dev.allizom.org/D1137

This might be related to the fact that phab seems to forget that a review was blocking after it is granted?

Before

{
  "reviewerPHID": "PHID-USER-c3l6wuzeb64pb5hvsorf",
  "status": "blocking",
  "isBlocking": true,
  "actorPHID": "PHID-USER-c3l6wuzeb64pb5hvsorf"
}

After

{
  "reviewerPHID": "PHID-USER-c3l6wuzeb64pb5hvsorf",
  "status": "accepted",
  "isBlocking": false,
  "actorPHID": "PHID-USER-c3l6wuzeb64pb5hvsorf"
}

(What's the point of the isBlocking field when it's redundant to status?)

Comment 1

[Steven MacLeod [:smacleod]]

Can you please provide steps to reproduce here? I'm more interested in the specific workflow case that you're running into and the end result.

Comment 2

[Ian Moody [:Kwan] (UTC+0)]

1. Upload a diff with one blocking and one non-blocking reviewer
2. Reviewers grant review, with nits
3. Upload an updated diff with nits addressed that does the reviewers.add transaction[0] again, in-case you've updated reviewers.
4. Blocking review is re-asked for, regular review is kept but tagged as being on a prior diff.

[0]

{"transactions": [{ "type": "reviewers.add", "value": ["blocking(PHID-USER-blocker)", "PHID-USER-non-blocker"]}]}