(superset) branch master updated: fix(table): Use extras in queries (#30335)

[arivero]

This is an automated email from the ASF dual-hosted git repository.

arivero pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 6c2bd2a968 fix(table): Use extras in queries (#30335)
6c2bd2a968 is described below

commit 6c2bd2a9689e1d96cb73c0145dd39d7297e6d230
Author: Antonio Rivero <38889534+antonio-riveromart...@users.noreply.github.com>
AuthorDate: Thu Sep 19 16:45:39 2024 +0200

    fix(table): Use extras in queries (#30335)
---
 .../plugins/plugin-chart-table/src/buildQuery.ts   | 22 ++++++-----
 .../plugin-chart-table/test/buildQuery.test.ts     | 43 ++++++++++++++++++++++
 2 files changed, 56 insertions(+), 9 deletions(-)

diff --git a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts 
b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts
index d63cf9ec38..7068ab1193 100644
--- a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts
+++ b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts
@@ -114,8 +114,8 @@ const buildQuery: BuildQuery<TableChartFormData> = (
       }
     }
 
-    let temporalColumAdded = false;
-    let temporalColum = null;
+    let temporalColumnAdded = false;
+    let temporalColumn = null;
 
     if (queryMode === QueryMode.Aggregate) {
       metrics = metrics || [];
@@ -169,23 +169,23 @@ const buildQuery: BuildQuery<TableChartFormData> = (
           time_grain_sqla &&
           temporalColumnsLookup?.[col];
 
-        if (shouldBeAdded && !temporalColumAdded) {
-          temporalColum = {
+        if (shouldBeAdded && !temporalColumnAdded) {
+          temporalColumn = {
             timeGrain: time_grain_sqla,
             columnType: 'BASE_AXIS',
             sqlExpression: col,
             label: col,
             expressionType: 'SQL',
           } as AdhocColumn;
-          temporalColumAdded = true;
+          temporalColumnAdded = true;
           return false; // Do not include this in the output; it's added 
separately
         }
         return true;
       });
 
       // So we ensure the temporal column is added first
-      if (temporalColum) {
-        columns = [temporalColum, ...columns];
+      if (temporalColumn) {
+        columns = [temporalColumn, ...columns];
       }
     }
 
@@ -198,10 +198,15 @@ const buildQuery: BuildQuery<TableChartFormData> = (
         (ownState.currentPage ?? 0) * (ownState.pageSize ?? 0);
     }
 
+    if (!temporalColumn) {
+      // This query is not using temporal column, 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
index a86f7d181b..f3eb2d0955 100644
--- a/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts
+++ b/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts
@@ -25,6 +25,28 @@ const basicFormData: TableChartFormData = {
   datasource: '11__table',
 };
 
+const extraQueryFormData: TableChartFormData = {
+  ...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,
+  ],
+};
 describe('plugin-chart-table', () => {
   describe('buildQuery', () => {
     it('should add post-processing and ignore duplicate metrics', () => {
@@ -114,5 +136,26 @@ 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({
+        ...extraQueryFormData,
+        temporal_columns_lookup: { col1: true },
+      });
+      // 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(extraQueryFormData);
+      // 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'))");
+    });
   });
 });