Fix duplicate data in calendar and board#1105
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
- Skip PluginObjectView data fetch when renderListView handles its own data, eliminating wasteful parallel fetches and re-renders that cause duplicate records - Add results.value (OData format) handling to ListView data extraction Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com> Agent-Logs-Url: https://github.com/objectstack-ai/objectui/sessions/c52a20c8-22a2-4511-85ed-d8bc9fd98fdc
|
@copilot 继续 |
… 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
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
All remaining work items are now complete in commit Fix 3 — ObjectKanban external data handling: Added explicit Regression tests: 4 new tests passing:
CHANGELOG.md updated. All 148+ existing tests continue to pass (128 kanban, 7 data fetch, 2 calendar nested config, 4 view switching, etc.). |
There was a problem hiding this comment.
Pull request overview
This PR fixes duplicate record rendering in Calendar and Kanban views by eliminating unintended parallel data fetches when a custom renderListView is used (console path), and by improving consistency of record extraction across response shapes.
Changes:
- Prevent
@object-ui/plugin-view’sObjectViewfrom fetching its own data whenrenderListViewis provided (avoids duplicate requests / re-render churn). - Extend
@object-ui/plugin-list’sListViewto extract records from OData-style{ value: [...] }responses and add a regression test. - Update
@object-ui/plugin-kanban’sObjectKanbanto accept externaldata/loadingprops and skip internal fetching when external data is supplied; add console regression tests and update the changelog.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/plugin-view/src/ObjectView.tsx | Skip internal fetch when renderListView is present to avoid duplicate requests and unstable downstream rendering. |
| packages/plugin-list/src/ListView.tsx | Add { value: [] } response-shape extraction support. |
| packages/plugin-list/src/tests/DataFetch.test.tsx | Add coverage for { value: [] } OData response extraction. |
| packages/plugin-kanban/src/ObjectKanban.tsx | Add external data/loading props + guard to avoid double-fetch when parent supplies data. |
| apps/console/src/tests/DuplicateData.test.tsx | Regression tests ensuring Calendar/Kanban render each record once. |
| CHANGELOG.md | Document the fixes and associated tests. |
| loading: externalLoading, | ||
| onRowClick, | ||
| onCardClick, | ||
| ...props |
There was a problem hiding this comment.
...props is no longer used after removing the (props as any).data access. Consider removing the rest destructuring to avoid keeping an unused binding and to make it clear which props are supported/passed through.
| ...props |
| useEffect(() => { | ||
| if (hasExternalData && externalLoading !== undefined) { | ||
| setLoading(externalLoading); | ||
| } | ||
| }, [externalLoading, hasExternalData]); |
There was a problem hiding this comment.
This useEffect updates the local loading state from the new loading prop, but loading is never used in the render output (not passed into KanbanRenderer and no loading UI is shown). Either wire loading through to the kanban renderer / show a loading skeleton/overlay, or remove the unused state+prop to avoid dead code.
Fixes a bug where Calendar and Kanban views displayed every record twice.
Root Cause
When
renderListViewis provided (console app path), PluginObjectView was unconditionally fetching data in its ownuseEffect, even though the ListView (created byrenderListView) independently fetches the same data. This caused:setData,setLoading) triggering unnecessary re-rendersAdditionally, ObjectKanban lacked proper external-data handling — it used fragile
(props as any).datacasting instead of explicit typed props, and didn't properly sync parent loading state changes.Changes Made
Fix 1:
packages/plugin-view/src/ObjectView.tsxSkip PluginObjectView's data fetch when
renderListViewis provided, since the custom list view (e.g. ListView) handles its own data fetching.Fix 2:
packages/plugin-list/src/ListView.tsxAdded missing
results.value(OData format) handling to the data extraction logic, consistent withextractRecordsutility used elsewhere in the codebase.Fix 3:
packages/plugin-kanban/src/ObjectKanban.tsxAdded explicit
data?: any[]andloading?: booleanprops toObjectKanbanPropsinterface. ImplementedhasExternalDataguard pattern (matchingObjectCalendar) that:dataSource.find()when external data is present(props as any).datawith typed destructured propsRegression Tests
DuplicateData.test.tsx: 3 tests verifying Calendar and Kanban views each render records exactly once (no duplicates), including wrapped response formatDataFetch.test.tsx: 1 new test for{ value: [] }OData response format extractionCHANGELOG.md
Updated with comprehensive description of all fixes.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.