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

fix(web): a board lane writes a value the schema can hold, not the lane key (BUG-3057) (#1353)

Browse Source 
* fix(web): a board lane writes a value the schema can hold, not the lane key (BUG-3057)

A lane key is a STRING projection of a field value — that is what lets a
view group by anything. Both write paths on the collection page assigned
that string straight back to the field: `fields[groupField] = newValue` on
the drag path, `defaultFields[groupField] = groupValue` on create-in-lane.

The item was filed as silent retyping. It is not: the server REFUSES these
writes. Measured against `internal/items.ValidatePartialFields`, the
validator behind `fields_patch`:

    number       "0"     -> field "score" must be a number
    number       ""      -> field "score" must be a number
    checkbox     "false" -> field "shipped" must be a boolean
    multi_select "c"     -> field "tags" must be an array of strings

So the symptom is FAILURE, not corruption: on a board whose group field is
not string-shaped, a legitimate drag between lanes and a legitimate
create-in-lane do not take. Nothing bad is stored, and there is nothing to
migrate.

`multi_select` is the reachable one — the display-settings picker offers
select AND multi_select, so no retype is needed to get there. The rest
arrive the way BUG-3041's and BUG-3016's cases do: a field retyped under a
`board_group_by` that still names it, or a setting written through the API.

`laneWriteValue` converts through the declared type: a number gets a
number (including 0), a checkbox gets a bool for the two spellings the
projection makes and nothing else, a multi_select gets a one-element array,
and everything string-shaped keeps its key unchanged. The UNCATEGORIZED
lane means "no value", whose honest spelling differs by type — `''` for a
select (a legal clear), `[]` for a multi_select, and the patch path's
DELETE sentinel `null` for a number or checkbox, which have no empty
string to hold. A lane key that cannot be a value at all (a stale `open`
lane over a retyped number field) refuses the write, toasts, and throws,
so BoardView unwinds its optimistic move instead of showing it as landed.

Tests: 8 unit legs on the converter, and the same table pinned from the GO
side (`TestLaneKeyWrites_BUG3057`) — that test is not a test of the fix,
it is a test of the facts the fix was derived from, so a validator change
that started accepting the string forms cannot leave the module correct
against a validator that has moved. The call sites are guarded at the
SOURCE, beside BUG-3053's read-side guards, because a behaviour test of
the converter stays green when a call site stops calling it. All 4 guard
legs fail against the unfixed page.

One prior guard updated: BUG-3049's "patches only the group field" leg
anchored on the literal `[groupField]: newValue`. The patched KEY is what
it is about and is unchanged; the value spelling moved. Re-anchored with
the reason in place.

Claude-Session: https://claude.ai/code/session_019EmDd3Wy8b3cPG1TZBSzBu

* fix(web): the table's status chip writes STATUS, not the lane field (BUG-3057)

From the enumeration round, verified in the wiring rather than taken on
report. `handleStatusChange` is one callback serving two intents that do
not name the same field: a drag between lanes means "put this item in THIS
LANE" (the group field), and a status chip means "set this item's STATUS"
(the `status` field). The group field was hardcoded, so the TABLE's status
chip — whose options come from the `status` schema field — wrote its value
into whatever `list_group_by` named. On a table grouped by `priority`,
clicking a status chip set the priority.

Pre-existing, and reachable with no retype: `list_group_by` takes any
select field from the display-settings picker. What this unit changed is
its failure mode, not its existence — with the type conversion in place a
status word sent to a number-typed group field is now refused and toasted
rather than written, so leaving it would have meant shipping a change that
alters an existing defect's behaviour without saying so.

The writer takes its target field explicitly, defaulting to `groupField`
so every drag caller is unchanged; the table names `status` at its call
site. It can, because the chip is its only consumer of that prop.

NOT fixed here: `ListView` and `BoardView` have the same conflation with
two consumers behind ONE prop — their cards' status chips send status
values to the group field too. That needs a second callback threaded
through both components into ItemCard, which is a prop-surface change with
its own test surface. Filed as BUG-3068, and cited from the writer's doc
comment. ListView's existing comment shows the defect was already
diagnosed there and closed for one arm only (a relation group field), the
arm that had a unit in flight at the time.

Claude-Session: https://claude.ai/code/session_019EmDd3Wy8b3cPG1TZBSzBu

* fix(web): a multi_select lane writes its COMBINATION, not the joined key (BUG-3057)

Lead's catch on the review, measured before changing anything.

`laneValue(['a','b'])` is `'a,b'`, and ListView MINTS a lane for every
distinct value it finds, so a list grouped by a multi_select really does
show an `a,b` lane. (The board seeds its lanes from the field's options
instead, so the same two-tag item lands in UNCATEGORIZED there — measured
both ways.) A drop onto that lane means "give this item that lane's tags".

Wrapping the key wrote `["a,b"]` — one tag, not two, and not a tag anyone
has. Measured on the pre-fix module:

    write for lane "a,b" = {"ok":true,"value":["a,b"]}

Both outcomes were wrong and neither was silent-and-harmless: with options
declared the server REFUSES it (`"a,b"` is not in allowed options, now
pinned from the Go side), and with no options declared it stores a tag
that exists nowhere.

The arm now inverts the projection against the DECLARED OPTIONS rather
than splitting blindly, so an option containing a comma is not torn in
half: the key IS an option -> that option; every comma-part is an option
-> those options in lane order; no options declared -> split, the plain
inverse of the join; otherwise -> the key whole. Rule 1 before rule 2 is a
stated tie-break, because with options `['a,b','a','b']` the lane is
genuinely ambiguous — the projection is lossy — and the reading that names
an option that EXISTS wins.

Six legs, including the two-tag lane, a single-tag CONTROL (without it,
"always split" and "always wrap" each pass for the wrong reason on one
side), a comma-in-option leg, the ambiguous tie-break, and the no-options
split. Mutating the arm back to the wrap fails exactly two of them and
leaves the control and the comma leg green, which is what those two are
for.

Claude-Session: https://claude.ai/code/session_019EmDd3Wy8b3cPG1TZBSzBu
This commit is contained in:
xarmian
2026-09-13 23:32:48 -04:00
committed by GitHub
parent 721a638f5a
commit 9e121bde64
6 changed files with 468 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/items/lane_write_bug3057_test.go
+80
View File
@@ -0,0 +1,80 @@
package items
import (
"strings"
"testing"
"github.com/PerpetualSoftware/pad/internal/models"
)
// The server half of BUG-3057's contract.
//
// A board/list lane key is a STRING projection of a field value, and the web
// write paths used to assign it straight back to the field. `laneWriteValue`
// (web/src/lib/collections/laneWriteValue.ts) now converts through the declared
// type, and its comments cite what this validator does with each form. That
// citation is only worth anything if it is pinned HERE: a change to the
// validator that started accepting the string forms, or stopped accepting the
// converted ones, would leave that module correct against a validator that has
// moved and nothing would notice.
//
// Not a test of the fix — the fix is in TypeScript. A test of the facts the fix
// was derived from.
func TestLaneKeyWrites_BUG3057(t *testing.T) {
schema := models.CollectionSchema{Fields: []models.FieldDef{
{Key: "score", Type: "number"},
{Key: "shipped", Type: "checkbox"},
{Key: "tags", Type: "multi_select", Options: []string{"a", "b", "c"}},
{Key: "stage", Type: "select", Options: []string{"open", "done"}},
}}
cases := []struct {
name string
patch map[string]any
wantErr string // "" means the write must be ACCEPTED
}{
// What the lane key IS, pre-fix. Every one of these is a legitimate
// drag or create-in-lane that failed.
{"number gets the raw lane key", map[string]any{"score": "0"}, "must be a number"},
{"number gets the uncategorised lane key", map[string]any{"score": ""}, "must be a number"},
{"checkbox gets the raw lane key", map[string]any{"shipped": "false"}, "must be a boolean"},
{"multi_select gets the raw lane key", map[string]any{"tags": "c"}, "must be an array of strings"},
// What `laneWriteValue` sends instead.
{"number gets a number", map[string]any{"score": float64(0)}, ""},
{"number cleared by the delete sentinel", map[string]any{"score": nil}, ""},
{"checkbox gets a bool", map[string]any{"shipped": false}, ""},
{"checkbox cleared by the delete sentinel", map[string]any{"shipped": nil}, ""},
{"multi_select gets a one-element array", map[string]any{"tags": []any{"c"}}, ""},
// A lane is a COMBINATION, so a two-tag lane writes two tags — and both
// have to be declared options or this arm refuses, which is exactly why
// the web side resolves the lane key against the options rather than
// splitting it blindly.
{"multi_select gets a two-element array", map[string]any{"tags": []any{"a", "b"}}, ""},
{"multi_select given the JOINED lane key as one tag", map[string]any{"tags": []any{"a,b"}}, "not in allowed options"},
{"multi_select cleared by an empty array", map[string]any{"tags": []any{}}, ""},
// The case that was never broken, pinned so a conversion applied to
// EVERY type would be caught from this side too.
{"select keeps taking its lane key", map[string]any{"stage": "done"}, ""},
{"select cleared by the empty lane key", map[string]any{"stage": ""}, ""},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
err := ValidatePartialFields(tc.patch, schema)
if tc.wantErr == "" {
if err != nil {
t.Fatalf("write must be accepted, got: %v", err)
}
return
}
if err == nil {
t.Fatalf("write must be refused (%s), got nil — laneWriteValue's premise has moved", tc.wantErr)
}
if !strings.Contains(err.Error(), tc.wantErr) {
t.Fatalf("refused for the wrong reason: want %q, got %v", tc.wantErr, err)
}
})
}
}
web/src/lib/collections/laneKeyCallers.test.ts
+59
View File
@@ -137,3 +137,62 @@ describe('no surface keeps a private copy of a lane-key helper', () => {
).toBe(true);
});
});
/**
* The WRITE side of the same seam (BUG-3057).
*
* BUG-3053 closed the read side — comparing a lane key against a raw value —
* and this file's guards are about surfaces READING through the shared helpers.
* The two write paths on the collection page had the mirror defect: they
* assigned the lane KEY back to the field, whatever its declared type, and the
* server refuses those writes (`"0"` into a number, `"c"` into a multi_select),
* so a legitimate drag or create-in-lane simply failed.
*
* Asserted at the SOURCE for the reason the guards above are: a behaviour test
* of `laneWriteValue` stays green if a call site stops calling it, and the
* call sites live in a 2000-line route component whose write paths need a
* board, a drag and a network mock to reach.
*/
describe('BUG-3057: the collection page writes a CONVERTED lane value', () => {
const page = read('../../routes/[username]/[workspace]/[collection]/+page.svelte');
it('imports the converter', () => {
expect(page).toContain("from '$lib/collections/laneWriteValue'");
expect(page).toContain('laneWriteValue');
expect(page).toContain('laneWriteRefusalMessage');
});
it('sends the CONVERTED value on the drag path, not the lane key', () => {
// The mutant this kills is the original line, restored. The patched key
// is `fieldKey` rather than `groupField` because the writer now takes its
// target explicitly — see the status-chip leg below.
expect(page).toContain('const fieldsPatch = { [fieldKey]: laneWrite.value };');
expect(page).not.toContain('const fieldsPatch = { [groupField]: newValue };');
});
it('lets the table status chip name the STATUS field, not the lane field', () => {
// The table has no lanes: its chip cycles the `status` schema field's
// options and used to send them to `groupField`, which in table view is
// `list_group_by`. On a table grouped by `priority`, a status click set
// the priority. The default argument keeps every drag caller on the lane
// field, so this leg is what says the table opted out of it.
expect(page).toContain(
"onStatusChange={(it, newStatus) => handleStatusChange(it, newStatus, 'status')}",
);
expect(page).toContain(
'async function handleStatusChange(item: Item, newValue: string, fieldKey: string = groupField)',
);
});
it('sends the CONVERTED value on the create-in-lane path, not the lane key', () => {
expect(page).toContain('laneWriteValue(groupFieldDef, groupValue)');
expect(page).not.toContain('defaultFields[groupField] = groupValue;');
});
it('resolves the group field against the SCHEMA, since the type is what converts', () => {
// Without this derive, both call sites would have nothing to convert
// through and the converter would fall into its undeclared-field arm —
// which writes the key, i.e. the defect, with a helper in front of it.
expect(page).toContain("let groupFieldDef = $derived(schema?.fields.find((f) => f.key === groupField));");
});
});
web/src/lib/collections/laneWriteValue.test.ts
+126
View File
@@ -0,0 +1,126 @@
// Node-project test: a lane key becomes a value the schema can hold (BUG-3057).
//
// The pairing that matters is with `internal/items`: every value this returns
// has to be one the server's validator ACCEPTS for that type, and every string
// it replaces is one the validator REFUSES. The Go side pins the same table
// from its end (TestLaneKeyWrites_BUG3057), so the two cannot drift into a
// state where this module is "correct" against a validator that has moved.
import { describe, it, expect } from 'vitest';
import { laneWriteValue, laneWriteRefusalMessage } from './laneWriteValue';
import type { FieldDef } from '$lib/types';
const f = (type: string, over: Partial<FieldDef> = {}): FieldDef =>
({ key: 'g', label: 'Group', type, ...over }) as unknown as FieldDef;
describe('laneWriteValue', () => {
it('passes a string-shaped field its lane key unchanged', () => {
// select is the common case and was never broken; it is here so a fix
// that converted everything would be caught.
expect(laneWriteValue(f('select'), 'done')).toEqual({ ok: true, value: 'done' });
expect(laneWriteValue(f('text'), 'done')).toEqual({ ok: true, value: 'done' });
expect(laneWriteValue(f('date'), '2026-01-01')).toEqual({ ok: true, value: '2026-01-01' });
// The established clear for a select — `''` is a legal write there.
expect(laneWriteValue(f('select'), '')).toEqual({ ok: true, value: '' });
});
it('gives a number field a NUMBER, including zero', () => {
// `0` is the case BUG-3053 was about on the read side: it is an ordinary
// value with an ordinary lane, and `'0'` is what the projection makes of
// it. The write has to undo exactly that.
expect(laneWriteValue(f('number'), '0')).toEqual({ ok: true, value: 0 });
expect(laneWriteValue(f('number'), '10')).toEqual({ ok: true, value: 10 });
expect(laneWriteValue(f('number'), '-2.5')).toEqual({ ok: true, value: -2.5 });
});
it('clears a number field with the DELETE sentinel, not an empty string', () => {
// `''` is what the uncategorised lane means, and the validator refuses it
// for a number ("must be a number"). `null` is the patch path's delete.
expect(laneWriteValue(f('number'), '')).toEqual({ ok: true, value: null });
});
it('refuses a number lane key that is not a number', () => {
// Reachable when a field is retyped under a `board_group_by` that still
// names it: the lanes are still the old options.
expect(laneWriteValue(f('number'), 'open')).toEqual({ ok: false, reason: 'not_a_number' });
// Whitespace is not zero. `Number(' ')` is 0, which would turn a blank
// lane into a real value.
expect(laneWriteValue(f('number'), ' ')).toEqual({ ok: false, reason: 'not_a_number' });
expect(laneWriteValue(f('number'), 'NaN')).toEqual({ ok: false, reason: 'not_a_number' });
expect(laneWriteValue(f('number'), 'Infinity')).toEqual({ ok: false, reason: 'not_a_number' });
});
it('gives a checkbox field a BOOLEAN, and only for the two spellings the projection makes', () => {
expect(laneWriteValue(f('checkbox'), 'true')).toEqual({ ok: true, value: true });
expect(laneWriteValue(f('checkbox'), 'false')).toEqual({ ok: true, value: false });
expect(laneWriteValue(f('checkbox'), '')).toEqual({ ok: true, value: null });
// Not guesses: nothing in this seam was typed by a user, so 'yes' / '1' /
// 'on' would be inventing an intent.
expect(laneWriteValue(f('checkbox'), '1')).toEqual({ ok: false, reason: 'not_a_boolean' });
expect(laneWriteValue(f('checkbox'), 'yes')).toEqual({ ok: false, reason: 'not_a_boolean' });
});
describe('multi_select — a lane is a COMBINATION, so the write inverts the projection', () => {
// The picker OFFERS multi_select, so all of this is reachable with no
// retype. `laneValue(['a','b'])` is `'a,b'` and ListView mints a lane per
// distinct value, so an `a,b` lane really exists on a grouped list
// (measured; the board seeds lanes from options, so that item sits in
// UNCATEGORIZED there instead).
const tags = f('multi_select', { options: ['a', 'b', 'c'] });
it('writes BOTH tags for a two-tag lane', () => {
// The defect this leg exists for: wrapping the key wrote the single
// tag `"a,b"`, which is not an option and not a tag anyone has.
expect(laneWriteValue(tags, 'a,b')).toEqual({ ok: true, value: ['a', 'b'] });
});
it('CONTROL: a single-tag lane still writes one tag', () => {
// Without this, "always split on comma" and "always wrap" both pass
// the leg above for the wrong reason on one side or the other.
expect(laneWriteValue(tags, 'c')).toEqual({ ok: true, value: ['c'] });
});
it('does not TEAR an option that contains a comma', () => {
// Rule 1 (the key IS an option) before rule 2 (every part is an
// option). A blind split would write two tags that do not exist.
const commas = f('multi_select', { options: ['a,b', 'c'] });
expect(laneWriteValue(commas, 'a,b')).toEqual({ ok: true, value: ['a,b'] });
});
it('prefers the REAL option when the lane is genuinely ambiguous', () => {
// Options `['a,b','a','b']` make the lane `a,b` unreadable in
// principle — the projection is lossy — so the tie-break is stated
// rather than left to whichever rule runs first: the reading that
// names an option that EXISTS wins.
const ambiguous = f('multi_select', { options: ['a,b', 'a', 'b'] });
expect(laneWriteValue(ambiguous, 'a,b')).toEqual({ ok: true, value: ['a,b'] });
});
it('splits when the field declares NO options', () => {
// Nothing to check a part against, and splitting is the plain inverse
// of the join that made the lane.
expect(laneWriteValue(f('multi_select'), 'a,b')).toEqual({ ok: true, value: ['a', 'b'] });
});
it('keeps an unrecognised lane whole', () => {
// A lane minted from a value that is not in the vocabulary at all:
// the honest write is the value the lane was named for.
expect(laneWriteValue(tags, 'legacy,x')).toEqual({ ok: true, value: ['legacy,x'] });
});
it('clears with an empty array', () => {
expect(laneWriteValue(tags, '')).toEqual({ ok: true, value: [] });
});
});
it('writes the key for a field the schema does not declare', () => {
// Orphan keys are accepted server-side and persist as-is; with no type
// there is nothing to convert through, and today's behaviour is right.
expect(laneWriteValue(undefined, 'anything')).toEqual({ ok: true, value: 'anything' });
expect(laneWriteValue(null, 'anything')).toEqual({ ok: true, value: 'anything' });
});
it('names the field in a refusal message', () => {
expect(laneWriteRefusalMessage('not_a_number', 'Score')).toContain('Score');
expect(laneWriteRefusalMessage('not_a_boolean', 'Shipped')).toContain('Shipped');
});
});
web/src/lib/collections/laneWriteValue.ts
+134
View File
@@ -0,0 +1,134 @@
// Turning a board/list LANE KEY back into a field VALUE (BUG-3057).
//
// A lane key is a STRING — `laneValue` in `boardColumns` projects any stored
// value onto one, which is what makes a view able to group by anything. The
// write paths then assigned that string straight back to the field, whatever
// the field's declared type. That projection has no free inverse: `'0'` came
// from `0`, but the key alone cannot say whether the field wanted `0`, `'0'` or
// `false`. The declared TYPE is what disambiguates, so the conversion belongs
// here rather than inline at either call site.
//
// WHAT THE SERVER ACTUALLY DOES, measured rather than assumed (probe against
// `internal/items.ValidatePartialFields`, the validator behind `fields_patch`):
//
// number "0" -> field "score" must be a number
// number "" -> field "score" must be a number
// number nil -> ok (nil is the patch path's DELETE sentinel)
// checkbox "false" -> field "shipped" must be a boolean
// multi_select "c" -> field "tags" must be an array of strings
// multi_select ["c"] -> ok
// multi_select [] -> ok
// select "" -> ok (clearing a select is a legal write)
//
// So the pre-fix symptom is NOT the silent retyping the item was filed as: the
// server REFUSES these writes. A legitimate drag between lanes, and a
// legitimate create-in-lane, simply FAIL on any board whose group field is not
// string-shaped. Nothing is corrupted; the user's move does not take.
//
// WHICH TYPES ARE REACHABLE. The display-settings picker offers `select` AND
// `multi_select` (EditCollectionModal's `selectFieldKeys`), so a multi_select
// board is reachable with no retype at all. Everything else arrives the way
// BUG-3041 and BUG-3016's cases arrive — a field retyped under a setting that
// still names it, or a `board_group_by` written through the API.
import type { FieldDef } from '$lib/types';
import { UNCATEGORIZED } from './boardColumns';
/**
* Why a lane key cannot become a value for this field.
*
* Only reachable when the lane key and the declared type disagree about what
* the field holds — a stale lane from before a retype, or a `board_group_by`
* pointing at a field whose values were never lane keys.
*/
export type LaneWriteRefusal = 'not_a_number' | 'not_a_boolean';
export type LaneWrite = { ok: true; value: unknown } | { ok: false; reason: LaneWriteRefusal };
/**
* The value to write for a drop into `laneKey`, or a refusal.
*
* UNCATEGORIZED (`''`) means "no value", and the honest spelling of that
* differs by type: `''` for a string-shaped field (a legal, established clear —
* `boardColumns` documents the empty lane as writing it back), `[]` for a
* multi_select, and the patch path's DELETE sentinel `null` for a number or a
* checkbox, which have no empty string to hold.
*
* An UNDECLARED field (no schema entry) keeps today's behaviour and writes the
* key: orphan keys are accepted and persist as-is server-side, and this module
* has nothing better to go on than the string it was handed.
*/
export function laneWriteValue(field: FieldDef | undefined | null, laneKey: string): LaneWrite {
const type = field?.type;
if (!field || !type) return { ok: true, value: laneKey };
switch (type) {
case 'number': {
if (laneKey === UNCATEGORIZED) return { ok: true, value: null };
// `Number('')` is 0 and `Number(' ')` is 0, which would turn an
// unlabelled lane into a real zero; the empty case is handled above
// and whitespace is not a number anyone typed.
const n = laneKey.trim() === '' ? NaN : Number(laneKey);
if (!Number.isFinite(n)) return { ok: false, reason: 'not_a_number' };
return { ok: true, value: n };
}
case 'checkbox': {
if (laneKey === UNCATEGORIZED) return { ok: true, value: null };
// Exactly the two spellings `laneValue(true)` / `laneValue(false)`
// produce. Nothing else is accepted: 'yes', '1' and 'on' are guesses
// about what a user meant, and this seam never had a user typing.
if (laneKey === 'true') return { ok: true, value: true };
if (laneKey === 'false') return { ok: true, value: false };
return { ok: false, reason: 'not_a_boolean' };
}
case 'multi_select': {
// A LANE IS A COMBINATION, so the write is the combination — which
// makes this the one arm that has to INVERT the projection rather
// than wrap the key.
//
// `laneValue(['a','b'])` is `'a,b'`, and ListView MINTS a lane for
// every distinct value it finds, so a list grouped by a multi_select
// really does show an `a,b` lane (measured; the board instead seeds
// its lanes from the field's options, so the same item lands in
// UNCATEGORIZED there). Dropping onto that lane means "give this item
// that lane's tags". Wrapping the key would have written the single
// tag `"a,b"`, which is not an option and not a tag anyone has.
//
// Resolved against the DECLARED OPTIONS rather than split blindly,
// so an option that CONTAINS a comma is not torn in half:
// 1. the key IS an option -> that one option
// 2. every comma-part is an option -> those options, in lane order
// 3. no options declared -> split, the plain inverse of the
// join, since there is no vocabulary to check against
// 4. otherwise -> the key as a single value, which
// is what a lane minted from an undeclared value means
//
// Rule 1 before rule 2 is a deliberate tie-break: with options
// `['a,b','a','b']` the lane `a,b` is genuinely ambiguous — the
// projection is lossy — and the reading that names a REAL option beats
// the one that reconstructs two.
if (laneKey === UNCATEGORIZED) return { ok: true, value: [] };
const options = field.options ?? [];
if (options.length === 0) {
return { ok: true, value: laneKey.split(',').filter((p) => p !== '') };
}
if (options.includes(laneKey)) return { ok: true, value: [laneKey] };
const parts = laneKey.split(',');
if (parts.length > 1 && parts.every((p) => options.includes(p))) {
return { ok: true, value: parts };
}
return { ok: true, value: [laneKey] };
}
default:
// `select`, `text`, `url`, `date`, `relation`, `json` — all hold a
// string, and a relation lane key is already the target's item id
// (the relation lane path resolves it before it gets here).
return { ok: true, value: laneKey };
}
}
/** What to tell the user when a lane key cannot become a value. */
export function laneWriteRefusalMessage(reason: LaneWriteRefusal, fieldLabel: string): string {
return reason === 'not_a_number'
? `Can't move: ${fieldLabel} holds a number and this lane isn't one`
: `Can't move: ${fieldLabel} holds a checkbox and this lane isn't true or false`;
}
web/src/routes/[username]/[workspace]/[collection]/+page.svelte
+61 -5
View File
@@ -8,6 +8,7 @@
import { parseSettings, parseFields, parseSchema, parseTags, getStatusOptions, itemUrlId, formatItemRef } from '$lib/types';
import { plansProgressToMap, fetchCollectionProgress } from '$lib/collections/progressMerge';
import { resolveRenameNavTarget, resolveSyncRenameTarget } from '$lib/collections/renameNav';
import { laneWriteValue, laneWriteRefusalMessage } from '$lib/collections/laneWriteValue';
import BoardView from '$lib/components/collections/BoardView.svelte';
import ListView from '$lib/components/collections/ListView.svelte';
import TableView from '$lib/components/collections/TableView.svelte';
@@ -1315,6 +1316,14 @@
: (settings?.list_group_by ?? 'status')
);
/**
* The group field's SCHEMA entry — what a lane key has to be converted back
* through before it is written (BUG-3057). `undefined` for a `board_group_by`
* naming a field the schema does not declare, which `laneWriteValue` treats
* as "write the key", matching the server's orphan-key handling.
*/
let groupFieldDef = $derived(schema?.fields.find((f) => f.key === groupField));
let statusOptions = $derived(collection ? getStatusOptions(collection) : []);
/** Schema keys whose filter value names an item rather than an option. */
@@ -1665,7 +1674,24 @@
searchResultRank = new Map(hits.map((h, i) => [h.id, i]));
});
async function handleStatusChange(item: Item, newValue: string) {
/**
* Write one field on one item, from a lane move or from a status chip.
*
* `fieldKey` is EXPLICIT because two different intents arrive here and they
* do not name the same field (BUG-3057, codex enumeration round). A drag
* between lanes means "put this item in THIS LANE" — the group field. A
* status chip means "set this item's STATUS" — the `status` field and
* nothing else. With `groupField` hardcoded, the TABLE's status chip, whose
* options come from the `status` schema field, wrote its value into whatever
* `list_group_by` named: on a table grouped by `priority`, clicking a status
* chip set the priority. Defaulting to `groupField` leaves every drag caller
* unchanged.
*
* The same conflation exists for the status chip on a LIST/BOARD card, where
* one prop serves both the drag and the chip — that needs a second callback
* through those components rather than an argument here, and is BUG-3068.
*/
async function handleStatusChange(item: Item, newValue: string, fieldKey: string = groupField) {
if (!wsSlug) return;
// BUG-3049: patch ONLY the group field. This used to read the item's
// fields, set one key and send the whole blob back as a full replace,
@@ -1673,7 +1699,24 @@
// reverted. `fields_patch` merges per key under the row lock
// (internal/store/items.go::mergeFieldsPatch), so no other field is
// reachable by this write at all.
const fieldsPatch = { [groupField]: newValue };
// BUG-3057: a lane key is a STRING projection of the value, and assigning
// it straight back sent a string to a field the schema says holds
// something else. The server REFUSES those writes (`"0"` into a number,
// `"c"` into a multi_select), so a legitimate drag between lanes simply
// failed on any board not grouped by a string-shaped field. Convert
// through the declared type; see `laneWriteValue` for the measured
// server behaviour behind each case.
const targetFieldDef = schema?.fields.find((f) => f.key === fieldKey);
const laneWrite = laneWriteValue(targetFieldDef, newValue);
if (!laneWrite.ok) {
// Thrown, not swallowed: BoardView unwinds its optimistic reorder on
// a rejection, which is the same contract the catch below relies on.
const label = targetFieldDef?.label || fieldKey;
const msg = laneWriteRefusalMessage(laneWrite.reason, label);
toastStore.show(msg, 'error');
throw new Error(msg);
}
const fieldsPatch = { [fieldKey]: laneWrite.value };
const ws = wsSlug;
const parentRef = formatItemRef(item) ?? item.slug;
@@ -1932,8 +1975,21 @@
defaultFields.status = statusField.options[0];
}
// Pre-fill the lane's group field (status, or a custom
// board_group_by select) so the item opens in this lane.
defaultFields[groupField] = groupValue;
// board_group_by select) so the item opens in this lane — converted
// through the declared type, for the reason on the drag path
// (BUG-3057). Creating in a `0` lane of a number field used to send
// the string `"0"`, which the server refuses, so the create failed.
const laneWrite = laneWriteValue(groupFieldDef, groupValue);
if (!laneWrite.ok) {
const label = groupFieldDef?.label || groupField;
toastStore.show(laneWriteRefusalMessage(laneWrite.reason, label), 'error');
return null;
}
// A null here is the PATCH path's delete sentinel and means nothing on
// create — the field is simply absent from a new item.
if (laneWrite.value !== null) {
defaultFields[groupField] = laneWrite.value;
}
// Epoch before create: a brand-new id is never in the fence set,
// so this is the case the epoch guard exists for (BUG-2098).
const epoch = localIndex.scopeEpochFor(wsSlug);
@@ -3552,7 +3608,7 @@
{collection}
{wsSlug}
{focusedItemId}
onStatusChange={handleStatusChange}
onStatusChange={(it, newStatus) => handleStatusChange(it, newStatus, 'status')}
onReorder={handleReorder}
oncreate={canEditThisCollection ? openQuickCreate : undefined}
{itemProgress}
web/src/routes/bug3049WebFullBlobWriters.test.ts
+8 -1
View File
@@ -75,11 +75,18 @@ describe('BUG-3049 — browser item-field writers send a patch, not a blob', ()
});
it('the collection page status move patches only the group field', () => {
// The patched KEY is what this leg is about, and it is unchanged. The
// VALUE spelling moved under BUG-3057: the lane key is now converted
// through the field's declared type before it is written, because
// assigning the raw key sent a string to a number / checkbox /
// multi_select field and the server refuses those writes. `laneWrite`
// IS `newValue` converted, so "patches only the group field" holds
// exactly as before — only the literal this guard anchors on changed.
const body = functionBody(
source('./[username]/[workspace]/[collection]/+page.svelte'),
'async function handleStatusChange(',
);
expectPatchOnly(body, 'handleStatusChange', ['[groupField]: newValue']);
expectPatchOnly(body, 'handleStatusChange', ['[fieldKey]: laneWrite.value']);
});
it('the conventions status toggle patches only status', () => {