685514 - Link to review in notification mail allows to review passed revision

Status: RESOLVED FIXED

(support.mozilla.org :: Knowledge Base Software, task)

Description

[Kadir Topal [:atopal]]

A notification mail about a new revision ready to be reviewed had this link on top: https://support.mozilla.com/en-US/kb/how-do-i-manage-website-permissions/review/16250

As you can see the new revision (16250) is older than the current revision. It's not immediately clear what's happening and rather confusing as the email includes a diff with a completely different revision. 

We should invalidate those links and display a message as in the cases where a revision is already approved. In this case the message should say that this revision was disregarded.

Comment 1

[Ricky Rosario [:rrosario, :r1cky]]

In the history view, we don't link to the review page for unreviewed revisions that are older than the current. We should probably redirect to the plain revision view [1] in this case and show a user message at the top explaining that there are newer revisions approved already.

[1] https://support.mozilla.com/en-US/kb/how-do-i-manage-website-permissions/revision/16250

Comment 2

[Ricky Rosario [:rrosario, :r1cky]]

Cleaning up 2011Q4

Comment 3

[Tim [:pollti]]

Attachment: PR by safwan and me

This PR has been created by safwan and me.

Comment 4

[Michael Cooper [:mythmon]]

This was deployed.

Comment 5

[Joni Chan]

This works great, thanks! I just have a wording request.

In instances when a new revision is available while a reviewer is on an old revision page, can we change the wording to:

"This revision is outdated, but there is a new revision available. <link>Please review the latest revision.<link>"

Comment 6

[Tim [:pollti]]

Currently, we show "A newer revision has already been reviewed." When the newest revision has not been approved yet, we add "<link>But there is another latest revision which is waiting for review.</link>".
In my opinion "This revision is outdated, but there is a new revision available." already sound like you could review a newer version. - When the newest revision has been reviewed already, SUMO also shows this first sentence. Of course, you have a much better english ;-) so you it's up to you to decide about the best wording...

Comment 7

[Joni Chan]

Tim, good point. 

Scenario 1: If there isn't already a pending revision, we can show "A newer revision has already been reviewed."

Scenario 2: If a newer revision has not yet been approved, I'd show the wording in comment #5. My issue with showing the same message as in scenario #1 is that the reviewer won't really care that another newer revision has already been reviewed; only that there's an even newer revision waiting. They'll want to disregard the already reviewed revisions and go right to the latest revision.

Comment 8

[Tim [:pollti]]

Thanks, Joni. I'll create a patch on saturday.

Comment 9

[Tim [:pollti]]

Attachment: PR (comment 5)

Comment 10

[Michael Cooper [:mythmon]]

I deployed this to prod.