From 8f7daa306c92a20bfda0f7a9ea871767acdd502d Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Tue, 26 Apr 2022 21:50:03 +0800 Subject: [PATCH 01/12] feat: add AA into mixed time series chart --- .../src/MixedTimeseries/buildQuery.ts | 106 ++++++++++-------- .../src/MixedTimeseries/controlPanel.tsx | 21 ++++ .../src/utils/removeFormDataSuffix.ts | 51 +++++++++ 3 files changed, 131 insertions(+), 47 deletions(-) create mode 100644 superset-frontend/plugins/plugin-chart-echarts/src/utils/removeFormDataSuffix.ts diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts index 9adc149489a2..a63a0d273af2 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts @@ -21,76 +21,88 @@ import { QueryFormData, QueryObject, normalizeOrderBy, + PostProcessingPivot, } from '@superset-ui/core'; import { pivotOperator, renameOperator, flattenOperator, + isTimeComparison, + timeComparePivotOperator, + rollingWindowOperator, + timeCompareOperator, + resampleOperator, } from '@superset-ui/chart-controls'; +import { + removeFormDataSuffix, + removeUnusedFormData, +} from '../utils/removeFormDataSuffix'; export default function buildQuery(formData: QueryFormData) { - const { - adhoc_filters, - adhoc_filters_b, - groupby, - groupby_b, - limit, - limit_b, - timeseries_limit_metric, - timeseries_limit_metric_b, - metrics, - metrics_b, - order_desc, - order_desc_b, - ...baseFormData - } = formData; - baseFormData.is_timeseries = true; - const formData1 = { - ...baseFormData, - adhoc_filters, - columns: groupby, - limit, - timeseries_limit_metric, - metrics, - order_desc, - }; - const formData2 = { - ...baseFormData, - adhoc_filters: adhoc_filters_b, - columns: groupby_b, - limit: limit_b, - timeseries_limit_metric: timeseries_limit_metric_b, - metrics: metrics_b, - order_desc: order_desc_b, + const baseFormData = { + ...formData, + is_timeseries: true, + columns: formData.groupby, + columns_b: formData.groupby_b, }; + const formData1 = removeUnusedFormData(baseFormData, '_b'); + const formData2 = removeFormDataSuffix(baseFormData, '_b'); const queryContextA = buildQueryContext(formData1, baseQueryObject => { - const queryObjectA = { + const queryObject = { ...baseQueryObject, is_timeseries: true, + }; + + const pivotOperatorInRuntime: PostProcessingPivot = isTimeComparison( + formData1, + queryObject, + ) + ? timeComparePivotOperator(formData1, queryObject) + : pivotOperator(formData1, queryObject); + + const queryObjectA = { + ...queryObject, + time_offsets: isTimeComparison(formData1, queryObject) + ? formData1.time_compare + : [], post_processing: [ - pivotOperator(formData1, { ...baseQueryObject, is_timeseries: true }), - renameOperator(formData1, { - ...baseQueryObject, - ...{ is_timeseries: true }, - }), - flattenOperator(formData1, baseQueryObject), + pivotOperatorInRuntime, + rollingWindowOperator(formData1, queryObject), + timeCompareOperator(formData1, queryObject), + resampleOperator(formData1, queryObject), + renameOperator(formData1, queryObject), + flattenOperator(formData1, queryObject), ], } as QueryObject; return [normalizeOrderBy(queryObjectA)]; }); const queryContextB = buildQueryContext(formData2, baseQueryObject => { - const queryObjectB = { + const queryObject = { ...baseQueryObject, is_timeseries: true, + }; + + const pivotOperatorInRuntime: PostProcessingPivot = isTimeComparison( + formData2, + queryObject, + ) + ? timeComparePivotOperator(formData2, queryObject) + : pivotOperator(formData2, queryObject); + + const queryObjectB = { + ...queryObject, + time_offsets: isTimeComparison(formData2, queryObject) + ? formData2.time_compare + : [], post_processing: [ - pivotOperator(formData2, { ...baseQueryObject, is_timeseries: true }), - renameOperator(formData2, { - ...baseQueryObject, - ...{ is_timeseries: true }, - }), - flattenOperator(formData2, baseQueryObject), + pivotOperatorInRuntime, + rollingWindowOperator(formData2, queryObject), + timeCompareOperator(formData2, queryObject), + resampleOperator(formData2, queryObject), + renameOperator(formData2, queryObject), + flattenOperator(formData2, queryObject), ], } as QueryObject; return [normalizeOrderBy(queryObjectB)]; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx index 97955eec3500..896466dfa0fe 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx @@ -18,10 +18,12 @@ */ import React from 'react'; import { t } from '@superset-ui/core'; +import { cloneDeep } from 'lodash'; import { ControlPanelConfig, ControlPanelSectionConfig, ControlSetRow, + CustomControlItem, emitFilterControl, sections, sharedControls, @@ -253,11 +255,30 @@ function createCustomizeSection( ]; } +function createAdvancedAnalyticsSection( + label: string, + controlSuffix: string, +): ControlPanelSectionConfig { + const aaWithSuffix = cloneDeep(sections.advancedAnalyticsControls); + aaWithSuffix.label = label; + aaWithSuffix.controlSetRows.forEach(row => + row.forEach((control: CustomControlItem) => { + if (control?.name && controlSuffix) { + // eslint-disable-next-line no-param-reassign + control.name = `${control.name}${controlSuffix}`; + } + }), + ); + return aaWithSuffix; +} + const config: ControlPanelConfig = { controlPanelSections: [ sections.legacyTimeseriesTime, createQuerySection(t('Query A'), ''), + createAdvancedAnalyticsSection(t('Advanced analytics Query A'), ''), createQuerySection(t('Query B'), '_b'), + createAdvancedAnalyticsSection(t('Advanced analytics Query B'), '_b'), { label: t('Annotations and Layers'), expanded: false, diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/removeFormDataSuffix.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/removeFormDataSuffix.ts new file mode 100644 index 000000000000..3695e2c63842 --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/removeFormDataSuffix.ts @@ -0,0 +1,51 @@ +/** + * 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 } from '@superset-ui/core'; + +export const removeFormDataSuffix = ( + formData: QueryFormData, + controlSuffix: string, +): QueryFormData => { + const newFormData = {}; + Object.entries(formData).forEach(([key, value]) => { + if (key.endsWith(controlSuffix)) { + newFormData[key.slice(0, -controlSuffix.length)] = value; + } + + if (!(key in newFormData)) { + newFormData[key] = value; + } + }); + + return newFormData as QueryFormData; +}; + +export const removeUnusedFormData = ( + formData: QueryFormData, + controlSuffix: string, +): QueryFormData => { + const newFormData = {}; + Object.entries(formData).forEach(([key, value]) => { + if (!key.endsWith(controlSuffix)) { + newFormData[key] = value; + } + }); + + return newFormData as QueryFormData; +}; From b7126696eda4a83cd67ce382376c49aea3a090eb Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Tue, 26 Apr 2022 21:53:02 +0800 Subject: [PATCH 02/12] skip loop section --- .../plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx index 896466dfa0fe..8e5c9c4b1f24 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx @@ -261,6 +261,9 @@ function createAdvancedAnalyticsSection( ): ControlPanelSectionConfig { const aaWithSuffix = cloneDeep(sections.advancedAnalyticsControls); aaWithSuffix.label = label; + if (!controlSuffix) { + return aaWithSuffix; + } aaWithSuffix.controlSetRows.forEach(row => row.forEach((control: CustomControlItem) => { if (control?.name && controlSuffix) { From f2f75643f8a8d18420dd2d8636c2989752c910a9 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 27 Apr 2022 08:56:09 +0800 Subject: [PATCH 03/12] rewrite qobj builder --- .../src/MixedTimeseries/buildQuery.ts | 88 ++++++------------- .../src/MixedTimeseries/controlPanel.tsx | 2 +- .../src/utils/removeFormDataSuffix.ts | 39 ++++++-- 3 files changed, 61 insertions(+), 68 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts index a63a0d273af2..71184df09acc 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts @@ -48,68 +48,38 @@ export default function buildQuery(formData: QueryFormData) { const formData1 = removeUnusedFormData(baseFormData, '_b'); const formData2 = removeFormDataSuffix(baseFormData, '_b'); - const queryContextA = buildQueryContext(formData1, baseQueryObject => { - const queryObject = { - ...baseQueryObject, - is_timeseries: true, - }; + const queryContexts = [formData1, formData2].map(fd => + buildQueryContext(fd, baseQueryObject => { + const queryObject = { + ...baseQueryObject, + is_timeseries: true, + }; - const pivotOperatorInRuntime: PostProcessingPivot = isTimeComparison( - formData1, - queryObject, - ) - ? timeComparePivotOperator(formData1, queryObject) - : pivotOperator(formData1, queryObject); + const pivotOperatorInRuntime: PostProcessingPivot = isTimeComparison( + fd, + queryObject, + ) + ? timeComparePivotOperator(fd, queryObject) + : pivotOperator(fd, queryObject); - const queryObjectA = { - ...queryObject, - time_offsets: isTimeComparison(formData1, queryObject) - ? formData1.time_compare - : [], - post_processing: [ - pivotOperatorInRuntime, - rollingWindowOperator(formData1, queryObject), - timeCompareOperator(formData1, queryObject), - resampleOperator(formData1, queryObject), - renameOperator(formData1, queryObject), - flattenOperator(formData1, queryObject), - ], - } as QueryObject; - return [normalizeOrderBy(queryObjectA)]; - }); - - const queryContextB = buildQueryContext(formData2, baseQueryObject => { - const queryObject = { - ...baseQueryObject, - is_timeseries: true, - }; - - const pivotOperatorInRuntime: PostProcessingPivot = isTimeComparison( - formData2, - queryObject, - ) - ? timeComparePivotOperator(formData2, queryObject) - : pivotOperator(formData2, queryObject); - - const queryObjectB = { - ...queryObject, - time_offsets: isTimeComparison(formData2, queryObject) - ? formData2.time_compare - : [], - post_processing: [ - pivotOperatorInRuntime, - rollingWindowOperator(formData2, queryObject), - timeCompareOperator(formData2, queryObject), - resampleOperator(formData2, queryObject), - renameOperator(formData2, queryObject), - flattenOperator(formData2, queryObject), - ], - } as QueryObject; - return [normalizeOrderBy(queryObjectB)]; - }); + const tmpQueryObject = { + ...queryObject, + time_offsets: isTimeComparison(fd, queryObject) ? fd.time_compare : [], + post_processing: [ + pivotOperatorInRuntime, + rollingWindowOperator(fd, queryObject), + timeCompareOperator(fd, queryObject), + resampleOperator(fd, queryObject), + renameOperator(fd, queryObject), + flattenOperator(fd, queryObject), + ], + } as QueryObject; + return [normalizeOrderBy(tmpQueryObject)]; + }), + ); return { - ...queryContextA, - queries: [...queryContextA.queries, ...queryContextB.queries], + ...queryContexts[0], + queries: [...queryContexts[0].queries, ...queryContexts[1].queries], }; } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx index 8e5c9c4b1f24..e839637885e4 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx @@ -266,7 +266,7 @@ function createAdvancedAnalyticsSection( } aaWithSuffix.controlSetRows.forEach(row => row.forEach((control: CustomControlItem) => { - if (control?.name && controlSuffix) { + if (control?.name) { // eslint-disable-next-line no-param-reassign control.name = `${control.name}${controlSuffix}`; } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/removeFormDataSuffix.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/removeFormDataSuffix.ts index 3695e2c63842..e9417b56ce6a 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/removeFormDataSuffix.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/removeFormDataSuffix.ts @@ -22,16 +22,32 @@ export const removeFormDataSuffix = ( formData: QueryFormData, controlSuffix: string, ): QueryFormData => { + /* + * remove specific suffix controls and return a new formData + * eg: + * > const fd = { metrics: ['foo', 'bar'], metrics_b: ['zee'], limit: 100, ... } + * > removeFormDataSuffix(fd, '_b') + * { metrics: ['zee'], limit: 100, ... } + * */ const newFormData = {}; - Object.entries(formData).forEach(([key, value]) => { - if (key.endsWith(controlSuffix)) { - newFormData[key.slice(0, -controlSuffix.length)] = value; - } - if (!(key in newFormData)) { - newFormData[key] = value; - } - }); + Object.entries(formData) + .sort(([a], [b]) => { + // items contained suffix before others + const weight_a = a.endsWith(controlSuffix) ? 1 : 0; + const weight_b = b.endsWith(controlSuffix) ? 1 : 0; + return weight_b - weight_a; + }) + .forEach(([key, value]) => { + if (key.endsWith(controlSuffix)) { + newFormData[key.slice(0, -controlSuffix.length)] = value; + } + + if (!(key in newFormData)) { + // ignore duplication + newFormData[key] = value; + } + }); return newFormData as QueryFormData; }; @@ -40,6 +56,13 @@ export const removeUnusedFormData = ( formData: QueryFormData, controlSuffix: string, ): QueryFormData => { + /* + * remove unused controls by suffix and return a new formData + * eg: + * > const fd = { metrics: ['foo', 'bar'], metrics_b: ['zee'], limit: 100, ... } + * > removeUnusedFormData(fd, '_b') + * { metrics: ['foo', 'bar'], limit: 100, ... } + * */ const newFormData = {}; Object.entries(formData).forEach(([key, value]) => { if (!key.endsWith(controlSuffix)) { From d264d45df9967e027c403d3e34765486634a67c8 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 27 Apr 2022 09:36:31 +0800 Subject: [PATCH 04/12] add ut --- .../src/utils/removeFormDataSuffix.ts | 2 +- .../test/utils/removeFormDataSuffix.test.ts | 58 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 superset-frontend/plugins/plugin-chart-echarts/test/utils/removeFormDataSuffix.test.ts diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/removeFormDataSuffix.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/removeFormDataSuffix.ts index e9417b56ce6a..3917189f77ec 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/removeFormDataSuffix.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/removeFormDataSuffix.ts @@ -43,7 +43,7 @@ export const removeFormDataSuffix = ( newFormData[key.slice(0, -controlSuffix.length)] = value; } - if (!(key in newFormData)) { + if (!key.endsWith(controlSuffix) && !(key in newFormData)) { // ignore duplication newFormData[key] = value; } diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/removeFormDataSuffix.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/removeFormDataSuffix.test.ts new file mode 100644 index 000000000000..aa3357d5349f --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/removeFormDataSuffix.test.ts @@ -0,0 +1,58 @@ +/** + * 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 } from '@superset-ui/core'; +import { + removeFormDataSuffix, + removeUnusedFormData, +} from '../../src/utils/removeFormDataSuffix'; + +const formData = { + datasource: 'dummy', + viz_type: 'table', + metrics: ['a', 'b'], + columns: ['foo', 'bar'], + limit: 100, + metrics_b: ['c', 'd'], + columns_b: ['hello', 'world'], + limit_b: 200, +} as QueryFormData; + +test('should keep controls with suffix', () => { + expect(removeFormDataSuffix(formData, '_b')).toEqual({ + datasource: 'dummy', + viz_type: 'table', + metrics: ['c', 'd'], + columns: ['hello', 'world'], + limit: 200, + }); + // no side effect + expect(removeFormDataSuffix(formData, '_b')).not.toEqual(formData); +}); + +test('should remove controls with suffix', () => { + expect(removeUnusedFormData(formData, '_b')).toEqual({ + datasource: 'dummy', + viz_type: 'table', + metrics: ['a', 'b'], + columns: ['foo', 'bar'], + limit: 100, + }); + // no side effect + expect(removeUnusedFormData(formData, '_b')).not.toEqual(formData); +}); From 3c2a4b0fb68053fe4f6b2cefd73cd75c47757ac3 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 27 Apr 2022 09:45:13 +0800 Subject: [PATCH 05/12] rename --- .../src/MixedTimeseries/buildQuery.ts | 8 ++++---- .../{removeFormDataSuffix.ts => formDataSuffix.ts} | 4 ++-- .../test/utils/removeFormDataSuffix.test.ts | 12 ++++++------ 3 files changed, 12 insertions(+), 12 deletions(-) rename superset-frontend/plugins/plugin-chart-echarts/src/utils/{removeFormDataSuffix.ts => formDataSuffix.ts} (98%) diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts index 71184df09acc..f35fc366f4b8 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts @@ -34,9 +34,9 @@ import { resampleOperator, } from '@superset-ui/chart-controls'; import { + formDataSuffix, removeFormDataSuffix, - removeUnusedFormData, -} from '../utils/removeFormDataSuffix'; +} from '../utils/retainFormDataSuffix'; export default function buildQuery(formData: QueryFormData) { const baseFormData = { @@ -45,8 +45,8 @@ export default function buildQuery(formData: QueryFormData) { columns: formData.groupby, columns_b: formData.groupby_b, }; - const formData1 = removeUnusedFormData(baseFormData, '_b'); - const formData2 = removeFormDataSuffix(baseFormData, '_b'); + const formData1 = removeFormDataSuffix(baseFormData, '_b'); + const formData2 = formDataSuffix(baseFormData, '_b'); const queryContexts = [formData1, formData2].map(fd => buildQueryContext(fd, baseQueryObject => { diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/removeFormDataSuffix.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/formDataSuffix.ts similarity index 98% rename from superset-frontend/plugins/plugin-chart-echarts/src/utils/removeFormDataSuffix.ts rename to superset-frontend/plugins/plugin-chart-echarts/src/utils/formDataSuffix.ts index 3917189f77ec..7f4eeb138573 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/removeFormDataSuffix.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/formDataSuffix.ts @@ -18,7 +18,7 @@ */ import { QueryFormData } from '@superset-ui/core'; -export const removeFormDataSuffix = ( +export const retainFormDataSuffix = ( formData: QueryFormData, controlSuffix: string, ): QueryFormData => { @@ -52,7 +52,7 @@ export const removeFormDataSuffix = ( return newFormData as QueryFormData; }; -export const removeUnusedFormData = ( +export const removeFormDataSuffix = ( formData: QueryFormData, controlSuffix: string, ): QueryFormData => { diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/removeFormDataSuffix.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/removeFormDataSuffix.test.ts index aa3357d5349f..1db65d39ddd8 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/removeFormDataSuffix.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/removeFormDataSuffix.test.ts @@ -18,9 +18,9 @@ */ import { QueryFormData } from '@superset-ui/core'; import { + retainFormDataSuffix, removeFormDataSuffix, - removeUnusedFormData, -} from '../../src/utils/removeFormDataSuffix'; +} from '../../src/utils/formDataSuffix'; const formData = { datasource: 'dummy', @@ -34,7 +34,7 @@ const formData = { } as QueryFormData; test('should keep controls with suffix', () => { - expect(removeFormDataSuffix(formData, '_b')).toEqual({ + expect(retainFormDataSuffix(formData, '_b')).toEqual({ datasource: 'dummy', viz_type: 'table', metrics: ['c', 'd'], @@ -42,11 +42,11 @@ test('should keep controls with suffix', () => { limit: 200, }); // no side effect - expect(removeFormDataSuffix(formData, '_b')).not.toEqual(formData); + expect(retainFormDataSuffix(formData, '_b')).not.toEqual(formData); }); test('should remove controls with suffix', () => { - expect(removeUnusedFormData(formData, '_b')).toEqual({ + expect(removeFormDataSuffix(formData, '_b')).toEqual({ datasource: 'dummy', viz_type: 'table', metrics: ['a', 'b'], @@ -54,5 +54,5 @@ test('should remove controls with suffix', () => { limit: 100, }); // no side effect - expect(removeUnusedFormData(formData, '_b')).not.toEqual(formData); + expect(removeFormDataSuffix(formData, '_b')).not.toEqual(formData); }); From dd3e3f548cc78693f6dd844492810ef8579327b4 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 27 Apr 2022 09:46:22 +0800 Subject: [PATCH 06/12] rename --- .../{removeFormDataSuffix.test.ts => formDataSuffix.test.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename superset-frontend/plugins/plugin-chart-echarts/test/utils/{removeFormDataSuffix.test.ts => formDataSuffix.test.ts} (100%) diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/removeFormDataSuffix.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/formDataSuffix.test.ts similarity index 100% rename from superset-frontend/plugins/plugin-chart-echarts/test/utils/removeFormDataSuffix.test.ts rename to superset-frontend/plugins/plugin-chart-echarts/test/utils/formDataSuffix.test.ts From 76ffa58cb3f2ccfbffe6175e5a1a74198d4ac7a9 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 27 Apr 2022 09:48:25 +0800 Subject: [PATCH 07/12] rename --- .../plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts index f35fc366f4b8..a8255b7d999f 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts @@ -34,9 +34,9 @@ import { resampleOperator, } from '@superset-ui/chart-controls'; import { - formDataSuffix, + retainFormDataSuffix, removeFormDataSuffix, -} from '../utils/retainFormDataSuffix'; +} from '../utils/formDataSuffix'; export default function buildQuery(formData: QueryFormData) { const baseFormData = { @@ -46,7 +46,7 @@ export default function buildQuery(formData: QueryFormData) { columns_b: formData.groupby_b, }; const formData1 = removeFormDataSuffix(baseFormData, '_b'); - const formData2 = formDataSuffix(baseFormData, '_b'); + const formData2 = retainFormDataSuffix(baseFormData, '_b'); const queryContexts = [formData1, formData2].map(fd => buildQueryContext(fd, baseQueryObject => { From 35a3c4939839aeb00b30b7f08d3f1f4c87949670 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 27 Apr 2022 09:49:28 +0800 Subject: [PATCH 08/12] typo --- .../plugins/plugin-chart-echarts/src/utils/formDataSuffix.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/formDataSuffix.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/formDataSuffix.ts index 7f4eeb138573..c256e6f87427 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/formDataSuffix.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/formDataSuffix.ts @@ -23,7 +23,7 @@ export const retainFormDataSuffix = ( controlSuffix: string, ): QueryFormData => { /* - * remove specific suffix controls and return a new formData + * retain controls by suffix and return a new formData * eg: * > const fd = { metrics: ['foo', 'bar'], metrics_b: ['zee'], limit: 100, ... } * > removeFormDataSuffix(fd, '_b') From 7c9c3c266b6c7a5a52da8918ec36dce06cec2bc3 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 27 Apr 2022 11:48:50 +0800 Subject: [PATCH 09/12] ut --- .../test/MixedTimeseries/buildQuery.test.ts | 186 ++++++++++++++++++ 1 file changed, 186 insertions(+) create mode 100644 superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts new file mode 100644 index 000000000000..40f4e5ad31be --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts @@ -0,0 +1,186 @@ +/** + * 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 { FreeFormAdhocFilter, TimeGranularity } from '@superset-ui/core'; +import buildQuery from '../../src/MixedTimeseries/buildQuery'; + +const formDataMixedChart = { + datasource: 'dummy', + viz_type: 'my_chart', + // query + // -- common + time_range: '1980 : 2000', + time_grain_sqla: TimeGranularity.WEEK, + granularity_sqla: 'ds', + // -- query a + groupby: ['foo'], + metrics: ['sum(sales)'], + adhoc_filters: [ + { + clause: 'WHERE', + expressionType: 'SQL', + sqlExpression: "foo in ('a', 'b')", + } as FreeFormAdhocFilter, + ], + limit: 5, + row_limit: 10, + timeseries_limit_metric: 'count', + order_desc: true, + emit_filter: true, + // -- query b + groupby_b: [], + metrics_b: ['count'], + adhoc_filters_b: [ + { + clause: 'WHERE', + expressionType: 'SQL', + sqlExpression: "name in ('c', 'd')", + } as FreeFormAdhocFilter, + ], + limit_b: undefined, + row_limit_b: 100, + timeseries_limit_metric_b: undefined, + order_desc_b: false, + emit_filter_b: undefined, + // chart configs + show_value: false, + show_valueB: undefined, +}; + +test('should compile query object A', () => { + const query_a = buildQuery(formDataMixedChart).queries[0]; + expect(query_a).toEqual({ + time_range: '1980 : 2000', + since: undefined, + until: undefined, + granularity: 'ds', + filters: [], + extras: { + having: '', + having_druid: [], + time_grain_sqla: 'P1W', + where: "(foo in ('a', 'b'))", + }, + applied_time_extras: {}, + columns: ['foo'], + metrics: ['sum(sales)'], + annotation_layers: [], + row_limit: 10, + row_offset: undefined, + series_columns: undefined, + series_limit: undefined, + series_limit_metric: undefined, + timeseries_limit: 5, + url_params: {}, + custom_params: {}, + custom_form_data: {}, + is_timeseries: true, + time_offsets: [], + post_processing: [ + { + operation: 'pivot', + options: { + aggregates: { + 'sum(sales)': { + operator: 'mean', + }, + }, + columns: ['foo'], + drop_missing_columns: false, + flatten_columns: false, + index: ['__timestamp'], + reset_index: false, + }, + }, + undefined, + undefined, + undefined, + { + operation: 'rename', + options: { + columns: { + 'sum(sales)': null, + }, + inplace: true, + level: 0, + }, + }, + { + operation: 'flatten', + }, + ], + orderby: [['count', false]], + }); +}); + +test('should compile query object B', () => { + const query_a = buildQuery(formDataMixedChart).queries[1]; + expect(query_a).toEqual({ + time_range: '1980 : 2000', + since: undefined, + until: undefined, + granularity: 'ds', + filters: [], + extras: { + having: '', + having_druid: [], + time_grain_sqla: 'P1W', + where: "(name in ('c', 'd'))", + }, + applied_time_extras: {}, + columns: [], + metrics: ['count'], + annotation_layers: [], + row_limit: 100, + row_offset: undefined, + series_columns: undefined, + series_limit: undefined, + series_limit_metric: undefined, + timeseries_limit: 0, + url_params: {}, + custom_params: {}, + custom_form_data: {}, + is_timeseries: true, + time_offsets: [], + post_processing: [ + { + operation: 'pivot', + options: { + aggregates: { + count: { + operator: 'mean', + }, + }, + columns: [], + drop_missing_columns: false, + flatten_columns: false, + index: ['__timestamp'], + reset_index: false, + }, + }, + undefined, + undefined, + undefined, + undefined, + { + operation: 'flatten', + }, + ], + orderby: [['count', true]], + }); +}); From dc415afd3885da0c6d05a986f6f7937b7235e08e Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 27 Apr 2022 13:21:26 +0800 Subject: [PATCH 10/12] ut --- .../test/MixedTimeseries/buildQuery.test.ts | 93 ++++++++++++++++++- 1 file changed, 92 insertions(+), 1 deletion(-) diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts index 40f4e5ad31be..72f16482cb77 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts @@ -16,7 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -import { FreeFormAdhocFilter, TimeGranularity } from '@superset-ui/core'; +import { + ComparisionType, + FreeFormAdhocFilter, + RollingType, + TimeGranularity, +} from '@superset-ui/core'; import buildQuery from '../../src/MixedTimeseries/buildQuery'; const formDataMixedChart = { @@ -61,6 +66,22 @@ const formDataMixedChart = { show_value: false, show_valueB: undefined, }; +const formDataMixedChartWithAA = { + ...formDataMixedChart, + rolling_type: RollingType.Cumsum, + time_compare: ['1 years ago'], + comparison_type: ComparisionType.Values, + resample_rule: '1AS', + resample_method: 'zerofill', + + rolling_type_b: RollingType.Sum, + rolling_periods_b: 1, + min_periods_b: 1, + comparison_type_b: ComparisionType.Difference, + time_compare_b: ['3 years ago'], + resample_rule_b: '1A', + resample_method_b: 'asfreq', +}; test('should compile query object A', () => { const query_a = buildQuery(formDataMixedChart).queries[0]; @@ -184,3 +205,73 @@ test('should compile query object B', () => { orderby: [['count', true]], }); }); + +test('should compile AA in query A', () => { + const query_a = buildQuery(formDataMixedChartWithAA).queries[0]; + // time comparison + expect(query_a?.time_offsets).toEqual(['1 years ago']); + + // cumsum + expect( + // prettier-ignore + query_a + .post_processing + ?.find(operator => operator?.operation === 'cum') + ?.operation, + ).toEqual('cum'); + + // resample + expect( + // prettier-ignore + query_a + .post_processing + ?.find(operator => operator?.operation === 'resample'), + ).toEqual({ + operation: 'resample', + options: { + method: 'asfreq', + rule: '1AS', + fill_value: 0, + }, + }); +}); + +test('should compile AA in query B', () => { + const query_b = buildQuery(formDataMixedChartWithAA).queries[1]; + // time comparison + expect(query_b?.time_offsets).toEqual(['3 years ago']); + + // rolling total + expect( + // prettier-ignore + query_b + .post_processing + ?.find(operator => operator?.operation === 'rolling'), + ).toEqual({ + operation: 'rolling', + options: { + rolling_type: 'sum', + window: 1, + min_periods: 1, + columns: { + count: 'count', + 'count__3 years ago': 'count__3 years ago', + }, + }, + }); + + // resample + expect( + // prettier-ignore + query_b + .post_processing + ?.find(operator => operator?.operation === 'resample'), + ).toEqual({ + operation: 'resample', + options: { + method: 'asfreq', + rule: '1A', + fill_value: null, + }, + }); +}); From d0a9d0ce3c101eb9309d433775a065bab3ef13d0 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 27 Apr 2022 15:26:25 +0800 Subject: [PATCH 11/12] remove unused import --- .../plugin-chart-echarts/test/utils/formDataSuffix.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/formDataSuffix.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/formDataSuffix.test.ts index 1db65d39ddd8..2e22583c76c7 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/formDataSuffix.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/formDataSuffix.test.ts @@ -16,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ -import { QueryFormData } from '@superset-ui/core'; import { retainFormDataSuffix, removeFormDataSuffix, @@ -31,7 +30,7 @@ const formData = { metrics_b: ['c', 'd'], columns_b: ['hello', 'world'], limit_b: 200, -} as QueryFormData; +}; test('should keep controls with suffix', () => { expect(retainFormDataSuffix(formData, '_b')).toEqual({ From 5299fde57c529d48b0d6230594d15c467a0dd4e2 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 27 Apr 2022 20:29:32 +0800 Subject: [PATCH 12/12] fix controls hiden --- .../src/sections/advancedAnalytics.tsx | 26 ++++++++++++++----- .../superset-ui-chart-controls/src/types.ts | 5 +++- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/advancedAnalytics.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/sections/advancedAnalytics.tsx index 3d562309ca94..1ed664b7de62 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/sections/advancedAnalytics.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/sections/advancedAnalytics.tsx @@ -59,9 +59,16 @@ export const advancedAnalyticsControls: ControlPanelSectionConfig = { 'Defines the size of the rolling window function, ' + 'relative to the time granularity selected', ), - visibility: ({ controls }) => - Boolean(controls?.rolling_type?.value) && - controls.rolling_type.value !== RollingType.Cumsum, + visibility: ({ controls }, { name }) => { + // `rolling_type_b` refer to rolling_type in mixed timeseries Query B + const rollingTypeControlName = name.endsWith('_b') + ? 'rolling_type_b' + : 'rolling_type'; + return ( + Boolean(controls[rollingTypeControlName]?.value) && + controls[rollingTypeControlName]?.value !== RollingType.Cumsum + ); + }, }, }, ], @@ -79,9 +86,16 @@ export const advancedAnalyticsControls: ControlPanelSectionConfig = { 'shown are the total of 7 periods. This will hide the "ramp up" ' + 'taking place over the first 7 periods', ), - visibility: ({ controls }) => - Boolean(controls?.rolling_type?.value) && - controls.rolling_type.value !== RollingType.Cumsum, + visibility: ({ controls }, { name }) => { + // `rolling_type_b` refer to rolling_type in mixed timeseries Query B + const rollingTypeControlName = name.endsWith('_b') + ? 'rolling_type_b' + : 'rolling_type'; + return ( + Boolean(controls[rollingTypeControlName]?.value) && + controls[rollingTypeControlName]?.value !== RollingType.Cumsum + ); + }, }, }, ], diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts index 4eec6e9c0b40..73985bfc743b 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts @@ -213,7 +213,10 @@ export interface BaseControlConfig< // TODO: add strict `chartState` typing (see superset-frontend/src/explore/types) chartState?: AnyDict, ) => ExtraControlProps; - visibility?: (props: ControlPanelsContainerProps) => boolean; + visibility?: ( + props: ControlPanelsContainerProps, + controlData: AnyDict, + ) => boolean; } export interface ControlValueValidator<