jesperct opened a new pull request, #40031:
URL: https://github.com/apache/superset/pull/40031
### SUMMARY
When a chart configured a `groupby` dimension and that same column was
offered as one of several options in a dashboard Dynamic Group By, picking it
alongside a new column would result in only the *new* column being grouped on.
The original was silently dropped.
Root cause is in `buildExistingColumnsSet`: the chart's base `groupby` was
added to the conflict set, so any user-selected column that overlapped was
filtered out of the selection. `applyChartSpecificGroupBy` then replaced the
base entirely with the leftover non-overlap, losing both the user's intent and
the original dimension.
This was the design encoded in #39416 ("restore groupby in
buildExistingColumnsSet"), which reversed the earlier #39356 fix. The two
together produced internally inconsistent behavior:
| User picks | Base `groupby` | Result before | Result after |
|---|---|---|---|
| `[a]` | `[a, b]` | `[a, b]` (subset → kept) | `[a]` (replaced) |
| `[a, c]` | `[a]` | `[c]` (BUG) | `[a, c]` |
| `[c, d]` | `[a]` | `[c, d]` (replaced) | `[c, d]` (unchanged) |
The fix removes base `groupby` from the conflict set so the user's full
selection becomes the new groupby — "what you see in the dropdown is what's in
the chart." The AdhocColumn-aware `extractColumnNames` logic added by #39416 is
preserved; it still applies to `box_plot.columns` and
`pivot_table_v2.groupbyColumns` slots.
Tests in `getFormDataWithExtraFilters.test.ts` that encoded the previous
design were updated to assert the new semantics; an explicit regression test
was added for the reported case.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — pure logic change. Behavior verified via the test suite (18/18 pass,
including the new regression test).
### TESTING INSTRUCTIONS
1. Create a bar chart on a dataset with at least two categorical columns
(e.g. \`cleaned_sales_data\` with \`product_line\` and \`deal_size\`). Set
\`product_line\` as the dimension. Save.
2. Add the chart to a new dashboard.
3. In dashboard edit mode, add a Dynamic Group By chart customization scoped
to the chart. Set its target options to include \`product_line\` and
\`deal_size\`.
4. In the Dynamic Group By dropdown, select both \`product_line\` and
\`deal_size\`.
5. Confirm the chart re-renders grouped by both dimensions. Before this fix
it would only group by \`deal_size\`.
6. Try selecting only \`product_line\`: chart groups by that single column.
7. Try selecting only \`deal_size\`: chart groups by that single column
(\`product_line\` is replaced).
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [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]
