diff --git a/frontend/src/lib/itemStore.svelte.ts b/frontend/src/lib/itemStore.svelte.ts index fe784da4ef..a37244e081 100644 --- a/frontend/src/lib/itemStore.svelte.ts +++ b/frontend/src/lib/itemStore.svelte.ts @@ -193,8 +193,7 @@ class Entry { waitingOn: Entry[] = [] handles = new Set>() stopWatch: (() => void) | undefined - /** Fields a `patch` has put on the deployed side ahead of the server; a save landing - * meanwhile keeps them rather than rolling the baseline back past them. */ + /** Fields a `patch` has put on the deployed side ahead of the server. */ private patched: Record = {} /** The value as last observed, serialized: `touch` acts only on a real change. */ private seen: string | undefined @@ -256,6 +255,14 @@ class Entry { this.ports.write(key, desired === null ? null : snapshot(this.value)) } + /** A deployed side the server gave, with the fields a patch is still ahead of it on. Every + * write of that side goes through here: rolling the baseline back past an optimistic toggle + * makes the value differ from it, which is a draft row of a change nobody made. */ + private withPatched(deployed: V | undefined): V | undefined { + if (deployed === undefined) return undefined + return { ...deployed, ...this.patched } as V + } + private replaceValue(value: V | undefined): void { this.value = snapshot(value) this.seen = serialize(this.value) @@ -335,7 +342,7 @@ class Entry { if (!keepValue) this.replaceValue(res.template) return { ok: true } } - this.deployed = snapshot(res.deployed) + this.deployed = this.withPatched(snapshot(res.deployed)) this.origin = res.deployed !== undefined ? 'deployed' : 'draft' this.row = serialize(res.draft) ?? null this.ports.seedSync(key, res.draftSavedAt) @@ -428,7 +435,7 @@ class Entry { wrote = true this.error = undefined if (held !== sent) this.adopt(sent, held) - this.deployed = { ...held, ...this.patched } as V + this.deployed = this.withPatched(held) this.origin = 'deployed' this.template = undefined if (moved) this.moveTo(to) @@ -575,7 +582,10 @@ class Entry { } settled() this.error = undefined - if (this[baseKey] !== undefined) this[baseKey] = { ...this[baseKey], ...sent } as V + // `settled` has dropped from `patched` whatever this patch owned, so what is left is a + // newer patch of the same field, whose value stands over this one's. + if (this[baseKey] !== undefined) + this[baseKey] = this.withPatched({ ...this[baseKey], ...sent } as V) this.touch() this.reconcile() return { ok: true } diff --git a/frontend/src/lib/itemStore.test.ts b/frontend/src/lib/itemStore.test.ts index 29b00a14c4..447347b918 100644 --- a/frontend/src/lib/itemStore.test.ts +++ b/frontend/src/lib/itemStore.test.ts @@ -251,6 +251,64 @@ describe('item store: commands', () => { expect(rows.writes.at(-1)).toEqual({ path: 's', value: null }) }) + it('lets the later of two overlapping toggles stand, without a row in between', async () => { + type Sched = { path: string; enabled: boolean; summary: string } + const rows = fakeRows() + const first = deferred() + const store = createItemStore(rows.port) + const { handle: item } = store.acquire( + { workspace: 'w', kind: 'trigger_schedule', path: 's' }, + { workspace: 'w', path: 's' }, + { + load: async () => ({ deployed: { path: 's', enabled: false, summary: 'a' } }) + } as ItemAdapter + ) + await settle() + const enabling = item.patch({ enabled: true }, () => first.promise) + const disabling = item.patch({ enabled: false }, async () => {}) + first.resolve() + + expect(await enabling).toEqual({ ok: true }) + // The first request landing must not put its own `enabled` back over the second's. + expect(item.dirty).toBe(false) + expect(await disabling).toEqual({ ok: true }) + expect(item.deployed).toEqual({ path: 's', enabled: false, summary: 'a' }) + expect(item.dirty).toBe(false) + expect(rows.writes).toEqual([]) + }) + + it('keeps a toggle made during a read the server answered before it', async () => { + type Sched = { path: string; enabled: boolean; summary: string } + const rows = fakeRows() + const reading = deferred() + let reads = 0 + const store = createItemStore(rows.port) + const { handle: item } = store.acquire( + { workspace: 'w', kind: 'trigger_schedule', path: 's' }, + { workspace: 'w', path: 's' }, + { + load: async () => { + // The read in flight predates the toggle, so it still reports it disabled. + if (reads++ > 0) await reading.promise + return { deployed: { path: 's', enabled: false, summary: 'a' } } + } + } as ItemAdapter + ) + await settle() + const reloading = item.reload() + await settle() + const toggling = item.patch({ enabled: true }, async () => {}) + reading.resolve() + + expect(await reloading).toEqual({ ok: true }) + // The read must not put the schedule back to disabled under the toggle: that reads as an + // unsaved change to `enabled` and would be written as a draft. + expect(item.dirty).toBe(false) + expect(await toggling).toEqual({ ok: true }) + expect(item.deployed).toEqual({ path: 's', enabled: true, summary: 'a' }) + expect(rows.writes).toEqual([]) + }) + it('holds a toggle behind a save, and keeps an unrelated edit through the toggle', async () => { type Sched = { path: string; enabled: boolean; summary: string } const rows = fakeRows()