Skip to content

Commit 3167e29

Browse files
os-zhuangclaude
andauthored
fix(analytics): sort dataset selections by the display label for select/lookup dimensions (#3680) (#3693)
`DatasetSelection.order` sorted a select/lookup dimension by its stored value (option value / FK id) while the response renders the resolved display label, so "sort by Account" presented as arbitrary. Order keys naming a label-bearing dimension now sort by the label the user reads: - `queryDataset` builds an `OrderLabelResolver` over the same label capabilities (and #3602 referenced-object read scope, fail-closed) as the display pass and injects it into the executor, which stays engine-free. - The executor substitutes the label as the SORT KEY only — rows keep raw values for the drill sidecars — and resolves it before `applyWindow`, so a "top 10 by account name" truncates the right ten. Such keys are excluded from SQL window pushdown. - A per-request label-fetch cache makes the display pass reuse the ids the sort already fetched (one id→name read total), and the plugin's `fetchRecordLabels` now chunks its `$in` (500/batch) since the sort pass hands over the PRE-window id set. - Measure / plain-dimension / date-bucket ordering is unchanged, pushdown included. Closes #3680. Claude-Session: https://claude.ai/code/session_016ZsNHvYoujMGq66RLrCice Co-authored-by: Claude <noreply@anthropic.com>
1 parent 735f850 commit 3167e29

8 files changed

Lines changed: 574 additions & 43 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
fix(analytics): sort dataset selections by the display label for select/lookup dimensions (#3680)
6+
7+
`DatasetSelection.order` (what a widget's `options.sortBy` lowers to) sorted a
8+
`select` or `lookup`/`master_detail` dimension by its STORED value — the option
9+
value or the foreign-key id — while the response rows carry the resolved display
10+
label. A "sort by Account" therefore ordered by opaque ids and read as arbitrary;
11+
a localized select sorted by its ASCII value while showing a non-ASCII label.
12+
13+
Order keys naming a label-bearing dimension now sort by the display label the
14+
user reads. The executor receives an injected sort-key hook (`OrderLabelResolver`,
15+
built by `queryDataset` over the same label-resolution capabilities and #3602
16+
read scoping as the display pass); only the COMPARISON substitutes the label —
17+
rows keep their raw values until the display pass, so drill metadata still
18+
snapshots stored values, and ordering + windowing stay one adjacent step (a
19+
"top 10 by account name" truncates the right ten).
20+
21+
Cost model: sorting by a measure or a plain/date dimension is unchanged (SQL
22+
pushdown included). A label-ordered `select` resolves from field metadata (no
23+
query). A label-ordered `lookup` costs one batched id→name read over the
24+
pre-window grouped ids (chunked, and reused by the display pass via a
25+
per-request cache), and its window can no longer be pushed into SQL — the
26+
inherent price of ordering by a value the database doesn't store.

content/docs/ui/dashboards.mdx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,11 @@ Notes on behaviour:
145145
truncates a reproducible window instead of an arbitrary subset.
146146
- Ordering is applied to the finished grid, so a **derived measure** is a valid
147147
`sortBy` even though no single SQL statement computes it.
148+
- A `sortBy` naming a **select** or **lookup** dimension orders by the
149+
**display label** the rows render (the option label / the related record's
150+
name), not the stored value or foreign-key id — and the label is resolved
151+
before `limit` applies, so a top-N by name truncates the right N. Sorting by
152+
a measure (the common case) involves no label lookup and is unaffected.
148153
- A `funnel` with no declared stage order falls back to sorting by value
149154
descending.
150155

Lines changed: 274 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,274 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #3680 — `DatasetSelection.order` on a select/lookup dimension sorts by the
5+
* DISPLAY label the user reads, not the stored value.
6+
*
7+
* The executor sorts the assembled grid before `queryDataset` rewrites stored
8+
* values into display labels, so a "sort by Account" used to order by the FK
9+
* id — deterministic, but arbitrary to the reader once the names render. These
10+
* tests pin the fixed contract: the sort key is the label, the window cuts
11+
* AFTER the label sort (top-N by name is the right N), the rows still carry
12+
* raw values until the display pass (drill metadata depends on that), and the
13+
* label fetch is paid at most once per request.
14+
*/
15+
16+
import { describe, it, expect } from 'vitest';
17+
import { DatasetSchema } from '@objectstack/spec/ui';
18+
import type { ExecutionContext } from '@objectstack/spec/kernel';
19+
import { AnalyticsService } from '../analytics-service.js';
20+
import { applyOrdering } from '../dataset-executor.js';
21+
import type { DimensionLabelDeps, FieldMetaLite } from '../dimension-labels.js';
22+
23+
const CTX = { tenantId: 'org_A' } as ExecutionContext;
24+
25+
/** Opportunities grouped by a lookup (account) and a select (status). */
26+
const byAccount = DatasetSchema.parse({
27+
name: 'sales_by_account', label: 'Sales', object: 'opportunity', include: [],
28+
dimensions: [
29+
{ name: 'account', field: 'account', type: 'lookup', label: 'Account' },
30+
{ name: 'status', field: 'status', type: 'string', label: 'Status' },
31+
],
32+
measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }],
33+
});
34+
35+
type FetchCall = { target: string; ids: unknown[]; scope: unknown };
36+
37+
/**
38+
* Label capabilities for the fixture: `account` references `crm_account`
39+
* (ids deliberately ordered OPPOSITE to their names), `status` is a select
40+
* whose option labels invert the stored-value order.
41+
*/
42+
const FIELDS: Record<string, Record<string, FieldMetaLite>> = {
43+
opportunity: {
44+
account: { type: 'lookup', reference: 'crm_account' },
45+
status: {
46+
type: 'select',
47+
options: [
48+
{ value: 'a_active', label: 'Working' },
49+
{ value: 'b_churned', label: 'Ended' },
50+
],
51+
},
52+
},
53+
crm_account: { name: { type: 'text' } },
54+
};
55+
56+
function labelResolver(names: Record<string, string>, calls: FetchCall[] = []): DimensionLabelDeps {
57+
return {
58+
getObjectFields: (obj) => FIELDS[obj],
59+
fetchRecordLabels: async (target, ids, scope) => {
60+
calls.push({ target, ids: [...ids], scope });
61+
const m = new Map<unknown, string>();
62+
if (target === 'crm_account') {
63+
for (const id of ids) if (names[String(id)]) m.set(id, names[String(id)]);
64+
}
65+
return m;
66+
},
67+
};
68+
}
69+
70+
/** ObjectQL-aggregate service (the path with no ordering grammar of its own). */
71+
function aggSvc(rows: Record<string, unknown>[], names: Record<string, string>, calls: FetchCall[] = []) {
72+
return new AnalyticsService({
73+
queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }),
74+
executeAggregate: async () => rows,
75+
labelResolver: labelResolver(names, calls),
76+
});
77+
}
78+
79+
/** Native-SQL service; captures every emitted statement. */
80+
function sqlSvc(rows: Record<string, unknown>[], names: Record<string, string>, captured: string[] = [], calls: FetchCall[] = []) {
81+
return new AnalyticsService({
82+
queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }),
83+
executeRawSql: async (_o, sql) => { captured.push(sql); return rows; },
84+
labelResolver: labelResolver(names, calls),
85+
});
86+
}
87+
88+
describe('#3680 — a lookup order key sorts by the related record name', () => {
89+
it('orders by display name, not the FK id, and keeps the drill sidecar aligned', async () => {
90+
// Raw-id ascending would put a1 (Zebra) before b2 (Apple).
91+
const svc = aggSvc(
92+
[
93+
{ account: 'a1', revenue: 10 },
94+
{ account: 'b2', revenue: 20 },
95+
],
96+
{ a1: 'Zebra', b2: 'Apple' },
97+
);
98+
const result = await svc.queryDataset(
99+
byAccount,
100+
{ dimensions: ['account'], measures: ['revenue'], order: { account: 'asc' } },
101+
CTX,
102+
) as never as { rows: Record<string, unknown>[]; drillRawRows: Record<string, unknown>[] };
103+
expect(result.rows.map((r) => r.account)).toEqual(['Apple', 'Zebra']);
104+
// The drill sidecar snapshots the STORED ids, aligned to the sorted rows.
105+
expect(result.drillRawRows).toEqual([{ account: 'b2' }, { account: 'a1' }]);
106+
});
107+
108+
it('a value the resolver cannot map sorts (and renders) by its raw form', async () => {
109+
const svc = aggSvc(
110+
[
111+
{ account: 'zzz_9', revenue: 1 }, // orphaned — no label
112+
{ account: 'a1', revenue: 2 },
113+
{ account: 'b2', revenue: 3 },
114+
],
115+
{ a1: 'Zebra', b2: 'Alpha' },
116+
);
117+
const result = await svc.queryDataset(
118+
byAccount,
119+
{ dimensions: ['account'], measures: ['revenue'], order: { account: 'asc' } },
120+
CTX,
121+
);
122+
expect(result.rows.map((r) => r.account)).toEqual(['Alpha', 'Zebra', 'zzz_9']);
123+
});
124+
});
125+
126+
describe('#3680 — a select order key sorts by the option label', () => {
127+
it('orders by label even when it inverts the stored-value order', async () => {
128+
// Stored ascending: a_active < b_churned. Labels invert it: Ended < Working.
129+
const svc = aggSvc(
130+
[
131+
{ status: 'a_active', revenue: 10 },
132+
{ status: 'b_churned', revenue: 20 },
133+
],
134+
{},
135+
);
136+
const result = await svc.queryDataset(
137+
byAccount,
138+
{ dimensions: ['status'], measures: ['revenue'], order: { status: 'asc' } },
139+
CTX,
140+
);
141+
expect(result.rows.map((r) => r.status)).toEqual(['Ended', 'Working']);
142+
});
143+
});
144+
145+
describe('#3680 — windowing and query pushdown around a label sort', () => {
146+
it('cuts the window AFTER the label sort, so a top-N by name is the right N', async () => {
147+
const calls: FetchCall[] = [];
148+
const svc = aggSvc(
149+
[
150+
{ account: 'r1', revenue: 1 },
151+
{ account: 'r2', revenue: 2 },
152+
{ account: 'r3', revenue: 3 },
153+
],
154+
{ r1: 'Charlie', r2: 'Alpha', r3: 'Bravo' },
155+
calls,
156+
);
157+
const result = await svc.queryDataset(
158+
byAccount,
159+
{ dimensions: ['account'], measures: ['revenue'], order: { account: 'asc' }, limit: 2 },
160+
CTX,
161+
);
162+
// By stored id the first two would be r1/r2 (Charlie, Alpha) — by name they
163+
// are Alpha and Bravo.
164+
expect(result.rows.map((r) => r.account)).toEqual(['Alpha', 'Bravo']);
165+
// The sort-key fetch covered the FULL pre-window id set…
166+
expect(new Set(calls[0]?.ids)).toEqual(new Set(['r1', 'r2', 'r3']));
167+
// …and the display pass reused it via the per-request cache: ONE fetch total.
168+
expect(calls).toHaveLength(1);
169+
});
170+
171+
it('keeps the window OUT of the SQL when ordering by a label-bearing dimension', async () => {
172+
const captured: string[] = [];
173+
const svc = sqlSvc(
174+
[
175+
{ account: 'a1', revenue: 10 },
176+
{ account: 'b2', revenue: 20 },
177+
],
178+
{ a1: 'Zebra', b2: 'Apple' },
179+
captured,
180+
);
181+
const result = await svc.queryDataset(
182+
byAccount,
183+
{ dimensions: ['account'], measures: ['revenue'], order: { account: 'asc' }, limit: 1 },
184+
CTX,
185+
);
186+
// A SQL ORDER BY would sort the stored id and LIMIT would truncate the
187+
// wrong window — both must stay in memory for a label-bearing order key.
188+
expect(captured[0]).not.toContain('ORDER BY');
189+
expect(captured[0]).not.toContain('LIMIT');
190+
expect(result.rows.map((r) => r.account)).toEqual(['Apple']);
191+
});
192+
193+
it('still pushes a MEASURE ordering into the SQL (the common case is unchanged)', async () => {
194+
const captured: string[] = [];
195+
const svc = sqlSvc([{ account: 'a1', revenue: 10 }], { a1: 'Zebra' }, captured);
196+
await svc.queryDataset(
197+
byAccount,
198+
{ dimensions: ['account'], measures: ['revenue'], order: { revenue: 'desc' }, limit: 5 },
199+
CTX,
200+
);
201+
expect(captured[0]).toContain('ORDER BY "revenue" DESC');
202+
expect(captured[0]).toContain('LIMIT 5');
203+
});
204+
});
205+
206+
describe('#3680 × #3602 — the sort-key label fetch stays scoped', () => {
207+
it('carries the REFERENCED object read scope into the sort-time fetch', async () => {
208+
const calls: FetchCall[] = [];
209+
const svc = new AnalyticsService({
210+
queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }),
211+
executeAggregate: async () => [
212+
{ account: 'a1', revenue: 10 },
213+
{ account: 'b2', revenue: 20 },
214+
],
215+
getReadScope: (object, ctx?: ExecutionContext) => {
216+
if (!ctx?.tenantId) return undefined;
217+
return object === 'crm_account'
218+
? { organization_id: ctx.tenantId, is_public: true }
219+
: { organization_id: ctx.tenantId };
220+
},
221+
labelResolver: labelResolver({ a1: 'Zebra', b2: 'Apple' }, calls),
222+
});
223+
const result = await svc.queryDataset(
224+
byAccount,
225+
{ dimensions: ['account'], measures: ['revenue'], order: { account: 'asc' } },
226+
CTX,
227+
);
228+
expect(result.rows.map((r) => r.account)).toEqual(['Apple', 'Zebra']);
229+
expect(calls).toHaveLength(1);
230+
expect(calls[0].scope).toEqual({ organization_id: 'org_A', is_public: true });
231+
});
232+
233+
it('fails CLOSED when the referenced object scope cannot be resolved: sorts by the stored value, fetches nothing', async () => {
234+
const calls: FetchCall[] = [];
235+
const svc = new AnalyticsService({
236+
queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }),
237+
executeAggregate: async () => [
238+
{ account: 'b2', revenue: 20 },
239+
{ account: 'a1', revenue: 10 },
240+
],
241+
getReadScope: (object) => {
242+
if (object === 'crm_account') throw new Error('scope resolution failed');
243+
return undefined;
244+
},
245+
labelResolver: labelResolver({ a1: 'Zebra', b2: 'Apple' }, calls),
246+
});
247+
const result = await svc.queryDataset(
248+
byAccount,
249+
{ dimensions: ['account'], measures: ['revenue'], order: { account: 'asc' } },
250+
CTX,
251+
);
252+
// No unscoped fetch happened, for the sort key OR the display pass…
253+
expect(calls).toHaveLength(0);
254+
// …and the rows fall back to stored-value order, rendering the raw ids —
255+
// exactly what the display pass does for an unresolvable label.
256+
expect(result.rows.map((r) => r.account)).toEqual(['a1', 'b2']);
257+
});
258+
});
259+
260+
describe('#3680 — applyOrdering sort-key substitution (unit)', () => {
261+
it('compares by the mapped value and falls back to the raw cell where unmapped', () => {
262+
const rows = [{ v: 'id_z' }, { v: 'id_a' }, { v: 'raw' }];
263+
const sorted = applyOrdering(rows, { v: 'asc' }, {
264+
v: new Map<unknown, unknown>([['id_z', 'Apple'], ['id_a', 'Zebra']]),
265+
});
266+
expect(sorted.map((r) => r.v)).toEqual(['id_z', 'raw', 'id_a']);
267+
});
268+
269+
it('keeps nulls last regardless of the substitution map', () => {
270+
const rows = [{ v: null }, { v: 'id_a' }];
271+
const sorted = applyOrdering(rows, { v: 'desc' }, { v: new Map([['id_a', 'Alpha']]) });
272+
expect(sorted.map((r) => r.v)).toEqual(['id_a', null]);
273+
});
274+
});

0 commit comments

Comments
 (0)