fix(table): Use extras in query, only remove time_grain_sqla when not…

… needed
apache · Sep 19, 2024 · ecc2376 · ecc2376
1 parent f315a4f
commit ecc2376
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 2 deletions.
diff --git a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts
@@ -198,10 +198,15 @@ const buildQuery: BuildQuery<TableChartFormData> = (
         (ownState.currentPage ?? 0) * (ownState.pageSize ?? 0);
     }
 
+    if (!temporalColum) {
+      // This query is using only textual columns, so it doesn't need time grain
+      extras.time_grain_sqla = undefined;
+    }
+
     let queryObject = {
       ...baseQueryObject,
       columns,
-      extras: !isEmpty(timeOffsets) && !temporalColum ? {} : extras,
+      extras,
       orderby,
       metrics,
       post_processing: postProcessing,
@@ -239,7 +244,6 @@ const buildQuery: BuildQuery<TableChartFormData> = (
         row_limit: 0,
         row_offset: 0,
         post_processing: [],
-        extras: undefined, // we don't need time grain here
         order_desc: undefined, // we don't need orderby stuff here,
         orderby: undefined, // because this query will be used for get total aggregation.
       });

diff --git a/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts b/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts
@@ -114,5 +114,66 @@ describe('plugin-chart-table', () => {
         expressionType: 'SQL',
       });
     });
+    it('should include time_grain_sqla in extras if temporal colum is used and keep the rest', () => {
+      const { queries } = buildQuery({
+        ...basicFormData,
+        time_grain_sqla: TimeGranularity.MONTH,
+        groupby: ['col1'],
+        query_mode: QueryMode.Aggregate,
+        temporal_columns_lookup: { col1: true },
+        show_totals: true,
+        metrics: ['aaa', 'aaa'],
+        adhoc_filters: [
+          {
+            expressionType: 'SQL',
+            sqlExpression: "status IN ('In Process')",
+            clause: 'WHERE',
+            subject: null,
+            operator: null,
+            comparator: null,
+            isExtra: false,
+            isNew: false,
+            datasourceWarning: false,
+            filterOptionName: 'filter_v8m9t9oq5re_ndzk6g5am7',
+          } as any,
+        ],
+      });
+      // Extras in regular query
+      expect(queries[0].extras?.time_grain_sqla).toEqual(TimeGranularity.MONTH);
+      expect(queries[0].extras?.where).toEqual("(status IN ('In Process'))");
+      // Extras in summary query
+      expect(queries[1].extras?.time_grain_sqla).toEqual(TimeGranularity.MONTH);
+      expect(queries[1].extras?.where).toEqual("(status IN ('In Process'))");
+    });
+    it('should not include time_grain_sqla in extras if temporal colum is not used and keep the rest', () => {
+      const { queries } = buildQuery({
+        ...basicFormData,
+        time_grain_sqla: TimeGranularity.MONTH,
+        groupby: ['col1'],
+        query_mode: QueryMode.Aggregate,
+        show_totals: true,
+        metrics: ['aaa', 'aaa'],
+        adhoc_filters: [
+          {
+            expressionType: 'SQL',
+            sqlExpression: "status IN ('In Process')",
+            clause: 'WHERE',
+            subject: null,
+            operator: null,
+            comparator: null,
+            isExtra: false,
+            isNew: false,
+            datasourceWarning: false,
+            filterOptionName: 'filter_v8m9t9oq5re_ndzk6g5am7',
+          } as any,
+        ],
+      });
+      // Extras in regular query
+      expect(queries[0].extras?.time_grain_sqla).toBeUndefined();
+      expect(queries[0].extras?.where).toEqual("(status IN ('In Process'))");
+      // Extras in summary query
+      expect(queries[1].extras?.time_grain_sqla).toBeUndefined();
+      expect(queries[1].extras?.where).toEqual("(status IN ('In Process'))");
+    });
   });
 });