Merge pull request #3643 from cisagov/za/3635-add-member-wrong-error

#3635 / #3657: Wrong error for "add member" and bug-fix for insensitive email checks - [HOTGOV]

src/registrar/admin.py

@@ -1846,7 +1846,7 @@ class PortfolioInvitationAdmin(BaseInvitationAdmin):
                 requested_user = get_requested_user(requested_email)
 
                 permission_exists = UserPortfolioPermission.objects.filter(
-                    user__email=requested_email, portfolio=portfolio, user__email__isnull=False
+                    user__email__iexact=requested_email, portfolio=portfolio, user__email__isnull=False
                 ).exists()
                 if not permission_exists:
                     # if permission does not exist for a user with requested_email, send email
@@ -1856,9 +1856,7 @@ class PortfolioInvitationAdmin(BaseInvitationAdmin):
                         portfolio=portfolio,
                         is_admin_invitation=is_admin_invitation,
                     ):
-                        messages.warning(
+                        messages.warning(request, "Could not send email notification to existing organization admins.")
-                            self.request, "Could not send email notification to existing organization admins."
-                        )
                     # if user exists for email, immediately retrieve portfolio invitation upon creation
                     if requested_user is not None:
                         obj.retrieve()

src/registrar/forms/portfolio.py

@@ -22,6 +22,7 @@ from registrar.models.utility.portfolio_helper import (
     get_domains_display,
     get_members_description_display,
     get_members_display,
+    get_portfolio_invitation_associations,
 )
 
 logger = logging.getLogger(__name__)
@@ -459,7 +460,14 @@ class PortfolioNewMemberForm(BasePortfolioMemberForm):
             if hasattr(e, "code"):
                 field = "email" if "email" in self.fields else None
                 if e.code == "has_existing_permissions":
