Skip to content

Commit

Permalink
Inline editing issues
Browse files Browse the repository at this point in the history
Inline editing while filters were applied would cause edited values to be removed from the table and would remain hidden even after resetting all filters.

This was caused because the SET filter did not update to include the new value, so the record would be filtered out from the table - and even after resetting, the filter values were not re-calculated.

In addition the onRecordChanged emits the filtered values, not all values, so the parent component was saving just the filtered records thinking that was the universe - this also caused some (most?) records to not be available to be saved when submitting to Salesforce.

To solve, the data table component intercepts the change and ensures that the table set filters are updated to include the new value in the list and select the new record column value.

Unfortunately this only solves the set filter and does not solve any other filters (text, date, etc..) - that would require a complete refactor of how state is managed for the table, which is high-risk and a lot of work - so we will deal with it for now. To help this, a number has been added to the submit button to show the number of changed records.

resolves #1113
  • Loading branch information
paustint committed Dec 18, 2024
1 parent 2a8348e commit 8736121
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 14 deletions.
7 changes: 7 additions & 0 deletions libs/ui/src/lib/data-table/DataTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export const DataTable = forwardRef<any, DataTableProps<any>>(
handleCellKeydown,
handleCellContextMenu,
handleCloseContextMenu,
handleRowChange,
} = useDataTable({
data,
columns: _columns,
Expand Down Expand Up @@ -130,6 +131,12 @@ export const DataTable = forwardRef<any, DataTableProps<any>>(
// @ts-expect-error Types are incorrect, but they are generic and difficult to get correct
onCellContextMenu={handleCellContextMenu}
{...rest}
onRowsChange={(rows, data) => {
if (rest.onRowsChange) {
handleRowChange(rows, data);
rest.onRowsChange(rows as any, data as any);
}
}}
/>
{contextMenuProps && contextMenuItems && contextMenuAction && (
<ContextMenu
Expand Down
30 changes: 20 additions & 10 deletions libs/ui/src/lib/data-table/SalesforceRecordDataTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { css } from '@emotion/react';
import { logger } from '@jetstream/shared/client-logger';
import { queryRemaining } from '@jetstream/shared/data';
import { formatNumber, useRollbar } from '@jetstream/shared/ui-utils';
import { flattenRecord, getIdFromRecordUrl, nullifyEmptyStrings } from '@jetstream/shared/utils';
import { flattenRecord, getIdFromRecordUrl, groupByFlat, nullifyEmptyStrings } from '@jetstream/shared/utils';
import { CloneEditView, ContextMenuItem, Field, Maybe, QueryResults, SalesforceOrgUi, SobjectCollectionResponse } from '@jetstream/types';
import uniqueId from 'lodash/uniqueId';
import { Fragment, FunctionComponent, ReactNode, memo, useCallback, useEffect, useRef, useState } from 'react';
Expand Down Expand Up @@ -294,12 +294,23 @@ export const SalesforceRecordDataTable: FunctionComponent<SalesforceRecordDataTa
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

const handleRowsChange = useCallback((rows: RowSalesforceRecordWithKey[], data: RowsChangeData<RowSalesforceRecordWithKey[]>) => {
setRows(rows);
setDirtyRows(
rows.filter((row) => row._touchedColumns.size > 0 && Array.from(row._touchedColumns).some((col) => row[col] !== row._record[col]))
);
}, []);
const handleRowsChange = useCallback(
(
allRows: RowSalesforceRecordWithKey[],
filteredRows: RowSalesforceRecordWithKey[],
data: RowsChangeData<RowSalesforceRecordWithKey>
) => {
const rowsByKey = groupByFlat(filteredRows, '_key');
const newRows = allRows.map((row) => rowsByKey[row._key] || row);
setRows(newRows);
setDirtyRows(
newRows.filter(
(row) => row._touchedColumns.size > 0 && Array.from(row._touchedColumns).some((col) => row[col] !== row._record[col])
)
);
},
[]
);

const handleCancelEditMode = () => {
setRecords((records) => (records ? [...records] : records));
Expand Down Expand Up @@ -435,8 +446,7 @@ export const SalesforceRecordDataTable: FunctionComponent<SalesforceRecordDataTa
onClick={handleSaveRecords}
disabled={isSavingRecords}
>
Save
{isSavingRecords && <Spinner size="small" />}
Save ({formatNumber(dirtyRows.length)}){isSavingRecords && <Spinner size="small" />}
</button>
</Grid>
)}
Expand Down Expand Up @@ -469,7 +479,7 @@ export const SalesforceRecordDataTable: FunctionComponent<SalesforceRecordDataTa
onReorderColumns={handleColumnReorder}
onSelectedRowsChange={handleSelectedRowsChange}
onSortedAndFilteredRowsChange={handleSortedAndFilteredRowsChange}
onRowsChange={handleRowsChange}
onRowsChange={(changedRows, data) => handleRowsChange(rows || [], changedRows, data)}
context={{
org,
defaultApiVersion,
Expand Down
82 changes: 78 additions & 4 deletions libs/ui/src/lib/data-table/useDataTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
CellMouseEvent,
RenderSortStatusProps,
Renderers,
RowsChangeData,
SortColumn,
} from 'react-data-grid';
import 'react-data-grid/lib/styles.css';
Expand Down Expand Up @@ -246,6 +247,10 @@ export function useDataTable<T = RowWithKey>({
[filteredRows]
);

const handleRowChange = useCallback((rows: any[], data: RowsChangeData<any>) => {
dispatch({ type: 'ADD_MODIFIED_VALUE_TO_SET_FILTER', payload: { rows, data } });
}, []);

const handleCloseContextMenu = useCallback(() => setContextMenuProps(null), []);

// NOTE: this is not used anywhere, so we may consider removing it.
Expand Down Expand Up @@ -288,6 +293,7 @@ export function useDataTable<T = RowWithKey>({
handleCellKeydown,
handleCellContextMenu: contextMenuItems && contextMenuAction ? handleCellContextMenu : undefined,
handleCloseContextMenu: handleCloseContextMenu,
handleRowChange,
};
}

Expand All @@ -314,6 +320,7 @@ interface State<T> {

type Action =
| { type: 'INIT'; payload: { columns: ColumnWithFilter<any>[]; data: any[]; ignoreRowInSetFilter?: (row: any) => boolean } }
| { type: 'ADD_MODIFIED_VALUE_TO_SET_FILTER'; payload: { rows: any[]; data: RowsChangeData<any> } }
| { type: 'UPDATE_FILTER'; payload: { column: string; filter: DataTableFilter } };

// Reducer is used to limit the number of re-renders because of dependent state
Expand Down Expand Up @@ -376,16 +383,83 @@ function reducer<T>(state: State<T>, action: Action): State<T> {
filterSetValues,
};
}
case 'UPDATE_FILTER': {
const { column, filter } = action.payload;
case 'ADD_MODIFIED_VALUE_TO_SET_FILTER': {
const {
data: { column, indexes },
rows,
} = action.payload;
if (!state.filters[column.key]) {
return state;
}
const newValues = indexes.map((index) => {
const value = rows[index][column.key];
if (value === '' || value === null) {
return EMPTY_FIELD;
}
return value;
});
// NOTE: we don't have access to every record here, so we just add the values and don't worry about removing on subsequent record change
// Calculate new list of available values
const filterSetValues = {
...state.filterSetValues,
[column.key]: Array.from(new Set([...state.filterSetValues[column.key], ...newValues])),
};
// ensure that current values are included in the set filter so they are retained on the page while editing
const columnFilter = [...state.filters[column.key]].map((item) => {
if (item.type !== 'SET') {
return item;
}
return {
...item,
value: Array.from(new Set(item.value.concat(newValues))),
};
});
return {
...state,
hasFilters: true,
filterSetValues,
filters: {
...state.filters,
[column]: state.filters[column].map((currFilter) => (currFilter.type === filter.type ? filter : currFilter)),
[column.key]: columnFilter,
},
};
}
case 'UPDATE_FILTER': {
const { column, filter } = action.payload;
const filters = {
...state.filters,
[column]: state.filters[column].map((currFilter) => (currFilter.type === filter.type ? filter : currFilter)),
};
const hasFilters = hasFilterApplied(filters, state.filterSetValues);
return {
...state,
hasFilters,
filters,
};
}
}
}

function hasFilterApplied(filters: Record<string, DataTableFilter[]>, filterSetValues: Record<string, string[]>) {
return Object.entries(filters).some(([key, columnFilters]) =>
columnFilters.some((filter) => {
let applied = false;
switch (filter.type) {
case 'SET':
applied = filter.value.length < (filterSetValues[key]?.length || 0);
break;
case 'BOOLEAN_SET':
applied = filter.value.length < 2; // true/false
break;
case 'DATE':
case 'NUMBER':
case 'TEXT':
case 'TIME':
applied = !!filter.value;
break;
default:
return false;
}
return applied;
})
);
}

0 comments on commit 8736121

Please sign in to comment.