diff --git a/.changeset/lucky-forms-omit-server-defaults.md b/.changeset/lucky-forms-omit-server-defaults.md new file mode 100644 index 0000000000..2df9bd659d --- /dev/null +++ b/.changeset/lucky-forms-omit-server-defaults.md @@ -0,0 +1,19 @@ +--- +'@object-ui/console': patch +--- + +Console form pages no longer submit a cleared server-owned field as a blank. + +A field whose declared `defaultValue` is an instruction the server resolves per +insert (a `NOW()` / `current_user` token, or a CEL expression envelope) opens +with an empty control on a create form, and its key stays out of the payload +while nothing touches it. But a submitter who typed into that control and then +cleared it put the key back holding `''` — and `ObjectQL.applyFieldDefaults` +resolves a declared default only for a field arriving absent or null, so the +blank was stored and the declaration silently defeated. + +Such a key is now dropped from a CREATE submit on both the internal +`/forms/:name` and the anonymous `/f/:slug` route. A blank cleared from a field +with no runtime default — or with a static one — is still submitted, because +that is the user removing a value; and an edit submit is untouched, where a +cleared column is a deliberate removal. diff --git a/apps/console/src/components/FormPage.serverDefaults.test.tsx b/apps/console/src/components/FormPage.serverDefaults.test.tsx new file mode 100644 index 0000000000..d8fb2b596b --- /dev/null +++ b/apps/console/src/components/FormPage.serverDefaults.test.tsx @@ -0,0 +1,358 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#5883 — a CLEARED server-owned control must not put its key in the + * create payload. + * + * ## The mechanism these tests pin + * + * Since objectui#5727 this renderer opens a runtime-default control EMPTY and + * leaves the key out of `values` (`readPrefill` skips `isRuntimeDefault` + * defaults). But `handleSubmit` submitted `values` WHOLESALE, and a control the + * user touches writes back through `onChange` — so a user who types into such a + * control and then clears it puts the key back, holding `''`. + * + * `ObjectQL.applyFieldDefaults` resolves a declared default only for a field + * that arrives ABSENT or NULL. A blank string is neither, so submitting one + * stores `''` and silently defeats the declaration — the same suppression + * #5727 closed, reached by the other door. The sibling chain already names the + * pairing: `@object-ui/plugin-form`'s `omitServerResolvedDefaults` says + * "excusing a server-owned field from `required` is only half an answer if the + * form then submits the key anyway." + * + * ## What each arm of `FieldInput` actually writes when cleared + * + * Measured on this file's own controls rather than assumed, because the + * filter's notion of "empty" is the whole fix: + * + * - text / email / url / date / time / datetime / textarea / select → `''` + * - number / integer / decimal / currency → `null` (the arm spells + * `e.target.value === '' ? null : Number(...)`, so no `NaN` is reachable) + * - boolean / radio → nothing "cleared" exists; `false` and a picked option + * are real values, and `isMissingForRequired` deliberately does not treat + * `false` as absent. + * + * Both reachable spellings — `''` and `null` — are inside + * `isMissingForRequired`, which is exactly why the fix reads THAT predicate + * rather than testing for `''`. (A submitted `null` is one the engine would + * still have resolved; a submitted `''` is not. Dropping both keeps this + * renderer's notion of empty identical to the sibling's instead of inventing a + * narrower second one.) + * + * ## The counter-probes are the load-bearing half + * + * "The key is omitted" is satisfiable by omitting everything, so the positive + * assertions are worth nothing without these: + * + * - a field with NO runtime default that the user clears still submits `''` + * — clearing it is intent, and a filter that ate it would be a new defect; + * - a field the user typed a real value into submits that value; + * - an EDIT form submits the cleared key, because there the token was + * resolved at insert and a blank is a deliberate removal. + */ + +import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, waitFor, fireEvent } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { MemoryRouter, Route, Routes } from 'react-router-dom'; +import { FormPage } from './FormPage'; + +vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +/** + * One object carrying every shape the fix has to tell apart: + * + * - `title` — no default at all (counter-probe: typed value, and a cleared + * blank, both survive) + * - `status` — a STATIC literal default (counter-probe: seeded, and a clear + * is a real removal) + * - `owner` — a runtime TOKEN default on the text arm + * - `remind_at` — a runtime TOKEN default on the date arm + * - `priority` — a CEL envelope default on the number arm (writes `null`) + * - `stage` — a CEL envelope default on the select arm + */ +const OBJECT_SCHEMA = { + name: 'showcase_task', + label: 'Task', + fields: { + title: { type: 'text', label: 'Title' }, + status: { type: 'text', label: 'Status', defaultValue: 'draft' }, + owner: { type: 'user', label: 'Owner', defaultValue: 'current_user' }, + remind_at: { type: 'date', label: 'Remind At', defaultValue: 'NOW()' }, + priority: { + type: 'number', + label: 'Priority', + defaultValue: { dialect: 'cel', source: 'defaultPriority()' }, + }, + stage: { + type: 'select', + label: 'Stage', + options: [ + { value: 'new', label: 'New' }, + { value: 'done', label: 'Done' }, + ], + defaultValue: { dialect: 'cel', source: 'initialStage()' }, + }, + }, +}; + +const FIELD_NAMES = ['title', 'status', 'owner', 'remind_at', 'priority', 'stage']; + +function viewEnvelope() { + return { + name: 'showcase_task.edit', + object: 'showcase_task', + viewKind: 'form', + label: 'Task', + config: { type: 'simple', sections: [{ label: 'Task', fields: FIELD_NAMES }] }, + }; +} + +const CREATE_RESPONSE = { + object: 'showcase_task', + id: 'task-42', + record: { id: 'task-42' }, +}; + +/** The stored row an EDIT form opens on. */ +const STORED_RECORD = { + object: 'showcase_task', + id: 'task-7', + record: { + id: 'task-7', + title: 'Stored title', + status: 'active', + owner: 'user-1', + remind_at: '2026-01-02', + priority: 3, + stage: 'new', + }, +}; + +/** Every write this page makes, with its parsed body — the PAYLOAD, not state. */ +let writes: Array<{ url: string; method: string; body: Record }> = []; + +function stubFetch(routes: Record) { + return vi.fn(async (url: string, init?: RequestInit) => { + const method = init?.method ?? 'GET'; + if (method === 'POST' || method === 'PATCH') { + writes.push({ + url: String(url), + method, + body: init?.body ? JSON.parse(String(init.body)) : {}, + }); + } + const key = Object.keys(routes).find((k) => String(url).includes(k)); + if (!key) throw new Error(`unstubbed fetch: ${url}`); + return { + ok: true, + status: 200, + statusText: 'OK', + json: async () => routes[key], + text: async () => JSON.stringify(routes[key]), + } as unknown as Response; + }); +} + +function renderInternal(query = '') { + return render( + + + } /> + + , + ); +} + +function renderPublic() { + return render( + + + } /> + + , + ); +} + +const CREATE_ROUTES = { + '/meta/view/': viewEnvelope(), + '/meta/object/': OBJECT_SCHEMA, + '/data/showcase_task': CREATE_RESPONSE, +}; + +/** The single write this page made, as the server would receive it. */ +async function submittedPayload(): Promise> { + await waitFor(() => expect(writes).toHaveLength(1)); + return writes[0].body; +} + +beforeEach(() => { + writes = []; +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe('create submit — a touched-then-cleared server-owned field', () => { + it('omits the key entirely on the text arm, so the declared token still resolves', async () => { + vi.stubGlobal('fetch', stubFetch(CREATE_ROUTES)); + renderInternal(); + + const owner = await screen.findByLabelText(/Owner/); + // #5727 already guarantees the control opens EMPTY — the token is not + // seeded. The user is what puts the key back. + expect(owner).toHaveValue(''); + await userEvent.type(owner, 'someone'); + await userEvent.clear(owner); + + await userEvent.click(screen.getByRole('button', { name: /Submit/ })); + + const body = await submittedPayload(); + expect(Object.prototype.hasOwnProperty.call(body, 'owner')).toBe(false); + }); + + it('omits it on the date arm too', async () => { + vi.stubGlobal('fetch', stubFetch(CREATE_ROUTES)); + renderInternal(); + + const remind = await screen.findByLabelText(/Remind At/); + // `fireEvent.change` rather than `userEvent.type` on the date/number/select + // arms: what is under test is the `onChange` write itself, and a date input + // is exactly where synthetic keystroke emulation is least faithful. + fireEvent.change(remind, { target: { value: '2026-03-04' } }); + fireEvent.change(remind, { target: { value: '' } }); + + await userEvent.click(screen.getByRole('button', { name: /Submit/ })); + + const body = await submittedPayload(); + expect(Object.prototype.hasOwnProperty.call(body, 'remind_at')).toBe(false); + }); + + it('omits it on the number arm, whose cleared write is `null` rather than a blank string', async () => { + vi.stubGlobal('fetch', stubFetch(CREATE_ROUTES)); + renderInternal(); + + const priority = await screen.findByLabelText(/Priority/); + fireEvent.change(priority, { target: { value: '7' } }); + fireEvent.change(priority, { target: { value: '' } }); + + await userEvent.click(screen.getByRole('button', { name: /Submit/ })); + + const body = await submittedPayload(); + expect(Object.prototype.hasOwnProperty.call(body, 'priority')).toBe(false); + }); + + it('omits it on the select arm, cleared back to the placeholder option', async () => { + vi.stubGlobal('fetch', stubFetch(CREATE_ROUTES)); + renderInternal(); + + const stage = await screen.findByLabelText(/Stage/); + await userEvent.selectOptions(stage, 'done'); + await userEvent.selectOptions(stage, ''); + + await userEvent.click(screen.getByRole('button', { name: /Submit/ })); + + const body = await submittedPayload(); + expect(Object.prototype.hasOwnProperty.call(body, 'stage')).toBe(false); + }); + + it('does the same on the anonymous public route', async () => { + vi.stubGlobal( + 'fetch', + stubFetch({ + '/forms/contact-us/submit': { ok: true }, + '/forms/contact-us': { + slug: 'contact-us', + object: 'showcase_task', + label: 'Contact us', + form: { type: 'simple', sections: [{ fields: FIELD_NAMES }] }, + objectSchema: OBJECT_SCHEMA, + }, + }), + ); + renderPublic(); + + const owner = await screen.findByLabelText(/Owner/); + await userEvent.type(owner, 'someone'); + await userEvent.clear(owner); + + await userEvent.click(screen.getByRole('button', { name: /Submit/ })); + + const body = await submittedPayload(); + expect(Object.prototype.hasOwnProperty.call(body, 'owner')).toBe(false); + }); +}); + +describe('counter-probes — what the filter must NOT eat', () => { + it('still submits a blank the user cleared from a field with no runtime default', async () => { + vi.stubGlobal('fetch', stubFetch(CREATE_ROUTES)); + renderInternal(); + + const title = await screen.findByLabelText(/Title/); + await userEvent.type(title, 'draft idea'); + await userEvent.clear(title); + + await userEvent.click(screen.getByRole('button', { name: /Submit/ })); + + const body = await submittedPayload(); + expect(Object.prototype.hasOwnProperty.call(body, 'title')).toBe(true); + expect(body.title).toBe(''); + }); + + it('still submits a blank the user cleared from a STATIC default, which is a real removal', async () => { + vi.stubGlobal('fetch', stubFetch(CREATE_ROUTES)); + renderInternal(); + + const status = await screen.findByLabelText(/Status/); + // The static default IS seeded (#4068), so clearing it removes a value the + // user could see. + expect(status).toHaveValue('draft'); + await userEvent.clear(status); + + await userEvent.click(screen.getByRole('button', { name: /Submit/ })); + + const body = await submittedPayload(); + expect(Object.prototype.hasOwnProperty.call(body, 'status')).toBe(true); + expect(body.status).toBe(''); + }); + + it('submits the real value a user typed into a server-owned field', async () => { + vi.stubGlobal('fetch', stubFetch(CREATE_ROUTES)); + renderInternal(); + + const owner = await screen.findByLabelText(/Owner/); + await userEvent.type(owner, 'user-99'); + + await userEvent.click(screen.getByRole('button', { name: /Submit/ })); + + const body = await submittedPayload(); + expect(body.owner).toBe('user-99'); + // And the rest of the payload is still there — "omit the key" must not + // become "drop the payload". + expect(body.status).toBe('draft'); + }); + + it('leaves an EDIT submit alone: a cleared server-owned column is a deliberate removal', async () => { + vi.stubGlobal( + 'fetch', + stubFetch({ + '/meta/view/': viewEnvelope(), + '/meta/object/': OBJECT_SCHEMA, + '/data/showcase_task/task-7': STORED_RECORD, + }), + ); + renderInternal('?recordId=task-7&recordObject=showcase_task'); + + const owner = await screen.findByLabelText(/Owner/); + expect(owner).toHaveValue('user-1'); + await userEvent.clear(owner); + + await userEvent.click(screen.getByRole('button', { name: /Submit/ })); + + const body = await submittedPayload(); + expect(writes[0].method).toBe('PATCH'); + expect(Object.prototype.hasOwnProperty.call(body, 'owner')).toBe(true); + expect(body.owner).toBe(''); + }); +}); diff --git a/apps/console/src/components/FormPage.tsx b/apps/console/src/components/FormPage.tsx index 9919d1e172..b2b447d6d4 100644 --- a/apps/console/src/components/FormPage.tsx +++ b/apps/console/src/components/FormPage.tsx @@ -91,6 +91,16 @@ * values, exactly as a hidden field's do (triage ruling, 2026-08-22, following * #5594 and the plugin-form chain). Nothing in this file makes visibility a * submit-payload rule, at either granularity. + * + * ## The one thing that IS a submit-payload rule (objectui#5883) + * + * A field whose declared `defaultValue` is an instruction the SERVER resolves + * per insert is left out of a create payload when the control is empty — see + * {@link omitServerOwnedBlanks}. That is not a visibility rule wearing a + * different hat: the key is withheld precisely so the producer supplies the + * value, which is the whole meaning of the declaration. It is the submit half + * of the fence {@link readPrefill} put on seeding for objectui#5727, and the + * sibling of `@object-ui/plugin-form`'s `omitServerResolvedDefaults`. */ import { useEffect, useMemo, useState, type FormEvent } from 'react'; @@ -98,6 +108,7 @@ import { useNavigate, useParams, useSearchParams } from 'react-router-dom'; import { toast } from 'sonner'; import { evalFieldPredicate, + isMissingForRequired, isRuntimeDefault, isServerOwnedValue, resolveFieldRuleState, @@ -971,6 +982,93 @@ export function readPrefill( return out; } +/** + * Drop the keys a CREATE payload must leave to the producer (objectui#5883). + * + * ## The other half of {@link readPrefill}'s #5727 fence + * + * #5727 stopped this renderer from SEEDING a runtime default, so such a control + * opens empty and its key never enters `values` from the declaration. That is + * the half that handles a form nobody touched. This is the half that handles a + * key the USER put there: a rendered control writes back through its `onChange` + * the moment it is touched, so a submitter who types into a server-owned field + * and then clears it puts the key back — holding a blank. + * + * `ObjectQL.applyFieldDefaults` resolves a declared default only for a field + * that arrives ABSENT or NULL. A blank string is neither, so submitting one + * stores `''` and silently defeats the declaration — the same suppression + * #5727 closed, reached by the other door. Omitting the key is what makes the + * server the single authority for the value. + * + * ## What "empty" means here, and why it is not spelled out + * + * The arms of {@link FieldInput} do not agree on what a cleared control writes, + * and the filter's notion of empty is the whole fix. Measured on this + * renderer's own controls: the text / email / url / date / time / datetime / + * textarea / select arms write `''`; the number family writes `null` (the arm + * spells `e.target.value === '' ? null : Number(...)`, so no `NaN` is + * reachable); the boolean and radio arms have no "cleared" state at all — + * `false` and a picked option are real values. A filter testing for `''` would + * therefore have left the number arm behind. + * + * So emptiness is `@object-ui/core`'s {@link isMissingForRequired} — the same + * PRESENCE predicate the `required` rule reads, which covers `undefined`, + * `null`, a blank string and an empty array while deliberately keeping `false` + * and `0` as values. "Left empty" cannot come to mean two different things in + * the two halves of this fix, and it cannot come to mean something narrower + * here than it does on the sibling chain. + * + * ## Why the two predicates rather than the sibling function + * + * `@object-ui/plugin-form`'s `omitServerResolvedDefaults` + * (`src/schemaDefaults.ts`) is this rule on the OTHER form chain and states the + * pairing outright — "excusing a server-owned field from `required` is only + * half an answer if the form then submits the key anyway". Calling it from here + * is not available: that package's `exports` map publishes `.` alone, and its + * root (`src/index.tsx`) does not re-export `schemaDefaults`, so the function + * has no spelling a consumer can import. + * + * What matters is that the CLASSIFIERS are not copied, and they are not: both + * are imported from `@object-ui/core`, which is where they live precisely so + * every layer that decides server-ownership reads one answer + * (`validation/server-owned-value.ts` says so, and lists this renderer's + * `resolveFieldRuleState` among the consumers). `omitServerResolvedDefaults` is + * itself only such a local application of the same two calls. This file already + * reads `isRuntimeDefault` directly for #5727's seeding fence, and + * `plugin-kanban` and `@object-ui/components`' form renderer read + * `isMissingForRequired` the same way. Publishing the sibling helper from + * plugin-form's root would let this call site shrink to one line; that is a + * change to another package's published surface, not to this one. + * + * ## The two boundaries + * + * - **CREATE only.** On an edit form the token was resolved at insert, so a + * cleared column is a deliberate removal and dropping the key would silently + * discard the user's edit. `isCreateForm` is the caller's fact, read off the + * same URL switch `resolveRowState` uses. + * - **Runtime defaults only.** A field with no default, or with a STATIC one + * (which #4068 says IS seeded into the control), keeps its blank: the user + * cleared something that was there, or is expressing "leave this empty", and + * either way that `''` is intent the form must carry. + */ +export function omitServerOwnedBlanks( + values: Record, + fields: RenderableField[], + isCreateForm: boolean, +): Record { + if (!isCreateForm) return values; + const serverOwned = new Set( + fields.filter((f) => isRuntimeDefault(f.defaultValue)).map((f) => f.name), + ); + if (serverOwned.size === 0) return values; + const out: Record = {}; + for (const [key, value] of Object.entries(values)) { + if (serverOwned.has(key) && isMissingForRequired(value)) continue; + out[key] = value; + } + return out; +} + /** Authed/anonymous fetch — credentials included so cookies (auth) flow. */ async function apiFetch(path: string, init?: RequestInit): Promise { return fetch(`${API_BASE}${path}`, { @@ -1534,10 +1632,19 @@ export function FormPage({ mode, recordPath }: FormPageProps) { setSubmitting(true); setError(null); try { + // A server-owned field the user touched and cleared must not travel as a + // blank — that stores `''` and defeats the declaration the empty control + // was honouring (objectui#5883). Both routes, because `/f/:slug` is a + // create by construction and its payload reaches the same insert path. + const payload = omitServerOwnedBlanks( + values, + sections.flatMap((s) => s.fields), + isCreateForm, + ); const result = mode === 'public' - ? await submitPublic(identifier, values) - : await submitInternal(loaded.object, values, editingId); + ? await submitPublic(identifier, payload) + : await submitInternal(loaded.object, payload, editingId); toast.success('Submitted'); // Behaviour after submit switch (behavior.kind) { @@ -1582,7 +1689,13 @@ export function FormPage({ mode, recordPath }: FormPageProps) { const written = unwrapTransportEnvelope(result)?.record; const writtenId = editingId ?? readCreatedRecordId(result); const verdict = resolveSubmitRedirect(behavior.url, { - ...values, + // `payload`, not `values` — "the values as SUBMITTED" is what this + // scope has always claimed to be, and since objectui#5883 the two + // differ: a key the submit deliberately withheld is not part of the + // record it wrote, so resolving `{{record.x}}` from it would put + // the very blank we refused to send into a URL. The server echo + // below still supplies the value the producer resolved. + ...payload, ...(written && typeof written === 'object' ? (written as Record) : {}), ...(writtenId ? { id: writtenId } : {}), });