From 010950ef0c0ab90f12637c4040d1dfa9d9afca74 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 9 Dec 2024 16:26:12 -0500 Subject: [PATCH] unit tests and linting --- src/registrar/context_processors.py | 6 +- .../tests/test_views_member_domains_json.py | 231 ++++++++++++++++-- src/registrar/tests/test_views_portfolio.py | 3 +- src/registrar/views/member_domains_json.py | 19 +- src/registrar/views/portfolios.py | 2 +- src/registrar/views/utility/mixins.py | 2 +- .../views/utility/permission_views.py | 6 +- 7 files changed, 226 insertions(+), 43 deletions(-) diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index f91017af3..9f5d0162f 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -112,10 +112,10 @@ def is_widescreen_mode(request): exclude_paths = [ "/domains/edit", ] - + # Check if the current path matches a widescreen path or the root path. is_widescreen = any(path in request.path for path in widescreen_paths) or request.path == "/" - + # Check if the user is an organization user and the path matches portfolio paths. is_portfolio_widescreen = ( hasattr(request.user, "is_org_user") @@ -123,6 +123,6 @@ def is_widescreen_mode(request): and any(path in request.path for path in portfolio_widescreen_paths) and not any(exclude_path in request.path for exclude_path in exclude_paths) ) - + # Return a dictionary with the widescreen mode status. return {"is_widescreen_mode": is_widescreen or is_portfolio_widescreen} diff --git a/src/registrar/tests/test_views_member_domains_json.py b/src/registrar/tests/test_views_member_domains_json.py index 68b4f4de2..c9f1e38cc 100644 --- a/src/registrar/tests/test_views_member_domains_json.py +++ b/src/registrar/tests/test_views_member_domains_json.py @@ -58,10 +58,13 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): 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") cls.domain3 = Domain.objects.create(name="example3.com", expiration_date="2024-03-01", state="ready") + cls.domain4 = Domain.objects.create(name="example4.com", expiration_date="2024-03-01", state="ready") + # Add domain1 and domain2 to portfolio DomainInformation.objects.create(creator=cls.user, domain=cls.domain1, portfolio=cls.portfolio) DomainInformation.objects.create(creator=cls.user, domain=cls.domain2, portfolio=cls.portfolio) DomainInformation.objects.create(creator=cls.user, domain=cls.domain3, portfolio=cls.portfolio) + DomainInformation.objects.create(creator=cls.user, domain=cls.domain4, portfolio=cls.portfolio) # Assign user_member to view all domains UserPortfolioPermission.objects.create( @@ -70,8 +73,10 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], ) # Add user_member as manager of domains - UserDomainRole.objects.create(user=cls.user_member, domain=cls.domain1) - UserDomainRole.objects.create(user=cls.user_member, domain=cls.domain2) + UserDomainRole.objects.create(user=cls.user_member, domain=cls.domain1, role=UserDomainRole.Roles.MANAGER) + UserDomainRole.objects.create(user=cls.user_member, domain=cls.domain2, role=UserDomainRole.Roles.MANAGER) + UserDomainRole.objects.create(user=cls.user_member, domain=cls.domain3, role=UserDomainRole.Roles.MANAGER) + UserDomainRole.objects.create(user=cls.user_no_perms, domain=cls.domain3, role=UserDomainRole.Roles.MANAGER) # Add an invited member who has been invited to manage domains cls.invited_member_email = "invited@example.com" @@ -123,11 +128,11 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): self.assertFalse(data["has_previous"]) self.assertFalse(data["has_next"]) self.assertEqual(data["num_pages"], 1) - self.assertEqual(data["total"], 2) - self.assertEqual(data["unfiltered_total"], 2) + self.assertEqual(data["total"], 3) + self.assertEqual(data["unfiltered_total"], 3) # Check the number of domains - self.assertEqual(len(data["domains"]), 2) + self.assertEqual(len(data["domains"]), 3) @less_console_noise_decorator @override_flag("organization_feature", active=True) @@ -169,11 +174,11 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): self.assertFalse(data["has_previous"]) self.assertFalse(data["has_next"]) self.assertEqual(data["num_pages"], 1) - self.assertEqual(data["total"], 3) - self.assertEqual(data["unfiltered_total"], 3) + self.assertEqual(data["total"], 4) + self.assertEqual(data["unfiltered_total"], 4) # Check the number of domains - self.assertEqual(len(data["domains"]), 3) + self.assertEqual(len(data["domains"]), 4) @less_console_noise_decorator @override_flag("organization_feature", active=True) @@ -192,11 +197,11 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): self.assertFalse(data["has_previous"]) self.assertFalse(data["has_next"]) self.assertEqual(data["num_pages"], 1) - self.assertEqual(data["total"], 3) - self.assertEqual(data["unfiltered_total"], 3) + self.assertEqual(data["total"], 4) + self.assertEqual(data["unfiltered_total"], 4) # Check the number of domains - self.assertEqual(len(data["domains"]), 3) + self.assertEqual(len(data["domains"]), 4) @less_console_noise_decorator @override_flag("organization_feature", active=True) @@ -221,7 +226,7 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): self.assertFalse(data["has_next"]) self.assertEqual(data["num_pages"], 1) self.assertEqual(data["total"], 1) - self.assertEqual(data["unfiltered_total"], 3) + self.assertEqual(data["unfiltered_total"], 4) # Check the number of domains self.assertEqual(len(data["domains"]), 1) @@ -249,7 +254,7 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): self.assertFalse(data["has_next"]) self.assertEqual(data["num_pages"], 1) self.assertEqual(data["total"], 1) - self.assertEqual(data["unfiltered_total"], 3) + self.assertEqual(data["unfiltered_total"], 4) # Check the number of domains self.assertEqual(len(data["domains"]), 1) @@ -278,11 +283,11 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): self.assertFalse(data["has_previous"]) self.assertFalse(data["has_next"]) self.assertEqual(data["num_pages"], 1) - self.assertEqual(data["total"], 3) - self.assertEqual(data["unfiltered_total"], 3) + self.assertEqual(data["total"], 4) + self.assertEqual(data["unfiltered_total"], 4) # Check the number of domains - self.assertEqual(len(data["domains"]), 3) + self.assertEqual(len(data["domains"]), 4) # Check the name of the first domain is example1.com self.assertEqual(data["domains"][0]["name"], "example1.com") @@ -306,14 +311,121 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): self.assertFalse(data["has_previous"]) self.assertFalse(data["has_next"]) self.assertEqual(data["num_pages"], 1) - self.assertEqual(data["total"], 3) - self.assertEqual(data["unfiltered_total"], 3) + self.assertEqual(data["total"], 4) + self.assertEqual(data["unfiltered_total"], 4) # Check the number of domains - self.assertEqual(len(data["domains"]), 3) + self.assertEqual(len(data["domains"]), 4) # Check the name of the first domain is example1.com - self.assertEqual(data["domains"][0]["name"], "example3.com") + self.assertEqual(data["domains"][0]["name"], "example4.com") + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_get_portfolio_member_domains_json_authenticated_sort_by_checked(self): + """Test that sort returns results in correct order.""" + # Test by checked in ascending order + response = self.app.get( + reverse("get_member_domains_json"), + params={ + "portfolio": self.portfolio.id, + "email": self.user_member.id, + "member_only": "false", + "checkedDomainIds": f"{self.domain2.id},{self.domain3.id}", + "sort_by": "checked", + "order": "asc", + }, + ) + self.assertEqual(response.status_code, 200) + data = response.json + + # Check pagination info + self.assertEqual(data["page"], 1) + self.assertFalse(data["has_previous"]) + self.assertFalse(data["has_next"]) + self.assertEqual(data["num_pages"], 1) + self.assertEqual(data["total"], 4) + self.assertEqual(data["unfiltered_total"], 4) + + # Check the number of domains + self.assertEqual(len(data["domains"]), 4) + + # Check the name of the first domain is the first unchecked domain sorted alphabetically + self.assertEqual(data["domains"][0]["name"], "example1.com") + self.assertEqual(data["domains"][1]["name"], "example4.com") + + # Test by checked in descending order + response = self.app.get( + reverse("get_member_domains_json"), + params={ + "portfolio": self.portfolio.id, + "email": self.user_member.id, + "member_only": "false", + "checkedDomainIds": f"{self.domain2.id},{self.domain3.id}", + "sort_by": "checked", + "order": "desc", + }, + ) + self.assertEqual(response.status_code, 200) + data = response.json + + # Check pagination info + self.assertEqual(data["page"], 1) + self.assertFalse(data["has_previous"]) + self.assertFalse(data["has_next"]) + self.assertEqual(data["num_pages"], 1) + self.assertEqual(data["total"], 4) + self.assertEqual(data["unfiltered_total"], 4) + + # Check the number of domains + self.assertEqual(len(data["domains"]), 4) + + # Check the name of the first domain is the first checked domain sorted alphabetically + self.assertEqual(data["domains"][0]["name"], "example2.com") + self.assertEqual(data["domains"][1]["name"], "example3.com") + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_get_portfolio_member_domains_json_authenticated_member_is_only_manager(self): + """Test that sort returns member_is_only_manager when member_domain_role_exists + and member_domain_role_count == 1""" + 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 + + # Check pagination info + self.assertEqual(data["page"], 1) + self.assertFalse(data["has_previous"]) + self.assertFalse(data["has_next"]) + self.assertEqual(data["num_pages"], 1) + self.assertEqual(data["total"], 4) + self.assertEqual(data["unfiltered_total"], 4) + + # Check the number of domains + self.assertEqual(len(data["domains"]), 4) + + self.assertEqual(data["domains"][0]["name"], "example1.com") + self.assertEqual(data["domains"][1]["name"], "example2.com") + self.assertEqual(data["domains"][2]["name"], "example3.com") + self.assertEqual(data["domains"][3]["name"], "example4.com") + + self.assertEqual(data["domains"][0]["member_is_only_manager"], True) + self.assertEqual(data["domains"][1]["member_is_only_manager"], True) + # domain3 has 2 managers + self.assertEqual(data["domains"][2]["member_is_only_manager"], False) + # no managers on this one + self.assertEqual(data["domains"][3]["member_is_only_manager"], False) @less_console_noise_decorator @override_flag("organization_feature", active=True) @@ -339,11 +451,11 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): self.assertFalse(data["has_previous"]) self.assertFalse(data["has_next"]) self.assertEqual(data["num_pages"], 1) - self.assertEqual(data["total"], 3) - self.assertEqual(data["unfiltered_total"], 3) + self.assertEqual(data["total"], 4) + self.assertEqual(data["unfiltered_total"], 4) # Check the number of domains - self.assertEqual(len(data["domains"]), 3) + self.assertEqual(len(data["domains"]), 4) # Check the name of the first domain is example1.com self.assertEqual(data["domains"][0]["name"], "example1.com") @@ -367,14 +479,79 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): self.assertFalse(data["has_previous"]) self.assertFalse(data["has_next"]) self.assertEqual(data["num_pages"], 1) - self.assertEqual(data["total"], 3) - self.assertEqual(data["unfiltered_total"], 3) + self.assertEqual(data["total"], 4) + self.assertEqual(data["unfiltered_total"], 4) # Check the number of domains - self.assertEqual(len(data["domains"]), 3) + self.assertEqual(len(data["domains"]), 4) # Check the name of the first domain is example1.com - self.assertEqual(data["domains"][0]["name"], "example3.com") + self.assertEqual(data["domains"][0]["name"], "example4.com") + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_get_portfolio_invitedmember_domains_json_authenticated_sort_by_checked(self): + """Test that sort returns results in correct order.""" + # Test by checked 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", + "checkedDomainIds": f"{self.domain2.id},{self.domain3.id}", + "sort_by": "checked", + "order": "asc", + }, + ) + self.assertEqual(response.status_code, 200) + data = response.json + + # Check pagination info + self.assertEqual(data["page"], 1) + self.assertFalse(data["has_previous"]) + self.assertFalse(data["has_next"]) + self.assertEqual(data["num_pages"], 1) + self.assertEqual(data["total"], 4) + self.assertEqual(data["unfiltered_total"], 4) + + # Check the number of domains + self.assertEqual(len(data["domains"]), 4) + + # Check the name of the first domain is the first unchecked domain sorted alphabetically + self.assertEqual(data["domains"][0]["name"], "example1.com") + self.assertEqual(data["domains"][1]["name"], "example4.com") + + # Test by checked 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", + "checkedDomainIds": f"{self.domain2.id},{self.domain3.id}", + "sort_by": "checked", + "order": "desc", + }, + ) + self.assertEqual(response.status_code, 200) + data = response.json + + # Check pagination info + self.assertEqual(data["page"], 1) + self.assertFalse(data["has_previous"]) + self.assertFalse(data["has_next"]) + self.assertEqual(data["num_pages"], 1) + self.assertEqual(data["total"], 4) + self.assertEqual(data["unfiltered_total"], 4) + + # Check the number of domains + self.assertEqual(len(data["domains"]), 4) + + # Check the name of the first domain is the first checked domain sorted alphabetically + self.assertEqual(data["domains"][0]["name"], "example2.com") + self.assertEqual(data["domains"][1]["name"], "example3.com") @less_console_noise_decorator @override_flag("organization_feature", active=True) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 514c31007..de27b7059 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -2153,7 +2153,8 @@ class TestPortfolioMemberDomainsEditView(TestPortfolioMemberDomainsView): @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) def test_member_domains_edit_not_found(self): - """Tests that the portfolio member domains edit view returns not found if user portfolio permission not found.""" + """Tests that the portfolio member domains edit view returns not found if user + portfolio permission not found.""" self.client.force_login(self.user) response = self.client.get(reverse("member-domains-edit", kwargs={"pk": "0"})) diff --git a/src/registrar/views/member_domains_json.py b/src/registrar/views/member_domains_json.py index cfe82251e..125059692 100644 --- a/src/registrar/views/member_domains_json.py +++ b/src/registrar/views/member_domains_json.py @@ -50,7 +50,7 @@ class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): def get_page_size(self, request): """Gets the page size. - + If member_only, need to return the entire result set every time, so need to set to a very large page size. If not member_only, this can be adjusted to provide a smaller page size""" @@ -64,7 +64,7 @@ class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): # This number can be adjusted if we want to add pagination to the result page # later return 1000 - + def get_domain_ids_from_request(self, request): """Get domain ids from request. @@ -128,17 +128,16 @@ class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): ) # Add ordering logic for 'checked' if order == "desc": - queryset = queryset.order_by("-checked","name") + queryset = queryset.order_by("-checked", "name") else: - queryset = queryset.order_by("checked","name") + queryset = queryset.order_by("checked", "name") else: # Handle other fields as normal if order == "desc": sort_by = f"-{sort_by}" queryset = queryset.order_by(sort_by) - - return queryset + return queryset def serialize_domain(self, domain, member_id, user): suborganization_name = None @@ -159,8 +158,12 @@ class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): # Check if the specified member is the only member assigned as manager of domain only_member_assigned_to_domain = False if member_id: - member_domain_role_count = UserDomainRole.objects.filter(domain_id=domain.id).count() - member_domain_role_exists = UserDomainRole.objects.filter(domain_id=domain.id, user_id=member_id).exists() + member_domain_role_count = UserDomainRole.objects.filter( + domain_id=domain.id, role=UserDomainRole.Roles.MANAGER + ).count() + member_domain_role_exists = UserDomainRole.objects.filter( + domain_id=domain.id, user_id=member_id, role=UserDomainRole.Roles.MANAGER + ).exists() only_member_assigned_to_domain = member_domain_role_exists and member_domain_role_count == 1 return { diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 9aa2b6d8b..90313339b 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -324,7 +324,7 @@ class PortfolioInvitedMemberDomainsView(PortfolioMemberDomainsPermissionView, Vi "portfolio_invitation": portfolio_invitation, }, ) - + class PortfolioInvitedMemberDomainsEditView(PortfolioMemberDomainsEditPermissionView, View): diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index 4c6ffe691..11384ca09 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -572,7 +572,7 @@ class PortfolioMemberDomainsPermission(PortfolioBasePermission): return False return super().has_permission() - + class PortfolioMemberDomainsEditPermission(PortfolioBasePermission): """Permission mixin that allows access to portfolio member or invited member domains edit pages if user diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py index 2034e9988..a3067d3a2 100644 --- a/src/registrar/views/utility/permission_views.py +++ b/src/registrar/views/utility/permission_views.py @@ -282,9 +282,11 @@ class PortfolioMemberDomainsPermissionView(PortfolioMemberDomainsPermission, Por """ -class PortfolioMemberDomainsEditPermissionView(PortfolioMemberDomainsEditPermission, PortfolioBasePermissionView, abc.ABC): +class PortfolioMemberDomainsEditPermissionView( + PortfolioMemberDomainsEditPermission, PortfolioBasePermissionView, abc.ABC +): """Abstract base view for portfolio member domains edit views that enforces permissions. This abstract view cannot be instantiated. Actual views must specify `template_name`. - """ \ No newline at end of file + """