From a9f8d8b02fbf806e91cac50adea812bec15b0e37 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 07:50:31 +0000 Subject: [PATCH] fix(rest): resolve the public-lookup picker target from the canonical `reference` (#7486) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GET /forms/:slug/lookup/:field` presents `publicPicker.object` as an optional override — omit it and the server resolves the target from the field's own definition on the parent object. It could not. The fallback chain read three LEGACY spellings only: referenceTo = def?.referenceTo ?? def?.target ?? def?.options?.objectName; while `packages/spec/src/data/field.zod.ts:450` folds `relatedTo` / `referenceTo` / `target` / `targetObject` / `lookupObject` ALL onto `reference` at parse. A parsed, canonical object schema therefore carried none of the three keys the route read: the chain resolved `undefined` and the route answered `500 LOOKUP_TARGET_MISSING` for exactly the well-formed metadata the platform produces. `publicPicker.object` was de-facto REQUIRED while the schema and docs presented it as optional. `def?.reference` now heads the chain. The legacy spellings are kept AFTER it, not replaced — stored pre-fold rows never went through the alias table, so this widens the resolution rather than moving it. Precedence: `reference` -> `referenceTo` -> `target` -> `options.objectName`. No spec change: the spec was right, the consumer read the wrong keys. Tests (`packages/rest/src/public-form-lookup-picker.test.ts`, extended rather than left as-is — it declared `object` explicitly *because of* this bug, so only ever exercised the override branch): - a canonical `{ type: 'lookup', reference: 'sys_user' }` field resolves with NO `object` override (the headline case; 500 before this change) - the three legacy spellings still resolve (stored pre-fold rows) - `reference` WINS over a legacy spelling on the same def (head-of-chain, not merely present-in-chain) - GUARD: a def naming no target at all STILL answers 500 LOOKUP_TARGET_MISSING — the error became rare, not unreachable Docs: the `content/docs/ui/forms.mdx` error table pointed authors hitting this 500 at declaring `object`, describing the defect as if it were the design. That row and the `object` key row are corrected here. --- .../public-lookup-canonical-reference.md | 40 +++++++ content/docs/ui/forms.mdx | 4 +- .../src/public-form-lookup-picker.test.ts | 113 ++++++++++++++++-- packages/rest/src/rest-server.ts | 17 ++- 4 files changed, 164 insertions(+), 10 deletions(-) create mode 100644 .changeset/public-lookup-canonical-reference.md diff --git a/.changeset/public-lookup-canonical-reference.md b/.changeset/public-lookup-canonical-reference.md new file mode 100644 index 0000000000..b988ac0781 --- /dev/null +++ b/.changeset/public-lookup-canonical-reference.md @@ -0,0 +1,40 @@ +--- +"@objectstack/rest": patch +--- + +fix(rest): resolve the public-lookup picker target from the canonical `reference` key (#7486) + +`GET /forms/:slug/lookup/:field` treats `publicPicker.object` as an optional +override: omit it and the server is supposed to resolve the target from the +field's own definition on the parent object. It could not. The fallback chain +read three **legacy** spellings only — + +```ts +referenceTo = def?.referenceTo ?? def?.target ?? def?.options?.objectName; +``` + +— while `packages/spec/src/data/field.zod.ts` folds `relatedTo` / `referenceTo` +/ `target` / `targetObject` / `lookupObject` **all onto `reference`** at parse. +A parsed, canonical object schema therefore carries none of the three keys the +route read: the chain resolved `undefined` and the route answered +`500 LOOKUP_TARGET_MISSING` for exactly the well-formed metadata the platform +produces. Net effect, `publicPicker.object` was de-facto **required** while the +schema and the docs presented it as optional. + +The canonical `reference` now heads the chain. A field declared +`{ type: 'lookup', reference: 'sys_user' }` resolves with no `object` override, +which is the form authors are told to write. + +The three legacy spellings are **kept after it**, not replaced: rows stored +before the alias fold never went through the alias table and still carry them, +so this widens the resolution rather than moving it. Precedence is +`reference` → `referenceTo` → `target` → `options.objectName`, so a +partially-migrated def carrying both follows the canonical key. + +`LOOKUP_TARGET_MISSING` did not become unreachable — it became rare. A field +naming no target object at all (or one whose object metadata cannot be read) +still gets the loud 500 rather than a silent search of nothing. + +No spec change: the spec was already right, the consumer was reading the wrong +keys. The docs table in `content/docs/ui/forms.mdx`, which pointed authors +hitting this 500 at declaring `object`, is corrected in the same change. diff --git a/content/docs/ui/forms.mdx b/content/docs/ui/forms.mdx index 510f3cd742..11ce4bb022 100644 --- a/content/docs/ui/forms.mdx +++ b/content/docs/ui/forms.mdx @@ -255,7 +255,7 @@ sections: [{ | `displayFields` | Fields projected into each result row (plus `id`); the visitor's `q` is `contains`-matched against the **first** entry. At most 5; omitted → `['name']`. | | `maxResults` | Rows per request, integer 1–50 (default 20). 50 is a hard server ceiling; there is **no pagination** on this surface (`offset` is pinned to 0), so a leaked endpoint cannot enumerate the table. | | `filter` | Static pre-filter rows (same `{ field, operator, value }` dialect as list-view filters), ANDed ahead of the visitor's search. | -| `object` | The object to search; omit to let the server resolve it from the field definition. | +| `object` | The object to search. Optional — omit it and the server resolves the target from the field's own definition on the parent object (its `reference`, or a legacy `referenceTo` / `target` / `options.objectName` on a pre-fold stored row). Declare it only to search something other than what the field points at. | Those four keys are the whole block. It admits exactly what the route enforces — an unknown subkey, a 6th display field, or `maxResults: 51` is a **parse @@ -282,7 +282,7 @@ Errors: | `400 INVALID_REQUEST` | missing / blank slug or field | | `403 LOOKUP_NOT_PUBLIC` | the field has no `publicPicker` block — the deliberate loud default (#3022); also any server-managed anchor (`owner_id`, `organization_id`, …), which never gets a picker even if one is declared | | `404 FORM_NOT_FOUND` | slug not registered on any `sharing.allowAnonymous: true` view | -| `500 LOOKUP_TARGET_MISSING` | the referenced object could not be resolved — declare `publicPicker.object` | +| `500 LOOKUP_TARGET_MISSING` | the referenced object could not be resolved from either `publicPicker.object` or the field definition — the field names no target object at all (or its object metadata is unreachable). Until #7486 this also fired for a perfectly well-formed field, because the fallback read only the legacy spellings and not the canonical `reference`; declaring `object` was the workaround and is no longer needed. | ### Auth model diff --git a/packages/rest/src/public-form-lookup-picker.test.ts b/packages/rest/src/public-form-lookup-picker.test.ts index 046a85b96e..901f08a2de 100644 --- a/packages/rest/src/public-form-lookup-picker.test.ts +++ b/packages/rest/src/public-form-lookup-picker.test.ts @@ -81,10 +81,12 @@ async function persistedBody(item: unknown): Promise { /** * The picker under test: every key is one the route reads, nothing more. - * `object` is declared because the route's fallback resolution reads only the - * LEGACY field-def spellings (`referenceTo` / `target` / `options.objectName`) - * and not the canonical `reference` — recorded as a follow-up finding on - * #7467; against a canonical object schema the override is what works today. + * `object` is declared here to exercise the explicit override branch. It used + * to be declared because it was the ONLY branch that worked — the route's + * fallback read the legacy field-def spellings and not the canonical + * `reference` (#7486, fixed). The omitted-`object` case now has its own suite + * at the bottom of this file; leaving that path untested would leave the fix + * unguarded at the level users actually hit. */ const PICKER = { displayFields: ['name', 'email'], @@ -134,15 +136,19 @@ function mockRes() { return res; } -/** Mount the real routes over a protocol that serves the STORED view body. */ -function routesOver(storedView: any, foundRows: any[]) { +/** + * Mount the real routes over a protocol that serves the STORED view body. + * `objectDef` defaults to the canonical `leadObject`; the #7486 suite passes + * variants to cover the legacy pre-fold spellings of the same field. + */ +function routesOver(storedView: any, foundRows: any[], objectDef: any = leadObject) { const findData = vi.fn().mockResolvedValue({ data: foundRows }); const protocol: any = { getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', routes: { data: '', metadata: '' } }), getMetaTypes: vi.fn().mockResolvedValue([]), getMetaItems: vi.fn(async ({ type }: { type: string }) => { if (type === 'view') return [storedView]; - if (type === 'object') return [leadObject]; + if (type === 'object') return [objectDef]; return []; }), createData: vi.fn().mockResolvedValue({ object: 'lead', id: 'rec_1', record: {} }), @@ -309,3 +315,96 @@ describe('#7485 publicPicker.sort is retired — not declarable, and not read', expect(second.findData.mock.calls[0][0].query.sort).toEqual([{ field: 'email', order: 'asc' }]); }); }); + +// ─── [#7486] the fallback resolution, against CANONICAL metadata ──────────── + +/** + * The defect this suite pins: the route's fallback chain read only the LEGACY + * field-def spellings (`referenceTo` / `target` / `options.objectName`), while + * `packages/spec/src/data/field.zod.ts` folds every one of those onto the + * canonical `reference` at parse. A parsed object schema therefore carried + * NONE of the keys the route read — the chain resolved `undefined` and a + * well-formed form got `500 LOOKUP_TARGET_MISSING`, making `publicPicker.object` + * de-facto REQUIRED while the schema and docs present it as optional. + * + * Every case below omits `object` deliberately: that is the axis under test. + * The suite above covers the override branch and must stay that way — between + * them the two branches of the resolution are both pinned. + */ +describe('#7486 the picker target resolves from the field definition when `object` is omitted', () => { + /** The picker an author writes when they take the docs at their word. */ + const NO_OBJECT_PICKER = { displayFields: ['name', 'email'], maxResults: 10 }; + + const savedWithoutObject = () => persistedBody(studioForm([{ field: 'owner', publicPicker: NO_OBJECT_PICKER }])); + + it('a CANONICAL `{ type: lookup, reference: sys_user }` field resolves with no `object` override', async () => { + // The headline case, and the one the platform actually produces: this + // exact request answered 500 LOOKUP_TARGET_MISSING before #7486. + const stored = await savedWithoutObject(); + expect(stored.config.sections[0].fields[0].publicPicker.object).toBeUndefined(); + + const { findData, lookup } = routesOver(stored, [{ id: 'usr_1', name: 'Ada', email: 'ada@example.com' }]); + const res = mockRes(); + await lookup.handler({ params: { slug: 'contact', field: 'owner' }, query: { q: 'ad' } } as any, res); + + expect(res.statusCode).toBe(200); + expect(res.body.data).toEqual([{ id: 'usr_1', name: 'Ada', email: 'ada@example.com' }]); + // Resolved from `leadObject.fields.owner.reference` — not from the + // picker, which declares no object at all. + expect(findData).toHaveBeenCalledTimes(1); + expect(findData.mock.calls[0][0].object).toBe('sys_user'); + }); + + // Stored pre-fold rows never went through the alias table, so the legacy + // spellings are live data, not history. The fix EXTENDS the chain; one that + // replaced it would turn these three green cases into 500s. + const LEGACY_DEFS: Array<[string, Record]> = [ + ['referenceTo', { type: 'lookup', referenceTo: 'sys_user' }], + ['target', { type: 'lookup', target: 'sys_user' }], + ['options.objectName', { type: 'lookup', options: { objectName: 'sys_user' } }], + ]; + for (const [spelling, ownerDef] of LEGACY_DEFS) { + it(`a stored PRE-FOLD row spelling the target \`${spelling}\` still resolves`, async () => { + const stored = await savedWithoutObject(); + const legacyObject = { ...leadObject, fields: { ...leadObject.fields, owner: ownerDef } }; + const { findData, lookup } = routesOver(stored, [], legacyObject); + const res = mockRes(); + await lookup.handler({ params: { slug: 'contact', field: 'owner' }, query: {} } as any, res); + + expect(res.statusCode).toBe(200); + expect(findData.mock.calls[0][0].object).toBe('sys_user'); + }); + } + + it('the canonical `reference` WINS over a legacy spelling on the same def', async () => { + // Head-of-chain, not merely present-in-chain: a row carrying both (a + // partially-migrated def) must follow the canonical key. Appending + // `reference` to the tail of the chain would pass every case above and + // fail only this one. + const stored = await savedWithoutObject(); + const bothObject = { + ...leadObject, + fields: { ...leadObject.fields, owner: { type: 'lookup', reference: 'sys_user', referenceTo: 'stale_legacy' } }, + }; + const { findData, lookup } = routesOver(stored, [], bothObject); + const res = mockRes(); + await lookup.handler({ params: { slug: 'contact', field: 'owner' }, query: {} } as any, res); + + expect(res.statusCode).toBe(200); + expect(findData.mock.calls[0][0].object).toBe('sys_user'); + }); + + it('GUARD: a field def carrying NO target at all still answers 500 LOOKUP_TARGET_MISSING', async () => { + // The error did not become unreachable — it became RARE. Widening the + // chain must not make an unresolvable picker silently search nothing. + const stored = await savedWithoutObject(); + const targetless = { ...leadObject, fields: { ...leadObject.fields, owner: { type: 'lookup' } } }; + const { findData, lookup } = routesOver(stored, [], targetless); + const res = mockRes(); + await lookup.handler({ params: { slug: 'contact', field: 'owner' }, query: {} } as any, res); + + expect(res.statusCode).toBe(500); + expect(res.body.code).toBe('LOOKUP_TARGET_MISSING'); + expect(findData).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index cb2482bc6a..80b851aca3 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -7990,7 +7990,22 @@ export class RestServer { const items: any[] = Array.isArray(r?.items) ? r.items : Array.isArray(r) ? r : []; const obj = items.find((o: any) => o?.name === match.object); const def = obj?.fields?.[fieldName]; - referenceTo = def?.referenceTo ?? def?.target ?? def?.options?.objectName; + // [#7486] `reference` FIRST — it is the canonical + // key on `FieldSchema`, and `data/field.zod.ts` + // folds `relatedTo` / `referenceTo` / `target` / + // `targetObject` / `lookupObject` all onto it at + // parse. Reading only the legacy spellings meant a + // well-formed object schema carried NONE of them, + // the chain resolved `undefined`, and the route + // answered 500 — making `publicPicker.object` + // de-facto required while the schema and docs + // present it as an optional override. The legacy + // spellings stay after it for stored pre-fold rows, + // which never went through the alias table. + referenceTo = def?.reference + ?? def?.referenceTo + ?? def?.target + ?? def?.options?.objectName; } catch {/* ignore */} } if (!referenceTo) {