codeant-ai-for-open-source[bot] commented on code in PR #40449:
URL: https://github.com/apache/superset/pull/40449#discussion_r3305710192
##########
superset-frontend/plugins/plugin-chart-echarts/src/Gauge/transformProps.ts:
##########
@@ -377,6 +377,7 @@ export default function transformProps(
groupby,
selectedValues: filterState.selectedValues || [],
onContextMenu,
+ onDrillDown,
Review Comment:
**Suggestion:** Passing `onDrillDown` into Gauge transformed props activates
the drill-only click handler path in `allEventHandlers`, which does not apply a
data mask. As a result, clicks drill but do not broadcast cross-filters to
other charts when hierarchy is configured. Add cross-filter emission in the
drill-down branch before exposing this hook. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Gauge drill-down no longer updates dashboard cross-filters.
- ⚠️ Other charts ignore Gauge click interactions under drill.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In Gauge transform props, observe that hooks are destructured to include
`onDrillDown`
at
`superset-frontend/plugins/plugin-chart-echarts/src/Gauge/transformProps.ts:229`
(`const { setDataMask = () => {}, onContextMenu, onDrillDown } = hooks;`)
and that
`onDrillDown` is returned to the renderer at lines 369-383, specifically
line 380
(`onDrillDown,`).
2. See that `EchartsGauge` at
`superset-frontend/plugins/plugin-chart-echarts/src/Gauge/EchartsGauge.tsx:23-27`
constructs `eventHandlers = allEventHandlers(props);`, passing `setDataMask`,
`emitCrossFilters`, `groupby`, `labelMap`, `selectedValues`, `formData`, and
the newly
wired `onDrillDown` into `allEventHandlers`.
3. Inspect `allEventHandlers` in
`superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts:152-215`,
where
`onDrillDown` is read from `transformedProps` (lines 155-165),
`hasDrillHierarchy` is set
based on `!!onDrillDown` (line 170), and `drillDownClickHandler` is created
when
`hasDrillHierarchy && groupby.length > 0` (lines 172-195). In this case,
`eventHandlers.click` is set to `drillDownClickHandler` (lines 197-200).
4. Compare with `clickEventHandler` at lines 90-106 in the same file: it uses
`getCrossFilterDataMask(selectedValues, groupby, labelMap)` and calls
`setDataMask` when
`emitCrossFilters` is true, emitting cross-filters. Because wiring
`onDrillDown` for Gauge
causes `allEventHandlers` to choose `drillDownClickHandler` (which only calls
`onDrillDown` and never `setDataMask`) instead of `clickEventHandler`, Gauge
clicks in a
dashboard with drill hierarchy configured will drill but no longer update
the data mask,
so cross-filters are not propagated to other charts.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=38c2d1cbd2234731aaf1bc740ede6b84&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=38c2d1cbd2234731aaf1bc740ede6b84&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/plugin-chart-echarts/src/Gauge/transformProps.ts
**Line:** 380:380
**Comment:**
*Logic Error: Passing `onDrillDown` into Gauge transformed props
activates the drill-only click handler path in `allEventHandlers`, which does
not apply a data mask. As a result, clicks drill but do not broadcast
cross-filters to other charts when hierarchy is configured. Add cross-filter
emission in the drill-down branch before exposing this hook.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=d72e781b94256132c101bf66bdbed64acc49275ce5b67ff954b153b2fb932bd4&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=d72e781b94256132c101bf66bdbed64acc49275ce5b67ff954b153b2fb932bd4&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/transformProps.ts:
##########
@@ -320,6 +320,7 @@ export default function transformProps(
groupby,
selectedValues,
onContextMenu,
+ onDrillDown,
Review Comment:
**Suggestion:** Forwarding `onDrillDown` here switches BoxPlot clicks into
the drill-down branch in `allEventHandlers`, which currently skips
`setDataMask` updates. That means cross-filter is no longer emitted when drill
hierarchy is enabled, so linked dashboard charts stop updating. Update the
drill-down click path to also emit cross-filter (as done in Timeseries) before
wiring this prop through. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ BoxPlot drill-down stops emitting dashboard cross-filters.
- ⚠️ Linked charts remain stale when clicking BoxPlot points.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure BoxPlot transform props to expose `onDrillDown` by returning it
from
`transformProps` at
`superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/transformProps.ts:312-324`,
where the object includes `setDataMask`, `emitCrossFilters`, `labelMap`,
`groupby`,
`selectedValues`, `onContextMenu`, and `onDrillDown` (line 323).
2. Observe that `EchartsBoxPlot` at
`superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/EchartsBoxPlot.tsx:23-28`
calls `allEventHandlers(props)` and passes the transformed props including
`onDrillDown`,
`setDataMask`, `emitCrossFilters`, `groupby`, `labelMap`, and
`selectedValues` into the
generic event handler factory.
3. Follow the click handling logic in `allEventHandlers` at
`superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts:152-215`,
where
`onDrillDown` is read (lines 155-165), `hasDrillHierarchy` is computed as
`!!onDrillDown`
(line 170), and when `hasDrillHierarchy && groupby.length > 0`,
`eventHandlers.click` is
set to `drillDownClickHandler` (lines 172-195, 197-200) instead of
`clickEventHandler`.
4. Note that `drillDownClickHandler` builds `drillFilters` and calls
`onDrillDown(drillFilters, label)` (lines 175-193) but never calls
`clickEventHandler` or
`setDataMask`, while `clickEventHandler` (lines 90-106) is the only place
that computes a
cross-filter data mask via `getCrossFilterDataMask(...)(name)` and calls
`setDataMask`.
Therefore, once a drill hierarchy is configured and `onDrillDown` is
non-null, BoxPlot
clicks invoke drill-down only and no longer emit cross-filters, leaving
linked dashboard
charts unfiltered.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=929a6dea9ad348d7bc533d5bb701a198&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=929a6dea9ad348d7bc533d5bb701a198&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/transformProps.ts
**Line:** 323:323
**Comment:**
*Logic Error: Forwarding `onDrillDown` here switches BoxPlot clicks
into the drill-down branch in `allEventHandlers`, which currently skips
`setDataMask` updates. That means cross-filter is no longer emitted when drill
hierarchy is enabled, so linked dashboard charts stop updating. Update the
drill-down click path to also emit cross-filter (as done in Timeseries) before
wiring this prop through.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=b54d342886724e8c7a88099ee0c8296d3078892e64678c108d6eff9379710822&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=b54d342886724e8c7a88099ee0c8296d3078892e64678c108d6eff9379710822&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/Radar/transformProps.ts:
##########
@@ -414,6 +414,7 @@ export default function transformProps(
groupby,
selectedValues,
onContextMenu,
+ onDrillDown,
Review Comment:
**Suggestion:** This new `onDrillDown` passthrough makes Radar use the
drill-down click path, but that path currently omits cross-filter updates. In
dashboards with drill hierarchy enabled, Radar clicks will drill without
filtering sibling charts. Align the drill-down handler with Timeseries behavior
by emitting cross-filter and drill-down together. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Radar drill-down stops broadcasting cross-filters to dashboards.
- ⚠️ Radar clicks no longer synchronize with linked charts.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In Radar transform props, verify that hooks are destructured to include
`onDrillDown`
at
`superset-frontend/plugins/plugin-chart-echarts/src/Radar/transformProps.ts:129`
(`const { setDataMask = () => {}, onContextMenu, onDrillDown } = hooks;`)
and that
`onDrillDown` is propagated in the returned props at lines 406-420,
specifically line 417
(`onDrillDown,`).
2. Confirm that `EchartsRadar` at
`superset-frontend/plugins/plugin-chart-echarts/src/Radar/EchartsRadar.tsx:23-27`
calls
`const eventHandlers = allEventHandlers(props);`, which passes Radar's
`groupby`,
`labelMap`, `selectedValues`, `setDataMask`, `emitCrossFilters`, `formData`,
`coltypeMapping`, and now `onDrillDown` into the shared `allEventHandlers`
function.
3. Inspect `allEventHandlers` in
`superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts:152-215`:
it
reads `onDrillDown` (lines 155-165), sets `hasDrillHierarchy =
!!onDrillDown` (line 170),
and when `hasDrillHierarchy && groupby.length > 0` constructs
`drillDownClickHandler` that
builds `drillFilters` and calls `onDrillDown(drillFilters, label)` (lines
172-193).
4. Note that in this branch, `eventHandlers.click` is assigned to
`drillDownClickHandler`
(lines 197-200) and never calls `clickEventHandler`, while
`clickEventHandler` (lines
90-106) is the only function that computes a cross-filter `dataMask` and
calls
`setDataMask`. Thus, for Radar charts with a configured drill hierarchy
(where
`onDrillDown` is provided by the host), clicks now trigger `onDrillDown`
only and stop
emitting cross-filters, so sibling charts on the dashboard no longer filter
in response to
Radar clicks.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=bf906782130e4bde8924bee1678f70d3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=bf906782130e4bde8924bee1678f70d3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/plugin-chart-echarts/src/Radar/transformProps.ts
**Line:** 417:417
**Comment:**
*Logic Error: This new `onDrillDown` passthrough makes Radar use the
drill-down click path, but that path currently omits cross-filter updates. In
dashboards with drill hierarchy enabled, Radar clicks will drill without
filtering sibling charts. Align the drill-down handler with Timeseries behavior
by emitting cross-filter and drill-down together.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=29488d6e86519c4ad1c15208296b205cd3c62622c95a1450ce987e3e6f46ad7d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=29488d6e86519c4ad1c15208296b205cd3c62622c95a1450ce987e3e6f46ad7d&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts:
##########
@@ -404,6 +404,7 @@ export default function transformProps(
groupby,
selectedValues: filterState.selectedValues || [],
onContextMenu,
+ onDrillDown,
Review Comment:
**Suggestion:** `onDrillDown` is now included in transformed props, but
`EchartsSunburst` does not read or invoke it in its click handlers. This leaves
Sunburst drill-down non-functional despite the new wiring, so the feature
appears enabled in data flow but never executes. Wire `onDrillDown` into
Sunburst click handling and build drill filters from the clicked tree path.
[incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Sunburst drill-down never triggers despite onDrillDown wiring.
- ⚠️ Users get cross-filters only, no Sunburst hierarchy drill.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In Sunburst transform props, verify that `hooks` are destructured to
include
`onDrillDown` at
`superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts:242`
(`const { setDataMask = () => {}, onContextMenu, onDrillDown } = hooks;`)
and that
`onDrillDown` is returned to the renderer at lines 396-410, specifically
line 407
(`onDrillDown,`).
2. Inspect `EchartsSunburst` at
`superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/EchartsSunburst.tsx:36-50`:
the component destructures `height`, `width`, `echartOptions`, `setDataMask`,
`selectedValues`, `formData`, `onContextMenu`, `refs`, `emitCrossFilters`,
and
`coltypeMapping` from `props`, but does not read or destructure
`onDrillDown` anywhere, so
the drill-down hook returned from `transformProps` is effectively ignored.
3. Examine Sunburst's click handling in the same file:
`getCrossFilterDataMask` (lines
51-93) constructs a data mask from `treePathInfo`, and `handleChange` (lines
97-106) calls
`setDataMask(getCrossFilterDataMask(treePathInfo).dataMask)` when
`emitCrossFilters` is
true. The `eventHandlers.click` implementation (lines 108-112) calls
`handleChange(treePathInfo)` but never calls `onDrillDown`, so clicks only
update
cross-filters and never invoke the drill-down hook.
4. As a result, when a drill hierarchy is configured and the host provides
`onDrillDown`
for Sunburst (demonstrated by transform props returning it at line 407),
clicking on
Sunburst segments will continue to perform cross-filtering via `setDataMask`
but will
never call `onDrillDown`, meaning the new drill-down feature (drill state
and breadcrumb
updates managed by `DrillDownHost`) remains non-functional for Sunburst
despite being
wired through transform props.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=dc5d731afb72459c96a1aa59f465125f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=dc5d731afb72459c96a1aa59f465125f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts
**Line:** 407:407
**Comment:**
*Incomplete Implementation: `onDrillDown` is now included in
transformed props, but `EchartsSunburst` does not read or invoke it in its
click handlers. This leaves Sunburst drill-down non-functional despite the new
wiring, so the feature appears enabled in data flow but never executes. Wire
`onDrillDown` into Sunburst click handling and build drill filters from the
clicked tree path.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=e337247bdf4d60e98f9aad7e66caabd10e73f9b9b5ee5c42673bf95a81b054b8&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40449&comment_hash=e337247bdf4d60e98f9aad7e66caabd10e73f9b9b5ee5c42673bf95a81b054b8&reaction=dislike'>👎</a>
--
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]
