642941 - Submitting to full review queue does not set file status correctly

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Admin/Editor Tools, defect, P3)

Description

[krupa raj[:krupa]]

Attachment: screenshot

steps to reproduce:
1. Load https://addons.allizom.org/z/en-US/editors/review/396865?num=4#more-about
2. Notice the file status under 'Files and Validation Results' section

expected behavior:
File status is 'Awaiting full review'

observed behavior:
File status is 'Awaiting preliminary review'

Comment 1

[Wil Clouser [:clouserw]]

The file status is '1' in the database which corresponds to preliminary review.  The add-on status is 3, which is awaiting full review.  Is this bug about the discrepancy between file and add-on review status?  If so, please add steps to reproduce.

Comment 2

[krupa raj[:krupa]]

STR:
1. Upload an add-on into the full review queue
2. As an editor, navigate to the review detail page for that add-on
3. Notice the file status under 'Files and Validation Results' section

expected behavior:
File status is 'Awaiting full review' which is same as the add-on status.

observed behavior:
File status is 'Awaiting preliminary review'

Comment 3

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

Note that the problem is not that the file status is incorrect, but that the text for the status is confusing. All files pending review have the "Awaiting preliminary review" status, but this can have a different meaning depending on the add-on's status.

Comment 4

[Andy McKay]

jorgev just wants the status label to say "Pending Full Review or Pending Preliminary Review depending on the add-on status".

Comment 5

[Andy McKay]

https://github.com/jbalogh/zamboni/commit/09ec1cdf899de80d0facc8264a1bd27bcf6d9430

Comment 6

[krupa raj[:krupa]]

File status is not being set for pending updates.

STR:
1. Load https://addons.allizom.org/z/en-US/editors/review/399285?num=1
2. Notice the file status under 'Files and Validation Result'

expected result:
File status for all pending updates is 'Pending Full review'

actual result:
File status for all pending updates is 'Awaiting Preliminary Review'

Also, small nitpick:
Add-ons in Full review queue have status set to "Pending Full review" and add-ons in lite queue have "Awaiting preliminary review".

can we make the copy same(use "awaiting" for "pending" in both file status)?

Comment 7

[krupa raj[:krupa]]

(In reply to comment #6)
> File status is not being set for pending updates.
> 
> STR:
> 1. Load https://addons.allizom.org/z/en-US/editors/review/399285?num=1
> 2. Notice the file status under 'Files and Validation Result'
> 
> expected result:
> File status for all pending updates is 'Pending Full review'
> 
> actual result:
> File status for all pending updates is 'Awaiting Preliminary Review'
> 
 Filed this as a separate bug. See bug 645171 for details

Comment 8

[krupa raj[:krupa]]

(In reply to comment #3)
> Note that the problem is not that the file status is incorrect, but that the
> text for the status is confusing. All files pending review have the "Awaiting
> preliminary review" status, but this can have a different meaning depending on
> the add-on's status.

As per Wil's comment 1, all new submissions have file status set to 1 which is incorrect.

File status should be 1 if it awaiting lite review and 3 if it is in full review queue.

Comment 9

[Andy McKay]

I'm inclined to agree with Krupa and Wil and would like to change that file status when submitting to the full review queue.

Comment 10

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

(In reply to comment #8) 
> As per Wil's comment 1, all new submissions have file status set to 1 which is
> incorrect.

This is how things are working in remora right now, and everything is fine except for the text presented to the developer. Having a different status is unnecessary.

> File status should be 1 if it awaiting lite review and 3 if it is in full
> review queue.

You're confusing the file status with the add-on status. When awaiting review, the file status can be 1 for all cases. The add-on status can be different, though, and that's what determines if it is up for full review or preliminary.

Comment 11

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

(In reply to comment #10)
> Having a different status is unnecessary.

To elaborate on this point, all we need to know is that the file is pending review. The text that needs to be presented to the developer (and where to locate it in the review queues) depends on the status of the add-on.

If it is "Public", "In Sandbox; Nominated" or "Preliminarily Reviewed; Nominated", then the text should say it is awaiting full review.

If it is "In Sandbox; Awaiting Prelim Review" or "Preliminarily Reviewed", then it should say it is awaiting preliminary review.

For "Admin Disabled" and "Incomplete" it should probably say something else.

Comment 12

[Andy McKay]

https://github.com/jbalogh/zamboni/commit/ef14266c9dc01861926d1f866b537288dc4d37a3

Comment 13

[krupa raj[:krupa]]

verified @ https://addons.allizom.org/z/en-US/editors/review/398914?num=7

Comment 14

[krupa raj[:krupa]]

Attachment: post-fix screenshot