From 4783bf4f0a2ff1bf1b0a1cef998aeee385e92e9b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 18:01:15 +0000 Subject: [PATCH] fix(plugin-charts,core): conjoin the chart drill filter instead of spreading it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ObjectChartSchema.filter` admits both a spec `FilterArray` and the ObjectQL `$filter` object, and both are read. 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': [...], stage: 'won' }` and the widget's own conditions were dropped for a key the query layer ignores. The direction matters: the widget's filter is what narrows, so dropping it made the drilled list a superset of what the chart itself was scoped to. Compose through a new `composeDrillFilter` seam in `@object-ui/core`, which applies the rule `widget.filter AND drill.filter` via `mergeFilterNodes` — the repo's single filter sink, whose contract already states it — and lowers the result back to the FilterCondition object dialect with `parseFilterAST`. A lone surviving source lowers back to the flat object the spread produced, so a chart with no filter of its own drills exactly as before. `serializeDrillFilterParams` learns to flatten that `$and` into the flat `filter[...]` params its own read side already ANDs back together; without it a composed filter took the `String(value)` path and emitted a bogus `filter[$and]` while both real conditions vanished. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB --- .../8944-chart-drill-filter-composition.md | 44 ++++ .../src/views/drillUrlFilters.test.ts | 51 ++++ .../app-shell/src/views/drillUrlFilters.ts | 43 +++- packages/core/src/utils/drill-down.ts | 72 ++++++ .../src/ObjectChart.drillNavigate.test.tsx | 10 +- packages/plugin-charts/src/ObjectChart.tsx | 21 +- ...Chart.drillFilterComposition-8944.test.tsx | 233 ++++++++++++++++++ packages/types/src/objectql.ts | 30 ++- 8 files changed, 484 insertions(+), 20 deletions(-) create mode 100644 .changeset/8944-chart-drill-filter-composition.md create mode 100644 packages/plugin-charts/src/__tests__/ObjectChart.drillFilterComposition-8944.test.tsx 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.