From 76a9ead402cc173ce7c9bea00eaf6daed59c2bb9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 10:53:43 +0000 Subject: [PATCH 1/2] fix(plugin-grid): refuse the retired string `sort` clause at object-grid's own read site (objectui#8767) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_01Jmxdo7bmeqCQHLSfmLVX9w --- .../8767-object-grid-refuses-string-sort.md | 36 ++++ packages/plugin-grid/src/ObjectGrid.tsx | 47 ++++- .../gridRetiredStringSort-8767.test.tsx | 173 ++++++++++++++++++ .../src/__tests__/serverSorting.test.tsx | 17 +- 4 files changed, 266 insertions(+), 7 deletions(-) create mode 100644 .changeset/8767-object-grid-refuses-string-sort.md create mode 100644 packages/plugin-grid/src/__tests__/gridRetiredStringSort-8767.test.tsx diff --git a/.changeset/8767-object-grid-refuses-string-sort.md b/.changeset/8767-object-grid-refuses-string-sort.md new file mode 100644 index 0000000000..545e8ea597 --- /dev/null +++ b/.changeset/8767-object-grid-refuses-string-sort.md @@ -0,0 +1,36 @@ +--- +'@object-ui/plugin-grid': minor +--- + +`object-grid` REFUSES the retired string `sort` clause at its own read site +(objectui#8767, maintainer ruling 2026-09-10 — route C). + +**Breaking, deliberately.** A `sort: "name desc"` on an `object-grid` node no +longer reaches `$orderby`. It is reported once per spelling with the diagnostic +objectui#8221 / PR #8758 already ship — the message names the offending value +and prescribes the array form — and the query goes out carrying no ordering at +all, exactly as the same key already behaves on `object-view`. + +**Why it was still lowering.** The #8221 ruling retired the legacy string +clause: one spelling, the array, everywhere. PR #8758 narrowed the shared sink +`convertSortToQueryParams` and every declaration that published a string arm, +but `ObjectGrid` never used that sink — it reads `schema.sort` and lowers it +with private code, so a bare grid went on honouring at runtime a spelling +`object-view` refuses. One key, two meanings, chosen by which block you are on +— which is the per-block divergence the ruling declined by name when it +rejected option A. + +**Migration.** Write the array: `sort: [{ field: 'name', order: 'desc' }]` +(`order` is optional and means `'asc'`). That is the only spelling +`ObjectGridSchema.sort` has declared since #8221, the only one the registered +`sort` input publishes, and the only one `@objectstack/spec` accepts — so +type-checked metadata is already on it, and only untyped JSON or a stored +`sys_metadata` row can still carry the string. + +**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 +`{ 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/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 95d8fbbab5..064b293dcd 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -36,7 +36,7 @@ import { RefreshIndicator, } from '@object-ui/components'; import { usePullToRefresh } from '@object-ui/mobile'; -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'; +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'; import { usePermissions } from '@object-ui/permissions'; import { ChevronRight, ChevronLeft, ChevronsLeft, ChevronsRight, Download, Rows2, Rows3, Rows4, AlignJustify, Type, Hash, Calendar, CheckSquare, User, Tag, Clock, Loader2 } from 'lucide-react'; import { useRowColor } from './useRowColor'; @@ -59,13 +59,22 @@ import type { BulkActionDef } from '@object-ui/types'; /** * A view's declared `sort` → the shape the table's header indicators read. * - * `@objectstack/spec` allows `"name desc"`, `["name desc", …]` and - * `[{ field, order }, …]`, and this grid's own fetch path already reads all - * three. The headers have to agree with it: a view that arrives sorted by + * `[{ 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. */ export function parseSchemaSort(sort: unknown): TableSortItem[] { @@ -1851,7 +1860,30 @@ export const ObjectGrid: React.FC = ({ params.$orderby = headerSort.map((s) => ({ field: s.field, order: s.order })); } else if (schemaSort) { if (typeof schemaSort === 'string') { - params.$orderby = schemaSort; + // objectui#8767 — the legacy string `sort` clause is RETIRED + // (objectui#8221, decision batch #77) and is REFUSED here rather + // than lowered. This block owns a PRIVATE lowering, so #8758's + // narrowing of the shared sink never reached it: a bare + // `object-grid` went on honouring a spelling `object-view` + // already refuses — one key meaning two things depending on which + // block you are on, which is the per-block divergence the #8221 + // ruling declined by name. + // + // The refusal is #8758's OWN, not a second one: calling the + // shared sink on this arm reports the retired spelling once per + // 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. + // + // Read through `unknown`, exactly as the sink does: types are + // erased, so the array-only `ObjectGridSchema.sort` declaration + // cannot stop a string arriving from authored JSON, a stored + // `sys_metadata` row or an `as any` bag. + convertSortToQueryParams(schemaSort as unknown as QuerySortEntry[]); } else if (Array.isArray(schemaSort)) { params.$orderby = schemaSort .map((s: any) => `${s.field} ${s.order}`) @@ -4023,6 +4055,11 @@ 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. + // // 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 // order. Parsing at most a handful of sort keys costs nothing worth a hook. diff --git a/packages/plugin-grid/src/__tests__/gridRetiredStringSort-8767.test.tsx b/packages/plugin-grid/src/__tests__/gridRetiredStringSort-8767.test.tsx new file mode 100644 index 0000000000..555c67b3b9 --- /dev/null +++ b/packages/plugin-grid/src/__tests__/gridRetiredStringSort-8767.test.tsx @@ -0,0 +1,173 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * `object-grid` REFUSES the retired string `sort` clause (objectui#8767). + * + * ## What was wrong + * + * The #8221 ruling (decision batch #77) retired the legacy OData-ish string + * clause: one spelling, the array, everywhere. PR #8758 narrowed the shared + * sink `convertSortToQueryParams` so a string that still arrives at runtime is + * refused out loud and the query carries no `$orderby`. + * + * `ObjectGrid` never went through that sink. It reads `schema.sort` at its own + * site and lowers it with private code, so after #8758 a bare `object-grid` + * still forwarded a string verbatim to `$orderby` while the SAME key routed + * through `object-view` was refused with a diagnostic — one key meaning two + * things depending on which block you are on. That is precisely the shape the + * ruling rejected when it declined option A by name: + * + * > per-block arms would make one key mean different things on different + * > blocks and keep a spelling the spec already refuses on one of them. + * + * ## What these tests pin, and what they deliberately do NOT + * + * Route C, as ruled: the string arm is refused with #8758's own reporter, and + * NOTHING else about this block moves. So the file has two halves, and the + * second is what stops it passing by refusing everything: + * + * 1. a string `sort` reaches no `$orderby`, and says so once per spelling; + * 2. the array arm still lowers to the very same `"field order"` join string + * this block has always sent — byte for byte, single- and multi-key. + * + * The join string is the wire shape route B would change and route C keeps. + * The header-arrow reader `parseSchemaSort` and the export path read the same + * key and are untouched here; they belong to that other card. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, waitFor } from '@testing-library/react'; +import React from 'react'; +import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react'; +import { resetRetiredSortSpellingReports } from '@object-ui/core'; +// Registers `object-grid` and its `view:grid` alias. +import '../index'; + +function makeAdapter() { + return { + find: vi.fn().mockResolvedValue({ + data: [{ id: '1', name: 'Acme', status: 'active' }], + total: 1, + }), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ + name: 'account', + fields: { id: { type: 'text' }, name: { type: 'text' }, status: { type: 'text' } }, + }), + }; +} + +/** + * Render the block and return the params of its first `find` call — the same + * harness shape as the sibling `gridDefaultFiltersLowering.test.tsx`, so both + * legs of this read site are observed through one lens. + */ +async function findParamsFor(schema: Record) { + const adapter = makeAdapter(); + render( + + + , + ); + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + return (adapter.find.mock.calls[0] as [string, any])[1]; +} + +const BASE = { type: 'object-grid', objectName: 'account', columns: [{ field: 'name' }] }; + +/** + * The retired spelling, reached the only way it still can be. The declaration + * is `SortConfig[]` since #8221, so a string arrives from authored JSON, a + * stored `sys_metadata` row or an `as any` bag — never from a typed caller. + */ +const authoredAtRuntime = (sort: unknown) => ({ ...BASE, sort }) as Record; + +let errorSpy: ReturnType; + +beforeEach(() => { + // The reporter dedupes per spelling in MODULE state. Without this reset the + // second test to assert the diagnostic would observe silence and pass for + // the wrong reason. + resetRetiredSortSpellingReports(); + errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); +}); + +afterEach(() => { + errorSpy.mockRestore(); +}); + +describe('object-grid — the retired string `sort` clause is REFUSED at this block’s own read site (objectui#8767)', () => { + it('carries NO `$orderby` for a string `sort`, and names the array form', async () => { + const params = await findParamsFor(authoredAtRuntime('name desc')); + + // The defect in one line: this used to be the string `'name desc'`, i.e. + // the grid honoured at runtime what `object-view` refuses. + expect(params.$orderby).toBeUndefined(); + expect(Object.prototype.hasOwnProperty.call(params, '$orderby')).toBe(false); + + expect(errorSpy).toHaveBeenCalled(); + const message = String(errorSpy.mock.calls[0][0]); + // #8758's OWN diagnostic, not a second one written here: it quotes the + // offending spelling and prescribes the array form, because a refusal with + // no prescription only moves the author's problem. + expect(message).toContain('"name desc"'); + expect(message).toContain("[{ field: 'name', order: 'desc' }]"); + expect(message).toContain('objectui#8221'); + }); + + it('refuses a bare field string too — every retired spelling, not just the two-word one', async () => { + const params = await findParamsFor(authoredAtRuntime('name')); + expect(params.$orderby).toBeUndefined(); + expect(errorSpy).toHaveBeenCalled(); + }); + + it('reports once per spelling, not once per grid', async () => { + await findParamsFor(authoredAtRuntime('name desc')); + await findParamsFor(authoredAtRuntime('name desc')); + // Two blocks inheriting the same bad view sort print one line between them + // — the dedupe is the reporter's, and reusing it is what keeps this read + // site from becoming a second, differently-behaved diagnostic. + expect(errorSpy).toHaveBeenCalledTimes(1); + }); + + it('does not fall through to the legacy `defaultSort` leg when it refuses', async () => { + // A refusal that quietly handed the query to the next arm would be a + // silent substitution — a different ordering than either the author asked + // for or the refusal announced. + const params = await findParamsFor({ + ...authoredAtRuntime('name desc'), + defaultSort: { field: 'status', order: 'asc' }, + }); + expect(params.$orderby).toBeUndefined(); + }); +}); + +describe('CONTROL — the array arm lowers UNCHANGED (route C keeps this block’s wire shape)', () => { + it('still sends the single-key `"field order"` join string', async () => { + const params = await findParamsFor({ ...BASE, sort: [{ field: 'name', order: 'desc' }] }); + // Byte-identical to what this block sent before #8767. Route B would send + // the shared sink's `{ name: 'desc' }` map here; that is a different card, + // and this line is what would catch it arriving by accident. + expect(params.$orderby).toBe('name desc'); + expect(errorSpy).not.toHaveBeenCalled(); + }); + + it('still joins a multi-key array with `, `', async () => { + const params = await findParamsFor({ + ...BASE, + sort: [ + { field: 'status', order: 'asc' }, + { field: 'name', order: 'desc' }, + ], + }); + expect(params.$orderby).toBe('status asc, name desc'); + expect(errorSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/plugin-grid/src/__tests__/serverSorting.test.tsx b/packages/plugin-grid/src/__tests__/serverSorting.test.tsx index 468d09856e..336d251e7d 100644 --- a/packages/plugin-grid/src/__tests__/serverSorting.test.tsx +++ b/packages/plugin-grid/src/__tests__/serverSorting.test.tsx @@ -138,9 +138,14 @@ describe('ObjectGrid — column-header sorting is server-side (#3106)', () => { it('replaces the view\'s declared sort rather than stacking on it', async () => { const ds = makeDataSource(); - const { container } = renderGrid(ds, { sort: 'name desc' }); + const { container } = renderGrid(ds, { sort: [{ field: 'name', order: 'desc' }] }); await waitFor(() => expect(screen.getByText('Row 0')).toBeInTheDocument()); - // The declared sort goes out as the string form it was authored in. + // Authored in the ONE declared spelling. This case used to author the + // retired string clause and assert that it went out verbatim; objectui#8767 + // made this block REFUSE a string (objectui#8221), so a string here would + // reach no `$orderby` at all and the click would have nothing to replace. + // The array arm still lowers to this block's own `"field order"` string — + // unchanged, which is the point of route C. expect(lastFindParams(ds).$orderby).toBe('name desc'); fireEvent.click(headerCell(container, 'Status')); @@ -153,6 +158,14 @@ describe('ObjectGrid — column-header sorting is server-side (#3106)', () => { it('shows the view\'s declared sort before anyone clicks', async () => { // Otherwise the first click on that column asks for `asc` on a list that is // already `desc`, and the arrow only tells the truth from click two on. + // + // ⚠️ This case deliberately still authors the RETIRED string spelling, and + // is left that way: it is the one place in the suite that shows the header + // reader (`parseSchemaSort`) is now WIDER than the fetch path — since + // objectui#8767 the query carries no `$orderby` for this schema while the + // arrow below still appears. Narrowing the reader moves the wire shape and + // the export path with it; that is the route-B card #8767 did not take, so + // the divergence is recorded here rather than papered over. const ds = makeDataSource(); const { container } = renderGrid(ds, { sort: 'status desc' }); await waitFor(() => expect(screen.getByText('Row 0')).toBeInTheDocument()); From 52d28eb4b4551fffe3ffc9a46fde2d532fc99785 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 12:12:45 +0000 Subject: [PATCH 2/2] fix(plugin-grid): repair two false contract claims in the changeset, and pin the arrow-vs-wire divergence instead of freezing it green (objectui#8767) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Contract review VERDICT: REWORK, two bounded items. The refusal mechanism, the control pins, the `minor` grading and objectui#8961's parking are judged correct and are untouched — `ObjectGrid.tsx` is byte-identical to the reviewed head. R1 — the changeset body carried two sentences that are false about the contract: - "the only one `@objectstack/spec` accepts". Measured against the installed `@objectstack/spec@17.4.0`: `ui.ObjectGridPropsSchema.sort` is `z.unknown().optional()`, so `safeParse` PARSES `'name desc'`, `'name'`, `42`, `['name desc']` and `{ name: 'desc' }` alike, while an undeclared `bogusProp` is REFUSED with `unrecognized_keys` — the control that shows those readings are real. The protocol's validator does not refuse the string. What the protocol DECLARES is the array, in three places: the key's own `describe` ("Initial sort (array of { field, order })"), the sibling `ElementRecordPickerPropsSchema.sort` typed as `z.array(SortItemSchema)` (which refuses a string outright), and the `defaultSort` retirement text ("wrap the value in an array"). The sentence is now a claim about what the protocol declares, which is true, rather than about what it accepts. - "(`order` is optional and means `'asc'`)". `SortConfig.order` is REQUIRED in `@object-ui/types` and in its zod mirror, and the protocol's `SortItemSchema` requires it too (measured: it refuses `[{ field: 'name' }]` with `invalid_value` at `0.order`). Worse, this block's untouched array arm interpolates whatever is there, so an omitted `order` lowers to `$orderby: 'name undefined'`. The changeset now says both keys are required and names that behaviour; the defect itself is pre-existing, is NOT widened into this diff, and is filed as a successor card. R2 — `serverSorting.test.tsx` had a defect frozen green at the arrow case: - its fixture is re-authored in the declared array spelling, the same principle this PR already applied to the sibling case; - the string's behaviour is now an EXPLICIT pin naming objectui#8961, asserting both halves — the arrow is drawn AND the query carries no `$orderby` AND the disagreement is announced once. A test that asserted only the arrow was a green certificate that the state is intended, not a record of a divergence; - the `parseSchemaSort` describe title ("the header reads what the fetch path reads") was falsified by this PR and is renamed to say what that block actually pins: the reader's own contract, wider than the fetch path. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Jmxdo7bmeqCQHLSfmLVX9w --- .../8767-object-grid-refuses-string-sort.md | 36 ++++++++-- .../src/__tests__/serverSorting.test.tsx | 68 ++++++++++++++++--- 2 files changed, 90 insertions(+), 14 deletions(-) diff --git a/.changeset/8767-object-grid-refuses-string-sort.md b/.changeset/8767-object-grid-refuses-string-sort.md index 545e8ea597..5ab4c24690 100644 --- a/.changeset/8767-object-grid-refuses-string-sort.md +++ b/.changeset/8767-object-grid-refuses-string-sort.md @@ -20,12 +20,38 @@ with private code, so a bare grid went on honouring at runtime a spelling — which is the per-block divergence the ruling declined by name when it rejected option A. -**Migration.** Write the array: `sort: [{ field: 'name', order: 'desc' }]` -(`order` is optional and means `'asc'`). That is the only spelling +**Migration.** Write the array: `sort: [{ field: 'name', order: 'desc' }]`. +**Both keys are required.** `SortConfig.order` carries no `?` in +`@object-ui/types` (`packages/types/src/objectql.ts`) and no `.optional()` in +its zod mirror, and the protocol's own reusable `SortItemSchema` requires +`order` as well — measured: that schema refuses `[{ field: 'name' }]` with +`invalid_value` at `0.order`. Do not omit it: this block's array arm +interpolates whatever is present, so an omitted `order` lowers to +`$orderby: 'name undefined'` today. That is pre-existing behaviour on the arm +this change does not touch, and it is filed as a successor card rather than +widened into here. + +**What the spec face does and does not say.** Measured against the installed +`@objectstack/spec@17.4.0`: `ui.ObjectGridPropsSchema` is **value-agnostic** on +this key — `sort` is `z.unknown().optional()`, so `safeParse` accepts +`'name desc'`, `'name'`, `42`, `['name desc']` and `{ name: 'desc' }` alike, +while an undeclared `bogusProp` is refused with `unrecognized_keys` (the +control that shows those parse readings are real and not a schema that accepts +everything). So the protocol's **validator** does not refuse the string, and +this change is not a narrowing the validator already performed. + +What the protocol **declares and documents** is the array, in three places: +that same key's own `describe` reads `Initial sort (array of { field, order })`; +the sibling `ElementRecordPickerPropsSchema.sort` spells the identical intent as +a typed `z.array(SortItemSchema)`, which refuses a string outright; and the +`defaultSort` retirement text instructs authors to rename the key to `sort` and +`wrap the value in an array`. The array is likewise the only spelling `ObjectGridSchema.sort` has declared since #8221, the only one the registered -`sort` input publishes, and the only one `@objectstack/spec` accepts — so -type-checked metadata is already on it, and only untyped JSON or a stored -`sys_metadata` row can still carry the string. +`sort` input publishes (`type: 'array'`), and the only one +`convertSortToQueryParams` lowers. So type-checked metadata is already on it, +and only untyped JSON or a stored `sys_metadata` row can still carry the string +— which is exactly why the refusal is a loud runtime diagnostic rather than a +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 diff --git a/packages/plugin-grid/src/__tests__/serverSorting.test.tsx b/packages/plugin-grid/src/__tests__/serverSorting.test.tsx index 336d251e7d..79021cbff5 100644 --- a/packages/plugin-grid/src/__tests__/serverSorting.test.tsx +++ b/packages/plugin-grid/src/__tests__/serverSorting.test.tsx @@ -22,6 +22,7 @@ import React from 'react'; import { ObjectGrid, parseSchemaSort } from '../ObjectGrid'; import { registerAllFields } from '@object-ui/fields'; +import { resetRetiredSortSpellingReports } from '@object-ui/core'; import { ActionProvider } from '@object-ui/react'; registerAllFields(); @@ -159,15 +160,13 @@ describe('ObjectGrid — column-header sorting is server-side (#3106)', () => { // Otherwise the first click on that column asks for `asc` on a list that is // already `desc`, and the arrow only tells the truth from click two on. // - // ⚠️ This case deliberately still authors the RETIRED string spelling, and - // is left that way: it is the one place in the suite that shows the header - // reader (`parseSchemaSort`) is now WIDER than the fetch path — since - // objectui#8767 the query carries no `$orderby` for this schema while the - // arrow below still appears. Narrowing the reader moves the wire shape and - // the export path with it; that is the route-B card #8767 did not take, so - // the divergence is recorded here rather than papered over. + // Authored in the ONE declared spelling. This case used to author the + // retired string clause; the subject is the header behaviour and the + // spelling was incidental, so it moves to the array for the same reason + // the sibling case above did. What a string does to this same schema is + // pinned deliberately, in the case below. const ds = makeDataSource(); - const { container } = renderGrid(ds, { sort: 'status desc' }); + const { container } = renderGrid(ds, { sort: [{ field: 'status', order: 'desc' }] }); await waitFor(() => expect(screen.getByText('Row 0')).toBeInTheDocument()); expect(headerCell(container, 'Status').querySelector('[class*="chevron-down"]')).not.toBeNull(); @@ -179,6 +178,48 @@ 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. + // + // 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. + resetRetiredSortSpellingReports(); + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + const ds = makeDataSource(); + 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 two — and the query it was fetched with carries no ordering at + // all. `hasOwnProperty`, not `toBeUndefined`: the key is absent, which is + // a different claim from "present and undefined". + 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. + const retired = errorSpy.mock.calls.filter((c) => + String(c[0]).includes('objectui#8221'), + ); + expect(retired).toHaveLength(1); + } finally { + errorSpy.mockRestore(); + } + }); + it('withholds the sort affordance from a relational column (#3096)', async () => { const ds = makeDataSource(); const { container } = renderGrid(ds); @@ -195,7 +236,16 @@ describe('ObjectGrid — column-header sorting is server-side (#3106)', () => { }); }); -describe('parseSchemaSort — the header reads what the fetch path reads', () => { +/** + * ⚠️ 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". + */ +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' }]); });