-                    self.add_error(field, f"{self.instance.email} is already a member of another .gov organization.")
+                    existing_permissions, existing_invitations = get_portfolio_invitation_associations(self.instance)
+
+                    same_portfolio_for_permissions = existing_permissions.exclude(portfolio=self.instance.portfolio)
+                    same_portfolio_for_invitations = existing_invitations.exclude(portfolio=self.instance.portfolio)
+                    if same_portfolio_for_permissions.exists() or same_portfolio_for_invitations.exists():
+                        self.add_error(
+                            field, f"{self.instance.email} is already a member of another .gov organization."
+                        )
                     override_error = True
                 elif e.code == "has_existing_invitations":
                     self.add_error(

src/registrar/models/utility/portfolio_helper.py

@@ -257,9 +257,6 @@ def validate_user_portfolio_permission(user_portfolio_permission):
     Raises:
         ValidationError: If any of the validation rules are violated.
     """
-    PortfolioInvitation = apps.get_model("registrar.PortfolioInvitation")
-    UserPortfolioPermission = apps.get_model("registrar.UserPortfolioPermission")
-
     has_portfolio = bool(user_portfolio_permission.portfolio_id)
     portfolio_permissions = set(user_portfolio_permission._get_portfolio_permissions())
 
@@ -286,8 +283,8 @@ def validate_user_portfolio_permission(user_portfolio_permission):
 
     # == Validate the multiple_porfolios flag. == #
     if not flag_is_active_for_user(user_portfolio_permission.user, "multiple_portfolios"):
-        existing_permissions = UserPortfolioPermission.objects.exclude(id=user_portfolio_permission.id).filter(
+        existing_permissions, existing_invitations = get_user_portfolio_permission_associations(
-            user=user_portfolio_permission.user
+            user_portfolio_permission
         )
         if existing_permissions.exists():
             raise ValidationError(
@@ -296,10 +293,6 @@ def validate_user_portfolio_permission(user_portfolio_permission):
                 code="has_existing_permissions",
             )
 
-        existing_invitations = PortfolioInvitation.objects.filter(email=user_portfolio_permission.user.email).exclude(
-            Q(portfolio=user_portfolio_permission.portfolio)
-            | Q(status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED)
-        )
         if existing_invitations.exists():
             raise ValidationError(
                 "This user is already assigned to a portfolio invitation. "
@@ -308,6 +301,32 @@ def validate_user_portfolio_permission(user_portfolio_permission):
             )
 
 
+def get_user_portfolio_permission_associations(user_portfolio_permission):
+    """
+    Retrieves the associations for a user portfolio invitation.
+
+    Returns:
+      A tuple:
+        (existing_permissions, existing_invitations)
+      where:
+        - existing_permissions: UserPortfolioPermission objects excluding the current permission.
+        - existing_invitations: PortfolioInvitation objects for the user email excluding
+        the current invitation and those with status RETRIEVED.
+    """
+    PortfolioInvitation = apps.get_model("registrar.PortfolioInvitation")
+    UserPortfolioPermission = apps.get_model("registrar.UserPortfolioPermission")
+    existing_permissions = UserPortfolioPermission.objects.exclude(id=user_portfolio_permission.id).filter(
+        user=user_portfolio_permission.user
+    )
+    existing_invitations = PortfolioInvitation.objects.filter(
+        email__iexact=user_portfolio_permission.user.email
+    ).exclude(
+        Q(portfolio=user_portfolio_permission.portfolio)
+        | Q(status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED)
+    )
+    return (existing_permissions, existing_invitations)
+
+
 def validate_portfolio_invitation(portfolio_invitation):
     """
     Validates a PortfolioInvitation instance. Located in portfolio_helper to avoid circular imports
@@ -324,7 +343,6 @@ def validate_portfolio_invitation(portfolio_invitation):
     Raises:
         ValidationError: If any of the validation rules are violated.
     """
-    PortfolioInvitation = apps.get_model("registrar.PortfolioInvitation")
     UserPortfolioPermission = apps.get_model("registrar.UserPortfolioPermission")
     User = get_user_model()
 
@@ -351,17 +369,12 @@ def validate_portfolio_invitation(portfolio_invitation):
         )
 
     # == Validate the multiple_porfolios flag. == #
-    user = User.objects.filter(email=portfolio_invitation.email).first()
+    user = User.objects.filter(email__iexact=portfolio_invitation.email).first()
 
     # If user returns None, then we check for global assignment of multiple_portfolios.
     # Otherwise we just check on the user.
     if not flag_is_active_for_user(user, "multiple_portfolios"):
-        existing_permissions = UserPortfolioPermission.objects.filter(user=user)
+        existing_permissions, existing_invitations = get_portfolio_invitation_associations(portfolio_invitation)
-
-        existing_invitations = PortfolioInvitation.objects.filter(email=portfolio_invitation.email).exclude(
-            Q(id=portfolio_invitation.id) | Q(status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED)
-        )
-
         if existing_permissions.exists():
             raise ValidationError(
                 "This user is already assigned to a portfolio. "
@@ -377,6 +390,27 @@ def validate_portfolio_invitation(portfolio_invitation):
             )
 
 
+def get_portfolio_invitation_associations(portfolio_invitation):
+    """
+    Retrieves the associations for a portfolio invitation.
+
+    Returns:
+      A tuple:
+        (existing_permissions, existing_invitations)
+      where:
+        - existing_permissions: UserPortfolioPermission objects matching the email.
+        - existing_invitations: PortfolioInvitation objects for the email excluding
+        the current invitation and those with status RETRIEVED.
+    """
+    PortfolioInvitation = apps.get_model("registrar.PortfolioInvitation")
+    UserPortfolioPermission = apps.get_model("registrar.UserPortfolioPermission")
+    existing_permissions = UserPortfolioPermission.objects.filter(user__email__iexact=portfolio_invitation.email)
+    existing_invitations = PortfolioInvitation.objects.filter(email__iexact=portfolio_invitation.email).exclude(
+        Q(id=portfolio_invitation.id) | Q(status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED)
+    )
+    return (existing_permissions, existing_invitations)
+
+
 def cleanup_after_portfolio_member_deletion(portfolio, email, user=None):
     """
     Cleans up after removing a portfolio member or a portfolio invitation.

src/registrar/tests/test_views_portfolio.py

@@ -3930,17 +3930,59 @@ class TestPortfolioInviteNewMemberView(MockEppLib, WebTest):
         response = self.client.post(
             reverse("new-member"),
             {
-                "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value,
+                "role": UserPortfolioRoleChoices.ORGANIZATION_ADMIN,
-                "domain_request_permission_member": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value,
                 "email": self.user.email,
             },
+            follow=True,
         )
         self.assertEqual(response.status_code, 200)
+        with open("debug_response.html", "w") as f:
+            f.write(response.content.decode("utf-8"))
 
         # Verify messages
         self.assertContains(
             response,
-            f"{self.user.email} is already a member of another .gov organization.",
+            "User is already a member of this portfolio.",
+        )
+
+        # Validate Database has not changed
+        invite_count_after = PortfolioInvitation.objects.count()
+        self.assertEqual(invite_count_after, invite_count_before)
+
+        # assert that send_portfolio_invitation_email is not called
+        mock_send_email.assert_not_called()
+
+    @less_console_noise_decorator
+    @override_flag("organization_feature", active=True)
+    @override_flag("organization_members", active=True)
+    @patch("registrar.views.portfolios.send_portfolio_invitation_email")
+    def test_member_invite_for_existing_member_uppercase(self, mock_send_email):
+        """Tests the member invitation flow for existing portfolio member with a different case."""
+        self.client.force_login(self.user)
+
+        # Simulate a session to ensure continuity
+        session_id = self.client.session.session_key
+        self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
+
+        invite_count_before = PortfolioInvitation.objects.count()
+
+        # Simulate submission of member invite for user who has already been invited
+        response = self.client.post(
+            reverse("new-member"),
+            {
+                "role": UserPortfolioRoleChoices.ORGANIZATION_ADMIN,
+                "email": self.user.email.upper(),
+            },
+            follow=True,
+        )
+        self.assertEqual(response.status_code, 200)
+        with open("debug_response.html", "w") as f:
+            f.write(response.content.decode("utf-8"))
+
+        # Verify messages
+        self.assertContains(
+            response,
+            "User is already a member of this portfolio.",
         )
 
         # Validate Database has not changed

src/registrar/views/portfolios.py

@@ -970,7 +970,7 @@ class PortfolioAddMemberView(DetailView, FormMixin):
         portfolio = form.cleaned_data["portfolio"]
         is_admin_invitation = UserPortfolioRoleChoices.ORGANIZATION_ADMIN in form.cleaned_data["roles"]
 
-        requested_user = User.objects.filter(email=requested_email).first()
+        requested_user = User.objects.filter(email__iexact=requested_email).first()
         permission_exists = UserPortfolioPermission.objects.filter(user=requested_user, portfolio=portfolio).exists()
         try:
             if not requested_user or not permission_exists: