Skip to content

Commit c7a194e

Browse files
Copilothotlong
andcommitted
fix: add proper external data handling to ObjectKanban and regression tests
- ObjectKanban now accepts explicit `data` and `loading` props with `hasExternalData` guard (matching ObjectCalendar's pattern) to skip internal fetch when parent provides pre-fetched data - Remove stale console.log in ObjectKanban - Add OData `{ value: [] }` format test for ListView - Add regression tests for duplicate data in Calendar and Kanban views - Update CHANGELOG.md Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com> Agent-Logs-Url: https://github.com/objectstack-ai/objectui/sessions/7998282e-22f9-4b45-aca5-8f86440779c9
1 parent 3486b9d commit c7a194e

4 files changed

Lines changed: 252 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1313

1414
### Fixed
1515

16+
- **Duplicate Data in Calendar and Kanban Views** (`@object-ui/plugin-view`, `@object-ui/plugin-kanban`, `@object-ui/plugin-list`): Fixed a bug where Calendar and Kanban views displayed every record twice. The root cause was that `ObjectView` (plugin-view) unconditionally fetched data even when a `renderListView` callback was provided — causing parallel duplicate requests since `ListView` (plugin-list) independently fetches its own data. The duplicate fetch results triggered re-renders that destabilised child component rendering, leading to duplicate events in the calendar and duplicate cards on the kanban board. Additionally, `ObjectKanban` lacked proper external-data handling (`data`/`loading` props with `hasExternalData` guard), unlike `ObjectCalendar` which already had this pattern. Fixes: (1) `ObjectView` now skips its own fetch when `renderListView` is provided, (2) `ObjectKanban` now accepts explicit `data`/`loading` props and skips internal fetch when external data is supplied (matching `ObjectCalendar`'s pattern), (3) `ListView` now handles `{ value: [] }` OData response format consistently with `extractRecords` utility. Includes regression tests.
17+
1618
- **CI Build Fix: Replace dynamic `require()` with static imports** (`@object-ui/plugin-view`): Replaced module-level `require('@object-ui/react')` calls in `index.tsx` and `ObjectView.tsx` with static `import` statements. The dynamic `require()` pattern is not supported by Next.js Turbopack, causing the docs site build to fail during SSR prerendering of the `/docs/components/complex/view-switcher` page with `Error: dynamic usage of require is not supported`. Since `@object-ui/react` is already a declared dependency and other files in the package use static imports from it, replacing the `require()` with static imports is safe and eliminates the SSR compatibility issue.
1719

1820
- **CI Build Fix: Remove unused `...rest` parameter** (`@object-ui/plugin-calendar`): Removed unused `...rest` destructured parameter from `ObjectCalendar` component props (TS6133), which caused the declaration file generation to emit a TypeScript error during the build.
Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
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, vi, beforeEach } from 'vitest';
10+
import { render, screen, waitFor } from '@testing-library/react';
11+
import { ObjectView } from '../components/ObjectView';
12+
import { SchemaRendererProvider } from '@object-ui/react';
13+
import '@object-ui/plugin-grid';
14+
import '@object-ui/plugin-kanban';
15+
import '@object-ui/plugin-calendar';
16+
import '@object-ui/plugin-list';
17+
18+
vi.mock('@object-ui/components', async () => {
19+
const actual = await vi.importActual('@object-ui/components');
20+
return { ...actual };
21+
});
22+
23+
vi.mock('@object-ui/react', async (importOriginal) => {
24+
const actual = await importOriginal<any>();
25+
return {
26+
...actual,
27+
useDataScope: () => undefined,
28+
};
29+
});
30+
31+
const mockSetSearchParams = vi.fn();
32+
let mockSearchParams = new URLSearchParams();
33+
let mockViewId = 'calendar';
34+
35+
vi.mock('react-router-dom', () => ({
36+
useParams: () => ({ objectName: 'todo_task', viewId: mockViewId }),
37+
useSearchParams: () => [mockSearchParams, mockSetSearchParams],
38+
useNavigate: () => vi.fn(),
39+
}));
40+
41+
// Use dates in current month so events show on the visible calendar
42+
const now = new Date();
43+
const year = now.getFullYear();
44+
const month = String(now.getMonth() + 1).padStart(2, '0');
45+
46+
const records = [
47+
{ id: 't1', name: 'Set up CI/CD pipeline', due_date: `${year}-${month}-16T09:00:00`, status: 'todo' },
48+
{ id: 't2', name: 'Design dashboard', due_date: `${year}-${month}-18T10:00:00`, status: 'in_progress' },
49+
{ id: 't3', name: 'Fix mobile responsive', due_date: `${year}-${month}-20T14:00:00`, status: 'done' },
50+
];
51+
52+
let mockDataSource: any;
53+
54+
const mockObjects = [
55+
{
56+
name: 'todo_task',
57+
label: 'Task',
58+
fields: {
59+
name: { label: 'Name', type: 'text' },
60+
due_date: { label: 'Due Date', type: 'datetime' },
61+
status: {
62+
label: 'Status', type: 'select',
63+
options: [
64+
{ value: 'todo', label: 'To Do' },
65+
{ value: 'in_progress', label: 'In Progress' },
66+
{ value: 'done', label: 'Done' },
67+
],
68+
},
69+
},
70+
listViews: {
71+
all: {
72+
name: 'all',
73+
label: 'All Tasks',
74+
type: 'grid',
75+
columns: ['name', 'due_date', 'status'],
76+
},
77+
calendar: {
78+
name: 'calendar',
79+
label: 'Calendar',
80+
type: 'calendar',
81+
columns: ['name', 'due_date', 'status'],
82+
calendar: {
83+
startDateField: 'due_date',
84+
titleField: 'name',
85+
},
86+
},
87+
board: {
88+
name: 'board',
89+
label: 'Board',
90+
type: 'kanban',
91+
columns: ['name', 'due_date', 'status'],
92+
kanban: {
93+
groupField: 'status',
94+
titleField: 'name',
95+
},
96+
},
97+
},
98+
},
99+
];
100+
101+
describe('Duplicate Data Prevention', () => {
102+
beforeEach(() => {
103+
vi.clearAllMocks();
104+
mockSearchParams = new URLSearchParams();
105+
mockDataSource = {
106+
find: vi.fn().mockResolvedValue(records),
107+
findOne: vi.fn().mockResolvedValue({}),
108+
create: vi.fn().mockResolvedValue({}),
109+
update: vi.fn().mockResolvedValue({}),
110+
delete: vi.fn().mockResolvedValue(true),
111+
getObjectSchema: vi.fn().mockResolvedValue({
112+
name: 'todo_task',
113+
label: 'Task',
114+
fields: mockObjects[0].fields,
115+
}),
116+
};
117+
});
118+
119+
describe('Calendar view', () => {
120+
beforeEach(() => {
121+
mockViewId = 'calendar';
122+
});
123+
124+
it('should not show duplicate events in calendar view', async () => {
125+
render(
126+
<SchemaRendererProvider dataSource={mockDataSource}>
127+
<ObjectView dataSource={mockDataSource} objects={mockObjects} onEdit={vi.fn()} />
128+
</SchemaRendererProvider>
129+
);
130+
131+
// Calendar region should render
132+
await waitFor(() => {
133+
const calendarRegion = document.querySelector('[aria-label="Calendar"]');
134+
expect(calendarRegion).toBeInTheDocument();
135+
}, { timeout: 5000 });
136+
137+
// Wait for events to appear
138+
await waitFor(() => {
139+
expect(screen.getByText('Set up CI/CD pipeline')).toBeInTheDocument();
140+
}, { timeout: 5000 });
141+
142+
// Each event should appear exactly ONCE (no duplicates)
143+
expect(screen.getAllByText('Set up CI/CD pipeline')).toHaveLength(1);
144+
expect(screen.getAllByText('Design dashboard')).toHaveLength(1);
145+
expect(screen.getAllByText('Fix mobile responsive')).toHaveLength(1);
146+
});
147+
148+
it('should not show duplicate events with wrapped response format', async () => {
149+
// ObjectStack adapter returns { data: [...], total, page, pageSize }
150+
mockDataSource.find.mockResolvedValue({
151+
data: records,
152+
total: records.length,
153+
page: 1,
154+
pageSize: 100,
155+
hasMore: false,
156+
});
157+
158+
render(
159+
<SchemaRendererProvider dataSource={mockDataSource}>
160+
<ObjectView dataSource={mockDataSource} objects={mockObjects} onEdit={vi.fn()} />
161+
</SchemaRendererProvider>
162+
);
163+
164+
await waitFor(() => {
165+
expect(screen.getByText('Set up CI/CD pipeline')).toBeInTheDocument();
166+
}, { timeout: 5000 });
167+
168+
expect(screen.getAllByText('Set up CI/CD pipeline')).toHaveLength(1);
169+
expect(screen.getAllByText('Design dashboard')).toHaveLength(1);
170+
});
171+
});
172+
173+
describe('Kanban view', () => {
174+
beforeEach(() => {
175+
mockViewId = 'board';
176+
});
177+
178+
it('should not show duplicate cards in kanban view', async () => {
179+
render(
180+
<SchemaRendererProvider dataSource={mockDataSource}>
181+
<ObjectView dataSource={mockDataSource} objects={mockObjects} onEdit={vi.fn()} />
182+
</SchemaRendererProvider>
183+
);
184+
185+
// Wait for kanban cards to render
186+
await waitFor(() => {
187+
expect(screen.getByText('Set up CI/CD pipeline')).toBeInTheDocument();
188+
}, { timeout: 5000 });
189+
190+
// Each card should appear exactly ONCE (no duplicates)
191+
expect(screen.getAllByText('Set up CI/CD pipeline')).toHaveLength(1);
192+
expect(screen.getAllByText('Design dashboard')).toHaveLength(1);
193+
expect(screen.getAllByText('Fix mobile responsive')).toHaveLength(1);
194+
});
195+
});
196+
});

packages/plugin-kanban/src/ObjectKanban.tsx

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ export interface ObjectKanbanProps {
1818
schema: KanbanSchema;
1919
dataSource?: DataSource;
2020
className?: string; // Allow override
21+
/** Pre-fetched records passed by a parent (e.g. ListView). When provided, skips internal data fetching. */
22+
data?: any[];
23+
/** Loading state propagated from a parent. Respected only when `data` is also provided. */
24+
loading?: boolean;
2125
onRowClick?: (record: any) => void;
2226
onCardClick?: (record: any) => void;
2327
}
@@ -26,19 +30,32 @@ export const ObjectKanban: React.FC<ObjectKanbanProps> = ({
2630
schema,
2731
dataSource,
2832
className,
33+
data: externalData,
34+
loading: externalLoading,
2935
onRowClick,
3036
onCardClick,
3137
...props
3238
}) => {
39+
// When a parent (e.g. ListView) pre-fetches data and passes it via the `data` prop,
40+
// we must not trigger a second fetch. Detect external data by checking for an array.
41+
const hasExternalData = Array.isArray(externalData);
42+
3343
const [fetchedData, setFetchedData] = useState<any[]>([]);
3444
const [objectDef, setObjectDef] = useState<any>(null);
3545
// loading state
36-
const [loading, setLoading] = useState(false);
46+
const [loading, setLoading] = useState(hasExternalData ? (externalLoading ?? false) : false);
3747
const [error, setError] = useState<Error | null>(null);
3848

3949
// Resolve bound data if 'bind' property exists
4050
const boundData = useDataScope(schema.bind);
4151

52+
// Sync external data changes from parent (e.g. ListView re-fetches after filter change)
53+
useEffect(() => {
54+
if (hasExternalData && externalLoading !== undefined) {
55+
setLoading(externalLoading);
56+
}
57+
}, [externalLoading, hasExternalData]);
58+
4259
// Fetch object definition for metadata (labels, options)
4360
useEffect(() => {
4461
let isMounted = true;
@@ -56,6 +73,9 @@ export const ObjectKanban: React.FC<ObjectKanbanProps> = ({
5673
}, [schema.objectName, dataSource]);
5774

5875
useEffect(() => {
76+
// Skip internal fetch when data is managed by a parent component
77+
if (hasExternalData) return;
78+
5979
let isMounted = true;
6080
const fetchData = async () => {
6181
if (!dataSource || typeof dataSource.find !== 'function' || !schema.objectName) return;
@@ -72,8 +92,6 @@ export const ObjectKanban: React.FC<ObjectKanbanProps> = ({
7292
// Handle { value: [] } OData shape or { data: [] } shape or direct array
7393
const data = extractRecords(results);
7494

75-
console.log(`[ObjectKanban] Extracted data (length: ${data.length})`);
76-
7795
if (isMounted) {
7896
setFetchedData(data);
7997
}
@@ -86,15 +104,14 @@ export const ObjectKanban: React.FC<ObjectKanbanProps> = ({
86104
};
87105

88106
// Trigger fetch if we have an objectName AND verify no inline/bound data overrides it
89-
// And NO props.data passed from ListView
90-
if (schema.objectName && !boundData && !schema.data && !(props as any).data) {
107+
if (schema.objectName && !boundData && !schema.data) {
91108
fetchData();
92109
}
93110
return () => { isMounted = false; };
94-
}, [schema.objectName, dataSource, boundData, schema.data, schema.filter, (props as any).data, objectDef]);
111+
}, [schema.objectName, dataSource, boundData, schema.data, schema.filter, hasExternalData, objectDef]);
95112

96-
// Determine which data to use: props.data -> bound -> inline -> fetched
97-
const rawData = (props as any).data || boundData || schema.data || fetchedData;
113+
// Determine which data to use: external -> bound -> inline -> fetched
114+
const rawData = (hasExternalData ? externalData : undefined) || boundData || schema.data || fetchedData;
98115

99116
// Enhance data with title mapping and ensure IDs
100117
const effectiveData = useMemo(() => {

packages/plugin-list/src/__tests__/DataFetch.test.tsx

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,35 @@ describe('ListView Data Fetch', () => {
113113
});
114114
});
115115

116+
// =========================================================================
117+
// OData { value: [] } response format (duplicate data fix)
118+
// =========================================================================
119+
describe('OData value response format', () => {
120+
it('should extract records from { value: [] } OData response', async () => {
121+
const items = [
122+
{ id: '1', name: 'Alice' },
123+
{ id: '2', name: 'Bob' },
124+
];
125+
mockDataSource.find.mockResolvedValue({ value: items, '@odata.count': 2 });
126+
127+
const schema: ListViewSchema = {
128+
type: 'list-view',
129+
objectName: 'contacts',
130+
viewType: 'grid',
131+
fields: ['name'],
132+
};
133+
134+
renderWithProvider(<ListView schema={schema} dataSource={mockDataSource} />);
135+
136+
await vi.waitFor(() => {
137+
expect(screen.getByTestId('record-count-bar')).toBeInTheDocument();
138+
});
139+
140+
// Records should be extracted and rendered (not empty)
141+
expect(screen.queryByTestId('empty-state')).not.toBeInTheDocument();
142+
});
143+
});
144+
116145
// =========================================================================
117146
// $expand race condition fix (Issue #939 Bug 1)
118147
// =========================================================================

0 commit comments

Comments
 (0)