From 85a3fd62104a545c34a7f8cb8ff4e45b6a1e88e8 Mon Sep 17 00:00:00 2001
From: chasegawa <chasegawa@unicon.net>
Date: Fri, 23 Jul 2021 00:36:36 -0700
Subject: [PATCH] SHIBUI-1740

Testing cleanup
---
 .../admin/ui/security/model/User.java         | 32 ++++++++++---------
 .../ui/security/service/UserService.java      |  2 +-
 .../security/service/UserServiceTests.groovy  |  4 +--
 3 files changed, 20 insertions(+), 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
index 94dda83aa..9e6204ea8 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
@@ -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/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java
index 097491947..497c06b1f 100644
--- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java
+++ b/backend/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/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/UserServiceTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/UserServiceTests.groovy
index d5b0d3620..91c329056 100644
--- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/UserServiceTests.groovy
+++ b/backend/src/test/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: