You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2023/05/23 22:40:26 UTC
[superset] 02/14: feat(generic-x-axis): add x sorting on series limit metric (#23274)
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 70313dd21c27f15590e4d9c681ec591333b1fab6
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Mon Mar 6 11:50:20 2023 +0200
feat(generic-x-axis): add x sorting on series limit metric (#23274)
(cherry picked from commit 1b139d074852e13c113a408a920991b2abc98387)
---
.../src/operators/pivotOperator.ts | 6 +-
.../src/operators/sortOperator.ts | 4 +-
.../utils/{index.ts => extractExtraMetrics.ts} | 27 +++++--
.../src/operators/utils/index.ts | 1 +
.../src/shared-controls/customControls.tsx | 83 ++++++++++---------
.../test/operators/pivotOperator.test.ts | 30 +++++++
.../test/operators/sortOperator.test.ts | 25 ++++++
.../operators/utils/extractExtraMetrics.test.ts | 94 ++++++++++++++++++++++
.../src/Timeseries/buildQuery.ts | 35 ++++----
.../src/Timeseries/transformProps.ts | 51 +++++++-----
.../plugin-chart-echarts/src/utils/series.ts | 3 +
11 files changed, 277 insertions(+), 82 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts
index 3adec29fec..2b8a2bd2f0 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts
@@ -24,12 +24,16 @@ import {
getXAxisLabel,
} from '@superset-ui/core';
import { PostProcessingFactory } from './types';
+import { extractExtraMetrics } from './utils';
export const pivotOperator: PostProcessingFactory<PostProcessingPivot> = (
formData,
queryObject,
) => {
- const metricLabels = ensureIsArray(queryObject.metrics).map(getMetricLabel);
+ const metricLabels = [
+ ...ensureIsArray(queryObject.metrics),
+ ...extractExtraMetrics(formData),
+ ].map(getMetricLabel);
const xAxisLabel = getXAxisLabel(formData);
const columns = queryObject.series_columns || queryObject.columns;
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts
index 0650c8b577..4d7b5deaf4 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts
@@ -26,6 +26,7 @@ import {
PostProcessingSort,
} from '@superset-ui/core';
import { PostProcessingFactory } from './types';
+import { extractExtraMetrics } from './utils';
export const sortOperator: PostProcessingFactory<PostProcessingSort> = (
formData,
@@ -34,7 +35,8 @@ export const sortOperator: PostProcessingFactory<PostProcessingSort> = (
// the sortOperator only used in the barchart v2
const sortableLabels = [
getXAxisLabel(formData),
- ...ensureIsArray(formData.metrics).map(metric => getMetricLabel(metric)),
+ ...ensureIsArray(formData.metrics).map(getMetricLabel),
+ ...extractExtraMetrics(formData).map(getMetricLabel),
].filter(Boolean);
if (
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/extractExtraMetrics.ts
similarity index 56%
copy from superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts
copy to superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/extractExtraMetrics.ts
index 8d65ca1e59..74928f836f 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/extractExtraMetrics.ts
@@ -1,4 +1,3 @@
-/* eslint-disable camelcase */
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
@@ -14,10 +13,26 @@
* 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 limitationsxw
+ * specific language governing permissions and limitations
* under the License.
*/
-export { getMetricOffsetsMap } from './getMetricOffsetsMap';
-export { isTimeComparison } from './isTimeComparison';
-export { isDerivedSeries } from './isDerivedSeries';
-export { TIME_COMPARISON_SEPARATOR } from './constants';
+import {
+ getMetricLabel,
+ QueryFormData,
+ QueryFormMetric,
+} from '@superset-ui/core';
+
+export function extractExtraMetrics(
+ formData: QueryFormData,
+): QueryFormMetric[] {
+ const { groupby, timeseries_limit_metric, x_axis_sort } = formData;
+ const extra_metrics: QueryFormMetric[] = [];
+ if (
+ !(groupby || []).length &&
+ timeseries_limit_metric &&
+ getMetricLabel(timeseries_limit_metric) === x_axis_sort
+ ) {
+ extra_metrics.push(timeseries_limit_metric);
+ }
+ return extra_metrics;
+}
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts
index 8d65ca1e59..67f230dd63 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts
@@ -20,4 +20,5 @@
export { getMetricOffsetsMap } from './getMetricOffsetsMap';
export { isTimeComparison } from './isTimeComparison';
export { isDerivedSeries } from './isDerivedSeries';
+export { extractExtraMetrics } from './extractExtraMetrics';
export { TIME_COMPARISON_SEPARATOR } from './constants';
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx
index 979912e58f..8e8f4d8400 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx
@@ -23,12 +23,16 @@ import {
getColumnLabel,
getMetricLabel,
isDefined,
- isEqualArray,
QueryFormColumn,
QueryFormMetric,
t,
} from '@superset-ui/core';
-import { ControlPanelState, ControlState, ControlStateMapping } from '../types';
+import {
+ ControlPanelState,
+ ControlState,
+ ControlStateMapping,
+ isDataset,
+} from '../types';
import { isTemporalColumn } from '../utils';
export const contributionModeControl = {
@@ -59,39 +63,42 @@ export const xAxisSortControl = {
name: 'x_axis_sort',
config: {
type: 'XAxisSortControl',
- label: t('X-Axis Sort By'),
- description: t('Whether to sort descending or ascending on the X-Axis.'),
- shouldMapStateToProps: (
- prevState: ControlPanelState,
- state: ControlPanelState,
- ) => {
- const prevOptions = [
- getColumnLabel(prevState?.controls?.x_axis?.value as QueryFormColumn),
- ...ensureIsArray(prevState?.controls?.metrics?.value).map(metric =>
- getMetricLabel(metric as QueryFormMetric),
- ),
- ];
- const currOptions = [
- getColumnLabel(state?.controls?.x_axis?.value as QueryFormColumn),
- ...ensureIsArray(state?.controls?.metrics?.value).map(metric =>
- getMetricLabel(metric as QueryFormMetric),
- ),
- ];
- return !isEqualArray(prevOptions, currOptions);
- },
- mapStateToProps: (
- { controls }: { controls: ControlStateMapping },
- controlState: ControlState,
- ) => {
- const choices = [
- getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
- ...ensureIsArray(controls?.metrics?.value).map(metric =>
- getMetricLabel(metric as QueryFormMetric),
- ),
+ label: (state: ControlPanelState) =>
+ state.form_data?.orientation === 'horizontal'
+ ? t('Y-Axis Sort By')
+ : t('X-Axis Sort By'),
+ description: t('Decides which column to sort the base axis by.'),
+ shouldMapStateToProps: () => true,
+ mapStateToProps: (state: ControlPanelState, controlState: ControlState) => {
+ const { controls, datasource } = state;
+ const dataset = isDataset(datasource) ? datasource : undefined;
+ const columns = [controls?.x_axis?.value as QueryFormColumn].filter(
+ Boolean,
+ );
+ const metrics = [
+ ...ensureIsArray(controls?.metrics?.value as QueryFormMetric),
+ controls?.timeseries_limit_metric?.value as QueryFormMetric,
].filter(Boolean);
+ const options = [
+ ...columns.map(column => {
+ const value = getColumnLabel(column);
+ return {
+ value,
+ label: dataset?.verbose_map?.[value] || value,
+ };
+ }),
+ ...metrics.map(metric => {
+ const value = getMetricLabel(metric);
+ return {
+ value,
+ label: dataset?.verbose_map?.[value] || value,
+ };
+ }),
+ ];
+
const shouldReset = !(
typeof controlState.value === 'string' &&
- choices.includes(controlState.value) &&
+ options.map(option => option.value).includes(controlState.value) &&
!isTemporalColumn(
getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
controls?.datasource?.datasource,
@@ -100,10 +107,7 @@ export const xAxisSortControl = {
return {
shouldReset,
- options: choices.map(entry => ({
- value: entry,
- label: entry,
- })),
+ options,
};
},
visibility: xAxisSortVisibility,
@@ -114,9 +118,12 @@ export const xAxisSortAscControl = {
name: 'x_axis_sort_asc',
config: {
type: 'CheckboxControl',
- label: t('X-Axis Sort Ascending'),
+ label: (state: ControlPanelState) =>
+ state.form_data?.orientation === 'horizontal'
+ ? t('Y-Axis Sort Ascending')
+ : t('X-Axis Sort Ascending'),
default: true,
- description: t('Whether to sort descending or ascending on the X-Axis.'),
+ description: t('Whether to sort ascending or descending on the base Axis.'),
visibility: xAxisSortVisibility,
},
};
diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/operators/pivotOperator.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/pivotOperator.test.ts
index 2a527a04fe..6101fc19e5 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/test/operators/pivotOperator.test.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/test/operators/pivotOperator.test.ts
@@ -185,3 +185,33 @@ test('pivot by adhoc x_axis', () => {
},
});
});
+
+test('pivot by x_axis with extra metrics', () => {
+ expect(
+ pivotOperator(
+ {
+ ...formData,
+ x_axis: 'foo',
+ x_axis_sort: 'bar',
+ groupby: [],
+ timeseries_limit_metric: 'bar',
+ },
+ {
+ ...queryObject,
+ series_columns: [],
+ },
+ ),
+ ).toEqual({
+ operation: 'pivot',
+ options: {
+ index: ['foo'],
+ columns: [],
+ aggregates: {
+ 'count(*)': { operator: 'mean' },
+ 'sum(val)': { operator: 'mean' },
+ bar: { operator: 'mean' },
+ },
+ drop_missing_columns: false,
+ },
+ });
+});
diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/operators/sortOperator.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/sortOperator.test.ts
index 750d726c90..41d44b50a8 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/test/operators/sortOperator.test.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/test/operators/sortOperator.test.ts
@@ -146,3 +146,28 @@ test('should sort by axis', () => {
},
});
});
+
+test('should sort by extra metric', () => {
+ Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', {
+ value: true,
+ });
+ expect(
+ sortOperator(
+ {
+ ...formData,
+ x_axis_sort: 'my_limit_metric',
+ x_axis_sort_asc: true,
+ x_axis: 'Categorical Column',
+ groupby: [],
+ timeseries_limit_metric: 'my_limit_metric',
+ },
+ queryObject,
+ ),
+ ).toEqual({
+ operation: 'sort',
+ options: {
+ by: 'my_limit_metric',
+ ascending: true,
+ },
+ });
+});
diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/extractExtraMetrics.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/extractExtraMetrics.test.ts
new file mode 100644
index 0000000000..89f4c11181
--- /dev/null
+++ b/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/extractExtraMetrics.test.ts
@@ -0,0 +1,94 @@
+/**
+ * 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, QueryFormMetric } from '@superset-ui/core';
+import { extractExtraMetrics } from '@superset-ui/chart-controls';
+
+const baseFormData: QueryFormData = {
+ datasource: 'dummy',
+ viz_type: 'table',
+ metrics: ['a', 'b'],
+ columns: ['foo', 'bar'],
+ limit: 100,
+ metrics_b: ['c', 'd'],
+ columns_b: ['hello', 'world'],
+ limit_b: 200,
+};
+
+const metric: QueryFormMetric = {
+ expressionType: 'SQL',
+ sqlExpression: 'case when 1 then 1 else 2 end',
+ label: 'foo',
+};
+
+test('returns empty array if relevant controls missing', () => {
+ expect(
+ extractExtraMetrics({
+ ...baseFormData,
+ }),
+ ).toEqual([]);
+});
+
+test('returns empty array if x_axis_sort is not same as timeseries_limit_metric', () => {
+ expect(
+ extractExtraMetrics({
+ ...baseFormData,
+ timeseries_limit_metric: 'foo',
+ x_axis_sort: 'bar',
+ }),
+ ).toEqual([]);
+});
+
+test('returns correct column if sort columns match', () => {
+ expect(
+ extractExtraMetrics({
+ ...baseFormData,
+ timeseries_limit_metric: 'foo',
+ x_axis_sort: 'foo',
+ }),
+ ).toEqual(['foo']);
+});
+
+test('handles adhoc metrics correctly', () => {
+ expect(
+ extractExtraMetrics({
+ ...baseFormData,
+ timeseries_limit_metric: metric,
+ x_axis_sort: 'foo',
+ }),
+ ).toEqual([metric]);
+
+ expect(
+ extractExtraMetrics({
+ ...baseFormData,
+ timeseries_limit_metric: metric,
+ x_axis_sort: 'bar',
+ }),
+ ).toEqual([]);
+});
+
+test('returns empty array if groupby populated', () => {
+ expect(
+ extractExtraMetrics({
+ ...baseFormData,
+ groupby: ['bar'],
+ timeseries_limit_metric: 'foo',
+ x_axis_sort: 'foo',
+ }),
+ ).toEqual([]);
+});
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts
index ad021f92b9..69a8020657 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts
@@ -19,24 +19,25 @@
import {
buildQueryContext,
ensureIsArray,
+ getXAxisColumn,
+ isXAxisSet,
normalizeOrderBy,
PostProcessingPivot,
QueryFormData,
- getXAxisColumn,
- isXAxisSet,
} from '@superset-ui/core';
import {
- rollingWindowOperator,
- timeCompareOperator,
+ contributionOperator,
+ extractExtraMetrics,
+ flattenOperator,
isTimeComparison,
pivotOperator,
- resampleOperator,
- renameOperator,
- contributionOperator,
prophetOperator,
- timeComparePivotOperator,
- flattenOperator,
+ renameOperator,
+ resampleOperator,
+ rollingWindowOperator,
sortOperator,
+ timeComparePivotOperator,
+ timeCompareOperator,
} from '@superset-ui/chart-controls';
export default function buildQuery(formData: QueryFormData) {
@@ -62,6 +63,9 @@ export default function buildQuery(formData: QueryFormData) {
2015-03-01 318.0 0.0
*/
+ // only add series limit metric if it's explicitly needed e.g. for sorting
+ const extra_metrics = extractExtraMetrics(formData);
+
const pivotOperatorInRuntime: PostProcessingPivot = isTimeComparison(
formData,
baseQueryObject,
@@ -69,15 +73,16 @@ export default function buildQuery(formData: QueryFormData) {
? timeComparePivotOperator(formData, baseQueryObject)
: pivotOperator(formData, baseQueryObject);
+ const columns = [
+ ...(isXAxisSet(formData) ? ensureIsArray(getXAxisColumn(formData)) : []),
+ ...ensureIsArray(groupby),
+ ];
+
return [
{
...baseQueryObject,
- columns: [
- ...(isXAxisSet(formData)
- ? ensureIsArray(getXAxisColumn(formData))
- : []),
- ...ensureIsArray(groupby),
- ],
+ metrics: [...(baseQueryObject.metrics || []), ...extra_metrics],
+ columns,
series_columns: groupby,
...(isXAxisSet(formData) ? {} : { is_timeseries: true }),
// todo: move `normalizeOrderBy to extractQueryFields`
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
index eadded44a9..52f472b3ff 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -19,21 +19,25 @@
/* eslint-disable camelcase */
import {
AnnotationLayer,
+ AxisType,
CategoricalColorNamespace,
GenericDataType,
+ getMetricLabel,
getNumberFormatter,
+ getXAxisLabel,
+ isDefined,
isEventAnnotationLayer,
isFormulaAnnotationLayer,
isIntervalAnnotationLayer,
+ isPhysicalColumn,
isTimeseriesAnnotationLayer,
- TimeseriesChartDataResponseResult,
t,
- AxisType,
- getXAxisLabel,
- isPhysicalColumn,
- isDefined,
+ TimeseriesChartDataResponseResult,
} from '@superset-ui/core';
-import { isDerivedSeries } from '@superset-ui/chart-controls';
+import {
+ extractExtraMetrics,
+ isDerivedSeries,
+} from '@superset-ui/chart-controls';
import { EChartsCoreOption, SeriesOption } from 'echarts';
import { ZRLineType } from 'echarts/types/src/util/types';
import {
@@ -114,39 +118,39 @@ export default function transformProps(
colorScheme,
contributionMode,
forecastEnabled,
+ groupby,
legendOrientation,
legendType,
legendMargin,
logAxis,
markerEnabled,
markerSize,
- opacity,
minorSplitLine,
+ onlyTotal,
+ opacity,
+ orientation,
+ percentageThreshold,
+ richTooltip,
seriesType,
showLegend,
+ showValue,
+ sliceId,
+ timeGrainSqla,
stack,
- truncateYAxis,
- yAxisFormat,
- xAxisTimeFormat,
- yAxisBounds,
tooltipTimeFormat,
tooltipSortByMetric,
- zoomable,
- richTooltip,
+ truncateYAxis,
xAxis: xAxisOrig,
xAxisLabelRotation,
- groupby,
- showValue,
- onlyTotal,
- percentageThreshold,
+ xAxisTimeFormat,
xAxisTitle,
- yAxisTitle,
xAxisTitleMargin,
+ yAxisBounds,
+ yAxisFormat,
+ yAxisTitle,
yAxisTitleMargin,
yAxisTitlePosition,
- sliceId,
- timeGrainSqla,
- orientation,
+ zoomable,
}: EchartsTimeseriesFormData = { ...DEFAULT_FORM_DATA, ...formData };
const refs: Refs = {};
@@ -168,9 +172,14 @@ export default function transformProps(
xAxisCol: xAxisLabel,
},
);
+ const extraMetricLabels = extractExtraMetrics(chartProps.rawFormData).map(
+ getMetricLabel,
+ );
+
const rawSeries = extractSeries(rebasedData, {
fillNeighborValue: stack && !forecastEnabled ? 0 : undefined,
xAxis: xAxisLabel,
+ extraMetricLabels,
removeNulls: seriesType === EchartsTimeseriesSeriesType.Scatter,
stack,
totalStackedValues,
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
index c1b61233b6..649dedd680 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
@@ -113,6 +113,7 @@ export function extractSeries(
opts: {
fillNeighborValue?: number;
xAxis?: string;
+ extraMetricLabels?: string[];
removeNulls?: boolean;
stack?: StackType;
totalStackedValues?: number[];
@@ -122,6 +123,7 @@ export function extractSeries(
const {
fillNeighborValue,
xAxis = DTTM_ALIAS,
+ extraMetricLabels = [],
removeNulls = false,
stack = false,
totalStackedValues = [],
@@ -135,6 +137,7 @@ export function extractSeries(
return Object.keys(rows[0])
.filter(key => key !== xAxis && key !== DTTM_ALIAS)
+ .filter(key => !extraMetricLabels.includes(key))
.map(key => ({
id: key,
name: key,