Skip to content

Commit e1049c9

Browse files
nsdeschenesClaude Opus 4.6
andcommitted
ref(explore): Accept selection in validateQuery, deduplicate via cache
Move page filter selection out of useAttributeValidation and accept it as a parameter. This gives the hook a stable identity and lets the call site control when re-validation happens. Use staleTime: Infinity on the query options so TanStack Query deduplicates calls with the same filter keys and selection without extra refs. Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
1 parent 3e8d022 commit e1049c9

File tree

4 files changed

+186
-39
lines changed

4 files changed

+186
-39
lines changed

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

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

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

55
import {PageFiltersStore} from 'sentry/components/pageFilters/store';
6+
import {parseSearch} from 'sentry/components/searchSyntax/parser';
67
import {FieldKind} from 'sentry/utils/fields';
78
import {
89
useTraceItemSearchQueryBuilderProps,
@@ -247,4 +248,100 @@ describe('useTraceItemSearchQueryBuilderProps', () => {
247248
'log.flag',
248249
]);
249250
});
251+
252+
it('calls validateQuery when filter keys change', async () => {
253+
const validateMock = MockApiClient.addMockResponse({
254+
url: '/organizations/org-slug/trace-items/attributes/validate/',
255+
method: 'POST',
256+
body: {attributes: {'span.op': {valid: true}}},
257+
});
258+
259+
const {result} = renderHookWithProviders(useTraceItemSearchQueryBuilderProps, {
260+
initialProps: defaultInitialProps,
261+
organization,
262+
});
263+
264+
act(() => {
265+
result.current.onChange?.('span.op:db', {
266+
parsedQuery: parseSearch('span.op:db'),
267+
queryIsValid: true,
268+
});
269+
});
270+
271+
await waitFor(() => {
272+
expect(validateMock).toHaveBeenCalledTimes(1);
273+
});
274+
});
275+
276+
it('does not call validateQuery when only filter values change', async () => {
277+
const validateMock = MockApiClient.addMockResponse({
278+
url: '/organizations/org-slug/trace-items/attributes/validate/',
279+
method: 'POST',
280+
body: {attributes: {'span.op': {valid: true}}},
281+
});
282+
283+
const {result} = renderHookWithProviders(useTraceItemSearchQueryBuilderProps, {
284+
initialProps: defaultInitialProps,
285+
organization,
286+
});
287+
288+
await act(async () => {
289+
result.current.onChange?.('span.op:db', {
290+
parsedQuery: parseSearch('span.op:db'),
291+
queryIsValid: true,
292+
});
293+
});
294+
295+
expect(validateMock).toHaveBeenCalledTimes(1);
296+
297+
await act(async () => {
298+
result.current.onChange?.('span.op:web', {
299+
parsedQuery: parseSearch('span.op:web'),
300+
queryIsValid: true,
301+
});
302+
});
303+
304+
// Still only 1 call — value changed but keys didn't
305+
expect(validateMock).toHaveBeenCalledTimes(1);
306+
});
307+
308+
it('calls validateQuery when a new filter key is added', async () => {
309+
const validateMock = MockApiClient.addMockResponse({
310+
url: '/organizations/org-slug/trace-items/attributes/validate/',
311+
method: 'POST',
312+
body: {
313+
attributes: {
314+
'span.op': {valid: true},
315+
'other.key': {valid: true},
316+
},
317+
},
318+
});
319+
320+
const {result} = renderHookWithProviders(useTraceItemSearchQueryBuilderProps, {
321+
initialProps: defaultInitialProps,
322+
organization,
323+
});
324+
325+
act(() => {
326+
result.current.onChange?.('span.op:db', {
327+
parsedQuery: parseSearch('span.op:db'),
328+
queryIsValid: true,
329+
});
330+
});
331+
332+
await waitFor(() => {
333+
expect(validateMock).toHaveBeenCalledTimes(1);
334+
});
335+
336+
act(() => {
337+
result.current.onChange?.('span.op:db other.key:val', {
338+
parsedQuery: parseSearch('span.op:db other.key:val'),
339+
queryIsValid: true,
340+
});
341+
});
342+
343+
await waitFor(() => {
344+
expect(validateMock).toHaveBeenCalledTimes(2);
345+
});
346+
});
250347
});

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {useCallback, useEffect, useMemo} from 'react';
22

