diff --git a/.changeset/address-field-postal-code-os5143.md b/.changeset/address-field-postal-code-os5143.md new file mode 100644 index 0000000000..208f7d8963 --- /dev/null +++ b/.changeset/address-field-postal-code-os5143.md @@ -0,0 +1,31 @@ +--- +"@object-ui/fields": patch +--- + +`address` widget: the ZIP box now reads and writes `postalCode`, the part name the platform stores + +`AddressField` bound its ZIP input to `zipCode` — a part name that appears +nowhere in `@objectstack/spec`. The stored value uses `postalCode`, which +`AddressValueSchema` declares and enforces with `$strip` semantics, so the two +sides never met: + +- opening a stored address showed an **empty** ZIP box (no input read + `postalCode`), while street / city / state / country all populated — four of + five parts working is what let this survive review; +- anything the user then typed into that box was written back as `zipCode` and + **stripped at the contract boundary**. On a new record the postal code was + lost outright; on an existing one the correction was silently discarded and + the stale stored code remained, with no error anywhere. + +The widget now uses `postalCode` throughout — state key, sub-input id, the +`onChange` part name and the read-only formatter. Data written by the previous +builds is still **read** through `zipCode` as a compatibility limb, and the +first edit of any part normalizes such a record onto `postalCode` rather than +writing the split shape back out. Nothing writes `zipCode` any more, and it is +deliberately not part of the exported `AddressValue` type, so authoring code +cannot spell it. + +`AddressValue` is otherwise unchanged in shape; consumers that referenced +`AddressValue['zipCode']` must read `postalCode`. + +Fixes objectstack-ai/objectstack#5143. diff --git a/packages/fields/src/widgets/AddressField.postalCode.test.tsx b/packages/fields/src/widgets/AddressField.postalCode.test.tsx new file mode 100644 index 0000000000..82903bec17 --- /dev/null +++ b/packages/fields/src/widgets/AddressField.postalCode.test.tsx @@ -0,0 +1,257 @@ +/** + * 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. + */ + +/** + * Postal-code round trip of the composite `address` widget (objectstack#5143). + * + * The widget used to read and write the part name `zipCode`. The platform + * stores — and `/api/v1/data/**` serves back — `postalCode`, the name + * `@objectstack/spec`'s `AddressValueSchema` declares. The two never met: + * + * - opening a stored address showed an EMPTY ZIP box (the stored + * `postalCode` had no input reading it), and + * - anything typed into that box was written back as `zipCode`, which the + * enforced value schema strips (`$strip`) — destroying it at the contract + * boundary: lost outright on a new record, and on an existing one silently + * discarded while the stale stored code survived. + * + * Four of five parts round-tripped, which is what made it survive review. + * + * These tests pin BOTH directions, plus the read-time compatibility limb for + * data the broken build already wrote: + * + * 1. stored `postalCode` reaches the ZIP input; + * 2. every edit writes the postal code back under `postalCode` and never + * under `zipCode` — verified against `AddressValueSchema.parse`, i.e. the + * platform's own contract, not a local restatement of it; + * 3. a legacy `zipCode`-shaped value still DISPLAYS, and the first edit of + * any part migrates it onto `postalCode` instead of writing the split + * shape back out. + * + * Reverse verification (measured, not presumed — `git show origin/main` of the + * widget, this file unchanged): 9 of 12 go red — 1, 2, 5, 6, 7, 8, 9, 10, 12. + * The three that stay GREEN are worth naming, because one of them refines the + * issue's stated mechanism: + * + * - 3 stays green. The old widget spread the WHOLE stored object through on + * every write (`{...address, [part]: v}`), so an untouched `postalCode` + * survived an edit of an unrelated part. The issue's "saving drops the + * postal code" therefore does NOT reproduce for a value nobody touched; + * the loss the old code really caused is test 6's: a postal code the user + * TYPES is written under a key `AddressValueSchema` strips, so the typed + * correction is silently discarded and the stale stored code stays — and on + * a new record (test 12) it is lost outright. 3 is kept as a contract pin + * for the pass-through, not as evidence of the fix. + * - 4 stays green: it pins the legacy READ, and the old code read that key + * natively. It is meaningful as a pair with 5, which no revert satisfies. + * - 11 stays green: it reads the spec, not the widget. That is the point — + * it fails only if the CONTRACT moves under us. + */ +import { describe, it, expect, vi } from 'vitest'; +import React from 'react'; +import { render, screen, fireEvent } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { AddressValueSchema } from '@objectstack/spec/data'; +import { AddressField, type AddressValue } from './AddressField'; + +const field = { name: 'billing_address', type: 'address' } as any; + +const ZIP_LABEL = 'ZIP / Postal Code'; + +/** The document the REST API served back in the issue's evidence. */ +const STORED: AddressValue = { + street: '中策路 1 号', + city: '杭州', + state: '浙江', + postalCode: '310018', + country: 'CN', +}; + +/** The same address as written by builds up to 17.0.0-rc.1 (the broken key). */ +const LEGACY = { + street: '中策路 1 号', + city: '杭州', + state: '浙江', + zipCode: '310018', + country: 'CN', +} as AddressValue; + +/** Resolve a sub-input exactly as a browser does on label activation. */ +function subInput(labelText: string): HTMLInputElement { + const label = screen.getByText(labelText); + const forId = label.getAttribute('for'); + expect(forId, `label "${labelText}" must carry htmlFor`).toBeTruthy(); + const control = document.getElementById(forId!); + expect(control, `htmlFor of "${labelText}" must resolve`).not.toBeNull(); + return control as HTMLInputElement; +} + +/** The most recent object the widget handed to `onChange`. */ +function lastWrite(written: AddressValue[]): AddressValue { + expect(written.length, 'the widget must have written at least once').toBeGreaterThan(0); + return written[written.length - 1]; +} + +/** + * Controlled harness — the widget is controlled, so a round trip needs a + * parent that stores what the widget writes and feeds it straight back. + */ +function Harness({ initial, onWrite }: { initial: AddressValue; onWrite: (v: AddressValue) => void }) { + const [value, setValue] = React.useState(initial); + return ( + { + onWrite(next); + setValue(next); + }} + field={field} + /> + ); +} + +describe('AddressField — postal code round trip (objectstack#5143)', () => { + it('1. loads the stored `postalCode` into the ZIP input', () => { + render(); + + expect(subInput(ZIP_LABEL).value).toBe('310018'); + // The four parts that always worked must keep working. + expect(subInput('Street Address').value).toBe('中策路 1 号'); + expect(subInput('City').value).toBe('杭州'); + expect(subInput('State / Province').value).toBe('浙江'); + expect(subInput('Country').value).toBe('CN'); + }); + + it('2. keeps the postal code when an UNRELATED part is edited, under `postalCode`', () => { + const written: AddressValue[] = []; + render( written.push(v)} />); + + fireEvent.change(subInput('City'), { target: { value: '宁波' } }); + + const last = lastWrite(written); + expect(last.city).toBe('宁波'); + expect(last.postalCode).toBe('310018'); + expect(Object.keys(last)).not.toContain('zipCode'); + // …and it is still on screen after the write is fed back in. + expect(subInput(ZIP_LABEL).value).toBe('310018'); + }); + + it('3. the written address survives the enforced platform value contract', () => { + const written: AddressValue[] = []; + render( written.push(v)} />); + + fireEvent.change(subInput('City'), { target: { value: '宁波' } }); + + // `AddressValueSchema` is `$strip`: an unknown part name is DROPPED, which + // is how a `zipCode`-shaped write lost the postal code on the way to + // storage. Parsing the widget's own output is the end-to-end pin. + const parsed = AddressValueSchema.parse(lastWrite(written)); + expect(parsed.postalCode).toBe('310018'); + }); + + it('4. still DISPLAYS a legacy `zipCode`-shaped value written by earlier builds', () => { + render(); + + expect(subInput(ZIP_LABEL).value).toBe('310018'); + }); + + it('5. migrates a legacy value onto `postalCode` on the first edit of any part', () => { + const written: AddressValue[] = []; + render( written.push(v)} />); + + fireEvent.change(subInput('Street Address'), { target: { value: '中策路 2 号' } }); + + const last = lastWrite(written); + expect(last.street).toBe('中策路 2 号'); + expect(last.postalCode).toBe('310018'); + expect(Object.keys(last)).not.toContain('zipCode'); + expect(AddressValueSchema.parse(last).postalCode).toBe('310018'); + }); + + it('6. writes an edit of the ZIP box itself under `postalCode`', () => { + const written: AddressValue[] = []; + render( written.push(v)} />); + + fireEvent.change(subInput(ZIP_LABEL), { target: { value: '310019' } }); + + const last = lastWrite(written); + expect(last.postalCode).toBe('310019'); + expect(Object.keys(last)).not.toContain('zipCode'); + expect(subInput(ZIP_LABEL).value).toBe('310019'); + // The sharpest statement of the bug: under the old key the CORRECTION was + // stripped by the contract and the stale stored code survived, with no + // error anywhere — the user saw 310019 in the box and 310018 in the record. + expect(AddressValueSchema.parse(last).postalCode).toBe('310019'); + }); + + it('7. prefers the canonical key when a record carries both', () => { + const both = { ...STORED, zipCode: '999999' } as AddressValue; + const written: AddressValue[] = []; + render( written.push(v)} />); + + expect(subInput(ZIP_LABEL).value).toBe('310018'); + + fireEvent.change(subInput('City'), { target: { value: '宁波' } }); + + const last = lastWrite(written); + expect(last.postalCode).toBe('310018'); + expect(Object.keys(last)).not.toContain('zipCode'); + }); + + it('8. clearing the ZIP box clears the postal code instead of resurrecting the legacy value', () => { + const written: AddressValue[] = []; + render( written.push(v)} />); + + fireEvent.change(subInput(ZIP_LABEL), { target: { value: '' } }); + + const last = lastWrite(written); + expect(last.postalCode).toBe(''); + expect(Object.keys(last)).not.toContain('zipCode'); + expect(subInput(ZIP_LABEL).value).toBe(''); + }); + + it('9. formats the postal code in read-only mode, canonical and legacy alike', () => { + const canonical = render( + , + ); + expect(canonical.container.textContent).toContain('浙江 310018'); + canonical.unmount(); + + const legacy = render( + , + ); + expect(legacy.container.textContent).toContain('浙江 310018'); + }); + + it('10. binds the ZIP sub-input to the canonical part name, and nothing to the legacy one', () => { + const { baseElement } = render( + , + ); + + expect(subInput(ZIP_LABEL).id).toMatch(/-postalCode$/); + const ids = Array.from(baseElement.querySelectorAll('[id]')).map((el) => el.id); + expect(ids.filter((id) => id.endsWith('-zipCode'))).toEqual([]); + }); + + it('11. spells the part the way the spec spells it (direction guard)', () => { + const specParts = Object.keys(AddressValueSchema.shape); + expect(specParts).toContain('postalCode'); + expect(specParts).not.toContain('zipCode'); + }); + + it('12. a postal code entered on a NEW record reaches storage instead of being stripped', () => { + const written: AddressValue[] = []; + render( written.push(v)} />); + + fireEvent.change(subInput(ZIP_LABEL), { target: { value: '310018' } }); + + // Nothing stored to mask the loss: under the old key the parse produced an + // address with no postal code at all. + expect(AddressValueSchema.parse(lastWrite(written)).postalCode).toBe('310018'); + }); +}); diff --git a/packages/fields/src/widgets/AddressField.tsx b/packages/fields/src/widgets/AddressField.tsx index 6df896be79..f3a6308cd8 100644 --- a/packages/fields/src/widgets/AddressField.tsx +++ b/packages/fields/src/widgets/AddressField.tsx @@ -4,19 +4,48 @@ import { FieldWidgetComponentProps } from './types'; import { toDomProps } from './toDomProps'; /** - * Address data structure + * Address data structure — the part names of `@objectstack/spec`'s + * `AddressSchema`, which is what the platform stores and what + * `/api/v1/data/**` serves back. + * + * The postal code is spelled `postalCode` (objectstack#5143). This widget used + * to read and write `zipCode`, a key that appears nowhere in the spec, so the + * stored postal code never reached the input — and whatever the user then typed + * into that apparently-empty box was written under a key `AddressValueSchema` + * strips: lost outright on a new record, and on an existing one silently + * discarded while the stale stored value survived. (A postal code nobody + * touched did survive an unrelated edit, because the write spread the whole + * stored object through; the issue's account of that half is corrected in + * `AddressField.postalCode.test.tsx`.) One name, on both sides: the contract's. */ export interface AddressValue { street?: string; city?: string; state?: string; - zipCode?: string; + postalCode?: string; country?: string; } +/** + * The shape written by builds up to and including 17.0.0-rc.1, whose postal + * code landed under `zipCode` (objectstack#5143). Read-time compatibility ONLY, + * and deliberately not part of {@link AddressValue}: authoring code must not be + * able to spell `zipCode`, and nothing here ever writes it back — the first + * edit of any part normalizes the record onto `postalCode` (see + * `handleFieldChange`). This is not a producer alias to be honoured forever; + * it exists to carry data THIS widget mis-wrote, and can go once no such data + * remains. + */ +type LegacyAddressValue = AddressValue & { zipCode?: string }; + +/** Postal code of a stored address, preferring the canonical key. */ +function readPostalCode(addr: AddressValue): string | undefined { + return addr.postalCode ?? (addr as LegacyAddressValue).zipCode; +} + /** * Address field widget - provides a structured address input - * Supports street, city, state, zip code, and country + * Supports street, city, state, postal code, and country */ export function AddressField({ value, onChange, field, readonly, error, ...props }: FieldWidgetComponentProps) { const address = value || {}; @@ -33,9 +62,20 @@ export function AddressField({ value, onChange, field, readonly, error, ...props const groupId = useId(); const subId = (name: keyof AddressValue) => `${groupId}-${name}`; + // Read through the legacy key, write only the canonical one (objectstack#5143). + const postalCode = readPostalCode(address); + const handleFieldChange = (fieldName: keyof AddressValue, fieldValue: string) => { + // Normalize while we are here: the object we write back never carries + // `zipCode`, and a legacy postal code is carried forward under + // `postalCode`. Without the carry, editing an unrelated part of a + // legacy-shaped address would write the postal code straight back out of + // the record — the very data loss this fixes, one build later. + const canonical: LegacyAddressValue = { ...address }; + delete canonical.zipCode; onChange({ - ...address, + ...canonical, + ...(postalCode ? { postalCode } : null), [fieldName]: fieldValue, }); }; @@ -44,7 +84,7 @@ export function AddressField({ value, onChange, field, readonly, error, ...props const parts = [ addr.street, addr.city, - [addr.state, addr.zipCode].filter(Boolean).join(' '), + [addr.state, readPostalCode(addr)].filter(Boolean).join(' '), addr.country, ].filter(Boolean); return parts.join(', '); @@ -102,12 +142,12 @@ export function AddressField({ value, onChange, field, readonly, error, ...props
- + handleFieldChange('zipCode', e.target.value)} + value={postalCode || ''} + onChange={(e) => handleFieldChange('postalCode', e.target.value)} placeholder="94102" disabled={readonly || props.disabled} aria-invalid={!!error} diff --git a/packages/fields/src/widgets/AddressField.uniqueIds.test.tsx b/packages/fields/src/widgets/AddressField.uniqueIds.test.tsx index be55fed4ff..a3950ea550 100644 --- a/packages/fields/src/widgets/AddressField.uniqueIds.test.tsx +++ b/packages/fields/src/widgets/AddressField.uniqueIds.test.tsx @@ -10,7 +10,8 @@ * Unique sub-input ids for the composite `address` widget (objectui#3343). * * The widget used to hardcode literal ids ("street", "city", "state", - * "zipCode", "country") on its sub-inputs. Two address fields in one form — + * "postalCode" — spelled "zipCode" until objectstack#5143 —, "country") on its + * sub-inputs. Two address fields in one form — * e.g. billing + shipping — then produced duplicate DOM ids, and every * `Label htmlFor` resolved to the FIRST match in the document: each sub-label * of the second field clicked/announced the first field's input.