Skip to content

Commit 0b3be01

Browse files
baozhoutaoclaude
andauthored
fix(app-shell): give inline lookup action params a real record picker (#3405) (#2786)
An action param declared inline as `{ name: 'inspector', type: 'lookup', reference: 'sys_user' }` always rendered as a plain text input asking the user to paste a record id (UUID). A QC supervisor assigning an inspector had to go find that person's UUID by hand, while the very same reference field picks records by name in the create/edit dialog. `paramToField()` degrades a picker param to text when it has no `referenceTo` target, and `referenceTo` was only ever populated on the field-backed branch of `resolveActionParams()`. The inline branch dropped the authored `reference` key entirely — as did the spec schema, which stripped it as an unknown key without an error — so an inline picker could never reach `<LookupField>` no matter how it was authored. The user's config was correct and silently discarded. - `resolveActionParam()` maps an inline `reference` onto `referenceTo`: on the inline branch, on the missing-field fallback branch, and as an override on the field-backed branch (matching how every other inline value overrides the resolved field). - The text degradation warns in dev naming the offending param. With `@objectstack/spec` now rejecting a targetless inline picker at parse time, reaching that path means the metadata is broken, not merely partial. - The fallback's placeholder and help text no longer claim "a picker is coming soon" — it shipped long ago, and that copy misreported a dropped config as a missing feature. All 10 locales now say the param has no reference object configured. Verified against the framework showcase (examples/app-showcase) driven in a browser: the inline `p_account` param renders the searchable picker, queries `/api/v1/data/showcase_account`, filters server-side on typed input (incl. the CJK-named account), and resolves the selection by record id. Refs objectstack-ai/objectstack#3405 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent cc5eca9 commit 0b3be01

15 files changed

Lines changed: 162 additions & 22 deletions

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
"@object-ui/app-shell": patch
3+
"@object-ui/i18n": patch
4+
---
5+
6+
fix(app-shell): give inline `lookup` action params a real record picker (#3405)
7+
8+
An action parameter declared inline as `{ name: 'inspector', type: 'lookup',
9+
reference: 'sys_user' }` always rendered as a plain text input asking the user
10+
to paste a record id (UUID) — a supervisor assigning an inspector had to go
11+
find that person's UUID by hand, while the same reference field picks records
12+
by name in the create/edit dialog.
13+
14+
`paramToField()` degrades a picker param to text when it has no `referenceTo`
15+
target, and `referenceTo` was only ever populated on the field-backed branch of
16+
`resolveActionParams()`. The inline branch dropped the authored `reference`
17+
key entirely (as did the spec schema, which stripped it as unknown), so an
18+
inline picker could never reach `<LookupField>` no matter how it was authored.
19+
20+
- `resolveActionParam()` now maps an inline `reference` onto `referenceTo` — on
21+
the inline branch, on the missing-field fallback branch, and as an override
22+
on the field-backed branch (matching how every other inline value overrides
23+
the resolved field).
24+
- The text degradation now warns in dev naming the offending param, since with
25+
`@objectstack/spec` rejecting a targetless inline picker at parse time it
26+
means the metadata is broken, not merely partial.
27+
- The fallback's placeholder and help text no longer claim "a picker is coming
28+
soon" — the picker has shipped, and the message now says the parameter has no
29+
reference object configured. Updated across all 10 locales.

packages/app-shell/src/utils/paramToField.test.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* `FORM_FIELD_TYPES` + this drift test makes that class of bug impossible to
1616
* reintroduce silently.
1717
*/
18-
import { describe, it, expect } from 'vitest';
18+
import { describe, it, expect, vi } from 'vitest';
1919
import { FORM_FIELD_TYPES } from '@object-ui/fields';
2020
import type { ActionParamDef } from '@object-ui/core';
2121
import { paramToField, resolveParamWidgetType } from './paramToField';
@@ -130,6 +130,25 @@ describe('paramToField', () => {
130130
expect(paramToField(p({ type: 'reference' }))).toMatchObject({ type: 'text' });
131131
});
132132

133+
// #3405 — the fallback is now a broken-metadata signal, not a normal path,
134+
// so it must be audible in dev instead of silently handing the user a box
135+
// that wants a raw UUID.
136+
it('warns in dev when a picker param degrades for want of a target', () => {
137+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
138+
try {
139+
paramToField(p({ name: 'inspector', type: 'lookup' }));
140+
expect(warn).toHaveBeenCalledTimes(1);
141+
expect(warn.mock.calls[0][0]).toContain('inspector');
142+
expect(warn.mock.calls[0][0]).toContain('reference');
143+
144+
warn.mockClear();
145+
paramToField(p({ name: 'inspector', type: 'lookup', referenceTo: 'sys_user' }));
146+
expect(warn).not.toHaveBeenCalled();
147+
} finally {
148+
warn.mockRestore();
149+
}
150+
});
151+
133152
it('user params keep their picker without needing referenceTo (implicit sys_user)', () => {
134153
expect(paramToField(p({ type: 'user' }))).toMatchObject({ type: 'user' });
135154
});

packages/app-shell/src/utils/paramToField.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,23 @@ const LOOKUP_WIDGET_TYPES = new Set(['lookup', 'master_detail']);
5353
* `referenceTo` target renders as a plain text input (the picker cannot query
5454
* without a target object) — preserving the dialog's long-standing behavior
5555
* for partially-resolved metadata.
56+
*
57+
* That fallback is now a last resort, not an expected path (#3405): inline
58+
* params declare `reference` and field-backed ones inherit it, and the spec
59+
* rejects a targetless inline picker at parse time. Reaching it means the
60+
* metadata is broken or partial, so say so in dev instead of silently handing
61+
* the user a box that wants a raw UUID.
5662
*/
5763
export function paramToField(param: ActionParamDef): Record<string, any> {
5864
let type = resolveParamWidgetType(param.type);
5965
if (LOOKUP_WIDGET_TYPES.has(type) && !param.referenceTo) {
66+
if (process.env.NODE_ENV !== 'production') {
67+
console.warn(
68+
`[ActionParamDialog] Param "${param.name}" is type "${param.type}" but has no reference target, ` +
69+
'so it degrades to a plain record-id text input. Declare `reference: \'<object>\'` on the param, ' +
70+
'or make it field-backed (`{ field: \'<lookup_field>\' }`) to inherit the target.',
71+
);
72+
}
6073
type = 'text';
6174
}
6275

packages/app-shell/src/utils/resolveActionParams.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,3 +130,66 @@ describe('resolveActionParams — widget config (ADR-0059)', () => {
130130
});
131131
});
132132
});
133+
134+
/**
135+
* #3405 — an inline `lookup` param carries its own picker target.
136+
*
137+
* Real-world break (PLAT-DEF-005): a QC dispatch action declared
138+
* `{ name: 'inspector', type: 'lookup', reference: 'sys_user' }`. The key was
139+
* unknown to both the spec schema and this resolver, so it was dropped twice
140+
* over and `paramToField()` degraded the param to a "paste the record id
141+
* (UUID)" text box — a supervisor had to go find a user's UUID by hand.
142+
*/
143+
describe('resolveActionParams — inline lookup reference target (#3405)', () => {
144+
const lookupCtx = () =>
145+
ctx({
146+
objects: [
147+
{
148+
name: 'quality_dispatch',
149+
fields: {
150+
inspector: { type: 'lookup', label: '质检人', reference: 'sys_user' },
151+
reviewer: { type: 'lookup', label: 'Reviewer', reference_to: 'sys_user', display_field: 'name' },
152+
},
153+
},
154+
],
155+
objectName: 'quality_dispatch',
156+
});
157+
158+
it('maps an inline `reference` onto referenceTo so the picker can query', () => {
159+
const params: RawActionParam[] = [
160+
{ name: 'inspector', label: '质检员', type: 'lookup', reference: 'sys_user', required: true },
161+
];
162+
expect(resolveActionParams(params, lookupCtx())[0]).toMatchObject({
163+
name: 'inspector',
164+
type: 'lookup',
165+
referenceTo: 'sys_user',
166+
});
167+
});
168+
169+
it('still inherits the target from the referenced field when field-backed', () => {
170+
expect(resolveActionParams([{ field: 'inspector' }], lookupCtx())[0]).toMatchObject({
171+
type: 'lookup',
172+
referenceTo: 'sys_user',
173+
});
174+
expect(resolveActionParams([{ field: 'reviewer' }], lookupCtx())[0]).toMatchObject({
175+
referenceTo: 'sys_user',
176+
displayField: 'name',
177+
});
178+
});
179+
180+
it('lets an inline reference override the field metadata', () => {
181+
const params: RawActionParam[] = [{ field: 'inspector', reference: 'sys_member' }];
182+
expect(resolveActionParams(params, lookupCtx())[0].referenceTo).toBe('sys_member');
183+
});
184+
185+
it('keeps the inline reference on the missing-field fallback branch', () => {
186+
const params: RawActionParam[] = [
187+
{ field: 'does_not_exist', type: 'lookup', reference: 'sys_user' },
188+
];
189+
expect(resolveActionParams(params, lookupCtx())[0].referenceTo).toBe('sys_user');
190+
});
191+
192+
it('leaves referenceTo undefined for a non-picker inline param', () => {
193+
expect(resolveActionParams([{ name: 'note', type: 'textarea' }], lookupCtx())[0].referenceTo).toBeUndefined();
194+
});
195+
});

