Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
… DetailSection Bug 1 (ListView): Gate data fetch on objectDefLoaded to prevent race condition where $expand is empty on first fetch due to async objectDef loading. Bug 2 (DetailView): Load objectSchema via getObjectSchema, compute $expand fields, and pass them to findOne() so lookup/master_detail fields are expanded. Bug 3 (DetailSection): Accept objectSchema prop and enrich fields missing type/options/currency from objectSchema metadata before rendering. Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
@copilot 改动完全没有效果，还是显示的ID |
… with expand The @objectstack/client's data.find() and data.get() don't support expand in their API. When $expand is needed: - find() now uses data.query() (POST) which supports the full query AST including expand - findOne() now uses data.query() with an _id filter when $expand is present instead of ignoring the params argument entirely Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Found the deeper root cause — the previous UI-level fix was correct but the
Fix in |
|
@copilot 00:21:44.035 @object-ui/plugin-designer:build: �[96msrc/AppCreationWizard.tsx�[0m:�[93m51�[0m:�[93m34�[0m - �[91merror�[0m�[90m TS2307: �[0mCannot find module '@object-ui/react' or its corresponding type declarations. |
- data-objectstack: expand entries now include required `object` property to satisfy QueryAST type; findOne uses `where` instead of `filters` - DetailView: convert resourceId to string before calling startsWith/slice Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
github-actions
Bot
added
documentation
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
…ble error state timing Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes lookup/master_detail display across ListView and DetailView by ensuring $expand is computed/applied consistently, enriching detail fields from objectSchema, and adding an ObjectStackAdapter fallback path that expands via raw REST GET populate= when the SDK/server path can’t support expand.
Changes:
- ListView: gate the initial fetch until
getObjectSchema()completes so$expandis available on the first request. - DetailView/DetailSection: load
objectSchema, compute$expandforfindOne(), and enrich fields without explicittypeusing schema metadata. - ObjectStackAdapter: when
$expandis requested, routefind()/findOne()through a raw GET/data/:object?...&populate=...and normalize responses; add targeted tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/plugin-list/src/ListView.tsx | Adds objectDefLoaded gating to avoid $expand race on initial fetch. |
| packages/plugin-list/src/tests/DataFetch.test.tsx | Adds regression tests for $expand propagation + fetch gating. |
| packages/plugin-detail/src/DetailView.tsx | Fetches objectSchema, computes $expand, passes schema down to DetailSection. |
| packages/plugin-detail/src/DetailSection.tsx | Enriches field metadata from objectSchema to select correct cell renderer. |
| packages/plugin-detail/src/tests/DetailView.test.tsx | Tests $expand behavior and getObjectSchema absence fallback. |
| packages/plugin-detail/src/tests/DetailSection.test.tsx | Tests enrichment/type precedence and fallback rendering. |
| packages/data-objectstack/src/index.ts | Adds raw GET populate route for $expand and shared result normalization. |
| packages/data-objectstack/src/expand.test.ts | Tests raw GET routing, query params, auth header, envelope unwrapping. |
| ROADMAP.md | Documents the initiative and intended fixes (currently mismatched with implementation). |
| // Fetch object definition | ||
| React.useEffect(() => { | ||
| let isMounted = true; | ||
| const fetchObjectDef = async () => { | ||
| if (!dataSource || !schema.objectName) return; | ||
| if (!dataSource || !schema.objectName) { | ||
| setObjectDefLoaded(true); | ||
| return; | ||
| } | ||
| try { | ||
| const def = await dataSource.getObjectSchema(schema.objectName); | ||
| if (isMounted) { | ||
| setObjectDef(def); | ||
| } | ||
| } catch (err) { | ||
| console.warn("Failed to fetch object schema for ListView:", err); | ||
| } finally { | ||
| if (isMounted) { | ||
| setObjectDefLoaded(true); | ||
| } | ||
| } |
There was a problem hiding this comment.
objectDefLoaded is never reset when schema.objectName / dataSource changes. That means the data-fetch effect can run immediately for a new object while objectDef is still from the previous object (or null), producing an incorrect $expand and potentially fetching the wrong shape of data. Consider setting objectDefLoaded back to false (and clearing objectDef) at the start of this effect whenever the objectName/dataSource changes, then setting it to true in finally once the new schema load attempt completes.
| if (dataSource && schema.objectName && schema.resourceId) { | ||
| setLoading(true); | ||
| const objectName = schema.objectName; | ||
| const resourceId = schema.resourceId; | ||
| const prefix = `${objectName}-`; | ||
|
|
||
| dataSource.findOne(objectName, resourceId).then((result) => { | ||
| if (result) { | ||
| setData(result); | ||
| setLoading(false); | ||
| return; | ||
| // Collect all visible fields from sections and top-level fields | ||
| const allFields = [ | ||
| ...(schema.sections?.flatMap(s => s.fields) || []), | ||
| ...(schema.fields || []), | ||
| ]; | ||
|
|
||
| // Load objectSchema first, then fetch data with $expand | ||
| const schemaPromise = dataSource.getObjectSchema | ||
| ? dataSource.getObjectSchema(objectName).catch(() => null) | ||
| : Promise.resolve(null); | ||
|
|
||
| schemaPromise.then((resolvedSchema) => { | ||
| if (!isMounted) return; | ||
| if (resolvedSchema) { | ||
| setObjectSchema(resolvedSchema); | ||
| } | ||
| // Fallback: try alternate ID format for backward compatibility | ||
| const altId = resourceId.startsWith(prefix) | ||
| ? resourceId.slice(prefix.length) // strip prefix | ||
| : `${prefix}${resourceId}`; // prepend prefix | ||
| return dataSource.findOne(objectName, altId).then((fallbackResult) => { | ||
| setData(fallbackResult); | ||
| setLoading(false); | ||
| }).catch(() => { | ||
| setData(null); | ||
| setLoading(false); | ||
|
|
||
| // Compute $expand from objectSchema | ||
| const expandFields = buildExpandFields(resolvedSchema?.fields, allFields); | ||
| const params = expandFields.length > 0 ? { $expand: expandFields } : undefined; | ||
|
|
There was a problem hiding this comment.
objectSchema state can become stale when navigating between objects/records in the same mounted DetailView. If schema.objectName changes (or getObjectSchema fails/returns null), the previous objectSchema remains and will still be passed to DetailSection, potentially enriching fields with the wrong metadata. Consider clearing objectSchema (and possibly data) when starting a new fetch cycle for a different objectName/resourceId, and explicitly setting objectSchema to null when schema resolution fails.
| // Enrich field with objectSchema metadata when field.type is not set | ||
| const objectDefField = objectSchema?.fields?.[field.name]; | ||
| const resolvedType = field.type || objectDefField?.type; | ||
| const enrichedField: Record<string, any> = { ...field }; | ||
| if (!field.type && objectDefField) { | ||
| if (objectDefField.type) enrichedField.type = objectDefField.type; | ||
| if (objectDefField.options && !enrichedField.options) enrichedField.options = objectDefField.options; | ||
| if (objectDefField.currency && !enrichedField.currency) enrichedField.currency = objectDefField.currency; | ||
| if (objectDefField.precision !== undefined && enrichedField.precision === undefined) enrichedField.precision = objectDefField.precision; | ||
| if (objectDefField.format && !enrichedField.format) enrichedField.format = objectDefField.format; | ||
| if (objectDefField.reference_to && !enrichedField.reference_to) enrichedField.reference_to = objectDefField.reference_to; | ||
| if (objectDefField.reference_field && !enrichedField.reference_field) enrichedField.reference_field = objectDefField.reference_field; | ||
| } | ||
| // Use type-aware cell renderer when field type is available (explicit or enriched) | ||
| if (resolvedType) { | ||
| const CellRenderer = getCellRenderer(resolvedType); | ||
| if (CellRenderer) { | ||
| return <CellRenderer value={value} field={field as unknown as FieldMetadata} />; | ||
| return <CellRenderer value={value} field={enrichedField as unknown as FieldMetadata} />; | ||
| } |
There was a problem hiding this comment.
Field enrichment from objectSchema only runs when field.type is missing. If a schema field sets type but omits supporting metadata (e.g., type: 'select' without options, or lookup without reference_field), the cell renderer will run without the needed metadata and may still display raw values/IDs. To match ObjectGrid’s enrichment behavior, consider merging missing properties (options/currency/precision/format/reference_*) from objectSchema regardless of whether field.type is explicitly set, while still keeping the explicit type as the resolved type.
|
|
||
| const baseUrl = this.baseUrl.replace(/\/$/, ''); | ||
| const qs = queryParams.toString(); | ||
| const url = `${baseUrl}/api/v1/data/${resource}${qs ? `?${qs}` : ''}`; |
There was a problem hiding this comment.
rawFindWithPopulate() hardcodes the /api/v1 prefix when building the URL (${baseUrl}/api/v1/data/...). Elsewhere in the repo there are examples/configs that use baseUrl: '/api/v1' (API-prefix style), which would produce a doubled path (/api/v1/api/v1/data/...) for the raw $expand route. Consider normalizing baseUrl (e.g., detect and avoid duplicating /api/v1, or derive the data path from the same baseUrl semantics used by @objectstack/client) so raw and SDK requests are consistent.
| const url = `${baseUrl}/api/v1/data/${resource}${qs ? `?${qs}` : ''}`; | |
| const hasApiVersionSuffix = /\/api\/v\d+$/i.test(baseUrl); | |
| const dataPath = hasApiVersionSuffix ? '/data' : '/api/v1/data'; | |
| const url = `${baseUrl}${dataPath}/${resource}${qs ? `?${qs}` : ''}`; |
| - **ObjectStackAdapter `find()`** (`packages/data-objectstack/src/index.ts`): When `$expand` is present, routes to `client.data.query()` (POST) which supports the full query AST including expand. Added `buildQueryAST()` and `normalizeQueryResult()` private helpers. | ||
| - **ObjectStackAdapter `findOne()`** (`packages/data-objectstack/src/index.ts`): When `$expand` is present, uses `client.data.query()` with `['_id', '=', id]` filter and `expand` map. Falls back to `client.data.get()` when no expand is needed. |
There was a problem hiding this comment.
The ROADMAP entry for the ObjectStackAdapter $expand fix doesn’t match the implementation in this PR. It says $expand routes through client.data.query() (POST) and mentions buildQueryAST(), but the adapter changes here route $expand through a raw GET with populate= (via rawFindWithPopulate()), and there is no buildQueryAST() helper. Please update this section to reflect the actual approach (raw GET + populate) and the correct helper names, and ensure the listed “Root Causes” count matches the PR description.
| - **ObjectStackAdapter `find()`** (`packages/data-objectstack/src/index.ts`): When `$expand` is present, routes to `client.data.query()` (POST) which supports the full query AST including expand. Added `buildQueryAST()` and `normalizeQueryResult()` private helpers. | |
| - **ObjectStackAdapter `findOne()`** (`packages/data-objectstack/src/index.ts`): When `$expand` is present, uses `client.data.query()` with `['_id', '=', id]` filter and `expand` map. Falls back to `client.data.get()` when no expand is needed. | |
| - **ObjectStackAdapter `find()`** (`packages/data-objectstack/src/index.ts`): When `$expand` is present, routes through `rawFindWithPopulate()` which issues a raw GET request with a `populate=` query parameter derived from the `$expand` map. When no expand is needed, the existing `client.data.find()` path is used unchanged. | |
| - **ObjectStackAdapter `findOne()`** (`packages/data-objectstack/src/index.ts`): When `$expand` is present, uses the same raw GET + `populate=` mechanism (via `rawFindWithPopulate()` or an equivalent helper) to fetch the single record with its related data; falls back to `client.data.get()` when no expand is needed. |
|
@copilot fix all and ci errors |
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
- Fix RecordDetailEdit test: only pass 3rd arg to findOne when $expand params exist (avoids undefined 3rd arg assertion failure) - ListView: reset objectDefLoaded + objectDef when objectName changes to prevent stale $expand on navigation - DetailView: clear objectSchema + data when starting new fetch cycle to prevent stale metadata on navigation - DetailSection: merge missing metadata props (options, reference_to, reference_field, etc.) from objectSchema even when field.type is set - data-objectstack: detect /api/v1 suffix in baseUrl to avoid doubling the path prefix in rawFindWithPopulate() - ROADMAP.md: update adapter descriptions to reflect raw GET + populate approach instead of data.query() POST - Add test for baseUrl /api/v1 double-prefix prevention Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…hout undefined 3rd arg DetailView now only passes 3rd arg to findOne when $expand params exist, matching the original 2-arg call signature when no expand is needed. Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Fixed all issues in CI failures:
Review comments addressed:
Build: 42/42 tasks pass. Tests: 354 pass across 25 affected test files. CodeQL: 0 alerts. |
Lookup and master_detail fields render raw IDs (
o1,p1,2) instead of expanded names across list and detail views due to six independent root causes spanning the UI layer and the data adapter layer.Bug 1: ListView
$expandrace conditionexpandFieldsdepends on asyncobjectDef, but the data fetch fires immediately on mount whenobjectDefis stillnull→expandFields = []→ no$expand→ server returns raw IDs.objectDefLoadedflag; data fetch is gated until schema resolution completesfinallyblock to handle both success and error pathsobjectDefLoadedandobjectDefare reset tofalse/nullwhenobjectName/dataSourcechanges, preventing stale$expandon navigationBug 2: DetailView never passes
$expandfindOne()was called without$expandand without loadingobjectSchemaat all — unlike ObjectGrid which does both.getObjectSchema()→buildExpandFields()→findOne(objectName, id, { $expand })objectSchemais forwarded toDetailSectionfor field enrichmentgetObjectSchemais absent on the DataSourceobjectSchemaanddataare cleared when starting a new fetch cycle to prevent stale metadata on navigation between objects/recordsfindOne()is only called with 3 args when$expandparams exist; falls back to the original 2-arg signature when no expand is neededBug 3: DetailSection ignores fields without explicit
typeWhen
field.typeis unset,displayValuefalls through toString(value)— even if objectSchema has the metadata to select a proper CellRenderer.objectSchemaprop onDetailSectionPropsobjectSchemaregardless of whetherfield.typeis explicitly set — matchingObjectGrid's enrichment behaviorfield.typeis not set, the type is also resolved from objectSchemafield.typealways wins for the resolved typeBug 4: ObjectStackAdapter
find()silently drops$expandThe
@objectstack/clientv3.0.10'sdata.find()(GET) does not supportexpand/populatein itsQueryOptionsinterface. The server's REST plugin (rest-api-plugin/RestServer) does not register aPOST /data/:object/queryendpoint — only thedispatcher-plugindoes. The only viable path for field expansion isGET /data/:objectwhich routes tofindData()that supports thepopulatequery parameter.$expandis present,find()now makes a rawGET /api/v1/data/:object?populate=field1,field2&top=50&...request viarawFindWithPopulate(), bypassing the client SDKpopulate(comma-separated field names) which is the format the server'sfindData→toQueryASTpipeline processes for lookup expansionfetchImpl(custom orglobalThis.fetch) from the constructor config for making these raw requestsrawFindWithPopulate()detects/api/v\d+suffix inbaseUrlto avoid doubling the API path prefix (e.g.,baseUrl: '/api/v1'won't produce/api/v1/api/v1/data/...)client.data.find()path when no expand is neededBug 5: ObjectStackAdapter
findOne()ignores params entirelyThe
findOne()method declared its params argument as_params(underscore prefix = unused), so$expandwas never sent to the server. Additionally, the installed server v3.0.10'sgetData()does not supportexpandorpopulateat all.$expandis present,findOne()now routes throughfindDatavia rawGET /api/v1/data/:object?filter={"_id":"..."}&populate=field1,field2&top=1client.data.get()path when no expand is neededBug 6: Server API format mismatch (root cause of earlier adapter-level failures)
In
@objectstack/specv3.0.10:findData()supportspopulate(comma-separated string) but does not have$expand/expand→populatenormalization (added in a newer spec version)getData()does not support expand/populate at allPOST /data/:object/query— only the dispatcher plugin doesPrevious attempts using
client.data.query()(POST) silently failed because the endpoint doesn't exist in the REST plugin, and attempts to useexpand(object map format) failed because v3.0.10 only processespopulate(comma-separated string).populateas a URL query parameter, which correctly routes through the REST plugin'sGET /data/:object→findData()pipelineTests
Tests across DataFetch, DetailView, DetailSection, and data-objectstack covering
$expandparam propagation, fetch gating, schema enrichment, explicit-type precedence, raw GET routing with populate format, findOne expand support, Authorization header propagation, response envelope unwrapping, and baseUrl/api/v1double-prefix prevention. All 42 build tasks pass. CodeQL: 0 alerts.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.