Introduce 'deleted' flag to RecurringTransaction

Hard-deleting recurring transaction cannot be done reliable because a transaction may have a foreign key reference to a recurring transaction of this transaction has been created from a recurring transaction. Thus hard-deleting a recurring transaction may lead to data inconsistencies.
This commit is contained in:
2019-05-05 02:01:12 +02:00
parent 2d780267ef
commit 9b82084f21
10 changed files with 46 additions and 17 deletions

View File

@@ -12,5 +12,7 @@ import java.time.LocalDate;
public interface RecurringTransactionRepository extends CrudRepository<RecurringTransaction, Long> {
Iterable<RecurringTransaction> findRecurringTransactionsByFromAccountOrToAccount(Account fromAccount, Account toAccount);
Iterable<RecurringTransaction> findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(LocalDate lastOccurrence);
Iterable<RecurringTransaction> findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(LocalDate lastOccurrence);
Iterable<RecurringTransaction> findByDeletedFalse();
}

View File

@@ -20,6 +20,7 @@ public class RecurringTransaction {
private LocalDate lastOccurrence;
@Enumerated(EnumType.STRING)
private HolidayWeekendType holidayWeekendType;
private boolean deleted;
public Long getId() {
return id;
@@ -88,4 +89,12 @@ public class RecurringTransaction {
public void setIntervalType(IntervalType intervalType) {
this.intervalType = intervalType;
}
public boolean isDeleted() {
return deleted;
}
public void setDeleted(boolean deleted) {
this.deleted = deleted;
}
}

View File

@@ -44,12 +44,12 @@ public class RecurringTransactionService {
private TransactionService transactionService;
public Iterable<RecurringTransaction> getAll() {
return this.recurringTransactionRepository.findAll();
return this.recurringTransactionRepository.findByDeletedFalse();
}
public Iterable<RecurringTransaction> getAllActive() {
return this.recurringTransactionRepository
.findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(LocalDate.now());
.findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(LocalDate.now());
}
public Iterable<RecurringTransaction> getAllForAccount(String accountKey) {
@@ -79,7 +79,7 @@ public class RecurringTransactionService {
// Visible for unit tests
/* package */ Iterable<RecurringTransaction> getAllDueToday(LocalDate now) {
final Iterable<RecurringTransaction> allRecurringTransactions = this.recurringTransactionRepository
.findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(now);
.findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(now);
//@formatter:off
return IterableUtils.toList(allRecurringTransactions).stream()
@@ -411,7 +411,11 @@ public class RecurringTransactionService {
}
try {
this.recurringTransactionRepository.deleteById(Long.valueOf(recurringTransactionId));
RecurringTransaction recurringTransaction = optionalRecurringTransaction.get();
recurringTransaction.setDeleted(true);
this.recurringTransactionRepository.save(recurringTransaction);
} catch (Exception e) {
LOGGER.error("Could not delete recurring transaction!", e);

View File

@@ -24,6 +24,7 @@ CREATE TABLE recurring_transaction (
first_occurrence DATE NOT NULL,
last_occurrence DATE,
holiday_weekend_type VARCHAR(255) NOT NULL,
deleted BOOLEAN DEFAULT FALSE NOT NULL,
CONSTRAINT fk_recurring_transaction_from_account FOREIGN KEY (from_account_id) REFERENCES account (id),
CONSTRAINT fk_recurring_transaction_to_account FOREIGN KEY (to_account_id) REFERENCES account (id)

View File

@@ -24,6 +24,7 @@ CREATE TABLE recurring_transaction (
first_occurrence DATE NOT NULL,
last_occurrence DATE,
holiday_weekend_type VARCHAR(255) NOT NULL,
deleted BOOLEAN DEFAULT 'TRUE' NOT NULL,
CONSTRAINT fk_recurring_transaction_from_account FOREIGN KEY (from_account_id) REFERENCES account (id),
CONSTRAINT fk_recurring_transaction_to_account FOREIGN KEY (to_account_id) REFERENCES account (id)

View File

@@ -71,7 +71,7 @@ public class RecurringTransactionService_deleteRecurringTransactionTest {
public void test_deleteRecurringTransaction_UNKNOWN_ERROR() {
// Arrange
Mockito.when(this.recurringTransactionRepository.findById(Mockito.anyLong())).thenReturn(Optional.of(new RecurringTransaction()));
Mockito.doThrow(new NullPointerException()).when(this.recurringTransactionRepository).deleteById(Mockito.anyLong());
Mockito.doThrow(new NullPointerException()).when(this.recurringTransactionRepository).save(Mockito.any());
// Act
final ResponseReason response = this.classUnderTest.deleteRecurringTransaction("123");

View File

@@ -54,7 +54,7 @@ public class RecurringTransactionService_getAllDueToday_DAILY_NEXT_WORKDAYTest {
// Arrange
// Implicitly: ruleService.isHoliday().return(false) and ruleService.isWeekend().return(false)
Mockito.when(this.recurringTransactionRepository
.findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.thenReturn(Collections.singletonList(createRecurringTransaction(-3)));
final LocalDate now = LocalDate.now();
@@ -74,7 +74,7 @@ public class RecurringTransactionService_getAllDueToday_DAILY_NEXT_WORKDAYTest {
// Arrange
// Implicitly: ruleService.isWeekend().return(false)
Mockito.when(this.recurringTransactionRepository
.findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.thenReturn(Collections.singletonList(createRecurringTransaction(0)));
// Today is a holiday, but yesterday was not
Mockito.when(this.ruleService.isHoliday(Mockito.any())).thenReturn(Boolean.TRUE, Boolean.FALSE);
@@ -96,7 +96,7 @@ public class RecurringTransactionService_getAllDueToday_DAILY_NEXT_WORKDAYTest {
// Arrange
// Implicitly: ruleService.isHoliday().return(false)
Mockito.when(this.recurringTransactionRepository
.findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.thenReturn(Collections.singletonList(createRecurringTransaction(0)));
// Today is a weekend day, but yesterday was not
Mockito.when(this.ruleService.isWeekend(Mockito.any())).thenReturn(Boolean.TRUE, Boolean.FALSE);
@@ -118,7 +118,7 @@ public class RecurringTransactionService_getAllDueToday_DAILY_NEXT_WORKDAYTest {
// Arrange
// Implicitly: ruleService.isHoliday().return(false) and ruleService.isWeekend().return(false)
Mockito.when(this.recurringTransactionRepository
.findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.thenReturn(Collections.singletonList(createRecurringTransaction(1)));
final LocalDate now = LocalDate.now();
@@ -136,6 +136,7 @@ public class RecurringTransactionService_getAllDueToday_DAILY_NEXT_WORKDAYTest {
recurringTransaction.setHolidayWeekendType(HolidayWeekendType.NEXT_WORKDAY);
recurringTransaction.setIntervalType(IntervalType.DAILY);
recurringTransaction.setDeleted(false);
return recurringTransaction;
}

View File

@@ -7,6 +7,7 @@ import de.financer.model.RecurringTransaction;
import org.apache.commons.collections4.IterableUtils;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
@@ -43,7 +44,7 @@ public class RecurringTransactionService_getAllDueToday_MONTHLY_NEXT_WORKDAYTest
public void test_getAllDueToday_duePast_holiday() {
// Arrange
Mockito.when(this.recurringTransactionRepository
.findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.thenReturn(Collections.singletonList(createRecurringTransaction(-1)));
// Today is not a holiday but yesterday was
Mockito.when(this.ruleService.isHoliday(Mockito.any())).thenReturn(Boolean.FALSE, Boolean.TRUE);
@@ -81,7 +82,7 @@ public class RecurringTransactionService_getAllDueToday_MONTHLY_NEXT_WORKDAYTest
final LocalDate monday = now.minusDays(now.getDayOfWeek().getValue() - 1);
// The transaction occurs on a friday
Mockito.when(this.recurringTransactionRepository
.findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.thenReturn(Collections.singletonList(createRecurringTransaction(-(now.getDayOfWeek().getValue() + 2))));
// First False for the dueToday check, 2x True for actual weekend, second False for Friday
Mockito.when(this.ruleService.isWeekend(Mockito.any()))
@@ -103,14 +104,20 @@ public class RecurringTransactionService_getAllDueToday_MONTHLY_NEXT_WORKDAYTest
* (monday)
*/
@Test
@Ignore
// This test does not work as expected: if go back to the last sunday and then again one month back, we do
// not necessarily end up on on a date that causes the transaction to be due on monday
// e.g. 01.04.19 -> monday, 31.03.19 -> sunday, minus one month -> 28.02.19
// whereas the resulting 28.02.19 would be the first occurrence of the transaction. The next due dates would
// be 28.03.19 and 28.04.19 and not the 01.04.19 as expected
public void test_getAllDueToday_duePast_weekend_sunday() {
// Arrange
final LocalDate now = LocalDate.now();
final LocalDate monday = now.minusDays(now.getDayOfWeek().getValue() - 1);
// The transaction occurs on a sunday
Mockito.when(this.recurringTransactionRepository
.findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.thenReturn(Collections.singletonList(createRecurringTransaction(-now.getDayOfWeek().getValue())));
.findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.thenReturn(Collections.singletonList(createRecurringTransaction(-(now.getDayOfWeek().getValue()))));
// First False for the dueToday check, 2x True for actual weekend, second False for Friday
Mockito.when(this.ruleService.isWeekend(Mockito.any()))
.thenReturn(Boolean.FALSE, Boolean.TRUE, Boolean.TRUE, Boolean.FALSE);
@@ -128,13 +135,15 @@ public class RecurringTransactionService_getAllDueToday_MONTHLY_NEXT_WORKDAYTest
* today (monday)
*/
@Test
@Ignore
// Same as with the _sunday test -> does not work as expected
public void test_getAllDueToday_duePast_weekend_saturday() {
// Arrange
final LocalDate now = LocalDate.now();
final LocalDate monday = now.minusDays(now.getDayOfWeek().getValue() - 1);
// The transaction occurs on a saturday
Mockito.when(this.recurringTransactionRepository
.findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.thenReturn(Collections.singletonList(createRecurringTransaction(-(now.getDayOfWeek().getValue() + 1))));
// First False for the dueToday check, 2x True for actual weekend, second False for Friday
Mockito.when(this.ruleService.isWeekend(Mockito.any()))
@@ -154,6 +163,7 @@ public class RecurringTransactionService_getAllDueToday_MONTHLY_NEXT_WORKDAYTest
recurringTransaction.setHolidayWeekendType(HolidayWeekendType.NEXT_WORKDAY);
recurringTransaction.setIntervalType(IntervalType.MONTHLY);
recurringTransaction.setDeleted(false);
return recurringTransaction;
}

View File

@@ -43,7 +43,7 @@ public class RecurringTransactionService_getAllDueToday_MONTHLY_PREVIOUS_WORKDAY
public void test_getAllDueToday_dueFuture_holiday() {
// Arrange
Mockito.when(this.recurringTransactionRepository
.findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.thenReturn(Collections.singletonList(createRecurringTransaction(1)));
// Today is not a holiday but tomorrow is
Mockito.when(this.ruleService.isHoliday(Mockito.any())).thenReturn(Boolean.FALSE, Boolean.TRUE);

View File

@@ -43,7 +43,7 @@ public class RecurringTransactionService_getAllDueToday_MONTHLY_SAME_DAYTest {
public void test_getAllDueToday_duePast_holiday() {
// Arrange
Mockito.when(this.recurringTransactionRepository
.findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any()))
.thenReturn(Collections.singletonList(createRecurringTransaction(-1)));
// Today is not a holiday but yesterday was
Mockito.when(this.ruleService.isHoliday(Mockito.any())).thenReturn(Boolean.FALSE, Boolean.TRUE);
@@ -63,6 +63,7 @@ public class RecurringTransactionService_getAllDueToday_MONTHLY_SAME_DAYTest {
recurringTransaction.setHolidayWeekendType(HolidayWeekendType.SAME_DAY);
recurringTransaction.setIntervalType(IntervalType.MONTHLY);
recurringTransaction.setDeleted(false);
return recurringTransaction;
}