diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 9eb005b36..8dd0fcf14 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1930,7 +1930,7 @@ class MembersTable extends LoadTableBase { row.innerHTML = ` - ${member_email} ${admin_tagHTML} + ${member_email ? member_email : member_name} ${admin_tagHTML} ${last_active} diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index 046dcb65f..2cf056858 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -243,7 +243,7 @@ def is_portfolio_subpage(path): @register.filter(name="is_members_subpage") def is_members_subpage(path): - """Checks if the given page is a subpage of portfolio. + """Checks if the given page is a subpage of members. Takes a path name, like '/organization/'.""" # Since our pages aren't unified under a common path, we need this approach for now. url_names = [ diff --git a/src/registrar/tests/test_views_members_json.py b/src/registrar/tests/test_views_members_json.py new file mode 100644 index 000000000..22c7bab33 --- /dev/null +++ b/src/registrar/tests/test_views_members_json.py @@ -0,0 +1,175 @@ +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 GetPortfolioMembersJsonTest(TestWithUser, WebTest): + @classmethod + def setUpClass(cls): + super().setUpClass() + + # Create additional users + cls.user2 = User.objects.create( + username="test_user2", + first_name="Second", + last_name="User", + email="second@example.com", + phone="8003112345", + title="Member", + ) + cls.user3 = User.objects.create( + username="test_user3", + first_name="Third", + last_name="User", + email="third@example.com", + phone="8003113456", + title="Member", + ) + cls.user4 = User.objects.create( + username="test_user4", + first_name="Fourth", + last_name="User", + email="fourth@example.com", + phone="8003114567", + title="Admin", + ) + + # Create Portfolio + cls.portfolio = Portfolio.objects.create(creator=cls.user, organization_name="Test Portfolio") + + # Assign permissions + UserPortfolioPermission.objects.create( + user=cls.user, + portfolio=cls.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ] + ) + UserPortfolioPermission.objects.create( + user=cls.user2, + portfolio=cls.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + UserPortfolioPermission.objects.create( + user=cls.user3, + portfolio=cls.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + UserPortfolioPermission.objects.create( + user=cls.user4, + portfolio=cls.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + ) + + def setUp(self): + super().setUp() + self.app.set_user(self.user.username) + + def test_get_portfolio_members_json_authenticated(self): + """Test that portfolio members are returned properly for an authenticated user.""" + response = self.app.get(reverse("get_portfolio_members_json"), params={"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_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 members + self.assertEqual(len(data["members"]), 4) + + # Check member fields + expected_emails = {self.user.email, self.user2.email, self.user3.email, self.user4.email} + actual_emails = {member["email"] for member in data["members"]} + self.assertEqual(expected_emails, actual_emails) + + def test_pagination(self): + """Test that pagination works properly when there are more members than page size.""" + # Create additional members to exceed page size of 10 + for i in range(5, 15): + user, _ = User.objects.get_or_create( + username=f"test_user{i}", + first_name=f"User{i}", + last_name=f"Last{i}", + email=f"user{i}@example.com", + phone=f"80031156{i}", + title="Member", + ) + UserPortfolioPermission.objects.create( + user=user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + + response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id, "page": 1}) + self.assertEqual(response.status_code, 200) + data = response.json + + # Check pagination info + self.assertEqual(data["page"], 1) + self.assertTrue(data["has_next"]) + self.assertFalse(data["has_previous"]) + self.assertEqual(data["num_pages"], 2) + self.assertEqual(data["total"], 14) + self.assertEqual(data["unfiltered_total"], 14) + + # Check the number of members on page 1 + self.assertEqual(len(data["members"]), 10) + + response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id, "page": 2}) + self.assertEqual(response.status_code, 200) + data = response.json + + # Check pagination info for page 2 + self.assertEqual(data["page"], 2) + self.assertFalse(data["has_next"]) + self.assertTrue(data["has_previous"]) + self.assertEqual(data["num_pages"], 2) + + # Check the number of members on page 2 + self.assertEqual(len(data["members"]), 4) + + def test_search(self): + """Test search functionality for portfolio members.""" + # Search by first name + response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id, "search_term": "Second"}) + self.assertEqual(response.status_code, 200) + data = response.json + self.assertEqual(len(data["members"]), 1) + self.assertEqual(data["members"][0]["first_name"], "Second") + self.assertEqual(data["members"][0]["email"], "second@example.com") + + # Search by last name + response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id, "search_term": "Last3"}) + self.assertEqual(response.status_code, 200) + data = response.json + self.assertEqual(len(data["members"]), 1) + self.assertEqual(data["members"][0]["last_name"], "User") + + # Search by email + response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id, "search_term": "fourth@example.com"}) + self.assertEqual(response.status_code, 200) + data = response.json + self.assertEqual(len(data["members"]), 1) + self.assertEqual(data["members"][0]["email"], "fourth@example.com") + + # Search with no matching results + response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id, "search_term": "NonExistent"}) + self.assertEqual(response.status_code, 200) + data = response.json + self.assertEqual(len(data["members"]), 0) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index dfb0469d0..9435f888d 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -781,7 +781,7 @@ class TestPortfolio(WebTest): # Verify that manage settings are sent in the dynamic HTML self.client.force_login(self.user) - response = self.client.get(reverse("get_portfolio_members_json") + f"?portfolio={self.portfolio.pk}") + response = self.client.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id}) self.assertContains(response, '"action_label": "Manage"') self.assertContains(response, '"svg_icon": "settings"') @@ -816,7 +816,7 @@ class TestPortfolio(WebTest): self.assertEqual(response.status_code, 200) # Verify that view-only settings are sent in the dynamic HTML - response = self.client.get(reverse("get_portfolio_members_json") + f"?portfolio={self.portfolio.pk}") + response = self.client.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id}) print(response.content) self.assertContains(response, '"action_label": "View"') self.assertContains(response, '"svg_icon": "visibility"') @@ -852,7 +852,7 @@ class TestPortfolio(WebTest): # Make sure the page loaded self.assertEqual(response.status_code, 200) # Verify that admin info is sent in the dynamic HTML - response = self.client.get(reverse("get_portfolio_members_json") + f"?portfolio={self.portfolio.pk}") + response = self.client.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id}) # TerminalHelper.colorful_logger(logger.info, TerminalColors.OKCYAN, f"{response.content}") self.assertContains(response, '"is_admin": true') diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index 1f493e7be..133e6750e 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -80,9 +80,15 @@ def apply_sorting(queryset, request): sort_by = request.GET.get("sort_by", "id") # Default to 'id' order = request.GET.get("order", "asc") # Default to 'asc' + if sort_by == "member": + sort_by = ["email", "first_name", "middle_name", "last_name"] + else: + sort_by = [sort_by] + if order == "desc": - sort_by = f"-{sort_by}" - return queryset.order_by(sort_by) + sort_by = [f"-{field}" for field in sort_by] + + return queryset.order_by(*sort_by) def serialize_members(request, portfolio, member, user, admin_ids, portfolio_invitation_emails):