From ee254e1206f62ab89d72a0bf756037657d3bbce4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 01:10:52 +0000 Subject: [PATCH 1/3] refactor(fields): the record picker calls core's withoutDeniedFields (objectui#10594) RecordPickerDialog's display column now reads its title through `withoutDeniedFields` from `@object-ui/core`, with `[idField]` as the extra key kept, and its module-private copy and that copy's docblock are deleted. For a named object nothing drawn changes. On an empty object name the export passes the row through, as on the other seven surfaces (ruling 5861445694). New pin: a named object strips a denied field from the title and keeps the declared id field even when denied; an empty object name issues no read and draws no row, against the production policy provider. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DuWo5bdP9SdVebamn99GGk --- ...dPickerDialog.fieldReadRule-10594.test.tsx | 163 ++++++++++++++++++ .../fields/src/widgets/RecordPickerDialog.tsx | 35 +--- 2 files changed, 168 insertions(+), 30 deletions(-) create mode 100644 packages/fields/src/widgets/RecordPickerDialog.fieldReadRule-10594.test.tsx diff --git a/packages/fields/src/widgets/RecordPickerDialog.fieldReadRule-10594.test.tsx b/packages/fields/src/widgets/RecordPickerDialog.fieldReadRule-10594.test.tsx new file mode 100644 index 0000000000..4ae73cd219 --- /dev/null +++ b/packages/fields/src/widgets/RecordPickerDialog.fieldReadRule-10594.test.tsx @@ -0,0 +1,163 @@ +/** + * 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 record picker's display column reads its title through + * `@object-ui/core`'s `withoutDeniedFields`, the one field-read rule every + * display surface calls (objectui#10594). The picker's module-private copy is + * gone. + * + * Pinned against the real dialog and the production policy provider + * (`MePermissionsProvider`, an authenticated session: an object the payload + * does not name is denied, #2926 ④): + * + * - a named object: a field the loaded policy denies is not in the display + * column's title, and the declared `idField` is kept in the row the title + * reads even when the policy denies it (the `[idField]` this site hands the + * export), so the title still shows it and a row click still commits it; + * - an empty object name: the export passes the row through where the old + * copy judged every field against `''` (ruling 5861445694 on + * objectui#10594). The picker reads nothing without an object name, so + * there is no row to judge: no read is issued and no row is drawn, although + * the backend here answers for any object name. + * + * `RecordPickerDialog.displayFls-10373.test.tsx` pins the column gates (which + * columns are headed and drawn) against the role-based provider. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor, act, fireEvent } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { SchemaRendererContext } from '@object-ui/react'; +import { MePermissionsProvider, type MePermissionsResponse } from '@object-ui/permissions'; +import { RecordPickerDialog } from './RecordPickerDialog'; +import { getCellRenderer } from '../index'; + +const ACCOUNT_FIELDS: Record = { + name: { type: 'text', label: 'Name' }, + code: { type: 'text', label: 'Code' }, + secret: { type: 'text', label: 'Secret' }, +}; + +/** + * A backend that does NOT strip denied keys (ObjectStack's `FieldMasker` + * would), and that answers for ANY object name, so a read issued for an empty + * name would put rows on screen. + */ +function makeBackend(prefix: string) { + const rows = [0, 1, 2].map((i) => ({ + id: `${prefix}_acct_${i}`, + code: `C-${prefix}-${i}`, + name: `Account ${i}`, + secret: `S-${prefix}-${i}`, + })); + const find = vi.fn(async () => ({ data: rows.map((r) => ({ ...r })), total: rows.length })); + return { find, findOne: vi.fn(async () => null) } as any; +} + +/** + * An authenticated `/me/permissions` payload: `account` is readable, and each + * named `account` field is not. + */ +function policyDenying(...fields: string[]): MePermissionsResponse { + return { + authenticated: true, + userId: 'u1', + tenantId: null, + roles: ['viewer'], + permissionSets: [], + objects: { account: { allowRead: true } }, + fields: Object.fromEntries(fields.map((f) => [`account.${f}`, { readable: false }])), + }; +} + +function Picker({ + ds, + objectName, + onSelect = () => {}, +}: { + ds: ReturnType; + objectName: string; + onSelect?: (v: unknown) => void; +}) { + return ( + + {}} + dataSource={ds} + objectName={objectName} + idField="code" + onSelect={onSelect} + cellRenderer={getCellRenderer} + fieldsMeta={ACCOUNT_FIELDS} + objectSchema={{ name: 'account', fields: ACCOUNT_FIELDS, titleFormat: '{name} - {code} - {secret}' }} + /> + + ); +} + +async function settle(): Promise { + for (let i = 0; i < 6; i++) { + await act(async () => { + await new Promise((r) => setTimeout(r, 0)); + }); + } +} + +function cells(field: string): string[] { + return Array.from(document.querySelectorAll(`[data-lookup-cell="${field}"]`)).map( + (el) => el.textContent ?? '', + ); +} + +afterEach(() => { + cleanup(); +}); + +describe('RecordPickerDialog — the display title reads the shared field-read rule (objectui#10594)', () => { + it('a named object: a denied field is not in the title; the declared id field is kept even when denied, and a row click commits it', async () => { + const ds = makeBackend('named'); + const onSelect = vi.fn(); + render( + + + , + ); + await waitFor(() => expect(screen.getByTestId('record-row-C-named-1')).toBeInTheDocument()); + await settle(); + + expect(cells('name')).toEqual([ + 'Account 0 - C-named-0', + 'Account 1 - C-named-1', + 'Account 2 - C-named-2', + ]); + expect(document.body.textContent).not.toContain('S-named-'); + await act(async () => { + fireEvent.click(screen.getByTestId('record-row-C-named-1')); + }); + expect(onSelect).toHaveBeenCalledWith('C-named-1'); + }); + + it('an empty object name under a loaded policy: no read is issued and no row is drawn', async () => { + const ds = makeBackend('empty'); + render( + + + , + ); + await waitFor(() => expect(screen.getByRole('dialog')).toBeInTheDocument()); + await settle(); + + expect(ds.find).not.toHaveBeenCalled(); + expect(document.querySelectorAll('[data-testid^="record-row-"]')).toHaveLength(0); + expect(document.querySelectorAll('[data-lookup-cell]')).toHaveLength(0); + expect(document.body.textContent).not.toContain('Account 0'); + }); +}); diff --git a/packages/fields/src/widgets/RecordPickerDialog.tsx b/packages/fields/src/widgets/RecordPickerDialog.tsx index d5ed88d48f..6ce5768b4a 100644 --- a/packages/fields/src/widgets/RecordPickerDialog.tsx +++ b/packages/fields/src/widgets/RecordPickerDialog.tsx @@ -40,7 +40,7 @@ import type { DataSource, LookupColumnDef, LookupFilterDef } from '@object-ui/ty // The repo's single filter sink (`packages/core/src/utils/filter-converter.ts`) // — shared with plugin-list's `buildEffectiveFilter` and plugin-view's // ObjectView, so a spec `ViewFilterRule[]` lowers in exactly one place. -import { buildExpandFields, mergeFilterNodes, toFilterNodeSafely, toPredicateRecord, type FilterNodeResult } from '@object-ui/core'; +import { buildExpandFields, mergeFilterNodes, toFilterNodeSafely, toPredicateRecord, withoutDeniedFields, type FilterNodeResult } from '@object-ui/core'; import { useSafeFieldLabel, useDisplayLocale } from '@object-ui/i18n'; import { usePermissions } from '@object-ui/permissions'; import { useFieldTranslation } from './useFieldTranslation.js'; @@ -439,33 +439,6 @@ export interface RecordPickerDialogProps { renderGrid?: (props: RecordPickerGridSlotProps) => React.ReactNode; } -/** - * `record` without the fields the loaded permission policy denies on - * `objectName` (objectui#10373) — the row ObjectStack's `FieldMasker` already - * serves. The identity columns are never judged: the id is the committed - * value, not a display value. Before a policy loads, the record comes back as - * is. `LookupField` applies the same rule to its option labels - * (`withoutDeniedFields` there); each file keeps its own copy because this one - * is re-exported whole from the package entry and the helper is not public. - */ -function withoutDeniedFields( - record: T, - perms: ReturnType, - objectName: string, - idField: string, -): T { - if (!perms.isLoaded || !record || typeof record !== 'object') return record; - const shown: Record = {}; - let withheld = false; - for (const [key, value] of Object.entries(record)) { - const readable = - key === idField || key === 'id' || key === '_id' || perms.checkField(objectName, key, 'read'); - if (readable) shown[key] = value; - else withheld = true; - } - return withheld ? (shown as T) : record; -} - /** * RecordPickerDialog — Enterprise-grade record selection dialog. * @@ -907,12 +880,14 @@ export function RecordPickerDialog({ // `titleFormat` template, a `nameField`), so it reads the row with the fields // the loaded policy denies removed (objectui#10373) — the row ObjectStack's // `FieldMasker` already serves. A denied field falls through the resolver - // exactly as it does for that row. + // exactly as it does for that row. The rule is `@object-ui/core`'s + // `withoutDeniedFields`, the one every surface calls (objectui#10594). The id + // field is never judged: it is the value committed, not a display value. const renderCellContent = useCallback( (record: any, col: LookupColumnDef): React.ReactNode => renderLookupColumnValue( objectSchema && col.field === displayField - ? withoutDeniedFields(toPredicateRecord(record, fieldsMeta), perms, objectName, idField) + ? withoutDeniedFields(toPredicateRecord(record, fieldsMeta), perms, objectName, [idField]) : record, col, { From 50cca18c0d44a2be40d4bf3fa9abf04af8ae15f1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 01:15:04 +0000 Subject: [PATCH 2/3] chore(changeset): patch @object-ui/fields for the record picker's shared field-read rule (objectui#10594) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DuWo5bdP9SdVebamn99GGk --- .changeset/10594-record-picker-shared-field-read-rule.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/10594-record-picker-shared-field-read-rule.md diff --git a/.changeset/10594-record-picker-shared-field-read-rule.md b/.changeset/10594-record-picker-shared-field-read-rule.md new file mode 100644 index 0000000000..a8d13bd860 --- /dev/null +++ b/.changeset/10594-record-picker-shared-field-read-rule.md @@ -0,0 +1,7 @@ +--- +'@object-ui/fields': patch +--- + +`RecordPickerDialog` builds its display column's title from the row that `withoutDeniedFields` from `@object-ui/core` returns, and its module-private copy of that field-read rule is deleted (objectui#10594). It was the last hand-written copy of the rule; the export is now its only definition. + +For a named object nothing the picker draws changes: a field the loaded policy denies is still removed before the title is built, and `id`, `_id` and the declared `idField` are still kept. The one change is an empty `objectName`: the copy judged every field against `''`, while the export passes the row through, as the other surfaces do. The picker reads no records without an object name, so the change can show only in the single render after a host empties the object name of an open picker, before the rows of the previous object are cleared. From 24824c7cbd6874823f205063405eeb4f02029acb Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 02:02:17 +0000 Subject: [PATCH 3/3] test(fields): pin the record picker's pass-through on an empty object name (objectui#10594) Adds the leg the contract review measured: a host empties the name of an open picker, whose idField is the display field, and a layout effect records the drawn titles each host commit. No title drawn under the empty name may be one stripped against it. Red on the deleted copy, green on the export, and still green if the query kernel ever clears the rows in the same commit. The pin's docblock now claims only what each leg observes. The changeset now says the copy exempted id, _id and the declared idField, and that the change shows only in the display column. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01DuWo5bdP9SdVebamn99GGk --- ...94-record-picker-shared-field-read-rule.md | 2 +- ...dPickerDialog.fieldReadRule-10594.test.tsx | 110 ++++++++++++++++-- 2 files changed, 101 insertions(+), 11 deletions(-) diff --git a/.changeset/10594-record-picker-shared-field-read-rule.md b/.changeset/10594-record-picker-shared-field-read-rule.md index a8d13bd860..21fbac7f1b 100644 --- a/.changeset/10594-record-picker-shared-field-read-rule.md +++ b/.changeset/10594-record-picker-shared-field-read-rule.md @@ -4,4 +4,4 @@ `RecordPickerDialog` builds its display column's title from the row that `withoutDeniedFields` from `@object-ui/core` returns, and its module-private copy of that field-read rule is deleted (objectui#10594). It was the last hand-written copy of the rule; the export is now its only definition. -For a named object nothing the picker draws changes: a field the loaded policy denies is still removed before the title is built, and `id`, `_id` and the declared `idField` are still kept. The one change is an empty `objectName`: the copy judged every field against `''`, while the export passes the row through, as the other surfaces do. The picker reads no records without an object name, so the change can show only in the single render after a host empties the object name of an open picker, before the rows of the previous object are cleared. +For a named object nothing the picker draws changes: a field the loaded policy denies is still removed before the title is built, and `id`, `_id` and the declared `idField` are still kept. The one change is an empty `objectName`: the copy judged every field but `id`, `_id` and the declared `idField` against `''`, while the export passes the row through, as the other surfaces do. The picker reads no records without an object name, so the change can show only in the single render after a host empties the object name of an open picker, before the rows of the previous object are cleared, and only in the display column. diff --git a/packages/fields/src/widgets/RecordPickerDialog.fieldReadRule-10594.test.tsx b/packages/fields/src/widgets/RecordPickerDialog.fieldReadRule-10594.test.tsx index 4ae73cd219..c753644f48 100644 --- a/packages/fields/src/widgets/RecordPickerDialog.fieldReadRule-10594.test.tsx +++ b/packages/fields/src/widgets/RecordPickerDialog.fieldReadRule-10594.test.tsx @@ -16,15 +16,28 @@ * (`MePermissionsProvider`, an authenticated session: an object the payload * does not name is denied, #2926 ④): * - * - a named object: a field the loaded policy denies is not in the display - * column's title, and the declared `idField` is kept in the row the title - * reads even when the policy denies it (the `[idField]` this site hands the - * export), so the title still shows it and a row click still commits it; - * - an empty object name: the export passes the row through where the old - * copy judged every field against `''` (ruling 5861445694 on - * objectui#10594). The picker reads nothing without an object name, so - * there is no row to judge: no read is issued and no row is drawn, although - * the backend here answers for any object name. + * - a named object (control): a field the loaded policy denies is not in the + * display column's title, and the declared `idField` is kept in the row the + * title reads even when the policy denies it (the `[idField]` this site + * hands the export), so the title still shows it and a row click still + * commits it; + * - a picker OPENED with an empty object name issues no read and draws no + * row, although the backend here answers for any object name. This leg + * observes the query kernel's gate (`useRecordQuery` reads nothing without + * an object name), not the field-read rule; it records why a host that + * opens the picker with no name sees an empty table under either rule; + * - the ruled pass-through on an empty object name (ruling 5861445694 on + * objectui#10594): the deleted copy judged every field but `id`, `_id` and + * `idField` against `''`, which this provider denies, and the export passes + * the row through. The picker holds rows under an empty name only in the + * commit after a host empties the name of an OPEN picker, before the kernel + * clears the previous object's rows. With `idField` equal to the display + * field, the column gate keeps that column by identity on `''`, so its title + * is drawn there. The leg asserts the NEGATIVE: no title drawn under the + * empty name is one stripped against `''`. It is red on the deleted copy and + * on an export or dialog that fails closed on `''`, and it stays green if + * the kernel ever clears the rows in the same commit (the snapshot then + * holds no cell). * * `RecordPickerDialog.displayFls-10373.test.tsx` pins the column gates (which * columns are headed and drawn) against the role-based provider. @@ -117,6 +130,38 @@ function cells(field: string): string[] { ); } +type Snapshot = { objectName: string; cells: string[] }; + +/** One module-level payload, so a host rerender hands the provider the same object. */ +const SECRET_DENIED = policyDenying('secret'); + +/** + * A host that keeps the picker OPEN while its object name changes, and records + * the drawn display cells after every commit of its own (a layout effect runs + * after the commit's DOM writes). `idField` is the display field, so the + * picker's column gate keeps the display column by identity even on `''`. + */ +function RenamingHost({ ds, objectName, log }: { ds: ReturnType; objectName: string; log: Snapshot[] }) { + React.useLayoutEffect(() => { + log.push({ objectName, cells: cells('name') }); + }); + return ( + + {}} + dataSource={ds} + objectName={objectName} + idField="name" + onSelect={() => {}} + cellRenderer={getCellRenderer} + fieldsMeta={ACCOUNT_FIELDS} + objectSchema={{ name: 'account', fields: ACCOUNT_FIELDS, titleFormat: '{name} - {secret}' }} + /> + + ); +} + afterEach(() => { cleanup(); }); @@ -145,7 +190,7 @@ describe('RecordPickerDialog — the display title reads the shared field-read r expect(onSelect).toHaveBeenCalledWith('C-named-1'); }); - it('an empty object name under a loaded policy: no read is issued and no row is drawn', async () => { + it('a picker opened with an empty object name under a loaded policy: no read is issued and no row is drawn', async () => { const ds = makeBackend('empty'); render( @@ -160,4 +205,49 @@ describe('RecordPickerDialog — the display title reads the shared field-read r expect(document.querySelectorAll('[data-lookup-cell]')).toHaveLength(0); expect(document.body.textContent).not.toContain('Account 0'); }); + + it('the ruled pass-through: after a host empties the name of an open picker, no drawn title is one stripped against the empty name', async () => { + const ds = makeBackend('renamed'); + const log: Snapshot[] = []; + const { rerender } = render( + + + , + ); + await waitFor(() => expect(screen.getByTestId('record-row-Account 1')).toBeInTheDocument()); + await settle(); + + // The snapshot is live: a host commit on the named object records the drawn + // titles, with the denied `secret` stripped and the id field kept. + await act(async () => { + rerender( + + + , + ); + }); + expect(log[log.length - 1]).toEqual({ + objectName: 'account', + cells: ['Account 0', 'Account 1', 'Account 2'], + }); + + await act(async () => { + rerender( + + + , + ); + }); + await settle(); + + const underEmptyName = log.filter((s) => s.objectName === ''); + expect(underEmptyName.length).toBeGreaterThan(0); + // A title stripped against `''` is the bare name: this provider denies every + // field of an object it does not name, and only the id field is exempt. + for (const snapshot of underEmptyName) { + for (const cell of snapshot.cells) expect(cell).not.toMatch(/^Account \d$/); + } + expect(ds.find).toHaveBeenCalledTimes(1); + expect(document.querySelectorAll('[data-testid^="record-row-"]')).toHaveLength(0); + }); });