diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 1ea54acc7..74ef9901e 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -5,6 +5,13 @@ from registrar.models.utility.portfolio_helper import UserPortfolioPermissionCho from .utility.time_stamped_model import TimeStampedModel from django.contrib.postgres.fields import ArrayField +# ---Logger +import logging +from venv import logger +from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper +logger = logging.getLogger(__name__) + + class UserPortfolioPermission(TimeStampedModel): """This is a linking table that connects a user with a role on a portfolio.""" @@ -105,12 +112,15 @@ class UserPortfolioPermission(TimeStampedModel): if has_portfolio and not self._get_portfolio_permissions(): raise ValidationError("When portfolio is assigned, portfolio roles or additional permissions are required.") - + # Check if a user is set without accessing the related object. has_user = bool(self.user_id) + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKCYAN, f"***CLEANING***") if has_user: - existing_permissions = UserPortfolioPermission.objects.filter(user=self.user) - if not flag_is_active_for_user(self.user, "multiple_portfolios") and existing_permissions.exists(): + existing_permission_pks = UserPortfolioPermission.objects.filter(user=self.user).values_list("pk", flat=True) + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKCYAN, f"existing_permission_pks: {existing_permission_pks}") + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKCYAN, f"self pk: {self.pk}") + if not flag_is_active_for_user(self.user, "multiple_portfolios") and existing_permission_pks.exists() and not self.pk in existing_permission_pks: raise ValidationError( "This user is already assigned to a portfolio. " "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 015c67dab..681d04c92 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -41,7 +41,6 @@ from waffle.testutils import override_flag from api.tests.common import less_console_noise_decorator - @boto3_mocking.patching class TestDomainRequest(TestCase): @less_console_noise_decorator @@ -1274,6 +1273,7 @@ class TestUserPortfolioPermission(TestCase): @less_console_noise_decorator def setUp(self): self.user, _ = User.objects.get_or_create(email="mayor@igorville.gov") + self.user2, _ = User.objects.get_or_create(email="user2@igorville.gov", username="user2") super().setUp() def tearDown(self): @@ -1306,21 +1306,20 @@ class TestUserPortfolioPermission(TestCase): portfolio_permission_2.clean() except ValidationError as error: self.fail(f"Raised ValidationError unexpectedly: {error}") - + @less_console_noise_decorator @override_flag("multiple_portfolios", active=False) def test_clean_on_creates_multiple_portfolios(self): """Ensures that a user cannot create multiple portfolio permission objects when the flag is disabled""" - # Create an instance of User with a portfolio but no roles or additional permissions + # Create an instance of User with a single portfolio portfolio, _ = Portfolio.objects.get_or_create(creator=self.user, organization_name="Hotel California") - portfolio_2, _ = Portfolio.objects.get_or_create(creator=self.user, organization_name="Motel California") portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( portfolio=portfolio, user=self.user, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] ) + portfolio_2, _ = Portfolio.objects.get_or_create(creator=self.user, organization_name="Motel California") portfolio_permission_2 = UserPortfolioPermission( portfolio=portfolio_2, user=self.user, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] ) - # This should work as intended portfolio_permission.clean() @@ -1328,7 +1327,35 @@ class TestUserPortfolioPermission(TestCase): with self.assertRaises(ValidationError) as cm: portfolio_permission_2.clean() - portfolio_permission_2, _ = UserPortfolioPermission.objects.get_or_create(portfolio=portfolio, user=self.user) + self.assertEqual( + cm.exception.message, + ( + "This user is already assigned to a portfolio. " + "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." + ), + ) + + @less_console_noise_decorator + @override_flag("multiple_portfolios", active=False) + def test_multiple_portfolio_reassignment(self): + """Ensures that a user cannot be assigned to multiple portfolios based on reassignment""" + # Create an instance of two users with separate portfolios + portfolio, _ = Portfolio.objects.get_or_create(creator=self.user, organization_name="Hotel California") + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( + portfolio=portfolio, user=self.user, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + portfolio_2, _ = Portfolio.objects.get_or_create(creator=self.user2, organization_name="Motel California") + portfolio_permission_2 = UserPortfolioPermission( + portfolio=portfolio_2, user=self.user2, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + + # This should work as intended + portfolio_permission.clean() + portfolio_permission_2.clean() + + with self.assertRaises(ValidationError) as cm: + portfolio_permission_2.user = self.user + portfolio_permission_2.clean() self.assertEqual( cm.exception.message,