From fa1693dc5fdbcd67bb524c7228ecbc522072cb94 Mon Sep 17 00:00:00 2001 From: Vladislav Korenkov <73882772+Quatters@users.noreply.github.com> Date: Thu, 15 May 2025 04:20:30 +1000 Subject: [PATCH] feat(Pie Chart): threshold for Other (#33348) --- .../src/Pie/buildQuery.ts | 18 +++- .../plugin-chart-echarts/src/Pie/constants.ts | 19 ++++ .../src/Pie/controlPanel.tsx | 17 ++++ .../src/Pie/transformProps.ts | 87 +++++++++++++++---- .../plugin-chart-echarts/src/Pie/types.ts | 12 +++ .../plugin-chart-echarts/src/Pie/utils.ts | 22 +++++ .../test/Pie/transformProps.test.ts | 85 +++++++++++++++++- .../NumberControl/NumberControl.test.tsx | 67 ++++++++++++++ .../controls/NumberControl/index.tsx | 78 +++++++++++++++++ .../src/explore/components/controls/index.js | 2 + 10 files changed, 387 insertions(+), 20 deletions(-) create mode 100644 superset-frontend/plugins/plugin-chart-echarts/src/Pie/constants.ts create mode 100644 superset-frontend/plugins/plugin-chart-echarts/src/Pie/utils.ts create mode 100644 superset-frontend/src/explore/components/controls/NumberControl/NumberControl.test.tsx create mode 100644 superset-frontend/src/explore/components/controls/NumberControl/index.tsx diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/buildQuery.ts index 8b47fb5e72..6985eb9286 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/buildQuery.ts @@ -16,14 +16,30 @@ * specific language governing permissions and limitations * under the License. */ -import { buildQueryContext, QueryFormData } from '@superset-ui/core'; +import { + buildQueryContext, + getMetricLabel, + QueryFormData, +} from '@superset-ui/core'; +import { getContributionLabel } from './utils'; export default function buildQuery(formData: QueryFormData) { const { metric, sort_by_metric } = formData; + const metricLabel = getMetricLabel(metric); + return buildQueryContext(formData, baseQueryObject => [ { ...baseQueryObject, ...(sort_by_metric && { orderby: [[metric, false]] }), + post_processing: [ + { + operation: 'contribution', + options: { + columns: [metricLabel], + rename_columns: [getContributionLabel(metricLabel)], + }, + }, + ], }, ]); } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/constants.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/constants.ts new file mode 100644 index 0000000000..4724bf1f26 --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/constants.ts @@ -0,0 +1,19 @@ +/** + * 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. + */ +export const CONTRIBUTION_SUFFIX = '__contribution' as const; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/controlPanel.tsx index 583444508d..4f93c0ed2c 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/controlPanel.tsx @@ -84,6 +84,23 @@ const config: ControlPanelConfig = { }, }, ], + [ + { + name: 'threshold_for_other', + config: { + type: 'NumberControl', + label: t('Threshold for Other'), + min: 0, + step: 0.5, + max: 100, + default: 0, + renderTrigger: true, + description: t( + 'Values less than this percentage will be grouped into the Other category.', + ), + }, + }, + ], [ { name: 'roseType', diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts index b02f86b232..c0ec8f9242 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts @@ -27,6 +27,7 @@ import { ValueFormatter, getValueFormatter, tooltipHtml, + DataRecord, } from '@superset-ui/core'; import type { CallbackDataParams } from 'echarts/types/src/util/types'; import type { EChartsCoreOption } from 'echarts/core'; @@ -36,6 +37,7 @@ import { EchartsPieChartProps, EchartsPieFormData, EchartsPieLabelType, + PieChartDataItem, PieChartTransformedProps, } from './types'; import { DEFAULT_LEGEND_FORM_DATA, OpacityEnum } from '../constants'; @@ -50,6 +52,7 @@ import { defaultGrid } from '../defaults'; import { convertInteger } from '../utils/convertInteger'; import { getDefaultTooltip } from '../utils/tooltip'; import { Refs } from '../types'; +import { getContributionLabel } from './utils'; const percentFormatter = getNumberFormatter(NumberFormats.PERCENT_2_POINT); @@ -133,7 +136,7 @@ export default function transformProps( datasource, } = chartProps; const { columnFormats = {}, currencyFormats = {} } = datasource; - const { data = [] } = queriesData[0]; + const { data: rawData = [] } = queriesData[0]; const coltypeMapping = getColtypesMapping(queriesData[0]); const { @@ -159,6 +162,7 @@ export default function transformProps( sliceId, showTotal, roseType, + thresholdForOther, }: EchartsPieFormData = { ...DEFAULT_LEGEND_FORM_DATA, ...DEFAULT_PIE_FORM_DATA, @@ -166,17 +170,68 @@ export default function transformProps( }; const refs: Refs = {}; const metricLabel = getMetricLabel(metric); + const contributionLabel = getContributionLabel(metricLabel); const groupbyLabels = groupby.map(getColumnLabel); const minShowLabelAngle = (showLabelsThreshold || 0) * 3.6; - const keys = data.map(datum => - extractGroupbyLabel({ - datum, - groupby: groupbyLabels, - coltypeMapping, - timeFormatter: getTimeFormatter(dateFormat), - }), + const numberFormatter = getValueFormatter( + metric, + currencyFormats, + columnFormats, + numberFormat, + currencyFormat, ); + + let data = rawData; + const otherRows: DataRecord[] = []; + const otherTooltipData: string[][] = []; + let otherDatum: PieChartDataItem | null = null; + let otherSum = 0; + if (thresholdForOther) { + let contributionSum = 0; + data = data.filter(datum => { + const contribution = datum[contributionLabel] as number; + if (!contribution || contribution * 100 >= thresholdForOther) { + return true; + } + otherSum += datum[metricLabel] as number; + contributionSum += contribution; + otherRows.push(datum); + otherTooltipData.push([ + extractGroupbyLabel({ + datum, + groupby: groupbyLabels, + coltypeMapping, + timeFormatter: getTimeFormatter(dateFormat), + }), + numberFormatter(datum[metricLabel] as number), + percentFormatter(contribution), + ]); + return false; + }); + const otherName = t('Other'); + otherTooltipData.push([ + t('Total'), + numberFormatter(otherSum), + percentFormatter(contributionSum), + ]); + if (otherSum) { + otherDatum = { + name: otherName, + value: otherSum, + itemStyle: { + color: theme.colors.grayscale.dark1, + opacity: + filterState.selectedValues && + !filterState.selectedValues.includes(otherName) + ? OpacityEnum.SemiTransparent + : OpacityEnum.NonTransparent, + }, + isOther: true, + }; + } + } + const labelMap = data.reduce((acc: Record<string, string[]>, datum) => { const label = extractGroupbyLabel({ datum, @@ -192,13 +247,6 @@ export default function transformProps( const { setDataMask = () => {}, onContextMenu } = hooks; const colorFn = CategoricalColorNamespace.getScale(colorScheme as string); - const numberFormatter = getValueFormatter( - metric, - currencyFormats, - columnFormats, - numberFormat, - currencyFormat, - ); let totalValue = 0; @@ -229,6 +277,10 @@ export default function transformProps( }, }; }); + if (otherDatum) { + transformedData.push(otherDatum); + totalValue += otherSum; + } const selectedValues = (filterState.selectedValues || []).reduce( (acc: Record<string, number>, selectedValue: string) => { @@ -372,6 +424,9 @@ export default function transformProps( numberFormatter, sanitizeName: true, }); + if (params?.data?.isOther) { + return tooltipHtml(otherTooltipData, name); + } return tooltipHtml( [[metricLabel, formattedValue, formattedPercent]], name, @@ -380,7 +435,7 @@ export default function transformProps( }, legend: { ...getLegendProps(legendType, legendOrientation, showLegend, theme), - data: keys, + data: transformedData.map(datum => datum.name), }, graphic: showTotal ? { diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/types.ts index 06ff2bc41a..c684dcf457 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/types.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/types.ts @@ -47,6 +47,7 @@ export type EchartsPieFormData = QueryFormData & dateFormat: string; showLabelsThreshold: number; roseType: 'radius' | 'area' | null; + thresholdForOther: number; }; export enum EchartsPieLabelType { @@ -82,9 +83,20 @@ export const DEFAULT_FORM_DATA: EchartsPieFormData = { showLabelsThreshold: 5, dateFormat: 'smart_date', roseType: null, + thresholdForOther: 0, }; export type PieChartTransformedProps = BaseTransformedProps<EchartsPieFormData> & ContextMenuTransformedProps & CrossFilterTransformedProps; + +export interface PieChartDataItem { + name: string; + value: number; + itemStyle: { + color: string; + opacity: number; + }; + isOther?: boolean; +} diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/utils.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/utils.ts new file mode 100644 index 0000000000..36b67e4320 --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/utils.ts @@ -0,0 +1,22 @@ +/** + * 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 { CONTRIBUTION_SUFFIX } from './constants'; + +export const getContributionLabel = (metricLabel: string) => + `${metricLabel}${CONTRIBUTION_SUFFIX}`; diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Pie/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Pie/transformProps.test.ts index 34f8e5fd56..36fefc1f49 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Pie/transformProps.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Pie/transformProps.test.ts @@ -28,7 +28,7 @@ import type { CallbackDataParams, } from 'echarts/types/src/util/types'; import transformProps, { parseParams } from '../../src/Pie/transformProps'; -import { EchartsPieChartProps } from '../../src/Pie/types'; +import { EchartsPieChartProps, PieChartDataItem } from '../../src/Pie/types'; describe('Pie transformProps', () => { const formData: SqlaFormData = { @@ -46,8 +46,13 @@ describe('Pie transformProps', () => { queriesData: [ { data: [ - { foo: 'Sylvester', bar: 1, sum__num: 10 }, - { foo: 'Arnold', bar: 2, sum__num: 2.5 }, + { + foo: 'Sylvester', + bar: 1, + sum__num: 10, + sum__num__contribution: 0.8, + }, + { foo: 'Arnold', bar: 2, sum__num: 2.5, sum__num__contribution: 0.2 }, ], }, ], @@ -215,3 +220,77 @@ describe('Pie label string template', () => { ).toEqual('Tablet:123456\n55.5'); }); }); + +describe('Other category', () => { + const defaultFormData: SqlaFormData = { + colorScheme: 'bnbColors', + datasource: '3__table', + granularity_sqla: 'ds', + metric: 'metric', + groupby: ['foo', 'bar'], + viz_type: 'my_viz', + }; + + const getChartProps = (formData: Partial<SqlaFormData>) => + new ChartProps({ + formData: { + ...defaultFormData, + ...formData, + }, + width: 800, + height: 600, + queriesData: [ + { + data: [ + { + foo: 'foo 1', + bar: 'bar 1', + metric: 1, + metric__contribution: 1 / 15, // 6.7% + }, + { + foo: 'foo 2', + bar: 'bar 2', + metric: 2, + metric__contribution: 2 / 15, // 13.3% + }, + { + foo: 'foo 3', + bar: 'bar 3', + metric: 3, + metric__contribution: 3 / 15, // 20% + }, + { + foo: 'foo 4', + bar: 'bar 4', + metric: 4, + metric__contribution: 4 / 15, // 26.7% + }, + { + foo: 'foo 5', + bar: 'bar 5', + metric: 5, + metric__contribution: 5 / 15, // 33.3% + }, + ], + }, + ], + theme: supersetTheme, + }); + + it('generates Other category', () => { + const chartProps = getChartProps({ + threshold_for_other: 20, + }); + const transformed = transformProps(chartProps as EchartsPieChartProps); + const series = transformed.echartOptions.series as PieSeriesOption[]; + const data = series[0].data as PieChartDataItem[]; + expect(data).toHaveLength(4); + expect(data[0].value).toBe(3); + expect(data[1].value).toBe(4); + expect(data[2].value).toBe(5); + expect(data[3].value).toBe(1 + 2); + expect(data[3].name).toBe('Other'); + expect(data[3].isOther).toBe(true); + }); +}); diff --git a/superset-frontend/src/explore/components/controls/NumberControl/NumberControl.test.tsx b/superset-frontend/src/explore/components/controls/NumberControl/NumberControl.test.tsx new file mode 100644 index 0000000000..92e41f437a --- /dev/null +++ b/superset-frontend/src/explore/components/controls/NumberControl/NumberControl.test.tsx @@ -0,0 +1,67 @@ +/** + * 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 { render, screen, userEvent } from 'spec/helpers/testing-library'; +import NumberControl from '.'; + +const mockedProps = { + min: -5, + max: 10, + step: 1, + default: 0, +}; + +test('render', () => { + const { container } = render(<NumberControl {...mockedProps} />); + expect(container).toBeInTheDocument(); +}); + +test('type number', async () => { + const props = { + ...mockedProps, + onChange: jest.fn(), + }; + render(<NumberControl {...props} />); + const input = screen.getByRole('spinbutton'); + await userEvent.type(input, '9'); + expect(props.onChange).toHaveBeenCalledTimes(1); + expect(props.onChange).toHaveBeenLastCalledWith(9); +}); + +test('type >max', async () => { + const props = { + ...mockedProps, + onChange: jest.fn(), + }; + render(<NumberControl {...props} />); + const input = screen.getByRole('spinbutton'); + await userEvent.type(input, '20'); + expect(props.onChange).toHaveBeenCalledTimes(1); + expect(props.onChange).toHaveBeenLastCalledWith(2); +}); + +test('type NaN', async () => { + const props = { + ...mockedProps, + onChange: jest.fn(), + }; + render(<NumberControl {...props} />); + const input = screen.getByRole('spinbutton'); + await userEvent.type(input, 'not a number'); + expect(props.onChange).toHaveBeenCalledTimes(0); +}); diff --git a/superset-frontend/src/explore/components/controls/NumberControl/index.tsx b/superset-frontend/src/explore/components/controls/NumberControl/index.tsx new file mode 100644 index 0000000000..e06fd0f426 --- /dev/null +++ b/superset-frontend/src/explore/components/controls/NumberControl/index.tsx @@ -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 { styled } from '@superset-ui/core'; +import { InputNumber } from 'src/components/Input'; +import ControlHeader, { ControlHeaderProps } from '../../ControlHeader'; + +type NumberValueType = number | undefined; + +export interface NumberControlProps extends ControlHeaderProps { + onChange?: (value: NumberValueType) => void; + value?: NumberValueType; + label?: string; + description?: string; + min?: number; + max?: number; + step?: number; + placeholder?: string; + disabled?: boolean; +} + +const FullWidthDiv = styled.div` + width: 100%; +`; + +const FullWidthInputNumber = styled(InputNumber)` + width: 100%; +`; + +function parseValue(value: string | number | null | undefined) { + if (value === null || value === undefined || value === '') { + return undefined; + } + const num = Number(value); + return Number.isNaN(num) ? undefined : num; +} + +export default function NumberControl({ + min, + max, + step, + placeholder, + value, + onChange, + disabled, + ...rest +}: NumberControlProps) { + return ( + <FullWidthDiv> + <ControlHeader {...rest} /> + <FullWidthInputNumber + min={min} + max={max} + step={step} + placeholder={placeholder} + value={value} + onChange={value => onChange?.(parseValue(value))} + disabled={disabled} + aria-label={rest.label} + /> + </FullWidthDiv> + ); +} diff --git a/superset-frontend/src/explore/components/controls/index.js b/superset-frontend/src/explore/components/controls/index.js index 055093a72e..95dd6f25f0 100644 --- a/superset-frontend/src/explore/components/controls/index.js +++ b/superset-frontend/src/explore/components/controls/index.js @@ -53,6 +53,7 @@ import { ComparisonRangeLabel } from './ComparisonRangeLabel'; import LayerConfigsControl from './LayerConfigsControl/LayerConfigsControl'; import MapViewControl from './MapViewControl/MapViewControl'; import ZoomConfigControl from './ZoomConfigControl/ZoomConfigControl'; +import NumberControl from './NumberControl'; const controlMap = { AnnotationLayerControl, @@ -90,6 +91,7 @@ const controlMap = { ComparisonRangeLabel, TimeOffsetControl, ZoomConfigControl, + NumberControl, ...sharedControlComponents, }; export default controlMap; -- GitLab