Skip to content

Commit 1a25032

Browse files
authored
Merge pull request #944 from objectstack-ai/copilot/fix-field-type-display-issue
2 parents 2754196 + bfc9017 commit 1a25032

File tree

10 files changed

+735
-54
lines changed

10 files changed

+735
-54
lines changed

ROADMAP.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,6 +1364,30 @@ All 313 `@object-ui/fields` tests pass.
13641364

13651365
---
13661366

1367+
### ListView/DetailView/DetailSection — Lookup Field & Type Display Consistency (March 2026)
1368+
1369+
> **Issue #939:** Lookup/master_detail fields displayed raw IDs (e.g. `o1`, `p1`) instead of expanded names across ListView, DetailView, and DetailSection.
1370+
1371+
**Root Causes (5 independent bugs):**
1372+
1373+
1. **ListView `$expand` race condition**`expandFields` depended on async `objectDef` which could be `null` on first fetch, causing data to be requested without `$expand` and returning raw foreign-key IDs.
1374+
2. **DetailView missing `$expand` and objectSchema**`findOne()` was called without `$expand` parameters and without loading `objectSchema`, so lookup fields could never be expanded.
1375+
3. **DetailSection missing objectSchema enrichment** — When `field.type` was not explicitly set, `displayValue` fell through to `String(value)`, bypassing type-aware CellRenderers even when objectSchema metadata was available.
1376+
4. **ObjectStackAdapter `find()` dropping `$expand`** — The `@objectstack/client` v3.0.10's `data.find()` (GET) does not support `expand` in its `QueryOptions` interface, so `$expand` was silently dropped during `convertQueryParams()`.
1377+
5. **ObjectStackAdapter `findOne()` ignoring params** — The `findOne()` method declared its params argument as `_params` (unused), meaning `$expand` was never sent to the server.
1378+
1379+
**Fix:**
1380+
1381+
- **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.
1382+
- **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. 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.
1386+
1387+
**Tests:** 15 new tests added (2 ListView, 3 DetailView, 4 DetailSection, 8 data-objectstack). All 505 plugin tests + 94 data-objectstack tests pass.
1388+
1389+
---
1390+
13671391
## ⚠️ Risk Management
13681392

