fix(data): updateView reads and writes the same row, so renaming a draft view persists - #4212
Merged
Merged
Conversation
…aft view persists
ADR-0034 stages a runtime-created view as a per-item draft, so a view made
from the `+` tab exists only as a draft row until an explicit Publish.
`updateView` addressed neither half of that: the read went to the published
overlay and 404ed, a `catch {}` labelled "treat missing as create-equivalent"
substituted `current = {}`, and the read-merge-write cycle emitted a
`{label, name, object}` partial the server rejects (422) — while the draft
row the UI reads back through `?preview=draft` kept the old label.
The read now probes the draft first and, on a hit, merges onto that body and
writes it back with `mode: 'draft'` — whichever row the read resolved is the
row the write updates. Draft-before-published also keeps an edit from landing
where a pending draft would shadow it and Publish would later overwrite it.
The silent catch is gone: a view found in neither home throws naming the view
and object, and a transport failure on either read propagates rather than
degrading into the partial write.
Fixes #4139
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
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.
Fixes #4139
Premise
Verified on my tip, and it holds exactly as filed. The pre-fix write emitted for a rename of a draft-only view is, verbatim:
That is the
{label, name, object}partial-write signature the issue predicted — noviewKind, noconfig, which is what the server rejects with 422.Root cause, and the half the issue did not name
ADR-0034 stages every runtime-created view as a per-item draft: the
+tab goes throughcreateRuntimeMetadata→metadataClient.save(..., { mode: 'draft' }), so the view lives only in the draft row until an explicit Publish, and the UI reads it back through?preview=draft.updateViewaddressed neither half of that. Measuring the write half as the card asked — doessaveItemwrite the draft? — gives anothat changes the fix:client.meta.saveItemhas no draft mode at all (the SDK'smeta.getItem/saveItemtake onlypackageId), so the write half was published-addressed too. The two halves therefore already agreed — both pointed at the published overlay — and the mismatch was betweenupdateViewas a whole and the draft row the create path produces.That matters because fixing only the read would not fix the reported symptom: the merge would be correct, but the write would still land on the published overlay while the draft row keeps shadowing it in preview, so the rename stays invisible and Publish later overwrites it with the pre-edit body. Making the halves agree on the right row requires moving both.
The fix
packages/data-objectstack/src/index.ts— the read probes the draft first and, on a hit, merges onto that body and writes it straight back withmode: 'draft'via the existingMetadataClientseam (the same onelistViews({ previewDrafts })already uses). Whichever row the read resolved is the row the write updates — agreement by construction, not coincidence.Draft before published is load-bearing for a view that has both: writing the published row while a draft is pending puts the edit where the draft shadows it, and Publish then overwrites it with the pre-edit body — losing the change a second time, later, where nothing connects it to this call. A draft edit stays a draft, so ADR-0037's guarantee that nothing the preview shows goes live until Publish is preserved. Renaming a published view with no draft pending is untouched: published read, published write.
The silent catch is gone (issue requirement 3). Call sites measured first — all five are in
ObjectView.tsx(rename, pin, set-default ×2, reorder), every one an update to an already-listed view, and none creates a view;createViewis the operation that means "create". So no caller depended on "create-equivalent", and it is not preserved. A view resolving in neither home now throws naming the view and object; a network/permission/server fault on either read propagates. All five call sites alreadytry/catchand toast, so this surfaces a previously-invisible failure rather than introducing a new one.Set-default (issue requirement 4) — measured, and it splits
Two symptoms, two causes:
{isDefault: true}patch — so it is fixed here and carries its own pin.updateView, which writes unconditionally for every patch shape (no early return between read and write; measured against the pre-fix code, an{isDefault: true}patch produced a write every time). It can only come fromObjectView.tsx'sisSavedViewguard — the sole early return on that path — or fromViewTabBarhiding the menu item underonSetDefaultView && !isReadonlywhen nosavedViewsrow matches the tab id.That is UI-layer, above this adapter and on #4155's surface, so per the dispatch scope it is not fixed here — filed with the full measurement as #4211.
Tests
New pins in
packages/data-objectstack/src/updateView.draft.test.ts(7): draft rename carries the full merged document to amode=draftwrite; draft probed before published; set-default patch likewise; control — published view with no draft still updates the published overlay; genuinely-missing view throws with no write; transport failure on the published read throws with no write; transport failure on the draft probe throws rather than falling through to published.Downstream consumers of
updateView(ObjectView.defaultViewIdentity,clientValidation.viewShapes) re-run green — 20 passed — since the error semantics changed. Suites were scoped topackages/data-objectstack/plus those two files per the repo's root-only rule (objectui#3378); the wide sweep is left to CI shards.Reverse verification
Direction predicted before running: removing the fix should turn the pins RED with the partial-write signature, and leave the control green, since the control exercises the published path the fix deliberately does not change.
git checkout origin/main -- packages/data-objectstack/src/index.ts→ 6 of 7 red, control green, first failure showing{label, name, object}verbatim. Restored withgit checkout HEAD -- ...; full suite green again. Confirmed as predicted, including the control staying green.Changeset
@object-ui/data-objectstackpatch.Generated by Claude Code