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.

Sign up for GitHub

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

Jump to bottom

feat: adding XAxis to BigNumberTrend #21577

Merged
merged 5 commits into from
Sep 26, 2022
Merged

feat: adding XAxis to BigNumberTrend #21577

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
77c9f2a
feat: adding XAxis to BigNumberTrend
zhaoyongjie Sep 26, 2022
75b138d
fix UT
zhaoyongjie Sep 26, 2022
a0d801e
temporal XAxis mixin
zhaoyongjie Sep 26, 2022
f14ccd4
remove unused
zhaoyongjie Sep 26, 2022
788c635
typing
zhaoyongjie Sep 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions superset-frontend/packages/superset-ui-chart-controls/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,4 @@ export { legacySortBy } from './shared-controls/legacySortBy';
export * from './shared-controls/emitFilterControl';
export * from './shared-controls/components';
export * from './types';
export { xAxisMixin, temporalXAxisMixin } from './shared-controls/constants';
31 changes: 29 additions & 2 deletions superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/constants.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ import {
FeatureFlag,
isFeatureEnabled,
QueryFormData,
QueryResponse,
t,
validateNonEmpty,
} from '@superset-ui/core';
import { ControlPanelState, ControlState } from '../types';
import { ControlPanelState, ControlState, Dataset } from '../types';

