Skip to content

fix(discover): put in discover dataset for null widget types during split - #121468

Merged
nikkikapadia merged 3 commits into
masterfrom
nikki/fix/save-replace-null-dataset-discover
Aug 7, 2026
Merged

fix(discover): put in discover dataset for null widget types during split#121468
nikkikapadia merged 3 commits into
masterfrom
nikki/fix/save-replace-null-dataset-discover

Conversation

@nikkikapadia

@nikkikapadia nikkikapadia commented Aug 6, 2026

Copy link
Copy Markdown
Member

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.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Fix All in Cursor

❌ 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 835160e. Configure here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is for another job that we're going to have

@nikkikapadia
nikkikapadia marked this pull request as ready for review August 6, 2026 18:36
@nikkikapadia
nikkikapadia requested a review from a team as a code owner August 6, 2026 18:36
Comment on lines +455 to +462
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}},
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ya fair enough 👍

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 6, 2026
@nikkikapadia
nikkikapadia merged commit 06bf4ca into master Aug 7, 2026
75 of 79 checks passed
@nikkikapadia
nikkikapadia deleted the nikki/fix/save-replace-null-dataset-discover branch August 7, 2026 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants