From b4ac6e84f921fe2d0101d3a96dac86277a3afdea Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Wed, 22 Apr 2026 07:41:10 -0400 Subject: [PATCH 1/4] refactor: Move filtersToQuery to common-utils --- packages/app/src/DashboardFilters.tsx | 2 +- .../app/src/__tests__/searchFilters.test.ts | 120 +---------------- .../src/components/DBSearchPageFilters.tsx | 2 +- .../DBSearchPageFilters/NestedFilterGroup.tsx | 3 +- .../usePresetDashboardFilters.test.tsx | 2 +- .../app/src/hooks/useDashboardFilters.tsx | 6 +- packages/app/src/searchFilters.tsx | 57 +-------- .../src/__tests__/filters.test.ts | 121 ++++++++++++++++++ packages/common-utils/src/filters.ts | 54 ++++++++ 9 files changed, 189 insertions(+), 178 deletions(-) create mode 100644 packages/common-utils/src/__tests__/filters.test.ts create mode 100644 packages/common-utils/src/filters.ts diff --git a/packages/app/src/DashboardFilters.tsx b/packages/app/src/DashboardFilters.tsx index 331984c36f..38cc9d0d72 100644 --- a/packages/app/src/DashboardFilters.tsx +++ b/packages/app/src/DashboardFilters.tsx @@ -1,9 +1,9 @@ +import { FilterState } from '@hyperdx/common-utils/dist/filters'; import { DashboardFilter } from '@hyperdx/common-utils/dist/types'; import { Group, MultiSelect } from '@mantine/core'; import { IconRefresh } from '@tabler/icons-react'; import { useDashboardFilterValues } from './hooks/useDashboardFilterValues'; -import { FilterState } from './searchFilters'; interface DashboardFilterSelectProps { filter: DashboardFilter; diff --git a/packages/app/src/__tests__/searchFilters.test.ts b/packages/app/src/__tests__/searchFilters.test.ts index 5a001e0a70..e797ccf736 100644 --- a/packages/app/src/__tests__/searchFilters.test.ts +++ b/packages/app/src/__tests__/searchFilters.test.ts @@ -1,9 +1,9 @@ import { enableMapSet } from 'immer'; +import { filtersToQuery } from '@hyperdx/common-utils/dist/filters'; import { act, renderHook } from '@testing-library/react'; import { areFiltersEqual, - filtersToQuery, parseQuery, useSearchPageFilterState, } from '../searchFilters'; @@ -11,124 +11,6 @@ import { enableMapSet(); describe('searchFilters', () => { - describe('filtersToQuery', () => { - it('should return empty string when no filters', () => { - const filters = {}; - expect(filtersToQuery(filters)).toEqual([]); - }); - - it('should return query for one filter', () => { - const filters = { - a: { included: new Set(['b']), excluded: new Set() }, - }; - expect(filtersToQuery(filters)).toEqual([ - { type: 'sql', condition: "a IN ('b')" }, - ]); - }); - - it('should return query for multiple filters', () => { - const filters = { - a: { included: new Set(['b']), excluded: new Set() }, - c: { - included: new Set(['d', 'x']), - excluded: new Set(), - }, - }; - expect(filtersToQuery(filters)).toEqual([ - { type: 'sql', condition: "a IN ('b')" }, - { type: 'sql', condition: "c IN ('d', 'x')" }, - ]); - }); - - it('should handle excluded values', () => { - const filters = { - a: { - included: new Set(['b']), - excluded: new Set(['c']), - }, - }; - expect(filtersToQuery(filters)).toEqual([ - { type: 'sql', condition: "a IN ('b')" }, - { type: 'sql', condition: "a NOT IN ('c')" }, - ]); - }); - - it('should wrap keys with toString() when specified', () => { - const filters = { - 'json.key': { - included: new Set(['value']), - excluded: new Set(['other value']), - }, - }; - expect(filtersToQuery(filters, { stringifyKeys: true })).toEqual([ - { type: 'sql', condition: "toString(json.key) IN ('value')" }, - { type: 'sql', condition: "toString(json.key) NOT IN ('other value')" }, - ]); - }); - - it('should should handle boolean filter values', () => { - const filters = { - isRootSpan: { - included: new Set([true]), - excluded: new Set([]), - }, - another_column: { - included: new Set([]), - excluded: new Set([true, false]), - }, - }; - expect(filtersToQuery(filters)).toEqual([ - { type: 'sql', condition: 'isRootSpan IN (true)' }, - { type: 'sql', condition: 'another_column NOT IN (true, false)' }, - ]); - }); - - it('should escape single quotes in filter values', () => { - const filters = { - message: { - included: new Set(["my 'filter' key"]), - excluded: new Set(), - }, - }; - expect(filtersToQuery(filters)).toEqual([ - { - type: 'sql', - condition: "message IN ('my ''filter'' key')", - }, - ]); - }); - - it('should escape single quotes in excluded filter values', () => { - const filters = { - message: { - included: new Set(), - excluded: new Set(["it's a test"]), - }, - }; - expect(filtersToQuery(filters)).toEqual([ - { - type: 'sql', - condition: "message NOT IN ('it''s a test')", - }, - ]); - }); - - it('should escape single quotes with stringifyKeys', () => { - const filters = { - 'json.key': { - included: new Set(["value with 'quotes'"]), - excluded: new Set(), - }, - }; - expect(filtersToQuery(filters, { stringifyKeys: true })).toEqual([ - { - type: 'sql', - condition: "toString(json.key) IN ('value with ''quotes''')", - }, - ]); - }); - }); - describe('parseQuery', () => { it('empty query', () => { const result = parseQuery([]); diff --git a/packages/app/src/components/DBSearchPageFilters.tsx b/packages/app/src/components/DBSearchPageFilters.tsx index 49f0645598..2b5a541537 100644 --- a/packages/app/src/components/DBSearchPageFilters.tsx +++ b/packages/app/src/components/DBSearchPageFilters.tsx @@ -4,6 +4,7 @@ import { TableMetadata, tcFromSource, } from '@hyperdx/common-utils/dist/core/metadata'; +import { FilterState } from '@hyperdx/common-utils/dist/filters'; import { BuilderChartConfigWithDateRange, SourceKind, @@ -60,7 +61,6 @@ import { useMetadataWithSettings } from '@/hooks/useMetadata'; import useResizable from '@/hooks/useResizable'; import { usePinnedFiltersApi } from '@/pinnedFilters'; import { - type FilterState, FilterStateHook, IS_ROOT_SPAN_COLUMN_NAME, usePinnedFilters, diff --git a/packages/app/src/components/DBSearchPageFilters/NestedFilterGroup.tsx b/packages/app/src/components/DBSearchPageFilters/NestedFilterGroup.tsx index 027aeee10e..8a3106b5cc 100644 --- a/packages/app/src/components/DBSearchPageFilters/NestedFilterGroup.tsx +++ b/packages/app/src/components/DBSearchPageFilters/NestedFilterGroup.tsx @@ -1,9 +1,8 @@ import { useMemo, useRef, useState } from 'react'; +import { FilterState } from '@hyperdx/common-utils/dist/filters'; import { Accordion, Group, Text, Tooltip, UnstyledButton } from '@mantine/core'; import { useVirtualizer } from '@tanstack/react-virtual'; -import { FilterState } from '@/searchFilters'; - import { FilterGroup } from '../DBSearchPageFilters'; import classes from '../../../styles/SearchPage.module.scss'; diff --git a/packages/app/src/hooks/__tests__/usePresetDashboardFilters.test.tsx b/packages/app/src/hooks/__tests__/usePresetDashboardFilters.test.tsx index 4c9ccbce58..85d27fe04b 100644 --- a/packages/app/src/hooks/__tests__/usePresetDashboardFilters.test.tsx +++ b/packages/app/src/hooks/__tests__/usePresetDashboardFilters.test.tsx @@ -1,3 +1,4 @@ +import { FilterState } from '@hyperdx/common-utils/dist/filters'; import { DashboardFilter, Filter, @@ -7,7 +8,6 @@ import { import { act, renderHook } from '@testing-library/react'; import api from '@/api'; -import { FilterState } from '@/searchFilters'; import useDashboardFilters from '../useDashboardFilters'; import usePresetDashboardFilters from '../usePresetDashboardFilters'; diff --git a/packages/app/src/hooks/useDashboardFilters.tsx b/packages/app/src/hooks/useDashboardFilters.tsx index 7cdcf7f0ee..4270016fe5 100644 --- a/packages/app/src/hooks/useDashboardFilters.tsx +++ b/packages/app/src/hooks/useDashboardFilters.tsx @@ -1,8 +1,12 @@ import { useCallback, useMemo } from 'react'; import { useQueryState } from 'nuqs'; +import { + FilterState, + filtersToQuery, +} from '@hyperdx/common-utils/dist/filters'; import { DashboardFilter, Filter } from '@hyperdx/common-utils/dist/types'; -import { FilterState, filtersToQuery, parseQuery } from '@/searchFilters'; +import { parseQuery } from '@/searchFilters'; import { parseAsJsonEncoded } from '@/utils/queryParsers'; const filterQueriesParser = parseAsJsonEncoded(); diff --git a/packages/app/src/searchFilters.tsx b/packages/app/src/searchFilters.tsx index 4deb6e07f7..3f9db3cc1a 100644 --- a/packages/app/src/searchFilters.tsx +++ b/packages/app/src/searchFilters.tsx @@ -1,5 +1,9 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import produce from 'immer'; +import { + type FilterState, + filtersToQuery, +} from '@hyperdx/common-utils/dist/filters'; import type { Filter } from '@hyperdx/common-utils/dist/types'; import { usePinnedFiltersApi, useUpdatePinnedFilters } from './pinnedFilters'; @@ -7,59 +11,6 @@ import { useLocalStorage } from './utils'; export const IS_ROOT_SPAN_COLUMN_NAME = 'isRootSpan'; -export type FilterState = { - [key: string]: { - included: Set; - excluded: Set; - range?: { min: number; max: number }; // For BETWEEN conditions - }; -}; - -export const filtersToQuery = ( - filters: FilterState, - { stringifyKeys = false }: { stringifyKeys?: boolean } = {}, -): Filter[] => { - return Object.entries(filters) - .filter( - ([_, values]) => - values.included.size > 0 || - values.excluded.size > 0 || - values.range != null, - ) - .flatMap(([key, values]) => { - const conditions = []; - const actualKey = stringifyKeys ? `toString(${key})` : key; - - if (values.included.size > 0) { - conditions.push({ - type: 'sql' as const, - condition: `${actualKey} IN (${Array.from(values.included) - .map(v => - typeof v === 'string' ? `'${v.replace(/'/g, "''")}'` : v, - ) - .join(', ')})`, - }); - } - if (values.excluded.size > 0) { - conditions.push({ - type: 'sql' as const, - condition: `${actualKey} NOT IN (${Array.from(values.excluded) - .map(v => - typeof v === 'string' ? `'${v.replace(/'/g, "''")}'` : v, - ) - .join(', ')})`, - }); - } - if (values.range != null) { - conditions.push({ - type: 'sql' as const, - condition: `${actualKey} BETWEEN ${values.range.min} AND ${values.range.max}`, - }); - } - return conditions; - }); -}; - export const areFiltersEqual = (a: FilterState, b: FilterState) => { const aKeys = Object.keys(a); const bKeys = Object.keys(b); diff --git a/packages/common-utils/src/__tests__/filters.test.ts b/packages/common-utils/src/__tests__/filters.test.ts new file mode 100644 index 0000000000..743ca2fbff --- /dev/null +++ b/packages/common-utils/src/__tests__/filters.test.ts @@ -0,0 +1,121 @@ +import { filtersToQuery } from '@/filters'; + +describe('searchFilters', () => { + describe('filtersToQuery', () => { + it('should return empty string when no filters', () => { + const filters = {}; + expect(filtersToQuery(filters)).toEqual([]); + }); + + it('should return query for one filter', () => { + const filters = { + a: { included: new Set(['b']), excluded: new Set() }, + }; + expect(filtersToQuery(filters)).toEqual([ + { type: 'sql', condition: "a IN ('b')" }, + ]); + }); + + it('should return query for multiple filters', () => { + const filters = { + a: { included: new Set(['b']), excluded: new Set() }, + c: { + included: new Set(['d', 'x']), + excluded: new Set(), + }, + }; + expect(filtersToQuery(filters)).toEqual([ + { type: 'sql', condition: "a IN ('b')" }, + { type: 'sql', condition: "c IN ('d', 'x')" }, + ]); + }); + + it('should handle excluded values', () => { + const filters = { + a: { + included: new Set(['b']), + excluded: new Set(['c']), + }, + }; + expect(filtersToQuery(filters)).toEqual([ + { type: 'sql', condition: "a IN ('b')" }, + { type: 'sql', condition: "a NOT IN ('c')" }, + ]); + }); + + it('should wrap keys with toString() when specified', () => { + const filters = { + 'json.key': { + included: new Set(['value']), + excluded: new Set(['other value']), + }, + }; + expect(filtersToQuery(filters, { stringifyKeys: true })).toEqual([ + { type: 'sql', condition: "toString(json.key) IN ('value')" }, + { type: 'sql', condition: "toString(json.key) NOT IN ('other value')" }, + ]); + }); + + it('should should handle boolean filter values', () => { + const filters = { + isRootSpan: { + included: new Set([true]), + excluded: new Set([]), + }, + another_column: { + included: new Set([]), + excluded: new Set([true, false]), + }, + }; + expect(filtersToQuery(filters)).toEqual([ + { type: 'sql', condition: 'isRootSpan IN (true)' }, + { type: 'sql', condition: 'another_column NOT IN (true, false)' }, + ]); + }); + + it('should escape single quotes in filter values', () => { + const filters = { + message: { + included: new Set(["my 'filter' key"]), + excluded: new Set(), + }, + }; + expect(filtersToQuery(filters)).toEqual([ + { + type: 'sql', + condition: "message IN ('my ''filter'' key')", + }, + ]); + }); + + it('should escape single quotes in excluded filter values', () => { + const filters = { + message: { + included: new Set(), + excluded: new Set(["it's a test"]), + }, + }; + expect(filtersToQuery(filters)).toEqual([ + { + type: 'sql', + condition: "message NOT IN ('it''s a test')", + }, + ]); + }); + + it('should escape single quotes with stringifyKeys', () => { + const filters = { + 'json.key': { + included: new Set(["value with 'quotes'"]), + excluded: new Set(), + }, + }; + expect(filtersToQuery(filters, { stringifyKeys: true })).toEqual([ + { + type: 'sql', + condition: "toString(json.key) IN ('value with ''quotes''')", + }, + ]); + }); + }); +}); diff --git a/packages/common-utils/src/filters.ts b/packages/common-utils/src/filters.ts new file mode 100644 index 0000000000..29ca8665cc --- /dev/null +++ b/packages/common-utils/src/filters.ts @@ -0,0 +1,54 @@ +import { Filter } from '@/types'; + +export type FilterState = { + [key: string]: { + included: Set; + excluded: Set; + range?: { min: number; max: number }; // For BETWEEN conditions + }; +}; + +export const filtersToQuery = ( + filters: FilterState, + { stringifyKeys = false }: { stringifyKeys?: boolean } = {}, +): Filter[] => { + return Object.entries(filters) + .filter( + ([_, values]) => + values.included.size > 0 || + values.excluded.size > 0 || + values.range != null, + ) + .flatMap(([key, values]) => { + const conditions: Filter[] = []; + const actualKey = stringifyKeys ? `toString(${key})` : key; + + if (values.included.size > 0) { + conditions.push({ + type: 'sql' as const, + condition: `${actualKey} IN (${Array.from(values.included) + .map(v => + typeof v === 'string' ? `'${v.replace(/'/g, "''")}'` : v, + ) + .join(', ')})`, + }); + } + if (values.excluded.size > 0) { + conditions.push({ + type: 'sql' as const, + condition: `${actualKey} NOT IN (${Array.from(values.excluded) + .map(v => + typeof v === 'string' ? `'${v.replace(/'/g, "''")}'` : v, + ) + .join(', ')})`, + }); + } + if (values.range != null) { + conditions.push({ + type: 'sql' as const, + condition: `${actualKey} BETWEEN ${values.range.min} AND ${values.range.max}`, + }); + } + return conditions; + }); +}; From f24c74067e08863ff8c7fdf837a883111063fa0e Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Wed, 22 Apr 2026 08:26:53 -0400 Subject: [PATCH 2/4] feat: Add filter templating to custom dashboard on-click --- .changeset/shaggy-experts-collect.md | 6 + packages/app/src/DBDashboardPage.tsx | 44 ++- .../OnClickForm/FilterTemplateList.tsx | 82 +++++ .../OnClickForm/OnClickDrawer.tsx | 88 ++++- .../OnClickTargetInputControlled.tsx | 14 +- .../usePresetDashboardFilters.test.tsx | 1 + .../app/src/hooks/useDashboardFilters.tsx | 51 ++- .../e2e/components/ChartEditorComponent.ts | 43 +++ .../features/dashboard-table-linking.spec.ts | 227 ++++++++++++ .../tests/e2e/page-objects/DashboardPage.ts | 19 + .../src/core/__tests__/linkUrlBuilder.test.ts | 346 ++++++++++++++++++ .../common-utils/src/core/linkUrlBuilder.ts | 64 +++- packages/common-utils/src/types.ts | 9 + 13 files changed, 959 insertions(+), 35 deletions(-) create mode 100644 .changeset/shaggy-experts-collect.md create mode 100644 packages/app/src/components/DBEditTimeChartForm/OnClickForm/FilterTemplateList.tsx diff --git a/.changeset/shaggy-experts-collect.md b/.changeset/shaggy-experts-collect.md new file mode 100644 index 0000000000..123e15f84a --- /dev/null +++ b/.changeset/shaggy-experts-collect.md @@ -0,0 +1,6 @@ +--- +"@hyperdx/common-utils": patch +"@hyperdx/app": patch +--- + +feat: Add filter templating to custom dashboard on-click diff --git a/packages/app/src/DBDashboardPage.tsx b/packages/app/src/DBDashboardPage.tsx index 4370020943..272c7b3aa7 100644 --- a/packages/app/src/DBDashboardPage.tsx +++ b/packages/app/src/DBDashboardPage.tsx @@ -47,6 +47,7 @@ import { } from '@hyperdx/common-utils/dist/types'; import { ActionIcon, + Alert, Anchor, Box, Breadcrumbs, @@ -64,6 +65,7 @@ import { import { useHotkeys } from '@mantine/hooks'; import { notifications } from '@mantine/notifications'; import { + IconAlertTriangle, IconArrowsMaximize, IconBell, IconChartBar, @@ -1088,8 +1090,29 @@ function DBDashboardPage({ presetConfig }: { presetConfig?: Dashboard }) { const [showFiltersModal, setShowFiltersModal] = useState(false); const filters = dashboard?.filters ?? []; - const { filterValues, setFilterValue, filterQueries, setFilterQueries } = - useDashboardFilters(filters); + const { + filterValues, + setFilterValue, + filterQueries, + setFilterQueries, + ignoredFilterExpressions, + } = useDashboardFilters(filters); + + // Warn when the URL has filter values that don't correspond to any declared + // dashboard filter — they'd otherwise be silently dropped, and users who + // arrive via a shared link, bookmark, or onClick action might not notice. + // Only consider URL filters ignored once the dashboard has finished loading + // so we don't flash the banner before `dashboard.filters` is available. + const dashboardReady = + !!dashboard?.id && + router.isReady && + (isLocalDashboard || !isFetchingDashboard); + const [dismissedIgnoredFilters, setDismissedIgnoredFilters] = + useState(false); + const shouldShowIgnoredFiltersBanner = + dashboardReady && + ignoredFilterExpressions.length > 0 && + !dismissedIgnoredFilters; const handleSaveFilter = (filter: DashboardFilter) => { if (!dashboard) return; @@ -2137,6 +2160,23 @@ function DBDashboardPage({ presetConfig }: { presetConfig?: Dashboard }) { Run + {shouldShowIgnoredFiltersBanner && ( + } + title="Some filters could not be applied" + data-testid="ignored-url-filters-banner" + withCloseButton + closeButtonLabel="Dismiss" + onClose={() => setDismissedIgnoredFilters(true)} + > + No dashboard filter(s) found for{' '} + {ignoredFilterExpressions.length === 1 ? 'expression' : 'expressions'}{' '} + in the URL: {ignoredFilterExpressions.join(', ')}. Add a filter with a + matching expression to apply these filters. + + )} + Filters + + Enter an expression (e.g. a column name) and a template for its value. + + + {filters.map((filter, i) => ( + + + + remove(i)} + mt={3} + data-testid="onclick-filter-remove-button" + > + + + + ))} + + + + ); +} diff --git a/packages/app/src/components/DBEditTimeChartForm/OnClickForm/OnClickDrawer.tsx b/packages/app/src/components/DBEditTimeChartForm/OnClickForm/OnClickDrawer.tsx index c92f5af652..5fe005c7b6 100644 --- a/packages/app/src/components/DBEditTimeChartForm/OnClickForm/OnClickDrawer.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/OnClickForm/OnClickDrawer.tsx @@ -1,8 +1,18 @@ import { useCallback, useEffect, useMemo } from 'react'; -import { Controller, useForm, useWatch } from 'react-hook-form'; +import { + Controller, + useForm, + UseFormGetValues, + UseFormSetValue, + useWatch, +} from 'react-hook-form'; import { zodResolver } from '@hookform/resolvers/zod'; import { validateOnClickTemplate } from '@hyperdx/common-utils/dist/core/linkUrlBuilder'; -import { isSearchableSource, OnClick } from '@hyperdx/common-utils/dist/types'; +import { + isSearchableSource, + OnClick, + OnClickTarget, +} from '@hyperdx/common-utils/dist/types'; import { Box, Button, @@ -20,6 +30,7 @@ import SearchWhereInput from '@/components/SearchInput/SearchWhereInput'; import { useDashboards } from '@/dashboard'; import { useSources } from '@/source'; +import { FilterTemplateList } from './FilterTemplateList'; import { OnClickTargetInputControlled } from './OnClickTargetInputControlled'; import { DrawerControl, @@ -53,6 +64,8 @@ function SearchOnClickFields({ control }: { control: DrawerControl }) { objectType="source" /> + + ; + getValues: UseFormGetValues; +}) { const { data: dashboards } = useDashboards(); const dashboardOptions = useMemo(() => { return dashboards?.map(dashboard => ({ @@ -81,6 +102,31 @@ function DashboardOnClickFields({ control }: { control: DrawerControl }) { })); }, [dashboards]); + // When the target dashboard changes, create empty filter templates + // for each of the target dashboard's existing filters + // (if the current templates are all empty). + const handleTargetChange = useCallback( + (target: OnClickTarget) => { + if (target.mode !== 'id') return; + const selected = dashboards?.find(d => d.id === target.id); + const dashboardFilters = selected?.filters ?? []; + + const currentFilters = getValues('onClick.filters') ?? []; + const allTemplatesEmpty = currentFilters.every(f => f.template === ''); + if (!allTemplatesEmpty) return; + + setValue( + 'onClick.filters', + dashboardFilters.map(f => ({ + kind: 'expressionTemplate' as const, + expression: f.expression, + template: '', + })), + ); + }, + [dashboards, setValue, getValues], + ); + return ( @@ -91,8 +137,11 @@ function DashboardOnClickFields({ control }: { control: DrawerControl }) { control={control} options={dashboardOptions} objectType="dashboard" + onTargetChange={handleTargetChange} /> + + ; + getValues: UseFormGetValues; +}) { const onClick = useWatch({ control, name: 'onClick' }); if (onClick?.type === 'search') { return ; } else if (onClick?.type === 'dashboard') { - return ; + return ( + + ); } return ( @@ -147,10 +210,11 @@ export default function OnClickDrawer({ [value], ); - const { control, handleSubmit, reset, setValue } = useForm({ - defaultValues: appliedDefaults, - resolver: zodResolver(DrawerSchema), - }); + const { control, handleSubmit, reset, setValue, getValues } = + useForm({ + defaultValues: appliedDefaults, + resolver: zodResolver(DrawerSchema), + }); // Whenever the drawer is (re)opened with a fresh value from the parent, // sync the local form to that value. Reopening after a cancel should not @@ -239,7 +303,11 @@ export default function OnClickDrawer({ )} /> - + diff --git a/packages/app/src/components/DBEditTimeChartForm/OnClickForm/OnClickTargetInputControlled.tsx b/packages/app/src/components/DBEditTimeChartForm/OnClickForm/OnClickTargetInputControlled.tsx index 94c5e25de8..d516c442b1 100644 --- a/packages/app/src/components/DBEditTimeChartForm/OnClickForm/OnClickTargetInputControlled.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/OnClickForm/OnClickTargetInputControlled.tsx @@ -1,5 +1,6 @@ import { useMemo } from 'react'; import { Controller } from 'react-hook-form'; +import { OnClickTarget } from '@hyperdx/common-utils/dist/types'; import { Select } from '@mantine/core'; import { TextInputControlled } from '@/components/InputControlled'; @@ -13,10 +14,12 @@ export function OnClickTargetInputControlled({ control, options, objectType, + onTargetChange, }: { control: DrawerControl; options: { label: string; value: string }[] | undefined; objectType: 'source' | 'dashboard'; + onTargetChange?: (target: OnClickTarget) => void; }) { const optionsWithTemplate = useMemo(() => { return [ @@ -76,11 +79,12 @@ export function OnClickTargetInputControlled({ : undefined } onChange={value => { - if (value === TEMPLATE_SELECT_VALUE) { - field.onChange({ mode: 'template', template: '' }); - } else { - field.onChange({ mode: 'id', id: value ?? '' }); - } + const newTarget: OnClickTarget = + value === TEMPLATE_SELECT_VALUE + ? { mode: 'template', template: '' } + : { mode: 'id', id: value ?? '' }; + field.onChange(newTarget); + onTargetChange?.(newTarget); }} /> {field.value?.mode === 'template' && ( diff --git a/packages/app/src/hooks/__tests__/usePresetDashboardFilters.test.tsx b/packages/app/src/hooks/__tests__/usePresetDashboardFilters.test.tsx index 85d27fe04b..377cb45f16 100644 --- a/packages/app/src/hooks/__tests__/usePresetDashboardFilters.test.tsx +++ b/packages/app/src/hooks/__tests__/usePresetDashboardFilters.test.tsx @@ -109,6 +109,7 @@ describe('usePresetDashboardFilters', () => { setFilterValue: mockSetFilterValue, filterQueries: mockFilterQueries, setFilterQueries: jest.fn(), + ignoredFilterExpressions: [], }); }); diff --git a/packages/app/src/hooks/useDashboardFilters.tsx b/packages/app/src/hooks/useDashboardFilters.tsx index 4270016fe5..e1b03c6c40 100644 --- a/packages/app/src/hooks/useDashboardFilters.tsx +++ b/packages/app/src/hooks/useDashboardFilters.tsx @@ -39,33 +39,50 @@ const useDashboardFilters = (filters: DashboardFilter[]) => { [setFilterQueries], ); - const { valuesForExistingFilters, queriesForExistingFilters } = - useMemo(() => { - const { filters: parsedFilters } = parseQuery(filterQueries ?? []); - const valuesForExistingFilters: FilterState = {}; + const { + valuesForExistingFilters, + queriesForExistingFilters, + ignoredExpressions, + } = useMemo(() => { + const { filters: parsedFilters } = parseQuery(filterQueries ?? []); + const valuesForExistingFilters: FilterState = {}; + const knownExpressions = new Set(filters.map(f => f.expression)); + const ignored: string[] = []; - for (const { expression } of filters) { - if (expression in parsedFilters) { - valuesForExistingFilters[expression] = parsedFilters[expression]; - } + for (const { expression } of filters) { + if (expression in parsedFilters) { + valuesForExistingFilters[expression] = parsedFilters[expression]; + } + } + for (const key of Object.keys(parsedFilters)) { + if (!knownExpressions.has(key)) { + ignored.push(key); } + } - return { + return { + valuesForExistingFilters, + queriesForExistingFilters: filtersToQuery( valuesForExistingFilters, - queriesForExistingFilters: filtersToQuery( - valuesForExistingFilters, - // Wrap keys in `toString()` to support JSON/Dynamic-type columns. - // All keys can be stringified, since filter select values are stringified as well. - { stringifyKeys: true }, - ), - }; - }, [filterQueries, filters]); + // Wrap keys in `toString()` to support JSON/Dynamic-type columns. + // All keys can be stringified, since filter select values are stringified as well. + { stringifyKeys: true }, + ), + ignoredExpressions: ignored, + }; + }, [filterQueries, filters]); return { filterValues: valuesForExistingFilters, filterQueries: queriesForExistingFilters, setFilterValue, setFilterQueries, + /** + * Expressions parsed from the URL `filters=` param that don't correspond + * to any of this dashboard's declared filters — i.e., values that would + * be silently dropped. Callers can surface a warning. + */ + ignoredFilterExpressions: ignoredExpressions, }; }; diff --git a/packages/app/tests/e2e/components/ChartEditorComponent.ts b/packages/app/tests/e2e/components/ChartEditorComponent.ts index cd9124e170..27a65dfc2f 100644 --- a/packages/app/tests/e2e/components/ChartEditorComponent.ts +++ b/packages/app/tests/e2e/components/ChartEditorComponent.ts @@ -385,6 +385,49 @@ export class ChartEditorComponent { await this.rowClickDrawer.waitFor({ state: 'hidden', timeout: 5000 }); } + /** + * Add a row of filter templates to the Row Click drawer by clicking + * "Add filter" and filling the expression and template inputs for the + * newly-added row (placed at position `index`). + */ + async addOnClickFilterTemplate( + index: number, + expression: string, + template: string, + ) { + await this.rowClickDrawer + .getByRole('button', { name: 'Add filter' }) + .click(); + await this.rowClickDrawer + .getByTestId('onclick-filter-expression-input') + .nth(index) + .fill(expression); + await this.rowClickDrawer + .getByTestId('onclick-filter-template-input') + .nth(index) + .fill(template); + } + + /** + * Read the current value of the expression input for the filter at + * position `index` within the Row Click drawer. + */ + onClickFilterExpressionInput(index: number) { + return this.rowClickDrawer + .getByTestId('onclick-filter-expression-input') + .nth(index); + } + + /** + * Read the current value of the template input for the filter at + * position `index` within the Row Click drawer. + */ + onClickFilterTemplateInput(index: number) { + return this.rowClickDrawer + .getByTestId('onclick-filter-template-input') + .nth(index); + } + get rowClickDrawer() { return this.page.getByTestId('onclick-drawer'); } diff --git a/packages/app/tests/e2e/features/dashboard-table-linking.spec.ts b/packages/app/tests/e2e/features/dashboard-table-linking.spec.ts index 75c623753a..8bee9c3d12 100644 --- a/packages/app/tests/e2e/features/dashboard-table-linking.spec.ts +++ b/packages/app/tests/e2e/features/dashboard-table-linking.spec.ts @@ -513,5 +513,232 @@ test.describe( await expect(dashboardPage.getLinkErrorNotification()).toBeHidden(); }); }); + + test('Search mode: filter templates render into the /search URL filters param', async ({ + page, + }) => { + const ts = Date.now(); + const logSources = await getSources(page, 'log'); + const logsSource = logSources.find( + (s: { name: string }) => s.name === DEFAULT_LOGS_SOURCE_NAME, + ); + expect(logsSource).toBeDefined(); + const logsSourceId: string = logsSource.id; + + await test.step('Configure Search-mode row click with a filter template', async () => { + await addTableTile(`E2E Search Filter ${ts}`); + await dashboardPage.chartEditor.openRowClickDrawer(); + await dashboardPage.chartEditor.setRowClickMode('Search'); + await dashboardPage.chartEditor.fillRowClickTemplate( + DEFAULT_LOGS_SOURCE_NAME, + ); + await dashboardPage.chartEditor.addOnClickFilterTemplate( + 0, + 'ServiceName', + '{{ServiceName}}', + ); + await dashboardPage.chartEditor.applyRowClickDrawer(); + await dashboardPage.saveTile(); + }); + + await test.step('Set dashboard time range to Last 6 hours', async () => { + await dashboardPage.timePicker.selectRelativeTime('Last 6 hours'); + }); + + await dashboardPage.waitForTableTileRows(0); + // ServiceName is the second column in the rendered table (count is col 0). + const serviceName = await dashboardPage.getFirstTableRowValue(0, 1); + expect(serviceName.length).toBeGreaterThan(0); + + await test.step('Click first table row', async () => { + await dashboardPage.clickFirstTableRow(0); + }); + + await test.step('Verify /search URL has the rendered filter template in filters param', async () => { + await expect(page).toHaveURL(/\/search\?/, { timeout: 10000 }); + const url = new URL(page.url()); + expect(url.searchParams.get('source')).toBe(logsSourceId); + const filtersRaw = url.searchParams.get('filters'); + expect(filtersRaw).not.toBeNull(); + const filters = JSON.parse(filtersRaw!); + expect(filters).toEqual([ + { + type: 'sql', + condition: `ServiceName IN ('${serviceName}')`, + }, + ]); + }); + + await test.step('Verify search page reflects the selected source', async () => { + await expect(searchPage.currentSource).toHaveValue( + DEFAULT_LOGS_SOURCE_NAME, + { timeout: 10000 }, + ); + }); + }); + + test('Dashboard mode: filter templates render into the destination dashboard URL filters param', async ({ + page, + }) => { + const ts = Date.now(); + const targetDashboardName = `E2E Filter Target ${ts}`; + let targetDashboardId = ''; + + await test.step('Create the target dashboard with a declared filter matching ServiceName', async () => { + // beforeEach already created a dashboard; repurpose it as the target. + await dashboardPage.editDashboardName(targetDashboardName); + await dashboardPage.addTile(); + await dashboardPage.chartEditor.createBasicChart( + `Filter Target Tile ${ts}`, + ); + targetDashboardId = dashboardPage.getCurrentDashboardId(); + // Declare a dashboard filter for ServiceName so the ignored-filters + // banner does NOT appear after navigation. + await dashboardPage.openEditFiltersModal(); + await dashboardPage.addFilterToDashboard( + 'Service Filter', + DEFAULT_LOGS_SOURCE_NAME, + 'ServiceName', + ); + await dashboardPage.closeFiltersModal(); + }); + + await test.step('Create source dashboard with Dashboard-mode tile using a filter template', async () => { + await dashboardPage.goto(); + await dashboardPage.createNewDashboard(); + await addTableTile(`E2E Dashboard Filter ${ts}`); + await dashboardPage.chartEditor.openRowClickDrawer(); + await dashboardPage.chartEditor.setRowClickMode('Dashboard'); + // Selecting the target dashboard auto-populates a filter row per + // declared DashboardFilter (expression filled, template empty). + // Just fill the template of the auto-populated ServiceName row. + await dashboardPage.chartEditor.selectRowClickTarget( + targetDashboardName, + ); + await expect( + dashboardPage.chartEditor.onClickFilterExpressionInput(0), + ).toHaveValue('ServiceName'); + await dashboardPage.chartEditor + .onClickFilterTemplateInput(0) + .fill('{{ServiceName}}'); + await dashboardPage.chartEditor.applyRowClickDrawer(); + await dashboardPage.saveTile(); + }); + + await test.step('Set dashboard time range to Last 6 hours', async () => { + await dashboardPage.timePicker.selectRelativeTime('Last 6 hours'); + }); + + await dashboardPage.waitForTableTileRows(0); + const serviceName = await dashboardPage.getFirstTableRowValue(0, 1); + expect(serviceName.length).toBeGreaterThan(0); + + await test.step('Click first table row', async () => { + await dashboardPage.clickFirstTableRow(0); + }); + + await test.step('Verify navigated to target dashboard with rendered filter in URL', async () => { + await expect(page).toHaveURL( + new RegExp(`/dashboards/${targetDashboardId}`), + { timeout: 10000 }, + ); + const url = new URL(page.url()); + expect(url.pathname).toBe(`/dashboards/${targetDashboardId}`); + const filtersRaw = url.searchParams.get('filters'); + expect(filtersRaw).not.toBeNull(); + const filters = JSON.parse(filtersRaw!); + expect(filters).toEqual([ + { + type: 'sql', + condition: `ServiceName IN ('${serviceName}')`, + }, + ]); + }); + + await test.step("Verify target dashboard's heading is visible", async () => { + await expect( + dashboardPage.getDashboardHeading(targetDashboardName), + ).toBeVisible({ timeout: 10000 }); + }); + + await test.step('Verify no Link error notification appeared', async () => { + await expect(dashboardPage.getLinkErrorNotification()).toBeHidden(); + }); + + await test.step('Verify ignored-filters banner is hidden (filter was declared on target)', async () => { + await expect(dashboardPage.ignoredUrlFiltersBanner).toBeHidden(); + }); + }); + + test('Dashboard mode: ignored-filter warning banner is dismissable', async ({ + page, + }) => { + const ts = Date.now(); + const targetDashboardName = `E2E Orphan Filter Target ${ts}`; + let targetDashboardId = ''; + + await test.step('Create the target dashboard with NO declared filters', async () => { + // beforeEach already created a dashboard; repurpose it as the target. + await dashboardPage.editDashboardName(targetDashboardName); + await dashboardPage.addTile(); + await dashboardPage.chartEditor.createBasicChart( + `Orphan Filter Tile ${ts}`, + ); + targetDashboardId = dashboardPage.getCurrentDashboardId(); + }); + + await test.step('Create source dashboard with Dashboard-mode tile using an undeclared filter expression', async () => { + await dashboardPage.goto(); + await dashboardPage.createNewDashboard(); + await addTableTile(`E2E Orphan Filter Source ${ts}`); + await dashboardPage.chartEditor.openRowClickDrawer(); + await dashboardPage.chartEditor.setRowClickMode('Dashboard'); + await dashboardPage.chartEditor.selectRowClickTarget( + targetDashboardName, + ); + // OrphanExpression is not declared as a filter on the target dashboard, + // so the warning banner should appear after navigation. + await dashboardPage.chartEditor.addOnClickFilterTemplate( + 0, + 'OrphanExpression', + '{{ServiceName}}', + ); + await dashboardPage.chartEditor.applyRowClickDrawer(); + await dashboardPage.saveTile(); + }); + + await test.step('Set dashboard time range to Last 6 hours', async () => { + await dashboardPage.timePicker.selectRelativeTime('Last 6 hours'); + }); + + await dashboardPage.waitForTableTileRows(0); + + await test.step('Click first table row', async () => { + await dashboardPage.clickFirstTableRow(0); + }); + + await test.step('Verify navigated to target dashboard with filters param in URL', async () => { + await expect(page).toHaveURL( + new RegExp(`/dashboards/${targetDashboardId}`), + { timeout: 10000 }, + ); + const url = new URL(page.url()); + expect(url.searchParams.get('filters')).not.toBeNull(); + }); + + await test.step('Verify ignored-filters banner is visible and mentions the orphan expression', async () => { + await expect(dashboardPage.ignoredUrlFiltersBanner).toBeVisible({ + timeout: 10000, + }); + await expect(dashboardPage.ignoredUrlFiltersBanner).toContainText( + 'OrphanExpression', + ); + }); + + await test.step('Dismiss the banner and verify it disappears', async () => { + await dashboardPage.dismissIgnoredUrlFiltersBanner(); + await expect(dashboardPage.ignoredUrlFiltersBanner).toBeHidden(); + }); + }); }, ); diff --git a/packages/app/tests/e2e/page-objects/DashboardPage.ts b/packages/app/tests/e2e/page-objects/DashboardPage.ts index 5de4dd591f..8f8e6a45b0 100644 --- a/packages/app/tests/e2e/page-objects/DashboardPage.ts +++ b/packages/app/tests/e2e/page-objects/DashboardPage.ts @@ -664,6 +664,25 @@ export class DashboardPage { .filter({ hasText: 'Link error' }); } + /** + * Banner shown on the dashboard page when the URL's `filters=` param + * references expressions that don't correspond to any declared dashboard + * filter. Users can dismiss it with the close button. + */ + get ignoredUrlFiltersBanner() { + return this.page.getByTestId('ignored-url-filters-banner'); + } + + /** + * Dismiss the ignored-URL-filters banner by clicking its close button. + * Mantine's Alert renders the close button with `aria-label="Dismiss"`. + */ + async dismissIgnoredUrlFiltersBanner() { + await this.ignoredUrlFiltersBanner + .getByRole('button', { name: 'Dismiss' }) + .click(); + } + // Getters for assertions get createButton() { diff --git a/packages/common-utils/src/core/__tests__/linkUrlBuilder.test.ts b/packages/common-utils/src/core/__tests__/linkUrlBuilder.test.ts index fa265011fb..ce9c61a526 100644 --- a/packages/common-utils/src/core/__tests__/linkUrlBuilder.test.ts +++ b/packages/common-utils/src/core/__tests__/linkUrlBuilder.test.ts @@ -190,6 +190,179 @@ describe('renderOnClickSearch', () => { "Multiple sources named 'Duplicated' — source names must be unique to use them in a link", ); }); + + it('omits the filters param when no filter templates are provided', () => { + const onClick: OnClickSearch = { + type: 'search', + target: { mode: 'id', id: 'src_1' }, + whereLanguage: 'sql', + filters: [], + }; + const result = renderOnClickSearch({ + onClick, + row: {}, + sourceIds, + sourceIdsByName, + dateRange, + }); + expect(result.ok).toBe(true); + if (result.ok) { + const params = new URLSearchParams(result.url.split('?')[1]); + expect(params.has('filters')).toBe(false); + } + }); + + it('renders a single filter template as an IN clause', () => { + const onClick: OnClickSearch = { + type: 'search', + target: { mode: 'id', id: 'src_1' }, + whereLanguage: 'sql', + filters: [ + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: '{{ServiceName}}', + }, + ], + }; + const result = renderOnClickSearch({ + onClick, + row: { ServiceName: 'MyService' }, + sourceIds, + sourceIdsByName, + dateRange, + }); + expect(result.ok).toBe(true); + if (result.ok) { + const params = new URLSearchParams(result.url.split('?')[1]); + expect(JSON.parse(params.get('filters') ?? '')).toEqual([ + { type: 'sql', condition: "ServiceName IN ('MyService')" }, + ]); + } + }); + + it('merges filter templates sharing an expression into one IN clause', () => { + const onClick: OnClickSearch = { + type: 'search', + target: { mode: 'id', id: 'src_1' }, + whereLanguage: 'sql', + filters: [ + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: '{{Service1}}', + }, + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: '{{Service2}}', + }, + ], + }; + const result = renderOnClickSearch({ + onClick, + row: { Service1: 'A', Service2: 'B' }, + sourceIds, + sourceIdsByName, + dateRange, + }); + expect(result.ok).toBe(true); + if (result.ok) { + const params = new URLSearchParams(result.url.split('?')[1]); + expect(JSON.parse(params.get('filters') ?? '')).toEqual([ + { type: 'sql', condition: "ServiceName IN ('A', 'B')" }, + ]); + } + }); + + it('emits separate filters for distinct expressions', () => { + const onClick: OnClickSearch = { + type: 'search', + target: { mode: 'id', id: 'src_1' }, + whereLanguage: 'sql', + filters: [ + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: '{{Service}}', + }, + { + kind: 'expressionTemplate', + expression: 'SeverityText', + template: '{{Severity}}', + }, + ], + }; + const result = renderOnClickSearch({ + onClick, + row: { Service: 'MyService', Severity: 'error' }, + sourceIds, + sourceIdsByName, + dateRange, + }); + expect(result.ok).toBe(true); + if (result.ok) { + const params = new URLSearchParams(result.url.split('?')[1]); + expect(JSON.parse(params.get('filters') ?? '')).toEqual([ + { type: 'sql', condition: "ServiceName IN ('MyService')" }, + { type: 'sql', condition: "SeverityText IN ('error')" }, + ]); + } + }); + + it('escapes single quotes in rendered filter values', () => { + const onClick: OnClickSearch = { + type: 'search', + target: { mode: 'id', id: 'src_1' }, + whereLanguage: 'sql', + filters: [ + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: '{{ServiceName}}', + }, + ], + }; + const result = renderOnClickSearch({ + onClick, + row: { ServiceName: "O'Malley" }, + sourceIds, + sourceIdsByName, + dateRange, + }); + expect(result.ok).toBe(true); + if (result.ok) { + const params = new URLSearchParams(result.url.split('?')[1]); + expect(JSON.parse(params.get('filters') ?? '')).toEqual([ + { type: 'sql', condition: "ServiceName IN ('O''Malley')" }, + ]); + } + }); + + it('errors when a filter template references a missing column', () => { + const onClick: OnClickSearch = { + type: 'search', + target: { mode: 'id', id: 'src_1' }, + whereLanguage: 'sql', + filters: [ + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: '{{MissingColumn}}', + }, + ], + }; + const result = renderOnClickSearch({ + onClick, + row: {}, + sourceIds, + sourceIdsByName, + dateRange, + }); + expect(result.ok).toBe(false); + if (!result.ok) + expect(result.error).toBe("Row has no column 'MissingColumn'"); + }); }); describe('renderOnClickDashboard', () => { @@ -376,6 +549,179 @@ describe('renderOnClickDashboard', () => { "Multiple dashboards named 'Duplicated' — dashboard names must be unique to use them in a link", ); }); + + it('omits the filters param when no filter templates are provided', () => { + const onClick: OnClickDashboard = { + type: 'dashboard', + target: { mode: 'id', id: 'dash_1' }, + whereLanguage: 'sql', + filters: [], + }; + const result = renderOnClickDashboard({ + onClick, + row: {}, + dashboardIds, + dashboardIdsByName, + dateRange, + }); + expect(result.ok).toBe(true); + if (result.ok) { + const params = new URLSearchParams(result.url.split('?')[1]); + expect(params.has('filters')).toBe(false); + } + }); + + it('renders a single filter template as an IN clause', () => { + const onClick: OnClickDashboard = { + type: 'dashboard', + target: { mode: 'id', id: 'dash_1' }, + whereLanguage: 'sql', + filters: [ + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: '{{ServiceName}}', + }, + ], + }; + const result = renderOnClickDashboard({ + onClick, + row: { ServiceName: 'MyService' }, + dashboardIds, + dashboardIdsByName, + dateRange, + }); + expect(result.ok).toBe(true); + if (result.ok) { + const params = new URLSearchParams(result.url.split('?')[1]); + expect(JSON.parse(params.get('filters') ?? '')).toEqual([ + { type: 'sql', condition: "ServiceName IN ('MyService')" }, + ]); + } + }); + + it('merges filter templates sharing an expression into one IN clause', () => { + const onClick: OnClickDashboard = { + type: 'dashboard', + target: { mode: 'id', id: 'dash_1' }, + whereLanguage: 'sql', + filters: [ + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: '{{Service1}}', + }, + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: '{{Service2}}', + }, + ], + }; + const result = renderOnClickDashboard({ + onClick, + row: { Service1: 'A', Service2: 'B' }, + dashboardIds, + dashboardIdsByName, + dateRange, + }); + expect(result.ok).toBe(true); + if (result.ok) { + const params = new URLSearchParams(result.url.split('?')[1]); + expect(JSON.parse(params.get('filters') ?? '')).toEqual([ + { type: 'sql', condition: "ServiceName IN ('A', 'B')" }, + ]); + } + }); + + it('emits separate filters for distinct expressions', () => { + const onClick: OnClickDashboard = { + type: 'dashboard', + target: { mode: 'id', id: 'dash_1' }, + whereLanguage: 'sql', + filters: [ + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: '{{Service}}', + }, + { + kind: 'expressionTemplate', + expression: 'SeverityText', + template: '{{Severity}}', + }, + ], + }; + const result = renderOnClickDashboard({ + onClick, + row: { Service: 'MyService', Severity: 'error' }, + dashboardIds, + dashboardIdsByName, + dateRange, + }); + expect(result.ok).toBe(true); + if (result.ok) { + const params = new URLSearchParams(result.url.split('?')[1]); + expect(JSON.parse(params.get('filters') ?? '')).toEqual([ + { type: 'sql', condition: "ServiceName IN ('MyService')" }, + { type: 'sql', condition: "SeverityText IN ('error')" }, + ]); + } + }); + + it('escapes single quotes in rendered filter values', () => { + const onClick: OnClickDashboard = { + type: 'dashboard', + target: { mode: 'id', id: 'dash_1' }, + whereLanguage: 'sql', + filters: [ + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: '{{ServiceName}}', + }, + ], + }; + const result = renderOnClickDashboard({ + onClick, + row: { ServiceName: "O'Malley" }, + dashboardIds, + dashboardIdsByName, + dateRange, + }); + expect(result.ok).toBe(true); + if (result.ok) { + const params = new URLSearchParams(result.url.split('?')[1]); + expect(JSON.parse(params.get('filters') ?? '')).toEqual([ + { type: 'sql', condition: "ServiceName IN ('O''Malley')" }, + ]); + } + }); + + it('errors when a filter template references a missing column', () => { + const onClick: OnClickDashboard = { + type: 'dashboard', + target: { mode: 'id', id: 'dash_1' }, + whereLanguage: 'sql', + filters: [ + { + kind: 'expressionTemplate', + expression: 'ServiceName', + template: '{{MissingColumn}}', + }, + ], + }; + const result = renderOnClickDashboard({ + onClick, + row: {}, + dashboardIds, + dashboardIdsByName, + dateRange, + }); + expect(result.ok).toBe(false); + if (!result.ok) + expect(result.error).toBe("Row has no column 'MissingColumn'"); + }); }); describe('validateOnClickTemplate', () => { diff --git a/packages/common-utils/src/core/linkUrlBuilder.ts b/packages/common-utils/src/core/linkUrlBuilder.ts index f026d704f6..690375aa9f 100644 --- a/packages/common-utils/src/core/linkUrlBuilder.ts +++ b/packages/common-utils/src/core/linkUrlBuilder.ts @@ -1,4 +1,11 @@ -import type { OnClick, OnClickDashboard, OnClickSearch } from '../types'; +import { FilterState, filtersToQuery } from '../filters'; +import type { + Filter, + OnClick, + OnClickDashboard, + OnClickFilterTemplate, + OnClickSearch, +} from '../types'; import { LinkTemplateError, MissingTemplateVariableError, @@ -30,6 +37,37 @@ function renderOrError( } } +/** + * Render the filter entries into `{expression} IN (v1, v2, ...)` SQL + * conditions. Entries that share the same `filter` expression are merged + * into a single IN clause so the destination sees all requested values. + * Expressions appear in the URL in order of first occurrence; values within + * a group retain their input order. + */ +function renderFilterTemplates( + templates: OnClickFilterTemplate[] | undefined, + row: Record, +): { ok: true; filters: Filter[] } | { ok: false; error: string } { + if (!templates || templates.length === 0) return { ok: true, filters: [] }; + + const state: FilterState = {}; + for (const template of templates) { + const rendered = renderOrError(template.template, row); + if (!rendered.ok) return rendered; + + if (state[template.expression]) { + state[template.expression].included.add(rendered.value); + } else { + state[template.expression] = { + included: new Set([rendered.value]), + excluded: new Set(), + }; + } + } + + return { ok: true, filters: filtersToQuery(state) }; +} + /** * Render an OnClickSearch to a URL. */ @@ -100,6 +138,14 @@ export function renderOnClickSearch({ from: String(dateRange[0].getTime()), to: String(dateRange[1].getTime()), }); + + const filterRenderResult = renderFilterTemplates(onClick.filters, row); + if (!filterRenderResult.ok) return filterRenderResult; + + if (filterRenderResult.filters.length > 0) { + params.set('filters', JSON.stringify(filterRenderResult.filters)); + } + return { ok: true, url: `/search?${params.toString()}` }; } @@ -176,6 +222,13 @@ export function renderOnClickDashboard({ to: String(dateRange[1].getTime()), }); + const filterRenderResult = renderFilterTemplates(onClick.filters, row); + if (!filterRenderResult.ok) return filterRenderResult; + + if (filterRenderResult.filters.length > 0) { + params.set('filters', JSON.stringify(filterRenderResult.filters)); + } + return { ok: true, url: `/dashboards/${dashboardId}?${params.toString()}` }; } @@ -184,6 +237,15 @@ export function validateOnClickTemplate(onClick: OnClick) { if (onClick.target.mode === 'template') { validateTemplate(onClick.target.template); } + + if (onClick.filters) { + for (const filter of onClick.filters) { + if (filter.kind === 'expressionTemplate') { + validateTemplate(filter.template); + } + } + } + if (onClick.whereTemplate) { validateTemplate(onClick.whereTemplate); } diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index acab616ff9..b2646ac76d 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -667,6 +667,13 @@ export const NumberFormatSchema = z.object({ export type NumberFormat = z.infer; +export const OnClickFilterTemplateSchema = z.object({ + kind: z.literal('expressionTemplate'), + expression: z.string().min(1, 'Expression is required'), + template: z.string().min(1, 'Template is required'), +}); +export type OnClickFilterTemplate = z.infer; + const OnClickTargetSchema = z.discriminatedUnion('mode', [ z.object({ mode: z.literal('id'), id: z.string().min(1) }), z.object({ mode: z.literal('template'), template: z.string().min(1) }), @@ -678,6 +685,7 @@ const OnClickSearchSchema = z.object({ target: OnClickTargetSchema, whereTemplate: z.string().optional(), whereLanguage: SearchConditionLanguageSchema, + filters: z.array(OnClickFilterTemplateSchema).optional(), }); export type OnClickSearch = z.infer; @@ -686,6 +694,7 @@ export const OnClickDashboardSchema = z.object({ target: OnClickTargetSchema, whereTemplate: z.string().optional(), whereLanguage: SearchConditionLanguageSchema, + filters: z.array(OnClickFilterTemplateSchema).optional(), }); export type OnClickDashboard = z.infer; From 226d4ffb85a5fbae48ab8508fba3f018c99bcfb4 Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Thu, 23 Apr 2026 16:40:12 -0400 Subject: [PATCH 3/4] fix: Prevent banner flash when navigating away from dashboard --- packages/app/src/DBDashboardPage.tsx | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/app/src/DBDashboardPage.tsx b/packages/app/src/DBDashboardPage.tsx index 272c7b3aa7..5f9aa65fdc 100644 --- a/packages/app/src/DBDashboardPage.tsx +++ b/packages/app/src/DBDashboardPage.tsx @@ -1107,12 +1107,18 @@ function DBDashboardPage({ presetConfig }: { presetConfig?: Dashboard }) { !!dashboard?.id && router.isReady && (isLocalDashboard || !isFetchingDashboard); - const [dismissedIgnoredFilters, setDismissedIgnoredFilters] = + + // Latched on dashboard load only — not on every URL change — so the banner + // doesn't flash while navigating between dashboards. Known bug - when navigating + // to the current dashboard with new and invalid filters in the URL, the banner + // will not show up. + const [shouldShowIgnoredFiltersBanner, setShouldShowIgnoredFiltersBanner] = useState(false); - const shouldShowIgnoredFiltersBanner = - dashboardReady && - ignoredFilterExpressions.length > 0 && - !dismissedIgnoredFilters; + useEffect(() => { + if (!dashboardReady) return; + setShouldShowIgnoredFiltersBanner(ignoredFilterExpressions.length > 0); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [dashboard?.id, dashboardReady]); const handleSaveFilter = (filter: DashboardFilter) => { if (!dashboard) return; @@ -2169,7 +2175,7 @@ function DBDashboardPage({ presetConfig }: { presetConfig?: Dashboard }) { data-testid="ignored-url-filters-banner" withCloseButton closeButtonLabel="Dismiss" - onClose={() => setDismissedIgnoredFilters(true)} + onClose={() => setShouldShowIgnoredFiltersBanner(false)} > No dashboard filter(s) found for{' '} {ignoredFilterExpressions.length === 1 ? 'expression' : 'expressions'}{' '} From d806f59ee2509ca967fae1ba53fa00f64be600ca Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Mon, 27 Apr 2026 11:59:23 +0900 Subject: [PATCH 4/4] review: Address Claude feedback --- packages/app/src/DBDashboardPage.tsx | 17 +++++++++-------- .../OnClickForm/FilterTemplateList.tsx | 2 -- .../common-utils/src/__tests__/filters.test.ts | 2 +- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/app/src/DBDashboardPage.tsx b/packages/app/src/DBDashboardPage.tsx index 5f9aa65fdc..57033ff91a 100644 --- a/packages/app/src/DBDashboardPage.tsx +++ b/packages/app/src/DBDashboardPage.tsx @@ -1098,20 +1098,21 @@ function DBDashboardPage({ presetConfig }: { presetConfig?: Dashboard }) { ignoredFilterExpressions, } = useDashboardFilters(filters); - // Warn when the URL has filter values that don't correspond to any declared - // dashboard filter — they'd otherwise be silently dropped, and users who - // arrive via a shared link, bookmark, or onClick action might not notice. - // Only consider URL filters ignored once the dashboard has finished loading - // so we don't flash the banner before `dashboard.filters` is available. const dashboardReady = !!dashboard?.id && router.isReady && (isLocalDashboard || !isFetchingDashboard); + // Warn when the URL has filter values that don't correspond to any declared + // dashboard filter — they'd otherwise be silently dropped, and users who + // arrive via a shared link, bookmark, or onClick action might not notice. + // Only consider URL filters ignored once the dashboard has finished loading + // so we don't flash the banner before `dashboard.filters` is available. + // // Latched on dashboard load only — not on every URL change — so the banner - // doesn't flash while navigating between dashboards. Known bug - when navigating - // to the current dashboard with new and invalid filters in the URL, the banner - // will not show up. + // doesn't flash while navigating between dashboards due to nuqs state changing + // before the next router state. Known limitation - when navigating to the current + // dashboard with new and invalid filters in the URL, the banner will not show up. const [shouldShowIgnoredFiltersBanner, setShouldShowIgnoredFiltersBanner] = useState(false); useEffect(() => { diff --git a/packages/app/src/components/DBEditTimeChartForm/OnClickForm/FilterTemplateList.tsx b/packages/app/src/components/DBEditTimeChartForm/OnClickForm/FilterTemplateList.tsx index ab68d2553d..cb950c8f91 100644 --- a/packages/app/src/components/DBEditTimeChartForm/OnClickForm/FilterTemplateList.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/OnClickForm/FilterTemplateList.tsx @@ -37,7 +37,6 @@ export function FilterTemplateList({ control }: { control: DrawerControl }) { control={control} name={`onClick.filters.${i}.expression` as const} placeholder="Expression" - value={filter.expression} style={{ flex: 1 }} data-testid="onclick-filter-expression-input" /> @@ -45,7 +44,6 @@ export function FilterTemplateList({ control }: { control: DrawerControl }) { control={control} name={`onClick.filters.${i}.template` as const} placeholder="Template (e.g. {{ServiceName}})" - value={filter.template} style={{ flex: 1 }} data-testid="onclick-filter-template-input" /> diff --git a/packages/common-utils/src/__tests__/filters.test.ts b/packages/common-utils/src/__tests__/filters.test.ts index 743ca2fbff..679780804e 100644 --- a/packages/common-utils/src/__tests__/filters.test.ts +++ b/packages/common-utils/src/__tests__/filters.test.ts @@ -1,6 +1,6 @@ import { filtersToQuery } from '@/filters'; -describe('searchFilters', () => { +describe('filters', () => { describe('filtersToQuery', () => { it('should return empty string when no filters', () => { const filters = {};