From dbf208c2bb1b8a924aed5616c86ea58583e94f4b Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 21 Oct 2024 19:55:05 -0400 Subject: [PATCH] formatted for code readability --- src/registrar/config/urls.py | 2 +- .../tests/test_views_member_domains_json.py | 114 +++++++++++++++--- .../tests/test_views_members_json.py | 8 +- src/registrar/tests/test_views_portfolio.py | 11 +- src/registrar/views/member_domains_json.py | 15 ++- src/registrar/views/portfolio_members_json.py | 8 +- src/registrar/views/portfolios.py | 7 +- .../views/utility/permission_views.py | 1 + 8 files changed, 117 insertions(+), 49 deletions(-) diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index c493f1a08..f61e31e54 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -338,7 +338,7 @@ urlpatterns = [ path("get-domains-json/", get_domains_json, name="get_domains_json"), path("get-domain-requests-json/", get_domain_requests_json, name="get_domain_requests_json"), path("get-portfolio-members-json/", views.PortfolioMembersJson.as_view(), name="get_portfolio_members_json"), - path('get-member-domains-json/', views.PortfolioMemberDomainsJson.as_view(), name="get_member_domains_json"), + path("get-member-domains-json/", views.PortfolioMemberDomainsJson.as_view(), name="get_member_domains_json"), ] # Djangooidc strips out context data from that context, so we define a custom error diff --git a/src/registrar/tests/test_views_member_domains_json.py b/src/registrar/tests/test_views_member_domains_json.py index 11b77576e..68b4f4de2 100644 --- a/src/registrar/tests/test_views_member_domains_json.py +++ b/src/registrar/tests/test_views_member_domains_json.py @@ -53,7 +53,7 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): UserPortfolioPermissionChoices.EDIT_MEMBERS, ], ) - + # Assign some domains cls.domain1 = Domain.objects.create(name="example1.com", expiration_date="2024-03-01", state="ready") cls.domain2 = Domain.objects.create(name="example2.com", expiration_date="2024-03-01", state="ready") @@ -83,8 +83,12 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): UserPortfolioPermissionChoices.VIEW_MEMBERS, ], ) - DomainInvitation.objects.create(email=cls.invited_member_email, domain=cls.domain1, status=DomainInvitation.DomainInvitationStatus.INVITED) - DomainInvitation.objects.create(email=cls.invited_member_email, domain=cls.domain2, status=DomainInvitation.DomainInvitationStatus.INVITED) + DomainInvitation.objects.create( + email=cls.invited_member_email, domain=cls.domain1, status=DomainInvitation.DomainInvitationStatus.INVITED + ) + DomainInvitation.objects.create( + email=cls.invited_member_email, domain=cls.domain2, status=DomainInvitation.DomainInvitationStatus.INVITED + ) @classmethod def tearDownClass(cls): @@ -107,7 +111,10 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): @override_flag("organization_members", active=True) def test_get_portfolio_member_domains_json_authenticated(self): """Test that portfolio member's domains are returned properly for an authenticated user.""" - response = self.app.get(reverse("get_member_domains_json"), params={"portfolio": self.portfolio.id, "member_id": self.user_member.id, "member_only": "true"}) + response = self.app.get( + reverse("get_member_domains_json"), + params={"portfolio": self.portfolio.id, "member_id": self.user_member.id, "member_only": "true"}, + ) self.assertEqual(response.status_code, 200) data = response.json @@ -127,7 +134,10 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): @override_flag("organization_members", active=True) def test_get_portfolio_invitedmember_domains_json_authenticated(self): """Test that portfolio invitedmember's domains are returned properly for an authenticated user.""" - response = self.app.get(reverse("get_member_domains_json"), params={"portfolio": self.portfolio.id, "email": self.invited_member_email, "member_only": "true"}) + response = self.app.get( + reverse("get_member_domains_json"), + params={"portfolio": self.portfolio.id, "email": self.invited_member_email, "member_only": "true"}, + ) self.assertEqual(response.status_code, 200) data = response.json @@ -147,7 +157,10 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): @override_flag("organization_members", active=True) def test_get_portfolio_member_domains_json_authenticated_include_all_domains(self): """Test that all portfolio domains are returned properly for an authenticated user.""" - response = self.app.get(reverse("get_member_domains_json"), params={"portfolio": self.portfolio.id, "member_id": self.user_member.id, "member_only": "false"}) + response = self.app.get( + reverse("get_member_domains_json"), + params={"portfolio": self.portfolio.id, "member_id": self.user_member.id, "member_only": "false"}, + ) self.assertEqual(response.status_code, 200) data = response.json @@ -167,7 +180,10 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): @override_flag("organization_members", active=True) def test_get_portfolio_invitedmember_domains_json_authenticated_include_all_domains(self): """Test that all portfolio domains are returned properly for an authenticated user.""" - response = self.app.get(reverse("get_member_domains_json"), params={"portfolio": self.portfolio.id, "email": self.invited_member_email, "member_only": "false"}) + response = self.app.get( + reverse("get_member_domains_json"), + params={"portfolio": self.portfolio.id, "email": self.invited_member_email, "member_only": "false"}, + ) self.assertEqual(response.status_code, 200) data = response.json @@ -187,7 +203,15 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): @override_flag("organization_members", active=True) def test_get_portfolio_member_domains_json_authenticated_search(self): """Test that search_term yields correct domain.""" - response = self.app.get(reverse("get_member_domains_json"), params={"portfolio": self.portfolio.id, "member_id": self.user_member.id, "member_only": "false", "search_term": "example1"}) + response = self.app.get( + reverse("get_member_domains_json"), + params={ + "portfolio": self.portfolio.id, + "member_id": self.user_member.id, + "member_only": "false", + "search_term": "example1", + }, + ) self.assertEqual(response.status_code, 200) data = response.json @@ -207,7 +231,15 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): @override_flag("organization_members", active=True) def test_get_portfolio_invitedmember_domains_json_authenticated_search(self): """Test that search_term yields correct domain.""" - response = self.app.get(reverse("get_member_domains_json"), params={"portfolio": self.portfolio.id, "email": self.invited_member_email, "member_only": "false", "search_term": "example1"}) + response = self.app.get( + reverse("get_member_domains_json"), + params={ + "portfolio": self.portfolio.id, + "email": self.invited_member_email, + "member_only": "false", + "search_term": "example1", + }, + ) self.assertEqual(response.status_code, 200) data = response.json @@ -228,7 +260,16 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): def test_get_portfolio_member_domains_json_authenticated_sort(self): """Test that sort returns results in correct order.""" # Test by name in ascending order - response = self.app.get(reverse("get_member_domains_json"), params={"portfolio": self.portfolio.id, "member_id": self.user_member.id, "member_only": "false", "sort_by": "name", "order":"asc"}) + response = self.app.get( + reverse("get_member_domains_json"), + params={ + "portfolio": self.portfolio.id, + "member_id": self.user_member.id, + "member_only": "false", + "sort_by": "name", + "order": "asc", + }, + ) self.assertEqual(response.status_code, 200) data = response.json @@ -247,7 +288,16 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): self.assertEqual(data["domains"][0]["name"], "example1.com") # Test by name in descending order - response = self.app.get(reverse("get_member_domains_json"), params={"portfolio": self.portfolio.id, "member_id": self.user_member.id, "member_only": "false", "sort_by": "name", "order":"desc"}) + response = self.app.get( + reverse("get_member_domains_json"), + params={ + "portfolio": self.portfolio.id, + "member_id": self.user_member.id, + "member_only": "false", + "sort_by": "name", + "order": "desc", + }, + ) self.assertEqual(response.status_code, 200) data = response.json @@ -271,7 +321,16 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): def test_get_portfolio_invitedmember_domains_json_authenticated_sort(self): """Test that sort returns results in correct order.""" # Test by name in ascending order - response = self.app.get(reverse("get_member_domains_json"), params={"portfolio": self.portfolio.id, "email": self.invited_member_email, "member_only": "false", "sort_by": "name", "order":"asc"}) + response = self.app.get( + reverse("get_member_domains_json"), + params={ + "portfolio": self.portfolio.id, + "email": self.invited_member_email, + "member_only": "false", + "sort_by": "name", + "order": "asc", + }, + ) self.assertEqual(response.status_code, 200) data = response.json @@ -290,7 +349,16 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): self.assertEqual(data["domains"][0]["name"], "example1.com") # Test by name in descending order - response = self.app.get(reverse("get_member_domains_json"), params={"portfolio": self.portfolio.id, "email": self.invited_member_email, "member_only": "false", "sort_by": "name", "order":"desc"}) + response = self.app.get( + reverse("get_member_domains_json"), + params={ + "portfolio": self.portfolio.id, + "email": self.invited_member_email, + "member_only": "false", + "sort_by": "name", + "order": "desc", + }, + ) self.assertEqual(response.status_code, 200) data = response.json @@ -315,10 +383,14 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): """Test that an restricted user is denied access.""" # set user to a user with no permissions self.app.set_user(self.user_no_perms) - + # Try to access the portfolio members without being authenticated - response = self.app.get(reverse("get_member_domains_json"), params={"portfolio": self.portfolio.id, "member_id": self.user_member.id, "member_only": "true"}, expect_errors=True) - + response = self.app.get( + reverse("get_member_domains_json"), + params={"portfolio": self.portfolio.id, "member_id": self.user_member.id, "member_only": "true"}, + expect_errors=True, + ) + # Assert that the response is a 403 self.assertEqual(response.status_code, 403) @@ -329,10 +401,14 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): """Test that an unauthenticated user is redirected to login.""" # set app to unauthenticated self.app.set_user(None) - + # Try to access the portfolio members without being authenticated - response = self.app.get(reverse("get_member_domains_json"), params={"portfolio": self.portfolio.id, "member_id": self.user_member.id, "member_only": "true"}, expect_errors=True) - + response = self.app.get( + reverse("get_member_domains_json"), + params={"portfolio": self.portfolio.id, "member_id": self.user_member.id, "member_only": "true"}, + expect_errors=True, + ) + # Assert that the response is a redirect to openid login self.assertEqual(response.status_code, 302) self.assertIn("/openid/login", response.location) diff --git a/src/registrar/tests/test_views_members_json.py b/src/registrar/tests/test_views_members_json.py index 1ae4aadf5..47dbb79d0 100644 --- a/src/registrar/tests/test_views_members_json.py +++ b/src/registrar/tests/test_views_members_json.py @@ -132,10 +132,12 @@ class GetPortfolioMembersJsonTest(TestWithUser, WebTest): """Test that an unauthenticated user is redirected or denied access.""" # Log out the user by setting the user to None self.app.set_user(None) - + # Try to access the portfolio members without being authenticated - response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id}, expect_errors=True) - + response = self.app.get( + reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id}, expect_errors=True + ) + # Assert that the response is a redirect to the login page self.assertEqual(response.status_code, 302) # Redirect to openid login self.assertIn("/openid/login", response.location) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 3ac4ac3dd..2213d6339 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -1437,7 +1437,7 @@ class TestPortfolioMemberDomainsView(TestWithUser, WebTest): UserPortfolioPermissionChoices.EDIT_MEMBERS, ], ) - + @classmethod def tearDownClass(cls): UserPortfolioPermission.objects.all().delete() @@ -1454,7 +1454,6 @@ class TestPortfolioMemberDomainsView(TestWithUser, WebTest): response = self.client.get(reverse("member-domains", kwargs={"pk": self.permission.id})) - # Make sure the page loaded, and that we're on the right page self.assertEqual(response.status_code, 200) self.assertContains(response, self.user_member.email) @@ -1493,10 +1492,9 @@ class TestPortfolioMemberDomainsView(TestWithUser, WebTest): response = self.client.get(reverse("member-domains", kwargs={"pk": "0"})) - # Make sure the response is not found self.assertEqual(response.status_code, 404) - + class TestPortfolioInvitedMemberDomainsView(TestWithUser, WebTest): @classmethod @@ -1536,7 +1534,7 @@ class TestPortfolioInvitedMemberDomainsView(TestWithUser, WebTest): UserPortfolioPermissionChoices.EDIT_MEMBERS, ], ) - + @classmethod def tearDownClass(cls): PortfolioInvitation.objects.all().delete() @@ -1554,7 +1552,6 @@ class TestPortfolioInvitedMemberDomainsView(TestWithUser, WebTest): response = self.client.get(reverse("invitedmember-domains", kwargs={"pk": self.invitation.id})) - # Make sure the page loaded, and that we're on the right page self.assertEqual(response.status_code, 200) self.assertContains(response, self.invited_member_email) @@ -1593,7 +1590,5 @@ class TestPortfolioInvitedMemberDomainsView(TestWithUser, WebTest): response = self.client.get(reverse("invitedmember-domains", kwargs={"pk": "0"})) - # Make sure the response is not found self.assertEqual(response.status_code, 404) - \ No newline at end of file diff --git a/src/registrar/views/member_domains_json.py b/src/registrar/views/member_domains_json.py index 7dcec6bef..f37afdff0 100644 --- a/src/registrar/views/member_domains_json.py +++ b/src/registrar/views/member_domains_json.py @@ -4,7 +4,6 @@ from django.core.paginator import Paginator from django.shortcuts import get_object_or_404 from django.views import View from registrar.models import UserDomainRole, Domain, DomainInformation, User -from django.contrib.auth.decorators import login_required from django.urls import reverse from django.db.models import Q @@ -47,7 +46,6 @@ class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): } ) - def get_domain_ids_from_request(self, request): """Get domain ids from request. @@ -64,27 +62,29 @@ class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): if member_only: if member_id: member = get_object_or_404(User, pk=member_id) - domain_info_ids = DomainInformation.objects.filter(portfolio=portfolio).values_list("domain_id", flat=True) + domain_info_ids = DomainInformation.objects.filter(portfolio=portfolio).values_list( + "domain_id", flat=True + ) user_domain_roles = UserDomainRole.objects.filter(user=member).values_list("domain_id", flat=True) return domain_info_ids.intersection(user_domain_roles) elif email: - domain_info_ids = DomainInformation.objects.filter(portfolio=portfolio).values_list("domain_id", flat=True) + domain_info_ids = DomainInformation.objects.filter(portfolio=portfolio).values_list( + "domain_id", flat=True + ) domain_invitations = DomainInvitation.objects.filter(email=email).values_list("domain_id", flat=True) return domain_info_ids.intersection(domain_invitations) else: domain_infos = DomainInformation.objects.filter(portfolio=portfolio) return domain_infos.values_list("domain_id", flat=True) - logger.warning("Invalid search criteria, returning empty results list") + logger.warning("Invalid search criteria, returning empty results list") return [] - def apply_search(self, queryset, request): search_term = request.GET.get("search_term") if search_term: queryset = queryset.filter(Q(name__icontains=search_term)) return queryset - def apply_sorting(self, queryset, request): sort_by = request.GET.get("sort_by", "name") order = request.GET.get("order", "asc") @@ -92,7 +92,6 @@ class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): sort_by = f"-{sort_by}" return queryset.order_by(sort_by) - def serialize_domain(self, domain, user): suborganization_name = None try: diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index 94ca9ac69..f7df4f30a 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -1,6 +1,5 @@ from django.http import JsonResponse from django.core.paginator import Paginator -from django.contrib.auth.decorators import login_required from django.db.models import Value, F, CharField, TextField, Q, Case, When from django.db.models.functions import Concat, Coalesce from django.urls import reverse @@ -14,7 +13,7 @@ from registrar.views.utility.mixins import PortfolioMembersPermission class PortfolioMembersJson(PortfolioMembersPermission, View): - + def get(self, request): """Fetch members (permissions and invitations) for the given portfolio.""" @@ -52,7 +51,6 @@ class PortfolioMembersJson(PortfolioMembersPermission, View): } ) - def initial_permissions_search(self, portfolio): """Perform initial search for permissions before applying any filters.""" permissions = UserPortfolioPermission.objects.filter(portfolio=portfolio) @@ -96,7 +94,6 @@ class PortfolioMembersJson(PortfolioMembersPermission, View): ) return permissions - def initial_invitations_search(self, portfolio): """Perform initial invitations search before applying any filters.""" invitations = PortfolioInvitation.objects.filter(portfolio=portfolio) @@ -121,7 +118,6 @@ class PortfolioMembersJson(PortfolioMembersPermission, View): ) return invitations - def apply_search_term(self, queryset, request): """Apply search term to the queryset.""" search_term = request.GET.get("search_term", "").lower() @@ -133,7 +129,6 @@ class PortfolioMembersJson(PortfolioMembersPermission, View): ) return queryset - def apply_sorting(self, queryset, request): """Apply sorting to the queryset.""" sort_by = request.GET.get("sort_by", "id") # Default to 'id' @@ -147,7 +142,6 @@ class PortfolioMembersJson(PortfolioMembersPermission, View): queryset = queryset.order_by(sort_by) return queryset - def serialize_members(self, request, portfolio, item, user): # Check if the user can edit other users user_can_edit_other_users = any( diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index b587ea9c9..6fb976d5c 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -136,7 +136,8 @@ class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View): "member": user, # Pass the user object again to the template }, ) - + + class PortfolioMemberDomainsView(PortfolioMemberDomainsPermissionView, View): template_name = "portfolio_member_domains.html" @@ -155,7 +156,6 @@ class PortfolioMemberDomainsView(PortfolioMemberDomainsPermissionView, View): ) - class PortfolioInvitedMemberView(PortfolioMemberPermissionView, View): template_name = "portfolio_member.html" @@ -227,7 +227,8 @@ class PortfolioInvitedMemberEditView(PortfolioMemberEditPermissionView, View): "invitation": portfolio_invitation, # Pass the user object again to the template }, ) - + + class PortfolioInvitedMemberDomainsView(PortfolioMemberDomainsPermissionView, View): template_name = "portfolio_member_domains.html" diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py index 5c05a3a20..1b6db24de 100644 --- a/src/registrar/views/utility/permission_views.py +++ b/src/registrar/views/utility/permission_views.py @@ -278,6 +278,7 @@ class PortfolioMemberEditPermissionView(PortfolioMemberEditPermission, Portfolio `template_name`. """ + class PortfolioMemberDomainsPermissionView(PortfolioMemberDomainsPermission, PortfolioBasePermissionView, abc.ABC): """Abstract base view for portfolio member domains views that enforces permissions.