Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions .changeset/public-lookup-canonical-reference.md
Original file line numberDiff line numberDiff line change
@@ -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.
4 changes: 2 additions & 2 deletions content/docs/ui/forms.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 fielddefinition. |
| `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
Expand All@@ -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

Expand Down
113 changes: 106 additions & 7 deletions packages/rest/src/public-form-lookup-picker.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -81,10 +81,12 @@ async function persistedBody(item: unknown): Promise<any> {

/**
* 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'],
Expand DownExpand Up@@ -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: {} }),
Expand DownExpand Up@@ -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<string, unknown>]> = [
['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();
});
});
17 changes: 16 additions & 1 deletion packages/rest/src/rest-server.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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) {
Expand Down
Loading