diff --git a/.changeset/8944-chart-drill-filter-composition.md b/.changeset/8944-chart-drill-filter-composition.md new file mode 100644 index 0000000000..5f93e7468d --- /dev/null +++ b/.changeset/8944-chart-drill-filter-composition.md @@ -0,0 +1,44 @@ +--- +'@object-ui/core': minor +'@object-ui/plugin-charts': patch +'@object-ui/app-shell': patch +'@object-ui/types': patch +--- + +Compose an `ObjectChart` drill-down filter instead of spreading it, so the widget's own +filter survives into the drilled query for BOTH arms of `ObjectChartSchema.filter` +(objectui#8944). + +**The defect.** `ObjectChartSchema.filter` admits a spec `FilterArray` +(`[['region','=','emea']]`) and the ObjectQL `$filter` object (`{ region: 'emea' }`), +and both are read — both travel verbatim to `ds.aggregate` / `ds.find`. The drill seam +composed them by spreading the widget's filter into an object literal, which is correct +for the object arm and silent nonsense for the array arm: spreading an array yields +index keys, so an authored `FilterArray` drilled as +`{ '0': ['region','=','emea'], stage: 'won' }` — the widget's conditions replaced by a +key the query layer ignores. Nothing errored; the drawer opened and looked right. + +**Direction of the failure.** The widget's filter is what narrows. Dropping it made the +drilled list a **superset** — it showed records the chart itself was scoped to exclude. +Not a security boundary, but the worse direction for a silent bug. + +**The composition rule, named rather than picked.** `widget.filter ∧ drill.filter`. The +two are independent filter sources and a drill must satisfy both: the click context only +says which bucket of the widget's scope was asked for, so it may narrow that scope and +never widen it. This is not a new rule — it is the contract `mergeFilterNodes` already +states ("combine filter sources under a single `and`, each as its OWN child"), the sink +every other multi-source filter in this repo goes through. A new +`composeDrillFilter` helper in `@object-ui/core` applies it at the drill seam and +documents it, then lowers the result back to the `FilterCondition` object dialect with +`parseFilterAST` — the spec's single lowering sink — because that is the dialect both +drill sinks take. + +**Compatibility.** A lone surviving source lowers back to exactly the flat object the +spread produced, so a chart with no filter of its own drills byte-identically to before. +Only a genuinely composed pair gains the `$and`. + +`serializeDrillFilterParams` (the drill "Open in list" / `target: 'navigate'` URL writer) +learns to flatten that `$and` into the flat `filter[...]` params its own read side already +ANDs back together. Without that it took the `String(value)` path — `$and` holds an array — +and emitted `filter[$and]=[object Object],[object Object]` while both real conditions +vanished, which is the outcome that function's contract says it never produces. diff --git a/packages/app-shell/src/views/drillUrlFilters.test.ts b/packages/app-shell/src/views/drillUrlFilters.test.ts index 7fef515313..62ef6a953c 100644 --- a/packages/app-shell/src/views/drillUrlFilters.test.ts +++ b/packages/app-shell/src/views/drillUrlFilters.test.ts @@ -59,6 +59,57 @@ describe('serializeDrillFilterParams', () => { }); }); +describe('serializeDrillFilterParams — a COMPOSED drill filter (objectui#8944)', () => { + /** + * `composeDrillFilter` lowers `widget.filter ∧ click context` to + * `{ $and: […] }` whenever both sources survive. Before this branch existed + * that value fell to the `String(value)` path — `$and` holds an ARRAY, so it + * was neither null nor a non-array object — and produced a bogus + * `filter[$and]=[object Object],[object Object]` while BOTH real conditions + * vanished, i.e. the list landed scoped by nothing the user clicked. + */ + it('flattens a top-level $and into the params of each child', () => { + const qs = serializeDrillFilterParams({ $and: [{ region: 'emea' }, { stage: 'won' }] }); + expect(qs.get('filter[region]')).toBe('emea'); + expect(qs.get('filter[stage]')).toBe('won'); + // The hazard, named: no key spells the combinator, and nothing stringified. + expect(qs.get('filter[$and]')).toBeNull(); + expect(qs.toString()).not.toContain('object%20Object'); + }); + + it('walks a NESTED $and, which is what composing an array arm produces', () => { + // `[['stage','=','won'],['amount','>',100]]` conjoined with a click context + // lowers to an $and whose first child is itself an $and. + const qs = serializeDrillFilterParams({ + $and: [{ $and: [{ stage: 'won' }, { amount: { $gt: 100 } }] }, { region: 'emea' }], + }); + expect(qs.get('filter[stage]')).toBe('won'); + expect(qs.get('filter[amount][gt]')).toBe('100'); + expect(qs.get('filter[region]')).toBe('emea'); + }); + + it('a composed filter survives the URL round-trip as a conjunction of triples', () => { + // The read side ANDs its triples, so the conjunction is preserved in + // meaning, not just in bytes. + const triples = parseUrlFilterTriples( + serializeDrillFilterParams({ + $and: [{ region: 'emea' }, { close_date: { $gte: '2026-06-01', $lt: '2026-07-01' } }], + }), + ); + expect(triples).toEqual([ + ['region', '=', 'emea'], + ['close_date', '>=', '2026-06-01'], + ['close_date', '<', '2026-07-01'], + ]); + }); + + it('skips a bare ARRAY comparand rather than stringifying it', () => { + // The same promise the unknown-object case makes, for the shape that used + // to escape it. + expect(serializeDrillFilterParams({ tags: ['a', 'b'] }).toString()).toBe(''); + }); +}); + describe('round-trip: serialize → parse (write and read sides agree)', () => { it('a mixed equality + date-range drill filter survives the URL round-trip', () => { const filter = { stage: 'qualification', close_date: { $gte: '2026-06-01', $lt: '2026-07-01' } }; diff --git a/packages/app-shell/src/views/drillUrlFilters.ts b/packages/app-shell/src/views/drillUrlFilters.ts index 48e00780e7..0a49b09637 100644 --- a/packages/app-shell/src/views/drillUrlFilters.ts +++ b/packages/app-shell/src/views/drillUrlFilters.ts @@ -57,14 +57,53 @@ export function parseUrlFilterTriples(searchParams: URLSearchParams): FilterTrip * plain value becomes `filter[field]`. `null`/`undefined` values and objects * with no recognized operator are skipped (drill degrades to a superset) rather * than stringified to `"[object Object]"`. + * + * ## `$and` is flattened, because this dialect's conjunction is implicit + * + * A drill filter composed from more than one source arrives as + * `{ $and: [, ] }` — what `composeDrillFilter` + * (`@object-ui/core`) lowers `widget.filter ∧ drill.filter` to (objectui#8944). + * The READ side already returns a FLAT list of triples that the query layer ANDs + * together, so a top-level `$and` is expressible here: emit each child's params + * into the same set and `parseUrlFilterTriples` reads the conjunction straight + * back. Nesting is walked too, since composing three sources nests. + * + * ⚠️ Without this branch a composed filter took the `String(value)` path below — + * `$and` holds an ARRAY, so it was neither `null` nor a non-array object — and + * the URL grew a bogus `filter[$and]=[object Object],[object Object]` while BOTH + * real conditions vanished. That is the very outcome this function's contract + * says it never produces, and it is wrong in the widening direction: the list + * lands unscoped by anything the user actually clicked. + * + * ⚠️ Two conditions on the SAME field and operator are NOT expressible here (one + * param key, one value). The later source wins, so a click context still + * overrides the widget's condition on that field exactly as it did when this + * value was built by spreading — the drill degrades to a superset there, the + * same posture this function already takes for operators it cannot spell. */ export function serializeDrillFilterParams( filter: Record | undefined, ): URLSearchParams { const params = new URLSearchParams(); if (!filter) return params; + collectFilterParams(filter, params); + return params; +} + +/** One source's conditions, written into the shared param set. Recurses on `$and`. */ +function collectFilterParams(filter: Record, params: URLSearchParams): void { for (const [field, value] of Object.entries(filter)) { if (value == null) continue; + if (field === '$and' && Array.isArray(value)) { + // Implicit-AND dialect: each child contributes its own params, in order, + // so a later source overrides an earlier one on a field they share. + for (const child of value) { + if (child && typeof child === 'object' && !Array.isArray(child)) { + collectFilterParams(child as Record, params); + } + } + continue; + } if (typeof value === 'object' && !Array.isArray(value)) { for (const [op, suffix] of Object.entries(RANGE_OP_PARAM)) { const bound = (value as Record)[op]; @@ -72,9 +111,11 @@ export function serializeDrillFilterParams( } continue; // handled (range ops) or skipped — never String(object) } + // Arrays reach here as `$in`-style comparands this dialect cannot spell; + // skipping keeps the promise above (never `String(array)`). + if (Array.isArray(value)) continue; params.set(`filter[${field}]`, String(value)); } - return params; } /** diff --git a/packages/core/src/utils/drill-down.ts b/packages/core/src/utils/drill-down.ts index a21fdb2da4..141384e7ec 100644 --- a/packages/core/src/utils/drill-down.ts +++ b/packages/core/src/utils/drill-down.ts @@ -22,8 +22,11 @@ * synthesize an `event` object and rely on the same defaults / templating. */ +import { parseFilterAST } from '@objectstack/spec/data'; import type { DrillDownConfig } from '@object-ui/types'; +import { mergeFilterNodes } from './filter-converter.js'; + /** * Generic click payload. Pivots provide row/col, charts provide * category/series. Extra fields are passed through verbatim so callers @@ -183,3 +186,72 @@ export function isDrillEnabled(config: DrillDownConfig | undefined): boolean { if (!config) return false; return config.enabled !== false; } + +/** + * Compose a widget's OWN filter with the filter a drill click derived, into the + * one filter the drilled list is scoped by. + * + * ## The rule: `widget.filter ∧ drill.filter` + * + * The two are independent filter SOURCES and a drill must satisfy BOTH. The + * widget's filter is what scopes the chart; the click context only says WHICH + * bucket of that scope the user asked to see. So a drill may narrow the widget's + * scope and may never widen it — which makes the composition a conjunction, not + * a merge and emphatically not a spread. + * + * ⛔ The rule is NOT invented here. It is the contract {@link mergeFilterNodes} + * already states — "combine filter sources under a single `and`, each as its OWN + * child" — the sink every other multi-source filter in this repo goes through + * (`ObjectView`, `RelatedList`, `LineItemsPanel`, `RecordPickerDialog`, + * `ElementDataSourceGate`, `buildEffectiveFilter`). This function only applies + * it at the drill seam and names it, so the answer is in one place rather than + * re-derived per widget. + * + * ## Why the two arms needed a sink at all (objectui#8944) + * + * `ObjectChartSchema.filter` admits BOTH a spec `FilterArray` + * (`[['stage','=','won']]`) and the ObjectQL `$filter` object + * (`{ close_date: { $gte } }`), because both are read — both are forwarded + * verbatim to `ds.aggregate` / `ds.find`. The drill seam used to compose them by + * SPREADING the widget's filter into an object literal, which is correct for the + * object arm and silent nonsense for the array arm: spreading `[['stage','=', + * 'won']]` yields the index key `{ '0': ['stage','=','won'] }`, so the widget's + * own conditions were replaced by a key the query layer ignores and the drilled + * list showed rows the chart itself was scoped to exclude. `toFilterNode` (via + * `mergeFilterNodes`) already lowers all three shapes in circulation, so routing + * the pair through it is what makes the array arm survive. + * + * ## Why the result is lowered back to the object dialect + * + * {@link mergeFilterNodes} answers in the ObjectQL AST + * (`['and', , ]`). Both drill sinks take the `FilterCondition` + * OBJECT dialect instead — the drawer hands the value to `object-data-table`'s + * `filter` (which becomes `$filter`), and `DrillNavigationContext.openRecordList` + * declares `Record` and serializes it to `filter[...]` URL + * params. `parseFilterAST` is the spec's single lowering sink between the two + * dialects, so it is what converts, rather than a second local translation. + * + * ⭐ A lone surviving source lowers back to exactly the flat object the spread + * produced (`{ stage: 'won' }`), so a chart with no filter of its own drills + * identically to before; only a genuinely composed pair gains the `$and`. + * + * Returns `undefined` when neither source carries anything, so callers can omit + * the key rather than send an empty filter. + * + * ⚠️ Refusals are the sink's, not this seam's: `mergeFilterNodes` rejects a + * comparand the wire would also reject (a bare array on `=`, a `RegExp`) with + * 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. + */ +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; +} diff --git a/packages/plugin-charts/src/ObjectChart.drillNavigate.test.tsx b/packages/plugin-charts/src/ObjectChart.drillNavigate.test.tsx index 7b69f5a43f..32b0bc866c 100644 --- a/packages/plugin-charts/src/ObjectChart.drillNavigate.test.tsx +++ b/packages/plugin-charts/src/ObjectChart.drillNavigate.test.tsx @@ -133,9 +133,15 @@ describe("ObjectChart — DrillDownConfig.target: 'navigate' (objectui#3354)", ( fireEvent.click(screen.getByTestId('fake-segment')); - // Widget filter ∧ click context — the same merge the drawer would have used. + // Widget filter ∧ click context — the same composition the drawer would have + // used. The conjunction was always the intent this comment stated; since + // objectui#8944 it is spelled by the repo's single filter sink + // (`composeDrillFilter`) instead of by spreading the widget's filter into an + // object literal, which only worked when that filter was the object arm. await waitFor(() => - expect(openRecordList).toHaveBeenCalledWith('opportunity', { owner: 'me', stage: 'won' }), + expect(openRecordList).toHaveBeenCalledWith('opportunity', { + $and: [{ owner: 'me' }, { stage: 'won' }], + }), ); expect(screen.queryByTestId('chart-drill-body')).toBeNull(); // …and it does not fire again on subsequent renders. diff --git a/packages/plugin-charts/src/ObjectChart.tsx b/packages/plugin-charts/src/ObjectChart.tsx index 1152e58b6c..c3a53695e6 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, type ElementDataSourceMapping } from '@object-ui/react'; import { ChartRenderer } from './ChartRenderer'; import { normalizeChartSchema } from './normalizeChartSchema'; -import { ComponentRegistry, chartMeasureKey, humanizeLabel, extractRecords, computeDrillFilter, isDrillEnabled, resolveDrillTitle, resolveFilterPlaceholders, resolveContextTokens, shiftFilterByCompareTo, compareToTrendLabelKey, 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, humanizeLabel, extractRecords, computeDrillFilter, composeDrillFilter, isDrillEnabled, resolveDrillTitle, resolveFilterPlaceholders, resolveContextTokens, shiftFilterByCompareTo, compareToTrendLabelKey, 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'; @@ -1007,12 +1007,23 @@ export const ObjectChart = (props: ObjectChartProps) => { // target needs it from an effect, and effects may not live after an early // return. The drawer reads the same value, so both targets drill by exactly // one filter. + // + // ⛔ Composed through `composeDrillFilter`, NOT by spreading the widget's + // filter into an object literal. `schema.filter` admits two arms — a spec + // `FilterArray` and the ObjectQL `$filter` object, both of them read (both go + // to `ds.aggregate` / `ds.find` verbatim above) — and a spread is only correct + // for the second. Spreading the ARRAY arm produced index keys + // (`{ '0': ['stage','=','won'] }`), so the widget's own conditions were + // dropped for a key the query layer ignores and the drilled list showed rows + // 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(() => { if (!drillEvent) return undefined; - return { - ...(schema.filter || {}), - ...computeDrillFilter(drillDown, drillEvent, { groupByField }), - }; + return composeDrillFilter( + schema.filter, + computeDrillFilter(drillDown, drillEvent, { groupByField }), + ); // eslint-disable-next-line react-hooks/exhaustive-deps }, [drillEvent, drillDown, groupByField, filterKey]); diff --git a/packages/plugin-charts/src/__tests__/ObjectChart.drillFilterComposition-8944.test.tsx b/packages/plugin-charts/src/__tests__/ObjectChart.drillFilterComposition-8944.test.tsx new file mode 100644 index 0000000000..a05a3b5fb2 --- /dev/null +++ b/packages/plugin-charts/src/__tests__/ObjectChart.drillFilterComposition-8944.test.tsx @@ -0,0 +1,233 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#8944 — the widget's OWN filter survives into the drill-down query, + * for BOTH arms of `ObjectChartSchema.filter`. + * + * ## The defect + * + * `ObjectChartSchema.filter` admits a spec `FilterArray` + * (`[['region','=','emea']]`) and the ObjectQL `$filter` object + * (`{ region: 'emea' }`), and both are read — both travel verbatim to + * `ds.aggregate` / `ds.find`. The drill seam composed them by SPREADING the + * widget's filter into an object literal: + * + * { ...(schema.filter || {}), ...computeDrillFilter(…) } + * + * Spreading an ARRAY yields index keys, so an authored `FilterArray` drilled as + * `{ '0': ['region','=','emea'], stage: 'won' }` — the widget's conditions + * replaced by a key the query layer ignores. Nothing errored; the drawer opened + * and looked right, scoped by the clicked category ALONE. + * + * ⚠️ The direction matters: the widget's filter is what NARROWS. Dropping it + * makes the drilled list a SUPERSET — it shows records the chart itself was + * scoped to exclude. + * + * ## What these cases assert, and why not the index keys + * + * Asserting that `'0'` is absent from the composed object would pin the SYMPTOM. + * These cases assert the SEMANTICS instead: the composed filter is run through + * `ValueDataSource` — a real matcher for both `$filter` dialects — over a + * fixture built so the three possible outcomes are three different row sets. + * + * widget filter alone (`region = emea`) → a, c + * click context alone (`stage = won`) → a, b ← the pre-fix superset + * both, conjoined → a ← the only correct answer + * + * So a dropped widget filter reads as `['a','b']` (the card's exact failure) and + * a dropped click context as `['a','c']`, rather than both reading as "not the + * expected object". The two single-source answers are asserted as live controls, + * so the fixture cannot go vacuous without saying so. + * + * ## Why the observation point is `openRecordList` + * + * `target: 'navigate'` hands the composed filter to the host verbatim — one spy, + * no DOM archaeology over a drilled table. The drawer arm is pinned through the + * same spy via its "Open in list" escape hatch, which passes the drawer's own + * `merged` value; the two are the one hoisted `drillFilter` memo, and the last + * case proves they agree rather than assuming it. + */ +import React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor, fireEvent } from '@testing-library/react'; +import { DrillNavigationProvider } from '@object-ui/react'; +import { ValueDataSource } from '@object-ui/core'; +import type { ObjectChartSchema } from '@object-ui/types'; + +vi.mock('../ChartRenderer', () => ({ + ChartRenderer: ({ onChartClick }: any) => ( + + ), +})); + +import { ObjectChart } from '../ObjectChart'; + +const OBJECT = 'crm_opportunity'; + +/** + * Three rows chosen so each source excludes a DIFFERENT one: `b` survives only + * if the widget filter is lost, `c` only if the click context is lost. + */ +const ROWS = [ + { id: 'a', stage: 'won', region: 'emea' }, + { id: 'b', stage: 'won', region: 'apac' }, + { id: 'c', stage: 'lost', region: 'emea' }, +]; + +/** Run a composed filter through a real matcher and project the ids it selects. */ +async function selectedIds(filter: unknown): Promise { + const ds = new ValueDataSource({ items: ROWS }); + const result = await ds.find(OBJECT, { $filter: filter as any }); + return result.data.map((r: any) => r.id as string); +} + +let metaCalls: string[] = []; +beforeEach(() => { + metaCalls = []; + vi.stubGlobal( + 'fetch', + vi.fn(async (input: unknown) => { + metaCalls.push(String(input)); + return { ok: true, json: async () => ({}) }; + }), + ); +}); +afterEach(() => { + expect(metaCalls.filter((u) => u !== `/api/v1/meta/object/${OBJECT}`)).toEqual([]); + vi.unstubAllGlobals(); + cleanup(); +}); + +function renderChart( + filter: ObjectChartSchema['filter'], + drillDown: Record, +) { + const openRecordList = vi.fn(); + render( + + ({ data: [] }) }} + /> + , + ); + return openRecordList; +} + +/** Click a segment through the navigate arm and hand back the composed filter. */ +async function drillViaNavigate(filter: ObjectChartSchema['filter']): Promise { + const openRecordList = renderChart(filter, { enabled: true, target: 'navigate' }); + fireEvent.click(screen.getByTestId('fake-segment')); + await waitFor(() => expect(openRecordList).toHaveBeenCalled()); + const [objectName, composed] = openRecordList.mock.calls[0]; + expect(objectName).toBe(OBJECT); + return composed; +} + +describe('objectui#8944 — the fixture discriminates (live controls)', () => { + it('each source alone selects a DIFFERENT row set, so a dropped source is visible', async () => { + // Green before and after the fix. Its job is to prove the rows and the + // matcher work, so a red below is about the composition rather than the + // harness — and to name the pre-fix superset explicitly. + expect(await selectedIds({ region: 'emea' })).toEqual(['a', 'c']); + expect(await selectedIds({ stage: 'won' })).toEqual(['a', 'b']); + expect(await selectedIds(undefined)).toEqual(['a', 'b', 'c']); + }); +}); + +describe('objectui#8944 — the ARRAY arm survives the drill', () => { + it('conjoins a spec FilterArray with the click context, and the result still CONSTRAINS', async () => { + const composed = await drillViaNavigate([['region', '=', 'emea']]); + + // The semantics: only the intersection. `['a','b']` here would be the + // pre-fix answer — the widget's filter dropped, the list widened to the + // clicked category alone. + expect(await selectedIds(composed)).toEqual(['a']); + + // The spelling the repo's single filter sink produces for two sources. + expect(composed).toEqual({ $and: [{ region: 'emea' }, { stage: 'won' }] }); + }); + + it('carries EVERY condition of a multi-condition FilterArray, not just the first', async () => { + // A second condition the click context does not mention: if the array arm + // were being lowered one-condition-deep, `c` would come back. + const composed = await drillViaNavigate([ + ['region', '=', 'emea'], + ['stage', '!=', 'lost'], + ]); + expect(await selectedIds(composed)).toEqual(['a']); + }); +}); + +describe('objectui#8944 — the OBJECT arm still composes (no regression)', () => { + it('conjoins the ObjectQL $filter object with the click context', async () => { + const composed = await drillViaNavigate({ region: 'emea' }); + expect(await selectedIds(composed)).toEqual(['a']); + expect(composed).toEqual({ $and: [{ region: 'emea' }, { stage: 'won' }] }); + }); + + it('a chart with NO filter of its own drills exactly as it did before', async () => { + // One surviving source lowers back to the flat object the spread produced, + // so this path is byte-identical to the pre-fix behaviour. + const composed = await drillViaNavigate(undefined); + expect(composed).toEqual({ stage: 'won' }); + expect(await selectedIds(composed)).toEqual(['a', 'b']); + }); +}); + +describe('objectui#8944 — both filter sources together', () => { + it('conjoins the widget ARRAY arm with an authored drillDown.filter', async () => { + // `drillDown.filter` replaces the derived click context, so this composes + // two independently authored sources in two different dialects. + const openRecordList = renderChart([['region', '=', 'emea']], { + enabled: true, + target: 'navigate', + filter: { stage: '${event.category}' }, + }); + fireEvent.click(screen.getByTestId('fake-segment')); + await waitFor(() => expect(openRecordList).toHaveBeenCalled()); + const composed = openRecordList.mock.calls[0][1]; + + expect(await selectedIds(composed)).toEqual(['a']); + expect(composed).toEqual({ $and: [{ region: 'emea' }, { stage: 'won' }] }); + }); +}); + +describe('objectui#8944 — the drawer sink drills by the SAME composed filter', () => { + it("the drawer's 'Open in list' hands the host what the navigate arm hands it", async () => { + // The drawer builds `merged` from the same hoisted memo. Pinning it through + // the escape hatch keeps the assertion on the composed VALUE rather than on + // rows rendered by a table this package does not own. + const openRecordList = renderChart([['region', '=', 'emea']], { enabled: true }); + fireEvent.click(screen.getByTestId('fake-segment')); + + await waitFor(() => expect(screen.getByTestId('chart-drill-body')).toBeTruthy()); + fireEvent.click(screen.getByTestId('drill-open-in-list')); + + await waitFor(() => expect(openRecordList).toHaveBeenCalled()); + const composed = openRecordList.mock.calls[0][1]; + expect(composed).toEqual({ $and: [{ region: 'emea' }, { stage: 'won' }] }); + expect(await selectedIds(composed)).toEqual(['a']); + }); +}); diff --git a/packages/types/src/objectql.ts b/packages/types/src/objectql.ts index 49a3b557a5..8ac14f830b 100644 --- a/packages/types/src/objectql.ts +++ b/packages/types/src/objectql.ts @@ -3306,8 +3306,8 @@ export interface ObjectChartSchema extends BaseSchema { values?: string[]; /** * AUTHORABLE — query filter, forwarded verbatim as `$filter` on both query - * legs (`ds.aggregate` and `ds.find`), then spread into the drill-down - * filter. + * legs (`ds.aggregate` and `ds.find`), and conjoined with the click context + * to scope a drill-down. * * ⚠️ BOTH shapes, and the union is measured rather than tidied. The array arm * is what `@objectstack/spec` publishes for this prop (`ObjectChart.filter` @@ -3315,10 +3315,9 @@ export interface ObjectChartSchema extends BaseSchema { * registry `inputs` advertises (`{ name: 'filter', type: 'array' }`) — it is * the spelling {@link ObjectGanttSchema.filter} and * {@link ObjectKanbanSchema.filter} carry. The RECORD arm is what the reads - * require: the drill-down filter is built by spreading this value into an - * object (`{ ...(schema.filter || {}), ...computeDrillFilter(…) }`), and the - * in-repo corpus authors the ObjectQL object form - * (`{ close_date: { $gte, $lte } }`) against fakes that read it that way. + * require: the in-repo corpus authors the ObjectQL object form + * (`{ close_date: { $gte, $lte } }`) against fakes that read it that way, and + * both arms travel verbatim to `ds.aggregate` / `ds.find` as `$filter`. * Declaring only the array arm would have refused live, working charts. * * ⚠️ Narrowing to ONE arm is a decision LOCAL TO THIS NODE, not a @@ -3331,12 +3330,19 @@ export interface ObjectChartSchema extends BaseSchema { * only this component's own two-armed read, and objectui#7946 declares the * accept set it measured rather than picking an arm without a ruling. * - * ⭐ Successor, named rather than implied: the drill-down spread below - * (`{ ...(schema.filter || {}), ...computeDrillFilter(…) }`) MIS-COMPOSES the - * array arm — spreading a `FilterArray` into an object yields index keys - * (`{ 0: […] }`), not conditions. Fixing that composition is the work that - * makes narrowing to the spec's array-only `FilterArray` possible; until it - * lands, declaring only the array arm would refuse live, working charts. + * ⭐ The composition that blocked narrowing is FIXED (objectui#8944). The + * drill-down used to compose this value by SPREADING it into an object + * literal, which mis-composed the array arm into index keys (`{ 0: […] }`) + * instead of conditions, so an authored `FilterArray` was silently dropped + * from the drilled query. `ObjectChart` now composes through + * `composeDrillFilter` (`@object-ui/core`), which routes both arms into the + * repo's single filter sink — so the array arm survives the drill, and the + * read that forced the record arm to be declared is gone. + * + * ⇒ What remains before this node can narrow to the spec's array-only + * `FilterArray` is a DEPRECATION, not a defect: live charts author the object + * form today, and narrowing stops them compiling. That migration is its own + * card; this docblock no longer names a bug as the blocker. * * What this declaration buys today is that `filter: 'stage=won'` and * `filter: 42` are compile errors, where before they were not.