Skip to content

Commit fd8f2f6

Browse files
fix: address all 3 PR review comments
1. Default metric aggregation groupBy to '_all' instead of 'name' for single-bucket results in DashboardRenderer. 2. Reset fetchedValue/error in ObjectMetricWidget when dataSource becomes unavailable, preventing stale server data from masking fallback values. 3. Add test with failing dataSource to verify error state routing in DashboardRenderer object-metric path. Agent-Logs-Url: https://github.com/objectstack-ai/objectui/sessions/7a05b366-541b-4772-8f1f-91bcb9a25ff7 Co-authored-by: xuyushun441-sys <255036401+xuyushun441-sys@users.noreply.github.com>
1 parent 930d802 commit fd8f2f6

File tree

3 files changed

+54
-4
lines changed

3 files changed

+54
-4
lines changed

packages/plugin-dashboard/src/DashboardRenderer.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,12 +135,14 @@ export const DashboardRenderer = forwardRef<HTMLDivElement, DashboardRendererPro
135135
? {
136136
field: widget.valueField || widgetData.aggregate.field,
137137
function: widget.aggregate || widgetData.aggregate.function,
138-
groupBy: widget.categoryField || widgetData.aggregate.groupBy,
138+
// Prefer explicit categoryField or aggregate.groupBy; otherwise, default to a single bucket.
139+
groupBy: widget.categoryField ?? widgetData.aggregate.groupBy ?? '_all',
139140
}
140141
: widget.aggregate ? {
141142
field: widget.valueField || 'value',
142143
function: widget.aggregate,
143-
groupBy: widget.categoryField || 'name',
144+
// Default to a single group unless the user explicitly configures a categoryField.
145+
groupBy: widget.categoryField || '_all',
144146
} : undefined;
145147

146148
return {

packages/plugin-dashboard/src/ObjectMetricWidget.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,11 @@ export const ObjectMetricWidget: React.FC<ObjectMetricWidgetProps> = ({
127127

128128
if (dataSource && objectName) {
129129
fetchMetric(dataSource, mounted);
130+
} else {
131+
// Reset state when dataSource becomes unavailable so we fall back
132+
// to the static fallbackValue instead of showing stale server data.
133+
setFetchedValue(null);
134+
setError(null);
130135
}
131136

132137
return () => { mounted.current = false; };

packages/plugin-dashboard/src/__tests__/DashboardRenderer.widgetData.test.tsx

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@
66
* LICENSE file in the root directory of this source tree.
77
*/
88

9-
import { describe, it, expect } from 'vitest';
10-
import { render } from '@testing-library/react';
9+
import { describe, it, expect, vi } from 'vitest';
10+
import { render, waitFor } from '@testing-library/react';
11+
import { SchemaRendererProvider } from '@object-ui/react';
1112
import { DashboardRenderer } from '../DashboardRenderer';
1213

1314
/**
@@ -1365,4 +1366,46 @@ describe('DashboardRenderer widget data extraction', () => {
13651366
expect(container.textContent).toContain('Revenue Sum');
13661367
expect(container.textContent).toContain('$0');
13671368
});
1369+
1370+
it('should show error state when object-metric dataSource fails', async () => {
1371+
const dataSource = {
1372+
aggregate: vi.fn().mockRejectedValue(new Error('Cube name is required')),
1373+
find: vi.fn(),
1374+
};
1375+
1376+
const schema = {
1377+
type: 'dashboard' as const,
1378+
name: 'test',
1379+
title: 'Test',
1380+
widgets: [
1381+
{
1382+
type: 'metric',
1383+
object: 'opportunity',
1384+
aggregate: 'sum',
1385+
valueField: 'amount',
1386+
layout: { x: 0, y: 0, w: 1, h: 1 },
1387+
options: {
1388+
label: 'Revenue',
1389+
value: '$999',
1390+
},
1391+
},
1392+
],
1393+
} as any;
1394+
1395+
const { container } = render(
1396+
<SchemaRendererProvider dataSource={dataSource}>
1397+
<DashboardRenderer schema={schema} />
1398+
</SchemaRendererProvider>,
1399+
);
1400+
1401+
// Should display the error from the failing aggregate, not the fallback value
1402+
await waitFor(() => {
1403+
const errorEl = container.querySelector('[data-testid="metric-error"]');
1404+
expect(errorEl).toBeTruthy();
1405+
});
1406+
1407+
expect(container.textContent).toContain('Revenue');
1408+
expect(container.textContent).toContain('Cube name is required');
1409+
expect(container.textContent).not.toContain('$999');
1410+
});
13681411
});

0 commit comments

Comments
 (0)