diff --git a/.changeset/6598-listview-unauthored-columns.md b/.changeset/6598-listview-unauthored-columns.md new file mode 100644 index 0000000000..79a8102be9 --- /dev/null +++ b/.changeset/6598-listview-unauthored-columns.md @@ -0,0 +1,41 @@ +--- +'@object-ui/plugin-list': patch +--- + +`list-view` stops spelling "the author declared no columns" as an explicit empty +projection (objectui#6598). + +A production `kind:'html'` page carried `` +with no `columns` and rendered the row count, the filter/group/sort toolbar and +the index column — and **not one data column**, with no diagnostic anywhere. +`ObjectGrid` derives default columns for exactly that case ("Default columns +priority (when schema doesn't specify columns)"), and it never ran: the +derivation is gated on `schema.fields` being ABSENT, `ListView` sent +`fields: []`, and an empty array is truthy. `normalizeColumns` had already read +the empty `columns` as unauthored, so the two keys disagreed about the same fact +and the stricter reading won. + +`ListView` now asks whether the AUTHOR declared a projection — `columns` present +and non-empty, after the legacy `fields` fold — and hands the child grid nothing +at all when they did not, so the grid's own defaults apply. + +⚠️ The predicate reads the authored value and never what survived filtering, and +that distinction is load-bearing: when the author DID declare columns and the +field gate removed every one of them, the empty projection is still sent. +`ObjectGrid` re-applies FLS on its derived column path only, never on the +explicit-columns path, so falling through to the derivation there would put +fields on screen that the author never asked for and the principal may not read. + +Measured single-variable on the html tier: a bare +`` renders the object's default columns; +the same object behind `` rendered none. Pinned at the handoff +(`ListView.unauthoredColumnProjection-6598.test.tsx`) and end to end over the +real grid on a real html-kind page +(`htmlTierListViewDefaultColumns-6598.test.tsx`). + +This is one half of the reported symptom. Which columns the defaults resolve to +still depends on who owns the fetch — with a host like `ListView` fetching, the +grid takes its inline-data branch and derives from the row payload's keys rather +than from the object schema's policy (hidden and readonly system-managed fields +dropped, `highlightFields` honoured). That precedence sits in +`packages/plugin-grid` and is filed separately. diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index f52456576e..9e68c5983c 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -2058,6 +2058,41 @@ export const ListView = React.forwardRef(({ return fields; }, [schema.columns, schema.objectName, hiddenFields, schema.fieldOrder, perms]); + /** + * Did the AUTHOR declare a column projection at all? (objectui#6598) + * + * `effectiveFields` is `[]` in two situations the child grid cannot tell + * apart, and sending the same empty array for both is what produced this + * issue's headline symptom: `` on an + * html-kind page rendered the row count, the toolbar and the index column — + * and not one data column, with no diagnostic anywhere. + * + * 1. The author declared none (`columns` absent, or `[]`). `ObjectGrid` + * derives defaults from the object schema for exactly this case + * ("Default columns priority (when schema doesn't specify columns)"), + * and `normalizeColumns` already reads an empty `columns` as unauthored + * — the same rule `ElementDataSourceGate`'s precedence table states. + * But that derivation is gated on `schema.fields` being ABSENT, and an + * empty array is truthy, so the `fields: []` this component sent read as + * "show exactly these zero columns" and the defaults never ran. + * Single-variable measurement: a bare `` + * on the same tier, same data source, renders four default columns; the + * same object behind `` renders none. + * 2. The author declared some and the gates above removed them all — FLS + * denied every one, or every one is hidden. That case must KEEP sending + * the empty projection. Falling through to the grid's defaults there + * would show fields the author never asked for, and `ObjectGrid` + * re-applies FLS only on the DERIVED path, not on the explicit-columns + * one — so widening here would be a widening past the field gate. + * + * Hence the question is about the AUTHORED value and never about what + * survived filtering. + */ + const hasAuthoredColumns = React.useMemo( + () => Array.isArray(schema.columns) && schema.columns.length > 0, + [schema.columns], + ); + // Generate the appropriate view component schema const viewComponentSchema = React.useMemo(() => { const densityRowHeight = density.mode === 'compact' @@ -2101,7 +2136,13 @@ export const ListView = React.forwardRef(({ return { type: 'object-grid', ...baseProps, - columns: effectiveFields, + // Unauthored ⇒ hand the grid NO projection, so its default-columns + // derivation runs (see `hasAuthoredColumns`). `fields` has to be + // cleared with it: it rides in on `baseProps`, and it is the key the + // derivation is gated on. + ...(hasAuthoredColumns + ? { columns: effectiveFields } + : { fields: undefined, columns: undefined }), ...(schema.conditionalFormatting ? { conditionalFormatting: schema.conditionalFormatting } : {}), // [#4647] The MODE, not just its toggle. Gating only the toggle would // leave the issue's own consequence reachable by a different door: a @@ -2329,7 +2370,7 @@ export const ListView = React.forwardRef(({ // asynchronously (`/me/permissions`) and `objectDef` loads into state, so a // grid schema built before either resolved must be rebuilt when they do — // otherwise `editable` keeps the pre-verdict answer for the session. - }, [currentView, schema, currentSort, effectiveFields, groupingConfig, rowColorConfig, navigation.handleClick, density.mode, galleryCardSize, inlineEdit, inlineEditOffered, objectDef]); + }, [currentView, schema, currentSort, effectiveFields, hasAuthoredColumns, groupingConfig, rowColorConfig, navigation.handleClick, density.mode, galleryCardSize, inlineEdit, inlineEditOffered, objectDef]); const hasFilters = currentFilters.conditions && currentFilters.conditions.length > 0; diff --git a/packages/plugin-list/src/__tests__/ListView.unauthoredColumnProjection-6598.test.tsx b/packages/plugin-list/src/__tests__/ListView.unauthoredColumnProjection-6598.test.tsx new file mode 100644 index 0000000000..01732e542f --- /dev/null +++ b/packages/plugin-list/src/__tests__/ListView.unauthoredColumnProjection-6598.test.tsx @@ -0,0 +1,186 @@ +/** + * 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. + * + * "The author declared no columns" must not reach the child grid spelled as + * "the author declared exactly zero columns" (objectui#6598). + * + * ## The defect this pins + * + * A production `kind:'html'` page carried `` + * with no `columns`, and rendered the row count, the filter/group/sort toolbar + * and the index column — and NOT ONE data column, with no diagnostic anywhere. + * `ObjectGrid` has a default-columns derivation for exactly that case ("Default + * columns priority (when schema doesn't specify columns)"), and it never ran: + * the derivation is gated on `schema.fields` being ABSENT, ListView sent + * `fields: []`, and an empty array is truthy. `normalizeColumns` had already + * read the empty `columns` as unauthored — so the two keys disagreed about the + * same fact and the stricter reading won. + * + * The single-variable measurement that isolated it: a bare + * `` on the same tier, same page kind, + * same data source renders the object's default columns; the same object behind + * `` renders none. The only difference is this handoff. + * + * ## Why the FLS case is here and not in the permissions file + * + * `effectiveFields` is `[]` for two reasons that must NOT be handed down the + * same way, and only one of them is "unauthored". When the author DID declare + * columns and the field gate removed every one of them, the empty projection is + * the answer and has to survive: `ObjectGrid` re-applies FLS on its DERIVED + * column path only, never on the explicit-columns path, so falling through to + * the derivation there would put fields on screen that the author never asked + * for and the principal may not read. That is why the predicate reads the + * AUTHORED value and never what survived filtering — and why it is pinned next + * to the case it would otherwise be "simplified" into. + * + * plugin-grid is not a dependency of plugin-list (avoids a cycle), so — as in + * `ListView.findParamsHandoff.test.tsx` — a stub `object-grid` records what + * ListView feeds it. The end-to-end half, through the REAL grid on a real + * html-kind page, is `htmlTierListViewDefaultColumns-6598.test.tsx`. + */ +import { describe, it, expect, vi, beforeAll, afterAll, beforeEach, afterEach } from 'vitest'; +import { cleanup, render, waitFor } from '@testing-library/react'; +import React from 'react'; +import { ComponentRegistry } from '@object-ui/core'; +import { SchemaRendererProvider } from '@object-ui/react'; +import { PermissionProvider } from '@object-ui/permissions'; +import type { ListViewSchema, ObjectPermissionConfig, RoleDefinition } from '@object-ui/types'; +import { ListView } from '../ListView'; + +const OBJECT = 'opportunity'; + +let lastGridProps: any = null; + +function makeDataSource() { + return { + find: vi.fn(async () => ({ + data: [ + { id: 'o-1', name: 'Acme expansion', amount: 1000 }, + { id: 'o-2', name: 'Globex renewal', amount: 2000 }, + ], + total: 2, + hasMore: false, + })), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: async (name: string) => ({ + name, + fields: { + id: { type: 'text' }, + name: { type: 'text', label: 'Opportunity Name' }, + amount: { type: 'currency', label: 'Amount' }, + }, + }), + } as any; +} + +const listSchema = (over: Record = {}): ListViewSchema => + ({ type: 'list-view', objectName: OBJECT, ...over }) as unknown as ListViewSchema; + +let prevObjectGrid: any; +beforeAll(() => { + prevObjectGrid = ComponentRegistry.get('object-grid'); + ComponentRegistry.register('object-grid', (props: any) => { + lastGridProps = props; + return
; + }); +}); +afterAll(() => { + if (prevObjectGrid) ComponentRegistry.register('object-grid', prevObjectGrid); + else ComponentRegistry.unregister('object-grid'); +}); +beforeEach(() => { lastGridProps = null; }); +afterEach(() => { cleanup(); lastGridProps = null; }); + +/** + * What the child block actually reads. + * + * `SchemaRenderer` hands a registered component its schema AND spreads the + * schema's keys as props, and `ObjectGrid` reads the `schema` one — so the + * assertions below read it too rather than the spread, which is the copy that + * would still agree if the two ever diverged. `expect(gridSchema()).toBeTruthy()` + * in every case is the accessor's own positive control: without it a renamed + * prop would make every `toBeUndefined()` below pass while measuring nothing. + */ +const gridSchema = () => lastGridProps?.schema; + +async function renderList(schema: ListViewSchema, wrap?: (el: React.ReactElement) => React.ReactElement) { + const ds = makeDataSource(); + const inner = ; + render( + {wrap ? wrap(inner) : inner}, + ); + await waitFor(() => expect(lastGridProps).toBeTruthy()); + return ds; +} + +describe('ListView → object-grid: the unauthored column projection (#6598)', () => { + it('sends NO projection when the author declared no columns', async () => { + await renderList(listSchema()); + + // Both keys, because the grid reads both and either one alone re-pins the + // projection at zero: `columns` feeds `normalizeColumns`, `fields` gates the + // default-columns derivation. + expect(gridSchema()).toBeTruthy(); + expect(gridSchema().columns).toBeUndefined(); + expect(gridSchema().fields).toBeUndefined(); + }); + + it('treats an empty `columns` as unauthored too', async () => { + // The same rule `ElementDataSourceGate`'s precedence table states ("an empty + // `columns` counts as unauthored") and `normalizeColumns` already applies. + await renderList(listSchema({ columns: [] })); + + expect(gridSchema()).toBeTruthy(); + expect(gridSchema().columns).toBeUndefined(); + expect(gridSchema().fields).toBeUndefined(); + }); + + it('sends exactly the authored projection when the author declared one', async () => { + // The positive control for the two zeros above: this handoff does arrive, + // so their `undefined` is a decision and not a dead render path. + await renderList(listSchema({ columns: ['name', 'amount'] })); + + expect(gridSchema()).toBeTruthy(); + expect(gridSchema().columns).toEqual(['name', 'amount']); + expect(gridSchema().fields).toEqual(['name', 'amount']); + }); + + it('still sends an EMPTY projection when the field gate removed every authored column', async () => { + const roles: RoleDefinition[] = [{ name: 'restricted', label: 'Restricted' }]; + const permissions: ObjectPermissionConfig[] = [ + { + object: OBJECT, + roles: { + restricted: { + actions: ['read'], + fieldPermissions: [ + { field: 'name', read: false, write: false }, + { field: 'amount', read: false, write: false }, + ], + }, + }, + }, + ]; + + await renderList(listSchema({ columns: ['name', 'amount'] }), (el) => ( + + {el} + + )); + + // NOT `undefined`. The author declared a projection; every column of it was + // denied. Handing the grid "unauthored" here would run its derivation and + // put the object's other fields on screen — a widening past the field gate, + // which the explicit-columns path in ObjectGrid does not re-check. + expect(gridSchema()).toBeTruthy(); + expect(gridSchema().columns).toEqual([]); + expect(gridSchema().fields).toEqual([]); + }); +}); diff --git a/packages/plugin-list/src/__tests__/htmlTierListViewDefaultColumns-6598.test.tsx b/packages/plugin-list/src/__tests__/htmlTierListViewDefaultColumns-6598.test.tsx new file mode 100644 index 0000000000..6b8e5d30f3 --- /dev/null +++ b/packages/plugin-list/src/__tests__/htmlTierListViewDefaultColumns-6598.test.tsx @@ -0,0 +1,120 @@ +/** + * 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. + * + * The reported page, end to end: a `kind:'html'` page whose `` must + * render DATA COLUMNS and not just the index column (objectui#6598). + * + * The production report (objectstack#12649, hotcrm, promo-video recon) tried + * eight spellings of `columns` on an html-kind page and got the same picture + * every time: the row count, the filter/group/sort toolbar, and "no data + * columns at all — only the index column". Two of the eight were the grammar + * cases objectui#6614/#6669 legalised (single-quoted arrays, bare identifier + * object keys) and they render today; this file pins the one that was NOT a + * grammar question and was the only one that failed with **zero diagnostics** — + * declaring no `columns` at all and expecting the block's defaults. + * + * ## Why this file goes through the REAL grid + * + * `ListView.unauthoredColumnProjection-6598.test.tsx` pins the handoff — what + * this component feeds the child grid — against a stub. That is the mechanism, + * and it is not the symptom: the symptom is a `` count, and it emerges from + * ListView and ObjectGrid disagreeing about how "unauthored" is spelled. A stub + * cannot see a disagreement it is standing in for. So this file mounts the real + * page renderer, the real html-tier compile, the real `list-view` registration + * and the real `object-grid`, and counts headers. + * + * Registered in `heavyDomTests` for the setup's `@object-ui/plugin-grid` + * side-effect registration, the same route + * `ListView.crossPageSelectAll.test.tsx` takes — a plugin-list → plugin-grid + * import would be the heavier change. + * + * ⚠️ The assertions are deliberately "which business columns are present", not + * an exact header list. When the grid owns the fetch it derives defaults from + * the object schema (hidden and readonly system-managed fields dropped); when a + * host like ListView owns it, the grid takes its inline-data branch and derives + * them from the row payload's keys instead. That precedence is a separate, + * open question against `packages/plugin-grid` and it moves the exact list — + * it must not be able to move whether the page shows data columns at all. + */ +import { describe, it, expect } from 'vitest'; +import { render, waitFor } from '@testing-library/react'; +import React from 'react'; +import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react'; +import '../index'; + +const OBJECT = 'opportunity'; + +const dataSource = { + find: async () => ({ + data: [ + { id: 'o-1', name: 'Acme expansion', amount: 1000, stage: 'new' }, + { id: 'o-2', name: 'Globex renewal', amount: 2000, stage: 'won' }, + ], + total: 23, + hasMore: true, + }), + findOne: async () => null, + create: async () => ({}), + update: async () => ({}), + delete: async () => ({}), + count: async () => 23, + getObjectSchema: async (name: string) => ({ + name, + label: 'Opportunity', + fields: { + id: { type: 'text', label: 'Id', hidden: true }, + name: { type: 'text', label: 'Opportunity Name' }, + stage: { type: 'text', label: 'Stage' }, + amount: { type: 'currency', label: 'Amount' }, + }, + }), + getObjects: async () => [], + onMutation: () => () => {}, +} as any; + +async function renderHtmlPage(source: string) { + const { container } = render( + + + , + ); + await waitFor(() => expect(container.querySelector('table')).toBeTruthy()); + return container; +} + +const headersOf = (container: HTMLElement) => + Array.from(container.querySelectorAll('th')).map((th) => (th.textContent || '').trim()); + +describe("kind:'html' page — renders data columns (#6598)", () => { + it('renders the object\'s default columns when the author declared none', async () => { + const container = await renderHtmlPage(``); + + await waitFor(() => expect(headersOf(container).length).toBeGreaterThan(1)); + const headers = headersOf(container); + // Before: exactly ['#'] — the reporter's "only the index column", with no + // diagnostic anywhere, because ListView spelled "unauthored" as `fields: []` + // and that pinned the grid's projection at zero. + expect(headers).toContain('Opportunity Name'); + expect(headers).toContain('Amount'); + // The page still fetched all along — rows were never the problem. + expect(container.textContent).toContain('23 records'); + }); + + it('renders exactly the authored columns when the author declared them', async () => { + // The single-quoted array is the spelling every JSX author reaches for + // first, and the one objectui#6669 made legal; it is the control that keeps + // the default above from swallowing an authored projection. + const container = await renderHtmlPage( + ``, + ); + + await waitFor(() => expect(headersOf(container).length).toBeGreaterThan(1)); + const headers = headersOf(container); + expect(headers).toEqual(['#', 'Opportunity Name', 'Amount']); + expect(headers).not.toContain('Stage'); + }); +}); diff --git a/vitest.config.mts b/vitest.config.mts index 2b3869ebc6..b8dce7865b 100644 --- a/vitest.config.mts +++ b/vitest.config.mts @@ -126,6 +126,12 @@ const heavyDomTests = [ // side-effect registration, and taking a plugin-list -> plugin-grid // dependency to import it directly would be the heavier change. 'packages/plugin-list/src/__tests__/ListView.crossPageSelectAll.test.tsx', + // objectui#6598 — the reported html-kind page, end to end over the REAL + // object-grid. The defect is ListView and ObjectGrid disagreeing about how + // "the author declared no columns" is spelled, so a stub grid (what the + // sibling handoff pin registers) is standing in for one side of the + // disagreement and cannot see it. Same reason, same route as the entry above. + 'packages/plugin-list/src/__tests__/htmlTierListViewDefaultColumns-6598.test.tsx', ]; export default defineConfig({