fix(plugin-view): a named view's sort array rides the canonical grid slot, so the grid shows it and $orderby stops saying 'undefined undefined' (#5270)

.changeset/objectview-named-view-sort-arity.md

@@ -0,0 +1,42 @@
+---
+'@object-ui/plugin-view': patch
+---
+
+`ObjectView` sends a named view's `sort` to the grid slot that can hold it — the declared sort now reaches both the header indicator and `$orderby`.
+
+A named view's sort is an **array**: `NamedListView.sort` is
+`Array< { field, order } >`, and the `views` prop declares an array too.
+`ObjectView` forwarded the resolved view sort into `gridSchema.defaultSort`,
+which `ObjectGridSchema` declares as a **single** `{ field, order }`. The
+arity mismatch had no compile-time witness — `ObjectViewSchema.table`
+collapses to a bare index signature — and both of `ObjectGrid`'s readers then
+failed, in different ways:
+
+- **The header drew nothing.** `parseSchemaSort(schemaSort ?? (schema.defaultSort
+ ? [schema.defaultSort] : undefined))` re-wraps an already-array `defaultSort`
+ into `[[{ field, order }]]`. Each entry must be a string or an object with a
+ string `field`; a nested array is neither, so the entry was skipped and the
+ parse returned `[]`. A view that arrived sorted `name desc` looked unsorted,
+ and the first click on that column asked for `asc` on a list already `desc`.
+- **The fetch sent nonsense.** `` `${(schema.defaultSort as any).field} ${(schema
+ .defaultSort as any).order}` `` reads two absent keys off an array, so the
+ request carried the literal string `"undefined undefined"` as `$orderby`.
+ `serializeOrderBy` passes a non-empty string through untouched, so that
+ reached the server verbatim.
+
+The two view precedence segments (`listViews` entry, then the active `views`
+entry) now ride the **canonical** `sort` slot, declared `string | SortConfig[]`
+— the arity a view actually carries, and the only one of the pair that can
+express a multi-key sort at all. The legacy `defaultSort` slot keeps carrying
+the `table` segment alone and is read exactly as before.
+
+**Precedence is unchanged.** `ObjectGrid` resolves `sort ?? defaultSort`, so a
+view sort still outranks both `table.sort` and `table.defaultSort`, and a
+`table.sort` still outranks a `table.defaultSort` — the same order the non-grid
+fetch and the delegated `renderListView` schema already express. A view that
+supplies no sort forwards exactly what it forwarded before.
+
+This is also the shape the shared sort sink accepts (`convertSortToQueryParams`
+takes `string | SortConfig[]`), so the fix converges on the normalized dialect
+rather than adding another spelling for the sort-sink convergence work to fold
+in later.

packages/plugin-view/src/ObjectView.tsx

