This is an automated email from the ASF dual-hosted git repository. msyavuz pushed a commit to branch msyavuz/fix/scatter-point-radius in repository https://gitbox.apache.org/repos/asf/superset.git
commit f54d435445235c14c7dfbe4300a1e49c15bf7f86 Author: Mehmet Salih Yavuz <[email protected]> AuthorDate: Fri Jan 2 16:04:07 2026 +0300 fix(Scatter): fixed point radius --- .../src/layers/Scatter/Scatter.tsx | 19 +- .../src/layers/Scatter/buildQuery.test.ts | 260 ++++++++++++++++++ .../src/layers/Scatter/buildQuery.ts | 13 +- .../src/layers/Scatter/transformProps.test.ts | 303 +++++++++++++++++++++ .../src/layers/Scatter/transformProps.ts | 17 +- .../src/layers/transformUtils.test.ts | 150 ++++++++++ .../src/layers/transformUtils.ts | 8 +- 7 files changed, 759 insertions(+), 11 deletions(-) diff --git a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/Scatter.tsx b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/Scatter.tsx index 3ecd3eac0c..4772d5c1a8 100644 --- a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/Scatter.tsx +++ b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/Scatter.tsx @@ -44,9 +44,20 @@ function setTooltipContent( verboseMap?: Record<string, string>, ) { const defaultTooltipGenerator = (o: JsonObject) => { - const label = - verboseMap?.[formData.point_radius_fixed.value] || - getMetricLabel(formData.point_radius_fixed?.value); + // Check if point_radius_fixed is an object with type/value or just a value + let metricKey = null; + if (formData.point_radius_fixed) { + if (typeof formData.point_radius_fixed === 'object' && formData.point_radius_fixed.type === 'metric') { + metricKey = formData.point_radius_fixed.value; + } else if (typeof formData.point_radius_fixed === 'string') { + metricKey = formData.point_radius_fixed; + } + } + + const label = metricKey + ? (verboseMap?.[metricKey] || getMetricLabel(metricKey)) + : null; + return ( <div className="deckgl-tooltip"> <TooltipRow @@ -59,7 +70,7 @@ function setTooltipContent( value={`${o.object?.cat_color}`} /> )} - {o.object?.metric && ( + {o.object?.metric && label && ( <TooltipRow label={`${label}: `} value={`${o.object?.metric}`} /> )} </div> diff --git a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.test.ts b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.test.ts new file mode 100644 index 0000000000..2f6d515517 --- /dev/null +++ b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.test.ts @@ -0,0 +1,260 @@ +/** + * 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 buildQuery, { DeckScatterFormData } from './buildQuery'; + +const baseFormData: DeckScatterFormData = { + datasource: '1__table', + viz_type: 'deck_scatter', + spatial: { + type: 'latlong', + latCol: 'LATITUDE', + lonCol: 'LONGITUDE', + }, + row_limit: 100, +}; + +test('Scatter buildQuery should not include metric when point_radius_fixed is fixed type', () => { + const formData: DeckScatterFormData = { + ...baseFormData, + point_radius_fixed: { + type: 'fix', + value: '1000', + }, + }; + + const queryContext = buildQuery(formData); + const [query] = queryContext.queries; + + expect(query.metrics).toEqual([]); + expect(query.orderby).toEqual([]); + }); + +test('Scatter buildQuery should include metric when point_radius_fixed is metric type', () => { + const formData: DeckScatterFormData = { + ...baseFormData, + point_radius_fixed: { + type: 'metric', + value: 'AVG(radius_value)', + }, + }; + + const queryContext = buildQuery(formData); + const [query] = queryContext.queries; + + expect(query.metrics).toContain('AVG(radius_value)'); + expect(query.orderby).toEqual([['AVG(radius_value)', false]]); + }); + +test('Scatter buildQuery should handle numeric value in fixed type', () => { + const formData: DeckScatterFormData = { + ...baseFormData, + point_radius_fixed: { + type: 'fix', + value: 500, + }, + }; + + const queryContext = buildQuery(formData); + const [query] = queryContext.queries; + + // Fixed numeric value should not be included as a metric + expect(query.metrics).toEqual([]); + expect(query.orderby).toEqual([]); + }); + +test('Scatter buildQuery should handle missing point_radius_fixed', () => { + const formData: DeckScatterFormData = { + ...baseFormData, + // no point_radius_fixed + }; + + const queryContext = buildQuery(formData); + const [query] = queryContext.queries; + + expect(query.metrics).toEqual([]); + expect(query.orderby).toEqual([]); + }); + +test('Scatter buildQuery should include spatial columns in query', () => { + const formData: DeckScatterFormData = { + ...baseFormData, + point_radius_fixed: { + type: 'fix', + value: '1000', + }, + }; + + const queryContext = buildQuery(formData); + const [query] = queryContext.queries; + + expect(query.columns).toContain('LATITUDE'); + expect(query.columns).toContain('LONGITUDE'); + }); + +test('Scatter buildQuery should include dimension column when specified', () => { + const formData: DeckScatterFormData = { + ...baseFormData, + dimension: 'category', + point_radius_fixed: { + type: 'fix', + value: '1000', + }, + }; + + const queryContext = buildQuery(formData); + const [query] = queryContext.queries; + + expect(query.columns).toContain('category'); + }); + +test('Scatter buildQuery should add spatial null filters', () => { + const formData: DeckScatterFormData = { + ...baseFormData, + point_radius_fixed: { + type: 'fix', + value: '1000', + }, + }; + + const queryContext = buildQuery(formData); + const [query] = queryContext.queries; + + const latFilter = query.filters?.find( + f => f.col === 'LATITUDE' && f.op === 'IS NOT NULL', + ); + const lonFilter = query.filters?.find( + f => f.col === 'LONGITUDE' && f.op === 'IS NOT NULL', + ); + + expect(latFilter).toBeDefined(); + expect(lonFilter).toBeDefined(); + }); + +test('Scatter buildQuery should throw error when spatial is missing', () => { + const formData: DeckScatterFormData = { + ...baseFormData, + spatial: undefined, + }; + + expect(() => buildQuery(formData)).toThrow( + 'Spatial configuration is required for Scatter charts', + ); + }); + +test('Scatter buildQuery should handle geohash spatial type', () => { + const formData: DeckScatterFormData = { + ...baseFormData, + spatial: { + type: 'geohash', + geohashCol: 'geohash_column', + }, + point_radius_fixed: { + type: 'metric', + value: 'COUNT(*)', + }, + }; + + const queryContext = buildQuery(formData); + const [query] = queryContext.queries; + + expect(query.columns).toContain('geohash_column'); + expect(query.metrics).toContain('COUNT(*)'); + }); + +test('Scatter buildQuery should handle tooltip_contents', () => { + const formData: DeckScatterFormData = { + ...baseFormData, + tooltip_contents: ['name', 'description'], + point_radius_fixed: { + type: 'fix', + value: '1000', + }, + }; + + const queryContext = buildQuery(formData); + const [query] = queryContext.queries; + + expect(query.columns).toContain('name'); + expect(query.columns).toContain('description'); + }); + +test('Scatter buildQuery should handle js_columns', () => { + const formData: DeckScatterFormData = { + ...baseFormData, + js_columns: ['custom_col1', 'custom_col2'], + point_radius_fixed: { + type: 'fix', + value: '1000', + }, + }; + + const queryContext = buildQuery(formData); + const [query] = queryContext.queries; + + expect(query.columns).toContain('custom_col1'); + expect(query.columns).toContain('custom_col2'); + }); + +test('Scatter buildQuery should convert numeric metric value to string', () => { + const formData: DeckScatterFormData = { + ...baseFormData, + point_radius_fixed: { + type: 'metric', + value: 123, // numeric metric (edge case) + }, + }; + + const queryContext = buildQuery(formData); + const [query] = queryContext.queries; + + expect(query.metrics).toContain('123'); + expect(query.orderby).toEqual([['123', false]]); + }); + +test('Scatter buildQuery should set is_timeseries to false', () => { + const formData: DeckScatterFormData = { + ...baseFormData, + point_radius_fixed: { + type: 'fix', + value: '1000', + }, + }; + + const queryContext = buildQuery(formData); + const [query] = queryContext.queries; + + expect(query.is_timeseries).toBe(false); + }); + +test('Scatter buildQuery should preserve row_limit', () => { + const formData: DeckScatterFormData = { + ...baseFormData, + row_limit: 5000, + point_radius_fixed: { + type: 'fix', + value: '1000', + }, + }; + + const queryContext = buildQuery(formData); + const [query] = queryContext.queries; + + expect(query.row_limit).toBe(5000); +}); \ No newline at end of file diff --git a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.ts b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.ts index 598bbfdce9..7cd830a40d 100644 --- a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.ts +++ b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.ts @@ -38,7 +38,8 @@ import { export interface DeckScatterFormData extends Omit<SpatialFormData, 'color_picker'>, SqlaFormData { point_radius_fixed?: { - value?: string; + type?: 'fix' | 'metric'; + value?: string | number; }; multiplier?: number; point_unit?: string; @@ -78,14 +79,18 @@ export default function buildQuery(formData: DeckScatterFormData) { columns = withJsColumns as QueryFormColumn[]; columns = addTooltipColumnsToQuery(columns, tooltip_contents); - const metrics = processMetricsArray([point_radius_fixed?.value]); + // Only add metric if point_radius_fixed is a metric type + const isMetric = point_radius_fixed?.type === 'metric'; + const metricValue = isMetric && point_radius_fixed?.value ? String(point_radius_fixed.value) : null; + + const metrics = processMetricsArray(metricValue ? [metricValue] : []); const filters = addSpatialNullFilters( spatial, ensureIsArray(baseQueryObject.filters || []), ); - const orderby = point_radius_fixed?.value - ? ([[point_radius_fixed.value, false]] as QueryFormOrderBy[]) + const orderby = isMetric && metricValue + ? ([[metricValue, false]] as QueryFormOrderBy[]) : (baseQueryObject.orderby as QueryFormOrderBy[]) || []; return [ diff --git a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/transformProps.test.ts b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/transformProps.test.ts new file mode 100644 index 0000000000..8c3487fb6d --- /dev/null +++ b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/transformProps.test.ts @@ -0,0 +1,303 @@ +/** + * 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 { ChartProps, DatasourceType } from '@superset-ui/core'; +import transformProps from './transformProps'; + +interface ScatterFeature { + position: [number, number]; + radius?: number; + metric?: number; + cat_color?: string; + extraProps?: Record<string, unknown>; +} + +jest.mock('../spatialUtils', () => ({ + ...jest.requireActual('../spatialUtils'), + getMapboxApiKey: jest.fn(() => 'mock-mapbox-key'), +})); + +const mockChartProps: Partial<ChartProps> = { + rawFormData: { + spatial: { + type: 'latlong', + latCol: 'LATITUDE', + lonCol: 'LONGITUDE', + }, + viewport: {}, + }, + queriesData: [ + { + data: [ + { + LATITUDE: 37.8, + LONGITUDE: -122.4, + population: 50000, + 'AVG(radius_value)': 100, + }, + { + LATITUDE: 37.9, + LONGITUDE: -122.3, + population: 75000, + 'AVG(radius_value)': 200, + }, + ], + }, + ], + datasource: { + type: DatasourceType.Table, + id: 1, + name: 'test_datasource', + columns: [], + metrics: [], + }, + height: 400, + width: 600, + hooks: {}, + filterState: {}, + emitCrossFilters: false, +}; + +test('Scatter transformProps should use fixed radius value when point_radius_fixed type is "fix"', () => { + const fixedProps = { + ...mockChartProps, + rawFormData: { + ...mockChartProps.rawFormData, + point_radius_fixed: { + type: 'fix', + value: '1000', + }, + }, + }; + + const result = transformProps(fixedProps as ChartProps); + const features = result.payload.data.features as ScatterFeature[]; + + expect(features).toHaveLength(2); + expect(features[0]?.radius).toBe(1000); + expect(features[1]?.radius).toBe(1000); + // metric should not be set for fixed radius + expect(features[0]?.metric).toBeUndefined(); + expect(features[1]?.metric).toBeUndefined(); + }); + +test('Scatter transformProps should use metric value for radius when point_radius_fixed type is "metric"', () => { + const metricProps = { + ...mockChartProps, + rawFormData: { + ...mockChartProps.rawFormData, + point_radius_fixed: { + type: 'metric', + value: 'AVG(radius_value)', + }, + }, + }; + + const result = transformProps(metricProps as ChartProps); + const features = result.payload.data.features as ScatterFeature[]; + + expect(features).toHaveLength(2); + expect(features[0]?.radius).toBe(100); + expect(features[0]?.metric).toBe(100); + expect(features[1]?.radius).toBe(200); + expect(features[1]?.metric).toBe(200); + }); + +test('Scatter transformProps should handle numeric fixed radius value', () => { + const fixedProps = { + ...mockChartProps, + rawFormData: { + ...mockChartProps.rawFormData, + point_radius_fixed: { + type: 'fix', + value: 500, // numeric value + }, + }, + }; + + const result = transformProps(fixedProps as ChartProps); + const features = result.payload.data.features as ScatterFeature[]; + + expect(features).toHaveLength(2); + expect(features[0]?.radius).toBe(500); + expect(features[1]?.radius).toBe(500); + }); + +test('Scatter transformProps should handle missing point_radius_fixed', () => { + const propsWithoutRadius = { + ...mockChartProps, + rawFormData: { + ...mockChartProps.rawFormData, + // no point_radius_fixed + }, + }; + + const result = transformProps(propsWithoutRadius as ChartProps); + const features = result.payload.data.features as ScatterFeature[]; + + expect(features).toHaveLength(2); + // radius should not be set + expect(features[0]?.radius).toBeUndefined(); + expect(features[1]?.radius).toBeUndefined(); + }); + +test('Scatter transformProps should handle dimension for category colors', () => { + const propsWithDimension = { + ...mockChartProps, + rawFormData: { + ...mockChartProps.rawFormData, + dimension: 'category', + point_radius_fixed: { + type: 'fix', + value: '1000', + }, + }, + queriesData: [ + { + data: [ + { + LATITUDE: 37.8, + LONGITUDE: -122.4, + category: 'A', + }, + { + LATITUDE: 37.9, + LONGITUDE: -122.3, + category: 'B', + }, + ], + }, + ], + }; + + const result = transformProps(propsWithDimension as ChartProps); + const features = result.payload.data.features as ScatterFeature[]; + + expect(features).toHaveLength(2); + expect(features[0]?.cat_color).toBe('A'); + expect(features[1]?.cat_color).toBe('B'); + }); + +test('Scatter transformProps should not include metric labels for fixed radius', () => { + const fixedProps = { + ...mockChartProps, + rawFormData: { + ...mockChartProps.rawFormData, + point_radius_fixed: { + type: 'fix', + value: '1000', + }, + }, + }; + + const result = transformProps(fixedProps as ChartProps); + + // metricLabels should be empty for fixed radius + expect(result.payload.data.metricLabels).toEqual([]); + }); + +test('Scatter transformProps should include metric labels for metric-based radius', () => { + const metricProps = { + ...mockChartProps, + rawFormData: { + ...mockChartProps.rawFormData, + point_radius_fixed: { + type: 'metric', + value: 'AVG(radius_value)', + }, + }, + }; + + const result = transformProps(metricProps as ChartProps); + + // metricLabels should include the metric name + expect(result.payload.data.metricLabels).toContain('AVG(radius_value)'); + }); + +test('Scatter transformProps should handle no records', () => { + const noDataProps = { + ...mockChartProps, + queriesData: [{ data: [] }], + rawFormData: { + ...mockChartProps.rawFormData, + point_radius_fixed: { + type: 'fix', + value: '1000', + }, + }, + }; + + const result = transformProps(noDataProps as ChartProps); + const features = result.payload.data.features as ScatterFeature[]; + + expect(features).toHaveLength(0); + }); + +test('Scatter transformProps should handle missing spatial configuration gracefully', () => { + const noSpatialProps = { + ...mockChartProps, + rawFormData: { + ...mockChartProps.rawFormData, + spatial: undefined, + point_radius_fixed: { + type: 'fix', + value: '1000', + }, + }, + }; + + const result = transformProps(noSpatialProps as ChartProps); + const features = result.payload.data.features as ScatterFeature[]; + + expect(features).toHaveLength(0); + }); + +test('Scatter transformProps should preserve extra properties from records', () => { + const propsWithExtraData = { + ...mockChartProps, + rawFormData: { + ...mockChartProps.rawFormData, + point_radius_fixed: { + type: 'fix', + value: '1000', + }, + }, + queriesData: [ + { + data: [ + { + LATITUDE: 37.8, + LONGITUDE: -122.4, + custom_field: 'value1', + another_field: 123, + }, + ], + }, + ], + }; + + const result = transformProps(propsWithExtraData as ChartProps); + const features = result.payload.data.features as ScatterFeature[]; + + expect(features).toHaveLength(1); + expect(features[0]).toMatchObject({ + custom_field: 'value1', + another_field: 123, + }); +}); \ No newline at end of file diff --git a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/transformProps.ts b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/transformProps.ts index 7f4a300886..bceff246e3 100644 --- a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/transformProps.ts +++ b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/transformProps.ts @@ -43,6 +43,7 @@ function processScatterData( radiusMetricLabel?: string, categoryColumn?: string, jsColumns?: string[], + fixedRadiusValue?: number | string | null, ): ScatterPoint[] { if (!spatial || !records.length) { return []; @@ -72,7 +73,15 @@ function processScatterData( extraProps: feature.extraProps || {}, }; - if (radiusMetricLabel && feature[radiusMetricLabel] != null) { + // Handle radius: either from metric or fixed value + if (fixedRadiusValue != null) { + // Use fixed radius value for all points + const parsedFixedRadius = parseMetricValue(fixedRadiusValue); + if (parsedFixedRadius !== undefined) { + scatterPoint.radius = parsedFixedRadius; + } + } else if (radiusMetricLabel && feature[radiusMetricLabel] != null) { + // Use metric value for radius const radiusValue = parseMetricValue(feature[radiusMetricLabel]); if (radiusValue !== undefined) { scatterPoint.radius = radiusValue; @@ -98,14 +107,20 @@ export default function transformProps(chartProps: ChartProps) { const { spatial, point_radius_fixed, dimension, js_columns } = formData as DeckScatterFormData; + // Check if this is a fixed value or metric + const isFixedValue = point_radius_fixed?.type === 'fix'; + const fixedRadiusValue = isFixedValue ? point_radius_fixed?.value : null; + const radiusMetricLabel = getMetricLabelFromFormData(point_radius_fixed); const records = getRecordsFromQuery(chartProps.queriesData); + const features = processScatterData( records, spatial, radiusMetricLabel, dimension, js_columns, + isFixedValue ? fixedRadiusValue : null, ); return createBaseTransformResult( diff --git a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/transformUtils.test.ts b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/transformUtils.test.ts new file mode 100644 index 0000000000..d54a84252b --- /dev/null +++ b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/transformUtils.test.ts @@ -0,0 +1,150 @@ +/** + * 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 { getMetricLabel } from '@superset-ui/core'; +import { getMetricLabelFromFormData, parseMetricValue } from './transformUtils'; + +jest.mock('@superset-ui/core', () => ({ + ...jest.requireActual('@superset-ui/core'), + getMetricLabel: jest.fn((metric: string) => metric), +})); + +beforeEach(() => { + jest.clearAllMocks(); +}); + +test('getMetricLabelFromFormData should return undefined for undefined input', () => { + const result = getMetricLabelFromFormData(undefined); + expect(result).toBeUndefined(); + }); + +test('getMetricLabelFromFormData should return undefined for null input', () => { + const result = getMetricLabelFromFormData(null as any); + expect(result).toBeUndefined(); + }); + +test('getMetricLabelFromFormData should handle string metric directly', () => { + const result = getMetricLabelFromFormData('AVG(value)'); + expect(result).toBe('AVG(value)'); + expect(getMetricLabel).toHaveBeenCalledWith('AVG(value)'); + }); + +test('getMetricLabelFromFormData should return undefined for fixed type', () => { + const result = getMetricLabelFromFormData({ + type: 'fix', + value: '1000', + }); + expect(result).toBeUndefined(); + expect(getMetricLabel).not.toHaveBeenCalled(); + }); + +test('getMetricLabelFromFormData should return undefined for fixed type with numeric value', () => { + const result = getMetricLabelFromFormData({ + type: 'fix', + value: 1000, + }); + expect(result).toBeUndefined(); + expect(getMetricLabel).not.toHaveBeenCalled(); + }); + +test('getMetricLabelFromFormData should return metric label for metric type with string value', () => { + const result = getMetricLabelFromFormData({ + type: 'metric', + value: 'SUM(amount)', + }); + expect(result).toBe('SUM(amount)'); + expect(getMetricLabel).toHaveBeenCalledWith('SUM(amount)'); + }); + +test('getMetricLabelFromFormData should return undefined for metric type with numeric value', () => { + const result = getMetricLabelFromFormData({ + type: 'metric', + value: 123, + }); + expect(result).toBeUndefined(); + expect(getMetricLabel).not.toHaveBeenCalled(); + }); + +test('getMetricLabelFromFormData should return undefined for object without type', () => { + const result = getMetricLabelFromFormData({ + value: 'AVG(value)', + }); + expect(result).toBeUndefined(); + }); + +test('getMetricLabelFromFormData should return undefined for empty object', () => { + const result = getMetricLabelFromFormData({}); + expect(result).toBeUndefined(); + }); + +test('getMetricLabelFromFormData should return undefined for metric type without value', () => { + const result = getMetricLabelFromFormData({ + type: 'metric', + }); + expect(result).toBeUndefined(); +}); + +test('parseMetricValue should parse numeric strings', () => { + expect(parseMetricValue('123')).toBe(123); + expect(parseMetricValue('123.45')).toBe(123.45); + expect(parseMetricValue('0')).toBe(0); + expect(parseMetricValue('-123')).toBe(-123); + }); + +test('parseMetricValue should handle numbers directly', () => { + expect(parseMetricValue(123)).toBe(123); + expect(parseMetricValue(123.45)).toBe(123.45); + expect(parseMetricValue(0)).toBe(0); + expect(parseMetricValue(-123)).toBe(-123); + }); + +test('parseMetricValue should return undefined for null', () => { + expect(parseMetricValue(null)).toBeUndefined(); + }); + +test('parseMetricValue should return undefined for undefined', () => { + expect(parseMetricValue(undefined)).toBeUndefined(); + }); + +test('parseMetricValue should return undefined for non-numeric strings', () => { + expect(parseMetricValue('abc')).toBeUndefined(); + expect(parseMetricValue('12a34')).toBe(12); // parseFloat returns 12 + expect(parseMetricValue('')).toBeUndefined(); + }); + +test('parseMetricValue should handle edge cases', () => { + expect(parseMetricValue('Infinity')).toBe(Infinity); + expect(parseMetricValue('-Infinity')).toBe(-Infinity); + expect(parseMetricValue('NaN')).toBeUndefined(); + }); + +test('parseMetricValue should handle boolean values', () => { + expect(parseMetricValue(true as any)).toBeUndefined(); + expect(parseMetricValue(false as any)).toBeUndefined(); + }); + +test('parseMetricValue should handle objects', () => { + expect(parseMetricValue({} as any)).toBeUndefined(); + expect(parseMetricValue({ value: 123 } as any)).toBeUndefined(); + }); + +test('parseMetricValue should handle arrays', () => { + expect(parseMetricValue([] as any)).toBeUndefined(); + expect(parseMetricValue([123] as any)).toBe(123); // String([123]) = '123' +}); \ No newline at end of file diff --git a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/transformUtils.ts b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/transformUtils.ts index 6427db900a..2602ce64a2 100644 --- a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/transformUtils.ts +++ b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/transformUtils.ts @@ -134,9 +134,13 @@ export function addPropertiesToFeature<T extends Record<string, unknown>>( } export function getMetricLabelFromFormData( - metric: string | { value?: string } | undefined, + metric: string | { value?: string | number; type?: string } | undefined, ): string | undefined { if (!metric) return undefined; if (typeof metric === 'string') return getMetricLabel(metric); - return metric.value ? getMetricLabel(metric.value) : undefined; + // Only return metric label if it's a metric type, not a fixed value + if (typeof metric === 'object' && metric.type === 'metric' && metric.value && typeof metric.value === 'string') { + return getMetricLabel(metric.value); + } + return undefined; }
