diff --git a/src/sentry/api/serializers/rest_framework/dashboard.py b/src/sentry/api/serializers/rest_framework/dashboard.py index db6930c4c259e1..874c9686f8faf9 100644 --- a/src/sentry/api/serializers/rest_framework/dashboard.py +++ b/src/sentry/api/serializers/rest_framework/dashboard.py @@ -105,6 +105,77 @@ def is_table_display_type(display_type): MAX_WIDGET_COLS = 6 +_DEFAULT_CHART_AND_TABLE_TYPES: frozenset[int] = frozenset( + { + DashboardWidgetDisplayTypes.LINE_CHART, + DashboardWidgetDisplayTypes.AREA_CHART, + DashboardWidgetDisplayTypes.BAR_CHART, + DashboardWidgetDisplayTypes.TABLE, + DashboardWidgetDisplayTypes.BIG_NUMBER, + DashboardWidgetDisplayTypes.CATEGORICAL_BAR_CHART, + } +) + + +class DatasetConfig(TypedDict): + supported_display_types: frozenset[int] + + +# Per-dataset config mirroring the frontend dataset configs +# (``static/app/views/dashboards/datasetConfig/*.tsx``). A display type is +# allowed for a widget_type iff it appears in ``supported_display_types`` here. +DATASET_CONFIG: dict[int, DatasetConfig] = { + DashboardWidgetTypes.DISCOVER: {"supported_display_types": _DEFAULT_CHART_AND_TABLE_TYPES}, + # ERROR_EVENTS is intentionally omitted: it's the ``create_widget`` default + # when a request omits widget_type, so any system display type a prebuilt + # config doesn't tag will land here. Without a config entry the validation + # falls through and lets the request pass. + DashboardWidgetTypes.TRANSACTION_LIKE: { + "supported_display_types": _DEFAULT_CHART_AND_TABLE_TYPES + }, + DashboardWidgetTypes.RELEASE_HEALTH: { + "supported_display_types": _DEFAULT_CHART_AND_TABLE_TYPES + }, + DashboardWidgetTypes.METRICS: {"supported_display_types": _DEFAULT_CHART_AND_TABLE_TYPES}, + DashboardWidgetTypes.LOGS: {"supported_display_types": _DEFAULT_CHART_AND_TABLE_TYPES}, + DashboardWidgetTypes.ISSUE: { + "supported_display_types": frozenset( + { + DashboardWidgetDisplayTypes.TABLE, + DashboardWidgetDisplayTypes.LINE_CHART, + DashboardWidgetDisplayTypes.AREA_CHART, + DashboardWidgetDisplayTypes.BAR_CHART, + } + ) + }, + DashboardWidgetTypes.SPANS: { + "supported_display_types": _DEFAULT_CHART_AND_TABLE_TYPES + | frozenset( + { + DashboardWidgetDisplayTypes.DETAILS, + DashboardWidgetDisplayTypes.SERVER_TREE, + # WHEEL is used by built-in performance-score widgets. + DashboardWidgetDisplayTypes.WHEEL, + } + ) + }, + DashboardWidgetTypes.TRACEMETRICS: { + "supported_display_types": frozenset( + { + DashboardWidgetDisplayTypes.LINE_CHART, + DashboardWidgetDisplayTypes.AREA_CHART, + DashboardWidgetDisplayTypes.BAR_CHART, + DashboardWidgetDisplayTypes.BIG_NUMBER, + DashboardWidgetDisplayTypes.CATEGORICAL_BAR_CHART, + } + ) + }, + DashboardWidgetTypes.PREPROD_APP_SIZE: { + "supported_display_types": frozenset({DashboardWidgetDisplayTypes.LINE_CHART}) + }, +} + + class WidgetLayoutSerializer(CamelSnakeSerializer[Dashboard]): """Widget grid layout position and dimensions. @@ -326,7 +397,24 @@ class DashboardWidgetSerializer(CamelSnakeSerializer[Dashboard]): ) def validate_display_type(self, display_type): - return DashboardWidgetDisplayTypes.get_id_for_type_name(display_type) + display_type_id = DashboardWidgetDisplayTypes.get_id_for_type_name(display_type) + + widget_type_name = self.context.get("widget_type") + if widget_type_name is not None and display_type_id is not None: + widget_type_id = DashboardWidgetTypes.get_id_for_type_name(widget_type_name) + config = DATASET_CONFIG.get(widget_type_id) + if config is not None and display_type_id not in config["supported_display_types"]: + supported_names = sorted( + DashboardWidgetDisplayTypes.get_type_name(d) or str(d) + for d in config["supported_display_types"] + ) + raise serializers.ValidationError( + f"Display type '{display_type}' is not supported for the " + f"'{widget_type_name}' dataset. Supported display types: " + f"{', '.join(supported_names)}." + ) + + return display_type_id def _validate_widget_type(self, data): widget_type = DashboardWidgetTypes.get_id_for_type_name(data.get("widget_type")) @@ -358,6 +446,11 @@ def to_internal_value(self, data): queries_serializer = self.fields["queries"] additional_context = {} + # Always reset; with ``many=True`` DRF reuses one child serializer + # instance across items, so stale values would otherwise leak between + # widgets in the same request. + self.context["widget_type"] = data.get("widget_type") + if data.get("display_type"): additional_context["display_type"] = data.get("display_type") if data.get("widget_type"): diff --git a/tests/sentry/dashboards/endpoints/test_organization_dashboard_widget_details.py b/tests/sentry/dashboards/endpoints/test_organization_dashboard_widget_details.py index 75ec4434eacb62..3a11f45f97f09a 100644 --- a/tests/sentry/dashboards/endpoints/test_organization_dashboard_widget_details.py +++ b/tests/sentry/dashboards/endpoints/test_organization_dashboard_widget_details.py @@ -176,6 +176,30 @@ def test_invalid_display_type(self) -> None: assert response.status_code == 400, response.data assert "displayType" in response.data, response.data + def test_unsupported_display_type_for_widget_type(self) -> None: + data = { + "title": "Table on preprod-app-size", + "displayType": "table", + "widgetType": "preprod-app-size", + "queries": [ + { + "name": "", + "conditions": "", + "fields": ["count()"], + "columns": [], + "aggregates": ["count()"], + } + ], + } + response = self.do_request( + "post", + self.url(), + data=data, + ) + assert response.status_code == 400, response.data + assert "displayType" in response.data, response.data + assert "preprod-app-size" in str(response.data["displayType"]) + def test_invalid_equation(self) -> None: data = { "title": "Invalid query", @@ -1442,7 +1466,7 @@ def test_widget_type_tracemetrics(self) -> None: data = { "title": "Test Metrics Query", "widgetType": "tracemetrics", - "displayType": "table", + "displayType": "line", "queries": [ { "name": "", @@ -1461,6 +1485,30 @@ def test_widget_type_tracemetrics(self) -> None: ) assert response.status_code == 200, response.data + def test_widget_type_tracemetrics_rejects_table(self) -> None: + data = { + "title": "Test Metrics Query", + "widgetType": "tracemetrics", + "displayType": "table", + "queries": [ + { + "name": "", + "conditions": "metric.name:foo", + "fields": ["sum(value)"], + "columns": [], + "aggregates": ["sum(value)"], + }, + ], + } + + response = self.do_request( + "post", + self.url(), + data=data, + ) + assert response.status_code == 400, response.data + assert "displayType" in response.data, response.data + def test_text_widget_without_feature_flag(self) -> None: data = { "title": "Text Widget Title", diff --git a/tests/sentry/dashboards/endpoints/test_organization_dashboards.py b/tests/sentry/dashboards/endpoints/test_organization_dashboards.py index d5c51ef1449870..459d9096cdd162 100644 --- a/tests/sentry/dashboards/endpoints/test_organization_dashboards.py +++ b/tests/sentry/dashboards/endpoints/test_organization_dashboards.py @@ -2169,6 +2169,92 @@ def test_post_with_text_widget(self) -> None: assert DashboardWidgetQuery.objects.filter(widget=text_widget).count() == 0 + def test_agents_traces_table_dashboard_save_and_update(self) -> None: + # Regression: the AI Agents Overview prebuilt config has an + # agents_traces_table widget without a widget_type. The backend defaults + # it to error-events on create. On the next PUT the frontend round-trips + # widget_type=error-events, which would otherwise fail validation. + data = { + "title": "AI Agents Overview", + "widgets": [ + { + "title": "Traces", + "displayType": "agents_traces_table", + "queries": [ + { + "name": "", + "fields": [], + "columns": [], + "aggregates": [], + "conditions": "", + } + ], + }, + ], + } + create = self.do_request("post", self.url, data=data) + assert create.status_code == 201, create.data + dashboard_id = create.data["id"] + widget_id = create.data["widgets"][0]["id"] + widget_type = create.data["widgets"][0].get("widgetType") + + put_url = f"/api/0/organizations/{self.organization.slug}/dashboards/{dashboard_id}/" + put_data = { + "title": "AI Agents Overview", + "widgets": [ + { + "id": widget_id, + "title": "Traces", + "displayType": "agents_traces_table", + "widgetType": widget_type, + "queries": [ + { + "name": "", + "fields": [], + "columns": [], + "aggregates": [], + "conditions": "", + } + ], + }, + ], + } + update = self.do_request("put", put_url, data=put_data) + assert update.status_code == 200, update.data + + def test_post_text_widget_after_restrictive_dataset_widget(self) -> None: + # Regression: DRF reuses a single child serializer for ``many=True``, + # so a previous widget's widget_type can leak via serializer context + # and incorrectly fail validation for a later TEXT widget. + with self.feature("organizations:dashboards-text-widgets"): + data = { + "title": "Dashboard from Post", + "widgets": [ + { + "title": "Mobile Size", + "displayType": "line", + "widgetType": "preprod-app-size", + "interval": "5m", + "queries": [ + { + "name": "", + "fields": ["count()"], + "columns": [], + "aggregates": ["count()"], + "conditions": "", + } + ], + }, + { + "title": "Text Widget", + "displayType": "text", + "description": "Notes", + }, + ], + } + response = self.do_request("post", self.url, data=data) + assert response.status_code == 201, response.data + def test_post_with_text_widget_without_feature_flag(self) -> None: data = { "title": "Dashboard from Post",