Skip to content

Commit 02d0649

Browse files
authored
fix(dashboards): Fix global filter inconsistencies with page filters (#101930)
Global filter fixes: - filterSelector: - Display saved global filter value selections even if not present in fetched values - Add page filters as a debounce parameter to fetch new values upon page filter changes - addFilter: - Only display string and boolean filter keys - When disabling filter keys that already exist global filters, check that the dataset matches as well
1 parent 5fa98ed commit 02d0649

File tree

3 files changed

+74
-33
lines changed

3 files changed

+74
-33
lines changed

static/app/views/dashboards/globalFilter/addFilter.spec.tsx

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,22 @@ import {WidgetType} from 'sentry/views/dashboards/types';
99
describe('AddFilter', () => {
1010
// Mock filter keys returned by the search bar data provider
1111
const mockFilterKeys: TagCollection = {
12-
browser: {
13-
key: 'browser',
14-
name: 'Browser',
12+
'browser.name': {
13+
key: 'browser.name',
14+
name: 'Browser Name',
1515
kind: FieldKind.FIELD,
1616
},
1717
environment: {
1818
key: 'environment',
1919
name: 'Environment',
2020
kind: FieldKind.FIELD,
2121
},
22-
unsupportedFunction: {
22+
'unsupported.function': {
2323
key: 'unsupported.function',
2424
name: 'Unsupported Function',
2525
kind: FieldKind.FUNCTION,
2626
},
27-
unsupportedMeasurement: {
27+
'unsupported.measurement': {
2828
key: 'unsupported.measurement',
2929
name: 'Unsupported Measurement',
3030
kind: FieldKind.MEASUREMENT,
@@ -64,17 +64,15 @@ describe('AddFilter', () => {
6464
await userEvent.click(screen.getByRole('button', {name: 'Add Global Filter'}));
6565

6666
// Verify filter keys are shown for each dataset
67-
for (const datasetLabel of DATASET_CHOICES.values()) {
68-
await userEvent.click(screen.getByText(datasetLabel));
67+
await userEvent.click(screen.getByText('Errors'));
6968

70-
// Should see filter key options for the dataset
71-
expect(screen.getByText('Select Filter Tag')).toBeInTheDocument();
72-
expect(screen.getByText(mockFilterKeys.browser!.key)).toBeInTheDocument();
73-
expect(screen.getByText(mockFilterKeys.environment!.key)).toBeInTheDocument();
69+
// Should see filter key options for the dataset
70+
expect(screen.getByText('Select Filter Tag')).toBeInTheDocument();
71+
expect(screen.getByText(mockFilterKeys['browser.name']!.key)).toBeInTheDocument();
72+
expect(screen.getByText(mockFilterKeys.environment!.key)).toBeInTheDocument();
7473

75-
// Return to dataset selection
76-
await userEvent.click(screen.getByText('Back'));
77-
}
74+
// Return to dataset selection
75+
await userEvent.click(screen.getByText('Back'));
7876
});
7977

8078
it('does not render unsupported filter keys', async () => {
@@ -92,10 +90,10 @@ describe('AddFilter', () => {
9290

9391
// Unsupported filter keys should not be included in the options
9492
expect(
95-
screen.queryByText(mockFilterKeys.unsupportedFunction!.key)
93+
screen.queryByText(mockFilterKeys['unsupported.function']!.key)
9694
).not.toBeInTheDocument();
9795
expect(
98-
screen.queryByText(mockFilterKeys.unsupportedMeasurement!.key)
96+
screen.queryByText(mockFilterKeys['unsupported.measurement']!.key)
9997
).not.toBeInTheDocument();
10098
});
10199

@@ -114,14 +112,16 @@ describe('AddFilter', () => {
114112

115113
// Select arbitrary dataset and filter key
116114
await userEvent.click(screen.getByText('Errors'));
117-
await userEvent.click(screen.getByText(mockFilterKeys.browser!.key));
118-
await userEvent.click(screen.getByText('Add Filter'));
115+
await userEvent.click(
116+
screen.getByRole('option', {name: mockFilterKeys['browser.name']!.key})
117+
);
118+
await userEvent.click(screen.getByRole('button', {name: 'Add Filter'}));
119119

120120
// Verify onAddFilter was called with the added global filter object
121121
expect(onAddFilter).toHaveBeenCalledTimes(1);
122122
expect(onAddFilter).toHaveBeenCalledWith({
123123
dataset: WidgetType.ERRORS,
124-
tag: mockFilterKeys.browser,
124+
tag: mockFilterKeys['browser.name'],
125125
value: '',
126126
});
127127
});

static/app/views/dashboards/globalFilter/addFilter.tsx

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@ import {IconAdd, IconArrow} from 'sentry/icons';
1111
import {t} from 'sentry/locale';
1212
import {space} from 'sentry/styles/space';
1313
import type {Tag} from 'sentry/types/group';
14-
import {FieldKind, getFieldDefinition, prettifyTagKey} from 'sentry/utils/fields';
14+
import {
15+
FieldKind,
16+
FieldValueType,
17+
getFieldDefinition,
18+
prettifyTagKey,
19+
} from 'sentry/utils/fields';
1520
import type {SearchBarData} from 'sentry/views/dashboards/datasetConfig/base';
1621
import {WidgetType, type GlobalFilter} from 'sentry/views/dashboards/types';
1722
import {shouldExcludeTracingKeys} from 'sentry/views/performance/utils';
@@ -25,19 +30,12 @@ export const DATASET_CHOICES = new Map<WidgetType, string>([
2530
]);
2631

2732
const UNSUPPORTED_FIELD_KINDS = [FieldKind.FUNCTION, FieldKind.MEASUREMENT];
33+
const SUPPORTED_FIELD_VALUE_TYPES = [FieldValueType.STRING, FieldValueType.BOOLEAN];
2834

2935
export function getDatasetLabel(dataset: WidgetType) {
3036
return DATASET_CHOICES.get(dataset) ?? '';
3137
}
3238

33-
function getTagType(tag: Tag, dataset: WidgetType) {
34-
const fieldType =
35-
dataset === WidgetType.SPANS ? 'span' : dataset === WidgetType.LOGS ? 'log' : 'event';
36-
const fieldDefinition = getFieldDefinition(tag.key, fieldType, tag.kind);
37-
38-
return <ValueType fieldDefinition={fieldDefinition} fieldKind={tag.kind} />;
39-
}
40-
4139
type AddFilterProps = {
4240
getSearchBarData: (widgetType: WidgetType) => SearchBarData;
4341
globalFilters: GlobalFilter[];
@@ -70,12 +68,39 @@ function AddFilter({globalFilters, getSearchBarData, onAddFilter}: AddFilterProp
7068

7169
// Get filter keys for the selected dataset
7270
const filterKeyOptions = selectedDataset
73-
? Object.entries(filterKeys).map(([_, tag]) => {
71+
? Object.entries(filterKeys).flatMap(([_, tag]) => {
72+
const fieldType = (datasetType: WidgetType) => {
73+
switch (datasetType) {
74+
case WidgetType.SPANS:
75+
return 'span';
76+
case WidgetType.LOGS:
77+
return 'log';
78+
default:
79+
return 'event';
80+
}
81+
};
82+
const fieldDefinition = getFieldDefinition(
83+
tag.key,
84+
fieldType(selectedDataset),
85+
tag.kind
86+
);
87+
const valueType = fieldDefinition?.valueType;
88+
89+
if (!valueType || !SUPPORTED_FIELD_VALUE_TYPES.includes(valueType)) {
90+
return [];
91+
}
92+
7493
return {
7594
value: tag.key,
7695
label: prettifyTagKey(tag.key),
77-
trailingItems: <TagBadge>{getTagType(tag, selectedDataset)}</TagBadge>,
78-
disabled: globalFilters.some(filter => filter.tag.key === tag.key),
96+
trailingItems: (
97+
<TagBadge>
98+
<ValueType fieldDefinition={fieldDefinition} fieldKind={tag.kind} />
99+
</TagBadge>
100+
),
101+
disabled: globalFilters.some(
102+
filter => filter.tag.key === tag.key && filter.dataset === selectedDataset
103+
),
79104
};
80105
})
81106
: [];

static/app/views/dashboards/globalFilter/filterSelector.tsx

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {t} from 'sentry/locale';
1111
import {space} from 'sentry/styles/space';
1212
import {keepPreviousData, useQuery} from 'sentry/utils/queryClient';
1313
import {useDebouncedValue} from 'sentry/utils/useDebouncedValue';
14+
import usePageFilters from 'sentry/utils/usePageFilters';
1415
import {type SearchBarData} from 'sentry/views/dashboards/datasetConfig/base';
1516
import {getDatasetLabel} from 'sentry/views/dashboards/globalFilter/addFilter';
1617
import FilterSelectorTrigger from 'sentry/views/dashboards/globalFilter/filterSelectorTrigger';
@@ -42,8 +43,12 @@ function FilterSelector({
4243
}, [initialValues]);
4344

4445
const {dataset, tag} = globalFilter;
46+
const pageFilters = usePageFilters();
4547

46-
const baseQueryKey = useMemo(() => ['global-dashboard-filters-tag-values', tag], [tag]);
48+
const baseQueryKey = useMemo(
49+
() => ['global-dashboard-filters-tag-values', tag, pageFilters.selection],
50+
[tag, pageFilters.selection]
51+
);
4752
const queryKey = useDebouncedValue(baseQueryKey);
4853

4954
const queryResult = useQuery<string[]>({
@@ -59,14 +64,25 @@ function FilterSelector({
5964
});
6065

6166
const {data, isFetching} = queryResult;
62-
const options = useMemo(() => {
67+
const fetchedOptions = useMemo(() => {
6368
if (!data) return [];
6469
return data.map(value => ({
6570
label: value,
6671
value,
6772
}));
6873
}, [data]);
6974

75+
const savedFilterValueOptions = useMemo(() => {
76+
return activeFilterValues
77+
.map(value => ({
78+
label: value,
79+
value,
80+
}))
81+
.filter(option => !fetchedOptions.some(o => o.value === option.value));
82+
}, [activeFilterValues, fetchedOptions]);
83+
84+
const options = [...savedFilterValueOptions, ...fetchedOptions];
85+
7086
const handleChange = (opts: string[]) => {
7187
if (isEqual(opts, activeFilterValues)) {
7288
return;

0 commit comments

Comments
 (0)