Skip to content

Commit dcc8792

Browse files
authored
fix slow performance in import csv modal (#5980)
1 parent 002f74a commit dcc8792

File tree

3 files changed

+24
-105
lines changed

3 files changed

+24
-105
lines changed

packages/desktop-client/e2e/accounts.test.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,11 +123,14 @@ test.describe('Accounts', () => {
123123
const fileChooser = await fileChooserPromise;
124124
await fileChooser.setFiles(join(__dirname, 'data/test.csv'));
125125

126-
if (screenshot) await expect(page).toMatchThemeScreenshots();
127-
128126
const importButton = accountPage.page.getByRole('button', {
129127
name: /Import \d+ transactions/,
130128
});
129+
130+
await importButton.waitFor({ state: 'visible' });
131+
132+
if (screenshot) await expect(page).toMatchThemeScreenshots();
133+
131134
await importButton.click();
132135

133136
await expect(importButton).not.toBeVisible();
@@ -146,12 +149,14 @@ test.describe('Accounts', () => {
146149
const fileChooser = await fileChooserPromise;
147150
await fileChooser.setFiles(join(__dirname, 'data/test.csv'));
148151

149-
await expect(page).toMatchThemeScreenshots();
150-
151152
const importButton = accountPage.page.getByRole('button', {
152153
name: /Import \d+ transactions/,
153154
});
154155

156+
await importButton.waitFor({ state: 'visible' });
157+
158+
await expect(page).toMatchThemeScreenshots();
159+
155160
await expect(importButton).toBeDisabled();
156161
await expect(await importButton.innerText()).toMatch(
157162
/Import 0 transactions/,

packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.tsx

Lines changed: 9 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,6 @@ export function ImportTransactionsModal({
381381
return trans;
382382
});
383383

384-
setLoadingState(null);
385384
setError(null);
386385

387386
/// Do fine grained reporting between the old and new OFX importers.
@@ -391,13 +390,8 @@ export function ImportTransactionsModal({
391390
message: errors[0].message || 'Internal error',
392391
});
393392
} else {
394-
let flipAmount = false;
395-
let fieldMappings = null;
396-
let splitMode = false;
397-
let parseDateFormat: string | null = null;
398-
399393
if (filetype === 'csv' || filetype === 'qif') {
400-
flipAmount =
394+
const flipAmount =
401395
String(prefs[`flip-amount-${accountId}-${filetype}`]) === 'true';
402396
setFlipAmount(flipAmount);
403397
}
@@ -408,23 +402,22 @@ export function ImportTransactionsModal({
408402
? JSON.parse(mappings)
409403
: getInitialMappings(transactions);
410404

411-
fieldMappings = mappings;
412405
// @ts-expect-error - mappings might not have outflow/inflow properties
413406
setFieldMappings(mappings);
414407

415408
// Set initial split mode based on any saved mapping
416409
// @ts-expect-error - mappings might not have outflow/inflow properties
417-
splitMode = !!(mappings.outflow || mappings.inflow);
410+
const splitMode = !!(mappings.outflow || mappings.inflow);
418411
setSplitMode(splitMode);
419412

420-
parseDateFormat =
413+
const parseDateFormat =
421414
prefs[`parse-date-${accountId}-${filetype}`] ||
422415
getInitialDateFormat(transactions, mappings);
423416
setParseDateFormat(
424417
isDateFormat(parseDateFormat) ? parseDateFormat : null,
425418
);
426419
} else if (filetype === 'qif') {
427-
parseDateFormat =
420+
const parseDateFormat =
428421
prefs[`parse-date-${accountId}-${filetype}`] ||
429422
getInitialDateFormat(transactions, { date: 'date' });
430423
setParseDateFormat(
@@ -441,24 +434,12 @@ export function ImportTransactionsModal({
441434
const reversedTransactions =
442435
transactions.reverse() as ImportTransaction[];
443436
setParsedTransactions(reversedTransactions);
444-
445-
const transactionPreview = await getImportPreview(
446-
reversedTransactions,
447-
filetype,
448-
flipAmount,
449-
fieldMappings,
450-
splitMode,
451-
isDateFormat(parseDateFormat) ? parseDateFormat : null,
452-
inOutMode,
453-
outValue,
454-
multiplierAmount,
455-
);
456-
setTransactions(transactionPreview);
457437
}
438+
439+
setLoadingState(null);
458440
},
459441
// We use some state variables from the component, but do not want to re-parse when they change
460-
// eslint-disable-next-line react-hooks/exhaustive-deps
461-
[accountId, getImportPreview, prefs],
442+
[accountId, prefs],
462443
);
463444

464445
function onMultiplierChange(e) {
@@ -723,17 +704,6 @@ export function ImportTransactionsModal({
723704
}
724705

725706
const runImportPreview = useCallback(async () => {
726-
// preserve user's selection choices before re-running preview
727-
const selectionMap = new Map();
728-
transactions.forEach(trans => {
729-
if (!trans.isMatchedTransaction) {
730-
selectionMap.set(trans.trx_id, {
731-
selected: trans.selected,
732-
selected_merge: trans.selected_merge,
733-
});
734-
}
735-
});
736-
737707
// always start from the original parsed transactions, not the previewed ones to ensure rules run
738708
const transactionPreview = await getImportPreview(
739709
parsedTransactions,
@@ -746,23 +716,7 @@ export function ImportTransactionsModal({
746716
outValue,
747717
multiplierAmount,
748718
);
749-
750-
// restore selections to the new preview results
751-
const transactionPreviewWithSelections = transactionPreview.map(trans => {
752-
if (!trans.isMatchedTransaction && selectionMap.has(trans.trx_id)) {
753-
const saved = selectionMap.get(trans.trx_id);
754-
return {
755-
...trans,
756-
selected: saved.selected,
757-
selected_merge: saved.selected_merge,
758-
};
759-
}
760-
return trans;
761-
});
762-
763-
setTransactions(transactionPreviewWithSelections);
764-
// intentionally exclude transactions from dependencies to avoid infinite rerenders
765-
// eslint-disable-next-line react-hooks/exhaustive-deps
719+
setTransactions(transactionPreview);
766720
}, [
767721
getImportPreview,
768722
parsedTransactions,
@@ -781,9 +735,7 @@ export function ImportTransactionsModal({
781735
return;
782736
}
783737

784-
if (filetype === 'csv' || filetype === 'qif') {
785-
runImportPreview();
786-
}
738+
runImportPreview();
787739
// intentionally exclude runImportPreview from dependencies to avoid infinite rerenders
788740
// eslint-disable-next-line react-hooks/exhaustive-deps
789741
}, [
@@ -954,13 +906,6 @@ export function ImportTransactionsModal({
954906
checked={fallbackMissingPayeeToMemo}
955907
onChange={() => {
956908
setFallbackMissingPayeeToMemo(state => !state);
957-
parse(
958-
filename,
959-
getParseOptions('ofx', {
960-
fallbackMissingPayeeToMemo: !fallbackMissingPayeeToMemo,
961-
importNotes,
962-
}),
963-
);
964909
}}
965910
>
966911
<Trans>Use Memo as a fallback for empty Payees</Trans>
@@ -973,16 +918,6 @@ export function ImportTransactionsModal({
973918
checked={importNotes}
974919
onChange={() => {
975920
setImportNotes(!importNotes);
976-
parse(
977-
filename,
978-
getParseOptions(filetype, {
979-
delimiter,
980-
hasHeaderRow,
981-
skipLines,
982-
fallbackMissingPayeeToMemo,
983-
importNotes: !importNotes,
984-
}),
985-
);
986921
}}
987922
>
988923
<Trans>Import notes from file</Trans>
@@ -1047,15 +982,6 @@ export function ImportTransactionsModal({
1047982
value={delimiter}
1048983
onChange={value => {
1049984
setDelimiter(value);
1050-
parse(
1051-
filename,
1052-
getParseOptions('csv', {
1053-
delimiter: value,
1054-
hasHeaderRow,
1055-
skipLines,
1056-
importNotes,
1057-
}),
1058-
);
1059985
}}
1060986
style={{ width: 50 }}
1061987
/>
@@ -1075,15 +1001,6 @@ export function ImportTransactionsModal({
10751001
min="0"
10761002
onChangeValue={value => {
10771003
setSkipLines(+value);
1078-
parse(
1079-
filename,
1080-
getParseOptions('csv', {
1081-
delimiter,
1082-
hasHeaderRow,
1083-
skipLines: +value,
1084-
importNotes,
1085-
}),
1086-
);
10871004
}}
10881005
style={{ width: 50 }}
10891006
/>
@@ -1093,15 +1010,6 @@ export function ImportTransactionsModal({
10931010
checked={hasHeaderRow}
10941011
onChange={() => {
10951012
setHasHeaderRow(!hasHeaderRow);
1096-
parse(
1097-
filename,
1098-
getParseOptions('csv', {
1099-
delimiter,
1100-
hasHeaderRow: !hasHeaderRow,
1101-
skipLines,
1102-
importNotes,
1103-
}),
1104-
);
11051013
}}
11061014
>
11071015
<Trans>File has header row</Trans>

upcoming-release-notes/5980.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
category: Bugfix
3+
authors: [matt-fidd]
4+
---
5+
6+
Fix slow performance in import csv modal

0 commit comments

Comments
 (0)