From 9f557fc1e171e4f20278f768575563534bd97848 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 6 Sep 2024 17:27:56 -0400 Subject: [PATCH] requests_json tests and some linting --- src/registrar/context_processors.py | 28 ++-- src/registrar/models/user.py | 9 +- src/registrar/registrar_middleware.py | 4 +- src/registrar/tests/test_models.py | 4 +- .../tests/test_views_requests_json.py | 141 ++++++++++++++++++ src/registrar/views/domain_requests_json.py | 16 +- 6 files changed, 175 insertions(+), 27 deletions(-) diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index 5d04d5b70..ac30bb0af 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -61,27 +61,29 @@ 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_domains_portfolio_permission": False, - "has_requests_portfolio_permission": False, - "has_edit_request_portfolio_permission": False, - "has_view_suborganization_portfolio_permission": False, - "has_edit_suborganization_portfolio_permission": False, - "portfolio": None, - "has_organization_feature_flag": False, - } + "has_base_portfolio_permission": False, + "has_domains_portfolio_permission": False, + "has_requests_portfolio_permission": False, + "has_edit_request_portfolio_permission": False, + "has_view_suborganization_portfolio_permission": False, + "has_edit_suborganization_portfolio_permission": False, + "portfolio": None, + "has_organization_feature_flag": False, + } try: portfolio = request.session.get("portfolio") if portfolio: return { "has_base_portfolio_permission": request.user.has_base_portfolio_permission(portfolio), "has_domains_portfolio_permission": request.user.has_domains_portfolio_permission(portfolio), - "has_requests_portfolio_permission": request.user.has_requests_portfolio_permission( + "has_requests_portfolio_permission": request.user.has_requests_portfolio_permission(portfolio), + "has_edit_request_portfolio_permission": request.user.has_edit_request_portfolio_permission(portfolio), + "has_view_suborganization_portfolio_permission": request.user.has_view_suborganization_portfolio_permission( + portfolio + ), + "has_edit_suborganization_portfolio_permission": request.user.has_edit_suborganization_portfolio_permission( portfolio ), - "has_edit_request_portfolio_permission": request.user.has_edit_request_portfolio_permission(portfolio), - "has_view_suborganization_portfolio_permission": request.user.has_view_suborganization_portfolio_permission(portfolio), - "has_edit_suborganization_portfolio_permission": request.user.has_edit_suborganization_portfolio_permission(portfolio), "portfolio": portfolio, "has_organization_feature_flag": True, } diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 0b5dc3f8d..9b4924231 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -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_requests_portfolio_permission(self, portfolio): return self._has_portfolio_permission( portfolio, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS @@ -238,7 +238,7 @@ class User(AbstractUser): def has_edit_request_portfolio_permission(self, portfolio): return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS) - + # Field specific permission checks def has_view_suborganization_portfolio_permission(self, portfolio): return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION) @@ -276,7 +276,10 @@ class User(AbstractUser): and self.has_domains_portfolio_permission(portfolio), ["Domain requestor", "Domain manager"], ), - (self.has_base_portfolio_permission(portfolio) and self.has_edit_request_portfolio_permission(portfolio), ["Domain requestor"]), + ( + self.has_base_portfolio_permission(portfolio) and self.has_edit_request_portfolio_permission(portfolio), + ["Domain requestor"], + ), ( self.has_base_portfolio_permission(portfolio) and self.has_domains_portfolio_permission(portfolio), ["Domain manager"], diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index 3ce6fde5d..85f7b7ffc 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -149,8 +149,8 @@ class CheckPortfolioMiddleware: old_updated_at = request.session.get("portfolio__updated_at") request.session["portfolio__updated_at"] = request.session.get("portfolio").updated_at - should_update_portfolio = ( - not request.session.get("portfolio") or old_updated_at != request.session.get("portfolio__updated_at") + should_update_portfolio = not request.session.get("portfolio") or old_updated_at != request.session.get( + "portfolio__updated_at" ) if request.user.is_org_user(request) or should_update_portfolio: # if multiple portfolios are allowed for this user diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index cdf008c57..140299be5 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1361,7 +1361,9 @@ class TestUser(TestCase): self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain requestor", "Domain manager"]) @patch.multiple( - User, has_base_portfolio_permission=lambda self, portfolio: True, has_edit_request_portfolio_permission=lambda self, portfolio: True + User, + has_base_portfolio_permission=lambda self, portfolio: True, + has_edit_request_portfolio_permission=lambda self, portfolio: True, ) def test_portfolio_role_summary_member_domain_requestor(self): # Test if the user has 'Member' and 'Domain requestor' roles diff --git a/src/registrar/tests/test_views_requests_json.py b/src/registrar/tests/test_views_requests_json.py index 20a4069f7..cef608567 100644 --- a/src/registrar/tests/test_views_requests_json.py +++ b/src/registrar/tests/test_views_requests_json.py @@ -2,9 +2,14 @@ from registrar.models import DomainRequest from django.urls import reverse from registrar.models.draft_domain import DraftDomain +from registrar.models.portfolio import Portfolio +from registrar.models.user import User +from registrar.models.user_portfolio_permission import UserPortfolioPermission +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from .test_views import TestWithUser from django_webtest import WebTest # type: ignore from django.utils.dateparse import parse_datetime +from waffle.testutils import override_flag class GetRequestsJsonTest(TestWithUser, WebTest): @@ -20,6 +25,19 @@ class GetRequestsJsonTest(TestWithUser, WebTest): beef_chuck, _ = DraftDomain.objects.get_or_create(name="beef-chuck.gov") stew_beef, _ = DraftDomain.objects.get_or_create(name="stew-beef.gov") + # Create Portfolio + cls.portfolio = Portfolio.objects.create(creator=cls.user, organization_name="Example org") + + # create a second user to assign requests to + cls.user2 = User.objects.create( + username="test_user2", + first_name="Second", + last_name="last", + email="info2@example.com", + phone="8003111234", + title="title", + ) + # Create domain requests for the user cls.domain_requests = [ DomainRequest.objects.create( @@ -28,6 +46,7 @@ class GetRequestsJsonTest(TestWithUser, WebTest): last_submitted_date="2024-01-01", status=DomainRequest.DomainRequestStatus.STARTED, created_at="2024-01-01", + portfolio=cls.portfolio, ), DomainRequest.objects.create( creator=cls.user, @@ -42,6 +61,7 @@ class GetRequestsJsonTest(TestWithUser, WebTest): last_submitted_date="2024-03-01", status=DomainRequest.DomainRequestStatus.REJECTED, created_at="2024-03-01", + portfolio=cls.portfolio, ), DomainRequest.objects.create( creator=cls.user, @@ -113,6 +133,14 @@ class GetRequestsJsonTest(TestWithUser, WebTest): status=DomainRequest.DomainRequestStatus.APPROVED, created_at="2024-12-01", ), + DomainRequest.objects.create( + creator=cls.user2, + requested_domain=None, + last_submitted_date="2024-12-01", + status=DomainRequest.DomainRequestStatus.STARTED, + created_at="2024-12-01", + portfolio=cls.portfolio, + ), ] @classmethod @@ -120,6 +148,7 @@ class GetRequestsJsonTest(TestWithUser, WebTest): super().tearDownClass() DomainRequest.objects.all().delete() DraftDomain.objects.all().delete() + Portfolio.objects.all().delete() def test_get_domain_requests_json_authenticated(self): """Test that domain requests are returned properly for an authenticated user.""" @@ -262,6 +291,118 @@ class GetRequestsJsonTest(TestWithUser, WebTest): for expected_value, actual_value in zip(expected_domain_values, requested_domains): self.assertEqual(expected_value, actual_value) + @override_flag("organization_feature", active=True) + def test_get_domain_requests_json_with_portfolio_view_all_requests(self): + """Test that an authenticated user gets the list of 3 requests for portfolio. The 3 requests + are the requests that are associated with the portfolio.""" + + UserPortfolioPermission.objects.get_or_create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS], + ) + + response = self.app.get(reverse("get_domain_requests_json"), {"portfolio": self.portfolio.id}) + self.assertEqual(response.status_code, 200) + data = response.json + + # Check pagination info + self.assertEqual(data["page"], 1) + self.assertFalse(data["has_next"]) + self.assertFalse(data["has_previous"]) + self.assertEqual(data["num_pages"], 1) + + # Check the number of requests + self.assertEqual(len(data["domain_requests"]), 3) + + # Expected domain requests + expected_domain_requests = [self.domain_requests[0], self.domain_requests[2], self.domain_requests[13]] + + # Extract fields from response + domain_request_ids = [domain_request["id"] for domain_request in data["domain_requests"]] + requested_domain = [domain_request["requested_domain"] for domain_request in data["domain_requests"]] + creator = [domain_request["creator"] for domain_request in data["domain_requests"]] + status = [domain_request["status"] for domain_request in data["domain_requests"]] + action_urls = [domain_request["action_url"] for domain_request in data["domain_requests"]] + action_labels = [domain_request["action_label"] for domain_request in data["domain_requests"]] + svg_icons = [domain_request["svg_icon"] for domain_request in data["domain_requests"]] + + # Check fields for each domain_request + for i, expected_domain_request in enumerate(expected_domain_requests): + self.assertEqual(expected_domain_request.id, domain_request_ids[i]) + if expected_domain_request.requested_domain: + self.assertEqual(expected_domain_request.requested_domain.name, requested_domain[i]) + else: + self.assertIsNone(requested_domain[i]) + self.assertEqual(expected_domain_request.creator.email, creator[i]) + # Check action url, action label and svg icon + # Example domain requests will test each of below three scenarios + if creator[i] != self.user.email: + # Test case where action is View + self.assertEqual("View", action_labels[i]) + self.assertEqual("#", action_urls[i]) + self.assertEqual("visibility", svg_icons[i]) + elif status[i] in [ + DomainRequest.DomainRequestStatus.STARTED.label, + DomainRequest.DomainRequestStatus.ACTION_NEEDED.label, + DomainRequest.DomainRequestStatus.WITHDRAWN.label, + ]: + # Test case where action is Edit + self.assertEqual("Edit", action_labels[i]) + self.assertEqual( + reverse("edit-domain-request", kwargs={"id": expected_domain_request.id}), action_urls[i] + ) + self.assertEqual("edit", svg_icons[i]) + else: + # Test case where action is Manage + self.assertEqual("Manage", action_labels[i]) + self.assertEqual( + reverse("domain-request-status", kwargs={"pk": expected_domain_request.id}), action_urls[i] + ) + self.assertEqual("settings", svg_icons[i]) + + @override_flag("organization_feature", active=True) + def test_get_domain_requests_json_with_portfolio_edit_requests(self): + """Test that an authenticated user gets the list of 2 requests for portfolio. The 2 requests + are the requests that are associated with the portfolio and owned by self.user.""" + + UserPortfolioPermission.objects.get_or_create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[UserPortfolioPermissionChoices.EDIT_REQUESTS], + ) + + response = self.app.get(reverse("get_domain_requests_json"), {"portfolio": self.portfolio.id}) + self.assertEqual(response.status_code, 200) + data = response.json + + # Check pagination info + self.assertEqual(data["page"], 1) + self.assertFalse(data["has_next"]) + self.assertFalse(data["has_previous"]) + self.assertEqual(data["num_pages"], 1) + + # Check the number of requests + self.assertEqual(len(data["domain_requests"]), 2) + + # Expected domain requests + expected_domain_requests = [self.domain_requests[0], self.domain_requests[2]] + + # Extract fields from response, since other tests test all fields, only ids and requested + # domains tested in this test + domain_request_ids = [domain_request["id"] for domain_request in data["domain_requests"]] + requested_domain = [domain_request["requested_domain"] for domain_request in data["domain_requests"]] + + # Check fields for each domain_request + for i, expected_domain_request in enumerate(expected_domain_requests): + self.assertEqual(expected_domain_request.id, domain_request_ids[i]) + if expected_domain_request.requested_domain: + self.assertEqual(expected_domain_request.requested_domain.name, requested_domain[i]) + else: + self.assertIsNone(requested_domain[i]) + def test_pagination(self): """Test that pagination works properly. There are 11 total non-approved requests and a page size of 10""" diff --git a/src/registrar/views/domain_requests_json.py b/src/registrar/views/domain_requests_json.py index ffb4d51c9..fe86e7a7f 100644 --- a/src/registrar/views/domain_requests_json.py +++ b/src/registrar/views/domain_requests_json.py @@ -24,7 +24,9 @@ def get_domain_requests_json(request): page_number = request.GET.get("page", 1) page_obj = paginator.get_page(page_number) - domain_requests = [serialize_domain_request(domain_request, request.user) for domain_request in page_obj.object_list] + domain_requests = [ + serialize_domain_request(domain_request, request.user) for domain_request in page_obj.object_list + ] return JsonResponse( { @@ -52,7 +54,9 @@ def get_domain_request_ids_from_request(request): filter_condition = Q(portfolio=portfolio) else: filter_condition = Q(portfolio=portfolio, creator=request.user) - domain_requests = DomainRequest.objects.filter(filter_condition).exclude(status=DomainRequest.DomainRequestStatus.APPROVED) + domain_requests = DomainRequest.objects.filter(filter_condition).exclude( + status=DomainRequest.DomainRequestStatus.APPROVED + ) return domain_requests.values_list("id", flat=True) @@ -107,14 +111,10 @@ def serialize_domain_request(domain_request, user): action_url_map = { "Edit": reverse("edit-domain-request", kwargs={"id": domain_request.id}), "Manage": reverse("domain-request-status", kwargs={"pk": domain_request.id}), - "View": "#" + "View": "#", } - svg_icon_map = { - "Edit": "edit", - "Manage": "settings", - "View": "visibility" - } + svg_icon_map = {"Edit": "edit", "Manage": "settings", "View": "visibility"} return { "requested_domain": domain_request.requested_domain.name if domain_request.requested_domain else None,