From 26286e56cf9728cc4871cab7181ba94df31220ff Mon Sep 17 00:00:00 2001 From: Gavin Reynolds Date: Thu, 11 Jul 2024 11:54:06 +0100 Subject: [PATCH] Reduce duplication of values/ids/labels etc so they are defined in fewer places Signed-off-by: Gavin Reynolds --- .../ColumnsModal/ColumnsModalComponent.jsx | 64 +++++-------------- .../ColumnsModalComponent.test.js | 32 +++++++++- .../subcomponents/ColumnsModalItem.jsx | 7 +- .../DraggableColumnsModalItem.jsx | 27 +++----- src/config/column-generator.jsx | 34 +++++++--- src/redux/incident_table/reducers.js | 16 ++--- 6 files changed, 90 insertions(+), 90 deletions(-) diff --git a/src/components/ColumnsModal/ColumnsModalComponent.jsx b/src/components/ColumnsModal/ColumnsModalComponent.jsx index db0d9e89a..07c026e32 100644 --- a/src/components/ColumnsModal/ColumnsModalComponent.jsx +++ b/src/components/ColumnsModal/ColumnsModalComponent.jsx @@ -90,51 +90,18 @@ 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, } )); @@ -142,14 +109,14 @@ const TableColumnsModalComponent = () => { 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 = () => { {selectedColumns.map((column) => ( unselectColumn(column)} itemType="selected" @@ -286,7 +254,7 @@ const TableColumnsModalComponent = () => { {unselectedColumns.map((column) => ( selectColumn(column)} itemType="available" @@ -301,17 +269,20 @@ const TableColumnsModalComponent = () => { + alert: {alertCustomDetailFields.map((column) => ( removeCustomAlertColumn(column)} itemType="custom" /> ))} +
+ computed: {computedFields.map((column) => ( 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 index a0ac3759d..c265cc2d3 100644 --- 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 index d253e4513..bcf748c92 100644 --- a/src/components/ColumnsModal/subcomponents/ColumnsModalItem.jsx +++ b/src/components/ColumnsModal/subcomponents/ColumnsModalItem.jsx @@ -33,7 +33,6 @@ 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; @@ -41,7 +40,7 @@ const ColumnsModalItem = ({ case 'available': ariaLabel = 'Add column'; iconComponent = ( - + ); break; case 'selected': @@ -49,12 +48,12 @@ const ColumnsModalItem = ({ default: ariaLabel = 'Remove column'; iconComponent = ( - + ); 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 index bcb7fd108..9e343361d 100644 --- 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,7 +48,7 @@ const DraggableColumnsModalItem = ({ } }, }), - [columnValue(column), originalIndex, moveColumnInSelectedColumns], + [column.id, originalIndex, moveColumnInSelectedColumns], ); const [, drop] = useDrop( () => ({ @@ -67,10 +56,10 @@ const DraggableColumnsModalItem = ({ hover({ id: draggedId, }) { - if (draggedId !== columnValue(column)) { + if (draggedId !== column.id) { const { index: overIndex, - } = findColumnInSelectedColumns(columnValue(column)); + } = findColumnInSelectedColumns(column.id); moveColumnInSelectedColumns(draggedId, overIndex); } }, diff --git a/src/config/column-generator.jsx b/src/config/column-generator.jsx index 018128e48..57b373c7e 100644 --- a/src/config/column-generator.jsx +++ b/src/config/column-generator.jsx @@ -193,6 +193,8 @@ export const incidentColumn = ({ }; const column = { + id, + value: id, originalHeader: header, Header: i18next.t(header), i18n: i18next.t(header), @@ -204,12 +206,6 @@ export const incidentColumn = ({ Filter: ColumnFilterComponent, }; - if (id) { - column.id = id; - } else if (typeof accessor === 'string') { - column.id = accessor; - } - if (sortType) { column.sortType = sortType; } @@ -219,6 +215,7 @@ export const incidentColumn = ({ export const defaultIncidentColumns = () => [ incidentColumn({ + id: 'incident_number', header: '#', accessor: 'incident_number', minWidth: 80, @@ -230,6 +227,7 @@ export const defaultIncidentColumns = () => [ }), }), incidentColumn({ + id: 'title', header: 'Title', accessor: 'title', minWidth: 200, @@ -247,6 +245,7 @@ export const defaultIncidentColumns = () => [ minWidth: 200, }), incidentColumn({ + id: 'created_at', header: 'Created At', accessor: 'created_at', minWidth: 200, @@ -272,6 +271,7 @@ export const defaultIncidentColumns = () => [ sortType: dateValueSortType, }), incidentColumn({ + id: 'updated_at', header: 'Updated At', accessor: 'updated_at', minWidth: 200, @@ -283,6 +283,7 @@ export const defaultIncidentColumns = () => [ sortType: dateValueSortType, }), incidentColumn({ + id: 'status', header: 'Status', accessor: 'status', minWidth: 90, @@ -291,6 +292,7 @@ export const defaultIncidentColumns = () => [ }) => , }), incidentColumn({ + id: 'incident_key', header: 'Incident Key', accessor: 'incident_key', minWidth: 300, @@ -323,6 +325,7 @@ export const defaultIncidentColumns = () => [ ), }), incidentColumn({ + id: 'last_status_change_at', header: 'Last Status Change At', accessor: 'last_status_change_at', minWidth: 200, @@ -398,6 +401,7 @@ export const defaultIncidentColumns = () => [ ), }), incidentColumn({ + id: 'last_status_change_by', header: 'Last Status Change By', accessor: 'last_status_change_by.summary', minWidth: 150, @@ -440,6 +444,7 @@ export const defaultIncidentColumns = () => [ }, }), incidentColumn({ + id: 'urgency', header: 'Urgency', accessor: 'urgency', minWidth: 120, @@ -471,11 +476,13 @@ export const defaultIncidentColumns = () => [ }, }), incidentColumn({ + id: 'id', header: 'Incident ID', accessor: 'id', minWidth: 160, }), incidentColumn({ + id: 'summary', header: 'Summary', accessor: 'summary', minWidth: 300, @@ -818,8 +825,11 @@ export const computedColumnForSavedColumn = (savedColumn) => { return null; } }; + + const id = header; const column = incidentColumn({ - id: `${header}-accessorPath`, + id, + value: id, header, columnType: 'computed', accessor, @@ -856,8 +866,11 @@ export const customAlertColumnForSavedColumn = (savedColumn) => { } return result[0]; }; + + const id = header + (accessorPath ? `:${accessorPath}` : '') + (aggregator ? `:${aggregator}` : ''); const column = incidentColumn({ - id: accessorPath, + id, + value: id, header, columnType: 'alert', accessor, @@ -895,8 +908,11 @@ export const customComputedColumnForSavedColumn = (savedColumn) => { } return result[0]; }; + + const id = `${header}:${accessorPath}:${expression}`; const column = incidentColumn({ - id: accessorPath, + id, + value: id, header, columnType: 'computed', accessor, diff --git a/src/redux/incident_table/reducers.js b/src/redux/incident_table/reducers.js index a7f8d3f82..43728ed64 100644 --- a/src/redux/incident_table/reducers.js +++ b/src/redux/incident_table/reducers.js @@ -17,14 +17,14 @@ import { } from './actions'; const defaultColumns = [ - { Header: '#', width: 60, columnType: 'incident' }, - { Header: 'Status', width: 100, columnType: 'incident' }, - { Header: 'Priority', width: 90, columnType: 'incident' }, - { Header: 'Title', width: 400, columnType: 'incident' }, - { Header: 'Assignees', width: 160, columnType: 'incident' }, - { Header: 'Created At', width: 180, columnType: 'incident' }, - { Header: 'Service', width: 300, columnType: 'incident' }, - { Header: 'Latest Note', width: 200, columnType: 'incident' }, + { value: 'incident_number', Header: '#', width: 60, columnType: 'incident' }, + { value: 'status', Header: 'Status', width: 100, columnType: 'incident' }, + { value: 'priority', Header: 'Priority', width: 90, columnType: 'incident' }, + { value: 'title', Header: 'Title', width: 400, columnType: 'incident' }, + { value: 'assignees', Header: 'Assignees', width: 160, columnType: 'incident' }, + { value: 'created_at', Header: 'Created At', width: 180, columnType: 'incident' }, + { value: 'service', Header: 'Service', width: 300, columnType: 'incident' }, + { value: 'latest_note', Header: 'Latest Note', width: 200, columnType: 'incident' }, ]; const incidentTable = produce(