diff --git a/.changeset/30535-non-grid-bulk-bar-gate.md b/.changeset/30535-non-grid-bulk-bar-gate.md new file mode 100644 index 00000000000..95705baae13 --- /dev/null +++ b/.changeset/30535-non-grid-bulk-bar-gate.md @@ -0,0 +1,13 @@ +--- +'@object-ui/plugin-list': patch +'@object-ui/plugin-grid': patch +--- + +fix(plugin-list): a list's selection bar over a non-grid view (kanban, calendar, gallery, …) draws a declared bulk action only for a user who holds its `requiredPermissions` and only when its `visible` admits a selected record, through the grid bar's own gate (#30535) + +Clause-②: yes (widening) — `@object-ui/plugin-grid` exports `useBulkActionItems` (with the `BulkActionItems` and `LegacyBulkActionItem` types), the grid selection bar's gate, so `@object-ui/plugin-list` reuses it instead of a second copy. +Metadata-protocol: none + +- **Before.** The bar ListView draws itself for every view but the grid folded `bulkActions` through the grid's `resolveBulkActions` and kept only each action's label. An action declaring `requiredPermissions` drew for every user, and one declaring `visible` drew for every selection, while the grid's bar hid both. A name repeated in `bulkActions` drew two buttons under one React key. +- **Now.** The bar passes the resolved actions through `useBulkActionItems`, the hook the grid's bar and its right-click menu read: an action whose `requiredPermissions` the user does not hold is not drawn (unknown capabilities fail open, as on the grid), an action switched off is not drawn, and `visible` is evaluated once per selected record, drawing the action when at least one selected record passes. A repeated name draws one button. The development warning for an object whose schema could not be read now names the component it came from (`ListView` or `ObjectGrid`) instead of "the grid". +- **Remedy.** Nothing to change in metadata. A user who should see a gated action needs the capability it declares. diff --git a/ui/packages/plugin-grid/src/__tests__/undeclaredActionNames.test.ts b/ui/packages/plugin-grid/src/__tests__/undeclaredActionNames.test.ts index df1012b27eb..ccf5356b584 100644 --- a/ui/packages/plugin-grid/src/__tests__/undeclaredActionNames.test.ts +++ b/ui/packages/plugin-grid/src/__tests__/undeclaredActionNames.test.ts @@ -69,6 +69,16 @@ describe('describeUndeclaredActionNames', () => { expect(list).toContain('the selection bar draws no button'); }); + it('names the component from the address in the unread prescription, not "the grid" (objectstack#30535)', () => { + const grid = describeUndeclaredActionNames('bulkActions', ['a'], ADDRESS, 'unread'); + expect(grid).toContain('Give the ObjectGrid a data source that answers getObjectSchema'); + const list = describeUndeclaredActionNames('bulkActions', ['a'], { + component: 'ListView', blockType: 'calendar', objectName: 'lead', + }, 'unread'); + expect(list).toContain('Give the ListView a data source that answers getObjectSchema'); + expect(list).not.toContain('the grid'); + }); + it('says nothing for no names', () => { expect(describeUndeclaredActionNames('bulkActions', [], ADDRESS)).toBeNull(); }); diff --git a/ui/packages/plugin-grid/src/components/BulkActionBar.tsx b/ui/packages/plugin-grid/src/components/BulkActionBar.tsx index a0ef236e1c6..1a1672d92a1 100644 --- a/ui/packages/plugin-grid/src/components/BulkActionBar.tsx +++ b/ui/packages/plugin-grid/src/components/BulkActionBar.tsx @@ -87,7 +87,8 @@ export interface BulkActionItems { export function useBulkActionItems(input: { selectedRows: any[]; actions: string[] | undefined; - actionDefs?: BulkActionDef[]; + /** Read, never mutated: `resolveBulkActions` hands its fold over as a readonly list. */ + actionDefs?: readonly BulkActionDef[]; objectFields?: unknown; }): BulkActionItems { const { selectedRows, actions, actionDefs, objectFields } = input; diff --git a/ui/packages/plugin-grid/src/index.tsx b/ui/packages/plugin-grid/src/index.tsx index 25262ef21be..9e0cd234830 100644 --- a/ui/packages/plugin-grid/src/index.tsx +++ b/ui/packages/plugin-grid/src/index.tsx @@ -32,9 +32,11 @@ export { useGroupedData } from './useGroupedData'; export { GroupRow } from './GroupRow'; export { RowActionMenu, formatActionLabel } from './components/RowActionMenu'; export { BulkActionBar } from './components/BulkActionBar'; -// The bare-name fold and its development warning, for the one other surface -// that draws a selection bar of its own: plugin-list's ListView over a -// non-grid view (objectstack#30394). One rule, one copy. +// The bare-name fold, its development warning and the bar's own gate (the +// capability check, the switch-off and the per-record `visible`), for the one +// other surface that draws a selection bar of its own: plugin-list's ListView +// over a non-grid view (objectstack#30394, objectstack#30535). One rule, one copy. +export { useBulkActionItems } from './components/BulkActionBar'; export { resolveBulkActions } from './resolveBulkActions'; export { useUndeclaredActionNamesWarning } from './undeclaredActionNames'; export { useCellClipboard } from './useCellClipboard'; @@ -59,7 +61,7 @@ export type { ImportWizardProps, ImportResult } from './ImportWizard'; export type { GroupEntry, UseGroupedDataResult, AggregationType, GroupAggregationConfig, AggregationResult } from './useGroupedData'; export type { GroupRowProps } from './GroupRow'; export type { RowActionMenuProps } from './components/RowActionMenu'; -export type { BulkActionBarProps } from './components/BulkActionBar'; +export type { BulkActionBarProps, BulkActionItems, LegacyBulkActionItem } from './components/BulkActionBar'; export type { ActionLabelResolver, NamedActionDef } from './resolveBulkActions'; export type { UndeclaredActionNameKey, diff --git a/ui/packages/plugin-grid/src/undeclaredActionNames.ts b/ui/packages/plugin-grid/src/undeclaredActionNames.ts index 3602646c8b8..baf3f36dd5d 100644 --- a/ui/packages/plugin-grid/src/undeclaredActionNames.ts +++ b/ui/packages/plugin-grid/src/undeclaredActionNames.ts @@ -108,7 +108,7 @@ export function describeUndeclaredActionNames( return `[ObjectUI] ${describeComponent(address.component)} ${key}: ${describeAddress(address)} — the object's schema could not ` + `be read (no getObjectSchema on the data source, or the read failed), so its declared actions ` + `are unknown and ${SURFACE[key]} for ${count}: ${listed}.\n` - + `${rule} Give the grid a data source that answers getObjectSchema, or check why the read failed.`; + + `${rule} Give the ${describeComponent(address.component)} a data source that answers getObjectSchema, or check why the read failed.`; } const count = names.length === 1 ? '1 name matches' : `${names.length} names match`; return `[ObjectUI] ${describeComponent(address.component)} ${key}: ${describeAddress(address)} — ${count} no action the object ` diff --git a/ui/packages/plugin-list/src/ListView.tsx b/ui/packages/plugin-list/src/ListView.tsx index a8ee9a7179b..2675189af30 100644 --- a/ui/packages/plugin-list/src/ListView.tsx +++ b/ui/packages/plugin-list/src/ListView.tsx @@ -54,7 +54,7 @@ import { platformProvisionsStorage, resolveInjectedSystemColumns, resolveSearchF import { SystemFieldName } from '@objectstack/spec/system'; import { usePermissions } from '@object-ui/permissions'; import { RecordDetailPanel } from '@object-ui/plugin-detail'; -import { resolveBulkActions, useUndeclaredActionNamesWarning } from '@object-ui/plugin-grid'; +import { resolveBulkActions, useBulkActionItems, useUndeclaredActionNamesWarning } from '@object-ui/plugin-grid'; /** * The `case 'map'` branch below builds an `object-map` schema by flattening @@ -2558,9 +2558,25 @@ export const ListView = React.forwardRef(({ * below. Before this fold every member drew a button, so a name the object * never declared, an action a disabled package withholds included, showed a * button that only toasted "not executable" when pressed. + * + * [objectstack#30535] A declared action then passes the grid bar's OWN gate, + * `useBulkActionItems`, before it draws: its `requiredPermissions` (ADR-0066 + * D4, the caller's held capabilities), its activation switch, and its + * `visible`, evaluated once per selected record and offered when at least + * one record passes. The fold carried the resolved definition all along; the + * bar read only its label, so a gated action drew for every user and every + * selection. A repeated name draws once, as the fold surfaces it once. */ - const nonGridBulkBar = React.useMemo(() => { - const names = (permittedBulkActions ?? []).filter((a: unknown) => typeof a === 'string' && a !== '') as string[]; + const nonGridBulkFold = React.useMemo(() => { + const seen = new Set(); + const names = ((permittedBulkActions ?? []) as unknown[]).filter((a): a is string => { + if (typeof a !== 'string' || a === '') return false; + // The built-in `delete` is one button whatever its spelling's case. + const key = a.toLowerCase() === 'delete' ? 'delete' : a; + if (seen.has(key)) return false; + seen.add(key); + return true; + }); const { defs, undeclared } = resolveBulkActions({ bulkActions: names.filter((a) => a.toLowerCase() !== 'delete'), objectActions: objectDef?.actions, @@ -2569,13 +2585,22 @@ export const ListView = React.forwardRef(({ ? resolveActionLabel(schema.objectName, actionName, fallback) : fallback, }); - const labels = new Map(defs.map((def) => [def.name, def.label])); + return { names, defs, undeclared }; + }, [permittedBulkActions, objectDef, schema.objectName, resolveActionLabel]); + const nonGridBulkItems = useBulkActionItems({ + selectedRows, + actions: undefined, + actionDefs: nonGridBulkFold.defs, + objectFields: objectDef?.fields, + }); + const nonGridBulkBar = React.useMemo(() => { + const labels = new Map(nonGridBulkItems.defs.map((def) => [def.name, def.label])); // Authored order, the built-in `delete` where the author put it. - const drawn = names + const drawn = nonGridBulkFold.names .filter((a) => a.toLowerCase() === 'delete' || labels.has(a)) .map((name) => ({ name, label: labels.get(name) })); - return { drawn, undeclared }; - }, [permittedBulkActions, objectDef, schema.objectName, resolveActionLabel]); + return { drawn, undeclared: nonGridBulkFold.undeclared }; + }, [nonGridBulkFold, nonGridBulkItems]); // The grid says its own (ObjectGrid warns for the names it leaves undrawn), // so this one speaks only while a non-grid view is on screen, and only once // the object's schema read has settled. A list with an object but no schema diff --git a/ui/packages/plugin-list/src/__tests__/ListView.nonGridBulkGate-30535.test.tsx b/ui/packages/plugin-list/src/__tests__/ListView.nonGridBulkGate-30535.test.tsx new file mode 100644 index 00000000000..5c4cef86a21 --- /dev/null +++ b/ui/packages/plugin-list/src/__tests__/ListView.nonGridBulkGate-30535.test.tsx @@ -0,0 +1,160 @@ +/** + * 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. + */ + +/** + * objectstack#30535 — the NON-grid selection bar (kanban / calendar / gallery + * / …) applies the grid bar's own gate to a declared bulk action, through the + * grid's own `useBulkActionItems`: + * + * - an action whose `requiredPermissions` the caller does not hold is not + * drawn (ADR-0066 D4; unknown capabilities fail open, as on the grid); + * - `visible` is evaluated once per selected record, and the action is drawn + * when at least one selected record passes; + * - a name repeated in `bulkActions` draws one button. + * + * The grid's half is pinned in plugin-grid's `bulkActionCapabilityGate.test.tsx` + * and `bulk-action-visible.test.tsx`; this file pins the same gate on the bar + * ListView renders for every other view. + */ + +import React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, waitFor, screen, cleanup } from '@testing-library/react'; +import { ComponentRegistry } from '@object-ui/core'; +import { ActionProvider, SchemaRendererProvider } from '@object-ui/react'; +import type { ListViewSchema } from '@object-ui/types'; +import { ListView } from '../ListView'; + +type Row = { id: string; name: string; status: string }; + +function makeDataSource(objectName: string, rows: Row[], actions: unknown[]) { + return { + find: vi.fn().mockResolvedValue(rows.map(r => ({ ...r }))), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ + name: objectName, + fields: { id: { type: 'text' }, name: { type: 'text' }, status: { type: 'text' } }, + actions, + }), + }; +} + +/** A gallery stand-in that selects `rows` as soon as it mounts. */ +function registerSelectingGallery(rows: Row[]) { + ComponentRegistry.register('object-gallery', (props: any) => { + const onRowSelect = props.onRowSelect; + React.useEffect(() => { + onRowSelect?.(rows); + }, [onRowSelect]); + return
; + }); +} + +function renderGallery( + objectName: string, + bulkActions: string[], + ds: ReturnType, + user?: unknown, +) { + const schema: ListViewSchema = { + type: 'list-view', + objectName, + viewType: 'gallery', + fields: ['name'], + bulkActions, + }; + const tree = ( + + + + ); + return render(user === undefined ? tree : {tree}); +} + +const OPEN: Row = { id: 'l1', name: 'Lead One', status: 'open' }; +const CLOSED: Row = { id: 'l2', name: 'Lead Two', status: 'closed' }; + +const ACTIONS = [ + { name: 'tag_lead', label: 'Tag' }, + { name: 'push_down', label: 'Push Down', requiredPermissions: ['plan.push'] }, + { name: 'reopen', label: 'Reopen', visible: "record.status == 'closed'" }, +]; + +describe('ListView non-grid bulk bar applies the grid bar gate (objectstack#30535)', () => { + let prevGallery: ReturnType; + + beforeEach(() => { + prevGallery = ComponentRegistry.get('object-gallery'); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + cleanup(); + vi.restoreAllMocks(); + if (prevGallery) ComponentRegistry.register('object-gallery', prevGallery); + else ComponentRegistry.unregister('object-gallery'); + }); + + it('does not draw an action whose requiredPermissions the caller lacks', async () => { + registerSelectingGallery([OPEN]); + const ds = makeDataSource('lead_g1', [OPEN], ACTIONS); + renderGallery('lead_g1', ['tag_lead', 'push_down'], ds, { id: 'u1', systemPermissions: ['setup.access'] }); + + await screen.findByTestId('bulk-action-tag_lead'); + expect(screen.queryByTestId('bulk-action-push_down')).toBeNull(); + }); + + it('draws it when the caller holds every declared capability', async () => { + registerSelectingGallery([OPEN]); + const ds = makeDataSource('lead_g2', [OPEN], ACTIONS); + renderGallery('lead_g2', ['tag_lead', 'push_down'], ds, { id: 'u1', systemPermissions: ['plan.push'] }); + + expect(await screen.findByTestId('bulk-action-push_down')).toHaveTextContent('Push Down'); + }); + + it('draws it when the caller capabilities are unknown (fail open, as the grid)', async () => { + registerSelectingGallery([OPEN]); + const ds = makeDataSource('lead_g3', [OPEN], ACTIONS); + renderGallery('lead_g3', ['push_down'], ds); + + expect(await screen.findByTestId('bulk-action-push_down')).toBeInTheDocument(); + }); + + it('evaluates visible per selected record: no passing record, no button', async () => { + registerSelectingGallery([OPEN]); + const ds = makeDataSource('lead_v1', [OPEN], ACTIONS); + renderGallery('lead_v1', ['tag_lead', 'reopen'], ds); + + await screen.findByTestId('bulk-action-tag_lead'); + expect(screen.queryByTestId('bulk-action-reopen')).toBeNull(); + }); + + it('evaluates visible per selected record: one passing record draws the button', async () => { + registerSelectingGallery([OPEN, CLOSED]); + const ds = makeDataSource('lead_v2', [OPEN, CLOSED], ACTIONS); + renderGallery('lead_v2', ['tag_lead', 'reopen'], ds); + + expect(await screen.findByTestId('bulk-action-reopen')).toHaveTextContent('Reopen'); + }); + + it('draws a repeated name once, with no duplicate React key', async () => { + registerSelectingGallery([OPEN]); + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + const ds = makeDataSource('lead_d1', [OPEN], ACTIONS); + renderGallery('lead_d1', ['tag_lead', 'tag_lead', 'delete', 'delete'], ds); + + await screen.findByTestId('bulk-action-tag_lead'); + await waitFor(() => expect(screen.getAllByTestId('bulk-action-tag_lead')).toHaveLength(1)); + expect(screen.getAllByTestId('bulk-action-delete')).toHaveLength(1); + const keyErrors = error.mock.calls.filter(c => String(c[0]).includes('same key')); + expect(keyErrors).toEqual([]); + }); +});