sadpandajoe opened a new pull request, #43650: URL: https://github.com/apache/superset/pull/43650
### SUMMARY Hiding a control-panel section silently and permanently deleted that section's saved configuration from the chart. `StashFormDataContainer` moves a hidden section's controls out of `form_data` (into `hiddenFormData`) so they stop affecting the live query — that part is intentional. But `updateSlice`/`createSlice` in `saveModalActions.ts` built the save payload directly from that already-stripped `form_data`, so saving the chart while a section was hidden wrote the keys out of `slices.params` for good, with no warning at any point. The most visible case is Advanced Analytics → Time Comparison. Its section is gated on `displayTimeRelatedControls`, which resolves to `false` when the x-axis column loses `is_dttm` or is renamed/removed on the dataset. From that moment, opening and saving any affected chart drops `time_compare` and `comparison_type`. Because the gate is dataset-scoped, every chart on a dashboard sharing that dataset can lose its configuration at once — which is how this surfaced as a customer-reported issue. The fix merges `hiddenFormData` back in when building the save payload, so a merely-hidden section no longer destroys previously-saved values. `form_data` is spread last, so anything currently visible still wins. The live query path does not go through these actions and is unaffected. Worth noting: the feature-flagged chart-normalization audit trail in `src/features/versionHistory/` already *detects* this exact drop at save time and can record it as a `normalization_change` — but it only observes the loss, it doesn't prevent it, and only when the flag is on. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF Not applicable — no UI change. The behavioral change is in what gets persisted to `slices.params`, demonstrated by the tests below. ### TESTING INSTRUCTIONS Automated: ``` cd superset-frontend npm run test -- src/explore/actions/saveModalActions.test.ts ``` Added `updateSlice keeps values stashed by a hidden control section in the saved params` (fails without the fix, for the expected reason: the keys are missing from the saved `params`), plus a matching `createSlice` regression test. Two pre-existing tests encoded the old drop-and-log behavior as expected and were updated to assert the value now survives and no drop is reported. Manual: 1. Build an ECharts time-series chart on a dataset whose x-axis is a physical temporal column, configure Advanced Analytics → Time Comparison, and save. 2. On the dataset, unset `is_dttm` on that column so the Advanced Analytics section hides. 3. Open the chart in Explore and save it again without changing anything. 4. Inspect the chart's saved params: on `master`, `time_compare`/`comparison_type` are gone; with this change they survive. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
