Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Add buildExpandFields() utility in @object-ui/core that scans schema fields for lookup/master_detail types and returns field names for $expand. Integrated into ListView, ObjectGrid, and ObjectKanban data fetches. Supports column-scoped expansion and fallback. Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
@copilot 仔细评估是否已经完成了任务中要求的所有开发功能 |
Integrate buildExpandFields() into ObjectCalendar, ObjectGantt, ObjectMap, ObjectTimeline, ObjectGallery, ObjectView, and ObjectAgGrid standalone data fetch paths. Gallery and Timeline now also fetch objectDef for schema awareness. Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
经过仔细评估，发现初始实现有遗漏。原先只在 ListView、ObjectGrid、ObjectKanban 三个组件中注入了 已在 5 个有
2 个无 schema 的插件 — 新增 schema 获取 + 注入：
逐项检查任务要求：
|
There was a problem hiding this comment.
Pull request overview
Adds a core utility to automatically derive $expand fields from object schema metadata and wires it into all major data-fetching plugins so lookup/master_detail fields render expanded objects (labels) instead of raw foreign-key IDs.
Changes:
- Introduces
buildExpandFields()in@object-ui/core(with unit tests) and exports it from the core package. - Injects
$expandintodataSource.find()across plugins (ListView, ObjectGrid, ObjectKanban, ObjectCalendar, ObjectGantt, ObjectMap, ObjectTimeline, ObjectGallery, ObjectView, ObjectAgGrid). - Updates
ROADMAP.mdto mark$expandauto-injection as complete.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/utils/expand-fields.ts | New helper to compute $expand fields from schema metadata (+ optional column scoping). |
| packages/core/src/utils/tests/expand-fields.test.ts | Unit tests for buildExpandFields() behavior and edge cases. |
| packages/core/src/index.ts | Exports the new expand-fields utility from @object-ui/core. |
| packages/plugin-list/src/ListView.tsx | Computes expand fields from objectDef and injects $expand into list fetches. |
| packages/plugin-grid/src/ObjectGrid.tsx | Injects $expand into grid query params based on resolved schema + configured columns/fields. |
| packages/plugin-kanban/src/ObjectKanban.tsx | Injects $expand into kanban fetches using object schema metadata. |
| packages/plugin-calendar/src/ObjectCalendar.tsx | Injects $expand into calendar fetches using object schema metadata. |
| packages/plugin-gantt/src/ObjectGantt.tsx | Injects $expand into gantt fetches using object schema metadata. |
| packages/plugin-map/src/ObjectMap.tsx | Injects $expand into map fetches using object schema metadata. |
| packages/plugin-timeline/src/ObjectTimeline.tsx | Adds schema fetch + injects $expand into timeline fetches. |
| packages/plugin-list/src/ObjectGallery.tsx | Adds schema fetch + injects $expand into gallery fetches. |
| packages/plugin-view/src/ObjectView.tsx | Injects $expand into ObjectView standalone fetch path for non-grid views. |
| packages/plugin-aggrid/src/ObjectAgGridImpl.tsx | Injects $expand into AG Grid query params based on object schema. |
| ROADMAP.md | Documents completion of the $expand auto-injection work. |
| if (typeof col === 'string') { | ||
| columnFieldNames.add(col); | ||
| } else if (col && typeof col === 'object') { | ||
| const name = col.field ?? col.name ?? col.fieldName; | ||
| if (name) columnFieldNames.add(name); |
There was a problem hiding this comment.
When columns are provided, the filtering only matches exact field names. If a visible column uses a nested path like account.owner, the schema will contain account (lookup) but the column set contains account.owner, so the lookup field is incorrectly excluded and $expand becomes empty. Consider treating dot-path columns as implying their root field (and/or matching lookup fields when any column starts with ${fieldName}.) and add a unit test for this case.
| if (typeof col === 'string') { | |
| columnFieldNames.add(col); | |
| } else if (col && typeof col === 'object') { | |
| const name = col.field ?? col.name ?? col.fieldName; | |
| if (name) columnFieldNames.add(name); | |
| const rawName = | |
| typeof col === 'string' | |
| ? col | |
| : col && typeof col === 'object' | |
| ? col.field ?? col.name ?? col.fieldName | |
| : undefined; | |
| if (!rawName) continue; | |
| const name = String(rawName).trim(); | |
| if (!name) continue; | |
| // Always add the full column field name | |
| columnFieldNames.add(name); | |
| // If the column uses a nested path (e.g. "account.owner"), | |
| // also treat it as implying the root field ("account") so | |
| // lookup/master_detail fields are not excluded from $expand. | |
| const dotIndex = name.indexOf('.'); | |
| if (dotIndex > 0) { | |
| const rootField = name.slice(0, dotIndex); | |
| if (rootField) { | |
| columnFieldNames.add(rootField); | |
| } |
| // Auto-inject $expand for lookup/master_detail fields | ||
| const expand = buildExpandFields(objectSchema?.fields); | ||
| const result = await dataSource.find(objectName, { | ||
| $filter: schema.filter, | ||
| $orderby: convertSortToQueryParams(schema.sort), | ||
| ...(expand.length > 0 ? { $expand: expand } : {}), |
There was a problem hiding this comment.
objectSchema is fetched in a separate effect defined after this one, so this effect will typically fire once with objectSchema === null (no $expand) and then fire again when objectSchema is set—duplicating the data request and briefly showing raw lookup IDs. Consider combining schema+data fetch, or early-returning from this effect until objectSchema is available for provider: 'object'.
| // Auto-inject $expand for lookup/master_detail fields | ||
| const expand = buildExpandFields(objectSchema?.fields); | ||
| const result = await dataSource.find(objectName, { | ||
| $filter: schema.filter, | ||
| $orderby: convertSortToQueryParams(schema.sort), | ||
| ...(expand.length > 0 ? { $expand: expand } : {}), | ||
| }); |
There was a problem hiding this comment.
objectSchema is fetched in a separate effect defined after this one, so the first fetch typically runs with objectSchema === null (no $expand) and then re-fetches once schema arrives. Consider fetching schema+data together or skipping the initial find() until objectSchema is loaded for provider: 'object' to avoid duplicate requests and ID flicker.
| // Auto-inject $expand for lookup/master_detail fields | ||
| const expand = buildExpandFields(objectDef?.fields); | ||
| const results = await dataSource.find(schema.objectName, { | ||
| options: { $top: 100 }, | ||
| $filter: schema.filter | ||
| $filter: schema.filter, | ||
| ...(expand.length > 0 ? { $expand: expand } : {}), | ||
| }); |
There was a problem hiding this comment.
Two issues here: (1) options: { $top: 100 } is not honored by the built-in ApiDataSource/ValueDataSource (they read params.$top), so this likely doesn't paginate as intended; (2) because objectDef loads asynchronously, the effect will fetch once with no $expand and then refetch when objectDef arrives. Prefer {$top: 100} at the top level and avoid the double-fetch by waiting for objectDef (or fetching schema+data together).
| // Auto-inject $expand for lookup/master_detail fields | ||
| const expand = buildExpandFields(objectDef?.fields); | ||
| const results = await dataSource.find(schema.objectName, { | ||
| options: { $top: 100 } | ||
| options: { $top: 100 }, | ||
| ...(expand.length > 0 ? { $expand: expand } : {}), | ||
| }); |
There was a problem hiding this comment.
options: { $top: 100 } will be ignored by built-in data sources (they read params.$top), so this request may return unbounded results. Also, since objectDef is fetched separately, the first find() runs with expand === [] and then re-runs when objectDef arrives. Consider switching to {$top: 100} at the top level and fetching schema+data in a single flow to avoid duplicate requests.
| const fetchData = async () => { | ||
| if (!dataSource || typeof dataSource.find !== 'function' || !schema.objectName) return; | ||
| if (isMounted) setLoading(true); | ||
| try { | ||
| // Auto-inject $expand for lookup/master_detail fields | ||
| const expand = buildExpandFields(objectDef?.fields); | ||
| const results = await dataSource.find(schema.objectName, { | ||
| $filter: schema.filter, | ||
| ...(expand.length > 0 ? { $expand: expand } : {}), | ||
| }); |
There was a problem hiding this comment.
Because objectDef is fetched in a separate effect, this data fetch will typically run once with objectDef === null (no $expand) and then re-run when objectDef arrives—duplicating requests and briefly rendering raw lookup IDs. Consider deferring the initial find() until objectDef is available (when getObjectSchema is supported) or combining the schema+data load into a single async flow with caching.
| // Auto-inject $expand for lookup/master_detail fields | ||
| const expand = buildExpandFields(resolvedSchema?.fields, schemaColumns ?? schemaFields); | ||
| if (expand.length > 0) { | ||
| params.$expand = expand; | ||
| } | ||
|
|
There was a problem hiding this comment.
buildExpandFields(resolvedSchema?.fields, ...) will always return [] in the code path where resolvedSchema is the "minimal schema stub" (fields {}) used when explicit columns/fields are configured. That means $expand won't actually be injected for the common explicit-columns scenario. Consider fetching the real object schema (or otherwise providing field metadata) before computing expand fields.
| // Auto-compute $expand fields from objectDef (lookup / master_detail) | ||
| const expandFields = React.useMemo( | ||
| () => buildExpandFields(objectDef?.fields, schema.fields), | ||
| [objectDef?.fields, schema.fields], | ||
| ); |
There was a problem hiding this comment.
expandFields depends on objectDef?.fields, but the data-fetch effect runs before getObjectSchema() resolves. This causes an initial fetch without $expand, followed by a second fetch when expandFields updates—extra network load and a brief "raw ID" render. Consider gating the initial fetch until objectDef is available (when getObjectSchema is supported) or combining schema+data fetch into one async flow with caching.
| // Auto-inject $expand for lookup/master_detail fields | ||
| const expand = buildExpandFields((objectSchema as any)?.fields); | ||
| const results = await dataSource.find(schema.objectName, { | ||
| $filter: finalFilter.length > 0 ? finalFilter : undefined, | ||
| $orderby: sort, | ||
| $top: 100, | ||
| ...(expand.length > 0 ? { $expand: expand } : {}), | ||
| }); |
There was a problem hiding this comment.
Because objectSchema is loaded in a separate effect, this fetch will run once with objectSchema === null (so expand is empty) and then re-run when objectSchema arrives—duplicating the request and momentarily rendering unexpanded lookup IDs. Consider waiting for objectSchema (or fetching schema+data in one flow) before calling find() when lookup expansion is desired.
| // Auto-inject $expand for lookup/master_detail fields | ||
| const expand = buildExpandFields(objectSchema?.fields); | ||
| const result = await dataSource.find(objectName, { | ||
| $filter: schema.filter, | ||
| $orderby: convertSortToQueryParams(schema.sort), | ||
| ...(expand.length > 0 ? { $expand: expand } : {}), | ||
| }); |
There was a problem hiding this comment.
objectSchema is fetched in a separate effect defined after this one, so the first fetch typically runs with objectSchema === null (no $expand) and then re-fetches once schema arrives. Consider fetching schema+data together (ObjectGrid already uses this pattern) or skipping the initial find() until objectSchema is loaded for provider: 'object'.
All data views display raw foreign-key IDs for lookup/master_detail fields because the API requests never include
$expand. This adds automatic$expandinjection by scanning schema field metadata before each fetch across all 10 data-fetching plugins.Core utility
New
buildExpandFields()in@object-ui/core— extracts lookup/master_detail field names from schema, optionally scoped to visible columns:Integration points
All 10 data-fetching plugins now auto-inject
$expand:objectDef, injects intodataSource.find()params. Covers all sub-views when rendered through ListView.resolvedSchema.fieldsafter schema resolution, scoped to configured columns.objectDef.fieldsin standalone fetch path.objectSchema.fields, injected in standalone fetch path.objectSchema.fields, injected in standalone fetch path.objectSchema.fields, injected in standalone fetch path.objectSchema.fields, injected in standalone fetch path.objectSchema.fields(already waits for schema before fetch).getObjectSchema()+$expandinjection.getObjectSchema()+$expandinjection.All plugins inject
$expandonly when lookup fields are detected; no-op when schema has none. Backends that don't support$expandignore the param — existing fallback (display raw ID) preserved. The$expandformat (string[]) is compatible with multi-level nesting paths (e.g.'account.owner'); the objectql backend handles recursive expansion.Tests
14 unit tests covering: field type detection, column scoping, mixed column formats, null/malformed inputs. 1031 existing plugin tests verified passing across all affected components.
Original prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.