From 9f1d0712e4aa0a6a3f1950186cdc887dd6b8bd28 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 17:26:09 +0000 Subject: [PATCH 1/7] fix(react,fields,charts,dashboard,report,list,core): draw the malformed-filter notice where render/click-time merges refused (objectui#10789) Render-time and click-time filter merges called the throwing converter form, so a malformed authored filter threw a FilterOperatorError out of render (or out of a click handler) instead of surfacing the refusal the way the sibling sites do. - ElementDataSourceGate / useElementDataSource: both merges (component filter with the composed binding; view filter with the binding's own) now report the refusal as `missing` + `filterRefusal`; the gate draws the malformed-filter notice the wrapped blocks draw (`view.malformedFilter`). - PeoplePicker recents and RecordPickerDialog mergedFilter lower through toFilterNodeSafely; the refusal lands in the picker's existing error state and no query runs without the filter. - ObjectChart, ObjectPivotTable, DatasetWidget and DatasetReportRenderer drills log the refusal and open/emit no drill (never a filter-less one). - ListView server export builds its filter inside its try, so the refusal is named in the export menu. - lowerLogicalGroup reads every $or member before the TRUE disjunct absorbs it, so a refused member is refused in either member order. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DuWo5bdP9SdVebamn99GGk --- ...ord-picker-malformed-filter-10789.test.tsx | 76 +++++++ .../filter-or-absorb-order-10789.test.ts | 72 +++++++ packages/core/src/utils/filter-converter.ts | 12 +- packages/fields/src/widgets/PeoplePicker.tsx | 47 +++-- .../fields/src/widgets/RecordPickerDialog.tsx | 51 +++-- .../pickerMalformedFilter-10789.test.tsx | 152 ++++++++++++++ packages/plugin-charts/src/ObjectChart.tsx | 48 ++++- .../ObjectChart.drillRefusal-10789.test.tsx | 108 ++++++++++ .../plugin-dashboard/src/DatasetWidget.tsx | 18 +- .../plugin-dashboard/src/ObjectPivotTable.tsx | 53 ++++- .../src/__tests__/drillRefusal-10789.test.tsx | 185 ++++++++++++++++++ packages/plugin-list/src/ListView.tsx | 11 +- .../ListView.exportRefusal-10789.test.tsx | 78 ++++++++ .../src/DatasetReportRenderer.tsx | 52 ++++- ...ReportRenderer.drillRefusal-10789.test.tsx | 114 +++++++++++ .../ElementDataSourceGate.tsx | 109 ++++++++++- ...aSourceGate.malformedFilter-10789.test.tsx | 115 +++++++++++ .../react/src/hooks/useElementDataSource.ts | 40 +++- 18 files changed, 1269 insertions(+), 72 deletions(-) create mode 100644 packages/components/src/__tests__/record-picker-malformed-filter-10789.test.tsx create mode 100644 packages/core/src/utils/__tests__/filter-or-absorb-order-10789.test.ts create mode 100644 packages/fields/src/widgets/pickerMalformedFilter-10789.test.tsx create mode 100644 packages/plugin-charts/src/__tests__/ObjectChart.drillRefusal-10789.test.tsx create mode 100644 packages/plugin-dashboard/src/__tests__/drillRefusal-10789.test.tsx create mode 100644 packages/plugin-list/src/__tests__/ListView.exportRefusal-10789.test.tsx create mode 100644 packages/plugin-report/src/__tests__/DatasetReportRenderer.drillRefusal-10789.test.tsx create mode 100644 packages/react/src/element-data-source/__tests__/ElementDataSourceGate.malformedFilter-10789.test.tsx diff --git a/packages/components/src/__tests__/record-picker-malformed-filter-10789.test.tsx b/packages/components/src/__tests__/record-picker-malformed-filter-10789.test.tsx new file mode 100644 index 0000000000..0298126ec1 --- /dev/null +++ b/packages/components/src/__tests__/record-picker-malformed-filter-10789.test.tsx @@ -0,0 +1,76 @@ +/** + * 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. + * + * `element:record_picker` reports a refused binding filter instead of throwing + * out of render — objectui#10789. + * + * The picker composes its binding through `useElementDataSource`, whose render + * `useMemo` called `composeElementDataSource` — and that merge of the saved + * view's filter with the binding's own used the THROWING converter form. A + * malformed binding filter beside a view filter threw a `FilterOperatorError` + * into the nearest error boundary. + * + * The hook now reports it as `missing` with the refusal, which is the state + * this block already answers with its configuration-error panel — so the + * picker says what is wrong, and never queries without the filter. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { render, waitFor } from '@testing-library/react'; +import React from 'react'; +import { AdapterCtx, SchemaRenderer } from '@object-ui/react'; +// Registers `element:record_picker` at module scope (the objectui#3010 rule). +import '../renderers'; + +const HOT_VIEW = { name: 'hot', filter: [['rating', '=', 'hot']] }; + +function makeAdapter() { + return { + find: vi.fn().mockResolvedValue({ data: [{ id: 'a1', name: 'Acme' }] }), + getObjectSchema: vi.fn().mockResolvedValue({ name: 'account', fields: {}, listViews: { hot: HOT_VIEW } }), + }; +} + +const renderPicker = (schema: Record, adapter: ReturnType) => + render( + + + , + ); + +describe('element:record_picker — a refused binding filter (objectui#10789)', () => { + it('renders the configuration-error panel carrying the refusal, and queries nothing', async () => { + const adapter = makeAdapter(); + const { queryByTestId, getByTestId, container } = renderPicker( + { dataSource: { object: 'account', view: 'hot', filter: { name: { $regex: '^A' } } } }, + adapter, + ); + + await waitFor(() => expect(queryByTestId('record-picker-datasource-error')).not.toBeNull()); + const panel = getByTestId('record-picker-datasource-error'); + // The refusal's own sentence, naming the operator the author wrote. + expect(panel.textContent).toContain('$regex'); + expect(panel.getAttribute('role')).toBe('alert'); + // Not the error boundary's "failed to render" banner. + expect(container.textContent).not.toMatch(/failed to render/i); + // Refused, not dropped: no query without the view's filter. + expect(adapter.find).not.toHaveBeenCalled(); + expect(queryByTestId('record-picker')).toBeNull(); + }); + + it('CONTROL — a well-formed binding filter still queries with both filters', async () => { + const adapter = makeAdapter(); + renderPicker( + { dataSource: { object: 'account', view: 'hot', filter: { name: 'Acme' } } }, + adapter, + ); + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + const json = JSON.stringify(adapter.find.mock.calls[0][1].$filter); + expect(json).toContain('rating'); + expect(json).toContain('Acme'); + }); +}); diff --git a/packages/core/src/utils/__tests__/filter-or-absorb-order-10789.test.ts b/packages/core/src/utils/__tests__/filter-or-absorb-order-10789.test.ts new file mode 100644 index 0000000000..c129500338 --- /dev/null +++ b/packages/core/src/utils/__tests__/filter-or-absorb-order-10789.test.ts @@ -0,0 +1,72 @@ +/** + * 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. + */ + +/** + * A TRUE disjunct no longer hides a refused sibling — objectui#10789, item 4. + * + * ## What was wrong + * + * `lowerLogicalGroup` returned `undefined` (the TRUE identity, objectui#5322) + * the moment an `$or` member lowered to nothing, so every member AFTER it was + * never read. The refusal of a malformed member therefore depended on MEMBER + * ORDER: `{ $or: [{ a: {} }, {}] }` was refused (objectui#9164), while + * `{ $or: [{}, { a: {} }] }` answered TRUE — every row — for the same two + * members. One authored filter, two fates, decided by the order the author + * happened to write the disjuncts in. + * + * ## What the assertions are + * + * - §1 — both member orders are refused, on the ENVELOPE (`code`, + * `httpStatus`, the `field` subject), never on `toThrow()` alone. + * - §2 — the absorption itself is unchanged: a TRUE disjunct beside members + * that lower still answers TRUE, in both orders. + */ + +import { describe, it, expect } from 'vitest'; +import { convertFiltersToAST, FilterOperatorError } from '../filter-converter'; + +function refusalOf(call: () => unknown): { thrown: unknown; returned: unknown } { + try { + return { thrown: undefined, returned: call() }; + } catch (error) { + return { thrown: error, returned: undefined }; + } +} + +describe('objectui#10789 — a refused `$or` member is refused in EITHER member order', () => { + it.each([ + ['TRUE disjunct first', { $or: [{}, { a: {} }] }], + ['TRUE disjunct last', { $or: [{ a: {} }, {}] }], + ['TRUE-identity group first', { $or: [{ $and: [] }, { a: {} }] }], + ])('%s — refused with the INVALID_FILTER envelope naming the field', (_label, filter) => { + const { thrown, returned } = refusalOf(() => convertFiltersToAST(filter)); + expect(returned).toBeUndefined(); + expect(thrown).toBeInstanceOf(FilterOperatorError); + const error = thrown as FilterOperatorError; + expect(error.code).toBe('INVALID_FILTER'); + expect(error.httpStatus).toBe(400); + expect(error.field).toBe('a'); + }); + + it('a non-object member after a TRUE disjunct is refused too', () => { + // The member-shape check sits in the same loop the early return cut short. + const { thrown } = refusalOf(() => convertFiltersToAST({ $or: [{}, 5] as unknown[] })); + expect(thrown).toBeInstanceOf(FilterOperatorError); + expect((thrown as FilterOperatorError).code).toBe('INVALID_FILTER'); + expect((thrown as FilterOperatorError).operator).toBe('$or'); + }); +}); + +describe('objectui#10789 — CONTROL: a TRUE disjunct still absorbs its `$or`', () => { + it.each([ + ['TRUE disjunct first', { $or: [{}, { status: 'open' }] }], + ['TRUE disjunct last', { $or: [{ status: 'open' }, {}] }], + ])('%s — lowers to the TRUE identity', (_label, filter) => { + expect(convertFiltersToAST(filter)).toBeUndefined(); + }); +}); diff --git a/packages/core/src/utils/filter-converter.ts b/packages/core/src/utils/filter-converter.ts index 084c1d1a1c..5fdf8b4537 100644 --- a/packages/core/src/utils/filter-converter.ts +++ b/packages/core/src/utils/filter-converter.ts @@ -236,6 +236,13 @@ function lowerLogicalGroup( } const children: FilterNode[] = []; + // Set when an `$or` member is the TRUE identity. Recorded, not returned + // on the spot (objectui#10789): the members AFTER it still have to be read, + // or a malformed one is refused only when it happens to come FIRST — + // `{ $or: [{ a: {} }, {}] }` refused while `{ $or: [{}, { a: {} }] }` + // answered TRUE, the same two members with two fates. The absorption itself + // is unchanged; it is decided after the loop, once every member was judged. + let absorbed = false; for (const child of value) { if (child === null || typeof child !== 'object' || Array.isArray(child)) { throw new FilterOperatorError( @@ -261,12 +268,15 @@ function lowerLogicalGroup( // child either way: an object in AST child position makes `isFilterAST` // false (measured), and the wire face answers `400 INVALID_FILTER` for // the whole filter. - if (keyword === 'or') return undefined; + if (keyword === 'or') absorbed = true; continue; } children.push(lowered as FilterNode); } + // A TRUE disjunct absorbs its `$or` (#5322) — now that every member was read. + if (absorbed) return undefined; + // Every conjunct reduced to TRUE, so the `$and` constrains nothing. if (children.length === 0) return undefined; diff --git a/packages/fields/src/widgets/PeoplePicker.tsx b/packages/fields/src/widgets/PeoplePicker.tsx index 1b3a98f451..cc62462a29 100644 --- a/packages/fields/src/widgets/PeoplePicker.tsx +++ b/packages/fields/src/widgets/PeoplePicker.tsx @@ -33,7 +33,7 @@ import { useRecordQuery } from './useRecordQuery.js'; // The repo's single filter sink — conjoins filter sources under one `and` // instead of spreading them, so an id restriction can never overwrite a // declared filter on the same field (#5195). -import { mergeFilterNodes } from '@object-ui/core'; +import { mergeFilterNodes, toFilterNodeSafely, type FilterNodeResult } from '@object-ui/core'; import { usePermissions } from '@object-ui/permissions'; import { lookupFiltersToRecord } from './RecordPickerDialog.js'; import { getPersonId, getPersonNameFields } from './personDisplay.js'; @@ -285,17 +285,28 @@ export function PeoplePicker({ expand: effectiveExpand, }); - const recentFilter = useMemo(() => { + // ⚠️ `baseFilter` is lowered through `toFilterNodeSafely` before the merge — + // objectui#10789. This is a RENDER-time `useMemo`, and the lowering refuses a + // malformed authored filter (`lookupFilters`, the host's `baseFilter`) with a + // `FilterOperatorError`: uncaught, that threw out of render into the error + // boundary. The refusal is kept as a VALUE: the recents query does not run + // (never "no filter", which would offer every record), and the candidate + // area below reports it in the error slot the main query's own refusal + // reaches through `dataSource.find`. + const recentFilter = useMemo(() => { const idRestriction = { [idField]: { $in: recentIds } }; - return baseFilter ? mergeFilterNodes(baseFilter, idRestriction) : idRestriction; + if (!baseFilter) return { ok: true, node: idRestriction }; + const lowered = toFilterNodeSafely(baseFilter); + return lowered.ok ? { ok: true, node: mergeFilterNodes(lowered.node, idRestriction) } : lowered; }, [baseFilter, idField, recentIds]); + const recentFilterRefusal = recentFilter.ok ? undefined : recentFilter.refusal; const recentQuery = useRecordQuery({ dataSource, objectName, - enabled: open && recentIds.length > 0, + enabled: open && recentIds.length > 0 && recentFilter.ok, pageSize: Math.max(1, recentIds.length), - filter: recentFilter, + filter: recentFilter.ok ? recentFilter.node : undefined, expand: effectiveExpand, }); @@ -497,12 +508,15 @@ export function PeoplePicker({ [navList, activeIndex, handleRowSelect, query.search, multiple, selectedRecords, handleRemove, idField], ); + // The candidate area's error: the main query's own failure, else the refusal + // of the declared filter the recents merge could not lower (objectui#10789). + const candidateError = query.error ?? recentFilterRefusal?.message ?? null; const initialLoading = - query.loading && !query.error && query.records.length === 0 && recentRecords.length === 0; + query.loading && !candidateError && query.records.length === 0 && recentRecords.length === 0; const refetching = query.loading && !initialLoading; const isEmpty = !query.loading && - !query.error && + !candidateError && resultRecords.length === 0 && recentRecords.length === 0; @@ -563,13 +577,20 @@ export function PeoplePicker({ aria-busy={query.loading} className={cn('flex flex-col gap-0.5 pr-2 transition-opacity', refetching && 'opacity-70')} > - {query.error ? ( -
+ {candidateError ? ( +
- {query.error} - + {candidateError} + {/* A retry re-runs a failed READ; it cannot repair an authored + filter this picker refused before reading. */} + {query.error ? ( + + ) : null}
) : initialLoading ? ( Array.from({ length: SKELETON_ROWS }).map((_, i) => ( diff --git a/packages/fields/src/widgets/RecordPickerDialog.tsx b/packages/fields/src/widgets/RecordPickerDialog.tsx index bdbe7c1a88..d5ed88d48f 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, toPredicateRecord } from '@object-ui/core'; +import { buildExpandFields, mergeFilterNodes, toFilterNodeSafely, toPredicateRecord, type FilterNodeResult } from '@object-ui/core'; import { useSafeFieldLabel, useDisplayLocale } from '@object-ui/i18n'; import { usePermissions } from '@object-ui/permissions'; import { useFieldTranslation } from './useFieldTranslation.js'; @@ -690,7 +690,16 @@ export function RecordPickerDialog({ // // 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(() => { + // + // ⚠️ Each side is lowered through `toFilterNodeSafely` before the merge — + // objectui#10789. This is a RENDER-time `useMemo`, and the lowering refuses a + // malformed authored rule (an `add.picker.filter` rule the converter cannot + // carry) or record condition with a `FilterOperatorError`: uncaught, that + // threw out of render into the error boundary. The refusal is kept as a + // VALUE: the query does not run (never "no filter", which would list every + // record) and the dialog's error state below reports it — the state a + // refusal already reaches when the ADAPTER lowers the record form in `find`. + const mergedFilter = useMemo(() => { const lookupBase = lookupFilters?.length ? lookupFiltersToRecord(lookupFilters) : {}; @@ -703,23 +712,31 @@ export function RecordPickerDialog({ : (baseFilter as Record | undefined); const combined = { ...lookupBase, ...userFilter, ...(recordBase ?? {}) }; const record = Object.keys(combined).length > 0 ? combined : undefined; - return rules ? mergeFilterNodes(record, rules) : record; + if (!rules) return { ok: true, node: record }; + const recordNode = toFilterNodeSafely(record); + if (!recordNode.ok) return recordNode; + const rulesNode = toFilterNodeSafely(rules); + if (!rulesNode.ok) return rulesNode; + return { ok: true, node: mergeFilterNodes(recordNode.node, rulesNode.node) }; }, [lookupFilters, effectiveFilterColumns, filterValues, baseFilter]); + const filterRefusal = mergedFilter.ok ? undefined : mergedFilter.refusal; // Shared query kernel: builds params, fetches, and owns records/loading/error/ // total plus the page/search/sort controls. Selection state stays local (above). const query = useRecordQuery({ dataSource, objectName, - enabled: open, + enabled: open && mergedFilter.ok, pageSize, paginate: true, - filter: mergedFilter, + filter: mergedFilter.ok ? mergedFilter.node : undefined, expand, }); // Preserve the previous local names so the handlers and render below are - // unchanged (the migration is a pure refactor). - const { records, loading, error } = query; + // unchanged (the migration is a pure refactor). A refused filter is this + // dialog's error too (objectui#10789): nothing was read, so it is the only one. + const { records, loading } = query; + const error = filterRefusal ? filterRefusal.message : query.error; const totalCount = query.total; const totalPages = query.totalPages; const searchQuery = query.search; @@ -1157,14 +1174,18 @@ export function RecordPickerDialog({

{error}

- + {/* A retry re-runs a failed READ; it cannot repair an authored + filter this dialog refused before reading (objectui#10789). */} + {filterRefusal ? null : ( + + )}
)} diff --git a/packages/fields/src/widgets/pickerMalformedFilter-10789.test.tsx b/packages/fields/src/widgets/pickerMalformedFilter-10789.test.tsx new file mode 100644 index 0000000000..0774fcff96 --- /dev/null +++ b/packages/fields/src/widgets/pickerMalformedFilter-10789.test.tsx @@ -0,0 +1,152 @@ +/** + * 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 two record pickers report a refused filter instead of throwing out of + * render — objectui#10789. + * + * Both merge an authored filter inside a RENDER-time `useMemo` through + * `mergeFilterNodes`, which lowers through the THROWING converter form: + * + * - `PeoplePicker`'s `recentFilter` (the declared `lookupFilters` / + * `baseFilter` beside the recents' id restriction), and + * - `RecordPickerDialog`'s `mergedFilter` (the record-form filters beside a + * spec `ViewFilterRule[]` — an author's `add.picker.filter`). + * + * A malformed authored filter threw a `FilterOperatorError` into the nearest + * error boundary. Each now lowers through `toFilterNodeSafely` and reports the + * refusal in the error state a refusal from `dataSource.find` already reaches, + * and runs no query without the filter. + * + * Assertions are the notice's CONTENT (the operator the refusal names) plus + * the absence of any unfiltered read — never a bare "did not throw". + */ + +import { render, screen, waitFor } from '@testing-library/react'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { PeoplePicker } from './PeoplePicker'; +import { RecordPickerDialog } from './RecordPickerDialog'; +import { pushRecentLookupId } from './recentLookups'; + +beforeEach(() => { + // jsdom has no matchMedia; useIsMobile needs it. Default to desktop width. + Object.defineProperty(window, 'innerWidth', { writable: true, configurable: true, value: 1280 }); + window.matchMedia = ((query: string) => ({ + matches: window.innerWidth < 768, + media: query, + onchange: null, + addEventListener: () => {}, + removeEventListener: () => {}, + addListener: () => {}, + removeListener: () => {}, + dispatchEvent: () => false, + })) as any; + try { + window.localStorage.clear(); + } catch { + /* ignore */ + } +}); + +const rows = [{ id: 'u1', name: 'Amy Lin' }]; + +/** A lenient source: it answers every read, so only the picker can refuse. */ +function makeDataSource() { + return { find: vi.fn(async () => ({ data: rows, total: rows.length })) } as any; +} + +/** + * The reads that carry the recents' id restriction — found by the remembered + * id itself, since the restriction reaches the wire either as the record form + * or lowered into an AST beside the declared filter. + */ +const recentsReads = (ds: any) => + ds.find.mock.calls.filter((c: any[]) => JSON.stringify(c[1]?.$filter ?? null).includes('"u1"')); + +describe('PeoplePicker — a refused declared filter (objectui#10789)', () => { + it('reports the refusal in the candidate area and skips the recents read', async () => { + pushRecentLookupId('sys_user', 'u1'); + const ds = makeDataSource(); + render( + , + ); + + const notice = await screen.findByTestId('people-picker-error'); + expect(notice.textContent).toContain('$regex'); + // The refused filter is never replaced by "no filter" on the recents read. + expect(recentsReads(ds)).toHaveLength(0); + // Nothing to retry: the picker refused before reading. + expect(screen.queryByRole('button', { name: /retry/i })).toBeNull(); + }); + + it('CONTROL — a well-formed declared filter still reads the recents with it', async () => { + pushRecentLookupId('sys_user', 'u1'); + const ds = makeDataSource(); + render( + , + ); + await waitFor(() => expect(recentsReads(ds)).toHaveLength(1)); + const recents = JSON.stringify(recentsReads(ds)[0][1].$filter); + expect(recents).toContain('Amy Lin'); + expect(screen.queryByTestId('people-picker-error')).toBeNull(); + }); +}); + +describe('RecordPickerDialog — a refused add.picker.filter rule (objectui#10789)', () => { + it('reports the refusal in the dialog error state and reads nothing', async () => { + const ds = makeDataSource(); + render( + , + ); + + const alert = await screen.findByRole('alert'); + expect(alert.textContent).toContain('icontains'); + // Refused, not dropped: no unfiltered read of `contacts`. + expect(ds.find).not.toHaveBeenCalled(); + expect(screen.queryByRole('button', { name: /retry/i })).toBeNull(); + }); + + it('CONTROL — a well-formed rule array still reads with the rule lowered', async () => { + const ds = makeDataSource(); + render( + , + ); + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + expect(JSON.stringify(ds.find.mock.calls[0][1].$filter)).toContain('icontains'); + }); +}); diff --git a/packages/plugin-charts/src/ObjectChart.tsx b/packages/plugin-charts/src/ObjectChart.tsx index fe96e1b50a..91c08298d3 100644 --- a/packages/plugin-charts/src/ObjectChart.tsx +++ b/packages/plugin-charts/src/ObjectChart.tsx @@ -3,7 +3,7 @@ import React, { useState, useEffect, useContext, useCallback, useMemo } from 're import { useDataScope, SchemaRendererContext, SchemaRenderer, useDrillNavigation, useFilterScope, ElementDataSourceGate, useDataInvalidation, type ElementDataSourceMapping } from '@object-ui/react'; import { ChartRenderer } from './ChartRenderer'; import { normalizeChartSchema } from './normalizeChartSchema'; -import { ComponentRegistry, chartMeasureKey, isStructuredGroupBy, objectAggregateSpecQuery, humanizeLabel, extractRecords, computeDrillFilter, composeDrillFilter, isDrillEnabled, resolveDrillTitle, resolveFilterPlaceholders, resolveContextTokens, shiftFilterByCompareTo, compareToTrendLabelKey, chartTypeIgnoresCompareTo, buildChartSeries, buildOptionColorMap, deriveDimensionLabelMaps, dimensionOptionTranslator, loadDimensionFieldMeta, relabelDimensions, localizeFieldOptions, elementDataSourceBlock, type DimensionFieldMeta, type CompareToConfig, type DrillEvent, type ChartResultField, type ChartSegmentClickEvent } from '@object-ui/core'; +import { ComponentRegistry, chartMeasureKey, isStructuredGroupBy, objectAggregateSpecQuery, humanizeLabel, extractRecords, computeDrillFilter, composeDrillFilter, FilterOperatorError, isDrillEnabled, resolveDrillTitle, resolveFilterPlaceholders, resolveContextTokens, shiftFilterByCompareTo, compareToTrendLabelKey, chartTypeIgnoresCompareTo, buildChartSeries, buildOptionColorMap, deriveDimensionLabelMaps, dimensionOptionTranslator, loadDimensionFieldMeta, relabelDimensions, localizeFieldOptions, elementDataSourceBlock, type DimensionFieldMeta, type CompareToConfig, type DrillEvent, type ChartResultField, type ChartSegmentClickEvent } from '@object-ui/core'; import { Sheet, SheetContent, SheetHeader, SheetTitle, Dialog, DialogContent, DialogHeader, DialogTitle, RefreshIndicator, Button, ChartSkeleton, DataEmptyState } from '@object-ui/components'; import { AlertCircle, ArrowUpRight, Inbox } from 'lucide-react'; import { builtinAggregateLabels, useSafeFieldLabel, useSafeTranslate, useObjectTranslation, pickLocalized } from '@object-ui/i18n'; @@ -1083,14 +1083,47 @@ export const ObjectChart = (props: ObjectChartProps) => { // this chart is scoped to exclude (objectui#8944). The seam's docblock names // the composition rule (`widget.filter ∧ drill.filter`, via the repo's single // filter sink `mergeFilterNodes`); it is not decided here. - const drillFilter = useMemo(() => { + // + // ⚠️ The composition lowers both filters through the throwing converter + // form, inside this RENDER-time `useMemo` — objectui#10789. A widget filter + // this layer refuses can still have drawn the chart (the ObjectStack + // adapter's aggregate route hands an OBJECT filter to the server without + // lowering it here), and the click then threw out of render into the error + // boundary, taking the chart with it. The refusal is kept as a VALUE: no drawer opens and nothing is + // navigated to (never with "no filter", which would list every record the + // chart is scoped to exclude), and it is logged, naming the operator — the + // channel a failed drill already reports on (`ReportView`'s drill handler) + // and the one `ValueDataSource` logs a refused filter on. Only a + // `FilterOperatorError` is caught, the rule `toFilterNodeSafely` states. + const drillComposition = useMemo((): + | { ok: true; filter: Record | undefined } + | { ok: false; refusal: FilterOperatorError } + | undefined => { if (!drillEvent) return undefined; - return composeDrillFilter( - schema.filter, - computeDrillFilter(drillDown, drillEvent, { groupByField }), - ); + try { + return { + ok: true, + filter: composeDrillFilter( + schema.filter, + computeDrillFilter(drillDown, drillEvent, { groupByField }), + ), + }; + } catch (error) { + if (!(error instanceof FilterOperatorError)) throw error; + return { ok: false, refusal: error }; + } // eslint-disable-next-line react-hooks/exhaustive-deps }, [drillEvent, drillDown, groupByField, filterKey]); + const drillFilter = drillComposition?.ok ? drillComposition.filter : undefined; + const drillRefused = drillComposition?.ok === false; + // Keyed on the MESSAGE, a primitive: one warning per refused declaration, + // from an effect rather than from render. + const drillRefusalMessage = drillComposition?.ok === false ? drillComposition.refusal.message : undefined; + useEffect(() => { + if (drillRefusalMessage) { + console.warn(`[ObjectChart] drill-down refused — the drilled list cannot be scoped: ${drillRefusalMessage}`); + } + }, [drillRefusalMessage]); // `target: 'navigate'` — skip the in-place peek and send the user straight to // the object's full list page, scoped by the same filter (objectui#3354). @@ -1109,6 +1142,7 @@ export const ObjectChart = (props: ObjectChartProps) => { const navigateOnly = !onSegmentClick && !!drillEvent && + !drillRefused && !!schema.objectName && drillDown?.target === 'navigate' && !!openRecordList; @@ -1486,7 +1520,7 @@ export const ObjectChart = (props: ObjectChartProps) => { // `navigateOnly` renders nothing: the effect above has already handed the // drill to the host's list page, so the in-place drawer must not flash. - const drillDrawer = !onSegmentClick && drillEvent && schema.objectName && !navigateOnly ? (() => { + const drillDrawer = !onSegmentClick && drillEvent && !drillRefused && schema.objectName && !navigateOnly ? (() => { const merged = drillFilter ?? {}; // `schema.title` is the drill drawer's heading FALLBACK, and it is not a // plain string: `@objectstack/spec`'s `ChartConfigSchema.title` is diff --git a/packages/plugin-charts/src/__tests__/ObjectChart.drillRefusal-10789.test.tsx b/packages/plugin-charts/src/__tests__/ObjectChart.drillRefusal-10789.test.tsx new file mode 100644 index 0000000000..73b003e9ae --- /dev/null +++ b/packages/plugin-charts/src/__tests__/ObjectChart.drillRefusal-10789.test.tsx @@ -0,0 +1,108 @@ +/** + * 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. + */ + +/** + * A segment click whose drill filter this layer refuses is reported, not + * thrown out of render — objectui#10789. + * + * `ObjectChart` composes its own `filter` with the clicked category inside a + * RENDER-time `useMemo` (`composeDrillFilter` → `mergeFilterNodes`), which + * lowers through the THROWING converter form. A spec `$not` in the widget + * filter is refused by this layer's converter, so the click that set the + * drill event re-rendered the chart into a `FilterOperatorError` — the error + * boundary instead of the chart. + * + * Pinned on both targets, because each had its own way of going wrong once the + * throw was caught: the drawer must not open, and `navigate` must not hand the + * host a filter-less list (`openRecordList(object, undefined)` lists every + * record the chart is scoped to exclude). The refusal is logged, naming the + * operator. + */ +import React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup, fireEvent } from '@testing-library/react'; +import { DrillNavigationProvider } from '@object-ui/react'; +import type { ObjectChartSchema } from '@object-ui/types'; + +vi.mock('../ChartRenderer', () => ({ + ChartRenderer: ({ onChartClick }: any) => ( + + ), +})); + +import { ObjectChart } from '../ObjectChart'; + +const OBJECT = 'crm_opportunity'; + +let warn: ReturnType; +beforeEach(() => { + vi.stubGlobal('fetch', vi.fn(async () => ({ ok: true, json: async () => ({}) }))); + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +function renderChart(filter: ObjectChartSchema['filter'], target: 'drawer' | 'navigate') { + const openRecordList = vi.fn(); + render( + + ({ data: [] }) }} + /> + , + ); + return openRecordList; +} + +const refusalWarnings = () => + warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes('drill-down refused')); + +/** `$not` — declared by the spec, refused by this layer's converter. */ +const REFUSED = { $not: { region: 'apac' } } as unknown as ObjectChartSchema['filter']; + +describe('ObjectChart — a refused drill filter (objectui#10789)', () => { + it.each(['drawer', 'navigate'] as const)('target %s — the chart stays, nothing opens, the refusal is logged', (target) => { + const openRecordList = renderChart(REFUSED, target); + fireEvent.click(screen.getByTestId('fake-segment')); + + // The chart is still mounted: the click did not throw it into a boundary. + expect(screen.getByTestId('fake-segment')).toBeTruthy(); + expect(screen.queryByTestId('chart-drill-body')).toBeNull(); + expect(openRecordList).not.toHaveBeenCalled(); + expect(refusalWarnings()).toHaveLength(1); + expect(refusalWarnings()[0]).toContain('$not'); + }); + + it('CONTROL — a well-formed widget filter still drills', () => { + const openRecordList = renderChart({ region: 'emea' } as ObjectChartSchema['filter'], 'navigate'); + fireEvent.click(screen.getByTestId('fake-segment')); + expect(openRecordList).toHaveBeenCalledWith(OBJECT, { + $and: [{ region: 'emea' }, { stage: 'won' }], + }); + expect(refusalWarnings()).toHaveLength(0); + }); +}); diff --git a/packages/plugin-dashboard/src/DatasetWidget.tsx b/packages/plugin-dashboard/src/DatasetWidget.tsx index 19d85e1ea2..40b8427ccc 100644 --- a/packages/plugin-dashboard/src/DatasetWidget.tsx +++ b/packages/plugin-dashboard/src/DatasetWidget.tsx @@ -47,6 +47,7 @@ import { formatDimensionValue, buildDatasetFieldHelpers, buildDatasetDrillFilter, + FilterOperatorError, // The pivot key encoders now live in `@object-ui/core` so this widget and the // report renderer's cross-tab share ONE implementation — each having written // its own is why the same collision had to be fixed twice (objectstack#5473, @@ -881,7 +882,22 @@ export function DatasetWidget({ widget, dataSource, subCaption }: { widget: any; const canDrill = !!drillObject && (drillDims.length > 0 || !!drillRanges?.length); const openDrill = (index: number, title: string) => { if (!drillObject) return; - const merged = buildDrillFilter(drillRawRows?.[index], drillDims, dimensionFields ?? {}, runtimeFilter, drillRanges?.[index]); + // objectui#10789 — composing the widget's filter with the clicked bucket + // lowers both through the throwing converter form. A widget filter the + // dataset query carried to the server can still be one this layer refuses + // (a spec `$not`), and the click then threw out of this handler uncaught. + // The refusal opens no drawer (never with "no filter", which would list + // every record the widget is scoped to exclude) and is logged, naming the + // operator — the channel a failed drill already reports on (`ReportView`'s + // drill handler). Only a `FilterOperatorError` is caught. + let merged: Record; + try { + merged = buildDrillFilter(drillRawRows?.[index], drillDims, dimensionFields ?? {}, runtimeFilter, drillRanges?.[index]); + } catch (error) { + if (!(error instanceof FilterOperatorError)) throw error; + console.warn(`[DatasetWidget] drill-down refused — the drilled list cannot be scoped: ${error.message}`); + return; + } setDrill({ filter: merged, title: title || String(widget?.title ?? '') }); }; const drillDrawer = drill && drillObject ? ( diff --git a/packages/plugin-dashboard/src/ObjectPivotTable.tsx b/packages/plugin-dashboard/src/ObjectPivotTable.tsx index eeba53e84a..231be1b2ea 100644 --- a/packages/plugin-dashboard/src/ObjectPivotTable.tsx +++ b/packages/plugin-dashboard/src/ObjectPivotTable.tsx @@ -6,10 +6,10 @@ * LICENSE file in the root directory of this source tree. */ -import React, { useState, useEffect, useContext } from 'react'; +import React, { useState, useEffect, useContext, useMemo } from 'react'; import { useDataScope, SchemaRendererContext, useFilterScope } from '@object-ui/react'; import { useSafeFieldLabel } from '@object-ui/i18n'; -import { extractRecords, computeDrillFilter, composeDrillFilter, isDrillEnabled, resolveDrillTitle, type DrillEvent } from '@object-ui/core'; +import { extractRecords, computeDrillFilter, composeDrillFilter, FilterOperatorError, isDrillEnabled, resolveDrillTitle, type DrillEvent } from '@object-ui/core'; import { Skeleton, cn } from '@object-ui/components'; import { PivotTable } from './PivotTable'; import { DrillDownDrawer } from './DrillDownDrawer'; @@ -235,6 +235,46 @@ export const ObjectPivotTable: React.FC = ({ schema, data return () => { isMounted = false; }; }, [schema.objectName, dataSource, boundData, schema.data, schema.filter, filterScope]); + // --- Drill-down composition -------------------------------------------- + // The filter the drilled list is scoped by: this pivot's own filter narrowed + // by the clicked cell (see `renderDrillDrawer` below for why it is composed + // and never spread). Computed HERE, above the early returns, so the refusal + // below can be reported from an effect rather than from render. + // + // ⚠️ The composition lowers both filters through the throwing converter + // form — objectui#10789. It ran inside `renderDrillDrawer`, i.e. during + // RENDER after a click, so a pivot filter this layer refuses (one that drew + // the pivot from inline or bound rows, which nothing lowers here) threw into + // the error boundary and took the pivot with it. The refusal is kept as a + // VALUE: no drawer opens and nothing is navigated to (never with "no + // filter", which would list every record the pivot is scoped to exclude), + // and it is logged, naming the operator — the channel a failed drill already + // reports on (`ReportView`'s drill handler). Only a `FilterOperatorError` is + // caught, the rule `toFilterNodeSafely` states. + const drillComposition = useMemo((): + | { ok: true; filter: Record | undefined } + | { ok: false; refusal: FilterOperatorError } + | undefined => { + if (!drillEvent) return undefined; + try { + const clicked = computeDrillFilter(schema.drillDown, drillEvent, { + rowField: schema.rowField, + columnField: schema.columnField, + }); + return { ok: true, filter: composeDrillFilter(schema.filter, clicked) }; + } catch (error) { + if (!(error instanceof FilterOperatorError)) throw error; + return { ok: false, refusal: error }; + } + }, [drillEvent, schema.drillDown, schema.rowField, schema.columnField, schema.filter]); + // Keyed on the MESSAGE, a primitive: one warning per refused declaration. + const drillRefusalMessage = drillComposition?.ok === false ? drillComposition.refusal.message : undefined; + useEffect(() => { + if (drillRefusalMessage) { + console.warn(`[ObjectPivotTable] drill-down refused — the drilled list cannot be scoped: ${drillRefusalMessage}`); + } + }, [drillRefusalMessage]); + // Resolve data: bound data > static schema data > fetched data const rawData = boundData || schema.data || fetchedData; const finalData = Array.isArray(rawData) ? rawData : EMPTY_ROWS; @@ -317,11 +357,8 @@ export const ObjectPivotTable: React.FC = ({ schema, data : undefined; const renderDrillDrawer = () => { - if (!drillEvent || !schema.objectName) return null; - const baseFilter = computeDrillFilter(drillDown, drillEvent, { - rowField: schema.rowField, - columnField: schema.columnField, - }); + // A refused composition opens nothing (objectui#10789, above). + if (!drillEvent || !schema.objectName || !drillComposition?.ok) return null; // ⛔ Composed through `composeDrillFilter`, NOT by spreading this pivot's // own filter into an object literal. `schema.filter` reaches this component // in BOTH dialects and a spread is only correct for the second: @@ -345,7 +382,7 @@ export const ObjectPivotTable: React.FC = ({ schema, data // names the composition rule (`widget.filter ∧ drill.filter`, via the // repo's single filter sink `mergeFilterNodes`); it is not decided here. // `ObjectChart` was routed through the same seam by objectui#8944. - const merged = composeDrillFilter(schema.filter, baseFilter); + const merged = drillComposition.filter; const title = resolveDrillTitle(drillDown, drillEvent, schema.title || 'Details'); return ( = []; +vi.mock('../DrillDownDrawer', () => ({ + DrillDownDrawer: ({ filter }: { filter: unknown }) => { + drawerProps.push({ filter }); + return
; + }, +})); + +import { ObjectPivotTable } from '../ObjectPivotTable'; +import { DatasetWidget } from '../DatasetWidget'; + +/** `$not` — declared by the spec, refused by this layer's converter. */ +const REFUSED = { $not: { region: 'apac' } }; + +let capturedChartProps: any = null; +beforeAll(() => { + ComponentRegistry.register('chart', (props: any) => { + capturedChartProps = props; + return null; + }); +}); + +let warn: ReturnType; +beforeEach(() => { + vi.stubGlobal('fetch', vi.fn(async () => ({ ok: true, json: async () => ({}) }))); + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); +afterEach(() => { + cleanup(); + capturedChartProps = null; + drawerProps.length = 0; + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +const refusalWarnings = () => + warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes('drill-down refused')); + +const ROWS = [ + { id: 'a', stage: 'won', source: 'web', region: 'emea', amount: 10 }, + { id: 'b', stage: 'won', source: 'web', region: 'apac', amount: 20 }, +]; + +function renderPivot(filter: unknown, target: 'drawer' | 'navigate') { + const openRecordList = vi.fn(); + render( + + ({ data: [] })) }} + /> + , + ); + return openRecordList; +} + +describe('ObjectPivotTable — a refused drill filter (objectui#10789)', () => { + it.each(['drawer', 'navigate'] as const)('target %s — the pivot stays, nothing opens, the refusal is logged', (target) => { + const openRecordList = renderPivot(REFUSED, target); + fireEvent.click(screen.getByLabelText('Drill into stage=won, source=web')); + + // Still mounted: the click did not throw the pivot into a boundary. + expect(screen.getByLabelText('Drill into stage=won, source=web')).toBeTruthy(); + expect(drawerProps).toHaveLength(0); + expect(openRecordList).not.toHaveBeenCalled(); + expect(refusalWarnings()).toHaveLength(1); + expect(refusalWarnings()[0]).toContain('$not'); + }); + + it('CONTROL — a well-formed pivot filter still opens the drawer, scoped', () => { + renderPivot({ region: 'emea' }, 'drawer'); + fireEvent.click(screen.getByLabelText('Drill into stage=won, source=web')); + expect(drawerProps.length).toBeGreaterThan(0); + expect(JSON.stringify(drawerProps[0].filter)).toContain('emea'); + expect(refusalWarnings()).toHaveLength(0); + }); +}); + +function renderDatasetWidget(filter: unknown) { + const dataSource = { + queryDataset: vi.fn(async () => ({ + rows: [{ status: 'Open', est_hours: 3 }], + fields: [ + { name: 'status', type: 'text', label: 'Status' }, + { name: 'est_hours', type: 'number', label: 'Hours' }, + ], + object: 'proj_task', + dimensionFields: { status: 'status_id' }, + drillRawRows: [{ status: 'open' }], + })), + }; + render( + + + , + ); +} + +/** Click the first drawn bucket of the first series, the way the chart would. */ +const clickFirstSegment = () => { + const series = capturedChartProps.schema.series[0]; + const row = capturedChartProps.schema.data[0]; + const ev: ChartSegmentClickEvent = { + category: String(row[capturedChartProps.schema.xAxisKey]), + categoryId: chartRowBucketId(row), + series: series.dataKey, + seriesLabel: series.label, + value: row[series.dataKey], + }; + capturedChartProps.onSegmentClick(ev); +}; + +describe('DatasetWidget — a refused drill filter (objectui#10789)', () => { + it('opens no drawer and logs the refusal instead of throwing from the click', async () => { + renderDatasetWidget(REFUSED); + await waitFor(() => expect(capturedChartProps?.onSegmentClick).toBeTypeOf('function')); + + expect(() => clickFirstSegment()).not.toThrow(); + expect(screen.queryByTestId('drill-drawer')).toBeNull(); + expect(drawerProps).toHaveLength(0); + expect(refusalWarnings()).toHaveLength(1); + expect(refusalWarnings()[0]).toContain('$not'); + }); + + it('CONTROL — a well-formed widget filter still opens the drawer, scoped', async () => { + renderDatasetWidget({ owner: 'me' }); + await waitFor(() => expect(capturedChartProps?.onSegmentClick).toBeTypeOf('function')); + React.act(() => clickFirstSegment()); + await waitFor(() => expect(drawerProps.length).toBeGreaterThan(0)); + expect(JSON.stringify(drawerProps[drawerProps.length - 1].filter)).toContain('owner'); + expect(refusalWarnings()).toHaveLength(0); + }); +}); diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index dffb7bf592..979cdf752a 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -4027,9 +4027,6 @@ export const ListView = React.forwardRef(({ .map((f: any) => columnIdentity(f)) .filter(Boolean) as string[]; - // The same three filter sources as the data fetch, from the same function. - const finalFilter = buildEffectiveFilter(authoredFilter, appliedFilters, appliedUserFilterConditions); - const sort = currentSort.length > 0 ? currentSort .filter(item => item.field) @@ -4040,6 +4037,14 @@ export const ListView = React.forwardRef(({ setExportBusy(true); void (async () => { try { + // The same three filter sources as the data fetch, from the same + // function — built INSIDE this `try` (objectui#10789). The lowering + // refuses a malformed filter with a `FilterOperatorError`, and built + // before it, the refusal threw out of the Export click uncaught: no + // file, no message. Here it lands in the catch below, which names it + // in the toolbar the way the data fetch's own `try` names it in the + // load-error panel — and nothing is exported unfiltered. + const finalFilter = buildEffectiveFilter(authoredFilter, appliedFilters, appliedUserFilterConditions); const blob = await dataSource!.exportDownload!(schema.objectName!, { format: format as 'csv' | 'xlsx' | 'json', fields: fields.length ? fields : undefined, diff --git a/packages/plugin-list/src/__tests__/ListView.exportRefusal-10789.test.tsx b/packages/plugin-list/src/__tests__/ListView.exportRefusal-10789.test.tsx new file mode 100644 index 0000000000..5dd61277e4 --- /dev/null +++ b/packages/plugin-list/src/__tests__/ListView.exportRefusal-10789.test.tsx @@ -0,0 +1,78 @@ +/** + * 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 server export reports a refused filter in the toolbar instead of + * throwing out of the Export click — objectui#10789. + * + * The export builds its filter with `buildEffectiveFilter`, the same function + * the data fetch uses, and that lowering refuses a malformed filter with a + * `FilterOperatorError`. The fetch calls it inside its load `try` (the + * load-error panel names the refusal); the export called it BEFORE its `try`, + * so the refusal escaped the click handler: no file and no message, while the + * toolbar's Export stayed clickable above the load-error panel. + * + * Asserted on the message's CONTENT (the refused operator) and on the absence + * of any export request — never on a bare "did not throw". + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, fireEvent, screen } from '@testing-library/react'; +import { ListView } from '../ListView'; +import { SchemaRendererProvider } from '@object-ui/react'; +import type { ListViewSchema } from '@object-ui/types'; + +function harness(schema: ListViewSchema) { + const exportDownload = vi.fn().mockResolvedValue(new Blob(['x'], { type: 'text/csv' })); + const find = vi.fn().mockResolvedValue({ data: [], total: 0 }); + const ds: any = { find, findOne: vi.fn(), create: vi.fn(), update: vi.fn(), delete: vi.fn(), exportDownload }; + render( + + + , + ); + return { exportDownload }; +} + +async function clickExportCsv() { + fireEvent.click(screen.getByRole('button', { name: /export/i })); + fireEvent.click(await screen.findByRole('button', { name: /export as csv/i })); +} + +const BASE: ListViewSchema = { + type: 'list-view', + objectName: 'account', + viewType: 'grid', + fields: ['name'], + exportOptions: { formats: ['csv'] }, +}; + +describe('ListView export — a refused filter (objectui#10789)', () => { + beforeEach(() => { + vi.spyOn(URL, 'createObjectURL').mockReturnValue('blob:x'); + vi.spyOn(URL, 'revokeObjectURL').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + afterEach(() => vi.restoreAllMocks()); + + it('names the refusal in the export menu and requests no file', async () => { + const { exportDownload } = harness({ ...BASE, filter: { name: { $regex: '^A' } } as any }); + await clickExportCsv(); + + const alert = await screen.findByText(/\$regex/, { selector: '[role="alert"]' }); + expect(alert).toBeTruthy(); + expect(exportDownload).not.toHaveBeenCalled(); + }); + + it('CONTROL — a well-formed filter still exports with it', async () => { + const { exportDownload } = harness({ ...BASE, filter: { name: 'Acme' } as any }); + await clickExportCsv(); + await vi.waitFor(() => expect(exportDownload).toHaveBeenCalledTimes(1)); + expect(exportDownload.mock.calls[0][1]?.filter).toEqual(['name', '=', 'Acme']); + }); +}); diff --git a/packages/plugin-report/src/DatasetReportRenderer.tsx b/packages/plugin-report/src/DatasetReportRenderer.tsx index 0b52e03999..aa9dd26881 100644 --- a/packages/plugin-report/src/DatasetReportRenderer.tsx +++ b/packages/plugin-report/src/DatasetReportRenderer.tsx @@ -64,6 +64,7 @@ import { formatDimensionValue, buildDatasetFieldHelpers, buildDatasetDrillFilter, + FilterOperatorError, relabelDimensions, // The dataset→chart derivation the dashboard and the chart view have always // used, adopted here by objectui#4878: it is where the whole null-category @@ -435,6 +436,35 @@ function useDatasetRows( return state; } +/** + * `buildDatasetDrillFilter`, with a refused filter REPORTED instead of thrown — + * objectui#10789. `null` means refused, and the caller then emits no drill. + * + * Composing the report's `runtimeFilter` with the clicked bucket lowers both + * through the throwing converter form. A `runtimeFilter` the dataset query + * carried to the server can still be one this layer refuses (a spec `$not`), + * and the click then threw out of its handler uncaught. + * + * ⛔ Not answered with `objectFilter: undefined`. The host reads an absent + * `objectFilter` as "older server" and rebuilds the filter from the clicked + * group ALONE (`ReportView`'s fallback), which would drop the `runtimeFilter` + * and drill into records the report is scoped to exclude. So the drill does + * not happen, and the refusal is logged, naming the operator — the channel the + * host's own drill failure reports on. Only a `FilterOperatorError` is caught, + * the rule `toFilterNodeSafely` states. + */ +function drillFilterOrRefusal( + ...args: Parameters +): Record | null { + try { + return buildDatasetDrillFilter(...args); + } catch (error) { + if (!(error instanceof FilterOperatorError)) throw error; + console.warn(`[DatasetReportRenderer] drill-down refused — the drilled list cannot be scoped: ${error.message}`); + return null; + } +} + function EmptyMeasures({ dataset }: { dataset: string }) { return (
@@ -547,10 +577,13 @@ function DatasetReportTable({ // covering a date-ONLY report that has no equality drill dim at all. const rowRanges = state.drillRanges?.[index]; const hasRange = !!rowRanges && Object.keys(rowRanges).length > 0; - const objectFilter = - state.object && (drillDims.length > 0 || hasRange) - ? buildDatasetDrillFilter(state.drillRawRows?.[index], drillDims, state.dimensionFields ?? {}, runtimeFilter, rowRanges) - : undefined; + let objectFilter: Record | undefined; + if (state.object && (drillDims.length > 0 || hasRange)) { + const built = drillFilterOrRefusal(state.drillRawRows?.[index], drillDims, state.dimensionFields ?? {}, runtimeFilter, rowRanges); + // Refused: no drill at all — see `drillFilterOrRefusal`. + if (built === null) return; + objectFilter = built; + } onDrill!({ dataset, groupKey, runtimeFilter, object: state.object, objectFilter }); }; @@ -1310,10 +1343,13 @@ function DatasetMatrixTable({ // to the clicked time bucket, not every bucket in that row/column. const cellRanges = state.drillRanges?.[index]; const hasRange = !!cellRanges && Object.keys(cellRanges).length > 0; - const objectFilter = - state.object && (drillDims.length > 0 || hasRange) - ? buildDatasetDrillFilter(state.drillRawRows?.[index], drillDims, state.dimensionFields ?? {}, runtimeFilter, cellRanges) - : undefined; + let objectFilter: Record | undefined; + if (state.object && (drillDims.length > 0 || hasRange)) { + const built = drillFilterOrRefusal(state.drillRawRows?.[index], drillDims, state.dimensionFields ?? {}, runtimeFilter, cellRanges); + // Refused: no drill at all — see `drillFilterOrRefusal`. + if (built === null) return; + objectFilter = built; + } onDrill!({ dataset, groupKey: { ...rowKey, ...colKey }, runtimeFilter, object: state.object, objectFilter }); }; diff --git a/packages/plugin-report/src/__tests__/DatasetReportRenderer.drillRefusal-10789.test.tsx b/packages/plugin-report/src/__tests__/DatasetReportRenderer.drillRefusal-10789.test.tsx new file mode 100644 index 0000000000..10a38ddb74 --- /dev/null +++ b/packages/plugin-report/src/__tests__/DatasetReportRenderer.drillRefusal-10789.test.tsx @@ -0,0 +1,114 @@ +/** + * 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. + */ + +/** + * A drill whose filter this layer refuses is reported, not thrown — and never + * emitted without the report's scope — objectui#10789. + * + * Both drill handlers (the grouped row and the matrix cell) compose the + * report's `runtimeFilter` with the clicked bucket through + * `buildDatasetDrillFilter` → `composeDrillFilter`, which lowers through the + * THROWING converter form. A spec `$not` — which `@objectstack/spec` declares + * and the dataset query carries to the server — is refused by this layer's + * converter, so the click threw out of its handler uncaught. + * + * ⛔ The repair may not emit the drill with `objectFilter: undefined`: the host + * reads that as an older server and rebuilds the filter from the clicked group + * ALONE (`ReportView`'s fallback), dropping `runtimeFilter` — a drill into + * records the report is scoped to exclude. So the pins assert that NO drill is + * emitted and that the refusal is logged naming the operator. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react'; +import { DatasetReportRenderer } from '../DatasetReportRenderer'; + +/** `$not` — declared by the spec, refused by this layer's converter. */ +const REFUSED_SCOPE = { $not: { owner: 'them' } }; + +function makeSource(rows: Array>, rawRows: Array>) { + return { + queryDataset: vi.fn(async () => ({ + rows, + object: 'task', + dimensionFields: { status: 'status', priority: 'priority' }, + drillRawRows: rawRows, + })), + }; +} + +let warn: ReturnType; +beforeEach(() => { + // The renderer's dimension-label probe reads object metadata over `fetch`. + vi.stubGlobal('fetch', vi.fn(async () => ({ ok: true, json: async () => ({}) }))); + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +const refusalWarnings = () => + warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes('drill-down refused')); + +describe('DatasetReportRenderer — a refused runtimeFilter drills nothing (objectui#10789)', () => { + it('the grouped-row drill logs the refusal and emits no drill', async () => { + const onDrill = vi.fn(); + render( + , + ); + await waitFor(() => expect(screen.getByTestId('dataset-drill-row')).toBeTruthy()); + fireEvent.click(screen.getByTestId('dataset-drill-row')); + + expect(onDrill).not.toHaveBeenCalled(); + expect(refusalWarnings()).toHaveLength(1); + expect(refusalWarnings()[0]).toContain('$not'); + }); + + it('the matrix-cell drill logs the refusal and emits no drill', async () => { + const onDrill = vi.fn(); + render( + , + ); + await waitFor(() => expect(screen.getAllByTestId('dataset-drill-cell').length).toBeGreaterThan(0)); + fireEvent.click(screen.getAllByTestId('dataset-drill-cell')[0]); + + expect(onDrill).not.toHaveBeenCalled(); + expect(refusalWarnings()).toHaveLength(1); + expect(refusalWarnings()[0]).toContain('$not'); + }); + + it('CONTROL — a well-formed runtimeFilter still drills with the scope conjoined', async () => { + const onDrill = vi.fn(); + render( + , + ); + await waitFor(() => expect(screen.getByTestId('dataset-drill-row')).toBeTruthy()); + fireEvent.click(screen.getByTestId('dataset-drill-row')); + expect(onDrill).toHaveBeenCalledWith(expect.objectContaining({ + objectFilter: { $and: [{ owner: 'me' }, { status: 'open' }] }, + })); + expect(refusalWarnings()).toHaveLength(0); + }); +}); diff --git a/packages/react/src/element-data-source/ElementDataSourceGate.tsx b/packages/react/src/element-data-source/ElementDataSourceGate.tsx index f2082ce4c4..da80a98ac5 100644 --- a/packages/react/src/element-data-source/ElementDataSourceGate.tsx +++ b/packages/react/src/element-data-source/ElementDataSourceGate.tsx @@ -103,11 +103,15 @@ import * as React from 'react'; import { elementDataSourceRefusedLimitMessage, + filterRefusalSubject, isElementDataSourceConfig, mergeFilterNodes, + toFilterNodeSafely, type ElementDataSourceConfig, type ElementSavedView, + type FilterOperatorError, } from '@object-ui/core'; +import { useObjectTranslation, en } from '@object-ui/i18n'; import { useElementDataSource, type ElementDataSourceStatus, @@ -174,6 +178,15 @@ export interface UseElementDataSourceSchemaResult { config?: ElementDataSourceConfig; /** Author-facing explanation, set only for `missing`. */ error?: string; + /** + * The refused filter that stopped the binding from applying — set only with + * `missing`, and `error` then carries its message (objectui#10789). Either + * merge can refuse: the view's filter with the binding's own + * ({@link useElementDataSource}), or the component's filter with that result + * (this hook). {@link ElementDataSourceGate} draws the malformed-filter + * notice from it. + */ + filterRefusal?: FilterOperatorError; } /** @@ -368,6 +381,7 @@ export function useElementDataSourceSchema( capMessage: string | null; viewCapMessage: string | null; bindingCapMessage: string | null; + filterRefusal?: FilterOperatorError; } => { const composed = binding.composed; // BY REFERENCE when there is nothing to apply — a fresh object every render @@ -394,7 +408,21 @@ export function useElementDataSourceSchema( // Component filter AND (view filter AND binding filter). `composed.filter` // already carries the latter pair; a single surviving source comes back // unwrapped, so the common "only the view filters" case stays flat. - const merged = mergeFilterNodes(base.filter, composed.filter); + // + // ⚠️ Each source is lowered through `toFilterNodeSafely` first, the way + // `RelatedList` and `LineItemsPanel` lower their own filter before they + // merge it — objectui#10789. This is a RENDER-time `useMemo`, and the + // lowering refuses malformed authored shapes with a `FilterOperatorError`: + // uncaught, that was a render error, thrown BEFORE the wrapped block + // reached its own safe lowering and its malformed-filter state. The + // refusal is returned as a VALUE and the block is not rendered at all — + // never merged as "no filter", which would run it unconstrained. The + // merge below then only re-reads nodes the lowering already produced. + const own = toFilterNodeSafely(base.filter); + if (!own.ok) return { schema, capMessage: null, viewCapMessage: null, bindingCapMessage: null, filterRefusal: own.refusal }; + const bound = toFilterNodeSafely(composed.filter); + if (!bound.ok) return { schema, capMessage: null, viewCapMessage: null, bindingCapMessage: null, filterRefusal: bound.refusal }; + const merged = mergeFilterNodes(own.node, bound.node); if (merged !== undefined) next.filter = merged; else delete next.filter; } @@ -446,7 +474,7 @@ export function useElementDataSourceSchema( // per render — and it fires from an effect, never from render, which is the // same shape the renderer sites use for "you declared it, we dropped it". // One effect per message, so a change to one never re-emits another. - const { schema: boundSchema, capMessage, viewCapMessage, bindingCapMessage } = mapped; + const { schema: boundSchema, capMessage, viewCapMessage, bindingCapMessage, filterRefusal: mergeRefusal } = mapped; React.useEffect(() => { if (capMessage) console.warn(capMessage); }, [capMessage]); @@ -457,14 +485,26 @@ export function useElementDataSourceSchema( if (bindingCapMessage) console.warn(bindingCapMessage); }, [bindingCapMessage]); + // A refused filter — from either merge — is `missing`, the one state every + // caller already answers by withholding the block (objectui#10789). + const filterRefusal = binding.filterRefusal ?? mergeRefusal; return React.useMemo( - () => ({ - status: binding.status, - schema: boundSchema, - config: binding.config, - error: binding.error, - }), - [binding.status, binding.config, binding.error, boundSchema], + () => + filterRefusal + ? { + status: 'missing' as const, + schema: boundSchema, + config: binding.config, + error: filterRefusal.message, + filterRefusal, + } + : { + status: binding.status, + schema: boundSchema, + config: binding.config, + error: binding.error, + }, + [binding.status, binding.config, binding.error, boundSchema, filterRefusal], ); } @@ -503,6 +543,51 @@ export function ElementDataSourceErrorPanel({ ); } +/** + * The malformed-filter notice, drawn IN PLACE OF the block when a filter the + * gate had to merge is refused (objectui#10789). + * + * It is the notice the wrapped blocks draw themselves (objectui#9050 step 2 — + * `record:related_list`, the line-items panel, `object-grid`): the same + * sentence (`view.malformedFilter`), naming the refused operator or field in a + * separately addressable headline, with the refusal's own message beneath it. + * The gate draws it because its merge refuses FIRST — before the block's own + * safe lowering ever runs — so without it the author would get the error + * boundary's "failed to render" instead of the sentence those blocks give. + * + * ⚠️ A separate component, so `useObjectTranslation` runs only on this path: + * the gate wraps every object-bound block, and a translation hook in the + * gate's own body would run for all of them, including under the many suites + * that mock `@object-ui/i18n` by hand. `en` is dereferenced here, at render, + * for the same reason (see `NonGridRowCeilingNote`). + */ +function ElementDataSourceMalformedFilterPanel({ + testId, + refusal, +}: { + testId: string; + refusal: FilterOperatorError; +}): React.ReactElement { + const { t } = useObjectTranslation(); + return ( +
+ {/* The headline is the half that has to NAME the operator; the technical + line below repeats it incidentally. */} +

+ {t('view.malformedFilter', { + subject: filterRefusalSubject(refusal) ?? '', + defaultValue: en.view.malformedFilter, + })} +

+

{refusal.message}

+
+ ); +} + /** * The "saved views are still being fetched" placeholder. Distinct from the error * panel because a component that treated "not resolved yet" as "does not exist" @@ -728,6 +813,12 @@ export function ElementDataSourceGate({ return ; } + // A refused filter is `missing` too, but it gets the malformed-filter notice + // rather than the "could not be resolved" panel: the binding resolved, and + // what the author has to fix is a filter condition (objectui#10789). + if (bound.filterRefusal) { + return ; + } if (bound.status === 'missing') { return ; } diff --git a/packages/react/src/element-data-source/__tests__/ElementDataSourceGate.malformedFilter-10789.test.tsx b/packages/react/src/element-data-source/__tests__/ElementDataSourceGate.malformedFilter-10789.test.tsx new file mode 100644 index 0000000000..3f5effcdb9 --- /dev/null +++ b/packages/react/src/element-data-source/__tests__/ElementDataSourceGate.malformedFilter-10789.test.tsx @@ -0,0 +1,115 @@ +/** + * 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 gate draws the malformed-filter notice instead of throwing out of render + * — objectui#10789. + * + * The gate lowers filters at two render-time merges, and both called the + * THROWING converter form: + * + * 1. its own merge of the component's `filter` with the composed binding + * (`useElementDataSourceSchema`'s `useMemo`), and + * 2. `composeElementDataSource`'s merge of the saved view's filter with the + * binding's own (`useElementDataSource`'s `useMemo`). + * + * A malformed authored filter therefore threw a `FilterOperatorError` out of + * render, into the nearest error boundary, BEFORE the wrapped block — + * `record:related_list`, `object-grid`, a line-items panel — reached its own + * `toFilterNodeSafely` and drew the malformed-filter state (objectui#9050). + * + * Each merge gets one malformed filter here. The assertions are the notice's + * CONTENT (the operator it names, the refusal's own message) and the refusal's + * envelope (`code` / `httpStatus`), never a bare "did not throw": `render` + * returning at all is the no-throw half, and the block staying unmounted is + * the no-widening half — the block cannot run a query the gate could not build. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { render, renderHook, waitFor } from '@testing-library/react'; +import * as React from 'react'; +import { + ElementDataSourceGate, + useElementDataSourceSchema, + type ElementDataSourceMapping, +} from '../ElementDataSourceGate'; + +/** Refused by the converter since objectui#8530: `$regex` has no AST target. */ +const MALFORMED = { name: { $regex: '^A' } }; + +const HOT_VIEW = { name: 'hot', filter: [['rating', '=', 'hot']] }; + +const makeAdapter = () => ({ + find: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ name: 'account', listViews: { hot: HOT_VIEW } }), +}); + +const MAPPING: ElementDataSourceMapping = { filter: true }; + +function Block({ schema }: { schema: any }) { + return
{JSON.stringify(schema.filter ?? null)}
; +} + +const renderGate = (schema: Record) => + render( + + {(bound) => } + , + ); + +/** The two merges, each fed one malformed filter. */ +const CASES: ReadonlyArray<[string, Record]> = [ + [ + "the gate's own merge — the component's filter", + { filter: MALFORMED, dataSource: { object: 'account' } }, + ], + [ + "composeElementDataSource's merge — the binding's filter beside the view's", + { dataSource: { object: 'account', view: 'hot', filter: MALFORMED } }, + ], +]; + +describe('ElementDataSourceGate — a malformed filter draws the notice (objectui#10789)', () => { + it.each(CASES)('%s', async (_label, schema) => { + const { queryByTestId, getByTestId } = renderGate(schema); + + await waitFor(() => expect(queryByTestId('probe-malformed-filter')).not.toBeNull()); + // The headline NAMES the operator the author has to fix — the sibling + // blocks' sentence, not the "data source could not be resolved" panel. + expect(getByTestId('probe-malformed-filter-subject').textContent).toContain('$regex'); + expect(getByTestId('probe-malformed-filter').getAttribute('role')).toBe('alert'); + // Refused, not dropped: the block never mounts, so it cannot run the query + // unconstrained. + expect(queryByTestId('block')).toBeNull(); + expect(queryByTestId('probe-datasource-error')).toBeNull(); + }); + + it.each(CASES)('%s — the hook reports the refusal as a value', async (_label, schema) => { + const adapter = makeAdapter(); + const { result } = renderHook(() => useElementDataSourceSchema(schema, MAPPING, adapter)); + await waitFor(() => expect(result.current.status).not.toBe('loading')); + + expect(result.current.status).toBe('missing'); + const refusal = result.current.filterRefusal; + expect(refusal?.code).toBe('INVALID_FILTER'); + expect(refusal?.httpStatus).toBe(400); + expect(refusal?.operator).toBe('$regex'); + expect(result.current.error).toBe(refusal?.message); + }); + + it('CONTROL — a well-formed filter still renders the block with the merged filter', async () => { + const { queryByTestId, getByTestId } = renderGate({ + filter: { name: 'Acme' }, + dataSource: { object: 'account', view: 'hot', filter: { amount: { $gt: 100 } } }, + }); + await waitFor(() => expect(queryByTestId('block')).not.toBeNull()); + const merged = getByTestId('block').textContent ?? ''; + expect(merged).toContain('rating'); + expect(merged).toContain('amount'); + expect(merged).toContain('Acme'); + expect(queryByTestId('probe-malformed-filter')).toBeNull(); + }); +}); diff --git a/packages/react/src/hooks/useElementDataSource.ts b/packages/react/src/hooks/useElementDataSource.ts index 4a01194c67..e070893163 100644 --- a/packages/react/src/hooks/useElementDataSource.ts +++ b/packages/react/src/hooks/useElementDataSource.ts @@ -25,6 +25,7 @@ import { collectSavedViews, composeElementDataSource, elementDataSourceViewNotFoundMessage, + FilterOperatorError, isElementDataSourceConfig, resolveSavedView, type ComposedElementDataSource, @@ -47,7 +48,12 @@ export type ElementDataSourceStatus = | 'loading' /** The named `view` was found and composed in. */ | 'resolved' - /** The named `view` does not exist on the object (or could not be read). */ + /** + * The binding cannot be applied: the named `view` does not exist on the + * object (or could not be read), or a filter it combines is REFUSED by the + * filter converter — then `filterRefusal` carries the refusal (objectui#10789). + * Either way `composed` is withheld, so no query runs without the filter. + */ | 'missing'; export interface UseElementDataSourceResult { @@ -64,6 +70,14 @@ export interface UseElementDataSourceResult { view?: ElementSavedView; /** Author-facing explanation, set only for `missing`. */ error?: string; + /** + * Why the binding could not be composed, when the reason is a refused + * filter — set only with `missing`, and `error` then carries its message + * (objectui#10789). A caller that draws the malformed-filter notice reads the + * refused operator or field off it (`filterRefusalSubject`) rather than + * scraping `error`. + */ + filterRefusal?: FilterOperatorError; } /** A data source able to answer "what saved views does this object have?". */ @@ -207,11 +221,23 @@ export function useElementDataSource( } if (resolved?.key !== requestKey) return { status: 'loading', config }; if (resolved.view === null) return { status: 'missing', config, error: resolved.error }; - return { - status: 'resolved', - config, - view: resolved.view, - composed: composeElementDataSource(config, resolved.view), - }; + // objectui#10789 — composing the view's filter with the binding's own is a + // MERGE, and the merge lowers both through the throwing converter form. + // This is a render-time `useMemo`, so an uncaught refusal was a render + // error: the block's error boundary instead of a sentence naming the + // operator. The refusal is kept as a VALUE and reported as `missing`, + // which every caller already answers by withholding the query — never as + // `undefined`, which would read as "no filter" and widen the read. Only a + // `FilterOperatorError` is caught, the rule `toFilterNodeSafely` states: + // anything else is a defect, not a statement about the author's filter. + // (The `ready` branch above composes no view, so nothing is merged there.) + let composed: ComposedElementDataSource; + try { + composed = composeElementDataSource(config, resolved.view); + } catch (error) { + if (!(error instanceof FilterOperatorError)) throw error; + return { status: 'missing', config, error: error.message, filterRefusal: error }; + } + return { status: 'resolved', config, view: resolved.view, composed }; }, [config, resolved, requestKey]); } From cae4c5ed928b0b668e3f8eba077df2d8621f1c86 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 17:48:45 +0000 Subject: [PATCH 2/7] chore(changeset): declare the objectui#10789 patch for the seven touched packages Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DuWo5bdP9SdVebamn99GGk --- .../10789-render-time-filter-refusals.md | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 .changeset/10789-render-time-filter-refusals.md diff --git a/.changeset/10789-render-time-filter-refusals.md b/.changeset/10789-render-time-filter-refusals.md new file mode 100644 index 0000000000..1102593bd4 --- /dev/null +++ b/.changeset/10789-render-time-filter-refusals.md @@ -0,0 +1,36 @@ +--- +'@object-ui/core': patch +'@object-ui/react': patch +'@object-ui/fields': patch +'@object-ui/plugin-charts': patch +'@object-ui/plugin-dashboard': patch +'@object-ui/plugin-report': patch +'@object-ui/plugin-list': patch +--- + +A malformed authored filter no longer throws out of render or out of a click handler at the +sites that merge filters there; each now surfaces the refusal the way its siblings already do +(objectui#10789). The filter stays **refused** — nothing that was refused is accepted, and no +site falls back to "no filter". + +- **`ElementDataSourceGate` / `useElementDataSource`** (`@object-ui/react`): the gate's merge of + the component's `filter` with the composed binding, and the merge of a saved view's filter + with the binding's own, lowered through the throwing converter inside render. A refused + filter from either now makes the binding `missing` with a new optional `filterRefusal` on the + result (`error` carries its message), and the gate draws the malformed-filter notice the + wrapped blocks draw (`view.malformedFilter`, naming the refused operator or field) in place of + the block. `element:record_picker`, which reads the same hook, shows its configuration-error + panel with the refusal. +- **`PeoplePicker` / `RecordPickerDialog`** (`@object-ui/fields`): the recents merge and the + dialog's `mergedFilter` lower through `toFilterNodeSafely`; a refusal is shown in the picker's + existing error state and no query runs without the filter. +- **Drill-downs** (`ObjectChart`, `ObjectPivotTable`, `DatasetWidget`, `DatasetReportRenderer`): + a widget or report filter this layer refuses (a spec `$not`, say) made the drill click throw. + The drill is now not opened or emitted — never with the scope dropped — and the refusal is + logged with `console.warn`, naming the operator. +- **`ListView` export** (`@object-ui/plugin-list`): the server export builds its filter inside + its `try`, so a refusal is named in the export menu instead of escaping the click. +- **`convertFiltersToAST`** (`@object-ui/core`): an `$or` member that is the TRUE identity no + longer stops the members after it from being read. `{ $or: [{}, { a: {} }] }` is now refused + like `{ $or: [{ a: {} }, {}] }` already was, instead of answering TRUE (every row); a TRUE + disjunct beside members that lower still absorbs its `$or`. From f2bc82a451bb3a35058e010e0dd1152e383c17db Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 17:55:27 +0000 Subject: [PATCH 3/7] test(charts,dashboard,report): type the warn-call mappers in the objectui#10789 drill pins Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DuWo5bdP9SdVebamn99GGk --- .../src/__tests__/ObjectChart.drillRefusal-10789.test.tsx | 2 +- .../plugin-dashboard/src/__tests__/drillRefusal-10789.test.tsx | 2 +- .../__tests__/DatasetReportRenderer.drillRefusal-10789.test.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/plugin-charts/src/__tests__/ObjectChart.drillRefusal-10789.test.tsx b/packages/plugin-charts/src/__tests__/ObjectChart.drillRefusal-10789.test.tsx index 73b003e9ae..eb8b3e9e7a 100644 --- a/packages/plugin-charts/src/__tests__/ObjectChart.drillRefusal-10789.test.tsx +++ b/packages/plugin-charts/src/__tests__/ObjectChart.drillRefusal-10789.test.tsx @@ -79,7 +79,7 @@ function renderChart(filter: ObjectChartSchema['filter'], target: 'drawer' | 'na } const refusalWarnings = () => - warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes('drill-down refused')); + warn.mock.calls.map((c: unknown[]) => String(c[0])).filter((m: string) => m.includes('drill-down refused')); /** `$not` — declared by the spec, refused by this layer's converter. */ const REFUSED = { $not: { region: 'apac' } } as unknown as ObjectChartSchema['filter']; diff --git a/packages/plugin-dashboard/src/__tests__/drillRefusal-10789.test.tsx b/packages/plugin-dashboard/src/__tests__/drillRefusal-10789.test.tsx index 24769afd02..06239291fa 100644 --- a/packages/plugin-dashboard/src/__tests__/drillRefusal-10789.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/drillRefusal-10789.test.tsx @@ -66,7 +66,7 @@ afterEach(() => { }); const refusalWarnings = () => - warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes('drill-down refused')); + warn.mock.calls.map((c: unknown[]) => String(c[0])).filter((m: string) => m.includes('drill-down refused')); const ROWS = [ { id: 'a', stage: 'won', source: 'web', region: 'emea', amount: 10 }, diff --git a/packages/plugin-report/src/__tests__/DatasetReportRenderer.drillRefusal-10789.test.tsx b/packages/plugin-report/src/__tests__/DatasetReportRenderer.drillRefusal-10789.test.tsx index 10a38ddb74..f2a352a3ca 100644 --- a/packages/plugin-report/src/__tests__/DatasetReportRenderer.drillRefusal-10789.test.tsx +++ b/packages/plugin-report/src/__tests__/DatasetReportRenderer.drillRefusal-10789.test.tsx @@ -55,7 +55,7 @@ afterEach(() => { }); const refusalWarnings = () => - warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes('drill-down refused')); + warn.mock.calls.map((c: unknown[]) => String(c[0])).filter((m: string) => m.includes('drill-down refused')); describe('DatasetReportRenderer — a refused runtimeFilter drills nothing (objectui#10789)', () => { it('the grouped-row drill logs the refusal and emits no drill', async () => { From cab1c7d1f1620fd7da1b7fbdf3402de551919b89 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 18:53:35 +0000 Subject: [PATCH 4/7] chore(scripts): classify ElementDataSourceGate as a pack-object importer in check-i18n-dead-keys (objectui#10789) The malformed-filter panel objectui#10789 added to ElementDataSourceGate reads the `en` pack at render for a `t()` defaultValue, which makes the file a pack-object importer the header's enumeration must read by hand. Its reading: a class-4 read (two-segment key) that no leg sees and that the co-located `t()` literal keeps visible BY COINCIDENCE. The key itself is not at risk: the sibling malformed-filter states spell it as a literal `t()` argument too. The bullet is written first; the path joins ANALYSED_PACK_OBJECT_IMPORTERS after it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DuWo5bdP9SdVebamn99GGk --- scripts/check-i18n-dead-keys.mjs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/scripts/check-i18n-dead-keys.mjs b/scripts/check-i18n-dead-keys.mjs index dd6a9de783..e0965a47f9 100644 --- a/scripts/check-i18n-dead-keys.mjs +++ b/scripts/check-i18n-dead-keys.mjs @@ -267,6 +267,25 @@ * default away from its call site, and both go dark to every leg while a * shipping component still renders them — and that edit would read as a * tidy-up. + * - `packages/react/src/element-data-source/ElementDataSourceGate.tsx` — the + * second instance of class 4, and the same shape as `nonGridRowCeiling` + * above on purpose (objectui#10789). Its malformed-filter panel renders + * only when a filter the gate merges is refused; it dereferences the `en` + * pack at render to supply the `defaultValue` beside a `t()` call, and the + * ONE key it reads is TWO SEGMENTS. So BOTH LEGS ARE BLIND to the property + * read, for the reason given in that bullet. The CLI row for this file + * reports it as seen by the full-key leg, and that is the co-located + * spelling doing the work: the key is also the literal first argument of + * the `t()` call that default sits in. ⇒ the READ is covered BY + * COINCIDENCE. The KEY is NOTHING AT RISK, and not by that coincidence: + * the same key is the literal `t()` argument of the malformed-filter states + * this panel copies (objectui#9050: `record:related_list`, the line-items + * panel, `object-grid`), so the AST pass reads it at call sites outside + * this file too. Re-derive both halves rather than trusting this sentence: + * the class-4 row this script prints for the file, and a search for the + * key as a quoted `t()` argument. Template the key HERE and the read goes + * dark while those sites still hold it out of the tiers. Template it at + * ALL of them and it lands in CONFIRMED with this panel still rendering it. * * The rest of the matches are TEST-only importers, deliberately not listed one * by one. A key read only by a test is not a key a user can see, so a test @@ -743,6 +762,7 @@ export const ANALYSED_PACK_OBJECT_IMPORTERS = Object.freeze([ 'packages/plugin-grid/demo/main.tsx', 'packages/plugin-grid/demo/bulk-actions.tsx', 'packages/react/src/utils/nonGridRowCeiling.tsx', + 'packages/react/src/element-data-source/ElementDataSourceGate.tsx', ]); /** A path is a TEST importer when it lives in a `__tests__` directory or is From 3dbcabab9a99d6ca9875c100b51cff536ba2b689 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 19:00:25 +0000 Subject: [PATCH 5/7] Revert "chore(scripts): classify ElementDataSourceGate as a pack-object importer in check-i18n-dead-keys (objectui#10789)" This reverts commit cab1c7d1f; it was never pushed. The patch round takes the reviewer's option (b) instead: the gate's malformed-filter panel stops reading the `en` pack, so the file leaves the pack-object importer population and needs no bullet. An entry for a file outside the population would fail the gate's other direction, "EVERY analysed importer still exists". Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DuWo5bdP9SdVebamn99GGk --- scripts/check-i18n-dead-keys.mjs | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/scripts/check-i18n-dead-keys.mjs b/scripts/check-i18n-dead-keys.mjs index e0965a47f9..dd6a9de783 100644 --- a/scripts/check-i18n-dead-keys.mjs +++ b/scripts/check-i18n-dead-keys.mjs @@ -267,25 +267,6 @@ * default away from its call site, and both go dark to every leg while a * shipping component still renders them — and that edit would read as a * tidy-up. - * - `packages/react/src/element-data-source/ElementDataSourceGate.tsx` — the - * second instance of class 4, and the same shape as `nonGridRowCeiling` - * above on purpose (objectui#10789). Its malformed-filter panel renders - * only when a filter the gate merges is refused; it dereferences the `en` - * pack at render to supply the `defaultValue` beside a `t()` call, and the - * ONE key it reads is TWO SEGMENTS. So BOTH LEGS ARE BLIND to the property - * read, for the reason given in that bullet. The CLI row for this file - * reports it as seen by the full-key leg, and that is the co-located - * spelling doing the work: the key is also the literal first argument of - * the `t()` call that default sits in. ⇒ the READ is covered BY - * COINCIDENCE. The KEY is NOTHING AT RISK, and not by that coincidence: - * the same key is the literal `t()` argument of the malformed-filter states - * this panel copies (objectui#9050: `record:related_list`, the line-items - * panel, `object-grid`), so the AST pass reads it at call sites outside - * this file too. Re-derive both halves rather than trusting this sentence: - * the class-4 row this script prints for the file, and a search for the - * key as a quoted `t()` argument. Template the key HERE and the read goes - * dark while those sites still hold it out of the tiers. Template it at - * ALL of them and it lands in CONFIRMED with this panel still rendering it. * * The rest of the matches are TEST-only importers, deliberately not listed one * by one. A key read only by a test is not a key a user can see, so a test @@ -762,7 +743,6 @@ export const ANALYSED_PACK_OBJECT_IMPORTERS = Object.freeze([ 'packages/plugin-grid/demo/main.tsx', 'packages/plugin-grid/demo/bulk-actions.tsx', 'packages/react/src/utils/nonGridRowCeiling.tsx', - 'packages/react/src/element-data-source/ElementDataSourceGate.tsx', ]); /** A path is a TEST importer when it lives in a `__tests__` directory or is From c5cee60463309ed16ad7dd2e8da936af3573d457 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 19:02:42 +0000 Subject: [PATCH 6/7] fix(core,react): close the drill seam's second refusal shape, and drop the gate's pack-object default (objectui#10789) Contract review 5858758764 on the first head found two defects; this round fixes both. 1. composeDrillFilter ends in the spec's parseFilterAST, which refuses comparand shapes the converter passes through, e.g. an array-dialect widget filter [['stage', 'in', 'won']] or { stage: { $in: 'won' } }. It threw a plain Error with code INVALID_FILTER / status 400. The four drill sites catch FilterOperatorError only, so the chart and pivot were still thrown into the error boundary on a click. The seam now re-raises that refusal as a FilterOperatorError with its message, so all four sites treat it exactly like the converter's refusals: no drill opened or emitted, and a console.warn naming the operator. Only INVALID_FILTER is re-raised; any other error passes through. 2. ElementDataSourceGate's malformed-filter panel read the `en` pack object for its defaultValue, which made the file an unclassified pack-object importer (check-i18n-dead-keys). It now carries the sentence as a literal inline default, as the sibling blocks carry theirs, held byte-identical to the pack by check:i18n-keys. The gate pin now proves the text reaches the screen with and without an I18nProvider. The changeset's opening sentence is rewritten to hold at every site. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DuWo5bdP9SdVebamn99GGk --- .../10789-render-time-filter-refusals.md | 28 +++--- .../drill-filter-spec-refusal-10789.test.ts | 89 +++++++++++++++++++ packages/core/src/utils/drill-down.ts | 38 ++++++-- .../ObjectChart.drillRefusal-10789.test.tsx | 23 +++-- .../src/__tests__/drillRefusal-10789.test.tsx | 28 ++++-- ...ReportRenderer.drillRefusal-10789.test.tsx | 23 +++-- .../ElementDataSourceGate.tsx | 18 +++- ...aSourceGate.malformedFilter-10789.test.tsx | 22 ++++- 8 files changed, 227 insertions(+), 42 deletions(-) create mode 100644 packages/core/src/utils/__tests__/drill-filter-spec-refusal-10789.test.ts diff --git a/.changeset/10789-render-time-filter-refusals.md b/.changeset/10789-render-time-filter-refusals.md index 1102593bd4..516a93c8d5 100644 --- a/.changeset/10789-render-time-filter-refusals.md +++ b/.changeset/10789-render-time-filter-refusals.md @@ -8,26 +8,32 @@ '@object-ui/plugin-list': patch --- -A malformed authored filter no longer throws out of render or out of a click handler at the -sites that merge filters there; each now surfaces the refusal the way its siblings already do -(objectui#10789). The filter stays **refused** — nothing that was refused is accepted, and no -site falls back to "no filter". +A filter refused as malformed (`INVALID_FILTER`) no longer throws out of render or out of a click +handler at the sites that merge filters there. That holds whether this layer's filter converter +refused it or, at the drill seam, the spec's own `parseFilterAST`. Each site now surfaces the +refusal the way its siblings already do (objectui#10789). The filter stays **refused**: nothing +that was refused is accepted, and no site falls back to "no filter". - **`ElementDataSourceGate` / `useElementDataSource`** (`@object-ui/react`): the gate's merge of the component's `filter` with the composed binding, and the merge of a saved view's filter with the binding's own, lowered through the throwing converter inside render. A refused filter from either now makes the binding `missing` with a new optional `filterRefusal` on the - result (`error` carries its message), and the gate draws the malformed-filter notice the - wrapped blocks draw (`view.malformedFilter`, naming the refused operator or field) in place of - the block. `element:record_picker`, which reads the same hook, shows its configuration-error - panel with the refusal. + result (`error` carries its message). The gate then draws, in place of the block, the + malformed-filter notice the wrapped blocks draw: `view.malformedFilter`, naming the refused + operator or field. `element:record_picker`, which reads the same hook, shows its + configuration-error panel with the refusal. - **`PeoplePicker` / `RecordPickerDialog`** (`@object-ui/fields`): the recents merge and the dialog's `mergedFilter` lower through `toFilterNodeSafely`; a refusal is shown in the picker's existing error state and no query runs without the filter. - **Drill-downs** (`ObjectChart`, `ObjectPivotTable`, `DatasetWidget`, `DatasetReportRenderer`): - a widget or report filter this layer refuses (a spec `$not`, say) made the drill click throw. - The drill is now not opened or emitted — never with the scope dropped — and the refusal is - logged with `console.warn`, naming the operator. + a widget or report filter refused at the drill seam made the drill click throw. The converter + refuses some (a spec `$not`, say). The spec's `parseFilterAST` refuses others, such as a scalar + on a list operator in either dialect: `[['stage', 'in', 'won']]` or `{ stage: { $in: 'won' } }`. + The drill is now not opened or emitted (never with the scope dropped), and the refusal is logged + with `console.warn`, naming the operator. `composeDrillFilter` (`@object-ui/core`) now throws + one refusal type: the spec's `INVALID_FILTER` error is re-raised as a `FilterOperatorError` with + its message and the same `INVALID_FILTER` / 400 envelope, and any other error passes through + unchanged. - **`ListView` export** (`@object-ui/plugin-list`): the server export builds its filter inside its `try`, so a refusal is named in the export menu instead of escaping the click. - **`convertFiltersToAST`** (`@object-ui/core`): an `$or` member that is the TRUE identity no diff --git a/packages/core/src/utils/__tests__/drill-filter-spec-refusal-10789.test.ts b/packages/core/src/utils/__tests__/drill-filter-spec-refusal-10789.test.ts new file mode 100644 index 0000000000..4c3564ee28 --- /dev/null +++ b/packages/core/src/utils/__tests__/drill-filter-spec-refusal-10789.test.ts @@ -0,0 +1,89 @@ +/** + * 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. + */ + +/** + * `composeDrillFilter` throws ONE refusal type — objectui#10789. + * + * The seam refuses at two steps. `mergeFilterNodes` throws a + * `FilterOperatorError`; the spec's `parseFilterAST`, which judges comparand + * shapes the converter passes through, threw a plain `Error` carrying + * `code: 'INVALID_FILTER'` / `status: 400`. The drill callers catch the first + * type only, so an array-dialect widget filter such as + * `[['stage', 'in', 'won']]` (never judged by the converter) still threw the + * chart into its error boundary on a click. + * + * - §1 — both spec-refused spellings come out as a `FilterOperatorError`, + * asserted on the ENVELOPE (`code`, `httpStatus`) and on the refusal's own + * sentence naming the operator. + * - §2 — only `INVALID_FILTER` is re-raised; any other error from the spec + * passes through untouched (no broad catch). + * - §3 — CONTROL: a well-formed pair still composes. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; + +/** Set by §2 to make the spec's lowering throw a non-refusal error. */ +let parseOverride: (() => never) | null = null; + +vi.mock('@objectstack/spec/data', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + parseFilterAST: ((...args: Parameters) => + parseOverride ? parseOverride() : actual.parseFilterAST(...args)) as typeof actual.parseFilterAST, + }; +}); + +import { composeDrillFilter } from '../drill-down'; +import { FilterOperatorError } from '../filter-converter'; + +afterEach(() => { + parseOverride = null; +}); + +function refusalOf(call: () => unknown): unknown { + try { + call(); + } catch (error) { + return error; + } + return undefined; +} + +describe('objectui#10789 — the spec’s refusal leaves the drill seam as a FilterOperatorError', () => { + it.each([ + ['array dialect, scalar on a list operator', [['stage', 'in', 'won']]], + ['object dialect, scalar on a list operator', { stage: { $in: 'won' } }], + ])('%s', (_label, widgetFilter) => { + const thrown = refusalOf(() => composeDrillFilter(widgetFilter, { region: 'emea' })); + expect(thrown).toBeInstanceOf(FilterOperatorError); + const error = thrown as FilterOperatorError; + expect(error.code).toBe('INVALID_FILTER'); + expect(error.httpStatus).toBe(400); + // The spec's own sentence, verbatim, naming the operator it refused. + expect(error.message).toContain('$in'); + }); +}); + +describe('objectui#10789 — only INVALID_FILTER is re-raised', () => { + it('any other error from the spec passes through untouched', () => { + const defect = new TypeError('not a refusal'); + parseOverride = () => { + throw defect; + }; + expect(refusalOf(() => composeDrillFilter({ region: 'emea' }, { stage: 'won' }))).toBe(defect); + }); +}); + +describe('objectui#10789 — CONTROL', () => { + it('a well-formed pair still composes to the object dialect', () => { + expect(composeDrillFilter({ region: 'emea' }, { stage: 'won' })).toEqual({ + $and: [{ region: 'emea' }, { stage: 'won' }], + }); + }); +}); diff --git a/packages/core/src/utils/drill-down.ts b/packages/core/src/utils/drill-down.ts index 141384e7ec..a2aec1d41a 100644 --- a/packages/core/src/utils/drill-down.ts +++ b/packages/core/src/utils/drill-down.ts @@ -25,7 +25,7 @@ import { parseFilterAST } from '@objectstack/spec/data'; import type { DrillDownConfig } from '@object-ui/types'; -import { mergeFilterNodes } from './filter-converter.js'; +import { FilterOperatorError, mergeFilterNodes } from './filter-converter.js'; /** * Generic click payload. Pivots provide row/col, charts provide @@ -243,15 +243,39 @@ export function isDrillEnabled(config: DrillDownConfig | undefined): boolean { * the `INVALID_FILTER` / 400 envelope. Such a filter already fails the widget's * OWN query for the same reason, so the drill and the chart now agree instead of * the drill quietly sending something the chart could not. + * + * ## One refusal type out of this seam (objectui#10789) + * + * The seam has TWO refusing steps, and they used to throw two different + * things. `mergeFilterNodes` throws a {@link FilterOperatorError}. The spec's + * `parseFilterAST`, which judges comparand shapes the converter passes + * through, throws a plain `Error` that carries the same envelope as own + * properties: `code: 'INVALID_FILTER'` and `status: 400`. An array-dialect + * widget filter holds no rule objects, so the converter never judges it, and + * `[['stage', 'in', 'won']]` (a scalar on a list operator) is refused only + * there. So is the object spelling `{ stage: { $in: 'won' } }`. + * + * Every caller catches the refusal as a `FilterOperatorError`: the chart, + * pivot and dataset drills report it and open nothing (never a filter-less + * drill). So the spec's refusal is re-raised here AS one, with its message + * verbatim and the same `INVALID_FILTER` / 400 envelope, rather than taught to + * each caller as a second shape. Only an error whose `code` is + * `INVALID_FILTER` is re-raised; anything else from `parseFilterAST` is a + * defect rather than a statement about the author's filter, and passes + * through untouched. */ export function composeDrillFilter( widgetFilter: unknown, drillFilter: Record | undefined, ): Record | undefined { - // `FilterCondition` is the spec's object-dialect filter; the drill sinks type - // the same value as `Record`, and this is the one seam where - // the two names meet. - return parseFilterAST(mergeFilterNodes(widgetFilter, drillFilter)) as - | Record - | undefined; + const merged = mergeFilterNodes(widgetFilter, drillFilter); + try { + // `FilterCondition` is the spec's object-dialect filter; the drill sinks + // type the same value as `Record`, and this is the one + // seam where the two names meet. + return parseFilterAST(merged) as Record | undefined; + } catch (error) { + if ((error as { code?: unknown } | null)?.code !== 'INVALID_FILTER') throw error; + throw new FilterOperatorError((error as Error).message); + } } diff --git a/packages/plugin-charts/src/__tests__/ObjectChart.drillRefusal-10789.test.tsx b/packages/plugin-charts/src/__tests__/ObjectChart.drillRefusal-10789.test.tsx index eb8b3e9e7a..e40b9583f1 100644 --- a/packages/plugin-charts/src/__tests__/ObjectChart.drillRefusal-10789.test.tsx +++ b/packages/plugin-charts/src/__tests__/ObjectChart.drillRefusal-10789.test.tsx @@ -81,12 +81,25 @@ function renderChart(filter: ObjectChartSchema['filter'], target: 'drawer' | 'na const refusalWarnings = () => warn.mock.calls.map((c: unknown[]) => String(c[0])).filter((m: string) => m.includes('drill-down refused')); -/** `$not` — declared by the spec, refused by this layer's converter. */ -const REFUSED = { $not: { region: 'apac' } } as unknown as ObjectChartSchema['filter']; +/** + * Two refusals, one per refusing step of the drill seam: + * - `$not` — declared by the spec, refused by this layer's CONVERTER; + * - `[['stage', 'in', 'won']]` — the array dialect `ObjectChartSchema.filter` + * admits, which the converter passes through untouched and the spec's own + * `parseFilterAST` refuses (a scalar on a list operator). The contract + * review's probe of this PR's first head. + */ +const REFUSALS: ReadonlyArray<[string, ObjectChartSchema['filter'], string]> = [ + ['converter refusal', { $not: { region: 'apac' } } as unknown as ObjectChartSchema['filter'], '$not'], + ['spec refusal (array dialect)', [['stage', 'in', 'won']] as unknown as ObjectChartSchema['filter'], '$in'], +]; +const CASES = REFUSALS.flatMap(([label, filter, operator]) => + (['drawer', 'navigate'] as const).map((target) => [label, target, filter, operator] as const), +); describe('ObjectChart — a refused drill filter (objectui#10789)', () => { - it.each(['drawer', 'navigate'] as const)('target %s — the chart stays, nothing opens, the refusal is logged', (target) => { - const openRecordList = renderChart(REFUSED, target); + it.each(CASES)('%s, target %s — the chart stays, nothing opens, the refusal is logged', (_label, target, filter, operator) => { + const openRecordList = renderChart(filter, target); fireEvent.click(screen.getByTestId('fake-segment')); // The chart is still mounted: the click did not throw it into a boundary. @@ -94,7 +107,7 @@ describe('ObjectChart — a refused drill filter (objectui#10789)', () => { expect(screen.queryByTestId('chart-drill-body')).toBeNull(); expect(openRecordList).not.toHaveBeenCalled(); expect(refusalWarnings()).toHaveLength(1); - expect(refusalWarnings()[0]).toContain('$not'); + expect(refusalWarnings()[0]).toContain(operator); }); it('CONTROL — a well-formed widget filter still drills', () => { diff --git a/packages/plugin-dashboard/src/__tests__/drillRefusal-10789.test.tsx b/packages/plugin-dashboard/src/__tests__/drillRefusal-10789.test.tsx index 06239291fa..8ded1d584f 100644 --- a/packages/plugin-dashboard/src/__tests__/drillRefusal-10789.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/drillRefusal-10789.test.tsx @@ -96,9 +96,21 @@ function renderPivot(filter: unknown, target: 'drawer' | 'navigate') { return openRecordList; } +/** + * The spec's own refusal, the drill seam's second refusing step: the array + * dialect passes the converter untouched and `parseFilterAST` refuses a scalar + * on a list operator. + */ +const SPEC_REFUSED = [['region', 'in', 'emea']]; + describe('ObjectPivotTable — a refused drill filter (objectui#10789)', () => { - it.each(['drawer', 'navigate'] as const)('target %s — the pivot stays, nothing opens, the refusal is logged', (target) => { - const openRecordList = renderPivot(REFUSED, target); + it.each([ + ['converter refusal', 'drawer', REFUSED, '$not'], + ['converter refusal', 'navigate', REFUSED, '$not'], + ['spec refusal (array dialect)', 'drawer', SPEC_REFUSED, '$in'], + ['spec refusal (array dialect)', 'navigate', SPEC_REFUSED, '$in'], + ] as const)('%s, target %s — the pivot stays, nothing opens, the refusal is logged', (_label, target, filter, operator) => { + const openRecordList = renderPivot(filter, target); fireEvent.click(screen.getByLabelText('Drill into stage=won, source=web')); // Still mounted: the click did not throw the pivot into a boundary. @@ -106,7 +118,7 @@ describe('ObjectPivotTable — a refused drill filter (objectui#10789)', () => { expect(drawerProps).toHaveLength(0); expect(openRecordList).not.toHaveBeenCalled(); expect(refusalWarnings()).toHaveLength(1); - expect(refusalWarnings()[0]).toContain('$not'); + expect(refusalWarnings()[0]).toContain(operator); }); it('CONTROL — a well-formed pivot filter still opens the drawer, scoped', () => { @@ -163,15 +175,19 @@ const clickFirstSegment = () => { }; describe('DatasetWidget — a refused drill filter (objectui#10789)', () => { - it('opens no drawer and logs the refusal instead of throwing from the click', async () => { - renderDatasetWidget(REFUSED); + it.each([ + ['converter refusal', REFUSED, '$not'], + // Object dialect, passed by the converter, refused by the spec's lowering. + ['spec refusal', { owner: { $in: 'me' } }, '$in'], + ] as const)('%s — opens no drawer and logs the refusal instead of throwing from the click', async (_label, filter, operator) => { + renderDatasetWidget(filter); await waitFor(() => expect(capturedChartProps?.onSegmentClick).toBeTypeOf('function')); expect(() => clickFirstSegment()).not.toThrow(); expect(screen.queryByTestId('drill-drawer')).toBeNull(); expect(drawerProps).toHaveLength(0); expect(refusalWarnings()).toHaveLength(1); - expect(refusalWarnings()[0]).toContain('$not'); + expect(refusalWarnings()[0]).toContain(operator); }); it('CONTROL — a well-formed widget filter still opens the drawer, scoped', async () => { diff --git a/packages/plugin-report/src/__tests__/DatasetReportRenderer.drillRefusal-10789.test.tsx b/packages/plugin-report/src/__tests__/DatasetReportRenderer.drillRefusal-10789.test.tsx index f2a352a3ca..2720a63e6b 100644 --- a/packages/plugin-report/src/__tests__/DatasetReportRenderer.drillRefusal-10789.test.tsx +++ b/packages/plugin-report/src/__tests__/DatasetReportRenderer.drillRefusal-10789.test.tsx @@ -28,8 +28,15 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react'; import { DatasetReportRenderer } from '../DatasetReportRenderer'; -/** `$not` — declared by the spec, refused by this layer's converter. */ -const REFUSED_SCOPE = { $not: { owner: 'them' } }; +/** + * Two refusals, one per refusing step of the drill seam: `$not` is declared by + * the spec and refused by this layer's CONVERTER; a scalar on `$in` passes the + * converter and is refused by the spec's own `parseFilterAST`. + */ +const REFUSALS = [ + ['converter refusal', { $not: { owner: 'them' } }, '$not'], + ['spec refusal', { owner: { $in: 'me' } }, '$in'], +] as const; function makeSource(rows: Array>, rawRows: Array>) { return { @@ -58,13 +65,13 @@ const refusalWarnings = () => warn.mock.calls.map((c: unknown[]) => String(c[0])).filter((m: string) => m.includes('drill-down refused')); describe('DatasetReportRenderer — a refused runtimeFilter drills nothing (objectui#10789)', () => { - it('the grouped-row drill logs the refusal and emits no drill', async () => { + it.each(REFUSALS)('%s — the grouped-row drill logs the refusal and emits no drill', async (_label, scope, operator) => { const onDrill = vi.fn(); render( , ); @@ -73,16 +80,16 @@ describe('DatasetReportRenderer — a refused runtimeFilter drills nothing (obje expect(onDrill).not.toHaveBeenCalled(); expect(refusalWarnings()).toHaveLength(1); - expect(refusalWarnings()[0]).toContain('$not'); + expect(refusalWarnings()[0]).toContain(operator); }); - it('the matrix-cell drill logs the refusal and emits no drill', async () => { + it.each(REFUSALS)('%s — the matrix-cell drill logs the refusal and emits no drill', async (_label, scope, operator) => { const onDrill = vi.fn(); render( , ); @@ -91,7 +98,7 @@ describe('DatasetReportRenderer — a refused runtimeFilter drills nothing (obje expect(onDrill).not.toHaveBeenCalled(); expect(refusalWarnings()).toHaveLength(1); - expect(refusalWarnings()[0]).toContain('$not'); + expect(refusalWarnings()[0]).toContain(operator); }); it('CONTROL — a well-formed runtimeFilter still drills with the scope conjoined', async () => { diff --git a/packages/react/src/element-data-source/ElementDataSourceGate.tsx b/packages/react/src/element-data-source/ElementDataSourceGate.tsx index da80a98ac5..e027f66ace 100644 --- a/packages/react/src/element-data-source/ElementDataSourceGate.tsx +++ b/packages/react/src/element-data-source/ElementDataSourceGate.tsx @@ -111,7 +111,7 @@ import { type ElementSavedView, type FilterOperatorError, } from '@object-ui/core'; -import { useObjectTranslation, en } from '@object-ui/i18n'; +import { useObjectTranslation } from '@object-ui/i18n'; import { useElementDataSource, type ElementDataSourceStatus, @@ -558,8 +558,18 @@ export function ElementDataSourceErrorPanel({ * ⚠️ A separate component, so `useObjectTranslation` runs only on this path: * the gate wraps every object-bound block, and a translation hook in the * gate's own body would run for all of them, including under the many suites - * that mock `@object-ui/i18n` by hand. `en` is dereferenced here, at render, - * for the same reason (see `NonGridRowCeilingNote`). + * that mock `@object-ui/i18n` by hand. + * + * The provider-less default is the sentence itself, written the way the + * sibling blocks carry it: a LITERAL copy of the `en` pack's value, never a + * read of the pack object. The siblings keep theirs in a module-scope + * `createSafeTranslation` table, which this module cannot have: it is + * re-exported from the package entry, so a factory call at module scope would + * run on import for every consumer (`NonGridRowCeilingNote` records that + * hazard). So the copy sits in the call's inline `defaultValue`, where + * `pnpm check:i18n-keys` holds it byte-identical to the pack, as the siblings' + * tables are held. With an `I18nProvider` the pack's value wins, and without + * one this literal is interpolated (objectui#6219). */ function ElementDataSourceMalformedFilterPanel({ testId, @@ -580,7 +590,7 @@ function ElementDataSourceMalformedFilterPanel({

{t('view.malformedFilter', { subject: filterRefusalSubject(refusal) ?? '', - defaultValue: en.view.malformedFilter, + defaultValue: 'This view’s filter is malformed, so no records are shown: the {{subject}} condition cannot be applied.', })}

{refusal.message}

diff --git a/packages/react/src/element-data-source/__tests__/ElementDataSourceGate.malformedFilter-10789.test.tsx b/packages/react/src/element-data-source/__tests__/ElementDataSourceGate.malformedFilter-10789.test.tsx index 3f5effcdb9..dbea76fd03 100644 --- a/packages/react/src/element-data-source/__tests__/ElementDataSourceGate.malformedFilter-10789.test.tsx +++ b/packages/react/src/element-data-source/__tests__/ElementDataSourceGate.malformedFilter-10789.test.tsx @@ -31,6 +31,7 @@ import { describe, it, expect, vi } from 'vitest'; import { render, renderHook, waitFor } from '@testing-library/react'; import * as React from 'react'; +import { I18nProvider } from '@object-ui/i18n'; import { ElementDataSourceGate, useElementDataSourceSchema, @@ -79,7 +80,12 @@ describe('ElementDataSourceGate — a malformed filter draws the notice (objectu await waitFor(() => expect(queryByTestId('probe-malformed-filter')).not.toBeNull()); // The headline NAMES the operator the author has to fix — the sibling // blocks' sentence, not the "data source could not be resolved" panel. - expect(getByTestId('probe-malformed-filter-subject').textContent).toContain('$regex'); + // No `I18nProvider` here: the sentence itself reaches the screen, not the + // raw key, and its `{{subject}}` hole is filled. + const headline = getByTestId('probe-malformed-filter-subject').textContent ?? ''; + expect(headline).toContain('filter is malformed'); + expect(headline).toContain('the $regex condition'); + expect(headline).not.toContain('view.malformedFilter'); expect(getByTestId('probe-malformed-filter').getAttribute('role')).toBe('alert'); // Refused, not dropped: the block never mounts, so it cannot run the query // unconstrained. @@ -100,6 +106,20 @@ describe('ElementDataSourceGate — a malformed filter draws the notice (objectu expect(result.current.error).toBe(refusal?.message); }); + it('with an I18nProvider, the headline is the pack sentence naming the operator', async () => { + const { queryByTestId, getByTestId } = render( + + + {(bound) => } + + , + ); + await waitFor(() => expect(queryByTestId('probe-malformed-filter')).not.toBeNull()); + const headline = getByTestId('probe-malformed-filter-subject').textContent ?? ''; + expect(headline).toContain('filter is malformed'); + expect(headline).toContain('the $regex condition'); + }); + it('CONTROL — a well-formed filter still renders the block with the merged filter', async () => { const { queryByTestId, getByTestId } = renderGate({ filter: { name: 'Acme' }, From 6f4bee84658bf84753acd58b78476e4ae994b442 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 19:10:10 +0000 Subject: [PATCH 7/7] test(core): make the drill-seam passthrough pin order-independent under isolate:false (objectui#10789) The `unit` project shares one module graph per worker, so a module-scope vi.mock of the spec only applied when no earlier file had loaded the seam. The pin now swaps the spec's lowering in a fresh module graph (vi.resetModules + vi.doMock + dynamic import) and undoes it in `finally`, so nothing leaks into later files. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DuWo5bdP9SdVebamn99GGk --- .../drill-filter-spec-refusal-10789.test.ts | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/packages/core/src/utils/__tests__/drill-filter-spec-refusal-10789.test.ts b/packages/core/src/utils/__tests__/drill-filter-spec-refusal-10789.test.ts index 4c3564ee28..1813b1a041 100644 --- a/packages/core/src/utils/__tests__/drill-filter-spec-refusal-10789.test.ts +++ b/packages/core/src/utils/__tests__/drill-filter-spec-refusal-10789.test.ts @@ -25,27 +25,10 @@ * - §3 — CONTROL: a well-formed pair still composes. */ -import { describe, it, expect, vi, afterEach } from 'vitest'; - -/** Set by §2 to make the spec's lowering throw a non-refusal error. */ -let parseOverride: (() => never) | null = null; - -vi.mock('@objectstack/spec/data', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - parseFilterAST: ((...args: Parameters) => - parseOverride ? parseOverride() : actual.parseFilterAST(...args)) as typeof actual.parseFilterAST, - }; -}); - +import { describe, it, expect, vi } from 'vitest'; import { composeDrillFilter } from '../drill-down'; import { FilterOperatorError } from '../filter-converter'; -afterEach(() => { - parseOverride = null; -}); - function refusalOf(call: () => unknown): unknown { try { call(); @@ -71,12 +54,29 @@ describe('objectui#10789 — the spec’s refusal leaves the drill seam as a Fil }); describe('objectui#10789 — only INVALID_FILTER is re-raised', () => { - it('any other error from the spec passes through untouched', () => { + it('any other error from the spec passes through untouched', async () => { + // A FRESH module graph with the spec's lowering replaced, undone in + // `finally`: the `unit` project runs with `isolate: false`, so a + // module-scope `vi.mock` would depend on whether an earlier file in the + // worker had already loaded this seam — and would leak into later files. const defect = new TypeError('not a refusal'); - parseOverride = () => { - throw defect; - }; - expect(refusalOf(() => composeDrillFilter({ region: 'emea' }, { stage: 'won' }))).toBe(defect); + vi.resetModules(); + vi.doMock('@objectstack/spec/data', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + parseFilterAST: (() => { + throw defect; + }) as typeof actual.parseFilterAST, + }; + }); + try { + const fresh = await import('../drill-down'); + expect(refusalOf(() => fresh.composeDrillFilter({ region: 'emea' }, { stage: 'won' }))).toBe(defect); + } finally { + vi.doUnmock('@objectstack/spec/data'); + vi.resetModules(); + } }); });