rusackas opened a new pull request, #42530: URL: https://github.com/apache/superset/pull/42530
### SUMMARY Follow-up hardening identified during a self-review of #41714 (the legacy viz pipeline removal). Targets that branch so it can land as a stack once #41714 merges. Each commit is independent and reviewable on its own: - **deck.gl `Multi`**: removed a dead code path that read a `payload.data.slices`/`.features` shape the v1 endpoint never returns, and fixed a real race where an out-of-order/stale async layer response could clobber a newer one — added a load-generation guard and rewrote the tests around the actual v1 fetch shape instead of the dead legacy one. - **ECharts Timeseries**: the percent-change draggable baseline read its own series data back out of `chart.getOption()` after `setOption()`, which is fragile and untested; switched to reading the `echartOptions` prop directly and added the plugin's first test coverage for the drag-to-rebase interaction. - **Migration downgrade scoping**: `MigrateViz.downgrade` matched slices by `target_viz_type` alone, so when two source viz types share one target (e.g. `MigrateLineChart` and `MigrateCompareChart` both migrate onto `echarts_timeseries_line`), downgrading one could revert slices the other had upgraded. Added a precise per-row check and removed the CLI's separate, now-inconsistent `PREVIOUS_VERSION` mapping in favor of deriving the source viz type from the stored backup. - **Legend-row estimation**: Bullet and TimePivot each estimated wrapped-legend row count with similar, subtly different logic; extracted one shared, tested helper and used it to fix a legend/chart overlap bug in TimePivot that Bullet's version didn't have. - **Sort-metric/orderby**: `partition`, `paired_ttest`, and `parallel_coordinates` each reimplemented "append the sort metric if not already selected, order by it" with slightly different rules; extracted a single shared, tested helper. - **Rose**: added test coverage for click-to-drill and, in particular, for the color-priming order invariant (colors come from priming the scale in `seriesNames` order, not the drilled pie's value-sorted order) -- a case that's easy to accidentally break during future refactors. - **Migration hardening**: `upgrade_slice` assumed a stored `query_context` always has a `"queries"` key; an atypical one (e.g. hand-edited via the API) missing it raised an uncaught `KeyError` inside the broad `except`, after `viz_type` had already been flipped, leaving the slice half-migrated. Now degrades gracefully. - **Partition transformProps**: added the plugin's first test coverage for `transformProps.ts` itself (as opposed to `transformData.ts`, which was already well covered) -- the v1-flat-record vs. legacy-nested-hierarchy branch, verboseMap level mapping, and `partitionLimit`/`partitionThreshold` parsing. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A -- no user-visible behavior changes; this is bug fixes and test hardening on top of #41714. ### TESTING INSTRUCTIONS - `pytest tests/unit_tests/migrations/` -- all migration unit tests, including two new regression tests (downgrade scoping, malformed `query_context`) - `npm run test -- plugins/preset-chart-deckgl/src/Multi plugins/plugin-chart-echarts/src/utils/legendLayout.test.ts plugins/plugin-chart-echarts/test/TimePivot/transformProps.test.ts plugins/plugin-chart-echarts/test/Timeseries/EchartsTimeseries.test.tsx plugins/plugin-chart-echarts/test/Rose/EchartsRose.test.tsx packages/superset-ui-chart-controls/test/utils/buildSortMetricOrderby.test.ts plugins/plugin-chart-paired-t-test/test/buildQuery.test.ts plugins/plugin-chart-parallel-coordinates/test plugins/plugin-chart-partition/test` - Two of the regression tests (Rose's color-priming test, the `query_context` `KeyError` fix) were verified by deliberately reverting the corresponding fix and confirming the new test fails, then restoring it and confirming it passes. ### 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 Stacked on #41714 -- targets `remove-legacy-viz-pipeline`, not `master`. Should be reviewed/merged after (or alongside) that PR. -- 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]
