This is an automated email from the ASF dual-hosted git repository. elizabeth pushed a commit to branch elizabeth/fix-subtitle in repository https://gitbox.apache.org/repos/asf/superset.git
commit 26213e996b7a7333c008fbd1ccca7460c1efb606 Author: Elizabeth Thompson <[email protected]> AuthorDate: Thu Apr 17 16:26:17 2025 -0700 fix subtitle --- .../BigNumber/BigNumberTotal/controlPanel.test.ts | 79 ++++++++ .../BigNumberTotal/transformProps.test.ts | 209 +++++++++++++++++++++ .../src/BigNumber/BigNumberTotal/transformProps.ts | 11 +- .../src/BigNumber/sharedControls.ts | 3 + 4 files changed, 297 insertions(+), 5 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/controlPanel.test.ts b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/controlPanel.test.ts new file mode 100644 index 0000000000..72bb906493 --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/controlPanel.test.ts @@ -0,0 +1,79 @@ +import { SqlaFormData } from '@superset-ui/core'; +import * as ChartControls from '@superset-ui/chart-controls'; +import controlPanel from './controlPanel'; + +const { __mockShiftMetric } = ChartControls as any; + +jest.mock('@superset-ui/core', () => ({ + GenericDataType: { Numeric: 'numeric' }, + SMART_DATE_ID: 'SMART_DATE_ID', + t: (str: string) => str, +})); + +jest.mock('@superset-ui/chart-controls', () => { + // Define the mock function inside the factory + const mockShiftMetric = jest.fn(() => 'shiftedMetric'); + return { + ControlPanelConfig: {}, + D3_FORMAT_DOCS: 'Format docs', + D3_TIME_FORMAT_OPTIONS: [['', 'default']], + getStandardizedControls: () => ({ + shiftMetric: mockShiftMetric, + }), + // Optional export to let tests access the mock + __mockShiftMetric: mockShiftMetric, + }; +}); + +describe('BigNumber Total Control Panel Config', () => { + it('should have the required control panel sections', () => { + expect(controlPanel).toHaveProperty('controlPanelSections'); + const sections = controlPanel.controlPanelSections; + expect(Array.isArray(sections)).toBe(true); + expect(sections.length).toBe(2); + + // First section should have label 'Query' and contain rows with metric and adhoc_filters + expect(sections[0]!.label).toBe('Query'); + expect(Array.isArray(sections[0]!.controlSetRows)).toBe(true); + expect(sections[0]!.controlSetRows[0]).toEqual(['metric']); + expect(sections[0]!.controlSetRows[1]).toEqual(['adhoc_filters']); + + // Second section should contain a control named subheader + const secondSectionRow = sections[1]!.controlSetRows[0]; + expect(secondSectionRow[0]).toHaveProperty('name', 'subheader'); + + // Third section should include controls for time_format and conditional_formatting + const thirdSection = sections[2]!.controlSetRows; + // Check time_format control exists in one of the rows + const timeFormatRow = thirdSection.find(row => + row.some((control: any) => control.name === 'time_format'), + ); + expect(timeFormatRow).toBeTruthy(); + // Check conditional_formatting control exists in one of the rows + const conditionalFormattingRow = thirdSection.find(row => + row.some((control: any) => control.name === 'conditional_formatting'), + ); + expect(conditionalFormattingRow).toBeTruthy(); + }); + + it('should have y_axis_format override with correct label', () => { + expect(controlPanel).toHaveProperty('controlOverrides'); + expect(controlPanel.controlOverrides).toHaveProperty('y_axis_format'); + expect(controlPanel.controlOverrides!.y_axis_format!.label).toBe( + 'Number format', + ); + }); + + it('should override formData metric using getStandardizedControls', () => { + const dummyFormData = { someProp: 'test' } as unknown as SqlaFormData; + const newFormData = controlPanel.formDataOverrides!(dummyFormData); + + // The original properties are spread correctly. + expect(newFormData.someProp).toBe('test'); + // The metric property should be replaced by the output of shiftMetric. + expect(newFormData.metric).toBe('shiftedMetric'); + + // Ensure that the mockShiftMetric function was called. + expect(__mockShiftMetric).toHaveBeenCalled(); + }); +}); diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.test.ts new file mode 100644 index 0000000000..0c831a7866 --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.test.ts @@ -0,0 +1,209 @@ +/** + * 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 { GenericDataType } from '@superset-ui/core'; +import { getColorFormatters } from '@superset-ui/chart-controls'; +import { BigNumberTotalChartProps } from '../types'; +import transformProps from './transformProps'; + +jest.mock('@superset-ui/chart-controls', () => ({ + getColorFormatters: jest.fn(), +})); + +jest.mock('@superset-ui/core', () => ({ + GenericDataType: { Temporal: 2, String: 1 }, + getMetricLabel: jest.fn(metric => metric), + extractTimegrain: jest.fn(() => 'P1D'), + getValueFormatter: jest.fn(() => (v: any) => `num-${v}`), +})); + +jest.mock('../utils', () => ({ + getDateFormatter: jest.fn(() => (v: any) => `date-${v}`), + parseMetricValue: jest.fn(val => Number(val)), +})); + +describe('BigNumberTotal transformProps', () => { + const onContextMenu = jest.fn(); + const baseFormData = { + headerFontSize: 20, + metric: 'value', + subheader: 'sub header text', + subheaderFontSize: 14, + forceTimestampFormatting: false, + timeFormat: 'YYYY-MM-DD', + yAxisFormat: 'SMART_NUMBER', + conditionalFormatting: [{ color: 'red', op: '>', value: 0 }], + currencyFormat: { symbol: '$', symbolPosition: 'prefix' }, + }; + + const baseDatasource = { + currencyFormats: { value: '$0,0.00' }, + columnFormats: { value: '$0,0.00' }, + metrics: [{ metric_name: 'value', d3format: '.2f' }], + }; + + const baseHooks = { onContextMenu }; + + const baseRawFormData = { dummy: 'raw' }; + + it('should return null bigNumber when no data is provided', () => { + const chartProps = { + width: 400, + height: 300, + queriesData: [{ data: [], coltypes: [] }], + formData: baseFormData, + rawFormData: baseRawFormData, + hooks: baseHooks, + datasource: baseDatasource, + }; + + const result = transformProps( + chartProps as unknown as BigNumberTotalChartProps, + ); + expect(result.bigNumber).toBeNull(); + expect(result.width).toBe(400); + expect(result.height).toBe(300); + expect(result.subtitle).toBe(baseFormData.subheader); + expect(result.onContextMenu).toBe(onContextMenu); + expect(result.refs).toEqual({}); + // headerFormatter should be set even if there's no data + expect(typeof result.headerFormatter).toBe('function'); + // colorThresholdFormatters fallback to empty array when getColorFormatters returns falsy + expect(result.colorThresholdFormatters).toEqual([]); + }); + it('should convert subheader to subtitle', () => { + const chartProps = { + width: 400, + height: 300, + queriesData: [{ data: [], coltypes: [] }], + formData: { ...baseFormData, subheader: 'test' }, + rawFormData: baseRawFormData, + hooks: baseHooks, + datasource: baseDatasource, + }; + const result = transformProps( + chartProps as unknown as BigNumberTotalChartProps, + ); + expect(result.subtitle).toBe('test'); + }); + + it('should compute bigNumber using parseMetricValue when data exists', () => { + const chartProps = { + width: 500, + height: 400, + queriesData: [ + { data: [{ value: '456' }], coltypes: [GenericDataType.String] }, + ], + formData: { ...baseFormData, forceTimestampFormatting: false }, + rawFormData: baseRawFormData, + hooks: baseHooks, + datasource: baseDatasource, + }; + + const result = transformProps(chartProps); + // parseMetricValue converts '456' to number 456 by our mock + expect(result.bigNumber).toEqual(456); + }); + + it('should use formatTime as headerFormatter for Temporal or String types or forced formatting', () => { + // Case 1: Temporal type + const chartPropsTemporal = { + width: 600, + height: 450, + queriesData: [ + { data: [{ value: '789' }], coltypes: [GenericDataType.Temporal] }, + ], + formData: { ...baseFormData, forceTimestampFormatting: false }, + rawFormData: baseRawFormData, + hooks: baseHooks, + datasource: baseDatasource, + }; + + const resultTemporal = transformProps(chartPropsTemporal); + // getDateFormatter returns function that prefixes with "date-" + expect(resultTemporal.headerFormatter('test')).toBe('date-test'); + + // Case 2: String type regardless of forcing formatting + const chartPropsString = { + width: 600, + height: 450, + queriesData: [ + { data: [{ value: '789' }], coltypes: [GenericDataType.String] }, + ], + formData: { ...baseFormData, forceTimestampFormatting: false }, + rawFormData: baseRawFormData, + hooks: baseHooks, + datasource: baseDatasource, + }; + + const resultString = transformProps(chartPropsString); + expect(resultString.headerFormatter('test')).toBe('date-test'); + + // Case 3: Forced timestamp formatting + const chartPropsForced = { + width: 600, + height: 450, + queriesData: [{ data: [{ value: '789' }], coltypes: [0] }], // non-temporal/non-string + formData: { ...baseFormData, forceTimestampFormatting: true }, + rawFormData: baseRawFormData, + hooks: baseHooks, + datasource: baseDatasource, + }; + + const resultForced = transformProps(chartPropsForced); + expect(resultForced.headerFormatter('test')).toBe('date-test'); + }); + + it('should use numberFormatter as headerFormatter when not Temporal/String and no forced formatting', () => { + const chartProps = { + width: 700, + height: 500, + queriesData: [{ data: [{ value: '321' }], coltypes: [0] }], // non-temporal/non-string + formData: { ...baseFormData, forceTimestampFormatting: false }, + rawFormData: baseRawFormData, + hooks: baseHooks, + datasource: baseDatasource, + }; + + const result = transformProps(chartProps); + // getValueFormatter mock returns function that prefixes with "num-" + expect(result.headerFormatter('test')).toBe('num-test'); + }); + + it('should propagate colorThresholdFormatters from getColorFormatters', () => { + // Override the getColorFormatters mock to return specific value + const mockFormatters = [{ formatter: 'red' }]; + (getColorFormatters as jest.Mock).mockReturnValueOnce(mockFormatters); + + const chartProps = { + width: 800, + height: 600, + queriesData: [ + { data: [{ value: '100' }], coltypes: [GenericDataType.Temporal] }, + ], + formData: baseFormData, + rawFormData: baseRawFormData, + hooks: baseHooks, + datasource: baseDatasource, + }; + + const result = transformProps(chartProps); + expect(result.colorThresholdFormatters).toEqual(mockFormatters); + }); +}); diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.ts index c673ebd9f5..e892ca33f7 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.ts @@ -47,19 +47,22 @@ export default function transformProps( const { headerFontSize, metric = 'value', - subtitle = '', + subtitle, subtitleFontSize, forceTimestampFormatting, timeFormat, yAxisFormat, conditionalFormatting, currencyFormat, + subheader, + subheaderFontSize, } = formData; const refs: Refs = {}; const { data = [], coltypes = [] } = queriesData[0]; const granularity = extractTimegrain(rawFormData as QueryFormData); const metricName = getMetricLabel(metric); - const formattedSubtitle = subtitle; + const formattedSubtitle = subtitle || subheader || ''; + const formattedSubtitleFontSize = subheaderFontSize || subtitleFontSize; const bigNumber = data.length === 0 ? null : parseMetricValue(data[0][metricName]); @@ -105,10 +108,8 @@ export default function transformProps( bigNumber, headerFormatter, headerFontSize, - subtitleFontSize, + subtitleFontSize: formattedSubtitleFontSize, subtitle: formattedSubtitle, - subheader: '', - subheaderFontSize: subtitleFontSize, onContextMenu, refs, colorThresholdFormatters, diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/sharedControls.ts b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/sharedControls.ts index 09766ed4bf..9de4065b67 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/sharedControls.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/sharedControls.ts @@ -129,5 +129,8 @@ export const subtitleControl: CustomControlItem = { label: t('Subtitle'), renderTrigger: true, description: t('Description text that shows up below your Big Number'), + mapStateToProps: state => ({ + value: state.form_data.subheader || state.form_data.subtitle, + }), }, };
