diff --git a/src/registrar/tests/test_email_invitations.py b/src/registrar/tests/test_email_invitations.py index 87384d3be..b9fef1bf8 100644 --- a/src/registrar/tests/test_email_invitations.py +++ b/src/registrar/tests/test_email_invitations.py @@ -12,7 +12,7 @@ class DomainInvitationEmail(unittest.TestCase): @less_console_noise_decorator @patch("registrar.utility.email_invitations.send_templated_email") @patch("registrar.utility.email_invitations.UserDomainRole.objects.filter") - @patch("registrar.utility.email_invitations.validate_invitation") + @patch("registrar.utility.email_invitations._validate_invitation") @patch("registrar.utility.email_invitations.get_requestor_email") @patch("registrar.utility.email_invitations.send_invitation_email") @patch("registrar.utility.email_invitations.normalize_domains") @@ -57,7 +57,7 @@ class DomainInvitationEmail(unittest.TestCase): mock_normalize_domains.assert_called_once_with(mock_domain) mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain]) mock_validate_invitation.assert_called_once_with( - email, [mock_domain], mock_requestor, is_member_of_different_org + email, None, [mock_domain], mock_requestor, is_member_of_different_org ) mock_send_invitation_email.assert_called_once_with(email, mock_requestor_email, [mock_domain], None) mock_user_domain_role_filter.assert_called_once_with(domain=mock_domain) @@ -77,7 +77,7 @@ class DomainInvitationEmail(unittest.TestCase): @less_console_noise_decorator @patch("registrar.utility.email_invitations.send_templated_email") @patch("registrar.utility.email_invitations.UserDomainRole.objects.filter") - @patch("registrar.utility.email_invitations.validate_invitation") + @patch("registrar.utility.email_invitations._validate_invitation") @patch("registrar.utility.email_invitations.get_requestor_email") @patch("registrar.utility.email_invitations.send_invitation_email") @patch("registrar.utility.email_invitations.normalize_domains") @@ -136,7 +136,7 @@ class DomainInvitationEmail(unittest.TestCase): mock_normalize_domains.assert_called_once_with([mock_domain1, mock_domain2]) mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain1, mock_domain2]) mock_validate_invitation.assert_called_once_with( - email, [mock_domain1, mock_domain2], mock_requestor, is_member_of_different_org + email, None, [mock_domain1, mock_domain2], mock_requestor, is_member_of_different_org ) mock_send_invitation_email.assert_called_once_with( email, mock_requestor_email, [mock_domain1, mock_domain2], None @@ -175,7 +175,7 @@ class DomainInvitationEmail(unittest.TestCase): self.assertEqual(mock_send_templated_email.call_count, 2) @less_console_noise_decorator - @patch("registrar.utility.email_invitations.validate_invitation") + @patch("registrar.utility.email_invitations._validate_invitation") def test_send_domain_invitation_email_raises_invite_validation_exception(self, mock_validate_invitation): """Test sending domain invitation email for one domain and assert exception when invite validation fails. @@ -213,7 +213,7 @@ class DomainInvitationEmail(unittest.TestCase): mock_get_requestor_email.assert_called_once() @less_console_noise_decorator - @patch("registrar.utility.email_invitations.validate_invitation") + @patch("registrar.utility.email_invitations._validate_invitation") @patch("registrar.utility.email_invitations.get_requestor_email") @patch("registrar.utility.email_invitations.send_invitation_email") @patch("registrar.utility.email_invitations.normalize_domains") @@ -257,13 +257,13 @@ class DomainInvitationEmail(unittest.TestCase): mock_normalize_domains.assert_called_once_with(mock_domain) mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain]) mock_validate_invitation.assert_called_once_with( - email, [mock_domain], mock_requestor, is_member_of_different_org + email, None, [mock_domain], mock_requestor, is_member_of_different_org ) self.assertEqual(str(context.exception), "Error sending email") @less_console_noise_decorator @patch("registrar.utility.email_invitations.send_emails_to_domain_managers") - @patch("registrar.utility.email_invitations.validate_invitation") + @patch("registrar.utility.email_invitations._validate_invitation") @patch("registrar.utility.email_invitations.get_requestor_email") @patch("registrar.utility.email_invitations.send_invitation_email") @patch("registrar.utility.email_invitations.normalize_domains") @@ -305,7 +305,7 @@ class DomainInvitationEmail(unittest.TestCase): mock_normalize_domains.assert_called_once_with(mock_domain) mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain]) mock_validate_invitation.assert_called_once_with( - email, [mock_domain], mock_requestor, is_member_of_different_org + email, None, [mock_domain], mock_requestor, is_member_of_different_org ) mock_send_invitation_email.assert_called_once_with(email, mock_requestor_email, [mock_domain], None) self.assertEqual(str(context.exception), "Error sending email") diff --git a/src/registrar/tests/test_views_members_json.py b/src/registrar/tests/test_views_members_json.py index 8082a1a30..ceae1e35f 100644 --- a/src/registrar/tests/test_views_members_json.py +++ b/src/registrar/tests/test_views_members_json.py @@ -54,6 +54,7 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest): title="Admin", ) self.email6 = "fifth@example.com" + self.email7 = "sixth@example.com" # Create Portfolio self.portfolio = Portfolio.objects.create(creator=self.user, organization_name="Test Portfolio") @@ -302,7 +303,7 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest): @override_flag("organization_members", active=True) def test_get_portfolio_invited_json_with_domains(self): """Test that portfolio invited members are returned properly for an authenticated user and the response includes - the domains that the member manages..""" + the domains that the member manages. Test also verifies that retrieved invitations are not included.""" UserPortfolioPermission.objects.create( user=self.user, portfolio=self.portfolio, @@ -319,6 +320,16 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest): UserPortfolioPermissionChoices.EDIT_MEMBERS, ], ) + PortfolioInvitation.objects.create( + email=self.email7, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ], + status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED, + ) # create a domain in the portfolio domain = Domain.objects.create( diff --git a/src/registrar/utility/email_invitations.py b/src/registrar/utility/email_invitations.py index 25e9db0f3..d589f814f 100644 --- a/src/registrar/utility/email_invitations.py +++ b/src/registrar/utility/email_invitations.py @@ -37,7 +37,7 @@ def send_domain_invitation_email( domains = normalize_domains(domains) requestor_email = get_requestor_email(requestor, domains) - validate_invitation(email, domains, requestor, is_member_of_different_org) + _validate_invitation(email, requested_user, domains, requestor, is_member_of_different_org) send_invitation_email(email, requestor_email, domains, requested_user) @@ -101,12 +101,12 @@ def get_requestor_email(requestor, domains): return requestor.email -def validate_invitation(email, domains, requestor, is_member_of_different_org): +def _validate_invitation(email, user, domains, requestor, is_member_of_different_org): """Validate the invitation conditions.""" check_outside_org_membership(email, requestor, is_member_of_different_org) for domain in domains: - validate_existing_invitation(email, domain) + _validate_existing_invitation(email, user, domain) # NOTE: should we also be validating against existing user_domain_roles @@ -121,7 +121,7 @@ def check_outside_org_membership(email, requestor, is_member_of_different_org): raise OutsideOrgMemberError(email=email) -def validate_existing_invitation(email, domain): +def _validate_existing_invitation(email, user, domain): """Check for existing invitations and handle their status.""" try: invite = DomainInvitation.objects.get(email=email, domain=domain) @@ -134,6 +134,9 @@ def validate_existing_invitation(email, domain): raise AlreadyDomainInvitedError(email) except DomainInvitation.DoesNotExist: pass + if user: + if UserDomainRole.objects.filter(user=user, domain=domain).exists(): + raise AlreadyDomainManagerError(email) def send_invitation_email(email, requestor_email, domains, requested_user): diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index cc18d7269..0a6f00c36 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -59,13 +59,18 @@ class MissingEmailError(InvitationError): super().__init__(message) -class OutsideOrgMemberError(ValueError): +class OutsideOrgMemberError(InvitationError): """ Error raised when an org member tries adding a user from a different .gov org. To be deleted when users can be members of multiple orgs. """ - pass + def __init__(self, email=None): + # Default message if no additional info is provided + message = "Can not invite member of a .gov organization to a different organization." + if email: + message = f"{email} is already a member of another .gov organization." + super().__init__(message) class ActionNotAllowed(Exception): diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index 11e58e112..a45ad66e9 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -137,7 +137,9 @@ class PortfolioMembersJson(PortfolioMembersPermission, View): ) # PortfolioInvitation query - invitations = PortfolioInvitation.objects.filter(portfolio=portfolio) + invitations = PortfolioInvitation.objects.filter( + portfolio=portfolio, status=PortfolioInvitation.PortfolioInvitationStatus.INVITED + ) invitations = invitations.annotate( first_name=Value(None, output_field=CharField()), last_name=Value(None, output_field=CharField()), diff --git a/src/registrar/views/utility/invitation_helper.py b/src/registrar/views/utility/invitation_helper.py index 5c730d0c3..98c36b308 100644 --- a/src/registrar/views/utility/invitation_helper.py +++ b/src/registrar/views/utility/invitation_helper.py @@ -67,14 +67,8 @@ def handle_invitation_exceptions(request, exception, email): messages.error(request, str(exception)) logger.error(str(exception), exc_info=True) elif isinstance(exception, OutsideOrgMemberError): - logger.warning( - "Could not send email. Can not invite member of a .gov organization to a different organization.", - exc_info=True, - ) - messages.error( - request, - f"{email} is already a member of another .gov organization.", - ) + messages.error(request, str(exception)) + logger.warning(str(exception), exc_info=True) elif isinstance(exception, AlreadyDomainManagerError): messages.warning(request, str(exception)) elif isinstance(exception, AlreadyDomainInvitedError): diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py index a3067d3a2..3234ea701 100644 --- a/src/registrar/views/utility/permission_views.py +++ b/src/registrar/views/utility/permission_views.py @@ -65,7 +65,6 @@ class DomainPermissionView(DomainPermission, DetailView, abc.ABC): def is_editable(self): """Returns whether domain is editable in the context of the view""" - logger.info("checking if is_editable") domain_editable = self.object.is_editable() if not domain_editable: return False