Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 87 additions & 1 deletion src/sentry/api/serializers/rest_framework/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,73 @@
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},
DashboardWidgetTypes.ERROR_EVENTS: {"supported_display_types": _DEFAULT_CHART_AND_TABLE_TYPES},
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},
Comment thread
DominikB2014 marked this conversation as resolved.
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,
# WHEEL is used by built-in performance-score widgets.
DashboardWidgetDisplayTypes.WHEEL,
}
)
},
Comment thread
cursor[bot] marked this conversation as resolved.
Comment thread
sentry[bot] marked this conversation as resolved.
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})
},
}
Comment thread
sentry[bot] marked this conversation as resolved.


class WidgetLayoutSerializer(CamelSnakeSerializer[Dashboard]):
"""Widget grid layout position and dimensions.

Expand Down Expand Up @@ -326,7 +393,24 @@
)

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 widget_type_id is not None else None
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
Comment thread
DominikB2014 marked this conversation as resolved.

def _validate_widget_type(self, data):
widget_type = DashboardWidgetTypes.get_id_for_type_name(data.get("widget_type"))
Expand Down Expand Up @@ -360,9 +444,11 @@

if data.get("display_type"):
additional_context["display_type"] = data.get("display_type")
if data.get("widget_type"):
additional_context["widget_type"] = data.get("widget_type")
# Also expose to this serializer's field-level validators.
self.context["widget_type"] = data.get("widget_type")
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
if self.context.get("request") and self.context["request"].user:

Check warning on line 451 in src/sentry/api/serializers/rest_framework/dashboard.py

View check run for this annotation

@sentry/warden / warden: sentry-backend-bugs

Stale `widget_type` in shared serializer context causes incorrect display-type validation for subsequent widgets

When a dashboard update includes multiple widgets, the second (and later) widget without a `widget_type` key in its payload inherits the `widget_type` set by the previous widget, causing `validate_display_type` to validate against the wrong dataset and either wrongly reject valid combinations or (if the check is skipped) silently pass invalid ones.
additional_context["user"] = self.context["request"].user

queries_serializer.context.update(additional_context)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -1442,7 +1466,7 @@ def test_widget_type_tracemetrics(self) -> None:
data = {
"title": "Test Metrics Query",
"widgetType": "tracemetrics",
"displayType": "table",
"displayType": "line",
"queries": [
{
"name": "",
Expand All @@ -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",
Expand Down
Loading