Merge pull request #2566 from cisagov/dk/2519-user-table-portfolio

Issue #2519: On User table, portfolio cannot be added without role or permission

src/registrar/models/user.py

@@ -3,6 +3,7 @@ import logging
 from django.contrib.auth.models import AbstractUser
 from django.db import models
 from django.db.models import Q
+from django.forms import ValidationError
 
 from registrar.models.domain_information import DomainInformation
 from registrar.models.user_domain_role import UserDomainRole
@@ -229,6 +230,16 @@ class User(AbstractUser):
     def has_contact_info(self):
         return bool(self.title or self.email or self.phone)
 
+    def clean(self):
+        """Extends clean method to perform additional validation, which can raise errors in django admin."""
+        super().clean()
+
+        if self.portfolio is None and self._get_portfolio_permissions():
+            raise ValidationError("When portfolio roles or additional permissions are assigned, portfolio is required.")
+
+        if self.portfolio is not None and not self._get_portfolio_permissions():
+            raise ValidationError("When portfolio is assigned, portfolio roles or additional permissions are required.")
+
     def _get_portfolio_permissions(self):
         """
         Retrieve the permissions for the user's portfolio roles.

src/registrar/tests/test_models.py

@@ -1,3 +1,4 @@
+from django.forms import ValidationError
 from django.test import TestCase
 from django.db.utils import IntegrityError
 from django.db import transaction
@@ -1348,6 +1349,7 @@ class TestUser(TestCase):
         self.user.phone = None
         self.assertFalse(self.user.has_contact_info())
 
+    @less_console_noise_decorator
     def test_has_portfolio_permission(self):
         """
         0. Returns False when user does not have a permission
@@ -1401,6 +1403,37 @@ class TestUser(TestCase):
 
         Portfolio.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_user_with_portfolio_but_no_roles(self):
+        # Create an instance of User with a portfolio but no roles or additional permissions
+        portfolio, _ = Portfolio.objects.get_or_create(creator=self.user, organization_name="Hotel California")
+
+        self.user.portfolio = portfolio
+        self.user.portfolio_roles = []
+
+        # Test if the ValidationError is raised with the correct message
+        with self.assertRaises(ValidationError) as cm:
+            self.user.clean()
+
+        self.assertEqual(
+            cm.exception.message, "When portfolio is assigned, portfolio roles or additional permissions are required."
+        )
+        Portfolio.objects.all().delete()
+
+    @less_console_noise_decorator
+    def test_user_with_portfolio_roles_but_no_portfolio(self):
+        # Create an instance of User with a portfolio role but no portfolio
+        self.user.portfolio = None
+        self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
+
+        # Test if the ValidationError is raised with the correct message
+        with self.assertRaises(ValidationError) as cm:
+            self.user.clean()
+
+        self.assertEqual(
+            cm.exception.message, "When portfolio roles or additional permissions are assigned, portfolio is required."
+        )
+
 
 class TestContact(TestCase):
     @less_console_noise_decorator

src/zap.conf

@@ -71,6 +71,7 @@
 10038	OUTOFSCOPE	http://app:8080/domain_requests/
 10038	OUTOFSCOPE	http://app:8080/domains/
 10038	OUTOFSCOPE	http://app:8080/organization/
+10038	OUTOFSCOPE	http://app:8080/suborganization/
 # This URL always returns 404, so include it as well.
 10038	OUTOFSCOPE	http://app:8080/todo
 # OIDC isn't configured in the test environment and DEBUG=True so this gives a 500 without CSP headers