From 0ebeb8e1f7be9444506eac54d086961ed68b8a9b Mon Sep 17 00:00:00 2001
From: Robert Goldmann <deadlocker@gmx.de>
Date: Wed, 28 Aug 2019 20:19:24 +0200
Subject: [PATCH] Fixed #484 - import adds transactions to wrong accounts

---
 .../budgetmaster/services/ImportService.java  |  47 ++++--
 .../unit/database/DatabaseImportTest.java     | 152 +++++++++++++++++-
 2 files changed, 180 insertions(+), 19 deletions(-)

diff --git a/src/main/java/de/deadlocker8/budgetmaster/services/ImportService.java b/src/main/java/de/deadlocker8/budgetmaster/services/ImportService.java
index 50a119234..09f0a8bc8 100644
--- a/src/main/java/de/deadlocker8/budgetmaster/services/ImportService.java
+++ b/src/main/java/de/deadlocker8/budgetmaster/services/ImportService.java
@@ -1,5 +1,6 @@
 package de.deadlocker8.budgetmaster.services;
 
+import de.deadlocker8.budgetmaster.accounts.Account;
 import de.deadlocker8.budgetmaster.database.Database;
 import de.deadlocker8.budgetmaster.database.accountmatches.AccountMatch;
 import de.deadlocker8.budgetmaster.database.accountmatches.AccountMatchList;
@@ -29,6 +30,8 @@ public class ImportService
 	@Autowired
 	private TagRepository tagRepository;
 
+	private Database database;
+
 	@Autowired
 	public ImportService(CategoryRepository categoryRepository, TransactionRepository transactionRepository, TagRepository tagRepository)
 	{
@@ -39,14 +42,20 @@ public class ImportService
 
 	public void importDatabase(Database database, AccountMatchList accountMatchList)
 	{
+		this.database = database;
 		LOGGER.debug("Importing database...");
-		importCategories(database);
-		importAccounts(database, accountMatchList);
-		importTransactions(database);
+		importCategories();
+		importAccounts(accountMatchList);
+		importTransactions();
 		LOGGER.debug("Importing database DONE");
 	}
 
-	private void importCategories(Database database)
+	public Database getDatabase()
+	{
+		return database;
+	}
+
+	private void importCategories()
 	{
 		List<Category> categories = database.getCategories();
 		LOGGER.debug("Importing " + categories.size() + " categories...");
@@ -112,7 +121,7 @@ public class ImportService
 		return updatedTransactions;
 	}
 
-	private void importAccounts(Database database, AccountMatchList accountMatchList)
+	private void importAccounts(AccountMatchList accountMatchList)
 	{
 		LOGGER.debug("Importing " + accountMatchList.getAccountMatches().size() + " accounts...");
 		List<Transaction> alreadyUpdatedTransactions = new ArrayList<>();
@@ -127,13 +136,14 @@ public class ImportService
 			List<Transaction> transferTransactions = new ArrayList<>(database.getTransactions());
 			transferTransactions.removeAll(alreadyUpdatedTransferTransactions);
 
-			alreadyUpdatedTransactions.addAll(updateAccountsForTransactions(transactions, accountMatch.getAccountSource().getID(), accountMatch.getAccountDestination().getID()));
-			alreadyUpdatedTransferTransactions.addAll(updateTransferAccountsForTransactions(transferTransactions, accountMatch.getAccountSource().getID(), accountMatch.getAccountDestination().getID()));
+			alreadyUpdatedTransactions.addAll(updateAccountsForTransactions(transactions, accountMatch.getAccountSource().getID(), accountMatch.getAccountDestination()));
+			alreadyUpdatedTransferTransactions.addAll(updateTransferAccountsForTransactions(transferTransactions, accountMatch.getAccountSource().getID(), accountMatch.getAccountDestination()));
 		}
+
 		LOGGER.debug("Importing accounts DONE");
 	}
 
-	private List<Transaction> updateAccountsForTransactions(List<Transaction> transactions, int oldAccountID, int newAccountID)
+	public List<Transaction> updateAccountsForTransactions(List<Transaction> transactions, int oldAccountID, Account newAccount)
 	{
 		List<Transaction> updatedTransactions = new ArrayList<>();
 		for(Transaction transaction : transactions)
@@ -141,27 +151,32 @@ public class ImportService
 			// legacy database
 			if(oldAccountID == -1)
 			{
-				transaction.getAccount().setID(newAccountID);
+				transaction.setAccount(newAccount);
 				updatedTransactions.add(transaction);
+				continue;
 			}
-			else if(transaction.getAccount().getID() == oldAccountID)
+
+			// account needs to be updated
+			if(transaction.getAccount().getID() != oldAccountID)
 			{
-				transaction.getAccount().setID(newAccountID);
-				updatedTransactions.add(transaction);
+				continue;
 			}
+
+			transaction.setAccount(newAccount);
+			updatedTransactions.add(transaction);
 		}
 
 		return updatedTransactions;
 	}
 
