asdf2014 opened a new pull request, #41933:
URL: https://github.com/apache/superset/pull/41933

   ### SUMMARY
   
   Cross filters emitted by the Mixed Timeseries chart are broken: clicking a 
data point renders no filter value in the dashboard filter bar, while other 
charts in the dashboard go blank because a bogus `IS NULL` filter is applied 
(#41622).
   
   Root cause: the chart renders its ECharts series under *display names* 
(metric display name prefix when dimensions are present, optional "(Query A/B)" 
identifiers, verbose names), while the `labelMap`/`labelMapB` props handed to 
`EchartsMixedTimeseries` stayed keyed by the raw backend `label_map` keys. 
Every click-time lookup missed the map, and the resulting empty groupby values 
made the vacuous `Array.every()` check emit an `IS NULL` clause for every 
groupby column.
   
   Changes:
   - `transformProps` now exposes `labelMap`/`labelMapB` re-keyed by the 
rendered series names. The maps are built right where the display names are 
minted, so future changes to the display-name format stay in sync 
automatically; the backend-keyed maps are renamed `rawLabelMap`/`rawLabelMapB` 
internally to keep the two key domains apart.
   - The component no longer emits column filters when a clicked series cannot 
be resolved through the label map — previously an empty lookup produced the `IS 
NULL` clauses.
   - The context-menu drill-to-detail/drill-by filters now anchor `label_map` 
values from the tail (skipping leading metric/offset labels), matching 
`getCrossFilterDataMask` and the equivalent handler in the Timeseries chart. 
This path used to always miss the map; with the lookups fixed it would 
otherwise have emitted metric labels as dimension values.
   - Two adjacent lookups with the same root cause are fixed as well: the Query 
B series formatter no longer queries `labelMapB` with the frontend-only `" 
(1)"` suffix (which can never match a backend key), and the tooltip formatter 
keys now resolve through the display-keyed maps.
   
   Known pre-existing limitation (unchanged, but now degrades gracefully): 
`seriesBreakdown` does not account for annotation series, so charts with 
visible annotation layers can misroute A/B lookups; with the new empty-lookup 
guard this degrades to a no-op click instead of a bogus filter.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   Before (see the screenshots in #41622): clicking a data point on a mixed 
timeseries chart applies an empty cross filter — dependent charts go blank and 
no filter value is displayed in the filter panel.
   
   After: the cross filter carries the clicked dimension value, the filter 
panel shows it, and dependent charts filter correctly.
   
   ### TESTING INSTRUCTIONS
   
   1. Create an empty dashboard.
   2. Add the "Publishers with most titles" chart (video_game_sales dataset) to 
it.
   3. Create a Mixed Chart on the same dataset (a metric plus a Dimension in 
Query A), add it to the dashboard.
   4. Click a data point on the mixed chart: a cross-filter value should appear 
in the filter panel and "Publishers with most titles" should be filtered 
accordingly (instead of going blank).
   5. Right-click a data point → Drill to detail by / Drill by should carry the 
clicked dimension value.
   
   Unit tests: `npm run test -- 
plugins/plugin-chart-echarts/test/MixedTimeseries` — three new cases assert the 
exposed label maps are keyed by the rendered series names (including 
verbose-name and query-identifier variants); they fail without the fix.
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [x] Has associated issue: Fixes #41622
   - [ ] 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
   


-- 
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]

Reply via email to