From b19cdd026408b321cc1f8f72613639ffab4e88b1 Mon Sep 17 00:00:00 2001 From: Cedric Hornberger Date: Mon, 6 Jul 2026 12:40:32 +0200 Subject: [PATCH] feat: record history and lastModified on expense mutations --- .../controller/ExpenseListController.java | 6 +- .../services/ExpenseListService.java | 41 +++++++++--- .../controller/ExpenseListControllerTest.java | 2 +- .../services/ExpenseListServiceTest.java | 67 +++++++++++++++++++ 4 files changed, 104 insertions(+), 12 deletions(-) diff --git a/src/main/java/de/zendric/app/xpensely_server/controller/ExpenseListController.java b/src/main/java/de/zendric/app/xpensely_server/controller/ExpenseListController.java index 03ce5a9..f107299 100644 --- a/src/main/java/de/zendric/app/xpensely_server/controller/ExpenseListController.java +++ b/src/main/java/de/zendric/app/xpensely_server/controller/ExpenseListController.java @@ -104,7 +104,7 @@ public class ExpenseListController { assertMember(user, listOpt.get()); AppUser expenseOwner = resolveListMember(listOpt.get(), expenseInput.getOwner()); Expense expense = expenseInput.convertToExpense(expenseOwner.getId()); - Expense addedExpense = expenseListService.addExpenseToList(expenseListId, expense); + Expense addedExpense = expenseListService.addExpenseToList(expenseListId, expense, user); return new ResponseEntity<>(addedExpense, HttpStatus.CREATED); } @@ -120,7 +120,7 @@ public class ExpenseListController { assertMember(user, expenseListOpt.get()); AppUser expenseOwner = resolveListMember(expenseListOpt.get(), expenseChangeRequest.getOwnerName()); Expense expense = expenseChangeRequest.convertToExpense(expenseOwner.getId(), expenseListOpt.get()); - Expense updatedExpense = expenseListService.updateExpense(expenseListId, expense); + Expense updatedExpense = expenseListService.updateExpense(expenseListId, expense, user); return new ResponseEntity<>(updatedExpense, HttpStatus.OK); } @@ -134,7 +134,7 @@ public class ExpenseListController { if (listOpt.isEmpty()) return new ResponseEntity<>(HttpStatus.NOT_FOUND); assertMember(user, listOpt.get()); - expenseListService.deleteExpenseFromList(expenseListId, expenseId); + expenseListService.deleteExpenseFromList(expenseListId, expenseId, user); return new ResponseEntity<>(HttpStatus.NO_CONTENT); } diff --git a/src/main/java/de/zendric/app/xpensely_server/services/ExpenseListService.java b/src/main/java/de/zendric/app/xpensely_server/services/ExpenseListService.java index 7ab97f7..e090e4d 100644 --- a/src/main/java/de/zendric/app/xpensely_server/services/ExpenseListService.java +++ b/src/main/java/de/zendric/app/xpensely_server/services/ExpenseListService.java @@ -8,6 +8,7 @@ import java.util.UUID; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import de.zendric.app.xpensely_server.model.AppUser; import de.zendric.app.xpensely_server.model.Expense; import de.zendric.app.xpensely_server.model.ExpenseList; import de.zendric.app.xpensely_server.model.Exception.ResourceNotFoundException; @@ -23,12 +24,14 @@ public class ExpenseListService { private final ExpenseListRepository repository; private final ExpenseRepository expenseRepository; private final XpenselyCustomCategoryRepository customCategoryRepository; + private final HistoryService historyService; public ExpenseListService(ExpenseListRepository repository, ExpenseRepository expenseRepository, - XpenselyCustomCategoryRepository customCategoryRepository) { + XpenselyCustomCategoryRepository customCategoryRepository, HistoryService historyService) { this.repository = repository; this.expenseRepository = expenseRepository; this.customCategoryRepository = customCategoryRepository; + this.historyService = historyService; } public ExpenseList createList(ExpenseList list) { @@ -62,15 +65,17 @@ public class ExpenseListService { return repository.findByOwnerUsernameOrSharedWithUsername(username); } - public Expense addExpenseToList(Long expenseListId, Expense expense) { + public Expense addExpenseToList(Long expenseListId, Expense expense, AppUser actor) { ExpenseList expenseList = repository.findById(expenseListId) .orElseThrow(() -> new ResourceNotFoundException("ExpenseList not found with id: " + expenseListId)); + expense.setLastModified(LocalDateTime.now()); expenseList.addExpense(expense); repository.save(expenseList); + historyService.recordCreated(expenseList, expense, actor); return expense; } - public void deleteExpenseFromList(Long expenseListId, Long expenseId) { + public void deleteExpenseFromList(Long expenseListId, Long expenseId, AppUser actor) { ExpenseList expenseList = repository.findById(expenseListId) .orElseThrow(() -> new ResourceNotFoundException("ExpenseList not found with id: " + expenseListId)); Expense expenseToRemove = null; @@ -80,11 +85,11 @@ public class ExpenseListService { break; } } - if (expenseToRemove != null) { - expenseList.removeExpense(expenseToRemove); - } else { + if (expenseToRemove == null) { throw new ResourceNotFoundException("Expense not found with id: " + expenseId); } + historyService.recordDeleted(expenseList, expenseToRemove, actor); + expenseList.removeExpense(expenseToRemove); repository.save(expenseList); } @@ -110,7 +115,7 @@ public class ExpenseListService { return repository.findByInviteCode(inviteCode); } - public Expense updateExpense(Long expenseListId, Expense updatedExpense) { + public Expense updateExpense(Long expenseListId, Expense updatedExpense, AppUser actor) { ExpenseList expenseList = repository.findById(expenseListId) .orElseThrow(() -> new ResourceNotFoundException("ExpenseList not found with id: " + expenseListId)); @@ -121,6 +126,9 @@ public class ExpenseListService { Expense existingExpense = expenseRepository.findById(updatedExpense.getId()) .orElseThrow(() -> new ResourceNotFoundException("Expense not found with id: " + updatedExpense.getId())); + + Expense before = snapshot(existingExpense); + existingExpense.setTitle(updatedExpense.getTitle()); existingExpense.setAmount(updatedExpense.getAmount()); existingExpense.setPersonalUseAmount(updatedExpense.getPersonalUseAmount()); @@ -128,8 +136,25 @@ public class ExpenseListService { existingExpense.setDate(updatedExpense.getDate()); existingExpense.setOwner(updatedExpense.getOwner()); existingExpense.setCategory(updatedExpense.getCategory()); + existingExpense.setLastModified(LocalDateTime.now()); - return expenseRepository.save(existingExpense); + Expense saved = expenseRepository.save(existingExpense); + historyService.recordUpdated(expenseList, before, saved, actor); + return saved; + } + + /** Detached copy of the mutable fields, taken before applying an update. */ + private Expense snapshot(Expense e) { + Expense copy = new Expense(); + copy.setId(e.getId()); + copy.setTitle(e.getTitle()); + copy.setAmount(e.getAmount()); + copy.setPersonalUseAmount(e.getPersonalUseAmount()); + copy.setOtherPersonAmount(e.getOtherPersonAmount()); + copy.setCategory(e.getCategory()); + copy.setDate(e.getDate()); + copy.setOwner(e.getOwner()); + return copy; } // TODO implement API for this diff --git a/src/test/java/de/zendric/app/xpensely_Server/controller/ExpenseListControllerTest.java b/src/test/java/de/zendric/app/xpensely_Server/controller/ExpenseListControllerTest.java index e203b64..5461ff2 100644 --- a/src/test/java/de/zendric/app/xpensely_Server/controller/ExpenseListControllerTest.java +++ b/src/test/java/de/zendric/app/xpensely_Server/controller/ExpenseListControllerTest.java @@ -199,7 +199,7 @@ class ExpenseListControllerTest { when(expenseListService.findById(1L)).thenReturn(Optional.of(list)); when(authenticatedUserResolver.resolveCurrentUser(any())).thenReturn(member); - when(expenseListService.addExpenseToList(eq(1L), any())).thenReturn(new Expense()); + when(expenseListService.addExpenseToList(eq(1L), any(), any())).thenReturn(new Expense()); mockMvc.perform(post("/api/expenselist/1/add") .contentType(MediaType.APPLICATION_JSON) diff --git a/src/test/java/de/zendric/app/xpensely_Server/services/ExpenseListServiceTest.java b/src/test/java/de/zendric/app/xpensely_Server/services/ExpenseListServiceTest.java index 0f832a5..ad7a25f 100644 --- a/src/test/java/de/zendric/app/xpensely_Server/services/ExpenseListServiceTest.java +++ b/src/test/java/de/zendric/app/xpensely_Server/services/ExpenseListServiceTest.java @@ -1,20 +1,27 @@ package de.zendric.app.xpensely_Server.services; import de.zendric.app.xpensely_server.model.AppUser; +import de.zendric.app.xpensely_server.model.Expense; import de.zendric.app.xpensely_server.model.ExpenseList; import de.zendric.app.xpensely_server.repo.ExpenseListRepository; import de.zendric.app.xpensely_server.repo.ExpenseRepository; import de.zendric.app.xpensely_server.repo.XpenselyCustomCategoryRepository; import de.zendric.app.xpensely_server.services.ExpenseListService; +import de.zendric.app.xpensely_server.services.HistoryService; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import java.time.LocalDate; +import java.util.ArrayList; import java.util.List; +import java.util.Optional; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.eq; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.Mockito.never; @@ -25,6 +32,7 @@ class ExpenseListServiceTest { @Mock ExpenseListRepository repository; @Mock ExpenseRepository expenseRepository; @Mock XpenselyCustomCategoryRepository customCategoryRepository; + @Mock HistoryService historyService; @InjectMocks ExpenseListService service; @@ -54,4 +62,63 @@ class ExpenseListServiceTest { verify(repository).findByOwnerUsernameOrSharedWithUsername("alice"); verify(repository, never()).findAll(); } + + private Expense makeExpense(Long id, String title, double amount) { + Expense e = new Expense(); + e.setId(id); + e.setTitle(title); + e.setAmount(amount); + e.setPersonalUseAmount(0.0); + e.setOtherPersonAmount(0.0); + e.setDate(LocalDate.of(2026, 7, 1)); + return e; + } + + @Test + void addExpenseToList_setsLastModified_andRecordsCreated() { + AppUser actor = new AppUser(); actor.setId(1L); + ExpenseList list = new ExpenseList(); list.setId(10L); + list.setExpenses(new ArrayList<>()); + when(repository.findById(10L)).thenReturn(Optional.of(list)); + + Expense expense = makeExpense(null, "Lunch", 10.0); + service.addExpenseToList(10L, expense, actor); + + assertThat(expense.getLastModified()).isNotNull(); + verify(historyService).recordCreated(list, expense, actor); + } + + @Test + void updateExpense_setsLastModified_andRecordsUpdatedWithBeforeSnapshot() { + AppUser actor = new AppUser(); actor.setId(1L); + ExpenseList list = new ExpenseList(); list.setId(10L); + Expense existing = makeExpense(42L, "Old title", 12.0); + list.setExpenses(new ArrayList<>(List.of(existing))); + when(repository.findById(10L)).thenReturn(Optional.of(list)); + when(expenseRepository.findById(42L)).thenReturn(Optional.of(existing)); + when(expenseRepository.save(any(Expense.class))).thenAnswer(inv -> inv.getArgument(0)); + + Expense updated = makeExpense(42L, "New title", 15.0); + service.updateExpense(10L, updated, actor); + + assertThat(existing.getLastModified()).isNotNull(); + org.mockito.ArgumentCaptor beforeCaptor = + org.mockito.ArgumentCaptor.forClass(Expense.class); + verify(historyService).recordUpdated(eq(list), beforeCaptor.capture(), eq(existing), eq(actor)); + assertThat(beforeCaptor.getValue().getTitle()).isEqualTo("Old title"); + assertThat(beforeCaptor.getValue().getAmount()).isEqualTo(12.0); + } + + @Test + void deleteExpenseFromList_recordsDeleted() { + AppUser actor = new AppUser(); actor.setId(1L); + ExpenseList list = new ExpenseList(); list.setId(10L); + Expense existing = makeExpense(42L, "Taxi", 9.0); + list.setExpenses(new ArrayList<>(List.of(existing))); + when(repository.findById(10L)).thenReturn(Optional.of(list)); + + service.deleteExpenseFromList(10L, 42L, actor); + + verify(historyService).recordDeleted(eq(list), any(Expense.class), eq(actor)); + } }