sadpandajoe opened a new pull request, #43831: URL: https://github.com/apache/superset/pull/43831
fix(explore): clear time_grain_sqla for unaggregated table charts ### SUMMARY The Table chart's `time_grain_sqla` control visibility only checked whether the `groupby` control's value contained a temporal column — it never checked the chart's query mode. Because `groupby` intentionally keeps its value when hidden (`resetOnHide: false`, so switching back to Aggregate mode doesn't lose the selection), a chart that was first built as an aggregated view (e.g. a Line chart with a temporal X-axis and a time grain) kept `time_grain_sqla` visible and populated even after switching to Table's Raw Records mode. The stale time grain was then sent with the outgoing query and used to format the column, so a temporal column added in Raw Records mode still rendered with the old aggregated (e.g. monthly) formatting instead of its raw value. The fix gates `time_grain_sqla`'s visibility on aggregate query mode (`isAggMode`), matching every other query-mode-dependent control in the same file (`groupby`, `metrics`, `percent_metrics`, `timeseries_limit_metric`, `show_totals`, `totals_aggregate`). Once hidden, the existing form-data stash mechanism removes `time_grain_sqla` from the outgoing query, so Raw Records mode no longer carries a stale time grain and the column renders unaggregated. Switching back to Aggregate mode (with a temporal `groupby` column) restores the control and its previous value, so existing aggregated-table behavior is unchanged. The same unguarded pattern existed in the AG Grid Table chart plugin (`plugin-chart-ag-grid-table`, gated behind the `AG_GRID_TABLE_ENABLED` feature flag) and is fixed identically. Full root-cause analysis, including the code history showing this was never query-mode-gated since the control was introduced, is in `RCA.md` at the repo root. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF This is a control-visibility and formatting fix covered by unit-level regression tests (see Testing Instructions). Independent browser QA with before/after evidence will be attached to this PR separately. ### TESTING INSTRUCTIONS Manual repro (before this fix, on `master`): 1. Create a Line chart on a dataset with a temporal column (e.g. `order_date`), set that column as the X-axis with **Month** as Time Grain, add any metric, and create the chart. 2. Switch the visualization type to **Table**. 3. Switch **Query Mode** to **Raw Records**. 4. Add the same temporal column as a column, then update the chart. 5. Before the fix: the temporal column renders with monthly-grain formatting, and the outgoing request's query object still contains `time_grain_sqla: "P1M"`. After the fix: the column renders its raw, unaggregated value, and `time_grain_sqla` is no longer present in the outgoing query. Automated tests: ``` cd superset-frontend npx jest plugins/plugin-chart-table/test/controlPanel.test.ts npx jest plugins/plugin-chart-ag-grid-table/test/controlPanel.test.tsx ``` ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [x] Required feature flags: `AG_GRID_TABLE_ENABLED` (only needed to exercise the AG Grid Table portion of this fix; the primary Table chart fix requires no flag) - [x] 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]
