Let me say, broadly speaking, the goals of the "in review" system are to reduce frustration. While our review queue system essentially gets locks for free because we only hand a reviewer a single post at a time, it gets much more complicated when we allow reviewers in SG the flexibility to pick whatever post they want, but still simultaneously try to prevent multiple people from wasting their time by all reviewing the same exact post all at once.
It's not like we have a standup where all the SG reviewers can pick and choose who gets what ticket, and I know it can also be equally frustrating on the other end to spend a lot of time reviewing a ticket, making careful edits, only to have it scooped out from under you by someone who clicks "approve" right away. So there's some balance to maintain.
Let me at least detail the current behavior in this answer to at least inform more about how the system works and see if there are other opportunities on the UX side.
Current post locking logic
- When a reviewer opens a post, at that moment in time we try to get a lock on the post.
- If the post doesn't currently have a lock, the reviewer obtains one and can proceed with review
- If the post is already under review, we still allow it to be loaded, but add the
🔒 In Review
label
- When the post is submitted, we again confirm if you have a lock.
- If you still have the one you claimed when loading the post, you're good to go.
- If you don't have a lock, but no-one else does either, you're good to go.
- If someone else has a lock, you'll get the error:
A review for this post is currently being processed. Please try again
- Reviewers try to claim and need a lock to perform a review or an edit
- Authors will add a lock if they're editing their post, but are always allowed to submit edits, even if someone else is reviewing
- A lock is actively dismissed when
- The reviewer completes their review action (including skip)
- The reviewer clicks
Return to Questions
- The author submits an edit or navigates off the edit page
- The reviewer or author closes the page or navigates away (relies on
onbeforeunload
)
- Locks are stored using redis and will automatically expire after 5 minutes of inactivity
- On the client we listens for keyup events in the title, editor, tag, or comments field and will submit
refresh-or-try-claim-lock
. This function is debounced every 30 seconds
- A list of all active locks is used to exclude posts under active review from
- the
/staging-ground
listing page
- the feeds on home & questions page
- loading the next reviewable post after a review
- If you have the page open and someone else makes an edit to the page, we should alert you to the change (via sockets), and ask you to refresh the page.
- Changes include edits to title, body, and tags and also changes to post state (i.e.
New
, Major Changes
, etc)
I'll leave the rest of the question to our normal feature-request lifecycle, but wanted to at least document some extra details, in particular hopes that knowing the specifics might avoid some frustrating elements when you don't know what's going on or why.