Merge branch 'main' into nl/2871-bundle-screenreader

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")

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(

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):

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):

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()),

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(
+        messages.error(request, str(exception))
-            "Could not send email. Can not invite member of a .gov organization to a different organization.",
+        logger.warning(str(exception), exc_info=True)
-            exc_info=True,
-        )
-        messages.error(
-            request,
-            f"{email} is already a member of another .gov organization.",
-        )
     elif isinstance(exception, AlreadyDomainManagerError):
         messages.warning(request, str(exception))
     elif isinstance(exception, AlreadyDomainInvitedError):

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