Merge pull request #488 from dod-ccpo/workspace-role-change-bug-#162345640

Workspace role change bug #162345640

atst/domain/environment_roles.py

@@ -21,3 +21,6 @@ class EnvironmentRoles(object):
         if existing_env_role:
             db.session.delete(existing_env_role)
             db.session.commit()
+            return True
+        else:
+            return False

atst/domain/environments.py

@@ -65,27 +65,38 @@ class Environments(object):
             Permissions.ADD_AND_ASSIGN_CSP_ROLES,
             "assign environment roles",
         )
+        updated = False
 
         for id_and_role in ids_and_roles:
             new_role = id_and_role["role"]
             environment = Environments.get(id_and_role["id"])
 
             if new_role is None:
-                EnvironmentRoles.delete(workspace_role.user.id, environment.id)
+                role_deleted = EnvironmentRoles.delete(
+                    workspace_role.user.id, environment.id
+                )
+                if role_deleted:
+                    updated = True
             else:
                 env_role = EnvironmentRoles.get(
                     workspace_role.user.id, id_and_role["id"]
                 )
-                if env_role:
+                if env_role and env_role.role != new_role:
                     env_role.role = new_role
-                else:
+                    updated = True
+                    db.session.add(env_role)
+                elif not env_role:
                     env_role = EnvironmentRole(
                         user=workspace_role.user, environment=environment, role=new_role
                     )
+                    updated = True
                     db.session.add(env_role)
 
+        if updated:
             db.session.commit()
 
+        return updated
+
     @classmethod
     def revoke_access(cls, user, environment, target_user):
         Authorization.check_workspace_permission(

atst/routes/workspaces/members.py

@@ -147,22 +147,23 @@ def update_member(workspace_id, member_id):
     form = EditMemberForm(http_request.form)
     if form.validate():
         new_role_name = None
-        if form.data["workspace_role"] != member.role:
+        if form.data["workspace_role"] != member.role.name:
             member = Workspaces.update_member(
                 g.current_user, workspace, member, form.data["workspace_role"]
             )
             new_role_name = member.role_displayname
-
-        Environments.update_environment_roles(
-            g.current_user, workspace, member, ids_and_roles
-        )
-
             flash(
                 "workspace_role_updated",
                 member_name=member.user_name,
                 updated_role=new_role_name,
             )
 
+        updated_roles = Environments.update_environment_roles(
+            g.current_user, workspace, member, ids_and_roles
+        )
+        if updated_roles:
+            flash("environment_access_changed")
+
         return redirect(
             url_for("workspaces.workspace_members", workspace_id=workspace.id)
         )

atst/utils/flash.py

@@ -96,6 +96,11 @@ MESSAGES = {
         """,
         "category": "warning",
     },
+    "environment_access_changed": {
+        "title_template": "User access successfully changed.",
+        "message_template": "",
+        "category": "success",
+    },
 }
 
 

tests/domain/test_environments.py

@@ -38,7 +38,7 @@ def test_update_environment_roles():
     ]
 
     workspace_role = workspace.members[0]
-    Environments.update_environment_roles(
+    assert Environments.update_environment_roles(
         owner, workspace, workspace_role, new_ids_and_roles
     )
     new_dev_env_role = EnvironmentRoles.get(workspace_role.user.id, dev_env.id)
@@ -89,7 +89,7 @@ def test_remove_environment_role():
     ]
 
     workspace_role = WorkspaceRoles.get(workspace.id, developer.id)
-    Environments.update_environment_roles(
+    assert Environments.update_environment_roles(
         owner, workspace, workspace_role, new_environment_roles
     )
 
@@ -99,6 +99,35 @@ def test_remove_environment_role():
     assert EnvironmentRoles.get(developer.id, still_fa).role == "financial_auditor"
 
 
+def test_no_update_to_environment_roles():
+    owner = UserFactory.create()
+    developer = UserFactory.from_atat_role("developer")
+
+    workspace = WorkspaceFactory.create(
+        owner=owner,
+        members=[{"user": developer, "role_name": "developer"}],
+        projects=[
+            {
+                "name": "project1",
+                "environments": [
+                    {
+                        "name": "project1 dev",
+                        "members": [{"user": developer, "role_name": "devops"}],
+                    }
+                ],
+            }
+        ],
+    )
+
+    dev_env = workspace.projects[0].environments[0]
+    new_ids_and_roles = [{"id": dev_env.id, "role": "devops"}]
+
+    workspace_role = WorkspaceRoles.get(workspace.id, developer.id)
+    assert not Environments.update_environment_roles(
+        owner, workspace, workspace_role, new_ids_and_roles
+    )
+
+
 def test_get_scoped_environments(db):
     developer = UserFactory.create()
     workspace = WorkspaceFactory.create(

tests/routes/workspaces/test_members.py

@@ -93,6 +93,7 @@ def test_update_member_workspace_role(client, user_session):
         follow_redirects=True,
     )
     assert response.status_code == 200
+    assert b"role updated successfully" in response.data
     assert member.role_name == "security_auditor"
 
 
@@ -140,6 +141,8 @@ def test_update_member_environment_role(client, user_session):
         follow_redirects=True,
     )
     assert response.status_code == 200
+    assert b"role updated successfully" not in response.data
+    assert b"access successfully changed" in response.data
     assert EnvironmentRoles.get(user.id, env1_id).role == "security_auditor"
     assert EnvironmentRoles.get(user.id, env2_id).role == "devops"
 
@@ -167,6 +170,7 @@ def test_update_member_environment_role_with_no_data(client, user_session):
         follow_redirects=True,
     )
     assert response.status_code == 200
+    assert b"access successfully changed" not in response.data
     assert EnvironmentRoles.get(user.id, env1_id).role == "developer"