Add unit test on multiple portfolio flag

src/registrar/models/user_portfolio_permission.py

@@ -28,14 +28,14 @@ class UserPortfolioPermission(TimeStampedModel):
             UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION,
             UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION,
         ],
-        # NOTE: We currently forbid members from posessing view_members or view_all_domains.
+        # NOTE: Check FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS before adding roles here.
-        # If those are added here, clean() will throw errors.
         UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [
             UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
         ],
     }
 
     # Determines which roles are forbidden for certain role types to possess.
+    # Used to throw a ValidationError on clean() for UserPortfolioPermission and PortfolioInvitation.
     FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS = {
         UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [
             UserPortfolioPermissionChoices.VIEW_MEMBERS,
@@ -155,7 +155,10 @@ class UserPortfolioPermission(TimeStampedModel):
     @classmethod
     def get_forbidden_permissions(cls, roles, additional_permissions):
         """Some permissions are forbidden for certain roles, like member.
-        This checks for conflicts between the role and additional_permissions."""
+        This checks for conflicts between the current permission list and forbidden perms."""
+
+        # Get the portfolio permissions that the user currently possesses
+        portfolio_permissions = set(cls.get_portfolio_permissions(roles, additional_permissions))
 
         # Get intersection of forbidden permissions across all roles.
         # This is because if you have roles ["admin", "member"], then they can have the
@@ -169,8 +172,7 @@ class UserPortfolioPermission(TimeStampedModel):
 
         # Check if the users current permissions overlap with any forbidden permissions
         # by getting the intersection between current user permissions, and forbidden ones.
-        # This is the same as portfolio_permissions & common_forbidden_perms. 
+        # This is the same as portfolio_permissions & common_forbidden_perms.
-        portfolio_permissions = set(cls.get_portfolio_permissions(roles, additional_permissions))
         return portfolio_permissions.intersection(common_forbidden_perms)
 
     def clean(self):

src/registrar/models/utility/portfolio_helper.py

@@ -107,7 +107,7 @@ def validate_user_portfolio_permission(user_portfolio_permission):
     # == Validate role permissions. Compares existing permissions to forbidden ones. == #
     roles = user_portfolio_permission.roles if user_portfolio_permission.roles is not None else []
     bad_perms = user_portfolio_permission.get_forbidden_permissions(
-        user_portfolio_permission.roles, user_portfolio_permission.additional_permissions
+        roles, user_portfolio_permission.additional_permissions
     )
     if bad_perms:
         readable_perms = [
@@ -118,19 +118,18 @@ def validate_user_portfolio_permission(user_portfolio_permission):
             f"These permissions cannot be assigned to {', '.join(readable_roles)}: <{', '.join(readable_perms)}>"
         )
 
+    # == 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(
             user=user_portfolio_permission.user
         )
-
-        existing_invitations = PortfolioInvitation.objects.filter(email=user_portfolio_permission.user.email)
-
         if existing_permissions.exists():
             raise ValidationError(
                 "This user is already assigned to a portfolio. "
                 "Based on current waffle flag settings, users cannot be assigned to multiple portfolios."
             )
 
+        existing_invitations = PortfolioInvitation.objects.filter(email=user_portfolio_permission.user.email)
         if existing_invitations.exists():
             raise ValidationError(
                 "This user is already assigned to a portfolio invitation. "
@@ -168,10 +167,9 @@ def validate_portfolio_invitation(portfolio_invitation):
     if has_portfolio and not portfolio_permissions:
         raise ValidationError("When portfolio is assigned, portfolio roles or additional permissions are required.")
 
+    # == Validate role permissions. Compares existing permissions to forbidden ones. == #
     roles = portfolio_invitation.roles if portfolio_invitation.roles is not None else []
-    bad_perms = UserPortfolioPermission.get_forbidden_permissions(
+    bad_perms = UserPortfolioPermission.get_forbidden_permissions(roles, portfolio_invitation.additional_permissions)
-        portfolio_invitation.roles, portfolio_invitation.additional_permissions
-    )
     if bad_perms:
         readable_perms = [
             UserPortfolioPermissionChoices.get_user_portfolio_permission_label(perm) for perm in bad_perms
@@ -181,6 +179,7 @@ def validate_portfolio_invitation(portfolio_invitation):
             f"These permissions cannot be assigned to {', '.join(readable_roles)}: <{', '.join(readable_perms)}>"
         )
 
+    # == Validate the multiple_porfolios flag. == #
     user = User.objects.filter(email=portfolio_invitation.email).first()
     if not flag_is_active_for_user(user, "multiple_portfolios"):
         existing_permissions = UserPortfolioPermission.objects.filter(user=user)

src/registrar/tests/test_admin.py

@@ -2,6 +2,7 @@ from datetime import datetime
 from django.utils import timezone
 from django.test import TestCase, RequestFactory, Client
 from django.contrib.admin.sites import AdminSite
+from waffle.testutils import override_flag
 from django_webtest import WebTest  # type: ignore
 from api.tests.common import less_console_noise_decorator
 from django.urls import reverse
@@ -209,13 +210,13 @@ class TestUserPortfolioPermissionAdmin(TestCase):
         User.objects.all().delete()
 
     @less_console_noise_decorator
-    def test_validate_user_portfolio_permission(self):
+    def test_clean_user_portfolio_permission(self):
         """Tests validation of user portfolio permission"""
 
         # Test validation fails when portfolio missing but permissions are present
         permission = UserPortfolioPermission(user=self.superuser, roles=["organization_admin"], portfolio=None)
         with self.assertRaises(ValidationError) as err:
-            validate_user_portfolio_permission(permission)
+            permission.clean()
             self.assertEqual(
                 str(err.exception),
                 "When portfolio roles or additional permissions are assigned, portfolio is required.",
@@ -224,7 +225,7 @@ class TestUserPortfolioPermissionAdmin(TestCase):
         # Test validation fails when portfolio present but no permissions are present
         permission = UserPortfolioPermission(user=self.superuser, roles=None, portfolio=self.portfolio)
         with self.assertRaises(ValidationError) as err:
-            validate_user_portfolio_permission(permission)
+            permission.clean()
             self.assertEqual(
                 str(err.exception),
                 "When portfolio is assigned, portfolio roles or additional permissions are required.",
@@ -307,7 +308,73 @@ class TestPortfolioInvitationAdmin(TestCase):
         User.objects.all().delete()
 
     @less_console_noise_decorator
-    def test_portfolio_invitation_clean(self):
+    @override_flag("multiple_portfolios", active=False)
+    def test_clean_multiple_portfolios_inactive(self):
+        """Tests that users cannot have multiple portfolios or invitations when flag is inactive"""
+        # Create the first portfolio permission
+        UserPortfolioPermission.objects.create(
+            user=self.superuser, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
+        )
+
+        # Test a second portfolio permission object (should fail)
+        second_portfolio = Portfolio.objects.create(organization_name="Second Portfolio", creator=self.superuser)
+        second_permission = UserPortfolioPermission(
+            user=self.superuser, portfolio=second_portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
+        )
+
+        with self.assertRaises(ValidationError) as err:
+            second_permission.clean()
+        self.assertIn("users cannot be assigned to multiple portfolios", str(err.exception))
+
+        # Test that adding a new portfolio invitation also fails
+        third_portfolio = Portfolio.objects.create(organization_name="Third Portfolio", creator=self.superuser)
+        invitation = PortfolioInvitation(
+            email=self.superuser.email, portfolio=third_portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
+        )
+
+        with self.assertRaises(ValidationError) as err:
+            invitation.clean()
+        self.assertIn("users cannot be assigned to multiple portfolios", str(err.exception))
+
+    @less_console_noise_decorator
+    @override_flag("multiple_portfolios", active=True)
+    def test_clean_multiple_portfolios_active(self):
+        """Tests that users can have multiple portfolios and invitations when flag is active"""
+        # Create first portfolio permission
+        UserPortfolioPermission.objects.create(
+            user=self.superuser, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
+        )
+
+        # Second portfolio permission should succeed
+        second_portfolio = Portfolio.objects.create(organization_name="Second Portfolio", creator=self.superuser)
+        second_permission = UserPortfolioPermission(
+            user=self.superuser, portfolio=second_portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
+        )
+        second_permission.clean()
+        second_permission.save()
+
+        # Verify both permissions exist
+        user_permissions = UserPortfolioPermission.objects.filter(user=self.superuser)
+        self.assertEqual(user_permissions.count(), 2)
+
+        # Portfolio invitation should also succeed
+        third_portfolio = Portfolio.objects.create(organization_name="Third Portfolio", creator=self.superuser)
+        invitation = PortfolioInvitation(
+            email=self.superuser.email, portfolio=third_portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
+        )
+        invitation.clean()
+        invitation.save()
+
+        # Verify invitation exists
+        self.assertTrue(
+            PortfolioInvitation.objects.filter(
+                email=self.superuser.email,
+                portfolio=third_portfolio,
+            ).exists()
+        )
+
+    @less_console_noise_decorator
+    def test_clean_portfolio_invitation(self):
         """Tests validation of portfolio invitation permissions"""
 
         # Test validation fails when portfolio missing but permissions present