clean up extract types and fix tests

packages/superset-ui-core/src/query/buildQueryObject.ts

@@ -42,9 +42,9 @@ export default function buildQueryObject<T extends QueryFormData>(
   const numericRowOffset = Number(row_offset);
   const { metrics, columns, orderby } = extractQueryFields(residualFormData, queryFields);
 
-  const extras = extractExtras(formData);
   // collect all filters for conversion to simple filters/freeform clauses
-  const { filters: extraFilters = [] } = extras;
+  const extras = extractExtras(formData);
+  const { filters: extraFilters } = extras;
   const { adhoc_filters: appendAdhocFilters = [], filters: appendFilters = [] } = append_form_data;
   const filterFormData: {
     filters: QueryObjectFilterClause[];

packages/superset-ui-core/src/query/extractExtras.ts

@@ -3,20 +3,36 @@ import {
   AppliedTimeExtras,
   isDruidFormData,
   QueryFormData,
-  QueryObject,
+  QueryObjectExtras,
+  QueryObjectFilterClause,
   TimeColumnConfigKey,
 } from './types';
 
-export default function extractExtras(formData: QueryFormData): Partial<QueryObject> {
+type ExtraFilterQueryFields = {
+  time_range?: string;
+  granularity_sqla?: string;
+  time_grain_sqla?: string;
+  druid_time_origin?: string;
+  granularity?: string;
+};
+
+type ExtractedExtras = ExtraFilters & {
+  filters: QueryObjectFilterClause[];
+  extras: QueryObjectExtras;
+  applied_time_extras: AppliedTimeExtras;
+};
+
+export default function extractExtras(formData: QueryFormData): ExtractedExtras {
   const applied_time_extras: AppliedTimeExtras = {};
-  const { extras = {}, filters = [] } = formData;
-  const partialQueryObject: Partial<QueryObject> = {
+  const filters: QueryObjectFilterClause[] = [];
+  const extras: QueryObjectExtras = {};
+  const extract: ExtractedExtras = {
     filters,
     extras,
     applied_time_extras,
   };
 
-  const reservedColumnsToQueryField: Record<TimeColumnConfigKey, keyof QueryObject> = {
+  const reservedColumnsToQueryField: Record<TimeColumnConfigKey, keyof ExtraFilterQueryFields> = {
     __time_range: 'time_range',
     __time_col: 'granularity_sqla',
     __time_grain: 'time_grain_sqla',
@@ -28,28 +44,28 @@ export default function extractExtras(formData: QueryFormData): Partial<QueryObj
     if (filter.col in reservedColumnsToQueryField) {
       const key = filter.col as TimeColumnConfigKey;
       const queryField = reservedColumnsToQueryField[key];
-      partialQueryObject[queryField] = filter.val;
+      extract[queryField] = filter.val as string;
       applied_time_extras[key] = filter.val as string;
     } else {
       filters.push(filter);
     }
   });
 
   // map to undeprecated names and remove deprecated fields
-  if (isDruidFormData(formData) && !partialQueryObject.druid_time_origin) {
+  if (isDruidFormData(formData) && !extract.druid_time_origin) {
     extras.druid_time_origin = formData.druid_time_origin;
-    delete partialQueryObject.druid_time_origin;
+    delete extract.druid_time_origin;
   } else {
     // SQL
-    extras.time_grain_sqla = partialQueryObject.time_grain_sqla || formData.time_grain_sqla;
-    partialQueryObject.granularity =
-      partialQueryObject.granularity_sqla || formData.granularity || formData.granularity_sqla;
-    delete partialQueryObject.granularity_sqla;
-    delete partialQueryObject.time_grain_sqla;
+    extras.time_grain_sqla = extract.time_grain_sqla || formData.time_grain_sqla;
+    extract.granularity =
+      extract.granularity_sqla || formData.granularity || formData.granularity_sqla;
+    delete extract.granularity_sqla;
+    delete extract.time_grain_sqla;
   }
 
   // map time range endpoints:
   if (formData.time_range_endpoints) extras.time_range_endpoints = formData.time_range_endpoints;
 
-  return partialQueryObject;
+  return extract;
 }

packages/superset-ui-core/test/query/buildQueryObject.test.ts

@@ -63,7 +63,7 @@ describe('buildQueryObject', () => {
       datasource: '5__table',
       granularity_sqla: 'ds',
       viz_type: 'table',
-      filters: [{ col: 'abc', op: '==', val: 'qwerty' }],
+      extra_filters: [{ col: 'abc', op: '==', val: 'qwerty' }],
       adhoc_filters: [
         {
           expressionType: 'SIMPLE',

packages/superset-ui-core/test/query/extractExtras.test.ts

@@ -24,13 +24,6 @@ describe('extractExtras', () => {
     granularity_sqla: 'ds',
     time_grain_sqla: 'PT1M',
     viz_type: 'my_viz',
-    filters: [
-      {
-        col: 'gender',
-        op: '==',
-        val: 'girl',
-      },
-    ],
   };
 
   it('should populate time range endpoints and override formData with double underscored date options', () => {
@@ -66,13 +59,7 @@ describe('extractExtras', () => {
         time_grain_sqla: 'PT5M',
         time_range_endpoints: ['inclusive', 'exclusive'],
       },
-      filters: [
-        {
-          col: 'gender',
-          op: '==',
-          val: 'girl',
-        },
-      ],
+      filters: [],
       granularity: 'ds2',
       time_range: '2009-07-17T00:00:00 : 2020-07-17T00:00:00',
     });
@@ -83,6 +70,11 @@ describe('extractExtras', () => {
       extractExtras({
         ...baseQueryFormData,
         extra_filters: [
+          {
+            col: 'gender',
+            op: '==',
+            val: 'girl',
+          },
           {
             col: 'name',
             op: 'IN',
@@ -110,4 +102,54 @@ describe('extractExtras', () => {
       granularity: 'ds',
     });
   });
+
+  it('should create regular filters from reserved and non-reserved columns', () => {
+    expect(
+      extractExtras({
+        ...baseQueryFormData,
+        time_range_endpoints: ['inclusive', 'exclusive'],
+        extra_filters: [
+          {
+            col: 'gender',
+            op: '==',
+            val: 'girl',
+          },
+          {
+            col: '__time_col',
+            op: '==',
+            val: 'ds2',
+          },
+          {
+            col: '__time_grain',
+            op: '==',
+            val: 'PT5M',
+          },
+          {
+            col: '__time_range',
+            op: '==',
+            val: '2009-07-17T00:00:00 : 2020-07-17T00:00:00',
+          },
+        ],
+      }),
+    ).toEqual({
+      applied_time_extras: {
+        __time_col: 'ds2',
+        __time_grain: 'PT5M',
+        __time_range: '2009-07-17T00:00:00 : 2020-07-17T00:00:00',
+      },
+      extras: {
+        time_grain_sqla: 'PT5M',
+        time_range_endpoints: ['inclusive', 'exclusive'],
+      },
+      filters: [
+        {
+          col: 'gender',
+          op: '==',
+          val: 'girl',
+        },
+      ],
+      granularity: 'ds2',
+      time_range: '2009-07-17T00:00:00 : 2020-07-17T00:00:00',
+    });
+  });
 });