From 8ab578e121b3057d9086fbae3c7c3eafb916dc32 Mon Sep 17 00:00:00 2001
From: Jesse Yang <jesse.yang@airbnb.com>
Date: Mon, 27 Apr 2020 12:47:32 -0700
Subject: [PATCH] fix(superset-ui-query): add unit tests for #387 (#390)

---
 .../superset-ui-query/src/buildQueryObject.ts |  2 +-
 .../src/types/QueryFormData.ts                |  2 +-
 .../test/buildQueryContext.test.ts            |  2 +-
 .../test/buildQueryObject.test.ts             | 30 ++++++++++++++++++-
 4 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/buildQueryObject.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/buildQueryObject.ts
index ff7a4fe3ad1a..9585ffc1e1fb 100644
--- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/buildQueryObject.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/buildQueryObject.ts
@@ -43,7 +43,7 @@ export default function buildQueryObject<T extends QueryFormData>(formData: T):
     metrics: processMetrics(formData),
     order_desc: typeof order_desc === 'undefined' ? true : order_desc,
     orderby: [],
-    row_limit: row_limit == null || isNaN(numericRowLimit) ? undefined : numericRowLimit,
+    row_limit: row_limit == null || Number.isNaN(numericRowLimit) ? undefined : numericRowLimit,
     since,
     time_range,
     timeseries_limit: limit ? Number(limit) : 0,
diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/types/QueryFormData.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/types/QueryFormData.ts
index fbd17594de1f..512e70bdfd97 100644
--- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/types/QueryFormData.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/types/QueryFormData.ts
@@ -42,7 +42,7 @@ export type BaseFormData = {
   /** limit number of time series */
   limit?: number;
   /** limit number of row in the results */
-  row_limit?: number;
+  row_limit?: string | number | null;
   /** The metric used to order timeseries for limiting */
   timeseries_limit_metric?: QueryFormDataMetric;
 } & TimeRange &
diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/buildQueryContext.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/buildQueryContext.test.ts
index b51f0a182382..3e1ad6107ac0 100644
--- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/buildQueryContext.test.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/buildQueryContext.test.ts
@@ -1,6 +1,6 @@
 import { buildQueryContext } from '../src';
 
-describe('queryContextBuilder', () => {
+describe('buildQueryContext', () => {
   it('should build datasource for table sources', () => {
     const queryContext = buildQueryContext({
       datasource: '5__table',
diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/buildQueryObject.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/buildQueryObject.test.ts
index 76be285b02e2..e901b4040398 100644
--- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/buildQueryObject.test.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/buildQueryObject.test.ts
@@ -1,6 +1,6 @@
 import { buildQueryObject, QueryObject } from '../src';
 
-describe('queryObjectBuilder', () => {
+describe('buildQueryObject', () => {
   let query: QueryObject;
 
   it('should build granularity for sql alchemy datasources', () => {
@@ -63,4 +63,32 @@ describe('queryObjectBuilder', () => {
     });
     expect(query.timeseries_limit_metric).toEqual({ label: metric });
   });
+
+  it('should handle null and non-numeric row_limit', () => {
+    const baseQuery = {
+      datasource: '5__table',
+      granularity_sqla: 'ds',
+      viz_type: 'table',
+      row_limit: null,
+    };
+
+    // undefined
+    query = buildQueryObject({ ...baseQuery });
+    expect(query.row_limit).toBeUndefined();
+
+    // null value
+    query = buildQueryObject({ ...baseQuery, row_limit: null });
+    expect(query.row_limit).toBeUndefined();
+
+    query = buildQueryObject({ ...baseQuery, row_limit: 1000 });
+    expect(query.row_limit).toStrictEqual(1000);
+
+    // valid string
+    query = buildQueryObject({ ...baseQuery, row_limit: '200' });
+    expect(query.row_limit).toStrictEqual(200);
+
+    // invalid string
+    query = buildQueryObject({ ...baseQuery, row_limit: 'two hundred' });
+    expect(query.row_limit).toBeUndefined();
+  });
 });