Skip to content

Deleting a draft-only saved view no-ops — deleteView has no draft qualifier, and the mirror of #4139 is NOT mechanical (it would break published+draft pairs) #4479

Description

@yinlianghui

Split out of #4463 per the ruling on that card (#4463 comment 5270772373): the secondary defect was to be fixed in the same PR only if teaching deleteView the draft qualifier is the mechanical mirror of what updateView gained in #4139. It is measured below as not mechanical, so it becomes this card. The primary fix (the unbound updateView calls) shipped separately.

Symptom (from #4463's QA run)

Deleting a view that exists only as a draft issues DELETE /api/v1/meta/view/ against the published overlay and returns

200 {"success":true,"reset":false,"message":"No view '' found — nothing to delete."}

The draft survives and the tab is still present after reload. Delete on a published view works correctly (reset:true, tab gone).

Where it is

packages/data-objectstack/src/index.tsdeleteView is unqualified:

async deleteView(objectName: string, viewName: string): Promise< { deleted: boolean } > {
  await this.connect();
  const result: any = await this.client.meta.deleteItem('view', viewName);
  this.invalidateViewKeys(objectName, viewName);
  return { deleted: !!(result?.deleted ?? result?.reset ?? true) };
}

updateView, by contrast, probes the draft first (metaClient.get('view', name, { state: 'draft' })) and writes back to whichever home the read resolved.

Why the mirror is NOT mechanical — measured

The draft-discard primitive exists and is reachable, so a naive mirror is easy to write:

  • MetadataClient.reset(type, name, { state: 'draft' }) issues DELETE /meta/:type/:name?state=draft (packages/data-objectstack/src/metadata-client.ts).
  • MetadataDeleteOptions.state is documented there as: "'draft' discards the pending draft (keeps the published overlay intact)."
  • Server side confirms it: deleteMetaItem in framework packages/metadata-protocol/src/protocol.ts probes repo.get(ref, { state: targetState }) and targets only that state.

The problem is what draft-first means for delete, in the case where a view has both homes:

case today naive draft-first mirror
draft-only view bug — hits published overlay, reset:false, draft survives correct — draft discarded, view gone
published-only view correct — reset:true, tab gone correct — probe misses, falls through to published
published + pending draft correct enough — published overlay deleted, tab gone new regression — only the draft is discarded, the published view survives and the tab is still there after reload

The third row is not a corner case. persistRuntimeMetadata (packages/app-shell/src/views/runtime-metadata-persistence.ts) stages every runtime edit as a draft — metadataClient.save(type, name, body, { mode: 'draft' }) — so "publish a view, then edit it" routinely produces a published+draft pair. A draft-first delete on that pair silently turns Delete view into Discard draft.

And Discard draft is a distinct operation that already exists, in the same file:

export async function discardRuntimeDraft(type, name, ctx): Promise< void > {
  await ctx.metadataClient.reset(type, name, { state: 'draft' });
  invalidateViewCaches(type, name, ctx);
}

documented as "Discard the pending draft of a runtime artifact (Studio's 'Discard draft'). The published overlay is untouched." So the mechanical mirror would make deleteView collide with an existing, deliberately different semantic.

The asymmetry has a clean statement: for update, draft-first is right in all three cases, because a draft shadows the published row and Publish would otherwise overwrite the edit with the pre-edit body (the reasoning written into updateView's docblock as point 2). For delete, "remove this view" is only satisfied when no home is left serving it — which is one call in two of the three cases and two calls in the third.

What has to be decided (why this needs its own card)

  1. Pair semantics. Should deleteView on a published+draft pair remove both homes? That is the only reading under which the tab reliably disappears, but it is two calls, not one.
  2. Ordering and partial failure. If both are deleted, in what order, and what does { deleted: boolean } report when one succeeds and the other fails? The current return type cannot express "draft gone, overlay left".
  3. Probe or blind. reset(..., { state: 'draft' }) throws on any non-OK response, so a blind two-call implementation needs the no-draft answer pinned first (the server's no-row branch returns a 200 reset:false rather than a 404, but that is worth a test rather than an assumption).
  4. Transport. deleteView currently goes through this.client.meta.deleteItem, while updateView's draft half uses this.metadataClient(). The mirror crosses transports, so the two halves need to agree on error shapes.

Suggested shape (not a ruling)

Delete both homes, draft first, and widen the receipt so a partial outcome is observable rather than rounded to true. Red-first coverage for all three rows of the table above, including the pair case that today's code accidentally gets right.

Provenance

Measured while implementing #4463 (branch claude/issue-4463-setdefault-unbound). No code for this half was written; the primary card's PR is scoped to the unbound-call fix.


Generated by Claude Code

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions