Skip to content

Commit 091c6bb

Browse files
authored
fix(explore): Remove unnecessary validation on group bys (#78712)
This validation means the group bys are now dependant on the tags endpoint to work. Remove it as it should still work just return an empty value.
1 parent f7b1c56 commit 091c6bb

File tree

6 files changed

+34
-39
lines changed

6 files changed

+34
-39
lines changed

static/app/views/explore/charts/index.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export function ExploreCharts({query}: ExploreChartsProps) {
5757
const [dataset] = useDataset();
5858
const [visualizes, setVisualizes] = useVisualizes();
5959
const [interval, setInterval, intervalOptions] = useChartInterval();
60-
const {groupBys, isLoadingGroupBys} = useGroupBys();
60+
const {groupBys} = useGroupBys();
6161
const [resultMode] = useResultMode();
6262
const topEvents = useTopEvents();
6363

@@ -91,7 +91,6 @@ export function ExploreCharts({query}: ExploreChartsProps) {
9191
search: new MutableSearch(query ?? ''),
9292
yAxis: yAxes,
9393
interval: interval ?? getInterval(pageFilters.selection.datetime, 'metrics'),
94-
enabled: !isLoadingGroupBys,
9594
fields,
9695
orderby,
9796
topEvents,

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ describe('useGroupBys', function () {
2828
],
2929
});
3030

31-
let groupBys, setGroupBys, isLoadingGroupBys;
31+
let groupBys, setGroupBys;
3232

3333
function TestPage() {
34-
({groupBys, setGroupBys, isLoadingGroupBys} = useGroupBys());
34+
({groupBys, setGroupBys} = useGroupBys());
3535
return null;
3636
}
3737

@@ -54,10 +54,8 @@ describe('useGroupBys', function () {
5454
</SpanTagsProvider>
5555
);
5656

57-
expect(isLoadingGroupBys).toBe(true);
5857
await waitFor(() => expect(mockSpanTagsApiCall).toHaveBeenCalledTimes(1));
5958
expect(groupBys).toEqual(['']); // default
60-
expect(isLoadingGroupBys).toBe(false);
6159

6260
act(() => setGroupBys(['foo', 'bar']));
6361
expect(groupBys).toEqual(['foo', 'bar']);
Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,57 +1,39 @@
11
import {useCallback, useMemo} from 'react';
22
import type {Location} from 'history';
33

4-
import type {TagCollection} from 'sentry/types/group';
5-
import type {UseApiQueryResult} from 'sentry/utils/queryClient';
64
import {decodeList} from 'sentry/utils/queryString';
7-
import type RequestError from 'sentry/utils/requestError/requestError';
85
import {useLocation} from 'sentry/utils/useLocation';
96
import {useNavigate} from 'sentry/utils/useNavigate';
107
import type {Field} from 'sentry/views/explore/hooks/useSampleFields';
11-
import type {SpanFieldsResponse} from 'sentry/views/performance/utils/useSpanFieldSupportedTags';
12-
13-
import {useSpanTags} from '../contexts/spanTagsContext';
148

159
interface Options {
1610
location: Location;
1711
navigate: ReturnType<typeof useNavigate>;
18-
tagsResults: Omit<UseApiQueryResult<SpanFieldsResponse, RequestError>, 'data'> & {
19-
data: TagCollection;
20-
};
2112
}
2213

2314
type GroupBysResult = {
2415
groupBys: Field[];
25-
isLoadingGroupBys: boolean;
2616
setGroupBys: (fields: Field[]) => void;
2717
};
2818

2919
export function useGroupBys(): GroupBysResult {
3020
const location = useLocation();
3121
const navigate = useNavigate();
32-
const tagsResults = useSpanTags();
33-
const options = {location, navigate, tagsResults};
22+
const options = {location, navigate};
3423

3524
return useGroupBysImpl(options);
3625
}
3726

38-
function useGroupBysImpl({location, navigate, tagsResults}: Options): GroupBysResult {
39-
const {data: tags, isLoading: isLoadingTags} = tagsResults;
40-
27+
function useGroupBysImpl({location, navigate}: Options): GroupBysResult {
4128
const groupBys = useMemo(() => {
4229
const rawGroupBys = decodeList(location.query.groupBy);
4330

44-
// Filter out groupBys that are not in span field supported tags
45-
const validGroupBys = isLoadingTags
46-
? []
47-
: rawGroupBys.filter(groupBy => groupBy === '' || tags?.hasOwnProperty(groupBy));
48-
49-
if (validGroupBys.length) {
50-
return validGroupBys;
31+
if (rawGroupBys.length) {
32+
return rawGroupBys;
5133
}
5234

5335
return [''];
54-
}, [location.query.groupBy, tags, isLoadingTags]);
36+
}, [location.query.groupBy]);
5537

5638
const setGroupBys = useCallback(
5739
(newGroupBys: Field[]) => {
@@ -69,6 +51,5 @@ function useGroupBysImpl({location, navigate, tagsResults}: Options): GroupBysRe
6951
return {
7052
groupBys,
7153
setGroupBys,
72-
isLoadingGroupBys: isLoadingTags,
7354
};
7455
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ describe('useResultMode', function () {
7676
'span.description',
7777
'span.duration',
7878
'timestamp',
79+
'release',
7980
]);
8081
});
8182
});

static/app/views/explore/tables/aggregatesTable.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export function AggregatesTable({}: AggregatesTableProps) {
4646
const {selection} = usePageFilters();
4747
const topEvents = useTopEvents();
4848
const [dataset] = useDataset();
49-
const {groupBys, isLoadingGroupBys} = useGroupBys();
49+
const {groupBys} = useGroupBys();
5050
const [visualizes] = useVisualizes();
5151
const fields = useMemo(() => {
5252
return [...groupBys, ...visualizes.flatMap(visualize => visualize.yAxes)].filter(
@@ -74,7 +74,6 @@ export function AggregatesTable({}: AggregatesTableProps) {
7474
eventView,
7575
initialData: [],
7676
referrer: 'api.explore.spans-aggregates-table',
77-
enabled: !isLoadingGroupBys,
7877
});
7978

8079
const {tableStyles} = useTableStyles({

static/app/views/explore/toolbar/toolbarGroupBy.tsx

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,34 @@ export function ToolbarGroupBy({disabled}: ToolbarGroupByProps) {
2929
const {groupBys, setGroupBys} = useGroupBys();
3030

3131
const options: SelectOption<Field>[] = useMemo(() => {
32+
// These options aren't known to exist on this project but it was inserted into
33+
// the group bys somehow so it should be a valid options in the group bys.
34+
//
35+
// One place this may come from is when switching projects/environment/date range,
36+
// a tag may disappear based on the selection.
37+
const unknownOptions = groupBys
38+
.filter(groupBy => groupBy && !tags.hasOwnProperty(groupBy))
39+
.map(groupBy => {
40+
return {
41+
label: groupBy,
42+
value: groupBy,
43+
};
44+
});
45+
46+
const knownOptions = Object.keys(tags).map(tagKey => {
47+
return {
48+
label: tagKey,
49+
value: tagKey,
50+
};
51+
});
52+
3253
return [
3354
// hard code in an empty option
3455
{label: t('None'), value: ''},
35-
...Object.keys(tags).map(tagKey => {
36-
return {
37-
label: tagKey,
38-
value: tagKey,
39-
};
40-
}),
56+
...unknownOptions,
57+
...knownOptions,
4158
];
42-
}, [tags]);
59+
}, [groupBys, tags]);
4360

4461
const addGroupBy = useCallback(() => {
4562
setGroupBys([...groupBys, '']);

0 commit comments

Comments
 (0)