diff --git a/.changeset/13107-grid-select-namefield.md b/.changeset/13107-grid-select-namefield.md new file mode 100644 index 0000000000..2e04d061db --- /dev/null +++ b/.changeset/13107-grid-select-namefield.md @@ -0,0 +1,10 @@ +--- +'@object-ui/plugin-grid': patch +--- + +The bulk-action confirm step names each affected record by the object's declared `nameField`, even when no column of the view shows it, and never by another text field (objectui#13107). + +- **The grid asks for the declared `nameField`.** A grid that runs its own query (a standalone `object-grid`, or a grouped grid that groups on the server) now puts the object's `nameField` in its `$select`, a formula field included. It is fetched, not drawn: no column is added. It passes the same two checks as the grid's other extra fields: the object must declare it, and the viewer must be allowed to read it. +- **No other field stands in for the declared name.** For an object that declares a `nameField`, a row without that value is listed as `Record #ID`. Before, the step fell through to a guessed field: on a contact list whose columns were first name, last name and job title, it listed the contacts' job titles. Objects that declare no `nameField` are named as before. + +Nothing is added to the package entry: no export, prop or type member. diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 4c2fbdbb62..c025fe82f5 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -61,7 +61,7 @@ import { toast, } from '@object-ui/components'; import { usePullToRefresh } from '@object-ui/mobile'; -import { resolveConditionalFormatting, isDeadlineSettled, leadWithNameField, buildExpandFields, buildExportFileName, columnIdentity, collectPredicateFieldRefs, collectGroupingFieldRefs, listViewPredicates, resolveAffordance, resolveFieldAffordance, type SchemaLike, isProjectableField, isExpandableFieldType, isUnmaterializedFieldType, readObjectSortability, isPlatformSortableField, filterPlatformSortableSort, toFilterNodeSafely, filterRefusalSubject, FilterOperatorError, convertSortToQueryParams, normalizeSortEntries, type QuerySortEntry, ROW_HEIGHT_TO_DENSITY_MODE, resolveRecordSourceConfig, resolveRecordSourceObjectName, resolveFilterPlaceholders, partitionRowsByPredicate, buildCategoryOrder, buildCategoryRank, type FilterTokenScope } from '@object-ui/core'; +import { resolveConditionalFormatting, isDeadlineSettled, leadWithNameField, declaredNameField, buildExpandFields, buildExportFileName, columnIdentity, collectPredicateFieldRefs, collectGroupingFieldRefs, listViewPredicates, resolveAffordance, resolveFieldAffordance, type SchemaLike, isProjectableField, isExpandableFieldType, isUnmaterializedFieldType, readObjectSortability, isPlatformSortableField, filterPlatformSortableSort, toFilterNodeSafely, filterRefusalSubject, FilterOperatorError, convertSortToQueryParams, normalizeSortEntries, type QuerySortEntry, ROW_HEIGHT_TO_DENSITY_MODE, resolveRecordSourceConfig, resolveRecordSourceObjectName, resolveFilterPlaceholders, partitionRowsByPredicate, buildCategoryOrder, buildCategoryRank, type FilterTokenScope } from '@object-ui/core'; import { usePermissions } from '@object-ui/permissions'; import { RECORD_OVERLAY_DEFAULT_WIDTH, @@ -2857,9 +2857,23 @@ export const ObjectGrid: React.FC = ({ .filter((f) => isProjectableField(f, declared as Record)) .filter((f) => passesProjectionGate(f)) : []; + // [objectui#13107] The object's DECLARED name field. The bulk + // confirm step names each affected record from the row this + // projection fetched (`getRecordDisplayName`, ADR-0079), so a + // `nameField` no column shows arrived absent and the step listed + // another text field in its place (a contact's job title). Asked + // by name whatever its type: the server serves a formula field + // like any other. Fetched, never drawn: it joins the projection, + // not the columns. The same two gates as the harvests above. + const declaredName = declaredNameField(resolvedSchema); + const nameFields = declared && typeof declaredName === 'string' && declaredName !== '' + ? [declaredName] + .filter((f) => isProjectableField(f, declared as Record)) + .filter((f) => passesProjectionGate(f)) + : []; // A UNION, never an append: a grouping field that IS also a column // must not produce a duplicate `$select` entry. - const extraFields = [...predicateFields, ...groupingFields, ...concurrencyFields, ...rowColorFields, ...deadlineFields, ...referenceViaFields]; + const extraFields = [...predicateFields, ...groupingFields, ...concurrencyFields, ...rowColorFields, ...deadlineFields, ...referenceViaFields, ...nameFields]; const withHarvestedFields = (list: any[]): any[] => { if (extraFields.length === 0) return list; const names = new Set(list.map((f: any) => columnIdentity(f))); diff --git a/packages/plugin-grid/src/__tests__/bulkConfirmNameField-13107.test.tsx b/packages/plugin-grid/src/__tests__/bulkConfirmNameField-13107.test.tsx new file mode 100644 index 0000000000..3fc2c75466 --- /dev/null +++ b/packages/plugin-grid/src/__tests__/bulkConfirmNameField-13107.test.tsx @@ -0,0 +1,273 @@ +/** + * 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#13107 — the bulk confirm step names a record by the object's + * declared `nameField` even when no column shows it, and never by another + * text field. + * + * Measured on HotCRM's All Contacts: `crm_contact` declares + * `nameField: 'full_name'` (a formula), the view's columns are `first_name`, + * `last_name`, `title`, and the confirm step listed the contacts' job titles. + * The step reads the GRID ROW, the row carried only the projected columns, and + * the resolver's derive steps then picked `title`. + * + * Two halves, both pinned here on the real ObjectGrid + BulkActionDialog: + * - the grid's own `$select` asks for the declared `nameField` (a formula + * included), so the row carries it and the step lists it; the measured + * list groups by account, so a server-grouped grid's row pages are pinned + * too; + * - when the declared value still is not on the row, the step shows the + * resolver's `Record #` floor, never a derived field's value. + * Controls: an object whose `nameField` is a column (the lead list) and an + * object that declares no `nameField` (the derive ladder) are unchanged. + * + * The data source honours `$select` the way a server does: a row carries the + * selected keys only, so a field left out of the projection is really absent. + */ + +import { describe, it, expect, vi, beforeAll, beforeEach, afterEach } from 'vitest'; +import { render, screen, waitFor, fireEvent, cleanup, within } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +/** Stable stub identity: `ObjectGrid` carries `perms` in memo dep arrays. */ +const { permsStub, state } = vi.hoisted(() => { + const state: { denied: string[] } = { denied: [] }; + return { + state, + permsStub: { + isLoaded: true, + checkField: (_object: string, field: string, action: string) => + action === 'read' ? !state.denied.includes(field) : true, + check: () => ({ allowed: true }), + getFieldPermissions: () => [], + getRowFilter: () => undefined, + getObjectApiOperations: () => undefined, + roles: [], + userId: null, + systemPermissions: undefined, + hasCapabilities: () => true, + can: () => true, + cannot: () => false, + }, + }; +}); + +vi.mock('@object-ui/permissions', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, usePermissions: () => permsStub as any }; +}); + +import { ObjectGrid } from '../ObjectGrid'; +import { registerAllFields } from '@object-ui/fields'; +import { ActionProvider } from '@object-ui/react'; + +registerAllFields(); + +beforeAll(() => { + if (!Element.prototype.scrollIntoView) { + Element.prototype.scrollIntoView = vi.fn() as any; + } +}); + +beforeEach(() => { + state.denied = []; +}); + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); +}); + +const ADD_TO_CAMPAIGN = { + name: 'add_to_campaign', + label: 'Add to Campaign', + type: 'api', + target: '/api/v1/contacts/enroll', +}; + +/** HotCRM's contact shape: a formula `nameField` beside a job `title`. */ +const CONTACT_FIELDS = { + id: { type: 'text' }, + full_name: { type: 'formula', label: 'Full Name' }, + first_name: { type: 'text', label: 'First Name' }, + last_name: { type: 'text', label: 'Last Name' }, + title: { type: 'text', label: 'Title' }, +}; +const CONTACT_ROWS = [ + { id: 'c1', full_name: 'John Smith', first_name: 'John', last_name: 'Smith', title: 'VP of Engineering' }, + { id: 'c2', full_name: 'Marcus Reed', first_name: 'Marcus', last_name: 'Reed', title: 'Director of Commercial Systems' }, +]; +/** The All Contacts view: `full_name` is not a column. One array, one identity. */ +const CONTACT_COLUMNS = [{ field: 'first_name' }, { field: 'last_name' }, { field: 'title' }]; + +function renderGrid(opts: { + objectName: string; + fields: Record; + rows: Array>; + columns: unknown[]; + objectExtra?: Record; + /** Keys the server leaves off every row, selected or not. */ + withheld?: string[]; + /** A view grouping (the measured All Contacts list groups by account). */ + grouping?: Record; +}) { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: true, + status: 200, + statusText: 'OK', + headers: { get: () => 'application/json' }, + json: async () => ({ success: true }), + })); + const dataSource = { + // Present, so a grouped grid groups on the server and pages its own rows. + queryGroupHeaders: vi.fn(async () => [{ last_name: null, count: opts.rows.length }]), + find: vi.fn(async (_object: string, params: Record) => { + const select = params?.$select as string[] | undefined; + const data = opts.rows.map((row) => { + const keys = (select ?? Object.keys(row)).filter((k) => k in row && !(opts.withheld ?? []).includes(k)); + return Object.fromEntries(keys.map((k) => [k, row[k]])); + }); + return { data, total: data.length, hasMore: false, pageSize: 50 }; + }), + getObjectSchema: async (name: string) => ({ + name, + fields: opts.fields, + actions: [ADD_TO_CAMPAIGN], + ...opts.objectExtra, + }), + }; + render( + + + , + ); + return dataSource; +} + +const GROUPED = { fields: [{ field: 'last_name', order: 'asc', collapsed: false }] }; + +/** The last `$select` the grid sent, sorted. */ +const lastSelect = (ds: { find: { mock: { calls: any[][] } } }) => + [...((ds.find.mock.calls.at(-1)?.[1]?.$select as string[] | undefined) ?? [])].sort(); + +/** Select every row, click the bulk button, and return the confirm step's list. */ +async function openConfirm(firstCellText: string): Promise { + await waitFor(() => expect(screen.getByText(firstCellText)).toBeInTheDocument()); + await waitFor(() => expect(document.querySelector('thead [role="checkbox"]')).toBeTruthy()); + fireEvent.click(document.querySelector('thead [role="checkbox"]') as HTMLElement); + fireEvent.click(await screen.findByTestId('bulk-action-add_to_campaign')); + await screen.findByRole('button', { name: 'Run' }); + return within(screen.getByRole('dialog')).getByRole('list'); +} + +const listed = (list: HTMLElement) => + within(list).getAllByRole('listitem').map((li) => li.textContent?.replace(/^•\s*/, '').trim()); + +describe('bulk confirm step names a record by its declared nameField when no column shows it (objectui#13107)', () => { + it('(a) the grid asks for a formula nameField that is not a column, and the step lists its values', async () => { + const ds = renderGrid({ + objectName: 'crm_contact', + fields: CONTACT_FIELDS, + rows: CONTACT_ROWS, + columns: CONTACT_COLUMNS, + objectExtra: { nameField: 'full_name' }, + }); + const list = await openConfirm('VP of Engineering'); + expect(lastSelect(ds)).toEqual(['first_name', 'full_name', 'id', 'last_name', 'title']); + expect(listed(list)).toEqual(['John Smith', 'Marcus Reed']); + // Fetched, never drawn: no column header names it. + expect(screen.queryByRole('columnheader', { name: /Full Name/ })).toBeNull(); + }); + + it('(a) a server-grouped grid (the measured list groups) asks for it on every group row page', async () => { + const ds = renderGrid({ + objectName: 'crm_contact', + fields: CONTACT_FIELDS, + rows: CONTACT_ROWS, + columns: CONTACT_COLUMNS, + objectExtra: { nameField: 'full_name' }, + grouping: GROUPED, + }); + await waitFor(() => expect(ds.queryGroupHeaders).toHaveBeenCalled()); + await waitFor(() => expect(screen.getByText('VP of Engineering')).toBeInTheDocument()); + const selects = ds.find.mock.calls.map(([, params]) => [...((params?.$select as string[] | undefined) ?? [])].sort()); + expect(selects.length).toBeGreaterThan(0); + for (const select of selects) { + expect(select).toEqual(['first_name', 'full_name', 'id', 'last_name', 'title']); + } + }); + + it('(b) a nameField value absent from the answer reads `Record #id`, never the job title', async () => { + const ds = renderGrid({ + objectName: 'crm_contact', + fields: CONTACT_FIELDS, + rows: CONTACT_ROWS, + columns: CONTACT_COLUMNS, + objectExtra: { nameField: 'full_name' }, + withheld: ['full_name'], + }); + const list = await openConfirm('VP of Engineering'); + expect(lastSelect(ds)).toContain('full_name'); + expect(listed(list)).toEqual(['Record #c1', 'Record #c2']); + }); + + it('(b) a nameField the principal may not read is not asked for, and the step reads `Record #id`', async () => { + state.denied = ['full_name']; + const ds = renderGrid({ + objectName: 'crm_contact', + fields: CONTACT_FIELDS, + rows: CONTACT_ROWS, + columns: CONTACT_COLUMNS, + objectExtra: { nameField: 'full_name' }, + }); + const list = await openConfirm('VP of Engineering'); + expect(lastSelect(ds)).toEqual(['first_name', 'id', 'last_name', 'title']); + expect(listed(list)).toEqual(['Record #c1', 'Record #c2']); + }); + + it('CONTROL (c): a nameField that is a column (the lead list) is sent once and listed', async () => { + const ds = renderGrid({ + objectName: 'crm_lead', + fields: { + id: { type: 'text' }, + display_title: { type: 'formula', label: 'Display Title' }, + company: { type: 'text', label: 'Company' }, + }, + rows: [{ id: 'l1', display_title: 'Mira Costa - Atlas Construction', company: 'Atlas Construction' }], + columns: [{ field: 'display_title' }, { field: 'company' }], + objectExtra: { nameField: 'display_title' }, + }); + const list = await openConfirm('Mira Costa - Atlas Construction'); + expect(lastSelect(ds)).toEqual(['company', 'display_title', 'id']); + expect(listed(list)).toEqual(['Mira Costa - Atlas Construction']); + }); + + it('CONTROL (c): an object that declares no nameField keeps the derive ladder, and asks for nothing more', async () => { + const ds = renderGrid({ + objectName: 'crm_contact', + fields: CONTACT_FIELDS, + rows: CONTACT_ROWS, + columns: CONTACT_COLUMNS, + }); + const list = await openConfirm('VP of Engineering'); + expect(lastSelect(ds)).toEqual(['first_name', 'id', 'last_name', 'title']); + expect(listed(list)).toEqual(['VP of Engineering', 'Director of Commercial Systems']); + }); +}); diff --git a/packages/plugin-grid/src/components/BulkActionDialog.tsx b/packages/plugin-grid/src/components/BulkActionDialog.tsx index 0e33d2d0ec..15c5e833e1 100644 --- a/packages/plugin-grid/src/components/BulkActionDialog.tsx +++ b/packages/plugin-grid/src/components/BulkActionDialog.tsx @@ -28,7 +28,7 @@ import { getLazyFieldWidget } from '@object-ui/fields'; // Their TSDoc carries the rationale that used to be repeated in each copy. // See `paramNeedsDependentValues` below for why this surface ORs them; // ⛔ never copied into a local literal, and ⛔ never merged into one set. -import { CASCADE_OPTION_WIDGET_TYPES, EXPANDABLE_FIELD_TYPES, getRecordDisplayName } from '@object-ui/core'; +import { CASCADE_OPTION_WIDGET_TYPES, EXPANDABLE_FIELD_TYPES, declaredNameField, getRecordDisplayName, recordDisplayValueAt } from '@object-ui/core'; import type { BulkActionDef, BulkActionParam } from '@object-ui/types'; import { useBulkExecutor, type BulkExecutorOptions, type BulkResult } from '../hooks/useBulkExecutor'; import { hasMultiValueShape, type MultiValueFieldDef } from '../hooks/multiValueFields'; @@ -40,6 +40,25 @@ import { } from './bulkParamToField'; import { unresolvedFieldOf } from '../resolveBulkActions'; +/** + * The name the confirm step lists an affected record by (objectui#13107). + * + * The resolver's own ladder (`getRecordDisplayName`, ADR-0079) for an object + * that declares no `nameField`. For one that does, the declared field or the + * resolver's `Record #` floor, never a derived one: when the row lacks + * the declared value (a projection that did not ask for it, a value the + * principal may not read), the derive steps would name the record by another + * text field, and a contact's job title is not the record a write targets. + * The floor is the resolver's own, reached with nothing left to derive from. + */ +function affectedRecordName(objectDef: unknown, row: Record, fallback: string): string { + const nameField = declaredNameField(objectDef); + if (typeof nameField !== 'string' || nameField === '' || recordDisplayValueAt(row, nameField)) { + return getRecordDisplayName(objectDef, row, { fallback }); + } + return getRecordDisplayName(undefined, { id: row.id ?? row._id }, { fallback, deriveFromRecordKeys: false }); +} + export interface BulkActionDialogProps { /** The action being executed. */ def: BulkActionDef | null; @@ -475,9 +494,7 @@ export const BulkActionDialog: React.FC = ({
    {previewRows.map((r, i) => (
  • - • {getRecordDisplayName(objectDef, r, { - fallback: t('grid.bulk.rowFallback', { index: i + 1, defaultValue: `Row ${i + 1}` }), - })} + • {affectedRecordName(objectDef, r, t('grid.bulk.rowFallback', { index: i + 1, defaultValue: `Row ${i + 1}` }))}
    • ))} {restCount > 0 && (