@@ -1038,13 +1038,42 @@ export const ObjectView: React.FC<ObjectViewProps> = ({
  // free choice here: emitting both slots lets ObjectGrid's existing
  // canonical-wins rule decide, which is the only answer that keeps the two
  // layers saying the same thing.
+ //
+ // objectui#5270: the two segments AHEAD of `table` had a second, separate
+ // problem — an ARITY mismatch, not a spelling one. Both of them carry an
+ // ARRAY of sort keys (`NamedListView.sort` is `Array< { field, order } >`;
+ // the `views` prop declares an array too) and both were being written into
+ // `defaultSort`, which is declared a SINGLE `{ field, order }`. Neither of
+ // ObjectGrid's two readers survives that:
+ //
+ // header `parseSchemaSort(schemaSort ?? [schema.defaultSort])` becomes
+ // `parseSchemaSort([[{ field, order }]])`. The outer array is
+ // iterated and each entry must be a string or an object with a
+ // string `field`; a nested ARRAY is neither, so the entry is
+ // dropped and the result is `[]` — no arrow, the view arrives
+ // looking unsorted.
+ // fetch `` `${(schema.defaultSort as any).field} ${….order}` `` reads two
+ // missing keys off an array and sends the literal string
+ // `"undefined undefined"` as `$orderby`.
+ //
+ // So the view's sort now rides the CANONICAL slot, which is declared
+ // `string | SortConfig[]` and therefore already holds the arity a view
+ // carries. That is also the shape the shared sort sink accepts
+ // (`convertSortToQueryParams`, `string | SortConfig[]` — objectui#4869), so
+ // this converges on the normalized dialect instead of introducing another.
+ // Precedence is unchanged: ObjectGrid resolves `sort ?? defaultSort`, so a
+ // view sort still outranks a `table.defaultSort`, and `table.sort` still
+ // outranks it too — the same order `mergedSort` and the non-grid fetch use.
  const gridSchema: ObjectGridSchema = useMemo(() => {
- // The two segments ahead of the `table` one, resolved once. They keep
- // riding the LEGACY slots they ride today: `filter`/`defaultFilters` are
- // not interchangeable downstream — ObjectGrid lowers the canonical slot
- // through `toFilterNode` and raw-assigns the legacy one — so moving a
- // named-view filter across would change the wire shape of a path this
- // card does not own.
+ // The two segments ahead of the `table` one, resolved once.
+ //
+ // `viewFilter` keeps riding the LEGACY slot it rides today:
+ // `filter`/`defaultFilters` are not interchangeable downstream —
+ // ObjectGrid lowers the canonical slot through `toFilterNode` and
+ // raw-assigns the legacy one — so moving a named-view filter across would
+ // change the wire shape of a path objectui#5270 does not own. The sort
+ // pair has no such asymmetry: both slots reach `$orderby` unlowered, and
+ // only the canonical one can hold more than a single key.
  const viewFilter = currentNamedViewConfig?.filter || activeView?.filter;
  const viewSort = currentNamedViewConfig?.sort || activeView?.sort;
 
@@ -1060,15 +1089,23 @@ export const ObjectView: React.FC<ObjectViewProps> = ({
  create: false, // Create is handled by the view's create button
  },
  defaultFilters: viewFilter || schema.table?.defaultFilters,
- defaultSort: viewSort || schema.table?.defaultSort,
- // Canonical `table` keys, at last forwarded. `filter`/`sort` carry the
- // `table` segment ONLY: a view segment resolved above already occupies
+ // Legacy slot, `table` segment ONLY (objectui#5270). The view segments
+ // moved to the canonical `sort` below because this one holds a single
+ // `{ field, order }` and they carry arrays; ObjectGrid resolves
+ // `sort ?? defaultSort`, so a view sort still outranks this default.
+ defaultSort: schema.table?.defaultSort,
+ // Canonical `table` keys, at last forwarded. `filter` carries the
+ // `table` segment ONLY: the view segment resolved above already occupies
  // the legacy slot, and ObjectGrid prefers this slot over that one — so
  // handing it `table.filter` while a named view is active would let the
  // table default outrank the view, inverting the precedence the two
  // untouched segments exist to express.
  filter: viewFilter ? undefined : schema.table?.filter,
- sort: viewSort ? undefined : schema.table?.sort,
+ // `sort` carries the WHOLE chain instead — view segments first, then the
+ // `table` one. Same precedence as `mergedSort` and the non-grid fetch
+ // express; what changes is only WHICH slot a view's sort arrives in, and
+ // this is the one whose declared arity can hold it.
+ sort: viewSort || schema.table?.sort,
  pagination: schema.table?.pagination,
  selection: schema.table?.selection,
  pageSize: schema.table?.pageSize,

packages/plugin-view/src/__tests__/ObjectView.canonicalTableKeys.test.tsx

@@ -239,11 +239,16 @@ describe('grid path: both spellings written — each rides its own slot', () =>
 });
 
 describe('grid path: a named view still outranks the table segment', () => {
- // The two segments ahead of `table` (`listViews` entry, then `activeView`)
- // are untouched by objectui#5102 — they keep riding the legacy slots. The
+ // The FILTER segments ahead of `table` (`listViews` entry, then `activeView`)
+ // are untouched by objectui#5102 — they keep riding the legacy slot. The
  // canonical slot must therefore stay EMPTY while one of them is active:
  // ObjectGrid prefers the canonical slot, so a `table.filter` forwarded
  // unconditionally would outrank the view the user is looking at.
+ //
+ // The SORT half moved in objectui#5270 — see the sort assertion below for
+ // why, and `ObjectView.namedViewSortArity.test.tsx` for the two consumers
+ // that made the old slot unreadable. What is pinned here either way is the
+ // PRECEDENCE, which the move preserves: the view still outranks `table`.
  const namedView = {
  listViews: {
  won: {
@@ -265,12 +270,33 @@ describe('grid path: a named view still outranks the table segment', () => {
  });
 
  it('keeps the named view sort in force over a table.sort', () => {
+ // objectui#5102 pinned this as `sort: undefined` +
+ // `defaultSort: [{ field: 'name', order: 'desc' }]` and said in so many
+ // words that the pin should be UPDATED, not deleted, by whoever fixed the
+ // arity. Updated here: the array now rides the canonical slot — the only
+ // one of the two whose declared type (`string | SortConfig[]`) can hold
+ // more than one key — and the legacy slot carries the `table` segment
+ // alone. Precedence is what this block asserts and it is unchanged:
+ // ObjectGrid resolves `schemaSort ?? defaultSort`, so the named view wins
+ // over `table.sort` because it is what reaches the canonical slot.
  const grid = forwardedGridSchema({
  ...namedView,
  table: { sort: 'created asc' } as any,
  } as any);
- expect(grid.sort).toBeUndefined();
- expect(grid.defaultSort).toEqual([{ field: 'name', order: 'desc' }]);
+ expect(grid.sort).toEqual([{ field: 'name', order: 'desc' }]);
+ expect(grid.defaultSort).toBeUndefined();
+ });
+
+ it('keeps the named view sort in force over a legacy table.defaultSort too', () => {
+ // The other side of the same precedence: the `table` segment holds the
+ // legacy slot alone, and ObjectGrid prefers the canonical one, so the view
+ // still wins. Without this the move could have inverted the pair silently.
+ const grid = forwardedGridSchema({
+ ...namedView,
+ table: { defaultSort: { field: 'created', order: 'asc' } } as any,
+ } as any);
+ expect(grid.sort).toEqual([{ field: 'name', order: 'desc' }]);
+ expect(grid.defaultSort).toEqual({ field: 'created', order: 'asc' });
  });
 });
 

packages/plugin-view/src/__tests__/ObjectView.namedViewSortArity.test.tsx

@@ -0,0 +1,195 @@
+/**
+ * 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.
+ */
+
+/**
+ * objectui#5270 — a named view's `sort` is an ARRAY, and it used to be handed
+ * to a slot declared to hold ONE key.
+ *
+ * `NamedListView.sort` is `Array< { field, order } >`. `ObjectView` forwarded
+ * the resolved view sort into `gridSchema.defaultSort`, which
+ * `ObjectGridSchema` declares as a single `{ field: string; order: 'asc' |
+ * 'desc' }`. Nothing complained — `ObjectViewSchema.table` collapses to a bare
+ * index signature (objectui#5102), so the arity mismatch had no compile-time
+ * witness — and BOTH of `ObjectGrid`'s readers then failed, in different ways:
+ *
+ * header `parseSchemaSort(schemaSort ?? (schema.defaultSort ?
+ * [schema.defaultSort] : undefined))` re-wraps an already-array
+ * `defaultSort` into `[[{ field, order }]]`. `parseSchemaSort`
+ * accepts a string or an object with a string `field` per entry; a
+ * nested ARRAY is neither, so the entry is skipped and the parse
+ * returns `[]`. The user saw NO sort indicator at all.
+ * fetch `` params.$orderby = `${(schema.defaultSort as any).field} ${…
+ * .order}` `` reads two absent keys off an array, so the request
+ * carried the literal string `"undefined undefined"`.
+ *
+ * The fix routes the view segments into the CANONICAL `sort` slot, declared
+ * `string | SortConfig[]` — the arity a view actually carries, and the same
+ * spelling the shared sort sink `convertSortToQueryParams` accepts
+ * (objectui#4869), so no fourth dialect is introduced to make this work.
+ *
+ * These tests drive the REAL `ObjectGrid` rather than a probe that records the
+ * forwarded schema: the defect was invisible in the forwarded object (the array
+ * was right there, in the wrong slot) and only appeared at the two consumers.
+ * Pinning the forwarded shape alone would have re-pinned the bug.
+ *
+ * Resolution note for anyone running an ablation on this file: nothing here
+ * goes through a build. `../ObjectView` is a relative source import, and
+ * `@object-ui/plugin-grid` / `@object-ui/components` / `@object-ui/core` are
+ * aliased to each package's own `src` directory in the root
+ * `vitest.config.mts`, so a stale `dist` build cannot make a reverted source
+ * read green.
+ */
+
+import { describe, it, expect, vi, beforeAll } from 'vitest';
+import { render, screen, waitFor } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import React from 'react';
+
+import { ObjectView } from '../ObjectView';
+import { ActionProvider } from '@object-ui/react';
+import type { ObjectViewSchema } from '@object-ui/types';
+
+// No `registerAllFields()` here, deliberately: `@object-ui/fields` is not a
+// dependency of this package, and nothing asserted below needs a registered
+// cell renderer. The header cells and `$orderby` are produced by ObjectGrid
+// itself; an unregistered column still renders its label and its sort
+// affordance.
+beforeAll(() => {
+ if (!Element.prototype.hasPointerCapture) {
+ Element.prototype.hasPointerCapture = vi.fn(() => false) as any;
+ }
+ if (!Element.prototype.scrollIntoView) {
+ Element.prototype.scrollIntoView = vi.fn() as any;
+ }
+});
+
+function makeDataSource() {
+ const find = vi.fn(async () => ({
+ data: [
+ { id: 'a', name: 'Alpha', status: 'open' },
+ { id: 'b', name: 'Beta', status: 'open' },
+ ],
+ total: 2,
+ }));
+ return {
+ find,
+ findOne: vi.fn(),
+ create: vi.fn(),
+ update: vi.fn(),
+ delete: vi.fn(),
+ getObjectSchema: vi.fn(async (name: string) => ({
+ name,
+ label: 'Task',
+ fields: {
+ name: { label: 'Name', type: 'text' },
+ status: { label: 'Status', type: 'text' },
+ },
+ })),
+ } as any;
+}
+
+/** A view whose sort lives on a named `listViews` entry — the card's subject. */
+const namedViewSchema = (sort: unknown): ObjectViewSchema =>
+ ({
+ type: 'object-view',
+ objectName: 'task',
+ listViews: {
+ won: { label: 'Won', columns: ['name', 'status'], sort },
+ },
+ defaultListView: 'won',
+ }) as unknown as ObjectViewSchema;
+
+function renderView(schema: ObjectViewSchema, ds: any) {
+ return render(
+ <ActionProvider>
+ <ObjectView schema={schema} dataSource={ds} />
+ </ActionProvider>,
+ );
+}
+
+const lastFindParams = (ds: any) =>
+ ds.find.mock.calls[ds.find.mock.calls.length - 1][1];
+
+const headerCell = (container: HTMLElement, label: string) =>
+ Array.from(container.querySelectorAll('thead th')).find((th) =>
+ th.textContent?.includes(label),
+ ) as HTMLElement;
+
+describe("objectui#5270 — a named view's sort reaches the grid", () => {
+ it('draws the declared sort indicator before anyone clicks', async () => {
+ // Half one. The array used to be re-wrapped to `[[…]]` and parsed to `[]`,
+ // so the column the view was sorted by showed no arrow — and the first
+ // click on it then asked for `asc` on a list already ordered `desc`.
+ const ds = makeDataSource();
+ const { container } = renderView(
+ namedViewSchema([{ field: 'name', order: 'desc' }]),
+ ds,
+ );
+ await waitFor(() => expect(screen.getByText('Alpha')).toBeInTheDocument());
+
+ const name = headerCell(container, 'Name');
+ expect(name).toBeTruthy();
+ expect(name.querySelector('[class*="chevron-down"]')).not.toBeNull();
+ });
+
+ it('sends the declared sort as $orderby, not the string "undefined undefined"', async () => {
+ // Half two. `${arr.field} ${arr.order}` on an array is two `undefined`s,
+ // and the resulting `"undefined undefined"` reached the wire verbatim —
+ // `serializeOrderBy` passes a non-empty string through untouched, so the
+ // server got an unparseable sort rather than none.
+ const ds = makeDataSource();
+ renderView(namedViewSchema([{ field: 'name', order: 'desc' }]), ds);
+
+ await waitFor(() => expect(ds.find).toHaveBeenCalled());
+ await waitFor(() => {
+ expect(lastFindParams(ds).$orderby).toBe('name desc');
+ });
+ expect(lastFindParams(ds).$orderby).not.toBe('undefined undefined');
+ expect(String(lastFindParams(ds).$orderby)).not.toContain('undefined');
+ });
+
+ it('carries every key of a multi-key sort, which the legacy slot could not hold', async () => {
+ // The arity is the whole point: `defaultSort` is ONE `{ field, order }`,
+ // so a two-key view sort had nowhere to land even if the single-key case
+ // had somehow been made to work.
+ const ds = makeDataSource();
+ const { container } = renderView(
+ namedViewSchema([
+ { field: 'status', order: 'asc' },
+ { field: 'name', order: 'desc' },
+ ]),
+ ds,
+ );
+ await waitFor(() => expect(screen.getByText('Alpha')).toBeInTheDocument());
+
+ await waitFor(() => {
+ expect(lastFindParams(ds).$orderby).toBe('status asc, name desc');
+ });
+ expect(headerCell(container, 'Status').querySelector('[class*="chevron-up"]')).not.toBeNull();
+ expect(headerCell(container, 'Name').querySelector('[class*="chevron-down"]')).not.toBeNull();
+ });
+
+ it("still honours a table.defaultSort when no view supplies one", async () => {
+ // The legacy `table` slot keeps working: it is the one shape `defaultSort`
+ // was always declared to hold, and this fix narrows what is written into
+ // it rather than retiring it.
+ const ds = makeDataSource();
+ const { container } = renderView(
+ {
+ type: 'object-view',
+ objectName: 'task',
+ table: { columns: ['name', 'status'], defaultSort: { field: 'name', order: 'desc' } },
+ } as unknown as ObjectViewSchema,
+ ds,
+ );
+ await waitFor(() => expect(screen.getByText('Alpha')).toBeInTheDocument());
+
+ await waitFor(() => expect(lastFindParams(ds).$orderby).toBe('name desc'));
+ expect(headerCell(container, 'Name').querySelector('[class*="chevron-down"]')).not.toBeNull();
+ });
+});