You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2026/03/20 19:43:27 UTC
(superset) 26/32: fix(timeseries-table): enable proper column sorting in timeseries-table chart (#38579)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 6.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 884f8f7a35ecd7a96d335436ec13d85fad090d86
Author: Levis Mbote <11...@users.noreply.github.com>
AuthorDate: Thu Mar 19 22:01:40 2026 +0300
fix(timeseries-table): enable proper column sorting in timeseries-table chart (#38579)
---
.../TimeTable/utils/sortUtils/sortUtils.test.ts | 78 ++++++++++++++--------
.../TimeTable/utils/sortUtils/sortUtils.ts | 55 +++++++++++++--
2 files changed, 99 insertions(+), 34 deletions(-)
diff --git a/superset-frontend/src/visualizations/TimeTable/utils/sortUtils/sortUtils.test.ts b/superset-frontend/src/visualizations/TimeTable/utils/sortUtils/sortUtils.test.ts
index 879a5fc861c..90e2f287458 100644
--- a/superset-frontend/src/visualizations/TimeTable/utils/sortUtils/sortUtils.test.ts
+++ b/superset-frontend/src/visualizations/TimeTable/utils/sortUtils/sortUtils.test.ts
@@ -17,29 +17,25 @@
* under the License.
*/
import { sortNumberWithMixedTypes } from './sortUtils';
-
-jest.mock('src/utils/sortNumericValues', () => ({
- sortNumericValues: jest.fn((a, b, options) => {
- const numA = Number(a);
- const numB = Number(b);
-
- if (Number.isNaN(numA) && Number.isNaN(numB)) return 0;
- if (Number.isNaN(numA))
- return options.nanTreatment === 'asSmallest' ? -1 : 1;
- if (Number.isNaN(numB))
- return options.nanTreatment === 'asSmallest' ? 1 : -1;
-
- return numA - numB;
- }),
-}));
+import type { ColumnConfig } from '../../types';
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
describe('sortNumberWithMixedTypes', () => {
- const createMockRow = (value: any) => ({
+ const createMockRow = (
+ value: any,
+ columnOverrides: Partial<ColumnConfig> = {},
+ ) => ({
values: {
testColumn: {
props: {
- 'data-value': value,
+ valueField: 'metric',
+ column: {
+ key: 'testColumn',
+ colType: 'time',
+ bounds: undefined,
+ ...columnOverrides,
+ },
+ reversedEntries: [{ metric: value }],
},
},
},
@@ -49,7 +45,7 @@ describe('sortNumberWithMixedTypes', () => {
const rowA = createMockRow(10);
const rowB = createMockRow(20);
- const result = sortNumberWithMixedTypes(rowA, rowB, 'testColumn', false);
+ const result = sortNumberWithMixedTypes(rowA, rowB, 'testColumn');
expect(result).toBeLessThan(0); // rowA should come before rowB
});
@@ -58,16 +54,16 @@ describe('sortNumberWithMixedTypes', () => {
const rowA = createMockRow(10);
const rowB = createMockRow(20);
- const result = sortNumberWithMixedTypes(rowA, rowB, 'testColumn', true);
+ const result = sortNumberWithMixedTypes(rowA, rowB, 'testColumn');
- expect(result).toBeGreaterThan(0); // rowB should come before rowA in descending
+ expect(result).toBeLessThan(0);
});
test('should handle equal values', () => {
const rowA = createMockRow(15);
const rowB = createMockRow(15);
- const result = sortNumberWithMixedTypes(rowA, rowB, 'testColumn', false);
+ const result = sortNumberWithMixedTypes(rowA, rowB, 'testColumn');
expect(result).toBe(0);
});
@@ -76,16 +72,17 @@ describe('sortNumberWithMixedTypes', () => {
const rowA = createMockRow(null);
const rowB = createMockRow(10);
- const result = sortNumberWithMixedTypes(rowA, rowB, 'testColumn', false);
+ const result = sortNumberWithMixedTypes(rowA, rowB, 'testColumn');
expect(typeof result).toBe('number');
});
test('should handle string numbers', () => {
- const rowA = createMockRow('10');
- const rowB = createMockRow('20');
+ const rowA = createMockRow('10', { colType: undefined });
+ const rowB = createMockRow('20', { colType: undefined });
- const result = sortNumberWithMixedTypes(rowA, rowB, 'testColumn', false);
+ const result = sortNumberWithMixedTypes(rowA, rowB, 'testColumn');
+ expect(typeof result).toBe('number');
expect(result).toBeLessThan(0);
});
@@ -93,7 +90,7 @@ describe('sortNumberWithMixedTypes', () => {
const rowA = createMockRow(10);
const rowB = createMockRow('20');
- const result = sortNumberWithMixedTypes(rowA, rowB, 'testColumn', false);
+ const result = sortNumberWithMixedTypes(rowA, rowB, 'testColumn');
expect(typeof result).toBe('number');
});
@@ -102,7 +99,7 @@ describe('sortNumberWithMixedTypes', () => {
const rowA = createMockRow(-10);
const rowB = createMockRow(5);
- const result = sortNumberWithMixedTypes(rowA, rowB, 'testColumn', false);
+ const result = sortNumberWithMixedTypes(rowA, rowB, 'testColumn');
expect(result).toBeLessThan(0);
});
@@ -111,7 +108,32 @@ describe('sortNumberWithMixedTypes', () => {
const rowA = createMockRow(0);
const rowB = createMockRow(10);
- const result = sortNumberWithMixedTypes(rowA, rowB, 'testColumn', false);
+ const result = sortNumberWithMixedTypes(rowA, rowB, 'testColumn');
+
+ expect(result).toBeLessThan(0);
+ });
+
+ test('should sort ValueCell-like props numerically', () => {
+ const createValueCellRow = (metricValue: number | null) => ({
+ values: {
+ testColumn: {
+ props: {
+ valueField: 'metric',
+ column: {
+ key: 'testColumn',
+ colType: 'time',
+ bounds: undefined,
+ },
+ reversedEntries: [{ metric: metricValue }],
+ },
+ },
+ },
+ });
+
+ const smaller = createValueCellRow(1);
+ const larger = createValueCellRow(5);
+
+ const result = sortNumberWithMixedTypes(smaller, larger, 'testColumn');
expect(result).toBeLessThan(0);
});
diff --git a/superset-frontend/src/visualizations/TimeTable/utils/sortUtils/sortUtils.ts b/superset-frontend/src/visualizations/TimeTable/utils/sortUtils/sortUtils.ts
index a5d055aa76e..c18ce7ca948 100644
--- a/superset-frontend/src/visualizations/TimeTable/utils/sortUtils/sortUtils.ts
+++ b/superset-frontend/src/visualizations/TimeTable/utils/sortUtils/sortUtils.ts
@@ -16,6 +16,8 @@
* specific language governing permissions and limitations
* under the License.
*/
+import type { ColumnConfig, Entry } from '../../types';
+import { calculateCellValue } from '../valueCalculations/valueCalculations';
/**
* Simple numeric value comparison that handles null, undefined, and mixed types
* @param a - First value to compare
@@ -42,23 +44,64 @@ function compareValues(
}
/**
- * Sorts table rows with mixed data types for react-table
+ * Sorts table rows with mixed data types for react-table.
+ *
* @param rowA - First row to compare
* @param rowB - Second row to compare
* @param columnId - Column identifier for sorting
* @param descending - Whether to sort in descending order
* @returns Numeric comparison result for react-table
+ * react-table handles the asc/desc direction flip internally after calling
+ * this function, so we only return the raw comparison result.
*/
export function sortNumberWithMixedTypes(
rowA: any,
rowB: any,
columnId: string,
- descending: boolean,
) {
- const valueA = rowA.values[columnId].props['data-value'];
- const valueB = rowB.values[columnId].props['data-value'];
+ const cellA = rowA.values?.[columnId];
+ const cellB = rowB.values?.[columnId];
+
+ // Both ValueCell and Sparkline cells pass React elements here.
+ // ValueCell uses { valueField, column, reversedEntries }
+ // Sparkline/SparklineCell uses { valueField, column, entries }
+ // Normalize to reversedEntries before delegating to calculateCellValue.
+ const propsA = cellA?.props as
+ | {
+ valueField: string;
+ column: ColumnConfig;
+ reversedEntries?: Entry[];
+ entries?: Entry[];
+ }
+ | undefined;
+ const propsB = cellB?.props as
+ | {
+ valueField: string;
+ column: ColumnConfig;
+ reversedEntries?: Entry[];
+ entries?: Entry[];
+ }
+ | undefined;
+
+ const reversedEntriesA =
+ propsA?.reversedEntries ?? propsA?.entries?.slice().reverse();
+ const reversedEntriesB =
+ propsB?.reversedEntries ?? propsB?.entries?.slice().reverse();
+
+ if (!propsA || !propsB || !reversedEntriesA || !reversedEntriesB) {
+ return 0;
+ }
- const comparison = compareValues(valueA, valueB, 'asSmallest');
+ const { value: valueA } = calculateCellValue(
+ propsA.valueField,
+ propsA.column,
+ reversedEntriesA,
+ );
+ const { value: valueB } = calculateCellValue(
+ propsB.valueField,
+ propsB.column,
+ reversedEntriesB,
+ );
- return comparison * (descending ? -1 : 1);
+ return compareValues(valueA, valueB, 'asSmallest');
}