From e625c4f5cb7e5df2fd71c9df60ca3adf99cc3512 Mon Sep 17 00:00:00 2001
From: Robert Goldmann <deadlocker@gmx.de>
Date: Wed, 23 Feb 2022 22:54:09 +0100
Subject: [PATCH] Fixed #682 - filter: transactions without tags are filtered
 if one tag is de-selected

---
 .../TransactionSpecifications.java            |  6 ++-
 .../unit/TransactionSpecificationsTest.java   | 50 ++++++++++++++-----
 2 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/src/main/java/de/deadlocker8/budgetmaster/transactions/TransactionSpecifications.java b/src/main/java/de/deadlocker8/budgetmaster/transactions/TransactionSpecifications.java
index 47a0ba632..3b2931cc8 100644
--- a/src/main/java/de/deadlocker8/budgetmaster/transactions/TransactionSpecifications.java
+++ b/src/main/java/de/deadlocker8/budgetmaster/transactions/TransactionSpecifications.java
@@ -7,6 +7,7 @@ import org.springframework.data.jpa.domain.Specification;
 
 import javax.persistence.criteria.CriteriaBuilder;
 import javax.persistence.criteria.Join;
+import javax.persistence.criteria.JoinType;
 import javax.persistence.criteria.Predicate;
 import java.time.LocalDate;
 import java.util.ArrayList;
@@ -99,13 +100,16 @@ public class TransactionSpecifications
 
 			if(!tagIDs.isEmpty())
 			{
-				Join<Transaction, Tag> join = transaction.join(Transaction_.tags);
+				Join<Transaction, Tag> join = transaction.join(Transaction_.tags, JoinType.LEFT);
 				Predicate tagPredicate = builder.disjunction();
 				for(Integer tagID : tagIDs)
 				{
 					tagPredicate.getExpressions().add(builder.equal(join.get(Tag_.ID), tagID));
 				}
 
+				// transactions without any tags should be included in results
+				tagPredicate.getExpressions().add(builder.isEmpty(transaction.get(Transaction_.tags)));
+
 				predicates.add(tagPredicate);
 				transferPredicates.add(tagPredicate);
 			}
diff --git a/src/test/java/de/deadlocker8/budgetmaster/unit/TransactionSpecificationsTest.java b/src/test/java/de/deadlocker8/budgetmaster/unit/TransactionSpecificationsTest.java
index d05b137c5..65e5c8871 100644
--- a/src/test/java/de/deadlocker8/budgetmaster/unit/TransactionSpecificationsTest.java
+++ b/src/test/java/de/deadlocker8/budgetmaster/unit/TransactionSpecificationsTest.java
@@ -38,6 +38,7 @@ class TransactionSpecificationsTest
 	private TransactionRepository transactionRepository;
 	private Transaction transaction1;
 	private Transaction transaction2;
+	private Transaction transaction3;
 	private Transaction repeatingTransaction;
 	private Transaction transferTransaction;
 	private Transaction transferTransactionWrongAccount;
@@ -102,6 +103,14 @@ class TransactionSpecificationsTest
 		transaction2.setAccount(account);
 		transaction2 = transactionRepository.save(transaction2);
 
+		transaction3 = new Transaction();
+		transaction3.setName("Income without tags");
+		transaction3.setAmount(100);
+		transaction3.setDate(LocalDate.of(2018, 12, 3));
+		transaction3.setCategory(category2);
+		transaction3.setAccount(account);
+		transaction3 = transactionRepository.save(transaction3);
+
 		LocalDate repeatingTransactionDate = LocalDate.of(2018, 3, 13);
 		repeatingOption = new RepeatingOption();
 		repeatingOption.setModifier(new RepeatingModifierDays(10));
@@ -154,9 +163,10 @@ class TransactionSpecificationsTest
 		Specification spec = TransactionSpecifications.withDynamicQuery(startDate, LocalDate.now(), account, true, true, true, null, List.of(), List.of(), null);
 
 		List<Transaction> results = transactionRepository.findAll(spec);
-		assertThat(results).hasSize(4)
+		assertThat(results).hasSize(5)
 				.contains(transaction1)
 				.contains(transaction2)
+				.contains(transaction3)
 				.contains(repeatingTransaction)
 				.contains(transferTransaction);
 	}
