diff --git a/src/sentry/integrations/slack/unfurl/discover.py b/src/sentry/integrations/slack/unfurl/discover.py index 6f9da5a7a881a4..5a22df69ceacae 100644 --- a/src/sentry/integrations/slack/unfurl/discover.py +++ b/src/sentry/integrations/slack/unfurl/discover.py @@ -10,6 +10,7 @@ from sentry.api import client from sentry.charts import generate_chart from sentry.charts.types import ChartType +from sentry.discover.arithmetic import is_equation from sentry.integrations.slack.message_builder.discover import build_discover_attachment from sentry.models import ApiKey, Integration from sentry.models.user import User @@ -31,6 +32,9 @@ TOP_N = 5 MAX_PERIOD_DAYS_INCLUDE_PREVIOUS = 45 DEFAULT_PERIOD = "14d" +DEFAULT_AXIS_OPTION = "count()" +AGGREGATE_PATTERN = r"^(\w+)\((.*)?\)$" +AGGREGATE_BASE = r"(\w+)\((.*)?\)" def get_double_period(period: str) -> str: @@ -44,6 +48,18 @@ def get_double_period(period: str) -> str: return f"{value * 2}{unit}" +def is_aggregate(field: str) -> bool: + field_match = re.match(AGGREGATE_PATTERN, field) + if field_match: + return True + + equation_match = re.match(AGGREGATE_BASE, field) and is_equation(field) + if equation_match: + return True + + return False + + def unfurl_discover( data: HttpRequest, integration: Integration, @@ -90,8 +106,13 @@ def unfurl_discover( or (to_list(saved_query.get("orderby")) if saved_query.get("orderby") else []), ) params.setlist("name", params.getlist("name") or to_list(saved_query.get("name"))) + + fields = params.getlist("field") or to_list(saved_query.get("fields")) + # Mimic Discover to pick the first aggregate as the yAxis option if + # one isn't specified. + axis_options = [field for field in fields if is_aggregate(field)] + [DEFAULT_AXIS_OPTION] params.setlist( - "yAxis", params.getlist("yAxis") or to_list(saved_query.get("yAxis", "count()")) + "yAxis", params.getlist("yAxis") or to_list(saved_query.get("yAxis", axis_options[0])) ) params.setlist("field", params.getlist("field") or to_list(saved_query.get("fields"))) diff --git a/tests/sentry/integrations/slack/test_unfurl.py b/tests/sentry/integrations/slack/test_unfurl.py index 7f37be5518cf88..298d704aa638e1 100644 --- a/tests/sentry/integrations/slack/test_unfurl.py +++ b/tests/sentry/integrations/slack/test_unfurl.py @@ -300,6 +300,66 @@ def test_unfurl_discover_short_url(self, mock_generate_chart): first_key = list(chart_data["stats"].keys())[0] assert len(chart_data["stats"][first_key]["data"]) == 288 + @patch("sentry.integrations.slack.unfurl.discover.generate_chart", return_value="chart-url") + def test_unfurl_correct_y_axis_for_saved_query(self, mock_generate_chart): + query = { + "fields": [ + "message", + "event.type", + "project", + "user.display", + "p50(transaction.duration)", + ], + } + saved_query = DiscoverSavedQuery.objects.create( + organization=self.organization, + created_by=self.user, + name="Test query", + query=query, + version=2, + ) + saved_query.set_projects([self.project.id]) + + min_ago = iso_format(before_now(minutes=1)) + self.store_event( + data={"message": "first", "fingerprint": ["group2"], "timestamp": min_ago}, + project_id=self.project.id, + ) + self.store_event( + data={"message": "second", "fingerprint": ["group2"], "timestamp": min_ago}, + project_id=self.project.id, + ) + + url = f"https://sentry.io/organizations/{self.organization.slug}/discover/results/?id={saved_query.id}&statsPeriod=24h&project={self.project.id}" + link_type, args = match_link(url) + + if not args or not link_type: + raise Exception("Missing link_type/args") + + links = [ + UnfurlableUrl(url=url, args=args), + ] + + with self.feature( + [ + "organizations:discover", + "organizations:discover-basic", + "organizations:chart-unfurls", + "organizations:discover-top-events", + ] + ): + unfurls = link_handlers[link_type].fn(self.request, self.integration, links, self.user) + + assert unfurls[url] == build_discover_attachment( + title=args["query"].get("name"), chart_url="chart-url" + ) + assert len(mock_generate_chart.mock_calls) == 1 + + assert mock_generate_chart.call_args[0][0] == ChartType.SLACK_DISCOVER_TOTAL_PERIOD + chart_data = mock_generate_chart.call_args[0][1] + assert chart_data["seriesName"] == "p50(transaction.duration)" + assert len(chart_data["stats"]["data"]) == 288 + @patch("sentry.integrations.slack.unfurl.discover.generate_chart", return_value="chart-url") def test_top_events_url_param(self, mock_generate_chart): min_ago = iso_format(before_now(minutes=1))