SHIBUI-1740

Testing cleanup
TIER · Jul 23, 2021 · 85a3fd6 · 85a3fd6
1 parent e7d89c3
commit 85a3fd6
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 18 deletions.
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
@@ -112,41 +112,43 @@ public Set<UserGroup> getUserGroups() {
         return userGroups;
     }
 
+    public void setGroup(Group g) {
+        groupId = g.getResourceId();
+        updateUserGroupsWithGroup(g);
+    }
+
+    public void setGroups(Set<Group> groups) {
+        oldUserGroups.addAll(getUserGroups());
+        getUserGroups().clear();
+        groups.forEach(g -> userGroups.add(new UserGroup(g, this)));
+    }
+
     /**
      * If we change groups, we have to manually manage the set of UserGroups so that we don't have group associations 
      * we didn't intend (thanks JPA!!). 
      */
-    public void setGroup(Group assignedGroup) {
-        UserGroup theUserGroup = new UserGroup();
+    public void updateUserGroupsWithGroup(Group assignedGroup) {
+        final Set<UserGroup> setWithNewGroup= new HashSet<>();
         // 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());
+                ug.setGroup(assignedGroup);
+                setWithNewGroup.add(ug);
             } else {
                 oldUserGroups.add(ug);
             }
         });
-        userGroups.clear();
+        userGroups = setWithNewGroup;
 
         // Assign the new group
-        if (theUserGroup.getUser() == null) {
+        if (userGroups.isEmpty()) {
             UserGroup ug = new UserGroup(assignedGroup, this);
             userGroups.add(ug);
-        } else {
-            userGroups.add(theUserGroup);
         }
 
         // Set reference for the UI
         groupId = assignedGroup.getResourceId();
     }
-
-    public void setGroups(Set<Group> groups) {
-        oldUserGroups.addAll(getUserGroups());
-        getUserGroups().clear();
-        groups.forEach(g -> userGroups.add(new UserGroup(g, this)));
-    }
 }
diff --git a/...nd/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java b/...nd/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java
@@ -148,7 +148,7 @@ public User save(User user) {
             } else {
                 g = groupService.find(user.getGroupId());
             }
-            user.setGroup(g);
+            user.updateUserGroupsWithGroup(g);
         } else {
             user.getUserGroups().forEach(ug -> {
                 Group g = groupService.find(ug.getGroup().getResourceId());

diff --git a/...st/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/UserServiceTests.groovy b/...st/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/UserServiceTests.groovy
@@ -120,8 +120,8 @@ class UserServiceTests extends Specification {
         def User userInB = userService.save(user)
 
         when:
-        userInB.setGroup(ga)
-        def User result = userService.save(user)
+        userInB.setGroupId("testingGroup") // changing groups will happen by updating the user's groupid
+        def User result = userService.save(userInB)
         def List<UserGroup> usersGroups = userGroupRepository.findAllByUser(result)
 
         then: