Repository navigation
feat: Support click-to-adhoc-filter on search tiles #3380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| '@hyperdx/common-utils': patch | ||
| '@hyperdx/app': patch | ||
| --- | ||
|
|
||
| feat: Filter search tile cells into adhoc dashboard filters |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,7 +34,10 @@ import { | |
| Granularity, | ||
| isTimeSeriesDisplayType, | ||
| } from '@hyperdx/common-utils/dist/core/utils'; | ||
| import { getBlockingRequiredFilterNames } from '@hyperdx/common-utils/dist/dashboardFilterValues'; | ||
| import { | ||
| getAdhocFilterTargetsForTile, | ||
| getBlockingRequiredFilterNames, | ||
| } from '@hyperdx/common-utils/dist/dashboardFilterValues'; | ||
| import { | ||
| getPromqlFilters, | ||
| getSqlFilters, | ||
|
|
@@ -127,6 +130,7 @@ import { | |
| } from '@tabler/icons-react'; | ||
|
|
||
| import { IsolatedChartSyncProvider } from '@/chartSync'; | ||
| import { getAllowedSourceKinds } from '@/components/ChartEditor/utils'; | ||
| import { mergeAnnotations } from '@/components/charts/chartAnnotations'; | ||
| import { ContactSupportText } from '@/components/ContactSupportText'; | ||
| import { | ||
|
|
@@ -148,6 +152,7 @@ import { DashboardKioskHeader } from '@/components/DashboardKioskHeader'; | |
| import DashboardTableOfContents from '@/components/DashboardTableOfContents'; | ||
| import EditTimeChartForm from '@/components/DBEditTimeChartForm'; | ||
| import DBNumberChart from '@/components/DBNumberChart'; | ||
| import type { RowSidePanelContextProps } from '@/components/DBRowSidePanel'; | ||
| import DBTableChart from '@/components/DBTableChart'; | ||
| import { DBTimeChart } from '@/components/DBTimeChart'; | ||
| import { FavoriteButton } from '@/components/FavoriteButton'; | ||
|
|
@@ -177,6 +182,7 @@ import useDashboardContainers, { | |
| import { useDashboardKioskMode } from '@/hooks/useDashboardKioskMode'; | ||
| import { useReleaseAnnotations } from '@/hooks/useReleaseAnnotations'; | ||
| import { QueryAttributionProvider } from '@/queryAttribution'; | ||
| import type { TileFilterHandler } from '@/types'; | ||
| import { calculateNextTilePosition, makeId } from '@/utils/tilePositioning'; | ||
|
|
||
| import ChartContainer, { | ||
|
|
@@ -447,6 +453,7 @@ const Tile = ({ | |
| showReleaseAnnotations, | ||
| isLive, | ||
| readOnly, | ||
| onFilter, | ||
|
|
||
| // Properties forwarded by grid layout | ||
| className, | ||
|
|
@@ -482,6 +489,8 @@ const Tile = ({ | |
| showReleaseAnnotations?: boolean; | ||
| isLive?: boolean; | ||
| readOnly?: boolean; | ||
| /** Set when an ad hoc filter broadcasts to this tile; clicked values update it. */ | ||
| onFilter?: TileFilterHandler; | ||
|
|
||
| // Properties forwarded by grid layout | ||
| className?: string; | ||
|
|
@@ -659,6 +668,30 @@ const Tile = ({ | |
| () => (filters ? getSqlFilters(filters) : undefined), | ||
| [filters], | ||
| ); | ||
|
|
||
| // `source` keeps the handler off rows from other sources, e.g. a linked trace | ||
| const searchRowContext = useMemo<RowSidePanelContextProps | undefined>( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 major — Side panel's surrounding-context table writes unshaped cells (DateTime, Map, aliases) into ad hoc filters
Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to reproduce this but couldn't
pulpdrew marked this conversation as resolved.
pulpdrew marked this conversation as resolved.
|
||
| () => | ||
| onFilter && | ||
| source && | ||
| getAllowedSourceKinds({ | ||
| configType: 'builder', | ||
| displayType: DisplayType.Search, | ||
| }).includes(source.kind) | ||
| ? { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — Side panel "Add to Filters" removes an existing ad hoc condition while the toast says "Added"
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond |
||
| // Only the cell popover passes an action; the side panel's | ||
| // "Add to filters" passes none and shouldn't toggle a condition off. | ||
| onPropertyAddClick: (key, value, action) => | ||
|
pulpdrew marked this conversation as resolved.
pulpdrew marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — The side panel's "Add to Filters" skips the DateTime guard that table cells use Table cells go through Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond |
||
| onFilter( | ||
| [{ key, value: String(value) }], | ||
| action === 'include' || action === 'exclude' ? action : 'add', | ||
| ), | ||
| source, | ||
| isAdhocFilterTarget: true, | ||
| } | ||
| : undefined, | ||
| [onFilter, source], | ||
| ); | ||
| const promqlFilters = useMemo( | ||
| () => (filters ? getPromqlFilters(filters) : undefined), | ||
| [filters], | ||
|
|
@@ -1379,6 +1412,7 @@ const Tile = ({ | |
| queryKeyPrefix={'search'} | ||
| variant="default" | ||
| errorVariant="collapsible" | ||
| context={searchRowContext} | ||
| /> | ||
| </ChartContainer> | ||
| )} | ||
|
|
@@ -1460,6 +1494,7 @@ const Tile = ({ | |
| readOnly, | ||
| tileVariables, | ||
| variableNames, | ||
| searchRowContext, | ||
| ], | ||
| ); | ||
|
|
||
|
|
@@ -1910,6 +1945,7 @@ function DBDashboardPage({ | |
| setFilterValue, | ||
| adhocConditionsByFilterId, | ||
| setAdhocConditions, | ||
| applyAdhocAction, | ||
| setFilterValueEntries, | ||
| filterValueEntries, | ||
| ignoredFilterExpressions, | ||
|
|
@@ -2348,6 +2384,30 @@ function DBDashboardPage({ | |
| [where, whereLanguage, getFilterQueriesForSource], | ||
| ); | ||
|
|
||
| // Built once per tile list so each tile's `onFilter` keeps its identity | ||
| // across renders; a fresh closure would rebuild the tile's row-panel context | ||
| // and re-render every cell in its search table. | ||
| const tileFilterHandlers = useMemo(() => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — PromQL, raw SQL and multi-entry targeting are built but nothing in this PR can use them Only the Search tile passes Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond |
||
| const handlers = new Map<string, TileFilterHandler>(); | ||
| // The filter bar is hidden in kiosk mode, so a click would add conditions | ||
| // the viewer can't see or remove. | ||
| if (isKioskMode) return handlers; | ||
| for (const tile of dashboard?.tiles ?? []) { | ||
| const sourceId = 'source' in tile.config ? tile.config.source : undefined; | ||
| const targetIds = getAdhocFilterTargetsForTile( | ||
|
pulpdrew marked this conversation as resolved.
|
||
| filters, | ||
| tile.config, | ||
| sourceId, | ||
| ).map(filter => filter.id); | ||
| if (targetIds.length > 0) { | ||
| handlers.set(tile.id, (entries, action) => | ||
| applyAdhocAction(targetIds, entries, action), | ||
| ); | ||
| } | ||
| } | ||
| return handlers; | ||
| }, [isKioskMode, dashboard?.tiles, filters, applyAdhocAction]); | ||
|
|
||
| const renderTileComponent = useCallback( | ||
| (chart: Tile) => { | ||
| // Resolve the tile's source ID so per-source-scoped filters can be | ||
|
|
@@ -2367,6 +2427,7 @@ function DBDashboardPage({ | |
| isRefreshEnabled ? granularityOverride : (granularity ?? undefined) | ||
| } | ||
| filters={getTileFilters(tileSourceId)} | ||
| onFilter={tileFilterHandlers.get(chart.id)} | ||
| variables={variables} | ||
| unsatisfiedRequiredFilters={unsatisfiedRequiredFilters} | ||
| onTimeRangeSelect={onTimeRangeSelect} | ||
|
|
@@ -2469,6 +2530,7 @@ function DBDashboardPage({ | |
| showAlertAnnotations, | ||
| showReleaseAnnotations, | ||
| getTileFilters, | ||
| tileFilterHandlers, | ||
| variables, | ||
| unsatisfiedRequiredFilters, | ||
| moveTargetContainers, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import { | ||
| ADHOC_FILTER_MAX_CONDITIONS, | ||
| AdhocDashboardFilter, | ||
| AdhocFilterConditionSchema, | ||
| } from '@hyperdx/common-utils/dist/types'; | ||
| import { notifications } from '@mantine/notifications'; | ||
|
|
||
| export function notifyAdhocFilterFull( | ||
| filter: Pick<AdhocDashboardFilter, 'id' | 'name'>, | ||
| ) { | ||
| notifications.show({ | ||
| id: `adhoc-filter-full-${filter.id}`, | ||
| color: 'yellow', | ||
| title: `${filter.name} is full`, | ||
| message: `A filter holds up to ${ADHOC_FILTER_MAX_CONDITIONS} conditions. Remove one to add another.`, | ||
| }); | ||
| } | ||
|
|
||
| export function notifyAdhocConditionTooLong( | ||
| filter: Pick<AdhocDashboardFilter, 'id'>, | ||
| ) { | ||
| const maxLength = AdhocFilterConditionSchema.shape.value.maxLength; | ||
| notifications.show({ | ||
| id: `adhoc-filter-too-long-${filter.id}`, | ||
| color: 'yellow', | ||
| title: 'Condition is too long', | ||
| message: `Keys and values can be up to ${maxLength?.toLocaleString()} characters.`, | ||
| }); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,10 @@ import { | |
| QueryAttribution, | ||
| } from '@hyperdx/common-utils/dist/clickhouse'; | ||
| import { Field, tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; | ||
| import { getAdhocFilterSourceKinds } from '@hyperdx/common-utils/dist/filters'; | ||
| import { | ||
| getAdhocFilterSourceKinds, | ||
| isAdhocComparableDataType, | ||
| } from '@hyperdx/common-utils/dist/filters'; | ||
| import { | ||
| AdhocDashboardFilter, | ||
| TSource, | ||
|
|
@@ -63,16 +66,22 @@ export function getSqlSourceKeys(fields: Field[], columns: ColumnMeta[]) { | |
| const jsonColumns = columnNames([JSDataType.JSON]); | ||
| const mapColumns = columnNames([JSDataType.Map]); | ||
| const keys = fields | ||
| .filter(field => field.path.length > 1 || isJSDataTypeScalar(field.jsType)) | ||
| .filter( | ||
| field => | ||
| (field.path.length > 1 || isJSDataTypeScalar(field.jsType)) && | ||
| isAdhocComparableDataType(field.type), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — DateTime keys are now missing from the condition editor's key suggestions, though conditions on them still work The reason given (query output is ISO UTC, so it never equals Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond |
||
| ) | ||
| .map(field => mergePath(field.path, jsonColumns, mapColumns)); | ||
| return toQuotedKeys(keys, columns); | ||
| } | ||
|
|
||
| /** A SQL source's scalar columns, the only keys known without field metadata. */ | ||
| export function getSqlSourceColumnKeys(columns: ColumnMeta[]) { | ||
| const keys = columns | ||
| .filter(column => | ||
| isJSDataTypeScalar(convertCHDataTypeToJSType(column.type)), | ||
| .filter( | ||
| column => | ||
| isJSDataTypeScalar(convertCHDataTypeToJSType(column.type)) && | ||
| isAdhocComparableDataType(column.type), | ||
| ) | ||
| .map(column => column.name); | ||
| return toQuotedKeys(keys, columns); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.