diff --git a/clients/tui/__tests__/schemaToForm.test.ts b/clients/tui/__tests__/schemaToForm.test.ts index 1ced5357..521ba528 100644 --- a/clients/tui/__tests__/schemaToForm.test.ts +++ b/clients/tui/__tests__/schemaToForm.test.ts @@ -465,6 +465,35 @@ describe("schemaToForm", () => { ]); }); + it("labels a branch field by the name the schema declared", () => { + // The prefix is an internal field name; `buildFields` falls back to its + // map key for a label, so the user would otherwise see `__b0__address`. + const form = schemaToForm(UNION, "union_tool"); + expect(form.sections[1]!.fields[1]).toMatchObject({ + name: "__b0__address", + label: "address", + }); + }); + + it("keeps a declared title ahead of the fallback", () => { + const form = schemaToForm( + { + type: "object", + anyOf: [ + { + type: "object", + properties: { a: { type: "string", title: "Street address" } }, + }, + { type: "object", properties: { b: { type: "string" } } }, + ], + }, + "titled", + ); + expect(form.sections[1]!.fields[0]).toMatchObject({ + label: "Street address", + }); + }); + it("offers a variant select listing the alternatives", () => { const form = schemaToForm(UNION, "union_tool"); expect(form.sections[0]!.fields[0]).toMatchObject({ diff --git a/clients/tui/src/utils/schemaToForm.ts b/clients/tui/src/utils/schemaToForm.ts index 4eb4fc9f..1c276dbd 100644 --- a/clients/tui/src/utils/schemaToForm.ts +++ b/clients/tui/src/utils/schemaToForm.ts @@ -143,6 +143,19 @@ function branchFields( return [...new Set([...own, ...sharedFieldNames(base, branches)])]; } +/** + * A property declaration carrying the name it was declared under as its + * fallback `title`, so a renamed field still displays the schema's own name. + */ +function labelled(property: unknown, name: string): unknown { + if (typeof property !== "object" || property === null) { + // JSON Schema's `true` constrains nothing, so a title-only object says the + // same thing and carries the label. + return { title: name }; + } + return { title: name, ...property }; +} + /** The `const` a property schema pins its value to, if any. */ function constOf(schema: unknown): unknown { if (typeof schema !== "object" || schema === null) return undefined; @@ -217,7 +230,11 @@ export function schemaToForm( const properties = Object.fromEntries( branchFields(base, branches, index).map((name) => [ branchFieldName(prefix, index, name), - branch.schema.properties?.[name], + // The prefix is an internal field NAME, never a label: `buildFields` + // falls back to its map key when a declaration carries no `title`, so + // without this the form would show `__b0__address` where the schema + // says `address`. + labelled(branch.schema.properties?.[name], name), ]), ); sections.push({ diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx index 9aa55d23..66a529bb 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx @@ -2391,6 +2391,43 @@ describe("SchemaForm multiline strings (#2042)", () => { expect(screen.getByRole("textbox", { name: /Address/ })).toBeTruthy(); }); + it("corrects a stale const when the schema changes in place", async () => { + // The user cannot edit a read-only field, so a `const` that moves under + // an unchanged `resetKey` would display the new value while the old one + // sat in `values`, waiting to be submitted. + const onChange = vi.fn(); + const pinned = (value: string): InspectorFormSchema => ({ + type: "object", + properties: { + kind: { type: "string", const: value }, + note: { type: "string", title: "Note" }, + }, + }); + const { rerender } = renderWithMantine( + , + ); + await Promise.resolve(); + onChange.mockClear(); + + rerender( + , + ); + // The constant is corrected; what the user typed is left alone. + await waitFor(() => + expect(onChange).toHaveBeenCalledWith({ kind: "sms", note: "kept" }), + ); + }); + it("renders no picker for a single-branch union but still shows its fields", () => { const schema: InspectorFormSchema = { type: "object", diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx index 24d978db..c62641e0 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx @@ -36,7 +36,10 @@ import { resolveRootUnion, selectBranchIndex, } from "@inspector/core/json/rootUnion.js"; -import { collectSchemaDefaults } from "../../../utils/jsonUtils"; +import { + applySchemaConstants, + collectSchemaDefaults, +} from "../../../utils/jsonUtils"; const FieldLabel = Text.withProps({ fw: 500, @@ -751,9 +754,20 @@ export function SchemaForm({ const missing = Object.entries(collectSchemaDefaults(current)).filter( ([name]) => !Object.hasOwn(held, name), ); - if (missing.length > 0) { - report({ ...held, ...Object.fromEntries(missing) }); - } + // Constants are re-applied as well as seeded: an in-place schema change can + // move a `const` the user cannot edit — the read-only field then displays + // the new value while `values` still holds the old one, which is what would + // be submitted. Ordinary defaults are only ever *added*, so an edited field + // keeps what the user put there. + const next = applySchemaConstants( + current, + missing.length > 0 ? { ...held, ...Object.fromEntries(missing) } : held, + ); + const changed = Object.keys(next).some( + (name) => + !Object.hasOwn(held, name) || !Object.is(next[name], held[name]), + ); + if (changed) report(next); // Keyed by the entity and branch being edited, and by the fixed values // themselves: a tool refreshed in place keeps its `resetKey` and its branch // while its schema changes underneath, and a newly pinned field would diff --git a/clients/web/src/utils/jsonUtils.test.ts b/clients/web/src/utils/jsonUtils.test.ts index 62d0e123..10d7870b 100644 --- a/clients/web/src/utils/jsonUtils.test.ts +++ b/clients/web/src/utils/jsonUtils.test.ts @@ -601,6 +601,20 @@ describe("root composition (#2123)", () => { ).toBe(true); }); + it("does not read an inherited property as a supplied answer", () => { + // `constructor` is a legal argument name; reading it off the prototype + // would enable a submit the schema rejects. + const schema: InspectorFormSchema = { + type: "object", + // Built through `fromEntries`: in an object literal, `constructor` is + // TypeScript's own inherited member rather than a plain key. + properties: Object.fromEntries([["constructor", { type: "string" }]]), + required: ["constructor"], + }; + expect(hasMissingRequiredFields(schema, {})).toBe(true); + expect(hasMissingRequiredFields(schema, { constructor: "x" })).toBe(false); + }); + it("blocks submission while no branch is satisfied", () => { expect(hasMissingRequiredFields(UNION, {})).toBe(true); expect(hasMissingRequiredFields(UNION, { kind: "email" })).toBe(true); diff --git a/clients/web/src/utils/jsonUtils.ts b/clients/web/src/utils/jsonUtils.ts index 5293c697..f2f9a56c 100644 --- a/clients/web/src/utils/jsonUtils.ts +++ b/clients/web/src/utils/jsonUtils.ts @@ -352,6 +352,10 @@ function hasMissingIn( const required = Array.isArray(schema.required) ? schema.required : []; const properties = schema.properties ?? {}; return required.some((field) => { + // `hasOwn` first: an argument legally named `constructor` would otherwise + // resolve to the inherited one and read as supplied, enabling a submit the + // schema rejects. + if (!Object.hasOwn(values, field)) return true; const value = values[field]; if (value === null) { const fieldSchema = properties[field];