fix(Explore): Pivot table V2 sort by failure with D&D enabled (#18835)

* wip * Add tests and clean up * Clean up * Remove unused import
apache · Feb 28, 2022 · eafe0cf · eafe0cf
1 parent c56dc8e
commit eafe0cf
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 2 deletions.
diff --git a/superset-frontend/packages/superset-ui-core/src/query/buildQueryObject.ts b/superset-frontend/packages/superset-ui-core/src/query/buildQueryObject.ts
@@ -21,9 +21,12 @@
 import {
   AdhocFilter,
   QueryFieldAliases,
+  QueryFormColumn,
   QueryFormData,
   QueryObject,
   QueryObjectFilterClause,
+  isPhysicalColumn,
+  isAdhocColumn,
 } from './types';
 import processFilters from './processFilters';
 import extractExtras from './extractExtras';
@@ -89,6 +92,12 @@ export default function buildQueryObject<T extends QueryFormData>(
     ...extras,
     ...filterFormData,
   });
+  const normalizeSeriesLimitMetric = (column: QueryFormColumn | undefined) => {
+    if (isAdhocColumn(column) || isPhysicalColumn(column)) {
+      return column;
+    }
+    return undefined;
+  };
 
   let queryObject: QueryObject = {
     // fallback `null` to `undefined` so they won't be sent to the backend
@@ -113,13 +122,14 @@ export default function buildQueryObject<T extends QueryFormData>(
         : numericRowOffset,
     series_columns,
     series_limit,
-    series_limit_metric,
+    series_limit_metric: normalizeSeriesLimitMetric(series_limit_metric),
     timeseries_limit: limit ? Number(limit) : 0,
     timeseries_limit_metric: timeseries_limit_metric || undefined,
     order_desc: typeof order_desc === 'undefined' ? true : order_desc,
     url_params: url_params || undefined,
     custom_params,
   };
+
   // override extra form data used by native and cross filters
   queryObject = overrideExtraFormData(queryObject, overrides);
 

diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/Column.ts b/superset-frontend/packages/superset-ui-core/src/query/types/Column.ts
@@ -54,7 +54,11 @@ export interface Column {
 export default {};
 
 export function isPhysicalColumn(
-  column: AdhocColumn | PhysicalColumn,
+  column?: AdhocColumn | PhysicalColumn,
 ): column is PhysicalColumn {
   return typeof column === 'string';
 }
+
+export function isAdhocColumn(column?: AdhocColumn | PhysicalColumn) {
+  return (column as AdhocColumn)?.sqlExpression !== undefined;
+}
diff --git a/superset-frontend/packages/superset-ui-core/test/query/buildQueryObject.test.ts b/superset-frontend/packages/superset-ui-core/test/query/buildQueryObject.test.ts
@@ -152,6 +152,28 @@ describe('buildQueryObject', () => {
     expect(query.timeseries_limit_metric).toEqual(metric);
   });
 
+  it('should build series_limit_metric', () => {
+    const metric = 'country';
+    query = buildQueryObject({
+      datasource: '5__table',
+      granularity_sqla: 'ds',
+      viz_type: 'pivot_table_v2',
+      series_limit_metric: metric,
+    });
+    expect(query.series_limit_metric).toEqual(metric);
+  });
+
+  it('should build series_limit_metric as undefined when empty array', () => {
+    const metric: any = [];
+    query = buildQueryObject({
+      datasource: '5__table',
+      granularity_sqla: 'ds',
+      viz_type: 'pivot_table_v2',
+      series_limit_metric: metric,
+    });
+    expect(query.series_limit_metric).toEqual(undefined);
+  });
+
   it('should handle null and non-numeric row_limit and row_offset', () => {
     const baseQuery = {
       datasource: '5__table',