Skip to content

Commit 93838c9

Browse files
nsdeschenesClaude Opus 4.6
nsdeschenes
and
Claude Opus 4.6
committed
ref(explore): Replace imperative validateQuery with declarative useQuery
Move useAttributeValidation from a callback-based approach (manual fetchQuery + useState) to a declarative useQuery hook. The hook now accepts query and selection as parameters, letting React Query handle caching, deduplication, and cancellation automatically. This simplifies the consumer in traceItemSearchQueryBuilder by removing the useEffect and wrapped onChange callback. Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
1 parent a35224b commit 93838c9

File tree

3 files changed

+77
-130
lines changed

3 files changed

+77
-130
lines changed

‎static/app/views/explore/components/traceItemSearchQueryBuilder.tsx‎

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {useCallback, useEffect, useMemo} from 'react';
1+
import {useMemo} from 'react';
22

33
import {usePageFilters} from 'sentry/components/pageFilters/usePageFilters';
44
import type {SpanSearchQueryBuilderProps} from 'sentry/components/performance/spanSearchQueryBuilder';
@@ -7,7 +7,6 @@ import {
77
type SearchQueryBuilderProps,
88
} from 'sentry/components/searchQueryBuilder';
99
import type {CaseInsensitive} from 'sentry/components/searchQueryBuilder/hooks';
10-
import type {CallbackSearchState} from 'sentry/components/searchQueryBuilder/types';
1110
import {t} from 'sentry/locale';
1211
import {SavedSearchType, type TagCollection} from 'sentry/types/group';
1312
import type {AggregationKey} from 'sentry/utils/fields';
@@ -109,26 +108,17 @@ export function useTraceItemSearchQueryBuilderProps({
109108
}: TraceItemSearchQueryBuilderProps) {
110109
const placeholderText = itemTypeToDefaultPlaceholder(itemType);
111110

112-
const {invalidFilterKeys, validateQuery} = useAttributeValidation(itemType);
113111
const {selection} = usePageFilters();
114112
const effectiveProjects = projects ?? selection.projects;
115113
const validationSelection = useMemo(
116114
() => ({datetime: selection.datetime, projects: effectiveProjects}),
117115
[selection.datetime, effectiveProjects]
118116
);
119117

120-
useEffect(() => {
121-
if (initialQuery) {
122-
validateQuery(initialQuery, validationSelection);
123-
}
124-
}, [initialQuery, validateQuery, validationSelection]);
125-
126-
const wrappedOnChange = useCallback(
127-
(query: string, state: CallbackSearchState) => {
128-
validateQuery(query, validationSelection);
129-
onChange?.(query, state);
130-
},
131-
[validateQuery, validationSelection, onChange]
118+
const {invalidFilterKeys} = useAttributeValidation(
119+
itemType,
120+
initialQuery ?? '',
121+
validationSelection
132122
);
133123
const functionTags = useFunctionTags(itemType, supportedAggregates);
134124
const filterKeySections = useFilterKeySections(itemType, stringAttributes);
@@ -165,7 +155,7 @@ export function useTraceItemSearchQueryBuilderProps({
165155
initialQuery,
166156
fieldDefinitionGetter: getTraceItemFieldDefinitionFunction(itemType, filterTags),
167157
onSearch,
168-
onChange: wrappedOnChange,
158+
onChange,
169159
onBlur,
170160
getFilterTokenWarning,
171161
searchSource,
@@ -213,7 +203,7 @@ export function useTraceItemSearchQueryBuilderProps({
213203
numberSecondaryAliases,
214204
onBlur,
215205
onCaseInsensitiveClick,
216-
wrappedOnChange,
206+
onChange,
217207
onSearch,
218208
placeholderText,
219209
portalTarget,

‎static/app/views/explore/hooks/useAttributeValidation.spec.tsx‎

Lines changed: 43 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
import {OrganizationFixture} from 'sentry-fixture/organization';
22

3-
import {act, renderHookWithProviders, waitFor} from 'sentry-test/reactTestingLibrary';
3+
import {renderHookWithProviders, waitFor} from 'sentry-test/reactTestingLibrary';
44

5-
import {parseSearch} from 'sentry/components/searchSyntax/parser';
65
import {
7-
extractFilterKeys,
86
useAttributeValidation,
97
type AttributeValidationSelection,
108
} from 'sentry/views/explore/hooks/useAttributeValidation';
@@ -16,69 +14,31 @@ const DEFAULT_SELECTION: AttributeValidationSelection = {
1614
projects: [],
1715
};
1816

19-
describe('extractFilterKeys', () => {
20-
it('returns empty array for null parsedQuery', () => {
21-
expect(extractFilterKeys(null)).toEqual([]);
22-
});
23-
24-
it('returns empty array for empty parsedQuery', () => {
25-
expect(extractFilterKeys(parseSearch(''))).toEqual([]);
26-
});
27-
28-
it('returns empty array when there are no filter tokens', () => {
29-
expect(extractFilterKeys(parseSearch('hello world'))).toEqual([]);
30-
});
31-
32-
it('extracts and sorts filter keys', () => {
33-
expect(extractFilterKeys(parseSearch('zebra:value apple:value'))).toEqual([
34-
'apple',
35-
'zebra',
36-
]);
37-
});
38-
39-
it('deduplicates keys', () => {
40-
expect(extractFilterKeys(parseSearch('duplicate:value duplicate:other'))).toEqual([
41-
'duplicate',
42-
]);
43-
});
44-
45-
it('returns stable reference for empty results', () => {
46-
const result1 = extractFilterKeys(null);
47-
const result2 = extractFilterKeys(null);
48-
expect(result1).toBe(result2);
49-
});
50-
});
51-
5217
describe('useAttributeValidation', () => {
5318
const organization = OrganizationFixture({slug: ORG_SLUG});
5419

55-
it('returns empty invalidFilterKeys initially', () => {
20+
it('returns empty invalidFilterKeys for empty query', () => {
5621
const {result} = renderHookWithProviders(
57-
() => useAttributeValidation(TraceItemDataset.SPANS),
22+
() => useAttributeValidation(TraceItemDataset.SPANS, '', DEFAULT_SELECTION),
5823
{organization}
5924
);
6025

6126
expect(result.current.invalidFilterKeys).toEqual([]);
6227
});
6328

64-
it('does not call the API when query has no filter keys', async () => {
29+
it('does not call the API when query has no filter keys', () => {
6530
const mockValidate = MockApiClient.addMockResponse({
6631
url: `/organizations/${ORG_SLUG}/trace-items/attributes/validate/`,
6732
method: 'POST',
6833
body: {attributes: {}},
6934
});
7035

71-
const {result} = renderHookWithProviders(
72-
() => useAttributeValidation(TraceItemDataset.SPANS),
36+
renderHookWithProviders(
37+
() => useAttributeValidation(TraceItemDataset.SPANS, '', DEFAULT_SELECTION),
7338
{organization}
7439
);
7540

76-
await act(async () => {
77-
await result.current.validateQuery('', DEFAULT_SELECTION);
78-
});
79-
8041
expect(mockValidate).not.toHaveBeenCalled();
81-
expect(result.current.invalidFilterKeys).toEqual([]);
8242
});
8343

8444
it('sets invalidFilterKeys for keys the API reports as invalid', async () => {
@@ -94,17 +54,15 @@ describe('useAttributeValidation', () => {
9454
});
9555

9656
const {result} = renderHookWithProviders(
97-
() => useAttributeValidation(TraceItemDataset.SPANS),
57+
() =>
58+
useAttributeValidation(
59+
TraceItemDataset.SPANS,
60+
'span.op:db unknown_key:value',
61+
DEFAULT_SELECTION
62+
),
9863
{organization}
9964
);
10065

101-
await act(async () => {
102-
await result.current.validateQuery(
103-
'span.op:db unknown_key:value',
104-
DEFAULT_SELECTION
105-
);
106-
});
107-
10866
await waitFor(() => {
10967
expect(result.current.invalidFilterKeys).toEqual(['unknown_key']);
11068
});
@@ -118,14 +76,11 @@ describe('useAttributeValidation', () => {
11876
});
11977

12078
const {result} = renderHookWithProviders(
121-
() => useAttributeValidation(TraceItemDataset.SPANS),
79+
() =>
80+
useAttributeValidation(TraceItemDataset.SPANS, 'span.op:db', DEFAULT_SELECTION),
12281
{organization}
12382
);
12483

125-
await act(async () => {
126-
await result.current.validateQuery('span.op:db', DEFAULT_SELECTION);
127-
});
128-
12984
await waitFor(() => {
13085
expect(result.current.invalidFilterKeys).toEqual([]);
13186
});
@@ -138,17 +93,23 @@ describe('useAttributeValidation', () => {
13893
body: {attributes: {'span.op': {valid: true}}},
13994
});
14095

141-
const {result} = renderHookWithProviders(
142-
() => useAttributeValidation(TraceItemDataset.SPANS),
143-
{organization}
96+
const {result, rerender} = renderHookWithProviders(
97+
({query, selection}: {query: string; selection: AttributeValidationSelection}) =>
98+
useAttributeValidation(TraceItemDataset.SPANS, query, selection),
99+
{
100+
organization,
101+
initialProps: {query: 'span.op:db', selection: DEFAULT_SELECTION},
102+
}
144103
);
145104

146-
await act(async () => {
147-
await result.current.validateQuery('span.op:db', DEFAULT_SELECTION);
105+
await waitFor(() => {
106+
expect(result.current.invalidFilterKeys).toEqual([]);
148107
});
149108

150-
await act(async () => {
151-
await result.current.validateQuery('span.op:web', DEFAULT_SELECTION);
109+
rerender({query: 'span.op:web', selection: DEFAULT_SELECTION});
110+
111+
await waitFor(() => {
112+
expect(result.current.invalidFilterKeys).toEqual([]);
152113
});
153114

154115
expect(mockValidate).toHaveBeenCalledTimes(1);
@@ -161,22 +122,29 @@ describe('useAttributeValidation', () => {
161122
body: {attributes: {'span.op': {valid: true}}},
162123
});
163124

164-
const {result} = renderHookWithProviders(
165-
() => useAttributeValidation(TraceItemDataset.SPANS),
166-
{organization}
125+
const {result, rerender} = renderHookWithProviders(
126+
({query, selection}: {query: string; selection: AttributeValidationSelection}) =>
127+
useAttributeValidation(TraceItemDataset.SPANS, query, selection),
128+
{
129+
organization,
130+
initialProps: {query: 'span.op:db', selection: DEFAULT_SELECTION},
131+
}
167132
);
168133

169-
await act(async () => {
170-
await result.current.validateQuery('span.op:db', DEFAULT_SELECTION);
134+
await waitFor(() => {
135+
expect(result.current.invalidFilterKeys).toEqual([]);
171136
});
172137

173-
await act(async () => {
174-
await result.current.validateQuery('span.op:db', {
138+
rerender({
139+
query: 'span.op:db',
140+
selection: {
175141
datetime: {period: '7d', start: null, end: null, utc: false},
176142
projects: [],
177-
});
143+
},
178144
});
179145

180-
expect(mockValidate).toHaveBeenCalledTimes(2);
146+
await waitFor(() => {
147+
expect(mockValidate).toHaveBeenCalledTimes(2);
148+
});
181149
});
182150
});

‎static/app/views/explore/hooks/useAttributeValidation.tsx‎

Lines changed: 27 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {useCallback, useState} from 'react';
1+
import {useMemo} from 'react';
22

33
import {normalizeDateTimeParams} from 'sentry/components/pageFilters/parse';
44
import type {ParseResult} from 'sentry/components/searchSyntax/parser';
@@ -8,7 +8,7 @@ import {getKeyName} from 'sentry/components/searchSyntax/utils';
88
import type {PageFilters} from 'sentry/types/core';
99
import type {ApiQueryKey} from 'sentry/utils/api/apiQueryKey';
1010
import {getApiUrl} from 'sentry/utils/api/getApiUrl';
11-
import {fetchDataQuery, queryOptions, useQueryClient} from 'sentry/utils/queryClient';
11+
import {fetchDataQuery, queryOptions, useQuery} from 'sentry/utils/queryClient';
1212
import {useOrganization} from 'sentry/utils/useOrganization';
1313
import type {TraceItemDataset} from 'sentry/views/explore/types';
1414

@@ -79,46 +79,35 @@ function validateAttributesQueryOptions({
7979
});
8080
}
8181

82-
export function useAttributeValidation(itemType: TraceItemDataset): {
82+
export function useAttributeValidation(
83+
itemType: TraceItemDataset,
84+
query: string,
85+
selection: AttributeValidationSelection
86+
): {
8387
invalidFilterKeys: string[];
84-
validateQuery: (
85-
query: string,
86-
selection: AttributeValidationSelection
87-
) => Promise<void>;
8888
} {
89-
const [invalidFilterKeys, setInvalidFilterKeys] = useState<string[]>([]);
90-
const queryClient = useQueryClient();
9189
const organization = useOrganization();
9290

93-
const validateQuery = useCallback(
94-
async (query: string, selection: AttributeValidationSelection) => {
95-
const keys = extractFilterKeys(parseSearch(query));
96-
if (!keys.length) {
97-
setInvalidFilterKeys([]);
98-
return;
99-
}
100-
try {
101-
const options = validateAttributesQueryOptions({
102-
itemType,
103-
filterKeys: keys,
104-
organizationSlug: organization.slug,
105-
...selection,
106-
});
107-
await queryClient.cancelQueries({
108-
queryKey: [options.queryKey[0], {method: 'POST', data: {itemType}}],
109-
});
110-
const [data] = await queryClient.fetchQuery(options);
111-
setInvalidFilterKeys(
112-
Object.entries(data.attributes)
113-
.filter(([, result]) => !result.valid)
114-
.map(([key]) => key)
115-
);
116-
} catch {
117-
// leave previous state on error
118-
}
119-
},
120-
[queryClient, organization.slug, itemType]
91+
const filterKeys = useMemo(() => extractFilterKeys(parseSearch(query)), [query]);
92+
93+
const {data} = useQuery(
94+
validateAttributesQueryOptions({
95+
itemType,
96+
filterKeys,
97+
organizationSlug: organization.slug,
98+
...selection,
99+
})
121100
);
122101

123-
return {invalidFilterKeys, validateQuery};
102+
const invalidFilterKeys = useMemo(() => {
103+
if (!data) {
104+
return EMPTY_KEYS;
105+
}
106+
107+
return Object.entries(data?.[0]?.attributes)
108+
.filter(([, result]) => !result.valid)
109+
.map(([key]) => key);
110+
}, [data]);
111+
112+
return {invalidFilterKeys};
124113
}

0 commit comments

Comments
 (0)