From 8f70a8b7402bc0e919fc2926ad1d4f3c41609bbf Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 17:43:03 +0000 Subject: [PATCH 1/6] fix(fields): lookup candidate queries expand the reference columns they display (objectui#10223) Opening a lookup's dropdown sent a candidate query with no `$expand`, so every previewed lookup / master_detail column came back as a bare foreign key and the lookup cell renderer resolved each one with its own `findOne`: one request per candidate per such column on every open. The browse-all picker had the same shape. Both candidate queries now ask for `$expand` over the reference columns they display, by `buildExpandFields`' rule (the inline dropdown's previewed columns; the picker's rendered columns minus the id column). The recents rail asks for the same expansion as the main list. Expansion stays a display concern: options, the `titleFormat` reading of a row and the records handed to `onSelectRecord` / `onSelectRecords` are built from the row with its relations collapsed back to ids (core's `toPredicateRecord`), so labels, committed values and host payloads read as before; only the previews and table cells render the expanded record. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C --- ...LookupField.candidateExpand-10223.test.tsx | 340 ++++++++++++++++++ packages/fields/src/widgets/LookupField.tsx | 102 +++++- .../fields/src/widgets/RecordPickerDialog.tsx | 55 ++- 3 files changed, 467 insertions(+), 30 deletions(-) create mode 100644 packages/fields/src/widgets/LookupField.candidateExpand-10223.test.tsx diff --git a/packages/fields/src/widgets/LookupField.candidateExpand-10223.test.tsx b/packages/fields/src/widgets/LookupField.candidateExpand-10223.test.tsx new file mode 100644 index 0000000000..592ae6b204 --- /dev/null +++ b/packages/fields/src/widgets/LookupField.candidateExpand-10223.test.tsx @@ -0,0 +1,340 @@ +/** + * 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. + */ + +/** + * One dropdown open, one candidate query — objectui#10223. + * + * Reported from a real deployment: a scheduling form's lookup to a + * `task_version` object, whose `highlightFields` include the master_detail + * field `task`. The lookup declares no `lookup_columns`, so the dropdown + * previews the first highlight columns — `task` among them. The candidate + * query sent no `$expand`, every candidate's `task` came back as a bare + * foreign key, and the lookup cell renderer resolved each one with its own + * `findOne`: one request per candidate on every open, on top of the list + * query. The display was right; the load was not. + * + * The fixture mirrors that shape: a full page of candidates, one master_detail + * column inside the derived preview columns, no `lookup_columns`. What is + * pinned: + * + * - the open costs ONE candidate query, and it carries `$expand` for exactly + * the previewed reference column — no per-row `findOne` follows, and the + * subtitle still names each related record; + * - the control: with no reference column previewed the query carries no + * `$expand` key at all; + * - the browse-all picker behind the dropdown, the same way; + * - expansion changes nothing a host or a user reads besides the request + * count. The option labels (including a `titleFormat` template that names + * the expanded field), the preview text, and the record handed to + * `onSelectRecord` / `onSelectRecords` are the same whether or not the + * backend honours `$expand` — the second backend returns bare ids, which is + * also the fallback for a backend that ignores the parameter. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor, act, fireEvent } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { SchemaRendererContext } from '@object-ui/react'; +import { LookupField } from './LookupField'; +import { RecordPickerDialog } from './RecordPickerDialog'; +import { getCellRenderer } from '../index'; + +/** A full dropdown page — the page size the inline dropdown asks for. */ +const CANDIDATES = 50; + +const TASK_VERSION_FIELDS: Record = { + name: { type: 'text', label: 'Name' }, + code: { type: 'text', label: 'Code' }, + task: { type: 'master_detail', label: 'Task', reference_to: 'task' }, + version: { type: 'number', label: 'Version' }, +}; + +interface BackendOptions { + /** + * Unique per test. The lookup cell renderer's name cache is module-level, so + * a task id resolved by one test would read as "no fetch needed" in the next. + */ + prefix: string; + highlightFields: string[]; + /** A backend that ignores `$expand` returns bare foreign keys. */ + honoursExpand?: boolean; + titleFormat?: string; +} + +function makeBackend({ prefix, highlightFields, honoursExpand = true, titleFormat }: BackendOptions) { + const tasks: Record = {}; + const rows: Record[] = []; + for (let i = 0; i < CANDIDATES; i++) { + const taskId = `${prefix}_task_${i}`; + tasks[taskId] = { id: taskId, name: `Assembly step ${i}` }; + rows.push({ id: `${prefix}_tv_${i}`, name: `TV-${i}`, code: `C-${i}`, task: taskId, version: i + 1 }); + } + + const find = vi.fn(async (objectName: string, params?: Record) => { + if (objectName !== 'task_version') return { data: [], total: 0 }; + const expand: string[] = honoursExpand && Array.isArray(params?.$expand) ? params!.$expand : []; + const skip = Number(params?.$skip ?? 0); + const top = Number(params?.$top ?? rows.length); + const data = rows + .slice(skip, skip + top) + .map((r) => (expand.includes('task') ? { ...r, task: tasks[r.task] } : { ...r })); + return { data, total: rows.length }; + }); + const findOne = vi.fn(async (objectName: string, id: string) => + objectName === 'task' ? tasks[id] ?? null : null, + ); + const getObjectSchema = vi.fn(async (objectName: string) => { + if (objectName === 'task_version') { + return { + name: 'task_version', + fields: TASK_VERSION_FIELDS, + highlightFields, + ...(titleFormat ? { titleFormat } : {}), + }; + } + if (objectName === 'task') { + return { name: 'task', fields: { name: { type: 'text', label: 'Name' } } }; + } + return undefined; + }); + + return { dataSource: { find, findOne, getObjectSchema } as any, rows }; +} + +type Backend = ReturnType; + +/** The candidate queries — `find` on the referenced object itself. */ +function candidateQueries(backend: Backend): Array> { + return backend.dataSource.find.mock.calls + .filter(([objectName]: [string]) => objectName === 'task_version') + .map(([, params]: [string, Record]) => params); +} + +/** Every read that is NOT a candidate query: per-row resolution of `task`. */ +function perRowReads(backend: Backend): number { + const finds = backend.dataSource.find.mock.calls.filter( + ([objectName]: [string]) => objectName !== 'task_version', + ).length; + return finds + backend.dataSource.findOne.mock.calls.length; +} + +/** + * Macrotask flushes, as in `LookupField.pickerAgreement.test.tsx`: the schema + * fetches, any per-id resolution and the re-render that carries a resolved + * name all settle, without a predicate that would have to encode the answer. + */ +async function settle(): Promise { + for (let i = 0; i < 6; i++) { + await act(async () => { + await new Promise((r) => setTimeout(r, 0)); + }); + } +} + +/** + * Mount the lookup the way a form does and wait for the referenced schema, + * which lands at mount — before any user can reach the trigger. + */ +async function mountLookup(backend: Backend, extra: Record = {}): Promise { + render( + + {}} + dataSource={backend.dataSource} + field={{ reference_to: 'task_version' } as never} + {...(extra as object)} + /> + , + ); + await waitFor(() => expect(backend.dataSource.getObjectSchema).toHaveBeenCalledWith('task_version')); + await settle(); +} + +async function openDropdown(backend: Backend, extra: Record = {}): Promise { + await mountLookup(backend, extra); + await act(async () => { + fireEvent.click(screen.getByTestId('lookup-trigger')); + }); + await waitFor(() => expect(screen.getAllByRole('option')).toHaveLength(CANDIDATES)); + await settle(); +} + +function previewTexts(field: string): string[] { + return Array.from(document.querySelectorAll(`[data-lookup-preview="${field}"]`)).map( + (el) => el.textContent ?? '', + ); +} + +afterEach(() => { + cleanup(); + // Selecting an option records it as "recently used"; keep tests independent. + try { + localStorage.clear(); + } catch { + /* no storage in this environment */ + } +}); + +describe('LookupField — the dropdown expands the reference columns it previews (objectui#10223)', () => { + it('one open costs one candidate query carrying `$expand`, and no per-row read', async () => { + const backend = makeBackend({ prefix: 'card', highlightFields: ['code', 'task', 'version'] }); + await openDropdown(backend); + + const queries = candidateQueries(backend); + expect(queries).toHaveLength(1); + expect(queries[0].$expand).toEqual(['task']); + expect(perRowReads(backend)).toBe(0); + + // The subtitle still names every related record. + const tasks = previewTexts('task'); + expect(tasks).toHaveLength(CANDIDATES); + expect(tasks[0]).toBe('Assembly step 0'); + expect(tasks[CANDIDATES - 1]).toBe(`Assembly step ${CANDIDATES - 1}`); + }); + + it('control: with no reference column previewed, the query carries no `$expand` key', async () => { + const backend = makeBackend({ prefix: 'ctrl', highlightFields: ['code', 'version'] }); + await openDropdown(backend); + + const queries = candidateQueries(backend); + expect(queries).toHaveLength(1); + expect('$expand' in queries[0]).toBe(false); + expect(perRowReads(backend)).toBe(0); + expect(previewTexts('task')).toHaveLength(0); + expect(previewTexts('code')[0]).toBe('C-0'); + }); + + it('the browse-all picker behind it expands the same column, and reads nothing per row', async () => { + const backend = makeBackend({ prefix: 'pick', highlightFields: ['code', 'task', 'version'] }); + await mountLookup(backend); + await act(async () => { + fireEvent.click(screen.getByTestId('browse-all-records')); + }); + await waitFor(() => expect(screen.getByText('TV-0')).toBeInTheDocument()); + await settle(); + + const queries = candidateQueries(backend); + expect(queries).toHaveLength(1); + expect(queries[0].$expand).toEqual(['task']); + expect(perRowReads(backend)).toBe(0); + + const cells = Array.from(document.querySelectorAll('[data-lookup-cell="task"]')); + expect(cells.length).toBeGreaterThan(0); + expect(cells[0].textContent).toBe('Assembly step 0'); + }); +}); + +describe('LookupField — expansion changes the request count and nothing else (objectui#10223)', () => { + /** + * `titleFormat` names the expanded field on purpose: a template that + * printed an expanded record would print an object where it used to print + * the key. + */ + const TITLE_FORMAT = '{code} - {task}'; + + interface DropdownReading { + labels: string[]; + previews: string[]; + picked: Record; + } + + async function readDropdown(honoursExpand: boolean): Promise { + const backend = makeBackend({ + prefix: 'same', + highlightFields: ['code', 'task', 'version'], + honoursExpand, + titleFormat: TITLE_FORMAT, + }); + const onSelectRecord = vi.fn(); + await openDropdown(backend, { onSelectRecord }); + const options = screen.getAllByRole('option').slice(0, 3); + const labels = options.map((o) => o.getAttribute('title') ?? ''); + const previews = previewTexts('task').slice(0, 3); + await act(async () => { + fireEvent.click(options[0]); + }); + expect(onSelectRecord).toHaveBeenCalledTimes(1); + const picked = onSelectRecord.mock.calls[0][0] as Record; + cleanup(); + localStorage.clear(); + return { labels, previews, picked }; + } + + it('dropdown: same labels, same subtitles, same record handed to onSelectRecord', async () => { + // The expanding backend first: it resolves nothing per row, so it leaves + // the module-level name cache empty for the bare-id run that follows. + const expanded = await readDropdown(true); + const bare = await readDropdown(false); + + expect(expanded).toEqual(bare); + expect(expanded.labels[0]).toBe('C-0 - same_task_0'); + expect(expanded.previews[0]).toBe('Assembly step 0'); + expect(expanded.picked.task).toBe('same_task_0'); + }); + + interface PickerReading { + titles: string[]; + tasks: string[]; + picked: Record[]; + } + + async function readPicker(honoursExpand: boolean): Promise { + const backend = makeBackend({ + prefix: 'samepick', + highlightFields: ['code', 'task', 'version'], + honoursExpand, + }); + const onSelectRecords = vi.fn(); + render( + + {}} + dataSource={backend.dataSource} + objectName="task_version" + displayField="name" + titleFormat={TITLE_FORMAT} + columns={['name', 'code', 'task', 'version']} + onSelect={() => {}} + onSelectRecords={onSelectRecords} + cellRenderer={getCellRenderer} + fieldsMeta={TASK_VERSION_FIELDS} + /> + , + ); + await waitFor(() => expect(screen.getByTestId('record-row-samepick_tv_0')).toBeInTheDocument()); + await settle(); + const read = (field: string) => + Array.from(document.querySelectorAll(`[data-lookup-cell="${field}"]`)) + .slice(0, 3) + .map((el) => el.textContent ?? ''); + const titles = read('name'); + const tasks = read('task'); + await act(async () => { + fireEvent.click(screen.getByTestId('record-row-samepick_tv_0')); + }); + expect(onSelectRecords).toHaveBeenCalledTimes(1); + const picked = onSelectRecords.mock.calls[0][0] as Record[]; + cleanup(); + return { titles, tasks, picked }; + } + + it('picker: same title cells, same task cells, same records handed to onSelectRecords', async () => { + const expanded = await readPicker(true); + const bare = await readPicker(false); + + expect(expanded).toEqual(bare); + expect(expanded.titles[0]).toBe('C-0 - samepick_task_0'); + expect(expanded.tasks[0]).toBe('Assembly step 0'); + expect(expanded.picked).toEqual([ + { id: 'samepick_tv_0', name: 'TV-0', code: 'C-0', task: 'samepick_task_0', version: 1 }, + ]); + }); +}); diff --git a/packages/fields/src/widgets/LookupField.tsx b/packages/fields/src/widgets/LookupField.tsx index f00c2301d3..8c5869aebc 100644 --- a/packages/fields/src/widgets/LookupField.tsx +++ b/packages/fields/src/widgets/LookupField.tsx @@ -18,7 +18,7 @@ import type { RecordPickerFilterColumn } from './RecordPickerDialog.js'; import { PeoplePicker } from './PeoplePicker.js'; import { useRecordQuery } from './useRecordQuery.js'; import { deriveLookupColumns } from './deriveLookupColumns.js'; -import { getRecordDisplayName, mergeFilterNodes } from '@object-ui/core'; +import { buildExpandFields, getRecordDisplayName, mergeFilterNodes, toPredicateRecord } from '@object-ui/core'; import { getRecentLookupIds, pushRecentLookupId } from './recentLookups.js'; import { getPersonInitials } from './personDisplay.js'; import { getCellRendererResolver } from './_cell-renderer-bridge.js'; @@ -496,6 +496,30 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro [previewColumns, refObjectSchema, referenceTo, translateOptions], ); + /** + * `$expand` for the candidate queries (objectui#10223): the reference columns + * among the ones this dropdown PREVIEWS, by `buildExpandFields`' rule — the + * same one the list views use for their visible columns. + * + * Without it every previewed `lookup` / `master_detail` column arrived as a + * bare foreign key, and the lookup cell renderer resolved each one with its + * own `findOne` — one request per candidate per such column, on every open. + * An expanded value is rendered by that same cell renderer with no fetch, so + * the preview reads the same and the per-row requests go. + * + * Expansion is a DISPLAY concern here and stays one: options are built from + * the row with its relations collapsed back to ids (`toPredicateRecord`), so + * the label, the committed value and the record `onSelectRecord` hands a host + * are what they were before any column was expanded. Only the preview reads + * the expanded row (`previewRows` below). A backend that ignores `$expand` + * returns bare ids, and the cell renderer's per-id resolution takes over as + * before. + */ + const candidateExpand = useMemo( + () => buildExpandFields(refObjectSchema?.fields, previewColumns), + [refObjectSchema, previewColumns], + ); + // Derive filter-bar columns from any typed picker columns. const filterColumns = useMemo(() => { if (!pickerColumns) return undefined; @@ -548,6 +572,7 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro enabled: isOpen && hasDataSource && !dependenciesMissing, pageSize: LOOKUP_PAGE_SIZE, filter: popoverFilter, + expand: candidateExpand, }); // Re-source the popover's fetch state from the kernel; all existing read sites @@ -556,10 +581,15 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro const loading = popoverQuery.loading; const totalCount = popoverQuery.total; const error = popoverQuery.error ?? createError; + // Built from the row with its relations collapsed to ids — see + // `candidateExpand` for why the option never sees the expanded form. const fetchedOptions = useMemo( () => popoverQuery.records.map(r => - recordToOption(r, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema), + recordToOption( + toPredicateRecord(r, refObjectSchema?.fields), + displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema, + ), ), [popoverQuery.records, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema], ); @@ -894,12 +924,20 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro // filtered response; ids it does not return are dropped, whether they fail // the filters or no longer exist. It is also one request instead of up to // MAX_RECENT serial round-trips. - const [recentOptions, setRecentOptions] = useState([]); + // + // The rail previews the same columns as the main list, so it asks for the + // same `$expand` (objectui#10223). It keeps the rows it was served — the + // preview reads them — and derives its options from them exactly as the main + // list does, relations collapsed to ids. + const [recentRows, setRecentRows] = useState([]); + // The expansion as a primitive, for the effect below: a memoised array's + // identity is not a dependency to key a fetch on (AGENTS.md #10). + const candidateExpandKey = candidateExpand.join(','); useEffect(() => { if (!isOpen || !hasDataSource || !dataSource || !referenceTo || searchQuery) return; - if (dependenciesMissing) { setRecentOptions([]); return; } + if (dependenciesMissing) { setRecentRows([]); return; } const ids = getRecentLookupIds(referenceTo); - if (!ids.length) { setRecentOptions([]); return; } + if (!ids.length) { setRecentRows([]); return; } let cancelled = false; (async () => { try { @@ -909,29 +947,53 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro ? mergeFilterNodes(popoverFilter, idRestriction) : idRestriction, $top: ids.length, + ...(candidateExpand.length > 0 ? { $expand: candidateExpand } : {}), } as QueryParams); const rows = (res as any)?.data ?? res ?? []; const byId = new Map(); if (Array.isArray(rows)) { for (const row of rows) { - const rid = row?.[idField] ?? row?.id ?? row?._id; + const plain = toPredicateRecord(row, refObjectSchema?.fields); + const rid = plain?.[idField] ?? plain?.id ?? plain?._id; if (rid !== undefined && rid !== null) byId.set(String(rid), row); } } // Most-recent-first order is preserved; the response only decides // WHICH ids survive, never their order. - const recs = ids - .map((id) => byId.get(String(id))) - .filter(Boolean) - .map((r) => - recordToOption(r, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema), - ); - if (!cancelled) setRecentOptions(recs); - } catch { if (!cancelled) setRecentOptions([]); } + const kept = ids.map((id) => byId.get(String(id))).filter(Boolean); + if (!cancelled) setRecentRows(kept); + } catch { if (!cancelled) setRecentRows([]); } })(); return () => { cancelled = true; }; // eslint-disable-next-line react-hooks/exhaustive-deps - }, [isOpen, hasDataSource, referenceTo, searchQuery, dependenciesMissing, popoverFilter, idField]); + }, [isOpen, hasDataSource, referenceTo, searchQuery, dependenciesMissing, popoverFilter, idField, candidateExpandKey]); + const recentOptions = useMemo( + () => + recentRows.map((r) => + recordToOption( + toPredicateRecord(r, refObjectSchema?.fields), + displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema, + ), + ), + [recentRows, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema], + ); + + /** + * The rows previews render from (objectui#10223): each as the server returned + * it, with any `$expand`-ed relation still expanded, so the lookup cell + * renderer names it without a fetch. Keyed by the option's value (a + * primitive, never an option object's identity). An option with no served + * row — a static option, a just-created record — previews from itself. + */ + const previewRows = useMemo(() => { + const byValue = new Map(); + for (const raw of [...recentRows, ...popoverQuery.records]) { + const plain = toPredicateRecord(raw, refObjectSchema?.fields); + const v = plain?.[idField] ?? plain?.id ?? plain?._id ?? plain?.externalId; + if (v !== undefined && v !== null && !byValue.has(String(v))) byValue.set(String(v), raw); + } + return byValue; + }, [recentRows, popoverQuery.records, refObjectSchema, idField]); // Recently-used first (only before the user types), then live results — one // de-duped list that drives BOTH rendering and arrow-key navigation. @@ -1028,8 +1090,12 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro */ const previewOf = useCallback( (option: LookupOption): React.ReactNode => { + // The option with its relations as the server served them — every other + // key reads exactly as it did before `$expand` (objectui#10223). + const served = previewRows.get(String(option.value)); + const row = served ? { ...option, ...served } : option; const cols = previewColumns.filter((c) => { - const v = (option as any)[c.field]; + const v = (row as any)[c.field]; return v !== null && v !== undefined && v !== ''; }); if (cols.length === 0) return null; @@ -1039,7 +1105,7 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro {`${col.label || fieldToLabel(col.field)}:`} - {renderLookupColumnValue(option, col, { + {renderLookupColumnValue(row, col, { descriptors: previewDescriptors, cellRenderer: getCellRendererResolver(), displayLocale, @@ -1049,7 +1115,7 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro )); }, - [previewColumns, previewDescriptors, displayLocale], + [previewRows, previewColumns, previewDescriptors, displayLocale], ); // Keyboard handler for the search input — arrow keys + Enter diff --git a/packages/fields/src/widgets/RecordPickerDialog.tsx b/packages/fields/src/widgets/RecordPickerDialog.tsx index 123a3efa22..3b1e44581f 100644 --- a/packages/fields/src/widgets/RecordPickerDialog.tsx +++ b/packages/fields/src/widgets/RecordPickerDialog.tsx @@ -40,7 +40,7 @@ import type { DataSource, LookupColumnDef, LookupFilterDef } from '@object-ui/ty // The repo's single filter sink (`packages/core/src/utils/filter-converter.ts`) // — shared with plugin-list's `buildEffectiveFilter` and plugin-view's // ObjectView, so a spec `ViewFilterRule[]` lowers in exactly one place. -import { mergeFilterNodes } from '@object-ui/core'; +import { buildExpandFields, mergeFilterNodes, toPredicateRecord } from '@object-ui/core'; import { useSafeFieldLabel, useDisplayLocale } from '@object-ui/i18n'; import { useFieldTranslation } from './useFieldTranslation.js'; import { useRecordQuery } from './useRecordQuery.js'; @@ -499,6 +499,26 @@ export function RecordPickerDialog({ [resolvedColumns, fieldsMeta, objectName, translateOptions], ); + /** + * `$expand` for the picker's query (objectui#10223): the reference columns + * among the ones this table renders, by `buildExpandFields`' rule — the one + * LookupField's inline dropdown applies to its previewed columns. Without it + * each such cell arrived as a bare foreign key and the lookup cell renderer + * resolved it with its own `findOne`, one request per row per column. + * + * The id column is left out: the row's identity (`getRecordId`) reads it + * raw, so it must stay the key it always was. + * + * The table renders the rows as served. What leaves it — the records + * `onSelectRecords` hands a host, and the `titleFormat` template's reading + * of a row — sees the row with its relations collapsed to ids + * (`toPredicateRecord`), as it did before any column was expanded. + */ + const expand = useMemo( + () => buildExpandFields(fieldsMeta, resolvedColumns.filter((c) => c.field !== idField)), + [fieldsMeta, resolvedColumns, idField], + ); + // Auto-generate filter columns from lookupFilters when no explicit filterColumns given. // Each LookupFilterDef becomes a filterable field with inferred type. const baseFilterColumns = useMemo(() => { @@ -600,6 +620,7 @@ export function RecordPickerDialog({ pageSize, paginate: true, filter: mergedFilter, + expand, }); // Preserve the previous local names so the handlers and render below are // unchanged (the migration is a pure refactor). @@ -670,6 +691,8 @@ export function RecordPickerDialog({ const handleRowClick = useCallback( (record: any) => { const rid = getRecordId(record); + // A host receives the row as it was before `$expand` (objectui#10223). + const selected = toPredicateRecord(record, fieldsMeta); if (multiple) { setPendingSelection(prev => { @@ -679,18 +702,18 @@ export function RecordPickerDialog({ selectedRecordsMap.current.delete(rid); } else { next.add(rid); - selectedRecordsMap.current.set(rid, record); + selectedRecordsMap.current.set(rid, selected); } return next; }); } else { // Single select — immediately close onSelect(rid); - onSelectRecords?.([record]); + onSelectRecords?.([selected]); onOpenChange(false); } }, - [multiple, getRecordId, onSelect, onSelectRecords, onOpenChange], + [multiple, getRecordId, fieldsMeta, onSelect, onSelectRecords, onOpenChange], ); // Confirm multi-select @@ -762,16 +785,24 @@ export function RecordPickerDialog({ // renderer (objectui#5492) — the inline dropdown in LookupField calls the // very same function, so this table and that popover cannot answer one // `lookup_columns` declaration two different ways. + // + // The display column's `titleFormat` template reads the row with its + // relations collapsed to ids, so an expanded reference it names prints what + // it printed before `$expand` rather than an object (objectui#10223). const renderCellContent = useCallback( (record: any, col: LookupColumnDef): React.ReactNode => - renderLookupColumnValue(record, col, { - descriptors: columnFieldDescriptors, - cellRenderer, - titleFormat, - displayField, - displayLocale, - }), - [cellRenderer, titleFormat, displayField, columnFieldDescriptors, displayLocale], + renderLookupColumnValue( + titleFormat && col.field === displayField ? toPredicateRecord(record, fieldsMeta) : record, + col, + { + descriptors: columnFieldDescriptors, + cellRenderer, + titleFormat, + displayField, + displayLocale, + }, + ), + [cellRenderer, titleFormat, displayField, fieldsMeta, columnFieldDescriptors, displayLocale], ); // Render sort indicator for a column From 33e0b130d7284fcd811b52aaa1600ae688f04267 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 18:13:49 +0000 Subject: [PATCH 2/6] test(fields): pin the user-column expansion; changeset; correct the picker-agreement prose (objectui#10223) - The candidate-expand suite now also pins that a previewed `user` column rides `buildExpandFields`' rule and names the person. - `LookupField.pickerAgreement.test.tsx` said neither surface's query carries `$expand`; both now do, and that file's backend ignores the parameter, which is what makes it the bare-id fallback pin. Prose corrected, assertions unchanged. - Changeset: patch for `@object-ui/fields`. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C --- .changeset/10223-lookup-candidates-expand.md | 33 +++++++++++++++ ...LookupField.candidateExpand-10223.test.tsx | 42 ++++++++++++++++--- .../LookupField.pickerAgreement.test.tsx | 26 ++++++------ 3 files changed, 84 insertions(+), 17 deletions(-) create mode 100644 .changeset/10223-lookup-candidates-expand.md diff --git a/.changeset/10223-lookup-candidates-expand.md b/.changeset/10223-lookup-candidates-expand.md new file mode 100644 index 0000000000..06a9ad22cd --- /dev/null +++ b/.changeset/10223-lookup-candidates-expand.md @@ -0,0 +1,33 @@ +--- +'@object-ui/fields': patch +--- + +fix(fields): a lookup's candidate queries expand the reference columns they display + +Opening a lookup field's dropdown sent the candidate query without `$expand`. +Every reference column the dropdown previews under each candidate (by default +the referenced object's leading `highlightFields`) therefore arrived as a bare +foreign key, and the lookup cell renderer fetched each related record on its +own: one extra request per candidate per such column, on every open. The +browse-all picker (`RecordPickerDialog`) did the same once per table row. + +Both queries now ask for `$expand` on the reference columns they display, +chosen by `buildExpandFields` from `@object-ui/core` (the rule the list views +apply to their visible columns): the dropdown's previewed columns, and the +picker's columns other than its id column. The dropdown's recently-used rail +asks for the same expansion. Related records now render from the expanded +values with no request per row. A backend that ignores `$expand` still returns +bare ids, and those are resolved one by one as before. + +`buildExpandFields` covers `user` columns as well as `lookup`, +`master_detail` and `tree`. A previewed `user` column therefore now shows the +person, as the picker table and the list views do, where it used to show the +raw id marked as unresolved. + +What the dropdown and the picker hand onward is built from each row with its +expanded relations collapsed back to ids, through `toPredicateRecord` from +`@object-ui/core`. That covers option labels, `titleFormat` titles, the +committed value, and the records passed to `onSelectRecord` and +`onSelectRecords`, so none of them carries an expanded object. One difference +remains: `toPredicateRecord` returns ids as strings, so a numeric foreign key +in an expanded column reaches those callbacks in its string form. diff --git a/packages/fields/src/widgets/LookupField.candidateExpand-10223.test.tsx b/packages/fields/src/widgets/LookupField.candidateExpand-10223.test.tsx index 592ae6b204..4e5dd8ac76 100644 --- a/packages/fields/src/widgets/LookupField.candidateExpand-10223.test.tsx +++ b/packages/fields/src/widgets/LookupField.candidateExpand-10223.test.tsx @@ -25,6 +25,8 @@ * - the open costs ONE candidate query, and it carries `$expand` for exactly * the previewed reference column — no per-row `findOne` follows, and the * subtitle still names each related record; + * - a previewed `user` column rides the same rule (`buildExpandFields` is the + * one reference-bearing family), and now names the person; * - the control: with no reference column previewed the query carries no * `$expand` key at all; * - the browse-all picker behind the dropdown, the same way; @@ -53,6 +55,7 @@ const TASK_VERSION_FIELDS: Record = { code: { type: 'text', label: 'Code' }, task: { type: 'master_detail', label: 'Task', reference_to: 'task' }, version: { type: 'number', label: 'Version' }, + owner: { type: 'user', label: 'Owner', reference_to: 'sys_user' }, }; interface BackendOptions { @@ -69,21 +72,28 @@ interface BackendOptions { function makeBackend({ prefix, highlightFields, honoursExpand = true, titleFormat }: BackendOptions) { const tasks: Record = {}; + const users: Record = {}; const rows: Record[] = []; for (let i = 0; i < CANDIDATES; i++) { const taskId = `${prefix}_task_${i}`; + const userId = `${prefix}_user_${i}`; tasks[taskId] = { id: taskId, name: `Assembly step ${i}` }; - rows.push({ id: `${prefix}_tv_${i}`, name: `TV-${i}`, code: `C-${i}`, task: taskId, version: i + 1 }); + users[userId] = { id: userId, name: `Owner ${i}` }; + rows.push({ id: `${prefix}_tv_${i}`, name: `TV-${i}`, code: `C-${i}`, task: taskId, version: i + 1, owner: userId }); } + /** What `$expand` puts in place of each relation's key. */ + const related: Record> = { task: tasks, owner: users }; const find = vi.fn(async (objectName: string, params?: Record) => { if (objectName !== 'task_version') return { data: [], total: 0 }; const expand: string[] = honoursExpand && Array.isArray(params?.$expand) ? params!.$expand : []; const skip = Number(params?.$skip ?? 0); const top = Number(params?.$top ?? rows.length); - const data = rows - .slice(skip, skip + top) - .map((r) => (expand.includes('task') ? { ...r, task: tasks[r.task] } : { ...r })); + const data = rows.slice(skip, skip + top).map((r) => { + const row = { ...r }; + for (const f of expand) if (related[f]) row[f] = related[f][r[f]]; + return row; + }); return { data, total: rows.length }; }); const findOne = vi.fn(async (objectName: string, id: string) => @@ -199,6 +209,21 @@ describe('LookupField — the dropdown expands the reference columns it previews expect(tasks[CANDIDATES - 1]).toBe(`Assembly step ${CANDIDATES - 1}`); }); + it('a previewed `user` column is expanded by the same rule, and names the person', async () => { + const backend = makeBackend({ prefix: 'usr', highlightFields: ['code', 'owner'] }); + await openDropdown(backend); + + const queries = candidateQueries(backend); + expect(queries).toHaveLength(1); + expect(queries[0].$expand).toEqual(['owner']); + expect(perRowReads(backend)).toBe(0); + // The user cell renderer names an expanded record; a bare id it can only + // mark as unresolved. + const owners = previewTexts('owner'); + expect(owners[0]).toContain('Owner 0'); + expect(owners[0]).not.toContain('usr_user_0'); + }); + it('control: with no reference column previewed, the query carries no `$expand` key', async () => { const backend = makeBackend({ prefix: 'ctrl', highlightFields: ['code', 'version'] }); await openDropdown(backend); @@ -334,7 +359,14 @@ describe('LookupField — expansion changes the request count and nothing else ( expect(expanded.titles[0]).toBe('C-0 - samepick_task_0'); expect(expanded.tasks[0]).toBe('Assembly step 0'); expect(expanded.picked).toEqual([ - { id: 'samepick_tv_0', name: 'TV-0', code: 'C-0', task: 'samepick_task_0', version: 1 }, + { + id: 'samepick_tv_0', + name: 'TV-0', + code: 'C-0', + task: 'samepick_task_0', + version: 1, + owner: 'samepick_user_0', + }, ]); }); }); diff --git a/packages/fields/src/widgets/LookupField.pickerAgreement.test.tsx b/packages/fields/src/widgets/LookupField.pickerAgreement.test.tsx index 47dd56dcff..8a4bbbdbe3 100644 --- a/packages/fields/src/widgets/LookupField.pickerAgreement.test.tsx +++ b/packages/fields/src/widgets/LookupField.pickerAgreement.test.tsx @@ -31,15 +31,17 @@ * declaration is driven through both, and the rendered values must match * column for column. * - * They also pin the fallback. Neither surface's query carries populate/expand - * — that is unchanged, and widening `lookupColumns` with dot-path/populate - * semantics is explicitly NOT what fixes this — so a lookup value can - * legitimately arrive as an unresolved foreign-key id. The rule is that the - * dropdown adopts whatever the picker shows for it, and that the column is - * never silently dropped: a held value keeps its slot and renders the lookup - * cell renderer's own placeholder, because a blank column is worse than a - * bare id (the field report records a dot-path attempt producing exactly that - * silent-empty outcome). + * They also pin the fallback. Both surfaces' queries now ask for `$expand` on + * the reference columns they show (objectui#10223), but this file's backend + * ignores that parameter and returns the bare foreign key, as any backend that + * does not honour it would — and widening `lookupColumns` with + * dot-path/populate semantics is explicitly NOT what fixes this — so a lookup + * value can legitimately arrive as an unresolved foreign-key id. The rule is + * that the dropdown adopts whatever the picker shows for it, and that the + * column is never silently dropped: a held value keeps its slot and renders + * the lookup cell renderer's own placeholder, because a blank column is worse + * than a bare id (the field report records a dot-path attempt producing + * exactly that silent-empty outcome). */ import * as React from 'react'; @@ -234,9 +236,9 @@ describe('LookupField — inline dropdown agrees with the browse-all picker (obj const dropdown = await readInlineDropdown(UNRESOLVED_STEP_ID); const picker = await readBrowseAllPicker(UNRESOLVED_STEP_ID); - // The dropdown's query carries no populate/expand, so an id that resolves - // to nothing is a legitimate state. Whatever it renders, it renders the - // picker's answer — the two surfaces do not get to disagree here either. + // This backend ignores `$expand` and serves the bare key, so an id that + // resolves to nothing is a legitimate state. Whatever it renders, it renders + // the picker's answer — the two surfaces do not get to disagree here either. expect(dropdown).toEqual(picker); // The column must still be THERE, and it must not have degraded into the From e836ccd2974186294225b1536d1117170f3e2a7c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 18:20:07 +0000 Subject: [PATCH 3/6] refactor(fields): type the recents rows and preview map without `any` (objectui#10223) Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C --- packages/fields/src/widgets/LookupField.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/fields/src/widgets/LookupField.tsx b/packages/fields/src/widgets/LookupField.tsx index 8c5869aebc..7197b3a907 100644 --- a/packages/fields/src/widgets/LookupField.tsx +++ b/packages/fields/src/widgets/LookupField.tsx @@ -929,7 +929,7 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro // same `$expand` (objectui#10223). It keeps the rows it was served — the // preview reads them — and derives its options from them exactly as the main // list does, relations collapsed to ids. - const [recentRows, setRecentRows] = useState([]); + const [recentRows, setRecentRows] = useState[]>([]); // The expansion as a primitive, for the effect below: a memoised array's // identity is not a dependency to key a fetch on (AGENTS.md #10). const candidateExpandKey = candidateExpand.join(','); @@ -986,7 +986,7 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro * row — a static option, a just-created record — previews from itself. */ const previewRows = useMemo(() => { - const byValue = new Map(); + const byValue = new Map>(); for (const raw of [...recentRows, ...popoverQuery.records]) { const plain = toPredicateRecord(raw, refObjectSchema?.fields); const v = plain?.[idField] ?? plain?.id ?? plain?._id ?? plain?.externalId; From 977512e97ddcc68ff40f715255f9fb9c5b686e24 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 18:53:43 +0000 Subject: [PATCH 4/6] build(fields): depend on @object-ui/permissions for the candidate-expand FLS gate (objectui#10223) @object-ui/permissions depends only on @object-ui/types, so the edge adds no cycle. The lockfile hunk is the new importer link alone, generated by 'pnpm install --lockfile-only' over an installed tree. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C --- packages/fields/package.json | 1 + pnpm-lock.yaml | 3 +++ 2 files changed, 4 insertions(+) diff --git a/packages/fields/package.json b/packages/fields/package.json index 8177869d7c..564873eccd 100644 --- a/packages/fields/package.json +++ b/packages/fields/package.json @@ -33,6 +33,7 @@ "@object-ui/components": "workspace:*", "@object-ui/core": "workspace:*", "@object-ui/i18n": "workspace:*", + "@object-ui/permissions": "workspace:*", "@object-ui/providers": "workspace:*", "@object-ui/react": "workspace:*", "@object-ui/types": "workspace:*", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8741a84a89..f5d49da13f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1273,6 +1273,9 @@ importers: '@object-ui/i18n': specifier: workspace:* version: link:../i18n + '@object-ui/permissions': + specifier: workspace:* + version: link:../permissions '@object-ui/providers': specifier: workspace:* version: link:../providers From b7ee692eb03eb5aa8bbfb5aea2ac676f065da95a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 18:58:17 +0000 Subject: [PATCH 5/6] fix(fields): field-level security gates the lookup candidate expansion (objectui#10223) The dropdown's candidate query, its recents rail and RecordPickerDialog now filter `buildExpandFields`' output through `usePermissions().checkField( object, field, 'read')` once the policy has loaded, the shape the objectui#7429 sweep applied at every other call site; with no policy loaded nothing is filtered, and `perms` in the memo deps rebuilds the list when the answer arrives. Pins (real PermissionProvider): a denied `task_version.task` is left out of `$expand` while the readable `owner` stays, on the dropdown, the recents rail and the picker; a readable `task` is expanded; no provider filters nothing. `lookupColumnDisplay.tsx`: its module header said the two surfaces agree "without either query changing", which this change made false; that one sentence is corrected. Changeset updated for the gate and the new dependency. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C --- .changeset/10223-lookup-candidates-expand.md | 13 +- ...LookupField.candidateExpand-10223.test.tsx | 126 ++++++++++++++++-- packages/fields/src/widgets/LookupField.tsx | 19 ++- .../fields/src/widgets/RecordPickerDialog.tsx | 15 ++- .../src/widgets/lookupColumnDisplay.tsx | 5 +- 5 files changed, 154 insertions(+), 24 deletions(-) diff --git a/.changeset/10223-lookup-candidates-expand.md b/.changeset/10223-lookup-candidates-expand.md index 06a9ad22cd..9c7fb95d56 100644 --- a/.changeset/10223-lookup-candidates-expand.md +++ b/.changeset/10223-lookup-candidates-expand.md @@ -15,9 +15,16 @@ Both queries now ask for `$expand` on the reference columns they display, chosen by `buildExpandFields` from `@object-ui/core` (the rule the list views apply to their visible columns): the dropdown's previewed columns, and the picker's columns other than its id column. The dropdown's recently-used rail -asks for the same expansion. Related records now render from the expanded -values with no request per row. A backend that ignores `$expand` still returns -bare ids, and those are resolved one by one as before. +asks for the same expansion. Related records in the expanded columns now +render with no request per row. + +Field-level security gates that list the way it gates the other +`buildExpandFields` call sites: once the permission policy has loaded, a +reference column the user may not read on the referenced object is left out of +`$expand`; before the policy loads, nothing is filtered. `@object-ui/fields` +now depends on `@object-ui/permissions` for that check. A column left out of +`$expand`, like any column from a backend that ignores the parameter, still +arrives as a bare id and is resolved one by one as before. `buildExpandFields` covers `user` columns as well as `lookup`, `master_detail` and `tree`. A previewed `user` column therefore now shows the diff --git a/packages/fields/src/widgets/LookupField.candidateExpand-10223.test.tsx b/packages/fields/src/widgets/LookupField.candidateExpand-10223.test.tsx index 4e5dd8ac76..b81a652973 100644 --- a/packages/fields/src/widgets/LookupField.candidateExpand-10223.test.tsx +++ b/packages/fields/src/widgets/LookupField.candidateExpand-10223.test.tsx @@ -30,6 +30,10 @@ * - the control: with no reference column previewed the query carries no * `$expand` key at all; * - the browse-all picker behind the dropdown, the same way; + * - field-level security gates the expansion, as at every other + * `buildExpandFields` call site: a relation the loaded policy denies is not + * asked for (dropdown, recents rail, picker), a readable one is, and with + * no policy loaded nothing is filtered; * - expansion changes nothing a host or a user reads besides the request * count. The option labels (including a `titleFormat` template that names * the expanded field), the preview text, and the record handed to @@ -43,8 +47,10 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; import { render, screen, cleanup, waitFor, act, fireEvent } from '@testing-library/react'; import '@testing-library/jest-dom'; import { SchemaRendererContext } from '@object-ui/react'; +import { PermissionProvider } from '@object-ui/permissions'; import { LookupField } from './LookupField'; import { RecordPickerDialog } from './RecordPickerDialog'; +import { pushRecentLookupId } from './recentLookups'; import { getCellRenderer } from '../index'; /** A full dropdown page — the page size the inline dropdown asks for. */ @@ -151,24 +157,38 @@ async function settle(): Promise { * Mount the lookup the way a form does and wait for the referenced schema, * which lands at mount — before any user can reach the trigger. */ -async function mountLookup(backend: Backend, extra: Record = {}): Promise { +/** Wraps the rendered tree — a permission provider, or nothing. */ +type Wrap = (node: React.ReactElement) => React.ReactElement; +const bare: Wrap = (node) => node; + +async function mountLookup( + backend: Backend, + extra: Record = {}, + wrap: Wrap = bare, +): Promise { render( - - {}} - dataSource={backend.dataSource} - field={{ reference_to: 'task_version' } as never} - {...(extra as object)} - /> - , + wrap( + + {}} + dataSource={backend.dataSource} + field={{ reference_to: 'task_version' } as never} + {...(extra as object)} + /> + , + ), ); await waitFor(() => expect(backend.dataSource.getObjectSchema).toHaveBeenCalledWith('task_version')); await settle(); } -async function openDropdown(backend: Backend, extra: Record = {}): Promise { - await mountLookup(backend, extra); +async function openDropdown( + backend: Backend, + extra: Record = {}, + wrap: Wrap = bare, +): Promise { + await mountLookup(backend, extra, wrap); await act(async () => { fireEvent.click(screen.getByTestId('lookup-trigger')); }); @@ -370,3 +390,85 @@ describe('LookupField — expansion changes the request count and nothing else ( ]); }); }); + +describe('LookupField — field-level security gates the expansion (objectui#10223)', () => { + /** + * The real provider, not a stub: `checkField` answers from a policy that + * denies `task_version.task` to the `viewer` role and says nothing about + * `owner`. Both relations are previewed, so the pin reads a FILTER — one + * name removed, the other kept — rather than an expansion that vanished. + */ + function withPolicy(taskReadable: boolean): Wrap { + return (node) => ( + + {node} + + ); + } + + const HIGHLIGHTS = ['code', 'task', 'owner']; + + it('dropdown: a relation the policy denies is not expanded; the readable one still is', async () => { + const backend = makeBackend({ prefix: 'flsdeny', highlightFields: HIGHLIGHTS }); + await openDropdown(backend, {}, withPolicy(false)); + const queries = candidateQueries(backend); + expect(queries).toHaveLength(1); + expect(queries[0].$expand).toEqual(['owner']); + }); + + it('dropdown: a readable relation is expanded as before', async () => { + const backend = makeBackend({ prefix: 'flsallow', highlightFields: HIGHLIGHTS }); + await openDropdown(backend, {}, withPolicy(true)); + expect(candidateQueries(backend)[0].$expand).toEqual(['task', 'owner']); + }); + + it('dropdown: no policy loaded (no provider) filters nothing', async () => { + const backend = makeBackend({ prefix: 'flsnone', highlightFields: HIGHLIGHTS }); + await openDropdown(backend); + expect(candidateQueries(backend)[0].$expand).toEqual(['task', 'owner']); + }); + + it('recents rail: the same gate applies to its query', async () => { + const backend = makeBackend({ prefix: 'flsrecent', highlightFields: HIGHLIGHTS }); + pushRecentLookupId('task_version', 'flsrecent_tv_1'); + pushRecentLookupId('task_version', 'flsrecent_tv_0'); + await openDropdown(backend, {}, withPolicy(false)); + const recents = candidateQueries(backend).filter((p) => JSON.stringify(p.$filter ?? {}).includes('$in')); + expect(recents).toHaveLength(1); + expect(recents[0].$expand).toEqual(['owner']); + }); + + it('picker: a relation the policy denies is not expanded; the readable one still is', async () => { + const backend = makeBackend({ prefix: 'flspick', highlightFields: HIGHLIGHTS }); + render( + withPolicy(false)( + + {}} + dataSource={backend.dataSource} + objectName="task_version" + displayField="name" + columns={['name', 'code', 'task', 'owner']} + onSelect={() => {}} + cellRenderer={getCellRenderer} + fieldsMeta={TASK_VERSION_FIELDS} + /> + , + ), + ); + await waitFor(() => expect(screen.getByTestId('record-row-flspick_tv_0')).toBeInTheDocument()); + const queries = candidateQueries(backend); + expect(queries).toHaveLength(1); + expect(queries[0].$expand).toEqual(['owner']); + }); +}); diff --git a/packages/fields/src/widgets/LookupField.tsx b/packages/fields/src/widgets/LookupField.tsx index 7197b3a907..b2141f2a9e 100644 --- a/packages/fields/src/widgets/LookupField.tsx +++ b/packages/fields/src/widgets/LookupField.tsx @@ -33,6 +33,7 @@ import { } from './lookupColumnDisplay.js'; import { useSafeFieldLabel, useDisplayLocale } from '@object-ui/i18n'; import { SchemaRendererContext as ImportedSchemaRendererContext, useAction, useHasActionProvider } from '@object-ui/react'; +import { usePermissions } from '@object-ui/permissions'; import { useFieldTranslation } from './useFieldTranslation.js'; export interface LookupOption { @@ -514,11 +515,21 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro * the expanded row (`previewRows` below). A backend that ignores `$expand` * returns bare ids, and the cell renderer's per-id resolution takes over as * before. + * + * Field-level security gates the OUTPUT, in the shape the objectui#7429 sweep + * applied at every other `buildExpandFields` call site: once the policy has + * loaded, a relation the user may not read on the referenced object is not + * asked for; before it loads, nothing is filtered and `perms` in the deps + * rebuilds the list when the answer arrives. Every name judged here is one + * the referenced object declares, so the "`checkField` answers false for an + * undeclared key" trap cannot be reached. */ - const candidateExpand = useMemo( - () => buildExpandFields(refObjectSchema?.fields, previewColumns), - [refObjectSchema, previewColumns], - ); + const perms = usePermissions(); + const candidateExpand = useMemo(() => { + const expandable = buildExpandFields(refObjectSchema?.fields, previewColumns); + if (!perms.isLoaded || !referenceTo) return expandable; + return expandable.filter((f) => perms.checkField(referenceTo, f, 'read')); + }, [refObjectSchema, previewColumns, perms, referenceTo]); // Derive filter-bar columns from any typed picker columns. const filterColumns = useMemo(() => { diff --git a/packages/fields/src/widgets/RecordPickerDialog.tsx b/packages/fields/src/widgets/RecordPickerDialog.tsx index 3b1e44581f..7e55268a08 100644 --- a/packages/fields/src/widgets/RecordPickerDialog.tsx +++ b/packages/fields/src/widgets/RecordPickerDialog.tsx @@ -42,6 +42,7 @@ import type { DataSource, LookupColumnDef, LookupFilterDef } from '@object-ui/ty // ObjectView, so a spec `ViewFilterRule[]` lowers in exactly one place. import { buildExpandFields, mergeFilterNodes, toPredicateRecord } from '@object-ui/core'; import { useSafeFieldLabel, useDisplayLocale } from '@object-ui/i18n'; +import { usePermissions } from '@object-ui/permissions'; import { useFieldTranslation } from './useFieldTranslation.js'; import { useRecordQuery } from './useRecordQuery.js'; // The one place a lookup column's display value is decided — shared with the @@ -513,11 +514,17 @@ export function RecordPickerDialog({ * `onSelectRecords` hands a host, and the `titleFormat` template's reading * of a row — sees the row with its relations collapsed to ids * (`toPredicateRecord`), as it did before any column was expanded. + * + * Field-level security gates the OUTPUT, in the objectui#7429 sweep's shape + * — the same gate as LookupField's `candidateExpand`: once the policy has + * loaded, a relation the user may not read on `objectName` is not asked for. */ - const expand = useMemo( - () => buildExpandFields(fieldsMeta, resolvedColumns.filter((c) => c.field !== idField)), - [fieldsMeta, resolvedColumns, idField], - ); + const perms = usePermissions(); + const expand = useMemo(() => { + const expandable = buildExpandFields(fieldsMeta, resolvedColumns.filter((c) => c.field !== idField)); + if (!perms.isLoaded) return expandable; + return expandable.filter((f) => perms.checkField(objectName, f, 'read')); + }, [fieldsMeta, resolvedColumns, idField, perms, objectName]); // Auto-generate filter columns from lookupFilters when no explicit filterColumns given. // Each LookupFilterDef becomes a filterable field with inferred type. diff --git a/packages/fields/src/widgets/lookupColumnDisplay.tsx b/packages/fields/src/widgets/lookupColumnDisplay.tsx index 390d3877dd..d05ac7e7cd 100644 --- a/packages/fields/src/widgets/lookupColumnDisplay.tsx +++ b/packages/fields/src/widgets/lookupColumnDisplay.tsx @@ -29,7 +29,10 @@ * paths, no populate/expand semantics. A lookup column whose value arrives as * an unresolved foreign-key id is resolved the same way the picker has always * resolved it — client-side, by the lookup cell renderer — so the two surfaces - * agree without either query changing. + * agree whether or not the related record came back expanded: both queries ask + * for `$expand` on the readable reference columns they display + * (objectui#10223), and an expanded value renders through that same cell + * renderer without a fetch. */ import React from 'react'; From 9796fdbb46bdfa6919b21e624ba0a994c84c4386 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 19:51:54 +0000 Subject: [PATCH 6/6] fix(fields): no previewed column, no `$expand` on the lookup candidate queries (objectui#10223) `buildExpandFields` reads an EMPTY column list as "no restriction" and returns every relation the object declares. The dropdown's preview list is empty whenever its picker columns are the display field alone (a `highlightFields` naming only it, or every other field system-managed), so the dropdown and the recents rail asked for every declared relation, none of which they render. Both candidate expansions now return nothing for an empty column list; the picker's list (its columns minus the id column) gets the same one-line guard for the `columns: ['id']` / `displayField === idField` shape. Pins: `highlightFields ['name']` over an object declaring `created_by` (user), `owner_id` (lookup), `task` (master_detail) and `owner` (user) sends no `$expand` key on the dropdown or the recents rail; a picker whose only column is the id sends none either. `.changeset/lookup-dropdown-cell-renderer-5492.md` (pending, same package) said in the present tense that neither surface's request carries populate; this PR makes that false, so the clause now reads as history and names the change that superseded it. Frontmatter untouched. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C --- .../lookup-dropdown-cell-renderer-5492.md | 11 +-- ...LookupField.candidateExpand-10223.test.tsx | 69 ++++++++++++++++++- packages/fields/src/widgets/LookupField.tsx | 8 +++ .../fields/src/widgets/RecordPickerDialog.tsx | 7 +- 4 files changed, 87 insertions(+), 8 deletions(-) diff --git a/.changeset/lookup-dropdown-cell-renderer-5492.md b/.changeset/lookup-dropdown-cell-renderer-5492.md index d43f0e19a2..802c997fbc 100644 --- a/.changeset/lookup-dropdown-cell-renderer-5492.md +++ b/.changeset/lookup-dropdown-cell-renderer-5492.md @@ -27,11 +27,14 @@ there is a single renderer left to drift from. The dropdown's extra columns are rendered into the option row itself; the row's `title` keeps the full option label, which is what a truncated label needs, instead of a raw-value dump. -No query changed and no contract widened. `lookupColumns` entries stay bare -field names — no dot paths, no populate/expand semantics — because neither -surface's request carries populate to begin with: the picker resolves a +This change touched no query and widened no contract. `lookupColumns` entries +stay bare field names — no dot paths, no populate/expand semantics — because +at the time neither surface's request carried populate: the picker resolved a foreign-key id to a name client-side, in the lookup cell renderer, and the -dropdown now inherits exactly that. An unresolved reference therefore renders +dropdown inherited exactly that. A later change (objectui#10223) has both +requests ask for `$expand` on the reference columns they display, minus any +the loaded permission policy denies; a value that still arrives as a bare id +is resolved by that same cell renderer. An unresolved reference therefore renders what the picker renders for it, and keeps its column: a slot is dropped only when the record holds no value for the field, decided on the raw value and never on what the renderer makes of it, so an unresolved id can never degrade diff --git a/packages/fields/src/widgets/LookupField.candidateExpand-10223.test.tsx b/packages/fields/src/widgets/LookupField.candidateExpand-10223.test.tsx index b81a652973..a5c77aa633 100644 --- a/packages/fields/src/widgets/LookupField.candidateExpand-10223.test.tsx +++ b/packages/fields/src/widgets/LookupField.candidateExpand-10223.test.tsx @@ -28,7 +28,8 @@ * - a previewed `user` column rides the same rule (`buildExpandFields` is the * one reference-bearing family), and now names the person; * - the control: with no reference column previewed the query carries no - * `$expand` key at all; + * `$expand` key at all — nor with no column previewed at all (the display + * field alone), where an empty list would otherwise mean "every relation"; * - the browse-all picker behind the dropdown, the same way; * - field-level security gates the expansion, as at every other * `buildExpandFields` call site: a relation the loaded policy denies is not @@ -74,9 +75,17 @@ interface BackendOptions { /** A backend that ignores `$expand` returns bare foreign keys. */ honoursExpand?: boolean; titleFormat?: string; + /** The referenced object's declared fields (default `TASK_VERSION_FIELDS`). */ + fields?: Record; } -function makeBackend({ prefix, highlightFields, honoursExpand = true, titleFormat }: BackendOptions) { +function makeBackend({ + prefix, + highlightFields, + honoursExpand = true, + titleFormat, + fields = TASK_VERSION_FIELDS, +}: BackendOptions) { const tasks: Record = {}; const users: Record = {}; const rows: Record[] = []; @@ -109,7 +118,7 @@ function makeBackend({ prefix, highlightFields, honoursExpand = true, titleForma if (objectName === 'task_version') { return { name: 'task_version', - fields: TASK_VERSION_FIELDS, + fields, highlightFields, ...(titleFormat ? { titleFormat } : {}), }; @@ -276,6 +285,60 @@ describe('LookupField — the dropdown expands the reference columns it previews }); }); +describe('LookupField — no displayed reference column, no `$expand` (objectui#10223)', () => { + /** + * `buildExpandFields` reads an EMPTY column list as "no restriction" and + * returns every relation the object declares. A dropdown whose picker + * columns are the display field alone previews nothing, so it must not + * hand that function an empty list: the object below declares audit and + * ownership relations (`created_by`, `owner_id`) plus the master_detail + * `task` and the `user` field `owner`, and the dropdown renders none of them. + */ + const WITH_AUDIT_RELATIONS: Record = { + ...TASK_VERSION_FIELDS, + created_by: { type: 'user', label: 'Created By', reference_to: 'sys_user' }, + owner_id: { type: 'lookup', label: 'Owner', reference_to: 'sys_user' }, + }; + + it('dropdown and recents rail: `highlightFields` naming only the display field send no `$expand` key', async () => { + const backend = makeBackend({ prefix: 'nameonly', highlightFields: ['name'], fields: WITH_AUDIT_RELATIONS }); + pushRecentLookupId('task_version', 'nameonly_tv_1'); + await openDropdown(backend); + + const queries = candidateQueries(backend); + const recents = queries.filter((p) => JSON.stringify(p.$filter ?? {}).includes('$in')); + const main = queries.filter((p) => !recents.includes(p)); + expect(main).toHaveLength(1); + expect(recents).toHaveLength(1); + expect('$expand' in main[0]).toBe(false); + expect('$expand' in recents[0]).toBe(false); + expect(previewTexts('task')).toHaveLength(0); + }); + + it('picker: no rendered column besides the id sends no `$expand` key', async () => { + const backend = makeBackend({ prefix: 'idonly', highlightFields: ['name'], fields: WITH_AUDIT_RELATIONS }); + render( + + {}} + dataSource={backend.dataSource} + objectName="task_version" + displayField="name" + columns={['id']} + onSelect={() => {}} + cellRenderer={getCellRenderer} + fieldsMeta={WITH_AUDIT_RELATIONS} + /> + , + ); + await waitFor(() => expect(screen.getByTestId('record-row-idonly_tv_0')).toBeInTheDocument()); + const queries = candidateQueries(backend); + expect(queries).toHaveLength(1); + expect('$expand' in queries[0]).toBe(false); + }); +}); + describe('LookupField — expansion changes the request count and nothing else (objectui#10223)', () => { /** * `titleFormat` names the expanded field on purpose: a template that diff --git a/packages/fields/src/widgets/LookupField.tsx b/packages/fields/src/widgets/LookupField.tsx index b2141f2a9e..17a4686616 100644 --- a/packages/fields/src/widgets/LookupField.tsx +++ b/packages/fields/src/widgets/LookupField.tsx @@ -523,9 +523,17 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro * rebuilds the list when the answer arrives. Every name judged here is one * the referenced object declares, so the "`checkField` answers false for an * undeclared key" trap cannot be reached. + * + * No previewed column ⇒ no `$expand`. `buildExpandFields` reads an EMPTY + * column list as "no column restriction" and returns every relation the + * object declares; a dropdown that previews only its display field (a + * `highlightFields` naming just that field, or every other field + * system-managed) would then ask for `created_by`, `owner_id`, … — none of + * which it renders. */ const perms = usePermissions(); const candidateExpand = useMemo(() => { + if (previewColumns.length === 0) return []; const expandable = buildExpandFields(refObjectSchema?.fields, previewColumns); if (!perms.isLoaded || !referenceTo) return expandable; return expandable.filter((f) => perms.checkField(referenceTo, f, 'read')); diff --git a/packages/fields/src/widgets/RecordPickerDialog.tsx b/packages/fields/src/widgets/RecordPickerDialog.tsx index 7e55268a08..132060ef9a 100644 --- a/packages/fields/src/widgets/RecordPickerDialog.tsx +++ b/packages/fields/src/widgets/RecordPickerDialog.tsx @@ -518,10 +518,15 @@ export function RecordPickerDialog({ * Field-level security gates the OUTPUT, in the objectui#7429 sweep's shape * — the same gate as LookupField's `candidateExpand`: once the policy has * loaded, a relation the user may not read on `objectName` is not asked for. + * + * No rendered column besides the id ⇒ no `$expand`: `buildExpandFields` + * reads an EMPTY column list as "every relation the object declares". */ const perms = usePermissions(); const expand = useMemo(() => { - const expandable = buildExpandFields(fieldsMeta, resolvedColumns.filter((c) => c.field !== idField)); + const rendered = resolvedColumns.filter((c) => c.field !== idField); + if (rendered.length === 0) return []; + const expandable = buildExpandFields(fieldsMeta, rendered); if (!perms.isLoaded) return expandable; return expandable.filter((f) => perms.checkField(objectName, f, 'read')); }, [fieldsMeta, resolvedColumns, idField, perms, objectName]);