Skip to content

Commit e16ed2d

Browse files
os-zhuangclaude
andauthored
fix(dashboard,charts): send widget query options to the server, order funnel stages by the pipeline (#2864)
`DatasetWidget` never read `widget.options`. Four keys an author writes there change the query the server compiles, so a widget declaring `options: { dateGranularity: 'month' }` grouped by the raw timestamp and drew one bar per record, and `sortBy`/`sortOrder` produced no ordering at all. - `DatasetWidget` lowers `options.dateGranularity`, `options.sortBy` + `options.sortOrder`, and `options.limit` into the `DatasetSelection` it posts. A `sortBy` naming something the widget does not project is dropped rather than sent, so a stale sort key left by an edit degrades to "unordered" instead of failing the widget against the server's stricter validation. These keys join the refetch signature, so editing one in the designer refetches instead of re-rendering the previous grid. - Funnel stages follow a declared order. `AdvancedChartImpl` sorted funnel segments by value descending, unconditionally — overriding any server ordering and rendering a sales pipeline as a tidy narrowing shape whatever the stages' real sequence, which hides the very anomaly (a bulge at Proposal) the chart exists to show. It now honours `categoryOrder`, derived from the dimension field's picklist option order — the pipeline order already declared on the object — or from an explicit `options.stageOrder`. With no declared order the value-descending default is unchanged, and a category missing from the order is kept after the declared ones rather than dropped. - New `@object-ui/core` helpers `buildCategoryOrder` / `buildCategoryRank`, keyed by both stored value and display label like `buildOptionColorMap`, so ordering works whether or not the server resolved the dimension's labels. Requires the framework-side fix in objectstack#3588 for the selection keys to take effect server-side. Claude-Session: https://claude.ai/code/session_01SEZLBGyNoJMLvtGPqsMPRC Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7b21891 commit e16ed2d

8 files changed

Lines changed: 560 additions & 16 deletions

File tree

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
"@object-ui/core": patch
3+
"@object-ui/plugin-charts": patch
4+
"@object-ui/plugin-dashboard": patch
5+
---
6+
7+
fix(dashboard,charts): send widget `dateGranularity`/`sortBy`/`limit` to the query, and give funnels a real stage order (framework#3588)
8+
9+
`DatasetWidget` never read `widget.options`. Four keys an author writes there
10+
change the query the server compiles, so a widget declaring
11+
`options: { dateGranularity: 'month' }` grouped by the raw timestamp and drew
12+
one bar per record, and `sortBy`/`sortOrder` produced no ordering at all.
13+
14+
- `DatasetWidget` lowers `options.dateGranularity`, `options.sortBy` +
15+
`options.sortOrder`, and `options.limit` into the `DatasetSelection` it posts.
16+
A `sortBy` naming something the widget does not project is dropped rather than
17+
sent, so a stale sort key left by an edit degrades to "unordered" instead of
18+
failing the widget against the server's stricter validation. These keys also
19+
join the refetch signature, so editing one in the designer refetches instead
20+
of re-rendering the previous grid.
21+
- Funnel stages follow a **declared order**. `AdvancedChartImpl` sorted funnel
22+
segments by value descending, unconditionally — which overrode any server
23+
ordering and rendered a sales pipeline as a tidy narrowing shape whatever the
24+
stages' real sequence, hiding the very anomaly (a bulge at Proposal) the chart
25+
exists to show. It now honours a `categoryOrder`, which `DatasetWidget`
26+
derives from the dimension field's picklist option order — the pipeline order
27+
an author already declared on the object — or from an explicit
28+
`options.stageOrder`. With no declared order the value-descending default is
29+
unchanged, and a category missing from the order is kept (after the declared
30+
ones), never dropped.
31+
- New `@object-ui/core` helpers `buildCategoryOrder` / `buildCategoryRank`,
32+
keyed by both the stored value and the display label like the existing
33+
`buildOptionColorMap`, so ordering works whether or not the server resolved
34+
the dimension's labels.
35+
36+
Requires the framework-side fix in objectstack#3588 for the selection keys to
37+
take effect server-side.
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
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+
* framework#3588 — a picklist's declared option order is the domain order, and
11+
* ordered-sequence charts (funnel/pyramid) must render along it rather than
12+
* alphabetically or by value.
13+
*/
14+
15+
import { describe, it, expect } from 'vitest';
16+
import { buildCategoryOrder, buildCategoryRank } from '../chart-series';
17+
18+
const STAGES = [
19+
{ value: 'qualification', label: 'Qualification' },
20+
{ value: 'needs_analysis', label: 'Needs Analysis' },
21+
{ value: 'proposal', label: 'Proposal' },
22+
{ value: 'negotiation', label: 'Negotiation' },
23+
];
24+
25+
describe('buildCategoryOrder', () => {
26+
it('keeps the declared order and keys by BOTH value and label', () => {
27+
expect(buildCategoryOrder(STAGES)).toEqual([
28+
'qualification', 'Qualification',
29+
'needs_analysis', 'Needs Analysis',
30+
'proposal', 'Proposal',
31+
'negotiation', 'Negotiation',
32+
]);
33+
});
34+
35+
it('accepts bare string options', () => {
36+
expect(buildCategoryOrder(['a', 'b'])).toEqual(['a', 'b']);
37+
});
38+
39+
it('does not duplicate a label identical to its value', () => {
40+
expect(buildCategoryOrder([{ value: 'open', label: 'open' }])).toEqual(['open']);
41+
});
42+
43+
it('returns null for a field with no options, so callers keep their default', () => {
44+
expect(buildCategoryOrder(undefined)).toBeNull();
45+
expect(buildCategoryOrder([])).toBeNull();
46+
expect(buildCategoryOrder('nope')).toBeNull();
47+
});
48+
});
49+
50+
describe('buildCategoryRank', () => {
51+
it('ranks value and label of the same option identically enough to sort together', () => {
52+
const rank = buildCategoryRank(buildCategoryOrder(STAGES))!;
53+
// A row keyed by the stored value and one keyed by the display label both
54+
// sort into the same slot relative to the other stages.
55+
expect(rank.get('qualification')).toBeLessThan(rank.get('Needs Analysis')!);
56+
expect(rank.get('Proposal')).toBeLessThan(rank.get('negotiation')!);
57+
});
58+
59+
it('sorts the real pipeline out of alphabetical order', () => {
60+
const rank = buildCategoryRank(buildCategoryOrder(STAGES))!;
61+
// Alphabetical is the order analytics returns; the declared order is not it.
62+
const alphabetical = ['Needs Analysis', 'Negotiation', 'Proposal', 'Qualification'];
63+
const sorted = [...alphabetical].sort(
64+
(a, b) => (rank.get(a) ?? Infinity) - (rank.get(b) ?? Infinity),
65+
);
66+
expect(sorted).toEqual(['Qualification', 'Needs Analysis', 'Proposal', 'Negotiation']);
67+
});
68+
69+
it('first occurrence wins for a repeated key', () => {
70+
const rank = buildCategoryRank(['a', 'b', 'a'])!;
71+
expect(rank.get('a')).toBe(0);
72+
});
73+
74+
it('returns null for an empty/absent order', () => {
75+
expect(buildCategoryRank(null)).toBeNull();
76+
expect(buildCategoryRank([])).toBeNull();
77+
});
78+
});

packages/core/src/utils/chart-series.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,58 @@ export function buildOptionColorMap(options: unknown): Record<string, string> |
197197
return Object.keys(map).length > 0 ? map : null;
198198
}
199199

200+
/**
201+
* Build the DECLARED category order from a select field's `options` — the
202+
* sequence the author wrote them in on the object (framework#3588).
203+
*
204+
* A picklist's option order is already the domain order: a sales `stage` field
205+
* lists Qualification → Needs Analysis → Proposal → Negotiation because that IS
206+
* the pipeline. Analytics groups by that field and returns buckets in whatever
207+
* order the GROUP BY produced (usually alphabetical), which for an
208+
* ordered-sequence chart — a funnel above all — draws a shape that reads as a
209+
* pipeline but isn't one.
210+
*
211+
* Emits BOTH the stored `value` and its display `label` per option, adjacent
212+
* and in declared order, for the same reason {@link buildOptionColorMap} keys
213+
* by both: a chart row's category may carry either, depending on whether the
214+
* server resolved the dimension's labels. Rank is "index of first match", so
215+
* either key ranks the option identically.
216+
*
217+
* Returns `null` for a field with no options, so callers keep their existing
218+
* ordering (a funnel falls back to descending by value).
219+
*/
220+
export function buildCategoryOrder(options: unknown): string[] | null {
221+
if (!Array.isArray(options) || options.length === 0) return null;
222+
const keys: string[] = [];
223+
for (const opt of options) {
224+
if (typeof opt === 'string' || typeof opt === 'number' || typeof opt === 'boolean') {
225+
keys.push(String(opt));
226+
continue;
227+
}
228+
if (opt && typeof opt === 'object') {
229+
const o = opt as { value?: unknown; label?: unknown };
230+
if (o.value != null) keys.push(String(o.value));
231+
if (o.label != null && String(o.label) !== String(o.value)) keys.push(String(o.label));
232+
}
233+
}
234+
return keys.length > 0 ? keys : null;
235+
}
236+
237+
/**
238+
* Rank map for {@link buildCategoryOrder} keys — `category → position`, first
239+
* occurrence wins. Categories absent from the declared order get no entry; the
240+
* caller decides where those sort (see `AdvancedChartImpl`, which keeps them
241+
* after the declared ones rather than dropping them).
242+
*/
243+
export function buildCategoryRank(order: string[] | null | undefined): Map<string, number> | null {
244+
if (!Array.isArray(order) || order.length === 0) return null;
245+
const rank = new Map<string, number>();
246+
order.forEach((key, i) => {
247+
if (!rank.has(key)) rank.set(key, i);
248+
});
249+
return rank.size > 0 ? rank : null;
250+
}
251+
200252
/**
201253
* Build a `{ value → label }` map from a select/enum field's `options`, for
202254
* resolving a grouped dimension's stored value to its display label (fed to
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
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+
* framework#3588 — a funnel's shape asserts a sequence.
11+
*
12+
* The renderer used to sort funnel segments by value descending,
13+
* unconditionally — so a sales pipeline rendered as a tidy narrowing shape
14+
* regardless of the stages' real order, and any server-side ordering was
15+
* silently overridden. With a declared `categoryOrder` the pipeline order wins;
16+
* without one the value-descending default is preserved.
17+
*/
18+
19+
import React from 'react';
20+
import { describe, it, expect, vi, afterEach } from 'vitest';
21+
import { render, cleanup } from '@testing-library/react';
22+
23+
// Recharts' ResponsiveContainer measures its parent via ResizeObserver, which
24+
// reports 0×0 under the headless DOM — so the funnel never paints. Replace it
25+
// with a fixed-size passthrough so segments actually render.
26+
vi.mock('recharts', async () => {
27+
const actual = await vi.importActual<any>('recharts');
28+
return {
29+
...actual,
30+
ResponsiveContainer: ({ children }: any) =>
31+
React.cloneElement(children, { width: 320, height: 320 }),
32+
};
33+
});
34+
35+
import AdvancedChartImpl from './AdvancedChartImpl';
36+
37+
afterEach(cleanup);
38+
39+
/**
40+
* Rows as analytics returns them: alphabetical, and deliberately NOT
41+
* monotonically narrowing — Proposal (200) is the largest, so a value sort and
42+
* the declared pipeline order produce visibly different funnels.
43+
*/
44+
const PIPELINE = [
45+
{ stage: 'Needs Analysis', total_amount: 120 },
46+
{ stage: 'Negotiation', total_amount: 90 },
47+
{ stage: 'Proposal', total_amount: 200 },
48+
{ stage: 'Qualification', total_amount: 150 },
49+
];
50+
51+
/** As `buildCategoryOrder` emits it: value and label adjacent, in declared order. */
52+
const DECLARED = [
53+
'qualification', 'Qualification',
54+
'needs_analysis', 'Needs Analysis',
55+
'proposal', 'Proposal',
56+
'negotiation', 'Negotiation',
57+
];
58+
59+
/**
60+
* The segment labels in the order Recharts drew them. `<Funnel>` renders one
61+
* `<text>` label per trapezoid in source order, which is exactly the ordering
62+
* under test.
63+
*/
64+
function drawnOrder(container: HTMLElement): string[] {
65+
return Array.from(container.querySelectorAll('text'))
66+
.map((t) => t.textContent?.trim() ?? '')
67+
.filter(Boolean);
68+
}
69+
70+
function renderFunnel(extra: Record<string, unknown>) {
71+
return render(
72+
<AdvancedChartImpl
73+
chartType="funnel"
74+
data={PIPELINE}
75+
xAxisKey="stage"
76+
series={[{ dataKey: 'total_amount' }]}
77+
config={{ total_amount: { label: 'Amount' } }}
78+
isAnimationActive={false}
79+
{...extra}
80+
/>,
81+
);
82+
}
83+
84+
describe('funnel stage order (#3588)', () => {
85+
it('draws every stage (guards the assertions below against an empty render)', () => {
86+
const { container } = renderFunnel({ categoryOrder: DECLARED });
87+
expect(container.querySelectorAll('.recharts-funnel-trapezoid')).toHaveLength(4);
88+
expect(drawnOrder(container)).toHaveLength(4);
89+
});
90+
91+
it('draws in the DECLARED pipeline order, not by value', () => {
92+
const { container } = renderFunnel({ categoryOrder: DECLARED });
93+
expect(drawnOrder(container)).toEqual([
94+
'Qualification', 'Needs Analysis', 'Proposal', 'Negotiation',
95+
]);
96+
});
97+
98+
it('keeps the value-descending default when no order is declared', () => {
99+
const { container } = renderFunnel({});
100+
expect(drawnOrder(container)).toEqual([
101+
'Proposal', 'Qualification', 'Needs Analysis', 'Negotiation',
102+
]);
103+
});
104+
105+
it('keeps a category missing from the declared order, after the declared ones', () => {
106+
const { container } = render(
107+
<AdvancedChartImpl
108+
chartType="funnel"
109+
data={[...PIPELINE, { stage: 'Closed Won', total_amount: 10 }]}
110+
xAxisKey="stage"
111+
series={[{ dataKey: 'total_amount' }]}
112+
config={{ total_amount: { label: 'Amount' } }}
113+
isAnimationActive={false}
114+
categoryOrder={DECLARED}
115+
/>,
116+
);
117+
// Never dropped — an unlisted stage still renders, and lands last.
118+
expect(drawnOrder(container)).toEqual([
119+
'Qualification', 'Needs Analysis', 'Proposal', 'Negotiation', 'Closed Won',
120+
]);
121+
});
122+
123+
it('orders by the stored VALUE too, for rows the server left unlabeled', () => {
124+
const { container } = render(
125+
<AdvancedChartImpl
126+
chartType="funnel"
127+
data={[
128+
{ stage: 'negotiation', total_amount: 90 },
129+
{ stage: 'qualification', total_amount: 150 },
130+
{ stage: 'proposal', total_amount: 200 },
131+
]}
132+
xAxisKey="stage"
133+
series={[{ dataKey: 'total_amount' }]}
134+
config={{ total_amount: { label: 'Amount' } }}
135+
isAnimationActive={false}
136+
categoryOrder={DECLARED}
137+
/>,
138+
);
139+
expect(drawnOrder(container)).toEqual(['qualification', 'proposal', 'negotiation']);
140+
});
141+
});

packages/plugin-charts/src/AdvancedChartImpl.tsx

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import {
4444
ChartConfig
4545
} from './ChartContainerImpl';
4646
import { mapScatterClick, mapTreemapClick, mapSankeyClick } from './chartDrillEvents';
47+
import { buildCategoryRank } from '@object-ui/core';
4748

4849
// Default color fallback for chart series
4950
const DEFAULT_CHART_COLOR = 'hsl(var(--primary))';
@@ -109,6 +110,20 @@ export interface AdvancedChartImplProps {
109110
* categories absent here fall back to the positional palette, then the theme.
110111
*/
111112
categoryColors?: Record<string, string>;
113+
/**
114+
* Declared category order for ordered-sequence charts (funnel / pyramid),
115+
* keyed like {@link categoryColors} by the category VALUE or its display
116+
* LABEL, listed in domain order.
117+
*
118+
* A funnel's shape asserts a sequence, so without this the renderer can only
119+
* guess at one and sorts by value descending. That is right for a generic
120+
* "biggest first" funnel and wrong for a sales pipeline, where the stages
121+
* have a declared order (Qualification → Needs Analysis → Proposal →
122+
* Negotiation) that a healthy pipeline happens to narrow along — and an
123+
* unhealthy one does not. When supplied, this order wins; categories not
124+
* listed keep their incoming relative order after the listed ones.
125+
*/
126+
categoryOrder?: string[];
112127
/**
113128
* Optional drill-down click handler. Fires when a chart segment is clicked
114129
* with `{ category, series, value }`. Wired for bar/horizontal-bar/line/
@@ -160,6 +175,7 @@ export default function AdvancedChartImpl({
160175
className = '',
161176
colors,
162177
categoryColors,
178+
categoryOrder,
163179
onChartClick,
164180
isAnimationActive,
165181
}: AdvancedChartImplProps) {
@@ -410,15 +426,32 @@ export default function AdvancedChartImpl({
410426
const funnelClickProps = handleFunnelClick
411427
? { onClick: handleFunnelClick, style: { cursor: 'pointer' as const } }
412428
: {};
413-
// Recharts <Funnel> draws segments in source order. For a visually
414-
// correct funnel (largest at top, narrowing down) we sort descending
415-
// by the numeric value of `dataKey` so authors don't have to pre-sort
416-
// their dashboard data.
417-
const funnelData = [...data].sort((a, b) => {
418-
const av = Number(a?.[dataKey] ?? 0);
419-
const bv = Number(b?.[dataKey] ?? 0);
420-
return bv - av;
421-
});
429+
// Recharts <Funnel> draws segments in source order, so this decides the
430+
// sequence the funnel asserts.
431+
//
432+
// With a DECLARED order (framework#3588) — a stage picklist's own option
433+
// order, or an explicit `stageOrder` — that order wins: a sales funnel must
434+
// read Qualification → Needs Analysis → Proposal → Negotiation whether or
435+
// not each stage happens to hold more value than the next. Sorting such a
436+
// pipeline by value would manufacture a tidy narrowing shape and hide the
437+
// very anomaly (a bulge at Proposal) the chart exists to reveal. Categories
438+
// missing from the declared order keep their incoming relative order,
439+
// after the declared ones — never dropped.
440+
//
441+
// Without one, keep the long-standing default: descending by value, so a
442+
// generic funnel still narrows downward without authors pre-sorting.
443+
const categoryRank = buildCategoryRank(categoryOrder);
444+
const funnelData = categoryRank
445+
? [...data].sort((a, b) => {
446+
const ar = categoryRank.get(String(a?.[xAxisKey] ?? '')) ?? Number.MAX_SAFE_INTEGER;
447+
const br = categoryRank.get(String(b?.[xAxisKey] ?? '')) ?? Number.MAX_SAFE_INTEGER;
448+
return ar - br;
449+
})
450+
: [...data].sort((a, b) => {
451+
const av = Number(a?.[dataKey] ?? 0);
452+
const bv = Number(b?.[dataKey] ?? 0);
453+
return bv - av;
454+
});
422455
return (
423456
<ChartContainer config={config} className={className} {...containerProps}>
424457
<FunnelChart>

0 commit comments

Comments
 (0)