Skip to content

Commit c197734

Browse files
authored
Merge pull request #836 from objectstack-ai/copilot/fix-revenue-by-account-chart
2 parents df7deed + bd65d52 commit c197734

File tree

14 files changed

+214
-65
lines changed

14 files changed

+214
-65
lines changed

ROADMAP.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -856,6 +856,7 @@ The `FlowDesigner` is a canvas-based flow editor that bridges the gap between th
856856
- [x] **P1: Opportunity ↔ Contact Junction**`opportunity_contacts` object with role-based relationships + 7 seed records
857857
- [x] **P1: Contact ↔ Event Attendees**`participants` field populated on all event seed data
858858
- [x] **P2: Dashboard Dynamic Data** — "Revenue by Account" widget using `provider: 'object'` aggregation. DashboardRenderer now delegates `provider: 'object'` widgets to ObjectChart (`type: 'object-chart'`) for async data loading + client-side aggregation (sum/count/avg/min/max)
859+
- [x] **P2: Fix Revenue by Account Chart** — Fixed 3 bugs preventing "Revenue by Account" chart from displaying data: (1) ObjectChart `extractRecords()` now handles `results.data` and `results.value` response formats in addition to `results.records`, (2) DashboardRenderer auto-adapts series `dataKey` from `aggregate.field` when aggregate config is present, (3) CRM dashboard `yField` aligned to aggregate field `amount` (was `total`). Centralized `extractRecords()` utility in `@object-ui/core` and unified data extraction across all 6 data components (ObjectChart, ObjectMap, ObjectCalendar, ObjectGantt, ObjectTimeline, ObjectKanban). Added 16 new tests.
859860
- [x] **P2: App Branding**`logo`, `favicon`, `backgroundColor` fields on CRM app
860861
- [x] **P3: Pages** — Settings page (utility) and Getting Started page (onboarding)
861862
- [x] **P2: Spec Compliance Audit** — Fixed `variant: 'danger'``'destructive'` (4 actions), `columns: string``number` (33 form sections), added `type: 'dashboard'` to dashboard