3+
import {usePageFilters} from 'sentry/components/pageFilters/usePageFilters';
34
import type {SpanSearchQueryBuilderProps} from 'sentry/components/performance/spanSearchQueryBuilder';
45
import {
56
SearchQueryBuilder,
@@ -108,20 +109,26 @@ export function useTraceItemSearchQueryBuilderProps({
108109
}: TraceItemSearchQueryBuilderProps) {
109110
const placeholderText = itemTypeToDefaultPlaceholder(itemType);
110111

111-
const {invalidFilterKeys, validateQuery} = useAttributeValidation(itemType, projects);
112+
const {invalidFilterKeys, validateQuery} = useAttributeValidation(itemType);
113+
const {selection} = usePageFilters();
114+
const effectiveProjects = projects ?? selection.projects;
115+
const validationSelection = useMemo(
116+
() => ({datetime: selection.datetime, projects: effectiveProjects}),
117+
[selection.datetime, effectiveProjects]
118+
);
112119

113120
useEffect(() => {
114121
if (initialQuery) {
115-
validateQuery(initialQuery);
122+
validateQuery(initialQuery, validationSelection);
116123
}
117-
}, [initialQuery, validateQuery]);
124+
}, [initialQuery, validateQuery, validationSelection]);
118125

119126
const wrappedOnChange = useCallback(
120127
(query: string, state: CallbackSearchState) => {
121-
validateQuery(query);
128+
validateQuery(query, validationSelection);
122129
onChange?.(query, state);
123130
},
124-
[validateQuery, onChange]
131+
[validateQuery, validationSelection, onChange]
125132
);
126133
const functionTags = useFunctionTags(itemType, supportedAggregates);
127134
const filterKeySections = useFilterKeySections(itemType, stringAttributes);

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

Lines changed: 60 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,19 @@ import {OrganizationFixture} from 'sentry-fixture/organization';
22

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

5-
import {usePageFilters} from 'sentry/components/pageFilters/usePageFilters';
65
import {parseSearch} from 'sentry/components/searchSyntax/parser';
76
import {
87
extractFilterKeys,
98
useAttributeValidation,
9+
type AttributeValidationSelection,
1010
} from 'sentry/views/explore/hooks/useAttributeValidation';
1111
import {TraceItemDataset} from 'sentry/views/explore/types';
1212

13-
jest.mock('sentry/components/pageFilters/usePageFilters');
14-
1513
const ORG_SLUG = 'org-slug';
16-
const DEFAULT_DATETIME = {period: '14d', start: null, end: null, utc: false};
14+
const DEFAULT_SELECTION: AttributeValidationSelection = {
15+
datetime: {period: '14d', start: null, end: null, utc: false},
16+
projects: [],
17+
};
1718

