From 30ef2a67ef171f85bb3ee0dcfb0062058fd23f21 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 10 Jun 2019 16:43:47 -0400 Subject: [PATCH] Fix bug in application member deletion --- atst/domain/applications.py | 8 ++------ atst/routes/applications/team.py | 11 ++++++----- tests/domain/test_applications.py | 2 +- tests/routes/applications/test_team.py | 24 ++++++++++++++++++++++++ 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/atst/domain/applications.py b/atst/domain/applications.py index 3c8cff64..103fe3ca 100644 --- a/atst/domain/applications.py +++ b/atst/domain/applications.py @@ -128,15 +128,11 @@ class Applications(BaseDomainClass): return invitation @classmethod - def remove_member(cls, application, user_id): - application_role = ApplicationRoles.get( - user_id=user_id, application_id=application.id - ) - + def remove_member(cls, application_role): application_role.status = ApplicationRoleStatus.DISABLED application_role.deleted = True - for env in application.environments: + for env in application_role.application.environments: EnvironmentRoles.delete( application_role_id=application_role.id, environment_id=env.id ) diff --git a/atst/routes/applications/team.py b/atst/routes/applications/team.py index 0cf4c6c7..81f79e28 100644 --- a/atst/routes/applications/team.py +++ b/atst/routes/applications/team.py @@ -189,14 +189,15 @@ def create_member(application_id): def remove_member(application_id, application_role_id): application_role = ApplicationRoles.get_by_id(application_role_id) - Applications.remove_member( - application=g.application, user_id=application_role.user_id - ) - user = Users.get(application_role.user_id) + user_name = "a user" + if application_role.user: + user_name = application_role.user.full_name + + Applications.remove_member(application_role) flash( "application_member_removed", - user_name=user.full_name, + user_name=user_name, application_name=g.application.name, ) diff --git a/tests/domain/test_applications.py b/tests/domain/test_applications.py index f405a245..acbddc11 100644 --- a/tests/domain/test_applications.py +++ b/tests/domain/test_applications.py @@ -147,7 +147,7 @@ def test_remove_member(): user_id=user.id, application_id=application.id ) - Applications.remove_member(application=application, user_id=member_role.user.id) + Applications.remove_member(member_role) assert ( ApplicationRoles.get(user_id=user.id, application_id=application.id).status diff --git a/tests/routes/applications/test_team.py b/tests/routes/applications/test_team.py index d7abec06..32120b89 100644 --- a/tests/routes/applications/test_team.py +++ b/tests/routes/applications/test_team.py @@ -218,6 +218,30 @@ def test_remove_member_success(client, user_session): ) +def test_remove_new_member_success(client, user_session): + application = ApplicationFactory.create() + application_role = ApplicationRoleFactory.create(application=application, user=None) + + user_session(application.portfolio.owner) + + response = client.post( + url_for( + "applications.remove_member", + application_id=application.id, + application_role_id=application_role.id, + ) + ) + + assert response.status_code == 302 + assert response.location == url_for( + "applications.team", + _anchor="application-members", + _external=True, + application_id=application.id, + fragment="application-members", + ) + + def test_remove_member_failure(client, user_session): user = UserFactory.create() application = ApplicationFactory.create()