codeant-ai-for-open-source[bot] commented on code in PR #41907:
URL: https://github.com/apache/superset/pull/41907#discussion_r3573623705
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:
##########
@@ -221,29 +232,50 @@ export default function EchartsTimeseries({
// Determine if X-axis can be used for cross-filtering (categorical axis
without dimensions)
const canCrossFilterByXAxis =
!hasDimensions && xAxis.type === AxisType.Category;
- const categoryAxisValueIndex =
- formData.orientation === OrientationType.Horizontal ? 1 : 0;
const getCategoryAxisValue = useCallback(
- (data: unknown, name: unknown) => {
- if (Array.isArray(data)) {
- const categoryAxisValue = data[categoryAxisValueIndex];
- if (
- typeof categoryAxisValue === 'string' ||
- typeof categoryAxisValue === 'number'
- ) {
- return categoryAxisValue;
- }
- }
- if (typeof name === 'string' || typeof name === 'number') {
- return name;
- }
- return undefined;
- },
- [categoryAxisValueIndex],
+ (data: unknown, name: unknown) =>
+ resolveCategoryAxisValue(data, name, formData.orientation),
+ [formData.orientation],
);
const eventHandlers: EventHandlers = {
click: props => {
+ // Drill-down takes priority over cross-filter when a hierarchy is
configured.
+ // The DrillDownHost provides onDrillDown only when a hierarchy exists.
+ const hasDrillHierarchy = !!onDrillDown;
+
+ if (hasDrillHierarchy) {
+ if (clickTimer.current) {
+ clearTimeout(clickTimer.current);
+ }
+ clickTimer.current = setTimeout(() => {
+ // Timeseries-family charts are always x-axis driven: the hierarchy
+ // advances along the x-axis column, and any groupby is only a series
+ // breakdown. So drill on the clicked x-axis value, never the series
+ // dimension. Non-series clicks (e.g. axis labels) yield no filters.
+ const drillFilters = buildTimeseriesDrillFilters({
+ componentType: props.componentType,
+ data: props.data,
+ name: props.name,
+ xAxisType: xAxis.type,
+ xAxisLabel: xAxis.label,
+ orientation: formData.orientation,
+ dateFormat: formData.dateFormat,
+ numberFormat: formData.numberFormat,
+ coltype: coltypeMapping?.[xAxis.label],
+ });
Review Comment:
**Suggestion:** For time-axis clicks, this forwards `props.name` into drill
filter construction, but ECharts click `name` is often the formatted axis label
while the raw filterable value is in the data tuple (the same file already uses
`data[0]` for time drill-to-detail). This can generate drill filters with
display strings instead of raw temporal values and return empty/incorrect
drilled results. Use the raw axis value from the clicked data point for
non-category axes when building drill filters. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Timeseries drill-down on time axes returns empty results.
- ❌ Dashboard cross-filters from drill path mis-scope linked charts.
- ⚠️ Breadcrumb labels no longer reflect actual filtered timestamps.
- ⚠️ Users experience inconsistent behavior between context menu drill modes.
```
</details>
<details>
<summary><b>Steps of Reproduction âś… </b></summary>
```mdx
1. Render a Timeseries chart with a time x-axis on a dashboard so that the
React component
`EchartsTimeseries` in
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:48-69`
is used with `xAxis.type === AxisType.Time` and `onDrillDown` provided by the
DrillDownHost (drill-down hierarchy configured in the chart controls).
2. Click a data point in the timeseries; the ECharts click event is handled
by
`eventHandlers.click` defined in `EchartsTimeseries.tsx:241-264`, which, when
`hasDrillHierarchy` is true, calls `buildTimeseriesDrillFilters` with `data:
props.data`
and `name: props.name` (see lines 256-266 where the drill filter params are
constructed).
3. In
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/drillFilters.ts:85-135`,
`buildTimeseriesDrillFilters` handles non-category axes (time axis) via the
branch
starting at line 122: it ignores `data` and builds the filter from `name`
(`val: name as
string | number`, `formattedVal: format(name as string | number)` on lines
124-131), while
the same Timeseries file uses `data[0]` as the raw timestamp for
drill-to-detail in the
context menu handler (`EchartsTimeseries.tsx:75-86` where `val: data[0]` and
`formattedVal: xValueFormatter(data[0])` are used when `xAxis.type ===
AxisType.Time`).
4. On time axes, the ECharts click event typically carries the raw timestamp
in the series
data tuple (`props.data[0]`) and a formatted axis label in `props.name`;
because
`buildTimeseriesDrillFilters` uses `name` instead of the raw axis value, the
emitted drill
filters (filtering `col: xAxisLabel` by `val: name`) can contain display
strings rather
than the underlying temporal values, causing the backend drill-down query to
be scoped to
mismatched values and returning empty or incorrect results, while the
drill-to-detail
context menu (which uses `data[0]`) still works as expected—demonstrating the
inconsistency.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=579d64ea322d4be4b2af8967c7cadb1d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=579d64ea322d4be4b2af8967c7cadb1d&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/Timeseries/EchartsTimeseries.tsx
**Line:** 256:266
**Comment:**
*Logic Error: For time-axis clicks, this forwards `props.name` into
drill filter construction, but ECharts click `name` is often the formatted axis
label while the raw filterable value is in the data tuple (the same file
already uses `data[0]` for time drill-to-detail). This can generate drill
filters with display strings instead of raw temporal values and return
empty/incorrect drilled results. Use the raw axis value from the clicked data
point for non-category axes when building drill filters.
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%2F41907&comment_hash=ba307b8ce3177bb3197883e1b40ab2e30818e05baa933a986823faa47d683a2c&reaction=like'>đź‘Ť</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41907&comment_hash=ba307b8ce3177bb3197883e1b40ab2e30818e05baa933a986823faa47d683a2c&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]
