From 432ee9c860f1348cbc53c35309b1516ee0b8d077 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 19 Aug 2024 15:18:54 -0600 Subject: [PATCH] pt. 1 of test refactors --- src/registrar/admin.py | 8 +- .../0119_remove_user_portfolio_and_more.py | 6 +- src/registrar/models/portfolio_invitation.py | 4 +- src/registrar/models/user.py | 11 --- .../models/user_portfolio_permission.py | 22 +++-- src/registrar/tests/common.py | 6 +- src/registrar/tests/test_admin.py | 1 + src/registrar/tests/test_models.py | 48 ++++++---- src/registrar/tests/test_reports.py | 17 ++-- src/registrar/tests/test_views_domain.py | 20 ++--- src/registrar/tests/test_views_portfolio.py | 89 ++++++------------- 11 files changed, 100 insertions(+), 132 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index c91bbde48..fd555520d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -166,11 +166,11 @@ class UserPortfolioPermissionsForm(forms.ModelForm): model = models.UserPortfolioPermission fields = "__all__" widgets = { - "portfolio_roles": FilteredSelectMultipleArrayWidget( - "portfolio_roles", is_stacked=False, choices=UserPortfolioRoleChoices.choices + "roles": FilteredSelectMultipleArrayWidget( + "roles", is_stacked=False, choices=UserPortfolioRoleChoices.choices ), - "portfolio_additional_permissions": FilteredSelectMultipleArrayWidget( - "portfolio_additional_permissions", + "additional_permissions": FilteredSelectMultipleArrayWidget( + "additional_permissions", is_stacked=False, choices=UserPortfolioPermissionChoices.choices, ), diff --git a/src/registrar/migrations/0119_remove_user_portfolio_and_more.py b/src/registrar/migrations/0119_remove_user_portfolio_and_more.py index ee65a91b7..7c9d2defc 100644 --- a/src/registrar/migrations/0119_remove_user_portfolio_and_more.py +++ b/src/registrar/migrations/0119_remove_user_portfolio_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-08-16 20:41 +# Generated by Django 4.2.10 on 2024-08-19 20:24 from django.conf import settings import django.contrib.postgres.fields @@ -43,7 +43,7 @@ class Migration(migrations.Migration): ("created_at", models.DateTimeField(auto_now_add=True)), ("updated_at", models.DateTimeField(auto_now=True)), ( - "portfolio_roles", + "roles", django.contrib.postgres.fields.ArrayField( base_field=models.CharField( choices=[ @@ -60,7 +60,7 @@ class Migration(migrations.Migration): ), ), ( - "portfolio_additional_permissions", + "additional_permissions", django.contrib.postgres.fields.ArrayField( base_field=models.CharField( choices=[ diff --git a/src/registrar/models/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py index de7bda893..46d7bf124 100644 --- a/src/registrar/models/portfolio_invitation.py +++ b/src/registrar/models/portfolio_invitation.py @@ -89,7 +89,7 @@ class PortfolioInvitation(TimeStampedModel): portfolio=self.portfolio, user=user ) if self.portfolio_roles and len(self.portfolio_roles) > 0: - user_portfolio_permission.portfolio_roles = self.portfolio_roles + user_portfolio_permission.roles = self.portfolio_roles if self.portfolio_additional_permissions and len(self.portfolio_additional_permissions) > 0: - user_portfolio_permission.portfolio_additional_permissions = self.portfolio_additional_permissions + user_portfolio_permission.additional_permissions = self.portfolio_additional_permissions user_portfolio_permission.save() diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 084b1926a..34f2f8bcf 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -208,17 +208,6 @@ 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() - - portfolio_perm = self.portfolio_permissions.filter(portfolio=self.last_selected_portfolio, user=self).first() - if self.last_selected_portfolio is None and portfolio_perm._get_portfolio_permissions(): - raise ValidationError("When portfolio roles or additional permissions are assigned, portfolio is required.") - - if self.last_selected_portfolio is not None and not portfolio_perm._get_portfolio_permissions(): - raise ValidationError("When portfolio is assigned, portfolio roles or additional permissions are required.") - def _has_portfolio_permission(self, portfolio_permission): """The views should only call this function when testing for perms and not rely on roles.""" diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 77639f569..e00bf37f9 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -54,7 +54,7 @@ class UserPortfolioPermission(TimeStampedModel): related_name="portfolio_users", ) - portfolio_roles = ArrayField( + roles = ArrayField( models.CharField( max_length=50, choices=UserPortfolioRoleChoices.choices, @@ -64,7 +64,7 @@ class UserPortfolioPermission(TimeStampedModel): help_text="Select one or more roles.", ) - portfolio_additional_permissions = ArrayField( + additional_permissions = ArrayField( models.CharField( max_length=50, choices=UserPortfolioPermissionChoices.choices, @@ -76,8 +76,8 @@ class UserPortfolioPermission(TimeStampedModel): def __str__(self): return ( - f"User '{self.user}' on Portfolio '{self.portfolio}' " f"" - if self.portfolio_roles + f"User '{self.user}' on Portfolio '{self.portfolio}' " f"" + if self.roles else "" ) @@ -88,12 +88,12 @@ class UserPortfolioPermission(TimeStampedModel): # Use a set to avoid duplicate permissions portfolio_permissions = set() - if self.portfolio_roles: - for role in self.portfolio_roles: + if self.roles: + for role in self.roles: portfolio_permissions.update(self.PORTFOLIO_ROLE_PERMISSIONS.get(role, [])) - if self.portfolio_additional_permissions: - portfolio_permissions.update(self.portfolio_additional_permissions) + if self.additional_permissions: + portfolio_permissions.update(self.additional_permissions) return list(portfolio_permissions) @@ -107,3 +107,9 @@ class UserPortfolioPermission(TimeStampedModel): raise ValidationError( "Only one portfolio permission is allowed per user when multiple portfolios are disabled." ) + + 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.") diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index ceb3b6e92..f3fd6709f 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -906,7 +906,7 @@ def completed_domain_request( # noqa federal_agency=None, federal_type=None, action_needed_reason=None, - portfolio=None, + last_selected_portfolio=None, ): """A completed domain request.""" if not user: @@ -977,8 +977,8 @@ def completed_domain_request( # noqa if action_needed_reason: domain_request_kwargs["action_needed_reason"] = action_needed_reason - if portfolio: - domain_request_kwargs["portfolio"] = portfolio + if last_selected_portfolio: + domain_request_kwargs["last_selected_portfolio"] = last_selected_portfolio domain_request, _ = DomainRequest.objects.get_or_create(**domain_request_kwargs) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 827742ef1..49d2b4802 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1209,6 +1209,7 @@ class TestMyUserAdmin(MockDbForSharedTests): domain_deleted.delete() role.delete() + # TODO - should refactor def test_analyst_cannot_see_selects_for_portfolio_role_and_permissions_in_user_form(self): """Can only test for the presence of a base element. The multiselects and the h2->h3 conversion are all dynamically generated.""" diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index f4e998fff..1c95257d3 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -17,6 +17,7 @@ from registrar.models import ( DomainInvitation, UserDomainRole, FederalAgency, + UserPortfolioPermission, ) import boto3_mocking @@ -1151,10 +1152,11 @@ class TestPortfolioInvitations(TestCase): self.assertFalse(self.user.portfolio) self.invitation.retrieve() self.user.refresh_from_db() - self.assertEqual(self.user.portfolio.organization_name, "Hotel California") - self.assertEqual(self.user.portfolio_roles, [self.portfolio_role_base, self.portfolio_role_admin]) + self.assertEqual(self.user.last_selected_portfolio.organization_name, "Hotel California") + portfolio_role, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio) + self.assertEqual(portfolio_role.roles, [self.portfolio_role_base, self.portfolio_role_admin]) self.assertEqual( - self.user.portfolio_additional_permissions, [self.portfolio_permission_1, self.portfolio_permission_2] + portfolio_role.additional_permissions, [self.portfolio_permission_1, self.portfolio_permission_2] ) self.assertEqual(self.invitation.status, PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED) @@ -1167,10 +1169,11 @@ class TestPortfolioInvitations(TestCase): @less_console_noise_decorator def test_retrieve_user_already_member_error(self): - self.assertFalse(self.user.portfolio) + self.assertFalse(self.user.last_selected_portfolio) portfolio2, _ = Portfolio.objects.get_or_create(creator=self.user2, organization_name="Tokyo Hotel") - self.user.portfolio = portfolio2 - self.assertEqual(self.user.portfolio.organization_name, "Tokyo Hotel") + portfolio_role, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=portfolio2) + self.assertEqual(self.user.last_selected_portfolio.organization_name, "Tokyo Hotel") + self.assertEqual(portfolio_role.portfolio, portfolio2) self.user.save() self.user.check_portfolio_invitations_on_login() self.user.refresh_from_db() @@ -1197,6 +1200,7 @@ class TestUser(TestCase): DomainRequest.objects.all().delete() DraftDomain.objects.all().delete() TransitionDomain.objects.all().delete() + UserPortfolioPermission.objects.all().delete() Portfolio.objects.all().delete() User.objects.all().delete() UserDomainRole.objects.all().delete() @@ -1360,8 +1364,7 @@ class TestUser(TestCase): Note: This tests _get_portfolio_permissions as a side effect """ portfolio, _ = Portfolio.objects.get_or_create(creator=self.user, organization_name="Hotel California") - - self.user.portfolio_additional_permissions = [UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS] + self.user.last_selected_portfolio = portfolio self.user.save() self.user.refresh_from_db() @@ -1371,8 +1374,10 @@ class TestUser(TestCase): self.assertFalse(user_can_view_all_domains) self.assertFalse(user_can_view_all_requests) - self.user.portfolio = portfolio - self.user.save() + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(portfolio=portfolio, user=self.user) + portfolio_permission.additional_permissions = [UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS] + portfolio_permission.save() + portfolio_permission.refresh_from_db() self.user.refresh_from_db() user_can_view_all_domains = self.user.has_domains_portfolio_permission() @@ -1381,8 +1386,10 @@ class TestUser(TestCase): self.assertTrue(user_can_view_all_domains) self.assertFalse(user_can_view_all_requests) - self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - self.user.save() + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(portfolio=portfolio, user=self.user) + portfolio_permission.roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + portfolio_permission.save() + portfolio_permission.refresh_from_db() self.user.refresh_from_db() user_can_view_all_domains = self.user.has_domains_portfolio_permission() @@ -1407,13 +1414,15 @@ class TestUser(TestCase): 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") + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(portfolio=portfolio, user=self.user) - self.user.portfolio = portfolio - self.user.portfolio_roles = [] + # Try to remove the role + portfolio_permission.portfolio = portfolio + portfolio_permission.roles = [] # Test if the ValidationError is raised with the correct message with self.assertRaises(ValidationError) as cm: - self.user.clean() + portfolio_permission.clean() self.assertEqual( cm.exception.message, "When portfolio is assigned, portfolio roles or additional permissions are required." @@ -1426,9 +1435,16 @@ class TestUser(TestCase): self.user.portfolio = None self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + 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) + + # Try to remove the portfolio + portfolio_permission.portfolio = None + portfolio_permission.roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + # Test if the ValidationError is raised with the correct message with self.assertRaises(ValidationError) as cm: - self.user.clean() + portfolio_permission.clean() self.assertEqual( cm.exception.message, "When portfolio roles or additional permissions are assigned, portfolio is required." diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 52aaa8c38..8440e54e1 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -7,6 +7,7 @@ from registrar.models import ( UserDomainRole, ) from registrar.models import Portfolio +from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices from registrar.utility.csv_export import ( DomainDataFull, @@ -321,8 +322,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): # Create a portfolio and assign it to the user portfolio = Portfolio.objects.create(creator=self.user, organization_name="Test Portfolio") - self.user.portfolio = portfolio - self.user.save() + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(portfolio=portfolio, user=self.user) UserDomainRole.objects.create(user=self.user, domain=self.domain_2) UserDomainRole.objects.filter(user=self.user, domain=self.domain_1).delete() @@ -336,8 +336,9 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): self.domain_3.domain_info.save() # Set up user permissions - self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - self.user.save() + portfolio_permission.roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + portfolio_permission.save() + portfolio_permission.refresh_from_db() self.user.refresh_from_db() # Create a request object @@ -354,16 +355,16 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): self.assertNotIn(self.domain_2.name, csv_content) # Test the output for readonly admin - self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY] - self.user.save() + portfolio_permission.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY] + portfolio_permission.save() self.assertIn(self.domain_1.name, csv_content) self.assertIn(self.domain_3.name, csv_content) self.assertNotIn(self.domain_2.name, csv_content) # Get the csv content - self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_MEMBER] - self.user.save() + portfolio_permission.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_MEMBER] + portfolio_permission.save() csv_content = self._run_domain_data_type_user_export(request) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 3a90543a2..f3f4d1051 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -36,6 +36,7 @@ from registrar.models import ( FederalAgency, Portfolio, Suborganization, + UserPortfolioPermission, ) from datetime import date, datetime, timedelta from django.utils import timezone @@ -328,9 +329,9 @@ class TestDomainDetail(TestDomainOverview): email="bogus@example.gov", phone="8003111234", title="test title", - portfolio=portfolio, - portfolio_roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + last_selected_portfolio=portfolio, ) + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]) domain, _ = Domain.objects.get_or_create(name="bogusdomain.gov") DomainInformation.objects.get_or_create(creator=user, domain=domain, portfolio=portfolio) self.client.force_login(user) @@ -1477,10 +1478,7 @@ class TestDomainSuborganization(TestDomainOverview): self.domain_information.refresh_from_db() # Add portfolio perms to the user object - self.user.portfolio = portfolio - self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - self.user.save() - self.user.refresh_from_db() + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]) self.assertEqual(self.domain_information.sub_organization, suborg) @@ -1536,10 +1534,7 @@ class TestDomainSuborganization(TestDomainOverview): self.domain_information.refresh_from_db() # Add portfolio perms to the user object - self.user.portfolio = portfolio - self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY] - self.user.save() - self.user.refresh_from_db() + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY]) self.assertEqual(self.domain_information.sub_organization, suborg) @@ -1577,10 +1572,7 @@ class TestDomainSuborganization(TestDomainOverview): self.domain_information.refresh_from_db() # Add portfolio perms to the user object - self.user.portfolio = portfolio - self.user.portfolio_additional_permissions = [UserPortfolioPermissionChoices.VIEW_PORTFOLIO] - self.user.save() - self.user.refresh_from_db() + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.VIEW_PORTFOLIO]) # Navigate to the domain overview page page = self.app.get(reverse("domain", kwargs={"pk": self.domain.id})) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 60764cf1c..cb72f29fe 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -10,6 +10,7 @@ from registrar.models import ( UserDomainRole, User, ) +from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from .common import create_test_user from waffle.testutils import override_flag @@ -52,10 +53,7 @@ class TestPortfolio(WebTest): self.portfolio.save() self.portfolio.refresh_from_db() - self.user.portfolio = self.portfolio - self.user.portfolio_additional_permissions = [UserPortfolioPermissionChoices.VIEW_PORTFOLIO] - self.user.save() - self.user.refresh_from_db() + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, additional_permissions=[UserPortfolioPermissionChoices.VIEW_PORTFOLIO]) so_portfolio_page = self.app.get(reverse("senior-official")) # Assert that we're on the right page @@ -72,6 +70,7 @@ class TestPortfolio(WebTest): def test_middleware_does_not_redirect_if_no_permission(self): """Test that user with no portfolio permission is not redirected when attempting to access home""" self.app.set_user(self.user.username) + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, additional_permissions=[]) self.user.portfolio = self.portfolio self.user.save() self.user.refresh_from_db() @@ -86,9 +85,7 @@ class TestPortfolio(WebTest): def test_middleware_does_not_redirect_if_no_portfolio(self): """Test that user with no assigned portfolio is not redirected when attempting to access home""" self.app.set_user(self.user.username) - self.user.portfolio_additional_permissions = [UserPortfolioPermissionChoices.VIEW_PORTFOLIO] - self.user.save() - self.user.refresh_from_db() + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, additional_permissions=[UserPortfolioPermissionChoices.VIEW_PORTFOLIO]) with override_flag("organization_feature", active=True): # This will redirect the user to the portfolio page. # Follow implicity checks if our redirect is working. @@ -100,10 +97,7 @@ class TestPortfolio(WebTest): def test_middleware_redirects_to_portfolio_organization_page(self): """Test that user with a portfolio and VIEW_PORTFOLIO is redirected to portfolio organization page""" self.app.set_user(self.user.username) - self.user.portfolio = self.portfolio - self.user.portfolio_additional_permissions = [UserPortfolioPermissionChoices.VIEW_PORTFOLIO] - self.user.save() - self.user.refresh_from_db() + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, additional_permissions=[UserPortfolioPermissionChoices.VIEW_PORTFOLIO]) with override_flag("organization_feature", active=True): # This will redirect the user to the portfolio page. # Follow implicity checks if our redirect is working. @@ -117,13 +111,7 @@ class TestPortfolio(WebTest): """Test that user with a portfolio, VIEW_PORTFOLIO, VIEW_ALL_DOMAINS is redirected to portfolio domains page""" self.app.set_user(self.user.username) - self.user.portfolio = self.portfolio - self.user.portfolio_additional_permissions = [ - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, - ] - self.user.save() - self.user.refresh_from_db() + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, additional_permissions=[UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS]) with override_flag("organization_feature", active=True): # This will redirect the user to the portfolio page. # Follow implicity checks if our redirect is working. @@ -137,9 +125,7 @@ class TestPortfolio(WebTest): def test_portfolio_domains_page_403_when_user_not_have_permission(self): """Test that user without proper permission is denied access to portfolio domain view""" self.app.set_user(self.user.username) - self.user.portfolio = self.portfolio - self.user.save() - self.user.refresh_from_db() + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, additional_permissions=[]) with override_flag("organization_feature", active=True): # This will redirect the user to the portfolio page. # Follow implicity checks if our redirect is working. @@ -151,9 +137,7 @@ class TestPortfolio(WebTest): def test_portfolio_domain_requests_page_403_when_user_not_have_permission(self): """Test that user without proper permission is denied access to portfolio domain view""" self.app.set_user(self.user.username) - self.user.portfolio = self.portfolio - self.user.save() - self.user.refresh_from_db() + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, additional_permissions=[]) with override_flag("organization_feature", active=True): # This will redirect the user to the portfolio page. # Follow implicity checks if our redirect is working. @@ -165,9 +149,7 @@ class TestPortfolio(WebTest): def test_portfolio_organization_page_403_when_user_not_have_permission(self): """Test that user without proper permission is not allowed access to portfolio organization page""" self.app.set_user(self.user.username) - self.user.portfolio = self.portfolio - self.user.save() - self.user.refresh_from_db() + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, additional_permissions=[]) with override_flag("organization_feature", active=True): # This will redirect the user to the portfolio page. # Follow implicity checks if our redirect is working. @@ -179,12 +161,9 @@ class TestPortfolio(WebTest): def test_portfolio_organization_page_read_only(self): """Test that user with a portfolio can access the portfolio organization page, read only""" self.app.set_user(self.user.username) - self.user.portfolio = self.portfolio + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, additional_permissions=[UserPortfolioPermissionChoices.VIEW_PORTFOLIO]) self.portfolio.city = "Los Angeles" - self.user.portfolio_additional_permissions = [UserPortfolioPermissionChoices.VIEW_PORTFOLIO] self.portfolio.save() - self.user.save() - self.user.refresh_from_db() with override_flag("organization_feature", active=True): response = self.app.get(reverse("organization")) # Assert the response is a 200 @@ -200,15 +179,9 @@ class TestPortfolio(WebTest): def test_portfolio_organization_page_edit_access(self): """Test that user with a portfolio can access the portfolio organization page, read only""" self.app.set_user(self.user.username) - self.user.portfolio = self.portfolio - self.user.portfolio_additional_permissions = [ - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - UserPortfolioPermissionChoices.EDIT_PORTFOLIO, - ] + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, additional_permissions=[UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.EDIT_PORTFOLIO]) self.portfolio.city = "Los Angeles" self.portfolio.save() - self.user.save() - self.user.refresh_from_db() with override_flag("organization_feature", active=True): response = self.app.get(reverse("organization")) # Assert the response is a 200 @@ -224,14 +197,12 @@ class TestPortfolio(WebTest): def test_navigation_links_hidden_when_user_not_have_permission(self): """Test that navigation links are hidden when user does not have portfolio permissions""" self.app.set_user(self.user.username) - self.user.portfolio = self.portfolio - self.user.portfolio_additional_permissions = [ + portfolio_additional_permissions = [ UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, ] - self.user.save() - self.user.refresh_from_db() + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, additional_permissions=portfolio_additional_permissions) with override_flag("organization_feature", active=True): # This will redirect the user to the portfolio page. # Follow implicity checks if our redirect is working. @@ -245,7 +216,8 @@ class TestPortfolio(WebTest): # removing non-basic portfolio perms, which should remove domains # and domain requests from nav - self.user.portfolio_additional_permissions = [UserPortfolioPermissionChoices.VIEW_PORTFOLIO] + portfolio_additional_permissions = [UserPortfolioPermissionChoices.VIEW_PORTFOLIO] + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, additional_permissions=portfolio_additional_permissions) self.user.save() self.user.refresh_from_db() @@ -261,10 +233,8 @@ class TestPortfolio(WebTest): def test_navigation_links_hidden_when_user_not_have_role(self): """Test that admin / memmber roles are associated with the right access""" self.app.set_user(self.user.username) - self.user.portfolio = self.portfolio - self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - self.user.save() - self.user.refresh_from_db() + portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, roles=portfolio_roles) with override_flag("organization_feature", active=True): # This will redirect the user to the portfolio page. # Follow implicity checks if our redirect is working. @@ -278,9 +248,9 @@ class TestPortfolio(WebTest): # removing non-basic portfolio role, which should remove domains # and domain requests from nav - self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_MEMBER] - self.user.save() - self.user.refresh_from_db() + portfolio_permission.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_MEMBER] + portfolio_permission.save() + portfolio_permission.refresh_from_db() portfolio_page = self.app.get(reverse("home")).follow() @@ -295,14 +265,11 @@ class TestPortfolio(WebTest): """Can load portfolio's org name page.""" with override_flag("organization_feature", active=True): self.app.set_user(self.user.username) - self.user.portfolio = self.portfolio - self.user.portfolio_additional_permissions = [ + portfolio_additional_permissions = [ UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.EDIT_PORTFOLIO, ] - self.user.save() - self.user.refresh_from_db() - + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, additional_permissions=portfolio_additional_permissions) page = self.app.get(reverse("organization")) self.assertContains( page, "The name of your federal agency will be publicly listed as the domain registrant." @@ -313,13 +280,11 @@ class TestPortfolio(WebTest): """Org name and address information appears on the page.""" with override_flag("organization_feature", active=True): self.app.set_user(self.user.username) - self.user.portfolio = self.portfolio - self.user.portfolio_additional_permissions = [ + portfolio_additional_permissions = [ UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.EDIT_PORTFOLIO, ] - self.user.save() - self.user.refresh_from_db() + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, additional_permissions=portfolio_additional_permissions) self.portfolio.organization_name = "Hotel California" self.portfolio.save() @@ -333,13 +298,11 @@ class TestPortfolio(WebTest): """Submitting changes works on the org name address page.""" with override_flag("organization_feature", active=True): self.app.set_user(self.user.username) - self.user.portfolio = self.portfolio - self.user.portfolio_additional_permissions = [ + portfolio_additional_permissions = [ UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.EDIT_PORTFOLIO, ] - self.user.save() - self.user.refresh_from_db() + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, additional_permissions=portfolio_additional_permissions) self.portfolio.address_line1 = "1600 Penn Ave" self.portfolio.save()