Skip to content

Commit 6903a98

Browse files
authoredMar 17, 2025··
fix(dashboards-eap): Sort by arbitrary aggregate should render chart (#87186)
Navigating to explore with a widget that has an arbitrary aggregate (i.e. not included in the fields or the aggregates) should add another visualize chart so we have access to the arbitrary aggregate for sorting I was trying to add this to where we process `eventViewFromWidget` but because it assumes an aggregate alias, it was more complex than necessary to properly expose it as a sort when we can just pick it off the widget query like this. We can't convert back from an aggregate alias to its original form anyways, so I'd have to pick the orderby off of the widget to get the right value for explore.
1 parent ef67ba7 commit 6903a98

File tree

2 files changed

+48
-9
lines changed

2 files changed

+48
-9
lines changed
 

‎static/app/views/dashboards/utils/getWidgetExploreUrl.spec.tsx

+24
Original file line numberDiff line numberDiff line change
@@ -79,4 +79,28 @@ describe('getWidgetExploreUrl', () => {
7979
'/organizations/org-slug/traces/?dataset=spansRpc&field=avg%28span.duration%29&groupBy=&interval=30m&mode=aggregate&statsPeriod=14d&visualize=%7B%22chartType%22%3A2%2C%22yAxes%22%3A%5B%22avg%28span.duration%29%22%5D%7D'
8080
);
8181
});
82+
83+
it('returns the correct URL for chart widgets where the sort is not in the yAxes', () => {
84+
// This is a widget plotting the avg(span.duration) for the most frequented spans
85+
const widget = WidgetFixture({
86+
displayType: DisplayType.LINE,
87+
queries: [
88+
{
89+
fields: [],
90+
aggregates: ['avg(span.duration)'],
91+
columns: ['span.description'],
92+
conditions: '',
93+
orderby: '-count(span.duration)',
94+
name: '',
95+
},
96+
],
97+
});
98+
99+
const url = getWidgetExploreUrl(widget, selection, organization);
100+
101+
// The URL should have the sort and another visualize to plot the sort
102+
expect(url).toBe(
103+
'/organizations/org-slug/traces/?dataset=spansRpc&field=span.description&field=avg%28span.duration%29&groupBy=span.description&interval=30m&mode=aggregate&sort=-count%28span.duration%29&statsPeriod=14d&visualize=%7B%22chartType%22%3A1%2C%22yAxes%22%3A%5B%22avg%28span.duration%29%22%5D%7D&visualize=%7B%22chartType%22%3A1%2C%22yAxes%22%3A%5B%22count%28span.duration%29%22%5D%7D'
104+
);
105+
});
82106
});

‎static/app/views/dashboards/utils/getWidgetExploreUrl.tsx

+24-9
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
import trimStart from 'lodash/trimStart';
2+
13
import type {PageFilters} from 'sentry/types/core';
24
import type {Organization} from 'sentry/types/organization';
35
import {defined} from 'sentry/utils';
6+
import toArray from 'sentry/utils/array/toArray';
47
import {
58
getAggregateAlias,
69
isAggregateFieldOrEquation,
@@ -28,6 +31,11 @@ export function getWidgetExploreUrl(
2831
undefined
2932
);
3033

34+
// Inject the sort field for cases of arbitrary sorting
35+
// The sort is not picked up by eventView unless it is
36+
// injected into the fields
37+
locationQueryParams.sort = widget.queries[0]!.orderby;
38+
3139
// Pull a max of 3 valid Y-Axis from the widget
3240
const yAxisOptions = eventView.getYAxisOptions().map(({value}) => value);
3341
locationQueryParams.yAxes = [
@@ -109,14 +117,23 @@ export function getWidgetExploreUrl(
109117
chartType,
110118
yAxes: locationQueryParams.yAxes,
111119
},
120+
// Explore widgets do not allow sorting by arbitrary aggregates
121+
// so dashboard widgets need to inject another visualize to plot the sort
122+
// and it available for sorting the main chart
123+
...(locationQueryParams.sort &&
124+
!_isSortIncluded(locationQueryParams.sort, locationQueryParams.yAxes)
125+
? [
126+
{
127+
chartType,
128+
yAxes: toArray(trimStart(locationQueryParams.sort, '-')),
129+
},
130+
]
131+
: []),
112132
],
113133
groupBy,
114134
field: decodeList(locationQueryParams.field),
115135
query: decodeScalar(locationQueryParams.query),
116-
sort:
117-
defined(fields) && defined(locationQueryParams.sort)
118-
? _getSort(fields, locationQueryParams.sort as string)
119-
: undefined,
136+
sort: locationQueryParams.sort || undefined,
120137
interval:
121138
decodeScalar(locationQueryParams.interval) ??
122139
getWidgetInterval(widget.displayType, selection.datetime),
@@ -125,9 +142,7 @@ export function getWidgetExploreUrl(
125142
return getExploreUrl(queryParams);
126143
}
127144

128-
function _getSort(fields: string[], sort: string) {
129-
const descending = sort.startsWith('-');
130-
const rawSort = descending ? sort.slice(1) : sort;
131-
const sortedField = fields?.find(field => getAggregateAlias(field) === rawSort);
132-
return descending ? `-${sortedField}` : sortedField;
145+
function _isSortIncluded(sort: string, yAxes: string[]) {
146+
const rawSort = trimStart(sort, '-');
147+
return yAxes.map(getAggregateAlias).includes(getAggregateAlias(rawSort));
133148
}

0 commit comments

Comments
 (0)
Please sign in to comment.