Repository navigation
fix(builder): rebase draft fields after save to stop duplicate POSTs - #56
Merged
Merged
Conversation
handleSaveAll only invalidated the fields query after saving, and the load effect is guarded by initializedRef, so local state was never re-synced. A field created in this session kept isNew: true / persistedId: null, and a second click on Save POSTed it again, creating a duplicate field on the table. Capture each mutation response keyed by draft id and rebase local state from it (isNew: false, persistedId from the created field), so the second save takes the PUT branch. Saves are settled per field rather than through a bare Promise.all: an all-or-nothing rejection would have discarded the rebase for every field that did save, reintroducing the duplicate on the next click. Fields whose save failed keep their draft state. Adds a regression test asserting two consecutive saves issue exactly one POST per new field, followed by a PUT to the created id.
Morialkar
force-pushed
the
claude/cool-blackburn-6e01b5
branch
from
July 31, 2026 23:59
906b7a2 to
87d3651
Compare
Conflict in handleSaveAll: #55 added a savedSnapshot rebase for the unsaved-changes guard in the same .then() this branch rewrote. Resolved to keep both. This branch settles each save individually instead of using a bare Promise.all, so .then() now runs even when a field fails — main's `setSavedSnapshot(snapshotFields(fields))` would have marked failed fields clean and silently disarmed the guard. The snapshot is therefore merged for the fields that actually round-tripped, leaving failed ones dirty. Also adds useBlocker to this branch's router mock, and covers the combined behaviour: dirty clears after a successful save, and a field whose save failed stays new and unsaved (retried as a create, not a duplicate update).
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
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.
Problem
In
apps/client/src/routes/builder.$databaseId.$tableId.tsx,handleSaveAllsplits fields into new ones (POST /fields) and existing ones (PUT /fields/:id), but after a successful save it only calledqueryClient.invalidateQueries(['fields', tableId]). It never updated local state, and the load effect is guarded byinitializedRef.current, sofieldsis never re-synced from the server.Consequence: a field created in this session keeps
isNew: trueandpersistedId: nulllocally. Clicking Save a second time POSTs it again, creating a duplicate field on the table.Change
handleSaveAllnow captures each mutation response keyed by the draft field id and rebases local state from it —isNew: false,persistedIdfrom the created field's id — so a second Save takes the PUT branch.Two details beyond the minimum fix:
Promise.all. With all-or-nothing rejection, one failing field would discard the rebase for every field that did save, reintroducing the duplicate on the next click.persistedIdstays null → stillisNew), so nothing is flipped to a PUT against a null id.The
setFieldsupdate is functional and only touchesisNew/persistedId, so edits made while a save is in flight survive the rebase.Test
New
apps/client/src/routes/__tests__/builder.$databaseId.$tableId.test.tsx: adds a field to an empty table, clicks Save twice, and asserts exactly onePOST /fieldsplus onePUT /fields/field-created-1.Verified the test fails with the fix reverted (the second click issues a second POST and never a PUT), then passes with it restored.
Checks
tsc -bclean, Prettier and oxlint clean (the one oxlint warning is pre-existing innavigation.$databaseId.tsx)Reviewer note
initializedRefstill permanently blocks re-syncingfieldsfrom the server, so the post-saveinvalidateQueriesrefetch only serves other consumers of that query. That's fine now that state is rebased locally, but it does mean the builder never picks up field changes made elsewhere after mount — a separate concern, not addressed here.🤖 Generated with Claude Code