diff --git a/atst/domain/environments.py b/atst/domain/environments.py index 38e23ff1..760d620a 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -65,6 +65,7 @@ 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"] @@ -72,19 +73,24 @@ class Environments(object): if new_role is None: EnvironmentRoles.delete(workspace_role.user.id, environment.id) + 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 ) - db.session.add(env_role) + updated = True + db.session.add(env_role) db.session.commit() + return updated @classmethod def revoke_access(cls, user, environment, target_user): diff --git a/atst/routes/workspaces/members.py b/atst/routes/workspaces/members.py index bfe85792..f3f06d4e 100644 --- a/atst/routes/workspaces/members.py +++ b/atst/routes/workspaces/members.py @@ -158,9 +158,12 @@ def update_member(workspace_id, member_id): updated_role=new_role_name, ) - Environments.update_environment_roles( + 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) ) diff --git a/atst/utils/flash.py b/atst/utils/flash.py index 6196d2d2..5a6c8e90 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -96,6 +96,11 @@ MESSAGES = { """, "category": "warning", }, + "environment_access_changed": { + "title_template": "User access successfully changed.", + "message_template": "", + "category": "success", + }, } diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py index 25efe8cc..9f5e9aab 100644 --- a/tests/domain/test_environments.py +++ b/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( diff --git a/tests/routes/workspaces/test_members.py b/tests/routes/workspaces/test_members.py index 47fe9325..44a63879 100644 --- a/tests/routes/workspaces/test_members.py +++ b/tests/routes/workspaces/test_members.py @@ -142,6 +142,7 @@ def test_update_member_environment_role(client, user_session): ) 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" @@ -169,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"