feat(dashboards): backend validation for text widget

[nikkikapadia]

Text widgets have special cases for backend validation. They don't have queries, widget_type or dataset, intervals and most fields. All they have is title and description. Hence some of these fields should be able to handle null values if its a Text widget type.

[nikkikapadia]

@cursor review

[gggritso]

LGTM overall, but I have, I think the same question as Cursor, which is about adding the _ before validate, like _validate_interval and _validate_widget_type. Doesn't that unhook them from actually validating the field? I'm not super familiar with DRF though, so let me know if that's not right

The only other thing that sicks out is how many conditionals there are for text widgets. They're pretty different from other types, does DRF have any support for inheritance, and applying a different kind of validator to some objects? Maybe worth trying! If not, maybe adding some early returns for the Text branches would be cleaner than adding the widget display type as a conditional in all the places

[nikkikapadia]

Doesn't that unhook them from actually validating the field? I'm not super familiar with DRF though, so let me know if that's not right

@gggritso ya you're right it unhooks them but i've called those functions in the validate function so it will validate them there instead of before hand. In other words they're still being validated, it's just being done slightly differently.

The only other thing that sicks out is how many conditionals there are for text widgets. They're pretty different from other types, does DRF have any support for inheritance, and applying a different kind of validator to some objects? Maybe worth trying! If not, maybe adding some early returns for the Text branches would be cleaner than adding the widget display type as a conditional in all the places

good point! i'll look into rearranging this a little

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

[cursor]

Null widget_type triggers misleading discover monitoring alert

Low Severity

Adding allow_null=True to the widget_type ChoiceField makes a previously-unreachable code path in _validate_widget_type reachable. When a non-text widget sends widget_type: null, get_id_for_type_name(None) returns None, triggering the widget_type is None branch. This fires sentry_sdk.capture_message("Created or updated widget with discover dataset.") and returns an error about "discover" being deprecated — both factually incorrect for a null input. The request is still correctly rejected, but the Sentry signal is a false positive that could pollute monitoring.

Additional Locations (1)

• src/sentry/api/serializers/rest_framework/dashboard.py#L305-L307

 

[nikkikapadia]

nope this is expected behaviour. No widget_type for non-text widgets will default to using the discover dataset.

[gggritso]

dkjhskjd