eschutho opened a new pull request, #43723:
URL: https://github.com/apache/superset/pull/43723
## Summary
Datadog logs for `apache/superset` production surface a recurring
`DeprecationWarning`: *"The field `groupby` is deprecated, please use `columns`
instead."* (`superset/common/query_object.py::DEPRECATED_FIELDS`).
The `groupby` -> `columns` rename was already applied to the
`birth_names`/`world_bank` example fixtures in #43468 (still open). This closes
the gap for a further batch of example dashboards: `fcc_new_coder_survey`,
`featured_charts`, `sales_dashboard`, `slack_dashboard`, `video_game_sales`.
**Decisions made that were not in the instructions**
None.
## What changed
Renamed the top-level `groupby:` key to `columns:` under `params:` in 16
exported chart YAML fixtures, scoped strictly to two viz_types verified safe at
the query-construction layer:
- **`table`** (8 files): `plugin-chart-table`'s `buildQuery.ts` only reads
`baseQueryObject.columns` (built via the generic `extractQueryFields`
groupby/columns aliasing), never `formData.groupby` directly, for plain string
columns like these fixtures use.
- **`pie`** (8 files): `Pie/buildQuery.ts` never destructures
`formData.groupby` at all; it relies entirely on the same generic
`extractQueryFields` aliasing.
Every changed file had exactly one top-level (`params:`-level) `groupby:`
key; unrelated same-named keys elsewhere in the same files (e.g. a per-column
`groupby: true/false` "is this groupable" flag nested inside a metric's
`column:` definition, and an inert `queryFields: {groupby: groupby}` mapping
block in one raw-mode table chart) were left untouched.
**Scope note:** this PR was originally drafted covering 26 files across 5
viz_types (adding `echarts_timeseries_line`, `echarts_area`, `treemap_v2`).
Self-review caught that those three plugins' `buildQuery.ts` destructure
`groupby` directly from `formData` (e.g. `Timeseries/buildQuery.ts`: `const {
groupby } = formData`, used to build both `columns` and `series_columns`;
`Treemap/buildQuery.ts`: `groupby` drives `orderby`) rather than reading the
renamed `columns` field — renaming those would have silently dropped the
grouping dimension / sort order from the affected example charts. Those 10
files were reverted before opening this PR; only the two types confirmed safe
by direct code inspection are included here.
No behavior change for the included files: this only renames a dict key that
both `table` and `pie` consume identically through the existing
`columns`/`groupby` alias in `extractQueryFields`.
## Test plan
- `yaml.safe_load()` on all 16 changed files: clean.
- `pre-commit run check-yaml / trailing-whitespace / end-of-file-fixer` on
all 16 files: clean.
- Verified via `git diff` that only the single top-level `groupby:` key
changed per file, with a per-file check confirming exactly one match for `^
groupby:` before editing.
- Independently grepped `superset-frontend/plugins/**/buildQuery.ts` for
both `formData.groupby` and destructuring (`const { groupby } = formData`)
usage across all 5 originally-considered viz_types; confirmed `table` and `pie`
never read `groupby`, while `echarts_timeseries_line`/`echarts_area`
(`Timeseries/buildQuery.ts`) and `treemap_v2` (`Treemap/buildQuery.ts`) do,
which is why those types were excluded from the final diff.
--
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]
