diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/User.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/User.java index 994c6b353..94dda83aa 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/User.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/User.java @@ -117,28 +117,28 @@ public Set getUserGroups() { * we didn't intend (thanks JPA!!). */ public void setGroup(Group assignedGroup) { - // if the incoming group is the current group, make no changes to our sets - if (assignedGroup.getResourceId().equals(groupId)) { - userGroups.forEach(ug -> { - if (ug.getGroup().getResourceId().equals(groupId)) { - ug.setGroup(assignedGroup); - } - }); - return; - } - - // stash the current groups for removal - getUserGroups().forEach(g -> { - // If the assignedGroup is in the current list, don't bother putting it in the "delete" list - if (!g.getGroup().equals(assignedGroup)) { - oldUserGroups.add(g); + UserGroup theUserGroup = new UserGroup(); + // Go through the existing UserGroups: + // 1) If a UG doesn't match the incoming assignment, move it out of the list and into the old for deletion + // 2) If it DOES match, update the group object so hibernate doesn't have a cow + userGroups.forEach(ug -> { + if (ug.getGroup().getResourceId().equals(groupId)) { + theUserGroup.setGroup(assignedGroup); + theUserGroup.setUser(this); + theUserGroup.setId(ug.getId()); + } else { + oldUserGroups.add(ug); } }); userGroups.clear(); // Assign the new group - UserGroup ug = new UserGroup(assignedGroup, this); - userGroups.add(ug); + if (theUserGroup.getUser() == null) { + UserGroup ug = new UserGroup(assignedGroup, this); + userGroups.add(ug); + } else { + userGroups.add(theUserGroup); + } // Set reference for the UI groupId = assignedGroup.getResourceId(); diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersControllerIntegrationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersControllerIntegrationTests.groovy index a9e3144fb..68979525d 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersControllerIntegrationTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersControllerIntegrationTests.groovy @@ -20,6 +20,8 @@ import org.springframework.test.web.servlet.result.MockMvcResultHandlers import spock.lang.Ignore import spock.lang.Specification import edu.internet2.tier.shibboleth.admin.ui.security.model.User +import edu.internet2.tier.shibboleth.admin.ui.security.model.UserGroup +import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserGroupRepository import edu.internet2.tier.shibboleth.admin.ui.security.service.IGroupService import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete @@ -41,10 +43,12 @@ import java.time.format.DateTimeFormatter @ActiveProfiles(["no-auth", "dev"]) @DirtiesContext class UsersControllerIntegrationTests extends Specification { - @Autowired IGroupService groupService + @Autowired + UserGroupRepository ugRepo + @Autowired private MockMvc mockMvc @@ -242,6 +246,9 @@ class UsersControllerIntegrationTests extends Specification { resultNewGroup.andExpect(status().isOk()) .andExpect(content().contentType(MediaType.APPLICATION_JSON)) .andExpect(jsonPath("\$.groupId").value("AAA")) + + def List groups = ugRepo.findAllByUser(user) + groups.size() == 1 } @WithMockUser(value = "admin", roles = ["ADMIN"])