Skip to content

Commit 19f2c0c

Browse files
Copilothotlong
andcommitted
fix: address CI failures and PR review comments
- 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>
1 parent 33ec888 commit 19f2c0c

File tree

7 files changed

+55
-13
lines changed

7 files changed

+55
-13
lines changed

ROADMAP.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1380,9 +1380,9 @@ All 313 `@object-ui/fields` tests pass.
13801380

13811381
- **ListView** (`packages/plugin-list/src/ListView.tsx`): Added `objectDefLoaded` state flag. Data fetch effect is gated on `objectDefLoaded` so the first fetch always includes correct `$expand` parameters. The `objectDef` fetch effect sets the flag in `finally` block to handle both success and error cases.
13821382
- **DetailView** (`packages/plugin-detail/src/DetailView.tsx`): Added `objectSchema` state. Data fetch effect now calls `getObjectSchema()` first, computes `$expand` via `buildExpandFields()`, and passes the params to `findOne()`. The resolved `objectSchema` is passed to `DetailSection` components.
1383-
- **DetailSection** (`packages/plugin-detail/src/DetailSection.tsx`): Added optional `objectSchema` prop. When `field.type` is not set, the field is enriched with type, options, currency, precision, format, reference_to, and reference_field from `objectSchema` before selecting a CellRenderer. Explicit `field.type` always takes precedence over objectSchema.
1384-
- **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.
1385-
- **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.
1383+
- **DetailSection** (`packages/plugin-detail/src/DetailSection.tsx`): Added optional `objectSchema` prop. Fields are enriched with missing metadata (options, currency, precision, format, reference_to, reference_field) from `objectSchema` regardless of whether `field.type` is explicitly set. When `field.type` is not set, the type is also resolved from objectSchema. Explicit `field.type` always takes precedence for the resolved type.
1384+
- **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` field names. The server's REST plugin routes `GET /data/:object` to `findData()` which processes `populate` (comma-separated string) for lookup expansion. Falls back to `client.data.find()` when no expand is needed.
1385+
- **ObjectStackAdapter `findOne()`** (`packages/data-objectstack/src/index.ts`): When `$expand` is present, uses the same `rawFindWithPopulate()` mechanism with `filter={"_id":"..."}` and `populate=` to fetch the single record with expanded lookup fields. Falls back to `client.data.get()` when no expand is needed.
13861386

13871387
**Tests:** 15 new tests added (2 ListView, 3 DetailView, 4 DetailSection, 8 data-objectstack). All 505 plugin tests + 94 data-objectstack tests pass.
13881388

packages/data-objectstack/src/expand.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,5 +215,31 @@ describe('ObjectStackAdapter $expand support', () => {
215215

216216
expect(result.data).toHaveLength(1);
217217
});
218+
219+
it('should not double /api/v1 when baseUrl already includes it', async () => {
220+
mockFetch.mockResolvedValue({
221+
ok: true,
222+
json: () => Promise.resolve({ records: [], total: 0 }),
223+
});
224+
225+
// Create adapter with /api/v1 in baseUrl
226+
const apiAdapter = new ObjectStackAdapter({
227+
baseUrl: 'http://localhost:3000/api/v1',
228+
autoReconnect: false,
229+
fetch: mockFetch,
230+
});
231+
(apiAdapter as any).client = mockClient;
232+
(apiAdapter as any).connected = true;
233+
234+
await apiAdapter.find('order', {
235+
$expand: ['customer'],
236+
});
237+
238+
expect(mockFetch).toHaveBeenCalled();
239+
const fetchUrl = mockFetch.mock.calls[0][0] as string;
240+
// Should be /api/v1/data/order not /api/v1/api/v1/data/order
241+
expect(fetchUrl).toBe('http://localhost:3000/api/v1/data/order?populate=customer');
242+
expect(fetchUrl).not.toContain('/api/v1/api/v1');
243+
});
218244
});
219245
});

packages/data-objectstack/src/index.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,10 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
583583

584584
const baseUrl = this.baseUrl.replace(/\/$/, '');
585585
const qs = queryParams.toString();
586-
const url = `${baseUrl}/api/v1/data/${resource}${qs ? `?${qs}` : ''}`;
586+
// Avoid doubling /api/v1 if baseUrl already includes it
587+
const hasApiVersionSuffix = /\/api\/v\d+$/i.test(baseUrl);
588+
const dataPath = hasApiVersionSuffix ? '/data' : '/api/v1/data';
589+
const url = `${baseUrl}${dataPath}/${resource}${qs ? `?${qs}` : ''}`;
587590

588591
const headers: Record<string, string> = {
589592
'Content-Type': 'application/json',

packages/plugin-detail/src/DetailSection.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,13 @@ export const DetailSection: React.FC<DetailSectionProps> = ({
7878

7979
const displayValue = (() => {
8080
if (value === null || value === undefined) return '-';
81-
// Enrich field with objectSchema metadata when field.type is not set
81+
// Enrich field with objectSchema metadata — merge missing properties
82+
// even when field.type is explicitly set (e.g., type: 'lookup' without reference_to)
8283
const objectDefField = objectSchema?.fields?.[field.name];
8384
const resolvedType = field.type || objectDefField?.type;
8485
const enrichedField: Record<string, any> = { ...field };
85-
if (!field.type && objectDefField) {
86-
if (objectDefField.type) enrichedField.type = objectDefField.type;
86+
if (objectDefField) {
87+
if (!enrichedField.type && objectDefField.type) enrichedField.type = objectDefField.type;
8788
if (objectDefField.options && !enrichedField.options) enrichedField.options = objectDefField.options;
8889
if (objectDefField.currency && !enrichedField.currency) enrichedField.currency = objectDefField.currency;
8990
if (objectDefField.precision !== undefined && enrichedField.precision === undefined) enrichedField.precision = objectDefField.precision;

packages/plugin-detail/src/DetailView.tsx

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,9 @@ export const DetailView: React.FC<DetailViewProps> = ({
8989

9090
if (dataSource && schema.objectName && schema.resourceId) {
9191
setLoading(true);
92+
// Clear stale state when navigating between objects/records
93+
setObjectSchema(null);
94+
setData(null);
9295
const objectName = schema.objectName;
9396
const resourceId = schema.resourceId;
9497
const prefix = `${objectName}-`;
@@ -106,15 +109,17 @@ export const DetailView: React.FC<DetailViewProps> = ({
106109

107110
schemaPromise.then((resolvedSchema) => {
108111
if (!isMounted) return;
109-
if (resolvedSchema) {
110-
setObjectSchema(resolvedSchema);
111-
}
112+
setObjectSchema(resolvedSchema);
112113

113114
// Compute $expand from objectSchema
114115
const expandFields = buildExpandFields(resolvedSchema?.fields, allFields);
115116
const params = expandFields.length > 0 ? { $expand: expandFields } : undefined;
116117

117-
return dataSource.findOne(objectName, resourceId, params).then((result) => {
118+
const findOnePromise = params
119+
? dataSource.findOne(objectName, resourceId, params)
120+
: dataSource.findOne(objectName, resourceId);
121+
122+
return findOnePromise.then((result) => {
118123
if (!isMounted) return;
119124
if (result) {
120125
setData(result);
@@ -126,7 +131,10 @@ export const DetailView: React.FC<DetailViewProps> = ({
126131
const altId = resIdStr.startsWith(prefix)
127132
? resIdStr.slice(prefix.length) // strip prefix
128133
: `${prefix}${resIdStr}`; // prepend prefix
129-
return dataSource.findOne(objectName, altId, params).then((fallbackResult) => {
134+
return (params
135+
? dataSource.findOne(objectName, altId, params)
136+
: dataSource.findOne(objectName, altId)
137+
).then((fallbackResult) => {
130138
if (isMounted) {
131139
setData(fallbackResult);
132140
setLoading(false);

packages/plugin-detail/src/__tests__/DetailView.test.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,8 @@ describe('DetailView', () => {
600600
render(<DetailView schema={schema} dataSource={mockDataSource} />);
601601

602602
await waitFor(() => {
603-
expect(mockDataSource.findOne).toHaveBeenCalledWith('contact', 'c1', undefined);
603+
// When no lookup fields exist, findOne should be called without $expand params
604+
expect(mockDataSource.findOne).toHaveBeenCalledWith('contact', 'c1');
604605
});
605606
});
606607

packages/plugin-list/src/ListView.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,9 @@ export const ListView: React.FC<ListViewProps> = ({
514514
// Fetch object definition
515515
React.useEffect(() => {
516516
let isMounted = true;
517+
// Reset loaded flag so data fetch waits for the new schema
518+
setObjectDefLoaded(false);
519+
setObjectDef(null);
517520
const fetchObjectDef = async () => {
518521
if (!dataSource || !schema.objectName) {
519522
setObjectDefLoaded(true);

0 commit comments

Comments
 (0)