13691393
| Risk | Mitigation |
Lines changed: 245 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,245 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
import { describe, it, expect, beforeEach, vi } from 'vitest';
10+
import { ObjectStackAdapter } from './index';
11+
12+
// We test the adapter's $expand handling.
13+
// When $expand is present, the adapter makes a raw GET request to the REST API
14+
// with `populate` as a URL query param (since the client SDK's data.find()
15+
// QueryOptions does not support populate/expand).
16+
// The key scenarios:
17+
// 1. find() with $expand → raw GET /api/v1/data/:object?populate=...
18+
// 2. find() without $expand → client.data.find() (GET) as before
19+
// 3. findOne() with $expand → raw GET /api/v1/data/:object?filter={_id:...}&populate=...
20+
// 4. findOne() without $expand → client.data.get() as before
21+
22+
describe('ObjectStackAdapter $expand support', () => {
23+
let adapter: ObjectStackAdapter;
24+
let mockClient: any;
25+
let mockFetch: ReturnType<typeof vi.fn>;
26+
27+
beforeEach(() => {
28+
// Create a mock fetch that returns a successful response
29+
mockFetch = vi.fn().mockResolvedValue({
30+
ok: true,
31+
json: () => Promise.resolve({ records: [], total: 0 }),
32+
});
33+
34+
adapter = new ObjectStackAdapter({
35+
baseUrl: 'http://localhost:3000',
36+
autoReconnect: false,
37+
fetch: mockFetch,
38+
});
39+
40+
// Mock the internal client after construction
41+
mockClient = {
42+
data: {
43+
find: vi.fn().mockResolvedValue({ records: [], total: 0 }),
44+
query: vi.fn().mockResolvedValue({ records: [], total: 0 }),
45+
get: vi.fn().mockResolvedValue({ record: { _id: '1', name: 'Test' } }),
46+
},
47+
connect: vi.fn().mockResolvedValue(undefined),
48+
discover: vi.fn().mockResolvedValue({ status: 'ok' }),
49+
};
50+
51+
// Inject mock client and mark as connected to bypass connect()
52+
(adapter as any).client = mockClient;
53+
(adapter as any).connected = true;
54+
});
55+
56+
describe('find() with $expand', () => {
57+
it('should make a raw GET request with populate query param when $expand is present', async () => {
58+
mockFetch.mockResolvedValue({
59+
ok: true,
60+
json: () => Promise.resolve({
61+
records: [{ _id: '1', name: 'Order 1', customer: { _id: '2', name: 'Alice' } }],
62+
total: 1,
63+
}),
64+
});
65+
66+
const result = await adapter.find('order', {
67+
$top: 10,
68+
$expand: ['customer', 'account'],
69+
});
70+
71+
// Should use raw fetch, not client.data.query or client.data.find
72+
expect(mockFetch).toHaveBeenCalled();
73+
const fetchUrl = mockFetch.mock.calls[0][0] as string;
74+
expect(fetchUrl).toContain('/api/v1/data/order');
75+
expect(fetchUrl).toContain('populate=customer%2Caccount');
76+
expect(fetchUrl).toContain('top=10');
77+
expect(mockClient.data.find).not.toHaveBeenCalled();
78+
expect(result.data).toHaveLength(1);
79+
expect(result.data[0].customer).toEqual({ _id: '2', name: 'Alice' });
80+
});
81+
82+
it('should pass filters and sort as query params', async () => {
83+
mockFetch.mockResolvedValue({
84+
ok: true,
85+
json: () => Promise.resolve({ records: [], total: 0 }),
86+
});
87+
88+
await adapter.find('order', {
89+
$filter: { status: 'active' },
90+
$orderby: [{ field: 'name', order: 'asc' }],
91+
$top: 50,
92+
$skip: 10,
93+
$expand: ['customer'],
94+
});
95+
96+
expect(mockFetch).toHaveBeenCalled();
97+
const fetchUrl = mockFetch.mock.calls[0][0] as string;
98+
expect(fetchUrl).toContain('populate=customer');
99+
expect(fetchUrl).toContain('top=50');
100+
expect(fetchUrl).toContain('skip=10');
101+
expect(fetchUrl).toContain('sort=name');
102+
expect(fetchUrl).toContain('filter=');
103+
});
104+
105+
it('should use data.find() when $expand is not present', async () => {
106+
mockClient.data.find.mockResolvedValue({ records: [{ _id: '1', name: 'Test' }], total: 1 });
107+
108+
const result = await adapter.find('order', { $top: 10 });
109+
110+
expect(mockClient.data.find).toHaveBeenCalled();
111+
expect(result.data).toHaveLength(1);
112+
});
113+
114+
it('should use data.find() when $expand is an empty array', async () => {
115+
mockClient.data.find.mockResolvedValue({ records: [], total: 0 });
116+
117+
await adapter.find('order', { $top: 10, $expand: [] });
118+
119+
expect(mockClient.data.find).toHaveBeenCalled();
120+
});
121+
});
122+
123+
describe('findOne() with $expand', () => {
124+
it('should make a raw GET request with _id filter and populate when $expand is present', async () => {
125+
mockFetch.mockResolvedValue({
126+
ok: true,
127+
json: () => Promise.resolve({
128+
records: [{ _id: 'order-1', name: 'Order 1', customer: { _id: '2', name: 'Alice' } }],
129+
}),
130+
});
131+
132+
const result = await adapter.findOne('order', 'order-1', {
133+
$expand: ['customer', 'account'],
134+
});
135+
136+
expect(mockFetch).toHaveBeenCalled();
137+
const fetchUrl = mockFetch.mock.calls[0][0] as string;
138+
expect(fetchUrl).toContain('/api/v1/data/order');
139+
expect(fetchUrl).toContain('populate=customer%2Caccount');
140+
expect(fetchUrl).toContain('top=1');
141+
expect(fetchUrl).toContain('filter=');
142+
// Verify the filter contains _id
143+
const filterParam = new URL(fetchUrl).searchParams.get('filter');
144+
expect(filterParam).toBeTruthy();
145+
const parsedFilter = JSON.parse(filterParam!);
146+
expect(parsedFilter).toEqual({ _id: 'order-1' });
147+
expect(mockClient.data.get).not.toHaveBeenCalled();
148+
expect(result).toEqual({ _id: 'order-1', name: 'Order 1', customer: { _id: '2', name: 'Alice' } });
149+
});
150+
151+
it('should return null when raw request returns no records', async () => {
152+
mockFetch.mockResolvedValue({
153+
ok: true,
154+
json: () => Promise.resolve({ records: [] }),
155+
});
156+
157+
const result = await adapter.findOne('order', 'nonexistent', {
158+
$expand: ['customer'],
159+
});
160+
161+
expect(result).toBeNull();
162+
});
163+
164+
it('should use data.get() when $expand is not present', async () => {
165+
mockClient.data.get.mockResolvedValue({ record: { _id: '1', name: 'Test' } });
166+
167+
const result = await adapter.findOne('order', '1');
168+
169+
expect(mockClient.data.get).toHaveBeenCalledWith('order', '1');
170+
expect(result).toEqual({ _id: '1', name: 'Test' });
171+
});
172+
173+
it('should return null for 404 errors without $expand', async () => {
174+
mockClient.data.get.mockRejectedValue({ status: 404 });
175+
176+
const result = await adapter.findOne('order', 'nonexistent');
177+
178+
expect(result).toBeNull();
179+
});
180+
});
181+
182+
describe('raw request format', () => {
183+
it('should include Authorization header when token is set', async () => {
184+
mockFetch.mockResolvedValue({
185+
ok: true,
186+
json: () => Promise.resolve({ records: [], total: 0 }),
187+
});
188+
189+
(adapter as any).token = 'test-token';
190+
191+
await adapter.find('order', {
192+
$expand: ['customer'],
193+
});
194+
195+
expect(mockFetch).toHaveBeenCalled();
196+
const fetchInit = mockFetch.mock.calls[0][1];
197+
expect(fetchInit.headers.Authorization).toBe('Bearer test-token');
198+
});
199+
200+
it('should unwrap response envelope with success/data wrapper', async () => {
201+
mockFetch.mockResolvedValue({
202+
ok: true,
203+
json: () => Promise.resolve({
204+
success: true,
205+
data: {
206+
records: [{ _id: '1', name: 'Order' }],
207+
total: 1,
208+
},
209+
}),
210+
});
211+
212+
const result = await adapter.find('order', {
213+
$expand: ['customer'],
214+
});
215+
216+
expect(result.data).toHaveLength(1);
217+
});
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+
});
244+
});
245+
});

0 commit comments

Comments
 (0)