Skip to content

Commit 271ebe4

Browse files
authored
chore(dashboards): Centralize search bar data hook invocation (#101622)
Motivation: - I previously looped through datasets and called the corresponding searchbar data hook to build a map on the fly. This is bad practice because hook calls should be explicit and unconditional. - When I create numeric & date filter components, I need to use the SearchSyntax parser instead of MutableSearch. SearchSyntax requires providing all filter keys of a dataset. So, instead of repeating filter key retrieval across components, it is more organized and performant to centralize it in a single hook. Changes: - Creates central hook for all dataset search bar data that explicitly invokes search bar data hooks for all datasets. - Replaces looped hook calls in the addFilter component with the centralized data hook.
1 parent dddcf05 commit 271ebe4

File tree

7 files changed

+103
-91
lines changed

7 files changed

+103
-91
lines changed

static/app/views/dashboards/detail.spec.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,10 @@ describe('Dashboards > Detail', () => {
149149
url: '/organizations/org-slug/releases/stats/',
150150
body: [],
151151
});
152+
MockApiClient.addMockResponse({
153+
url: '/organizations/org-slug/measurements-meta/',
154+
body: [],
155+
});
152156
});
153157

154158
afterEach(() => {

static/app/views/dashboards/filtersBar.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import usePageFilters from 'sentry/utils/usePageFilters';
1919
import {useUser} from 'sentry/utils/useUser';
2020
import {useUserTeams} from 'sentry/utils/useUserTeams';
2121
import AddFilter from 'sentry/views/dashboards/globalFilter/addFilter';
22+
import {useDatasetSearchBarData} from 'sentry/views/dashboards/hooks/useDatasetSearchBarData';
2223
import {useInvalidateStarredDashboards} from 'sentry/views/dashboards/hooks/useInvalidateStarredDashboards';
2324
import {getDashboardFiltersFromURL} from 'sentry/views/dashboards/utils';
2425

@@ -59,6 +60,8 @@ export default function FiltersBar({
5960
const organization = useOrganization();
6061
const currentUser = useUser();
6162
const {teams: userTeams} = useUserTeams();
63+
const getSearchBarData = useDatasetSearchBarData();
64+
6265
const hasEditAccess = checkUserHasEditAccess(
6366
currentUser,
6467
userTeams,
@@ -147,6 +150,7 @@ export default function FiltersBar({
147150
<FilterSelector
148151
key={filter.tag.key}
149152
globalFilter={filter}
153+
searchBarData={getSearchBarData(filter.dataset)}
150154
onUpdateFilter={updatedFilter => {
151155
updateGlobalFilters(
152156
activeGlobalFilters.map(f =>
@@ -162,6 +166,7 @@ export default function FiltersBar({
162166
/>
163167
))}
164168
<AddFilter
169+
getSearchBarData={getSearchBarData}
165170
onAddFilter={newFilter => {
166171
updateGlobalFilters([...activeGlobalFilters, newFilter]);
167172
}}

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

Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,10 @@ import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
22

33
import type {TagCollection} from 'sentry/types/group';
44
import {FieldKind} from 'sentry/utils/fields';
5-
import {getDatasetConfig} from 'sentry/views/dashboards/datasetConfig/base';
5+
import {type SearchBarData} from 'sentry/views/dashboards/datasetConfig/base';
66
import AddFilter, {DATASET_CHOICES} from 'sentry/views/dashboards/globalFilter/addFilter';
77
import {WidgetType} from 'sentry/views/dashboards/types';
88

9-
// Mock getDatasetConfig
10-
jest.mock('sentry/views/dashboards/datasetConfig/base');
11-
129
describe('AddFilter', () => {
1310
// Mock filter keys returned by the search bar data provider
1411
const mockFilterKeys: TagCollection = {
@@ -34,43 +31,26 @@ describe('AddFilter', () => {
3431
},
3532
};
3633

37-
const mockUseSearchBarDataProvider = jest.fn(() => ({
34+
const getSearchBarData = (_: WidgetType): SearchBarData => ({
3835
getFilterKeys: () => mockFilterKeys,
39-
}));
40-
41-
beforeEach(() => {
42-
MockApiClient.clearMockResponses();
43-
44-
// Mock getDatasetConfig that returns a config with a mock search bar data provider
45-
jest.mocked(getDatasetConfig).mockReturnValue({
46-
useSearchBarDataProvider: mockUseSearchBarDataProvider,
47-
} as any);
48-
});
49-
50-
afterEach(() => {
51-
jest.clearAllMocks();
36+
getFilterKeySections: () => [],
37+
getTagValues: () => Promise.resolve([]),
5238
});
5339

5440
it('renders all dataset options', async () => {
55-
render(<AddFilter onAddFilter={() => {}} />);
41+
render(<AddFilter getSearchBarData={getSearchBarData} onAddFilter={() => {}} />);
5642
await userEvent.click(screen.getByRole('button', {name: 'Add Global Filter'}));
5743
for (const dataset of DATASET_CHOICES.values()) {
5844
expect(screen.getByText(dataset)).toBeInTheDocument();
5945
}
6046
});
6147

6248
it('retrieves filter keys for each dataset', async () => {
63-
render(<AddFilter onAddFilter={() => {}} />);
49+
render(<AddFilter getSearchBarData={getSearchBarData} onAddFilter={() => {}} />);
6450

6551
// Open the add global filter drop down
6652
await userEvent.click(screen.getByRole('button', {name: 'Add Global Filter'}));
6753

68-
// Verify search bar data provider was called for each dataset type
69-
for (const [widgetType] of DATASET_CHOICES.entries()) {
70-
expect(getDatasetConfig).toHaveBeenCalledWith(widgetType);
71-
}
72-
expect(mockUseSearchBarDataProvider).toHaveBeenCalledTimes(DATASET_CHOICES.size);
73-
7454
// Verify filter keys are shown for each dataset
7555
for (const datasetLabel of DATASET_CHOICES.values()) {
7656
await userEvent.click(screen.getByText(datasetLabel));
@@ -86,7 +66,7 @@ describe('AddFilter', () => {
8666
});
8767

8868
it('does not render unsupported filter keys', async () => {
89-
render(<AddFilter onAddFilter={() => {}} />);
69+
render(<AddFilter getSearchBarData={getSearchBarData} onAddFilter={() => {}} />);
9070

9171
// Open the dropdown and select an arbitrary dataset
9272
await userEvent.click(screen.getByRole('button', {name: 'Add Global Filter'}));
@@ -103,7 +83,7 @@ describe('AddFilter', () => {
10383

10484
it('calls onAddFilter with expected global filter object', async () => {
10585
const onAddFilter = jest.fn();
106-
render(<AddFilter onAddFilter={onAddFilter} />);
86+
render(<AddFilter getSearchBarData={getSearchBarData} onAddFilter={onAddFilter} />);
10787

10888
// Open add global filter drop down
10989
await userEvent.click(screen.getByRole('button', {name: 'Add Global Filter'}));

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

Lines changed: 19 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,9 @@ import {ValueType} from 'sentry/components/searchQueryBuilder/tokens/filterKeyLi
1010
import {IconAdd, IconArrow} from 'sentry/icons';
1111
import {t} from 'sentry/locale';
1212
import {space} from 'sentry/styles/space';
13-
import type {Tag, TagCollection} from 'sentry/types/group';
13+
import type {Tag} from 'sentry/types/group';
1414
import {FieldKind, getFieldDefinition, prettifyTagKey} from 'sentry/utils/fields';
15-
import usePageFilters from 'sentry/utils/usePageFilters';
16-
import {getDatasetConfig} from 'sentry/views/dashboards/datasetConfig/base';
15+
import type {SearchBarData} from 'sentry/views/dashboards/datasetConfig/base';
1716
import {WidgetType, type GlobalFilter} from 'sentry/views/dashboards/types';
1817
import {shouldExcludeTracingKeys} from 'sentry/views/performance/utils';
1918

@@ -31,7 +30,7 @@ export function getDatasetLabel(dataset: WidgetType) {
3130
return DATASET_CHOICES.get(dataset) ?? '';
3231
}
3332

34-
function getTagType(tag: Tag, dataset: WidgetType | null) {
33+
function getTagType(tag: Tag, dataset: WidgetType) {
3534
const fieldType =
3635
dataset === WidgetType.SPANS ? 'span' : dataset === WidgetType.LOGS ? 'log' : 'event';
3736
const fieldDefinition = getFieldDefinition(tag.key, fieldType, tag.kind);
@@ -40,14 +39,14 @@ function getTagType(tag: Tag, dataset: WidgetType | null) {
4039
}
4140

4241
type AddFilterProps = {
42+
getSearchBarData: (widgetType: WidgetType) => SearchBarData;
4343
onAddFilter: (filter: GlobalFilter) => void;
4444
};
4545

46-
function AddFilter({onAddFilter}: AddFilterProps) {
46+
function AddFilter({getSearchBarData, onAddFilter}: AddFilterProps) {
4747
const [selectedDataset, setSelectedDataset] = useState<WidgetType | null>(null);
4848
const [selectedFilterKey, setSelectedFilterKey] = useState<Tag | null>(null);
4949
const [isSelectingFilterKey, setIsSelectingFilterKey] = useState(false);
50-
const {selection} = usePageFilters();
5150

5251
// Dataset selection before showing filter keys
5352
const datasetOptions = useMemo(() => {
@@ -58,34 +57,26 @@ function AddFilter({onAddFilter}: AddFilterProps) {
5857
}));
5958
}, []);
6059

61-
const datasetFilterKeysMap = new Map<WidgetType, TagCollection>();
62-
63-
DATASET_CHOICES.forEach((_, widgetType) => {
64-
const datasetConfig = getDatasetConfig(widgetType);
65-
if (datasetConfig.useSearchBarDataProvider) {
66-
const dataProvider = datasetConfig.useSearchBarDataProvider({
67-
pageFilters: selection,
68-
});
69-
const filterKeys = Object.fromEntries(
70-
Object.entries(dataProvider.getFilterKeys()).filter(
60+
const filterKeys: Record<string, Tag> = selectedDataset
61+
? Object.fromEntries(
62+
Object.entries(getSearchBarData(selectedDataset).getFilterKeys()).filter(
7163
([key, value]) =>
7264
!shouldExcludeTracingKeys(key) &&
7365
(!value.kind || !UNSUPPORTED_FIELD_KINDS.includes(value.kind))
7466
)
75-
);
76-
datasetFilterKeysMap.set(widgetType, filterKeys);
77-
}
78-
});
67+
)
68+
: {};
7969

8070
// Get filter keys for the selected dataset
81-
const filterKeys = (selectedDataset && datasetFilterKeysMap.get(selectedDataset)) || {};
82-
const filterKeyOptions = Object.entries(filterKeys).map(([_, tag]) => {
83-
return {
84-
value: tag.key,
85-
label: prettifyTagKey(tag.key),
86-
trailingItems: <TagBadge>{getTagType(tag, selectedDataset)}</TagBadge>,
87-
};
88-
});
71+
const filterKeyOptions = selectedDataset
72+
? Object.entries(filterKeys).map(([_, tag]) => {
73+
return {
74+
value: tag.key,
75+
label: prettifyTagKey(tag.key),
76+
trailingItems: <TagBadge>{getTagType(tag, selectedDataset)}</TagBadge>,
77+
};
78+
})
79+
: [];
8980

9081
// Footer for filter key selection for adding filters and returning to dataset selection
9182
const filterOptionsMenuFooter = ({closeOverlay}: {closeOverlay: () => void}) => (

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

Lines changed: 10 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
22

33
import {FieldKind} from 'sentry/utils/fields';
4+
import type {SearchBarData} from 'sentry/views/dashboards/datasetConfig/base';
45
import FilterSelector from 'sentry/views/dashboards/globalFilter/filterSelector';
56
import {WidgetType, type GlobalFilter} from 'sentry/views/dashboards/types';
67

@@ -17,40 +18,18 @@ describe('FilterSelector', () => {
1718
},
1819
value: '',
1920
};
20-
beforeEach(() => {
21-
MockApiClient.clearMockResponses();
2221

23-
// Mock tags endpoint
24-
MockApiClient.addMockResponse({
25-
url: '/organizations/org-slug/tags/',
26-
body: [],
27-
});
28-
29-
// Mock custom measurements endpoint
30-
MockApiClient.addMockResponse({
31-
url: '/organizations/org-slug/measurements-meta/',
32-
body: {},
33-
});
34-
35-
// Mock tag values endpoint
36-
MockApiClient.addMockResponse({
37-
url: `/organizations/org-slug/tags/${mockGlobalFilter.tag.key}/values/`,
38-
body: [
39-
{name: 'chrome', value: 'chrome', count: 100},
40-
{name: 'firefox', value: 'firefox', count: 50},
41-
{name: 'safari', value: 'safari', count: 25},
42-
],
43-
});
44-
});
45-
46-
afterEach(() => {
47-
jest.clearAllMocks();
48-
});
22+
const mockSearchBarData: SearchBarData = {
23+
getFilterKeySections: () => [],
24+
getFilterKeys: () => ({}),
25+
getTagValues: () => Promise.resolve(['chrome', 'firefox', 'safari']),
26+
};
4927

5028
it('renders all filter values', async () => {
5129
render(
5230
<FilterSelector
5331
globalFilter={mockGlobalFilter}
32+
searchBarData={mockSearchBarData}
5433
onUpdateFilter={mockOnUpdateFilter}
5534
onRemoveFilter={mockOnRemoveFilter}
5635
/>
@@ -68,6 +47,7 @@ describe('FilterSelector', () => {
6847
render(
6948
<FilterSelector
7049
globalFilter={mockGlobalFilter}
50+
searchBarData={mockSearchBarData}
7151
onUpdateFilter={mockOnUpdateFilter}
7252
onRemoveFilter={mockOnRemoveFilter}
7353
/>
@@ -98,6 +78,7 @@ describe('FilterSelector', () => {
9878
render(
9979
<FilterSelector
10080
globalFilter={{...mockGlobalFilter, value: 'browser:[firefox,chrome]'}}
81+
searchBarData={mockSearchBarData}
10182
onUpdateFilter={mockOnUpdateFilter}
10283
onRemoveFilter={mockOnRemoveFilter}
10384
/>
@@ -114,6 +95,7 @@ describe('FilterSelector', () => {
11495
render(
11596
<FilterSelector
11697
globalFilter={mockGlobalFilter}
98+
searchBarData={mockSearchBarData}
11799
onUpdateFilter={mockOnUpdateFilter}
118100
onRemoveFilter={mockOnRemoveFilter}
119101
/>

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@ import {MutableSearch} from 'sentry/components/searchSyntax/mutableSearch';
77
import {t} from 'sentry/locale';
88
import {keepPreviousData, useQuery} from 'sentry/utils/queryClient';
99
import {useDebouncedValue} from 'sentry/utils/useDebouncedValue';
10-
import usePageFilters from 'sentry/utils/usePageFilters';
11-
import {getDatasetConfig} from 'sentry/views/dashboards/datasetConfig/base';
10+
import {type SearchBarData} from 'sentry/views/dashboards/datasetConfig/base';
1211
import {getDatasetLabel} from 'sentry/views/dashboards/globalFilter/addFilter';
1312
import FilterSelectorTrigger from 'sentry/views/dashboards/globalFilter/filterSelectorTrigger';
1413
import type {GlobalFilter} from 'sentry/views/dashboards/types';
@@ -17,10 +16,12 @@ type FilterSelectorProps = {
1716
globalFilter: GlobalFilter;
1817
onRemoveFilter: (filter: GlobalFilter) => void;
1918
onUpdateFilter: (filter: GlobalFilter) => void;
19+
searchBarData: SearchBarData;
2020
};
2121

2222
function FilterSelector({
2323
globalFilter,
24+
searchBarData,
2425
onRemoveFilter,
2526
onUpdateFilter,
2627
}: FilterSelectorProps) {
@@ -37,10 +38,6 @@ function FilterSelector({
3738
}, [initialValues]);
3839

3940
const {dataset, tag} = globalFilter;
40-
const {selection} = usePageFilters();
41-
const dataProvider = getDatasetConfig(dataset).useSearchBarDataProvider!({
42-
pageFilters: selection,
43-
});
4441

4542
const baseQueryKey = useMemo(() => ['global-dashboard-filters-tag-values', tag], [tag]);
4643
const queryKey = useDebouncedValue(baseQueryKey);
@@ -50,7 +47,7 @@ function FilterSelector({
5047
// eslint-disable-next-line @tanstack/query/exhaustive-deps
5148
queryKey,
5249
queryFn: async () => {
53-
const result = await dataProvider?.getTagValues(tag, '');
50+
const result = await searchBarData.getTagValues(tag, '');
5451
return result ?? [];
5552
},
5653
placeholderData: keepPreviousData,
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import usePageFilters from 'sentry/utils/usePageFilters';
2+
import {
3+
getDatasetConfig,
4+
type SearchBarData,
5+
} from 'sentry/views/dashboards/datasetConfig/base';
6+
import {WidgetType} from 'sentry/views/dashboards/types';
7+
8+
export function useDatasetSearchBarData(): (widgetType: WidgetType) => SearchBarData {
9+
const {selection} = usePageFilters();
10+
11+
const errorsData = getDatasetConfig(WidgetType.ERRORS).useSearchBarDataProvider!({
12+
pageFilters: selection,
13+
});
14+
15+
const logsData = getDatasetConfig(WidgetType.LOGS).useSearchBarDataProvider!({
16+
pageFilters: selection,
17+
});
18+
19+
const spansData = getDatasetConfig(WidgetType.SPANS).useSearchBarDataProvider!({
20+
pageFilters: selection,
21+
});
22+
23+
const issuesData = getDatasetConfig(WidgetType.ISSUE).useSearchBarDataProvider!({
24+
pageFilters: selection,
25+
});
26+
27+
const releasesData = getDatasetConfig(WidgetType.RELEASE).useSearchBarDataProvider!({
28+
pageFilters: selection,
29+
});
30+
31+
const getSearchBarData = (widgetType: WidgetType): SearchBarData => {
32+
switch (widgetType) {
33+
case WidgetType.ERRORS:
34+
return errorsData;
35+
case WidgetType.LOGS:
36+
return logsData;
37+
case WidgetType.SPANS:
38+
return spansData;
39+
case WidgetType.ISSUE:
40+
return issuesData;
41+
case WidgetType.RELEASE:
42+
return releasesData;
43+
default:
44+
return {
45+
getFilterKeySections: () => [],
46+
getFilterKeys: () => ({}),
47+
getTagValues: () => Promise.resolve([]),
48+
};
49+
}
50+
};
51+
52+
return getSearchBarData;
53+
}

0 commit comments

Comments
 (0)