From eafe0cfc6f040670a9b35ebcd27f5c83eabe068e Mon Sep 17 00:00:00 2001
From: Geido <60598000+geido@users.noreply.github.com>
Date: Mon, 28 Feb 2022 18:56:13 +0200
Subject: [PATCH] fix(Explore): Pivot table V2 sort by failure with D&D enabled
 (#18835)

* wip

* Add tests and clean up

* Clean up

* Remove unused import
---
 .../src/query/buildQueryObject.ts             | 12 +++++++++-
 .../src/query/types/Column.ts                 |  6 ++++-
 .../test/query/buildQueryObject.test.ts       | 22 +++++++++++++++++++
 3 files changed, 38 insertions(+), 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
index c49bc5ce6aa5..52fa1ffed0c2 100644
--- 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
index 8fcf871647e3..78e4a301c960 100644
--- 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
index 1d3e19854c24..b2ee6f579d5c 100644
--- 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',