refactor: implement optimistic locking using version columns (#2408) - #2465
Merged
krushit1307 merged 2 commits intoAug 5, 2026
Merged
Conversation
…rushit1307#2408) Enforce optimistic concurrency control on event writes so simultaneous edits cannot silently overwrite each other (lost update anomaly). - EditEventDialog: UPDATE is now guarded with WHERE id AND version, and increments version atomically. When 0 rows are affected, the save is rejected and a merge-conflict modal shows the fresh database state. - Wire the EditEventDialog into the event detail page for organizers and select version/version_vector/category_id/tags in the event query. - adminEventRescheduleApi: reschedules bump the version and use the same OCC predicate, throwing a conflict error on a stale version. - Add pgTAP concurrency test and a component test covering the conflict path.
github-actions
Bot
added
backend
bug
📝 WalkthroughWalkthroughEvent editing and rescheduling now use optimistic concurrency control. Saves require the observed event version. Stale edits trigger three-way conflict recovery. The event route renders organizer editing controls, and database tests verify guarded version updates. ChangesEvent concurrency control
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Sequence Diagram(s)sequenceDiagram
participant Organizer
participant EditEventDialog
participant Supabase
participant ConflictResolutionModal
Organizer->>EditEventDialog: submit event edits
EditEventDialog->>Supabase: update where id and version match
Supabase-->>EditEventDialog: updated row or zero rows
EditEventDialog->>Supabase: fetch latest event after conflict
Supabase-->>EditEventDialog: latest server event
EditEventDialog->>ConflictResolutionModal: display merged server and local edits
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/EditEventDialog.test.tsx`:
- Around line 128-137: In the successful save test, add an assertion on
mockUpdate before the dialog-closed check to verify it was called with the
expected update payload, including version: 2. Keep the existing mocked response
and close assertion unchanged.
In `@src/services/adminEventRescheduleApi.ts`:
- Around line 30-42: Update the primary REST request in the rescheduling flow to
include targetVersion in its request body, then update the corresponding REST
handler to require that value for its guarded update before returning success.
Preserve the existing direct Supabase fallback behavior and ensure the handler
uses the supplied version for optimistic concurrency control.
In `@supabase/tests/event_version_concurrency.test.sql`:
- Around line 41-50: Update the concurrency test around the guarded UPDATE for
event ID 90000000-0000-0000-0000-000000000004 to first change version 1 to 2 and
assert that it succeeds, then issue a second guarded update expecting version 1
and assert it affects zero rows. Verify the stored description remains the first
writer’s value.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a21bf977-682a-4eab-aa76-78900967c897
📒 Files selected for processing (5)
src/components/EditEventDialog.test.tsxsrc/components/EditEventDialog.tsxsrc/routes/events.$eventId.tsxsrc/services/adminEventRescheduleApi.tssupabase/tests/event_version_concurrency.test.sql
Comment on lines
+30
to
+42
| // Read the current version so the write can be guarded with OCC | ||
| const { data: current, error: fetchError } = await supabase | ||
| .from("events") | ||
| .select("version") | ||
| .eq("id", eventId) | ||
| .maybeSingle(); | ||
|
|
||
| if (fetchError) { | ||
| throw new Error(`Failed to load event version: ${fetchError.message}`); | ||
| } | ||
|
|
||
| const targetVersion = current?.version ?? 1; | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Send the fetched version to the REST endpoint.
targetVersion is only used by the direct Supabase fallback. The primary REST request does not send an expected version. The REST handler cannot apply the client-fetched version on this path.
Send targetVersion in the request body and require the endpoint to use it in its guarded update before it returns success.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/adminEventRescheduleApi.ts` around lines 30 - 42, Update the
primary REST request in the rescheduling flow to include targetVersion in its
request body, then update the corresponding REST handler to require that value
for its guarded update before returning success. Preserve the existing direct
Supabase fallback behavior and ensure the handler uses the supplied version for
optimistic concurrency control.
…2408) - EditEventDialog.test.tsx: assert the successful save writes version 2 in the update payload before the dialog closes. - adminEventRescheduleApi: send the fetched targetVersion in the REST reschedule request body so the endpoint can enforce OCC. - Add the reschedule REST handler that requires the supplied version for its guarded UPDATE and returns 409 on a stale version. - event_version_concurrency.test.sql: simulate two writers - a guarded update from version 1 to 2 succeeds, then a stale version 1 update affects 0 rows and preserves the first writer's data.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/routes/api/events/`$id/reschedule.ts:
- Around line 46-50: The guarded-update conflict branch in the reschedule route
should return a structured 409 payload containing the current event fields and
version, rather than text alone. Update the client flow in the reschedule API
service to detect 409 responses, consume that payload for conflict recovery, and
avoid falling through to the direct-update fallback.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 586435ef-aa88-4911-9fc7-67a9b756f944
📒 Files selected for processing (4)
src/components/EditEventDialog.test.tsxsrc/routes/api/events/$id/reschedule.tssrc/services/adminEventRescheduleApi.tssupabase/tests/event_version_concurrency.test.sql
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/EditEventDialog.test.tsx
- src/services/adminEventRescheduleApi.ts
Comment on lines
+46
to
+50
| if (!data || data.length === 0) { | ||
| return new Response( | ||
| "Conflict: This event was modified by another user. Please refresh and try again.", | ||
| { status: 409 }, | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Return the current event state with the conflict response.
The 409 response contains only text. It does not provide the current database state required for conflict recovery. src/services/adminEventRescheduleApi.ts:23-108 also does not handle 409 before it attempts the fallback update.
After the guarded update affects zero rows, return a structured conflict payload with the current event fields and version. Update the client to handle 409 and use that payload instead of falling through to the direct-update fallback.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/api/events/`$id/reschedule.ts around lines 46 - 50, The
guarded-update conflict branch in the reschedule route should return a
structured 409 payload containing the current event fields and version, rather
than text alone. Update the client flow in the reschedule API service to detect
409 responses, consume that payload for conflict recovery, and avoid falling
through to the direct-update fallback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Implements optimistic database locking using
versioncolumns on theeventstable. Simultaneous edits to a single event no longer blindly overwrite each other: the UPDATE payload now requires the version the user fetched, the query usesWHERE id = $1 AND version = $2, and when 0 rows are affected the save is rejected with a conflict. The merge-conflict modal then shows the fresh database state so the user never loses their work (UI recovery). The same OCC guard is applied to event reschedules viaadminEventRescheduleApi, and the Edit Event dialog is now wired into the event detail page for organizers.Type of Change
Related Issue
Closes #2408
Testing
Describe the testing performed.
Ran
tsc --noEmit— 0 errors in all changed filessrc/components/EditEventDialog.test.tsx(2 tests): verifies a stale version (0 rows updated) triggers the merge-conflict modal with the new server state, and that a matching version saves successfullysupabase/tests/event_version_concurrency.test.sql: pgTAP test asserting a stale-version UPDATE affects 0 rowsExisting
conflictResolutiontests passTested locally
Existing functionality verified
No new warnings or errors
Screenshots
N/A
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Tests