#267 #268 Minor updates & fixes

conorheffron · Oct 15, 2024 · 83acf71 · 83acf71
1 parent 19f8f49
commit 83acf71
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 20 deletions.
diff --git 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> 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> 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
@@ -15,13 +15,15 @@
 import lombok.Builder;
 import lombok.Data;
 import lombok.NoArgsConstructor;
+import lombok.ToString;
 
 @Entity
 @Table(name="person")
 @Data
 @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
@@ -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:/"));
     }
 }