Skip to content

Commit 8fc8fc4

Browse files
authored
fix(Filters): ensure "is empty" filter is synced with URL (#205)
* fix: ensure "is empty" filter has value set This way it won't be filtered out during URL synchronization * Fix tests * Ensure "is-empty" is removed from the value when user switches away from "is empty" filter
1 parent d19cc97 commit 8fc8fc4

File tree

6 files changed

+30
-17
lines changed

6 files changed

+30
-17
lines changed

src/pages/ProfilesExplorerView/domain/variables/FiltersVariable/FiltersVariable.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { css } from '@emotion/css';
22
import { AdHocFiltersVariable, SceneComponentProps, sceneGraph } from '@grafana/scenes';
33
import { useStyles2 } from '@grafana/ui';
4-
import { CompleteFilters } from '@shared/components/QueryBuilder/domain/types';
4+
import { CompleteFilters, OperatorKind } from '@shared/components/QueryBuilder/domain/types';
55
import { QueryBuilder } from '@shared/components/QueryBuilder/QueryBuilder';
66
import React from 'react';
77

@@ -19,7 +19,11 @@ export class FiltersVariable extends AdHocFiltersVariable {
1919
label: 'Filters',
2020
filters: FiltersVariable.DEFAULT_VALUE,
2121
expressionBuilder: (filters) =>
22-
filters.map(({ key, operator, value }) => `${key}${operator}"${value}"`).join(','),
22+
filters
23+
.map(({ key, operator, value }) =>
24+
operator === OperatorKind['is-empty'] ? `${key}=""` : `${key}${operator}"${value}"`
25+
)
26+
.join(','),
2327
});
2428

2529
this.addActivationHandler(this.onActivate.bind(this));

src/pages/ProfilesExplorerView/domain/variables/FiltersVariable/filters-ops.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { AdHocVariableFilter } from '@grafana/data';
22
import { parseRawFilters } from '@shared/components/QueryBuilder/domain/helpers/queryToFilters';
3-
import { CompleteFilter, OperatorKind } from '@shared/components/QueryBuilder/domain/types';
3+
import { CompleteFilter } from '@shared/components/QueryBuilder/domain/types';
44

55
import { FiltersVariable } from './FiltersVariable';
66

77
export const convertPyroscopeToVariableFilter = (filter: CompleteFilter): AdHocVariableFilter => ({
88
key: filter.attribute.value,
9-
operator: filter.operator.value === OperatorKind['is-empty'] ? OperatorKind['='] : filter.operator.value,
9+
operator: filter.operator.value,
1010
value: filter.value.value,
1111
});
1212

src/shared/components/QueryBuilder/domain/actions.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
FilterKind,
1717
FilterPartKind,
1818
Filters,
19+
IsEmptyFilter,
1920
OperatorKind,
2021
PartialFilter,
2122
QueryBuilderContext,
@@ -87,7 +88,7 @@ export const actions: any = {
8788
// FILTER OPERATORS
8889
setFilterOperator: assign((context: QueryBuilderContext, event: SelectEvent) => {
8990
const newOperator = event.data;
90-
const newValue = newOperator.value === OperatorKind['is-empty'] ? { value: '', label: '' } : undefined;
91+
const newValue = newOperator.value === OperatorKind['is-empty'] ? IsEmptyFilter.value : undefined;
9192

9293
const newFilters = context.filters.map((filter) => {
9394
if (isPartialFilter(filter)) {
@@ -126,12 +127,14 @@ export const actions: any = {
126127

127128
const previousOperator = filter.operator!.value;
128129

130+
if (previousOperator === OperatorKind['is-empty']) {
131+
filter.value = { value: '', label: '' };
132+
}
133+
129134
if (newOperator.value === OperatorKind['is-empty']) {
130135
return {
131136
...filter,
132-
type: FilterKind['attribute-operator'],
133-
operator: newOperator,
134-
value: { value: '', label: '' },
137+
...IsEmptyFilter,
135138
active: false,
136139
};
137140
}

src/shared/components/QueryBuilder/domain/helpers/__tests__/queryToFilters.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ const cases: TestCase[] = [
7474
},
7575
value: {
7676
label: '',
77-
value: '',
77+
value: 'is-empty',
7878
},
7979
},
8080
],

src/shared/components/QueryBuilder/domain/helpers/queryToFilters.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { nanoid } from 'nanoid';
22

3-
import { FilterKind, Filters, OperatorKind } from '../types';
3+
import { FilterKind, Filters, IsEmptyFilter, OperatorKind } from '../types';
44

55
export const parseRawFilters = (rawFilters: string) =>
66
rawFilters.split(',').map((f) => {
@@ -36,15 +36,9 @@ export function queryToFilters(query: string): Filters {
3636
if (shouldChangeToIsEmptyOperator) {
3737
return {
3838
id: nanoid(10),
39-
type: FilterKind['attribute-operator'],
4039
active: true,
4140
attribute: { value: attribute, label: attribute },
42-
// TODO: don't hardcode the label
43-
operator: { value: OperatorKind['is-empty'], label: 'is empty' },
44-
value: {
45-
value: '',
46-
label: '',
47-
},
41+
...IsEmptyFilter,
4842
};
4943
}
5044

src/shared/components/QueryBuilder/domain/types.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,18 @@ export enum OperatorKind {
1515
'in' = 'in',
1616
}
1717

18+
export const IsEmptyFilter = {
19+
type: FilterKind['attribute-operator'],
20+
operator: {
21+
value: OperatorKind['is-empty'],
22+
label: 'is empty',
23+
},
24+
value: {
25+
value: OperatorKind['is-empty'],
26+
label: '',
27+
},
28+
};
29+
1830
export type PartialFilter = {
1931
id: string;
2032
type: FilterKind;

0 commit comments

Comments
 (0)