Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
92 changes: 91 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,74 @@ 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},
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,
DashboardWidgetDisplayTypes.SERVER_TREE,
# 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 +394,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
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 @@ -358,6 +443,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"):
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
Original file line number Diff line number Diff line change
Expand Up @@ -2169,6 +2169,39 @@ def test_post_with_text_widget(self) -> None:

assert DashboardWidgetQuery.objects.filter(widget=text_widget).count() == 0

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",
Expand Down
Loading