Compare commits

...

7 Commits

Author SHA1 Message Date
Evan
9826ce9100 fix(plugin-chart-echarts): import t from @apache-superset/core/translation
@superset-ui/core no longer exports t, so Radar/utils.ts failed to
type-check and its test threw "t is not a function". Import t the
same way the sibling Radar files (index.ts, controlPanel.tsx) already
do.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-12 08:55:18 -07:00
Evan
c346fba656 fix(plugin-chart-echarts): wrap radar tooltip N/A string for i18n
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-12 04:38:35 -07:00
Evan
8794b8a729 style(plugin-chart-echarts): fix prettier formatting in radar test
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-12 04:12:21 -07:00
Evan
53076ec5e7 fix(plugin-chart-echarts): denormalize missing radar metrics as null, not NaN
The null-preservation fix for #30270 kept a missing metric out of the
denormalization map, so label/tooltip lookups fell through to
Number("null") and rendered "NaN" instead of a gap. Record an explicit
null entry and skip formatting it in the label and tooltip renderers.
Also convert the single-case describe() block to standalone tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-11 20:27:07 -07:00
Evan Rusackas
8c959a3008 style(plugin-chart-echarts): prettier-format radar null test (#30270)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-11 18:17:55 -07:00
Evan
5881cc8de0 fix(plugin-chart-echarts): preserve null metrics in radar normalization
Radar transformProps normalized each series value with value / max in
normalizeArray. A null (missing) metric coerced to 0 in that division, so
a missing data point was plotted at the center of the radar as a real
zero instead of being left out as a gap (#30270).

Preserve null/non-finite values through normalization so ECharts renders
them as gaps, and exclude nulls when computing the per-series max.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-11 17:20:02 -07:00
Claude Code
b94aa33de9 test(plugin-chart-echarts): reproduce radar null values plotted as 0 (#30270)
Radar transformProps normalizes each value with null / max, coercing a
missing (null) metric into 0 so it is plotted at the center instead of
left as a gap. Adds a regression test asserting nulls are preserved.

Closes #30270

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-11 11:13:39 -07:00
5 changed files with 157 additions and 19 deletions

View File

@@ -62,24 +62,29 @@ export function formatLabel({
params: CallbackDataParams;
labelType: EchartsRadarLabelType;
numberFormatter: NumberFormatter;
getDenormalizedSeriesValue: (seriesName: string, value: string) => number;
getDenormalizedSeriesValue: (
seriesName: string,
value: string,
) => number | null;
metricsWithCustomBounds: Set<string>;
metricLabels: string[];
}): string {
const { name = '', value, dimensionIndex = 0 } = params;
const metricLabel = metricLabels[dimensionIndex];
const formattedValue = numberFormatter(
metricsWithCustomBounds.has(metricLabel)
? (value as number)
: (getDenormalizedSeriesValue(name, String(value)) as number),
);
const rawValue = metricsWithCustomBounds.has(metricLabel)
? (value as number | null)
: getDenormalizedSeriesValue(name, String(value));
// A missing metric is preserved as null so it renders as a gap; skip
// formatting it into a misleading "NaN" label.
const formattedValue = rawValue == null ? '' : numberFormatter(rawValue);
switch (labelType) {
case EchartsRadarLabelType.Value:
return formattedValue;
case EchartsRadarLabelType.KeyValue:
return `${name}: ${formattedValue}`;
return formattedValue ? `${name}: ${formattedValue}` : name;
default:
return name;
}
@@ -134,9 +139,20 @@ export default function transformProps(
const getDenormalizedSeriesValue = (
seriesName: string,
normalizedValue: string,
): number =>
denormalizedSeriesValues?.[seriesName]?.[normalizedValue] ??
Number(normalizedValue);
): number | null => {
const seriesMap = denormalizedSeriesValues?.[seriesName];
// A missing metric is recorded explicitly as `null`, so use
// `hasOwnProperty` rather than `??`/`?.` to distinguish "recorded as
// null" from "never recorded" (which falls back to parsing the raw
// normalized value).
if (
seriesMap &&
Object.prototype.hasOwnProperty.call(seriesMap, normalizedValue)
) {
return seriesMap[normalizedValue];
}
return Number(normalizedValue);
};
const metricLabels = metrics.map(getMetricLabel);
@@ -254,14 +270,31 @@ export default function transformProps(
{},
);
const normalizeArray = (arr: number[], decimals = 10, seriesName: string) =>
const normalizeArray = (
arr: (number | null)[],
decimals = 10,
seriesName: string,
): (number | null)[] =>
arr.map((value, index) => {
const metricLabel = metricLabels[index];
if (metricsWithCustomBounds.has(metricLabel)) {
return value;
}
const max = Math.max(...arr);
// Preserve missing (null/undefined) metric values so they render as a
// gap. Dividing null/undefined by max coerces it to 0, which would plot
// the point at the center of the radar as if it were a real zero.
// Explicitly record the denormalized entry as null (rather than
// leaving it unset) so downstream label/tooltip lookups can tell a
// recorded gap apart from an unrecorded value and skip formatting it.
if (value == null || !Number.isFinite(value)) {
denormalizedSeriesValues[seriesName][String(null)] = null;
return null;
}
const max = Math.max(
...arr.filter((v): v is number => v != null && Number.isFinite(v)),
);
const normalizedValue = Number((value / max).toFixed(decimals));
denormalizedSeriesValues[seriesName][String(normalizedValue)] = value;
@@ -276,7 +309,11 @@ export default function transformProps(
return {
...series,
value: normalizeArray(series.value as number[], 10, seriesName),
value: normalizeArray(
series.value as (number | null)[],
10,
seriesName,
),
};
}
return series;
@@ -356,7 +393,7 @@ export default function transformProps(
params: CallbackDataParams & {
color: string;
name: string;
value: number[];
value: (number | null)[];
},
) =>
renderNormalizedTooltip(

View File

@@ -85,10 +85,12 @@ export type RadarChartTransformedProps =
CrossFilterTransformedProps;
/**
* Represents a mapping from a normalized value (as string) to an original numeric value.
* Represents a mapping from a normalized value (as string) to an original
* numeric value. A `null` entry is an explicit record of a missing metric,
* distinct from an absent key (which has not been recorded at all).
*/
interface NormalizedValueMap {
[normalized: string]: number;
[normalized: string]: number | null;
}
/**

View File

@@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { t } from '@apache-superset/core/translation';
import { NumberFormatter } from '@superset-ui/core';
/*
@@ -44,7 +45,7 @@ export const findGlobalMax = (
interface TooltipParams {
color: string;
name?: string;
value: number[];
value: (number | null)[];
}
interface TooltipMetricValue {
@@ -55,7 +56,7 @@ interface TooltipMetricValue {
export const renderNormalizedTooltip = (
params: TooltipParams,
metrics: string[],
getDenormalizedValue: (seriesName: string, value: string) => number,
getDenormalizedValue: (seriesName: string, value: string) => number | null,
metricsWithCustomBounds: Set<string>,
formatter?: NumberFormatter,
): string => {
@@ -73,7 +74,14 @@ export const renderNormalizedTooltip = (
return {
metric,
value: formatter ? formatter(originalValue) : originalValue,
// A missing metric stays null; show it plainly rather than running it
// through the formatter, which would render a misleading "NaN".
value:
originalValue == null
? t('N/A')
: formatter
? formatter(originalValue)
: originalValue,
};
});

View File

@@ -42,6 +42,13 @@ interface RadarChartConfig {
interface RadarSeriesData {
value: number[];
name: string;
label?: {
formatter: (params: {
name: string;
value: number | null;
dimensionIndex: number;
}) => string;
};
}
const formData: Partial<EchartsRadarFormData> = {
@@ -204,6 +211,72 @@ describe('legend sorting', () => {
});
});
// Regression for #30270: "Wrong visualization of missing values in radar
// charts". A null metric value must NOT be transformed into 0. In
// `normalizeArray`, `null / max` coerces the null to 0, so a missing data
// point ends up plotted at the center of the radar as if it were a real
// zero, instead of being left out (a gap). This test feeds a datum with a
// null metric and asserts the null is preserved in the normalized series
// value rather than silently becoming 0.
const missingValueData = [
{
data: [
{
name: 'Series A',
'SUM(jp_sales)': 10,
'SUM(other_sales)': null,
'SUM(eu_sales)': 30,
},
],
},
];
const missingValueProps = new ChartProps({
formData: {
...formData,
// No columnConfig custom bounds so every metric is normalized.
columnConfig: {},
groupby: ['name'],
metrics: ['SUM(jp_sales)', 'SUM(other_sales)', 'SUM(eu_sales)'],
},
width: 800,
height: 600,
queriesData: missingValueData,
theme: supersetTheme,
});
test('preserves a null metric instead of plotting it as 0', () => {
const result = transformProps(missingValueProps as EchartsRadarChartProps);
const series = result.echartOptions.series as RadarSeriesOption[];
const value = (series[0].data as RadarSeriesData[])[0].value as (
number | null
)[];
// Index 1 corresponds to 'SUM(other_sales)', which was null in the datum.
// The correct behavior is to keep the gap (null/undefined) so ECharts does
// not draw a point at the center. On master this is 0, so this fails.
expect(value[1]).not.toBe(0);
expect(value[1] == null).toBe(true);
});
test('label formatter renders a missing metric as blank instead of NaN', () => {
const result = transformProps(missingValueProps as EchartsRadarChartProps);
const series = result.echartOptions.series as RadarSeriesOption[];
const seriesData = (series[0].data as RadarSeriesData[])[0];
const { label } = seriesData;
if (!label) throw new Error('expected series data to have a label config');
// Index 1 corresponds to 'SUM(other_sales)', which is null. Denormalizing
// it must not fall through to `Number('null')` (NaN) in the label text.
const formatted = label.formatter({
name: 'Series A',
value: null,
dimensionIndex: 1,
});
expect(formatted).not.toContain('NaN');
});
describe('radar center positioning', () => {
const getCenter = (overrides: Partial<EchartsRadarFormData> = {}) => {
const props = new ChartProps({

View File

@@ -52,4 +52,22 @@ describe('renderNormalizedTooltip', () => {
expect(tooltip).toContain('100');
expect(tooltip).toContain('200');
});
test('should render a missing metric as N/A instead of NaN', () => {
// A missing metric is denormalized to `null` (see #30270); the tooltip
// must not run it through the formatter, which would print "NaN".
const getDenormalizedValueWithGap = jest.fn((_, value) =>
value === 'null' ? null : Number(value),
);
const formatter = getNumberFormatter(',.2f');
const tooltip = renderNormalizedTooltip(
{ ...params, value: [100, null] },
metrics,
getDenormalizedValueWithGap,
metricsWithCustomBounds,
formatter,
);
expect(tooltip).toContain('N/A');
expect(tooltip).not.toContain('NaN');
});
});