const getAxisLabel = (
formData: QueryFormData,
Expand All @@ -32,7 +33,7 @@ const getAxisLabel = (
? { label: t('Y-axis'), description: t('Dimension to use on y-axis.') }
: { label: t('X-axis'), description: t('Dimension to use on x-axis.') };

export const xAxisControlConfig = {
export const xAxisMixin = {
label: (state: ControlPanelState) => getAxisLabel(state?.form_data).label,
multi: false,
description: (state: ControlPanelState) =>
Expand All @@ -51,3 +52,29 @@ export const xAxisControlConfig = {
},
default: undefined,
};

export const temporalXAxisMixin = {
Copy link
Member Author

Choose a reason for hiding this comment

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

copy/paste from dndGranularitySqlaControl

Copy link
Member

Choose a reason for hiding this comment

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

As this is a mixin, we maybe the type could be narrowed down to something like Pick[BaseControlConfig, 'label' | 'mapStateToProps']?

Also, I'm wondering if we should make this a function getXAXisControlMixin that takes an optional argument dataTypes: GenericDataType[], which if defined, would only return the requested column types? For instance, at some point in the future I'm assuming we want to support any ordinal or linear datatypes in this chart type, and in that case we would likely want to support GenericDataType.NUMERIC and GenericDataType.TEMPORAL.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the review and very useful case for this mixin. I changed this variable from the temporalXAxisMixin to the temporalColumnMixin and removed mapStateToProps from the original one from dndGranularitySqlaControl. I totally agree with making a function for returning specific column types from the original datasource. I will refactor this part in separate PR.

label: t('TEMPORAL X-AXIS'),
mapStateToProps: ({ datasource }: ControlPanelState) => {
if (datasource?.columns[0]?.hasOwnProperty('column_name')) {
const temporalColumns =
(datasource as Dataset)?.columns?.filter(c => c.is_dttm) ?? [];
return {
options: temporalColumns,
default:
(datasource as Dataset)?.main_dttm_col ||
temporalColumns[0]?.column_name ||
null,
isTemporal: true,
};
}
const sortedQueryColumns = (datasource as QueryResponse)?.columns?.sort(
query => (query?.is_dttm ? -1 : 1),
);
return {
options: sortedQueryColumns,
default: sortedQueryColumns[0]?.name || null,
isTemporal: true,
};
},
};
4 changes: 2 additions & 2 deletions superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import {
ColumnMeta,
FilterOption,
} from '..';
import { xAxisControlConfig } from './constants';
import { xAxisMixin } from './constants';

type Control = {
savedMetrics?: Metric[] | null;
Expand Down Expand Up @@ -273,7 +273,7 @@ export const dndGranularitySqlaControl: typeof dndSeriesControl = {

export const dndXAxisControl: typeof dndGroupByControl = {
...dndGroupByControl,
...xAxisControlConfig,
...xAxisMixin,
};

export function withDndFallback(
Expand Down
40 changes: 18 additions & 22 deletions ...-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/buildQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
*/
import {
buildQueryContext,
DTTM_ALIAS,
ensureIsArray,
getXAxis,
isXAxisSet,
QueryFormData,
} from '@superset-ui/core';
import {
Expand All @@ -29,25 +31,19 @@ import {
} from '@superset-ui/chart-controls';

export default function buildQuery(formData: QueryFormData) {
return buildQueryContext(formData, baseQueryObject => {
const { x_axis } = formData;
const is_timeseries = x_axis === DTTM_ALIAS || !x_axis;

return [
{
...baseQueryObject,
is_timeseries: true,
post_processing: [
pivotOperator(formData, {
...baseQueryObject,
index: x_axis,
is_timeseries,
}),
rollingWindowOperator(formData, baseQueryObject),
resampleOperator(formData, baseQueryObject),
flattenOperator(formData, baseQueryObject),
],
},
];
});
return buildQueryContext(formData, baseQueryObject => [
{
...baseQueryObject,
columns: [
...(isXAxisSet(formData) ? ensureIsArray(getXAxis(formData)) : []),
],
...(isXAxisSet(formData) ? {} : { is_timeseries: true }),
post_processing: [
pivotOperator(formData, baseQueryObject),
rollingWindowOperator(formData, baseQueryObject),
resampleOperator(formData, baseQueryObject),
flattenOperator(formData, baseQueryObject),
],
},
]);
}
22 changes: 19 additions & 3 deletions ...ontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/controlPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,40 @@
* specific language governing permissions and limitations
* under the License.
*/
import { smartDateFormatter, t } from '@superset-ui/core';
import {
FeatureFlag,
isFeatureEnabled,
smartDateFormatter,
t,
} from '@superset-ui/core';
import {
ControlPanelConfig,
D3_FORMAT_DOCS,
D3_TIME_FORMAT_OPTIONS,
formatSelectOptions,
getStandardizedControls,
sections,
temporalXAxisMixin,
} from '@superset-ui/chart-controls';
import React from 'react';
import { headerFontSize, subheaderFontSize } from '../sharedControls';

const config: ControlPanelConfig = {
controlPanelSections: [
sections.legacyTimeseriesTime,
sections.genericTime,
{
label: t('Query'),
expanded: true,
controlSetRows: [['metric'], ['adhoc_filters']],
controlSetRows: [
[isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES) ? 'x_axis' : null],
[
isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES)
? 'time_grain_sqla'
: null,
],
['metric'],
['adhoc_filters'],
],
},
{
label: t('Options'),
Expand Down Expand Up @@ -270,6 +285,7 @@ const config: ControlPanelConfig = {
y_axis_format: {
label: t('Number format'),
},
x_axis: temporalXAxisMixin,
},
formDataOverrides: formData => ({
...formData,
Expand Down
10 changes: 5 additions & 5 deletions ...ntend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,16 @@
* under the License.
*/
import {
DTTM_ALIAS,
extractTimegrain,
getNumberFormatter,
NumberFormats,
QueryFormData,
GenericDataType,
getMetricLabel,
t,
smartDateVerboseFormatter,
NumberFormatter,
TimeFormatter,
getXAxis,
} from '@superset-ui/core';
import { EChartsCoreOption, graphic } from 'echarts';
import {
Expand Down Expand Up @@ -88,7 +87,7 @@ export default function transformProps(
yAxisFormat,
timeRangeFixed,
} = formData;
const granularity = extractTimegrain(rawFormData as QueryFormData);
const granularity = extractTimegrain(rawFormData);
const {
data = [],
colnames = [],
Expand All @@ -103,10 +102,11 @@ export default function transformProps(
const { r, g, b } = colorPicker;
const mainColor = `rgb(${r}, ${g}, ${b})`;

const timeColumn = getXAxis(rawFormData) as string;
let trendLineData;
let percentChange = 0;
let bigNumber = data.length === 0 ? null : data[0][metricName];
let timestamp = data.length === 0 ? null : data[0][DTTM_ALIAS];
let timestamp = data.length === 0 ? null : data[0][timeColumn];
let bigNumberFallback;

const metricColtypeIndex = colnames.findIndex(name => name === metricName);
Expand All @@ -115,7 +115,7 @@ export default function transformProps(

if (data.length > 0) {
const sortedData = (data as BigNumberDatum[])
.map(d => [d[DTTM_ALIAS], parseMetricValue(d[metricName])])
.map(d => [d[timeColumn], parseMetricValue(d[metricName])])
// sort in time descending order
.sort((a, b) => (a[0] !== null && b[0] !== null ? b[0] - a[0] : 0));

Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export type BigNumberWithTrendlineFormData = BigNumberTotalFormData & {
compareLag?: string | number;
};

export type BigNumberTotalChartProps = ChartProps & {
export type BigNumberTotalChartProps = ChartProps<QueryFormData> & {
formData: BigNumberTotalFormData;
queriesData: (ChartDataResponseResult & {
data?: BigNumberDatum[];
Expand Down
7 changes: 5 additions & 2 deletions superset-frontend/plugins/plugin-chart-echarts/test/BigNumber/transformProps.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,16 @@ const formData = {
a: 1,
},
compareLag: 1,
timeGrainSqla: 'P3M' as TimeGranularity,
timeGrainSqla: TimeGranularity.QUARTER,
granularitySqla: 'ds',
compareSuffix: 'over last quarter',
viz_type: 'big_number',
yAxisFormat: '.3s',
datasource: 'test_datasource',
};

const rawFormData = {
datasource: '1__table',
metric: 'value',
color_picker: {
r: 0,
Expand All @@ -52,7 +54,8 @@ const rawFormData = {
a: 1,
},
compare_lag: 1,
time_grain_sqla: 'P3M' as TimeGranularity,
time_grain_sqla: TimeGranularity.QUARTER,
granularity_sqla: 'ds',
compare_suffix: 'over last quarter',
viz_type: 'big_number',
y_axis_format: '.3s',
Expand Down