refactor: Replace usage of Column.index with column name (#1126)

bmingles · web-flow · commit 7448a88a651f · 2023-03-07T14:54:42.000-06:00
resolves #965 BREAKING CHANGE: Removed index property from dh.types Column type. IrisGridUtils.dehydrateSort now returns column name instead of index. TableUtils now expects column name instead of index for functions that don't have access to a columns array.
diff --git a/.eslintignore b/.eslintignore
@@ -0,0 +1 @@
+__mocks__/dh-core.js
diff --git a/packages/dashboard-core-plugins/src/panels/ChartPanel.tsx b/packages/dashboard-core-plugins/src/panels/ChartPanel.tsx
@@ -24,6 +24,7 @@ import {
   DehydratedSort,
   DehydratedAdvancedFilter,
   DehydratedQuickFilter,
+  LegacyDehydratedSort,
 } from '@deephaven/iris-grid';
 import dh, {
   FigureDescriptor,
@@ -107,7 +108,7 @@ export interface ChartPanelTableSettings {
   quickFilters?: readonly DehydratedQuickFilter[];
   advancedFilters?: readonly DehydratedAdvancedFilter[];
   inputFilters?: readonly InputFilter[];
-  sorts?: readonly DehydratedSort[];
+  sorts?: readonly (DehydratedSort | LegacyDehydratedSort)[];
   partition?: unknown;
   partitionColumn?: string;
 }
diff --git a/packages/iris-grid/src/IrisGrid.tsx b/packages/iris-grid/src/IrisGrid.tsx
@@ -2654,7 +2654,8 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
     const { model } = this.props;
     const columnIndex = model.getColumnIndexByName(column.name);
     assertNotNull(columnIndex);
-    const oldSort = TableUtils.getSortForColumn(model.sort, columnIndex);
+    const columnName = model.columns[columnIndex].name;
+    const oldSort = TableUtils.getSortForColumn(model.sort, columnName);
     let newSort = null;
 
     if (oldSort == null || oldSort.direction !== direction) {
@@ -2667,7 +2668,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
 
     const sorts = TableUtils.setSortForColumn(
       model.sort,
-      columnIndex,
+      columnName,
       newSort,
       addToExisting
     );
@@ -4153,7 +4154,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
             const column = model.columns[modelColumn];
             const advancedFilter = advancedFilters.get(modelColumn);
             const { options: advancedFilterOptions } = advancedFilter || {};
-            const sort = TableUtils.getSortForColumn(model.sort, modelColumn);
+            const sort = TableUtils.getSortForColumn(model.sort, column.name);
             const sortDirection = sort ? sort.direction : null;
             const element = (
               <div
diff --git a/packages/iris-grid/src/IrisGridRenderer.ts b/packages/iris-grid/src/IrisGridRenderer.ts
@@ -450,7 +450,13 @@ class IrisGridRenderer extends GridRenderer {
       return;
     }
 
-    const sort = TableUtils.getSortForColumn(model.sort, modelColumn);
+    const columnName = model.columns[modelColumn]?.name;
+
+    if (columnName == null) {
+      return;
+    }
+
+    const sort = TableUtils.getSortForColumn(model.sort, columnName);
 
     if (!sort) {
       return;
diff --git a/packages/iris-grid/src/IrisGridUtils.test.ts b/packages/iris-grid/src/IrisGridUtils.test.ts
@@ -5,7 +5,10 @@ import { DateUtils } from '@deephaven/jsapi-utils';
 import type { AdvancedFilter } from './CommonTypes';
 import { FilterData } from './IrisGrid';
 import IrisGridTestUtils from './IrisGridTestUtils';
-import IrisGridUtils from './IrisGridUtils';
+import IrisGridUtils, {
+  DehydratedSort,
+  LegacyDehydratedSort,
+} from './IrisGridUtils';
 
 function makeFilter() {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -17,7 +20,7 @@ function makeColumns(count = 30) {
 
   for (let i = 0; i < count; i += 1) {
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    const column = new (dh as any).Column({ index: i, name: `${i}` });
+    const column = new (dh as any).Column({ index: i, name: `name_${i}` });
     columns.push(column);
   }
 
@@ -151,30 +154,52 @@ describe('sort exporting/importing', () => {
     expect(importedSort).toEqual(sort);
   });
 
-  it('exports/imports sorts', () => {
+  it('should export (dehydrate) sorts', () => {
     const columns = makeColumns();
     const sort = [columns[3].sort(), columns[7].sort().abs().desc()];
-    const table = makeTable({ columns, sort });
-    const exportedSort = IrisGridUtils.dehydrateSort(sort);
-    expect(exportedSort).toEqual([
-      { column: 3, isAbs: false, direction: 'ASC' },
-      { column: 7, isAbs: true, direction: 'DESC' },
-    ]);
+    const dehydratedSorts = IrisGridUtils.dehydrateSort(sort);
 
-    const importedSort = IrisGridUtils.hydrateSort(table.columns, exportedSort);
-    expect(importedSort).toEqual([
-      expect.objectContaining({
-        column: columns[3],
-        isAbs: false,
-        direction: 'ASC',
-      }),
-      expect.objectContaining({
-        column: columns[7],
-        isAbs: true,
-        direction: 'DESC',
-      }),
+    expect(dehydratedSorts).toEqual<DehydratedSort[]>([
+      { column: columns[3].name, isAbs: false, direction: 'ASC' },
+      { column: columns[7].name, isAbs: true, direction: 'DESC' },
     ]);
   });
+
+  describe('should import (hydrate) sorts', () => {
+    const columns = makeColumns();
+    const sort = [columns[3].sort(), columns[7].sort().abs().desc()];
+    const table = makeTable({ columns, sort });
+
+    const dehydratedSorts = IrisGridUtils.dehydrateSort(sort);
+
+    // Map `column` to a number to represent our LegacyDehydratedSort
+    const legacyDehydratedSorts: LegacyDehydratedSort[] = dehydratedSorts.map(
+      ({ column, ...rest }) => ({
+        column: Number(column.split('_')[1]),
+        ...rest,
+      })
+    );
+
+    it.each([
+      ['current', dehydratedSorts],
+      ['legacy', legacyDehydratedSorts],
+    ])('%s', (_label, sorts) => {
+      const importedSort = IrisGridUtils.hydrateSort(table.columns, sorts);
+
+      expect(importedSort).toEqual([
+        expect.objectContaining({
+          column: columns[3],
+          isAbs: false,
+          direction: 'ASC',
+        }),
+        expect.objectContaining({
+          column: columns[7],
+          isAbs: true,
+          direction: 'DESC',
+        }),
+      ]);
+    });
+  });
 });
 
 describe('pendingDataMap hydration/dehydration', () => {
@@ -222,15 +247,15 @@ describe('pendingDataMap hydration/dehydration', () => {
         1,
         expect.objectContaining({
           data: [
-            ['3', 'Foo'],
-            ['4', 'Bar'],
+            ['name_3', 'Foo'],
+            ['name_4', 'Bar'],
           ],
         }),
       ],
       [
         10,
         expect.objectContaining({
-          data: [['7', 'Baz']],
+          data: [['name_7', 'Baz']],
         }),
       ],
     ]);
@@ -256,7 +281,7 @@ describe('remove columns in moved columns', () => {
     const newMovedColumns = IrisGridUtils.removeColumnFromMovedColumns(
       table.columns,
       movedColumns,
-      ['3']
+      ['name_3']
     );
     expect(newMovedColumns).toEqual([]);
   });
@@ -268,7 +293,7 @@ describe('remove columns in moved columns', () => {
     const newMovedColumns = IrisGridUtils.removeColumnFromMovedColumns(
       table.columns,
       movedColumns,
-      ['3']
+      ['name_3']
     );
     expect(newMovedColumns).toEqual(GridUtils.moveItem(3, 1, [])); // new move should be {from: 3, to: 1}
   });
@@ -281,7 +306,7 @@ describe('remove columns in moved columns', () => {
     const newMovedColumns = IrisGridUtils.removeColumnFromMovedColumns(
       table.columns,
       movedColumns,
-      ['3']
+      ['name_3']
     );
     // columns' original state should be [0,1,2,4,5,...] after '3' is removed;
     // columns after move should be [4,0,1,2,5,...]; after columns '3' is removed;
@@ -297,7 +322,7 @@ describe('remove columns in moved columns', () => {
     const newMovedColumns = IrisGridUtils.removeColumnFromMovedColumns(
       table.columns,
       movedColumns,
-      ['4']
+      ['name_4']
     );
     // column for is removed, the moved column moved back to it's original place, so delete the move
     expect(newMovedColumns).toEqual([]);
@@ -310,7 +335,7 @@ describe('remove columns in moved columns', () => {
     const newMovedColumns = IrisGridUtils.removeColumnFromMovedColumns(
       table.columns,
       movedColumns,
-      ['2', '3']
+      ['name_2', 'name_3']
     );
     // columns' original state should be [0,1,4,5,...] after '2' & '3' are removed;
     // columns after move should be [0,1,4,5,...]; after columns '2' & '3' are removed;
@@ -325,7 +350,7 @@ describe('remove columns in moved columns', () => {
     const newMovedColumns = IrisGridUtils.removeColumnFromMovedColumns(
       table.columns,
       movedColumns,
-      ['2', '3']
+      ['name_2', 'name_3']
     );
     // columns' original state should be [0,1,4,5,...] after '2' & '3' are removed;
     // columns after move should be [0,4,1,5,...]; after columns '2' & '3' are removed;
@@ -341,7 +366,7 @@ describe('remove columns in moved columns', () => {
     const newMovedColumns = IrisGridUtils.removeColumnFromMovedColumns(
       table.columns,
       movedColumns,
-      ['2', '3']
+      ['name_2', 'name_3']
     );
     // columns' original state should be [0,1,4,5,6...] after '2' & '3' are removed;
     // columns after moves should be [0,4,1,6,5...]; after columns '2' & '3' are removed;
@@ -461,9 +486,9 @@ describe('changeFilterColumnNamesToIndexes', () => {
   const columns = makeColumns(10);
   it('Replaces column names with indexes', () => {
     const filters = [
-      { name: '1', filter: DEFAULT_FILTER },
-      { name: '3', filter: DEFAULT_FILTER },
-      { name: '5', filter: DEFAULT_FILTER },
+      { name: 'name_1', filter: DEFAULT_FILTER },
+      { name: 'name_3', filter: DEFAULT_FILTER },
+      { name: 'name_5', filter: DEFAULT_FILTER },
     ];
     expect(
       IrisGridUtils.changeFilterColumnNamesToIndexes(columns, filters)
@@ -477,7 +502,7 @@ describe('changeFilterColumnNamesToIndexes', () => {
   it('Omits missing columns', () => {
     const filters = [
       { name: 'missing_1', filter: DEFAULT_FILTER },
-      { name: '3', filter: DEFAULT_FILTER },
+      { name: 'name_3', filter: DEFAULT_FILTER },
       { name: 'missing_2', filter: DEFAULT_FILTER },
     ];
     expect(
diff --git a/packages/iris-grid/src/IrisGridUtils.ts b/packages/iris-grid/src/IrisGridUtils.ts
@@ -97,11 +97,18 @@ export type DehydratedUserColumnWidth = [ColumnName, number];
 
 export type DehydratedUserRowHeight = [number, number];
 
-export type DehydratedSort = {
+/** @deprecated Use `DehydratedSort` instead */
+export interface LegacyDehydratedSort {
   column: ModelIndex;
   isAbs: boolean;
   direction: SortDirection;
-};
+}
+
+export interface DehydratedSort {
+  column: ColumnName;
+  isAbs: boolean;
+  direction: SortDirection;
+}
 
 export interface DehydratedIrisGridState {
   advancedFilters: readonly DehydratedAdvancedFilter[];
@@ -770,7 +777,7 @@ class IrisGridUtils {
     return sorts.map(sort => {
       const { column, isAbs, direction } = sort;
       return {
-        column: column.index,
+        column: column.name,
         isAbs,
         direction,
       };
@@ -785,16 +792,21 @@ class IrisGridUtils {
    */
   static hydrateSort(
     columns: readonly Column[],
-    sorts: readonly DehydratedSort[]
+    sorts: readonly (DehydratedSort | LegacyDehydratedSort)[]
   ): Sort[] {
     return (
       sorts
         .map(sort => {
-          const { column: columnIndex, isAbs, direction } = sort;
+          const { column: columnIndexOrName, isAbs, direction } = sort;
           if (direction === TableUtils.sortDirection.reverse) {
             return dh.Table.reverse();
           }
-          const column = IrisGridUtils.getColumn(columns, columnIndex);
+
+          const column =
+            typeof columnIndexOrName === 'string'
+              ? IrisGridUtils.getColumnByName(columns, columnIndexOrName)
+              : IrisGridUtils.getColumn(columns, columnIndexOrName);
+
           if (column != null) {
             let columnSort = column.sort();
             if (isAbs) {
@@ -868,7 +880,7 @@ class IrisGridUtils {
       quickFilters?: readonly DehydratedQuickFilter[];
       advancedFilters?: readonly DehydratedAdvancedFilter[];
       inputFilters?: readonly InputFilter[];
-      sorts?: readonly DehydratedSort[];
+      sorts?: readonly (DehydratedSort | LegacyDehydratedSort)[];
       partition?: unknown;
       partitionColumn?: ColumnName;
     },
diff --git a/packages/iris-grid/src/mousehandlers/IrisGridContextMenuHandler.tsx b/packages/iris-grid/src/mousehandlers/IrisGridContextMenuHandler.tsx
@@ -229,7 +229,7 @@ class IrisGridContextMenuHandler extends GridMouseHandler {
     } = theme;
 
     const modelSort = model.sort;
-    const columnSort = TableUtils.getSortForColumn(modelSort, modelIndex);
+    const columnSort = TableUtils.getSortForColumn(modelSort, column.name);
     const hasReverse = reverseType !== TableUtils.REVERSE_TYPE.NONE;
     const { userColumnWidths } = metrics;
     const isColumnHidden = [...userColumnWidths.values()].some(
diff --git a/packages/jsapi-types/src/dh.types.ts b/packages/jsapi-types/src/dh.types.ts
@@ -524,11 +524,6 @@ export interface OneClick {
 }
 
 export interface Column {
-  /**
-   * @deprecated
-   */
-  readonly index: number;
-
   readonly type: string;
   readonly name: string;
   readonly description: string;
diff --git a/packages/jsapi-utils/src/TableUtils.test.ts b/packages/jsapi-utils/src/TableUtils.test.ts
diff --git a/packages/jsapi-utils/src/TableUtils.ts b/packages/jsapi-utils/src/TableUtils.ts
