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

Status: VERIFIED FIXED

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

Description

[Kris Maglione [:kmag]]

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

Comment 1

[Barry Chen]

Attachment: latest file by addon id now include preliminarily reviewed one

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.

Comment 2

[Jeff Balogh (: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.

Comment 3

[Barry Chen]

Committed @ r81230.

Comment 4

[Stephen Donner [:stephend] Not actively reading bugmail]

Kris: thanks for the bug; would you mind double-checking its fix, on https://addons.allizom.org/en-US/firefox/?

Comment 5

[Kris Maglione [:kmag]]

Sorry, bug 628852 is preventing me from effectively testing this at the moment. I'll test it when I'm able.

Comment 6

[Kris Maglione [:kmag]]

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.

Comment 7

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

Reopening based on comment #6.

Comment 8

[Barry Chen]

Hmm the patch didn't change the format of the diff link.  Is that file actually missing?

Comment 9

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

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.

Comment 10

[Barry Chen]

Attachment: added lite status to diff in the files controller

Comment 11

[Barry Chen]

committed @ r82021

Comment 12

[krupa raj[:krupa]]

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/

Comment 13

[krupa raj[:krupa]]

Attachment: post-fix screenshot