-
Notifications
You must be signed in to change notification settings - Fork 2
fix: useDataScope returns adapter as boundData, blocking dashboard widget data fetches — plus comprehensive fault tolerance across all data-fetching components #846
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
63c9aa5
3d92084
2a70901
cbd364e
7aa1345
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
|
|
||
| import React, { useState, useEffect } from 'react'; | ||
| import { useDataScope, useSchemaContext } from '@object-ui/react'; | ||
| import React, { useState, useEffect, useContext } from 'react'; | ||
| import { useDataScope, SchemaRendererContext } from '@object-ui/react'; | ||
| import { ChartRenderer } from './ChartRenderer'; | ||
| import { ComponentRegistry, extractRecords } from '@object-ui/core'; | ||
|
|
||
|
|
@@ -54,8 +54,8 @@ export { extractRecords } from '@object-ui/core'; | |
|
|
||
| export const ObjectChart = (props: any) => { | ||
| const { schema } = props; | ||
| const context = useSchemaContext(); | ||
| const dataSource = props.dataSource || context.dataSource; | ||
| const context = useContext(SchemaRendererContext); | ||
| const dataSource = props.dataSource || context?.dataSource; | ||
| const boundData = useDataScope(schema.bind); | ||
|
|
||
| const [fetchedData, setFetchedData] = useState<any[]>([]); | ||
|
|
@@ -64,7 +64,7 @@ export const ObjectChart = (props: any) => { | |
| useEffect(() => { | ||
| let isMounted = true; | ||
| const fetchData = async () => { | ||
| if (!dataSource || !schema.objectName) return; | ||
| if (!dataSource || typeof dataSource.find !== 'function' || !schema.objectName) return; | ||
| if (isMounted) setLoading(true); | ||
|
Comment on lines
66
to
68
|
||
| try { | ||
| const results = await dataSource.find(schema.objectName, { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,179 @@ | ||
| /** | ||
| * Tests for ObjectChart data fetching & fault tolerance. | ||
| * | ||
| * Verifies that ObjectChart: | ||
| * - Calls dataSource.find() when objectName is set and no bound data | ||
| * - Handles missing/invalid dataSource gracefully | ||
| * - Works without a SchemaRendererProvider | ||
| */ | ||
|
|
||
| import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; | ||
| import { render, waitFor } from '@testing-library/react'; | ||
| import React from 'react'; | ||
| import { SchemaRendererProvider } from '@object-ui/react'; | ||
| import { ObjectChart } from '../ObjectChart'; | ||
|
|
||
| // Suppress console.error from React error boundary / fetch errors | ||
| const originalConsoleError = console.error; | ||
| beforeEach(() => { | ||
| console.error = vi.fn(); | ||
| }); | ||
| afterEach(() => { | ||
| console.error = originalConsoleError; | ||
| }); | ||
|
|
||
| describe('ObjectChart data fetching', () => { | ||
| it('should call dataSource.find when objectName is set and no bind path', async () => { | ||
| const mockFind = vi.fn().mockResolvedValue([ | ||
| { stage: 'Prospect', amount: 100 }, | ||
| { stage: 'Proposal', amount: 200 }, | ||
| ]); | ||
| const dataSource = { find: mockFind }; | ||
|
|
||
| render( | ||
| <SchemaRendererProvider dataSource={dataSource}> | ||
| <ObjectChart | ||
| schema={{ | ||
| type: 'object-chart', | ||
| objectName: 'opportunity', | ||
| chartType: 'bar', | ||
| xAxisKey: 'stage', | ||
| series: [{ dataKey: 'amount' }], | ||
| }} | ||
| /> | ||
| </SchemaRendererProvider> | ||
| ); | ||
|
|
||
| await waitFor(() => { | ||
| expect(mockFind).toHaveBeenCalledWith('opportunity', { $filter: undefined }); | ||
| }); | ||
| }); | ||
|
|
||
| it('should NOT call dataSource.find when schema.data is provided', () => { | ||
| const mockFind = vi.fn(); | ||
| const dataSource = { find: mockFind }; | ||
|
|
||
| render( | ||
| <SchemaRendererProvider dataSource={dataSource}> | ||
| <ObjectChart | ||
| schema={{ | ||
| type: 'object-chart', | ||
| objectName: 'opportunity', | ||
| chartType: 'bar', | ||
| data: [{ stage: 'A', amount: 100 }], | ||
| xAxisKey: 'stage', | ||
| series: [{ dataKey: 'amount' }], | ||
| }} | ||
| /> | ||
| </SchemaRendererProvider> | ||
| ); | ||
|
|
||
| expect(mockFind).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should apply aggregation to fetched data', async () => { | ||
| const mockFind = vi.fn().mockResolvedValue([ | ||
| { stage: 'Prospect', amount: 100 }, | ||
| { stage: 'Prospect', amount: 200 }, | ||
| { stage: 'Proposal', amount: 300 }, | ||
| ]); | ||
| const dataSource = { find: mockFind }; | ||
|
|
||
| const { container } = render( | ||
| <SchemaRendererProvider dataSource={dataSource}> | ||
| <ObjectChart | ||
| schema={{ | ||
| type: 'object-chart', | ||
| objectName: 'opportunity', | ||
| chartType: 'bar', | ||
| xAxisKey: 'stage', | ||
| series: [{ dataKey: 'amount' }], | ||
| aggregate: { field: 'amount', function: 'sum', groupBy: 'stage' }, | ||
| }} | ||
| /> | ||
| </SchemaRendererProvider> | ||
| ); | ||
|
|
||
| await waitFor(() => { | ||
| expect(mockFind).toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('ObjectChart fault tolerance', () => { | ||
| it('should not crash when dataSource has no find method', () => { | ||
| const { container } = render( | ||
| <SchemaRendererProvider dataSource={{}}> | ||
| <ObjectChart | ||
| schema={{ | ||
| type: 'object-chart', | ||
| objectName: 'opportunity', | ||
| chartType: 'bar', | ||
| xAxisKey: 'stage', | ||
| series: [{ dataKey: 'amount' }], | ||
| }} | ||
| /> | ||
| </SchemaRendererProvider> | ||
| ); | ||
|
|
||
| // Should render without crashing | ||
| expect(container).toBeDefined(); | ||
| }); | ||
|
|
||
| it('should not crash when rendered outside SchemaRendererProvider', () => { | ||
| const { container } = render( | ||
| <ObjectChart | ||
| schema={{ | ||
| type: 'object-chart', | ||
| chartType: 'bar', | ||
| xAxisKey: 'stage', | ||
| series: [{ dataKey: 'amount' }], | ||
| }} | ||
| /> | ||
| ); | ||
|
|
||
| // Should render without crashing | ||
| expect(container).toBeDefined(); | ||
| }); | ||
|
|
||
| it('should show "No data source available" when no dataSource and objectName set', () => { | ||
| const { container } = render( | ||
| <ObjectChart | ||
| schema={{ | ||
| type: 'object-chart', | ||
| objectName: 'opportunity', | ||
| chartType: 'bar', | ||
| xAxisKey: 'stage', | ||
| series: [{ dataKey: 'amount' }], | ||
| }} | ||
| /> | ||
| ); | ||
|
|
||
| expect(container.textContent).toContain('No data source available'); | ||
| }); | ||
|
|
||
| it('should use dataSource prop over context when both are present', async () => { | ||
| const contextFind = vi.fn().mockResolvedValue([]); | ||
| const propFind = vi.fn().mockResolvedValue([{ stage: 'A', amount: 1 }]); | ||
|
|
||
| render( | ||
| <SchemaRendererProvider dataSource={{ find: contextFind }}> | ||
| <ObjectChart | ||
| dataSource={{ find: propFind }} | ||
| schema={{ | ||
| type: 'object-chart', | ||
| objectName: 'opportunity', | ||
| chartType: 'bar', | ||
| xAxisKey: 'stage', | ||
| series: [{ dataKey: 'amount' }], | ||
| }} | ||
| /> | ||
| </SchemaRendererProvider> | ||
| ); | ||
|
|
||
| await waitFor(() => { | ||
| expect(propFind).toHaveBeenCalled(); | ||
| }); | ||
| expect(contextFind).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,7 @@ export type { GanttViewProps, GanttTask, GanttViewMode } from './GanttView'; | |
|
|
||
| // Register component | ||
| export const ObjectGanttRenderer: React.FC<{ schema: any }> = ({ schema }) => { | ||
| const { dataSource } = useSchemaContext(); | ||
| const { dataSource } = useSchemaContext() || {}; | ||
| return <ObjectGantt schema={schema} dataSource={dataSource} />; | ||
|
Comment on lines
22
to
24
|
||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,7 @@ export type { ObjectMapProps }; | |
|
|
||
| // Register component | ||
| export const ObjectMapRenderer: React.FC<any> = ({ schema, ...props }) => { | ||
| const { dataSource } = useSchemaContext(); | ||
| const { dataSource } = useSchemaContext() || {}; | ||
| return <ObjectMap schema={schema} dataSource={dataSource} {...props} />; | ||
|
Comment on lines
19
to
21
|
||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -309,7 +309,7 @@ import { useSchemaContext } from '@object-ui/react'; | |
|
|
||
| // Register object-timeline component | ||
| export const ObjectTimelineRenderer: React.FC<any> = ({ schema, ...props }) => { | ||
| const { dataSource } = useSchemaContext(); | ||
| const { dataSource } = useSchemaContext() || {}; | ||
| return <ObjectTimeline schema={schema} dataSource={dataSource} {...props} />; | ||
|
Comment on lines
311
to
313
|
||
| }; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useSchemaContext()throws when noSchemaRendererProvideris present, souseSchemaContext() || {}will still throw and the|| {}fallback is never reached. If the goal is graceful degradation, useuseContext(SchemaRendererContext)with optional chaining (like ObjectChart/ObjectGallery) or introduce a non-throwing optional hook.