652028 - Don't allow reviewing a persona which has already been reviewed

Status: RESOLVED FIXED

(Websites Graveyard :: getpersonas.com, defect)

Description

[Matthew N. [:MattN]]

Similar to the fix for Bug 619088.

Check the state of the persona record in the DB on page load and on form submit to make sure that it hasn't already been reviewed.  The UPDATE query to the DB should also check the state with the WHERE clause so that it only updates unreviewed personas.

Multiple reviews of the same persona can be confusing since each review sends an email.  The check on page load will also save reviewers time from reviewing a persona that has already been reviewed.

Comment 1

[Deb Richardson [:dria] (plz NEEDINFO)]

This is a major issue for our reviewers right now so getting this fixed would be great.

Comment 2

[Matthew N. [:MattN]]

Attachment: v.1 check current status before reviewing pending

server/lib/review.php: 
* add constants for persona statuses to make code easier to read and grep.
* add pending_already_reviewed function.

server/lib/storage.php: 
* use new constants
* and WHERE condition to SQL for accept and reject to ensure persona is only changed from the expected status.
* Return the number of affected rows for accept/reject so that callers know whether they succeeded.

server/templates/user_error.php: 
* return proper HTTP status code to make quick_reject ajax handle the error message.

server/admin/pending.php:
* check status multiple times while reviewing
* make quick_reject AJAX handle already reviewed personas

Comment 4

[Brandon Savage [:brandon]]

The code looks decent, but I wasn't quite sure how to test this. Matt, can you give me some pointers on how you tested it?

Comment 5

[Matthew N. [:MattN]]

There are three checks to test:
1) Open up 2 tabs with the same pending review page for a specific persona.  Perform a review action in one tab and then perform another in the other tab.  The second request will display. The link should return you to the pending queue.

2) Open up a review page for a persona and review it.  Then from the pending queue, open up the same persona to review.  The error should be displayed instead of the review page.  The link should return you to the pending queue.

3) Check at DB insert time: 
Comment out admin/pending.php on line 90 so that the tests checking the return values from the review function will be hit:
90			pending_already_reviewed();
Then just repeat the steps for test #1 above.

Comment 6

[Brandon Savage [:brandon]]

Comment on attachment 532481 [details] [diff] [review]
v.1 check current status before reviewing pending

Review of attachment 532481 [details] [diff] [review]:
-----------------------------------------------------------------

Worked for me in dev, and seems to look generally good.

Comment 7

[Matthew N. [:MattN]]

Committed in r93018

Things to verify are in Comment 5 (#1 & #2)