fix PR comments

Signed-off-by: Anan Z <ananzh@amazon.com>
ananzh · Feb 2, 2024 · bd4ae1b · bd4ae1b
1 parent 07abc6e
commit bd4ae1b
Show file tree

Hide file tree

Showing 6 changed files with 97 additions and 9 deletions.
diff --git a/.../discover/public/application/components/default_discover_table/default_discover_table.tsx b/.../discover/public/application/components/default_discover_table/default_discover_table.tsx
@@ -57,13 +57,13 @@ export const LegacyDiscoverTable = ({
   defaultSortOrder,
   showPagination,
 }: DefaultDiscoverTableProps) => {
-  const displayedDefaultColumns = getLegacyDisplayedColumns(
+  const displayedColumns = getLegacyDisplayedColumns(
     columns,
     indexPattern,
     hideTimeColumn,
     isShortDots
   );
-  const adjustedDefaultColumnNames = displayedDefaultColumns.map((column) => column.name);
+  const displayedColumnNames = displayedColumns.map((column) => column.name);
   const pageSize = 50;
   const [renderedRowCount, setRenderedRowCount] = useState(50); // Start with 50 rows
   const [displayedRows, setDisplayedRows] = useState(rows.slice(0, pageSize));
@@ -145,13 +145,13 @@ export const LegacyDiscoverTable = ({
         <table data-test-subj="docTable" className="osd-table table">
           <thead>
             <TableHeader
-              displayedColumns={displayedDefaultColumns}
+              displayedColumns={displayedColumns}
               defaultSortOrder={defaultSortOrder}
               indexPattern={indexPattern}
               onChangeSortOrder={onSort}
               onReorderColumn={onReorderColumn}
               onRemoveColumn={onRemoveColumn}
-              sortOrder={sort as SortOrder[]}
+              sortOrder={sort}
             />
           </thead>
           <tbody>
@@ -161,7 +161,7 @@ export const LegacyDiscoverTable = ({
                   <TableRow
                     key={index}
                     row={row}
-                    columns={adjustedDefaultColumnNames}
+                    columns={displayedColumnNames}
                     indexPattern={indexPattern}
                     onRemoveColumn={onRemoveColumn}
                     onAddColumn={onAddColumn}

diff --git a/src/plugins/discover/public/application/components/default_discover_table/table_cell.tsx b/src/plugins/discover/public/application/components/default_discover_table/table_cell.tsx
@@ -31,11 +31,11 @@ export const TableCell = ({
   sanitizedCellValue,
 }: TableCellProps) => {
   return (
-    // eslint-disable-next-line react/no-danger
     <td
       data-test-subj="docTableField"
       className="osdDocTableCell eui-textBreakAll eui-textBreakWord"
     >
+      {/* eslint-disable-next-line react/no-danger */}
       <span dangerouslySetInnerHTML={{ __html: sanitizedCellValue }} />
       <span className="osdDocTableCell__filter">
         <EuiToolTip

diff --git a/...ins/discover/public/application/components/default_discover_table/table_header_column.tsx b/...ins/discover/public/application/components/default_discover_table/table_header_column.tsx
@@ -24,7 +24,6 @@ interface Props {
   isSortable?: boolean;
   name: string;
   onChangeSortOrder?: (sortOrder: SortOrder[]) => void;
-  onMoveColumn?: (name: string, idx: number) => void;
   onReorderColumn?: (colName: string, destination: number) => void;
   onRemoveColumn?: (name: string) => void;
   sortOrder: SortOrder[];

diff --git a/src/plugins/discover/public/application/utils/state_management/common.test.ts b/src/plugins/discover/public/application/utils/state_management/common.test.ts
@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { addColumn, removeColumn, reorderColumn } from './common';
+import { addColumn, removeColumn, reorderColumn, moveColumn } from './common';
 
 describe('commonUtils', () => {
   it('should handle addColumn', () => {
@@ -22,4 +22,41 @@ describe('commonUtils', () => {
       'column1',
     ]);
   });
+
+  it('should handle moveColumn', () => {
+    // test moving a column within the array
+    expect(moveColumn(['column1', 'column2', 'column3'], 'column2', 0)).toEqual([
+      'column2',
+      'column1',
+      'column3',
+    ]);
+
+    // test moving a column to the same index (should result in no change)
+    expect(moveColumn(['column1', 'column2', 'column3'], 'column2', 1)).toEqual([
+      'column1',
+      'column2',
+      'column3',
+    ]);
+
+    // test moving a column to the end
+    expect(moveColumn(['column1', 'column2', 'column3'], 'column1', 2)).toEqual([
+      'column2',
+      'column3',
+      'column1',
+    ]);
+
+    // test trying to move a column to an index out of bounds (should return original array)
+    expect(moveColumn(['column1', 'column2', 'column3'], 'column1', 3)).toEqual([
+      'column1',
+      'column2',
+      'column3',
+    ]);
+
+    // test trying to move a column that doesn't exist (should return original array)
+    expect(moveColumn(['column1', 'column2', 'column3'], 'column4', 1)).toEqual([
+      'column1',
+      'column2',
+      'column3',
+    ]);
+  });
 });
diff --git a/src/plugins/discover/public/application/utils/state_management/discover_slice.test.tsx b/src/plugins/discover/public/application/utils/state_management/discover_slice.test.tsx
@@ -82,4 +82,56 @@ describe('discoverSlice', () => {
     const result = discoverSlice.reducer(initialState, action);
     expect(result.sort).toEqual([['field2', 'desc']]);
   });
+
+  it('should handle moveColumn', () => {
+    initialState = {
+      columns: ['column1', 'column2', 'column3'],
+      sort: [],
+    };
+    const action = {
+      type: 'discover/moveColumn',
+      payload: { columnName: 'column2', destination: 0 },
+    };
+    const result = discoverSlice.reducer(initialState, action);
+    expect(result.columns).toEqual(['column2', 'column1', 'column3']);
+  });
+
+  it('should maintain columns order when moving a column to its current position', () => {
+    initialState = {
+      columns: ['column1', 'column2', 'column3'],
+      sort: [],
+    };
+    const action = {
+      type: 'discover/moveColumn',
+      payload: { columnName: 'column2', destination: 1 },
+    };
+    const result = discoverSlice.reducer(initialState, action);
+    expect(result.columns).toEqual(['column1', 'column2', 'column3']);
+  });
+
+  it('should handle moveColumn when destination is out of range', () => {
+    initialState = {
+      columns: ['column1', 'column2', 'column3'],
+      sort: [],
+    };
+    const action = {
+      type: 'discover/moveColumn',
+      payload: { columnName: 'column1', destination: 5 },
+    };
+    const result = discoverSlice.reducer(initialState, action);
+    expect(result.columns).toEqual(['column1', 'column2', 'column3']);
+  });
+
+  it('should not change columns if column to move does not exist', () => {
+    initialState = {
+      columns: ['column1', 'column2', 'column3'],
+      sort: [],
+    };
+    const action = {
+      type: 'discover/moveColumn',
+      payload: { columnName: 'nonExistingColumn', destination: 0 },
+    };
+    const result = discoverSlice.reducer(initialState, action);
+    expect(result.columns).toEqual(['column1', 'column2', 'column3']);
+  });
 });
diff --git a/src/plugins/discover/public/application/utils/state_management/discover_slice.tsx b/src/plugins/discover/public/application/utils/state_management/discover_slice.tsx
@@ -42,7 +42,7 @@ export interface DiscoverState {
    * dirty flag to indicate if the saved search has been modified
    * since the last save
    */
-  isDirty: boolean;
+  isDirty?: boolean;
 }
 
 export interface DiscoverRootState extends RootState {