diff --git a/app/integrations/google_next/directory.py b/app/integrations/google_next/directory.py index 35fc5fee..13a50d2b 100644 --- a/app/integrations/google_next/directory.py +++ b/app/integrations/google_next/directory.py @@ -37,7 +37,7 @@ def list_users( service: Resource, customer: str | None = None, **kwargs, -): +) -> list[dict]: """List all users in the Google Workspace domain. Args: @@ -90,7 +90,7 @@ def list_groups( service: Resource, customer: str | None = None, **kwargs, -): +) -> list[dict]: """List all groups in the Google Workspace domain. A query can be provided to filter the results (e.g. query="email:prefix-*" will filter for all groups where the email starts with 'prefix-'). Args: @@ -156,7 +156,7 @@ def list_groups_with_members( list: A list of group objects with members and their details. """ - groups = list_groups( + groups: list[dict] = list_groups( service, query=query, fields="groups(email, name, directMembersCount, description)", @@ -165,6 +165,8 @@ def list_groups_with_members( if len(groups) == 0: return [] + users: list[dict] = list_users(service) + if groups_filters is not None: for groups_filter in groups_filters: groups = filters.filter_by_condition(groups, groups_filter) @@ -187,14 +189,18 @@ def list_groups_with_members( group["error"] = f"Error getting members: {e}" logger.warning(f"Error getting members for group {group['email']}: {e}") continue - members = get_members_details(service, members) + members = get_members_details(members, users) if members: group.update({"members": members}) + if any(member.get("error") for member in members) and not group.get( + "error" + ): + group["error"] = "Error getting members details." groups_with_members.append(group) return groups_with_members -def get_members_details(service: Resource, members: list[dict]): +def get_members_details(members: list[dict], users: list[dict]): """Get user details for a list of members. Args: @@ -206,24 +212,14 @@ def get_members_details(service: Resource, members: list[dict]): """ for member in members: - user_details = {} - try: - logger.info(f"Getting user details for member: {member['email']}") - user_details = retry_request( - get_user, - service, - member["email"], - max_attempts=3, - delay=1, - fields="name, primaryEmail", - ) - except Exception as e: - member["error"] = f"{e}" - logger.warning( - f"Error getting user details for member {member['email']}: {e}" - ) - continue + logger.info(f"Getting user details for member: {member['email']}") + user_details = next( + (user for user in users if user["primaryEmail"] == member["email"]), None + ) if user_details: member.update(user_details) + else: + member["error"] = "User details not found" + logger.warning(f"User details not found for member: {member['email']}") return members diff --git a/app/integrations/google_next/service.py b/app/integrations/google_next/service.py index f40b9619..561fc283 100644 --- a/app/integrations/google_next/service.py +++ b/app/integrations/google_next/service.py @@ -121,12 +121,9 @@ def execute_google_api_call( """Execute a Google API call on a resource. Args: - service_name (str): The name of the Google service. - version (str): The version of the Google service. + service (Resource): The Google service resource. resource_path (str): The path to the resource, which can include nested resources separated by dots. method (str): The method to call on the resource. - scopes (list, optional): The scopes for the Google service. - delegated_user_email (str, optional): The email address of the user to impersonate. paginate (bool, optional): Whether to paginate the API call. **kwargs: Additional keyword arguments for the API call. diff --git a/app/tests/integrations/google_next/test_directory.py b/app/tests/integrations/google_next/test_directory.py index a11a7fa2..668ee592 100644 --- a/app/tests/integrations/google_next/test_directory.py +++ b/app/tests/integrations/google_next/test_directory.py @@ -1,6 +1,6 @@ """"Unit tests for the Google Directory API.""" -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, call, patch import pytest from integrations.google_next import directory @@ -169,9 +169,9 @@ def test_list_group_members_returns_group_members( @patch("integrations.google_next.directory.list_groups") @patch("integrations.google_next.directory.list_group_members") -@patch("integrations.google_next.directory.get_user") +@patch("integrations.google_next.directory.list_users") def test_list_groups_with_members_returns_groups_with_members( - mock_get_user: MagicMock, + mock_list_users: MagicMock, mock_list_group_members: MagicMock, mock_list_groups: MagicMock, google_groups, @@ -190,16 +190,16 @@ def test_list_groups_with_members_returns_groups_with_members( mock_list_groups.return_value = groups mock_list_group_members.side_effect = group_members - mock_get_user.side_effect = users + mock_list_users.return_value = users assert directory.list_groups_with_members(mock_service) == groups_with_users @patch("integrations.google_next.directory.list_groups") @patch("integrations.google_next.directory.list_group_members") -@patch("integrations.google_next.directory.get_user") +@patch("integrations.google_next.directory.list_users") def test_list_groups_with_members_returns_groups_with_no_members( - mock_get_user: MagicMock, + mock_list_user: MagicMock, mock_list_group_members: MagicMock, mock_list_groups: MagicMock, ): @@ -214,9 +214,9 @@ def test_list_groups_with_members_returns_groups_with_no_members( @patch("integrations.google_next.directory.filters.filter_by_condition") @patch("integrations.google_next.directory.list_groups") @patch("integrations.google_next.directory.list_group_members") -@patch("integrations.google_next.directory.get_user") +@patch("integrations.google_next.directory.list_users") def test_list_groups_with_members_filtered_by_condition( - mock_get_user, + mock_list_users, mock_list_group_members, mock_list_groups, mock_filter_by_condition, @@ -231,14 +231,14 @@ def test_list_groups_with_members_filtered_by_condition( groups.extend(groups_to_filter_out) group_members = [[], google_group_members(2)] users = google_users(2) - groups_with_users = google_groups_w_users(4, 2, group_prefix="test-")[:2] groups_with_users.remove(groups_with_users[0]) mock_list_groups.return_value = groups mock_list_group_members.side_effect = group_members - mock_get_user.side_effect = users + mock_list_users.return_value = users mock_filter_by_condition.return_value = groups[:2] + groups_filters = [lambda group: "test-" in group["name"]] assert ( @@ -247,7 +247,7 @@ def test_list_groups_with_members_filtered_by_condition( ) assert mock_filter_by_condition.called_once_with(groups, groups_filters) assert mock_list_group_members.call_count == 2 - assert mock_get_user.call_count == 2 + assert mock_list_users.call_count == 1 @patch("integrations.google_next.directory.logger") @@ -255,9 +255,9 @@ def test_list_groups_with_members_filtered_by_condition( @patch("integrations.google_next.directory.retry_request") @patch("integrations.google_next.directory.list_groups") @patch("integrations.google_next.directory.list_group_members") -@patch("integrations.google_next.directory.get_user") -def test_list_groups_with_members_update_group_with_error( - mock_get_user: MagicMock, +@patch("integrations.google_next.directory.list_users") +def test_list_groups_with_members_updates_group_with_group_error( + mock_list_users: MagicMock, mock_list_group_members: MagicMock, mock_list_groups: MagicMock, mock_retry_request: MagicMock, @@ -269,8 +269,8 @@ def test_list_groups_with_members_update_group_with_error( mock_service = MagicMock() groups = [ - {"id": "test_group_id1", "name": "test_group1", "email": "email1"}, - {"id": "test_group_id2", "name": "test_group2", "email": "email2"}, + {"id": "test_group_id1", "name": "test_group1", "email": "group_email1"}, + {"id": "test_group_id2", "name": "test_group2", "email": "group_email2"}, ] group_members = [ @@ -287,13 +287,13 @@ def test_list_groups_with_members_update_group_with_error( { "id": "test_group_id1", "name": "test_group1", - "email": "email1", + "email": "group_email1", "error": "Error getting members: Exception", }, { "id": "test_group_id2", "name": "test_group2", - "email": "email2", + "email": "group_email2", "members": [ { "id": "test_member_id2", @@ -307,10 +307,9 @@ def test_list_groups_with_members_update_group_with_error( mock_list_groups.return_value = groups mock_list_group_members.side_effect = group_members - mock_get_user.side_effect = users + mock_list_users.side_effect = users mock_retry_request.side_effect = [Exception("Exception"), group_members[1]] mock_get_members_details.side_effect = [ - Exception("Exception"), [ { "id": "test_member_id2", @@ -324,18 +323,147 @@ def test_list_groups_with_members_update_group_with_error( with pytest.raises(Exception): assert directory.list_groups_with_members(mock_service) == expected_output + mock_logger.info.assert_has_calls( + [ + call("Found 2 groups."), + call("Found 2 groups after filtering."), + call("Getting members for group: group_email1"), + call("Getting members for group: group_email2"), + ] + ) mock_logger.warning.assert_called_once_with( - "Error getting members for group email1: Exception" + "Error getting members for group group_email1: Exception" ) + mock_logger.error.assert_not_called() +@patch("integrations.google_next.directory.logger") +@patch("integrations.google_next.directory.get_members_details") @patch("integrations.google_next.directory.retry_request") -@patch("integrations.google_next.directory.get_user") -def test_get_members_details_returns_members( - mock_get_user: MagicMock, mock_retry_request: MagicMock +@patch("integrations.google_next.directory.list_groups") +@patch("integrations.google_next.directory.list_group_members") +@patch("integrations.google_next.directory.list_users") +def test_list_groups_with_members_updates_group_with_user_details_error( + mock_list_users: MagicMock, + mock_list_group_members: MagicMock, + mock_list_groups: MagicMock, + mock_retry_request: MagicMock, + mock_get_members_details: MagicMock, + mock_logger: MagicMock, ): - """Test get_members_details returns members.""" + """Test list_groups_with_members returns groups with members.""" + mock_service = MagicMock() + + groups = [ + {"id": "test_group_id1", "name": "test_group1", "email": "group_email1"}, + {"id": "test_group_id2", "name": "test_group2", "email": "group_email2"}, + ] + + group_members1 = [ + [{"id": "test_member_id1", "email": "email1"}], + [{"id": "test_member_id2", "email": "email2"}], + ] + group_members2 = [ + [{"id": "test_member_id2", "email": "email2"}], + [{"id": "test_member_id3", "email": "email3"}], + ] + + users = [ + {"id": "test_user_id2", "name": "test_user2", "email": "email2"}, + {"id": "test_user_id3", "name": "test_user3", "email": "email3"}, + ] + + expected_output = [ + { + "id": "test_group_id1", + "name": "test_group1", + "email": "group_email1", + "members": [ + { + "id": "test_member_id1", + "email": "email1", + "error": "User details not found", + }, + { + "id": "test_member_id2", + "email": "email2", + "name": "test_user2", + "primaryEmail": "email2", + }, + ], + "error": "Error getting members details.", + }, + { + "id": "test_group_id2", + "name": "test_group2", + "email": "group_email2", + "members": [ + { + "id": "test_member_id2", + "email": "email2", + "name": "test_user2", + "primaryEmail": "email2", + }, + { + "id": "test_member_id3", + "email": "email3", + "name": "test_user3", + "primaryEmail": "email3", + }, + ], + }, + ] + + mock_list_groups.return_value = groups + mock_list_users.side_effect = users + mock_retry_request.side_effect = [group_members1, group_members2] + mock_get_members_details.side_effect = [ + [ + { + "id": "test_member_id1", + "email": "email1", + "error": "User details not found", + }, + { + "id": "test_member_id2", + "email": "email2", + "name": "test_user2", + "primaryEmail": "email2", + }, + ], + [ + { + "id": "test_member_id2", + "email": "email2", + "name": "test_user2", + "primaryEmail": "email2", + }, + { + "id": "test_member_id3", + "email": "email3", + "name": "test_user3", + "primaryEmail": "email3", + }, + ], + ] + + assert directory.list_groups_with_members(mock_service) == expected_output + + mock_logger.info.assert_has_calls( + [ + call("Found 2 groups."), + call("Found 2 groups after filtering."), + call("Getting members for group: group_email1"), + call("Getting members for group: group_email2"), + ] + ) + mock_logger.warning.assert_not_called() + mock_logger.error.assert_not_called() + + +def test_get_members_details_returns_members(): + """Test get_members_details returns members.""" members = [ {"email": "email1", "role": "MEMBER", "type": "USER", "status": "ACTIVE"}, {"email": "email2", "role": "MEMBER", "type": "USER", "status": "ACTIVE"}, @@ -346,8 +474,6 @@ def test_get_members_details_returns_members( {"name": "user2", "primaryEmail": "email2"}, ] - mock_retry_request.side_effect = [users[0], users[1]] - expected_result = [ { "email": "email1", @@ -367,35 +493,30 @@ def test_get_members_details_returns_members( }, ] - assert directory.get_members_details(mock_service, members) == expected_result + assert directory.get_members_details(members, users) == expected_result -@patch("integrations.google_next.directory.retry_request") -@patch("integrations.google_next.directory.get_user") +@patch("integrations.google_next.directory.logger") def test_get_members_details_returns_members_with_error( - mock_get_user: MagicMock, mock_retry_request: MagicMock + mock_logger: MagicMock, ): """Test get_members_details returns members.""" - mock_service = MagicMock() members = [ {"email": "email1", "role": "MEMBER", "type": "USER", "status": "ACTIVE"}, {"email": "email2", "role": "MEMBER", "type": "USER", "status": "ACTIVE"}, ] users = [ - Exception("Error fetching user details"), {"name": "user2", "primaryEmail": "email2"}, ] - mock_retry_request.side_effect = [users[0], users[1]] - expected_result = [ { "email": "email1", "role": "MEMBER", "type": "USER", "status": "ACTIVE", - "error": "Error fetching user details", + "error": "User details not found", }, { "email": "email2", @@ -407,4 +528,14 @@ def test_get_members_details_returns_members_with_error( }, ] - assert directory.get_members_details(mock_service, members) == expected_result + assert directory.get_members_details(members, users) == expected_result + + mock_logger.info.assert_has_calls( + [ + call("Getting user details for member: email1"), + call("Getting user details for member: email2"), + ] + ) + mock_logger.warning.assert_called_once_with( + "User details not found for member: email1" + )