Closed Bug 626544 Opened 9 years ago Closed 9 years ago

Preliminary reviews should have "Compare with last successful review" link.

Categories

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

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
5.12.9

People

(Reporter: kmag, Assigned: chenba)

Details

Attachments

(3 files)

User-Agent:       Mozilla/5.0 (X11; Linux x86_64; rv:2.0b10pre) Gecko/20110116 Firefox/4.0b10pre
Build Identifier: 

When performing a preliminary review, it's currently only possible to automatically compare the submitted XPI with the last "full" review. This is especially troublesome in the cases of those authors who have decided to forgo "full" review altogether, in which case we have to review a large subset of changes repeatedly for each new submission. Preliminary reviews should therefore instead allow us to compare with the most recent successful review, whether it was preliminary or full.

Reproducible: Always
Assignee: nobody → chenba
Target Milestone: --- → 5.12.7
File::getLatestFileByAddonId() now check for preliminarily reviewed files as well.  This method is also used in the install view element and the InstallButtonHelper...stuff that's been replaced by zamboni I believe.
Attachment #505022 - Flags: review?(jbalogh)
Comment on attachment 505022 [details] [diff] [review]
latest file by addon id now include preliminarily reviewed one

You're right, the other callers of this function are dead.
Attachment #505022 - Flags: review?(jbalogh) → review+
Committed @ r81230.
Status: UNCONFIRMED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Kris: thanks for the bug; would you mind double-checking its fix, on https://addons.allizom.org/en-US/firefox/?
Sorry, bug 628852 is preventing me from effectively testing this at the moment. I'll test it when I'm able.
On the production server, the first affected review that I've seen does generate a "Compare with public version link", but it generates a "not found" page that annoyingly redirects to the AMO main page after a couple of seconds.

https://addons.mozilla.org/en-US/editors/review/126641
https://addons.mozilla.org/en-US/firefox/files/diff/109071/

I unfortunately submitted the review before I thought to add this reply, but the effect remains the same nonetheless.
Reopening based on comment #6.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: FIXED → ---
Target Milestone: 5.12.7 → 5.12.9
Hmm the patch didn't change the format of the diff link.  Is that file actually missing?
No, the file is there.

However, the test won't work anymore because the latest version has been approved already. It still throws a file not found error, though.
Attachment #509047 - Flags: review?(jbalogh)
Attachment #509047 - Flags: review?(jbalogh) → review+
committed @ r82021
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
verified @ https://addons-next.allizom.org/en-US/editors/review/126565?num=1

Clicking on the "Compare to the last public version" loads https://addons-next.allizom.org/en-US/firefox/files/diff/108947/
Status: RESOLVED → VERIFIED
Attached image post-fix screenshot
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.