diff --git a/docs/tests/web/project-intern-picker.md b/docs/tests/web/project-intern-picker.md index 5bd863b..36f96af 100644 --- a/docs/tests/web/project-intern-picker.md +++ b/docs/tests/web/project-intern-picker.md @@ -21,6 +21,8 @@ The combined Java RED command was `env JAVA_HOME=/opt/homebrew/opt/openjdk@25 PA After the producer API compiled, `env JAVA_HOME=/opt/homebrew/opt/openjdk@25 PATH=/opt/homebrew/opt/openjdk@25/bin:$PATH ./mvnw '-Dtest=ProjectControllerTest' test` ran 19 tests with 4 expected assertion failures for the missing eligible-option model, filtered multi-select markup, and retained selection rendering. A separate no-roster regression ran 1 test with 1 assertion failure because the disabled picker trigger had no reachable explanatory copy. +Independent review added rendered regressions before the correction. The same focused controller command ran 22 tests with exactly 3 failures and no errors: both closed-dialog radio contracts detected browser `required`, and stale batch recovery lacked the count-only replacement message. The new missing-selection POST contracts already passed through server Bean Validation. + ## GREEN `env JAVA_HOME=/opt/homebrew/opt/openjdk@25 PATH=/opt/homebrew/opt/openjdk@25/bin:$PATH ./mvnw '-Dtest=ProjectControllerTest' test` passed the initial rendered picker suite at 19/19. After adding the no-roster regression, the affected Project command below passed the expanded controller suite at 20/20. @@ -29,6 +31,8 @@ After the producer API compiled, `env JAVA_HOME=/opt/homebrew/opt/openjdk@25 PAT `env PATH=/opt/homebrew/opt/node@24/bin:$PATH npm run build` succeeded with Tailwind CSS 4.3.3 and the existing local icon builder. No dependency was added. +After the review correction, the focused controller command passed 22/22. Creation and leadership radios no longer use closed-dialog browser constraint validation; missing selections re-render their server field errors. A failed member batch retains submitted option 21 when refreshed eligibility contains only 21, omits all rendered value/ID markup for stale option 22, and reports one unavailable selection without exposing its identifier. + ## Affected suite `env JAVA_HOME=/opt/homebrew/opt/openjdk@25 PATH=/opt/homebrew/opt/openjdk@25/bin:$PATH DOCKER_HOST=unix:///Users/sechmachine/.orbstack/run/docker.sock ./mvnw '-Dtest=ProjectControllerTest,ProjectEntityTest,ProjectPersistenceStructureTest,ProjectServiceIntegrationTest,ProjectTaskMutationContextTest,LayerStructureTest' test` passed 38/38 tests with no failures, errors, or skips. @@ -39,4 +43,6 @@ After the producer API compiled, `env JAVA_HOME=/opt/homebrew/opt/openjdk@25 PAT No browser loop or Impeccable detector is run on this branch; the root owner performs one integrated pass. MockMvc proves rendered semantics and a dependency-free Node test executes the dialog/search/selection behavior with controlled DOM boundaries. -After merging exact reviewed `main` `32c8a2d315d2175760c5d4792988cd0aa5ab6dd0`, `npm ci`, the 1/1 UI test, frontend build, compile, Project-scoped Javadoc/doclint, and diff check all succeeded. The affected Java command added the updated shared `UiContractWebTest` and passed 45/45 tests with no failures, errors, or skips. +After merging exact reviewed `main` `32c8a2d315d2175760c5d4792988cd0aa5ab6dd0`, `npm ci`, the 1/1 UI test, frontend build, compile, Project-scoped Javadoc/doclint, and diff check all succeeded. The first affected Java command added the updated shared `UiContractWebTest` and passed 45/45 tests with no failures, errors, or skips. + +The bounded post-review affected command reran `ProjectControllerTest,ProjectEntityTest,ProjectPersistenceStructureTest,ProjectServiceIntegrationTest,ProjectTaskMutationContextTest,LayerStructureTest,UiContractWebTest` and passed 47/47 with no failures, errors, or skips, including 9/9 Project service tests against PostgreSQL 18.4. The UI test remained 1/1; frontend build, compile, Project-scoped Javadoc/doclint, and `git diff --check` also succeeded. diff --git a/src/main/java/com/lab/labtimesheet/feature/project/controller/ProjectController.java b/src/main/java/com/lab/labtimesheet/feature/project/controller/ProjectController.java index 3f713b8..4f01201 100644 --- a/src/main/java/com/lab/labtimesheet/feature/project/controller/ProjectController.java +++ b/src/main/java/com/lab/labtimesheet/feature/project/controller/ProjectController.java @@ -164,8 +164,8 @@ public class ProjectController { } /** - * Adds all selected eligible Interns atomically or re-renders membership history with the - * complete retained selection and a safe validation message. + * Adds all selected eligible Interns atomically or re-renders membership history with every + * still-eligible selection retained and a count of unavailable choices. * * @param principal authenticated user * @param projectId owning Project identifier @@ -182,16 +182,27 @@ public class ProjectController { BindingResult bindingResult, Model model) { long actorId = actorId(principal); + boolean rejectedByService = false; if (!bindingResult.hasErrors()) { try { projects.addMembers(actorId, projectId, membersForm.internUserIds()); return "redirect:/projects/" + projectId + "/members"; } catch (ProjectRuleViolationException exception) { + rejectedByService = true; bindingResult.rejectValue( "internUserIds", "project.members.ineligible", exception.getMessage()); } } - populateMembersModel(actorId, projectId, model); + var refreshedOptions = populateMembersModel(actorId, projectId, model); + if (rejectedByService) { + Set refreshedIds = refreshedOptions.stream() + .map(EligibleInternOption::userId) + .collect(Collectors.toUnmodifiableSet()); + long unavailableSelectionCount = membersForm.internUserIds().stream() + .filter(userId -> !refreshedIds.contains(userId)) + .count(); + model.addAttribute("unavailableSelectionCount", unavailableSelectionCount); + } return "projects/members"; } @@ -242,7 +253,7 @@ public class ProjectController { return "projects/leadership"; } - private void populateMembersModel(long actorId, long projectId, Model model) { + private List populateMembersModel(long actorId, long projectId, Model model) { var project = pages.detail(actorId, projectId); var members = pages.members(actorId, projectId); model.addAttribute("project", project); @@ -252,10 +263,13 @@ public class ProjectController { .filter(member -> member.leftAt() == null) .map(member -> member.internUserId()) .collect(Collectors.toUnmodifiableSet()); - model.addAttribute("eligibleInternOptions", eligibleInternOptions().stream() + var options = eligibleInternOptions().stream() .filter(option -> !currentMemberIds.contains(option.userId())) - .toList()); + .toList(); + model.addAttribute("eligibleInternOptions", options); + return options; } + return List.of(); } private void populateLeadershipModel(long actorId, long projectId, Model model) { diff --git a/src/main/resources/templates/projects/form.html b/src/main/resources/templates/projects/form.html index 40696e0..9b09333 100644 --- a/src/main/resources/templates/projects/form.html +++ b/src/main/resources/templates/projects/form.html @@ -41,7 +41,7 @@

