feat(time_comparison): Support all date formats when computing custom and inherit offsets (apache#30002)

Antonio-RiveroMartnez · nyohasstium · commit 72ddf8061602 · 2025-01-02T11:05:06.000Z
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/constants.ts b/superset-frontend/packages/superset-ui-chart-controls/src/constants.ts
@@ -78,3 +78,5 @@ export const DEFAULT_XAXIS_SORT_SERIES_DATA: SortSeriesData = {
   sort_series_type: SortSeriesType.Name,
   sort_series_ascending: true,
 };
+
+export const DEFAULT_DATE_PATTERN = /\d{4}-\d{2}-\d{2}/g;
diff --git a/superset-frontend/packages/superset-ui-core/src/time-comparison/getTimeOffset.ts b/superset-frontend/packages/superset-ui-core/src/time-comparison/getTimeOffset.ts
@@ -21,18 +21,19 @@ import { ensureIsArray } from '../utils';
 import { customTimeRangeDecode } from './customTimeRangeDecode';
 
 const DAY_IN_MS = 24 * 60 * 60 * 1000;
+
 export const parseDttmToDate = (
   dttm: string,
   isEndDate = false,
   computingShifts = false,
 ) => {
   const now = new Date();
-  if (
-    dttm === 'now' ||
-    dttm === 'today' ||
-    dttm === 'No filter' ||
-    dttm === ''
-  ) {
+  if (dttm === 'now' || dttm === 'No filter' || dttm === '') {
+    return now;
+  }
+
+  if (dttm === 'today') {
+    now.setHours(0, 0, 0, 0);
     return now;
   }
 
@@ -280,9 +281,12 @@ export const getTimeOffset = ({
 
   const customShift =
     customStartDateTime &&
+    filterStartDateTime &&
     Math.round((filterStartDateTime - customStartDateTime) / DAY_IN_MS);
   const inInheritShift =
     isInherit &&
+    filterEndDateTime &&
+    filterStartDateTime &&
     Math.round((filterEndDateTime - filterStartDateTime) / DAY_IN_MS);
 
   const newShifts = ensureIsArray(shifts)
@@ -292,7 +296,7 @@ export const getTimeOffset = ({
           if (includeFutureOffsets && customShift < 0) {
             return `${customShift * -1} days after`;
           }
-          if (customShift >= 0) {
+          if (customShift >= 0 && filterStartDateTime) {
             return `${customShift} days ago`;
           }
         }
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/PopKPI.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/PopKPI.tsx
@@ -26,7 +26,7 @@ import {
   t,
   useTheme,
 } from '@superset-ui/core';
-import { Tooltip } from '@superset-ui/chart-controls';
+import { DEFAULT_DATE_PATTERN, Tooltip } from '@superset-ui/chart-controls';
 import { isEmpty } from 'lodash';
 import {
   ColorSchemeEnum,
@@ -90,27 +90,33 @@ export default function PopKPI(props: PopKPIProps) {
     if (!currentTimeRangeFilter || (!shift && !startDateOffset)) {
       setComparisonRange('');
     } else if (!isEmpty(shift) || startDateOffset) {
-      const newShift = getTimeOffset({
-        timeRangeFilter: {
-          ...currentTimeRangeFilter,
-          comparator:
-            dashboardTimeRange ?? (currentTimeRangeFilter as any).comparator,
-        },
-        shifts: ensureIsArray(shift),
-        startDate: startDateOffset || '',
-      });
       const promise: any = fetchTimeRange(
         dashboardTimeRange ?? (currentTimeRangeFilter as any).comparator,
         currentTimeRangeFilter.subject,
-        newShift || [],
       );
       Promise.resolve(promise).then((res: any) => {
-        const response: string[] = ensureIsArray(res.value);
-        const firstRange: string = response.flat()[0];
-        const rangeText = firstRange.split('vs\n');
-        setComparisonRange(
-          rangeText.length > 1 ? rangeText[1].trim() : rangeText[0],
-        );
+        const dates = res?.value?.match(DEFAULT_DATE_PATTERN);
+        const [parsedStartDate, parsedEndDate] = dates ?? [];
+        const newShift = getTimeOffset({
+          timeRangeFilter: {
+            ...currentTimeRangeFilter,
+            comparator: `${parsedStartDate} : ${parsedEndDate}`,
+          },
+          shifts: ensureIsArray(shift),
+          startDate: startDateOffset || '',
+        });
+        fetchTimeRange(
+          dashboardTimeRange ?? (currentTimeRangeFilter as any).comparator,
+          currentTimeRangeFilter.subject,
+          ensureIsArray(newShift),
+        ).then(res => {
+          const response: string[] = ensureIsArray(res.value);
+          const firstRange: string = response.flat()[0];
+          const rangeText = firstRange.split('vs\n');
+          setComparisonRange(
+            rangeText.length > 1 ? rangeText[1].trim() : rangeText[0],
+          );
+        });
       });
     }
   }, [currentTimeRangeFilter, shift, startDateOffset, dashboardTimeRange]);
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/buildQuery.ts
@@ -21,9 +21,6 @@ import {
   QueryFormData,
   PostProcessingRule,
   ensureIsArray,
-  SimpleAdhocFilter,
-  getTimeOffset,
-  parseDttmToDate,
 } from '@superset-ui/core';
 import {
   isTimeComparison,
@@ -37,43 +34,30 @@ export default function buildQuery(formData: QueryFormData) {
   const queryContextA = buildQueryContext(formData, baseQueryObject => {
     const postProcessing: PostProcessingRule[] = [];
     postProcessing.push(timeCompareOperator(formData, baseQueryObject));
-    const TimeRangeFilters =
-      formData.adhoc_filters?.filter(
-        (filter: SimpleAdhocFilter) => filter.operator === 'TEMPORAL_RANGE',
-      ) || [];
 
-    // In case the viz is using all version of controls, we try to load them
-    const previousCustomTimeRangeFilters: any =
-      formData.adhoc_custom?.filter(
-        (filter: SimpleAdhocFilter) => filter.operator === 'TEMPORAL_RANGE',
-      ) || [];
+    const nonCustomNorInheritShifts = ensureIsArray(
+      formData.time_compare,
+    ).filter((shift: string) => shift !== 'custom' && shift !== 'inherit');
+    const customOrInheritShifts = ensureIsArray(formData.time_compare).filter(
+      (shift: string) => shift === 'custom' || shift === 'inherit',
+    );
+
+    let timeOffsets: string[] = [];
 
-    let previousCustomStartDate = '';
-    if (
-      !isEmpty(previousCustomTimeRangeFilters) &&
-      previousCustomTimeRangeFilters[0]?.comparator !== 'No Filter'
-    ) {
-      previousCustomStartDate =
-        previousCustomTimeRangeFilters[0]?.comparator.split(' : ')[0];
+    // Shifts for non-custom or non inherit time comparison
+    if (!isEmpty(nonCustomNorInheritShifts)) {
+      timeOffsets = nonCustomNorInheritShifts;
     }
 
-    const timeOffsets = ensureIsArray(
-      isTimeComparison(formData, baseQueryObject)
-        ? getTimeOffset({
-            timeRangeFilter: {
-              ...TimeRangeFilters[0],
-              comparator:
-                baseQueryObject?.time_range ??
-                (TimeRangeFilters[0] as any)?.comparator,
-            },
-            shifts: formData.time_compare,
-            startDate:
-              previousCustomStartDate && !formData.start_date_offset
-                ? parseDttmToDate(previousCustomStartDate)?.toUTCString()
-                : formData.start_date_offset,
-          })
-        : [],
-    );
+    // Shifts for custom or inherit time comparison
+    if (!isEmpty(customOrInheritShifts)) {
+      if (customOrInheritShifts.includes('custom')) {
+        timeOffsets = timeOffsets.concat([formData.start_date_offset]);
+      }
+      if (customOrInheritShifts.includes('inherit')) {
+        timeOffsets = timeOffsets.concat(['inherit']);
+      }
+    }
     return [
       {
         ...baseQueryObject,
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/transformProps.ts
@@ -24,10 +24,7 @@ import {
   getNumberFormatter,
   SimpleAdhocFilter,
   ensureIsArray,
-  getTimeOffset,
-  parseDttmToDate,
 } from '@superset-ui/core';
-import { isEmpty } from 'lodash';
 import { getComparisonFontSize, getHeaderFontSize } from './utils';
 
 export const parseMetricValue = (metricValue: number | string | null) => {
@@ -99,37 +96,16 @@ export default function transformProps(chartProps: ChartProps) {
     (adhoc_filter: SimpleAdhocFilter) =>
       adhoc_filter.operator === 'TEMPORAL_RANGE',
   )?.[0];
-  // In case the viz is using all version of controls, we try to load them
-  const previousCustomTimeRangeFilters: any =
-    chartProps.rawFormData?.adhoc_custom?.filter(
-      (filter: SimpleAdhocFilter) => filter.operator === 'TEMPORAL_RANGE',
-    ) || [];
 
-  let previousCustomStartDate = '';
-  if (
-    !isEmpty(previousCustomTimeRangeFilters) &&
-    previousCustomTimeRangeFilters[0]?.comparator !== 'No Filter'
-  ) {
-    previousCustomStartDate =
-      previousCustomTimeRangeFilters[0]?.comparator.split(' : ')[0];
-  }
   const isCustomOrInherit =
     timeComparison === 'custom' || timeComparison === 'inherit';
   let dataOffset: string[] = [];
   if (isCustomOrInherit) {
-    dataOffset = getTimeOffset({
-      timeRangeFilter: {
-        ...currentTimeRangeFilter,
-        comparator:
-          formData?.extraFormData?.time_range ??
-          (currentTimeRangeFilter as any)?.comparator,
-      },
-      shifts: ensureIsArray(timeComparison),
-      startDate:
-        previousCustomStartDate && !startDateOffset
-          ? parseDttmToDate(previousCustomStartDate)?.toUTCString()
-          : startDateOffset,
-    });
+    if (timeComparison && timeComparison === 'custom') {
+      dataOffset = [startDateOffset];
+    } else {
+      dataOffset = ensureIsArray(timeComparison) || [];
+    }
   }
 
   const { value1, value2 } = data.reduce(
diff --git a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts
@@ -21,13 +21,10 @@ import {
   buildQueryContext,
   ensureIsArray,
   getMetricLabel,
-  getTimeOffset,
   isPhysicalColumn,
-  parseDttmToDate,
   QueryMode,
   QueryObject,
   removeDuplicates,
-  SimpleAdhocFilter,
 } from '@superset-ui/core';
 import { PostProcessingRule } from '@superset-ui/core/src/query/types/PostProcessing';
 import { BuildQuery } from '@superset-ui/core/src/chart/registries/ChartBuildQueryRegistrySingleton';
@@ -87,43 +84,35 @@ const buildQuery: BuildQuery<TableChartFormData> = (
     let { metrics, orderby = [], columns = [] } = baseQueryObject;
     const { extras = {} } = baseQueryObject;
     let postProcessing: PostProcessingRule[] = [];
-    const TimeRangeFilters =
-      formData.adhoc_filters?.filter(
-        (filter: SimpleAdhocFilter) => filter.operator === 'TEMPORAL_RANGE',
-      ) || [];
+    const nonCustomNorInheritShifts = ensureIsArray(
+      formData.time_compare,
+    ).filter((shift: string) => shift !== 'custom' && shift !== 'inherit');
+    const customOrInheritShifts = ensureIsArray(formData.time_compare).filter(
+      (shift: string) => shift === 'custom' || shift === 'inherit',
+    );
 
-    // In case the viz is using all version of controls, we try to load them
-    const previousCustomTimeRangeFilters: any =
-      formData.adhoc_custom?.filter(
-        (filter: SimpleAdhocFilter) => filter.operator === 'TEMPORAL_RANGE',
-      ) || [];
+    let timeOffsets: string[] = [];
 
-    let previousCustomStartDate = '';
+    // Shifts for non-custom or non inherit time comparison
     if (
-      !isEmpty(previousCustomTimeRangeFilters) &&
-      previousCustomTimeRangeFilters[0]?.comparator !== 'No Filter'
+      isTimeComparison(formData, baseQueryObject) &&
+      !isEmpty(nonCustomNorInheritShifts)
     ) {
-      previousCustomStartDate =
-        previousCustomTimeRangeFilters[0]?.comparator.split(' : ')[0];
+      timeOffsets = nonCustomNorInheritShifts;
     }
 
-    const timeOffsets = ensureIsArray(
-      isTimeComparison(formData, baseQueryObject)
-        ? getTimeOffset({
-            timeRangeFilter: {
-              ...TimeRangeFilters[0],
-              comparator:
-                baseQueryObject?.time_range ??
-                (TimeRangeFilters[0] as any)?.comparator,
-            },
-            shifts: formData.time_compare,
-            startDate:
-              previousCustomStartDate && !formData.start_date_offset
-                ? parseDttmToDate(previousCustomStartDate)?.toUTCString()
-                : formData.start_date_offset,
-          })
-        : [],
-    );
+    // Shifts for custom or inherit time comparison
+    if (
+      isTimeComparison(formData, baseQueryObject) &&
+      !isEmpty(customOrInheritShifts)
+    ) {
+      if (customOrInheritShifts.includes('custom')) {
+        timeOffsets = timeOffsets.concat([formData.start_date_offset]);
+      }
+      if (customOrInheritShifts.includes('inherit')) {
+        timeOffsets = timeOffsets.concat(['inherit']);
+      }
+    }
 
     let temporalColumnAdded = false;
     let temporalColumn = null;
diff --git a/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts b/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts
@@ -35,9 +35,6 @@ import {
   SMART_DATE_ID,
   TimeFormats,
   TimeFormatter,
-  SimpleAdhocFilter,
-  getTimeOffset,
-  parseDttmToDate,
 } from '@superset-ui/core';
 import {
   ColorFormatters,
@@ -597,37 +594,29 @@ const transformProps = (
   };
 
   const timeGrain = extractTimegrain(formData);
-  const TimeRangeFilters =
-    chartProps.rawFormData?.adhoc_filters?.filter(
-      (filter: SimpleAdhocFilter) => filter.operator === 'TEMPORAL_RANGE',
-    ) || [];
-  const previousCustomTimeRangeFilters: any =
-    chartProps.rawFormData?.adhoc_custom?.filter(
-      (filter: SimpleAdhocFilter) => filter.operator === 'TEMPORAL_RANGE',
-    ) || [];
-
-  let previousCustomStartDate = '';
-  if (
-    !isEmpty(previousCustomTimeRangeFilters) &&
-    previousCustomTimeRangeFilters[0]?.comparator !== 'No Filter'
-  ) {
-    previousCustomStartDate =
-      previousCustomTimeRangeFilters[0]?.comparator.split(' : ')[0];
+
+  const nonCustomNorInheritShifts = ensureIsArray(formData.time_compare).filter(
+    (shift: string) => shift !== 'custom' && shift !== 'inherit',
+  );
+  const customOrInheritShifts = ensureIsArray(formData.time_compare).filter(
+    (shift: string) => shift === 'custom' || shift === 'inherit',
+  );
+
+  let timeOffsets: string[] = [];
+
+  if (isUsingTimeComparison && !isEmpty(nonCustomNorInheritShifts)) {
+    timeOffsets = nonCustomNorInheritShifts;
   }
 
-  const timeOffsets = getTimeOffset({
-    timeRangeFilter: {
-      ...TimeRangeFilters[0],
-      comparator:
-        formData?.extra_form_data?.time_range ??
-        (TimeRangeFilters[0] as any)?.comparator,
-    },
-    shifts: formData.time_compare,
-    startDate:
-      previousCustomStartDate && !formData.start_date_offset
-        ? parseDttmToDate(previousCustomStartDate)?.toUTCString()
-        : formData.start_date_offset,
-  });
+  // Shifts for custom or inherit time comparison
+  if (isUsingTimeComparison && !isEmpty(customOrInheritShifts)) {
+    if (customOrInheritShifts.includes('custom')) {
+      timeOffsets = timeOffsets.concat([formData.start_date_offset]);
+    }
+    if (customOrInheritShifts.includes('inherit')) {
+      timeOffsets = timeOffsets.concat(['inherit']);
+    }
+  }
   const comparisonSuffix = isUsingTimeComparison
     ? ensureIsArray(timeOffsets)[0]
     : '';
diff --git a/superset-frontend/src/explore/components/controls/ComparisonRangeLabel.tsx b/superset-frontend/src/explore/components/controls/ComparisonRangeLabel.tsx
diff --git a/superset-frontend/src/explore/components/controls/TimeOffsetControl.tsx b/superset-frontend/src/explore/components/controls/TimeOffsetControl.tsx
diff --git a/superset/common/query_context_processor.py b/superset/common/query_context_processor.py
