fix(dashboard): auto-default --limit for grouped widgets (CLI-WW)#799
Merged
fix(dashboard): auto-default --limit for grouped widgets (CLI-WW)#799
Conversation
Contributor
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛Dashboard
Init
Other
Documentation 📚
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
Contributor
|
Contributor
Codecov Results 📊✅ 138 passed | Total: 138 | 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 1761 uncovered lines. Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 95.62% 95.63% +0.01%
==========================================
Files 280 280 —
Lines 40208 40273 +65
Branches 0 0 —
==========================================
+ Hits 38444 38512 +68
- Misses 1764 1761 -3
- Partials 0 0 —Generated by Codecov Action |
The Sentry API rejects grouped widgets without a limit. Previously the CLI threw `ValidationError` when users passed `--group-by` without `--limit`, forcing them to read the error and retry. The Sentry UI defaults to 5 in this case, so the CLI now applies the same default transparently and emits an `[info]` nudge so users understand what happened. Both `dashboard widget add` and `dashboard widget edit` share the same `applyGroupLimitAutoDefault()` helper and `DEFAULT_GROUP_BY_LIMIT` constant (`src/commands/dashboard/resolve.ts`). The predicate only fires when the user *explicitly* passes `--group-by` — auto-defaulted columns like `["issue"]` on `dataset=issue display=table` are left alone, since those widgets legitimately work without a limit. Removed the now-dead `validateGroupByRequiresLimit` helper and its imports. Added unit tests for both helpers plus integration tests in `add.test.ts` and `edit.test.ts` covering: auto-default fires; explicit `--limit` wins; ungrouped widgets don't get a limit; existing widget limit is preserved when adding `--group-by`. Based on work in #784 by @cursor; split out per review.
BYK
force-pushed
the
byk/dashboard-widget-auto-limit
branch
from
842a4e5 to
57326a9
Compare
5 tasks
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
1 participant
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.
Summary
The Sentry API rejects grouped dashboard widgets without a limit. Previously the CLI threw
ValidationErrorwhen users passed--group-bywithout--limit:The Sentry UI defaults to 5 for grouped widgets, so the CLI now applies the same default transparently and emits an
[info]nudge so users understand what happened.Design
DEFAULT_GROUP_BY_LIMIT = 5+autoDefaultGroupLimit()+applyGroupLimitAutoDefault()live insrc/commands/dashboard/resolve.tsand are shared betweenwidget addandwidget edit(no duplication, no drift).--group-by. Auto-defaulted columns like["issue"]ondataset=issue display=tablestill produce a widget without a limit (regression-tested).--limitalways wins over the default. If the user literally passes--limit 5the log is silenced (no spurious info line).--group-byto a widget that already has a limit preserves the existing value; only unset limits get filled.validateGroupByRequiresLimithelper.Tests
autoDefaultGroupLimitandapplyGroupLimitAutoDefaultcovering all combinations of grouped/ungrouped × explicit/empty limit.addtests: auto-default fires on group-by; explicit--limitwins; ungrouped widgets don't get a limit; issue/table default columns do NOT trigger auto-default.edittests: auto-default fires when adding--group-by; explicit--limitwins; existing widget limit preserved when adding--group-by; no auto-default for ungrouped--query-only edits.All unit tests pass (5543 tests).
References
Based on work in #784 by @cursor; split out per review.
Fixes CLI-WW.