From 8f94d9e6ec5cb68d3f1dcc94b3b06d95c6e9be94 Mon Sep 17 00:00:00 2001
From: dandds <dan-ctr@friends.dds.mil>
Date: Fri, 13 Dec 2019 06:20:38 -0500
Subject: [PATCH] Log any CSP errors that occur when disabling a user.

When one user disables another's environment role in Azure, sometimes an
exception will be raised. Since we catch the exception and display an
error message to the user, we should also log the exception so that the
error is traceable later.
---
 atst/routes/applications/settings.py       |  4 ++-
 tests/routes/applications/test_settings.py | 39 ++++++++++++++++++++++
 tests/utils.py                             |  3 ++
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py
index a4aea550..f2d252a9 100644
--- a/atst/routes/applications/settings.py
+++ b/atst/routes/applications/settings.py
@@ -20,6 +20,7 @@ from atst.domain.permission_sets import PermissionSets
 from atst.utils.flash import formatted_flash as flash
 from atst.utils.localization import translate
 from atst.jobs import send_mail
+from atst.routes.errors import log_error
 
 
 def get_environments_obj_for_app(application):
@@ -234,7 +235,8 @@ def handle_update_member(application_id, application_role_id, form_data):
 
             flash("application_member_updated", user_name=app_role.user_name)
 
-        except GeneralCSPException:
+        except GeneralCSPException as exc:
+            log_error(exc)
             flash(
                 "application_member_update_error", user_name=app_role.user_name,
             )
diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py
index 4961065e..08c979ad 100644
--- a/tests/routes/applications/test_settings.py
+++ b/tests/routes/applications/test_settings.py
@@ -12,6 +12,7 @@ from atst.domain.application_roles import ApplicationRoles
 from atst.domain.environment_roles import EnvironmentRoles
 from atst.domain.invitations import ApplicationInvitations
 from atst.domain.common import Paginator
+from atst.domain.csp.cloud import GeneralCSPException
 from atst.domain.permission_sets import PermissionSets
 from atst.models.application_role import Status as ApplicationRoleStatus
 from atst.models.environment_role import CSPRole, EnvironmentRole
@@ -748,3 +749,41 @@ def test_handle_update_member(set_g):
     assert len(application.roles) == 1
     assert len(app_role.environment_roles) == 1
     assert app_role.environment_roles[0].environment == env
+
+
+def test_handle_update_member_with_error(set_g, monkeypatch, mock_logger):
+    exception = "An error occurred."
+
+    def _raise_csp_exception(*args, **kwargs):
+        raise GeneralCSPException(exception)
+
+    monkeypatch.setattr(
+        "atst.domain.environments.Environments.update_env_role", _raise_csp_exception
+    )
+
+    user = UserFactory.create()
+    application = ApplicationFactory.create(
+        environments=[{"name": "Naboo"}, {"name": "Endor"}]
+    )
+    (env, env_1) = application.environments
+    app_role = ApplicationRoleFactory(application=application)
+    set_g("current_user", application.portfolio.owner)
+    set_g("portfolio", application.portfolio)
+    set_g("application", application)
+
+    form_data = ImmutableMultiDict(
+        {
+            "environment_roles-0-environment_id": env.id,
+            "environment_roles-0-role": "Basic Access",
+            "environment_roles-0-environment_name": env.name,
+            "environment_roles-1-environment_id": env_1.id,
+            "environment_roles-1-role": NO_ACCESS,
+            "environment_roles-1-environment_name": env_1.name,
+            "perms_env_mgmt": True,
+            "perms_team_mgmt": True,
+            "perms_del_env": True,
+        }
+    )
+    handle_update_member(application.id, app_role.id, form_data)
+
+    assert mock_logger.messages[-1] == exception
diff --git a/tests/utils.py b/tests/utils.py
index 152c347a..66bf2b18 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -40,6 +40,9 @@ class FakeLogger:
     def error(self, msg, *args, **kwargs):
         self._log("error", msg, *args, **kwargs)
 
+    def exception(self, msg, *args, **kwargs):
+        self._log("exception", msg, *args, **kwargs)
+
     def _log(self, _lvl, msg, *args, **kwargs):
         self.messages.append(msg)
         if "extra" in kwargs: