Copilot commented on code in PR #43863: URL: https://github.com/apache/superset/pull/43863#discussion_r3936557422
########## superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/transformProps.ts: ########## @@ -0,0 +1,533 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { + AxisType, + CurrencyFormatter, + DataRecord, + ensureIsArray, + getColumnLabel, + getMetricLabel, + getNumberFormatter, + getTimeFormatter, + NumberFormatter, + rgbToHex, + tooltipHtml, +} from '@superset-ui/core'; +import { GenericDataType } from '@apache-superset/core/common'; +import type { EChartsCoreOption } from 'echarts/core'; +import type { CandlestickSeriesOption, LineSeriesOption } from 'echarts/charts'; +import type { CallbackDataParams } from 'echarts/types/src/util/types'; +import { + CandlestickChartTransformedProps, + EchartsCandlestickChartProps, +} from './types'; +import { + CANDLESTICK_SERIES_NAME, + DEFAULT_DECREASE_COLOR, + DEFAULT_FORM_DATA, + DEFAULT_INCREASE_COLOR, + DIRECTION_LABELS, + OHLC_LABELS, +} from './constants'; +import { defaultGrid, defaultYAxis } from '../defaults'; +import { getDefaultTooltip } from '../utils/tooltip'; +import { + extractGroupbyLabel, + getChartPadding, + getColtypesMapping, + getLegendProps, +} from '../utils/series'; +import { convertInteger } from '../utils/convertInteger'; +import { mergeCustomEChartOptions } from '../utils/mergeCustomEChartOptions'; +import { safeParseEChartOptions } from '../utils/safeEChartOptionsParser'; +import { NULL_STRING, TIMESERIES_CONSTANTS } from '../constants'; +import { LegendOrientation, LegendType, Refs } from '../types'; +import { resolveLegendLayout } from '../utils/legendLayout'; +import { + calculateMA, + MA_LINE_OPACITY, + movingAverageName, + parseMovingAveragePeriods, +} from './utils'; + +type OhlcValue = [number, number, number, number]; +type CandlestickDatum = NonNullable<CandlestickSeriesOption['data']>[number]; + +function toNumber(value: unknown): number | null { + if (value === null || value === undefined || value === '') { + return null; + } + const numeric = Number(value); + return Number.isFinite(numeric) ? numeric : null; +} + +function getOwnValue<T extends object>( + object: T, + key: string, +): T[keyof T] | undefined { + return key && Object.hasOwn(object, key) ? object[key as keyof T] : undefined; +} + +function toCategoryKey(value: unknown): string { + return value == null ? NULL_STRING : String(value); +} + +function getOhlc( + datum: DataRecord, + openLabel: string, + closeLabel: string, + lowLabel: string, + highLabel: string, +): OhlcValue | null { + const open = toNumber(getOwnValue(datum, openLabel)); + const close = toNumber(getOwnValue(datum, closeLabel)); + const low = toNumber(getOwnValue(datum, lowLabel)); + const high = toNumber(getOwnValue(datum, highLabel)); + if (open === null || close === null || low === null || high === null) { + return null; + } + return [open, close, low, high]; +} + +function toCandlestickDatum( + datum: DataRecord | undefined, + openLabel: string, + closeLabel: string, + lowLabel: string, + highLabel: string, +): CandlestickDatum { + if (!datum) { + return []; + } + return getOhlc(datum, openLabel, closeLabel, lowLabel, highLabel) ?? []; +} + +function extractOhlc(value: unknown): OhlcValue | null { + if (!Array.isArray(value)) { + return null; + } + const raw = value.length >= 5 ? value.slice(1, 5) : value.slice(0, 4); + if (raw.length !== 4) { + return null; + } + const [open, close, low, high] = raw.map(item => Number(item)); + if ([open, close, low, high].some(item => !Number.isFinite(item))) { + return null; + } + return [open, close, low, high]; +} + +function formatTooltip({ + params, + numberFormatter, + title, + increaseLabel, + decreaseLabel, +}: { + params: CallbackDataParams[]; + numberFormatter: NumberFormatter | CurrencyFormatter; + title: string; + increaseLabel: string; + decreaseLabel: string; +}) { + const rows: string[][] = []; + let heading = title; + const candle = params.find(item => extractOhlc(item.value ?? item.data)); + if (candle) { + const ohlc = extractOhlc(candle.value ?? candle.data); + if (ohlc) { + const [open, close, low, high] = ohlc; + const direction = close >= open ? increaseLabel : decreaseLabel; + heading = title ? `${title} (${direction})` : direction; + rows.push( + [OHLC_LABELS.OPEN, numberFormatter(open)], + [OHLC_LABELS.CLOSE, numberFormatter(close)], + [OHLC_LABELS.LOW, numberFormatter(low)], + [OHLC_LABELS.HIGH, numberFormatter(high)], + ); + } + } + params.forEach(item => { + if (item.seriesType !== 'line') { + return; + } + const value = Number(item.value); + if (!Number.isFinite(value)) { + return; + } + rows.push([String(item.seriesName ?? ''), numberFormatter(value)]); + }); + if (!rows.length) { + return ''; + } + return tooltipHtml(rows, heading); +} + +export default function transformProps( + chartProps: EchartsCandlestickChartProps, +): CandlestickChartTransformedProps { + const { + width, + height, + formData: { echartOptions: customEchartOptionsInput, ...rawFormData }, + hooks, + queriesData, + inContextMenu, + theme, + legendState = {}, + } = chartProps; + const formData = { + ...DEFAULT_FORM_DATA, + ...rawFormData, Review Comment: `transformProps` is reading camelCase formData keys (`xAxis`, `showLegend`, `legendOrientation`, `increaseColor`, `movingAverages`, `echartOptions`, etc.), but the control panel / tests use snake_case keys (`x_axis`, `show_legend`, `legend_orientation`, `increase_color`, `moving_averages`, `echart_options`, etc.). This will cause user-selected settings (including X axis column, colors, legend, tooltip formats, zoom, moving averages, and custom echart options) to be ignored at runtime. **Fix (required):** align the Candlestick plugin to Superset’s standard formData naming—either update this destructuring (and related references) to snake_case keys, or introduce an explicit mapping layer (snake_case → camelCase) used consistently across `controlPanel`, `DEFAULT_FORM_DATA`, `types`, and `transformProps`. ########## superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/transformProps.ts: ########## @@ -0,0 +1,533 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { + AxisType, + CurrencyFormatter, + DataRecord, + ensureIsArray, + getColumnLabel, + getMetricLabel, + getNumberFormatter, + getTimeFormatter, + NumberFormatter, + rgbToHex, + tooltipHtml, +} from '@superset-ui/core'; +import { GenericDataType } from '@apache-superset/core/common'; +import type { EChartsCoreOption } from 'echarts/core'; +import type { CandlestickSeriesOption, LineSeriesOption } from 'echarts/charts'; +import type { CallbackDataParams } from 'echarts/types/src/util/types'; +import { + CandlestickChartTransformedProps, + EchartsCandlestickChartProps, +} from './types'; +import { + CANDLESTICK_SERIES_NAME, + DEFAULT_DECREASE_COLOR, + DEFAULT_FORM_DATA, + DEFAULT_INCREASE_COLOR, + DIRECTION_LABELS, + OHLC_LABELS, +} from './constants'; +import { defaultGrid, defaultYAxis } from '../defaults'; +import { getDefaultTooltip } from '../utils/tooltip'; +import { + extractGroupbyLabel, + getChartPadding, + getColtypesMapping, + getLegendProps, +} from '../utils/series'; +import { convertInteger } from '../utils/convertInteger'; +import { mergeCustomEChartOptions } from '../utils/mergeCustomEChartOptions'; +import { safeParseEChartOptions } from '../utils/safeEChartOptionsParser'; +import { NULL_STRING, TIMESERIES_CONSTANTS } from '../constants'; +import { LegendOrientation, LegendType, Refs } from '../types'; +import { resolveLegendLayout } from '../utils/legendLayout'; +import { + calculateMA, + MA_LINE_OPACITY, + movingAverageName, + parseMovingAveragePeriods, +} from './utils'; + +type OhlcValue = [number, number, number, number]; +type CandlestickDatum = NonNullable<CandlestickSeriesOption['data']>[number]; + +function toNumber(value: unknown): number | null { + if (value === null || value === undefined || value === '') { + return null; + } + const numeric = Number(value); + return Number.isFinite(numeric) ? numeric : null; +} + +function getOwnValue<T extends object>( + object: T, + key: string, +): T[keyof T] | undefined { + return key && Object.hasOwn(object, key) ? object[key as keyof T] : undefined; +} + +function toCategoryKey(value: unknown): string { + return value == null ? NULL_STRING : String(value); +} + +function getOhlc( + datum: DataRecord, + openLabel: string, + closeLabel: string, + lowLabel: string, + highLabel: string, +): OhlcValue | null { + const open = toNumber(getOwnValue(datum, openLabel)); + const close = toNumber(getOwnValue(datum, closeLabel)); + const low = toNumber(getOwnValue(datum, lowLabel)); + const high = toNumber(getOwnValue(datum, highLabel)); + if (open === null || close === null || low === null || high === null) { + return null; + } + return [open, close, low, high]; +} + +function toCandlestickDatum( + datum: DataRecord | undefined, + openLabel: string, + closeLabel: string, + lowLabel: string, + highLabel: string, +): CandlestickDatum { + if (!datum) { + return []; + } + return getOhlc(datum, openLabel, closeLabel, lowLabel, highLabel) ?? []; +} + +function extractOhlc(value: unknown): OhlcValue | null { + if (!Array.isArray(value)) { + return null; + } + const raw = value.length >= 5 ? value.slice(1, 5) : value.slice(0, 4); + if (raw.length !== 4) { + return null; + } + const [open, close, low, high] = raw.map(item => Number(item)); + if ([open, close, low, high].some(item => !Number.isFinite(item))) { + return null; + } + return [open, close, low, high]; +} + +function formatTooltip({ + params, + numberFormatter, + title, + increaseLabel, + decreaseLabel, +}: { + params: CallbackDataParams[]; + numberFormatter: NumberFormatter | CurrencyFormatter; + title: string; + increaseLabel: string; + decreaseLabel: string; +}) { + const rows: string[][] = []; + let heading = title; + const candle = params.find(item => extractOhlc(item.value ?? item.data)); + if (candle) { + const ohlc = extractOhlc(candle.value ?? candle.data); + if (ohlc) { + const [open, close, low, high] = ohlc; + const direction = close >= open ? increaseLabel : decreaseLabel; + heading = title ? `${title} (${direction})` : direction; + rows.push( + [OHLC_LABELS.OPEN, numberFormatter(open)], + [OHLC_LABELS.CLOSE, numberFormatter(close)], + [OHLC_LABELS.LOW, numberFormatter(low)], + [OHLC_LABELS.HIGH, numberFormatter(high)], + ); + } + } + params.forEach(item => { + if (item.seriesType !== 'line') { + return; + } + const value = Number(item.value); + if (!Number.isFinite(value)) { + return; + } + rows.push([String(item.seriesName ?? ''), numberFormatter(value)]); + }); + if (!rows.length) { + return ''; + } + return tooltipHtml(rows, heading); +} + +export default function transformProps( + chartProps: EchartsCandlestickChartProps, +): CandlestickChartTransformedProps { + const { + width, + height, + formData: { echartOptions: customEchartOptionsInput, ...rawFormData }, + hooks, + queriesData, + inContextMenu, + theme, + legendState = {}, + } = chartProps; + const formData = { + ...DEFAULT_FORM_DATA, + ...rawFormData, + }; + const [queryData] = queriesData; + const { data = [] } = queryData; + const { onLegendStateChanged } = hooks; + const refs: Refs = {}; + const coltypeMapping = getColtypesMapping(queryData); + + const { + xAxis, + open, + close, + high, + low, + series: seriesControl, + increaseColor = DEFAULT_INCREASE_COLOR, + decreaseColor = DEFAULT_DECREASE_COLOR, + increaseLabel, + decreaseLabel, + showXAxis, + showYAxis, + xAxisTimeFormat, + xAxisTitle, + xAxisTitleMargin, + xAxisLabelRotation, + xAxisLabelInterval, + yAxisTitle, + yAxisTitleMargin, + yAxisTitlePosition, + yAxisFormat, + currencyFormat, + tooltipTimeFormat, + tooltipValuesFormat, + showLegend, + legendMargin, + legendOrientation = LegendOrientation.Top, + legendType = LegendType.Scroll, + legendSort, + zoomable, + movingAverages, Review Comment: `transformProps` is reading camelCase formData keys (`xAxis`, `showLegend`, `legendOrientation`, `increaseColor`, `movingAverages`, `echartOptions`, etc.), but the control panel / tests use snake_case keys (`x_axis`, `show_legend`, `legend_orientation`, `increase_color`, `moving_averages`, `echart_options`, etc.). This will cause user-selected settings (including X axis column, colors, legend, tooltip formats, zoom, moving averages, and custom echart options) to be ignored at runtime. **Fix (required):** align the Candlestick plugin to Superset’s standard formData naming—either update this destructuring (and related references) to snake_case keys, or introduce an explicit mapping layer (snake_case → camelCase) used consistently across `controlPanel`, `DEFAULT_FORM_DATA`, `types`, and `transformProps`. ########## superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/types.ts: ########## @@ -0,0 +1,68 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { + ChartDataResponseResult, + ChartProps, + QueryFormColumn, + QueryFormData, + QueryFormMetric, + RgbaColor, +} from '@superset-ui/core'; +import { + BaseChartProps, + BaseTransformedProps, + LegendFormData, + TitleFormData, +} from '../types'; + +export type EchartsCandlestickFormData = QueryFormData & + LegendFormData & + TitleFormData & { + xAxis: QueryFormColumn; + open: QueryFormMetric; + close: QueryFormMetric; + high: QueryFormMetric; + low: QueryFormMetric; + series?: QueryFormColumn | QueryFormColumn[]; + increaseColor: RgbaColor; + decreaseColor: RgbaColor; + increaseLabel?: string; + decreaseLabel?: string; + showXAxis: boolean; + showYAxis: boolean; + xAxisTimeFormat?: string; + xAxisLabelRotation: number; + xAxisLabelInterval: string; + yAxisFormat: string; + tooltipTimeFormat?: string; + tooltipValuesFormat?: string; + zoomable: boolean; + movingAverages?: (number | string)[]; + echartOptions?: string; Review Comment: The Candlestick `FormData` type uses camelCase field names (e.g. `xAxis`, `increaseColor`, `showXAxis`, `movingAverages`, `echartOptions`) but the corresponding controls are defined with snake_case names (e.g. `x_axis`, `increase_color`, `show_x_axis`, `moving_averages`, `echart_options`). This mismatch will both hide real integration errors (since the type won’t reflect what Explore actually passes) and contributes to runtime bugs. **Fix (required):** rename these fields in the type to the actual snake_case formData keys used by chart controls, and tighten `series` to a single optional column (since `controlOverrides.series` sets `multi: false`). ########## superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/EchartsCandlestick.tsx: ########## @@ -0,0 +1,51 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import Echart from '../components/Echart'; +import { EventHandlers } from '../types'; +import { CandlestickChartTransformedProps } from './types'; + +export default function EchartsCandlestick( + props: CandlestickChartTransformedProps, +) { + const { height, width, echartOptions, refs, onLegendStateChanged, formData } = + props; + + const eventHandlers: EventHandlers = { + legendselectchanged: payload => { + onLegendStateChanged?.(payload.selected); + }, + legendselectall: payload => { + onLegendStateChanged?.(payload.selected); + }, + legendinverseselect: payload => { + onLegendStateChanged?.(payload.selected); + }, + }; + + return ( + <Echart + refs={refs} + height={height} + width={width} + echartOptions={echartOptions} + eventHandlers={eventHandlers} + vizType={formData.vizType} Review Comment: Superset Explore form data uses `viz_type` (snake_case), not `vizType`. Passing `undefined` here prevents `Echart` from applying chart-type theme overrides (`theme.echartsOptionsOverridesByChartType`) and may break any chart-type-specific operational settings (e.g. aria/animation overrides by type). **Fix (required):** pass the correct viz type field from formData (typically `formData.viz_type`), consistent with other ECharts chart components. ########## superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/utils.ts: ########## @@ -0,0 +1,78 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { ensureIsArray } from '@superset-ui/core'; + +export const MOVING_AVERAGE_PERIODS = [5, 10, 15, 20, 30, 60]; +export const MA_LINE_OPACITY = 0.5; + +export function movingAverageName(period: number, seriesName?: string): string { + const label = `MA${period}`; + return seriesName ? `${seriesName} ${label}` : label; +} + +function parseMovingAveragePeriod(item: unknown): number | null { + if (typeof item === 'number') { + return Number.isInteger(item) && item > 1 ? item : null; + } + const match = String(item) + .trim() + .match(/^(?:MA)?(\d+)$/i); + if (!match) { + return null; + } + const period = Number(match[1]); + return Number.isInteger(period) && period > 1 ? period : null; +} + +export function parseMovingAveragePeriods(value: unknown): number[] { + const periods = ensureIsArray(value) + .map(parseMovingAveragePeriod) + .filter((period): period is number => period !== null); + return [...new Set(periods)].sort((left, right) => left - right); +} + +/** + * Simple moving average of close prices: the first `period - 1` points + * are omitted, then each value is the mean of the current close and the + * previous `period - 1` closes. + */ +export function calculateMA( + closes: Array<number | null>, + period: number, +): Array<number | '-'> { + const result: Array<number | '-'> = []; + for (let i = 0; i < closes.length; i += 1) { + if (i < period - 1) { + result.push('-'); + continue; + } + let sum = 0; + let valid = true; + for (let j = 0; j < period; j += 1) { + const close = closes[i - j]; + if (close === null) { + valid = false; + break; + } + sum += close; + } + result.push(valid ? sum / period : '-'); + } Review Comment: `calculateMA` is `O(n * period)` per series per MA period (nested loop summing each window from scratch). With long time series and multiple MA periods (and/or multiple series), this can become noticeably expensive. Consider a sliding-window approach (maintain rolling sum + count, reset/track invalid windows when encountering `null`) to reduce per-period work to `O(n)`. ########## superset-frontend/plugins/plugin-chart-echarts/src/Candlestick/transformProps.ts: ########## @@ -0,0 +1,533 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { + AxisType, + CurrencyFormatter, + DataRecord, + ensureIsArray, + getColumnLabel, + getMetricLabel, + getNumberFormatter, + getTimeFormatter, + NumberFormatter, + rgbToHex, + tooltipHtml, +} from '@superset-ui/core'; +import { GenericDataType } from '@apache-superset/core/common'; +import type { EChartsCoreOption } from 'echarts/core'; +import type { CandlestickSeriesOption, LineSeriesOption } from 'echarts/charts'; +import type { CallbackDataParams } from 'echarts/types/src/util/types'; +import { + CandlestickChartTransformedProps, + EchartsCandlestickChartProps, +} from './types'; +import { + CANDLESTICK_SERIES_NAME, + DEFAULT_DECREASE_COLOR, + DEFAULT_FORM_DATA, + DEFAULT_INCREASE_COLOR, + DIRECTION_LABELS, + OHLC_LABELS, +} from './constants'; +import { defaultGrid, defaultYAxis } from '../defaults'; +import { getDefaultTooltip } from '../utils/tooltip'; +import { + extractGroupbyLabel, + getChartPadding, + getColtypesMapping, + getLegendProps, +} from '../utils/series'; +import { convertInteger } from '../utils/convertInteger'; +import { mergeCustomEChartOptions } from '../utils/mergeCustomEChartOptions'; +import { safeParseEChartOptions } from '../utils/safeEChartOptionsParser'; +import { NULL_STRING, TIMESERIES_CONSTANTS } from '../constants'; +import { LegendOrientation, LegendType, Refs } from '../types'; +import { resolveLegendLayout } from '../utils/legendLayout'; +import { + calculateMA, + MA_LINE_OPACITY, + movingAverageName, + parseMovingAveragePeriods, +} from './utils'; + +type OhlcValue = [number, number, number, number]; +type CandlestickDatum = NonNullable<CandlestickSeriesOption['data']>[number]; + +function toNumber(value: unknown): number | null { + if (value === null || value === undefined || value === '') { + return null; + } + const numeric = Number(value); + return Number.isFinite(numeric) ? numeric : null; +} + +function getOwnValue<T extends object>( + object: T, + key: string, +): T[keyof T] | undefined { + return key && Object.hasOwn(object, key) ? object[key as keyof T] : undefined; +} + +function toCategoryKey(value: unknown): string { + return value == null ? NULL_STRING : String(value); +} + +function getOhlc( + datum: DataRecord, + openLabel: string, + closeLabel: string, + lowLabel: string, + highLabel: string, +): OhlcValue | null { + const open = toNumber(getOwnValue(datum, openLabel)); + const close = toNumber(getOwnValue(datum, closeLabel)); + const low = toNumber(getOwnValue(datum, lowLabel)); + const high = toNumber(getOwnValue(datum, highLabel)); + if (open === null || close === null || low === null || high === null) { + return null; + } + return [open, close, low, high]; +} + +function toCandlestickDatum( + datum: DataRecord | undefined, + openLabel: string, + closeLabel: string, + lowLabel: string, + highLabel: string, +): CandlestickDatum { + if (!datum) { + return []; + } + return getOhlc(datum, openLabel, closeLabel, lowLabel, highLabel) ?? []; +} + +function extractOhlc(value: unknown): OhlcValue | null { + if (!Array.isArray(value)) { + return null; + } + const raw = value.length >= 5 ? value.slice(1, 5) : value.slice(0, 4); + if (raw.length !== 4) { + return null; + } + const [open, close, low, high] = raw.map(item => Number(item)); + if ([open, close, low, high].some(item => !Number.isFinite(item))) { + return null; + } + return [open, close, low, high]; +} + +function formatTooltip({ + params, + numberFormatter, + title, + increaseLabel, + decreaseLabel, +}: { + params: CallbackDataParams[]; + numberFormatter: NumberFormatter | CurrencyFormatter; + title: string; + increaseLabel: string; + decreaseLabel: string; +}) { + const rows: string[][] = []; + let heading = title; + const candle = params.find(item => extractOhlc(item.value ?? item.data)); + if (candle) { + const ohlc = extractOhlc(candle.value ?? candle.data); + if (ohlc) { + const [open, close, low, high] = ohlc; + const direction = close >= open ? increaseLabel : decreaseLabel; + heading = title ? `${title} (${direction})` : direction; + rows.push( + [OHLC_LABELS.OPEN, numberFormatter(open)], + [OHLC_LABELS.CLOSE, numberFormatter(close)], + [OHLC_LABELS.LOW, numberFormatter(low)], + [OHLC_LABELS.HIGH, numberFormatter(high)], + ); + } + } + params.forEach(item => { + if (item.seriesType !== 'line') { + return; + } + const value = Number(item.value); + if (!Number.isFinite(value)) { + return; + } + rows.push([String(item.seriesName ?? ''), numberFormatter(value)]); + }); + if (!rows.length) { + return ''; + } + return tooltipHtml(rows, heading); +} + +export default function transformProps( + chartProps: EchartsCandlestickChartProps, +): CandlestickChartTransformedProps { + const { + width, + height, + formData: { echartOptions: customEchartOptionsInput, ...rawFormData }, + hooks, + queriesData, + inContextMenu, + theme, + legendState = {}, + } = chartProps; + const formData = { + ...DEFAULT_FORM_DATA, + ...rawFormData, + }; + const [queryData] = queriesData; + const { data = [] } = queryData; + const { onLegendStateChanged } = hooks; + const refs: Refs = {}; + const coltypeMapping = getColtypesMapping(queryData); + + const { + xAxis, + open, + close, + high, + low, + series: seriesControl, + increaseColor = DEFAULT_INCREASE_COLOR, + decreaseColor = DEFAULT_DECREASE_COLOR, + increaseLabel, + decreaseLabel, + showXAxis, + showYAxis, + xAxisTimeFormat, + xAxisTitle, + xAxisTitleMargin, + xAxisLabelRotation, + xAxisLabelInterval, Review Comment: `transformProps` is reading camelCase formData keys (`xAxis`, `showLegend`, `legendOrientation`, `increaseColor`, `movingAverages`, `echartOptions`, etc.), but the control panel / tests use snake_case keys (`x_axis`, `show_legend`, `legend_orientation`, `increase_color`, `moving_averages`, `echart_options`, etc.). This will cause user-selected settings (including X axis column, colors, legend, tooltip formats, zoom, moving averages, and custom echart options) to be ignored at runtime. **Fix (required):** align the Candlestick plugin to Superset’s standard formData naming—either update this destructuring (and related references) to snake_case keys, or introduce an explicit mapping layer (snake_case → camelCase) used consistently across `controlPanel`, `DEFAULT_FORM_DATA`, `types`, and `transformProps`. ########## superset-frontend/plugins/plugin-chart-echarts/test/Candlestick/buildQuery.test.ts: ########## @@ -0,0 +1,57 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { QueryFormData, VizType } from '@superset-ui/core'; +import buildQuery from '../../src/Candlestick/buildQuery'; + +const formData = { + datasource: '5__table', + viz_type: VizType.Candlestick, + x_axis: 'date', + open: 'open', + close: 'close', + high: 'high', + low: 'low', +} as QueryFormData; + +test('builds query fields from OHLC metrics and x-axis', () => { Review Comment: The PR’s “TESTING INSTRUCTIONS” mention running `.../test/Butterfly`, but this PR adds Candlestick tests under `.../test/Candlestick`. The instructions should be updated to point to the Candlestick test suite (or the intended aggregate test command) so reviewers can validate the feature quickly. -- 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]
