fix(reports): Fix/report type switch stale dimensions#245
Merged
therealbrad merged 3 commits intomainfrom Apr 24, 2026
Merged
Conversation
ReportBuilder preserved ?dimensions=...&metrics=... across tab switches,
causing the new report to auto-run with selections from the previous
report. When those dims were unsupported, the server returned errors
like "Unsupported dimension: creator".
- Fix handleTabChange to start URL params from a fresh URLSearchParams
instead of copying the old query string, matching the behavior of
handleReportTypeChange.
- Guard the metadata effect against the URL/state race: when
searchParams.get("reportType") is still pointing at the OLD report
(router.replace has not landed yet) skip loading URL-based
selections, so the new report's dimOpts are not populated with the
previous report's URL params.
- Add E2E regression test covering tab switch — asserts stale
dimension/metric params are dropped and no "Unsupported dimension"
error surfaces.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to the stale-URL-param fix. Extracts the URL-building logic from ReportBuilder into reportUrlUtils so the two invariants can be unit tested without mounting the full component: - buildCleanReportUrlParams — drops stale dimension/metric/date params when switching report types (prevents feeding them into auto-run against an incompatible report). - isUrlInSyncWithReportType — guards the metadata effect from loading URL-based selections when router.replace is in flight and the URL still points at the previous report. Tests cover the happy path, the race window, the initial-load case (no reportType in URL), and pageSize edge cases. Also removes an earlier E2E attempt that was testing the wrong code path — clicking the Reports tab in the current layout cascades into handleReportTypeChange via Radix Select behavior, making the selector unreliable. The unit tests cover the same invariants with higher precision. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
🎉 This PR is included in version 0.22.13 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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.
Description
Fixes a bug where switching report types could auto-run the newly selected report with dimensions/metrics from the PREVIOUS report, producing server errors like
Unsupported dimension: creator.Two root causes, both fixed:
handleTabChangepreserved stale URL params. The handler built the new URL vianew URLSearchParams(searchParams.toString()), inheritingdimensions=...&metrics=...from whatever the previous report was selected.handleReportTypeChangewas already correctly starting from a freshURLSearchParams— nowhandleTabChangematches.Metadata effect race window. When
reportTypechanges via either handler, React may fire the metadata effect once with stalesearchParams(beforerouter.replace's URL update lands). That window loaded URL-based selections from the OLD report into the NEW report's state, and auto-run picked them up. Added a guard that skips URL-param loading when the URL'sreportTypedoesn't match the current state.Refactor: extracted the URL-building + race-guard logic into
components/reports/reportUrlUtils.tsso both invariants can be unit tested without mounting the full component.Related Issue
Surfaced during manual QA of PR #243 / PR #244. No tracked issue number — caught during the React Compiler lint cleanup cycle.
Type of Change
How Has This Been Tested?
reportUrlUtils.test.tscovering the clean-URL contract and the URL/state sync guard)e2e/tests/reports/suite (23 tests) still passes against this branch. An E2E attempt to exercise the tab click was removed after diagnosis showed the Playwright selector was cascading through Radix Select behavior into the wrong handler — the unit tests cover the same invariants with higher precision and no flakiness.Test Configuration:
Suggested manual smoke test before merge:
dimensions=...&metrics=...— and no error toasts/banners appear.Checklist
Screenshots (if applicable)
N/A — no UI changes.
Additional Notes
Branch base: forked from
mainbefore PR #244 (refactor/ref-during-render-cleanup) was merged. If #244 lands first, this branch will need a trivial rebase — different sections ofReportBuilder.tsxso no conflicts expected.Commits: 2 atomic commits — the behavioral fix, then the helper-extraction + unit tests.
Stats: 4 files changed across the two commits (
ReportBuilder.tsx+ newreportUrlUtils.ts/reportUrlUtils.test.ts+ minor cleanup toe2e/tests/reports/report-builder-types.spec.ts).