mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: keep the edits of an editor a move replaces at its key
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015vn1jCHUcAdaG9C4NsUehF
This commit is contained in:
co-authored by
Claude Opus 5
parent
901e096380
commit
1648336e3e
@@ -417,6 +417,24 @@ class Entry<V> {
|
||||
return result
|
||||
}
|
||||
|
||||
/** Take over the edits of a deployed entry this one replaced at its key: each field its value
|
||||
* changed from its own deployed side, so they stay a draft over what this entry deployed. */
|
||||
carryEditsOf(old: Entry<V>): void {
|
||||
if (old.origin !== 'deployed' || !old.dirty || this.value === undefined) return
|
||||
const edited = old.value as Record<string, unknown>
|
||||
const base = old.deployed as Record<string, unknown>
|
||||
const next = snapshot(this.value) as Record<string, unknown>
|
||||
let changed = false
|
||||
for (const k of new Set([...Object.keys(edited), ...Object.keys(base)])) {
|
||||
if (deepEqual(edited[k], base[k])) continue
|
||||
next[k] = snapshot(edited[k])
|
||||
changed = true
|
||||
}
|
||||
if (!changed) return
|
||||
this.pristine = false
|
||||
this.replaceValue(next as V)
|
||||
}
|
||||
|
||||
/** Give the value each field the server set otherwise than `sent` asked, unless the user has
|
||||
* changed that field since: left as sent, it would read as a draft of a change nobody made. */
|
||||
private adopt(sent: V, held: V): void {
|
||||
@@ -811,10 +829,11 @@ export function createItemStore(ports: ItemRowPort) {
|
||||
|
||||
/**
|
||||
* An entry moved onto a key another live entry holds. One key has one row writer, so the
|
||||
* old one stops writing and every handle on it moves to the entry that now is the item —
|
||||
* the same outcome as when nobody had it open: what was there is superseded by the write.
|
||||
* old one stops writing and every handle on it moves to the entry that now is the item. Its
|
||||
* deployed side is superseded by the write; the edits on screen over it are not.
|
||||
*/
|
||||
function retire(old: Entry<any>, into: Entry<any>): void {
|
||||
into.carryEditsOf(old)
|
||||
old.retired = true
|
||||
old.stopWatch?.()
|
||||
for (const handle of old.handles) {
|
||||
@@ -996,6 +1015,8 @@ export function useItems<V>(
|
||||
): (ItemHandle<V> | undefined)[] {
|
||||
const handles = $state<(ItemHandle<V> | undefined)[]>([])
|
||||
const held = new Map<string, ItemAcquisition<V>>()
|
||||
const sessionOf = new Map<ItemAcquisition<V>, string>()
|
||||
const idOf = (key: ItemKey, session: unknown) => `${keyString(key)}#${String(session ?? '')}`
|
||||
|
||||
function reconcile() {
|
||||
const specs = getSpecs()
|
||||
@@ -1006,14 +1027,26 @@ export function useItems<V>(
|
||||
continue
|
||||
}
|
||||
const key: ItemKey = { workspace: spec.workspace, kind, path: spec.path }
|
||||
wanted.push({ id: `${keyString(key)}#${String(spec.session ?? '')}`, key, spec })
|
||||
wanted.push({ id: idOf(key, spec.session), key, spec })
|
||||
}
|
||||
const ids = new Set(wanted.map((w) => w.id))
|
||||
// A spec that caught up with where a save moved its item keeps that item, rather than
|
||||
// releasing it and reading it afresh over edits whose row may not have landed yet.
|
||||
for (const { id } of wanted) {
|
||||
if (!id || held.has(id)) continue
|
||||
for (const [heldId, acq] of held) {
|
||||
if (ids.has(heldId) || idOf(acq.handle.key, sessionOf.get(acq)) !== id) continue
|
||||
held.delete(heldId)
|
||||
held.set(id, acq)
|
||||
break
|
||||
}
|
||||
}
|
||||
// Release first, so an item reopened under a new session is read afresh when idle.
|
||||
for (const [id, acq] of [...held]) {
|
||||
if (!ids.has(id)) {
|
||||
acq.release()
|
||||
held.delete(id)
|
||||
sessionOf.delete(acq)
|
||||
}
|
||||
}
|
||||
const next = wanted.map(({ id, key, spec }) => {
|
||||
@@ -1022,6 +1055,7 @@ export function useItems<V>(
|
||||
if (!acq) {
|
||||
acq = itemStore.acquire(key, spec, adapter)
|
||||
held.set(id, acq)
|
||||
sessionOf.set(acq, String(spec.session ?? ''))
|
||||
}
|
||||
return acq.handle
|
||||
})
|
||||
@@ -1038,6 +1072,7 @@ export function useItems<V>(
|
||||
onDestroy(() => {
|
||||
for (const acq of held.values()) acq.release()
|
||||
held.clear()
|
||||
sessionOf.clear()
|
||||
})
|
||||
return handles
|
||||
}
|
||||
|
||||
@@ -500,6 +500,36 @@ describe('item store: one entry per key', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('keeps what was typed in an editor a move replaces, as a draft over what it wrote', async () => {
|
||||
const rows = fakeRows()
|
||||
const store = createItemStore(rows.port)
|
||||
const gate = deferred()
|
||||
const b = { ...deployedRes, path: 'u/me/b' }
|
||||
const { handle: open } = store.acquire(
|
||||
{ workspace: 'w', kind: 'resource', path: 'u/me/b' },
|
||||
{ workspace: 'w', path: 'u/me/b' },
|
||||
adapter({ deployed: b })
|
||||
)
|
||||
const temporary = newItemPath()
|
||||
const { handle: moving } = store.acquire(
|
||||
{ workspace: 'w', kind: 'resource', path: temporary },
|
||||
{ workspace: 'w', path: temporary, template: b },
|
||||
adapter({}, () => gate.promise)
|
||||
)
|
||||
await settle()
|
||||
moving.value = { ...b, args: { a: 2 } }
|
||||
const moved = moving.save()
|
||||
await settle()
|
||||
open.value = { ...b, description: 'typed while it moved' }
|
||||
|
||||
gate.resolve()
|
||||
expect(await moved).toMatchObject({ ok: true, moved: true })
|
||||
const kept = { ...b, args: { a: 2 }, description: 'typed while it moved' }
|
||||
expect(open.deployed).toEqual({ ...b, args: { a: 2 } })
|
||||
expect(open.value).toEqual(kept)
|
||||
expect(rows.writes.at(-1)).toEqual({ path: 'u/me/b', value: kept })
|
||||
})
|
||||
|
||||
it('writes an item it moves onto after the saves queued there, and supersedes later ones', async () => {
|
||||
const rows = fakeRows()
|
||||
const store = createItemStore(rows.port)
|
||||
@@ -617,7 +647,9 @@ describe('item store: one entry per key', () => {
|
||||
expect(await secondMove).toMatchObject({ ok: true, moved: true })
|
||||
expect(await later).toMatchObject({ ok: false })
|
||||
expect(order).toEqual(['first', 'second'])
|
||||
expect(first.value?.description).toBe('second')
|
||||
expect(first.deployed?.description).toBe('second')
|
||||
// Not written, but not lost either: a draft over what the later move wrote.
|
||||
expect(first.value?.description).toBe('saved again through the first')
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
Reference in New Issue
Block a user