@@ -167,9 +177,10 @@ class TransactionSpecificationsTest
 		Specification spec = TransactionSpecifications.withDynamicQuery(startDate, LocalDate.now(), account, true, true, false, null, List.of(), List.of(), null);
 
 		List<Transaction> results = transactionRepository.findAll(spec);
-		assertThat(results).hasSize(3)
+		assertThat(results).hasSize(4)
 				.contains(transaction1)
 				.contains(transaction2)
+				.contains(transaction3)
 				.contains(repeatingTransaction);
 	}
 
@@ -179,8 +190,9 @@ class TransactionSpecificationsTest
 		Specification spec = TransactionSpecifications.withDynamicQuery(startDate, LocalDate.now(), account, true, false, false, null, List.of(), List.of(), null);
 
 		List<Transaction> results = transactionRepository.findAll(spec);
-		assertThat(results).hasSize(1)
-				.contains(transaction1);
+		assertThat(results).hasSize(2)
+				.contains(transaction1)
+				.contains(transaction3);
 	}
 
 	@Test
@@ -210,9 +222,10 @@ class TransactionSpecificationsTest
 		Specification spec = TransactionSpecifications.withDynamicQuery(startDate, LocalDate.now(), account, false, false, false, null, List.of(), List.of(), null);
 
 		List<Transaction> results = transactionRepository.findAll(spec);
-		assertThat(results).hasSize(3)
+		assertThat(results).hasSize(4)
 				.contains(transaction1)
 				.contains(transaction2)
+				.contains(transaction3)
 				.contains(repeatingTransaction);
 	}
 
@@ -222,8 +235,9 @@ class TransactionSpecificationsTest
 		Specification spec = TransactionSpecifications.withDynamicQuery(startDate, LocalDate.now(), account, true, false, true, null, List.of(), List.of(), null);
 
 		List<Transaction> results = transactionRepository.findAll(spec);
-		assertThat(results).hasSize(1)
-				.contains(transaction1);
+		assertThat(results).hasSize(2)
+				.contains(transaction1)
+				.contains(transaction3);
 	}
 
 	@Test
@@ -283,9 +297,10 @@ class TransactionSpecificationsTest
 		Specification spec = TransactionSpecifications.withDynamicQuery(startDate, LocalDate.now(), account, true, true, true, false, List.of(), List.of(), null);
 
 		List<Transaction> results = transactionRepository.findAll(spec);
-		assertThat(results).hasSize(3)
+		assertThat(results).hasSize(4)
 				.contains(transaction1)
 				.contains(transaction2)
+				.contains(transaction3)
 				.contains(transferTransaction);
 	}
 
@@ -352,8 +367,11 @@ class TransactionSpecificationsTest
 		Specification spec = TransactionSpecifications.withDynamicQuery(startDate, LocalDate.now(), account, true, true, true, null, List.of(), tagIDs, null);
 
 		List<Transaction> results = transactionRepository.findAll(spec);
-		assertThat(results).hasSize(1)
-				.contains(transaction1);
+		assertThat(results).hasSize(4)
+				.contains(transaction1)
+				.contains(transaction2)
+				.contains(transaction3)
+				.contains(transferTransaction);
 	}
 
 	@Test
@@ -366,9 +384,12 @@ class TransactionSpecificationsTest
 		Specification spec = TransactionSpecifications.withDynamicQuery(startDate, LocalDate.now(), account, true, true, true, null, List.of(), tagIDs, null);
 
 		List<Transaction> results = transactionRepository.findAll(spec);
-		assertThat(results).hasSize(2)
+		assertThat(results).hasSize(5)
 				.contains(transaction1)
-				.contains(repeatingTransaction);
+				.contains(transaction2)
+				.contains(transaction3)
+				.contains(repeatingTransaction)
+				.contains(transferTransaction);
 	}
 
 
@@ -381,7 +402,10 @@ class TransactionSpecificationsTest
 		Specification spec = TransactionSpecifications.withDynamicQuery(startDate, LocalDate.now(), account, true, true, true, null, List.of(), tagIDs, null);
 
 		List<Transaction> results = transactionRepository.findAll(spec);
-		assertThat(results).isEmpty();
+		assertThat(results).hasSize(3)
+			.contains(transaction2)
+			.contains(transaction3)
+			.contains(transferTransaction);
 	}
 
 	@Test
-- 
GitLab