[Lens] Datatable improvements (elastic#174994)

## Summary Fixes elastic#160719 and elastic#164413 This PR contains some work about Lens datatable, here's a short list: * moved out the sorting logic of the datatable into an independent package: `@kbn/sort-predicates` * leverage the EUI Datagrid `schemaDetectors` for the table rows sorting * apply datatable columns sorting also to the CSV exporter ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Stratoula Kalafateli <[email protected]> Co-authored-by: kibanamachine <[email protected]>
gbamparop · Jan 23, 2024 · 73e5a96 · 73e5a96
1 parent 450c384
commit 73e5a96
Show file tree

Hide file tree

Showing 27 changed files with 509 additions and 98 deletions.
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
@@ -770,6 +770,7 @@ x-pack/packages/kbn-slo-schema @elastic/obs-ux-management-team
 x-pack/plugins/snapshot_restore @elastic/platform-deployment-management
 packages/kbn-some-dev-log @elastic/kibana-operations
 packages/kbn-sort-package-json @elastic/kibana-operations
+packages/kbn-sort-predicates @elastic/kibana-visualizations
 x-pack/plugins/spaces @elastic/kibana-security
 x-pack/test/spaces_api_integration/common/plugins/spaces_test_plugin @elastic/kibana-security
 packages/kbn-spec-to-console @elastic/platform-deployment-management

diff --git a/package.json b/package.json
@@ -766,6 +766,7 @@
     "@kbn/shared-ux-utility": "link:packages/kbn-shared-ux-utility",
     "@kbn/slo-schema": "link:x-pack/packages/kbn-slo-schema",
     "@kbn/snapshot-restore-plugin": "link:x-pack/plugins/snapshot_restore",
+    "@kbn/sort-predicates": "link:packages/kbn-sort-predicates",
     "@kbn/spaces-plugin": "link:x-pack/plugins/spaces",
     "@kbn/spaces-test-plugin": "link:x-pack/test/spaces_api_integration/common/plugins/spaces_test_plugin",
     "@kbn/stack-alerts-plugin": "link:x-pack/plugins/stack_alerts",

diff --git a/packages/kbn-sort-predicates/README.md b/packages/kbn-sort-predicates/README.md
@@ -0,0 +1,74 @@
+# @kbn/sort-predicates
+
+This package contains a flexible sorting function who supports the following types:
+
+* string
+* number
+* version
+* ip addresses (both IPv4 and IPv6) - handles `Others`/strings correcly in this case
+* dates
+* ranges open and closed (number type only for now)
+* null and undefined (always sorted as last entries, no matter the direction)
+* any multi-value version of the types above (version excluded)
+
+The function is intended to use with objects and to simplify the usage with sorting by a specific column/field.
+The functions has been extracted from Lens datatable where it was originally used.
+
+### How to use it
+
+Basic usage with an array of objects:
+
+```js
+import { getSortingCriteria } from '@kbn/sorting-predicates';
+
+...
+const predicate = getSortingCriteria( typeHint, columnId, formatterFn );
+
+const orderedRows = [{a: 1, b: 2}, {a: 3, b: 4}]
+    .sort( (rowA, rowB) => predicate(rowA, rowB, 'asc' /* or 'desc' */));
+```
+
+Basic usage with EUI DataGrid schemaDetector:
+
+```tsx
+const [data, setData] = useState(table);
+const dataGridColumns: EuiDataGridColumn[] = data.columns.map( (column) => ({
+    ...
+    schema: getColumnType(column)
+}));
+const [sortingColumns, setSortingColumns] = useState([
+  { id: 'custom', direction: 'asc' },
+]);
+
+const schemaDetectors = dataGridColumns.map((column) => {
+    const sortingHint = getColumnType(column);
+    const sortingCriteria = getSortingCriteria(
+        sortingHint,
+        column.id,
+        (val: unknwon) => String(val)
+    );
+    return {
+        sortTextAsc: 'asc'
+        sortTextDesc: 'desc',
+        icon: 'starFilled',
+        type: sortingHint || '',
+        detector: () => 1,
+        // This is the actual logic that is used to sort the table
+        comparator: (_a, _b, direction, { aIndex, bIndex }) =>
+        sortingCriteria(data.rows[aIndex], data.rows[bIndex], direction) as 0 | 1 | -1
+    };
+});
+
+return <EuiDataGrid
+    ...
+    inMemory={{ level: 'sorting' }}
+    columns={dataGridColumns}
+    schemaDetectors={schemaDetectors}
+    sorting={{
+        columns: sortingColumns,
+        // this is called only for those columns not covered by the schema detector
+        // and can use the sorting predica as well, manually applied to the data rows
+        onSort: () => { ... }
+    }}
+/>;
+```
diff --git a/packages/kbn-sort-predicates/index.ts b/packages/kbn-sort-predicates/index.ts
@@ -0,0 +1,9 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+export { getSortingCriteria } from './src/sorting';
diff --git a/packages/kbn-sort-predicates/jest.config.js b/packages/kbn-sort-predicates/jest.config.js
@@ -0,0 +1,13 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+module.exports = {
+  preset: '@kbn/test',
+  rootDir: '../..',
+  roots: ['<rootDir>/packages/kbn-sort-predicates'],
+};
diff --git a/packages/kbn-sort-predicates/kibana.jsonc b/packages/kbn-sort-predicates/kibana.jsonc
@@ -0,0 +1,5 @@
+{
+  "type": "shared-common",
+  "id": "@kbn/sort-predicates",
+  "owner": "@elastic/kibana-visualizations"
+}
diff --git a/packages/kbn-sort-predicates/package.json b/packages/kbn-sort-predicates/package.json
@@ -0,0 +1,7 @@
+{
+  "name": "@kbn/sort-predicates",
+  "private": true,
+  "version": "1.0.0",
+  "license": "SSPL-1.0 OR Elastic License 2.0",
+  "sideEffects": false
+}
diff --git a/...on/expressions/datatable/sorting.test.tsx → ...s/kbn-sort-predicates/src/sorting.test.ts b/...on/expressions/datatable/sorting.test.tsx → ...s/kbn-sort-predicates/src/sorting.test.ts
@@ -1,8 +1,9 @@
 /*
  * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
  * or more contributor license agreements. Licensed under the Elastic License
- * 2.0; you may not use this file except in compliance with the Elastic License
- * 2.0.
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
  */
 
 import { getSortingCriteria } from './sorting';
@@ -40,8 +41,8 @@ function testSorting({
       sorted.push(firstEl);
     }
   }
-  const criteria = getSortingCriteria(type, 'a', getMockFormatter(), direction);
-  expect(datatable.sort(criteria).map((row) => row.a)).toEqual(sorted);
+  const criteria = getSortingCriteria(type, 'a', getMockFormatter());
+  expect(datatable.sort((a, b) => criteria(a, b, direction)).map((row) => row.a)).toEqual(sorted);
 }
 
 describe('Data sorting criteria', () => {

diff --git a/.../common/expressions/datatable/sorting.tsx → packages/kbn-sort-predicates/src/sorting.ts b/.../common/expressions/datatable/sorting.tsx → packages/kbn-sort-predicates/src/sorting.ts
@@ -1,8 +1,9 @@
 /*
  * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
  * or more contributor license agreements. Licensed under the Elastic License
- * 2.0; you may not use this file except in compliance with the Elastic License
- * 2.0.
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
  */
 
 import versionCompare from 'compare-versions';
@@ -130,9 +131,15 @@ const rangeComparison: CompareFn<Omit<Range, 'type'>> = (v1, v2) => {
   return fromComparison || toComparison || 0;
 };
 
-function createArrayValuesHandler(sortBy: string, directionFactor: number, formatter: FieldFormat) {
+function createArrayValuesHandler(sortBy: string, formatter: FieldFormat) {
   return function <T>(criteriaFn: CompareFn<T>) {
-    return (rowA: Record<string, unknown>, rowB: Record<string, unknown>) => {
+    return (
+      rowA: Record<string, unknown>,
+      rowB: Record<string, unknown>,
+      direction: 'asc' | 'desc'
+    ) => {
+      // handle the direction with a multiply factor.
+      const directionFactor = direction === 'asc' ? 1 : -1;
       // if either side of the comparison is an array, make it also the other one become one
       // then perform an array comparison
       if (Array.isArray(rowA[sortBy]) || Array.isArray(rowB[sortBy])) {
@@ -157,13 +164,21 @@ function createArrayValuesHandler(sortBy: string, directionFactor: number, forma
 
 function getUndefinedHandler(
   sortBy: string,
-  sortingCriteria: (rowA: Record<string, unknown>, rowB: Record<string, unknown>) => number
+  sortingCriteria: (
+    rowA: Record<string, unknown>,
+    rowB: Record<string, unknown>,
+    directionFactor: 'asc' | 'desc'
+  ) => number
 ) {
-  return (rowA: Record<string, unknown>, rowB: Record<string, unknown>) => {
+  return (
+    rowA: Record<string, unknown>,
+    rowB: Record<string, unknown>,
+    direction: 'asc' | 'desc'
+  ) => {
     const valueA = rowA[sortBy];
     const valueB = rowB[sortBy];
     if (valueA != null && valueB != null && !Number.isNaN(valueA) && !Number.isNaN(valueB)) {
-      return sortingCriteria(rowA, rowB);
+      return sortingCriteria(rowA, rowB, direction);
     }
     if (valueA == null || Number.isNaN(valueA)) {
       return 1;
@@ -179,13 +194,9 @@ function getUndefinedHandler(
 export function getSortingCriteria(
   type: string | undefined,
   sortBy: string,
-  formatter: FieldFormat,
-  direction: string
+  formatter: FieldFormat
 ) {
-  // handle the direction with a multiply factor.
-  const directionFactor = direction === 'asc' ? 1 : -1;
-
-  const arrayValueHandler = createArrayValuesHandler(sortBy, directionFactor, formatter);
+  const arrayValueHandler = createArrayValuesHandler(sortBy, formatter);
 
   if (['number', 'date'].includes(type || '')) {
     return getUndefinedHandler(sortBy, arrayValueHandler(numberCompare));

diff --git a/packages/kbn-sort-predicates/tsconfig.json b/packages/kbn-sort-predicates/tsconfig.json
@@ -0,0 +1,20 @@
+{
+  "extends": "../../tsconfig.base.json",
+  "compilerOptions": {
+    "outDir": "target/types",
+    "types": [
+      "jest",
+      "node",
+    ]
+  },
+  "include": [
+    "**/*.ts"
+  ],
+  "exclude": [
+    "target/**/*"
+  ],
+  "kbn_references": [
+    "@kbn/field-formats-plugin",
+    "@kbn/expressions-plugin"
+  ]
+}
diff --git a/src/plugins/data/common/exports/export_csv.test.ts b/src/plugins/data/common/exports/export_csv.test.ts
@@ -96,4 +96,14 @@ describe('CSV exporter', () => {
       })
     ).toMatch('columnOne\r\n"a,b"\r\n');
   });
+
+  test('should respect the sorted columns order when passed', () => {
+    const datatable = getDataTable({ multipleColumns: true });
+    expect(
+      datatableToCSV(datatable, {
+        ...getDefaultOptions(),
+        columnsSorting: ['col2', 'col1'],
+      })
+    ).toMatch('columnTwo,columnOne\r\n"Formatted_5","Formatted_value"\r\n');
+  });
 });
diff --git a/src/plugins/data/common/exports/export_csv.tsx b/src/plugins/data/common/exports/export_csv.tsx
@@ -21,39 +21,51 @@ interface CSVOptions {
   escapeFormulaValues: boolean;
   formatFactory: FormatFactory;
   raw?: boolean;
+  columnsSorting?: string[];
 }
 
 export function datatableToCSV(
   { columns, rows }: Datatable,
-  { csvSeparator, quoteValues, formatFactory, raw, escapeFormulaValues }: CSVOptions
+  { csvSeparator, quoteValues, formatFactory, raw, escapeFormulaValues, columnsSorting }: CSVOptions
 ) {
   const escapeValues = createEscapeValue({
     separator: csvSeparator,
     quoteValues,
     escapeFormulaValues,
   });
+
+  const sortedIds = columnsSorting || columns.map((col) => col.id);
+
+  // Build an index lookup table
+  const columnIndexLookup = sortedIds.reduce((memo, id, index) => {
+    memo[id] = index;
+    return memo;
+  }, {} as Record<string, number>);
+
   // Build the header row by its names
-  const header = columns.map((col) => escapeValues(col.name));
+  const header: string[] = [];
+  const sortedColumnIds: string[] = [];
+  const formatters: Record<string, ReturnType<FormatFactory>> = {};
 
-  const formatters = columns.reduce<Record<string, ReturnType<FormatFactory>>>(
-    (memo, { id, meta }) => {
-      memo[id] = formatFactory(meta?.params);
-      return memo;
-    },
-    {}
-  );
+  for (const column of columns) {
+    const columnIndex = columnIndexLookup[column.id];
 
-  // Convert the array of row objects to an array of row arrays
-  const csvRows = rows.map((row) => {
-    return columns.map((column) =>
-      escapeValues(raw ? row[column.id] : formatters[column.id].convert(row[column.id]))
-    );
-  });
+    header[columnIndex] = escapeValues(column.name);
+    sortedColumnIds[columnIndex] = column.id;
+    formatters[column.id] = formatFactory(column.meta?.params);
+  }
 
   if (header.length === 0) {
     return '';
   }
 
+  // Convert the array of row objects to an array of row arrays
+  const csvRows = rows.map((row) => {
+    return sortedColumnIds.map((id) =>
+      escapeValues(raw ? row[id] : formatters[id].convert(row[id]))
+    );
+  });
+
   return (
     [header, ...csvRows].map((row) => row.join(csvSeparator)).join(LINE_FEED_CHARACTER) +
     LINE_FEED_CHARACTER

diff --git a/tsconfig.base.json b/tsconfig.base.json
@@ -1534,6 +1534,8 @@
       "@kbn/some-dev-log/*": ["packages/kbn-some-dev-log/*"],
       "@kbn/sort-package-json": ["packages/kbn-sort-package-json"],
       "@kbn/sort-package-json/*": ["packages/kbn-sort-package-json/*"],
+      "@kbn/sort-predicates": ["packages/kbn-sort-predicates"],
+      "@kbn/sort-predicates/*": ["packages/kbn-sort-predicates/*"],
       "@kbn/spaces-plugin": ["x-pack/plugins/spaces"],
       "@kbn/spaces-plugin/*": ["x-pack/plugins/spaces/*"],
       "@kbn/spaces-test-plugin": ["x-pack/test/spaces_api_integration/common/plugins/spaces_test_plugin"],