Skip to content

Commit 76a9ead

Browse files
committed
fix(plugin-grid): refuse the retired string sort clause at object-grid's own read site (objectui#8767)
`ObjectGrid` reads `schema.sort` and lowers it with PRIVATE code, so PR #8758's narrowing of the shared sink `convertSortToQueryParams` never reached it: after that PR a bare `object-grid` still forwarded a runtime string verbatim to `$orderby`, while the SAME key routed through `object-view` was refused with a diagnostic. One key, two meanings, chosen by which block you are on — the per-block divergence the objectui#8221 ruling declined by name. Route C, as ruled by the maintainer 2026-09-10: keep the wire shape, refuse the string. The string arm now calls the shared sink for its refusal alone — that reporter names the offending spelling once per spelling and answers `undefined`, so the query carries no `$orderby` — and the return value is deliberately unused, because the array arm keeps lowering to this block's own `"field order"` join string. Route B (routing the whole key through the sink, which would send its `{field: direction}` map instead) was explicitly not taken; the export path and the header-arrow reader `parseSchemaSort` are untouched. - `gridRetiredStringSort-8767.test.tsx` pins BOTH halves, so the pin cannot pass by refusing everything: a string reaches no `$orderby` and reports once per spelling; the array arm still emits `'name desc'` and `'status asc, name desc'` with no diagnostic. - `serverSorting.test.tsx`'s "replaces the declared sort" case authored the retired string and asserted it went out verbatim — that assertion is this card's own subject, so its fixture is re-authored in the declared array spelling and its #3106 subject is preserved. - Two comments this change falsifies are corrected, comment-only: the `parseSchemaSort` docblock no longer claims the fetch path reads all three spellings, and both it and the header-arrow read site now record that this reader is WIDER than the fetch path and that closing the gap is route B. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jmxdo7bmeqCQHLSfmLVX9w
1 parent 72bcd77 commit 76a9ead

4 files changed

