1482110 - Some reviews don't appear in my dashboard

Status: RESOLVED WONTFIX

(bugzilla.mozilla.org :: MyDashboard, enhancement)

Description

[Mike Hommey [:glandium]]

If I go to https://phabricator.services.mozilla.com/, I see "Must review" for D1605, but the list of phabricator review on my bmo dashboard is empty.

Comment 1

[David Lawrence [:dkl]]

THis is another corner case we didnt anticipate when adding the section to my dashboard. The reason it is not showing up is that you rejected an earlier diff, and then a new diff was added that shows up in the Must Review section of the Phabricator home page.

The BMO dashboard needs to to take this into account and look at the diff id you rejected and then see if there is a newer diff id associated with the revision. If so then show the revision in the Phab section of my dashboard.

Note to patch writer: 

$ echo '{
  "constraints": {
    "ids": [
      1605
    ]
  },
  "attachments": {
    "reviewers": 1,
    "reviewers-extra": 1
  }
}' | arc call-conduit --conduit-uri https://phabricator.services.mozilla.com/ --conduit-token <conduit-token> differential.revision.search

This should allow us to get the data needed to make the determination.

Comment 2

[Steven MacLeod [:smacleod]]

(In reply to David Lawrence [:dkl] from comment #1)
> Note to patch writer: 
> 
> $ echo '{
>   "constraints": {
>     "ids": [
>       1605
>     ]
>   },
>   "attachments": {
>     "reviewers": 1,
>     "reviewers-extra": 1
>   }
> }' | arc call-conduit --conduit-uri
> https://phabricator.services.mozilla.com/ --conduit-token <conduit-token>
> differential.revision.search
> 
> This should allow us to get the data needed to make the determination.

To take a look at how lando makes these decisions see https://github.com/mozilla-conduit/lando-api/blob/024d610ba85992d745403c5aa799cbbb6cb6ba9a/landoapi/reviews.py#L17

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

I have atm 3 reviews in Phabricator which don't show up in bugzilla dashboard.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

https://phabricator.services.mozilla.com/D3218
https://phabricator.services.mozilla.com/D3023
https://phabricator.services.mozilla.com/D3346

Comment 5

[Mark Côté [:mcote]]

This is an indication of how hard it is to map Phabricator's multiple review models to Bugzilla's simple one.  We may need a new strategy here.

Comment 6

[Jim Mathies [:jimm]]

I'm not checking phabricator for these, so I'm missing review requests left and right. This should block roll out of phabricator IMO.

Comment 7

[Mark Côté [:mcote]]

Well, Phabricator is already rolled out... More than 50% of commits to mozilla-central are being reviewed in Phabricator now.

We have faced many problems trying to jam Phabricator requests into Bugzilla's review model; they are incompatible if we want to maintain anything close to the expected semantics of BMO's review flags.  We are very likely going to be ripping Phabricator requests out of My Dashboard, although we may be adding a panel to show_bug.cgi to display info on associated Phabricator revisions, without translating data, as we did with the MozReview table.

Given that before too long pretty much all reviews, for mozilla-central at least, will be going through Phabricator, I suggest keeping a tab open with Phabricator's dashboard, which is intuitive and useful, as well as monitoring Phabricator's emails.

Comment 8

[Botond Ballo [:botond]]

(In reply to Mark Côté [:mcote] from comment #7)
> We are very
> likely going to be ripping Phabricator requests out of My Dashboard

Why would we do that, rather than fixing this bug?

Comment 9

[Mark Côté [:mcote]]

This is not the only bug with the dashboard.  As I said in comment 7, the models are just too different.  There will be a lot of edge cases and/or mappings that are misleading and unintuitive, not to mention that Phacility have said it is likely they will make changes to their system that will break any mappings we have.  Keeping the Phabricator dashboard open in another tab, given that most, if not all, of your reviews will be there eventually, is a much simpler and robust solution.

Comment 10

[Axel Hecht [:Pike]]

I have review requests open to people that mostly work on github, and having yet another system among github and bugzilla needinfo requests is likely going to be me poking people to look at that system where they get requests every now and then.

Comment 11

[Kohei Yoshino [:kohei]]

Bug 1487422 is removing Phab reviews from My Dashboard.