No matching eligible Interns.

diff --git a/src/main/resources/templates/projects/leadership.html b/src/main/resources/templates/projects/leadership.html index 708c5ee..868c70b 100644 --- a/src/main/resources/templates/projects/leadership.html +++ b/src/main/resources/templates/projects/leadership.html @@ -36,7 +36,7 @@

No matching eligible current members.

diff --git a/src/main/resources/templates/projects/members.html b/src/main/resources/templates/projects/members.html index 394a0ee..be855cb 100644 --- a/src/main/resources/templates/projects/members.html +++ b/src/main/resources/templates/projects/members.html @@ -29,6 +29,8 @@

No Interns selected

No eligible nonmembers are available.

+

Unavailable selection recovery

Add Project members

Select one or more eligible Interns who are not current members.

diff --git a/src/test/java/com/lab/labtimesheet/feature/project/controller/ProjectControllerTest.java b/src/test/java/com/lab/labtimesheet/feature/project/controller/ProjectControllerTest.java index c66fa1f..d844b12 100644 --- a/src/test/java/com/lab/labtimesheet/feature/project/controller/ProjectControllerTest.java +++ b/src/test/java/com/lab/labtimesheet/feature/project/controller/ProjectControllerTest.java @@ -35,6 +35,7 @@ import java.time.Clock; import java.time.LocalDate; import java.time.ZoneId; import java.util.List; +import java.util.regex.Pattern; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.params.ParameterizedTest; @@ -81,7 +82,7 @@ class ProjectControllerTest { option(20L, "Nguyen An", "STU-020"), option(21L, "Tran Binh", "STU-021"))); - mvc.perform(get("/projects/new")) + String html = mvc.perform(get("/projects/new")) .andExpect(status().isOk()) .andExpect(model().attributeExists("eligibleInternOptions")) .andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content() @@ -95,7 +96,9 @@ class ProjectControllerTest { .andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content() .string(containsString("01/08/2026 – 31/12/2026"))) .andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content() - .string(not(containsString("Initial Leader user ID")))); + .string(not(containsString("Initial Leader user ID")))) + .andReturn().getResponse().getContentAsString(); + assertFalse(containsRequiredRadio(html)); } @Test @@ -139,17 +142,17 @@ class ProjectControllerTest { assertTrue(leadershipHtml.contains("Current Member")); assertFalse(leadershipHtml.contains("Eligible Nonmember")); assertFalse(leadershipHtml.contains("data-picker-label>Current Leader")); + assertFalse(containsRequiredRadio(leadershipHtml)); } @Test @WithMockUser(username = "mentor@example.test") - void rejectedMemberBatchRetainsEverySelectionAndShowsRecoveryCopy() throws Exception { + void rejectedMemberBatchRetainsEligibleSelectionsAndExplainsUnavailableCountWithoutIds() throws Exception { when(pages.authenticatedUserId("mentor@example.test")).thenReturn(10L); when(pages.detail(10L, 30L)).thenReturn(plannedOwnerDetail()); when(pages.members(10L, 30L)).thenReturn(List.of()); when(accounts.eligibleInternOptions(LocalDate.of(2026, 8, 15))).thenReturn(List.of( - option(21L, "First Intern", "STU-021"), - option(22L, "Second Intern", "STU-022"))); + option(21L, "First Intern", "STU-021"))); doThrow(new ProjectRuleViolationException("One or more selected Interns are no longer eligible")) .when(projects).addMembers(10L, 30L, List.of(21L, 22L)); @@ -161,10 +164,12 @@ class ProjectControllerTest { .andExpect(model().attributeHasFieldErrors("projectMembersForm", "internUserIds")) .andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content() .string(containsString("One or more selected Interns are no longer eligible"))) + .andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content() + .string(containsString("1 previously selected Intern is no longer eligible; choose a replacement."))) .andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content() .string(containsString("value=\"21\" id=\"internUserIds1\" name=\"internUserIds\" checked=\"checked\""))) .andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content() - .string(containsString("value=\"22\" id=\"internUserIds2\" name=\"internUserIds\" checked=\"checked\""))); + .string(not(containsString("value=\"22\"")))); } @Test @@ -281,6 +286,50 @@ class ProjectControllerTest { .andExpect(redirectedUrl("/projects/30")); } + @Test + @WithMockUser(username = "mentor@example.test") + void missingInitialLeaderReRendersServerFieldError() throws Exception { + when(pages.authenticatedActor("mentor@example.test")) + .thenReturn(new ProjectActorView(10L, "MENTOR")); + + mvc.perform(post("/projects") + .with(csrf()) + .param("name", "Intern Portal Refresh") + .param("startDate", "2026-08-15") + .param("endDate", "2026-09-30")) + .andExpect(status().isOk()) + .andExpect(view().name("projects/form")) + .andExpect(model().attributeHasFieldErrors("projectForm", "initialLeaderUserId")) + .andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content() + .string(containsString("id=\"initialLeaderUserId-error\""))); + + verify(projects, never()).create(org.mockito.ArgumentMatchers.anyLong(), org.mockito.ArgumentMatchers.any()); + } + + @Test + @WithMockUser(username = "mentor@example.test") + void missingReplacementLeaderReRendersServerFieldError() throws Exception { + when(pages.authenticatedUserId("mentor@example.test")).thenReturn(10L); + when(pages.detail(10L, 30L)).thenReturn(plannedOwnerDetail()); + when(pages.leadership(10L, 30L)).thenReturn(List.of()); + when(pages.members(10L, 30L)).thenReturn(List.of(new ProjectMemberView( + 41L, 21L, "Current Member", Instant.parse("2026-08-15T00:00:00Z"), null, false))); + when(accounts.eligibleInternOptions(LocalDate.of(2026, 8, 15))).thenReturn(List.of( + option(21L, "Current Member", "STU-021"))); + + mvc.perform(post("/projects/30/leadership").with(csrf())) + .andExpect(status().isOk()) + .andExpect(view().name("projects/leadership")) + .andExpect(model().attributeHasFieldErrors("projectMemberForm", "internUserId")) + .andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content() + .string(containsString("id=\"leadership-intern-user-error\""))); + + verify(projects, never()).changeLeader( + org.mockito.ArgumentMatchers.anyLong(), + org.mockito.ArgumentMatchers.anyLong(), + org.mockito.ArgumentMatchers.anyLong()); + } + @Test @WithMockUser(username = "mentor@example.test") void owningMentorCanActivateAPlannedProject() throws Exception { @@ -521,4 +570,10 @@ class ProjectControllerTest { LocalDate.of(2026, 8, 1), LocalDate.of(2026, 12, 31)); } + + private static boolean containsRequiredRadio(String html) { + return Pattern.compile("]*type=\\\"radio\\\")(?=[^>]*required(?:=|\\s|>))[^>]*>") + .matcher(html) + .find(); + } }