Some of our queued posts were both approved and rejected

(Dan Fabulich) #1

Apropos my attempt to use the data explorer plugin to query post approval times, I discovered some queued_posts that had been both approved and rejected.

SELECT, q.state, q.approved_at, q.rejected_at,
  approvers.username, rejecters.username
FROM queued_posts q
JOIN users approvers
  ON = q.approved_by_id
JOIN users rejecters
  ON = q.rejected_by_id
WHERE q.approved_at is not null
  AND q.rejected_at is not null
  AND queue='default'

It turned up three posts whose approved_at and rejected_at values were within one second of each other, indicating to me that a race condition is possible (was possible? the last one was from May 2018) when two disagreeing moderators attempt to approve and reject the same post at approximately the same time.

(Dan Fabulich) #2

I don’t know if it’s worth redesigning a schema for this, but I would have wished for the queued_posts table to use resolved_at and resolved_by_id columns, rather than approved_at rejected_at approved_by_id rejected_by_id columns. That would have prevented contradictions like these from being represented in the DB.

(Sam Saffron) #3

Definitely a concern for the new reviewable queue @eviltrout

(Robin Ward) #5

The new schema doesn’t have rejected/approved/resolved. Instead you have a status and a separate table, reviewable_history which tracks all the status changes.

Note that it is totally valid to have an object go from approved -> rejected, or back and forth. We have to allow that reviewers can change their mind, or perhaps clicked on the wrong thing.

Now in terms of preventing two people from reviewing at the same time, that is more complicated, but probably worth doing. I’ll take a look at it.

Is it possible to get a report on post approvals?
(Robin Ward) #6

Okay, I can confirm in the latest review build there is code to prevent two users from updating the same reviewable item at the same time.

It’s implemented with a version parameter and column in the database, which relies on the database to atomically update the version. If the update fails, you receive an error message about an edit conflict.