fix(dashboard): validate display types against all datasets#577
Merged
fix(dashboard): validate display types against all datasets#577
Conversation
Two related bugs with the issue dataset:
1. Column default was too broad — applied to all display types, but only
table needs a default column ("issue"). Timeseries (line/area/bar)
don't use columns at all.
2. Missing validation for unsupported display types. big_number, top_n,
stacked_area, etc. were silently saved, producing broken widgets.
Fix: add ISSUE_DATASET_DISPLAY_TYPES = ["table","area","line","bar"]
sourced from sentry/static/app/views/dashboards/datasetConfig/issues.tsx,
cross-validate in validateWidgetEnums, and scope the column default to
table display only.
Fixes #535
Fixes #536
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Dashboard
Init
Other
Bug Fixes 🐛Dashboard
Event
Other
Internal Changes 🔧Coverage
Other
🤖 This preview updates automatically when you update the PR. |
Contributor
Codecov Results 📊✅ 126 passed | Total: 126 | Pass Rate: 100% | Execution Time: 0ms 📊 Comparison with Base Branch
✨ No test changes detected All tests are passing successfully. ✅ Patch coverage is 100.00%. Project has 1274 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 95.45% 95.46% +0.01%
==========================================
Files 194 194 —
Lines 27952 28040 +88
Branches 0 0 —
==========================================
+ Hits 26678 26766 +88
- Misses 1274 1274 —
- Partials 0 0 —Generated by Codecov Action |
Replaces the issue-only ISSUE_DATASET_DISPLAY_TYPES with a comprehensive DATASET_SUPPORTED_DISPLAY_TYPES map covering all 9 widget types, sourced directly from Sentry's frontend dataset configs at commit a42668e: https://github.com/getsentry/sentry/blob/a42668e/static/app/views/dashboards/datasetConfig/ Key constraints enforced: - preprod-app-size: line only (mobileAppSize.tsx#L255) - tracemetrics: no table or top_n (traceMetrics.tsx#L285-L291) - details/server_tree: spans only (spans.tsx#L287-L297) - issue: table/area/line/bar only (issues.tsx#L90-L95, already in #577) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…sting widget in edit validateWidgetEnums skips the cross-check when only one of display/dataset is provided. In the edit flow, the missing value is filled from the existing widget — so e.g. `--dataset preprod-app-size` on a table widget silently produced an invalid combo. Fix: re-run validateWidgetEnums inside buildReplacement after merging, using the effective (merged) display+dataset values. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…idget display types The re-validation added in buildReplacement was too broad: display types like text, wheel, rage_and_dead_clicks, and agents_traces_table are not in any dataset's supportedDisplayTypes because they bypass Sentry's standard query system entirely. Validating them against a dataset would always throw, blocking legitimate edits (e.g. changing --dataset on a text widget). Fix: only cross-validate when the inherited display type is tracked (present in DATASET_SUPPORTED_DISPLAY_TYPES). Untracked types are skipped. The Cursor bug fix is preserved — tracked types like big_number still validate when only --dataset changes. Also adds a comment to DATASET_SUPPORTED_DISPLAY_TYPES explaining why these four types are intentionally absent. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The previous fix used `if (flags.display || isTrackedDisplay)` which still triggered validateWidgetEnums when the user passed --display with an untracked type (e.g. text), causing a spurious ValidationError. Simplify to just `if (isTrackedDisplay)` — untracked display types carry no dataset constraints regardless of how the flag was set. Also adds a test covering --display text to prevent regression. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
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.
The isTrackedDisplay check was only applied in buildReplacement, but validateWidgetEnums is also called early in both edit and add (before any widget fetching) with the raw flags. Passing --display text --dataset spans would still throw there, contradicting the documented behavior that untracked types (text, wheel, rage_and_dead_clicks, agents_traces_table) "bypass Sentry's dataset system entirely and have no constraints." Move the guard into validateWidgetEnums itself so the fix applies to all callers uniformly. buildReplacement can now call validateWidgetEnums directly without duplicating the logic. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Fixes #535. Fixes #536. Supersedes the incorrect fixes in #570 (merged) and #571 (closed).
What was wrong
The CLI accepted any
--display+--datasetcombination and silently saved broken widgets. Three independent issues:Column default too broad (Dashboard: Top Issues Table widget created with no columns #536, partially fixed in fix(dashboard): default issue dataset table columns to ["issue"] #570) —
columns: ["issue"]was applied for all issue dataset display types, not justtable. A--display line --dataset issuechart got wrong columns.Issue display type validation (Dashboard: Big Number widget invalid for Issues dataset counts #535) — only
table/area/line/barare valid for the issue dataset but any display type was accepted.No validation for any other dataset —
--display table --dataset tracemetrics(not supported),--display table --dataset preprod-app-size(not supported),--display details --dataset logs(spans-only), all silently created broken widgets.Source of truth
All constraints sourced from Sentry's frontend dataset configs at commit
a42668e:issuetable,area,line,barspansarea,bar,big_number,categorical_bar,line,stacked_area,table,top_n,details,server_treeerror-events/transaction-like/metrics/logs/discoverarea,bar,big_number,categorical_bar,line,stacked_area,table,top_ntracemetricsarea,bar,big_number,categorical_bar,linepreprod-app-sizelineWhat this does
ISSUE_DATASET_DISPLAY_TYPESwithDATASET_SUPPORTED_DISPLAY_TYPES— a full map for all 9 widget typesvalidateWidgetEnumsdoes one lookup into the map instead of a hardcoded issue checkissue + tableonlyTest plan
All existing tests pass plus new coverage for:
preprod-app-size + table→ error;+ line→ oktracemetrics + table→ errorspans + details→ ok;logs + details→ error