fix(projects): preserve server picker recovery

This commit is contained in:
sechmachine
2026-08-15 15:16:28 +07:00
parent 5df9eff21e
commit f3ffdab48e
6 changed files with 92 additions and 15 deletions
+7 -1
View File
@@ -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. 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 ## 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. `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. `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 ## 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. `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. 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.
@@ -164,8 +164,8 @@ public class ProjectController {
} }
/** /**
* Adds all selected eligible Interns atomically or re-renders membership history with the * Adds all selected eligible Interns atomically or re-renders membership history with every
* complete retained selection and a safe validation message. * still-eligible selection retained and a count of unavailable choices.
* *
* @param principal authenticated user * @param principal authenticated user
* @param projectId owning Project identifier * @param projectId owning Project identifier
@@ -182,16 +182,27 @@ public class ProjectController {
BindingResult bindingResult, BindingResult bindingResult,
Model model) { Model model) {
long actorId = actorId(principal); long actorId = actorId(principal);
boolean rejectedByService = false;
if (!bindingResult.hasErrors()) { if (!bindingResult.hasErrors()) {
try { try {
projects.addMembers(actorId, projectId, membersForm.internUserIds()); projects.addMembers(actorId, projectId, membersForm.internUserIds());
return "redirect:/projects/" + projectId + "/members"; return "redirect:/projects/" + projectId + "/members";
} catch (ProjectRuleViolationException exception) { } catch (ProjectRuleViolationException exception) {
rejectedByService = true;
bindingResult.rejectValue( bindingResult.rejectValue(
"internUserIds", "project.members.ineligible", exception.getMessage()); "internUserIds", "project.members.ineligible", exception.getMessage());
} }
} }
populateMembersModel(actorId, projectId, model); var refreshedOptions = populateMembersModel(actorId, projectId, model);
if (rejectedByService) {
Set<Long> 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"; return "projects/members";
} }
@@ -242,7 +253,7 @@ public class ProjectController {
return "projects/leadership"; return "projects/leadership";
} }
private void populateMembersModel(long actorId, long projectId, Model model) { private List<EligibleInternOption> populateMembersModel(long actorId, long projectId, Model model) {
var project = pages.detail(actorId, projectId); var project = pages.detail(actorId, projectId);
var members = pages.members(actorId, projectId); var members = pages.members(actorId, projectId);
model.addAttribute("project", project); model.addAttribute("project", project);
@@ -252,10 +263,13 @@ public class ProjectController {
.filter(member -> member.leftAt() == null) .filter(member -> member.leftAt() == null)
.map(member -> member.internUserId()) .map(member -> member.internUserId())
.collect(Collectors.toUnmodifiableSet()); .collect(Collectors.toUnmodifiableSet());
model.addAttribute("eligibleInternOptions", eligibleInternOptions().stream() var options = eligibleInternOptions().stream()
.filter(option -> !currentMemberIds.contains(option.userId())) .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) { private void populateLeadershipModel(long actorId, long projectId, Model model) {
@@ -41,7 +41,7 @@
<input class="control" id="leader-search" type="search" autocomplete="off" data-picker-search> <input class="control" id="leader-search" type="search" autocomplete="off" data-picker-search>
<div class="picker-options"> <div class="picker-options">
<label class="picker-option" data-picker-option th:each="option : ${eligibleInternOptions}" th:attr="data-picker-search=${option.displayName + ' ' + option.studentCode}"> <label class="picker-option" data-picker-option th:each="option : ${eligibleInternOptions}" th:attr="data-picker-search=${option.displayName + ' ' + option.studentCode}">
<input type="radio" th:field="*{initialLeaderUserId}" th:value="${option.userId}" required> <input type="radio" th:field="*{initialLeaderUserId}" th:value="${option.userId}">
<span><strong data-picker-label th:text="|${option.displayName} (${option.studentCode})|">Intern (Code)</strong><small th:text="|${#temporals.format(option.internshipStart, 'dd/MM/yyyy')} ${#temporals.format(option.internshipEnd, 'dd/MM/yyyy')}|">Dates</small></span> <span><strong data-picker-label th:text="|${option.displayName} (${option.studentCode})|">Intern (Code)</strong><small th:text="|${#temporals.format(option.internshipStart, 'dd/MM/yyyy')} ${#temporals.format(option.internshipEnd, 'dd/MM/yyyy')}|">Dates</small></span>
</label> </label>
<p class="picker-empty" data-picker-empty th:hidden="${!#lists.isEmpty(eligibleInternOptions)}">No matching eligible Interns.</p> <p class="picker-empty" data-picker-empty th:hidden="${!#lists.isEmpty(eligibleInternOptions)}">No matching eligible Interns.</p>
@@ -36,7 +36,7 @@
<input class="control" id="leadership-search" type="search" autocomplete="off" data-picker-search> <input class="control" id="leadership-search" type="search" autocomplete="off" data-picker-search>
<div class="picker-options"> <div class="picker-options">
<label class="picker-option" data-picker-option th:each="option : ${eligibleInternOptions}" th:attr="data-picker-search=${option.displayName + ' ' + option.studentCode}"> <label class="picker-option" data-picker-option th:each="option : ${eligibleInternOptions}" th:attr="data-picker-search=${option.displayName + ' ' + option.studentCode}">
<input type="radio" th:field="*{internUserId}" th:value="${option.userId}" required> <input type="radio" th:field="*{internUserId}" th:value="${option.userId}">
<span><strong data-picker-label th:text="|${option.displayName} (${option.studentCode})|">Intern (Code)</strong><small th:text="|${#temporals.format(option.internshipStart, 'dd/MM/yyyy')} ${#temporals.format(option.internshipEnd, 'dd/MM/yyyy')}|">Dates</small></span> <span><strong data-picker-label th:text="|${option.displayName} (${option.studentCode})|">Intern (Code)</strong><small th:text="|${#temporals.format(option.internshipStart, 'dd/MM/yyyy')} ${#temporals.format(option.internshipEnd, 'dd/MM/yyyy')}|">Dates</small></span>
</label> </label>
<p class="picker-empty" data-picker-empty th:hidden="${!#lists.isEmpty(eligibleInternOptions)}">No matching eligible current members.</p> <p class="picker-empty" data-picker-empty th:hidden="${!#lists.isEmpty(eligibleInternOptions)}">No matching eligible current members.</p>
@@ -29,6 +29,8 @@
<p class="picker-summary" data-picker-summary aria-live="polite">No Interns selected</p> <p class="picker-summary" data-picker-summary aria-live="polite">No Interns selected</p>
<p class="field-help" th:if="${#lists.isEmpty(eligibleInternOptions)}">No eligible nonmembers are available.</p> <p class="field-help" th:if="${#lists.isEmpty(eligibleInternOptions)}">No eligible nonmembers are available.</p>
<p class="field-error" id="member-intern-user-error" role="alert" th:if="${#fields.hasErrors('internUserIds')}" th:errors="*{internUserIds}"></p> <p class="field-error" id="member-intern-user-error" role="alert" th:if="${#fields.hasErrors('internUserIds')}" th:errors="*{internUserIds}"></p>
<p class="field-help" role="status" th:if="${unavailableSelectionCount != null and unavailableSelectionCount > 0}"
th:text="${unavailableSelectionCount == 1 ? '1 previously selected Intern is no longer eligible; choose a replacement.' : unavailableSelectionCount + ' previously selected Interns are no longer eligible; choose replacements.'}">Unavailable selection recovery</p>
<dialog class="picker-drawer" data-picker-dialog aria-labelledby="member-picker-title"> <dialog class="picker-drawer" data-picker-dialog aria-labelledby="member-picker-title">
<div class="picker-header"><div><h2 class="panel-title" id="member-picker-title">Add Project members</h2><p class="field-help">Select one or more eligible Interns who are not current members.</p></div><button class="button" type="button" data-picker-cancel>Cancel</button></div> <div class="picker-header"><div><h2 class="panel-title" id="member-picker-title">Add Project members</h2><p class="field-help">Select one or more eligible Interns who are not current members.</p></div><button class="button" type="button" data-picker-cancel>Cancel</button></div>
<div class="picker-body"> <div class="picker-body">
@@ -35,6 +35,7 @@ import java.time.Clock;
import java.time.LocalDate; import java.time.LocalDate;
import java.time.ZoneId; import java.time.ZoneId;
import java.util.List; import java.util.List;
import java.util.regex.Pattern;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.ParameterizedTest;
@@ -81,7 +82,7 @@ class ProjectControllerTest {
option(20L, "Nguyen An", "STU-020"), option(20L, "Nguyen An", "STU-020"),
option(21L, "Tran Binh", "STU-021"))); option(21L, "Tran Binh", "STU-021")));
mvc.perform(get("/projects/new")) String html = mvc.perform(get("/projects/new"))
.andExpect(status().isOk()) .andExpect(status().isOk())
.andExpect(model().attributeExists("eligibleInternOptions")) .andExpect(model().attributeExists("eligibleInternOptions"))
.andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content() .andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content()
@@ -95,7 +96,9 @@ class ProjectControllerTest {
.andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content() .andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content()
.string(containsString("01/08/2026 31/12/2026"))) .string(containsString("01/08/2026 31/12/2026")))
.andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content() .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 @Test
@@ -139,17 +142,17 @@ class ProjectControllerTest {
assertTrue(leadershipHtml.contains("Current Member")); assertTrue(leadershipHtml.contains("Current Member"));
assertFalse(leadershipHtml.contains("Eligible Nonmember")); assertFalse(leadershipHtml.contains("Eligible Nonmember"));
assertFalse(leadershipHtml.contains("data-picker-label>Current Leader")); assertFalse(leadershipHtml.contains("data-picker-label>Current Leader"));
assertFalse(containsRequiredRadio(leadershipHtml));
} }
@Test @Test
@WithMockUser(username = "mentor@example.test") @WithMockUser(username = "mentor@example.test")
void rejectedMemberBatchRetainsEverySelectionAndShowsRecoveryCopy() throws Exception { void rejectedMemberBatchRetainsEligibleSelectionsAndExplainsUnavailableCountWithoutIds() throws Exception {
when(pages.authenticatedUserId("mentor@example.test")).thenReturn(10L); when(pages.authenticatedUserId("mentor@example.test")).thenReturn(10L);
when(pages.detail(10L, 30L)).thenReturn(plannedOwnerDetail()); when(pages.detail(10L, 30L)).thenReturn(plannedOwnerDetail());
when(pages.members(10L, 30L)).thenReturn(List.of()); when(pages.members(10L, 30L)).thenReturn(List.of());
when(accounts.eligibleInternOptions(LocalDate.of(2026, 8, 15))).thenReturn(List.of( when(accounts.eligibleInternOptions(LocalDate.of(2026, 8, 15))).thenReturn(List.of(
option(21L, "First Intern", "STU-021"), option(21L, "First Intern", "STU-021")));
option(22L, "Second Intern", "STU-022")));
doThrow(new ProjectRuleViolationException("One or more selected Interns are no longer eligible")) doThrow(new ProjectRuleViolationException("One or more selected Interns are no longer eligible"))
.when(projects).addMembers(10L, 30L, List.of(21L, 22L)); .when(projects).addMembers(10L, 30L, List.of(21L, 22L));
@@ -161,10 +164,12 @@ class ProjectControllerTest {
.andExpect(model().attributeHasFieldErrors("projectMembersForm", "internUserIds")) .andExpect(model().attributeHasFieldErrors("projectMembersForm", "internUserIds"))
.andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content() .andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content()
.string(containsString("One or more selected Interns are no longer eligible"))) .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() .andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content()
.string(containsString("value=\"21\" id=\"internUserIds1\" name=\"internUserIds\" checked=\"checked\""))) .string(containsString("value=\"21\" id=\"internUserIds1\" name=\"internUserIds\" checked=\"checked\"")))
.andExpect(org.springframework.test.web.servlet.result.MockMvcResultMatchers.content() .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 @Test
@@ -281,6 +286,50 @@ class ProjectControllerTest {
.andExpect(redirectedUrl("/projects/30")); .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 @Test
@WithMockUser(username = "mentor@example.test") @WithMockUser(username = "mentor@example.test")
void owningMentorCanActivateAPlannedProject() throws Exception { void owningMentorCanActivateAPlannedProject() throws Exception {
@@ -521,4 +570,10 @@ class ProjectControllerTest {
LocalDate.of(2026, 8, 1), LocalDate.of(2026, 8, 1),
LocalDate.of(2026, 12, 31)); LocalDate.of(2026, 12, 31));
} }
private static boolean containsRequiredRadio(String html) {
return Pattern.compile("<input(?=[^>]*type=\\\"radio\\\")(?=[^>]*required(?:=|\\s|>))[^>]*>")
.matcher(html)
.find();
}
} }