Closed Bug 685514 Opened 13 years ago Closed 9 years ago

Link to review in notification mail allows to review passed revision

Categories

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

task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: atopal, Assigned: pollti)

Details

Attachments

(2 files)

44 bytes, text/x-github-pull-request
Details | Review
44 bytes, text/x-github-pull-request
Details | Review
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.
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
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: 2011Q3 → 2011Q4
Cleaning up 2011Q4
Target Milestone: 2011Q4 → ---
Attached file PR by safwan and me
This PR has been created by safwan and me.
Assignee: nobody → pollti
Status: NEW → ASSIGNED
This was deployed.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
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>"
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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...
Flags: needinfo?(jsavage)
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.
Flags: needinfo?(jsavage)
Thanks, Joni. I'll create a patch on saturday.
Status: REOPENED → ASSIGNED
Attached file PR (comment 5)
I deployed this to prod.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.