feat: support mulitple temporal filters in AdhocFilter and move the T…

…ime Section away (#21767)

superset-frontend/packages/superset-ui-chart-controls/test/fixtures.ts → superset-frontend/packages/superset-ui-chart-controls/src/fixtures.ts

@@ -16,8 +16,8 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { Dataset } from '@superset-ui/chart-controls';
 import { DatasourceType } from '@superset-ui/core';
+import { Dataset } from './types';
 
 export const TestDataset: Dataset = {
   column_format: {},

superset-frontend/packages/superset-ui-chart-controls/src/index.ts

@@ -38,3 +38,4 @@ export * from './shared-controls/emitFilterControl';
 export * from './shared-controls/components';
 export * from './types';
 export * from './shared-controls/mixins';
+export * from './fixtures';

superset-frontend/packages/superset-ui-chart-controls/src/sections/sections.tsx

@@ -38,19 +38,23 @@ export const legacyTimeseriesTime: ControlPanelSectionConfig = {
   ],
 };
 
-export const genericTime: ControlPanelSectionConfig = {
-  ...baseTimeSection,
-  controlSetRows: [
-    ['granularity_sqla'],
-    [hasGenericChartAxes ? null : 'time_grain_sqla'],
-    ['time_range'],
-  ],
-};
+export const genericTime: ControlPanelSectionConfig = hasGenericChartAxes
+  ? { controlSetRows: [] }
+  : {
+      ...baseTimeSection,
+      controlSetRows: [
+        ['granularity_sqla'],
+        ['time_grain_sqla'],
+        ['time_range'],
+      ],
+    };
 
-export const legacyRegularTime: ControlPanelSectionConfig = {
-  ...baseTimeSection,
-  controlSetRows: [['granularity_sqla'], ['time_range']],
-};
+export const legacyRegularTime: ControlPanelSectionConfig = hasGenericChartAxes
+  ? { controlSetRows: [] }
+  : {
+      ...baseTimeSection,
+      controlSetRows: [['granularity_sqla'], ['time_range']],
+    };
 
 export const datasourceAndVizType: ControlPanelSectionConfig = {
   label: t('Datasource & Chart Type'),

superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx

@@ -40,8 +40,9 @@ import {
   ColumnMeta,
   FilterOption,
   temporalColumnMixin,
+  datePickerInAdhocFilterMixin,
+  xAxisMixin,
 } from '..';
-import { xAxisMixin } from './mixins';
 
 type Control = {
   savedMetrics?: Metric[] | null;
@@ -149,6 +150,7 @@ export const dndAdhocFilterControl: SharedControlConfig<
     datasource,
   }),
   provideFormDataToProps: true,
+  ...datePickerInAdhocFilterMixin,
 };
 
 export const dndAdhocMetricsControl: SharedControlConfig<

superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/mixins.tsx

@@ -17,7 +17,9 @@
  * under the License.
  */
 import {
+  ensureIsArray,
   hasGenericChartAxes,
+  NO_TIME_RANGE,
   QueryFormData,
   t,
   validateNonEmpty,
@@ -63,3 +65,57 @@ export const temporalColumnMixin: Pick<BaseControlConfig, 'mapStateToProps'> = {
     };
   },
 };
+
+export const datePickerInAdhocFilterMixin: Pick<
+  BaseControlConfig,
+  'initialValue'
+> = {
+  initialValue: (control: ControlState, state: ControlPanelState | null) => {
+    // skip initialValue if
+    // 1) GENERIC_CHART_AXES is disabled
+    // 2) there was a time filter in adhoc filters
+    if (
+      !hasGenericChartAxes ||
+      ensureIsArray(control.value).findIndex(
+        (flt: any) => flt?.operator === 'TEMPORAL_RANGE',
+      ) > -1
+    ) {
+      return undefined;
+    }
+
+    // should migrate original granularity_sqla and time_range into adhoc filter
+    // 1) granularity_sqla and time_range are existed
+    if (state?.form_data?.granularity_sqla && state?.form_data?.time_range) {
+      return [
+        ...ensureIsArray(control.value),
+        {
+          clause: 'WHERE',
+          subject: state.form_data.granularity_sqla,
+          operator: 'TEMPORAL_RANGE',
+          comparator: state.form_data.time_range,
+          expressionType: 'SIMPLE',
+        },
+      ];
+    }
+
+    // should apply the default time filter into adhoc filter
+    // 1) temporal column is existed in current datasource
+    const temporalColumn =
+      state?.datasource &&
+      getTemporalColumns(state.datasource).defaultTemporalColumn;
+    if (hasGenericChartAxes && temporalColumn) {
+      return [
+        ...ensureIsArray(control.value),
+        {
+          clause: 'WHERE',
+          subject: temporalColumn,
+          operator: 'TEMPORAL_RANGE',
+          comparator: state?.common?.conf?.DEFAULT_TIME_FILTER || NO_TIME_RANGE,
+          expressionType: 'SIMPLE',
+        },
+      ];
+    }
+
+    return undefined;
+  },
+};

