rebenitez1802 commented on code in PR #45612:
URL: https://github.com/apache/superset/pull/45612#discussion_r4378844998
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/xAxisDrillByFilter.ts:
##########
@@ -165,3 +165,72 @@ export const getTemporalXAxisDrillByFilter = (
formattedVal,
};
};
+
+/**
+ * Build the Drill by filter for a clicked temporal bucket. With a time grain
+ * it matches the grained column, like Drill to detail: the backend compares
+ * the bucket the chart grouped by (same grain, Hours offset and Time Column)
+ * and, unlike a TEMPORAL_RANGE, never replaces it with a query time range, so
+ * the drill keeps every other filter of the chart as the chart applied it.
+ * Without a grain it falls back to an exact match on the timestamp.
+ */
+export const getTemporalBucketDrillByFilter = (
+ col: QueryFormColumn,
+ value: unknown,
+ grain?: TimeGranularity,
+ formattedVal?: string,
+): BinaryQueryObjectFilterClause | undefined => {
+ const bucket = parseTemporalValue(value);
+ if (!col || !bucket) {
+ return undefined;
+ }
+ return grain
+ ? { col, op: '==', val: bucket.getTime(), grain, formattedVal }
+ : { col, op: '==', val: formatNaiveDateTime(bucket), formattedVal };
+};
+
+/** Resolve a markerless line/area click to a real bucket in that series. */
+export const getTimeAxisDrillByValue = (
+ data: unknown,
+ series: unknown,
+ coordinate: unknown,
+ valueIndex = 0,
+ categorical = false,
+): unknown => {
+ if (Array.isArray(data)) {
+ return data[valueIndex];
+ }
+ if (typeof data === 'number' || typeof data === 'string') {
+ return data;
+ }
+ const axisValue = Array.isArray(coordinate)
+ ? coordinate[valueIndex]
+ : coordinate;
+ if (
+ typeof axisValue !== 'number' ||
+ !Number.isFinite(axisValue) ||
+ typeof series !== 'object' ||
+ series === null ||
+ !('data' in series) ||
+ !Array.isArray(series.data)
+ ) {
+ return undefined;
+ }
+ if (categorical) {
+ const point: unknown = series.data[Math.round(axisValue)];
Review Comment:
🟡 Medium (non-blocking) — On a categorical axis this looks up the series'
own data with the axis category index
On a category axis, `convertFromPixel` returns an index into the axis's
category list, but this line reads `series.data[index]`. In a Mixed chart with
a forced-categorical temporal x-axis, query B can have fewer x values than
query A, so the two indexes don't line up.
Example: query A covers Jan–Mar and query B covers Feb–Mar. Clicking B's
markerless line at Feb drills into Mar (`formattedVal: "Mar 2024"`), and
clicking at Mar gives no bucket at all.
Single-query Timeseries charts aren't affected, because all their series
share the same x values. Master sent no x-axis bucket for this Mixed click, so
this is a bug in new code rather than a regression.
Fix: first map the index to the axis category value (from the x-axis
`data`), then find the point in `series.data` whose x value matches it.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:
##########
@@ -679,41 +683,64 @@ export default function EchartsTimeseries({
const xAxisCol =
// if the xAxis is '__timestamp', granularity_sqla will be the
column of filter
xAxis.label === DTTM_ALIAS ? formData.granularitySqla : xAxis.label;
- if (data && xAxis.type === AxisType.Time && xAxisCol) {
- // For horizontal orientation the [x, value] pair is swapped
- const xValue = Array.isArray(data) ? data[xAxisValueIndex] : data;
- const xAxisFilter = getTemporalXAxisDrillByFilter(
- xAxisCol,
+ const isTemporalCategory =
+ xAxis.type === AxisType.Category &&
+ coltypeMapping?.[getColumnLabel(xAxis.label)] ===
+ GenericDataType.Temporal;
+ if ((xAxis.type === AxisType.Time || isTemporalCategory) && xAxisCol) {
+ // A markerless line/area event identifies the series but no point.
+ // Snap its pixel coordinate to an actual bucket in that series.
+ const series = ensureIsArray(echartOptions.series)[
+ eventParams.seriesIndex
+ ];
+ const xValue = getTimeAxisDrillByValue(
+ data ??
+ (isTemporalCategory && eventParams.name
+ ? eventParams.name
+ : undefined),
+ series,
+ data
+ ? undefined
+ : echartRef.current
+ ?.getEchartInstance()
+ ?.convertFromPixel({ seriesIndex: eventParams.seriesIndex },
[
+ eventParams.event.offsetX,
+ eventParams.event.offsetY,
+ ]),
+ xAxisValueIndex,
+ isTemporalCategory,
+ );
+ const xAxisFilter = getTemporalBucketDrillByFilter(
+ // A grained '__timestamp' bucket resolves to the time column the
+ // query groups by, including a dashboard Time Column override.
+ resolvedTimeGrain ? xAxis.label : xAxisCol,
Review Comment:
🟡 Medium (non-blocking) — "Edit chart" drops the clicked bucket for charts
saved without `x_axis`
Charts saved before the `x_axis` control have only `granularity_sqla`, so
`xAxis.label` is `__timestamp` and a grained drill sends
`{"col":"__timestamp","op":"==","val":1064966400000,"grain":"P1M"}`.
- **The Drill by modal is correct:** it resolves `__timestamp` through the
legacy timeseries path.
- **"Edit chart" is not.** Explore sets `x_axis` from `granularity_sqla`, so
the query is no longer a legacy timeseries query. The backend can't resolve
`__timestamp`, silently skips the filter, and the chart shows every month.
- **On master** the drill filter was on the real column (`order_date`), and
Explore showed only the clicked month.
The same applies to `EchartsMixedTimeseries.tsx:254`.
A possible fix, which I haven't tested: resolve `__timestamp` here to the
effective time column, meaning the dashboard Time Column override if there is
one and `granularitySqla` otherwise. Alternatively, map a grained `__timestamp`
filter to the x-axis column in `helpers.py` when the query isn't a legacy
timeseries query.
--
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]
