Fix/integrations syncs (#494)

* chore: fix docstrings

* feat: simplify compare list function

* fix: include unit test handles user creation failed

* fix: conftest fixture for the aws groups with memberships

app/integrations/aws/client.py

@@ -23,8 +23,8 @@ def handle_aws_api_errors(func):
     Args:
         func (function): The function to decorate.
 
-        Returns:
-        The decorated function.
+    Returns:
+        The decorated function with error handling.
     """
 
     @wraps(func)

app/integrations/aws/identity_store.py

@@ -56,6 +56,9 @@ def delete_user(user_id, **kwargs):
     Args:
         user_id (str): The user ID of the user.
         **kwargs: Additional keyword arguments for the API call.
+
+    Returns:
+        bool: True if the user was deleted successfully, False otherwise.
     """
     kwargs = resolve_identity_store_id(kwargs)
     kwargs.update({"UserId": user_id})

app/tests/conftest.py

@@ -159,7 +159,6 @@ def _aws_groups_w_users(
             memberships = aws_groups_memberships(n_users, prefix, i + 1, store_id)[
                 "GroupMemberships"
             ]
-            group.update(memberships[0])
             group["GroupMemberships"] = [
                 {**membership, "MemberId": user}
                 for user, membership in zip(users, memberships)

app/tests/integrations/aws/test_identity_store.py

@@ -82,6 +82,33 @@ def test_create_user(mock_resolve_identity_store_id, mock_execute_aws_api_call):
     assert result == "test_user_id"
 
 
+@patch("integrations.aws.identity_store.execute_aws_api_call")
+@patch("integrations.aws.identity_store.resolve_identity_store_id")
+def test_create_user_failed(mock_resolve_identity_store_id, mock_execute_aws_api_call):
+    mock_resolve_identity_store_id.return_value = {
+        "IdentityStoreId": "test_instance_id"
+    }
+    mock_execute_aws_api_call.return_value = False
+    email = "[email protected]"
+    first_name = "Test"
+    family_name = "User"
+
+    # Act
+    result = identity_store.create_user(email, first_name, family_name)
+
+    # Assert
+    mock_execute_aws_api_call.assert_called_once_with(
+        "identitystore",
+        "create_user",
+        IdentityStoreId="test_instance_id",
+        UserName=email,
+        Emails=[{"Value": email, "Type": "WORK", "Primary": True}],
+        Name={"GivenName": first_name, "FamilyName": family_name},
+        DisplayName=f"{first_name} {family_name}",
+    )
+    assert result is None
+
+
 @patch("integrations.aws.identity_store.execute_aws_api_call")
 @patch("integrations.aws.identity_store.resolve_identity_store_id")
 def test_get_user_id(mock_resolve_identity_store_id, mock_execute_aws_api_call):

app/tests/utils/test_filters.py

@@ -95,7 +95,7 @@ def test_compare_lists_default():
     assert users_to_delete == []
 
 
-def test_compare_lists_with_different_values_enable_delete_true():
+def test_compare_lists_with_different_values():
     source = {
         "key": "userName",
         "values": [{"userName": "user1"}, {"userName": "user2"}],
@@ -105,57 +105,11 @@ def test_compare_lists_with_different_values_enable_delete_true():
         "values": [{"DisplayName": "user1"}, {"DisplayName": "user3"}],
     }
 
-    users_to_create, users_to_delete = filters.compare_lists(
-        source, target, enable_delete=True
-    )
+    users_to_create, users_to_delete = filters.compare_lists(source, target)
     assert users_to_create == [{"userName": "user2"}]
     assert users_to_delete == [{"DisplayName": "user3"}]
 
 
-def test_compare_lists_with_different_values_enable_delete_false():
-    source = {
-        "key": "userName",
-        "values": [{"userName": "user1"}, {"userName": "user2"}],
-    }
-    target = {
-        "key": "DisplayName",
-        "values": [{"DisplayName": "user1"}, {"DisplayName": "user3"}],
-    }
-
-    users_to_create, users_to_delete = filters.compare_lists(
-        source, target, enable_delete=False
-    )
-    assert users_to_create == [{"userName": "user2"}]
-    assert users_to_delete == []
-
-
-def test_compare_lists_with_different_values_delete_target_all_true():
-    source = {
-        "key": "userName",
-        "values": [{"userName": "user1"}, {"userName": "user2"}],
-    }
-    target = {
-        "key": "DisplayName",
-        "values": [
-            {"DisplayName": "user1"},
-            {"DisplayName": "user2"},
-            {"DisplayName": "user3"},
-            {"DisplayName": "user4"},
-        ],
-    }
-
-    users_to_create, users_to_delete = filters.compare_lists(
-        source, target, delete_target_all=True
-    )
-    assert users_to_create == []
-    assert users_to_delete == [
-        {"DisplayName": "user1"},
-        {"DisplayName": "user2"},
-        {"DisplayName": "user3"},
-        {"DisplayName": "user4"},
-    ]
-
-
 def test_compare_lists_match_mode():
     source = {
         "key": "userName",
@@ -197,9 +151,7 @@ def test_compare_lists_missing_key():
     assert response == ([], [])
 
 
-def test_compare_list_with_complex_values_match_mode(
-    google_groups, aws_groups, google_users, aws_users
-):
+def test_compare_list_with_complex_values_match_mode(google_groups, aws_groups):
     prefix = "aws-"
     source_values = google_groups(3, prefix, "test.com")
     for value in source_values:
@@ -226,6 +178,6 @@ def test_compare_list_with_complex_values_sync_mode(google_users, aws_users):
     source = {"values": source_users, "key": "primaryEmail"}
     target = {"values": target_users, "key": "UserName"}
 
-    response = filters.compare_lists(source, target, mode="sync", enable_delete=True)
+    response = filters.compare_lists(source, target, mode="sync")
 
     assert response == ([], target["values"][3:])

app/utils/filters.py

@@ -41,20 +41,14 @@ def get_nested_value(dictionary, key):
         return None
 
 
-def compare_lists(source, target, mode="sync", **kwargs):
+def compare_lists(source, target, mode="sync"):
     """Compares two lists and returns specific elements based on the comparison mode and keys provided.
 
     Args:
         `source (dict)`: Source data with `values` (list) and `key` (string).
         `target (dict)`: Target data with `values` (list) and `key` (string).
         `mode (str)`: Operation mode - `sync` or `match`.
 
-        **kwargs: Additional arguments:
-
-        - `filters (list)`: List of filters to apply to the users.
-        - `enable_delete (bool)`: Enable the deletion of users in the target system.
-        - `delete_target_all (bool)`: Mark all target system users for deletion.
-
          In `sync` mode (default), the function returns:
 
             1. Elements in the source list but not in the target list (to be added to the target).
@@ -83,12 +77,6 @@ def compare_lists(source, target, mode="sync", **kwargs):
     }
 
     if mode == "sync":
-        enable_delete = kwargs.get("enable_delete", False)
-        delete_target_all = kwargs.get("delete_target_all", False)
-
-        if delete_target_all:
-            return [], target_values
-
         values_to_add = [
             filtered_source_values[key]
             for key in filtered_source_values
@@ -100,9 +88,6 @@ def compare_lists(source, target, mode="sync", **kwargs):
             if key not in filtered_source_values
         ]
 
-        if not enable_delete:
-            values_to_remove = []
-
         return values_to_add, values_to_remove
 
     elif mode == "match":