diff --git a/.changeset/6014-tree-grid-field-formatting.md b/.changeset/6014-tree-grid-field-formatting.md new file mode 100644 index 0000000000..94d9c492ed --- /dev/null +++ b/.changeset/6014-tree-grid-field-formatting.md @@ -0,0 +1,47 @@ +--- +'@object-ui/plugin-tree': patch +--- + +`ObjectTree` formats its cells the way the flat table does: a lookup column renders the +referenced record's display name and a select column renders its translated option label, +instead of a raw record id and the raw stored value (objectui#6014). + +Reported against the built-in 业务单元 (`sys_business_unit`) page, whose 「组织架构」 tree tab +showed the manager column as a bare user id and the type column as `department`, while the +flat-table tab on the same page — over the same expanded records — showed the user's name +and 「部门」. + +The card carried its own control, and it pointed at the fetch rather than the formatter. The +tree treated "the host passed inline `data`" as "I do not need the object schema" and skipped +`getObjectSchema`, but its record-fetch branch prefers a live object dataSource over any +inline data. On the one mount shape `ListView` actually uses — `objectName` + a dataSource + +its own pre-fetched `data` — the tree therefore issued its OWN query with +`buildExpandFields(undefined)` → `[]` → no `$expand` at all, and had no field definitions to +format cells from. Both reported symptoms fall out of that single gap, which is why the flat +tab was unaffected and why the tree's existing tests (inline data, no dataSource — a path +that never runs the tree's own fetch) could not see it. + +Three changes, all inside `packages/plugin-tree`: + +- The object schema is fetched whenever the dataSource can serve one, not only when no host + passed inline data. The guard inside the fetch already no-ops without a dataSource, so the + pure inline/static path is unaffected. +- Records are no longer fetched until that schema has settled — settled, not necessarily + successful, so a rejected or inapplicable schema fetch can never block the tree. This also + removes a wasted first query whose lookup columns came back as bare ids and were painted + for a moment before the real query landed. +- Cell values route through a field-aware formatter that delegates both decisions rather than + re-deciding them: option labels through the `translateOptions` seam `ObjectGrid` already + uses for the flat tab (so both tabs read one `fieldOptions.*` i18n key, with the same + exact-then-case-insensitive match and `humanizeLabel` fallback as `SelectCellRenderer`), and + expanded references through `getRecordDisplayName`, the unified display-name resolver + (ADR-0079), with the family judged by `isExpandableFieldType` — the same predicate that + decided what to put in `$expand`. + +No new exports and no new package dependencies: both resolvers were already published from +`@object-ui/core`, and `translateOptions` was already reachable through the `useSafeFieldLabel` +hook this component calls for its column headers. + +One visible consequence beyond the report: an expanded record that comes back with no name-ish +field now reads as ADR-0079's `Record #` floor — the string every other surface shows for +it — rather than as the bare id. diff --git a/packages/plugin-tree/src/ObjectTree.fieldFormatting.test.tsx b/packages/plugin-tree/src/ObjectTree.fieldFormatting.test.tsx new file mode 100644 index 0000000000..729982f641 --- /dev/null +++ b/packages/plugin-tree/src/ObjectTree.fieldFormatting.test.tsx @@ -0,0 +1,222 @@ +/** + * 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. + */ + +/** + * The tree-grid formats its cells the way the flat table does — objectui#6014. + * + * ── The repro this file pins ─────────────────────────────────────────────── + * On the built-in 业务单元 (`sys_business_unit`) page the 「组织架构」 tree tab + * rendered the manager `lookup` column as a raw record id and the `type` + * `select` column as the raw stored value (`department`), while the flat-table + * tab on the SAME page rendered the user's name and the translated 「部门」. + * + * ── The mount shape matters, and it is why this was invisible ────────────── + * `ListView` renders the tree through `SchemaRenderer` with BOTH `objectName` + * (so `getDataConfig` yields `provider: 'object'`) and its own already-fetched + * `data` array. `ObjectTree` treats "a host passed inline data" as "I do not + * need the object schema" and skips `getObjectSchema`, yet its record-fetch + * branch still prefers a live object dataSource — so it issued its OWN query + * with `buildExpandFields(undefined)` → `[]` → no `$expand` at all, and had no + * field definitions to read `options` from either. Both halves of the card fall + * out of that one gap, which is why the tests below mount the tree the way + * `ListView` really does (dataSource AND inline `data`) rather than the way the + * older tests do (inline `data` only, no dataSource — a path that never runs + * the tree's own fetch and therefore cannot see this defect). + * + * ── The two halves are two mechanisms ───────────────────────────────────── + * The lookup half is object unwrapping over an `$expand`ed record; the select + * half is option-label resolution (`field.options` + the `fieldOptions.*` i18n + * convention). Fixing one does not fix the other, so both are asserted here. + */ + +import React from 'react'; +import { describe, it, expect, afterEach } from 'vitest'; +import { render, screen, waitFor, cleanup } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { I18nProvider } from '@object-ui/i18n'; +import { ObjectTree } from './ObjectTree'; + +afterEach(cleanup); + +/** `sys_business_unit`, trimmed to the two columns the card names. */ +const OBJECT_SCHEMA = { + name: 'sys_business_unit', + fields: { + name: { type: 'text', label: 'Name' }, + parent_id: { type: 'lookup', reference: 'sys_business_unit', label: 'Parent' }, + manager_user_id: { type: 'lookup', reference: 'sys_user', label: 'Manager' }, + type: { + type: 'select', + label: 'Type', + // Labels deliberately NOT derivable from their value: `humanizeLabel` + // (the unmatched-value fallback) turns `department` into `Department`, + // so an assertion expecting `Department` would pass even with option + // matching completely broken. `Business Department` can only come from + // the options array. + options: [ + { label: 'Business Department', value: 'department' }, + { label: 'Holding Company', value: 'company' }, + ], + }, + }, +}; + +/** What the server returns when `$expand` names the manager lookup. */ +const EXPANDED_ROWS = [ + { + id: 'bu1', + name: 'Acme', + parent_id: null, + manager_user_id: { id: 'u1', name: 'Zhang San' }, + type: 'department', + }, +]; + +/** What the server returns when `$expand` is absent — the reported symptom. */ +const BARE_ROWS = [ + { id: 'bu1', name: 'Acme', parent_id: null, manager_user_id: 'u1', type: 'department' }, +]; + +const TREE_SCHEMA = { + type: 'object-tree', + objectName: 'sys_business_unit', + labelField: 'name', + fields: ['name', 'manager_user_id', 'type'], +}; + +/** + * A dataSource that answers honestly: it expands only what it was ASKED to + * expand. A stub that always returned the expanded shape would make this whole + * file pass without a fix — the defect is in the request, not the response. + */ +function makeDataSource(rows?: any[]) { + const expandArgs: (string[] | null)[] = []; + const dataSource = { + find: async (_object: string, query: any) => { + const expand = query?.$expand ?? null; + expandArgs.push(expand); + if (rows) return rows; + return expand && expand.includes('manager_user_id') ? EXPANDED_ROWS : BARE_ROWS; + }, + getObjectSchema: async () => OBJECT_SCHEMA, + } as any; + return { dataSource, expandArgs }; +} + +/** The cells of the single rendered row, as text — what the user sees. */ +async function renderRowCells(children: (ds: any) => React.ReactElement) { + const { dataSource, expandArgs } = makeDataSource(); + render(children(dataSource)); + await waitFor(() => expect(screen.getByTestId('object-tree')).toBeInTheDocument()); + const row = screen.getAllByTestId('object-tree-row')[0]; + const cells = Array.from(row.querySelectorAll('td')).map((td) => td.textContent?.trim() ?? ''); + return { cells, expandArgs }; +} + +/** The ListView mount shape: `objectName` + a live dataSource + inline `data`. */ +function listViewShapedTree(dataSource: any) { + return ( + + ); +} + +describe('ObjectTree field formatting (objectui#6014)', () => { + it('renders a lookup column as the referenced record display name, not its id', async () => { + const { cells } = await renderRowCells(listViewShapedTree); + + // The manager column is the second cell (label column first). + expect(cells[1]).toBe('Zhang San'); + expect(cells[1]).not.toBe('u1'); + }); + + it('asks the dataSource to expand the lookup columns it is going to render', async () => { + // The mechanism behind the assertion above: a host passing inline `data` + // must not suppress the object-schema fetch that `$expand` is derived from. + const { expandArgs } = await renderRowCells(listViewShapedTree); + + expect(expandArgs.length).toBeGreaterThan(0); + expect(expandArgs[expandArgs.length - 1]).toContain('manager_user_id'); + }); + + it('renders a select column as its option label, not the raw stored value', async () => { + const { cells } = await renderRowCells(listViewShapedTree); + + // The type column is the third cell. + expect(cells[2]).toBe('Business Department'); + // The reported symptom: the raw stored value. + expect(cells[2]).not.toBe('department'); + // …and not the `humanizeLabel` fallback either. Without this line the + // assertion above could be satisfied by the fallback alone. + expect(cells[2]).not.toBe('Department'); + }); + + it('matches a stored value to its option case-insensitively, and humanizes an unmatched one', async () => { + // Mirrors `SelectCellRenderer`: seed data stores `Department` against a + // declared `department`, and a value no option declares still reads as + // words rather than as a raw token. + // Served through the dataSource, not as inline `data`: on this mount shape + // the tree's own query wins over anything the host passed down — the very + // precedence that made objectui#6014 invisible from the inline path. + const { dataSource } = makeDataSource([ + { id: 'bu9', name: 'Mixed', parent_id: null, manager_user_id: null, type: 'Department' }, + { id: 'bu8', name: 'Unknown', parent_id: null, manager_user_id: null, type: 'joint_venture' }, + ]); + render(); + await waitFor(() => expect(screen.getByTestId('object-tree')).toBeInTheDocument()); + const rows = screen.getAllByTestId('object-tree-row'); + const typeCellOf = (label: string) => { + const row = rows.find((r) => r.textContent?.includes(label))!; + return Array.from(row.querySelectorAll('td'))[2]?.textContent?.trim(); + }; + + expect(typeCellOf('Mixed')).toBe('Business Department'); + expect(typeCellOf('Unknown')).toBe('Joint Venture'); + }); + + it('renders a select option label through the fieldOptions i18n convention', async () => { + // The exact repro: a zh session must read 「部门」, not `department`. + // Key convention: `{appNamespace}.fieldOptions.{object}.{field}.{value}` + // — the same one `translateOptions` resolves for the flat grid. + const { dataSource } = makeDataSource(); + render( + + + , + ); + await waitFor(() => expect(screen.getByTestId('object-tree')).toBeInTheDocument()); + const row = screen.getAllByTestId('object-tree-row')[0]; + const cells = Array.from(row.querySelectorAll('td')).map((td) => td.textContent?.trim() ?? ''); + + expect(cells[2]).toBe('部门'); + }); + + it('still renders a plain value column untouched', async () => { + // Counter-probe: the formatting chain must not swallow ordinary values. + // Green both before and after the fix — it is here so a green on the four + // assertions above cannot be explained by "the tree renders nothing". + const { cells } = await renderRowCells(listViewShapedTree); + + expect(cells[0]).toBe('Acme'); + }); +}); diff --git a/packages/plugin-tree/src/ObjectTree.tsx b/packages/plugin-tree/src/ObjectTree.tsx index 29fdf4a297..9b1d1e3f6d 100644 --- a/packages/plugin-tree/src/ObjectTree.tsx +++ b/packages/plugin-tree/src/ObjectTree.tsx @@ -24,7 +24,14 @@ import type { DataSource, ViewData } from '@object-ui/types'; import { useNavigationOverlay, useSafeFieldLabel } from '@object-ui/react'; import { NavigationOverlay, cn } from '@object-ui/components'; import { createSafeTranslation } from '@object-ui/i18n'; -import { extractRecords, buildExpandFields, columnIdentity } from '@object-ui/core'; +import { + extractRecords, + buildExpandFields, + columnIdentity, + isExpandableFieldType, + getRecordDisplayName, + humanizeLabel, +} from '@object-ui/core'; import { ChevronRight, ChevronDown } from 'lucide-react'; /** @@ -224,8 +231,97 @@ function initialExpanded(roots: TreeNode[], depth?: number): Set { return set; } -function formatValue(value: any): string { +/** + * One entry of a field's `options`. The index signature is not incidental — it + * is what `useSafeFieldLabel().translateOptions` declares, and this alias exists + * to be assignable to that signature rather than to re-describe it. + */ +interface FieldOption { + value: string; + label: string; + [key: string]: unknown; +} + +/** Translates one field's `options` for the session locale. */ +type TranslateOptions = ( + objectName: string, + fieldName: string, + options: FieldOption[], +) => FieldOption[]; + +/** What {@link formatCellValue} needs to format one cell of one column. */ +interface CellFormatContext { + /** The object schema's definition for this column, when one was fetched. */ + fieldDef: any; + /** The column's field key — the i18n option keys are scoped by it. */ + fieldName: string; + /** The object the tree is rendering; absent for a schema-less inline mount. */ + objectName?: string; + /** `useSafeFieldLabel().translateOptions` — identity without a provider. */ + translateOptions: TranslateOptions; +} + +/** + * Format one cell the way the flat table formats the same field — objectui#6014. + * + * Both branches DELEGATE the decision rather than re-deciding it, so the tree + * cannot drift from the surfaces it is supposed to agree with: + * + * - **select-family** (any field carrying `options`): the stored value is + * resolved to its option label through `translateOptions`, which is the + * exact call `ObjectGrid` makes when it builds a column's `fieldMeta` + * (`packages/plugin-grid/src/ObjectGrid.tsx`, the `fieldMeta.options = + * translateOptions(...)` line), so both tabs read one `fieldOptions.*` i18n + * key. Matching is exact-then-case-insensitive and falls back to + * `humanizeLabel`, mirroring `SelectCellRenderer` in `@object-ui/fields` + * (seed data stores `Referral` against a declared `referral`). Keying the + * branch on "has options" rather than on a list of select spellings is + * deliberate: `select` / `status` / `multiselect` / `radio` / `checkboxes` / + * `tags` all resolve identically, and a copied type list is one more thing + * that can fall behind the registry. + * + * - **reference-family**: an expanded record resolves through + * `getRecordDisplayName`, THE unified display-name resolver (ADR-0079), and + * the family is judged by `isExpandableFieldType` — the SAME predicate that + * decided what to put in `$expand` a few lines up, so "what we expanded" and + * "what we unwrap as a reference" cannot disagree. + * + * A value with no field definition (an untyped column, or a mount that never + * fetched a schema) keeps the previous conservative unwrap. + */ +function formatCellValue(value: any, ctx?: CellFormatContext): string { if (value == null) return ''; + + const options: FieldOption[] | null = Array.isArray(ctx?.fieldDef?.options) + ? (ctx!.fieldDef.options as FieldOption[]) + : null; + if (options && options.length > 0) { + const translated = ctx!.objectName + ? ctx!.translateOptions(ctx!.objectName, ctx!.fieldName, options) + : options; + const labelFor = (raw: unknown): string => { + const exact = translated.find((opt) => opt?.value === raw); + if (exact) return String(exact.label ?? raw); + const normalized = String(raw).toLowerCase(); + const insensitive = translated.find( + (opt) => String(opt?.value).toLowerCase() === normalized, + ); + if (insensitive) return String(insensitive.label ?? raw); + return humanizeLabel(String(raw)); + }; + return Array.isArray(value) + ? value.filter((v) => v != null).map(labelFor).join(', ') + : labelFor(value); + } + + if (typeof value === 'object' && isExpandableFieldType(ctx?.fieldDef)) { + // No schema for the REFERENCED object here, so this lands on ADR-0079's + // record-key derivation (`name` / `full_name` / `*_name` / …) and, for an + // expanded record that came back without any name-ish field, its + // `Record #` floor — the same string every other surface shows. + return getRecordDisplayName(undefined, value); + } + if (typeof value === 'object') { return String(value.name ?? value.label ?? value.id ?? value._id ?? ''); } @@ -243,14 +339,41 @@ export const ObjectTree: React.FC = ({ const [loading, setLoading] = useState(true); const [error, setError] = useState(null); const [objectSchema, setObjectSchema] = useState(null); + /** + * Whether the object-schema fetch below has finished — settled, not + * successful: a dataSource that cannot serve a schema, an object with no + * name, and a rejected fetch all count, so the record fetch can never be + * blocked forever by a schema that is never going to arrive. + * + * A one-way latch on purpose. Re-arming it on every run of that effect would + * mean a `setState` in the effect body, and this component's dependency list + * includes `dataConfig` — a `useMemo` over the `schema` PROP object — so a + * host that rebuilds its schema each render would turn a benign re-run into a + * render loop. Not re-arming costs at most one fetch against a stale schema + * when `objectName` changes mid-life, which is exactly what happened on every + * fetch before this. + */ + const [schemaSettled, setSchemaSettled] = useState(false); const dataConfig = useMemo(() => getDataConfig(schema), [schema]); - const hasInlineData = - Array.isArray((rest as any).data) || - Array.isArray((schema as any).data) || - dataConfig?.provider === 'value'; - // Fetch object schema (for parent-field auto-detection + column labels). + // Fetch the object schema whenever the dataSource can serve one. + // + // It feeds FOUR things: parent-field auto-detection, column labels, the + // `$expand` list built below, and (objectui#6014) the per-field definitions + // the cell formatter reads to resolve select options and reference values. + // + // This used to be gated on "the host passed no inline data", which read as a + // cheap skip but disagreed with the record-fetch effect below: THAT branch + // prefers a live object dataSource over any inline `data`, so on the one + // mount shape `ListView` actually uses (objectName + dataSource + its own + // pre-fetched `data`) the tree ran its own query with + // `buildExpandFields(undefined)` → `[]` → no `$expand` at all, and had no + // field definitions to format cells with. That is the whole of objectui#6014: + // lookups rendered as bare ids and selects as raw stored values, on the very + // page whose flat-table tab rendered both correctly. The guard inside + // `fetchSchema` already no-ops without a dataSource, so dropping the gate + // costs nothing on the pure inline/static path. useEffect(() => { let cancelled = false; const fetchSchema = async () => { @@ -263,13 +386,17 @@ export const ObjectTree: React.FC = ({ if (!cancelled) setObjectSchema(result); } catch (err) { console.error('[ObjectTree] Failed to fetch object schema:', err); + } finally { + // `finally`, so the two early `return`s and a rejected fetch all settle + // too — see the latch's docstring. + if (!cancelled) setSchemaSettled(true); } }; - if (!hasInlineData) fetchSchema(); + fetchSchema(); return () => { cancelled = true; }; - }, [schema.objectName, dataSource, dataConfig, hasInlineData]); + }, [schema.objectName, dataSource, dataConfig]); // Fetch records. useEffect(() => { @@ -286,6 +413,13 @@ export const ObjectTree: React.FC = ({ // tree. Fetching our own records (no column projection) guarantees the // parent field is present so the hierarchy resolves. if (dataConfig?.provider === 'object' && dataSource && typeof dataSource.find === 'function') { + // Wait for the schema before querying. `$expand` is DERIVED from it, + // so firing early guaranteed one query whose lookup columns came back + // as bare ids — the user saw those raw ids painted, then replaced a + // moment later once the real query landed. `loading` stays true here + // so the tree shows its spinner instead of a wrong first answer, and + // this effect re-runs the moment the latch flips. + if (!schemaSettled) return; const expand = buildExpandFields(objectSchema?.fields); const result = await dataSource.find(dataConfig.object, { $filter: schema.filter, @@ -331,7 +465,7 @@ export const ObjectTree: React.FC = ({ return () => { cancelled = true; }; - }, [dataConfig, dataSource, schema.filter, objectSchema, (rest as any).data]); + }, [dataConfig, dataSource, schema.filter, objectSchema, schemaSettled, (rest as any).data]); const config = useMemo(() => getTreeConfig(schema), [schema]); const parentField = useMemo( @@ -375,6 +509,18 @@ export const ObjectTree: React.FC = ({ return headerObjectName ? i18n.fieldLabel(headerObjectName, field, fallback) : fallback; }; + /** + * Everything {@link formatCellValue} needs for one column. Built per cell + * from the SAME `objectSchema.fields` map the header labels read, so a column + * cannot be labelled from the schema and then formatted without it. + */ + const cellContext = (field: string): CellFormatContext => ({ + fieldDef: objectSchema?.fields?.[field], + fieldName: field, + objectName: headerObjectName, + translateOptions: i18n.translateOptions, + }); + const navigation = useNavigationOverlay({ navigation: (schema as any).navigation, objectName: schema.objectName, @@ -461,7 +607,7 @@ export const ObjectTree: React.FC = ({ )} - {formatValue(node.record[config.labelField]) || '—'} + {formatCellValue(node.record[config.labelField], cellContext(config.labelField)) || '—'} @@ -469,7 +615,7 @@ export const ObjectTree: React.FC = ({ .filter((f) => f !== config.labelField) .map((f) => ( - {formatValue(node.record[f])} + {formatCellValue(node.record[f], cellContext(f))} ))} @@ -495,7 +641,7 @@ export const ObjectTree: React.FC = ({ {key.replace(/_/g, ' ')} - {formatValue(value) || '—'} + {formatCellValue(value, cellContext(key)) || '—'} ))}