From ec66f8658135e2c29ebc5230d4976ddf2d1fd3fc Mon Sep 17 00:00:00 2001
From: James Stott <158563996+jamesstottmoj@users.noreply.github.com>
Date: Thu, 16 Jan 2025 13:56:11 +0000
Subject: [PATCH] fix/exception (#1434)

* Added logging to user and group creation. Fixed potential issue with exception getting caught sooner than intended

* Fixed gramatical error

* Added further logging

* Restructured code when getting user and group id

* Fixed order of parameters being passed into remove from group function. Refactored parameter order to match other methods
---
 controlpanel/api/aws.py | 44 +++++++++++++++++++++++++++++------------
 1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/controlpanel/api/aws.py b/controlpanel/api/aws.py
index d77c45b3..63cb66be 100644
--- a/controlpanel/api/aws.py
+++ b/controlpanel/api/aws.py
@@ -1482,20 +1482,23 @@ def get_group_id(self, group_name):
 
             return response["GroupId"]
         except self.client.exceptions.ResourceNotFoundException as error:
-            log.exception(error.response["Error"]["Message"])
+            log.error(error.response["Error"]["Message"])
             raise error
 
     def get_group_membership_id(self, group_name, user_email):
+        group_id = self.get_group_id(group_name)
+        user_id = self.get_user_id(user_email)
+
         try:
             response = self.client.get_group_membership_id(
                 IdentityStoreId=self.sso_client.get_identity_store_id(),
-                GroupId=self.get_group_id(group_name),
-                MemberId={"UserId": self.get_user_id(user_email)},
+                GroupId=group_id,
+                MemberId={"UserId": user_id},
             )
 
             return response["MembershipId"]
         except self.client.exceptions.ResourceNotFoundException as error:
-            log.info(error.response["Error"]["Message"])
+            log.error(error.response["Error"]["Message"])
             return None
 
     def get_name_from_email(self, user_email):
@@ -1535,11 +1538,17 @@ def create_user(self, user_email):
                 },
                 Emails=[{"Value": user_email, "Type": "EntraId", "Primary": True}],
             )
+
+            log.info(f"User {user_email} created in Identity Center")
         except Exception as error:
-            log.exception(error)
+            log.error(error)
             raise error
 
-    def create_group_membership(self, group_name, user_email):
+    def create_group_membership(self, user_email, group_name):
+
+        log.info(f"Attempting to add {user_email} to group {group_name}")
+        group_id = self.get_group_id(group_name)
+        user_id = self.get_user_id(user_email)
 
         try:
             membership_id = self.get_group_membership_id(group_name, user_email)
@@ -1550,16 +1559,20 @@ def create_group_membership(self, group_name, user_email):
 
             response = self.client.create_group_membership(
                 IdentityStoreId=self.sso_client.get_identity_store_id(),
-                GroupId=self.get_group_id(group_name),
-                MemberId={"UserId": self.get_user_id(user_email)},
+                GroupId=group_id,
+                MemberId={"UserId": user_id},
             )
 
+            log.info(f"User {user_email} added to group {group_name}")
             return response
         except Exception as error:
-            log.exception(error)
+            log.error(error)
             raise error
 
-    def delete_group_membership(self, group_name, user_email):
+    def delete_group_membership(self, user_email, group_name):
+
+        log.info(f"Attempting to remove {user_email} from group {group_name}")
+
         try:
             membership_id = self.get_group_membership_id(group_name, user_email)
 
@@ -1571,11 +1584,16 @@ def delete_group_membership(self, group_name, user_email):
                 IdentityStoreId=self.sso_client.get_identity_store_id(),
                 MembershipId=membership_id,
             )
+
+            log.info(f"User {user_email} removed from group {group_name}")
         except Exception as error:
-            log.exception(error.response["Error"]["Message"])
+            log.error(error.response["Error"]["Message"])
             raise error
 
     def add_user_to_group(self, justice_email, quicksight_group):
+
+        log.info(f"Attempting to add {justice_email} to azure and {quicksight_group} groups")
+
         if not justice_email:
             message = (
                 "Cannot create an Identity Center user without an associated @justice.gov.uk email"
@@ -1584,5 +1602,5 @@ def add_user_to_group(self, justice_email, quicksight_group):
             raise Exception(message)
 
         self.create_user(justice_email)
-        self.create_group_membership(quicksight_group, justice_email)
-        self.create_group_membership(settings.AZURE_HOLDING_GROUP_NAME, justice_email)
+        self.create_group_membership(justice_email, quicksight_group)
+        self.create_group_membership(justice_email, settings.AZURE_HOLDING_GROUP_NAME)