superset-frontend/packages/superset-ui-chart-controls/src/types.ts

@@ -22,9 +22,9 @@ import type {
   AdhocColumn,
   Column,
   DatasourceType,
+  JsonObject,
   JsonValue,
   Metric,
-  QueryColumn,
   QueryFormColumn,
   QueryFormData,
   QueryFormMetric,
@@ -80,12 +80,15 @@ export interface Dataset {
   description: string | null;
   uid?: string;
   owners?: Owner[];
+  filter_select?: boolean;
+  filter_select_enabled?: boolean;
 }
 
 export interface ControlPanelState {
   form_data: QueryFormData;
   datasource: Dataset | QueryResponse | null;
   controls: ControlStateMapping;
+  common: JsonObject;
 }
 
 /**
@@ -449,9 +452,7 @@ export type ColorFormatters = {
 
 export default {};
 
-export function isColumnMeta(
-  column: AdhocColumn | ColumnMeta | QueryColumn,
-): column is ColumnMeta {
+export function isColumnMeta(column: AnyDict): column is ColumnMeta {
   return !!column && 'column_name' in column;
 }
 

superset-frontend/packages/superset-ui-chart-controls/src/utils/getTemporalColumns.ts

@@ -29,9 +29,9 @@ import {
   isQueryResponse,
 } from '@superset-ui/chart-controls';
 
-export const getTemporalColumns = (
+export function getTemporalColumns(
   datasource: ValueOf<Pick<ControlPanelState, 'datasource'>>,
-) => {
+) {
   const rv: {
     temporalColumns: ColumnMeta[] | QueryColumn[];
     defaultTemporalColumn: string | null | undefined;
@@ -61,4 +61,17 @@ export const getTemporalColumns = (
   }
 
   return rv;
-};
+}
+
+export function isTemporalColumn(
+  columnName: string,
+  datasource: ValueOf<Pick<ControlPanelState, 'datasource'>>,
+): boolean {
+  const columns = getTemporalColumns(datasource).temporalColumns;
+  for (let i = 0; i < columns.length; i += 1) {
+    if (columns[i].column_name === columnName) {
+      return true;
+    }
+  }
+  return false;
+}

superset-frontend/packages/superset-ui-chart-controls/src/utils/index.ts

@@ -24,4 +24,4 @@ export { default as mainMetric } from './mainMetric';
 export { default as columnChoices } from './columnChoices';
 export * from './defineSavedMetrics';
 export * from './getStandardizedControls';
-export { getTemporalColumns } from './getTemporalColumns';
+export * from './getTemporalColumns';

superset-frontend/packages/superset-ui-chart-controls/test/utils/getTemporalColumns.test.ts

@@ -17,8 +17,12 @@
  * under the License.
  */
 import { testQueryResponse, testQueryResults } from '@superset-ui/core';
-import { Dataset, getTemporalColumns } from '../../src';
-import { TestDataset } from '../fixtures';
+import {
+  Dataset,
+  getTemporalColumns,
+  isTemporalColumn,
+  TestDataset,
+} from '../../src';
 
 test('get temporal columns from a Dataset', () => {
   expect(getTemporalColumns(TestDataset)).toEqual({
@@ -93,3 +97,8 @@ test('should accept empty Dataset or queryResponse', () => {
     defaultTemporalColumn: undefined,
   });
 });
+
+test('should determine temporal columns in a Dataset', () => {
+  expect(isTemporalColumn('ds', TestDataset)).toBeTruthy();
+  expect(isTemporalColumn('num', TestDataset)).toBeFalsy();
+});

superset-frontend/packages/superset-ui-core/src/query/buildQueryContext.ts

@@ -20,11 +20,16 @@
 import buildQueryObject from './buildQueryObject';
 import DatasourceKey from './DatasourceKey';
 import { QueryFieldAliases, QueryFormData } from './types/QueryFormData';
-import { QueryContext, QueryObject } from './types/Query';
+import {
+  BinaryQueryObjectFilterClause,
+  QueryContext,
+  QueryObject,
+} from './types/Query';
 import { SetDataMaskHook } from '../chart';
 import { JsonObject } from '../connection';
 import { normalizeTimeColumn } from './normalizeTimeColumn';
-import { isXAxisSet } from './getXAxis';
+import { hasGenericChartAxes, isXAxisSet } from './getXAxis';
+import { ensureIsArray } from '../utils';
 
 const WRAP_IN_ARRAY = (baseQueryObject: QueryObject) => [baseQueryObject];
 
@@ -48,15 +53,26 @@ export default function buildQueryContext(
       ? { buildQuery: options, queryFields: {} }
       : options || {};
   let queries = buildQuery(buildQueryObject(formData, queryFields));
+  // --- query mutator begin ---
+  // todo(Yongjie): move the query mutator into buildQueryObject instead of buildQueryContext
   queries.forEach(query => {
     if (Array.isArray(query.post_processing)) {
       // eslint-disable-next-line no-param-reassign
       query.post_processing = query.post_processing.filter(Boolean);
     }
+    if (hasGenericChartAxes && query.time_range) {
+      // eslint-disable-next-line no-param-reassign
+      query.filters = ensureIsArray(query.filters).map(flt =>
+        flt?.op === 'TEMPORAL_RANGE'
+          ? ({ ...flt, val: query.time_range } as BinaryQueryObjectFilterClause)
+          : flt,
+      );
+    }
   });
   if (isXAxisSet(formData)) {
     queries = queries.map(query => normalizeTimeColumn(formData, query));
   }
+  // --- query mutator end ---
   return {
     datasource: new DatasourceKey(formData.datasource).toObject(),
     force: formData.force || false,

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

@@ -31,6 +31,7 @@ const BINARY_OPERATORS = [
   'ILIKE',
   'LIKE',
   'REGEX',
+  'TEMPORAL_RANGE',
 ] as const;
 
 /** List of operators that require another operand that is a set */

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

@@ -258,6 +258,7 @@ export const CtasEnum = {
 
 export type QueryColumn = {
   name: string;
+  column_name?: string;
   type: string | null;
   is_dttm: boolean;
 };

superset-frontend/packages/superset-ui-core/test/query/buildQueryContext.test.ts

@@ -18,6 +18,7 @@
  */
 import { buildQueryContext } from '@superset-ui/core';
 import * as queryModule from '../../src/query/normalizeTimeColumn';
+import * as getXAxisModule from '../../src/query/getXAxis';
 
 describe('buildQueryContext', () => {
   it('should build datasource for table sources and apply defaults', () => {
@@ -98,6 +99,7 @@ describe('buildQueryContext', () => {
       ]),
     );
   });
+  // todo(Yongjie): move these test case into buildQueryObject.test.ts
   it('should remove undefined value in post_processing', () => {
     const queryContext = buildQueryContext(
       {
@@ -124,12 +126,9 @@ describe('buildQueryContext', () => {
     ]);
   });
   it('should call normalizeTimeColumn if GENERIC_CHART_AXES is enabled and has x_axis', () => {
-    // @ts-ignore
-    const spy = jest.spyOn(window, 'window', 'get').mockImplementation(() => ({
-      featureFlags: {
-        GENERIC_CHART_AXES: true,
-      },
-    }));
+    Object.defineProperty(getXAxisModule, 'hasGenericChartAxes', {
+      value: true,
+    });
     const spyNormalizeTimeColumn = jest.spyOn(
       queryModule,
       'normalizeTimeColumn',
@@ -144,16 +143,12 @@ describe('buildQueryContext', () => {
       () => [{}],
     );
     expect(spyNormalizeTimeColumn).toBeCalled();
-    spy.mockRestore();
     spyNormalizeTimeColumn.mockRestore();
   });
   it("shouldn't call normalizeTimeColumn if GENERIC_CHART_AXES is disabled", () => {
-    // @ts-ignore
-    const spy = jest.spyOn(window, 'window', 'get').mockImplementation(() => ({
-      featureFlags: {
-        GENERIC_CHART_AXES: false,
-      },
-    }));
+    Object.defineProperty(getXAxisModule, 'hasGenericChartAxes', {
+      value: false,
+    });
     const spyNormalizeTimeColumn = jest.spyOn(
       queryModule,
       'normalizeTimeColumn',
@@ -167,7 +162,43 @@ describe('buildQueryContext', () => {
       () => [{}],
     );
     expect(spyNormalizeTimeColumn).not.toBeCalled();
-    spy.mockRestore();
     spyNormalizeTimeColumn.mockRestore();
   });
+  it('should orverride time filter if GENERIC_CHART_AXES is enabled', () => {
+    Object.defineProperty(getXAxisModule, 'hasGenericChartAxes', {
+      value: true,
+    });
+
+    const queryContext = buildQueryContext(
+      {
+        datasource: '5__table',
+        viz_type: 'table',
+      },
+      () => [
+        {
+          filters: [
+            {
+              col: 'col1',
+              op: 'TEMPORAL_RANGE',
+              val: '2001 : 2002',
+            },
+            {
+              col: 'col2',
+              op: 'IN',
+              val: ['a', 'b'],
+            },
+          ],
+          time_range: '1990 : 1991',
+        },
+      ],
+    );
+    expect(queryContext.queries[0].filters).toEqual([
+      { col: 'col1', op: 'TEMPORAL_RANGE', val: '1990 : 1991' },
+      {
+        col: 'col2',
+        op: 'IN',
+        val: ['a', 'b'],
+      },
+    ]);
+  });
 });