Skip to content

Commit 382209c

Browse files
authored
fix(dashboard): OnDemand widget creation also includes transaction type (#79059)
Ensures that the proper on-demand objects are created when widgets are updated and created. Since the discover split can create transaction types, we need to catch these types when creating the corresponding on demand objects. I've colocated the ondemand tests into one test class and parameterized the widget type so we can test the functionality against transaction widgets as well.
1 parent cbf2227 commit 382209c

File tree

2 files changed

+54
-3
lines changed

2 files changed

+54
-3
lines changed

Diff for: src/sentry/api/serializers/rest_framework/dashboard.py

+10-3
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,8 @@ def validate(self, data):
343343

344344
if (
345345
ondemand_feature
346-
and data.get("widget_type") == DashboardWidgetTypes.DISCOVER
346+
and data.get("widget_type")
347+
in [DashboardWidgetTypes.DISCOVER, DashboardWidgetTypes.TRANSACTION_LIKE]
347348
and not query.get("on_demand_extraction_disabled", False)
348349
):
349350
if query.get("columns"):
@@ -654,7 +655,10 @@ def create_widget(self, dashboard, widget_data, order):
654655

655656
DashboardWidgetQuery.objects.bulk_create(new_queries)
656657

657-
if widget.widget_type == DashboardWidgetTypes.DISCOVER:
658+
if widget.widget_type in [
659+
DashboardWidgetTypes.DISCOVER,
660+
DashboardWidgetTypes.TRANSACTION_LIKE,
661+
]:
658662
self._check_query_cardinality(new_queries)
659663

660664
def _check_query_cardinality(self, new_queries: Sequence[DashboardWidgetQuery]):
@@ -734,7 +738,10 @@ def update_widget_queries(self, widget, data):
734738
raise serializers.ValidationError("You cannot use a query not owned by this widget")
735739
DashboardWidgetQuery.objects.bulk_create(new_queries)
736740

737-
if widget.widget_type == DashboardWidgetTypes.DISCOVER:
741+
if widget.widget_type in [
742+
DashboardWidgetTypes.DISCOVER,
743+
DashboardWidgetTypes.TRANSACTION_LIKE,
744+
]:
738745
self._check_query_cardinality(new_queries + update_queries)
739746

740747
def update_widget_query(self, query, data, order):

Diff for: tests/sentry/api/endpoints/test_organization_dashboard_details.py

+44
Original file line numberDiff line numberDiff line change
@@ -1769,6 +1769,33 @@ def test_update_dashboard_with_widget_filter_requiring_environment(self):
17691769
)
17701770
assert response.status_code == 200, response.data
17711771