-	private List<Transaction> updateTransferAccountsForTransactions(List<Transaction> transactions, int oldAccountID, int newAccountID)
+	public List<Transaction> updateTransferAccountsForTransactions(List<Transaction> transactions, int oldAccountID, Account newAccount)
 	{
 		List<Transaction> updatedTransactions = new ArrayList<>();
 		for(Transaction transaction : transactions)
 		{
 			if(transaction.getTransferAccount() != null && transaction.getTransferAccount().getID() == oldAccountID)
 			{
-				transaction.getTransferAccount().setID(newAccountID);
+				transaction.setTransferAccount(newAccount);
 				updatedTransactions.add(transaction);
 			}
 		}
@@ -169,14 +184,14 @@ public class ImportService
 		return updatedTransactions;
 	}
 
-	private void importTransactions(Database database)
+	private void importTransactions()
 	{
 		List<Transaction> transactions = database.getTransactions();
 		LOGGER.debug("Importing " + transactions.size() + " transactions...");
 		for(int i = 0; i < transactions.size(); i++)
 		{
 			Transaction transaction = transactions.get(i);
-			LOGGER.debug("Importing transaction " + (i+1) + "/" + transactions.size() + " (name: " + transaction.getName() + ", date: " + transaction.getDate() + ")");
+			LOGGER.debug("Importing transaction " + (i + 1) + "/" + transactions.size() + " (name: " + transaction.getName() + ", date: " + transaction.getDate() + ")");
 			updateTagsForTransaction(transaction);
 			transaction.setID(null);
 			transactionRepository.save(transaction);
diff --git a/src/test/java/de/deadlocker8/budgetmaster/unit/database/DatabaseImportTest.java b/src/test/java/de/deadlocker8/budgetmaster/unit/database/DatabaseImportTest.java
index 459f7df76..681a449b5 100644
--- a/src/test/java/de/deadlocker8/budgetmaster/unit/database/DatabaseImportTest.java
+++ b/src/test/java/de/deadlocker8/budgetmaster/unit/database/DatabaseImportTest.java
@@ -3,13 +3,16 @@ package de.deadlocker8.budgetmaster.unit.database;
 import de.deadlocker8.budgetmaster.accounts.Account;
 import de.deadlocker8.budgetmaster.accounts.AccountType;
 import de.deadlocker8.budgetmaster.categories.Category;
+import de.deadlocker8.budgetmaster.categories.CategoryRepository;
 import de.deadlocker8.budgetmaster.categories.CategoryType;
+import de.deadlocker8.budgetmaster.database.Database;
+import de.deadlocker8.budgetmaster.database.accountmatches.AccountMatch;
+import de.deadlocker8.budgetmaster.database.accountmatches.AccountMatchList;
+import de.deadlocker8.budgetmaster.services.ImportService;
 import de.deadlocker8.budgetmaster.tags.Tag;
-import de.deadlocker8.budgetmaster.transactions.Transaction;
-import de.deadlocker8.budgetmaster.categories.CategoryRepository;
 import de.deadlocker8.budgetmaster.tags.TagRepository;
+import de.deadlocker8.budgetmaster.transactions.Transaction;
 import de.deadlocker8.budgetmaster.transactions.TransactionRepository;
-import de.deadlocker8.budgetmaster.services.ImportService;
 import org.joda.time.DateTime;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -680,4 +683,147 @@ public class DatabaseImportTest
 		assertEquals(1, transactionList.size());
 		assertEquals(transaction2, transactionList.get(0));
 	}
+
+	@Test
+	public void test_updateAccountsForTransactions()
+	{
+		Account account1 = new Account("Account_1", AccountType.CUSTOM);
+		account1.setID(2);
+		Account account2 = new Account("Account_2", AccountType.CUSTOM);
+		account2.setID(3);
+
+		Account destinationAccount = new Account("DestinationAccount_1", AccountType.CUSTOM);
+		destinationAccount.setID(5);
+
+		List<Transaction> transactionList = new ArrayList<>();
+		Transaction transaction1 = new Transaction();
+		transaction1.setAccount(account1);
+		transaction1.setName("ShouldGoInAccount_1");
+		transaction1.setAmount(200);
+		transaction1.setDate(new DateTime(2018, 10, 3, 12, 0, 0, 0));
+		transactionList.add(transaction1);
+
+		Transaction transaction2 = new Transaction();
+		transaction2.setAccount(account2);
+		transaction2.setName("ImPartOfAccount_2");
+		transaction2.setAmount(-525);
+		transaction2.setDate(new DateTime(2018, 10, 3, 12, 0, 0, 0));
+		transactionList.add(transaction2);
+
+		List<Transaction> updatedTransactions = importService.updateAccountsForTransactions(transactionList, account1.getID(), destinationAccount);
+		assertEquals(1, updatedTransactions.size());
+		assertEquals(new Integer(5), updatedTransactions.get(0).getAccount().getID());
+	}
+
+	@Test
+	public void test_updateTransferAccountsForTransactions()
+	{
+		Account account1 = new Account("Account_1", AccountType.CUSTOM);
+		account1.setID(2);
+		Account transferAccount = new Account("TransferAccount", AccountType.CUSTOM);
+		transferAccount.setID(3);
+
+		Account destinationAccount = new Account("DestinationAccount_1", AccountType.CUSTOM);
+		destinationAccount.setID(5);
+
+		List<Transaction> transactionList = new ArrayList<>();
+		Transaction transaction = new Transaction();
+		transaction.setAccount(account1);
+		transaction.setTransferAccount(transferAccount);
+		transaction.setName("Whatever");
+		transaction.setAmount(-525);
+		transaction.setDate(new DateTime(2018, 10, 3, 12, 0, 0, 0));
+		transactionList.add(transaction);
+
+		// expected
+		Transaction expectedTransaction = new Transaction();
+		expectedTransaction.setAccount(account1);
+		expectedTransaction.setTransferAccount(destinationAccount);
+		expectedTransaction.setName("Whatever");
+		expectedTransaction.setAmount(-525);
+		expectedTransaction.setDate(new DateTime(2018, 10, 3, 12, 0, 0, 0));
+
+		List<Transaction> updatedTransactions = importService.updateTransferAccountsForTransactions(transactionList, transferAccount.getID(), destinationAccount);
+		assertEquals(1, updatedTransactions.size());
+		assertEquals(expectedTransaction, updatedTransactions.get(0));
+	}
+
+	@Test
+	public void test_importFullDatabase()
+	{
+		// source accounts
+		Account sourceAccount1 = new Account("Source_Account_1", AccountType.CUSTOM);
+		sourceAccount1.setID(2);
+		Account sourceAccount2 = new Account("Source_Account_2", AccountType.CUSTOM);
+		sourceAccount2.setID(3);
+
+		List<Account> accounts = new ArrayList<>();
+		accounts.add(sourceAccount1);
+		accounts.add(sourceAccount2);
+
+		// destination accounts
+		Account destAccount1 = new Account("Destination_Account_1", AccountType.CUSTOM);
+		destAccount1.setID(5);
+		Account destAccount2 = new Account("Destination_Account_2", AccountType.CUSTOM);
+		destAccount2.setID(2);
+
+		// transactions
+		List<Transaction> transactions = new ArrayList<>();
+		Transaction transaction1 = new Transaction();
+		transaction1.setAccount(sourceAccount1);
+		transaction1.setName("ShouldGoInAccount_1");
+		transaction1.setAmount(200);
+		transaction1.setDate(new DateTime(2018, 10, 3, 12, 0, 0, 0));
+		transaction1.setTags(new ArrayList<>());
+		transactions.add(transaction1);
+
+		Transaction transaction2 = new Transaction();
+		transaction2.setAccount(sourceAccount2);
+		transaction2.setName("ImPartOfAccount_2");
+		transaction2.setAmount(-525);
+		transaction2.setDate(new DateTime(2018, 10, 3, 12, 0, 0, 0));
+		transaction2.setTags(new ArrayList<>());
+		transactions.add(transaction2);
+
+		// database
+		Database database = new Database(new ArrayList<>(), accounts, transactions);
+
+		// account matches
+		AccountMatch match1 = new AccountMatch(sourceAccount1);
+		match1.setAccountDestination(destAccount1);
+
+		AccountMatch match2 = new AccountMatch(sourceAccount2);
+		match2.setAccountDestination(destAccount2);
+
+		List<AccountMatch> matches = new ArrayList<>();
+		matches.add(match1);
+		matches.add(match2);
+
+		AccountMatchList accountMatchList = new AccountMatchList(matches);
+
+		// expected
+		Transaction expectedTransaction1 = new Transaction();
+		expectedTransaction1.setAccount(destAccount1);
+		expectedTransaction1.setName("ShouldGoInAccount_1");
+		expectedTransaction1.setAmount(200);
+		expectedTransaction1.setDate(new DateTime(2018, 10, 3, 12, 0, 0, 0));
+		expectedTransaction1.setTags(new ArrayList<>());
+
+		Transaction expectedTransaction2 = new Transaction();
+		expectedTransaction2.setAccount(destAccount2);
+		expectedTransaction2.setName("ImPartOfAccount_2");
+		expectedTransaction2.setAmount(-525);
+		expectedTransaction2.setDate(new DateTime(2018, 10, 3, 12, 0, 0, 0));
+		expectedTransaction2.setTags(new ArrayList<>());
+
+		// act
+		importService.importDatabase(database, accountMatchList);
+		Database databaseResult = importService.getDatabase();
+
+		// assert
+		List<Transaction> resultTransactions = databaseResult.getTransactions();
+		assertEquals(2, resultTransactions.size());
+		assertEquals(expectedTransaction1, resultTransactions.get(0));
+		assertEquals(expectedTransaction2, resultTransactions.get(1));
+	}
 }
\ No newline at end of file
-- 
GitLab