Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changeset/adhoc-filter-finishing-touches.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
'@hyperdx/common-utils': patch
'@hyperdx/app': patch
'@hyperdx/api': patch
---

feat: Ad hoc dashboard filter finishing touches

- Add an `IN` operator for SQL and a `=|` operator for PromQL. Conditions on the
same key are OR'd into one `IN (...)` or `=~"a|b"` condition.
- Ad hoc filters always broadcast (`isBroadcastEnabled: true`), and their
variable name is only a unique identifier. It is no longer declared as a
`$variable`.
- Warn when another ad hoc filter already applies to the same sources.
- Collapse the filter input to three conditions behind a "+N more" toggle.
Original file line number Diff line number Diff line change
Expand Up @@ -1875,6 +1875,7 @@ describe('dashboard router', () => {
const makeAdhocFilter = (overrides = {}) => ({
id: new Types.ObjectId().toString(),
type: 'ADHOC' as const,
isBroadcastEnabled: true as const,
name: 'Ad hoc',
sourceType: 'sql' as const,
sources: [logSourceId],
Expand All @@ -1900,7 +1901,6 @@ describe('dashboard router', () => {
const filter = makeAdhocFilter({
sources: [logSourceId, traceSourceId],
appliesToSourceIds: [traceSourceId],
isBroadcastEnabled: false,
});

const created = await agent
Expand Down Expand Up @@ -1972,6 +1972,8 @@ describe('dashboard router', () => {
['an invalid variable name', { variableName: '1adhoc' }],
['not variable-enabled', { isVariableEnabled: false }],
['variables unset', { isVariableEnabled: undefined }],
['broadcast disabled', { isBroadcastEnabled: false }],
['broadcast unset', { isBroadcastEnabled: undefined }],
['a minimum selection', { minSelections: 1 }],
['a maximum selection', { maxSelections: 1 }],
['a global requirement', { isGlobalRequirement: true }],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8443,6 +8443,7 @@ describe('External API v2 Dashboards - new format', () => {
const adhocFilter = () => ({
id: new ObjectId().toString(),
type: 'ADHOC' as const,
isBroadcastEnabled: true as const,
name: 'Ad hoc',
sourceType: 'sql' as const,
sources: [traceSource._id.toString()],
Expand Down
4 changes: 2 additions & 2 deletions packages/app/src/DashboardFilters.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import {
getPendingFilterValuesVariables,
isAdhocFilter,
isFilterGlobalRequirement,
isFilterReferenceableVariable,
isFilterRequired,
isFilterVariableEnabled,
isQueryExpressionFilter,
isStaticListFilter,
} from '@hyperdx/common-utils/dist/filters';
Expand Down Expand Up @@ -73,7 +73,7 @@ export const getFilterEffect = (filter: DashboardFilter): FilterEffect => {
);
}

const variableName = isFilterVariableEnabled(filter)
const variableName = isFilterReferenceableVariable(filter)
? getFilterVariableName(filter)
: undefined;
if (variableName) {
Expand Down
2 changes: 2 additions & 0 deletions packages/app/src/__tests__/DBDashboardImportPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,7 @@ const adhocFilterTemplate = DashboardTemplateSchema.parse({
{
id: 'filter-adhoc',
type: 'ADHOC',
isBroadcastEnabled: true,
name: 'Conditions',
sourceType: 'sql',
sources: ['Logs', 'Traces', MISSING],
Expand Down Expand Up @@ -593,6 +594,7 @@ describe('Dashboard import - ad hoc filter sources', () => {
const payload = mockMutateAsync.mock.calls[0][0];
expect(payload.filters[0]).toMatchObject({
type: 'ADHOC',
isBroadcastEnabled: true,
sources: ['src-logs', 'src-traces'],
appliesToSourceIds: ['src-traces'],
});
Expand Down
5 changes: 3 additions & 2 deletions packages/app/src/__tests__/DashboardFilters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,12 @@ describe('getFilterEffect', () => {
).toEqual('Available as variable ($svc)');
});

it('counts the sources an ad hoc filter broadcasts to', () => {
it('counts the sources an ad hoc filter broadcasts to, without a variable', () => {
expect(
getFilterEffect({
id: 'adhoc1',
type: 'ADHOC',
isBroadcastEnabled: true,
name: 'Conditions',
sourceType: 'sql',
sources: ['logs', 'traces'],
Expand All @@ -128,7 +129,7 @@ describe('getFilterEffect', () => {
}),
).toEqual({
hasEffect: true,
tooltip: 'Filters 2 sources, available as variable ($conds)',
tooltip: 'Filters 2 sources',
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
import { getVariableReferences } from '@hyperdx/common-utils/dist/variables';
import {
ActionIcon,
Button,
CloseButton,
Group,
Popover,
Expand All @@ -39,6 +40,9 @@ import {
notifyAdhocFilterFull,
} from './notifications';

// Series with many labels (common in PromQL) would otherwise grow the input tall.
const MAX_COLLAPSED_CONDITIONS = 3;

type AdhocDashboardFilterProps = {
filter: AdhocDashboardFilterType;
conditions: AdhocFilterCondition[];
Expand All @@ -62,7 +66,12 @@ export function AdhocDashboardFilter({
unsatisfiedRequiredFilters,
}: AdhocDashboardFilterProps) {
const [isAdding, setIsAdding] = useState(false);
const [isExpanded, setIsExpanded] = useState(false);
const isAtLimit = conditions.length >= ADHOC_FILTER_MAX_CONDITIONS;
const hiddenCount = isExpanded
? 0
: Math.max(0, conditions.length - MAX_COLLAPSED_CONDITIONS);
const languageName = filter.sourceType === 'promql' ? 'PromQL' : 'SQL';

// The `where` narrowing the suggestions, and what it is still waiting on.
// Blocked only by the required filters it references: a global requirement
Expand Down Expand Up @@ -153,45 +162,57 @@ export function AdhocDashboardFilter({
borderRadius: 'var(--mantine-radius-sm)',
}}
>
{conditions.map((condition, index) => {
const operator = getFilterOperatorDisplay(
filter.sourceType,
condition.operator,
);
return (
<FilterPill
key={JSON.stringify(condition)}
field={condition.key}
operator={operator.label}
value={condition.value}
isExcluded={operator.isExcluded}
isInvalid={
!isAdhocConditionSupported(filter.sourceType, condition)
}
invalidReason={`Filter not applied: PromQL has no ${condition.operator} operator. Remove this condition and add it again with a supported operator.`}
onRemove={() => remove(index)}
// Editing would look up suggestions; removing still works.
renderPopover={
isDisabled
? undefined
: close => (
<AdhocConditionEditor
filter={filter}
dateRange={dateRange}
suggestionsWhere={suggestionsWhere}
initial={condition}
onSubmit={next => {
if (!isStorable(next)) return;
replace(index, next);
close();
}}
/>
)
}
data-testid="adhoc-condition-pill"
/>
);
})}
{conditions
.slice(0, conditions.length - hiddenCount)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 minor — New and invalid conditions are hidden while the filter is collapsed

New conditions are appended to the end of the list, and the list is cut at the first three. Once a filter holds three or more conditions, a condition the user adds with "+" or by clicking a tile value is hidden right away; the only sign of it is the "+N more" count going up. An invalid (data-invalid) chip past the third position is hidden the same way, so its "Filter not applied" warning never shows. Fix: expand the list when a condition is added (set isExpanded in the add and onChange paths), or always show invalid chips. At minimum, mark the "+N more" button when a hidden chip is invalid.

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

Comment thread
knudtty marked this conversation as resolved.
Resolved

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 minor — New conditions and invalid conditions get hidden behind the "+N more" toggle

When the filter is collapsed and already has 3 or more conditions, a new condition from the add popover or from click-to-filter is appended at the end. The slice hides it at once, so the user only sees the counter go up and never sees the chip they just added. Conditions past the first three that are marked invalid (=| on SQL, IN/LIKE on PromQL) are hidden in the same way, along with their 'Filter not applied' marker. Fix: expand when conditions.length grows, or always render the newest and invalid chips. As a fallback, show the invalid count on the toggle.

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((condition, index) => {
const operator = getFilterOperatorDisplay(
filter.sourceType,
condition.operator,
);
return (
<FilterPill
key={JSON.stringify(condition)}
field={condition.key}
operator={operator.label}
value={condition.value}
isExcluded={operator.isExcluded}
isInvalid={
!isAdhocConditionSupported(filter.sourceType, condition)
}
invalidReason={`Filter not applied: ${languageName} has no ${condition.operator} operator. Remove this condition and add it again with a supported operator.`}
onRemove={() => remove(index)}
// Editing would look up suggestions; removing still works.
renderPopover={
isDisabled
? undefined
: close => (
<AdhocConditionEditor
filter={filter}
dateRange={dateRange}
suggestionsWhere={suggestionsWhere}
initial={condition}
onSubmit={next => {
if (!isStorable(next)) return;
replace(index, next);
close();
}}
/>
)
}
data-testid="adhoc-condition-pill"
/>
);
})}
{conditions.length > MAX_COLLAPSED_CONDITIONS && (
<Button
variant="subtle"
size="compact-xs"
onClick={() => setIsExpanded(!isExpanded)}
data-testid={`adhoc-filter-toggle-more-${filter.name}`}
>
{isExpanded ? 'Show less' : `+${hiddenCount} more`}
</Button>
)}
{conditions.length === 0 && (
<Text
size="xs"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ jest.mock('@/components/AdhocDashboardFilter/useAdhocFilterOptions', () => ({
const filter: AdhocDashboardFilterType = {
id: 'adhoc1',
type: 'ADHOC',
isBroadcastEnabled: true,
name: 'Conditions',
sourceType: 'sql',
sources: ['logs'],
Expand Down Expand Up @@ -81,6 +82,39 @@ describe('AdhocDashboardFilter', () => {
expect(pills[1]).toHaveTextContent('Level regex ^err');
});

it('collapses conditions past the first three until expanded', async () => {
renderFilter(
Array.from({ length: 5 }, (_, i) => ({ ...existing, value: `${i}` })),
);
const toggle = screen.getByTestId('adhoc-filter-toggle-more-Conditions');

expect(screen.getAllByTestId('adhoc-condition-pill')).toHaveLength(3);
expect(toggle).toHaveTextContent('+2 more');

await userEvent.click(toggle);
expect(screen.getAllByTestId('adhoc-condition-pill')).toHaveLength(5);
expect(toggle).toHaveTextContent('Show less');

await userEvent.click(toggle);
expect(screen.getAllByTestId('adhoc-condition-pill')).toHaveLength(3);
});

it('shows no toggle for three conditions or fewer', () => {
renderFilter([existing, existing, existing]);
expect(
screen.queryByTestId('adhoc-filter-toggle-more-Conditions'),
).not.toBeInTheDocument();
});

it('marks conditions a SQL filter cannot apply as invalid', () => {
renderFilter([{ key: 'job', operator: '=|', value: 'api' }]);

expect(screen.getByTestId('adhoc-condition-pill')).toHaveAttribute(
'data-invalid',
'true',
);
});

it('marks conditions a PromQL filter cannot apply as invalid', () => {
renderFilter(
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ describe('useAdhocFilterOptions', () => {
): AdhocDashboardFilter => ({
id: 'f',
type: 'ADHOC',
isBroadcastEnabled: true,
name: 'Conditions',
sourceType,
sources,
Expand Down
Loading
Loading