Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Axis sort in the Bar Chart V2 #21993

Merged
merged 14 commits into from
Nov 26, 2022
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -17,25 +16,50 @@
* specific language governing permissions and limitationsxw
* under the License.
*/
import { DTTM_ALIAS, PostProcessingSort, RollingType } from '@superset-ui/core';
import { isEmpty } from 'lodash';
import {
ensureIsArray,
getMetricLabel,
getXAxisLabel,
hasGenericChartAxes,
isDefined,
PostProcessingSort,
} from '@superset-ui/core';
import { PostProcessingFactory } from './types';

export const sortOperator: PostProcessingFactory<PostProcessingSort> = (
formData,
queryObject,
) => {
const { x_axis: xAxis } = formData;
// the sortOperator only used in the barchart v2
const labelsInMetricsAndXAxis = [
zhaoyongjie marked this conversation as resolved.
Show resolved Hide resolved
getXAxisLabel(formData),
...ensureIsArray(formData.metrics).map(metric => getMetricLabel(metric)),
].filter(Boolean);

if (
(xAxis || queryObject.is_timeseries) &&
Object.values(RollingType).includes(formData.rolling_type)
hasGenericChartAxes &&
isDefined(formData?.x_axis_sort) &&
isDefined(formData?.x_axis_sort_asc) &&
labelsInMetricsAndXAxis.includes(formData.x_axis_sort) &&
// the sort operator doesn't support sort-by multiple series.
isEmpty(formData.groupby)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought for the future: As formData will always be unstandardized (can contain more or less any property names), it will be difficult to harmonize functionality based on formData. So at some point we should move to using a base queryObject, which has already been converted to specific format (columns for dimensional grouping).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly, these operators are used to "build" part of Query Object. I recently found that we might only use formData to generate an operator and don't use the QueryObject.

However, we need to consider more and more to see how to make an efficient and convenient abstraction on the queryObject generation.

) {
const index = xAxis || DTTM_ALIAS;
if (formData.x_axis_sort === getXAxisLabel(formData)) {
return {
operation: 'sort',
options: {
is_sort_index: true,
ascending: formData.x_axis_sort_asc,
},
};
}

return {
operation: 'sort',
options: {
columns: {
[index]: true,
},
by: formData.x_axis_sort,
ascending: formData.x_axis_sort_asc,
},
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,41 +16,47 @@
* specific language governing permissions and limitations
* under the License.
*/
import { ContributionType, hasGenericChartAxes, t } from '@superset-ui/core';
import { ControlPanelSectionConfig } from '../types';
import { emitFilterControl } from '../shared-controls';
import { hasGenericChartAxes, t } from '@superset-ui/core';
import { ControlPanelSectionConfig, ControlSetRow } from '../types';
import {
contributionModeControl,
emitFilterControl,
xAxisSortControl,
xAxisSortAscControl,
} from '../shared-controls';

const controlsWithoutXAxis: ControlSetRow[] = [
['metrics'],
['groupby'],
[contributionModeControl],
['adhoc_filters'],
emitFilterControl,
['limit'],
['timeseries_limit_metric'],
['order_desc'],
['row_limit'],
['truncate_metric'],
['show_empty_columns'],
];

export const echartsTimeSeriesQuery: ControlPanelSectionConfig = {
label: t('Query'),
expanded: true,
controlSetRows: [
[hasGenericChartAxes ? 'x_axis' : null],
[hasGenericChartAxes ? 'time_grain_sqla' : null],
['metrics'],
['groupby'],
[
{
name: 'contributionMode',
config: {
type: 'SelectControl',
label: t('Contribution Mode'),
default: null,
choices: [
[null, t('None')],
[ContributionType.Row, t('Row')],
[ContributionType.Column, t('Series')],
],
description: t('Calculate contribution per series or row'),
},
Comment on lines -32 to -44
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move contributionMode into CustomControls.ts

},
],
['adhoc_filters'],
emitFilterControl,
['limit'],
['timeseries_limit_metric'],
['order_desc'],
['row_limit'],
['truncate_metric'],
['show_empty_columns'],
...controlsWithoutXAxis,
],
};

export const echartsTimeSeriesQueryWithXAxisSort: ControlPanelSectionConfig = {
label: t('Query'),
expanded: true,
controlSetRows: [
[hasGenericChartAxes ? 'x_axis' : null],
[hasGenericChartAxes ? 'time_grain_sqla' : null],
[hasGenericChartAxes ? xAxisSortControl : null],
[hasGenericChartAxes ? xAxisSortAscControl : null],
...controlsWithoutXAxis,
],
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,21 @@
* under the License.
*/

import { FeatureFlag, isFeatureEnabled, t } from '@superset-ui/core';
import {
ContributionType,
ensureIsArray,
FeatureFlag,
getColumnLabel,
getMetricLabel,
isDefined,
isEqualArray,
isFeatureEnabled,
QueryFormColumn,
QueryFormMetric,
t,
} from '@superset-ui/core';
import { ControlPanelState, ControlState, ControlStateMapping } from '../types';
import { isTemporalColumn } from '../utils';

export const emitFilterControl = isFeatureEnabled(
FeatureFlag.DASHBOARD_CROSS_FILTERS,
Expand All @@ -35,3 +49,93 @@ export const emitFilterControl = isFeatureEnabled(
},
]
: [];

export const contributionModeControl = {
name: 'contributionMode',
config: {
type: 'SelectControl',
label: t('Contribution Mode'),
default: null,
choices: [
[null, t('None')],
[ContributionType.Row, t('Row')],
[ContributionType.Column, t('Series')],
],
description: t('Calculate contribution per series or row'),
},
};

const xAxisSortVisibility = ({ controls }: { controls: ControlStateMapping }) =>
isDefined(controls?.x_axis?.value) &&
!isTemporalColumn(
getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
controls?.datasource?.datasource,
) &&
Array.isArray(controls?.groupby?.value) &&
controls.groupby.value.length === 0;

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),
),
].filter(Boolean);
const shouldReset = !(
typeof controlState.value === 'string' &&
choices.includes(controlState.value) &&
!isTemporalColumn(
getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
controls?.datasource?.datasource,
)
);

return {
shouldReset,
options: choices.map(entry => ({
value: entry,
label: entry,
})),
};
},
visibility: xAxisSortVisibility,
},
};

export const xAxisSortAscControl = {
name: 'x_axis_sort_asc',
config: {
type: 'CheckboxControl',
label: t('X-Axis Sort Ascending'),
default: true,
description: t('Whether to sort descending or ascending on the X-Axis.'),
visibility: xAxisSortVisibility,
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ const show_empty_columns: SharedControlConfig<'CheckboxControl'> = {
description: t('Show empty columns'),
};

const datetime_columns_lookup: SharedControlConfig<'HiddenControl'> = {
const temporal_columns_lookup: SharedControlConfig<'HiddenControl'> = {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bycatch: rename the datetime_columns_lookup into temporal_columns_lookup.

type: 'HiddenControl',
initialValue: (control: ControlState, state: ControlPanelState | null) =>
Object.fromEntries(
Expand Down Expand Up @@ -400,5 +400,5 @@ export default {
truncate_metric,
x_axis: dndXAxisControl,
show_empty_columns,
datetime_columns_lookup,
temporal_columns_lookup,
};