diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 6811f2e55..86be4f9bf 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -35,7 +35,6 @@ from django_admin_multiple_choice_list_filter.list_filters import MultipleChoice from import_export import resources from import_export.admin import ImportExportModelAdmin from django.core.exceptions import ObjectDoesNotExist -from django.contrib.postgres.forms import SimpleArrayField from django.contrib.admin.widgets import FilteredSelectMultiple from django.utils.translation import gettext_lazy as _ @@ -91,6 +90,7 @@ class UserResource(resources.ModelResource): class Meta: model = models.User + class FilteredSelectMultipleArrayWidget(FilteredSelectMultiple): def __init__(self, verbose_name, is_stacked=False, choices=(), **kwargs): super().__init__(verbose_name, is_stacked, **kwargs) @@ -98,26 +98,25 @@ class FilteredSelectMultipleArrayWidget(FilteredSelectMultiple): def value_from_datadict(self, data, files, name): values = super().value_from_datadict(data, files, name) - # print(f'value_from_datadict - values: {values}') return values or [] def get_context(self, name, value, attrs): - # print(f'get_context - initial value: {value}') if value is None: value = [] elif isinstance(value, str): - value = value.split(',') - # print(f'get_context - processed value: {value}') - self.choices = [(choice, label) for choice, label in self.choices if choice in value] + [(choice, label) for choice, label in self.choices if choice not in value] - # print(f'get_context - choices: {self.choices}') + value = value.split(",") + self.choices = [(choice, label) for choice, label in self.choices if choice in value] + [ + (choice, label) for choice, label in self.choices if choice not in value + ] context = super().get_context(name, value, attrs) return context - + + class MyUserAdminForm(UserChangeForm): """This form utilizes the custom widget for its class's ManyToMany UIs. It inherits from UserChangeForm which has special handling for the password and username fields.""" - + class Meta: model = models.User fields = "__all__" @@ -125,8 +124,14 @@ class MyUserAdminForm(UserChangeForm): widgets = { "groups": NoAutocompleteFilteredSelectMultiple("groups", False), "user_permissions": NoAutocompleteFilteredSelectMultiple("user_permissions", False), - "portfolio_roles": FilteredSelectMultipleArrayWidget("portfolio_roles", is_stacked=False, choices=User.UserPortfolioRoleChoices.choices), - "portfolio_additional_permissions": FilteredSelectMultipleArrayWidget("portfolio_additional_permissions", is_stacked=False, choices=User.UserPortfolioPermissionChoices.choices), + "portfolio_roles": FilteredSelectMultipleArrayWidget( + "portfolio_roles", is_stacked=False, choices=User.UserPortfolioRoleChoices.choices + ), + "portfolio_additional_permissions": FilteredSelectMultipleArrayWidget( + "portfolio_additional_permissions", + is_stacked=False, + choices=User.UserPortfolioPermissionChoices.choices, + ), } def __init__(self, *args, **kwargs): diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index 2cb7c0280..810c8fafe 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -43,7 +43,13 @@ def is_production(request): def portfolio_permissions(request): """""" return { - "has_base_portfolio_permission": request.user.has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO), - "has_domains_portfolio_permission": request.user.has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_DOMAINS), - "has_domain_requests_portfolio_permission": request.user.has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_REQUESTS) + "has_base_portfolio_permission": request.user.has_portfolio_permission( + User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO + ), + "has_domains_portfolio_permission": request.user.has_portfolio_permission( + User.UserPortfolioPermissionChoices.VIEW_DOMAINS + ), + "has_domain_requests_portfolio_permission": request.user.has_portfolio_permission( + User.UserPortfolioPermissionChoices.VIEW_REQUESTS + ), } diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 8481b757b..d1e4470e0 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -23,7 +23,9 @@ class Portfolio(TimeStampedModel): # Stores who created this model. If no creator is specified in DJA, # then the creator will default to the current request user""" - creator = models.ForeignKey("registrar.User", on_delete=models.PROTECT, help_text="Associated user", related_name="creator", unique=False) + creator = models.ForeignKey( + "registrar.User", on_delete=models.PROTECT, help_text="Associated user", related_name="creator", unique=False + ) notes = models.TextField( null=True, diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 9d1f43d76..ef96bee38 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -63,7 +63,7 @@ class User(AbstractUser): class UserPortfolioRoleChoices(models.TextChoices): """ - Roles make it easier for admins to look at + Roles make it easier for admins to look at """ ORGANIZATION_ADMIN = "organization_admin", "Admin" @@ -71,8 +71,7 @@ class User(AbstractUser): ORGANIZATION_MEMBER = "organization_member", "Member" class UserPortfolioPermissionChoices(models.TextChoices): - """ - """ + """ """ VIEW_DOMAINS = "view_domains", "View all domains and domain reports" # EDIT_DOMAINS is really self.domains. We add is hear and leverage it in has_permission @@ -83,14 +82,13 @@ class User(AbstractUser): VIEW_MEMBER = "view_member", "View members" EDIT_MEMBER = "edit_member", "Create and edit members" - + VIEW_REQUESTS = "view_requests", "View requests" EDIT_REQUESTS = "edit_requests", "Create and edit requests" - + VIEW_PORTFOLIO = "view_portfolio", "View organization" EDIT_PORTFOLIO = "edit_portfolio", "Edit organization" - PORTFOLIO_ROLE_PERMISSIONS = { UserPortfolioRoleChoices.ORGANIZATION_ADMIN: [ UserPortfolioPermissionChoices.VIEW_DOMAINS, @@ -105,14 +103,13 @@ class User(AbstractUser): UserPortfolioPermissionChoices.VIEW_DOMAINS, UserPortfolioPermissionChoices.VIEW_MEMBER, UserPortfolioPermissionChoices.VIEW_REQUESTS, - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, ], UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, ], } - # #### Constants for choice fields #### RESTRICTED = "restricted" STATUS_CHOICES = ((RESTRICTED, RESTRICTED),) @@ -249,24 +246,22 @@ class User(AbstractUser): def has_contact_info(self): return bool(self.title or self.email or self.phone) - + def has_portfolio_permission(self, portfolio_permission): """The views should only call this guy when testing for perms and not rely on roles""" - print(f"IN has_portfolio_permission") - # EDIT_DOMAINS === user is a manager on a domain (has UserDomainRole) # NOTE: Should we check whether the domain is in the portfolio? if portfolio_permission == self.UserPortfolioPermissionChoices.EDIT_DOMAINS and self.domains.exists(): return True - + if not self.portfolio: return False - + portfolio_permissions = self._get_portfolio_permissions() - + return portfolio_permission in portfolio_permissions - + def _get_portfolio_permissions(self): """ Retrieve the permissions for the user's portfolio roles. diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index e3bc33c8f..d6518d316 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -149,20 +149,22 @@ class CheckPortfolioMiddleware: # user_portfolios = Portfolio.objects.filter(creator=request.user) permission_dict = portfolio_permissions(request) - has_portfolio_base_permission = permission_dict['has_base_portfolio_permission'] + has_portfolio_base_permission = permission_dict["has_base_portfolio_permission"] if has_portfolio_base_permission: portfolio = request.user.portfolio permission_dict = portfolio_permissions(request) - has_portfolio_domains_permission = permission_dict['has_domains_portfolio_permission'] + has_portfolio_domains_permission = permission_dict["has_domains_portfolio_permission"] if has_portfolio_domains_permission: portfolio_redirect = reverse("portfolio-domains", kwargs={"portfolio_id": portfolio.id}) else: # View organization is the lowest access - portfolio_redirect = reverse("portfolio-organization", kwargs={"portfolio_id": portfolio.id}) + portfolio_redirect = reverse( + "portfolio-organization", kwargs={"portfolio_id": portfolio.id} + ) return HttpResponseRedirect(portfolio_redirect) - + return None diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 23fc4c34d..781a815c4 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -3723,8 +3723,8 @@ class TestMyUserAdmin(MockDb): self.assertEqual(response.status_code, 200) - self.assertContains(response,"Portfolio roles:") - self.assertContains(response,"Portfolio additional permissions:") + self.assertContains(response, "Portfolio roles:") + self.assertContains(response, "Portfolio additional permissions:") class AuditedAdminTest(TestCase): diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index be0acf2fe..5e11779ed 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1217,13 +1217,13 @@ class TestUser(TestCase): def test_has_portfolio_permission(self): """ - 0. Returns False when user does not have a permission - 1. Returns False when a user does not have a portfolio - 2. Returns True when user has direct permission - 3. Returns True when user has permission through a role - 4. Returns True EDIT_DOMAINS when user does not have the perm but has UserDomainRole + 0. Returns False when user does not have a permission + 1. Returns False when a user does not have a portfolio + 2. Returns True when user has direct permission + 3. Returns True when user has permission through a role + 4. Returns True EDIT_DOMAINS when user does not have the perm but has UserDomainRole - Note: This tests _get_portfolio_permissions as a side effect + Note: This tests _get_portfolio_permissions as a side effect """ portfolio, _ = Portfolio.objects.get_or_create(creator=self.user, organization_name="Hotel California") @@ -1239,7 +1239,7 @@ class TestUser(TestCase): self.assertFalse(user_can_view_requests) self.assertFalse(user_can_edit_domains) - self.user.portfolio=portfolio + self.user.portfolio = portfolio self.user.save() self.user.refresh_from_db() @@ -1263,16 +1263,18 @@ class TestUser(TestCase): self.assertTrue(user_can_view_requests) self.assertFalse(user_can_edit_domains) - UserDomainRole.objects.all().get_or_create(user=self.user, domain=self.domain, role=UserDomainRole.Roles.MANAGER) + UserDomainRole.objects.all().get_or_create( + user=self.user, domain=self.domain, role=UserDomainRole.Roles.MANAGER + ) user_can_view_domains = self.user.has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_DOMAINS) user_can_view_requests = self.user.has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_REQUESTS) user_can_edit_domains = self.user.has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_DOMAINS) - + self.assertTrue(user_can_view_domains) self.assertTrue(user_can_view_requests) self.assertTrue(user_can_edit_domains) - + Portfolio.objects.all().delete() diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index ded04e31b..17764c35e 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -554,7 +554,6 @@ class ExportDataTest(MockDb, MockEppLib): csv_file.seek(0) # Read the content into a variable csv_content = csv_file.read() - print(csv_content) expected_content = ( # Header "Domain request,Status,Domain type,Federal type," diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 9f1f386f1..47459722a 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -1,45 +1,14 @@ -from unittest import skip -from unittest.mock import MagicMock, ANY, patch - -from django.conf import settings from django.urls import reverse -from django.contrib.auth import get_user_model - from api.tests.common import less_console_noise_decorator from registrar.models.portfolio import Portfolio - -from .common import MockEppLib, MockSESClient, create_user # type: ignore from django_webtest import WebTest # type: ignore -import boto3_mocking # type: ignore - -from registrar.utility.errors import ( - NameserverError, - NameserverErrorCodes, - SecurityEmailError, - SecurityEmailErrorCodes, - GenericError, - GenericErrorCodes, - DsDataError, - DsDataErrorCodes, -) - from registrar.models import ( DomainRequest, Domain, DomainInformation, - DomainInvitation, - Contact, - PublicContact, - Host, - HostIP, UserDomainRole, User, - FederalAgency, ) -from datetime import date, datetime, timedelta -from django.utils import timezone - -from .common import less_console_noise from .test_views import TestWithUser from waffle.testutils import override_flag @@ -61,7 +30,7 @@ class TestPortfolioViews(TestWithUser, WebTest): def test_middleware_does_not_redirect_if_no_permission(self): """""" self.app.set_user(self.user.username) - self.user.portfolio=self.portfolio + self.user.portfolio = self.portfolio self.user.save() self.user.refresh_from_db() with override_flag("organization_feature", active=True): @@ -89,7 +58,7 @@ class TestPortfolioViews(TestWithUser, WebTest): def test_middleware_redirects_to_portfolio_organization_page(self): """""" self.app.set_user(self.user.username) - self.user.portfolio=self.portfolio + self.user.portfolio = self.portfolio self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO] self.user.save() self.user.refresh_from_db() @@ -99,14 +68,17 @@ class TestPortfolioViews(TestWithUser, WebTest): portfolio_page = self.app.get(reverse("home")).follow() # Assert that we're on the right page self.assertContains(portfolio_page, self.portfolio.organization_name) - self.assertContains(portfolio_page, '