gsadmin/pad
1
0
Fork 0
mirror of https://github.com/PerpetualSoftware/pad.git synced 2026-10-25 14:00:40 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(web): a relation hold belongs to one item and one field type, and a retarget discards it instead of reviving it on return (TASK-3048) (#1582)

Browse Source 
The multi_relation hold (the list this editor last sent, kept as the base
until that write is over) was stamped with workspace + key + target
collection only. Two items in one workspace share those, and so does a field
retyped in place, so a reused editor could not tell another row's hold, or
one taken under different value rules, from its own. A hold also survived
X→Y→X: returning to X revived a hold from before the excursion, over a
value that may have moved meanwhile.

The stamp now includes the item id and the field type. A new effect
discards the hold on a real identity change, compared against the previous
string so an ordinary re-render keeps it. The pending write still settles
through its ticket, and that settlement cannot release a newer hold.

One existing test asserted the old X→Y→X behaviour ("the old hold must
still be in force"). It is changed to the ruling, with its original concern
(the new target shows and bases on its own value) kept.

Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9
This commit is contained in:
xarmian
2026-09-26 16:54:10 -04:00
committed by GitHub
parent a537ba84e2
commit bbf09ee4bb
2 changed files with 144 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
web/src/lib/components/fields/FieldEditor.multiRelation.svelte.test.ts
+110 -4
View File
@@ -780,13 +780,119 @@ describe('multi_relation — what the round-8 enumeration found in the round-8 f
expect(onchange.mock.calls[0][0]).toEqual([RED.id, BLUE.id]);
// Same instance, different workspace, and the new item's value is the
// list the OLD workspace's hold is carrying.
// list the OLD workspace's hold is carrying. The new target shows its
// own value, and the next edit bases on it.
await rerender({ field: multi, wsSlug: 'other-ws', username: 'dave', value: [RED.id, BLUE.id], onchange });
await tick();
// Back again. The old hold must still be in force — it was never
// confirmed by anything.
expect(titles()).toEqual(['Red', 'Blue']);
await fireEvent.click(buttons(/^\s*Remove\s*$/)[1]);
expect(onchange.mock.calls[1][0]).toEqual([RED.id]);
// Back again. This used to assert the old hold was still in force, "never
// confirmed by anything". TASK-3048 ruled the other way: a retarget
// DISCARDS a hold, because on the return it would stand over a value
// that may have moved meanwhile. The editor shows the prop.
await rerender({ ...editable, value: [GREEN.id, RED.id, BLUE.id], onchange });
await tick();
expect(titles()).toEqual(['Red', 'Blue']);
expect(titles()).toEqual(['Green', 'Red', 'Blue']);
});
});
describe('multi_relation — the hold is ONE item\'s, ONE type\'s, and ends on a retarget (TASK-3048)', () => {
const base = { field: multi, wsSlug: 'ws', username: 'dave', itemId: 'item-x' } as const;
const titles = () => [...document.querySelectorAll('.relation-title')].map((n) => n.textContent);
/** A consumer whose writes settle only when the test says so. */
function gatedConsumer() {
const gates: Array<() => void> = [];
const onchange = vi.fn(() => new Promise<void>((resolve) => gates.push(resolve)));
return { onchange, gates };
}
async function settle() {
await tick();
await tick();
await tick();
}
it('X→Y→X by WORKSPACE does not revive the hold from before the excursion', async () => {
// The stamp already hid the hold while on Y; the defect was the RETURN:
// the dormant hold came back and stood over a value that may have moved.
const { onchange } = gatedConsumer();
const { rerender } = render(FieldEditor, { props: { ...base, value: [GREEN.id, RED.id, BLUE.id], onchange } });
await tick();
await fireEvent.click(buttons(/^\s*Remove\s*$/)[0]);
expect(titles()).toEqual(['Red', 'Blue']);
await rerender({ ...base, wsSlug: 'other-ws', value: [BLUE.id], onchange });
await tick();
await rerender({ ...base, value: [GREEN.id, RED.id, BLUE.id], onchange });
await tick();
expect(titles()).toEqual(['Green', 'Red', 'Blue']);
await fireEvent.click(buttons(/^\s*Remove\s*$/)[2]);
expect(onchange.mock.calls[1][0]).toEqual([GREEN.id, RED.id]);
});
it('another ITEM in the same workspace neither shows nor bases on the held list', async () => {
// Same workspace, key and target: before the item joined the stamp, a
// reused editor could not tell this row's hold from the next row's.
const { onchange } = gatedConsumer();
const { rerender } = render(FieldEditor, { props: { ...base, value: [GREEN.id, RED.id, BLUE.id], onchange } });
await tick();
await fireEvent.click(buttons(/^\s*Remove\s*$/)[0]);
await rerender({ ...base, itemId: 'item-y', value: [BLUE.id, GREEN.id], onchange });
await tick();
expect(titles()).toEqual(['Blue', 'Green']);
await fireEvent.click(buttons(/^\s*Remove\s*$/)[0]);
expect(onchange.mock.calls[1][0]).toEqual([GREEN.id]);
});
it('a field retyped away and back does not revive a hold taken under the old rules', async () => {
const { onchange } = gatedConsumer();
const { rerender } = render(FieldEditor, { props: { ...base, value: [GREEN.id, RED.id, BLUE.id], onchange } });
await tick();
await fireEvent.click(buttons(/^\s*Remove\s*$/)[0]);
await rerender({ ...base, field: { ...multi, type: 'relation' as never }, value: RED.id, onchange });
await tick();
await rerender({ ...base, value: [GREEN.id, RED.id, BLUE.id], onchange });
await tick();
expect(titles()).toEqual(['Green', 'Red', 'Blue']);
});
it('CONTROL: a re-render with the SAME identity keeps the hold', async () => {
const { onchange } = gatedConsumer();
const { rerender } = render(FieldEditor, { props: { ...base, value: [GREEN.id, RED.id, BLUE.id], onchange } });
await tick();
await fireEvent.click(buttons(/^\s*Remove\s*$/)[0]);
await rerender({ ...base, value: [GREEN.id, RED.id, BLUE.id], onchange });
await tick();
expect(titles()).toEqual(['Red', 'Blue']);
await fireEvent.click(buttons(/^\s*Remove\s*$/)[0]);
expect(onchange.mock.calls[1][0]).toEqual([BLUE.id]);
});
it('the discarded write still settles, and its settlement cannot release a NEWER hold', async () => {
const { onchange, gates } = gatedConsumer();
const { rerender } = render(FieldEditor, { props: { ...base, value: [GREEN.id, RED.id, BLUE.id], onchange } });
await tick();
await fireEvent.click(buttons(/^\s*Remove\s*$/)[0]); // write 1, held
await rerender({ ...base, itemId: 'item-y', value: [BLUE.id], onchange });
await tick();
await rerender({ ...base, value: [GREEN.id, RED.id, BLUE.id], onchange });
await tick();
await fireEvent.click(buttons(/^\s*Remove\s*$/)[2]); // write 2, held: [Green, Red]
expect(titles()).toEqual(['Green', 'Red']);
gates[0](); // the discarded write settles
await settle();
expect(titles()).toEqual(['Green', 'Red']);
gates[1]();
await settle();
expect(titles()).toEqual(['Green', 'Red', 'Blue']);
});
});
web/src/lib/components/fields/FieldEditor.svelte
+34 -4
View File
@@ -15,7 +15,7 @@ visual language as the editor but with no inputs, dropdowns, or mutation
handlers — onchange is never called.
-->
<script lang="ts">
import { onDestroy, tick } from 'svelte';
import { onDestroy, tick, untrack } from 'svelte';
import { formatItemRef, type FieldDef, type ItemIndexRow, type PaneTarget } from '$lib/types';
import { localIndex } from '$lib/stores/localIndex.svelte';
import { rawText, readAs } from '$lib/fields/fieldShape';
@@ -203,10 +203,17 @@ handlers — onchange is never called.
});
/**
* What this component is resolving against: the workspace and the field's
* declared target. A held list belongs to ONE of these and to no other.
* What this component is resolving against: the workspace, the ITEM, the
* field and its type, and the field's declared target. A held list belongs to
* ONE of these and to no other. The item and the type joined in TASK-3048:
* two items share workspace + key + target, and a field retyped in place
* (a live schema edit) keeps its key, so without them a reused editor could
* not tell a hold taken for another row, or under another type's value
* rules, from its own.
*/
let relationIdentity = $derived(`${wsSlug ?? ''}\u0000${field.key}\u0000${field.collection ?? ''}`);
let relationIdentity = $derived(
`${wsSlug ?? ''}\u0000${itemId ?? ''}\u0000${field.key}\u0000${field.type}\u0000${field.collection ?? ''}`
);
/**
* The list this component last SENT, held until that write is OVER, STAMPED
@@ -360,6 +367,29 @@ handlers — onchange is never called.
}
});
// Discard a hold when the editor is RETARGETED, so X→Y→X does not revive a
// hold from before the excursion (TASK-3048). The stamp fence above already
// stops a foreign hold being shown or released by agreement; what it left is
// a DORMANT hold that comes back to life on the return, over a value that
// may have moved meanwhile. The write it protected still settles through its
// ticket (`holdOrder`), and its release finds nothing to release.
//
// The trigger is a real CHANGE of the identity string, compared against the
// previous one, never "the effect re-ran": the note on `pendingRelation`
// records that clearing from an effect reading the props directly released
// holds on ordinary re-renders mid-write. `lastRelationIdentity` is a plain `let` so writing
// it does not re-trigger this effect, and the hold is read untracked so a
// new hold being armed does not either.
let lastRelationIdentity: string | undefined;
$effect(() => {
const identity = relationIdentity;
if (lastRelationIdentity !== undefined && identity !== lastRelationIdentity) {
const pending = untrack(() => pendingRelation);
if (pending && pending.identity !== identity) pendingRelation = null;
}
lastRelationIdentity = identity;
});
/**
* Whether the field's declared target still names a live collection.
*