Lines changed: 266 additions & 7 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
'@object-ui/plugin-grid': minor
3+
---
4+
5+
`object-grid` REFUSES the retired string `sort` clause at its own read site
6+
(objectui#8767, maintainer ruling 2026-09-10 — route C).
7+
8+
**Breaking, deliberately.** A `sort: "name desc"` on an `object-grid` node no
9+
longer reaches `$orderby`. It is reported once per spelling with the diagnostic
10+
objectui#8221 / PR #8758 already ship — the message names the offending value
11+
and prescribes the array form — and the query goes out carrying no ordering at
12+
all, exactly as the same key already behaves on `object-view`.
13+
14+
**Why it was still lowering.** The #8221 ruling retired the legacy string
15+
clause: one spelling, the array, everywhere. PR #8758 narrowed the shared sink
16+
`convertSortToQueryParams` and every declaration that published a string arm,
17+
but `ObjectGrid` never used that sink — it reads `schema.sort` and lowers it
18+
with private code, so a bare grid went on honouring at runtime a spelling
19+
`object-view` refuses. One key, two meanings, chosen by which block you are on
20+
— which is the per-block divergence the ruling declined by name when it
21+
rejected option A.
22+
23+
**Migration.** Write the array: `sort: [{ field: 'name', order: 'desc' }]`
24+
(`order` is optional and means `'asc'`). That is the only spelling
25+
`ObjectGridSchema.sort` has declared since #8221, the only one the registered
26+
`sort` input publishes, and the only one `@objectstack/spec` accepts — so
27+
type-checked metadata is already on it, and only untyped JSON or a stored
28+
`sys_metadata` row can still carry the string.
29+
30+
**What is deliberately unchanged.** The wire shape. The array arm still lowers
31+
to this block's own `"field order[, field order]"` join string, and the export
32+
path and the header-arrow reader `parseSchemaSort` still read the key exactly as
33+
before. Routing the whole key through the shared sink would send its
34+
`{ field: direction }` map where every grid today sends a string; that is a
35+
separate change with its own blast radius, and this ruling explicitly did not
36+
take it.

‎packages/plugin-grid/src/ObjectGrid.tsx‎

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import {
3636
RefreshIndicator,
3737
} from '@object-ui/components';
3838
import { usePullToRefresh } from '@object-ui/mobile';
39-
import { resolveConditionalFormatting, leadWithNameField, buildExpandFields, buildExportFileName, columnIdentity, collectPredicateFieldRefs, collectGroupingFieldRefs, listViewPredicates, isObjectInlineEditable, isProjectableField, isExpandableFieldType, isUnmaterializedFieldType, readObjectSortability, isPlatformSortableField, filterPlatformSortableSort, toFilterNode, ROW_HEIGHT_TO_DENSITY_MODE, resolveRecordSourceConfig, resolveRecordSourceObjectName } from '@object-ui/core';
39+
import { resolveConditionalFormatting, leadWithNameField, buildExpandFields, buildExportFileName, columnIdentity, collectPredicateFieldRefs, collectGroupingFieldRefs, listViewPredicates, isObjectInlineEditable, isProjectableField, isExpandableFieldType, isUnmaterializedFieldType, readObjectSortability, isPlatformSortableField, filterPlatformSortableSort, toFilterNode, convertSortToQueryParams, type QuerySortEntry, ROW_HEIGHT_TO_DENSITY_MODE, resolveRecordSourceConfig, resolveRecordSourceObjectName } from '@object-ui/core';
4040
import { usePermissions } from '@object-ui/permissions';
4141
import { ChevronRight, ChevronLeft, ChevronsLeft, ChevronsRight, Download, Rows2, Rows3, Rows4, AlignJustify, Type, Hash, Calendar, CheckSquare, User, Tag, Clock, Loader2 } from 'lucide-react';
4242
import { useRowColor } from './useRowColor';
@@ -59,13 +59,22 @@ import type { BulkActionDef } from '@object-ui/types';
5959
/**
6060
* A view's declared `sort` → the shape the table's header indicators read.
6161
*
62-
* `@objectstack/spec` allows `"name desc"`, `["name desc", …]` and
63-
* `[{ field, order }, …]`, and this grid's own fetch path already reads all
64-
* three. The headers have to agree with it: a view that arrives sorted by
62+
* `[{ field, order }, …]` is the ONE spelling `@objectstack/spec` still
63+
* declares (objectui#8221 retired the string clauses). The headers have to
64+
* agree with the fetch path on it: a view that arrives sorted by
6565
* `created_at desc` should show that arrow before anyone clicks anything —
6666
* otherwise the first click on that column produces `asc` while the list was
6767
* already `desc`, and the arrow tells the truth only from the second click on.
6868
*
69+
* ⚠️ This reader is WIDER than the fetch path as of objectui#8767, and the
70+
* sentence this docblock used to carry — that the fetch path reads all three
71+
* spellings — is no longer true. The fetch path now REFUSES a string and sends
72+
* no `$orderby` at all, while this reader still parses `"name desc"` and
73+
* `["name desc", …]`, so a grid authored with a retired spelling shows an
74+
* arrow for an ordering its query does not carry. Narrowing this reader moves
75+
* the wire shape and takes the export path with it — the route the #8767
76+
* ruling deliberately did not take. It is NOT fixed here.
77+
*
6978
* Exported for the test that pins it against the fetch path's own reading.
7079
*/
7180
export function parseSchemaSort(sort: unknown): TableSortItem[] {
@@ -1851,7 +1860,30 @@ export const ObjectGrid: React.FC<ObjectGridComponentProps> = ({
18511860
params.$orderby = headerSort.map((s) => ({ field: s.field, order: s.order }));
18521861
} else if (schemaSort) {
18531862
if (typeof schemaSort === 'string') {
1854-
params.$orderby = schemaSort;
1863+
// objectui#8767 — the legacy string `sort` clause is RETIRED
1864+
// (objectui#8221, decision batch #77) and is REFUSED here rather
1865+
// than lowered. This block owns a PRIVATE lowering, so #8758's
1866+
// narrowing of the shared sink never reached it: a bare
1867+
// `object-grid` went on honouring a spelling `object-view`
1868+
// already refuses — one key meaning two things depending on which
1869+
// block you are on, which is the per-block divergence the #8221
1870+
// ruling declined by name.
1871+
//
1872+
// The refusal is #8758's OWN, not a second one: calling the
1873+
// shared sink on this arm reports the retired spelling once per
1874+
// spelling and answers `undefined`, so the query carries no
1875+
// `$orderby`. Its return value is deliberately unused — the wire
1876+
// shape stays this block's, and the array arm below is untouched
1877+
// (with it the export path and `parseSchemaSort`). Routing the
1878+
// whole key through the sink is a different card: it would send
1879+
// the sink's `{field: direction}` map where every grid today
1880+
// sends a `"field order"` string.
1881+
//
1882+
// Read through `unknown`, exactly as the sink does: types are
1883+
// erased, so the array-only `ObjectGridSchema.sort` declaration
1884+
// cannot stop a string arriving from authored JSON, a stored
1885+
// `sys_metadata` row or an `as any` bag.
1886+
convertSortToQueryParams(schemaSort as unknown as QuerySortEntry[]);
18551887
} else if (Array.isArray(schemaSort)) {
18561888
params.$orderby = schemaSort
18571889
.map((s: any) => `${s.field} ${s.order}`)
@@ -4023,6 +4055,11 @@ export const ObjectGrid: React.FC<ObjectGridComponentProps> = ({
40234055
// arrow, and the first click on that column would ask for `asc` on a list
40244056
// that was already `desc`.
40254057
//
4058+
// ⚠️ One spelling now escapes that agreement: since objectui#8767 the fetch
4059+
// path REFUSES a string `sort` and sends no `$orderby`, while
4060+
// {@link parseSchemaSort} still parses one. Closing that gap narrows this
4061+
// reader and moves the wire shape with it — the route #8767 did not take.
4062+
//
40264063
// A plain expression, not a `useMemo`: this sits below the component's early
40274064
// returns, where a hook would be skipped on some renders and change the hook
40284065
// order. Parsing at most a handful of sort keys costs nothing worth a hook.
Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
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+
* `object-grid` REFUSES the retired string `sort` clause (objectui#8767).
9+
*
10+
* ## What was wrong
11+
*
12+
* The #8221 ruling (decision batch #77) retired the legacy OData-ish string
13+
* clause: one spelling, the array, everywhere. PR #8758 narrowed the shared
14+
* sink `convertSortToQueryParams` so a string that still arrives at runtime is
15+
* refused out loud and the query carries no `$orderby`.
16+
*
17+
* `ObjectGrid` never went through that sink. It reads `schema.sort` at its own
18+
* site and lowers it with private code, so after #8758 a bare `object-grid`
19+
* still forwarded a string verbatim to `$orderby` while the SAME key routed
20+
* through `object-view` was refused with a diagnostic — one key meaning two
21+
* things depending on which block you are on. That is precisely the shape the
22+
* ruling rejected when it declined option A by name:
23+
*
24+
* > per-block arms would make one key mean different things on different
25+
* > blocks and keep a spelling the spec already refuses on one of them.
26+
*
27+
* ## What these tests pin, and what they deliberately do NOT
28+
*
29+
* Route C, as ruled: the string arm is refused with #8758's own reporter, and
30+
* NOTHING else about this block moves. So the file has two halves, and the
31+
* second is what stops it passing by refusing everything:
32+
*
33+
* 1. a string `sort` reaches no `$orderby`, and says so once per spelling;
34+
* 2. the array arm still lowers to the very same `"field order"` join string
35+
* this block has always sent — byte for byte, single- and multi-key.
36+
*
37+
* The join string is the wire shape route B would change and route C keeps.
38+
* The header-arrow reader `parseSchemaSort` and the export path read the same
39+
* key and are untouched here; they belong to that other card.
40+
*/
41+
42+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
43+
import { render, waitFor } from '@testing-library/react';
44+
import React from 'react';
45+
import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react';
46+
import { resetRetiredSortSpellingReports } from '@object-ui/core';
47+
// Registers `object-grid` and its `view:grid` alias.
48+
import '../index';
49+
50+
function makeAdapter() {
51+
return {
52+
find: vi.fn().mockResolvedValue({
53+
data: [{ id: '1', name: 'Acme', status: 'active' }],
54+
total: 1,
55+
}),
56+
findOne: vi.fn(),
57+
create: vi.fn(),
58+
update: vi.fn(),
59+
delete: vi.fn(),
60+
getObjectSchema: vi.fn().mockResolvedValue({
61+
name: 'account',
62+
fields: { id: { type: 'text' }, name: { type: 'text' }, status: { type: 'text' } },
63+
}),
64+
};
65+
}
66+
67+
/**
68+
* Render the block and return the params of its first `find` call — the same
69+
* harness shape as the sibling `gridDefaultFiltersLowering.test.tsx`, so both
70+
* legs of this read site are observed through one lens.
71+
*/
72+
async function findParamsFor(schema: Record<string, unknown>) {
73+
const adapter = makeAdapter();
74+
render(
75+
<SchemaRendererProvider dataSource={adapter as any}>
76+
<SchemaRenderer schema={schema as any} />
77+
</SchemaRendererProvider>,
78+
);
79+
await waitFor(() => expect(adapter.find).toHaveBeenCalled());
80+
return (adapter.find.mock.calls[0] as [string, any])[1];
81+
}
82+
83+
const BASE = { type: 'object-grid', objectName: 'account', columns: [{ field: 'name' }] };
84+
85+
/**
86+
* The retired spelling, reached the only way it still can be. The declaration
87+
* is `SortConfig[]` since #8221, so a string arrives from authored JSON, a
88+
* stored `sys_metadata` row or an `as any` bag — never from a typed caller.
89+
*/
90+
const authoredAtRuntime = (sort: unknown) => ({ ...BASE, sort }) as Record<string, unknown>;
91+
92+
let errorSpy: ReturnType<typeof vi.spyOn>;
93+
94+
beforeEach(() => {
95+
// The reporter dedupes per spelling in MODULE state. Without this reset the
96+
// second test to assert the diagnostic would observe silence and pass for
97+
// the wrong reason.
98+
resetRetiredSortSpellingReports();
99+
errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
100+
});
101+
102+
afterEach(() => {
103+
errorSpy.mockRestore();
104+
});
105+
106+
describe('object-grid — the retired string `sort` clause is REFUSED at this block’s own read site (objectui#8767)', () => {
107+
it('carries NO `$orderby` for a string `sort`, and names the array form', async () => {
108+
const params = await findParamsFor(authoredAtRuntime('name desc'));
109+
110+
// The defect in one line: this used to be the string `'name desc'`, i.e.
111+
// the grid honoured at runtime what `object-view` refuses.
112+
expect(params.$orderby).toBeUndefined();
113+
expect(Object.prototype.hasOwnProperty.call(params, '$orderby')).toBe(false);
114+
115+
expect(errorSpy).toHaveBeenCalled();
116+
const message = String(errorSpy.mock.calls[0][0]);
117+
// #8758's OWN diagnostic, not a second one written here: it quotes the
118+
// offending spelling and prescribes the array form, because a refusal with
119+
// no prescription only moves the author's problem.
120+
expect(message).toContain('"name desc"');
121+
expect(message).toContain("[{ field: 'name', order: 'desc' }]");
122+
expect(message).toContain('objectui#8221');
123+
});
124+
125+
it('refuses a bare field string too — every retired spelling, not just the two-word one', async () => {
126+
const params = await findParamsFor(authoredAtRuntime('name'));
127+
expect(params.$orderby).toBeUndefined();
128+
expect(errorSpy).toHaveBeenCalled();
129+
});
130+
131+
it('reports once per spelling, not once per grid', async () => {
132+
await findParamsFor(authoredAtRuntime('name desc'));
133+
await findParamsFor(authoredAtRuntime('name desc'));
134+
// Two blocks inheriting the same bad view sort print one line between them
135+
// — the dedupe is the reporter's, and reusing it is what keeps this read
136+
// site from becoming a second, differently-behaved diagnostic.
137+
expect(errorSpy).toHaveBeenCalledTimes(1);
138+
});
139+
140+
it('does not fall through to the legacy `defaultSort` leg when it refuses', async () => {
141+
// A refusal that quietly handed the query to the next arm would be a
142+
// silent substitution — a different ordering than either the author asked
143+
// for or the refusal announced.
144+
const params = await findParamsFor({
145+
...authoredAtRuntime('name desc'),
146+
defaultSort: { field: 'status', order: 'asc' },
147+
});
148+
expect(params.$orderby).toBeUndefined();
149+
});
150+
});
151+
152+
describe('CONTROL — the array arm lowers UNCHANGED (route C keeps this block’s wire shape)', () => {
153+
it('still sends the single-key `"field order"` join string', async () => {
154+
const params = await findParamsFor({ ...BASE, sort: [{ field: 'name', order: 'desc' }] });
155+
// Byte-identical to what this block sent before #8767. Route B would send
156+
// the shared sink's `{ name: 'desc' }` map here; that is a different card,
157+
// and this line is what would catch it arriving by accident.
158+
expect(params.$orderby).toBe('name desc');
159+
expect(errorSpy).not.toHaveBeenCalled();
160+
});
161+
162+
it('still joins a multi-key array with `, `', async () => {
163+
const params = await findParamsFor({
164+
...BASE,
165+
sort: [
166+
{ field: 'status', order: 'asc' },
167+
{ field: 'name', order: 'desc' },
168+
],
169+
});
170+
expect(params.$orderby).toBe('status asc, name desc');
171+
expect(errorSpy).not.toHaveBeenCalled();
172+
});
173+
});

‎packages/plugin-grid/src/__tests__/serverSorting.test.tsx‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,14 @@ describe('ObjectGrid — column-header sorting is server-side (#3106)', () => {
138138

139139
it('replaces the view\'s declared sort rather than stacking on it', async () => {
140140
const ds = makeDataSource();
141-
const { container } = renderGrid(ds, { sort: 'name desc' });
141+
const { container } = renderGrid(ds, { sort: [{ field: 'name', order: 'desc' }] });
142142
await waitFor(() => expect(screen.getByText('Row 0')).toBeInTheDocument());
143-
// The declared sort goes out as the string form it was authored in.
143+
// Authored in the ONE declared spelling. This case used to author the
144+
// retired string clause and assert that it went out verbatim; objectui#8767
145+
// made this block REFUSE a string (objectui#8221), so a string here would
146+
// reach no `$orderby` at all and the click would have nothing to replace.
147+
// The array arm still lowers to this block's own `"field order"` string —
148+
// unchanged, which is the point of route C.
144149
expect(lastFindParams(ds).$orderby).toBe('name desc');
145150

146151
fireEvent.click(headerCell(container, 'Status'));
@@ -153,6 +158,14 @@ describe('ObjectGrid — column-header sorting is server-side (#3106)', () => {
153158
it('shows the view\'s declared sort before anyone clicks', async () => {
154159
// Otherwise the first click on that column asks for `asc` on a list that is
155160
// already `desc`, and the arrow only tells the truth from click two on.
161+
//
162+
// ⚠️ This case deliberately still authors the RETIRED string spelling, and
163+
// is left that way: it is the one place in the suite that shows the header
164+
// reader (`parseSchemaSort`) is now WIDER than the fetch path — since
165+
// objectui#8767 the query carries no `$orderby` for this schema while the
166+
// arrow below still appears. Narrowing the reader moves the wire shape and
167+
// the export path with it; that is the route-B card #8767 did not take, so
168+
// the divergence is recorded here rather than papered over.
156169
const ds = makeDataSource();
157170
const { container } = renderGrid(ds, { sort: 'status desc' });
158171
await waitFor(() => expect(screen.getByText('Row 0')).toBeInTheDocument());

0 commit comments

Comments
 (0)