Phabricator dashboard wrongly showing revision as "Waiting on Authors," even though reviewer had changed state to approved
Categories
(Conduit :: Phabricator, defect)
Tracking
(Not tracked)
People
(Reporter: bugzilla, Unassigned)
Details
(Keywords: conduit-triaged)
There was a patch whose review I had neglected for a month because Phabricator was not categorizing it correctly in my dashboard.
If you look at the history of https://phabricator.services.mozilla.com/D14645, you will see that, at https://phabricator.services.mozilla.com/D14645#370126, njn requested changes to the revision.
However, at https://phabricator.services.mozilla.com/D14645#373877, njn then approved the revision.
At this point, the patch was approved by standard8 and njn, but I was still flagged as a blocking review.
Unfortunately, despite njn's approval, the revision remained in the "Waiting on Authors" section of my dashboard instead of being upgraded to "Must Review".
In fact, now that I have approved the revision, all reviewers have approved it, and it is now landable, the revision still appears on my dashboard as "Waiting on Authors."
Unfortunately, despite njn's approval, the revision remained in the "Waiting on Authors" section of my dashboard instead of being upgraded to "Must Review".
what are you using as a dashboard? is it the default phabricator homepage, or a custom search?
unfortunately custom searches can't always reflect the full state of a revision, especially when there's multiple reviewers.
|
||
Comment 3•7 years ago
|
||
:imadueme, is there any chance you could investigate what's going on here? My current theory is that Phabricator doesn't treat "Request Changes" and then "Approve" as removing the need for changes, even when it's from the same user. If this is the case it'd probably be worth marking as upstream and filing.
|
||
Comment 4•7 years ago
|
||
Hi, I've looked into this and I didn't find a bug with phabricator; looks like it was just human error.
Phabricator has non-blocking reviewers and blocking reviewers. Blocking reviewers are indicated by a ! when specifiying them on the command line or by the "blocking" option when choosing them manually from the UI dropdown.
If there are 2 non-blocking reviewers on a revision and one of them accepts it, then the revision is in the "Ready to Land" section for the author (since they got >= 1 approval) and the "Waiting on Authors" section for all the non-blocking reviewers (since only 1 of them must review by Phabricator's logic, the others don't need to take any action). This is the behavior you are seeing.
The revision title on https://phabricator.services.mozilla.com/D14645 would make it seem like njn! and aklotz! were both marked as blocking reviewers (which would make sure that everyone must review) but if you look at the timeline here, https://phabricator.services.mozilla.com/D14645#367804, you'll see that Standard8 changed the reviewer list and added those two as non-blocking reviewers.
There is potential for UI improvments here: when adding a reviewer in the Phabricator UI, you have to type their name, wait for the dropdown list to appear and then click on the "blocking" version of their name. Adding an '!' at the end of their name doesn't add them as a blocking reviewer - that only works on the command line.
Those are my findings, I'll let you all figure out how to resolve this bug.
|
|
||
Updated•7 years ago
|
|
|
||
Comment 5•7 years ago
•
|
||
:imadueme, is there any chance you could investigate what's going on here? My current theory is that Phabricator doesn't treat "Request Changes" and then "Approve" as removing the need for changes, even when it's from the same user.
I've confirmed that this is not the case and things work as they should on phabricator-dev.
Description
•