feat(generic-x-axis): add x sorting on series limit metric (apache#23274

)

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;
 

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 (

superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/extractExtraMetrics.ts

@@ -0,0 +1,38 @@
+/**
+ * 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,
+ 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;
+}

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';

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,
  },
 };

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,
+ },
+ });
+});

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,
+ },
+ });
+});

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([]);
+});