From caabbd7cdefd967da5b641381b35ed9edbfa62de Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 20:18:09 +0000 Subject: [PATCH] fix(plugin-detail): consume record:related_list.filter, gate Add on dataSource (objectstack#7118, objectui#3895) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `RecordRelatedListProps.filter` was declared by the spec and published as a registry input while nothing read it: `RelatedList` built its query from `{ [referenceField]: parentId }` alone, so an authored filter passed every gate and was dropped — the list answered with every child of the parent. It is now AND-combined with the parent condition (never substituted for it: "additional" criteria may only narrow), lowered through the repo's single filter sink so both the spec's `ViewFilterRule[]` vocabulary and the AST `ElementDataSourceGate` composes are accepted without a second dialect. With nothing authored the query is byte-identical to before. `RECORD_RELATED_LIST_DATA_SOURCE` gains `filter: true`, so a saved view named through `dataSource: { object, view }` no longer contributes columns/sort/limit while its filter is discarded, and #6953's honest reverse assertion flips to the positive one its own comment predicted. The legacy raw-URL fallback path cannot express an operator, so a declared filter there is refused with a console explanation rather than dropped: answering wider than the metadata asked is the class this wiring removes. Separately, the Add button now requires `dataSource`, matching the dialog it opens and the callback it ends in. Without it the button rendered and did nothing at all when clicked — real in hosts that bind no `RecordContext` (Studio designer previews, context-free embeds), since the renderer passes `dataSource={ctx?.dataSource}`. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01GTRjn8xBqp75dk7kFupVRt --- .../related-list-filter-and-add-gate.md | 25 +++ content/docs/guide/data-source.md | 16 +- packages/plugin-detail/README.md | 20 +++ packages/plugin-detail/src/RelatedList.tsx | 97 ++++++++++- ...tedListRenderer.elementDataSource.test.tsx | 69 +++++--- .../RelatedList.addGateDataSource.test.tsx | 150 ++++++++++++++++++ .../__tests__/RelatedList.listFilter.test.tsx | 148 +++++++++++++++++ packages/plugin-detail/src/index.tsx | 10 +- .../src/renderers/record-related-list.tsx | 29 ++-- 9 files changed, 517 insertions(+), 47 deletions(-) create mode 100644 .changeset/related-list-filter-and-add-gate.md create mode 100644 packages/plugin-detail/src/__tests__/RelatedList.addGateDataSource.test.tsx create mode 100644 packages/plugin-detail/src/__tests__/RelatedList.listFilter.test.tsx diff --git a/.changeset/related-list-filter-and-add-gate.md b/.changeset/related-list-filter-and-add-gate.md new file mode 100644 index 000000000..2b7a8c157 --- /dev/null +++ b/.changeset/related-list-filter-and-add-gate.md @@ -0,0 +1,25 @@ +--- +'@object-ui/plugin-detail': patch +--- + +`record:related_list` — the declared `filter` reaches the query, and the Add button answers to the same gate as its dialog + +- **`filter` is consumed** (objectstack#7118). The spec declares + `RecordRelatedListProps.filter` ("additional filter criteria") and this repo + published it as a registry input, but nothing read it: `RelatedList` built its + query from `{ [relationshipField]: parentId }` alone, so an authored filter was + accepted by every gate and silently dropped — the list answered with every child + of the parent. It is now AND-combined with the parent condition (never + substituted for it, so an additional criterion can only narrow), lowered through + the repo's single filter sink so the spec's `[{ field, operator, value }]` + vocabulary and a composed `dataSource` binding both work. With nothing authored + the query is unchanged. As a consequence a saved view named through + `dataSource: { object, view }` no longer contributes its columns/sort/limit while + its filter is discarded — the list can no longer be wider than the view it names. + On the legacy raw-URL fallback path, which cannot express an operator, a declared + filter is refused with a console explanation instead of dropped. +- **The Add button now requires `dataSource`** (objectui#3895), matching the picker + dialog and the add callback. In hosts that supply no `RecordContext` — Studio + designer previews, context-free embeds — the button rendered and did nothing at + all when clicked; the affordance is now withheld where the capability behind it + is absent. diff --git a/content/docs/guide/data-source.md b/content/docs/guide/data-source.md index 590936d2e..dc7b4d402 100644 --- a/content/docs/guide/data-source.md +++ b/content/docs/guide/data-source.md @@ -247,7 +247,7 @@ ignores would be accepted and dropped, which is the defect this binding removes. | `list-view` | ✅ | ✅ | ✅ | ✅ | ✅ | | `object-grid` | ✅ | ✅ | ✅ | ✅ | ✅ | | `element:record_picker` | ✅ | ✅ | ✅ | ✅ | ✅ | -| `record:related_list` | ✅ | columns / sort / limit | — (see below) | ✅ | ✅ | +| `record:related_list` | ✅ | columns / filter / sort / limit | ✅ | ✅ | ✅ | | `object-calendar` | ✅ | filter / sort | ✅ | ✅ | — no row cap | | `object-kanban` | ✅ | filter | ✅ | — no ordering | — fixed window | | `object-chart` | ✅ | filter | ✅ | — engine orders | — no page | @@ -259,12 +259,16 @@ on that block. A view name that does not resolve is reported as a configuration error on **every** block in the table, including the ones that take nothing else from the view — so a typo never passes silently, whatever the block. -Two current gaps, recorded rather than papered over: +On `record:related_list` the composed filter is AND-combined with the parent +relationship condition, never substituted for it: a related list is always scoped +to the record it appears on, and an *additional* criterion can only narrow that +set further. (Until objectstack#7118 this block declared `filter` without reading +it, so a named view contributed its columns / sort / limit while its filter was +dropped — the list could be wider than the view it named. That gap is closed; the +`filter` cell above is what closed it.) + +One current gap, recorded rather than papered over: -- `record:related_list` declares a flat `filter` its renderer does not read (the - list scopes itself by the parent relationship alone), so a view named there - contributes columns / sort / limit and its filter is dropped — the list can be - wider than the view it names. - `object-form` resolves `view` only to report an unresolvable name; a view that does resolve contributes nothing, because a list view's columns are not a form layout. diff --git a/packages/plugin-detail/README.md b/packages/plugin-detail/README.md index 034dad61d..b9783f05e 100644 --- a/packages/plugin-detail/README.md +++ b/packages/plugin-detail/README.md @@ -213,6 +213,26 @@ in the opt-in filter box temporarily falls back to the full-fetch client pipeline (the contains-filter sweeps every field, which no generic server filter can express). +The node's `filter` (spec `RecordRelatedListProps.filter`, "additional filter +criteria") narrows the list beyond the parent relationship: it is +**AND-combined** with `{ [relationshipField]: parentId }`, never substituted for +it, so a related list stays scoped to the record it appears on and an additional +criterion can only ever narrow that set. Authors write it in the spec's own +vocabulary (`[{ field, operator, value }]`); a `dataSource` binding's composed +filter (component AND saved view AND binding) lands on the same key. Both are +lowered to ObjectQL through the repo's single filter sink, so no second dialect +appears. On the legacy raw-URL fallback path (no `dataSource` adapter, where the +query language is `filter[]=` and cannot carry an operator) a +declared filter is refused with a console explanation rather than dropped — +answering with more rows than the metadata asked for is the failure this key's +wiring exists to remove. + +The **Add** affordance renders only where every link in its chain is available: +a spec-valid `add.picker.object` *and* a `dataSource`. The picker dialog and the +add callback both required the adapter already, so without it the button used to +render and do nothing at all when clicked — visible in hosts that supply no +`RecordContext` (Studio designer previews, context-free embeds). + The `record:related_list` renderer is automatically gated on the current user's object-level `read` permission for the child object: when the permission system (`@object-ui/permissions`) is loaded and denies read, diff --git a/packages/plugin-detail/src/RelatedList.tsx b/packages/plugin-detail/src/RelatedList.tsx index a91e4afe1..e849d7bff 100644 --- a/packages/plugin-detail/src/RelatedList.tsx +++ b/packages/plugin-detail/src/RelatedList.tsx @@ -47,7 +47,10 @@ import { getRecordDisplayName, getSortValue, isExpandableFieldType, + mergeFilterNodes, + toFilterNode, userActionPredicates, + type FilterNode, } from '@object-ui/core'; import { useSafeFieldLabel } from '@object-ui/react'; import { usePermissions } from '@object-ui/permissions'; @@ -136,6 +139,29 @@ export interface RelatedListProps { * @default false */ sortable?: boolean; + /** + * The list's OWN scope filter — spec `RecordRelatedListProps.filter` + * ("Additional filter criteria for related records"), which had no read site + * on this component at all until objectstack#7118: the query was built from + * `{ [referenceField]: parentId }` alone, so an authored `filter` (and the + * FILTER half of a `dataSource` binding's saved view) was accepted by every + * gate and silently dropped — the list answered wider than the metadata asked. + * + * ANDed with the parent-relationship condition, never substituted for it: + * "additional" means it may only narrow this parent's children. That is also + * why it is not routed through `data-table`'s `lookupFilters` — those render + * as filter-bar rows the user can edit, which demotes the author's constraint + * to a suggestion (#3831 argued this for `add.picker.filter`; it holds harder + * for the list's own scope). + * + * Two shapes arrive, both produced by our own layers: the spec vocabulary + * (`ViewFilterRule[]`) as authored, and an ObjectQL AST node as composed by + * `ElementDataSourceGate` (which ANDs component/view/binding filters through + * `mergeFilterNodes` before this component ever sees them). Both are lowered + * here through that same single sink — the repo's one filter→wire exit — so no + * second conversion dialect appears. + */ + filter?: ViewFilterRule[] | FilterNode; /** Enable text filtering */ filterable?: boolean; /** Whether the card is collapsible */ @@ -290,6 +316,7 @@ export const RelatedList: React.FC = ({ pageSize, defaultSort, sortable = false, + filter, filterable = false, collapsible = false, defaultCollapsed = false, @@ -360,6 +387,18 @@ export const RelatedList: React.FC = ({ // eslint-disable-next-line react-hooks/exhaustive-deps [defaultSortKey], ); + // The list's own scope filter, lowered to an ObjectQL node once. Keyed on + // CONTENT for the reason `defaultSortSpec` is: an inline `filter` array on a + // schema node is a new identity every render, and this value is a dependency + // of the fetch effect — keying on identity would refetch the collection on + // every render. `undefined` means "nothing authored", so the query below stays + // byte-identical to what it sent before this key had a read site. + const filterKey = JSON.stringify(filter ?? null); + const listFilterNode = React.useMemo( + () => toFilterNode(filter), + // eslint-disable-next-line react-hooks/exhaustive-deps + [filterKey], + ); // Sync internal state when data prop changes (e.g., parent fetches async data) React.useEffect(() => { @@ -454,9 +493,19 @@ export const RelatedList: React.FC = ({ return; } setLoading(true); - const filter = { [referenceField!]: parentId } as Record; + const parentScope = { [referenceField!]: parentId } as Record; + // Parent relationship AND the list's own scope (objectstack#7118). The + // parent condition is never negotiable — an "additional" criterion may only + // narrow this parent's children — and with nothing authored the query is + // the untouched MongoDB-style object it has always been, rather than a + // freshly lowered AST that means the same thing (the difference is + // invisible on screen and visible to every caller pinning the wire). + const queryFilter = + listFilterNode === undefined + ? parentScope + : mergeFilterNodes(parentScope, listFilterNode); if (dataSource && typeof dataSource.find === 'function') { - const params: Record = { $filter: filter }; + const params: Record = { $filter: queryFilter }; if (windowed) { params.$top = effectivePageSize; params.$skip = fetchPage * effectivePageSize; @@ -497,6 +546,22 @@ export const RelatedList: React.FC = ({ console.error('Failed to fetch related data:', err); if (!cancelled) setLoading(false); }); + } else if (listFilterNode !== undefined) { + // No adapter — the legacy raw-URL path, whose query language is + // `filter[]=` and cannot carry an operator, let alone a + // rule array. Dropping the authored filter here would answer with MORE + // rows than the metadata asked for, silently: the exact class this key's + // wiring exists to remove (objectstack#7118), so it refuses and says so + // instead. Empty-and-loud beats wider-and-quiet; the guard above refuses + // an unscoped fetch on the same reasoning. + // eslint-disable-next-line no-console + console.warn( + `[RelatedList] "${api}" declares a filter but has no dataSource adapter — the raw-URL fallback cannot express it, so no rows are fetched. Pass a dataSource (RecordContext) to use a filtered related list.`, + ); + setRelatedData([]); + setTotal(null); + setHasMore(false); + setLoading(false); } else { const qs = new URLSearchParams({ [`filter[${referenceField}]`]: String(parentId), @@ -519,7 +584,7 @@ export const RelatedList: React.FC = ({ return () => { cancelled = true; }; - }, [api, dataProvided, dataSource, referenceField, parentId, refreshNonce, windowed, effectivePageSize, fetchPage, fetchSortField, fetchSortDirection, defaultSortSpec]); + }, [api, dataProvided, dataSource, referenceField, parentId, refreshNonce, windowed, effectivePageSize, fetchPage, fetchSortField, fetchSortDirection, defaultSortSpec, listFilterNode]); // Windowed mode: a page beyond the (shrunken) collection — e.g. the last // row of the last page was just deleted — comes back empty. Step back one @@ -531,11 +596,14 @@ export const RelatedList: React.FC = ({ } }, [windowed, loading, relatedData, currentPage]); - // A different parent (or relationship) is a different collection — restart - // from the first page. + // A different parent (or relationship, or list scope) is a different + // collection — restart from the first page. `filterKey` belongs here for the + // same reason the other three do: page 3 of the unfiltered children is not + // page 3 of the filtered ones. React.useEffect(() => { setCurrentPage(0); - }, [api, referenceField, parentId]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [api, referenceField, parentId, filterKey]); // Refetch when a mutation elsewhere signals this related object changed — // e.g. a child row action executed through the host retargets `api` and @@ -1154,8 +1222,21 @@ export const RelatedList: React.FC = ({ truthy: an `add` without `picker` is metadata the spec rejects, and offering a button that could never open a picker is worse than withholding it (#3838 — the console hint above names the - missing key). */} - {add && pickerObject && ( + missing key). + + `dataSource` is part of the SAME gate, because the dialog this + button opens (below, `add && pickerObject && dataSource`) and the + callback it ends in (`handleAddRecords`: `if (!add || + !dataSource || …) return`) both require it. Without it the button + rendered, `setPickerOpen(true)` ran, and no dialog existed to + observe the flag: a click with NO visible reaction and no + message. Hosts where that is real are the ones passing + `dataSource={ctx?.dataSource}` with no `RecordContext` bound — + the Studio designer preview and context-free embeds + (`renderers/record-related-list.tsx`). Same principle as #3838 + one condition further: an affordance is offered only where the + capability behind it exists (objectui#3895). */} + {add && pickerObject && dataSource && (