refactor(plugins): Time Comparison Utils (#27145)

superset-frontend/packages/superset-ui-core/src/index.ts

@@ -37,3 +37,4 @@ export * from './math-expression';
 export * from './ui-overrides';
 export * from './hooks';
 export * from './currency-format';
+export * from './time-comparison';

superset-frontend/packages/superset-ui-core/src/query/types/Query.ts

@@ -69,6 +69,8 @@ export type QueryObjectExtras = Partial<{
   time_grain_sqla?: TimeGranularity;
   /** WHERE condition */
   where?: string;
+  /** Instant Time Comparison */
+  instant_time_comparison_range?: string;
 }>;
 
 export type ResidualQueryObjectData = {

superset-frontend/packages/superset-ui-core/src/time-comparison/README.md

@@ -0,0 +1,47 @@
+<!--
+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.
+-->
+
+## @superset-ui/core/time-comparison
+
+This is a collection of methods used to support Time Comparison in charts.
+
+#### Example usage
+
+```js
+import { getComparisonTimeRangeInfo } from '@superset-ui/core';
+const { since, until } = getComparisonTimeRangeInfo(
+  adhocFilters,
+  extraFormData,
+);
+console.log(adhocFilters, extraFormData);
+```
+
+or
+
+```js
+import { ComparisonTimeRangeType } from '@superset-ui/core';
+ComparisonTimeRangeType.Custom; // 'c'
+ComparisonTimeRangeType.InheritRange; // 'r'
+```
+
+#### API
+
+`fn(args)`
+
+- Do something

superset-frontend/packages/superset-ui-core/src/time-comparison/getComparisonFilters.ts

@@ -0,0 +1,67 @@
+/**
+ * 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 '../query';
+import { AdhocFilter } from '../types';
+
+/**
+ * This method is used to get the query filters to be applied to the comparison query after
+ * overriding the time range in case an extra form data is provided.
+ * For example when rendering a chart that uses time comparison in a dashboard with time filters.
+ * @param formData - the form data
+ * @param extraFormData - the extra form data
+ * @returns the query filters to be applied to the comparison query
+ */
+export const getComparisonFilters = (
+  formData: QueryFormData,
+  extraFormData: any,
+): AdhocFilter[] => {
+  const timeFilterIndex: number =
+    formData.adhoc_filters?.findIndex(
+      filter => 'operator' in filter && filter.operator === 'TEMPORAL_RANGE',
+    ) ?? -1;
+
+  const timeFilter: AdhocFilter | null =
+    timeFilterIndex !== -1 && formData.adhoc_filters
+      ? formData.adhoc_filters[timeFilterIndex]
+      : null;
+
+  if (
+    timeFilter &&
+    'comparator' in timeFilter &&
+    typeof timeFilter.comparator === 'string'
+  ) {
+    if (extraFormData?.time_range) {
+      timeFilter.comparator = extraFormData.time_range;
+    }
+  }
+
+  const comparisonQueryFilter = timeFilter ? [timeFilter] : [];
+
+  const otherFilters = formData.adhoc_filters?.filter(
+    (_value: any, index: number) => timeFilterIndex !== index,
+  );
+  const comparisonQueryFilters = otherFilters
+    ? [...comparisonQueryFilter, ...otherFilters]
+    : comparisonQueryFilter;
+
+  return comparisonQueryFilters;
+};
+
+export default getComparisonFilters;

superset-frontend/packages/superset-ui-core/src/time-comparison/getComparisonInfo.ts

@@ -0,0 +1,65 @@
+/**
+ * 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 '../query';
+import { getComparisonFilters } from './getComparisonFilters';
+import { ComparisonTimeRangeType } from './types';
+
+/**
+ * This is the main function to get the comparison info. It will return the formData
+ * that a viz can use to query the comparison data and the time shift text needed for
+ * the comparison time range based on the control value.
+ * @param formData
+ * @param timeComparison
+ * @param extraFormData
+ * @returns the processed formData
+ */
+
+export const getComparisonInfo = (
+  formData: QueryFormData,
+  timeComparison: string,
+  extraFormData: any,
+): QueryFormData => {
+  let comparisonFormData;
+
+  if (timeComparison !== ComparisonTimeRangeType.Custom) {
+    comparisonFormData = {
+      ...formData,
+      adhoc_filters: getComparisonFilters(formData, extraFormData),
+      extra_form_data: {
+        ...extraFormData,
+        time_range: undefined,
+      },
+    };
+  } else {
+    // This is when user selects Custom as time comparison
+    comparisonFormData = {
+      ...formData,
+      adhoc_filters: formData.adhoc_custom,
+      extra_form_data: {
+        ...extraFormData,
+        time_range: undefined,
+      },
+    };
+  }
+
+  return comparisonFormData;
+};
+
+export default getComparisonInfo;

superset-frontend/packages/superset-ui-core/src/time-comparison/index.ts

@@ -0,0 +1,23 @@
+/*
+ * 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.
+ */
+
+export * from './types';
+
+export { default as getComparisonInfo } from './getComparisonInfo';
+export { default as getComparisonFilters } from './getComparisonFilters';

superset-frontend/packages/superset-ui-core/src/time-comparison/types.ts

@@ -0,0 +1,30 @@
+/*
+ * 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.
+ */
+
+/**
+ * Supported comparison time ranges
+ */
+
+export enum ComparisonTimeRangeType {
+  Custom = 'c',
+  InheritedRange = 'r',
+  Month = 'm',
+  Week = 'w',
+  Year = 'y',
+}

superset-frontend/packages/superset-ui-core/src/validator/index.ts

@@ -24,3 +24,4 @@ export { default as validateNumber } from './validateNumber';
 export { default as validateNonEmpty } from './validateNonEmpty';
 export { default as validateMaxValue } from './validateMaxValue';
 export { default as validateMapboxStylesUrl } from './validateMapboxStylesUrl';
+export { default as validateTimeComparisonRangeValues } from './validateTimeComparisonRangeValues';

superset-frontend/packages/superset-ui-core/src/validator/validateTimeComparisonRangeValues.ts

@@ -0,0 +1,37 @@
+/*
+ * 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 { ComparisonTimeRangeType } from '../time-comparison';
+import { t } from '../translation';
+import { ensureIsArray } from '../utils';
+
+export const validateTimeComparisonRangeValues = (
+  timeRangeValue?: any,
+  controlValue?: any,
+) => {
+  const isCustomTimeRange = timeRangeValue === ComparisonTimeRangeType.Custom;
+  const isCustomControlEmpty = controlValue?.every(
+    (val: any) => ensureIsArray(val).length === 0,
+  );
+  return isCustomTimeRange && isCustomControlEmpty
+    ? [t('Filters for comparison must have a value')]
+    : [];
+};
+
+export default validateTimeComparisonRangeValues;

superset-frontend/packages/superset-ui-core/test/time-comparison/getComparisonFilters.test.ts

@@ -0,0 +1,144 @@
+/*
+ * 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 { getComparisonFilters } from '@superset-ui/core';
+
+const form_data = {
+  datasource: '22__table',
+  viz_type: 'pop_kpi',
+  slice_id: 97,
+  url_params: {
+    form_data_key:
+      'TaBakyDiAx2VsQ47gLmlsJKeN4foqnoxUKdbQrM05qnKMRjO9PDe42iZN1oxmxZ8',
+    save_action: 'overwrite',
+    slice_id: '97',
+  },
+  metrics: ['count'],
+  adhoc_filters: [
+    {
+      clause: 'WHERE',
+      comparator: '2004-02-16 : 2024-02-16',
+      datasourceWarning: false,
+      expressionType: 'SIMPLE',
+      filterOptionName: 'filter_8274fo9pogn_ihi8x28o7a',
+      isExtra: false,
+      isNew: false,
+      operator: 'TEMPORAL_RANGE',
+      sqlExpression: null,
+      subject: 'order_date',
+    } as any,
+  ],
+  time_comparison: 'y',
+  adhoc_custom: [
+    {
+      clause: 'WHERE',
+      comparator: 'No filter',
+      expressionType: 'SIMPLE',
+      operator: 'TEMPORAL_RANGE',
+      subject: 'order_date',
+    },
+  ],
+  row_limit: 10000,
+  y_axis_format: 'SMART_NUMBER',
+  header_font_size: 60,
+  subheader_font_size: 26,
+  comparison_color_enabled: true,
+  extra_form_data: {},
+  force: false,
+  result_format: 'json',
+  result_type: 'full',
+};
+
+const mockExtraFormData = {
+  time_range: 'new and cool range from extra form data',
+};
+
+describe('getComparisonFilters', () => {
+  it('Keeps the original adhoc_filters since no extra data was passed', () => {
+    const result = getComparisonFilters(form_data, {});
+
+    expect(result).toEqual(form_data.adhoc_filters);
+  });
+
+  it('Updates the time_range if the filter if extra form data is passed', () => {
+    const result = getComparisonFilters(form_data, mockExtraFormData);
+
+    const expectedFilters = [
+      {
+        clause: 'WHERE',
+        comparator: 'new and cool range from extra form data',
+        datasourceWarning: false,
+        expressionType: 'SIMPLE',
+        filterOptionName: 'filter_8274fo9pogn_ihi8x28o7a',
+        isExtra: false,
+        isNew: false,
+        operator: 'TEMPORAL_RANGE',
+        sqlExpression: null,
+        subject: 'order_date',
+      } as any,
+    ];
+
+    expect(result.length).toEqual(1);
+    expect(result[0]).toEqual(expectedFilters[0]);
+  });
+
+  it('handles no time range filters', () => {
+    const result = getComparisonFilters(
+      {
+        ...form_data,
+        adhoc_filters: [
+          {
+            expressionType: 'SIMPLE',
+            subject: 'address_line1',
+            operator: 'IN',
+            comparator: ['7734 Strong St.'],
+            clause: 'WHERE',
+            isExtra: false,
+          },
+        ],
+      },
+      {},
+    );
+
+    const expectedFilters = [
+      {
+        expressionType: 'SIMPLE',
+        subject: 'address_line1',
+        operator: 'IN',
+        comparator: ['7734 Strong St.'],
+        clause: 'WHERE',
+        isExtra: false,
+      },
+    ];
+    expect(result.length).toEqual(1);
+    expect(result[0]).toEqual(expectedFilters[0]);
+  });
+
+  it('If adhoc_filter is undefrined the code wont break', () => {
+    const result = getComparisonFilters(
+      {
+        ...form_data,
+        adhoc_filters: undefined,
+      },
+      {},
+    );
+
+    expect(result).toEqual([]);
+  });
+});

superset-frontend/packages/superset-ui-core/test/time-comparison/getComparisonInfo.test.ts

@@ -0,0 +1,174 @@
+/*
+ * 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 { getComparisonInfo, ComparisonTimeRangeType } from '@superset-ui/core';
+
+const form_data = {
+  datasource: '22__table',
+  viz_type: 'pop_kpi',
+  slice_id: 97,
+  url_params: {
+    form_data_key:
+      'TaBakyDiAx2VsQ47gLmlsJKeN4foqnoxUKdbQrM05qnKMRjO9PDe42iZN1oxmxZ8',
+    save_action: 'overwrite',
+    slice_id: '97',
+  },
+  metrics: ['count'],
+  adhoc_filters: [
+    {
+      clause: 'WHERE',
+      comparator: '2004-02-16 : 2024-02-16',
+      datasourceWarning: false,
+      expressionType: 'SIMPLE',
+      filterOptionName: 'filter_8274fo9pogn_ihi8x28o7a',
+      isExtra: false,
+      isNew: false,
+      operator: 'TEMPORAL_RANGE',
+      sqlExpression: null,
+      subject: 'order_date',
+    } as any,
+  ],
+  time_comparison: 'y',
+  adhoc_custom: [
+    {
+      clause: 'WHERE',
+      comparator: 'No filter',
+      expressionType: 'SIMPLE',
+      operator: 'TEMPORAL_RANGE',
+      subject: 'order_date',
+    },
+  ],
+  row_limit: 10000,
+  y_axis_format: 'SMART_NUMBER',
+  header_font_size: 60,
+  subheader_font_size: 26,
+  comparison_color_enabled: true,
+  extra_form_data: {},
+  force: false,
+  result_format: 'json',
+  result_type: 'full',
+};
+
+const mockExtraFormData = {
+  time_range: 'new and cool range from extra form data',
+};
+
+describe('getComparisonInfo', () => {
+  it('Keeps the original adhoc_filters since no extra data was passed', () => {
+    const resultFormData = getComparisonInfo(
+      form_data,
+      ComparisonTimeRangeType.Year,
+      {},
+    );
+    expect(resultFormData).toEqual(form_data);
+  });
+
+  it('Updates the time_range of the adhoc_filters when extra form data is passed', () => {
+    const resultFormData = getComparisonInfo(
+      form_data,
+      ComparisonTimeRangeType.Month,
+      mockExtraFormData,
+    );
+
+    const expectedFilters = [
+      {
+        clause: 'WHERE',
+        comparator: 'new and cool range from extra form data',
+        datasourceWarning: false,
+        expressionType: 'SIMPLE',
+        filterOptionName: 'filter_8274fo9pogn_ihi8x28o7a',
+        isExtra: false,
+        isNew: false,
+        operator: 'TEMPORAL_RANGE',
+        sqlExpression: null,
+        subject: 'order_date',
+      } as any,
+    ];
+
+    expect(resultFormData.adhoc_filters?.length).toEqual(1);
+    expect(resultFormData.adhoc_filters).toEqual(expectedFilters);
+  });
+
+  it('handles no time range filters', () => {
+    const resultFormData = getComparisonInfo(
+      {
+        ...form_data,
+        adhoc_filters: [
+          {
+            expressionType: 'SIMPLE',
+            subject: 'address_line1',
+            operator: 'IN',
+            comparator: ['7734 Strong St.'],
+            clause: 'WHERE',
+            isExtra: false,
+          },
+        ],
+      },
+      ComparisonTimeRangeType.Week,
+      {},
+    );
+
+    const expectedFilters = [
+      {
+        expressionType: 'SIMPLE',
+        subject: 'address_line1',
+        operator: 'IN',
+        comparator: ['7734 Strong St.'],
+        clause: 'WHERE',
+        isExtra: false,
+      },
+    ];
+    expect(resultFormData.adhoc_filters?.length).toEqual(1);
+    expect(resultFormData.adhoc_filters?.[0]).toEqual(expectedFilters[0]);
+  });
+
+  it('If adhoc_filter is undefrined the code wont break', () => {
+    const resultFormData = getComparisonInfo(
+      {
+        ...form_data,
+        adhoc_filters: undefined,
+      },
+      ComparisonTimeRangeType.InheritedRange,
+      {},
+    );
+
+    expect(resultFormData.adhoc_filters?.length).toEqual(0);
+    expect(resultFormData.adhoc_filters).toEqual([]);
+  });
+
+  it('Handles the custom time filters and return the correct time shift text', () => {
+    const resultFormData = getComparisonInfo(
+      form_data,
+      ComparisonTimeRangeType.Custom,
+      {},
+    );
+
+    const expectedFilters = [
+      {
+        clause: 'WHERE',
+        comparator: 'No filter',
+        expressionType: 'SIMPLE',
+        operator: 'TEMPORAL_RANGE',
+        subject: 'order_date',
+      },
+    ];
+    expect(resultFormData.adhoc_filters?.length).toEqual(1);
+    expect(resultFormData.adhoc_filters).toEqual(expectedFilters);
+  });
+});

superset-frontend/packages/superset-ui-core/test/time-comparison/index.test.ts

@@ -0,0 +1,32 @@
+/*
+ * 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 {
+  ComparisonTimeRangeType,
+  getComparisonFilters,
+  getComparisonInfo,
+} from '@superset-ui/core';
+
+describe('index', () => {
+  it('exports modules', () => {
+    [ComparisonTimeRangeType, getComparisonFilters, getComparisonInfo].forEach(
+      x => expect(x).toBeDefined(),
+    );
+  });
+});

superset-frontend/packages/superset-ui-core/test/validator/validateTimeComparisonRangeValues.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 {
+  ComparisonTimeRangeType,
+  validateTimeComparisonRangeValues,
+} from '@superset-ui/core';
+import './setup';
+
+describe('validateTimeComparisonRangeValues()', () => {
+  it('returns the warning message if invalid', () => {
+    expect(
+      validateTimeComparisonRangeValues(ComparisonTimeRangeType.Custom, []),
+    ).toBeTruthy();
+    expect(
+      validateTimeComparisonRangeValues(
+        ComparisonTimeRangeType.Custom,
+        undefined,
+      ),
+    ).toBeTruthy();
+    expect(
+      validateTimeComparisonRangeValues(ComparisonTimeRangeType.Custom, null),
+    ).toBeTruthy();
+  });
+  it('returns empty array if the input is valid', () => {
+    expect(
+      validateTimeComparisonRangeValues(ComparisonTimeRangeType.Year, []),
+    ).toEqual([]);
+    expect(
+      validateTimeComparisonRangeValues(
+        ComparisonTimeRangeType.Year,
+        undefined,
+      ),
+    ).toEqual([]);
+    expect(
+      validateTimeComparisonRangeValues(ComparisonTimeRangeType.Year, null),
+    ).toEqual([]);
+    expect(
+      validateTimeComparisonRangeValues(ComparisonTimeRangeType.Custom, [1]),
+    ).toEqual([]);
+  });
+});

superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/plugin/buildQuery.ts

@@ -17,11 +17,11 @@
  * under the License.
  */
 import {
-  AdhocFilter,
   buildQueryContext,
+  getComparisonInfo,
+  ComparisonTimeRangeType,
   QueryFormData,
 } from '@superset-ui/core';
-import { computeQueryBComparator } from '../utils';
 
 /**
  * The buildQuery function is used to create an instance of QueryContext that's
@@ -52,63 +52,28 @@ export default function buildQuery(formData: QueryFormData) {
     },
   ]);
 
-  const timeFilterIndex: number =
-    formData.adhoc_filters?.findIndex(
-      filter => 'operator' in filter && filter.operator === 'TEMPORAL_RANGE',
-    ) ?? -1;
+  const comparisonFormData = getComparisonInfo(
+    formData,
+    timeComparison,
+    extraFormData,
+  );
 
-  const timeFilter: AdhocFilter | null =
-    timeFilterIndex !== -1 && formData.adhoc_filters
-      ? formData.adhoc_filters[timeFilterIndex]
-      : null;
-
-  let formDataB: QueryFormData;
-  let queryBComparator = null;
-
-  if (timeComparison !== 'c') {
-    queryBComparator = computeQueryBComparator(
-      formData.adhoc_filters || [],
-      timeComparison,
-      extraFormData,
-    );
-
-    const queryBFilter: any = {
-      ...timeFilter,
-      comparator: queryBComparator,
-    };
-
-    const otherFilters = formData.adhoc_filters?.filter(
-      (_value: any, index: number) => timeFilterIndex !== index,
-    );
-    const queryBFilters = otherFilters
-      ? [queryBFilter, ...otherFilters]
-      : [queryBFilter];
-
-    formDataB = {
-      ...formData,
-      adhoc_filters: queryBFilters,
-      extra_form_data: {
-        ...extraFormData,
-        time_range: undefined,
+  const queryContextB = buildQueryContext(
+    comparisonFormData,
+    baseQueryObject => [
+      {
+        ...baseQueryObject,
+        groupby,
+        extras: {
+          ...baseQueryObject.extras,
+          instant_time_comparison_range:
+            timeComparison !== ComparisonTimeRangeType.Custom
+              ? timeComparison
+              : undefined,
+        },
       },
-    };
-  } else {
-    formDataB = {
-      ...formData,
-      adhoc_filters: formData.adhoc_custom,
-      extra_form_data: {
-        ...extraFormData,
-        time_range: undefined,
-      },
-    };
-  }
-
-  const queryContextB = buildQueryContext(formDataB, baseQueryObject => [
-    {
-      ...baseQueryObject,
-      groupby,
-    },
-  ]);
+    ],
+  );
 
   return {
     ...queryContextA,

superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/plugin/controlPanel.ts

@@ -16,7 +16,11 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { ensureIsArray, t } from '@superset-ui/core';
+import {
+  ComparisonTimeRangeType,
+  t,
+  validateTimeComparisonRangeValues,
+} from '@superset-ui/core';
 import {
   ControlPanelConfig,
   ControlPanelState,
@@ -25,19 +29,6 @@ import {
   sharedControls,
 } from '@superset-ui/chart-controls';
 
-const validateTimeComparisonRangeValues = (
-  timeRangeValue?: any,
-  controlValue?: any,
-) => {
-  const isCustomTimeRange = timeRangeValue === 'c';
-  const isCustomControlEmpty = controlValue?.every(
-    (val: any) => ensureIsArray(val).length === 0,
-  );
-  return isCustomTimeRange && isCustomControlEmpty
-    ? [t('Filters for comparison must have a value')]
-    : [];
-};
-
 const config: ControlPanelConfig = {
   controlPanelSections: [
     {
@@ -79,7 +70,8 @@ const config: ControlPanelConfig = {
               description:
                 'This only applies when selecting the Range for Comparison Type: Custom',
               visibility: ({ controls }) =>
-                controls?.time_comparison?.value === 'c',
+                controls?.time_comparison?.value ===
+                ComparisonTimeRangeType.Custom,
               mapStateToProps: (
                 state: ControlPanelState,
                 controlState: ControlState,

superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/plugin/transformProps.ts

@@ -23,8 +23,8 @@ import {
   getValueFormatter,
   NumberFormats,
   getNumberFormatter,
+  formatTimeRange,
 } from '@superset-ui/core';
-import { computeQueryBComparator, formatCustomComparator } from '../utils';
 
 export const parseMetricValue = (metricValue: number | string | null) => {
   if (typeof metricValue === 'string') {
@@ -85,7 +85,11 @@ export default function transformProps(chartProps: ChartProps) {
     comparisonColorEnabled,
   } = formData;
   const { data: dataA = [] } = queriesData[0];
-  const { data: dataB = [] } = queriesData[1];
+  const {
+    data: dataB = [],
+    from_dttm: comparisonFromDatetime,
+    to_dttm: comparisonToDatetime,
+  } = queriesData[1];
   const data = dataA;
   const metricName = getMetricLabel(metric);
   let bigNumber: number | string =
@@ -129,18 +133,10 @@ export default function transformProps(chartProps: ChartProps) {
   prevNumber = numberFormatter(prevNumber);
   valueDifference = numberFormatter(valueDifference);
   const percentDifference: string = formatPercentChange(percentDifferenceNum);
-  const comparatorText =
-    formData.timeComparison !== 'c'
-      ? ` ${computeQueryBComparator(
-          formData.adhocFilters,
-          formData.timeComparison,
-          formData.extraFormData,
-          ' - ',
-        )}`
-      : `${formatCustomComparator(
-          formData.adhocCustom,
-          formData.extraFormData,
-        )}`;
+  const comparatorText = formatTimeRange('%Y-%m-%d', [
+    comparisonFromDatetime,
+    comparisonToDatetime,
+  ]);
 
   return {
     width,

superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/utils.ts

@@ -1,277 +0,0 @@
-/**
- * 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 { AdhocFilter } from '@superset-ui/core';
-import moment, { Moment } from 'moment';
-
-type MomentTuple = [moment.Moment | null, moment.Moment | null];
-
-const getSinceUntil = (
-  timeRange: string | null = null,
-  relativeStart: string | null = null,
-  relativeEnd: string | null = null,
-): MomentTuple => {
-  const separator = ' : ';
-  const effectiveRelativeStart = relativeStart || 'today';
-  const effectiveRelativeEnd = relativeEnd || 'today';
-
-  if (!timeRange) {
-    return [null, null];
-  }
-
-  let modTimeRange: string | null = timeRange;
-
-  if (timeRange === 'NO_TIME_RANGE' || timeRange === '_(NO_TIME_RANGE)') {
-    return [null, null];
-  }
-
-  if (timeRange?.startsWith('last') && !timeRange.includes(separator)) {
-    modTimeRange = timeRange + separator + effectiveRelativeEnd;
-  }
-
-  if (timeRange?.startsWith('next') && !timeRange.includes(separator)) {
-    modTimeRange = effectiveRelativeStart + separator + timeRange;
-  }
-
-  if (
-    timeRange?.startsWith('previous calendar week') &&
-    !timeRange.includes(separator)
-  ) {
-    return [
-      moment().subtract(1, 'week').startOf('week'),
-      moment().startOf('week'),
-    ];
-  }
-
-  if (
-    timeRange?.startsWith('previous calendar month') &&
-    !timeRange.includes(separator)
-  ) {
-    return [
-      moment().subtract(1, 'month').startOf('month'),
-      moment().startOf('month'),
-    ];
-  }
-
-  if (
-    timeRange?.startsWith('previous calendar year') &&
-    !timeRange.includes(separator)
-  ) {
-    return [
-      moment().subtract(1, 'year').startOf('year'),
-      moment().startOf('year'),
-    ];
-  }
-
-  const timeRangeLookup: Array<[RegExp, (...args: string[]) => Moment]> = [
-    [
-      /^last\s+(day|week|month|quarter|year)$/i,
-      (unit: string) =>
-        moment().subtract(1, unit as moment.unitOfTime.DurationConstructor),
-    ],
-    [
-      /^last\s+([0-9]+)\s+(second|minute|hour|day|week|month|year)s?$/i,
-      (delta: string, unit: string) =>
-        moment().subtract(delta, unit as moment.unitOfTime.DurationConstructor),
-    ],
-    [
-      /^next\s+([0-9]+)\s+(second|minute|hour|day|week|month|year)s?$/i,
-      (delta: string, unit: string) =>
-        moment().add(delta, unit as moment.unitOfTime.DurationConstructor),
-    ],
-    [
-      // eslint-disable-next-line no-useless-escape
-      /DATEADD\(DATETIME\("([^"]+)"\),\s*(-?\d+),\s*([^\)]+)\)/i,
-      (timePart: string, delta: string, unit: string) => {
-        if (timePart === 'now') {
-          return moment().add(
-            delta,
-            unit as moment.unitOfTime.DurationConstructor,
-          );
-        }
-        if (moment(timePart.toUpperCase(), true).isValid()) {
-          return moment(timePart).add(
-            delta,
-            unit as moment.unitOfTime.DurationConstructor,
-          );
-        }
-        return moment();
-      },
-    ],
-  ];
-
-  const sinceAndUntilPartition = modTimeRange
-    .split(separator, 2)
-    .map(part => part.trim());
-
-  const sinceAndUntil: (Moment | null)[] = sinceAndUntilPartition.map(part => {
-    if (!part) {
-      return null;
-    }
-
-    let transformedValue: Moment | null = null;
-    // Matching time_range_lookup
-    const matched = timeRangeLookup.some(([pattern, fn]) => {
-      const result = part.match(pattern);
-      if (result) {
-        transformedValue = fn(...result.slice(1));
-        return true;
-      }
-
-      if (part === 'today') {
-        transformedValue = moment().startOf('day');
-        return true;
-      }
-
-      if (part === 'now') {
-        transformedValue = moment();
-        return true;
-      }
-      return false;
-    });
-
-    if (matched && transformedValue !== null) {
-      // Handle the transformed value
-    } else {
-      // Handle the case when there was no match
-      transformedValue = moment(`${part}`);
-    }
-
-    return transformedValue;
-  });
-
-  const [_since, _until] = sinceAndUntil;
-
-  if (_since && _until && _since.isAfter(_until)) {
-    throw new Error('From date cannot be larger than to date');
-  }
-
-  return [_since, _until];
-};
-
-const calculatePrev = (
-  startDate: Moment | null,
-  endDate: Moment | null,
-  calcType: String,
-) => {
-  if (!startDate || !endDate) {
-    return [null, null];
-  }
-
-  const daysBetween = endDate.diff(startDate, 'days');
-
-  let startDatePrev = moment();
-  let endDatePrev = moment();
-  if (calcType === 'y') {
-    startDatePrev = startDate.subtract(1, 'year');
-    endDatePrev = endDate.subtract(1, 'year');
-  } else if (calcType === 'w') {
-    startDatePrev = startDate.subtract(1, 'week');
-    endDatePrev = endDate.subtract(1, 'week');
-  } else if (calcType === 'm') {
-    startDatePrev = startDate.subtract(1, 'month');
-    endDatePrev = endDate.subtract(1, 'month');
-  } else if (calcType === 'r') {
-    startDatePrev = startDate.clone().subtract(daysBetween.valueOf(), 'day');
-    endDatePrev = startDate;
-  } else {
-    startDatePrev = startDate.subtract(1, 'year');
-    endDatePrev = endDate.subtract(1, 'year');
-  }
-
-  return [startDatePrev, endDatePrev];
-};
-
-const getTimeRange = (
-  adhocFilters: AdhocFilter[],
-  extraFormData: any,
-): string | null => {
-  const timeFilterIndex =
-    adhocFilters?.findIndex(
-      filter => 'operator' in filter && filter.operator === 'TEMPORAL_RANGE',
-    ) ?? -1;
-
-  const timeFilter =
-    timeFilterIndex !== -1 ? adhocFilters[timeFilterIndex] : null;
-
-  if (
-    timeFilter &&
-    'comparator' in timeFilter &&
-    typeof timeFilter.comparator === 'string'
-  ) {
-    let timeRange = timeFilter.comparator.toLocaleLowerCase();
-    if (extraFormData?.time_range) {
-      timeRange = extraFormData.time_range;
-    }
-    return timeRange;
-  }
-
-  return null;
-};
-
-export const computeQueryBComparator = (
-  adhocFilters: AdhocFilter[],
-  timeComparison: string,
-  extraFormData: any,
-  join = ':',
-) => {
-  const timeRange = getTimeRange(adhocFilters, extraFormData);
-
-  let testSince = null;
-  let testUntil = null;
-
-  if (timeRange) {
-    [testSince, testUntil] = getSinceUntil(timeRange);
-  }
-
-  if (timeComparison !== 'c') {
-    const [prevStartDateMoment, prevEndDateMoment] = calculatePrev(
-      testSince,
-      testUntil,
-      timeComparison,
-    );
-
-    return `${prevStartDateMoment?.format(
-      'YYYY-MM-DDTHH:mm:ss',
-    )} ${join} ${prevEndDateMoment?.format('YYYY-MM-DDTHH:mm:ss')}`.replace(
-      /Z/g,
-      '',
-    );
-  }
-
-  return null;
-};
-
-export const formatCustomComparator = (
-  adhocFilters: AdhocFilter[],
-  extraFormData: any,
-): string => {
-  const timeRange = getTimeRange(adhocFilters, extraFormData);
-
-  if (timeRange) {
-    const [start, end] = timeRange.split(' : ').map(dateStr => {
-      const formattedDate = moment(dateStr).format('YYYY-MM-DDTHH:mm:ss');
-      return formattedDate.replace(/Z/g, '');
-    });
-
-    return `${start} - ${end}`;
-  }
-
-  return '';
-};

superset/charts/schemas.py

@@ -990,6 +990,14 @@ class ChartDataExtrasSchema(Schema):
         ),
         allow_none=True,
     )
+    instant_time_comparison_range = fields.String(
+        metadata={
+            "description": "This is only set using the new time comparison controls "
+            "that is made available in some plugins behind the experimental "
+            "feature flag."
+        },
+        allow_none=True,
+    )
 
 
 class AnnotationLayerSchema(Schema):

superset/common/utils/time_range_utils.py

@@ -39,6 +39,9 @@ def get_since_until_from_time_range(
         ),
         time_range=time_range,
         time_shift=time_shift,
+        instant_time_comparison_range=(extras or {}).get(
+            "instant_time_comparison_range"
+        ),
     )
 
 

superset/constants.py

@@ -42,6 +42,14 @@ QUERY_EARLY_CANCEL_KEY = "early_cancel_query"
 LRU_CACHE_MAX_SIZE = 256
 
 
+# Used when calculating the time shift for time comparison
+class InstantTimeComparison(StrEnum):
+    INHERITED = "r"
+    YEAR = "y"
+    MONTH = "m"
+    WEEK = "w"
+
+
 class RouteMethod:  # pylint: disable=too-few-public-methods
     """
     Route methods are a FAB concept around ModelView and RestModelView

superset/utils/date_parser.py

@@ -46,7 +46,7 @@ from superset.commands.chart.exceptions import (
     TimeRangeAmbiguousError,
     TimeRangeParseFailError,
 )
-from superset.constants import LRU_CACHE_MAX_SIZE, NO_TIME_RANGE
+from superset.constants import InstantTimeComparison, LRU_CACHE_MAX_SIZE, NO_TIME_RANGE
 
 ParserElement.enablePackrat()
 
@@ -142,13 +142,14 @@ def parse_past_timedelta(
     )
 
 
-def get_since_until(  # pylint: disable=too-many-arguments,too-many-locals,too-many-branches
+def get_since_until(  # pylint: disable=too-many-arguments,too-many-locals,too-many-branches,too-many-statements
     time_range: Optional[str] = None,
     since: Optional[str] = None,
     until: Optional[str] = None,
     time_shift: Optional[str] = None,
     relative_start: Optional[str] = None,
     relative_end: Optional[str] = None,
+    instant_time_comparison_range: Optional[str] = None,
 ) -> tuple[Optional[datetime], Optional[datetime]]:
     """Return `since` and `until` date time tuple from string representations of
     time_range, since, until and time_shift.
@@ -263,6 +264,47 @@ def get_since_until(  # pylint: disable=too-many-arguments,too-many-locals,too-m
         _since = _since if _since is None else (_since - time_delta)
         _until = _until if _until is None else (_until - time_delta)
 
+    if instant_time_comparison_range:
+        # This is only set using the new time comparison controls
+        # that is made available in some plugins behind the experimental
+        # feature flag.
+        # pylint: disable=import-outside-toplevel
+        from superset import feature_flag_manager
+
+        if feature_flag_manager.is_feature_enabled("CHART_PLUGINS_EXPERIMENTAL"):
+            time_unit = ""
+            delta_in_days = None
+            if instant_time_comparison_range == InstantTimeComparison.YEAR:
+                time_unit = "YEAR"
+            elif instant_time_comparison_range == InstantTimeComparison.MONTH:
+                time_unit = "MONTH"
+            elif instant_time_comparison_range == InstantTimeComparison.WEEK:
+                time_unit = "WEEK"
+            elif instant_time_comparison_range == InstantTimeComparison.INHERITED:
+                delta_in_days = (_until - _since).days if _since and _until else None
+                time_unit = "DAY"
+
+            if time_unit:
+                strtfime_since = (
+                    _since.strftime("%Y-%m-%dT%H:%M:%S") if _since else relative_start
+                )
+                strtfime_until = (
+                    _until.strftime("%Y-%m-%dT%H:%M:%S") if _until else relative_end
+                )
+
+                since_and_until = [
+                    (
+                        f"DATEADD(DATETIME('{strtfime_since}'), "
+                        f"-{delta_in_days or 1}, {time_unit})"
+                    ),
+                    (
+                        f"DATEADD(DATETIME('{strtfime_until}'), "
+                        f"-{delta_in_days or 1}, {time_unit})"
+                    ),
+                ]
+
+                _since, _until = map(datetime_eval, since_and_until)
+
     if _since and _until and _since > _until:
         raise ValueError(_("From date cannot be larger than to date"))
 

tests/unit_tests/utils/date_parser_tests.py

@@ -35,6 +35,7 @@ from superset.utils.date_parser import (
     parse_human_timedelta,
     parse_past_timedelta,
 )
+from tests.unit_tests.conftest import with_feature_flags
 
 
 def mock_parse_human_datetime(s: str) -> Optional[datetime]:
@@ -157,10 +158,81 @@ def test_get_since_until() -> None:
     expected = datetime(2015, 1, 1, 0, 0, 0), datetime(2016, 1, 1, 0, 0, 0)
     assert result == expected
 
+    # Tests for our new instant_time_comparison logic and Feature Flag off
+    result = get_since_until(
+        time_range="2000-01-01T00:00:00 : 2018-01-01T00:00:00",
+        instant_time_comparison_range="y",
+    )
+    expected = datetime(2000, 1, 1), datetime(2018, 1, 1)
+    assert result == expected
+
+    result = get_since_until(
+        time_range="2000-01-01T00:00:00 : 2018-01-01T00:00:00",
+        instant_time_comparison_range="m",
+    )
+    expected = datetime(2000, 1, 1), datetime(2018, 1, 1)
+    assert result == expected
+
+    result = get_since_until(
+        time_range="2000-01-01T00:00:00 : 2018-01-01T00:00:00",
+        instant_time_comparison_range="w",
+    )
+    expected = datetime(2000, 1, 1), datetime(2018, 1, 1)
+    assert result == expected
+
+    result = get_since_until(
+        time_range="2000-01-01T00:00:00 : 2018-01-01T00:00:00",
+        instant_time_comparison_range="r",
+    )
+    expected = datetime(2000, 1, 1), datetime(2018, 1, 1)
+    assert result == expected
+
     with pytest.raises(ValueError):
         get_since_until(time_range="tomorrow : yesterday")
 
 
+@with_feature_flags(CHART_PLUGINS_EXPERIMENTAL=True)
+@patch("superset.utils.date_parser.parse_human_datetime", mock_parse_human_datetime)
+def test_get_since_until_instant_time_comparison_enabled() -> None:
+    result: tuple[Optional[datetime], Optional[datetime]]
+    expected: tuple[Optional[datetime], Optional[datetime]]
+
+    result = get_since_until(
+        time_range="2000-01-01T00:00:00 : 2018-01-01T00:00:00",
+        instant_time_comparison_range="y",
+    )
+    expected = datetime(1999, 1, 1), datetime(2017, 1, 1)
+    assert result == expected
+
+    result = get_since_until(
+        time_range="2000-01-01T00:00:00 : 2018-01-01T00:00:00",
+        instant_time_comparison_range="m",
+    )
+    expected = datetime(1999, 12, 1), datetime(2017, 12, 1)
+    assert result == expected
+
+    result = get_since_until(
+        time_range="2000-01-01T00:00:00 : 2018-01-01T00:00:00",
+        instant_time_comparison_range="w",
+    )
+    expected = datetime(1999, 12, 25), datetime(2017, 12, 25)
+    assert result == expected
+
+    result = get_since_until(
+        time_range="2000-01-01T00:00:00 : 2018-01-01T00:00:00",
+        instant_time_comparison_range="r",
+    )
+    expected = datetime(1981, 12, 31), datetime(2000, 1, 1)
+    assert result == expected
+
+    result = get_since_until(
+        time_range="2000-01-01T00:00:00 : 2018-01-01T00:00:00",
+        instant_time_comparison_range="unknown",
+    )
+    expected = datetime(2000, 1, 1), datetime(2018, 1, 1)
+    assert result == expected
+
+
 @patch("superset.utils.date_parser.parse_human_datetime", mock_parse_human_datetime)
 def test_datetime_eval() -> None:
     result = datetime_eval("datetime('now')")