1772+
1773+
class OrganizationDashboardDetailsOnDemandTest(OrganizationDashboardDetailsTestCase):
1774+
widget_type = DashboardWidgetTypes.DISCOVER
1775+
1776+
def setUp(self):
1777+
super().setUp()
1778+
self.project = self.create_project()
1779+
self.create_user_member_role()
1780+
self.widget_3 = DashboardWidget.objects.create(
1781+
dashboard=self.dashboard,
1782+
order=2,
1783+
title="Widget 3",
1784+
display_type=DashboardWidgetDisplayTypes.LINE_CHART,
1785+
widget_type=self.widget_type,
1786+
)
1787+
self.widget_4 = DashboardWidget.objects.create(
1788+
dashboard=self.dashboard,
1789+
order=3,
1790+
title="Widget 4",
1791+
display_type=DashboardWidgetDisplayTypes.LINE_CHART,
1792+
widget_type=self.widget_type,
1793+
)
1794+
self.widget_ids = [self.widget_1.id, self.widget_2.id, self.widget_3.id, self.widget_4.id]
1795+
1796+
def get_widget_queries(self, widget):
1797+
return DashboardWidgetQuery.objects.filter(widget=widget).order_by("order")
1798+
17721799
def test_ondemand_without_flags(self):
17731800
data: dict[str, Any] = {
17741801
"title": "First dashboard",
@@ -1777,6 +1804,7 @@ def test_ondemand_without_flags(self):
17771804
"title": "Errors per project",
17781805
"displayType": "table",
17791806
"interval": "5m",
1807+
"widgetType": DashboardWidgetTypes.get_type_name(self.widget_type),
17801808
"queries": [
17811809
{
17821810
"name": "Errors",
@@ -1813,6 +1841,7 @@ def test_ondemand_with_unapplicable_query(self):
18131841
"title": "Errors per project",
18141842
"displayType": "table",
18151843
"interval": "5m",
1844+
"widgetType": DashboardWidgetTypes.get_type_name(self.widget_type),
18161845
"queries": [
18171846
{
18181847
"name": "Errors",
@@ -1850,6 +1879,7 @@ def test_ondemand_with_flags(self):
18501879
"title": "Errors per project",
18511880
"displayType": "table",
18521881
"interval": "5m",
1882+
"widgetType": DashboardWidgetTypes.get_type_name(self.widget_type),
18531883
"queries": [
18541884
{
18551885
"name": "Errors",
@@ -1888,6 +1918,7 @@ def test_ondemand_hits_spec_limit(self, mock_max):
18881918
"title": "Errors per project",
18891919
"displayType": "table",
18901920
"interval": "5m",
1921+
"widgetType": DashboardWidgetTypes.get_type_name(self.widget_type),
18911922
"queries": [
18921923
{
18931924
"name": "Errors",
@@ -1932,6 +1963,7 @@ def test_ondemand_hits_card_limit(self, mock_query):
19321963
"title": "errors per project",
19331964
"displayType": "table",
19341965
"interval": "5m",
1966+
"widgetType": DashboardWidgetTypes.get_type_name(self.widget_type),
19351967
"queries": [
19361968
{
19371969
"name": "errors",
@@ -1973,6 +2005,7 @@ def test_ondemand_updates_existing_widget(self, mock_query):
19732005
"title": "errors per project",
19742006
"displayType": "table",
19752007
"interval": "5m",
2008+
"widgetType": DashboardWidgetTypes.get_type_name(self.widget_type),
19762009
"queries": [
19772010
{
19782011
"name": "errors",
@@ -2009,6 +2042,7 @@ def test_ondemand_updates_existing_widget(self, mock_query):
20092042
"title": "errors per project",
20102043
"displayType": "table",
20112044
"interval": "5m",
2045+
"widgetType": DashboardWidgetTypes.get_type_name(self.widget_type),
20122046
"queries": [
20132047
{
20142048
"id": str(queries[0].id),
@@ -2055,6 +2089,7 @@ def test_ondemand_updates_new_widget(self, mock_query):
20552089
"title": "errors per project",
20562090
"displayType": "table",
20572091
"interval": "5m",
2092+
"widgetType": DashboardWidgetTypes.get_type_name(self.widget_type),
20582093
"queries": [
20592094
{
20602095
"name": "errors",
@@ -2091,6 +2126,7 @@ def test_ondemand_updates_new_widget(self, mock_query):
20912126
"title": "errors per project",
20922127
"displayType": "table",
20932128
"interval": "5m",
2129+
"widgetType": DashboardWidgetTypes.get_type_name(self.widget_type),
20942130
"queries": [
20952131
{
20962132
# without id here we'll make a new query and delete the old one
@@ -2137,6 +2173,7 @@ def test_cardinality_precedence_over_feature_checks(self, mock_query):
21372173
"title": "errors per project",
21382174
"displayType": "table",
21392175
"interval": "5m",
2176+
"widgetType": DashboardWidgetTypes.get_type_name(self.widget_type),
21402177
"queries": [
21412178
{
21422179
"name": "errors",
@@ -2267,6 +2304,13 @@ def test_add_widget_with_split_widget_type_writes_to_split_decision(self):
22672304
assert widgets[2].discover_widget_split is None
22682305

22692306

2307+
class OrganizationDashboardDetailsOnDemandTransactionLikeTest(
2308+
OrganizationDashboardDetailsOnDemandTest
2309+
):
2310+
# Re-run the on-demand tests with the transaction-like widget type
2311+
widget_type = DashboardWidgetTypes.TRANSACTION_LIKE
2312+
2313+
22702314
class OrganizationDashboardVisitTest(OrganizationDashboardDetailsTestCase):
22712315
def url(self, dashboard_id):
22722316
return reverse(

0 commit comments

Comments
 (0)