From 6ec372828090d1526bc33ae207bc08b178e36142 Mon Sep 17 00:00:00 2001
From: Wizmaster <code@wizmaster.fr>
Date: Wed, 24 Apr 2024 03:16:33 +0200
Subject: [PATCH] Fix reconciling split translations from nYNAB import creates
orphan transfers (#2502)
* Reconciling split translations from nYNAB import creates orphan transfers
- Fixed link between subtransaction transfers and transaction transfers
- Reworked payee transaction and subtransaction import
* Reconciling split translations from nYNAB import creates orphan transfers
- Added release note
* Reconciling split translations from nYNAB import creates orphan transfers
- Reworkd data.transactions and data.subtransactions loops
* Reconciling split translations from nYNAB import creates orphan transfers
- Added comments
---------
Co-authored-by: DJ Mountney <david@twkie.net>
---
.../e2e/data/ynab5-demo-budget.json | 6 +-
.../src/server/importers/ynab5-types.d.ts | 1 +
.../loot-core/src/server/importers/ynab5.ts | 204 +++++++++++++++---
upcoming-release-notes/2502.md | 6 +
4 files changed, 188 insertions(+), 29 deletions(-)
create mode 100644 upcoming-release-notes/2502.md
diff --git a/packages/desktop-client/e2e/data/ynab5-demo-budget.json b/packages/desktop-client/e2e/data/ynab5-demo-budget.json
index dfe791345..fd9993975 100644
--- a/packages/desktop-client/e2e/data/ynab5-demo-budget.json
+++ b/packages/desktop-client/e2e/data/ynab5-demo-budget.json
@@ -1704,8 +1704,8 @@
"payee_id": "",
"category_id": null,
"transfer_account_id": "bc1d862f-bab0-41c3-bd1e-6cee8c688e32",
- "transfer_transaction_id": "213526fc-ba49-4790-8a96-cc2a50182728",
- "matched_transaction_id": "",
+ "transfer_transaction_id": null,
+ "matched_transaction_id": null,
"import_id": null,
"import_payee_name": null,
"import_payee_name_original": null,
@@ -1729,7 +1729,7 @@
"transaction_id": "213526fc-ba49-4790-8a96-cc2a50182728",
"amount": -50000,
"memo": "split part b",
- "payee_id": "2a20470a-634f-4efa-a7f6-f1c0b0bdda41",
+ "payee_id": "8d3017e0-2aa6-4fe2-b011-c53c9f147eb6",
"category_id": null,
"transfer_account_id": "125f339b-2a63-481e-84c0-f04d898905d2",
"deleted": false
diff --git a/packages/loot-core/src/server/importers/ynab5-types.d.ts b/packages/loot-core/src/server/importers/ynab5-types.d.ts
index c96d06a09..524ede77c 100644
--- a/packages/loot-core/src/server/importers/ynab5-types.d.ts
+++ b/packages/loot-core/src/server/importers/ynab5-types.d.ts
@@ -63,6 +63,7 @@ export namespace YNAB5 {
memo: string;
amount: number;
transfer_account_id: string;
+ payee_id: string;
}
interface Month {
diff --git a/packages/loot-core/src/server/importers/ynab5.ts b/packages/loot-core/src/server/importers/ynab5.ts
index 011b0d15b..4ef00137a 100644
--- a/packages/loot-core/src/server/importers/ynab5.ts
+++ b/packages/loot-core/src/server/importers/ynab5.ts
@@ -152,16 +152,157 @@ async function importTransactions(
const payeeTransferAcctHashMap = new Map<string, YNAB5.Payee>(
payeesByTransferAcct,
);
+ const orphanTransferMap = new Map<string, YNAB5.Transaction[]>();
+ const orphanSubtransfer = [] as YNAB5.Subtransaction[];
+ const orphanSubtransferTrxId = [] as string[];
+ const orphanSubtransferAcctIdByTrxIdMap = new Map<string, string>();
+ const orphanSubtransferDateByTrxIdMap = new Map<string, string>();
// Go ahead and generate ids for all of the transactions so we can
// reliably resolve transfers
- for (const transaction of data.transactions) {
+ // Also identify orphan transfer transactions and subtransactions.
+ for (const transaction of data.subtransactions) {
entityIdMap.set(transaction.id, uuidv4());
+
+ if (transaction.transfer_account_id) {
+ orphanSubtransfer.push(transaction);
+ orphanSubtransferTrxId.push(transaction.transaction_id);
+ }
}
- for (const transaction of data.subtransactions) {
+
+ for (const transaction of data.transactions) {
entityIdMap.set(transaction.id, uuidv4());
+
+ if (
+ transaction.transfer_account_id &&
+ !transaction.transfer_transaction_id
+ ) {
+ const key =
+ transaction.account_id + '#' + transaction.transfer_account_id;
+ if (!orphanTransferMap.has(key)) {
+ orphanTransferMap.set(key, [transaction]);
+ } else {
+ orphanTransferMap.get(key).push(transaction);
+ }
+ }
+
+ if (orphanSubtransferTrxId.includes(transaction.id)) {
+ orphanSubtransferAcctIdByTrxIdMap.set(
+ transaction.id,
+ transaction.account_id,
+ );
+ orphanSubtransferDateByTrxIdMap.set(transaction.id, transaction.date);
+ }
}
+ // Compute link between subtransaction transfers and orphaned transaction
+ // transfers. The goal is to match each transfer subtransaction to the related
+ // transfer transaction according to the accounts, date, amount and memo.
+ const orphanSubtransferMap = orphanSubtransfer.reduce(
+ (map, subtransaction) => {
+ const key =
+ subtransaction.transfer_account_id +
+ '#' +
+ orphanSubtransferAcctIdByTrxIdMap.get(subtransaction.transaction_id);
+ if (!map.has(key)) {
+ map.set(key, [subtransaction]);
+ } else {
+ map.get(key).push(subtransaction);
+ }
+ return map;
+ },
+ new Map<string, YNAB5.Subtransaction[]>(),
+ );
+
+ // The comparator will be used to order transfer transactions and their
+ // corresponding tranfer subtransaction in two aligned list. Hopefully
+ // for every list index in the transactions list, the related subtransaction
+ // will be at the same index.
+ const orphanTransferComparator = (
+ a: YNAB5.Transaction | YNAB5.Subtransaction,
+ b: YNAB5.Transaction | YNAB5.Subtransaction,
+ ) => {
+ // a and b can be a YNAB5.Transaction (having a date attribute) or a
+ // YNAB5.Subtransaction (missing that date attribute)
+
+ const date_a =
+ 'date' in a
+ ? a.date
+ : orphanSubtransferDateByTrxIdMap.get(a.transaction_id);
+ const date_b =
+ 'date' in b
+ ? b.date
+ : orphanSubtransferDateByTrxIdMap.get(b.transaction_id);
+ // A transaction and the related subtransaction have inverted amounts.
+ // To have those in the same order, the subtransaction has to be reversed
+ // to have the same amount.
+ const amount_a = 'date' in a ? a.amount : -a.amount;
+ const amount_b = 'date' in b ? b.amount : -b.amount;
+
+ // Transaction are ordered first by date, then by amount, and lastly by memo
+ if (date_a > date_b) return 1;
+ if (date_a < date_b) return -1;
+ if (amount_a > amount_b) return 1;
+ if (amount_a < amount_b) return -1;
+ if (a.memo > b.memo) return 1;
+ if (a.memo < b.memo) return -1;
+ return 0;
+ };
+
+ const orphanTrxIdSubtrxIdMap = new Map<string, string>();
+ orphanTransferMap.forEach((transactions, key) => {
+ const subtransactions = orphanSubtransferMap.get(key);
+ if (subtransactions) {
+ transactions.sort(orphanTransferComparator);
+ subtransactions.sort(orphanTransferComparator);
+
+ // Iterate on the two sorted lists transactions and subtransactions and
+ // find matching data to identify the related transaction ids.
+ let transactionIdx = 0;
+ let subtransactionIdx = 0;
+ do {
+ switch (
+ orphanTransferComparator(
+ transactions[transactionIdx],
+ subtransactions[subtransactionIdx],
+ )
+ ) {
+ case 0:
+ // The current list indexes are matching: the transaction and
+ // subtransaction are related (same date, amount and memo)
+ orphanTrxIdSubtrxIdMap.set(
+ transactions[transactionIdx].id,
+ entityIdMap.get(subtransactions[subtransactionIdx].id),
+ );
+ orphanTrxIdSubtrxIdMap.set(
+ subtransactions[subtransactionIdx].id,
+ entityIdMap.get(transactions[transactionIdx].id),
+ );
+ transactionIdx++;
+ subtransactionIdx++;
+ break;
+ case -1:
+ // The current list indexes are not matching:
+ // The current transaction is "smaller" than the current subtransaction
+ // (earlier date, smaller amount, memo value sorted before)
+ // So we advance to the next transaction and see if it match with
+ // the current subtransaction
+ transactionIdx++;
+ break;
+ case 1:
+ // Inverse of the previous case:
+ // The current subtransaction is "smaller" than the current transaction
+ // So we advance to the next subtransaction
+ subtransactionIdx++;
+ break;
+ }
+ } while (
+ transactionIdx < transactions.length &&
+ subtransactionIdx < subtransactions.length
+ );
+ }
+ });
+
await Promise.all(
[...transactionsGrouped.keys()].map(async accountId => {
const transactions = transactionsGrouped.get(accountId);
@@ -186,25 +327,20 @@ async function importTransactions(
notes: transaction.memo || null,
imported_id: transaction.import_id || null,
transfer_id:
- entityIdMap.get(transaction.transfer_transaction_id) || null,
+ entityIdMap.get(transaction.transfer_transaction_id) ||
+ orphanTrxIdSubtrxIdMap.get(transaction.id) ||
+ null,
subtransactions: subtransactions
? subtransactions.map(subtrans => {
- let payee = null;
- if (subtrans.transfer_account_id) {
- const mappedTransferAccountId = entityIdMap.get(
- subtrans.transfer_account_id,
- );
- payee = payeeTransferAcctHashMap.get(
- mappedTransferAccountId,
- )?.id;
- }
-
return {
id: entityIdMap.get(subtrans.id),
amount: amountFromYnab(subtrans.amount),
category: entityIdMap.get(subtrans.category_id) || null,
notes: subtrans.memo,
- payee,
+ transfer_id:
+ orphanTrxIdSubtrxIdMap.get(subtrans.id) || null,
+ payee: null,
+ imported_payee: null,
};
})
: null,
@@ -212,18 +348,34 @@ async function importTransactions(
imported_payee: null,
};
- // Handle transfer payee
- if (transaction.transfer_account_id) {
- newTransaction.payee = payees.find(
- p =>
- p.transfer_acct ===
- entityIdMap.get(transaction.transfer_account_id),
- ).id;
- } else {
- newTransaction.payee = entityIdMap.get(transaction.payee_id);
- newTransaction.imported_payee = data.payees.find(
- p => !p.deleted && p.id === transaction.payee_id,
- )?.name;
+ // Handle transactions and subtransactions payee
+ const transactionPayeeUpdate = (
+ trx: YNAB5.Transaction | YNAB5.Subtransaction,
+ newTrx,
+ ) => {
+ if (trx.transfer_account_id) {
+ const mappedTransferAccountId = entityIdMap.get(
+ trx.transfer_account_id,
+ );
+ newTrx.payee = payeeTransferAcctHashMap.get(
+ mappedTransferAccountId,
+ )?.id;
+ } else {
+ newTrx.payee = entityIdMap.get(trx.payee_id);
+ newTrx.imported_payee = data.payees.find(
+ p => !p.deleted && p.id === trx.payee_id,
+ )?.name;
+ }
+ };
+
+ transactionPayeeUpdate(transaction, newTransaction);
+ if (newTransaction.subtransactions) {
+ subtransactions.forEach(subtrans => {
+ const newSubtransaction = newTransaction.subtransactions.find(
+ newSubtrans => newSubtrans.id === entityIdMap.get(subtrans.id),
+ );
+ transactionPayeeUpdate(subtrans, newSubtransaction);
+ });
}
// Handle starting balances
diff --git a/upcoming-release-notes/2502.md b/upcoming-release-notes/2502.md
new file mode 100644
index 000000000..3be520069
--- /dev/null
+++ b/upcoming-release-notes/2502.md
@@ -0,0 +1,6 @@
+---
+category: Bugfix
+authors: [Wizmaster]
+---
+
+Fix reconciling split translations from nYNAB import creates orphan transfers
--
GitLab