examples/crm/src/dashboards/crm.dashboard.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,12 +181,12 @@ export const CrmDashboard = {
181181
type: 'bar' as const,
182182
object: 'opportunity',
183183
categoryField: 'account',
184-
valueField: 'total',
184+
valueField: 'amount',
185185
aggregate: 'sum',
186186
layout: { x: 0, y: 7, w: 4, h: 2 },
187187
options: {
188188
xField: 'account',
189-
yField: 'total',
189+
yField: 'amount',
190190
data: {
191191
provider: 'object' as const,
192192
object: 'opportunity',

packages/core/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export * from './validation/index.js';
1515
export * from './builder/schema-builder.js';
1616
export * from './utils/filter-converter.js';
1717
export * from './utils/normalize-quick-filter.js';
18+
export * from './utils/extract-records.js';
1819
export * from './evaluator/index.js';
1920
export * from './actions/index.js';
2021
export * from './query/index.js';
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
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 } from 'vitest';
10+
import { extractRecords } from '../extract-records';
11+
12+
describe('extractRecords', () => {
13+
const sampleData = [{ id: 1, name: 'A' }, { id: 2, name: 'B' }];
14+
15+
it('should return the array directly when results is an array', () => {
16+
expect(extractRecords(sampleData)).toEqual(sampleData);
17+
});
18+
19+
it('should extract from results.records', () => {
20+
expect(extractRecords({ records: sampleData })).toEqual(sampleData);
21+
});
22+
23+
it('should extract from results.data', () => {
24+
expect(extractRecords({ data: sampleData })).toEqual(sampleData);
25+
});
26+
27+
it('should extract from results.value', () => {
28+
expect(extractRecords({ value: sampleData })).toEqual(sampleData);
29+
});
30+
31+
it('should return empty array for null/undefined', () => {
32+
expect(extractRecords(null)).toEqual([]);
33+
expect(extractRecords(undefined)).toEqual([]);
34+
});
35+
36+
it('should return empty array for non-array/non-object', () => {
37+
expect(extractRecords('string')).toEqual([]);
38+
expect(extractRecords(42)).toEqual([]);
39+
});
40+
41+
it('should return empty array for object without recognized keys', () => {
42+
expect(extractRecords({ total: 100 })).toEqual([]);
43+
});
44+
45+
it('should prefer records over data and value', () => {
46+
const records = [{ id: 1 }];
47+
const data = [{ id: 2 }];
48+
expect(extractRecords({ records, data })).toEqual(records);
49+
});
50+
});
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
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+
/**
10+
* Extract an array of records from various API response formats.
11+
* Supports: raw array, { records: [] }, { data: [] }, { value: [] }.
12+
*
13+
* This utility normalises the different shapes returned by ObjectStack,
14+
* OData, and MSW mock endpoints so that every data-fetching component
15+
* can rely on a single extraction path.
16+
*/
17+
export function extractRecords(results: unknown): any[] {
18+
if (Array.isArray(results)) {
19+
return results;
20+
}
21+
if (results && typeof results === 'object') {
22+
if (Array.isArray((results as any).records)) {
23+
return (results as any).records;
24+
}
25+
if (Array.isArray((results as any).data)) {
26+
return (results as any).data;
27+
}
28+
if (Array.isArray((results as any).value)) {
29+
return (results as any).value;
30+
}
31+
}
32+
return [];
33+
}

packages/plugin-calendar/src/ObjectCalendar.tsx

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import { CalendarView, type CalendarEvent } from './CalendarView';
2828
import { usePullToRefresh } from '@object-ui/mobile';
2929
import { useNavigationOverlay } from '@object-ui/react';
3030
import { NavigationOverlay } from '@object-ui/components';
31+
import { extractRecords } from '@object-ui/core';
3132

3233
export interface CalendarSchema {
3334
type: 'calendar';
@@ -219,17 +220,7 @@ export const ObjectCalendar: React.FC<ObjectCalendarProps> = ({
219220
$orderby: convertSortToQueryParams(schema.sort),
220221
});
221222

222-
let items: any[] = [];
223-
224-
if (Array.isArray(result)) {
225-
items = result;
226-
} else if (result && typeof result === 'object') {
227-
if (Array.isArray((result as any).data)) {
228-
items = (result as any).data;
229-
} else if (Array.isArray((result as any).value)) {
230-
items = (result as any).value;
231-
}
232-
}
223+
let items: any[] = extractRecords(result);
233224

234225
if (isMounted) {
235226
setData(items);

packages/plugin-charts/src/ObjectChart.tsx

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import React, { useState, useEffect } from 'react';
33
import { useDataScope, useSchemaContext } from '@object-ui/react';
44
import { ChartRenderer } from './ChartRenderer';
5-
import { ComponentRegistry } from '@object-ui/core';
5+
import { ComponentRegistry, extractRecords } from '@object-ui/core';
66

77
/**
88
* Client-side aggregation for fetched records.
@@ -49,6 +49,9 @@ export function aggregateRecords(
4949
});
5050
}
5151

52+
// Re-export extractRecords from @object-ui/core for backward compatibility
53+
export { extractRecords } from '@object-ui/core';
54+
5255
export const ObjectChart = (props: any) => {
5356
const { schema } = props;
5457
const context = useSchemaContext();
@@ -68,14 +71,7 @@ export const ObjectChart = (props: any) => {
6871
$filter: schema.filter
6972
});
7073

71-
let data: any[] = [];
72-
if (Array.isArray(results)) {
73-
data = results;
74-
} else if (results && typeof results === 'object') {
75-
if (Array.isArray((results as any).records)) {
76-
data = (results as any).records;
77-
}
78-
}
74+
let data: any[] = extractRecords(results);
7975

8076
// Apply client-side aggregation when aggregate config is provided
8177
if (schema.aggregate && data.length > 0) {

packages/plugin-charts/src/__tests__/ObjectChart.aggregation.test.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import { describe, it, expect } from 'vitest';
10-
import { aggregateRecords } from '../ObjectChart';
10+
import { aggregateRecords, extractRecords } from '../ObjectChart';
1111

1212
describe('aggregateRecords', () => {
1313
const records = [
@@ -124,3 +124,43 @@ describe('aggregateRecords', () => {
124124
expect(result[0].amount).toBe(100); // non-numeric value coerced to 0, sum is 0 + 100
125125
});
126126
});
127+
128+
describe('extractRecords', () => {
129+
const sampleData = [{ id: 1, name: 'A' }, { id: 2, name: 'B' }];
130+
131+
it('should return the array directly when results is an array', () => {
132+
expect(extractRecords(sampleData)).toEqual(sampleData);
133+
});
134+
135+
it('should extract from results.records', () => {
136+
expect(extractRecords({ records: sampleData })).toEqual(sampleData);
137+
});
138+
139+
it('should extract from results.data', () => {
140+
expect(extractRecords({ data: sampleData })).toEqual(sampleData);
141+
});
142+
143+
it('should extract from results.value', () => {
144+
expect(extractRecords({ value: sampleData })).toEqual(sampleData);
145+
});
146+
147+
it('should return empty array for null/undefined', () => {
148+
expect(extractRecords(null)).toEqual([]);
149+
expect(extractRecords(undefined)).toEqual([]);
150+
});
151+
152+
it('should return empty array for non-array/non-object', () => {
153+
expect(extractRecords('string')).toEqual([]);
154+
expect(extractRecords(42)).toEqual([]);
155+
});
156+
157+
it('should return empty array for object without recognized keys', () => {
158+
expect(extractRecords({ total: 100 })).toEqual([]);
159+
});
160+
161+
it('should prefer records over data and value', () => {
162+
const records = [{ id: 1 }];
163+
const data = [{ id: 2 }];
164+
expect(extractRecords({ records, data })).toEqual(records);
165+
});
166+
});

packages/plugin-dashboard/src/DashboardRenderer.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,13 +129,17 @@ export const DashboardRenderer = forwardRef<HTMLDivElement, DashboardRendererPro
129129

130130
// provider: 'object' — delegate to ObjectChart for async data loading
131131
if (isObjectProvider(widgetData)) {
132+
// When aggregate is configured, use the aggregate field as the
133+
// series dataKey so it matches the output of aggregateRecords()
134+
// which produces { [groupBy]: key, [field]: result }.
135+
const effectiveYField = widgetData.aggregate?.field || yField;
132136
return {
133137
type: 'object-chart',
134138
chartType: widgetType,
135139
objectName: widgetData.object || widget.object,
136140
aggregate: widgetData.aggregate,
137141
xAxisKey: xAxisKey,
138-
series: [{ dataKey: yField }],
142+
series: [{ dataKey: effectiveYField }],
139143
colors: CHART_COLORS,
140144
className: "h-[200px] sm:h-[250px] md:h-[300px]"
141145
};

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

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ describe('DashboardRenderer widget data extraction', () => {
296296
groupBy: 'account',
297297
});
298298
expect(chartSchema.xAxisKey).toBe('account');
299-
expect(chartSchema.series).toEqual([{ dataKey: 'total' }]);
299+
expect(chartSchema.series).toEqual([{ dataKey: 'amount' }]);
300300
// Must NOT have an empty data array – data comes from the object source
301301
expect(chartSchema.data).toBeUndefined();
302302
});
@@ -411,4 +411,68 @@ describe('DashboardRenderer widget data extraction', () => {
411411
expect(pivotSchema.data).toBeUndefined();
412412
}
413413
});
414+
415+
it('should use yField as series dataKey when provider: object has no aggregate', () => {
416+
const schema = {
417+
type: 'dashboard' as const,
418+
name: 'test',
419+
title: 'Test',
420+
widgets: [
421+
{
422+
type: 'line',
423+
title: 'No Aggregate',
424+
object: 'opportunity',
425+
layout: { x: 0, y: 0, w: 4, h: 2 },
426+
options: {
427+
xField: 'date',
428+
yField: 'revenue',
429+
data: {
430+
provider: 'object',
431+
object: 'opportunity',
432+
},
433+
},
434+
},
435+
],
436+
} as any;
437+
438+
const { container } = render(<DashboardRenderer schema={schema} />);
439+
const schemas = getRenderedSchemas(container);
440+
const chartSchema = schemas.find(s => s.type === 'object-chart');
441+
442+
expect(chartSchema).toBeDefined();
443+
expect(chartSchema.series).toEqual([{ dataKey: 'revenue' }]);
444+
});
445+
446+
it('should auto-adapt series dataKey from aggregate.field even when yField differs', () => {
447+
const schema = {
448+
type: 'dashboard' as const,
449+
name: 'test',
450+
title: 'Test',
451+
widgets: [
452+
{
453+
type: 'bar',
454+
title: 'Mismatched yField',
455+
object: 'opportunity',
456+
layout: { x: 0, y: 0, w: 4, h: 2 },
457+
options: {
458+
xField: 'account',
459+
yField: 'total',
460+
data: {
461+
provider: 'object',
462+
object: 'opportunity',
463+
aggregate: { field: 'amount', function: 'sum', groupBy: 'account' },
464+
},
465+
},
466+
},
467+
],
468+
} as any;
469+
470+
const { container } = render(<DashboardRenderer schema={schema} />);
471+
const schemas = getRenderedSchemas(container);
472+
const chartSchema = schemas.find(s => s.type === 'object-chart');
473+
474+
expect(chartSchema).toBeDefined();
475+
// Even though yField is 'total', the series should use aggregate.field ('amount')
476+
expect(chartSchema.series).toEqual([{ dataKey: 'amount' }]);
477+
});
414478
});

0 commit comments

Comments
 (0)