mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-25 14:00:40 +00:00
fix(web): a field value whose shape does not match its declared type shows its raw text with a note and is replaced explicitly, never coerced (BUG-3052 unit 2) (#1556)
This commit is contained in:
@@ -0,0 +1,97 @@
|
||||
import { test, expect } from './fixtures';
|
||||
import { browserLogin } from './lib/collab-helpers';
|
||||
import { authJson, createCollection, deleteCollection } from './lib/attachment-viewer';
|
||||
import type { APIRequestContext } from '@playwright/test';
|
||||
import type { SuiteFixture } from './fixtures';
|
||||
|
||||
/**
|
||||
* BUG-3052 unit 2: a stored value whose shape does not match its field's
|
||||
* declared type is shown in the item page as its raw text with a note, and an
|
||||
* edit replaces it explicitly.
|
||||
*
|
||||
* The component legs mount FieldEditor with the value they choose. This leg is
|
||||
* about what the PAGE hands it, which the component legs cannot see: ItemDetail
|
||||
* passes an unset field as `''`, and the first version of the shape check read
|
||||
* that as "not a number" and marked every empty number field on every item.
|
||||
* So the proving pair is one item that must carry the marker and one that must
|
||||
* not.
|
||||
*/
|
||||
|
||||
async function createItem(
|
||||
fixture: SuiteFixture,
|
||||
request: APIRequestContext,
|
||||
collSlug: string,
|
||||
title: string,
|
||||
fields: Record<string, unknown>,
|
||||
): Promise<{ slug: string }> {
|
||||
const resp = await request.post(
|
||||
`/api/v1/workspaces/${fixture.workspaceSlug}/collections/${collSlug}/items`,
|
||||
{ headers: authJson(fixture), data: { title, fields: JSON.stringify(fields), content: '' } },
|
||||
);
|
||||
if (!resp.ok()) throw new Error(`item create failed (${resp.status()}): ${await resp.text()}`);
|
||||
return (await resp.json()) as { slug: string };
|
||||
}
|
||||
|
||||
async function setSchema(fixture: SuiteFixture, request: APIRequestContext, slug: string, schema: unknown) {
|
||||
const resp = await request.patch(`/api/v1/workspaces/${fixture.workspaceSlug}/collections/${slug}`, {
|
||||
headers: authJson(fixture),
|
||||
data: { schema: JSON.stringify(schema) },
|
||||
});
|
||||
if (!resp.ok()) throw new Error(`collection patch failed (${resp.status()}): ${await resp.text()}`);
|
||||
}
|
||||
|
||||
async function storedEffort(fixture: SuiteFixture, request: APIRequestContext, slug: string): Promise<unknown> {
|
||||
const resp = await request.get(`/api/v1/workspaces/${fixture.workspaceSlug}/items/${slug}`, {
|
||||
headers: authJson(fixture),
|
||||
});
|
||||
const body = (await resp.json()) as { fields: string | Record<string, unknown> };
|
||||
const fields = typeof body.fields === 'string' ? JSON.parse(body.fields) : body.fields;
|
||||
return (fields as Record<string, unknown>).effort;
|
||||
}
|
||||
|
||||
test.describe('a field value whose shape does not match its type (BUG-3052 unit 2)', () => {
|
||||
test.setTimeout(90_000);
|
||||
test.beforeEach(({}, testInfo) => {
|
||||
test.skip(testInfo.project.name !== 'desktop-chromium', 'one project is enough for a data-shape concern');
|
||||
});
|
||||
|
||||
test('is marked on the item that holds it, not on an item where the field is unset, and Replace writes a number', async ({
|
||||
page,
|
||||
fixture,
|
||||
request,
|
||||
}) => {
|
||||
const stamp = Date.now();
|
||||
const coll = await createCollection(fixture, request, `Efforts ${stamp}`);
|
||||
try {
|
||||
await setSchema(fixture, request, coll.slug, { fields: [{ key: 'effort', label: 'Effort', type: 'text' }] });
|
||||
const texty = await createItem(fixture, request, coll.slug, `Texty ${stamp}`, { effort: '5' });
|
||||
const unset = await createItem(fixture, request, coll.slug, `Unset ${stamp}`, {});
|
||||
// The retype rewrites nothing: `effort` is still the STRING "5".
|
||||
await setSchema(fixture, request, coll.slug, { fields: [{ key: 'effort', label: 'Effort', type: 'number' }] });
|
||||
|
||||
await browserLogin(page);
|
||||
const base = `/${fixture.adminUsername}/${fixture.workspaceSlug}/${coll.slug}`;
|
||||
|
||||
// Unset first: an empty number field is NOT a mismatch.
|
||||
await page.goto(`${base}/${unset.slug}`);
|
||||
await expect(page.locator('.number-input').first()).toBeVisible();
|
||||
await expect(page.locator('.field-mismatch')).toHaveCount(0);
|
||||
|
||||
await page.goto(`${base}/${texty.slug}`);
|
||||
const marker = page.locator('.field-mismatch');
|
||||
await expect(marker).toBeVisible();
|
||||
await expect(marker).toContainText('"5"');
|
||||
await expect(marker).toContainText("Doesn't match the field type (number)");
|
||||
|
||||
await marker.getByRole('button', { name: 'Replace' }).click();
|
||||
const input = page.locator('.number-input').first();
|
||||
await expect(input).toHaveValue('');
|
||||
await input.fill('3');
|
||||
await input.blur();
|
||||
await expect.poll(() => storedEffort(fixture, request, texty.slug)).toBe(3);
|
||||
await expect(page.locator('.field-mismatch')).toHaveCount(0);
|
||||
} finally {
|
||||
await deleteCollection(fixture, request, coll.slug);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -377,3 +377,30 @@ describe('a text cell holding a value String() cannot convert (BUG-3052)', () =>
|
||||
expect(screen.container.textContent).toContain('plain');
|
||||
});
|
||||
});
|
||||
|
||||
// BUG-3052 unit 2: a cell whose value does not match its field's declared type
|
||||
// shows the raw stored text, marked, instead of text that reads as well-typed.
|
||||
describe('a cell whose value does not match its field type (BUG-3052 unit 2)', () => {
|
||||
it('shows the raw text, marked, and leaves well-typed cells alone', () => {
|
||||
const coll = collection([
|
||||
{ key: 'effort', label: 'Effort', type: 'number' },
|
||||
{ key: 'note', label: 'Note', type: 'text' },
|
||||
]);
|
||||
const screen = render(TableView, {
|
||||
props: {
|
||||
items: [item('car-1', { effort: '5', note: { a: 1 } }), item('car-2', { effort: 3, note: 'plain' })],
|
||||
collection: coll,
|
||||
} as never,
|
||||
});
|
||||
const marked = [...screen.container.querySelectorAll('.cell-mismatch')];
|
||||
expect(marked.map((el) => el.getAttribute('title'))).toEqual([
|
||||
"Doesn't match the field type (number)",
|
||||
"Doesn't match the field type (text)",
|
||||
]);
|
||||
// `"5"` is visibly not the number 5, and an object is not `[object Object]`.
|
||||
expect(marked[0].textContent).toContain('"5"');
|
||||
expect(marked[1].textContent).toContain('{"a":1}');
|
||||
expect(screen.container.textContent).not.toContain('[object Object]');
|
||||
expect(screen.container.textContent).toContain('plain');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
<script lang="ts">
|
||||
import { safeText } from '$lib/fields/fieldShape';
|
||||
import { rawText, readAs, safeText } from '$lib/fields/fieldShape';
|
||||
import type { Item, Collection, FieldDef } from '$lib/types';
|
||||
import { isRelationType, relationValuesOf } from '$lib/items/relationFieldTypes';
|
||||
import { narrowRelationRow, relationChipFor, UNRESOLVED_TITLE } from '$lib/collections/relationGroups';
|
||||
@@ -355,8 +355,17 @@
|
||||
{/if}
|
||||
{:else}
|
||||
<!-- safeText (BUG-3052): Svelte's text conversion threw on a
|
||||
stored `{"toString":0}` and took the table down. -->
|
||||
<span class="cell-value">{safeText(fields[field.key])}</span>
|
||||
stored `{"toString":0}` and took the table down. A value
|
||||
whose shape does not match its declared type (unit 2)
|
||||
shows its raw stored text, marked, as FieldEditor does:
|
||||
an object no longer reads `[object Object]`, and `"5"`
|
||||
under a number is visibly not the number 5. -->
|
||||
{@const cellShape = readAs(fields[field.key], field.type)}
|
||||
{#if cellShape.ok}
|
||||
<span class="cell-value">{safeText(fields[field.key])}</span>
|
||||
{:else}
|
||||
<span class="cell-value cell-mismatch" title="Doesn't match the field type ({field.type})">{rawText(fields[field.key])}<span class="sr-only"> (doesn't match the field type {field.type})</span></span>
|
||||
{/if}
|
||||
{/if}
|
||||
</div>
|
||||
{/each}
|
||||
@@ -546,6 +555,14 @@
|
||||
font-size: 0.9em;
|
||||
}
|
||||
|
||||
/* A value whose shape does not match its declared type (BUG-3052 unit 2):
|
||||
raw stored text, set apart so it does not read as a well-typed value. */
|
||||
.cell-mismatch {
|
||||
font-family: var(--font-mono);
|
||||
text-decoration: underline dotted var(--text-muted);
|
||||
cursor: help;
|
||||
}
|
||||
|
||||
/* Relation cells (BUG-3016). Same vocabulary as the properties chip, sized
|
||||
for a table cell: a single row that ellipsises rather than wrapping, so a
|
||||
long title cannot change the row's height. */
|
||||
|
||||
@@ -0,0 +1,144 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { render, cleanup } from '@testing-library/svelte';
|
||||
import { tick } from 'svelte';
|
||||
|
||||
// A real epoch behind a mock, so the fence the component captures is the one an
|
||||
// identity change actually moves.
|
||||
const auth = vi.hoisted(() => {
|
||||
let epoch = 0;
|
||||
return {
|
||||
identityFence() {
|
||||
const captured = epoch;
|
||||
return () => epoch === captured;
|
||||
},
|
||||
changeIdentity() {
|
||||
epoch++;
|
||||
},
|
||||
};
|
||||
});
|
||||
vi.mock('$lib/stores/auth.svelte', () => ({ authStore: auth }));
|
||||
|
||||
vi.mock('$lib/api/client', () => ({ api: { search: vi.fn(), items: { create: vi.fn() } } }));
|
||||
vi.mock('$lib/stores/localIndex.svelte', () => ({
|
||||
localIndex: {
|
||||
bootstrapStateFor: vi.fn(() => 'ready'),
|
||||
findByIdOrSlug: vi.fn(),
|
||||
getByCollection: vi.fn(() => []),
|
||||
cursorFor: vi.fn(),
|
||||
upsert: vi.fn(),
|
||||
scopeEpochFor: vi.fn(),
|
||||
pendingResyncFor: vi.fn(),
|
||||
resetGenerationFor: vi.fn(),
|
||||
},
|
||||
}));
|
||||
vi.mock('$lib/stores/localSearch.svelte', () => ({
|
||||
localSearch: { search: vi.fn(() => []), epoch: vi.fn(() => 0) },
|
||||
}));
|
||||
vi.mock('$lib/stores/collections.svelte', () => ({
|
||||
collectionStore: { collections: [], collectionsAreFreshFor: vi.fn(() => true) },
|
||||
}));
|
||||
vi.mock('$lib/stores/workspace.svelte', () => ({
|
||||
workspaceStore: { canEditCollection: vi.fn(() => true) },
|
||||
}));
|
||||
vi.mock('$lib/stores/toast.svelte', () => ({ toastStore: { show: vi.fn() } }));
|
||||
|
||||
|
||||
import FieldEditor from './FieldEditor.svelte';
|
||||
|
||||
/**
|
||||
* BUG-3052 unit 2: a stored value whose SHAPE does not match its declared type
|
||||
* is shown as its raw text with a note, never handed to the type's editor.
|
||||
* The ruling: an edit REPLACES it explicitly, never a silent coercion.
|
||||
*
|
||||
* The defects these legs pin, each of which a strict reading of the type
|
||||
* produced: the checkbox read `!!"false"` as CHECKED and its toggle wrote a
|
||||
* boolean over the string; the number step turned `"5"` into 6 without asking.
|
||||
*/
|
||||
|
||||
const flag = { key: 'blocked', label: 'Blocked', type: 'checkbox' as const };
|
||||
const effort = { key: 'effort', label: 'Effort', type: 'number' as const };
|
||||
const labels = { key: 'labels', label: 'Labels', type: 'multi_select' as const, options: ['a', 'b'] };
|
||||
|
||||
afterEach(() => cleanup());
|
||||
|
||||
const mismatch = (c: HTMLElement) => c.querySelector('.field-mismatch');
|
||||
const button = (c: HTMLElement, name: string) =>
|
||||
[...c.querySelectorAll('button')].find((b) => b.textContent?.trim() === name) as HTMLButtonElement | undefined;
|
||||
|
||||
describe('a value whose shape does not match its field (BUG-3052 unit 2)', () => {
|
||||
it('checkbox holding the string "false": shown as its raw text, no switch, nothing written', async () => {
|
||||
const onchange = vi.fn();
|
||||
const { container } = render(FieldEditor, { props: { field: flag, value: 'false', onchange, itemId: 'i1' } });
|
||||
expect(mismatch(container)?.textContent).toContain('"false"');
|
||||
expect(mismatch(container)?.textContent).toContain("Doesn't match the field type (checkbox)");
|
||||
// The switch that read `!!"false"` as checked is not offered at all.
|
||||
expect(container.querySelector('[role="switch"]')).toBeNull();
|
||||
expect(onchange).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('number holding "5": Replace opens the editor EMPTY, and only a step the user takes writes', async () => {
|
||||
const onchange = vi.fn();
|
||||
const { container } = render(FieldEditor, { props: { field: effort, value: '5', onchange, itemId: 'i1' } });
|
||||
expect(container.querySelector('.number-input')).toBeNull();
|
||||
button(container, 'Replace')!.click();
|
||||
await tick();
|
||||
const input = container.querySelector('.number-input') as HTMLInputElement;
|
||||
expect(input).not.toBeNull();
|
||||
expect(input.value).toBe('');
|
||||
expect(onchange).not.toHaveBeenCalled();
|
||||
(container.querySelector('[aria-label="Increase"]') as HTMLButtonElement).click();
|
||||
await tick();
|
||||
// Stepped from nothing, not from the string: never 6.
|
||||
expect(onchange.mock.calls).toEqual([[1]]);
|
||||
});
|
||||
|
||||
it('Keep the stored value backs out of a replace without writing', async () => {
|
||||
const onchange = vi.fn();
|
||||
const { container } = render(FieldEditor, { props: { field: effort, value: '5', onchange, itemId: 'i1' } });
|
||||
button(container, 'Replace')!.click();
|
||||
await tick();
|
||||
button(container, 'Keep the stored value')!.click();
|
||||
await tick();
|
||||
expect(mismatch(container)?.textContent).toContain('"5"');
|
||||
expect(onchange).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('a replace does not carry over to another item holding the same value', async () => {
|
||||
const { container, rerender } = render(FieldEditor, {
|
||||
props: { field: effort, value: '5', onchange: vi.fn(), itemId: 'i1' },
|
||||
});
|
||||
button(container, 'Replace')!.click();
|
||||
await tick();
|
||||
expect(mismatch(container)).toBeNull();
|
||||
await rerender({ field: effort, value: '5', onchange: vi.fn(), itemId: 'i2' });
|
||||
expect(mismatch(container)?.textContent).toContain('"5"');
|
||||
});
|
||||
|
||||
it('read-only: the same marker, and no Replace', () => {
|
||||
const { container } = render(FieldEditor, {
|
||||
props: { field: effort, value: { n: 5 }, onchange: vi.fn(), itemId: 'i1', readonly: true },
|
||||
});
|
||||
expect(mismatch(container)?.textContent).toContain('{"n":5}');
|
||||
expect(button(container, 'Replace')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('control: well-typed values render their editors, with no marker', () => {
|
||||
const a = render(FieldEditor, { props: { field: flag, value: false, onchange: vi.fn(), itemId: 'i1' } });
|
||||
expect(mismatch(a.container)).toBeNull();
|
||||
expect(a.container.querySelector('[role="switch"]')?.getAttribute('aria-checked')).toBe('false');
|
||||
cleanup();
|
||||
const b = render(FieldEditor, { props: { field: effort, value: 5, onchange: vi.fn(), itemId: 'i1' } });
|
||||
expect(mismatch(b.container)).toBeNull();
|
||||
expect((b.container.querySelector('.number-input') as HTMLInputElement).value).toBe('5');
|
||||
});
|
||||
});
|
||||
|
||||
describe('multi_select has no editor here yet (BUG-3052 unit 2)', () => {
|
||||
it('shows its values read-only with a hint, and no text input that would write a string', () => {
|
||||
const onchange = vi.fn();
|
||||
const { container } = render(FieldEditor, { props: { field: labels, value: ['a', 'b'], onchange, itemId: 'i1' } });
|
||||
expect(container.querySelector('input')).toBeNull();
|
||||
expect(container.textContent).toContain('a, b');
|
||||
expect(container.textContent).toContain("Can't be edited here yet");
|
||||
});
|
||||
});
|
||||
@@ -18,6 +18,7 @@ handlers — onchange is never called.
|
||||
import { onDestroy, tick } 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';
|
||||
import { narrowRelationRow, UNRESOLVED_LABEL, UNRESOLVED_TITLE } from '$lib/collections/relationGroups';
|
||||
import {
|
||||
isMultiRelationType,
|
||||
@@ -106,7 +107,28 @@ handlers — onchange is never called.
|
||||
itemId?: string;
|
||||
}
|
||||
|
||||
let { field, value, onchange, readonly = false, ariaLabel, wsSlug, username = '', onOpenTarget, itemId }: Props = $props();
|
||||
let { field, value: storedValue, onchange, readonly = false, ariaLabel, wsSlug, username = '', onOpenTarget, itemId }: Props = $props();
|
||||
|
||||
// ── Shape mismatch (BUG-3052 unit 2) ───────────────────────────────────
|
||||
//
|
||||
// A declared type is not a promise about the stored value: a retype
|
||||
// rewrites nothing. A value whose SHAPE does not match (`"5"` under a
|
||||
// number, `"false"` under a checkbox) is shown as its raw stored text with a
|
||||
// note, never fed to the type's editor: the checkbox read `!!"false"` as
|
||||
// checked and its toggle wrote a boolean over the string, and the number
|
||||
// step converted the string. The ruling: an edit REPLACES the value
|
||||
// explicitly, never a silent coercion. Replace opens the ordinary editor on
|
||||
// an EMPTY value; nothing is written until the user sets one.
|
||||
//
|
||||
// The replace state is KEYED on (item, field, stored value) rather than a
|
||||
// flag an effect resets, so switching items or a remote write ends it and
|
||||
// the next item cannot inherit it.
|
||||
let shape = $derived(readAs(storedValue, field.type));
|
||||
let mismatchKey = $derived(`${itemId ?? ''}\u0000${field.key}\u0000${rawText(storedValue)}`);
|
||||
let replacingKey = $state<string | null>(null);
|
||||
let replacing = $derived(!shape.ok && replacingKey === mismatchKey);
|
||||
/** What every editor branch below reads: the stored value, or nothing while replacing it. */
|
||||
let value = $derived(shape.ok ? storedValue : undefined);
|
||||
|
||||
// ── Relation resolution ───────────────────────────────────────────────
|
||||
//
|
||||
@@ -1308,7 +1330,16 @@ handlers — onchange is never called.
|
||||
{/if}
|
||||
{/snippet}
|
||||
|
||||
{#if readonly}
|
||||
{#if !shape.ok && !replacing}
|
||||
<div class="field-mismatch">
|
||||
<span class="mismatch-raw">{rawText(storedValue)}</span>
|
||||
<span class="mismatch-note">Doesn't match the field type ({field.type})</span>
|
||||
{#if !readonly}
|
||||
<button type="button" class="mismatch-action" onclick={() => (replacingKey = mismatchKey)}>Replace</button>
|
||||
{/if}
|
||||
</div>
|
||||
|
||||
{:else if readonly}
|
||||
<!--
|
||||
Display-only mode (PLAN-1100 / TASK-1105). No inputs, no dropdowns,
|
||||
no mutation handlers — value rendered with the same visual language
|
||||
@@ -1681,8 +1712,18 @@ handlers — onchange is never called.
|
||||
<span>{value === undefined || value === null ? '—' : JSON.stringify(value)}</span>
|
||||
</div>
|
||||
|
||||
{:else if field.type === 'multi_select'}
|
||||
<!-- No multi_select editor exists here yet (BUG-3052 unit 2). The text
|
||||
input this used to fall to wrote a STRING, which the server refuses
|
||||
for a multi_select, so the values are shown read-only and the hint
|
||||
says so rather than leaving a dead field. -->
|
||||
<div class="readonly-display" title="Multi-select values can't be edited here yet.">
|
||||
<span>{Array.isArray(value) && value.length ? value.join(', ') : '—'}</span>
|
||||
<span class="mismatch-note">Can't be edited here yet</span>
|
||||
</div>
|
||||
|
||||
{:else}
|
||||
<!-- Text / Multi-select fallback -->
|
||||
<!-- Text fallback -->
|
||||
<input
|
||||
class="field-input"
|
||||
type="text"
|
||||
@@ -1692,8 +1733,47 @@ handlers — onchange is never called.
|
||||
onblur={flushPendingSave}
|
||||
/>
|
||||
{/if}
|
||||
{#if replacing}
|
||||
<button type="button" class="mismatch-action" onclick={() => (replacingKey = null)}>Keep the stored value</button>
|
||||
{/if}
|
||||
|
||||
<style>
|
||||
/* ── Shape mismatch (BUG-3052 unit 2) ─────────────────────────────── */
|
||||
|
||||
.field-mismatch {
|
||||
display: flex;
|
||||
flex-wrap: wrap;
|
||||
align-items: center;
|
||||
gap: var(--space-1) var(--space-2);
|
||||
padding: var(--space-1) var(--space-2);
|
||||
min-height: 30px;
|
||||
font-size: 0.88em;
|
||||
}
|
||||
|
||||
.mismatch-raw {
|
||||
font-family: var(--font-mono);
|
||||
color: var(--text-primary);
|
||||
overflow-wrap: anywhere;
|
||||
}
|
||||
|
||||
.mismatch-note {
|
||||
font-size: 0.85em;
|
||||
color: var(--text-muted);
|
||||
}
|
||||
|
||||
.mismatch-action {
|
||||
font-size: 0.85em;
|
||||
color: var(--accent-blue);
|
||||
background: none;
|
||||
border: none;
|
||||
padding: 0;
|
||||
cursor: pointer;
|
||||
}
|
||||
|
||||
.mismatch-action:hover {
|
||||
text-decoration: underline;
|
||||
}
|
||||
|
||||
/* ── Readonly display ─────────────────────────────────────────────── */
|
||||
|
||||
.readonly-display {
|
||||
|
||||
@@ -130,9 +130,9 @@ user hunting for an item that provably does not exist.
|
||||
* Everything else is NOT rendered as a text box. `json` is deliberately
|
||||
* read-only in FieldEditor (a plain text input would store the string
|
||||
* "[]" where an array belongs), `relation` has no picker here, and
|
||||
* `multi_select` falls through FieldEditor's TEXT fallback — which yields a
|
||||
* string where the server's validation requires `[]any`. That last one is
|
||||
* the dangerous case: enterable and silently invalid. So a required field of
|
||||
* `multi_select` has no editor in FieldEditor (it fell through to the TEXT
|
||||
* fallback, a string where the server's validation requires `[]any`, until
|
||||
* BUG-3052 unit 2 made it read-only). So a required field of
|
||||
* any uncollectable type renders an explicit blocked state naming the field
|
||||
* and its type, rather than a dead Confirm or a lying input.
|
||||
*
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
// BUG-3052 unit 1: the read-side text conversion never throws.
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { fieldMatches, safeString, safeText } from './fieldShape';
|
||||
import { fieldMatches, rawText, readAs, safeString, safeText } from './fieldShape';
|
||||
import { laneValue } from '$lib/collections/boardColumns';
|
||||
|
||||
// A value JSON.parse happily produces, on which String() / `${}` / new Date() throw.
|
||||
@@ -86,3 +86,64 @@ describe('fieldMatches', () => {
|
||||
expect(fieldMatches('true', true)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// BUG-3052 unit 2: the shape half of the server's validateFieldType, asked of a read.
|
||||
describe('readAs', () => {
|
||||
const ok = (raw: unknown, t: string) => readAs(raw, t).ok;
|
||||
|
||||
it('accepts each type\'s own shape, and an absent value for any type', () => {
|
||||
expect(ok('x', 'text')).toBe(true);
|
||||
expect(ok('https://x', 'url')).toBe(true);
|
||||
expect(ok('open', 'select')).toBe(true);
|
||||
expect(ok('2026-09-26', 'date')).toBe(true);
|
||||
expect(ok('2026-09-26T10:00:00Z', 'date')).toBe(true);
|
||||
expect(ok('2026-09-26T23:59:59,5+05:30', 'date')).toBe(true);
|
||||
expect(ok('', 'date')).toBe(true);
|
||||
expect(ok(5, 'number')).toBe(true);
|
||||
expect(ok(0, 'number')).toBe(true);
|
||||
expect(ok(false, 'checkbox')).toBe(true);
|
||||
expect(ok(['a'], 'multi_select')).toBe(true);
|
||||
expect(ok([], 'multi_select')).toBe(true);
|
||||
for (const t of ['text', 'number', 'checkbox', 'date', 'multi_select']) {
|
||||
expect(ok(null, t), t).toBe(true);
|
||||
expect(ok(undefined, t), t).toBe(true);
|
||||
// ItemDetail passes an unset field as `''` (`fields[key] ?? ''`).
|
||||
expect(ok('', t), t).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('refuses what the server would refuse to write', () => {
|
||||
expect(ok(5, 'text')).toBe(false);
|
||||
expect(ok(['a'], 'select')).toBe(false);
|
||||
expect(ok('5', 'number')).toBe(false);
|
||||
expect(ok(Infinity, 'number')).toBe(false);
|
||||
expect(ok('false', 'checkbox')).toBe(false);
|
||||
expect(ok(0, 'checkbox')).toBe(false);
|
||||
|
||||
expect(ok(20260926, 'date')).toBe(false);
|
||||
expect(ok('a', 'multi_select')).toBe(false);
|
||||
expect(ok(['a', 1], 'multi_select')).toBe(false);
|
||||
expect(ok(HOSTILE, 'text')).toBe(false);
|
||||
});
|
||||
|
||||
it('judges a date by its JSON type only, not by whether it parses', () => {
|
||||
// The value half (a date that parses, a select value that is an option)
|
||||
// is the editor's to show; see readAs's comment for why.
|
||||
expect(ok('next week', 'date')).toBe(true);
|
||||
expect(ok('2026-99-99', 'date')).toBe(true);
|
||||
expect(ok(20260926, 'date')).toBe(false);
|
||||
});
|
||||
|
||||
it('does not judge json, relations or an unknown type', () => {
|
||||
for (const t of ['json', 'relation', 'multi_relation', 'mystery']) expect(ok({ a: 1 }, t), t).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('rawText', () => {
|
||||
it('is the stored JSON text, so a string shows quoted and an object shows its members', () => {
|
||||
expect(rawText('5')).toBe('"5"');
|
||||
expect(rawText({ a: 1 })).toBe('{"a":1}');
|
||||
expect(rawText(HOSTILE)).toBe('{"toString":0}');
|
||||
expect(rawText(undefined)).toBe('');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -25,8 +25,11 @@
|
||||
// - playbooks page trigger sort
|
||||
// - ItemDetail: children signature, computed-field text, imported-at title
|
||||
// - ChildChart start/end dates
|
||||
// Unit 2 — renders garbage / writes back a changed type (FieldEditor,
|
||||
// TableView cells): `readAs` + a mismatch marker; waits on a UX ruling.
|
||||
// Unit 2 — renders garbage / writes back a changed type, routed through
|
||||
// `readAs` + `rawText`: FieldEditor (read-only and editable; a mismatch is
|
||||
// shown raw with a note and replaced explicitly, never coerced), the
|
||||
// TableView plain cell, and the bulk-move undo (offered only for a string
|
||||
// status). multi_select has no FieldEditor editor and renders read-only.
|
||||
// Unit 3 — web misfilters, routed through `fieldMatches`: the collection
|
||||
// page field filter, ChildItems + NestedChildren done checks, the command
|
||||
// palette status chip, the playbooks trigger/scope filter, shareView
|
||||
@@ -96,3 +99,70 @@ export function fieldMatches(raw: unknown, wanted: unknown, declaredType?: strin
|
||||
}
|
||||
return safeText(raw) === want;
|
||||
}
|
||||
|
||||
/** The answer `readAs` gives: the value as its declared type, or the raw value. */
|
||||
export type ReadAs = { ok: true; value: unknown } | { ok: false; raw: unknown };
|
||||
|
||||
/**
|
||||
* Does a stored value have the SHAPE its declared type promises? (BUG-3052
|
||||
* unit 2.) The question the server's `validateFieldType`
|
||||
* (internal/items/validate.go) asks of a write, asked here of a read, so a value
|
||||
* this marks as mismatched is one a write of the same bytes would refuse:
|
||||
* text / url / select / date need a string, number a finite number, checkbox a
|
||||
* boolean, multi_select an array of strings.
|
||||
*
|
||||
* Only the JSON-TYPE half. Two value questions the server also asks are NOT
|
||||
* asked here: whether a select's string is one of its OPTIONS, and whether a
|
||||
* date's string PARSES as a date. The type is what an editor misreads into a
|
||||
* coercing write (`!!"false"`, a step from `"5"`); a well-typed string with a
|
||||
* bad value is shown by its own editor and changed only when the user picks a
|
||||
* new one. Mirroring the date half means mirroring Go's `time.Parse`, which
|
||||
* accepts a one-digit hour and a `+24:00` offset and refuses `+25:00`, so a
|
||||
* copy of it here would drift from it (BUG-3052 unit 2, review round 2). `json`, the
|
||||
* relation types and any type this module does not know are not judged.
|
||||
*
|
||||
* NO VALUE is `null`, `undefined` and `''`, for every type, the same set
|
||||
* `laneValue` treats as absent. `''` has to be in it: ItemDetail hands its
|
||||
* editors `fields[key] ?? ''`, so an unset number or checkbox ARRIVES as `''`,
|
||||
* and judging that as "not a number" marked every empty field in the pane.
|
||||
*/
|
||||
export function readAs(raw: unknown, declaredType: string): ReadAs {
|
||||
if (raw == null || raw === '') return { ok: true, value: raw };
|
||||
let ok: boolean;
|
||||
switch (declaredType) {
|
||||
case 'text':
|
||||
case 'url':
|
||||
case 'select':
|
||||
ok = typeof raw === 'string';
|
||||
break;
|
||||
case 'date':
|
||||
ok = typeof raw === 'string';
|
||||
break;
|
||||
case 'number':
|
||||
ok = typeof raw === 'number' && Number.isFinite(raw);
|
||||
break;
|
||||
case 'checkbox':
|
||||
ok = typeof raw === 'boolean';
|
||||
break;
|
||||
case 'multi_select':
|
||||
ok = Array.isArray(raw) && raw.every((el) => typeof el === 'string');
|
||||
break;
|
||||
default:
|
||||
ok = true;
|
||||
}
|
||||
return ok ? { ok: true, value: raw } : { ok: false, raw };
|
||||
}
|
||||
|
||||
/**
|
||||
* A stored value as its JSON text, for showing a value that does not match its
|
||||
* field: a string shows QUOTED (`"5"` is visibly not the number 5), an object
|
||||
* shows its members instead of `[object Object]`. TOTAL.
|
||||
*/
|
||||
export function rawText(raw: unknown): string {
|
||||
if (raw === undefined) return '';
|
||||
try {
|
||||
return JSON.stringify(raw) ?? safeText(raw);
|
||||
} catch {
|
||||
return safeText(raw);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,9 +7,9 @@ import { isRelationType } from '$lib/items/relationFieldTypes';
|
||||
*
|
||||
* Everything else is NOT rendered as a text box. `json` is deliberately
|
||||
* read-only in FieldEditor (a plain text input would store the string "[]"
|
||||
* where an array belongs), and `multi_select` falls through FieldEditor's TEXT
|
||||
* fallback, which yields a string where the server requires `[]any`. That last
|
||||
* one is the dangerous case: enterable and silently invalid.
|
||||
* where an array belongs), and `multi_select` has no editor in FieldEditor: it
|
||||
* used to fall through to the TEXT fallback, which yielded a string where the
|
||||
* server requires `[]any`, and since BUG-3052 unit 2 it renders read-only.
|
||||
*/
|
||||
export const COLLECTABLE_TYPES = new Set([
|
||||
'text',
|
||||
|
||||
@@ -3357,8 +3357,13 @@
|
||||
}
|
||||
function handleBulkMove(items: Item[], status: string) {
|
||||
const ws = wsSlug;
|
||||
// All lane items shared the lane's status; undo moves them back.
|
||||
const sourceStatus = items.length ? String(parseFields(items[0]).status ?? '') : '';
|
||||
// All lane items shared the lane's status; undo moves them back. Only a
|
||||
// STRING status is written back (BUG-3052 unit 2): the lane key is the
|
||||
// value's text, so moving a `5` back would store `"5"`, a silent change
|
||||
// of type, and a `{"toString":0}` made `String` throw. No undo is offered
|
||||
// for those rather than a coercing one.
|
||||
const rawSource = items.length ? parseFields(items[0]).status : undefined;
|
||||
const sourceStatus = typeof rawSource === 'string' ? rawSource : '';
|
||||
return runBulkOn(ws, { op: 'move', ids: idsOf(items), status }, 'Moved', {
|
||||
undo: sourceStatus
|
||||
? (okIds) => runBulkOn(ws, { op: 'move', ids: okIds, status: sourceStatus }, 'Moved back')
|
||||
|
||||
Reference in New Issue
Block a user