fix(explore): drop inherit/custom time shifts when switching to a viz that can't honor them#40865
Conversation
…onor them Converting a chart from Big Number / Table period-over-period (which offer "inherit" and "custom" time shifts) to a timeseries chart carried the inherit/custom value into the new chart's Time shift control, where it lingered as a tag the user had to remove manually. The timeseries advanced-analytics Time shift reuses the same `time_compare` key but does not offer those choices, and since the control is free-form it accepted them. standardizedFormData.transform now strips inherit/custom from `time_compare` when the target viz type's control does not list them as choices, and keeps them when it does (e.g. Table to Big Number).
Code Review Agent Run #1e951eActionable Suggestions - 0Additional Suggestions - 1
Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40865 +/- ##
=======================================
Coverage 64.14% 64.14%
=======================================
Files 2652 2652
Lines 143488 143501 +13
Branches 33110 33115 +5
=======================================
+ Hits 92042 92052 +10
- Misses 49837 49840 +3
Partials 1609 1609
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes an Explore UX issue where inherit/custom time-shift markers (stored under the shared time_compare form-data key) could “stick” when switching from viz types that support them (e.g. Table / Big Number period-over-period) to viz types whose Time shift control does not offer those options (e.g. timeseries charts).
Changes:
- Add a
StandardizedFormData.dropUnsupportedTimeShifts()step during viz-type transform to removeinherit/customfromcontrolsState.time_compare.valuewhen the target viz’stime_comparecontrol choices don’t include them. - Preserve portable relative offsets (e.g.
1 year ago) and preserveinherit/customwhen the target viz does support them (via its control choices). - Add unit tests covering both stripping and preservation behaviors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| superset-frontend/src/explore/controlUtils/standardizedFormData.ts | Filters time_compare during viz-type switching based on the target control’s supported choices. |
| superset-frontend/src/explore/controlUtils/standardizedFormData.test.ts | Adds unit coverage for dropping vs preserving inherit/custom during transforms. |
SUMMARY
When you convert a chart from Big Number with Time Comparison or Table (both offer the "Inherit range from time filter" and "Custom" time shifts) into a timeseries chart such as Line or Bar, the
inherit/customvalue was carried over into the new chart's Time shift control. The timeseries Advanced Analytics "Time shift" reuses the sametime_compareform-data key but does not offer those options, and because the control is free-form the value was accepted and left as a stray tag the user had to delete by hand on every edit.StandardizedFormData.transformpreserves shared controls across viz-type switches. It now dropsinherit/customfromtime_comparewhen the target viz type's Time shift control does not list them as choices. Portable relative shifts (e.g. "1 year ago") are still carried over, andinherit/customare preserved when switching between viz types that both support them (e.g. Table to Big Number).BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
After converting a Big Number (Time Comparison) chart with Time shift = "Inherit" into a Line chart:
TESTING INSTRUCTIONS
inherittag.Unit tests in
standardizedFormData.test.tscover both stripping (target viz without inherit/custom) and preservation (target viz that supports them).ADDITIONAL INFORMATION