1007972 - Review queue length and position no longer visible to developers awaiting review

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Developer Pages, defect, P1)

Description

[Jorge Villalobos [:jorgev] (he/him)]

Go to https://addons.mozilla.org/developers/addon/http-useragent-cleaner/edit

The status box right below the name of the add-on in the title should include queue length and queue position, since this add-on has a version awaiting review.

It works on -dev, however: https://addons-dev.allizom.org/developers/addon/semantic-turkey/edit

This potentially affects hundreds of developers, and we've received complaints about it already, so it needs to be addressed as soon as possible.

Comment 1

[Wil Clouser [:clouserw]]

That add-on doesn't have any versions waiting for review:  https://addons.mozilla.org/en-US/developers/addon/http-useragent-cleaner/versions

On an add-on that does I see the queue position: https://addons.mozilla.org/en-US/developers/addon/hide-firefox-appmenu-button/edit

Comment 2

[Jorge Villalobos [:jorgev] (he/him)]

Yeah, that one turned out to be a bad example, sorry. It looks like this add-on does have that problem:

https://addons.mozilla.org/en-US/developers/addon/the-fox-only-better/edit

Comment 3

[Luís Miguel [:quicksaver] (not reading bugmail)]

(In reply to Jorge Villalobos [:jorgev] from comment #2)
> Yeah, that one turned out to be a bad example, sorry. It looks like this
> add-on does have that problem:
> 
> https://addons.mozilla.org/en-US/developers/addon/the-fox-only-better/edit
Indeed it does, it is mine and I can confirm that the latest version is awaiting review and I can't see its queue position in the dev pages.

Comment 4

[Jorge Villalobos [:jorgev] (he/him)]

I can see the same problem for a couple other add-ons of this same dev, and it looks like for all of the cases there's a previous version that wasn't reviewed (beta or disabled).

Comment 5

[Luís Miguel [:quicksaver] (not reading bugmail)]

(In reply to Jorge Villalobos [:jorgev] from comment #4)
> I can see the same problem for a couple other add-ons of this same dev, and
> it looks like for all of the cases there's a previous version that wasn't
> reviewed (beta or disabled).

FYI another add-on of mine, Omnibar Plus, was just reviewed (30 minutes ago). I also couldn't see its queue position as of today, and that one didn't have any previous non-reviewed version.

Comment 6

[Wil Clouser [:clouserw]]

Does that make this reproducible on -dev?  Is there an example on -dev you can link to?

Comment 7

[Kris Maglione [:kmag]]

Wait times are also broken in the queue. Add-ons uploaded before today all display 'None days'

Comment 8

[Luís Miguel [:quicksaver] (not reading bugmail)]

So, now I can see the queue positions... except I'm sure they aren't correct. For one, the three add-ons of mine that are in the queue were uploaded sequentially, so they had sequential positions, now they don't. Also, two of them are number 50, and that's just weird.

Comment 9

[Yohan Boniface [:ybon]]

Hi Luis, thanks for your infos.
ATM, we are not able to reproduce the bug neither in local nor in dev, and we don't have access to production data to check.
Are you able to reproduce the problem on dev, by chance?

@clouserw: we would also need to check that the 763-populate-version-nomination-from-files.sql migration has been run as expected (as some has been run manually). ATM we cannot access RO to the database, so maybe just check that together when you are up.

Comment 10

[Luís Miguel [:quicksaver] (not reading bugmail)]

(In reply to Yohan Boniface [:ybon] from comment #9)
> Are you able to reproduce the problem on dev, by chance?

If by dev you mean the dev pages, such as https://addons.mozilla.org/en-US/developers/addons, that's what I've been referring to.

The two add-ons that are in the same queue position (49 now):
https://addons.mozilla.org/en-US/developers/addon/the-fox-only-better/edit
https://addons.mozilla.org/en-US/developers/addon/the-puzzle-piece/edit

(Note that I'm not an AMO reviewer, so I'm limited to the add-on developer pages, and if you mean something else I may not have access to it.)

Comment 11

[Yohan Boniface [:ybon]]

No, I was referring to the dev environment, but don't worry.
One question, just to be sure: are your two addons having the same queue position in the same review type (full or preliminary)?

Little update: we have created a database with a schema from before the nomination migration, then update many addons, both on full review and preliminary review, then updated the code to HEAD, and ran migrations. But still no luck to reproduce the described issues.
High presumption of a SQL migration problem during the push (has the migration really run?).
But let's continue to investigate.

Comment 12

[Luís Miguel [:quicksaver] (not reading bugmail)]

(In reply to Yohan Boniface [:ybon] from comment #11)
> One question, just to be sure: are your two addons having the same queue
> position in the same review type (full or preliminary)?

Yes, both are updates awaiting full review, so same queue.

Comment 13

[Yohan Boniface [:ybon]]

Thanks for the update.
Another question, so: do they have been in preliminary review before?

Comment 14

[Luís Miguel [:quicksaver] (not reading bugmail)]

(In reply to Yohan Boniface [:ybon] from comment #13)
> Another question, so: do they have been in preliminary review before?

Those two haven't. But this one has (https://addons.mozilla.org/en-US/developers/addon/omnisidebar/edit, way back in 2011), and that's the one that should have a sequential queue position to the others but doesn't.

Also, I just noticed that the queues position is changing with each page refresh (for all of those three in the queue, OmniSidebar varies between 22 and 34, the other two between 48 and 51 and usually the same number for both), so I'm not sure what's happening...

Comment 15

[Yohan Boniface [:ybon]]

(In reply to Luís Miguel [:Quicksaver] from comment #14)
> Also, I just noticed that the queues position is changing with each page
> refresh (for all of those three in the queue, OmniSidebar varies between 22
> and 34, the other two between 48 and 51 and usually the same number for
> both), so I'm not sure what's happening...

This really sounds like all the rows have the same value, and give more presumption on the scenario where the migration has not run OR not all rows have been affected has expected.
We continue to investigate, thanks for all the infos you provide.

Comment 16

[Yohan Boniface [:ybon]]

We found at least a non handled case, PR here https://github.com/mozilla/olympia/pull/85

Quick technical overview: when a public addon has a new version uploaded, "watch_status" was called before the file was attached to the version, and then never called again with "status" key. Then the "nomination" was never set.

Comment 17

[Yohan Boniface [:ybon]]

Regarding the initial issue of this ticket, it has been caused by many versions having a bad nomination value. 8000 versions had a nomination value equal to the push time (~ 5minutes). It's unexplained. Not visible in dev nor in stage, and not reproducible in local.
Among those version, 80 where waiting for review, so we have run a SQL for giving those a more useful value (the file.created value).

Regarding the rank in the queue that was changing: it's linked to the same migration problem. In effect, many of the affected versions were then sharing a same datetime (down to the second), so the ordering of those versions was random at each page load.

Regarding the bug in the code exposed on Comment 16, a fix is now pushed in production (https://github.com/mozilla/olympia/commit/bd900427cd4312cd58d87c0cdc43dd6c6089b88f).

Comment 18

[Yohan Boniface [:ybon]]

Ah, one more, regarding the bug exposed by Kris in Comment 7: this was because the migration of the data was not targeting the addon in pending update status. A SQL script has been ran on production for that. It should then be fixed now too.

Comment 20

[Luís Miguel [:quicksaver] (not reading bugmail)]

You probably don't need a second "verified", but yeah, my add-ons' queue positions are now correct (sequential) and don't change with each page load.

Thanks for the quick patch, great work!