From bcc23bbacd67a5a461232b0b9e345fe74a81e4f9 Mon Sep 17 00:00:00 2001 From: stevetracvc <70416691+stevetracvc@users.noreply.github.com> Date: Thu, 23 Jun 2022 05:39:46 -0600 Subject: [PATCH] fix: issue with sorting by multiple columns in a table (#19920) Recent commit to sort alphanumeric columns via case insensitive comparison broke the multi-column sort option. React-table only sorts by the second (or third...) column if the first column matches. Since the alphanumeric sort only returned -1 or 1, it never would move to the subsequent columns when the earlier column values matched. (cherry picked from commit a45d011e74be7a52fee9b0e580187dd6f25509db) --- .../utils/sortAlphanumericCaseInsensitive.ts | 2 +- .../sortAlphanumericCaseInsensitive.test.ts | 108 ++++++++++++++++++ 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/superset-frontend/plugins/plugin-chart-table/src/DataTable/utils/sortAlphanumericCaseInsensitive.ts b/superset-frontend/plugins/plugin-chart-table/src/DataTable/utils/sortAlphanumericCaseInsensitive.ts index 789e0cef7f7..c1adc99949a 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/DataTable/utils/sortAlphanumericCaseInsensitive.ts +++ b/superset-frontend/plugins/plugin-chart-table/src/DataTable/utils/sortAlphanumericCaseInsensitive.ts @@ -33,5 +33,5 @@ export const sortAlphanumericCaseInsensitive = ( if (!valueB || typeof valueB !== 'string') { return 1; } - return valueA.localeCompare(valueB) > 0 ? 1 : -1; + return valueA.localeCompare(valueB); }; diff --git a/superset-frontend/plugins/plugin-chart-table/test/sortAlphanumericCaseInsensitive.test.ts b/superset-frontend/plugins/plugin-chart-table/test/sortAlphanumericCaseInsensitive.test.ts index 4ba35ba602d..356596ec210 100644 --- a/superset-frontend/plugins/plugin-chart-table/test/sortAlphanumericCaseInsensitive.test.ts +++ b/superset-frontend/plugins/plugin-chart-table/test/sortAlphanumericCaseInsensitive.test.ts @@ -17,8 +17,13 @@ * under the License. */ +import { defaultOrderByFn, Row } from 'react-table'; import { sortAlphanumericCaseInsensitive } from '../src/DataTable/utils/sortAlphanumericCaseInsensitive'; +type RecursivePartial = { + [P in keyof T]?: T[P] | RecursivePartial; +}; + const testData = [ { values: { @@ -133,3 +138,106 @@ describe('sortAlphanumericCaseInsensitive', () => { ]); }); }); + +const testDataMulti: Array>> = [ + { + values: { + colA: 'group 1', + colB: '10', + }, + }, + { + values: { + colA: 'group 1', + colB: '15', + }, + }, + { + values: { + colA: 'group 1', + colB: '20', + }, + }, + { + values: { + colA: 'group 2', + colB: '10', + }, + }, + { + values: { + colA: 'group 3', + colB: '10', + }, + }, + { + values: { + colA: 'group 3', + colB: '15', + }, + }, + { + values: { + colA: 'group 3', + colB: '10', + }, + }, +]; + +describe('sortAlphanumericCaseInsensitiveMulti', () => { + it('Sort rows', () => { + const sorted = defaultOrderByFn( + [...testDataMulti] as Array>, + [ + (a, b) => sortAlphanumericCaseInsensitive(a, b, 'colA'), + (a, b) => sortAlphanumericCaseInsensitive(a, b, 'colB'), + ], + [true, false], + ); + + expect(sorted).toEqual([ + { + values: { + colA: 'group 1', + colB: '20', + }, + }, + { + values: { + colA: 'group 1', + colB: '15', + }, + }, + { + values: { + colA: 'group 1', + colB: '10', + }, + }, + { + values: { + colA: 'group 2', + colB: '10', + }, + }, + { + values: { + colA: 'group 3', + colB: '15', + }, + }, + { + values: { + colA: 'group 3', + colB: '10', + }, + }, + { + values: { + colA: 'group 3', + colB: '10', + }, + }, + ]); + }); +});