From 83acf7187a55d42f7f87f40e2b5e7548c487d4d5 Mon Sep 17 00:00:00 2001 From: Conor H Date: Tue, 15 Oct 2024 10:38:05 +0100 Subject: [PATCH] #267 #268 Minor updates & fixes --- .../db/controller/PersonController.java | 21 +++++++++++-------- src/main/java/com/ironoc/db/model/Person.java | 2 ++ .../db/controller/PersonControllerTest.java | 18 +++++++--------- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/main/java/com/ironoc/db/controller/PersonController.java b/src/main/java/com/ironoc/db/controller/PersonController.java index 136f981..de83cf5 100644 --- a/src/main/java/com/ironoc/db/controller/PersonController.java +++ b/src/main/java/com/ironoc/db/controller/PersonController.java @@ -9,10 +9,11 @@ import org.springframework.ui.Model; import org.springframework.ui.ModelMap; import org.springframework.validation.BindingResult; +import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; -import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.ModelAttribute; import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; import java.util.List; import java.util.Optional; @@ -48,10 +49,10 @@ public String addPerson(ModelMap map, @Valid @ModelAttribute("person") Person pe } personService.addPerson(person); - return "redirect:/index"; + return "redirect:/"; } - @GetMapping(value = "/delete/{id}") + @DeleteMapping(value = "/delete/{id}") public String deletePersonById(ModelMap map, @PathVariable("id") Integer id) { log.info("Entering personController.deletePersonBySurname: map={}, id={}", map, id.longValue()); Optional person = personService.findPersonById(id.longValue()); @@ -61,11 +62,12 @@ public String deletePersonById(ModelMap map, @PathVariable("id") Integer id) { // no matching entries to delete map.addAttribute("deleteError", "There are no matching entries to delete"); } - return "redirect:/index"; + return "redirect:/"; } @GetMapping("/edit/{id}") public String showEditView(@PathVariable("id") Integer id, Model model) { + log.info("Entering personController.showEditView: ID={}, model={}", id, model.asMap()); Optional person = personService.findPersonById(id.longValue()); if (person.isPresent()) { model.addAttribute("person", person.get()); @@ -77,14 +79,15 @@ public String showEditView(@PathVariable("id") Integer id, Model model) { } @PostMapping("/update/{id}") - public String updatePerson(@PathVariable("id") Integer id, @Valid Person person, BindingResult result) { + public String updatePerson(ModelMap map, @PathVariable("id") Integer id, @Valid Person person, + BindingResult result) { + log.info("Entering personController.updatePerson: ID={}, person={}", id, person.toString()); if (result.hasErrors()) { person.setId(id.longValue()); - return "index"; + map.addAttribute("person", person); + return "edit-person"; } - personService.addPerson(person); - - return "redirect:/index"; + return "redirect:/"; } } diff --git a/src/main/java/com/ironoc/db/model/Person.java b/src/main/java/com/ironoc/db/model/Person.java index 5b1011c..f79b00b 100644 --- a/src/main/java/com/ironoc/db/model/Person.java +++ b/src/main/java/com/ironoc/db/model/Person.java @@ -15,6 +15,7 @@ import lombok.Builder; import lombok.Data; import lombok.NoArgsConstructor; +import lombok.ToString; @Entity @Table(name="person") @@ -22,6 +23,7 @@ @Builder @AllArgsConstructor @NoArgsConstructor +@ToString public class Person { @Id diff --git a/src/test/java/com/ironoc/db/controller/PersonControllerTest.java b/src/test/java/com/ironoc/db/controller/PersonControllerTest.java index 5ad9c50..5086771 100644 --- a/src/test/java/com/ironoc/db/controller/PersonControllerTest.java +++ b/src/test/java/com/ironoc/db/controller/PersonControllerTest.java @@ -13,7 +13,6 @@ import org.springframework.ui.ModelMap; import org.springframework.validation.BindingResult; -import jakarta.servlet.http.HttpServletRequest; import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -31,9 +30,6 @@ public class PersonControllerTest { @InjectMocks private PersonController personController;// controller under test - @Mock - private HttpServletRequest httpServletRequestMock; - @Mock private PersonService personServiceMock; @@ -88,7 +84,7 @@ public void test_AddPerson_success() { verify(personServiceMock).addPerson(personMock); verify(personServiceMock, never()).getAllPersons(); - assertThat(result, is("redirect:/index")); + assertThat(result, is("redirect:/")); } @Test @@ -97,13 +93,13 @@ public void test_updatePerson_fail() { when(bindingResultMock.hasErrors()).thenReturn(true); // when - String result = personController.updatePerson(TEST_ID, personMock, bindingResultMock); + String result = personController.updatePerson(modelMapMock, TEST_ID, personMock, bindingResultMock); // then verify(bindingResultMock).hasErrors(); verify(personServiceMock, never()).addPerson(personMock); - assertThat(result, is("index")); + assertThat(result, is("edit-person")); } @Test @@ -112,13 +108,13 @@ public void test_updatePerson_success() { when(bindingResultMock.hasErrors()).thenReturn(false); // when - String result = personController.updatePerson(TEST_ID, personMock, bindingResultMock); + String result = personController.updatePerson(modelMapMock, TEST_ID, personMock, bindingResultMock); // then verify(bindingResultMock).hasErrors(); verify(personServiceMock).addPerson(personMock); - assertThat(result, is("redirect:/index")); + assertThat(result, is("redirect:/")); } @Test @@ -180,7 +176,7 @@ public void test_deletePersonById_success() { verify(personServiceMock).deletePersonById(TEST_ID.longValue()); verify(personServiceMock, never()).getAllPersons(); - assertThat(result, is("redirect:/index")); + assertThat(result, is("redirect:/")); } @Test @@ -197,6 +193,6 @@ public void test_deletePersonById_fail() { verify(modelMapMock).addAttribute("deleteError", "There are no matching entries to delete"); verify(personServiceMock, never()).getAllPersons(); - assertThat(result, is("redirect:/index")); + assertThat(result, is("redirect:/")); } }