fix(plugin-list): the Sort picker's field list asks the column read check (objectui#11943)

.changeset/11943-sort-field-read.md

@@ -0,0 +1,10 @@
+---
+'@object-ui/plugin-list': patch
+---
+
+The list's Sort picker no longer offers a field the user may not read (objectui#11943). Choosing such a field sent a sort the server refuses, and the list went blank on "no access". The picker now asks the same field-level read check as the list's columns and its Filter panel, `checkField(objectName, field, 'read')` from the permission context. The Filter panel and the Sort picker share one predicate for it.
+
+- **A field the current sort already uses stays listed.** A stored or URL sort on an unreadable field still shows its row by name, so it can be removed. For now that field is also still offered in the picker's other rows. Listing it only as removable is a follow-up.
+- **Nothing else changes.** While the permission answer has not loaded, the list is as before, the same way the column check defers. A user who may read every field sees the same list, in the same order. A link field the user may not read no longer brings up the hint about link fields not being sortable. The Filter panel's list is unchanged.
+
+**Clause-②: no** — no export, prop, type member or accepted input changes.

packages/plugin-list/src/ListView.tsx

@@ -1106,6 +1106,30 @@ function describeRefusedPageSize(
   );
 }
 
+/**
+ * Field-level READ for the field lists the toolbar offers: the Filter panel's
+ * (`filterFields`, objectui#11925) and the Sort picker's (`sortFields`,
+ * objectui#11943). The two lists used to disagree: objectui#11925 wrote this
+ * predicate inside the filter memo, so the sort picker never asked it. It now
+ * lives once, here, and both lists call it.
+ *
+ * It is the same call the column gate (`effectiveFields`) makes,
+ * `perms.checkField(objectName, field, 'read')`, behind the same gate: until
+ * the permission answer has loaded, or with no object to ask about, it answers
+ * true. An unanswered policy filters nothing, exactly as the columns defer.
+ *
+ * A plain function of the permission value and the object name rather than a
+ * hook. Both memos already depend on those two, so neither memo depends on a
+ * function's identity (AGENTS.md #10).
+ */
+function canReadField(
+  perms: ReturnType<typeof usePermissions>,
+  objectName: string | undefined,
+  field: string,
+): boolean {
+  return !perms?.isLoaded || !objectName || perms.checkField(objectName, field, 'read');
+}
+
 /**
  * Imperative handle exposed by ListView via React.forwardRef.
  * Allows parent components to trigger a data refresh programmatically.
@@ -3150,12 +3174,22 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
         })
       : t('detail.recordDetail');
 
-  // Field-level permission gate. Filter unreadable columns from the
-  // field list BEFORE any downstream column construction so they also
-  // disappear from the hide-fields popover, filter/sort builders, and
-  // grid `$select`. (`perms` was hoisted to before the data-fetch
-  // effect so $select can be gated server-side too.)
-  // Apply hiddenFields and fieldOrder to produce effective fields
+  // The columns this view draws: the declared columns, minus the ones the user
+  // may not read (`perms.checkField(objectName, field, 'read')`, skipped until
+  // the permission answer loads or when there is no object name), minus the
+  // hidden ones, in `fieldOrder`. Every child view receives it as `fields`, and
+  // the grid also as `columns` when the author declared any. The kanban card
+  // fields and the tree fields fall back to it, and the export's columns are
+  // built from it. The Filter panel reads it only to order its list.
+  //
+  // The other field lists are not built from it, and each asks the same read
+  // itself. The `$select` projection in the data-fetch effect filters the
+  // declared columns with the same call; `perms` is read before that effect so
+  // it can. The Filter panel's list and the Sort picker's list ask through
+  // `canReadField` (objectui#11925, objectui#11943). One exception: the Sort
+  // picker keeps a field the current sort already names, so its row can be
+  // removed. Until objectui#11943's follow-up that field is also still
+  // choosable in the picker's other rows.
   const effectiveFields = React.useMemo(() => {
     // Defensive: `columns` is `string[] | ListColumn[]`, but metadata is
     // user-authored — anything non-array degrades to "no declared columns".
@@ -4001,10 +4035,10 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
    * drifted. Both candidate sources pass through this one gate, the
    * definition's fields and the declared-columns fallback alike. An
    * unanswered policy filters nothing, exactly as the column gate defers.
+   * The predicate is `canReadField`, which the Sort picker shares
+   * (objectui#11943).
    */
   const filterFields = React.useMemo(() => {
-    const canRead = (name: string) =>
-      !perms?.isLoaded || !schema.objectName || perms.checkField(schema.objectName, name, 'read');
     const whitelist =
       schema.filterableFields && schema.filterableFields.length > 0
         ? new Set<string>(schema.filterableFields)
@@ -4022,7 +4056,7 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
     const tierOf = (name: string) =>
       isHidden(name) ? 3 : columnRank.has(name) ? 0 : isSystemManagedField(name, defs?.[name]) ? 2 : 1;
     return candidateFields
-      .filter((f) => canRead(f.value))
+      .filter((f) => canReadField(perms, schema.objectName, f.value))
       .filter((f) => (!whitelist || whitelist.has(f.value)) && (!isHidden(f.value) || !!whitelist || held.has(f.value)))
       .map((field, index) => ({ field, index, tier: tierOf(field.value) }))
       .sort((a, b) =>
@@ -4084,6 +4118,18 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
   // For a platform-refused field that exception is the only way to REMOVE the
   // offending row, since the sort it names is one the server refuses outright.
   //
+  // Field-level read (objectui#11943) is asked before either rule, through
+  // `canReadField`, the predicate the Filter panel's list uses. A field the
+  // user may not read is not offered: the server answers a sort on it with a
+  // 403, and the list blanks to its no-access state. A dropped field does not
+  // raise the relational hint either; the hint explains a missing relation the
+  // user could otherwise read. The in-use exception covers this rule too. A
+  // field the current sort already names (a stored or URL sort) stays listed,
+  // exactly as the two rules above would list it and with no mark, so its row
+  // is not blank and can be removed. Until objectui#11943's follow-up it is
+  // also still choosable in every other row, and by "Add sort" when it comes
+  // first: listing it as removable only needs a `SortBuilder` change.
+  //
   // ONE read of the served projection, for BOTH legs below — the list this
   // picker renders, and the sort it emits for a host to persist. Read twice,
   // the two copies could answer differently about the same field on the same
@@ -4099,6 +4145,7 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
     let excluded = false;
     const fields: Array<{ value: string; label: string }> = [];
     for (const field of candidateFields) {
+      if (!inUse.has(field.value) && !canReadField(perms, schema.objectName, field.value)) continue;
       const relational = EXPANDABLE_FIELD_TYPES.has(field.type);
       const platformSortable = platformSortability
         ? isPlatformSortableField(platformSortability, field.value)
@@ -4117,7 +4164,7 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
       if (relational) excluded = true;
     }
     return { sortFields: fields, sortHasRelationalField: excluded };
-  }, [candidateFields, currentSort, t, platformSortability]);
+  }, [candidateFields, currentSort, t, platformSortability, perms, schema.objectName]);
 
   /**
    * [#6455] THE persist boundary: what this picker LISTS is not what it

packages/plugin-list/src/__tests__/ListView.sortFieldRead-11943.test.tsx

@@ -0,0 +1,224 @@
+/**
+ * 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#11943 — the Sort picker's field list applies the same field-level
+ * read check as the list's columns and the Filter panel's list.
+ *
+ * The columns drop a field the user may not read through
+ * `perms.checkField(objectName, field, 'read')`, and objectui#11925 gave the
+ * Filter panel the same read. The Sort picker was built from the object
+ * definition with no read check, so it offered a field the grid had dropped,
+ * and choosing it sent a sort the server refuses, blanking the list. Both
+ * lists now ask one predicate, `canReadField`.
+ *
+ * Everything here is read off the REAL `SortBuilder` dropdown, through the
+ * provider the console mounts (`MePermissionsProvider`, with a
+ * `/me/permissions`-shaped answer keyed `"object.field"`).
+ *
+ * The fixture is synthetic: one object, one field (`secret_note`) the
+ * restricted answer marks unreadable, and one lookup (`secret_owner`) used only
+ * by the relational-hint case.
+ */
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import { cleanup, render, screen, fireEvent, waitFor, within } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import React from 'react';
+import type { DataSource, ListViewSchema } from '@object-ui/types';
+import { SchemaRendererProvider } from '@object-ui/react';
+import { MePermissionsProvider, type MePermissionsResponse } from '@object-ui/permissions';
+import { ListView } from '../ListView';
+
+const OBJECT = 'fls_ticket';
+
+/** The object definition, in declaration order. `priority` is no column. */
+const FIELDS: Record<string, Record<string, unknown>> = {
+  title: { type: 'text', label: 'Title' },
+  status: { type: 'select', label: 'Status', options: [{ label: 'Open', value: 'open' }] },
+  secret_note: { type: 'textarea', label: 'Secret Note' },
+  priority: { type: 'number', label: 'Priority' },
+};
+
+const COLUMNS = ['title', 'status', 'secret_note'];
+
+/** Today's Sort picker for a user who may read every field. */
+const FULL_READ_LABELS = ['Title', 'Status', 'Secret Note', 'Priority'];
+
+function answer(fields: MePermissionsResponse['fields']): MePermissionsResponse {
+  return {
+    authenticated: true,
+    userId: 'u1',
+    tenantId: null,
+    roles: [],
+    permissionSets: ['fls_member'],
+    objects: { [OBJECT]: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false } },
+    fields,
+  };
+}
+
+const RESTRICTED = answer({
+  [`${OBJECT}.secret_note`]: { readable: false, editable: false },
+  [`${OBJECT}.secret_owner`]: { readable: false, editable: false },
+});
+const FULL_READ = answer({});
+
+function makeDataSource(fields: Record<string, Record<string, unknown>> = FIELDS) {
+  return {
+    find: vi.fn().mockResolvedValue({ data: [], total: 0 }),
+    findOne: vi.fn(),
+    create: vi.fn(),
+    update: vi.fn(),
+    delete: vi.fn(),
+    getObjectSchema: vi.fn().mockResolvedValue({ name: OBJECT, fields }),
+  };
+}
+
+function listFor(sort: Array<{ field: string; order: 'asc' | 'desc' }>, dataSource: ReturnType<typeof makeDataSource>) {
+  const schema = {
+    type: 'list-view',
+    objectName: OBJECT,
+    viewType: 'grid',
+    columns: COLUMNS.map((field) => ({ field })),
+    sort,
+  } as unknown as ListViewSchema;
+  return <ListView schema={schema} dataSource={dataSource} />;
+}
+
+function mount(
+  { perms, sort = [{ field: 'title', order: 'asc' }], fields = FIELDS }: {
+    perms?: MePermissionsResponse;
+    sort?: Array<{ field: string; order: 'asc' | 'desc' }>;
+    fields?: Record<string, Record<string, unknown>>;
+  } = {},
+) {
+  const dataSource = makeDataSource(fields);
+  const list = listFor(sort, dataSource);
+  render(
+    <SchemaRendererProvider dataSource={dataSource as unknown as DataSource}>
+      {perms ? <MePermissionsProvider initialPermissions={perms}>{list}</MePermissionsProvider> : list}
+    </SchemaRendererProvider>,
+  );
+  return dataSource;
+}
+
+function sortPanel(): HTMLElement {
+  const dialog = screen.getByText('Sort Records').closest('[role="dialog"]');
+  if (!dialog) throw new Error('no popover content around "Sort Records"');
+  return dialog as HTMLElement;
+}
+
+/** The field triggers of the sort rows: each row is `[field select, direction select]`. */
+function sortRowTriggers(): HTMLElement[] {
+  return within(sortPanel())
+    .getAllByRole('combobox')
+    .filter((_, i) => i % 2 === 0);
+}
+
+/** The labels a sort row's field dropdown offers (Radix renders items only once opened). */
+async function optionsOf(trigger: HTMLElement): Promise<string[]> {
+  fireEvent.click(trigger);
+  const listbox = await screen.findByRole('listbox');
+  const labels = within(listbox)
+    .getAllByRole('option')
+    .map((o) => o.textContent?.trim() ?? '');
+  fireEvent.keyDown(listbox, { key: 'Escape' });
+  return labels;
+}
+
+/**
+ * Open the Sort popover and return the first row's options once the object
+ * definition has built them (`priority` is no column, so seeing it proves the
+ * definition, not the columns-only fallback, built the list).
+ */
+async function openSortOptions(): Promise<string[]> {
+  fireEvent.click(await screen.findByRole('button', { name: /^sort/i }));
+  await screen.findByText('Sort Records');
+  let labels: string[] = [];
+  await waitFor(async () => {
+    labels = await optionsOf(sortRowTriggers()[0]);
+    expect(labels).toContain('Priority');
+  });
+  return labels;
+}
+
+afterEach(() => {
+  cleanup();
+});
+
+describe('the Sort picker field list asks the column read check (objectui#11943)', () => {
+  it('does not offer a field the user may not read, and offers every readable one', async () => {
+    mount({ perms: RESTRICTED });
+    const labels = await openSortOptions();
+    expect(labels).not.toContain('Secret Note');
+    expect(labels).toEqual(['Title', 'Status', 'Priority']);
+  });
+
+  it('control: a user who may read every field gets today’s list, the same as with no permission answer', async () => {
+    mount({ perms: FULL_READ });
+    expect(await openSortOptions()).toEqual(FULL_READ_LABELS);
+    cleanup();
+    mount();
+    expect(await openSortOptions()).toEqual(FULL_READ_LABELS);
+  });
+
+  it('before the permission answer is loaded nothing is filtered, as the columns defer', async () => {
+    // The real "not loaded" state with children mounted: the provider is
+    // refetching. It still holds the RESTRICTED answer, so `checkField` would
+    // deny `secret_note`, but `isLoaded` is false, and the column gate skips
+    // its filter on exactly that flag. The Sort picker must agree.
+    const dataSource = makeDataSource();
+    let calls = 0;
+    const fetcher = vi.fn(() => {
+      calls += 1;
+      return calls === 1
+        ? Promise.resolve(new Response(JSON.stringify(RESTRICTED), { status: 200 }))
+        : new Promise<Response>(() => {});
+    }) as unknown as typeof fetch;
+    const tree = (endpoint: string) => (
+      <SchemaRendererProvider dataSource={dataSource as unknown as DataSource}>
+        <MePermissionsProvider endpoint={endpoint} fetcher={fetcher} maxRetries={0}>
+          {listFor([{ field: 'title', order: 'asc' }], dataSource)}
+        </MePermissionsProvider>
+      </SchemaRendererProvider>
+    );
+    const { rerender } = render(tree('/me/permissions?first'));
+    // Loaded, restricted: the field is withheld.
+    expect(await openSortOptions()).toEqual(['Title', 'Status', 'Priority']);
+    // A refetch that never settles: the provider keeps its data, `isLoaded` drops.
+    rerender(tree('/me/permissions?second'));
+    await waitFor(() => expect(fetcher).toHaveBeenCalledTimes(2));
+    await waitFor(async () => {
+      expect(await optionsOf(sortRowTriggers()[0])).toEqual(FULL_READ_LABELS);
+    });
+  });
+
+  it('a field the current sort already names stays listed (the known half-state)', async () => {
+    // objectui#11943's ruling keeps such a field listed so its row is not
+    // blank and can be removed. This pins the half shipped here: it stays
+    // listed exactly as before, with no mark, and it is still choosable in the
+    // picker's other rows. Listing it as removable only (marked unavailable,
+    // never offered as a new choice) needs a `SortBuilder` change and is
+    // objectui#11943's follow-up, which will change this pin.
+    mount({ perms: RESTRICTED, sort: [{ field: 'secret_note', order: 'asc' }] });
+    const labels = await openSortOptions();
+    expect(labels).toContain('Secret Note');
+    expect(labels).toEqual(FULL_READ_LABELS);
+    expect(sortRowTriggers()[0]).toHaveTextContent('Secret Note');
+  });
+
+  it('an unreadable lookup does not raise the relational hint; a readable one still does', async () => {
+    const withLookup = { ...FIELDS, secret_owner: { type: 'lookup', label: 'Secret Owner', reference: 'sys_user' } };
+    mount({ perms: RESTRICTED, fields: withLookup });
+    await openSortOptions();
+    expect(screen.queryByTestId('sort-relational-hint')).toBeNull();
+    cleanup();
+    mount({ perms: FULL_READ, fields: withLookup });
+    await openSortOptions();
+    expect(screen.getByTestId('sort-relational-hint')).toBeInTheDocument();
+  });
+});