From 27c53b7ec76d892d6b775c036c3dd7171436fb8a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 16:14:15 +0000 Subject: [PATCH 1/2] fix(fields): validate a stored `location` value on an edit form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `buildValidationRules` is the producer of the host-side `error` prop that every field widget's published objectui#3222 slot reads, and it had no branch for `location`. A coordinate already in the record that violated the spec's range was therefore never validated on an edit form: the control rendered it, nothing marked it invalid, and submitting re-wrote it unchanged. It now compiles a `validate.location` entry that adjudicates a PRESENT value against `valueSchemaFor(field, 'stored')` — the platform's own value-shape contract (ADR-0104 D1), the same schema the engine's record validator checks a stored `location` against. The bounds are not restated in objectui and the message is built from the schema's own issues, so neither can drift from the spec (AGENTS.md #0.1). Absence stays `required`'s business: the spec's schema describes a present value and refuses null/undefined outright, so the rule asks core's `isMissingForRequired` rather than inventing a second definition of "empty". A field-authored `validate` composes under its own key instead of being replaced. Landed on a hard precondition from the maintainer ruling of 2026-08-29: 28 stored location values across every measurable dataset (app-showcase seed, qa dogfood field-zoo matrix, objectui schema-catalog) adjudicated by that same schema — 28 accepted, 0 refused. That zero means "zero within measurable scope"; customer deployments are not measurable here. The two sibling pins that asserted the branch's ABSENCE are rewritten to assert the property that actually survives: a refusal produces no value, so the host rule is handed `undefined` and the input-time announcement stays the widget's (objectui#6714/#6716 unchanged). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- .../6744-location-stored-value-validation.md | 38 +++ packages/fields/src/index.tsx | 116 ++++++- .../src/ObjectForm.locationRange.test.tsx | 16 +- .../src/ObjectForm.locationRefusal.test.tsx | 38 ++- .../src/ObjectForm.locationResidue.test.tsx | 31 +- .../ObjectForm.locationStoredRange.test.tsx | 302 ++++++++++++++++++ 6 files changed, 508 insertions(+), 33 deletions(-) create mode 100644 .changeset/6744-location-stored-value-validation.md create mode 100644 packages/plugin-form/src/ObjectForm.locationStoredRange.test.tsx diff --git a/.changeset/6744-location-stored-value-validation.md b/.changeset/6744-location-stored-value-validation.md new file mode 100644 index 0000000000..7f4d24f4ed --- /dev/null +++ b/.changeset/6744-location-stored-value-validation.md @@ -0,0 +1,38 @@ +--- +'@object-ui/fields': minor +--- + +fields: validate a STORED `location` value on an edit form (objectui#6744) + +`buildValidationRules` is the producer of the host-side `error` prop that every +field widget's published objectui#3222 slot reads, and it had no branch for +`location`. So a coordinate that was **already in the record** and violated the +spec's range was never validated on an edit form: the control rendered it, +nothing marked it invalid, and submitting re-wrote it unchanged. + +It now compiles a `validate.location` entry that adjudicates a present value +against `valueSchemaFor(field, 'stored')` — the platform's own value-shape +contract (ADR-0104 D1), the same schema the engine's record validator checks a +stored `location` against. An out-of-range stored value now marks the control +invalid, renders the spec's own complaint, and blocks the write; a legal value +is untouched. + +⛔ The bounds are not restated in objectui. A hand-copied range would be a second +contract free to drift from the spec (AGENTS.md #0.1), so the schema is asked and +the message is built from its issues — the same discipline `LocationField`'s own +range refusal already follows. + +Deliberately unchanged: + +- **Input-time refusal (objectui#6714/#6716) is still the widget's.** A refusal + means `onChange` never fires, so the typed text never becomes a form value and + this rule is handed `undefined`. The two do not overlap. +- **Absence is `required`'s business.** The spec's schema refuses `null` and + `undefined` outright because it describes a *present* value, so the rule asks + core's `isMissingForRequired` — the repo's single presence contract — rather + than inventing a second definition of "empty". A create form with an untouched + location field is unaffected. +- **A field-authored `validate` keeps running**, composed under its own key + rather than replaced. +- **Scope is `location` only.** Whether other field types have the same + stored-value gap is a separate question and was not surveyed here. diff --git a/packages/fields/src/index.tsx b/packages/fields/src/index.tsx index 57a82bd65f..1ca938326d 100644 --- a/packages/fields/src/index.tsx +++ b/packages/fields/src/index.tsx @@ -8,7 +8,11 @@ import React from 'react'; import type { FieldMetadata, SelectOptionMetadata } from '@object-ui/types'; -import { ComponentRegistry, percentDisplayValue, getRecordDisplayName, humanizeLabel, type ComponentMeta } from '@object-ui/core'; +import { ComponentRegistry, percentDisplayValue, getRecordDisplayName, humanizeLabel, isMissingForRequired, type ComponentMeta } from '@object-ui/core'; +// The platform's own value-shape contract, asked rather than restated +// (objectui#6744). See `locationStoredValueSchemaFor` below for why this is a +// runtime import in the barrel and not a hand-written coordinate range. +import { valueSchemaFor } from '@objectstack/spec/data'; import { useLocalization, useDisplayLocale, formatDisplayNumber } from '@object-ui/i18n'; import { Badge, Avatar, AvatarImage, AvatarFallback, Button, Checkbox, EmptyValue, cn } from '@object-ui/components'; import { Check, X, Copy, Phone as PhoneIcon, MapPin } from 'lucide-react'; @@ -2543,6 +2547,96 @@ export function formatFileSize(bytes: number): string { return `${size.toFixed(unitIndex > 0 ? 1 : 0)} ${units[unitIndex]}`; } +/** + * Per-field-definition cache of the spec's derived value schema + * (objectui#6744). + * + * `valueSchemaFor` states the requirement itself: "Pure derivation — no caching + * here; runtime consumers MUST cache per field definition (building a + * `z.object` per write is an order of magnitude costlier than parsing)." This + * is the same `WeakMap`-on-the-field-def idiom the platform's own write-path + * validator uses (`shapeSchemaFor`, + * `packages/objectql/src/validation/record-validator.ts`), so the hit rate is + * governed by the same condition there and here: whether the host hands the + * same field object back. + * + * ⛔ The def is passed through VERBATIM, never rebuilt from keys picked out of + * it. `valueSchemaFor` decides for itself what a location value is and how a + * field's multiplicity participates; a reconstructed def would be a second + * reading of the contract, free to drift from the first (AGENTS.md #0.1). + */ +const locationStoredValueSchemas = new WeakMap>(); + +function locationStoredValueSchemaFor(field: any): ReturnType { + let schema = locationStoredValueSchemas.get(field); + if (!schema) { + schema = valueSchemaFor(field, 'stored'); + locationStoredValueSchemas.set(field, schema); + } + return schema; +} + +/** + * The STORED-value rule for a `type: 'location'` field (objectui#6744). + * + * ## What was missing + * + * `buildValidationRules` is the producer of the host-side `error` prop that + * every field widget's published objectui#3222 slot reads, and it had no + * `location` branch. So a coordinate that is ALREADY IN THE RECORD and violates + * the spec's range was never validated on an edit form: the control rendered + * it, nothing marked it invalid, and submitting re-wrote it unchanged. + * + * ⚠️ This is a different defect from objectui#6714/#6716, which are about a + * refusal at INPUT time — the user types something the widget will not emit. A + * refusal never becomes a form value, so this rule is handed `undefined` in + * both of those arms; the two do not overlap and neither replaces the other. + * + * ## Why the whole value is handed to the spec, not a range test + * + * ⛔ The bounds are NOT restated here as `-90`/`90` literals — the same + * discipline `LocationField`'s own `isSpecAcceptedLocation` follows, and for the + * same reason: a hand-copied range is a SECOND contract that drifts from the + * spec silently (AGENTS.md #0.1). + * + * Asking `valueSchemaFor(field, 'stored')` also makes this rule agree, by + * construction, with the platform's own write path. The engine's record + * validator checks a stored `location` against THIS schema (ADR-0104 D1, + * `record-validator.ts`), warn-first until a deployment's `os migrate + * value-shapes` scan certifies zero violations and rejecting afterwards. So a + * value this rule marks invalid is a value the platform itself already refuses + * or has recorded as an admitted violation — the form now surfaces that + * verdict where the person who can correct it is standing, rather than + * inventing a verdict of its own. + * + * ## Absence is `required`'s business, not this rule's + * + * The spec's schema describes a PRESENT value — it refuses `null` and + * `undefined` outright, and its docblock says so ("null/undefined/required + * handling stays with the caller"). Deciding presence HERE would be a second + * definition of "empty" competing with the one `required` uses, so this asks + * core's `isMissingForRequired` — the repo's single presence contract, the same + * predicate the form renderer's own `required` validator calls. That is what + * keeps a CREATE form with an untouched location field valid. + * + * ## The message is the spec's own complaint + * + * Built from the schema's issues, exactly as `LocationField`'s + * `refusedRangeMessage` builds the widget-side sentence, so the day the schema + * moves both sentences move with it and neither can quote a stale bound. + */ +function buildLocationStoredValueValidator(field: any): (value: unknown) => true | string { + return (value: unknown) => { + if (isMissingForRequired(value)) return true; + const parsed = locationStoredValueSchemaFor(field).safeParse(value); + if (parsed.success) return true; + const detail = parsed.error.issues + .map((issue: any) => `${issue.path.join('.') || 'value'}: ${issue.message}`) + .join('; '); + return `Invalid location: ${detail}`; + }; +} + /** * Build validation rules from field metadata * @param field - Field metadata from ObjectStack @@ -2638,6 +2732,26 @@ export function buildValidationRules(field: any): any { rules.validate = field.validate; } + // Stored-value validation for `location` (objectui#6744). See + // `buildLocationStoredValueValidator` for what this rule is and what it is + // deliberately not. + // + // Emitted in react-hook-form's OBJECT form so a field-authored `validate` + // keeps running under its own key instead of being replaced — the same + // normalisation the form renderer applies when it adds its `required` entry + // (`packages/components/src/renderers/form/form.tsx`), spelled the same way + // so the two compose rather than clobber. RHF reports an object-form failure + // under its key, so this surfaces as `type: 'location'`. + if (field.type === 'location') { + const authoredValidate = rules.validate; + rules.validate = { + ...(typeof authoredValidate === 'function' + ? { validate: authoredValidate } + : (authoredValidate ?? {})), + location: buildLocationStoredValueValidator(field), + }; + } + return Object.keys(rules).length > 0 ? rules : undefined; } diff --git a/packages/plugin-form/src/ObjectForm.locationRange.test.tsx b/packages/plugin-form/src/ObjectForm.locationRange.test.tsx index 4ab2923c41..4ba9b184ce 100644 --- a/packages/plugin-form/src/ObjectForm.locationRange.test.tsx +++ b/packages/plugin-form/src/ObjectForm.locationRange.test.tsx @@ -29,10 +29,18 @@ * ⚠️ The pass-through assertion below is load-bearing, not decoration. The form * hands the widget's emission to `dataSource.create` UNCHANGED — * `sanitizeFormData` filters KEYS (server-managed, computed, read-only) and - * never inspects a value, and `buildValidationRules` has no branch for - * `location`, so its `min`/`max` rules only ever carry an author-declared bound - * on a scalar. If a later change ever did add repair or validation on this - * path, that assertion is what notices. + * never inspects a value. + * + * ⚠️ The second half of that sentence has since changed and the cases below + * still hold, which is worth stating rather than leaving to be re-derived. + * `buildValidationRules` DOES have a `location` branch now (objectui#6744): a + * stored value is adjudicated against `valueSchemaFor({ type: 'location' })` + * and an out-of-range one blocks the write. It does not touch the readings + * here, because every case in this file TYPES its coordinate — the widget + * refuses the out-of-range ones before they become form values, so the rule is + * handed `undefined` and has nothing to say. The measurement this file records + * is about the CREATE path at INPUT time; #6744's is about a value already in + * the record. `ObjectForm.locationStoredRange.test.tsx` pins that one. */ import { describe, it, expect, vi } from 'vitest'; import { render, waitFor, fireEvent } from '@testing-library/react'; diff --git a/packages/plugin-form/src/ObjectForm.locationRefusal.test.tsx b/packages/plugin-form/src/ObjectForm.locationRefusal.test.tsx index ceb82adb82..e4ccba46d7 100644 --- a/packages/plugin-form/src/ObjectForm.locationRefusal.test.tsx +++ b/packages/plugin-form/src/ObjectForm.locationRefusal.test.tsx @@ -29,9 +29,16 @@ * published objectui#3222 `error` slot. Measured: that route cannot see a * refusal at all — a refusal means `onChange` never fires, so the typed text * never becomes a form value, and a `location` branch installed there was - * invoked with `undefined` in both arms. The last test in this file pins the - * consequence that keeps the sibling file's docblock true: `buildValidationRules` - * STILL has no `location` branch, and this card did not give it one. + * invoked with `undefined` in both arms. + * + * ⚠️ `buildValidationRules` DOES have a `location` branch now — objectui#6744 + * added one, for the different defect of a STORED out-of-range coordinate that + * was never validated on an edit form. That does not weaken the reason above, + * it is the reason above seen from the other side: the branch adjudicates a + * value, and a refusal has no value to adjudicate. The last test in this file + * is now the pin for THAT — the rule exists and answers `true` for the + * `undefined` it is handed here — so the announcement stays the widget's, and + * this card is still not the one that installed the branch. */ import { describe, it, expect, vi } from 'vitest'; import { render, waitFor, fireEvent } from '@testing-library/react'; @@ -137,18 +144,21 @@ describe('ObjectForm shows WHY a location was refused (objectui#6716)', () => { }); }); -describe('the fix did NOT give `buildValidationRules` a location branch (objectui#6716)', () => { - it('compiles no rule for a `location` field', () => { - // The sibling #6714 pin's docblock states this as fact; it stays true under - // the widget-local route, and this is what keeps the two files honest. The - // measured reason it is not worth changing: a refusal never becomes a form - // value, so a rule here is handed `undefined`. - // It compiles rules key by key and returns `undefined` when none applied — - // so `undefined` here is "no branch matched a location field", not "the - // helper is missing". - expect(buildValidationRules({ type: 'location', name: 'place' })).toBeUndefined(); +describe('the announcement is still the widget\'s, not the host rule\'s (objectui#6716)', () => { + it('the `location` rule objectui#6744 added is handed `undefined` by a refusal', () => { + // This pin used to read "compiles no rule for a `location` field". #6744 + // answered that question — the branch exists, for STORED values — so the + // pin now asserts the property that actually kept #6716 widget-local, and + // that a future edit could still break: the host rule adjudicates a VALUE, + // and a refusal produces none. + const rules = buildValidationRules({ type: 'location', name: 'place' }); + expect(typeof rules.validate.location).toBe('function'); + // What the rule sees in both refusal arms above, because `onChange` never + // fired: nothing. A rule that answered anything but `true` here would be + // reporting a refusal it cannot observe. + expect(rules.validate.location(undefined)).toBe(true); // A control from the same call, so the reading above cannot be an artefact - // of a helper that returns `undefined` for everything. + // of a helper that answers the same way for everything. expect(buildValidationRules({ type: 'text', name: 'title', required: true })).toEqual({ required: true }); }); }); diff --git a/packages/plugin-form/src/ObjectForm.locationResidue.test.tsx b/packages/plugin-form/src/ObjectForm.locationResidue.test.tsx index 8c5f96d2cd..194accb2df 100644 --- a/packages/plugin-form/src/ObjectForm.locationResidue.test.tsx +++ b/packages/plugin-form/src/ObjectForm.locationResidue.test.tsx @@ -50,11 +50,12 @@ * ⛔ Degree/hemisphere notation is NOT parsed — the ruling declines it because * the paste route is unmeasured. The refusal is the whole change. * - * ⛔ `buildValidationRules` still has NO `location` branch, and this card did - * not give it one — the last test is the pin that keeps - * `ObjectForm.locationRange.test.tsx`'s docblock true. It could not have one: - * a refusal means `onChange` never fires, so the typed text never becomes a - * form value for a value-shaped rule to see (measured on objectui#6716). + * ⛔ This card did not give `buildValidationRules` a `location` branch — the + * last test is the pin for that. objectui#6744 later did, for the different + * defect of a STORED out-of-range coordinate on an edit form, and the pin was + * rewritten to assert the property that survives: a refusal means `onChange` + * never fires, so the typed text never becomes a form value, and the host rule + * — which adjudicates a value — is handed `undefined` (measured on #6716). */ import { describe, it, expect, vi } from 'vitest'; import { render, waitFor, fireEvent } from '@testing-library/react'; @@ -217,16 +218,18 @@ describe('ObjectForm still stores every clean numeric pair (objectui#6715)', () /* The scope fence objectui#6744 owns. */ /* -------------------------------------------------------------------------- */ -describe('objectui#6715 did not give buildValidationRules a `location` branch', () => { - it('compiles no rules for a location field', () => { - // The fact `ObjectForm.locationRange.test.tsx`'s docblock asserts, kept - // true here as well. objectui#6744 owns the question of whether it should - // have one; this card did not answer it. - // It compiles rules key by key and returns `undefined` when none applied, - // so `undefined` here means "no branch matched a location field". - expect(buildValidationRules({ type: 'location', name: 'place' })).toBeUndefined(); +describe('the residue refusal is the widget\'s, not a host rule\'s (objectui#6715)', () => { + it('the `location` rule objectui#6744 added is handed `undefined` by a refusal', () => { + // This pin used to read "compiles no rules for a location field", with the + // note that objectui#6744 owned the question. #6744 answered it: the branch + // exists, and it validates a STORED value. So the pin now asserts what kept + // this card's refusal widget-local in the first place. + const rules = buildValidationRules({ type: 'location', name: 'place' }); + expect(typeof rules.validate.location).toBe('function'); + // A residue refusal emits nothing, so the host rule sees nothing. + expect(rules.validate.location(undefined)).toBe(true); // A control from the same call, so the reading above cannot be an artefact - // of a helper that returns `undefined` for everything. + // of a helper that answers the same way for everything. expect(buildValidationRules({ type: 'text', name: 'title', required: true })).toEqual({ required: true }); }); }); diff --git a/packages/plugin-form/src/ObjectForm.locationStoredRange.test.tsx b/packages/plugin-form/src/ObjectForm.locationStoredRange.test.tsx new file mode 100644 index 0000000000..0b9a3abd5a --- /dev/null +++ b/packages/plugin-form/src/ObjectForm.locationStoredRange.test.tsx @@ -0,0 +1,302 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#6744, measured end to end: a STORED out-of-range coordinate must not + * survive an edit. + * + * ## The defect + * + * `buildValidationRules` is the producer of the host-side `error` prop every + * field widget's published objectui#3222 slot reads, and it had no branch for + * `location`. So a coordinate ALREADY IN THE RECORD that violates the spec's + * range was never validated on an edit form: the control rendered it, nothing + * marked it invalid, and submitting re-wrote it unchanged. + * + * ⚠️ A different defect from objectui#6714/#6716, which are about a refusal at + * INPUT time. A refusal never becomes a form value, so the rule pinned here is + * handed `undefined` in both of those arms. The create-mode cases at the bottom + * are the pin that says so: #6714's refusal is unchanged and is still the + * widget's, not this rule's. + * + * ## The ruling this file executes + * + * The maintainer ruling of 2026-08-29 (director session batch #4) adopted "add + * the `location` branch + a hard precondition": count stored out-of-range + * coordinates across everything measurable FIRST, land the cheap guard only on + * a zero reading, and stop and report a non-zero one instead of shipping a hard + * block on top of it. + * + * The reading taken for this branch, with the platform's own validator rather + * than a hand-written range: 28 stored location values across every measurable + * dataset — the app-showcase seed (`showcase_account.hq` x14, + * `showcase_task.location` x10, `showcase_field_zoo.f_location` x1), the qa + * dogfood field-zoo matrix, and the objectui schema-catalog location schemas — + * adjudicated by `valueSchemaFor({ type: 'location' }, 'stored')`. 28 accepted, + * 0 refused, with `{ lat: 999, lng: 999 }` refused by the same call as a + * positive control. + * + * ⚠️ That zero means "zero within measurable scope", never "does not exist". + * Customer deployments are not measurable from a development container, and + * nothing here is evidence about them. + * + * ## Why the whole value goes to the spec + * + * ⛔ The bounds are not restated here or in the rule. `valueSchemaFor` is the + * same schema the engine's record validator checks a stored `location` against + * (ADR-0104 D1), so the form now surfaces the platform's own verdict instead of + * inventing one — and the expected message below is BUILT from the schema's + * issues rather than typed out, so a bound that moves in the spec cannot leave + * a stale literal passing in here. + */ +import { describe, it, expect, vi } from 'vitest'; +import { render, waitFor, fireEvent } from '@testing-library/react'; +import React from 'react'; +import { valueSchemaFor } from '@objectstack/spec/data'; + +import { ObjectForm } from './ObjectForm'; +import { registerAllFields, buildValidationRules } from '@object-ui/fields'; + +registerAllFields(); + +const LOCATION_SCHEMA = valueSchemaFor({ type: 'location' } as any)!; + +/** The message the rule must produce, derived from the spec — never typed out. */ +const expectedMessage = (value: unknown): string => { + const parsed = LOCATION_SCHEMA.safeParse(value); + if (parsed.success) throw new Error('expectedMessage called with a value the spec accepts'); + const detail = parsed.error.issues + .map((i: any) => `${i.path.join('.') || 'value'}: ${i.message}`) + .join('; '); + return `Invalid location: ${detail}`; +}; + +const siteSchema = { + name: 'site', + fields: { + title: { type: 'text', label: 'Title' }, + place: { type: 'location', label: 'Place' }, + }, +}; + +const makeDS = (stored: Record) => ({ + getObjectSchema: vi.fn().mockResolvedValue(siteSchema), + findOne: vi.fn().mockResolvedValue({ id: 'r1', title: 'HQ', ...stored }), + create: vi.fn(async (_o: string, d: any) => ({ id: 'r2', ...d })), + update: vi.fn(async (_o: string, _id: string, d: any) => ({ id: 'r1', ...d })), +}); + +const waitInput = (c: HTMLElement, name: string) => + waitFor(() => { + const el = c.querySelector(`input[name="${name}"]`) as HTMLInputElement | null; + if (!el) throw new Error(`${name} not ready`); + return el; + }); + +/** + * Open a real EDIT form on a record that already holds `place`, submit it + * without touching the location box, and report what the form did. + * + * Nothing is typed into the coordinate box on purpose: the whole defect is + * about a value the user never touched being written back. + */ +async function submitStored(place: unknown) { + const ds = makeDS(place === undefined ? {} : { place }); + const { container } = render( + , + ); + const input = await waitInput(container, 'place'); + fireEvent.submit(container.querySelector('form') as HTMLFormElement); + // Let the submit settle either way: a blocked submit never calls `update`, so + // waiting on `update` would be waiting on the thing under test. + await waitFor(() => { + if (!ds.update.mock.calls.length && container.querySelector('[aria-invalid="true"]') === null) { + throw new Error('form has neither written nor refused yet'); + } + }); + return { ds, container, input, rowText: input.closest('div')?.parentElement?.textContent ?? '' }; +} + +/* -------------------------------------------------------------------------- */ +/* Direction 1 — an out-of-range STORED value is blocked on edit. */ +/* -------------------------------------------------------------------------- */ + +describe('a stored out-of-range location blocks the edit (objectui#6744)', () => { + it('marks the control invalid, says why, and writes nothing', async () => { + // The exact value the card measured, and the exact reason it matters. + const refused = LOCATION_SCHEMA.safeParse({ lat: 999, lng: 999 }); + expect(refused.success).toBe(false); + expect( + refused.success ? [] : refused.error.issues.map((i: any) => `${i.code}@[${i.path.join('.')}]`), + ).toEqual(['too_big@[lat]', 'too_big@[lng]']); + + const { ds, input, rowText } = await submitStored({ lat: 999, lng: 999 }); + + expect(input.getAttribute('aria-invalid')).toBe('true'); + expect(rowText).toContain(expectedMessage({ lat: 999, lng: 999 })); + expect(ds.update).toHaveBeenCalledTimes(0); + }); + + it('blocks every out-of-range pair the spec refuses, on each bound', async () => { + for (const stored of [ + { lat: 999, lng: 999 }, + { lat: 91, lng: 0 }, + { lat: -91, lng: 0 }, + { lat: 0, lng: 181 }, + { lat: 0, lng: -181 }, + ]) { + expect(LOCATION_SCHEMA.safeParse(stored).success, `spec verdict on ${JSON.stringify(stored)}`) + .toBe(false); + const { ds, input } = await submitStored(stored); + expect(input.getAttribute('aria-invalid'), `aria-invalid for ${JSON.stringify(stored)}`) + .toBe('true'); + expect(ds.update, `update for ${JSON.stringify(stored)}`).toHaveBeenCalledTimes(0); + } + }); + + it('the widget still RENDERS the bad coordinate, so its only fixer can see it', async () => { + // objectui#6272's read guard is deliberately not wired to the range: a + // record holding an out-of-range pair keeps rendering. Blanking it would + // hide the dirty data from the person who can correct it, and this rule + // does not change that — it only stops the value being written back. + const { input } = await submitStored({ lat: 999, lng: 999 }); + expect(input.value).toBe('999, 999'); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* Direction 2 — a legal stored value is untouched. */ +/* -------------------------------------------------------------------------- */ + +describe('a legal stored location is unaffected (objectui#6744)', () => { + it('saves it back unchanged, with nothing marked invalid', async () => { + const stored = { lat: 30.2741, lng: 120.1551 }; + expect(LOCATION_SCHEMA.safeParse(stored).success).toBe(true); + + const { ds, input, rowText } = await submitStored(stored); + + expect(input.getAttribute('aria-invalid')).toBe('false'); + expect(rowText).not.toContain('Invalid location:'); + await waitFor(() => expect(ds.update).toHaveBeenCalledTimes(1)); + expect(ds.update.mock.calls[0][2]).toMatchObject({ place: stored }); + }); + + it('carries the spec optional keys through without complaint', async () => { + // `altitude`/`accuracy` are declared by `LocationValueSchema` and a customer + // may write them, so the rule must not read a fuller value as a worse one. + const stored = { lat: 30.2741, lng: 120.1551, altitude: 12, accuracy: 5 }; + expect(LOCATION_SCHEMA.safeParse(stored).success).toBe(true); + const { ds, input } = await submitStored(stored); + expect(input.getAttribute('aria-invalid')).toBe('false'); + await waitFor(() => expect(ds.update).toHaveBeenCalledTimes(1)); + expect(ds.update.mock.calls[0][2]).toMatchObject({ place: stored }); + }); + + it('an ABSENT location does not block the edit', async () => { + // The spec's schema refuses `null` and `undefined` outright — it describes a + // PRESENT value and leaves presence to the caller. Deciding it here would be + // a second definition of "empty" competing with `required`'s, so the rule + // asks core's `isMissingForRequired` instead. This is that pin. + const { ds, input } = await submitStored(undefined); + expect(input.getAttribute('aria-invalid')).toBe('false'); + await waitFor(() => expect(ds.update).toHaveBeenCalledTimes(1)); + expect(ds.update.mock.calls[0][2].place).toBeUndefined(); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* Direction 3 — the CREATE form's behaviour is unchanged. */ +/* -------------------------------------------------------------------------- */ + +async function submitCreate(coordinateText?: string) { + const ds = makeDS({}); + const { container } = render( + , + ); + fireEvent.change(await waitInput(container, 'title'), { target: { value: 'HQ' } }); + if (coordinateText !== undefined) { + fireEvent.change(await waitInput(container, 'place'), { target: { value: coordinateText } }); + } + fireEvent.submit(container.querySelector('form') as HTMLFormElement); + await waitFor(() => expect(ds.create).toHaveBeenCalled()); + return { ds, container, payload: ds.create.mock.calls[0][1] as Record }; +} + +describe('the create form behaves exactly as before (objectui#6744)', () => { + it('an untouched location field still creates', async () => { + const { payload } = await submitCreate(); + expect(payload).toMatchObject({ title: 'HQ' }); + expect(payload.place).toBeUndefined(); + }); + + it('a legal typed coordinate is still stored', async () => { + const { payload } = await submitCreate('30.2741, 120.1551'); + expect(payload.place).toEqual({ lat: 30.2741, lng: 120.1551 }); + }); + + it('an out-of-range typed coordinate is still refused by the WIDGET, not by this rule', async () => { + // objectui#6714 unchanged: the widget never emits it, so the value never + // becomes a form value and this rule is handed `undefined`. The create + // payload carries no `place` — the same reading `ObjectForm.locationRange` + // pins — and the create still goes through, which is what proves the + // refusal happened upstream of the rule rather than in it. + const { payload } = await submitCreate('999, 999'); + expect(payload.place).toBeUndefined(); + expect(payload).toMatchObject({ title: 'HQ' }); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* The rule itself, at the seam objectui#6744 actually changed. */ +/* -------------------------------------------------------------------------- */ + +describe('buildValidationRules now compiles a `location` branch (objectui#6744)', () => { + it('emits a `validate.location` entry for a location field', () => { + const rules = buildValidationRules({ type: 'location', name: 'place' }); + expect(rules).toBeDefined(); + expect(typeof rules.validate.location).toBe('function'); + }); + + it('the entry passes a legal value and an absent one, and refuses what the spec refuses', () => { + const validate = buildValidationRules({ type: 'location', name: 'place' }).validate.location; + expect(validate({ lat: 30.2741, lng: 120.1551 })).toBe(true); + expect(validate(undefined)).toBe(true); + expect(validate(null)).toBe(true); + expect(validate('')).toBe(true); + expect(validate({ lat: 999, lng: 999 })).toBe(expectedMessage({ lat: 999, lng: 999 })); + expect(validate({ lat: 91, lng: 0 })).toBe(expectedMessage({ lat: 91, lng: 0 })); + }); + + it('a field-authored `validate` keeps running alongside it', () => { + // The object form is react-hook-form's own way of holding several named + // validators, and it is the shape the form renderer already normalises to + // when it adds `required`. Composing rather than replacing is what keeps an + // authored rule from being silently dropped on location fields. + const authored = () => 'authored says no'; + const rules = buildValidationRules({ type: 'location', name: 'place', validate: authored }); + expect(rules.validate.validate).toBe(authored); + expect(typeof rules.validate.location).toBe('function'); + }); + + it('no other field type grows a location rule', () => { + // A control from the same call, so the readings above cannot be an artefact + // of a helper that answers the same way for everything. + expect(buildValidationRules({ type: 'text', name: 'title', required: true })) + .toEqual({ required: true }); + expect(buildValidationRules({ type: 'text', name: 'title' })).toBeUndefined(); + const authored = () => true; + expect(buildValidationRules({ type: 'text', name: 'title', validate: authored })) + .toEqual({ validate: authored }); + }); +}); From aa8372ad4bcac7e7ddac827efdc2628f41f75978 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 16:39:04 +0000 Subject: [PATCH 2/2] docs(fields): correct two docblocks this branch falsifies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both said `buildValidationRules` "still has no `location` branch". This branch gives it one, so shipping that prose in the same package would leave a contradiction for the next reader — the failure mode the docblock exists to prevent. Each sentence was a compound claim and only half of it is falsified: - "still has no `location` branch" -> false as of this branch - "this card does not give it one" -> still true (#6716 / the refusal-diagnostic card did not add it; objectui#6744 did) So the true half is kept verbatim and only the false half is corrected, with a pointer to the card that added the branch and a note that it serves the STORED case and never these refusal arms — which is the property the surrounding paragraph is actually about, and which this branch confirms rather than contradicts. Prose only: no assertion, no behaviour, no export, no changeset. Neither file is in PR #6801's file list (verified against its 14 files). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- .../src/__tests__/LocationField.refusalDiagnostic.test.tsx | 4 ++-- packages/fields/src/widgets/LocationField.tsx | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/fields/src/__tests__/LocationField.refusalDiagnostic.test.tsx b/packages/fields/src/__tests__/LocationField.refusalDiagnostic.test.tsx index e2bc0cada6..386cc51c34 100644 --- a/packages/fields/src/__tests__/LocationField.refusalDiagnostic.test.tsx +++ b/packages/fields/src/__tests__/LocationField.refusalDiagnostic.test.tsx @@ -29,8 +29,8 @@ * arms, because a refusal means `onChange` never fires, so the typed text never * becomes a form value at all — while the same branch fires correctly for a * STORED out-of-range pair. `buildValidationRules` compiles value-shaped rules; - * a refusal has no value. It still has no `location` branch and this card does - * not give it one. + * a refusal has no value. It HAS one as of objectui#6744 — for that STORED + * case, never for these refusal arms — and this card did not give it one. * * So the state is the widget's own, following `ObjectField`'s live precedent in * this same directory: a second name (`parseError` there, `refusalError` here), diff --git a/packages/fields/src/widgets/LocationField.tsx b/packages/fields/src/widgets/LocationField.tsx index 3bf3a6172b..f7d32425f4 100644 --- a/packages/fields/src/widgets/LocationField.tsx +++ b/packages/fields/src/widgets/LocationField.tsx @@ -353,8 +353,9 @@ export function LocationField({ value, onChange, field, readonly, error, ...prop * `undefined`. That was measured on this card before the route was chosen: a * real `location` branch installed in `buildValidationRules` saw `undefined` * in both refusal arms, while the same branch fired correctly for a STORED - * out-of-range pair. `buildValidationRules` still has no `location` branch, - * and this card does not give it one. + * out-of-range pair. `buildValidationRules` HAS a `location` branch as of + * objectui#6744 — for that STORED case, never for these refusal arms — and + * this card did not give it one. */ const [refusalError, setRefusalError] = useState(null);