diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index 477711885..840df7389 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -61,19 +61,19 @@ def add_has_profile_feature_flag_to_context(request): def portfolio_permissions(request): """Make portfolio permissions for the request user available in global context""" default_context = { - "has_base_portfolio_permission": False, - "has_any_domains_portfolio_permission": False, - "has_any_requests_portfolio_permission": False, - "has_edit_request_portfolio_permission": False, - "has_view_suborganization_portfolio_permission": False, - "has_edit_suborganization_portfolio_permission": False, - "has_view_members_portfolio_permission": False, - "has_edit_members_portfolio_permission": False, - "portfolio": None, - "has_organization_feature_flag": False, - "has_organization_requests_flag": False, - "has_organization_members_flag": False, - } + "has_base_portfolio_permission": False, + "has_any_domains_portfolio_permission": False, + "has_any_requests_portfolio_permission": False, + "has_edit_request_portfolio_permission": False, + "has_view_suborganization_portfolio_permission": False, + "has_edit_suborganization_portfolio_permission": False, + "has_view_members_portfolio_permission": False, + "has_edit_members_portfolio_permission": False, + "portfolio": None, + "has_organization_feature_flag": False, + "has_organization_requests_flag": False, + "has_organization_members_flag": False, + } try: portfolio = request.session.get("portfolio") if portfolio: @@ -87,9 +87,7 @@ def portfolio_permissions(request): portfolio ), "has_any_domains_portfolio_permission": request.user.has_any_domains_portfolio_permission(portfolio), - "has_any_requests_portfolio_permission": request.user.has_any_requests_portfolio_permission( - portfolio - ), + "has_any_requests_portfolio_permission": request.user.has_any_requests_portfolio_permission(portfolio), "has_view_members_portfolio_permission": request.user.has_view_members_portfolio_permission(portfolio), "has_edit_members_portfolio_permission": request.user.has_edit_members_portfolio_permission(portfolio), "portfolio": portfolio, diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 4518cd728..12bfaa370 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -201,7 +201,7 @@ class User(AbstractUser): request = HttpRequest() request.user = self return flag_is_active(request, "organization_requests") - + def has_organization_members_flag(self): request = HttpRequest() request.user = self @@ -226,7 +226,7 @@ class User(AbstractUser): def has_view_all_domains_portfolio_permission(self, portfolio): """Determines if the current user can view all available domains in a given portfolio""" return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) - + def has_any_requests_portfolio_permission(self, portfolio): # BEGIN # Note code below is to add organization_request feature @@ -445,8 +445,6 @@ class User(AbstractUser): self.check_domain_invitations_on_login() self.check_portfolio_invitations_on_login() - # NOTE TO DAVE: I'd simply suggest that we move these functions outside of the user object, - # and move them to some sort of utility file. That way we aren't calling request inside here. def is_org_user(self, request): has_organization_feature_flag = flag_is_active(request, "organization_feature") portfolio = request.session.get("portfolio") diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index aa0b98c5a..7d97f7add 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -150,8 +150,11 @@ class CheckPortfolioMiddleware: elif request.session.get("portfolio"): # Edge case: User disables flag while already logged in request.session["portfolio"] = None + elif "portfolio" not in request.session: + # Set the portfolio in the session if its not already in it + request.session["portfolio"] = None - if request.session.get("portfolio"): + if request.user.is_org_user(request): if current_path == self.home: if request.user.has_any_domains_portfolio_permission(request.session["portfolio"]): portfolio_redirect = reverse("domains") diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index a52836694..843da428f 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1387,21 +1387,21 @@ class TestUser(TestCase): # Test if the user has no roles self.assertEqual(self.user.portfolio_role_summary(self.portfolio), []) - @patch('registrar.models.User._has_portfolio_permission') + @patch("registrar.models.User._has_portfolio_permission") def test_has_base_portfolio_permission(self, mock_has_permission): mock_has_permission.return_value = True self.assertTrue(self.user.has_base_portfolio_permission(self.portfolio)) mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.VIEW_PORTFOLIO) - @patch('registrar.models.User._has_portfolio_permission') + @patch("registrar.models.User._has_portfolio_permission") def test_has_edit_org_portfolio_permission(self, mock_has_permission): mock_has_permission.return_value = True self.assertTrue(self.user.has_edit_org_portfolio_permission(self.portfolio)) mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.EDIT_PORTFOLIO) - @patch('registrar.models.User._has_portfolio_permission') + @patch("registrar.models.User._has_portfolio_permission") def test_has_any_domains_portfolio_permission(self, mock_has_permission): mock_has_permission.side_effect = [False, True] # First permission false, second permission true @@ -1410,14 +1410,14 @@ class TestUser(TestCase): mock_has_permission.assert_any_call(self.portfolio, UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) mock_has_permission.assert_any_call(self.portfolio, UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS) - @patch('registrar.models.User._has_portfolio_permission') + @patch("registrar.models.User._has_portfolio_permission") def test_has_view_all_domains_portfolio_permission(self, mock_has_permission): mock_has_permission.return_value = True self.assertTrue(self.user.has_view_all_domains_portfolio_permission(self.portfolio)) mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) - @patch('registrar.models.User._has_portfolio_permission') + @patch("registrar.models.User._has_portfolio_permission") @override_flag("organization_requests", active=True) def test_has_any_requests_portfolio_permission(self, mock_has_permission): mock_has_permission.side_effect = [False, True] # First permission false, second permission true @@ -1427,28 +1427,28 @@ class TestUser(TestCase): mock_has_permission.assert_any_call(self.portfolio, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS) mock_has_permission.assert_any_call(self.portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS) - @patch('registrar.models.User._has_portfolio_permission') + @patch("registrar.models.User._has_portfolio_permission") def test_has_view_all_requests_portfolio_permission(self, mock_has_permission): mock_has_permission.return_value = True self.assertTrue(self.user.has_view_all_requests_portfolio_permission(self.portfolio)) mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS) - @patch('registrar.models.User._has_portfolio_permission') + @patch("registrar.models.User._has_portfolio_permission") def test_has_edit_request_portfolio_permission(self, mock_has_permission): mock_has_permission.return_value = True self.assertTrue(self.user.has_edit_request_portfolio_permission(self.portfolio)) mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS) - @patch('registrar.models.User._has_portfolio_permission') + @patch("registrar.models.User._has_portfolio_permission") def test_has_view_suborganization_portfolio_permission(self, mock_has_permission): mock_has_permission.return_value = True self.assertTrue(self.user.has_view_suborganization_portfolio_permission(self.portfolio)) mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION) - @patch('registrar.models.User._has_portfolio_permission') + @patch("registrar.models.User._has_portfolio_permission") def test_has_edit_suborganization_portfolio_permission(self, mock_has_permission): mock_has_permission.return_value = True diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 9e9ae128c..70a1269fe 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -711,7 +711,7 @@ class TestPortfolio(WebTest): requests_page = self.client.get(reverse("domain-requests")) # create new request btn - self.assertContains(requests_page, 'Start a new domain request') + self.assertContains(requests_page, "Start a new domain request") @less_console_noise_decorator @override_flag("organization_feature", active=True) @@ -720,7 +720,12 @@ class TestPortfolio(WebTest): """Test the nav contains a simple link to view requests Also test for the existence of the Create a new request btn on the requests page""" UserPortfolioPermission.objects.get_or_create( - user=self.user, portfolio=self.portfolio, roles=[UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS] + user=self.user, + portfolio=self.portfolio, + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + ], ) self.client.force_login(self.user) # create and submit a domain request @@ -744,7 +749,7 @@ class TestPortfolio(WebTest): requests_page = self.client.get(reverse("domain-requests")) # create new request btn - self.assertNotContains(requests_page, 'Start a new domain request') + self.assertNotContains(requests_page, "Start a new domain request") @less_console_noise_decorator def test_portfolio_cache_updates_when_modified(self): @@ -752,7 +757,7 @@ class TestPortfolio(WebTest): self.client.force_login(self.user) portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, roles=portfolio_roles) - + with override_flag("organization_feature", active=True): # Initial request to set the portfolio in session response = self.client.get(reverse("home"), follow=True) @@ -760,14 +765,14 @@ class TestPortfolio(WebTest): portfolio = self.client.session.get("portfolio") self.assertEqual(portfolio.organization_name, "Hotel California") self.assertContains(response, "Hotel California") - + # Modify the portfolio self.portfolio.organization_name = "Updated Hotel California" self.portfolio.save() - + # Make another request response = self.client.get(reverse("home"), follow=True) - + # Check if the updated portfolio name is in the response self.assertContains(response, "Updated Hotel California") @@ -781,14 +786,14 @@ class TestPortfolio(WebTest): self.client.force_login(self.user) portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] UserPortfolioPermission.objects.get_or_create(user=self.user, portfolio=self.portfolio, roles=portfolio_roles) - + with override_flag("organization_feature", active=True): # Initial request to set the portfolio in session response = self.client.get(reverse("home"), follow=True) portfolio = self.client.session.get("portfolio") self.assertEqual(portfolio.organization_name, "Hotel California") self.assertContains(response, "Hotel California") - + # Disable the organization_feature flag with override_flag("organization_feature", active=False): # Make another request diff --git a/src/registrar/tests/test_views_request.py b/src/registrar/tests/test_views_request.py index 73f79fe68..f3898df9c 100644 --- a/src/registrar/tests/test_views_request.py +++ b/src/registrar/tests/test_views_request.py @@ -18,7 +18,7 @@ from registrar.models import ( Website, FederalAgency, Portfolio, - UserPortfolioPermission + UserPortfolioPermission, ) from registrar.views.domain_request import DomainRequestWizard, Step @@ -2933,9 +2933,7 @@ class DomainRequestTestDifferentStatuses(TestWithUser, WebTest): """Tests that the withdraw button on portfolio redirects to the portfolio domain requests page""" portfolio, _ = Portfolio.objects.get_or_create(creator=self.user, organization_name="Test Portfolio") UserPortfolioPermission.objects.get_or_create( - user=self.user, - portfolio=portfolio, - roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + user=self.user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] ) domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.SUBMITTED, user=self.user) domain_request.save()