Skip to content

Commit 209d23c

Browse files
DominikB2014claude
andcommitted
fix(explore): Pass sort param for group-by explore Slack unfurls
When an Explore link with groupBy is unfurled in Slack, the unfurl sets topEvents=5 but omits the sort parameter. Without it, the timeseries endpoint returns whichever groups it defaults to rather than the top groups matching what Explore shows. Now copies the aggregateSort URL param (Explore's name) as sort (the API name), and defaults to descending by the charted yAxis when no explicit sort is present — matching the frontend's behavior. Fixes DAIN-1490 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 8eb4dae commit 209d23c

File tree

2 files changed

+41
-0
lines changed

2 files changed

+41
-0
lines changed

src/sentry/integrations/slack/unfurl/explore.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,10 @@ def _unfurl_explore(
127127
style = ChartType.SLACK_EXPLORE_LINE
128128
if group_bys:
129129
params.setlist("topEvents", [str(TOP_N)])
130+
if not params.getlist("sort"):
131+
# Default to descending by the first yAxis, matching Explore's
132+
# defaultAggregateSortBys behavior
133+
params.setlist("sort", [f"-{y_axes[0]}"])
130134

131135
if not params.get("statsPeriod") and not params.get("start"):
132136
params["statsPeriod"] = DEFAULT_PERIOD
@@ -227,6 +231,12 @@ def map_explore_query_args(url: str, args: Mapping[str, str | None]) -> Mapping[
227231
if values:
228232
query.setlist(param, values)
229233

234+
# Explore stores the aggregate sort as "aggregateSort" in the URL;
235+
# the events-timeseries endpoint expects it as "sort".
236+
aggregate_sort = raw_query.getlist("aggregateSort")
237+
if aggregate_sort:
238+
query.setlist("sort", aggregate_sort)
239+
230240
return dict(**args, query=query, chart_type=chart_type, dataset=explore_dataset)
231241

232242

tests/sentry/integrations/slack/test_unfurl.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1576,6 +1576,37 @@ def test_unfurl_explore_with_groupby(
15761576
assert len(mock_generate_chart.mock_calls) == 1
15771577
assert mock_generate_chart.call_args[0][0] == ChartType.SLACK_EXPLORE_LINE
15781578

1579+
# Verify sort is passed to the timeseries API for correct top events
1580+
api_params = mock_client_get.call_args[1]["params"]
1581+
assert api_params.getlist("sort") == ["-avg(span.duration)"]
1582+
1583+
@patch(
1584+
"sentry.integrations.slack.unfurl.explore.client.get",
1585+
)
1586+
@patch("sentry.charts.backend.generate_chart", return_value="chart-url")
1587+
def test_unfurl_explore_with_groupby_explicit_sort(
1588+
self, mock_generate_chart: MagicMock, mock_client_get: MagicMock
1589+
) -> None:
1590+
mock_client_get.return_value = MagicMock(data=self._build_mock_timeseries_response())
1591+
url = f"https://sentry.io/organizations/{self.organization.slug}/explore/traces/?aggregateField=%7B%22groupBy%22%3A%22span.op%22%7D&aggregateField=%7B%22yAxes%22%3A%5B%22avg(span.duration)%22%5D%7D&aggregateSort=span.op&project={self.project.id}&statsPeriod=24h"
1592+
link_type, args = match_link(url)
1593+
1594+
if not args or not link_type:
1595+
raise AssertionError("Missing link_type/args")
1596+
1597+
links = [
1598+
UnfurlableUrl(url=url, args=args),
1599+
]
1600+
1601+
with self.feature(["organizations:data-browsing-widget-unfurl"]):
1602+
unfurls = link_handlers[link_type].fn(self.integration, links, self.user)
1603+
1604+
assert len(unfurls) == 1
1605+
1606+
# Verify explicit aggregateSort from the URL is used instead of the default
1607+
api_params = mock_client_get.call_args[1]["params"]
1608+
assert api_params.getlist("sort") == ["span.op"]
1609+
15791610
@patch(
15801611
"sentry.integrations.slack.unfurl.explore.client.get",
15811612
)

0 commit comments

Comments
 (0)