packages/app-shell/src/utils/resolveActionParams.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,13 @@ export interface RawActionParam {
4343
accept?: string[];
4444
/** Max upload size in bytes for `file`/`image` params. */
4545
maxSize?: number;
46+
/**
47+
* Reference target for an INLINE `lookup`/`master_detail` param (#3405) —
48+
* the object whose records the picker searches. Field-backed params inherit
49+
* it from the referenced field instead (see `lookupExtras` below). Spelled
50+
* `reference` to match `FieldSchema.reference` / `ActionParamSchema.reference`.
51+
*/
52+
reference?: string;
4653
/**
4754
* Visibility predicate (CEL) — mirrors the spec `ActionParamSchema.visible`.
4855
* The server serialises it through `ExpressionInputSchema` as an
@@ -172,6 +179,10 @@ export function resolveActionParam(
172179
multiple: param.multiple,
173180
accept: param.accept,
174181
maxSize: param.maxSize,
182+
// Inline picker target (#3405). Without this an inline `lookup` param
183+
// could never reach `<LookupField>` — `paramToField()` degrades a
184+
// targetless picker to a raw record-id text input.
185+
referenceTo: param.reference,
175186
};
176187
}
177188

@@ -196,6 +207,9 @@ export function resolveActionParam(
196207
multiple: param.multiple,
197208
accept: param.accept,
198209
maxSize: param.maxSize,
210+
// The field is unresolvable, so an inline `reference` is the only picker
211+
// target available — keep it rather than dropping to a text input.
212+
referenceTo: param.reference,
199213
};
200214
}
201215

@@ -211,7 +225,9 @@ export function resolveActionParam(
211225
const isLookupResolvedType = resolvedType === 'lookup' || resolvedType === 'reference';
212226
const lookupExtras: Partial<ActionParamDef> = isLookupResolvedType
213227
? {
214-
referenceTo: field.reference_to ?? field.reference,
228+
// Inline `reference` wins, matching how every other inline value
229+
// overrides the resolved field (#3405).
230+
referenceTo: param.reference ?? field.reference_to ?? field.reference,
215231
displayField: field.display_field ?? field.reference_field,
216232
idField: field.id_field,
217233
descriptionField: field.description_field,

packages/i18n/src/locales/ar.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1496,8 +1496,8 @@ const ar = {
14961496
uploading: "جارٍ الرفع…",
14971497
defaultActionTitle: "إجراء",
14981498
ok: "موافق",
1499-
lookupPlaceholder: "لصق معرف السجل (UUID) لـ {{label}}",
1500-
lookupHelpText: "أدخل معرف سجل الكائن المرجع. سيتم إضافة أداة اختيار قريباً.",
1499+
lookupPlaceholder: "معرف السجل لـ {{label}}",
1500+
lookupHelpText: "لم يتم تكوين كائن مرجعي لهذه المعلمة، لذا فإن أداة اختيار السجلات غير متاحة. أدخل معرف السجل، أو اطلب من المسؤول تصحيح معلمة الإجراء.",
15011501
},
15021502
actionConfirm: {
15031503
title: "تأكيد الإجراء",

packages/i18n/src/locales/de.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1498,8 +1498,8 @@ const de = {
14981498
uploading: "Wird hochgeladen…",
14991499
defaultActionTitle: "Aktion",
15001500
ok: "OK",
1501-
lookupPlaceholder: "Datensatz-ID (UUID) für {{label}} einfügen",
1502-
lookupHelpText: "Geben Sie die Datensatz-ID des referenzierten Objekts ein. Eine Auswahlhilfe kommt bald.",
1501+
lookupPlaceholder: "Datensatz-ID für {{label}}",
1502+
lookupHelpText: "Für diesen Parameter ist kein Referenzobjekt konfiguriert, daher ist die Datensatzauswahl nicht verfügbar. Geben Sie eine Datensatz-ID ein oder bitten Sie einen Administrator, den Aktionsparameter zu korrigieren.",
15031503
},
15041504
actionConfirm: {
15051505
title: "Aktion bestätigen",

packages/i18n/src/locales/en.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2135,8 +2135,8 @@ const en = {
21352135
description: 'Please provide the required information to continue.',
21362136
selectPlaceholder: 'Select {{label}}',
21372137
requiredError: '{{label}} is required',
2138-
lookupPlaceholder: 'Paste the {{label}} record id (UUID)',
2139-
lookupHelpText: 'Enter the record id of the referenced object. A picker is coming soon.',
2138+
lookupPlaceholder: 'Record id for {{label}}',
2139+
lookupHelpText: 'No reference object is configured for this parameter, so the record picker is unavailable. Enter a record id, or ask an administrator to fix the action parameter.',
21402140
cancel: 'Cancel',
21412141
confirm: 'Confirm',
21422142
uploading: 'Uploading…',

packages/i18n/src/locales/es.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1498,8 +1498,8 @@ const es = {
14981498
uploading: "Subiendo…",
14991499
defaultActionTitle: "Acción",
15001500
ok: "Aceptar",
1501-
lookupPlaceholder: "Pegar ID de registro (UUID) para {{label}}",
1502-
lookupHelpText: "Ingrese el ID del registro del objeto referenciado. Pronto se añadirá un selector.",
1501+
lookupPlaceholder: "ID de registro para {{label}}",
1502+
lookupHelpText: "Este parámetro no tiene un objeto de referencia configurado, por lo que el selector de registros no está disponible. Ingrese un ID de registro o pida a un administrador que corrija el parámetro de la acción.",
15031503
},
15041504
actionConfirm: {
15051505
title: "Confirmar acción",

packages/i18n/src/locales/fr.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1496,8 +1496,8 @@ const fr = {
14961496
uploading: "Téléversement…",
14971497
defaultActionTitle: "Action",
14981498
ok: "OK",
1499-
lookupPlaceholder: "Coller l'ID d'enregistrement (UUID) pour {{label}}",
1500-
lookupHelpText: "Entrez l'ID de l'enregistrement de l'objet référencé. Un sélecteur sera bientôt ajouté.",
1499+
lookupPlaceholder: "ID d'enregistrement pour {{label}}",
1500+
lookupHelpText: "Aucun objet de référence n'est configuré pour ce paramètre, le sélecteur d'enregistrement est donc indisponible. Saisissez un ID d'enregistrement ou demandez à un administrateur de corriger le paramètre d'action.",
15011501
},
15021502
actionConfirm: {
15031503
title: "Confirmer l’action",

0 commit comments

Comments
 (0)