fix sorting + unit tests

src/registrar/assets/js/get-gov.js

@@ -1930,7 +1930,7 @@ class MembersTable extends LoadTableBase {
 
             row.innerHTML = `
               <th scope="row" role="rowheader" data-label="member email">
-                ${member_email} ${admin_tagHTML}
+                ${member_email ? member_email : member_name} ${admin_tagHTML}
               </th>
               <td data-sort-value="${last_active}" data-label="last_active">
                 ${last_active}

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 = [

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)

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')
 

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):