diff --git a/.changeset/8767-object-grid-refuses-string-sort.md b/.changeset/8767-object-grid-refuses-string-sort.md index 5ab4c24690..775a0ed8bf 100644 --- a/.changeset/8767-object-grid-refuses-string-sort.md +++ b/.changeset/8767-object-grid-refuses-string-sort.md @@ -55,8 +55,12 @@ type change. **What is deliberately unchanged.** The wire shape. The array arm still lowers to this block's own `"field order[, field order]"` join string, and the export -path and the header-arrow reader `parseSchemaSort` still read the key exactly as -before. Routing the whole key through the shared sink would send its +path still reads the key exactly as before. The header-arrow reader +`parseSchemaSort` was left untouched here as well — which is precisely what left +it reading this key more widely than the refusal above, drawing an arrow for an +ordering the query no longer carried. objectui#8961 has since narrowed it to the +declared array, so a retired string lights no arrow either and the two readers +agree again. Routing the whole key through the shared sink would send its `{ field: direction }` map where every grid today sends a string; that is a separate change with its own blast radius, and this ruling explicitly did not take it. diff --git a/.changeset/8961-parse-schema-sort-narrow.md b/.changeset/8961-parse-schema-sort-narrow.md new file mode 100644 index 0000000000..78fba02367 --- /dev/null +++ b/.changeset/8961-parse-schema-sort-narrow.md @@ -0,0 +1,43 @@ +--- +'@object-ui/plugin-grid': patch +--- + +`object-grid`'s header arrows read the ONE declared `sort` spelling — a retired +string no longer paints a pre-click arrow (objectui#8961, maintainer ruling +2026-09-15, letter A). + +**What a user saw.** A view authored `sort: "name desc"` lit a descending arrow +on the `name` header *before anyone clicked anything*, while the query that +fetched those rows carried no ordering at all. The arrow stated something about +the list that was not true of the rows beside it, and the first click on that +column then asked for `asc` on a list that was in no declared order. The only +signal was a `console.error` no end user reads. + +**Why the two halves disagreed.** objectui#8221 retired the legacy string +clause — one spelling, the array, everywhere — and objectui#8767 made this +block's fetch path REFUSE a string `sort` and send no `$orderby`. That ruling +deliberately left the other reader of the same key alone: `parseSchemaSort`, +which feeds the header indicators, went on parsing `"name desc"` and +`["name desc", …]`. One key, two readers, opposite answers. + +**What changes.** `parseSchemaSort` admits only `[{ field, order }, …]` — the +spelling `@objectstack/spec` declares for `ObjectGridPropsSchema.sort` and the +only one the fetch path still lowers. A retired string yields nothing, so it +lights no arrow, matching the query it produces. The refusal is per entry: a +mixed array still shows the arrows for the keys spelled in the declared form. + +**No second diagnostic.** The author is already told once per spelling by the +shared reporter at the fetch path, which quotes the offending value and +prescribes the array form. Narrowing the header reader adds no new message. + +**Migration.** Write the array: `sort: [{ field: 'name', order: 'desc' }]` — +the same edit the fetch path has required since objectui#8767. A declared entry +with no `order` still reads ascending; that behaviour is unchanged. + +**What is deliberately unchanged.** The wire shape. This block still lowers its +array arm to its own `"field order"` join string; routing the key through the +shared sink's `{field: direction}` map is route B on objectui#8767, which stays +declined until the protocol declares that shape (`$orderby` is declared +`string | string[]`). The server-side export path reads `schema.sort` itself +rather than through `parseSchemaSort`, so it was already array-only and does +not move. diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index b752a55980..faa5a1b86c 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -78,27 +78,6 @@ import { BulkActionDialog } from './components/BulkActionDialog'; import type { BulkResult } from './hooks/useBulkExecutor'; import type { BulkActionDef } from '@object-ui/types'; -/** - * A view's declared `sort` → the shape the table's header indicators read. - * - * `[{ field, order }, …]` is the ONE spelling `@objectstack/spec` still - * declares (objectui#8221 retired the string clauses). The headers have to - * agree with the fetch path on it: a view that arrives sorted by - * `created_at desc` should show that arrow before anyone clicks anything — - * otherwise the first click on that column produces `asc` while the list was - * already `desc`, and the arrow tells the truth only from the second click on. - * - * ⚠️ This reader is WIDER than the fetch path as of objectui#8767, and the - * sentence this docblock used to carry — that the fetch path reads all three - * spellings — is no longer true. The fetch path now REFUSES a string and sends - * no `$orderby` at all, while this reader still parses `"name desc"` and - * `["name desc", …]`, so a grid authored with a retired spelling shows an - * arrow for an ordering its query does not carry. Narrowing this reader moves - * the wire shape and takes the export path with it — the route the #8767 - * ruling deliberately did not take. It is NOT fixed here. - * - * Exported for the test that pins it against the fetch path's own reading. - */ /** * A declared `sort` → the `"field order"` join string THIS block sends as * `$orderby` (objectui#8973). @@ -131,14 +110,44 @@ function toOrderByClause(sort: QuerySortEntry[] | undefined | null): string | un return ordered.map((s) => `${s.field} ${s.order}`).join(', '); } +/** + * A view's declared `sort` → the shape the table's header indicators read. + * + * `[{ field, order }, …]` is the ONE spelling `@objectstack/spec` still + * declares for `ObjectGridPropsSchema.sort` (objectui#8221 retired the string + * clauses), and since objectui#8961 it is the only spelling this reader + * admits. The headers agree with the fetch path on it: a view that arrives + * sorted by `created_at desc` shows that arrow before anyone clicks anything — + * otherwise the first click on that column produces `asc` while the list was + * already `desc`, and the arrow tells the truth only from the second click on. + * + * ⭐ A retired string spelling (`"name desc"`, `["name desc", …]`) yields + * NOTHING here, so it lights no arrow. That is the agreement, not an omission: + * the fetch path REFUSES the same spelling and sends no `$orderby` + * (objectui#8767). Between #8767 and objectui#8961 this reader was WIDER than + * that path — it parsed the string and drew a confident arrow for an ordering + * the query did not carry, a UI element stating something untrue about the rows + * beside it, with nothing but a console line to say so. + * + * ⛔ Do not re-widen it for a stored `sys_metadata` row still carrying the old + * spelling. The author is already told, once per spelling, by PR #8758's own + * diagnostic at the fetch path — it quotes the offending value and prescribes + * the array form. A second reading here would restore exactly the arrow the + * wire cannot honour. + * + * The wire shape is NOT what moved: this block still sends its own + * `"field order"` join string (see {@link toOrderByClause}), the shared sink's + * `{field: direction}` map stays declined, and the server-side export path + * reads `schema.sort` itself rather than through this function, so it was + * already array-only and is untouched. + * + * Exported for the test that pins it against the fetch path's own reading. + */ export function parseSchemaSort(sort: unknown): TableSortItem[] { - const entries = typeof sort === 'string' ? [sort] : Array.isArray(sort) ? sort : []; + const entries = Array.isArray(sort) ? sort : []; const items: TableSortItem[] = []; for (const entry of entries) { - if (typeof entry === 'string') { - const [field, order] = entry.trim().split(/\s+/); - if (field) items.push({ field, order: order?.toLowerCase() === 'desc' ? 'desc' : 'asc' }); - } else if (entry && typeof entry === 'object' && typeof (entry as any).field === 'string') { + if (entry && typeof entry === 'object' && typeof (entry as any).field === 'string') { const { field, order } = entry as { field: string; order?: string }; items.push({ field, order: String(order).toLowerCase() === 'desc' ? 'desc' : 'asc' }); } @@ -2008,10 +2017,13 @@ export const ObjectGrid: React.FC = ({ // spelling and answers `undefined`, so the query carries no // `$orderby`. Its return value is deliberately unused — the wire // shape stays this block's, and the array arm below is untouched - // (with it the export path and `parseSchemaSort`). Routing the - // whole key through the sink is a different card: it would send - // the sink's `{field: direction}` map where every grid today - // sends a `"field order"` string. + // (with it the export path). The header-arrow reader + // `parseSchemaSort` was left parsing the string HERE, and read + // this same key more widely than this refusal until objectui#8961 + // narrowed it to the declared array; the two now agree. Routing + // the whole key through the sink is still a different card: it + // would send the sink's `{field: direction}` map where every grid + // today sends a `"field order"` string. // // Read through `unknown`, exactly as the sink does: types are // erased, so the array-only `ObjectGridSchema.sort` declaration @@ -4263,10 +4275,17 @@ export const ObjectGrid: React.FC = ({ // arrow, and the first click on that column would ask for `asc` on a list // that was already `desc`. // - // ⚠️ One spelling now escapes that agreement: since objectui#8767 the fetch - // path REFUSES a string `sort` and sends no `$orderby`, while - // {@link parseSchemaSort} still parses one. Closing that gap narrows this - // reader and moves the wire shape with it — the route #8767 did not take. + // ⭐ That agreement now covers the SPELLING too (objectui#8961). One used to + // escape it: since objectui#8767 the fetch path REFUSES a string `sort` and + // sends no `$orderby`, while {@link parseSchemaSort} went on parsing one, so + // a grid authored `sort: 'name desc'` painted a descending arrow over rows + // the server returned in no declared order. That reader now admits only the + // declared `[{ field, order }]` array — the one spelling the fetch path + // still lowers — so the arrow on screen and the `$orderby` on the wire read + // the same key the same way, and a retired spelling lights nothing on either + // side. What did NOT move is the wire shape: the array arm still lowers to + // this block's own `"field order"` join string, and the shared sink's + // `{field: direction}` map stays declined (see {@link toOrderByClause}). // // A plain expression, not a `useMemo`: this sits below the component's early // returns, where a hook would be skipped on some renders and change the hook diff --git a/packages/plugin-grid/src/__tests__/gridArrayArmOrderby-8973.test.tsx b/packages/plugin-grid/src/__tests__/gridArrayArmOrderby-8973.test.tsx index 8e12917851..1aad5833d5 100644 --- a/packages/plugin-grid/src/__tests__/gridArrayArmOrderby-8973.test.tsx +++ b/packages/plugin-grid/src/__tests__/gridArrayArmOrderby-8973.test.tsx @@ -46,10 +46,20 @@ * * ## Deliberately untouched * - * The header-arrow reader `parseSchemaSort` (objectui#8961, `pm:blocked`) and - * the export path's `{field, direction}` projection. The `headerSort` arm also - * keeps sending `SortNode[]` objects rather than a join string — a documented, - * deliberate difference, not a defect. + * The export path's `{field, direction}` projection, and the header-arrow + * reader `parseSchemaSort` — untouched BY THIS CARD. ⛔ Do not read the latter + * as a standing description of that reader: objectui#8961 has since narrowed it + * to the declared `[{ field, order }]` array (ruled letter A, director batch + * #135 item 5), so a retired string spelling now lights no arrow either and the + * two readers of this key agree. Nothing below reads the header indicators, so + * this file's pins are unaffected. The `headerSort` arm also keeps sending + * `SortNode[]` objects rather than a join string — a documented, deliberate + * difference, not a defect. + * + * ⚠️ The sentence this replaces cited objectui#8961 by its LABEL + * (`pm:blocked`), which had moved before anyone read the line again. A card's + * label is state nothing in this file re-derives; its RULING does not move, so + * that is what is named here. * * ⭐ The CONTROL rows are what make the rest a measurement rather than a block * that mangles everything: if the fix had broken lowering outright, the diff --git a/packages/plugin-grid/src/__tests__/serverSorting.test.tsx b/packages/plugin-grid/src/__tests__/serverSorting.test.tsx index 79021cbff5..0a6a4121ee 100644 --- a/packages/plugin-grid/src/__tests__/serverSorting.test.tsx +++ b/packages/plugin-grid/src/__tests__/serverSorting.test.tsx @@ -178,20 +178,21 @@ describe('ObjectGrid — column-header sorting is server-side (#3106)', () => { }); }); - it('DIVERGENCE, pinned not tolerated (objectui#8961): a retired STRING sort still draws the arrow while the wire carries NO ordering', async () => { - // objectui#8767 made the fetch path REFUSE a string `sort`. The header - // indicators are fed by `parseSchemaSort`, a second private reader that - // the ruling deliberately left alone — and it still parses a string. So - // the two readers of one key now disagree, and this asserts BOTH halves of - // that disagreement. + it('a retired STRING sort lights NO arrow — the header reads the key exactly as the fetch path does (objectui#8961)', async () => { + // Both readers of one key, in agreement. objectui#8767 made the fetch path + // REFUSE a string `sort`; the header indicators are fed by + // `parseSchemaSort`, a second private reader that went on parsing one, so + // this very schema drew a confident `status desc` arrow over rows the + // server had returned in NO declared order — and the first click on that + // column then asked for `asc` on a list that was in no order at all. // - // Asserting only the arrow (which is what this file did before) leaves a - // green test that reads as "string sort ⇒ arrow, as intended". A green - // test does not make a divergence visible; it certifies it. Naming both - // halves is what makes the state a recorded defect instead of an expected - // one, and makes THIS the case that has to change when objectui#8961 - // closes the gap — narrowing the reader moves the wire shape and takes the - // export path with it, the route objectui#8767 did not take. + // This case used to pin that divergence rather than tolerate it: it + // asserted the arrow WAS drawn beside the empty query, so the state read + // as a recorded defect instead of an expected one. The ruling on + // objectui#8961 (director batch #135 item 5, letter A) closed it by + // narrowing the reader to the declared array, so the ARROW half flips + // here. The wire half is unchanged and stays asserted: "no arrow" is the + // right answer only while the query really does carry no ordering. resetRetiredSortSpellingReports(); const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); try { @@ -199,10 +200,16 @@ describe('ObjectGrid — column-header sorting is server-side (#3106)', () => { const { container } = renderGrid(ds, { sort: 'status desc' }); await waitFor(() => expect(screen.getByText('Row 0')).toBeInTheDocument()); - // Half one — the header still tells the user this list is `status desc`. - expect( - headerCell(container, 'Status').querySelector('[class*="chevron-down"]'), - ).not.toBeNull(); + // Half one — the header states nothing about an ordering this list does + // not have. NEITHER direction, not merely "not descending". + const status = headerCell(container, 'Status'); + expect(status.querySelector('[class*="chevron-down"]')).toBeNull(); + expect(status.querySelector('[class*="chevron-up"]')).toBeNull(); + // CONTROL, non-vacuity — the column is still sortable and still renders + // its header chrome, so the two nulls above mean "no ACTIVE sort" rather + // than "no icons here at all", which is how they would also pass if the + // harness or the column had silently stopped offering sorting. + expect(status.querySelector('[class*="chevrons-up-down"]')).not.toBeNull(); // Half two — and the query it was fetched with carries no ordering at // all. `hasOwnProperty`, not `toBeUndefined`: the key is absent, which is @@ -210,7 +217,9 @@ describe('ObjectGrid — column-header sorting is server-side (#3106)', () => { const params = lastFindParams(ds); expect(Object.prototype.hasOwnProperty.call(params, '$orderby')).toBe(false); - // …and the disagreement is announced once, by PR #8758's own reporter. + // …and the author is told once, by PR #8758's own reporter on the fetch + // path. STILL once: narrowing the header reader deliberately did not add + // a second diagnostic for the same spelling — one refusal, one voice. const retired = errorSpy.mock.calls.filter((c) => String(c[0]).includes('objectui#8221'), ); @@ -237,34 +246,58 @@ describe('ObjectGrid — column-header sorting is server-side (#3106)', () => { }); /** - * ⚠️ This block pins the header reader's OWN contract, which since - * objectui#8767 is WIDER than the fetch path's — hence the renamed title. The - * cases below still admit the retired string spellings because this reader - * still admits them; the fetch path refuses them (see the divergence pin - * above). Re-judging these inputs belongs to objectui#8961, which narrows this - * reader and moves the wire shape with it. Until then they are read as "what - * `parseSchemaSort` accepts", never as "what the grid queries with". + * ⚠️ This block pins the header reader's OWN contract. Since objectui#8961 + * that contract is exactly as wide as the fetch path's: the declared + * `[{ field, order }]` array and nothing else (objectui#8221 retired the string + * clauses, objectui#8767 made the fetch path refuse them). The cases below used + * to admit the retired spellings because this reader still did, and re-judging + * them was named as this card's work; that is what has happened. + * + * These refusals are the unit-level half of the rendered pin above — the `[]` + * here is what the "no arrow" there is made of. + * + * Non-vacuity: every refusal is paired with a CONTROL in the same shape — the + * declared array must still parse, single- and multi-key. A reader that had + * simply stopped returning anything would fail those controls, so `[]` is a + * verdict here and not silence. */ -describe('parseSchemaSort — the header reader\'s own contract, wider than the fetch path (objectui#8961)', () => { - it('reads the bare-string form', () => { - expect(parseSchemaSort('name desc')).toEqual([{ field: 'name', order: 'desc' }]); +describe('parseSchemaSort — the header reader reads the ONE declared spelling (objectui#8961)', () => { + it('REFUSES the bare-string form — no arrow, matching the `$orderby` the fetch path does not send', () => { + expect(parseSchemaSort('name desc')).toEqual([]); + // The one-word spelling too, not just the two-word one. + expect(parseSchemaSort('name')).toEqual([]); }); - it('defaults an omitted direction to ascending', () => { - expect(parseSchemaSort('name')).toEqual([{ field: 'name', order: 'asc' }]); - }); - - it('reads the array-of-strings form', () => { - expect(parseSchemaSort(['status asc', 'name desc'])).toEqual([ - { field: 'status', order: 'asc' }, + it('REFUSES string entries INSIDE an array, entry by entry', () => { + expect(parseSchemaSort(['status asc', 'name desc'])).toEqual([]); + // Per entry, not a whole-value veto: one retired entry cannot blank the + // keys the author did spell in the declared form. + expect(parseSchemaSort(['status asc', { field: 'name', order: 'desc' }])).toEqual([ { field: 'name', order: 'desc' }, ]); }); - it('reads the SortNode[] form', () => { + it('CONTROL — reads the declared `SortConfig[]` form, single- and multi-key', () => { expect(parseSchemaSort([{ field: 'name', order: 'desc' }])).toEqual([ { field: 'name', order: 'desc' }, ]); + expect( + parseSchemaSort([ + { field: 'status', order: 'asc' }, + { field: 'name', order: 'desc' }, + ]), + ).toEqual([ + { field: 'status', order: 'asc' }, + { field: 'name', order: 'desc' }, + ]); + }); + + it('CONTROL — a declared entry with no `order` still reads ascending', () => { + // Untouched by objectui#8961, and named here so the narrowing is not read + // as a second, stricter judgement of the ENTRY: what moved is which + // SPELLING of the key is admitted, not how a declared entry is read. This + // is also the shape `defaultSort` arrives in, wrapped by the read site. + expect(parseSchemaSort([{ field: 'name' }])).toEqual([{ field: 'name', order: 'asc' }]); }); it('yields nothing for an absent or unreadable sort', () => {