fix(17): WR-02 keep one reorder save in flight and ignore stale responses
A move made while a save runs is queued and sent after it settles, so out-of-order completion can no longer leave the database on an older tree than the screen. A failure that a waiting save supersedes keeps the later moves, and a response tied to an earlier load or partition leaves the current tree and its last saved state alone.
This commit is contained in:
@@ -30,9 +30,13 @@ import { showToast } from '../state/useToasts'
|
||||
// or inside a row, never into the node's own subtree), by the arrow keys on
|
||||
// the handle and by the four buttons. There is no Save button: every move is
|
||||
// saved by one POST with the whole scoped node set, 400 ms after the last
|
||||
// move, and a pending save is flushed when the page is left. A failed save
|
||||
// restores the last saved tree; a 409 (the records changed meanwhile)
|
||||
// reloads it.
|
||||
// move, and a pending save is flushed when the page is left. Only one save is
|
||||
// in flight at a time: a save asked for meanwhile waits for it and then sends
|
||||
// the tree as it was when asked, so the server never ends on an older tree
|
||||
// than the screen. A failed save restores the last saved tree unless a later
|
||||
// save (which carries the whole tree) is waiting; a 409 (the records changed
|
||||
// meanwhile) reloads it. A response that arrives after the page reloaded or
|
||||
// switched partition leaves the screen alone.
|
||||
const SAVE_DEBOUNCE_MS = 400
|
||||
const SAVED_SHOWN_MS = 2000
|
||||
|
||||
@@ -59,7 +63,17 @@ const saving = ref(0)
|
||||
const savedShown = ref(false)
|
||||
const announcement = ref('')
|
||||
|
||||
// One save: the tree it sends, its partition and the load generation it
|
||||
// was taken from.
|
||||
interface SaveJob {
|
||||
sent: TreeNode[]
|
||||
partition: AdminReorderPartition['value'] | null
|
||||
generation: number
|
||||
}
|
||||
|
||||
let saveTimer: ReturnType<typeof setTimeout> | null = null
|
||||
let inFlight = false
|
||||
let queued: SaveJob | null = null
|
||||
let savedTimer: ReturnType<typeof setTimeout> | null = null
|
||||
let announceTimer: ReturnType<typeof setTimeout> | null = null
|
||||
let generation = 0
|
||||
@@ -106,31 +120,72 @@ function showSaved(): void {
|
||||
}, SAVED_SHOWN_MS)
|
||||
}
|
||||
|
||||
async function save(): Promise<void> {
|
||||
/** Saves the tree as it is now, or queues it behind the save in flight. */
|
||||
function save(): void {
|
||||
saveTimer = null
|
||||
const sent = cloneTree(tree.value)
|
||||
const body = { nodes: flatten(sent), partition: partition.value?.value ?? null } as unknown as AdminReorderRequest
|
||||
const job: SaveJob = { sent: cloneTree(tree.value), partition: partition.value?.value ?? null, generation }
|
||||
if (inFlight) {
|
||||
queued = job
|
||||
return
|
||||
}
|
||||
void send(job)
|
||||
}
|
||||
|
||||
/** Whether a later move of the same page is waiting to be saved. */
|
||||
function superseded(job: SaveJob): boolean {
|
||||
return saveTimer !== null || (queued !== null && queued.generation === job.generation)
|
||||
}
|
||||
|
||||
function failed(job: SaveJob): void {
|
||||
if (job.generation === generation) {
|
||||
if (superseded(job)) {
|
||||
// The waiting save carries the whole tree, later moves included.
|
||||
return
|
||||
}
|
||||
tree.value = cloneTree(savedTree)
|
||||
}
|
||||
showToast(t('backend::lang.reorder.failed'), 'danger')
|
||||
}
|
||||
|
||||
async function send(job: SaveJob): Promise<void> {
|
||||
inFlight = true
|
||||
const body = { nodes: flatten(job.sent), partition: job.partition } as unknown as AdminReorderRequest
|
||||
savedShown.value = false
|
||||
saving.value++
|
||||
try {
|
||||
const { data, response } = await api.POST('/{vendor}/{plugin}/{controller}/reorder', { params: { path }, body })
|
||||
const current = job.generation === generation
|
||||
if (data) {
|
||||
savedTree = sent
|
||||
showSaved()
|
||||
if (current) {
|
||||
savedTree = job.sent
|
||||
if (!superseded(job)) {
|
||||
showSaved()
|
||||
}
|
||||
}
|
||||
return
|
||||
}
|
||||
if (response.status === 409) {
|
||||
showToast(t('backend::lang.reorder.stale'), 'danger')
|
||||
await load()
|
||||
if (current) {
|
||||
// A waiting save of the same set would be refused too.
|
||||
if (queued !== null && queued.generation === job.generation) {
|
||||
queued = null
|
||||
}
|
||||
showToast(t('backend::lang.reorder.stale'), 'danger')
|
||||
await load()
|
||||
}
|
||||
return
|
||||
}
|
||||
tree.value = cloneTree(savedTree)
|
||||
showToast(t('backend::lang.reorder.failed'), 'danger')
|
||||
failed(job)
|
||||
} catch {
|
||||
tree.value = cloneTree(savedTree)
|
||||
showToast(t('backend::lang.reorder.failed'), 'danger')
|
||||
failed(job)
|
||||
} finally {
|
||||
saving.value--
|
||||
inFlight = false
|
||||
const next = queued
|
||||
queued = null
|
||||
if (next) {
|
||||
void send(next)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -138,14 +193,14 @@ function scheduleSave(): void {
|
||||
if (saveTimer !== null) {
|
||||
clearTimeout(saveTimer)
|
||||
}
|
||||
saveTimer = setTimeout(() => void save(), SAVE_DEBOUNCE_MS)
|
||||
saveTimer = setTimeout(save, SAVE_DEBOUNCE_MS)
|
||||
}
|
||||
|
||||
/** Sends a pending save at once (leaving the page, changing the partition). */
|
||||
function flush(): void {
|
||||
if (saveTimer !== null) {
|
||||
clearTimeout(saveTimer)
|
||||
void save()
|
||||
save()
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -318,6 +318,94 @@ describe('ReorderView moves', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// WR-02: one save in flight at a time, later moves sent after it settles,
|
||||
// and a failure that a waiting save supersedes does not throw moves away.
|
||||
describe('ReorderView save ordering', () => {
|
||||
it('sends a move made during a save after that save, with the latest tree', async () => {
|
||||
const bodies: { nodes: { id: number }[] }[] = []
|
||||
const releases: ((reply: Reply) => void)[] = []
|
||||
const { wrapper, calls } = await mountApp(
|
||||
'/acme/companions/links/reorder',
|
||||
{
|
||||
[`GET ${REORDER}`]: tree(),
|
||||
[`POST ${REORDER}`]: async (request) => {
|
||||
bodies.push((await request.json()) as { nodes: { id: number }[] })
|
||||
return new Promise<Reply>((resolve) => releases.push(resolve))
|
||||
},
|
||||
},
|
||||
{ attach: true },
|
||||
)
|
||||
keydown(handle(wrapper, 1).element, 'ArrowDown')
|
||||
await flushPromises()
|
||||
await vi.waitFor(() => expect(requestsTo(calls, 'POST', REORDER)).toHaveLength(1))
|
||||
|
||||
// A second move while the first save is in flight waits for it.
|
||||
keydown(handle(wrapper, 4).element, 'ArrowUp')
|
||||
await flushPromises()
|
||||
await wait(500)
|
||||
expect(requestsTo(calls, 'POST', REORDER)).toHaveLength(1)
|
||||
|
||||
// The first save fails: the later moves stay on screen, and the waiting
|
||||
// save sends them.
|
||||
releases[0]!({ status: 500, body: { error: { code: 'x', message: 'x', details: {} } } })
|
||||
await vi.waitFor(() => expect(requestsTo(calls, 'POST', REORDER)).toHaveLength(2))
|
||||
expect(shape(wrapper)).toBe('Blog(News) Bez tytułu (#4) Home')
|
||||
expect(toasts.value).toHaveLength(0)
|
||||
expect(bodies[1]!.nodes.map((node) => node.id)).toEqual([2, 3, 4, 1])
|
||||
releases[1]!(tree())
|
||||
await vi.waitFor(() => {
|
||||
expect(wrapper.find('[data-reorder-status]').text()).toBe('Zapisano kolejność')
|
||||
})
|
||||
expect(shape(wrapper)).toBe('Blog(News) Bez tytułu (#4) Home')
|
||||
await wait(500)
|
||||
expect(requestsTo(calls, 'POST', REORDER)).toHaveLength(2)
|
||||
})
|
||||
|
||||
it('ignores a save response that arrives after the partition changed', async () => {
|
||||
let releaseFirst: (reply: Reply) => void = () => undefined
|
||||
let posts = 0
|
||||
const footer = tree()
|
||||
;(footer.body as { data: { nodes: unknown[] } }).data.nodes = [
|
||||
{ id: 7, name: 'Imprint', parent_id: null, sort_order: 1 },
|
||||
{ id: 8, name: 'Privacy', parent_id: null, sort_order: 2 },
|
||||
]
|
||||
const { wrapper } = await mountApp(
|
||||
'/acme/companions/links/reorder',
|
||||
{
|
||||
[`GET ${REORDER}`]: (request) => (queryOf(request).get('partition') === '6' ? footer : tree()),
|
||||
[`POST ${REORDER}`]: () => {
|
||||
posts++
|
||||
if (posts === 1) {
|
||||
return new Promise<Reply>((resolve) => {
|
||||
releaseFirst = resolve
|
||||
})
|
||||
}
|
||||
return { status: 500, body: { error: { code: 'x', message: 'x', details: {} } } }
|
||||
},
|
||||
},
|
||||
{ attach: true },
|
||||
)
|
||||
keydown(handle(wrapper, 1).element, 'ArrowDown')
|
||||
await flushPromises()
|
||||
await wrapper.find('[data-reorder-partition]').setValue('6')
|
||||
await flushPromises()
|
||||
await vi.waitFor(() => expect(shape(wrapper)).toBe('Imprint Privacy'))
|
||||
|
||||
// The first partition's save succeeds late: the second partition's
|
||||
// screen and its last saved tree are not touched.
|
||||
releaseFirst(tree())
|
||||
await flushPromises()
|
||||
expect(shape(wrapper)).toBe('Imprint Privacy')
|
||||
|
||||
// A failed save on the second partition restores its own tree.
|
||||
keydown(handle(wrapper, 7).element, 'ArrowDown')
|
||||
await flushPromises()
|
||||
expect(shape(wrapper)).toBe('Privacy Imprint')
|
||||
await vi.waitFor(() => expect(toasts.value.length).toBeGreaterThan(0))
|
||||
expect(shape(wrapper)).toBe('Imprint Privacy')
|
||||
})
|
||||
})
|
||||
|
||||
describe('reorder tree rules', () => {
|
||||
// The OpenAPI document types parent_id as a number although the wire
|
||||
// carries null at the top (17-02 deferred item 3), hence the cast.
|
||||
|
||||
@@ -205,7 +205,7 @@ A save posts `{partition, nodes: [{id, parent_id, sort_order}]}` to `POST .../re
|
||||
|
||||
Both routes need the controller's permissions; the save is a write route (CSRF header, and a personal token needs a scope that allows POST).
|
||||
|
||||
In the admin, a list whose schema carries `reorder` shows a "Reorder records" button before "New record". It opens the reorder page, and when the list is filtered on the partition column it opens that partition. The page shows the tree with a drag handle and move up, move down, outdent and indent buttons on every row. A row can also be dragged before, after or into another row (never into its own subtree), and the arrow keys on a focused handle move it the same way as the buttons. There is no Save button: 400 ms after the last move the page posts the whole set, shows "Order saved", and sends a pending save at once when the administrator leaves. A refused save puts the last saved order back; a 409 reloads the tree. A partitioned page has a selector for the partition.
|
||||
In the admin, a list whose schema carries `reorder` shows a "Reorder records" button before "New record". It opens the reorder page, and when the list is filtered on the partition column it opens that partition. The page shows the tree with a drag handle and move up, move down, outdent and indent buttons on every row. A row can also be dragged before, after or into another row (never into its own subtree), and the arrow keys on a focused handle move it the same way as the buttons. There is no Save button: 400 ms after the last move the page posts the whole set, shows "Order saved", and sends a pending save at once when the administrator leaves. Only one save is in flight at a time: moves made while it runs are sent after it settles, with the tree as it is then, so the stored order never ends older than the screen. A refused save puts the last saved order back, unless a later save is waiting (it carries the whole tree); a 409 reloads the tree. A save that settles after the page switched partition does not touch the new partition's tree. A partitioned page has a selector for the partition.
|
||||
|
||||
## Remembered list state
|
||||
|
||||
|
||||
Reference in New Issue
Block a user