1819
describe('extractFilterKeys', () => {
1920
it('returns empty array for null parsedQuery', () => {
@@ -51,19 +52,6 @@ describe('extractFilterKeys', () => {
5152
describe('useAttributeValidation', () => {
5253
const organization = OrganizationFixture({slug: ORG_SLUG});
5354

54-
beforeEach(() => {
55-
jest.mocked(usePageFilters).mockReturnValue({
56-
isReady: true,
57-
pinnedFilters: new Set(),
58-
shouldPersist: true,
59-
selection: {
60-
datetime: DEFAULT_DATETIME,
61-
environments: [],
62-
projects: [],
63-
},
64-
});
65-
});
66-
6755
it('returns empty invalidFilterKeys initially', () => {
6856
const {result} = renderHookWithProviders(
6957
() => useAttributeValidation(TraceItemDataset.SPANS),
@@ -86,7 +74,7 @@ describe('useAttributeValidation', () => {
8674
);
8775

8876
await act(async () => {
89-
await result.current.validateQuery('');
77+
await result.current.validateQuery('', DEFAULT_SELECTION);
9078
});
9179

9280
expect(mockValidate).not.toHaveBeenCalled();
@@ -111,7 +99,10 @@ describe('useAttributeValidation', () => {
11199
);
112100

113101
await act(async () => {
114-
await result.current.validateQuery('span.op:db unknown_key:value');
102+
await result.current.validateQuery(
103+
'span.op:db unknown_key:value',
104+
DEFAULT_SELECTION
105+
);
115106
});
116107

117108
await waitFor(() => {
@@ -132,11 +123,60 @@ describe('useAttributeValidation', () => {
132123
);
133124

134125
await act(async () => {
135-
await result.current.validateQuery('span.op:db');
126+
await result.current.validateQuery('span.op:db', DEFAULT_SELECTION);
136127
});
137128

138129
await waitFor(() => {
139130
expect(result.current.invalidFilterKeys).toEqual([]);
140131
});
141132
});
133+
134+
it('skips validation when keys and selection are unchanged', async () => {
135+
const mockValidate = MockApiClient.addMockResponse({
136+
url: `/organizations/${ORG_SLUG}/trace-items/attributes/validate/`,
137+
method: 'POST',
138+
body: {attributes: {'span.op': {valid: true}}},
139+
});
140+
141+
const {result} = renderHookWithProviders(
142+
() => useAttributeValidation(TraceItemDataset.SPANS),
143+
{organization}
144+
);
145+
146+
await act(async () => {
147+
await result.current.validateQuery('span.op:db', DEFAULT_SELECTION);
148+
});
149+
150+
await act(async () => {
151+
await result.current.validateQuery('span.op:web', DEFAULT_SELECTION);
152+
});
153+
154+
expect(mockValidate).toHaveBeenCalledTimes(1);
155+
});
156+
157+
it('re-validates when selection changes', async () => {
158+
const mockValidate = MockApiClient.addMockResponse({
159+
url: `/organizations/${ORG_SLUG}/trace-items/attributes/validate/`,
160+
method: 'POST',
161+
body: {attributes: {'span.op': {valid: true}}},
162+
});
163+
164+
const {result} = renderHookWithProviders(
165+
() => useAttributeValidation(TraceItemDataset.SPANS),
166+
{organization}
167+
);
168+
169+
await act(async () => {
170+
await result.current.validateQuery('span.op:db', DEFAULT_SELECTION);
171+
});
172+
173+
await act(async () => {
174+
await result.current.validateQuery('span.op:db', {
175+
datetime: {period: '7d', start: null, end: null, utc: false},
176+
projects: [],
177+
});
178+
});
179+
180+
expect(mockValidate).toHaveBeenCalledTimes(2);
181+
});
142182
});

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

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import {useCallback, useState} from 'react';
22

33
import {normalizeDateTimeParams} from 'sentry/components/pageFilters/parse';
4-
import {usePageFilters} from 'sentry/components/pageFilters/usePageFilters';
54
import type {ParseResult} from 'sentry/components/searchSyntax/parser';
65
import {Token} from 'sentry/components/searchSyntax/parser';
76
import {parseSearch} from 'sentry/components/searchSyntax/parser';
@@ -23,12 +22,15 @@ interface ValidateAttributesResponse {
2322
attributes: Record<string, AttributeValidationResult>;
2423
}
2524

26-
interface ValidateAttributesParams {
25+
export interface AttributeValidationSelection {
2726
datetime: PageFilters['datetime'];
27+
projects?: PageFilters['projects'];
28+
}
29+
30+
interface ValidateAttributesParams extends AttributeValidationSelection {
2831
filterKeys: string[];
2932
itemType: TraceItemDataset;
3033
organizationSlug: string;
31-
projects?: PageFilters['projects'];
3234
}
3335

3436
const EMPTY_KEYS: string[] = [];
@@ -72,22 +74,24 @@ function validateAttributesQueryOptions({
7274
},
7375
] satisfies ApiQueryKey,
7476
queryFn: fetchDataQuery<ValidateAttributesResponse>,
77+
staleTime: Infinity,
7578
enabled: filterKeys.length > 0,
7679
});
7780
}
7881

79-
export function useAttributeValidation(
80-
itemType: TraceItemDataset,
81-
projects?: number[]
82-
): {invalidFilterKeys: string[]; validateQuery: (query: string) => Promise<void>} {
82+
export function useAttributeValidation(itemType: TraceItemDataset): {
83+
invalidFilterKeys: string[];
84+
validateQuery: (
85+
query: string,
86+
selection: AttributeValidationSelection
87+
) => Promise<void>;
88+
} {
8389
const [invalidFilterKeys, setInvalidFilterKeys] = useState<string[]>([]);
8490
const queryClient = useQueryClient();
8591
const organization = useOrganization();
86-
const {selection} = usePageFilters();
87-
const effectiveProjects = projects ?? selection.projects;
8892

8993
const validateQuery = useCallback(
90-
async (query: string) => {
94+
async (query: string, selection: AttributeValidationSelection) => {
9195
const keys = extractFilterKeys(parseSearch(query));
9296
if (!keys.length) {
9397
setInvalidFilterKeys([]);
@@ -98,8 +102,7 @@ export function useAttributeValidation(
98102
itemType,
99103
filterKeys: keys,
100104
organizationSlug: organization.slug,
101-
datetime: selection.datetime,
102-
projects: effectiveProjects,
105+
...selection,
103106
});
104107
await queryClient.cancelQueries({queryKey: [options.queryKey[0]]});
105108
const [data] = await queryClient.fetchQuery(options);
@@ -112,7 +115,7 @@ export function useAttributeValidation(
112115
// leave previous state on error
113116
}
114117
},
115-
[queryClient, organization.slug, itemType, selection.datetime, effectiveProjects]
118+
[queryClient, organization.slug, itemType]
116119
);
117120

118121
return {invalidFilterKeys, validateQuery};

0 commit comments

Comments
 (0)