fix(discover): put in discover dataset for null widget types during split - #121468
Conversation
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit 835160e. Configure here.
| if dataset_source is not None: | ||
| widget.dataset_source = dataset_source.value | ||
| if widget.widget_type is None: | ||
| widget.widget_type = DashboardWidgetTypes.DISCOVER |
There was a problem hiding this comment.
Null widgets never get backfilled
Medium Severity
The new assignment only runs through migration 0913, which filters for widget_type=DISCOVER and excludes null values. Existing installations have also already applied that migration, so the target widgets never reach this code and remain null.
Reviewed by Cursor Bugbot for commit 835160e. Configure here.
There was a problem hiding this comment.
this is for another job that we're going to have
| error_widget = DashboardWidget.objects.create( | ||
| dashboard=self.dashboard, | ||
| order=0, | ||
| title="error widget", | ||
| display_type=DashboardWidgetDisplayTypes.LINE_CHART, | ||
| interval="1d", | ||
| detail={"layout": {"x": 0, "y": 0, "w": 1, "h": 1, "minH": 2}}, | ||
| ) |
There was a problem hiding this comment.
nit: I know this implicitly leaves widget_type=None, but maybe we can explicitly set it in this test since it's testing for that case?


when setting all of the split decisions, we want to also set the widget type if there isn't one. This won't affect text widgets because the script queries for widget queries! Text widgets don't have widget queries so those won't be in the query. For null widget types we'll set it to Discover so that it's not confusing in redash and elsewhere.