Skip to content

fix(dashboard): validate display types against all datasets#577

Merged
betegon merged 11 commits intomainfrom
fix/issue-dataset-widget-validation
Mar 26, 2026
Merged

fix(dashboard): validate display types against all datasets#577
betegon merged 11 commits intomainfrom
fix/issue-dataset-widget-validation

Conversation

@betegon
Copy link
Copy Markdown
Member

@betegon betegon commented Mar 26, 2026

Fixes #535. Fixes #536. Supersedes the incorrect fixes in #570 (merged) and #571 (closed).

What was wrong

The CLI accepted any --display + --dataset combination and silently saved broken widgets. Three independent issues:

  1. 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 just table. A --display line --dataset issue chart got wrong columns.

  2. Issue display type validation (Dashboard: Big Number widget invalid for Issues dataset counts #535) — only table/area/line/bar are valid for the issue dataset but any display type was accepted.

  3. 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:

Dataset Supported display types
issue table, area, line, bar
spans area, bar, big_number, categorical_bar, line, stacked_area, table, top_n, details, server_tree
error-events / transaction-like / metrics / logs / discover area, bar, big_number, categorical_bar, line, stacked_area, table, top_n
tracemetrics area, bar, big_number, categorical_bar, line
preprod-app-size line

What this does

  • Replaces ISSUE_DATASET_DISPLAY_TYPES with DATASET_SUPPORTED_DISPLAY_TYPES — a full map for all 9 widget types
  • validateWidgetEnums does one lookup into the map instead of a hardcoded issue check
  • Column default remains scoped to issue + table only

Test plan

All existing tests pass plus new coverage for:

  • preprod-app-size + table → error; + line → ok
  • tracemetrics + table → error
  • spans + details → ok; logs + details → error

betegon and others added 2 commits March 26, 2026 17:59
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>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 26, 2026

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


New Features ✨

Dashboard

  • Add pagination and glob filtering to dashboard list by BYK in #560
  • Rich terminal chart rendering for dashboard view by BYK in #555

Init

  • Propagate sentry-trace headers to wizard API calls by betegon in #567
  • Treat bare slug as new project name when not found by BYK in #554

Other

  • (formatters) Colorize SQL in DB span descriptions by BYK in #546
  • (telemetry) Report unknown commands to Sentry by BYK in #563
  • Bidirectional cursor pagination (-c next / -c prev) by BYK in #564
  • Add sentry sourcemap inject and sentry sourcemap upload commands by BYK in #547
  • Native debug ID injection and sourcemap upload by BYK in #543

Bug Fixes 🐛

Dashboard

  • Validate display types against all datasets by betegon in #577
  • Auto-clamp widget limit instead of erroring by BYK in #573
  • Default issue dataset table columns to ["issue"] by betegon in #570
  • Scale timeseries bar width to fill chart area by BYK in #562
  • Resolve dashboard by ID/slug in addition to title by BYK in #559

Event

  • Detect SHORT-ID/EVENT-ID format in event view by BYK in #574
  • Auto-fallback to org-wide search when event 404s in project by BYK in #575

Other

  • (api) Show meaningful message for network errors instead of '0 Unknown' by BYK in #572
  • (event-view) Auto-redirect issue short IDs in two-arg form (CLI-MP) by BYK in #558
  • (help) Show help when user passes help as positional arg by BYK in #561
  • (issue) Auto-redirect bare org slug to org-all mode in issue list by BYK in #576
  • Reject @-selectors in parseOrgProjectArg with helpful redirect by BYK in #557

Internal Changes 🔧

Coverage

  • Use informational-patch input instead of sed hack by BYK in #544
  • Make checks informational on release branches by BYK in #541

Other

  • (api) Collapse stats on issue detail endpoints to save 100-300ms by BYK in #551
  • (ci) Upgrade GitHub Actions to Node 24 runtime by BYK in #542
  • (db) DRY up database layer with shared helpers and lint enforcement by BYK in #550
  • (issue-list) Use collapse parameter to skip unused Snuba queries by BYK in #545
  • Bump Bun from 1.3.9 to 1.3.11 by BYK in #552
  • Regenerate skill files by github-actions[bot] in ec1ffe28

🤖 This preview updates automatically when you update the PR.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 26, 2026

Codecov Results 📊

126 passed | Total: 126 | Pass Rate: 100% | Execution Time: 0ms

📊 Comparison with Base Branch

Metric Change
Total Tests
Passed Tests
Failed Tests
Skipped Tests

✨ No test changes detected

All tests are passing successfully.

✅ Patch coverage is 100.00%. Project has 1274 uncovered lines.
✅ Project coverage is 95.46%. Comparing base (base) to head (head).

Files with missing lines (1)
File Patch % Lines
dashboard.ts 99.80% ⚠️ 1 Missing
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

betegon and others added 2 commits March 26, 2026 18:09
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>
@betegon betegon changed the title fix(dashboard): correct issue dataset display types and column default fix(dashboard): validate display types against all datasets Mar 26, 2026
github-actions bot and others added 2 commits March 26, 2026 17:10
@betegon betegon marked this pull request as ready for review March 26, 2026 17:21
…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>
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

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.

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>
@betegon betegon merged commit a00a739 into main Mar 26, 2026
22 checks passed
@betegon betegon deleted the fix/issue-dataset-widget-validation branch March 26, 2026 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dashboard: Top Issues Table widget created with no columns Dashboard: Big Number widget invalid for Issues dataset counts

1 participant