diff --git a/.changeset/related-list-add-picker-filter-3831.md b/.changeset/related-list-add-picker-filter-3831.md new file mode 100644 index 000000000..8a5189fae --- /dev/null +++ b/.changeset/related-list-add-picker-filter-3831.md @@ -0,0 +1,17 @@ +--- +"@object-ui/fields": patch +"@object-ui/plugin-detail": patch +--- + +相关列表 Add 选择器兑现 `add.picker.filter`:作者限定的候选范围现在真的生效 + +`record:related_list.add.picker.filter` 被 spec 声明为「Restrict which records the picker offers」,但渲染器从未读过它 —— 挂 `RecordPickerDialog` 时不传任何 filter,对话框照样提供 `picker.object` 的全部记录,选中即建链接行或改父,`os validate` / `os build` 全绿、运行时零诊断。作者写下「只允许指派 active 的岗位」「只允许挂未过期的许可」,得到的是完整候选列表。 + +现在它按原样传给 `RecordPickerDialog` 的 `baseFilter` —— 不是 `lookupFilters`,后者会把条件渲染成用户可编辑的筛选栏行,等于把作者的硬性限制降级成建议。 + +`baseFilter` 因此接受两种形状,按结构判别(`Array.isArray`): + +- **`QueryParams.$filter` 记录形式**(依赖型 lookup 链)保持原有的键覆盖语义逐字节不变 —— 级联父值必须**替换**同字段上过期的 `lookupFilters` 条目,而不是与之求交。 +- **spec 的 `ViewFilterRule[]`** 经 `mergeFilterNodes`(仓内唯一的 filter 下沉口)下沉,19 个 operator 全部无损到达服务端,包括记录形式没有 `$op` 可用的 `before` / `after` / `is_empty` / `is_not_empty`。此处**不新增**第二份 operator 词汇表。 + +槽位类型同时从 `Record` 收紧为 `unknown`:前者会接受规则数组(数组满足 `any` 的字符串索引),旧的对象展开再把它压成 `{"0": {...}}`,于是查询去过滤名为 `0` 的列 —— 类型全绿、查询错误、无任何诊断。 diff --git a/packages/fields/src/widgets/RecordPickerDialog.baseFilterShapes.test.tsx b/packages/fields/src/widgets/RecordPickerDialog.baseFilterShapes.test.tsx new file mode 100644 index 000000000..88925e386 --- /dev/null +++ b/packages/fields/src/widgets/RecordPickerDialog.baseFilterShapes.test.tsx @@ -0,0 +1,191 @@ +/** + * 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. + */ + +/** + * `RecordPickerDialog.baseFilter` accepts TWO shapes — objectui#3831. + * + * The slot was declared `Record< string, any >` and merged by object spread, + * which serves the dependent-lookup chain (#2215) exactly right and cannot + * carry a spec `ViewFilterRule[]` at all: TypeScript accepts an array there + * (an array satisfies a string index of `any`), the spread flattens it to + * `{"0": rule, "1": rule}`, and the query then filters on columns literally + * named `0`/`1` — green types, wrong query, no diagnostic. That is the slot + * `record:related_list.add.picker.filter` has to reach. + * + * So the merge now discriminates on shape, and both directions are pinned here: + * + * - **record form** keeps KEY-OVERWRITE precedence, byte for byte. A cascaded + * parent value must REPLACE a stale same-field `lookupFilters` entry, not + * intersect with it — `account = 'stale' AND account = 'a1'` returns nothing. + * (`LookupField.dependsOn.test.tsx` owns the #2215 behaviour end-to-end and + * is deliberately left untouched; this file pins the same precedence at the + * dialog's own boundary, so a future merge change cannot pass by only + * satisfying the array path.) + * - **rule array** lowers through `mergeFilterNodes`, the repo's single filter + * sink, so all 19 `VIEW_FILTER_OPERATORS` survive — including the four + * (`before`, `after`, `is_empty`, `is_not_empty`) the MongoDB-style record + * form has no `$op` for and which a hand-rolled second lowering would have + * had to drop or throw on. + */ + +import { render, waitFor } from '@testing-library/react'; +import { describe, it, expect, vi } from 'vitest'; +import { RecordPickerDialog } from './RecordPickerDialog'; + +const records = [{ id: 'r1', name: 'One' }]; + +function makeDataSource() { + const find = vi.fn(async () => ({ data: records, total: records.length })); + return { find } as any; +} + +/** The `$filter` of the most recent query. */ +function lastFilter(ds: any) { + const calls = ds.find.mock.calls; + return calls[calls.length - 1][1]?.$filter; +} + +describe('RecordPickerDialog baseFilter — record form (#2215 precedence)', () => { + it('spreads a record baseFilter last, overwriting lookupFilters on the same field', async () => { + const ds = makeDataSource(); + render( + , + ); + + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + // Still the plain record form — NOT lowered to an AST, and emphatically not + // an `and` of the stale value with the cascaded one. + expect(lastFilter(ds)).toEqual({ account: 'a1' }); + expect(Array.isArray(lastFilter(ds))).toBe(false); + }); + + it('sends no $filter at all when nothing is constrained', async () => { + const ds = makeDataSource(); + render( + , + ); + + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + expect(lastFilter(ds)).toBeUndefined(); + }); +}); + +describe('RecordPickerDialog baseFilter — spec ViewFilterRule[] (#3831)', () => { + it('lowers a rule array to AST nodes instead of spreading it into index keys', async () => { + const ds = makeDataSource(); + render( + , + ); + + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + expect(lastFilter(ds)).toEqual([['is_active', 'equals', true]]); + // The old object-spread failure mode, pinned by structure rather than by + // key: the spread produced a PLAIN OBJECT whose values were still rule + // objects (`{"0": {field, operator, value}}`), so the query asked for a + // column named `0`. Asserting on `Object.keys` cannot tell the two apart — + // an array's keys are its indices too — so what is pinned is that each + // element is an AST TRIPLE and no longer carries a `field` member. + const filter = lastFilter(ds) as unknown[]; + expect(Array.isArray(filter)).toBe(true); + expect(Array.isArray(filter[0])).toBe(true); + expect(filter[0]).not.toHaveProperty('field'); + }); + + it('preserves the four operators the record form cannot spell', async () => { + const ds = makeDataSource(); + render( + , + ); + + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + expect(lastFilter(ds)).toEqual([ + ['starts_at', 'before', '2026-01-01'], + ['expires_at', 'after', '2026-01-01'], + // A rule with no `value` stays two-element: inventing `null` here would be + // a real `{revoked_at: null}` predicate, i.e. a different question. + ['revoked_at', 'is_empty'], + ['assigned_to', 'is_not_empty'], + ]); + }); + + it('keeps a record baseFilter and a rule array as separate `and` children', async () => { + const ds = makeDataSource(); + render( + , + ); + + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + expect(lastFilter(ds)).toEqual([ + 'and', + ['company', '=', 'c1'], + [['is_active', 'equals', true]], + ]); + }); + + it('sends no $filter for an empty rule array', async () => { + const ds = makeDataSource(); + render( + , + ); + + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + // An empty restriction is no restriction — not an empty `$filter` the wire + // has to interpret. + expect(lastFilter(ds)).toBeUndefined(); + }); +}); diff --git a/packages/fields/src/widgets/RecordPickerDialog.tsx b/packages/fields/src/widgets/RecordPickerDialog.tsx index 487d26288..10bc50030 100644 --- a/packages/fields/src/widgets/RecordPickerDialog.tsx +++ b/packages/fields/src/widgets/RecordPickerDialog.tsx @@ -38,6 +38,10 @@ import { X, } from 'lucide-react'; import type { DataSource, LookupColumnDef, LookupFilterDef } from '@object-ui/types'; +// 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 { mergeFilterNodes } from '@object-ui/core'; import { useSafeFieldLabel } from '@object-ui/i18n'; import { useFieldTranslation } from './useFieldTranslation'; import { useRecordQuery } from './useRecordQuery'; @@ -360,13 +364,39 @@ export interface RecordPickerDialogProps { lookupFilters?: LookupFilterDef[]; /** - * Hard filter constraints applied to every query, in QueryParams.$filter - * record form. Unlike `lookupFilters`, entries here never surface in the - * filter bar and cannot be overridden by user filter input — used for the - * dependent (cascading) lookup chain, where the parent field's value MUST - * scope the candidate set (#2215). + * Hard filter constraint applied to every query. Unlike `lookupFilters`, + * entries here never surface in the filter bar and cannot be overridden by + * user filter input. + * + * Two shapes, discriminated STRUCTURALLY (`Array.isArray`), because the two + * callers speak two legitimate vocabularies and neither should be bent into + * the other: + * + * - **`QueryParams.$filter` record form** (`{ account: 'a1' }`) — the + * dependent (cascading) lookup chain, where the parent field's value MUST + * scope the candidate set (#2215). Merged by KEY OVERWRITE, so a cascaded + * value REPLACES a stale `lookupFilters` entry on the same field instead of + * intersecting with it. That precedence is load-bearing: an `and` of both + * would ask for `account = 'stale' AND account = 'a1'` and return nothing. + * - **A spec `ViewFilterRule[]`** (`[{ field, operator, value? }]`) — an + * author's `record:related_list.add.picker.filter`, handed over VERBATIM + * (#3831). Lowered by `mergeFilterNodes`, the repo's single filter sink, so + * all 19 `VIEW_FILTER_OPERATORS` reach the wire — including the four + * (`before`, `after`, `is_empty`, `is_not_empty`) the record form has no + * `$op` for. No second operator vocabulary is introduced here: two already + * exist (the spec's `AST_OPERATOR_MAP`, data-objectstack's + * `FILTER_OPERATOR_ALIASES`) and #3948 is what a third costs. + * + * The discriminator is exact rather than heuristic — every AST node is an + * ARRAY and a rule is a plain OBJECT, the same predicate `toFilterNode` uses. + * + * Typed `unknown` rather than `Record< string, any >` on purpose: that type + * ACCEPTED a rule array (TypeScript lets an array satisfy a string index of + * `any`), the old object-spread merge then flattened it to + * `{"0": {...}, "1": {...}}`, and the query filtered on columns literally + * named `0`/`1` — type-check green, wrong query, no diagnostic anywhere. */ - baseFilter?: Record; + baseFilter?: unknown; /** * Cell renderer resolver function. @@ -586,18 +616,34 @@ export function RecordPickerDialog({ }); }, [baseFilterColumns, fieldsMeta, objectName, translateOptions]); - // Merge base lookup_filters with user filter bar values. The hard - // `baseFilter` constraint (dependent-lookup chain, #2215) is spread LAST so - // user filter-bar input can never widen it back out. - const mergedFilter = useMemo | undefined>(() => { + // Merge base lookup_filters with user filter bar values, then apply the hard + // `baseFilter` constraint BY SHAPE (see the prop's own doc for why the two + // shapes exist): + // + // record form → spread LAST, exactly as this merge has always done, so + // user filter-bar input can never widen it back out and a + // cascaded parent value replaces a stale same-field + // `lookupFilters` entry (#2215). + // rule array → its OWN `and` child via `mergeFilterNodes`, the shared + // sink, so a spec `ViewFilterRule[]` lowers losslessly + // (#3831) instead of being spread into `{0: rule}`. + // + // When BOTH are in play the record side is still built first and lowered as + // one node, so its key-overwrite precedence survives the conjunction. + const mergedFilter = useMemo(() => { const lookupBase = lookupFilters?.length ? lookupFiltersToRecord(lookupFilters) : {}; const userFilter = effectiveFilterColumns?.length ? filterValuesToRecord(filterValues, effectiveFilterColumns) : {}; - const combined = { ...lookupBase, ...userFilter, ...(baseFilter ?? {}) }; - return Object.keys(combined).length > 0 ? combined : undefined; + const rules = Array.isArray(baseFilter) ? baseFilter : undefined; + const recordBase = rules + ? undefined + : (baseFilter as Record | undefined); + const combined = { ...lookupBase, ...userFilter, ...(recordBase ?? {}) }; + const record = Object.keys(combined).length > 0 ? combined : undefined; + return rules ? mergeFilterNodes(record, rules) : record; }, [lookupFilters, effectiveFilterColumns, filterValues, baseFilter]); // Shared query kernel: builds params, fetches, and owns records/loading/error/ diff --git a/packages/fields/src/widgets/useRecordQuery.ts b/packages/fields/src/widgets/useRecordQuery.ts index 23a69a475..73ac4ba67 100644 --- a/packages/fields/src/widgets/useRecordQuery.ts +++ b/packages/fields/src/widgets/useRecordQuery.ts @@ -56,8 +56,14 @@ export interface UseRecordQueryOptions { * `$filter` — already merged by the caller (base `lookup_filters`, dependent * lookup chain, candidate hygiene like `banned != true`, …). Compared by * value, so a referentially-new-but-equal object each render will not loop. + * + * Either the `QueryParams.$filter` record form or a lowered ObjectQL AST node + * (an array), since the picker's merge now produces both (#3831). Typed + * `unknown` rather than `Record< string, any >` so an array cannot slip + * through a type that silently accepts it; emptiness is decided by + * {@link hasFilter}, not by `Object.keys`. */ - filter?: Record; + filter?: unknown; /** `$expand` — related entities to include (e.g. `['primary_business_unit_id']`). */ expand?: string[]; /** `$searchFields` — narrow the server searchable set (ADR-0061). */ @@ -97,6 +103,22 @@ export interface UseRecordQueryResult { refetch: () => void; } +/** + * Is this filter source worth sending as `$filter` at all? + * + * Array-aware on purpose. The picker's merge yields either the + * `QueryParams.$filter` record form or a lowered ObjectQL AST node (#3831), and + * `Object.keys` on an array returns its INDICES — so the record-only test read + * `['0','1','2']` for an AST node and was right only by accident. An empty array + * is "no filter" for the same reason an empty object is. + */ +function hasFilter(filter: unknown): boolean { + if (filter === null || filter === undefined) return false; + if (Array.isArray(filter)) return filter.length > 0; + if (typeof filter !== 'object') return false; + return Object.keys(filter as object).length > 0; +} + export function useRecordQuery(options: UseRecordQueryOptions): UseRecordQueryResult { const { dataSource, @@ -123,7 +145,7 @@ export function useRecordQuery(options: UseRecordQueryOptions): UseRecordQueryRe // Stable signatures for object/array inputs so the fetch effect keys on their // *value*, not a fresh reference every render. const filterSignature = useMemo( - () => (filter && Object.keys(filter).length ? JSON.stringify(filter) : ''), + () => (hasFilter(filter) ? JSON.stringify(filter) : ''), [filter], ); const expandSignature = useMemo(() => (expand?.length ? expand.join(',') : ''), [expand]); @@ -150,7 +172,12 @@ export function useRecordQuery(options: UseRecordQueryOptions): UseRecordQueryRe if (searchTerm && searchTerm.trim()) params.$search = searchTerm.trim(); if (searchFields && searchFields.length > 0) params.$searchFields = searchFields; if (sortArg) params.$orderby = { [sortArg.field]: sortArg.direction }; - if (filter && Object.keys(filter).length > 0) params.$filter = filter; + // `QueryParams.$filter` is declared `Record< string, any >`, which the + // AST-array form does not describe — the cast is at this ONE assignment + // rather than widening a shared type that several other producers + // (plugin-list's `buildEffectiveFilter`, plugin-view's ObjectView) + // already feed arrays through. + if (hasFilter(filter)) params.$filter = filter as Record; if (expand && expand.length > 0) params.$expand = expand; const result = await dataSource.find(objectName, params); diff --git a/packages/plugin-detail/src/RelatedList.tsx b/packages/plugin-detail/src/RelatedList.tsx index 3a9ac11a5..a91e4afe1 100644 --- a/packages/plugin-detail/src/RelatedList.tsx +++ b/packages/plugin-detail/src/RelatedList.tsx @@ -39,6 +39,7 @@ import { } from 'lucide-react'; import type { LucideIcon } from 'lucide-react'; import type { DataSource, FieldMetadata } from '@object-ui/types'; +import type { ViewFilterRule } from '@objectstack/spec/ui'; import { getCellRenderer, resolveCellRendererType, RecordPickerDialog, deriveLookupColumns } from '@object-ui/fields'; import { columnIdentity, @@ -79,9 +80,19 @@ export interface RelatedListProps { * case), or — when `linkField` is omitted — re-parents the picked child by * setting its `referenceField` to `parentId` (1:m case). Server-side rules on * insert (e.g. the AI-seat cap) surface as an inline error. + * + * `picker.filter` restricts which records the dialog offers, and is typed as + * the spec's own `ViewFilterRule[]` rather than `any` (#3831): it goes to + * `RecordPickerDialog`'s `baseFilter` VERBATIM, so the authored vocabulary is + * the one enforced — a looser type here is where a wrong shape would hide. */ add?: { - picker: { object: string; valueField?: string; labelField?: string; filter?: any }; + picker: { + object: string; + valueField?: string; + labelField?: string; + filter?: ViewFilterRule[]; + }; linkField?: string; label?: string; }; @@ -1334,6 +1345,12 @@ export const RelatedList: React.FC = ({ columns={pickerColumns} cellRenderer={getCellRenderer} fieldsMeta={pickerSchema?.fields} + // The author's candidate restriction, handed over VERBATIM (#3831). + // `baseFilter` — never `lookupFilters`, which renders its entries as + // filter-bar rows the user can edit, i.e. demotes the restriction to a + // suggestion. The picker lowers the rule array through the repo's + // single filter sink, so no conversion belongs on this side. + baseFilter={add.picker.filter} onSelect={() => {}} onSelectRecords={(records: any[]) => { void handleAddRecords(records); }} /> diff --git a/packages/plugin-detail/src/__tests__/RelatedList.addPickerFilter.test.tsx b/packages/plugin-detail/src/__tests__/RelatedList.addPickerFilter.test.tsx new file mode 100644 index 000000000..e457545a8 --- /dev/null +++ b/packages/plugin-detail/src/__tests__/RelatedList.addPickerFilter.test.tsx @@ -0,0 +1,162 @@ +/** + * 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#3831 — the related-list Add picker must honour + * `add.picker.filter`. + * + * The spec declares it ("Restrict which records the picker offers") and, until + * this fix, nothing in the repo read it: the dialog mount passed `objectName` / + * `title` / `displayField` / `columns` / `cellRenderer` / `fieldsMeta` / + * `multiple` / `onSelect` / `onSelectRecords` and no filter at all. An author + * who scoped the candidate set — only ACTIVE positions may be assigned, only + * unexpired licences may be attached — got every record of `picker.object`, + * with `os validate` / `os build` silently green and no runtime diagnostic. The + * pick then created the link row or re-parented outright. + * + * It is wired to `baseFilter`, not `lookupFilters`: `lookupFilters` renders its + * entries as filter-bar rows the user can edit, which would turn the author's + * restriction into a suggestion. `baseFilter` never surfaces in the UI and + * cannot be widened back out. + * + * These assertions read the `$filter` the picker's own query carries, i.e. the + * question actually asked of the data source — not merely that a prop was + * threaded somewhere. + */ +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, waitFor, fireEvent } from '@testing-library/react'; +import * as React from 'react'; +import { RelatedList } from '../RelatedList'; + +// The list body itself is irrelevant here — only the Add dialog matters. +vi.mock('@object-ui/react', async (importOriginal) => { + const actual = await importOriginal>(); + return { ...actual, SchemaRenderer: () => null }; +}); + +const junctionSchema = { + name: 'sys_user_position', + fields: { + position: { type: 'lookup', label: 'Position', reference_to: 'sys_position' }, + }, +}; + +const positionSchema = { + name: 'sys_position', + fields: { + name: { type: 'text', label: 'Name' }, + is_active: { type: 'boolean', label: 'Active' }, + }, +}; + +const makeDataSource = () => ({ + getObjectSchema: vi.fn(async (api: string) => + api === 'sys_position' ? positionSchema : junctionSchema, + ), + find: vi.fn(async (api: string) => + api === 'sys_position' + ? { data: [{ id: 'p1', name: 'Nurse', is_active: true }], total: 1 } + : { data: [], total: 0 }, + ), +}); + +/** `$filter` of the most recent query against the PICKER's object. */ +function lastPickerFilter(ds: any) { + const calls = ds.find.mock.calls.filter((c: any[]) => c[0] === 'sys_position'); + expect(calls.length).toBeGreaterThan(0); + return calls[calls.length - 1][1]?.$filter; +} + +const renderList = (ds: any, filter?: any) => + render( + , + ); + +const openPicker = () => + fireEvent.click(screen.getByRole('button', { name: /Assign position/ })); + +describe('RelatedList Add picker — add.picker.filter (#3831)', () => { + it('scopes the picker query with the authored restriction', async () => { + const ds = makeDataSource(); + renderList(ds, [{ field: 'is_active', operator: 'equals', value: true }]); + openPicker(); + + await waitFor(() => expect(lastPickerFilter(ds)).toBeDefined()); + // Lowered by the shared filter sink, so the rule reaches the wire as an AST + // node rather than as a bare rule object the server refuses. + expect(lastPickerFilter(ds)).toEqual([['is_active', 'equals', true]]); + }); + + it('carries every rule of a multi-rule restriction, operators intact', async () => { + const ds = makeDataSource(); + renderList(ds, [ + { field: 'is_active', operator: 'equals', value: true }, + // Two rules on ONE field — a range the field-keyed record form could not + // have expressed without merging them by hand. + { field: 'headcount', operator: 'greater_than', value: 0 }, + { field: 'headcount', operator: 'less_than', value: 100 }, + // The operators with no MongoDB-style `$op`, hence no lossless record + // representation: `before`/`after` and the emptiness pair. + { field: 'starts_at', operator: 'before', value: '2026-06-01' }, + { field: 'ends_at', operator: 'after', value: '2026-01-01' }, + { field: 'retired_at', operator: 'is_empty' }, + ]); + openPicker(); + + await waitFor(() => expect(lastPickerFilter(ds)).toBeDefined()); + expect(lastPickerFilter(ds)).toEqual([ + ['is_active', 'equals', true], + ['headcount', 'greater_than', 0], + ['headcount', 'less_than', 100], + ['starts_at', 'before', '2026-06-01'], + ['ends_at', 'after', '2026-01-01'], + ['retired_at', 'is_empty'], + ]); + }); + + it('leaves the picker unfiltered when the author declared no restriction', async () => { + const ds = makeDataSource(); + renderList(ds); + openPicker(); + + await waitFor(() => expect(ds.getObjectSchema).toHaveBeenCalledWith('sys_position')); + await waitFor(() => + expect(ds.find.mock.calls.some((c: any[]) => c[0] === 'sys_position')).toBe(true), + ); + // No `add.picker.filter` → no `$filter`. The status quo for every existing + // related list stays exactly as it was. + expect(lastPickerFilter(ds)).toBeUndefined(); + }); + + it('does not demote the restriction into an editable filter-bar row', async () => { + const ds = makeDataSource(); + renderList(ds, [{ field: 'is_active', operator: 'equals', value: true }]); + openPicker(); + + await waitFor(() => expect(lastPickerFilter(ds)).toBeDefined()); + // `lookupFilters` is what derives the picker's filter bar; routing the + // author's restriction there would have published it as a user-editable + // suggestion. Its absence is the observable difference. + expect(screen.queryByTestId('record-picker-filter-panel')).toBeNull(); + expect(screen.queryByRole('button', { name: /filters/i })).toBeNull(); + }); +}); diff --git a/packages/plugin-detail/src/__tests__/recordRelatedListInputs.spec-parity.test.ts b/packages/plugin-detail/src/__tests__/recordRelatedListInputs.spec-parity.test.ts index 35bf773ec..8c961e9b4 100644 --- a/packages/plugin-detail/src/__tests__/recordRelatedListInputs.spec-parity.test.ts +++ b/packages/plugin-detail/src/__tests__/recordRelatedListInputs.spec-parity.test.ts @@ -160,17 +160,21 @@ describe('record:related_list — registry inputs vs @objectstack/spec', () => { expect(description).not.toMatch(/labelField[^.]*title field/); }); - it('names `picker.filter` as a gap instead of documenting it as a restriction', () => { - // The `record:activity.showSubscriptionToggle` precedent applied at member - // level. The spec declares `add.picker.filter` ("Restrict which records the - // picker offers") and nothing in this repo reads it — `RelatedList` fills - // `RecordPickerDialog`'s `objectName` / `displayField` / `columns` and never - // its `baseFilter` slot. A description that merely listed `filter` among the - // members would tell an author their picker is scoped when it offers every - // record; objectui#3831 owns the wiring, and this assertion fails the moment - // someone deletes the warning without doing it. + it('documents `picker.filter` as a real restriction, with no gap warning left over', () => { + // The INVERSE of the assertion this used to carry. Until #3831 the spec + // declared `add.picker.filter` ("Restrict which records the picker offers") + // and nothing in this repo read it, so the description named it as a KNOWN + // GAP on the `record:activity.showSubscriptionToggle` precedent. The wiring + // landed (`RelatedList` hands it to `RecordPickerDialog`'s `baseFilter` + // verbatim), so the warning had to go — and this direction now fails if + // anyone puts a gap warning back, or reverts the wiring and leaves the + // description claiming a restriction the dialog does not apply. expect(specPickerKeys()).toContain('filter'); - expect(addDescription()).toMatch(/KNOWN GAP/); - expect(addDescription()).toMatch(/filter[\s\S]*not applied/); + expect(addDescription()).not.toMatch(/KNOWN GAP/); + expect(addDescription()).not.toMatch(/not applied/); + expect(addDescription()).toMatch(/`picker\.filter` restricts which records/); + // The restriction must be published as un-widenable, not as a suggestion: + // `lookupFilters` would have rendered it as an editable filter-bar row. + expect(addDescription()).toMatch(/hard constraint the user cannot widen/); }); }); diff --git a/packages/plugin-detail/src/index.tsx b/packages/plugin-detail/src/index.tsx index 5b20ac13d..77a6c6a9c 100644 --- a/packages/plugin-detail/src/index.tsx +++ b/packages/plugin-detail/src/index.tsx @@ -339,14 +339,14 @@ ComponentRegistry.register('related_list', RecordRelatedListRenderer, { // says. Publishing the spec's wording there would have been a description // the platform does not honour. // - // `picker.filter` is deliberately NOT documented as working: the spec - // declares it, and nothing in this repo reads it — `RelatedList` passes - // `picker.object` / `labelField` to `RecordPickerDialog` and never fills its - // `baseFilter` slot. Naming it here as a gap follows the - // `record:activity.showSubscriptionToggle` precedent above; silently - // documenting it as a restriction would tell an author their picker is - // scoped when it offers every record. - { name: 'add', type: 'object', label: 'Add Existing', description: 'Adds an "Add" button that assigns EXISTING records instead of creating one — the m2m/junction case. Shape: `{ picker: { object, valueField?, labelField?, filter? }, linkField?, label? }`. `picker.object` (required) is the object whose records the dialog offers. `picker.valueField` is the field of the picked record used as the link value (default "id"); `picker.labelField` is the column shown in the picker rows (default "name", and the other columns are derived from that object\'s schema). With `linkField` set, selecting records CREATES rows in this list\'s own object as `{ [relationshipField]: parentValue, [linkField]: pickedId }` — the junction case; omit `linkField` and the picked child is RE-PARENTED instead, by setting its own `relationshipField` to this parent. `label` is the button text (default "Add", localizable inline). Setting `add` also enables generic link removal on rows when no host delete handler is wired. KNOWN GAP: `picker.filter` is accepted by the spec but not applied — the dialog offers every record of `picker.object` whatever you put there.' }, + // `picker.filter` IS documented as working since #3831 wired it: it reaches + // `RecordPickerDialog`'s `baseFilter` verbatim (`RelatedList.tsx`, at the + // dialog mount), which is the un-editable slot — not `lookupFilters`, whose + // entries become filter-bar rows the user can widen back out. It carried a + // KNOWN GAP sentence here until then, on the + // `record:activity.showSubscriptionToggle` precedent; the sentence went away + // with the gap, not before it. + { name: 'add', type: 'object', label: 'Add Existing', description: 'Adds an "Add" button that assigns EXISTING records instead of creating one — the m2m/junction case. Shape: `{ picker: { object, valueField?, labelField?, filter? }, linkField?, label? }`. `picker.object` (required) is the object whose records the dialog offers. `picker.valueField` is the field of the picked record used as the link value (default "id"); `picker.labelField` is the column shown in the picker rows (default "name", and the other columns are derived from that object\'s schema). With `linkField` set, selecting records CREATES rows in this list\'s own object as `{ [relationshipField]: parentValue, [linkField]: pickedId }` — the junction case; omit `linkField` and the picked child is RE-PARENTED instead, by setting its own `relationshipField` to this parent. `label` is the button text (default "Add", localizable inline). Setting `add` also enables generic link removal on rows when no host delete handler is wired. `picker.filter` restricts which records the dialog offers — a list of `{ field, operator, value }` rules in the same vocabulary as this list\'s own `filter`, applied as a hard constraint the user cannot widen (it never appears as an editable filter row).' }, ], });