Reduce duplication of values/ids/labels etc so they are defined in fe…

…wer places Signed-off-by: Gavin Reynolds <[email protected]>
PagerDuty · Jul 11, 2024 · 26286e5 · 26286e5
1 parent d36a15d
commit 26286e5
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 90 deletions.
diff --git a/src/components/ColumnsModal/ColumnsModalComponent.jsx b/src/components/ColumnsModal/ColumnsModalComponent.jsx
@@ -90,66 +90,33 @@ const TableColumnsModalComponent = () => {
 
   const toast = useToast();
 
-  const columnValue = (column) => {
-    if (!column) return '';
-    let value;
-    if (column.columnType === 'alert') {
-      // Alert column based on aggregator
-      value = column.Header
-        + (column.accessorPath ? `:${column.accessorPath}` : '')
-        + (column.aggregator ? `:${column.aggregator}` : '');
-    } else if (column.columnType === 'computed') {
-      // Computed column based on expression
-      value = column.Header
-        + (column.accessorPath ? `:${column.accessorPath}` : '')
-        + (column.expression ? `:${column.expression}` : '');
-    } else {
-      // Incident column
-      value = column.Header;
-    }
-    return value;
-  };
-
   const getAllAvailableColumns = () => {
     // eslint-disable-next-line max-len
-    const v = [...defaultColumns(), ...customAlertColumns(alertCustomDetailFields), ...customComputedColumns(computedFields)].sort((a, b) => columnValue(a).localeCompare(columnValue(b)));
+    const v = [...defaultColumns(), ...customAlertColumns(alertCustomDetailFields), ...customComputedColumns(computedFields)].sort((a, b) => a.value.localeCompare(b.value));
     return v;
   };
 
   const allAvailableColumns = useMemo(getAllAvailableColumns, [alertCustomDetailFields, computedFields]);
 
-  // const getSelectedColumns = () => columnsForSavedColumns(incidentTableColumns).map((column) => {
-  //   // Recreate original value used from react-select in order to populate dual list
-  //   const value = columnValue(column);
-  //   return {
-  //     id: column.id,
-  //     Header: column.Header,
-  //     columnType: column.columnType,
-  //     accessor: column.accessor,
-  //     accessorPath: column.accessorPath,
-  //     label: column.i18n ? column.i18n : column.Header,
-  //     value,
-  //   };
-  // });
   const getSelectedColumns = () => incidentTableColumns.map((column) => (
     {
       ...column,
-      value: columnValue(column),
+      value: column.value,
       label: column.i18n ? column.i18n : column.Header,
     }
   ));
   const [selectedColumns, setSelectedColumns] = useState(getSelectedColumns());
 
   const getUnselectedColumns = () => {
     const unselected = allAvailableColumns.filter(
-      (c) => !selectedColumns.find((s) => s.value === columnValue(c)),
+      (c) => !selectedColumns.find((s) => s.value === c.value),
     );
     return unselected;
   };
   const unselectedColumns = useMemo(getUnselectedColumns, [allAvailableColumns, selectedColumns]);
 
   const unselectColumn = (column) => {
-    setSelectedColumns((prev) => prev.filter((c) => columnValue(c) !== columnValue(column)));
+    setSelectedColumns((prev) => prev.filter((c) => c.value !== column.value));
   };
   const selectColumn = (column) => {
     setSelectedColumns((prev) => [...prev, column]);
@@ -158,11 +125,12 @@ const TableColumnsModalComponent = () => {
   const addCustomAlertColumn = (value) => {
     const [Header, accessorPath, aggregator] = value.split(':');
     const newColumn = {
+      id: value,
       Header,
       accessorPath,
       aggregator,
       value,
-      // label: value,
+      label: value,
       columnType: 'alert',
     };
     const newAlertCustomDetailFields = [...alertCustomDetailFields, newColumn];
@@ -172,15 +140,15 @@ const TableColumnsModalComponent = () => {
   const addCustomComputedColumn = (value) => {
     const [Header, accessorPath, expression] = value.split(':');
     const newColumn = {
+      id: value,
       Header,
       accessorPath,
       value,
       expressionType: 'regex-single',
       expression,
-      // label: value,
+      label: value,
       columnType: 'computed',
     };
-    console.error('newColumn', newColumn);
     const newComputedFields = [...computedFields, newColumn];
     setComputedColumns(newComputedFields);
   };
@@ -223,7 +191,7 @@ const TableColumnsModalComponent = () => {
 
   const findColumnInSelectedColumns = useCallback(
     (value) => {
-      const column = selectedColumns.find((c) => columnValue(c) === value);
+      const column = selectedColumns.find((c) => c.value === value);
       return {
         column,
         index: selectedColumns.indexOf(column),
@@ -262,7 +230,7 @@ const TableColumnsModalComponent = () => {
                 <Box id="selected-columns-card-body" ref={drop}>
                   {selectedColumns.map((column) => (
                     <DraggableColumnsModalItem
-                      key={columnValue(column)}
+                      key={column.value}
                       column={column}
                       onClick={() => unselectColumn(column)}
                       itemType="selected"
@@ -286,7 +254,7 @@ const TableColumnsModalComponent = () => {
                 <Box id="available-columns-card-body">
                   {unselectedColumns.map((column) => (
                     <ColumnsModalItem
-                      key={columnValue(column)}
+                      key={column.value}
                       column={column}
                       onClick={() => selectColumn(column)}
                       itemType="available"
@@ -301,17 +269,20 @@ const TableColumnsModalComponent = () => {
               </CardHeader>
               <CardBody>
                 <Box id="custom-columns-card-body">
+                  alert:
                   {alertCustomDetailFields.map((column) => (
                     <ColumnsModalItem
-                      key={columnValue(column)}
+                      key={column.value}
                       column={column}
                       onClick={() => removeCustomAlertColumn(column)}
                       itemType="custom"
                     />
                   ))}
+                  <br />
+                  computed:
                   {computedFields.map((column) => (
                     <ColumnsModalItem
-                      key={columnValue(column)}
+                      key={column.value}
                       column={column}
                       onClick={() => removeCustomComputedColumn(column)}
                       itemType="custom"
@@ -324,7 +295,6 @@ const TableColumnsModalComponent = () => {
                     ref={columnTypeInputRef}
                     w="10%"
                     onChange={() => {
-                      // console.error(columnTypeInputRef.current.value);
                       setColumnType(columnTypeInputRef.current.value);
                     }}
                   >
@@ -395,7 +365,7 @@ const TableColumnsModalComponent = () => {
             mr={3}
             onClick={() => {
               // remove any filters on columns that are no longer selected
-              const selectedColumnIds = selectedColumns.map((c) => c.id);
+              const selectedColumnIds = selectedColumns.map((c) => c.value);
               const filters = incidentTableState.filters.filter((f) => selectedColumnIds.includes(f.id));
               updateIncidentTableState({ ...incidentTableState, filters });
 

diff --git a/src/components/ColumnsModal/ColumnsModalComponent.test.js b/src/components/ColumnsModal/ColumnsModalComponent.test.js
@@ -54,6 +54,18 @@ describe('ColumnsModalComponent', () => {
             aggregator: null,
           },
         ],
+        computedFields: [
+          {
+            label: 'regex-single in incident body:first_trigger_log_entry.channel.details:(.*.example.com)',
+            value: 'regex-single in incident body:first_trigger_log_entry.channel.details:(.*.example.com)',
+            columnType: 'computed',
+            Header: 'regex-single in incident body',
+            accessorPath: 'first_trigger_log_entry.channel.details',
+            expressionType: 'regex-single',
+            expression: '(.*.example.com)',
+            aggregator: null,
+          },
+        ],
       },
       incidentTable: {
         incidentTableColumns: [
@@ -81,8 +93,8 @@ describe('ColumnsModalComponent', () => {
     expect(screen.getByRole('banner')).toBeInTheDocument();
     expect(screen.getByRole('banner')).toHaveTextContent('Incident Table');
     expect(screen.getByRole('heading', { name: 'Selected' })).toBeInTheDocument();
-    // 2 standard fields, 3 custom fields, 2 added custom fields
-    expect(screen.getAllByLabelText('Remove column')).toHaveLength(7);
+    // 2 standard fields, 3 custom fields, 2 added custom alert fields, 1 added computed
+    expect(screen.getAllByLabelText('Remove column')).toHaveLength(8);
     expect(screen.getByRole('heading', { name: 'Available' })).toBeInTheDocument();
     expect(screen.getByRole('heading', { name: 'Custom' })).toBeInTheDocument();
     expect(screen.getByRole('button', { name: 'OK' })).toBeInTheDocument();
@@ -102,7 +114,7 @@ describe('ColumnsModalComponent', () => {
     expect(within(selectedColumn).getByLabelText('Remove column')).toBeInTheDocument();
   });
 
-  it('should render an available custom column option with unique header name', () => {
+  it('should render an available custom alert column option with unique header name', () => {
     const customColumns = screen.getByText('Custom').parentElement.parentElement;
     const customColumn = within(customColumns).getByText(
       'AnotherCustomField:details.to.some.other.path',
@@ -138,4 +150,18 @@ describe('ColumnsModalComponent', () => {
   //   tabElement.simulate('click');
   //   expect(wrapper.find('[value="Summary:details.to.some.path"]').prop('disabled')).toEqual(true);
   // });
+
+  it('should render an available computed option with unique header name', () => {
+    const customColumns = screen.getByText('Custom').parentElement.parentElement;
+    const customColumn = within(customColumns).getByText(
+      'regex-single in incident body:first_trigger_log_entry.channel.details:(.*.example.com)',
+    );
+    expect(customColumn).toBeInTheDocument();
+    expect(within(customColumn).getByLabelText('Remove column')).toBeInTheDocument();
+
+    const availableColumns = screen.getByText('Available').parentElement.parentElement;
+    const availableColumn = within(availableColumns).getByText('regex-single in incident body');
+    expect(availableColumn).toBeInTheDocument();
+    expect(within(availableColumn).getByLabelText('Add column')).toBeInTheDocument();
+  });
 });
diff --git a/src/components/ColumnsModal/subcomponents/ColumnsModalItem.jsx b/src/components/ColumnsModal/subcomponents/ColumnsModalItem.jsx
@@ -33,28 +33,27 @@ const ColumnsModalItem = ({
     // old default example column would give null here
     return null;
   }
-  const columnId = column.id || column.label || column.accessorPath;
 
   let ariaLabel = '';
   let iconComponent = null;
   switch (itemType) {
     case 'available':
       ariaLabel = 'Add column';
       iconComponent = (
-        <AddIcon id={`column-${columnId}-add-icon`} {...iconProps} aria-label={ariaLabel} />
+        <AddIcon id={`column-${column.id}-add-icon`} {...iconProps} aria-label={ariaLabel} />
       );
       break;
     case 'selected':
     case 'custom':
     default:
       ariaLabel = 'Remove column';
       iconComponent = (
-        <CloseIcon id={`column-${columnId}-remove-icon`} {...iconProps} aria-label={ariaLabel} />
+        <CloseIcon id={`column-${column.id}-remove-icon`} {...iconProps} aria-label={ariaLabel} />
       );
       break;
   }
   const tagProps = {
-    id: `column-${columnId}-tag`,
+    id: `column-${column.id}-tag`,
     m: 1,
     variant: itemType === 'available' ? 'subtle' : 'solid',
     colorScheme: itemType === 'available' ? 'gray' : 'blue',

diff --git a/src/components/ColumnsModal/subcomponents/DraggableColumnsModalItem.jsx b/src/components/ColumnsModal/subcomponents/DraggableColumnsModalItem.jsx
@@ -13,21 +13,10 @@ import {
   AddIcon, CloseIcon,
 } from '@chakra-ui/icons';
 
-const columnValue = (column) => {
-  let value;
-  if (column.columnType === 'alert') {
-    // Alert column based on aggregator
-    value = column.Header
-      + (column.accessorPath ? `:${column.accessorPath}` : '')
-      + (column.aggregator ? `:${column.aggregator}` : '');
-  } else {
-    // Incident column
-    value = column.Header;
-  }
-  return value;
-};
-
 const columnLabel = (column) => {
+  if (!column) {
+    return '';
+  }
   if (column.label) {
     return column.label;
   }
@@ -41,11 +30,11 @@ const DraggableColumnsModalItem = ({
   moveColumnInSelectedColumns,
   findColumnInSelectedColumns,
 }) => {
-  const originalIndex = findColumnInSelectedColumns(columnValue(column)).index;
+  const originalIndex = findColumnInSelectedColumns(column.id).index;
   const [, drag] = useDrag(
     () => ({
       type: 'DraggableColumnsModalItem',
-      item: { id: columnValue(column), originalIndex },
+      item: { id: column.id, originalIndex },
       collect: (monitor) => ({
         isDragging: monitor.isDragging(),
       }),
@@ -59,18 +48,18 @@ const DraggableColumnsModalItem = ({
         }
       },
     }),
-    [columnValue(column), originalIndex, moveColumnInSelectedColumns],
+    [column.id, originalIndex, moveColumnInSelectedColumns],
   );
   const [, drop] = useDrop(
     () => ({
       accept: 'DraggableColumnsModalItem',
       hover({
         id: draggedId,
       }) {
-        if (draggedId !== columnValue(column)) {
+        if (draggedId !== column.id) {
           const {
             index: overIndex,
-          } = findColumnInSelectedColumns(columnValue(column));
+          } = findColumnInSelectedColumns(column.id);
           moveColumnInSelectedColumns(draggedId, overIndex);
         }
       },