geido commented on PR #43719: URL: https://github.com/apache/superset/pull/43719#issuecomment-5511937137
Second round pushed — thanks @msyavuz and @EnxDev, both of these changed the shape of the fix. **@msyavuz's second point reframed the bug.** It isn't "metrics are missing from `drill_info`", it's that one response was being narrowed for the drill-by picker while also serving as the results grid's verbose map. Everything dropped for the picker's benefit lost its label — every metric *and* every non-dimension column, which a raw-records table routinely selects. So `post_dump` no longer filters at all; each column carries its `groupby` flag and `ChartContextMenu` narrows to dimensions client-side, which it already did on the extension path. **Two things I'd rather flag than bury:** - `drill_info` now returns every column, for all callers including embedded guests. That widens what @amaannawab923 raised in the first round beyond dimension columns. My reasoning is in [that thread](https://github.com/apache/superset/pull/43719#discussion_r3915699163) and unchanged in kind, but it is a real widening — please push back if you read the boundary differently. - It does resolve @amaannawab923's consistency point: `columns` and `metrics` are now scoped identically rather than one filtered and the other not. **@EnxDev found a genuine regression** in the first round's fix: adding metrics to a flat map let an unused metric relabel a column the chart actually selected. `createVerboseMap` now writes metrics first so columns win, matching `SqlaTable.data_for_slices` — the verbose map the dashboard's own charts already render with — rather than introducing a third resolution order. **Verification** (each new test verified failing against the previous code first): | | Before | After | |---|---|---| | 3 new frontend tests | 3 failed, 77 passed | 141 passed | | `schema_tests.py` drill_info | 2 failed | 16 passed | | `test_get_drill_info_admin_user` | column contract mismatch | passed | Full runs: 279 frontend tests across 33 suites (`components/Chart`, `hooks/apiResources`, `usePermissions`, `SliceHeaderControls`), the `drill_info` integration set, and all pre-commit hooks green over the branch diff. PR description updated to describe the broadened fix and the payload change. -- 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]
