From a01d1fa37801eaaf10e9374ea7ddff69d801b42c Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 16 Oct 2024 06:52:41 -0400
Subject: [PATCH 01/23] add roles, additional_permissions, domain_urls and
 domain_names to portfolio_members_json response

---
 src/registrar/views/portfolio_members_json.py | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py
index d2f2276cf..990db1108 100644
--- a/src/registrar/views/portfolio_members_json.py
+++ b/src/registrar/views/portfolio_members_json.py
@@ -3,10 +3,13 @@ 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.contrib.postgres.fields import ArrayField
+from django.contrib.postgres.aggregates import ArrayAgg
 from django.urls import reverse
 from django.db.models.functions import Cast
 
 from registrar.models.portfolio_invitation import PortfolioInvitation
+from registrar.models.user_domain_role import UserDomainRole
 from registrar.models.user_portfolio_permission import UserPortfolioPermission
 from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices
 
@@ -77,6 +80,17 @@ def initial_permissions_search(portfolio):
                 default=Value(""),
                 output_field=CharField(),
             ),
+            domain_info=ArrayAgg(
+                # an array of domains, with id and name, colon separated
+                Concat(
+                    F("user__permissions__domain_id"),
+                    Value(":"),
+                    F("user__permissions__domain__name"),
+                    # specify the output_field to ensure union has same column types
+                    output_field=CharField()
+                ),
+                distinct=True
+            ),
             source=Value("permission", output_field=CharField()),
         )
         .values(
@@ -88,6 +102,7 @@ def initial_permissions_search(portfolio):
             "roles",
             "additional_permissions_display",
             "member_display",
+            "domain_info",
             "source",
         )
     )
@@ -104,6 +119,7 @@ def initial_invitations_search(portfolio):
         last_active=Value("Invited", output_field=TextField()),
         additional_permissions_display=F("additional_permissions"),
         member_display=F("email"),
+        domain_info=Value([], output_field=ArrayField(TextField())),
         source=Value("invitation", output_field=CharField()),
     ).values(
         "id",
@@ -114,6 +130,7 @@ def initial_invitations_search(portfolio):
         "roles",
         "additional_permissions_display",
         "member_display",
+        "domain_info",
         "source",
     )
     return invitations
@@ -162,6 +179,11 @@ def serialize_members(request, portfolio, item, user):
         "name": " ".join(filter(None, [item.get("first_name", ""), item.get("last_name", "")])),
         "email": item.get("email_display", ""),
         "member_display": item.get("member_display", ""),
+        "roles": (item.get("roles") or []),
+        "additional_permissions": (item.get("additional_permissions_display") or []),
+        # split domain_info array values into ids to form urls, and names
+        "domain_urls": [reverse("domain", kwargs={"pk": domain_info.split(":")[0]}) for domain_info in item.get("domain_info")],
+        "domain_names": [domain_info.split(":")[1] for domain_info in item.get("domain_info")],
         "is_admin": is_admin,
         "last_active": item.get("last_active", ""),
         "action_url": action_url,

From 7f789eb4af7c74057f2be2baaab4c38f11100e79 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 16 Oct 2024 08:03:15 -0400
Subject: [PATCH 02/23] initial display of domains

---
 src/registrar/assets/js/get-gov.js | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js
index 337baf11c..8ad46c2f3 100644
--- a/src/registrar/assets/js/get-gov.js
+++ b/src/registrar/assets/js/get-gov.js
@@ -1929,6 +1929,8 @@ class MembersTable extends LoadTableBase {
           data.members.forEach(member => {
             const member_name = member.name;
             const member_display = member.member_display;
+            const domain_urls = member.domain_urls;
+            const domain_names = member.domain_names;
             const options = { year: 'numeric', month: 'short', day: 'numeric' };
             
             // Handle last_active values
@@ -1968,9 +1970,19 @@ class MembersTable extends LoadTableBase {
             if (member.is_admin)
               admin_tagHTML = `<span class="usa-tag margin-left-1 bg-primary">Admin</span>`
 
+            // domainsHTML block needs to be wrapped with hide/show toggle, Expand
+            let domainsHTML = '';
+            if (domain_urls.length > 0 && domain_names.length > 0) {
+              domainsHTML = "<ul>";
+              for (let i = 0; i < domain_urls.length; i++) {
+                  domainsHTML += `<li><a href="${domain_urls[i]}">${domain_names[i]}</a></li>`;
+              }
+              domainsHTML += "</ul>";
+            }
+
             row.innerHTML = `
               <th scope="row" role="rowheader" data-label="member email">
-                ${member_display} ${admin_tagHTML}
+                ${member_display} ${admin_tagHTML} ${domainsHTML}
               </th>
               <td data-sort-value="${last_active_sort_value}" data-label="last_active">
                 ${last_active_formatted}

From 026c70afeed09febdb946543611884dc43ba9c68 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 16 Oct 2024 08:25:40 -0400
Subject: [PATCH 03/23] refactor get_portfolio_permissions and return all
 permissions through json

---
 src/registrar/models/portfolio_invitation.py   | 13 +------------
 .../models/user_portfolio_permission.py        | 18 ++++++++++--------
 src/registrar/views/portfolio_members_json.py  |  8 +++-----
 3 files changed, 14 insertions(+), 25 deletions(-)

diff --git a/src/registrar/models/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py
index b1f22ae83..61a6b7397 100644
--- a/src/registrar/models/portfolio_invitation.py
+++ b/src/registrar/models/portfolio_invitation.py
@@ -79,19 +79,8 @@ class PortfolioInvitation(TimeStampedModel):
     def get_portfolio_permissions(self):
         """
         Retrieve the permissions for the user's portfolio roles from the invite.
-        This is similar logic to _get_portfolio_permissions in user_portfolio_permission
         """
-        # Use a set to avoid duplicate permissions
-        portfolio_permissions = set()
-
-        if self.roles:
-            for role in self.roles:
-                portfolio_permissions.update(UserPortfolioPermission.PORTFOLIO_ROLE_PERMISSIONS.get(role, []))
-
-        if self.additional_permissions:
-            portfolio_permissions.update(self.additional_permissions)
-
-        return list(portfolio_permissions)
+        return UserPortfolioPermission.get_portfolio_permissions(self.roles, self.additional_permissions)
 
     @transition(field="status", source=PortfolioInvitationStatus.INVITED, target=PortfolioInvitationStatus.RETRIEVED)
     def retrieve(self):
diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py
index c95a3f26b..b1eecd19b 100644
--- a/src/registrar/models/user_portfolio_permission.py
+++ b/src/registrar/models/user_portfolio_permission.py
@@ -92,16 +92,18 @@ class UserPortfolioPermission(TimeStampedModel):
         """
         Retrieve the permissions for the user's portfolio roles.
         """
+        return self.get_portfolio_permissions(self.roles, self.additional_permissions)
+
+    @classmethod
+    def get_portfolio_permissions(cls, roles, additional_permissions):
+        """Class method to return a list of permissions based on roles and addtl permissions"""
         # Use a set to avoid duplicate permissions
         portfolio_permissions = set()
-
-        if self.roles:
-            for role in self.roles:
-                portfolio_permissions.update(self.PORTFOLIO_ROLE_PERMISSIONS.get(role, []))
-
-        if self.additional_permissions:
-            portfolio_permissions.update(self.additional_permissions)
-
+        if roles:
+            for role in roles:
+                portfolio_permissions.update(cls.PORTFOLIO_ROLE_PERMISSIONS.get(role, []))
+        if additional_permissions:
+            portfolio_permissions.update(additional_permissions)
         return list(portfolio_permissions)
 
     def clean(self):
diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py
index 990db1108..6a0457aa2 100644
--- a/src/registrar/views/portfolio_members_json.py
+++ b/src/registrar/views/portfolio_members_json.py
@@ -63,7 +63,6 @@ def initial_permissions_search(portfolio):
             last_name=F("user__last_name"),
             email_display=F("user__email"),
             last_active=Cast(F("user__last_login"), output_field=TextField()),  # Cast last_login to text
-            additional_permissions_display=F("additional_permissions"),
             member_display=Case(
                 # If email is present and not blank, use email
                 When(Q(user__email__isnull=False) & ~Q(user__email=""), then=F("user__email")),
@@ -100,7 +99,7 @@ def initial_permissions_search(portfolio):
             "email_display",
             "last_active",
             "roles",
-            "additional_permissions_display",
+            "additional_permissions",
             "member_display",
             "domain_info",
             "source",
@@ -117,7 +116,6 @@ def initial_invitations_search(portfolio):
         last_name=Value(None, output_field=CharField()),
         email_display=F("email"),
         last_active=Value("Invited", output_field=TextField()),
-        additional_permissions_display=F("additional_permissions"),
         member_display=F("email"),
         domain_info=Value([], output_field=ArrayField(TextField())),
         source=Value("invitation", output_field=CharField()),
@@ -128,7 +126,7 @@ def initial_invitations_search(portfolio):
         "email_display",
         "last_active",
         "roles",
-        "additional_permissions_display",
+        "additional_permissions",
         "member_display",
         "domain_info",
         "source",
@@ -180,7 +178,7 @@ def serialize_members(request, portfolio, item, user):
         "email": item.get("email_display", ""),
         "member_display": item.get("member_display", ""),
         "roles": (item.get("roles") or []),
-        "additional_permissions": (item.get("additional_permissions_display") or []),
+        "permissions": UserPortfolioPermission.get_portfolio_permissions(item.get("roles"), item.get("additional_permissions")),
         # split domain_info array values into ids to form urls, and names
         "domain_urls": [reverse("domain", kwargs={"pk": domain_info.split(":")[0]}) for domain_info in item.get("domain_info")],
         "domain_names": [domain_info.split(":")[1] for domain_info in item.get("domain_info")],

From 6ebaab192009db3fce1fbd26024b596680f4f9ec Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 16 Oct 2024 10:46:14 -0400
Subject: [PATCH 04/23] initial display of permissions without formatting or
 display toggle

---
 src/registrar/assets/js/get-gov.js | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js
index 8ad46c2f3..0c89b4681 100644
--- a/src/registrar/assets/js/get-gov.js
+++ b/src/registrar/assets/js/get-gov.js
@@ -1929,6 +1929,7 @@ class MembersTable extends LoadTableBase {
           data.members.forEach(member => {
             const member_name = member.name;
             const member_display = member.member_display;
+            const member_permissions = member.permissions;
             const domain_urls = member.domain_urls;
             const domain_names = member.domain_names;
             const options = { year: 'numeric', month: 'short', day: 'numeric' };
@@ -1970,7 +1971,8 @@ class MembersTable extends LoadTableBase {
             if (member.is_admin)
               admin_tagHTML = `<span class="usa-tag margin-left-1 bg-primary">Admin</span>`
 
-            // domainsHTML block needs to be wrapped with hide/show toggle, Expand
+            // domainsHTML block and permissionsHTML block need to be wrapped with hide/show toggle, Expand
+
             let domainsHTML = '';
             if (domain_urls.length > 0 && domain_names.length > 0) {
               domainsHTML = "<ul>";
@@ -1980,9 +1982,32 @@ class MembersTable extends LoadTableBase {
               domainsHTML += "</ul>";
             }
 
+            // NOTE: need to replace strings below with constants from UserPortfolioPermission
+            // or return entire html block in json
+            console.log(member_permissions);
+            let permissionsHTML = '';
+            // only display domains permissions if domains assigned
+            if (domainsHTML) {
+              if (member_permissions.includes('view_all_domains')) {
+                permissionsHTML += "<p><b>Domains:</b> Can view all organization domains. Can manage domains they are assigned to and edit information about the domain (including DNS settings).</p>";
+              } else if (member_permissions.includes('view_managed_domains')) {
+                permissionsHTML += "<p><b>Domains:</b> Can manage domains they are assigned to and edit information about the domain (including DNS settings).</p>";
+              }
+            }
+            if (member_permissions.includes('edit_requests')) {
+              permissionsHTML += "<p><b>Domain requests:</b> Can view all organization domain requests. Can create domain requests and modify their own requests.</p>";
+            } else if (member_permissions.includes('view_all_requests')) {
+              permissionsHTML += "<p><b>Domain requests (view-only):</b> Can view all organization domain requests. Can't create or modify any domain requests.</p>";
+            }
+            if (member_permissions.includes('edit_members')) {
+              permissionsHTML += "<p><b>Members:</b> Can manage members including inviting new members, removing current members, and assigning domains to members.";
+            } else if (member_permissions.includes('view_members')) {
+              permissionsHTML += "<p><b>Members (view-only):</b> Can view all organizational members. Can't manage any members.";
+            }
+
             row.innerHTML = `
               <th scope="row" role="rowheader" data-label="member email">
-                ${member_display} ${admin_tagHTML} ${domainsHTML}
+                ${member_display} ${admin_tagHTML} ${domainsHTML} ${permissionsHTML}
               </th>
               <td data-sort-value="${last_active_sort_value}" data-label="last_active">
                 ${last_active_formatted}

From b2b66e0661fb003866b844102aab7450a49c9faa Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 16 Oct 2024 10:50:19 -0400
Subject: [PATCH 05/23] display of permissions when no additional permissions
 assigned

---
 src/registrar/assets/js/get-gov.js | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js
index 0c89b4681..3066c8441 100644
--- a/src/registrar/assets/js/get-gov.js
+++ b/src/registrar/assets/js/get-gov.js
@@ -2004,6 +2004,10 @@ class MembersTable extends LoadTableBase {
             } else if (member_permissions.includes('view_members')) {
               permissionsHTML += "<p><b>Members (view-only):</b> Can view all organizational members. Can't manage any members.";
             }
+            // if there are no additional permissions, display a no additional permissions message
+            if (!permissionsHTML) {
+              permissionsHTML += "<p><b>No additional permissions:</b> There are no additional permissions for this member.</p>";
+            }
 
             row.innerHTML = `
               <th scope="row" role="rowheader" data-label="member email">

From 12659c44922335990b147e6f4df0c8e14c39fc0e Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 16 Oct 2024 11:01:48 -0400
Subject: [PATCH 06/23] header info for domains and permissions display

---
 src/registrar/assets/js/get-gov.js | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js
index 3066c8441..4b8fa50b4 100644
--- a/src/registrar/assets/js/get-gov.js
+++ b/src/registrar/assets/js/get-gov.js
@@ -1932,6 +1932,7 @@ class MembersTable extends LoadTableBase {
             const member_permissions = member.permissions;
             const domain_urls = member.domain_urls;
             const domain_names = member.domain_names;
+            const num_domains = domain_urls.length;
             const options = { year: 'numeric', month: 'short', day: 'numeric' };
             
             // Handle last_active values
@@ -1974,12 +1975,17 @@ class MembersTable extends LoadTableBase {
             // domainsHTML block and permissionsHTML block need to be wrapped with hide/show toggle, Expand
 
             let domainsHTML = '';
-            if (domain_urls.length > 0 && domain_names.length > 0) {
-              domainsHTML = "<ul>";
-              for (let i = 0; i < domain_urls.length; i++) {
+            if (num_domains > 0) {
+              domainsHTML += "<h3>Domains assigned</h3>";
+              domainsHTML += "<p>This member is assigned to " + num_domains + " domains:";
+              domainsHTML += "<ul>";
+              for (let i = 0; i < num_domains && i < 6; i++) {
                   domainsHTML += `<li><a href="${domain_urls[i]}">${domain_names[i]}</a></li>`;
               }
               domainsHTML += "</ul>";
+              if (num_domains >= 6) {
+                domainsHTML += "<p><a href='#'>View assigned domains</a></p>";
+              }
             }
 
             // NOTE: need to replace strings below with constants from UserPortfolioPermission
@@ -2008,6 +2014,8 @@ class MembersTable extends LoadTableBase {
             if (!permissionsHTML) {
               permissionsHTML += "<p><b>No additional permissions:</b> There are no additional permissions for this member.</p>";
             }
+            // add permissions header in all cases
+            permissionsHTML = "<h3>Additional permissions for this member</h3>" + permissionsHTML;
 
             row.innerHTML = `
               <th scope="row" role="rowheader" data-label="member email">

From bf4ab6aa3517f1eb6dc1ba500930f7f6fef48649 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 16 Oct 2024 11:37:55 -0400
Subject: [PATCH 07/23] replaced strings in javascript with enums and pass from
 enum class to js

---
 src/registrar/assets/js/get-gov.js              | 17 ++++++++---------
 .../models/utility/portfolio_helper.py          |  4 ++++
 src/registrar/views/portfolio_members_json.py   |  3 ++-
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js
index 4b8fa50b4..547866ca5 100644
--- a/src/registrar/assets/js/get-gov.js
+++ b/src/registrar/assets/js/get-gov.js
@@ -1926,6 +1926,8 @@ class MembersTable extends LoadTableBase {
           const memberList = document.querySelector('.members__table tbody');
           memberList.innerHTML = '';
 
+          const UserPortfolioPermissionChoices = data.UserPortfolioPermissionChoices;
+
           data.members.forEach(member => {
             const member_name = member.name;
             const member_display = member.member_display;
@@ -1988,26 +1990,23 @@ class MembersTable extends LoadTableBase {
               }
             }
 
-            // NOTE: need to replace strings below with constants from UserPortfolioPermission
-            // or return entire html block in json
-            console.log(member_permissions);
             let permissionsHTML = '';
             // only display domains permissions if domains assigned
             if (domainsHTML) {
-              if (member_permissions.includes('view_all_domains')) {
+              if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS)) {
                 permissionsHTML += "<p><b>Domains:</b> Can view all organization domains. Can manage domains they are assigned to and edit information about the domain (including DNS settings).</p>";
-              } else if (member_permissions.includes('view_managed_domains')) {
+              } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS)) {
                 permissionsHTML += "<p><b>Domains:</b> Can manage domains they are assigned to and edit information about the domain (including DNS settings).</p>";
               }
             }
-            if (member_permissions.includes('edit_requests')) {
+            if (member_permissions.includes(UserPortfolioPermissionChoices.EDIT_REQUESTS)) {
               permissionsHTML += "<p><b>Domain requests:</b> Can view all organization domain requests. Can create domain requests and modify their own requests.</p>";
-            } else if (member_permissions.includes('view_all_requests')) {
+            } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS)) {
               permissionsHTML += "<p><b>Domain requests (view-only):</b> Can view all organization domain requests. Can't create or modify any domain requests.</p>";
             }
-            if (member_permissions.includes('edit_members')) {
+            if (member_permissions.includes(UserPortfolioPermissionChoices.EDIT_MEMBERS)) {
               permissionsHTML += "<p><b>Members:</b> Can manage members including inviting new members, removing current members, and assigning domains to members.";
-            } else if (member_permissions.includes('view_members')) {
+            } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_MEMBERS)) {
               permissionsHTML += "<p><b>Members (view-only):</b> Can view all organizational members. Can't manage any members.";
             }
             // if there are no additional permissions, display a no additional permissions message
diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py
index ddb487f71..a7367e6e3 100644
--- a/src/registrar/models/utility/portfolio_helper.py
+++ b/src/registrar/models/utility/portfolio_helper.py
@@ -36,3 +36,7 @@ class UserPortfolioPermissionChoices(models.TextChoices):
     @classmethod
     def get_user_portfolio_permission_label(cls, user_portfolio_permission):
         return cls(user_portfolio_permission).label if user_portfolio_permission else None
+    
+    @classmethod
+    def to_dict(cls):
+        return {key: value.value for key, value in cls.__members__.items()}
diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py
index 6a0457aa2..683044bca 100644
--- a/src/registrar/views/portfolio_members_json.py
+++ b/src/registrar/views/portfolio_members_json.py
@@ -11,7 +11,7 @@ from django.db.models.functions import Cast
 from registrar.models.portfolio_invitation import PortfolioInvitation
 from registrar.models.user_domain_role import UserDomainRole
 from registrar.models.user_portfolio_permission import UserPortfolioPermission
-from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices
+from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices
 
 
 @login_required
@@ -43,6 +43,7 @@ def get_portfolio_members_json(request):
     return JsonResponse(
         {
             "members": members,
+            "UserPortfolioPermissionChoices": UserPortfolioPermissionChoices.to_dict(),
             "page": page_obj.number,
             "num_pages": paginator.num_pages,
             "has_previous": page_obj.has_previous(),

From c5de4b3a1d40c3d5ec735b2e536d47608d1ce45f Mon Sep 17 00:00:00 2001
From: Rachid Mrad <rachid.mrad@ecstech.com>
Date: Wed, 16 Oct 2024 15:41:52 -0400
Subject: [PATCH 08/23] UI

---
 src/registrar/assets/js/get-gov.js            | 100 ++++++++++++++----
 .../assets/sass/_theme/_buttons.scss          |   8 ++
 src/registrar/assets/sass/_theme/_tables.scss |   6 +-
 .../assets/sass/_theme/_typography.scss       |   5 +
 .../templates/includes/members_table.html     |  10 +-
 src/registrar/views/portfolio_members_json.py |   1 +
 6 files changed, 104 insertions(+), 26 deletions(-)

diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js
index 7d1b52399..f3457649c 100644
--- a/src/registrar/assets/js/get-gov.js
+++ b/src/registrar/assets/js/get-gov.js
@@ -1873,6 +1873,41 @@ class MembersTable extends LoadTableBase {
   constructor() {
     super('.members__table', '.members__table-wrapper', '#members__search-field', '#members__search-field-submit', '.members__reset-search', '.members__reset-filters', '.members__no-data', '.members__no-search-results');
   }
+  
+  initShowMoreButtons() {
+    function toggleShowMoreButton(toggleButton, contentDiv, buttonParentRow) {
+      const spanElement = toggleButton.querySelector('span');
+      const useElement = toggleButton.querySelector('use');
+      if (contentDiv.classList.contains('display-none')) {
+        showElement(contentDiv);
+        spanElement.textContent = 'Close';
+        useElement.setAttribute('xlink:href', '/public/img/sprite.svg#expand_less');
+        buttonParentRow.classList.add('hide-td-borders');
+      } else {    
+        hideElement(contentDiv);
+        spanElement.textContent = 'Expand';
+        useElement.setAttribute('xlink:href', '/public/img/sprite.svg#expand_more');
+        buttonParentRow.classList.remove('hide-td-borders');
+      }
+    }
+  
+    let toggleButtons = document.querySelectorAll('.usa-button--show-more-button');
+    toggleButtons.forEach((toggleButton) => {
+      
+      let dataFor = toggleButton.dataset.for;
+      let contentDiv = document.getElementById(dataFor);
+      let buttonParentRow = toggleButton.parentElement.parentElement;
+      if (contentDiv && contentDiv.tagName.toLowerCase() === 'tr' && contentDiv.classList.contains('show-more-content') && buttonParentRow && buttonParentRow.tagName.toLowerCase() === 'tr') {
+        toggleButton.addEventListener('click', function() {
+          toggleShowMoreButton(toggleButton, contentDiv, buttonParentRow);
+        });
+      } else {
+        console.warn('Found a toggle button with no associated toggleable content or parent row');
+      }
+
+    });
+  }
+
   /**
      * Loads rows in the members list, as well as updates pagination around the members list
      * based on the supplied attributes.
@@ -1930,6 +1965,7 @@ class MembersTable extends LoadTableBase {
           const invited = 'Invited';
 
           data.members.forEach(member => {
+            const member_id = member.source + member.id;
             const member_name = member.name;
             const member_display = member.member_display;
             const member_permissions = member.permissions;
@@ -1979,9 +2015,10 @@ class MembersTable extends LoadTableBase {
 
             let domainsHTML = '';
             if (num_domains > 0) {
-              domainsHTML += "<h3>Domains assigned</h3>";
-              domainsHTML += "<p>This member is assigned to " + num_domains + " domains:";
-              domainsHTML += "<ul>";
+              domainsHTML += "<div class='desktop:grid-col-5 margin-bottom-2 desktop:margin-bottom-0'>";
+              domainsHTML += "<h4 class='margin-y-0 text-primary'>Domains assigned</h4>";
+              domainsHTML += "<p class='margin-y-0'>This member is assigned to " + num_domains + " domains:";
+              domainsHTML += "<ul class='usa-list usa-list--unstyled margin-y-0'>";
               for (let i = 0; i < num_domains && i < 6; i++) {
                   domainsHTML += `<li><a href="${domain_urls[i]}">${domain_names[i]}</a></li>`;
               }
@@ -1989,42 +2026,62 @@ class MembersTable extends LoadTableBase {
               if (num_domains >= 6) {
                 domainsHTML += "<p><a href='#'>View assigned domains</a></p>";
               }
+              domainsHTML += "</div>";
             }
 
             let permissionsHTML = '';
-            // only display domains permissions if domains assigned
-            if (domainsHTML) {
-              if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS)) {
-                permissionsHTML += "<p><b>Domains:</b> Can view all organization domains. Can manage domains they are assigned to and edit information about the domain (including DNS settings).</p>";
-              } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS)) {
-                permissionsHTML += "<p><b>Domains:</b> Can manage domains they are assigned to and edit information about the domain (including DNS settings).</p>";
-              }
+            if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS)) {
+              permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Domains:</strong> Can view all organization domains. Can manage domains they are assigned to and edit information about the domain (including DNS settings).</p>";
+            } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS)) {
+              permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Domains:</strong> Can manage domains they are assigned to and edit information about the domain (including DNS settings).</p>";
             }
             if (member_permissions.includes(UserPortfolioPermissionChoices.EDIT_REQUESTS)) {
-              permissionsHTML += "<p><b>Domain requests:</b> Can view all organization domain requests. Can create domain requests and modify their own requests.</p>";
+              permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Domain requests:</strong> Can view all organization domain requests. Can create domain requests and modify their own requests.</p>";
             } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS)) {
-              permissionsHTML += "<p><b>Domain requests (view-only):</b> Can view all organization domain requests. Can't create or modify any domain requests.</p>";
+              permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Domain requests (view-only):</strong> Can view all organization domain requests. Can't create or modify any domain requests.</p>";
             }
             if (member_permissions.includes(UserPortfolioPermissionChoices.EDIT_MEMBERS)) {
-              permissionsHTML += "<p><b>Members:</b> Can manage members including inviting new members, removing current members, and assigning domains to members.";
+              permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Members:</strong> Can manage members including inviting new members, removing current members, and assigning domains to members.";
             } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_MEMBERS)) {
-              permissionsHTML += "<p><b>Members (view-only):</b> Can view all organizational members. Can't manage any members.";
+              permissionsHTML += "<p> class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Members (view-only):</strong> Can view all organizational members. Can't manage any members.";
             }
             // if there are no additional permissions, display a no additional permissions message
             if (!permissionsHTML) {
               permissionsHTML += "<p><b>No additional permissions:</b> There are no additional permissions for this member.</p>";
             }
             // add permissions header in all cases
-            permissionsHTML = "<h3>Additional permissions for this member</h3>" + permissionsHTML;
+            permissionsHTML = "<div class='desktop:grid-col-7'><h4 class='margin-y-0 text-primary'>Additional permissions for this member</h4>" + permissionsHTML + "</div>";
+
+            let showMoreButton = '';
+            const showMoreRow = document.createElement('tr');
+            if (domainsHTML || permissionsHTML) {
+              showMoreButton = `
+                <button 
+                  type="button" 
+                  class="usa-button--show-more-button usa-button usa-button--unstyled display-block margin-top-2" 
+                  data-for=${member_id} 
+                >
+                  <span>Expand</span>
+                  <svg class="usa-icon usa-icon--big" aria-hidden="true" focusable="false" role="img" width="24">
+                    <use xlink:href="/public/img/sprite.svg#expand_more"></use>
+                  </svg>
+                </button>
+              `;
+
+              showMoreRow.innerHTML = `<td colspan='3' headers="header-member" class="padding-top-0"><div class='grid-row'>${domainsHTML} ${permissionsHTML}</div></td>`;
+              showMoreRow.classList.add('show-more-content');
+              showMoreRow.classList.add('display-none');
+              showMoreRow.id = member_id;
+            }
 
             row.innerHTML = `
-              <th scope="row" role="rowheader" data-label="member email">
-                ${member_display} ${admin_tagHTML} ${domainsHTML} ${permissionsHTML}
+              <th role="rowheader" headers="header-member" data-label="member email" id='row-header-${member_id}'>
+                ${member_display} ${admin_tagHTML} ${showMoreButton}
               </th>
-              <td data-sort-value="${last_active_sort_value}" data-label="last_active">
+              <td headers="header-last-active row-header-${member_id}" data-sort-value="${last_active_sort_value}" data-label="last_active">
                 ${last_active_formatted}
               </td>
-              <td>
+              <td headers="header-action row-header-${member_id}">
                 <a href="${action_url}">
                   <svg class="usa-icon" aria-hidden="true" focusable="false" role="img" width="24">
                     <use xlink:href="/public/img/sprite.svg#${svg_icon}"></use>
@@ -2034,8 +2091,13 @@ class MembersTable extends LoadTableBase {
               </td>
             `;
             memberList.appendChild(row);
+            if (domainsHTML || permissionsHTML) {
+              memberList.appendChild(showMoreRow);
+            }
           });
 
+          this.initShowMoreButtons();
+
           // Do not scroll on first page load
           if (scroll)
             ScrollToElement('class', 'members');
diff --git a/src/registrar/assets/sass/_theme/_buttons.scss b/src/registrar/assets/sass/_theme/_buttons.scss
index ff5ffb386..c58242fe7 100644
--- a/src/registrar/assets/sass/_theme/_buttons.scss
+++ b/src/registrar/assets/sass/_theme/_buttons.scss
@@ -258,3 +258,11 @@ a.text-secondary,
 a.text-secondary:hover {
   color: $theme-color-error;
 }
+
+.usa-button--show-more-button {
+  font-size: size('ui', 'xs');
+  text-decoration: none;
+  .usa-icon {
+    top: 6px;
+  }
+}
diff --git a/src/registrar/assets/sass/_theme/_tables.scss b/src/registrar/assets/sass/_theme/_tables.scss
index d57b51117..ed2d5685b 100644
--- a/src/registrar/assets/sass/_theme/_tables.scss
+++ b/src/registrar/assets/sass/_theme/_tables.scss
@@ -56,8 +56,10 @@ th {
       border: none;
     }
 
-    td, th {
-      border-bottom: 1px solid color('base-lighter');
+    tr:not(.hide-td-borders) {
+      td, th {
+        border-bottom: 1px solid color('base-lighter');
+      }
     }
 
     thead th {
diff --git a/src/registrar/assets/sass/_theme/_typography.scss b/src/registrar/assets/sass/_theme/_typography.scss
index cc0d39a5b..d815ef6dd 100644
--- a/src/registrar/assets/sass/_theme/_typography.scss
+++ b/src/registrar/assets/sass/_theme/_typography.scss
@@ -28,3 +28,8 @@ h2 {
 .usa-form fieldset {
   font-size: 1rem;
 }
+
+.p--blockquote {
+  padding-left: units(1);
+  border-left: 2px solid color('base-lighter');
+}
diff --git a/src/registrar/templates/includes/members_table.html b/src/registrar/templates/includes/members_table.html
index 529d2629d..742f1d4a4 100644
--- a/src/registrar/templates/includes/members_table.html
+++ b/src/registrar/templates/includes/members_table.html
@@ -39,16 +39,16 @@
     </div>
 
     <!-- ---------- MAIN TABLE ---------- -->
-    <div class="members__table-wrapper display-none usa-table-container--scrollable margin-top-0" tabindex="0">
+    <div class="members__table-wrapper display-none margin-top-0">
       <table class="usa-table usa-table--borderless usa-table--stacked dotgov-table dotgov-table--stacked members__table">
         <caption class="sr-only">Your registered members</caption>
         <thead>
           <tr>
-            <th data-sortable="member" scope="col" role="columnheader">Member</th>
-            <th data-sortable="last_active" scope="col" role="columnheader">Last Active</th>
+            <th data-sortable="member" role="columnheader" id="header-member">Member</th>
+            <th data-sortable="last_active" role="columnheader" id="header-last-active">Last Active</th>
             <th 
-              scope="col" 
-              role="columnheader" 
+              role="columnheader"
+              id="header-action"
             >
               <span class="usa-sr-only">Action</span>
             </th>
diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py
index 683044bca..b76d185f5 100644
--- a/src/registrar/views/portfolio_members_json.py
+++ b/src/registrar/views/portfolio_members_json.py
@@ -175,6 +175,7 @@ def serialize_members(request, portfolio, item, user):
     # Serialize member data
     member_json = {
         "id": item.get("id", ""),
+        "source": item.get("source", ""),
         "name": " ".join(filter(None, [item.get("first_name", ""), item.get("last_name", "")])),
         "email": item.get("email_display", ""),
         "member_display": item.get("member_display", ""),

From 3d6417bb995d6e56d1f8f9632f0819ee648dd1b5 Mon Sep 17 00:00:00 2001
From: Rachid Mrad <rachid.mrad@ecstech.com>
Date: Thu, 17 Oct 2024 13:27:57 -0400
Subject: [PATCH 09/23] Unit tests, get domains for invited member

---
 src/registrar/assets/js/get-gov.js            |   6 +-
 .../tests/test_views_members_json.py          | 340 +++++++++++++++---
 src/registrar/views/portfolio_members_json.py |  44 ++-
 3 files changed, 328 insertions(+), 62 deletions(-)

diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js
index f3457649c..cd2e251ab 100644
--- a/src/registrar/assets/js/get-gov.js
+++ b/src/registrar/assets/js/get-gov.js
@@ -2043,11 +2043,11 @@ class MembersTable extends LoadTableBase {
             if (member_permissions.includes(UserPortfolioPermissionChoices.EDIT_MEMBERS)) {
               permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Members:</strong> Can manage members including inviting new members, removing current members, and assigning domains to members.";
             } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_MEMBERS)) {
-              permissionsHTML += "<p> class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Members (view-only):</strong> Can view all organizational members. Can't manage any members.";
+              permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Members (view-only):</strong> Can view all organizational members. Can't manage any members.";
             }
             // if there are no additional permissions, display a no additional permissions message
             if (!permissionsHTML) {
-              permissionsHTML += "<p><b>No additional permissions:</b> There are no additional permissions for this member.</p>";
+              permissionsHTML += "<p class='margin-top-1 p--blockquote'><b>No additional permissions:</b> There are no additional permissions for this member.</p>";
             }
             // add permissions header in all cases
             permissionsHTML = "<div class='desktop:grid-col-7'><h4 class='margin-y-0 text-primary'>Additional permissions for this member</h4>" + permissionsHTML + "</div>";
@@ -2058,7 +2058,7 @@ class MembersTable extends LoadTableBase {
               showMoreButton = `
                 <button 
                   type="button" 
-                  class="usa-button--show-more-button usa-button usa-button--unstyled display-block margin-top-2" 
+                  class="usa-button--show-more-button usa-button usa-button--unstyled display-block margin-top-1" 
                   data-for=${member_id} 
                 >
                   <span>Expand</span>
diff --git a/src/registrar/tests/test_views_members_json.py b/src/registrar/tests/test_views_members_json.py
index 9cd4e823c..77a7a5566 100644
--- a/src/registrar/tests/test_views_members_json.py
+++ b/src/registrar/tests/test_views_members_json.py
@@ -1,21 +1,25 @@
 from django.urls import reverse
 
+from registrar.models.domain import Domain
+from registrar.models.domain_information import DomainInformation
+from registrar.models.domain_invitation import DomainInvitation
 from registrar.models.portfolio import Portfolio
 from registrar.models.portfolio_invitation import PortfolioInvitation
 from registrar.models.user import User
+from registrar.models.user_domain_role import UserDomainRole
 from registrar.models.user_portfolio_permission import UserPortfolioPermission
 from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices
-from .test_views import TestWithUser
+from registrar.tests.common import MockEppLib, create_test_user
 from django_webtest import WebTest  # type: ignore
 
 
-class GetPortfolioMembersJsonTest(TestWithUser, WebTest):
-    @classmethod
-    def setUpClass(cls):
-        super().setUpClass()
+class GetPortfolioMembersJsonTest(MockEppLib, WebTest):
+    def setUp(self):
+        super().setUp()
+        self.user = create_test_user()
 
         # Create additional users
-        cls.user2 = User.objects.create(
+        self.user2 = User.objects.create(
             username="test_user2",
             first_name="Second",
             last_name="User",
@@ -23,7 +27,7 @@ class GetPortfolioMembersJsonTest(TestWithUser, WebTest):
             phone="8003112345",
             title="Member",
         )
-        cls.user3 = User.objects.create(
+        self.user3 = User.objects.create(
             username="test_user3",
             first_name="Third",
             last_name="User",
@@ -31,7 +35,7 @@ class GetPortfolioMembersJsonTest(TestWithUser, WebTest):
             phone="8003113456",
             title="Member",
         )
-        cls.user4 = User.objects.create(
+        self.user4 = User.objects.create(
             username="test_user4",
             first_name="Fourth",
             last_name="User",
@@ -39,60 +43,66 @@ class GetPortfolioMembersJsonTest(TestWithUser, WebTest):
             phone="8003114567",
             title="Admin",
         )
-        cls.email5 = "fifth@example.com"
+        self.user5 = User.objects.create(
+            username="test_user5",
+            first_name="Fifth",
+            last_name="User",
+            email="fifth@example.com",
+            phone="8003114568",
+            title="Admin",
+        )
+        self.email6 = "fifth@example.com"
 
         # Create Portfolio
-        cls.portfolio = Portfolio.objects.create(creator=cls.user, organization_name="Test Portfolio")
+        self.portfolio = Portfolio.objects.create(creator=self.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],
-        )
-        PortfolioInvitation.objects.create(
-            email=cls.email5,
-            portfolio=cls.portfolio,
-            roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
-            additional_permissions=[
-                UserPortfolioPermissionChoices.VIEW_MEMBERS,
-                UserPortfolioPermissionChoices.EDIT_MEMBERS,
-            ],
-        )
 
-    @classmethod
-    def tearDownClass(cls):
+        self.app.set_user(self.user.username)
+
+    def tearDown(self):
+        UserDomainRole.objects.all().delete()
+        DomainInformation.objects.all().delete()
+        Domain.objects.all().delete()
         PortfolioInvitation.objects.all().delete()
         UserPortfolioPermission.objects.all().delete()
         Portfolio.objects.all().delete()
         User.objects.all().delete()
-        super().tearDownClass()
-
-    def setUp(self):
-        super().setUp()
-        self.app.set_user(self.user.username)
+        super().tearDown()
 
     def test_get_portfolio_members_json_authenticated(self):
         """Test that portfolio members are returned properly for an authenticated user."""
+        """Also tests that reposnse is 200 when no domains"""
+        UserPortfolioPermission.objects.create(
+            user=self.user,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
+            additional_permissions=[
+                UserPortfolioPermissionChoices.VIEW_MEMBERS,
+                UserPortfolioPermissionChoices.EDIT_MEMBERS,
+            ],
+        )
+        UserPortfolioPermission.objects.create(
+            user=self.user2,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
+        )
+        UserPortfolioPermission.objects.create(
+            user=self.user3,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
+        )
+        UserPortfolioPermission.objects.create(
+            user=self.user4,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
+        )
+        UserPortfolioPermission.objects.create(
+            user=self.user5,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
+        )
+
         response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id})
         self.assertEqual(response.status_code, 200)
         data = response.json
@@ -115,15 +125,207 @@ class GetPortfolioMembersJsonTest(TestWithUser, WebTest):
             self.user3.email,
             self.user4.email,
             self.user4.email,
-            self.email5,
+            self.user5.email,
         }
         actual_emails = {member["email"] for member in data["members"]}
         self.assertEqual(expected_emails, actual_emails)
 
+        expected_roles = {
+            UserPortfolioRoleChoices.ORGANIZATION_MEMBER,
+            UserPortfolioRoleChoices.ORGANIZATION_ADMIN,
+        }
+        # Convert each member's roles list to a frozenset
+        actual_roles = {role for member in data["members"] for role in member["roles"]}
+        self.assertEqual(expected_roles, actual_roles)
+        
+        expected_additional_permissions = {
+            UserPortfolioPermissionChoices.VIEW_MEMBERS,
+            UserPortfolioPermissionChoices.EDIT_MEMBERS,
+        }
+        actual_additional_permissions = {permission for member in data["members"]  for permission in member["permissions"]}
+        self.assertTrue(expected_additional_permissions.issubset(actual_additional_permissions))
+
+    def test_get_portfolio_invited_json_authenticated(self):
+        """Test that portfolio invitees are returned properly for an authenticated user."""
+        """Also tests that reposnse is 200 when no domains"""
+        PortfolioInvitation.objects.create(
+            email=self.email6,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
+            additional_permissions=[
+                UserPortfolioPermissionChoices.VIEW_MEMBERS,
+                UserPortfolioPermissionChoices.EDIT_MEMBERS,
+            ],
+        )
+
+        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.assertEqual(data["num_pages"], 1)
+        self.assertEqual(data["total"], 1)
+        self.assertEqual(data["unfiltered_total"], 1)
+
+        # Check the number of members
+        self.assertEqual(len(data["members"]), 1)
+
+        # Check member fields
+        expected_emails = {
+            self.email6
+        }
+        actual_emails = {member["email"] for member in data["members"]}
+        self.assertEqual(expected_emails, actual_emails)
+
+        expected_roles = {
+            UserPortfolioRoleChoices.ORGANIZATION_ADMIN,
+        }
+        # Convert each member's roles list to a frozenset
+        actual_roles = {role for member in data["members"] for role in member["roles"]}
+        self.assertEqual(expected_roles, actual_roles)
+        
+        expected_additional_permissions = {
+            UserPortfolioPermissionChoices.VIEW_MEMBERS,
+            UserPortfolioPermissionChoices.EDIT_MEMBERS,
+        }
+        actual_additional_permissions = {permission for member in data["members"]  for permission in member["permissions"]}
+        self.assertTrue(expected_additional_permissions.issubset(actual_additional_permissions))
+
+    def test_get_portfolio_members_json_with_domains(self):
+        """Test that portfolio members are returned properly for an authenticated user and the response includes
+        the domains that the member manages.."""
+        UserPortfolioPermission.objects.create(
+            user=self.user,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
+            additional_permissions=[
+                UserPortfolioPermissionChoices.VIEW_MEMBERS,
+                UserPortfolioPermissionChoices.EDIT_MEMBERS,
+            ],
+        )
+        UserPortfolioPermission.objects.create(
+            user=self.user2,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
+        )
+        UserPortfolioPermission.objects.create(
+            user=self.user3,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
+        )
+        UserPortfolioPermission.objects.create(
+            user=self.user4,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
+        )
+
+        domain = Domain.objects.create(
+            name="somedomain1.com",
+        )
+
+        DomainInformation.objects.create(
+            creator=self.user,
+            domain=domain,
+            portfolio=self.portfolio,
+        )
+
+        UserDomainRole.objects.create(
+            user=self.user,
+            domain=domain,
+            role=UserDomainRole.Roles.MANAGER,
+        )
+
+        response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id})
+        self.assertEqual(response.status_code, 200)
+        data = response.json
+
+        # Check if the domain appears in the response JSON
+        domain_names = [
+            domain_name
+            for member in data["members"]
+            for domain_name in member.get("domain_names", [])
+        ]
+        self.assertIn("somedomain1.com", domain_names)
+
+    def test_get_portfolio_invited_json_with_domains(self):
+        """Test that portfolio invited members are returned properly for an authenticated user and the response includes
+        the domains that the member manages.."""
+        PortfolioInvitation.objects.create(
+            email=self.email6,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
+            additional_permissions=[
+                UserPortfolioPermissionChoices.VIEW_MEMBERS,
+                UserPortfolioPermissionChoices.EDIT_MEMBERS,
+            ],
+        )
+
+        domain = Domain.objects.create(
+            name="somedomain1.com",
+        )
+
+        DomainInformation.objects.create(
+            creator=self.user,
+            domain=domain,
+            portfolio=self.portfolio,
+        )
+
+        DomainInvitation.objects.create(
+            email=self.email6,
+            domain=domain,
+        )
+
+        response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id})
+        self.assertEqual(response.status_code, 200)
+        data = response.json
+
+        # Check if the domain appears in the response JSON
+        domain_names = [
+            domain_name
+            for member in data["members"]
+            for domain_name in member.get("domain_names", [])
+        ]
+        self.assertIn("somedomain1.com", domain_names)
+
     def test_pagination(self):
         """Test that pagination works properly when there are more members than page size."""
+        UserPortfolioPermission.objects.create(
+            user=self.user,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
+            additional_permissions=[
+                UserPortfolioPermissionChoices.VIEW_MEMBERS,
+                UserPortfolioPermissionChoices.EDIT_MEMBERS,
+            ],
+        )
+        UserPortfolioPermission.objects.create(
+            user=self.user2,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
+        )
+        UserPortfolioPermission.objects.create(
+            user=self.user3,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
+        )
+        UserPortfolioPermission.objects.create(
+            user=self.user4,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
+        )
+        PortfolioInvitation.objects.create(
+            email=self.email6,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
+            additional_permissions=[
+                UserPortfolioPermissionChoices.VIEW_MEMBERS,
+                UserPortfolioPermissionChoices.EDIT_MEMBERS,
+            ],
+        )
+
         # Create additional members to exceed page size of 10
-        for i in range(5, 15):
+        for i in range(6, 16):
             user, _ = User.objects.get_or_create(
                 username=f"test_user{i}",
                 first_name=f"User{i}",
@@ -172,6 +374,40 @@ class GetPortfolioMembersJsonTest(TestWithUser, WebTest):
 
     def test_search(self):
         """Test search functionality for portfolio members."""
+        UserPortfolioPermission.objects.create(
+            user=self.user,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
+            additional_permissions=[
+                UserPortfolioPermissionChoices.VIEW_MEMBERS,
+                UserPortfolioPermissionChoices.EDIT_MEMBERS,
+            ],
+        )
+        UserPortfolioPermission.objects.create(
+            user=self.user2,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
+        )
+        UserPortfolioPermission.objects.create(
+            user=self.user3,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
+        )
+        UserPortfolioPermission.objects.create(
+            user=self.user4,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
+        )
+        PortfolioInvitation.objects.create(
+            email=self.email6,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
+            additional_permissions=[
+                UserPortfolioPermissionChoices.VIEW_MEMBERS,
+                UserPortfolioPermissionChoices.EDIT_MEMBERS,
+            ],
+        )
+
         # Search by name
         response = self.app.get(
             reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id, "search_term": "Second"}
diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py
index b76d185f5..e888feda5 100644
--- a/src/registrar/views/portfolio_members_json.py
+++ b/src/registrar/views/portfolio_members_json.py
@@ -1,13 +1,14 @@
 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 import Value, F, CharField, TextField, Q, Case, When, OuterRef, Subquery
 from django.db.models.functions import Concat, Coalesce
 from django.contrib.postgres.fields import ArrayField
 from django.contrib.postgres.aggregates import ArrayAgg
 from django.urls import reverse
 from django.db.models.functions import Cast
 
+from registrar.models.domain_invitation import DomainInvitation
 from registrar.models.portfolio_invitation import PortfolioInvitation
 from registrar.models.user_domain_role import UserDomainRole
 from registrar.models.user_portfolio_permission import UserPortfolioPermission
@@ -89,7 +90,8 @@ def initial_permissions_search(portfolio):
                     # specify the output_field to ensure union has same column types
                     output_field=CharField()
                 ),
-                distinct=True
+                distinct=True,
+                filter=Q(user__permissions__domain__isnull=False)
             ),
             source=Value("permission", output_field=CharField()),
         )
@@ -110,7 +112,22 @@ def initial_permissions_search(portfolio):
 
 
 def initial_invitations_search(portfolio):
-    """Perform initial invitations search before applying any filters."""
+    """Perform initial invitations search and get related DomainInvitation data based on the email."""
+
+
+    # Get DomainInvitation query for matching email
+    domain_invitations = DomainInvitation.objects.filter(
+        email=OuterRef('email'),
+        domain__isnull=False
+    ).annotate(
+        domain_info=Concat(
+            F('domain__id'),
+            Value(':'),
+            F('domain__name'),
+            output_field=CharField()
+        )
+    )
+
     invitations = PortfolioInvitation.objects.filter(portfolio=portfolio)
     invitations = invitations.annotate(
         first_name=Value(None, output_field=CharField()),
@@ -118,7 +135,14 @@ def initial_invitations_search(portfolio):
         email_display=F("email"),
         last_active=Value("Invited", output_field=TextField()),
         member_display=F("email"),
-        domain_info=Value([], output_field=ArrayField(TextField())),
+        # ArrayAgg for multiple domain_invitations matched by email, filtered to exclude nulls
+        domain_info=Coalesce(  # Use Coalesce to return an empty list if no domain invitations exist
+            ArrayAgg(
+                Subquery(domain_invitations.values('domain_info')),
+                distinct=True,
+            ),
+            Value([], output_field=ArrayField(CharField()))  # Ensure we return an empty list
+        ),
         source=Value("invitation", output_field=CharField()),
     ).values(
         "id",
@@ -181,9 +205,15 @@ def serialize_members(request, portfolio, item, user):
         "member_display": item.get("member_display", ""),
         "roles": (item.get("roles") or []),
         "permissions": UserPortfolioPermission.get_portfolio_permissions(item.get("roles"), item.get("additional_permissions")),
-        # split domain_info array values into ids to form urls, and names
-        "domain_urls": [reverse("domain", kwargs={"pk": domain_info.split(":")[0]}) for domain_info in item.get("domain_info")],
-        "domain_names": [domain_info.split(":")[1] for domain_info in item.get("domain_info")],
+        "domain_names": [
+            domain_info.split(":")[1] for domain_info in (item.get("domain_info") or [])
+            if domain_info is not None  # Prevent splitting None
+        ],
+        "domain_urls": [
+            reverse("domain", kwargs={"pk": domain_info.split(":")[0]}) 
+            for domain_info in (item.get("domain_info") or [])
+            if domain_info is not None  # Prevent splitting None
+        ],
         "is_admin": is_admin,
         "last_active": item.get("last_active", ""),
         "action_url": action_url,

From 68ea6db752016919bf79e995e327bcc77c043e76 Mon Sep 17 00:00:00 2001
From: Rachid Mrad <rachid.mrad@ecstech.com>
Date: Thu, 17 Oct 2024 13:37:48 -0400
Subject: [PATCH 10/23] Fix last active JS bug

---
 src/registrar/assets/js/get-gov.js | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js
index cd2e251ab..ab94bec7a 100644
--- a/src/registrar/assets/js/get-gov.js
+++ b/src/registrar/assets/js/get-gov.js
@@ -1975,20 +1975,23 @@ class MembersTable extends LoadTableBase {
             const options = { year: 'numeric', month: 'short', day: 'numeric' };
             
             // Handle last_active values
-            let last_active = member.last_active;
+            let last_active = NaN
+            last_active = member.last_active;
             let last_active_formatted = '';
             let last_active_sort_value = '';
 
             // Handle 'Invited' or null/empty values differently from valid dates
-            if (last_active && last_active !== invited) {
+            if (last_active !== invited) {
               try {
                 // Try to parse the last_active as a valid date
-                last_active = new Date(last_active);
+                // Try to parse the last_active as a valid date
+                const parsedDate = new Date(last_active);
                 if (!isNaN(last_active)) {
-                  last_active_formatted = last_active.toLocaleDateString('en-US', options);
-                  last_active_sort_value = last_active.getTime();  // For sorting purposes
+                  last_active_formatted = parsedDate.toLocaleDateString('en-US', options);
+                  last_active_sort_value = parsedDate.getTime();  // For sorting purposes
                 } else {
-                  last_active_formatted='Invalid date'                  
+                  last_active_formatted='Invalid date';
+                  last_active_sort_value = 'Invalid date';           
                 }
               } catch (e) {
                 console.error(`Error parsing date: ${last_active}. Error: ${e}`);

From ddd12f0e2f9554df0d4fd90a9986e776b27c811f Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 17 Oct 2024 15:25:31 -0400
Subject: [PATCH 11/23] edit to uswds js in order to fix sort when no column
 data

---
 src/registrar/assets/js/uswds-edited.js | 28 ++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/src/registrar/assets/js/uswds-edited.js b/src/registrar/assets/js/uswds-edited.js
index e73f3b6c0..97683d309 100644
--- a/src/registrar/assets/js/uswds-edited.js
+++ b/src/registrar/assets/js/uswds-edited.js
@@ -5506,6 +5506,8 @@ const SORT_BUTTON = `.${SORT_BUTTON_CLASS}`;
 const SORTABLE_HEADER = `th[data-sortable]`;
 const ANNOUNCEMENT_REGION = `.${PREFIX}-table__announcement-region[aria-live="polite"]`;
 
+// ---- DOTGOV EDIT
+
 /** Gets the data-sort-value attribute value, if provided — otherwise, gets
  * the innerText or textContent — of the child element (HTMLTableCellElement)
  * at the specified index of the given table row
@@ -5514,7 +5516,19 @@ const ANNOUNCEMENT_REGION = `.${PREFIX}-table__announcement-region[aria-live="po
  * @param {array<HTMLTableRowElement>} tr
  * @return {boolean}
  */
-const getCellValue = (tr, index) => tr.children[index].getAttribute(SORT_OVERRIDE) || tr.children[index].innerText || tr.children[index].textContent;
+const getCellValue = (tr, index) => {
+  if (tr.children[index])
+    return tr.children[index].getAttribute(SORT_OVERRIDE) || tr.children[index].innerText || tr.children[index].textContent;
+  return "";
+}
+
+// const getCellValue = (tr, index) => tr.children[index].getAttribute(SORT_OVERRIDE) || tr.children[index].innerText || tr.children[index].textContent;
+// DOTGOV: added check for tr.children[index] to protect from absent cells
+
+// ---- END DOTGOV EDIT
+
+
+
 
 /**
  * Compares the values of two row array items at the given index, then sorts by the given direction
@@ -5526,7 +5540,6 @@ const compareFunction = (index, isAscending) => (thisRow, nextRow) => {
   // get values to compare from data attribute or cell content
   const value1 = getCellValue(isAscending ? thisRow : nextRow, index);
   const value2 = getCellValue(isAscending ? nextRow : thisRow, index);
-
   // if neither value is empty, and if both values are already numbers, compare numerically
   if (value1 && value2 && !Number.isNaN(Number(value1)) && !Number.isNaN(Number(value2))) {
     return value1 - value2;
@@ -5601,7 +5614,16 @@ const sortRows = (header, isAscending) => {
   const thisHeaderIndex = allHeaders.indexOf(header);
   allRows.sort(compareFunction(thisHeaderIndex, !isAscending)).forEach(tr => {
     [].slice.call(tr.children).forEach(td => td.removeAttribute("data-sort-active"));
-    tr.children[thisHeaderIndex].setAttribute("data-sort-active", true);
+
+    // ---- DOTGOV EDIT
+
+    // tr.children[thisHeaderIndex].setAttribute("data-sort-active", true);
+    if (tr.children[thisHeaderIndex])
+      tr.children[thisHeaderIndex].setAttribute("data-sort-active", true);
+    // DOTGOV added conditional to protect from tr.children[thisHeaderIndex] being absent
+
+    // ---- END DOTGOV EDIT
+
     tbody.appendChild(tr);
   });
   return true;

From 89ea577dc95a4101463e0023f7f424e4fe9c7d63 Mon Sep 17 00:00:00 2001
From: Rachid Mrad <rachid.mrad@ecstech.com>
Date: Thu, 17 Oct 2024 15:46:44 -0400
Subject: [PATCH 12/23] cleanup last active date

---
 src/registrar/assets/js/get-gov.js            | 45 ++++++++++---------
 src/registrar/views/portfolio_members_json.py |  3 --
 2 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js
index ab94bec7a..06927ed51 100644
--- a/src/registrar/assets/js/get-gov.js
+++ b/src/registrar/assets/js/get-gov.js
@@ -1975,33 +1975,34 @@ class MembersTable extends LoadTableBase {
             const options = { year: 'numeric', month: 'short', day: 'numeric' };
             
             // Handle last_active values
-            let last_active = NaN
-            last_active = member.last_active;
+            let last_active = member.last_active; // Changed to let to allow for potential modification
             let last_active_formatted = '';
-            let last_active_sort_value = '';
+            let last_active_sort_value = NaN;
 
-            // Handle 'Invited' or null/empty values differently from valid dates
-            if (last_active !== invited) {
-              try {
-                // Try to parse the last_active as a valid date
-                // Try to parse the last_active as a valid date
+            // Check if last_active is valid before proceeding
+            if (last_active) {
+              if (last_active === invited) {
+                last_active_formatted = invited;
+                last_active_sort_value = invited; // Keep 'Invited' as a sortable value
+              } else {
                 const parsedDate = new Date(last_active);
-                if (!isNaN(last_active)) {
-                  last_active_formatted = parsedDate.toLocaleDateString('en-US', options);
-                  last_active_sort_value = parsedDate.getTime();  // For sorting purposes
-                } else {
-                  last_active_formatted='Invalid date';
-                  last_active_sort_value = 'Invalid date';           
+                
+                try {
+                  if (!isNaN(parsedDate.getTime())) { // Check if the date is valid
+                    last_active_formatted = parsedDate.toLocaleDateString('en-US', options);
+                    last_active_sort_value = parsedDate.getTime();  // For sorting purposes
+                  } else {
+                    throw new Error('Invalid date'); // Throw an error to catch in catch block
+                  }
+                } catch (e) {
+                  console.error(`Error parsing date: ${last_active}. Error: ${e}`);
+                  last_active_formatted = 'Invalid date';
+                  last_active_sort_value = 'Invalid date';
                 }
-              } catch (e) {
-                console.error(`Error parsing date: ${last_active}. Error: ${e}`);
-                last_active_formatted='Invalid date'
               }
-            } else {
-              // Handle 'Invited' or null
-              last_active = invited;
-              last_active_formatted = invited;
-              last_active_sort_value = invited; // Keep 'Invited' as a sortable string
+            } else { // last_active is null or undefined
+              last_active_formatted = 'Invalid date';
+              last_active_sort_value = 'Invalid date'; // Default value for invalid or missing last_active
             }
 
             const action_url = member.action_url;
diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py
index e888feda5..dc5d2d57c 100644
--- a/src/registrar/views/portfolio_members_json.py
+++ b/src/registrar/views/portfolio_members_json.py
@@ -113,8 +113,6 @@ def initial_permissions_search(portfolio):
 
 def initial_invitations_search(portfolio):
     """Perform initial invitations search and get related DomainInvitation data based on the email."""
-
-
     # Get DomainInvitation query for matching email
     domain_invitations = DomainInvitation.objects.filter(
         email=OuterRef('email'),
@@ -127,7 +125,6 @@ def initial_invitations_search(portfolio):
             output_field=CharField()
         )
     )
-
     invitations = PortfolioInvitation.objects.filter(portfolio=portfolio)
     invitations = invitations.annotate(
         first_name=Value(None, output_field=CharField()),

From 9724477ab3b8ab4fcd5f4053142a69c778cc8bd6 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 17 Oct 2024 15:55:58 -0400
Subject: [PATCH 13/23] formatting for code readability

---
 .../models/utility/portfolio_helper.py        |  2 +-
 .../tests/test_views_members_json.py          | 28 +++++++----------
 src/registrar/views/portfolio_members_json.py | 30 ++++++++-----------
 3 files changed, 24 insertions(+), 36 deletions(-)

diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py
index a7367e6e3..d998d7ffa 100644
--- a/src/registrar/models/utility/portfolio_helper.py
+++ b/src/registrar/models/utility/portfolio_helper.py
@@ -36,7 +36,7 @@ class UserPortfolioPermissionChoices(models.TextChoices):
     @classmethod
     def get_user_portfolio_permission_label(cls, user_portfolio_permission):
         return cls(user_portfolio_permission).label if user_portfolio_permission else None
-    
+
     @classmethod
     def to_dict(cls):
         return {key: value.value for key, value in cls.__members__.items()}
diff --git a/src/registrar/tests/test_views_members_json.py b/src/registrar/tests/test_views_members_json.py
index 77a7a5566..8c0495e3d 100644
--- a/src/registrar/tests/test_views_members_json.py
+++ b/src/registrar/tests/test_views_members_json.py
@@ -137,12 +137,14 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest):
         # Convert each member's roles list to a frozenset
         actual_roles = {role for member in data["members"] for role in member["roles"]}
         self.assertEqual(expected_roles, actual_roles)
-        
+
         expected_additional_permissions = {
             UserPortfolioPermissionChoices.VIEW_MEMBERS,
             UserPortfolioPermissionChoices.EDIT_MEMBERS,
         }
-        actual_additional_permissions = {permission for member in data["members"]  for permission in member["permissions"]}
+        actual_additional_permissions = {
+            permission for member in data["members"] for permission in member["permissions"]
+        }
         self.assertTrue(expected_additional_permissions.issubset(actual_additional_permissions))
 
     def test_get_portfolio_invited_json_authenticated(self):
@@ -172,9 +174,7 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest):
         self.assertEqual(len(data["members"]), 1)
 
         # Check member fields
-        expected_emails = {
-            self.email6
-        }
+        expected_emails = {self.email6}
         actual_emails = {member["email"] for member in data["members"]}
         self.assertEqual(expected_emails, actual_emails)
 
@@ -184,12 +184,14 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest):
         # Convert each member's roles list to a frozenset
         actual_roles = {role for member in data["members"] for role in member["roles"]}
         self.assertEqual(expected_roles, actual_roles)
-        
+
         expected_additional_permissions = {
             UserPortfolioPermissionChoices.VIEW_MEMBERS,
             UserPortfolioPermissionChoices.EDIT_MEMBERS,
         }
-        actual_additional_permissions = {permission for member in data["members"]  for permission in member["permissions"]}
+        actual_additional_permissions = {
+            permission for member in data["members"] for permission in member["permissions"]
+        }
         self.assertTrue(expected_additional_permissions.issubset(actual_additional_permissions))
 
     def test_get_portfolio_members_json_with_domains(self):
@@ -241,11 +243,7 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest):
         data = response.json
 
         # Check if the domain appears in the response JSON
-        domain_names = [
-            domain_name
-            for member in data["members"]
-            for domain_name in member.get("domain_names", [])
-        ]
+        domain_names = [domain_name for member in data["members"] for domain_name in member.get("domain_names", [])]
         self.assertIn("somedomain1.com", domain_names)
 
     def test_get_portfolio_invited_json_with_domains(self):
@@ -281,11 +279,7 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest):
         data = response.json
 
         # Check if the domain appears in the response JSON
-        domain_names = [
-            domain_name
-            for member in data["members"]
-            for domain_name in member.get("domain_names", [])
-        ]
+        domain_names = [domain_name for member in data["members"] for domain_name in member.get("domain_names", [])]
         self.assertIn("somedomain1.com", domain_names)
 
     def test_pagination(self):
diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py
index dc5d2d57c..1c34cc64a 100644
--- a/src/registrar/views/portfolio_members_json.py
+++ b/src/registrar/views/portfolio_members_json.py
@@ -10,7 +10,6 @@ from django.db.models.functions import Cast
 
 from registrar.models.domain_invitation import DomainInvitation
 from registrar.models.portfolio_invitation import PortfolioInvitation
-from registrar.models.user_domain_role import UserDomainRole
 from registrar.models.user_portfolio_permission import UserPortfolioPermission
 from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices
 
@@ -88,10 +87,10 @@ def initial_permissions_search(portfolio):
                     Value(":"),
                     F("user__permissions__domain__name"),
                     # specify the output_field to ensure union has same column types
-                    output_field=CharField()
+                    output_field=CharField(),
                 ),
                 distinct=True,
-                filter=Q(user__permissions__domain__isnull=False)
+                filter=Q(user__permissions__domain__isnull=False),
             ),
             source=Value("permission", output_field=CharField()),
         )
@@ -114,16 +113,8 @@ def initial_permissions_search(portfolio):
 def initial_invitations_search(portfolio):
     """Perform initial invitations search and get related DomainInvitation data based on the email."""
     # Get DomainInvitation query for matching email
-    domain_invitations = DomainInvitation.objects.filter(
-        email=OuterRef('email'),
-        domain__isnull=False
-    ).annotate(
-        domain_info=Concat(
-            F('domain__id'),
-            Value(':'),
-            F('domain__name'),
-            output_field=CharField()
-        )
+    domain_invitations = DomainInvitation.objects.filter(email=OuterRef("email"), domain__isnull=False).annotate(
+        domain_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField())
     )
     invitations = PortfolioInvitation.objects.filter(portfolio=portfolio)
     invitations = invitations.annotate(
@@ -135,10 +126,10 @@ def initial_invitations_search(portfolio):
         # ArrayAgg for multiple domain_invitations matched by email, filtered to exclude nulls
         domain_info=Coalesce(  # Use Coalesce to return an empty list if no domain invitations exist
             ArrayAgg(
-                Subquery(domain_invitations.values('domain_info')),
+                Subquery(domain_invitations.values("domain_info")),
                 distinct=True,
             ),
-            Value([], output_field=ArrayField(CharField()))  # Ensure we return an empty list
+            Value([], output_field=ArrayField(CharField())),  # Ensure we return an empty list
         ),
         source=Value("invitation", output_field=CharField()),
     ).values(
@@ -201,13 +192,16 @@ def serialize_members(request, portfolio, item, user):
         "email": item.get("email_display", ""),
         "member_display": item.get("member_display", ""),
         "roles": (item.get("roles") or []),
-        "permissions": UserPortfolioPermission.get_portfolio_permissions(item.get("roles"), item.get("additional_permissions")),
+        "permissions": UserPortfolioPermission.get_portfolio_permissions(
+            item.get("roles"), item.get("additional_permissions")
+        ),
         "domain_names": [
-            domain_info.split(":")[1] for domain_info in (item.get("domain_info") or [])
+            domain_info.split(":")[1]
+            for domain_info in (item.get("domain_info") or [])
             if domain_info is not None  # Prevent splitting None
         ],
         "domain_urls": [
-            reverse("domain", kwargs={"pk": domain_info.split(":")[0]}) 
+            reverse("domain", kwargs={"pk": domain_info.split(":")[0]})
             for domain_info in (item.get("domain_info") or [])
             if domain_info is not None  # Prevent splitting None
         ],

From f2abf0fdfcacc80ed9fbc264b873720b9b120d1a Mon Sep 17 00:00:00 2001
From: Rachid Mrad <rachid.mrad@ecstech.com>
Date: Thu, 17 Oct 2024 16:57:02 -0400
Subject: [PATCH 14/23] tweak header mapping on hidden rows

---
 src/registrar/assets/js/get-gov.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js
index 06927ed51..aa64f5121 100644
--- a/src/registrar/assets/js/get-gov.js
+++ b/src/registrar/assets/js/get-gov.js
@@ -2072,7 +2072,7 @@ class MembersTable extends LoadTableBase {
                 </button>
               `;
 
-              showMoreRow.innerHTML = `<td colspan='3' headers="header-member" class="padding-top-0"><div class='grid-row'>${domainsHTML} ${permissionsHTML}</div></td>`;
+              showMoreRow.innerHTML = `<td colspan='3' headers="header-member row-header-${member_id}" class="padding-top-0"><div class='grid-row'>${domainsHTML} ${permissionsHTML}</div></td>`;
               showMoreRow.classList.add('show-more-content');
               showMoreRow.classList.add('display-none');
               showMoreRow.id = member_id;

From 4701f1cc3a439a7905c50b64a87146140acc67f2 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 17 Oct 2024 18:42:37 -0400
Subject: [PATCH 15/23] filter out [None] to return [] instead

---
 src/registrar/views/portfolio_members_json.py | 34 +++++++++----------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py
index 1c34cc64a..918243628 100644
--- a/src/registrar/views/portfolio_members_json.py
+++ b/src/registrar/views/portfolio_members_json.py
@@ -2,11 +2,10 @@ 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, OuterRef, Subquery
-from django.db.models.functions import Concat, Coalesce
-from django.contrib.postgres.fields import ArrayField
+from django.db.models.expressions import Func
+from django.db.models.functions import Cast, Coalesce, Concat
 from django.contrib.postgres.aggregates import ArrayAgg
 from django.urls import reverse
-from django.db.models.functions import Cast
 
 from registrar.models.domain_invitation import DomainInvitation
 from registrar.models.portfolio_invitation import PortfolioInvitation
@@ -90,7 +89,7 @@ def initial_permissions_search(portfolio):
                     output_field=CharField(),
                 ),
                 distinct=True,
-                filter=Q(user__permissions__domain__isnull=False),
+                filter=Q(user__permissions__domain__isnull=False),  # filter out null values
             ),
             source=Value("permission", output_field=CharField()),
         )
@@ -110,12 +109,19 @@ def initial_permissions_search(portfolio):
     return permissions
 
 
+# Custom Func to use array_remove to remove null values
+class ArrayRemove(Func):
+    function = "array_remove"
+    template = "%(function)s(%(expressions)s, NULL)"
+
+
 def initial_invitations_search(portfolio):
     """Perform initial invitations search and get related DomainInvitation data based on the email."""
     # Get DomainInvitation query for matching email
-    domain_invitations = DomainInvitation.objects.filter(email=OuterRef("email"), domain__isnull=False).annotate(
+    domain_invitations = DomainInvitation.objects.filter(email=OuterRef("email")).annotate(
         domain_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField())
     )
+    # PortfolioInvitation query
     invitations = PortfolioInvitation.objects.filter(portfolio=portfolio)
     invitations = invitations.annotate(
         first_name=Value(None, output_field=CharField()),
@@ -123,13 +129,12 @@ def initial_invitations_search(portfolio):
         email_display=F("email"),
         last_active=Value("Invited", output_field=TextField()),
         member_display=F("email"),
-        # ArrayAgg for multiple domain_invitations matched by email, filtered to exclude nulls
-        domain_info=Coalesce(  # Use Coalesce to return an empty list if no domain invitations exist
+        # Use ArrayRemove to return an empty list when no domain invitations are found
+        domain_info=ArrayRemove(
             ArrayAgg(
                 Subquery(domain_invitations.values("domain_info")),
                 distinct=True,
-            ),
-            Value([], output_field=ArrayField(CharField())),  # Ensure we return an empty list
+            )
         ),
         source=Value("invitation", output_field=CharField()),
     ).values(
@@ -195,16 +200,11 @@ def serialize_members(request, portfolio, item, user):
         "permissions": UserPortfolioPermission.get_portfolio_permissions(
             item.get("roles"), item.get("additional_permissions")
         ),
-        "domain_names": [
-            domain_info.split(":")[1]
-            for domain_info in (item.get("domain_info") or [])
-            if domain_info is not None  # Prevent splitting None
-        ],
+        # split domain_info array values into ids to form urls, and names
         "domain_urls": [
-            reverse("domain", kwargs={"pk": domain_info.split(":")[0]})
-            for domain_info in (item.get("domain_info") or [])
-            if domain_info is not None  # Prevent splitting None
+            reverse("domain", kwargs={"pk": domain_info.split(":")[0]}) for domain_info in item.get("domain_info")
         ],
+        "domain_names": [domain_info.split(":")[1] for domain_info in item.get("domain_info")],
         "is_admin": is_admin,
         "last_active": item.get("last_active", ""),
         "action_url": action_url,

From 2a64ac65c05dc42395b87cdc8b666beb9eeae459 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 17 Oct 2024 19:04:38 -0400
Subject: [PATCH 16/23] fixed null values in response for last_active

---
 src/registrar/views/portfolio_members_json.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py
index 918243628..edb3c3b36 100644
--- a/src/registrar/views/portfolio_members_json.py
+++ b/src/registrar/views/portfolio_members_json.py
@@ -62,7 +62,10 @@ def initial_permissions_search(portfolio):
             first_name=F("user__first_name"),
             last_name=F("user__last_name"),
             email_display=F("user__email"),
-            last_active=Cast(F("user__last_login"), output_field=TextField()),  # Cast last_login to text
+            last_active=Coalesce(
+                Cast(F("user__last_login"), output_field=TextField()),  # Cast last_login to text
+                Value("Invalid date"), output_field=TextField()
+            ),
             member_display=Case(
                 # If email is present and not blank, use email
                 When(Q(user__email__isnull=False) & ~Q(user__email=""), then=F("user__email")),
@@ -206,7 +209,7 @@ def serialize_members(request, portfolio, item, user):
         ],
         "domain_names": [domain_info.split(":")[1] for domain_info in item.get("domain_info")],
         "is_admin": is_admin,
-        "last_active": item.get("last_active", ""),
+        "last_active": item.get("last_active"),
         "action_url": action_url,
         "action_label": ("View" if view_only else "Manage"),
         "svg_icon": ("visibility" if view_only else "settings"),

From e0269edc71f54fa8f7c287c678379a6358b355cf Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 17 Oct 2024 19:15:24 -0400
Subject: [PATCH 17/23] updated js for last_active

---
 src/registrar/assets/js/get-gov.js | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js
index aa64f5121..d62c24154 100644
--- a/src/registrar/assets/js/get-gov.js
+++ b/src/registrar/assets/js/get-gov.js
@@ -1963,6 +1963,7 @@ class MembersTable extends LoadTableBase {
 
           const UserPortfolioPermissionChoices = data.UserPortfolioPermissionChoices;
           const invited = 'Invited';
+          const invalid_date = 'Invalid date';
 
           data.members.forEach(member => {
             const member_id = member.source + member.id;
@@ -1981,9 +1982,9 @@ class MembersTable extends LoadTableBase {
 
             // Check if last_active is valid before proceeding
             if (last_active) {
-              if (last_active === invited) {
-                last_active_formatted = invited;
-                last_active_sort_value = invited; // Keep 'Invited' as a sortable value
+              if (last_active === invited || last_active === invalid_date) {
+                last_active_formatted = last_active;
+                last_active_sort_value = last_active;
               } else {
                 const parsedDate = new Date(last_active);
                 
@@ -1992,17 +1993,17 @@ class MembersTable extends LoadTableBase {
                     last_active_formatted = parsedDate.toLocaleDateString('en-US', options);
                     last_active_sort_value = parsedDate.getTime();  // For sorting purposes
                   } else {
-                    throw new Error('Invalid date'); // Throw an error to catch in catch block
+                    throw new Error(invalid_date); // Throw an error to catch in catch block
                   }
                 } catch (e) {
                   console.error(`Error parsing date: ${last_active}. Error: ${e}`);
-                  last_active_formatted = 'Invalid date';
-                  last_active_sort_value = 'Invalid date';
+                  last_active_formatted = invalid_date;
+                  last_active_sort_value = invalid_date;
                 }
               }
             } else { // last_active is null or undefined
-              last_active_formatted = 'Invalid date';
-              last_active_sort_value = 'Invalid date'; // Default value for invalid or missing last_active
+              last_active_formatted = invalid_date;
+              last_active_sort_value = invalid_date; // Default value for invalid or missing last_active
             }
 
             const action_url = member.action_url;

From 661f09d2e4fd89573a01a72e1f25273e601d0b15 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 17 Oct 2024 19:20:12 -0400
Subject: [PATCH 18/23] lint

---
 src/registrar/views/portfolio_members_json.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py
index edb3c3b36..11c650cef 100644
--- a/src/registrar/views/portfolio_members_json.py
+++ b/src/registrar/views/portfolio_members_json.py
@@ -64,7 +64,8 @@ def initial_permissions_search(portfolio):
             email_display=F("user__email"),
             last_active=Coalesce(
                 Cast(F("user__last_login"), output_field=TextField()),  # Cast last_login to text
-                Value("Invalid date"), output_field=TextField()
+                Value("Invalid date"),
+                output_field=TextField(),
             ),
             member_display=Case(
                 # If email is present and not blank, use email

From 3b74793d94151d789dc1da7468de41de03d7d09f Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Fri, 18 Oct 2024 12:47:50 -0400
Subject: [PATCH 19/23] refactored js handling of last_active date

---
 src/registrar/assets/js/get-gov.js | 81 ++++++++++++++++++------------
 1 file changed, 48 insertions(+), 33 deletions(-)

diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js
index d62c24154..5899328ec 100644
--- a/src/registrar/assets/js/get-gov.js
+++ b/src/registrar/assets/js/get-gov.js
@@ -1908,6 +1908,51 @@ class MembersTable extends LoadTableBase {
     });
   }
 
+  /**
+   * Helper function which takes last_active and returns display value and sort value
+   * @param {*} last_active 
+   * @returns 
+   */
+  handleLastActive(last_active) {
+    let last_active_display = '';
+    let last_active_sort_value = -1;
+  
+    const invited = 'Invited';           // Assuming "invited" is a constant string
+    const invalid_date = 'Invalid date'; // Assuming "invalid_date" is a constant string
+    const options = { year: 'numeric', month: 'long', day: 'numeric' }; // Format options for date
+  
+    // Check if last_active is valid before proceeding
+    if (last_active) {
+      if (last_active === invited) {
+        last_active_display = invited;
+        last_active_sort_value = 0;
+      } else if (last_active === invalid_date) {
+        last_active_display = invalid_date;
+      } else {
+        const parsedDate = new Date(last_active);
+        
+        try {
+          if (!isNaN(parsedDate.getTime())) { // Check if the date is valid
+            last_active_display = parsedDate.toLocaleDateString('en-US', options);
+            last_active_sort_value = parsedDate.getTime(); // For sorting purposes
+          } else {
+            throw new Error(invalid_date); // Throw an error to catch in catch block
+          }
+        } catch (e) {
+          console.error(`Error parsing date: ${last_active}. Error: ${e}`);
+          last_active_display = invalid_date;
+        }
+      }
+    } else { // last_active is null or undefined
+      last_active_display = invalid_date;
+    }
+  
+    return {
+      display_value: last_active_display,
+      sort_value: last_active_sort_value
+    };
+  }
+  
   /**
      * Loads rows in the members list, as well as updates pagination around the members list
      * based on the supplied attributes.
@@ -1973,38 +2018,8 @@ class MembersTable extends LoadTableBase {
             const domain_urls = member.domain_urls;
             const domain_names = member.domain_names;
             const num_domains = domain_urls.length;
-            const options = { year: 'numeric', month: 'short', day: 'numeric' };
             
-            // Handle last_active values
-            let last_active = member.last_active; // Changed to let to allow for potential modification
-            let last_active_formatted = '';
-            let last_active_sort_value = NaN;
-
-            // Check if last_active is valid before proceeding
-            if (last_active) {
-              if (last_active === invited || last_active === invalid_date) {
-                last_active_formatted = last_active;
-                last_active_sort_value = last_active;
-              } else {
-                const parsedDate = new Date(last_active);
-                
-                try {
-                  if (!isNaN(parsedDate.getTime())) { // Check if the date is valid
-                    last_active_formatted = parsedDate.toLocaleDateString('en-US', options);
-                    last_active_sort_value = parsedDate.getTime();  // For sorting purposes
-                  } else {
-                    throw new Error(invalid_date); // Throw an error to catch in catch block
-                  }
-                } catch (e) {
-                  console.error(`Error parsing date: ${last_active}. Error: ${e}`);
-                  last_active_formatted = invalid_date;
-                  last_active_sort_value = invalid_date;
-                }
-              }
-            } else { // last_active is null or undefined
-              last_active_formatted = invalid_date;
-              last_active_sort_value = invalid_date; // Default value for invalid or missing last_active
-            }
+            const last_active = this.handleLastActive(member.last_active);
 
             const action_url = member.action_url;
             const action_label = member.action_label;
@@ -2083,8 +2098,8 @@ class MembersTable extends LoadTableBase {
               <th role="rowheader" headers="header-member" data-label="member email" id='row-header-${member_id}'>
                 ${member_display} ${admin_tagHTML} ${showMoreButton}
               </th>
-              <td headers="header-last-active row-header-${member_id}" data-sort-value="${last_active_sort_value}" data-label="last_active">
-                ${last_active_formatted}
+              <td headers="header-last-active row-header-${member_id}" data-sort-value="${last_active.sort_value}" data-label="last_active">
+                ${last_active.display_value}
               </td>
               <td headers="header-action row-header-${member_id}">
                 <a href="${action_url}">

From 96c77cd476586655b57cbdc833ae1faa3330a75c Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Fri, 18 Oct 2024 13:09:18 -0400
Subject: [PATCH 20/23] commented js

---
 src/registrar/assets/js/get-gov.js | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js
index 5899328ec..f70c2f240 100644
--- a/src/registrar/assets/js/get-gov.js
+++ b/src/registrar/assets/js/get-gov.js
@@ -1910,22 +1910,22 @@ class MembersTable extends LoadTableBase {
 
   /**
    * Helper function which takes last_active and returns display value and sort value
-   * @param {*} last_active 
+   * @param {*} last_active - UTC date, or strings "Invited" or "Invalid date"
    * @returns 
    */
   handleLastActive(last_active) {
     let last_active_display = '';
-    let last_active_sort_value = -1;
+    let last_active_sort_value = -1; // sort by default as numeric value to sort with dates
   
-    const invited = 'Invited';           // Assuming "invited" is a constant string
-    const invalid_date = 'Invalid date'; // Assuming "invalid_date" is a constant string
+    const invited = 'Invited';
+    const invalid_date = 'Invalid date';
     const options = { year: 'numeric', month: 'long', day: 'numeric' }; // Format options for date
   
-    // Check if last_active is valid before proceeding
+    // Check if last_active is not null
     if (last_active) {
       if (last_active === invited) {
         last_active_display = invited;
-        last_active_sort_value = 0;
+        last_active_sort_value = 0; // sort as numeric value 
       } else if (last_active === invalid_date) {
         last_active_display = invalid_date;
       } else {
@@ -1934,11 +1934,11 @@ class MembersTable extends LoadTableBase {
         try {
           if (!isNaN(parsedDate.getTime())) { // Check if the date is valid
             last_active_display = parsedDate.toLocaleDateString('en-US', options);
-            last_active_sort_value = parsedDate.getTime(); // For sorting purposes
+            last_active_sort_value = parsedDate.getTime(); // sort as numeric value, seconds since 1970
           } else {
             throw new Error(invalid_date); // Throw an error to catch in catch block
           }
-        } catch (e) {
+        } catch (e) { // catch invalid values and treat as 'Invalid date'
           console.error(`Error parsing date: ${last_active}. Error: ${e}`);
           last_active_display = invalid_date;
         }

From b06ffec6b68136b401bbfaf05a3183c4dd21bb16 Mon Sep 17 00:00:00 2001
From: Rachid Mrad <rachid.mrad@ecstech.com>
Date: Fri, 18 Oct 2024 13:49:09 -0400
Subject: [PATCH 21/23] aria-label on expand / close button

---
 src/registrar/assets/js/get-gov.js | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js
index f70c2f240..8bad1c72b 100644
--- a/src/registrar/assets/js/get-gov.js
+++ b/src/registrar/assets/js/get-gov.js
@@ -1883,11 +1883,13 @@ class MembersTable extends LoadTableBase {
         spanElement.textContent = 'Close';
         useElement.setAttribute('xlink:href', '/public/img/sprite.svg#expand_less');
         buttonParentRow.classList.add('hide-td-borders');
+        toggleButton.setAttribute('aria-label', 'Close additional information');
       } else {    
         hideElement(contentDiv);
         spanElement.textContent = 'Expand';
         useElement.setAttribute('xlink:href', '/public/img/sprite.svg#expand_more');
         buttonParentRow.classList.remove('hide-td-borders');
+        toggleButton.setAttribute('aria-label', 'Expand for additional information');
       }
     }
   
@@ -2079,7 +2081,8 @@ class MembersTable extends LoadTableBase {
                 <button 
                   type="button" 
                   class="usa-button--show-more-button usa-button usa-button--unstyled display-block margin-top-1" 
-                  data-for=${member_id} 
+                  data-for=${member_id}
+                  aria-label="Expand for additional information"
                 >
                   <span>Expand</span>
                   <svg class="usa-icon usa-icon--big" aria-hidden="true" focusable="false" role="img" width="24">

From 07e4960cc60c3f4f69100e3ac5afde653ffcef06 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 23 Oct 2024 06:27:41 -0400
Subject: [PATCH 22/23] code refactors for readability and efficiency, comments
 for clarity

---
 src/registrar/assets/js/get-gov.js            | 225 ++++++++++++------
 .../tests/test_views_members_json.py          |   8 +-
 2 files changed, 153 insertions(+), 80 deletions(-)

diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js
index 8bad1c72b..5fd879954 100644
--- a/src/registrar/assets/js/get-gov.js
+++ b/src/registrar/assets/js/get-gov.js
@@ -1874,7 +1874,24 @@ class MembersTable extends LoadTableBase {
     super('.members__table', '.members__table-wrapper', '#members__search-field', '#members__search-field-submit', '.members__reset-search', '.members__reset-filters', '.members__no-data', '.members__no-search-results');
   }
   
+  /**
+   * Initializes "Show More" buttons on the page, enabling toggle functionality to show or hide content.
+   * 
+   * The function finds elements with "Show More" buttons and sets up a click event listener to toggle the visibility
+   * of a corresponding content div. When clicked, the button updates its visual state (e.g., text/icon change),
+   * and the associated content is shown or hidden based on its current visibility status.
+   *
+   * @function initShowMoreButtons
+   */
   initShowMoreButtons() {
+    /**
+     * Toggles the visibility of a content section when the "Show More" button is clicked.
+     * Updates the button text/icon based on whether the content is shown or hidden.
+     *
+     * @param {HTMLElement} toggleButton - The button that toggles the content visibility.
+     * @param {HTMLElement} contentDiv - The content div whose visibility is toggled.
+     * @param {HTMLElement} buttonParentRow - The parent row element containing the button.
+     */
     function toggleShowMoreButton(toggleButton, contentDiv, buttonParentRow) {
       const spanElement = toggleButton.querySelector('span');
       const useElement = toggleButton.querySelector('use');
@@ -1896,6 +1913,7 @@ class MembersTable extends LoadTableBase {
     let toggleButtons = document.querySelectorAll('.usa-button--show-more-button');
     toggleButtons.forEach((toggleButton) => {
       
+      // get contentDiv for element specified in data-for attribute of toggleButton
       let dataFor = toggleButton.dataset.for;
       let contentDiv = document.getElementById(dataFor);
       let buttonParentRow = toggleButton.parentElement.parentElement;
@@ -1911,50 +1929,141 @@ class MembersTable extends LoadTableBase {
   }
 
   /**
-   * Helper function which takes last_active and returns display value and sort value
-   * @param {*} last_active - UTC date, or strings "Invited" or "Invalid date"
-   * @returns 
+   * Converts a given `last_active` value into a display value and a numeric sort value.
+   * The input can be a UTC date, the strings "Invited", "Invalid date", or null/undefined.
+   * 
+   * @param {string} last_active - UTC date string or special status like "Invited" or "Invalid date".
+   * @returns {Object} - An object containing `display_value` (formatted date or status string) 
+   *                     and `sort_value` (numeric value for sorting).
    */
   handleLastActive(last_active) {
-    let last_active_display = '';
-    let last_active_sort_value = -1; // sort by default as numeric value to sort with dates
-  
     const invited = 'Invited';
     const invalid_date = 'Invalid date';
-    const options = { year: 'numeric', month: 'long', day: 'numeric' }; // Format options for date
-  
-    // Check if last_active is not null
-    if (last_active) {
-      if (last_active === invited) {
-        last_active_display = invited;
-        last_active_sort_value = 0; // sort as numeric value 
-      } else if (last_active === invalid_date) {
-        last_active_display = invalid_date;
+    const options = { year: 'numeric', month: 'long', day: 'numeric' }; // Date display format
+
+    let display_value = invalid_date; // Default display value for invalid or null dates
+    let sort_value = -1;              // Default sort value for invalid or null dates
+
+    if (last_active === invited) {
+      // Handle "Invited" status: special case with 0 sort value
+      display_value = invited;
+      sort_value = 0;
+    } else if (last_active && last_active !== invalid_date) {
+      // Parse and format valid UTC date strings
+      const parsedDate = new Date(last_active);
+
+      if (!isNaN(parsedDate.getTime())) {
+        // Valid date
+        display_value = parsedDate.toLocaleDateString('en-US', options);
+        sort_value = parsedDate.getTime(); // Use timestamp for sorting
       } else {
-        const parsedDate = new Date(last_active);
-        
-        try {
-          if (!isNaN(parsedDate.getTime())) { // Check if the date is valid
-            last_active_display = parsedDate.toLocaleDateString('en-US', options);
-            last_active_sort_value = parsedDate.getTime(); // sort as numeric value, seconds since 1970
-          } else {
-            throw new Error(invalid_date); // Throw an error to catch in catch block
-          }
-        } catch (e) { // catch invalid values and treat as 'Invalid date'
-          console.error(`Error parsing date: ${last_active}. Error: ${e}`);
-          last_active_display = invalid_date;
-        }
+        console.error(`Error: Invalid date string provided: ${last_active}`);
       }
-    } else { // last_active is null or undefined
-      last_active_display = invalid_date;
     }
-  
-    return {
-      display_value: last_active_display,
-      sort_value: last_active_sort_value
-    };
+
+    return { display_value, sort_value };
   }
-  
+
+  /**
+   * Generates HTML for the list of domains assigned to a member.
+   * 
+   * @param {number} num_domains - The number of domains the member is assigned to.
+   * @param {Array} domain_names - An array of domain names.
+   * @param {Array} domain_urls - An array of corresponding domain URLs.
+   * @returns {string} - A string of HTML displaying the domains assigned to the member.
+   */
+  generateDomainsHTML(num_domains, domain_names, domain_urls) {
+    // Initialize an empty string for the HTML
+    let domainsHTML = '';
+
+    // Only generate HTML if the member has one or more assigned domains
+    if (num_domains > 0) {
+      domainsHTML += "<div class='desktop:grid-col-5 margin-bottom-2 desktop:margin-bottom-0'>";
+      domainsHTML += "<h4 class='margin-y-0 text-primary'>Domains assigned</h4>";
+      domainsHTML += `<p class='margin-y-0'>This member is assigned to ${num_domains} domains:</p>`;
+      domainsHTML += "<ul class='usa-list usa-list--unstyled margin-y-0'>";
+
+      // Display up to 6 domains with their URLs
+      for (let i = 0; i < num_domains && i < 6; i++) {
+        domainsHTML += `<li><a href="${domain_urls[i]}">${domain_names[i]}</a></li>`;
+      }
+
+      domainsHTML += "</ul>";
+
+      // If there are more than 6 domains, display a "View assigned domains" link
+      if (num_domains >= 6) {
+        domainsHTML += "<p><a href='#'>View assigned domains</a></p>";
+      }
+
+      domainsHTML += "</div>";
+    }
+
+    return domainsHTML;
+  }
+
+  /**
+   * Generates an HTML string summarizing a user's additional permissions within a portfolio, 
+   * based on the user's permissions and predefined permission choices.
+   *
+   * @param {Array} member_permissions - An array of permission strings that the member has.
+   * @param {Object} UserPortfolioPermissionChoices - An object containing predefined permission choice constants.
+   *        Expected keys include:
+   *        - VIEW_ALL_DOMAINS
+   *        - VIEW_MANAGED_DOMAINS
+   *        - EDIT_REQUESTS
+   *        - VIEW_ALL_REQUESTS
+   *        - EDIT_MEMBERS
+   *        - VIEW_MEMBERS
+   * 
+   * @returns {string} - A string of HTML representing the user's additional permissions.
+   *                     If the user has no specific permissions, it returns a default message
+   *                     indicating no additional permissions.
+   *
+   * Behavior:
+   * - The function checks the user's permissions (`member_permissions`) and generates
+   *   corresponding HTML sections based on the permission choices defined in `UserPortfolioPermissionChoices`.
+   * - Permissions are categorized into domains, requests, and members:
+   *   - Domains: Determines whether the user can view or manage all or assigned domains.
+   *   - Requests: Differentiates between users who can edit requests, view all requests, or have no request privileges.
+   *   - Members: Distinguishes between members who can manage or only view other members.
+   * - If no relevant permissions are found, the function returns a message stating that the user has no additional permissions.
+   * - The resulting HTML always includes a header "Additional permissions for this member" and appends the relevant permission descriptions.
+   */
+  generatePermissionsHTML(member_permissions, UserPortfolioPermissionChoices) {
+    let permissionsHTML = '';
+
+    // Check domain-related permissions
+    if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS)) {
+      permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Domains:</strong> Can view all organization domains. Can manage domains they are assigned to and edit information about the domain (including DNS settings).</p>";
+    } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS)) {
+      permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Domains:</strong> Can manage domains they are assigned to and edit information about the domain (including DNS settings).</p>";
+    }
+
+    // Check request-related permissions
+    if (member_permissions.includes(UserPortfolioPermissionChoices.EDIT_REQUESTS)) {
+      permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Domain requests:</strong> Can view all organization domain requests. Can create domain requests and modify their own requests.</p>";
+    } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS)) {
+      permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Domain requests (view-only):</strong> Can view all organization domain requests. Can't create or modify any domain requests.</p>";
+    }
+
+    // Check member-related permissions
+    if (member_permissions.includes(UserPortfolioPermissionChoices.EDIT_MEMBERS)) {
+      permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Members:</strong> Can manage members including inviting new members, removing current members, and assigning domains to members.</p>";
+    } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_MEMBERS)) {
+      permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Members (view-only):</strong> Can view all organizational members. Can't manage any members.</p>";
+    }
+
+    // If no specific permissions are assigned, display a message indicating no additional permissions
+    if (!permissionsHTML) {
+      permissionsHTML += "<p class='margin-top-1 p--blockquote'><b>No additional permissions:</b> There are no additional permissions for this member.</p>";
+    }
+
+    // Add a permissions header and wrap the entire output in a container
+    permissionsHTML = "<div class='desktop:grid-col-7'><h4 class='margin-y-0 text-primary'>Additional permissions for this member</h4>" + permissionsHTML + "</div>";
+    
+    return permissionsHTML;
+  }
+
   /**
      * Loads rows in the members list, as well as updates pagination around the members list
      * based on the supplied attributes.
@@ -2033,47 +2142,11 @@ class MembersTable extends LoadTableBase {
             if (member.is_admin)
               admin_tagHTML = `<span class="usa-tag margin-left-1 bg-primary">Admin</span>`
 
+            // generate html blocks for domains and permissions for the member
+            let domainsHTML = this.generateDomainsHTML(num_domains, domain_names, domain_urls);
+            let permissionsHTML = this.generatePermissionsHTML(member_permissions, UserPortfolioPermissionChoices);
+            
             // domainsHTML block and permissionsHTML block need to be wrapped with hide/show toggle, Expand
-
-            let domainsHTML = '';
-            if (num_domains > 0) {
-              domainsHTML += "<div class='desktop:grid-col-5 margin-bottom-2 desktop:margin-bottom-0'>";
-              domainsHTML += "<h4 class='margin-y-0 text-primary'>Domains assigned</h4>";
-              domainsHTML += "<p class='margin-y-0'>This member is assigned to " + num_domains + " domains:";
-              domainsHTML += "<ul class='usa-list usa-list--unstyled margin-y-0'>";
-              for (let i = 0; i < num_domains && i < 6; i++) {
-                  domainsHTML += `<li><a href="${domain_urls[i]}">${domain_names[i]}</a></li>`;
-              }
-              domainsHTML += "</ul>";
-              if (num_domains >= 6) {
-                domainsHTML += "<p><a href='#'>View assigned domains</a></p>";
-              }
-              domainsHTML += "</div>";
-            }
-
-            let permissionsHTML = '';
-            if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS)) {
-              permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Domains:</strong> Can view all organization domains. Can manage domains they are assigned to and edit information about the domain (including DNS settings).</p>";
-            } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS)) {
-              permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Domains:</strong> Can manage domains they are assigned to and edit information about the domain (including DNS settings).</p>";
-            }
-            if (member_permissions.includes(UserPortfolioPermissionChoices.EDIT_REQUESTS)) {
-              permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Domain requests:</strong> Can view all organization domain requests. Can create domain requests and modify their own requests.</p>";
-            } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS)) {
-              permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Domain requests (view-only):</strong> Can view all organization domain requests. Can't create or modify any domain requests.</p>";
-            }
-            if (member_permissions.includes(UserPortfolioPermissionChoices.EDIT_MEMBERS)) {
-              permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Members:</strong> Can manage members including inviting new members, removing current members, and assigning domains to members.";
-            } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_MEMBERS)) {
-              permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Members (view-only):</strong> Can view all organizational members. Can't manage any members.";
-            }
-            // if there are no additional permissions, display a no additional permissions message
-            if (!permissionsHTML) {
-              permissionsHTML += "<p class='margin-top-1 p--blockquote'><b>No additional permissions:</b> There are no additional permissions for this member.</p>";
-            }
-            // add permissions header in all cases
-            permissionsHTML = "<div class='desktop:grid-col-7'><h4 class='margin-y-0 text-primary'>Additional permissions for this member</h4>" + permissionsHTML + "</div>";
-
             let showMoreButton = '';
             const showMoreRow = document.createElement('tr');
             if (domainsHTML || permissionsHTML) {
diff --git a/src/registrar/tests/test_views_members_json.py b/src/registrar/tests/test_views_members_json.py
index 8c0495e3d..b094ba2a4 100644
--- a/src/registrar/tests/test_views_members_json.py
+++ b/src/registrar/tests/test_views_members_json.py
@@ -138,14 +138,14 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest):
         actual_roles = {role for member in data["members"] for role in member["roles"]}
         self.assertEqual(expected_roles, actual_roles)
 
+        # Assert that the expected additional permissions are in the actual entire permissions list
         expected_additional_permissions = {
             UserPortfolioPermissionChoices.VIEW_MEMBERS,
             UserPortfolioPermissionChoices.EDIT_MEMBERS,
         }
-        actual_additional_permissions = {
-            permission for member in data["members"] for permission in member["permissions"]
-        }
-        self.assertTrue(expected_additional_permissions.issubset(actual_additional_permissions))
+        # actual_permissions includes additional permissions as well as permissions from roles
+        actual_permissions = {permission for member in data["members"] for permission in member["permissions"]}
+        self.assertTrue(expected_additional_permissions.issubset(actual_permissions))
 
     def test_get_portfolio_invited_json_authenticated(self):
         """Test that portfolio invitees are returned properly for an authenticated user."""

From 740eba74016cbc7bf1d04f24e5a33e3beda493ab Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 23 Oct 2024 12:48:56 -0400
Subject: [PATCH 23/23] fixed a bug where non-portfolio domains were appearing
 in members json response

---
 .../tests/test_views_members_json.py          | 39 ++++++++++++++++---
 src/registrar/views/portfolio_members_json.py | 12 +++---
 2 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/src/registrar/tests/test_views_members_json.py b/src/registrar/tests/test_views_members_json.py
index b094ba2a4..a97a517e2 100644
--- a/src/registrar/tests/test_views_members_json.py
+++ b/src/registrar/tests/test_views_members_json.py
@@ -222,29 +222,43 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest):
             roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
         )
 
+        # create domain for which user is manager and domain in portfolio
         domain = Domain.objects.create(
             name="somedomain1.com",
         )
-
         DomainInformation.objects.create(
             creator=self.user,
             domain=domain,
             portfolio=self.portfolio,
         )
-
         UserDomainRole.objects.create(
             user=self.user,
             domain=domain,
             role=UserDomainRole.Roles.MANAGER,
         )
 
+        # create domain for which user is manager and domain not in portfolio
+        domain2 = Domain.objects.create(
+            name="somedomain2.com",
+        )
+        DomainInformation.objects.create(
+            creator=self.user,
+            domain=domain2,
+        )
+        UserDomainRole.objects.create(
+            user=self.user,
+            domain=domain2,
+            role=UserDomainRole.Roles.MANAGER,
+        )
+
         response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id})
         self.assertEqual(response.status_code, 200)
         data = response.json
 
-        # Check if the domain appears in the response JSON
+        # Check if the domain appears in the response JSON and that domain2 does not
         domain_names = [domain_name for member in data["members"] for domain_name in member.get("domain_names", [])]
         self.assertIn("somedomain1.com", domain_names)
+        self.assertNotIn("somedomain2.com", domain_names)
 
     def test_get_portfolio_invited_json_with_domains(self):
         """Test that portfolio invited members are returned properly for an authenticated user and the response includes
@@ -259,28 +273,41 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest):
             ],
         )
 
+        # create a domain in the portfolio
         domain = Domain.objects.create(
             name="somedomain1.com",
         )
-
         DomainInformation.objects.create(
             creator=self.user,
             domain=domain,
             portfolio=self.portfolio,
         )
-
         DomainInvitation.objects.create(
             email=self.email6,
             domain=domain,
         )
 
+        # create a domain not in the portfolio
+        domain2 = Domain.objects.create(
+            name="somedomain2.com",
+        )
+        DomainInformation.objects.create(
+            creator=self.user,
+            domain=domain2,
+        )
+        DomainInvitation.objects.create(
+            email=self.email6,
+            domain=domain2,
+        )
+
         response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id})
         self.assertEqual(response.status_code, 200)
         data = response.json
 
-        # Check if the domain appears in the response JSON
+        # Check if the domain appears in the response JSON and domain2 does not
         domain_names = [domain_name for member in data["members"] for domain_name in member.get("domain_names", [])]
         self.assertIn("somedomain1.com", domain_names)
+        self.assertNotIn("somedomain2.com", domain_names)
 
     def test_pagination(self):
         """Test that pagination works properly when there are more members than page size."""
diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py
index 11c650cef..e78403659 100644
--- a/src/registrar/views/portfolio_members_json.py
+++ b/src/registrar/views/portfolio_members_json.py
@@ -93,7 +93,8 @@ def initial_permissions_search(portfolio):
                     output_field=CharField(),
                 ),
                 distinct=True,
-                filter=Q(user__permissions__domain__isnull=False),  # filter out null values
+                filter=Q(user__permissions__domain__isnull=False)  # filter out null values
+                & Q(user__permissions__domain__domain_info__portfolio=portfolio),  # only include domains in portfolio
             ),
             source=Value("permission", output_field=CharField()),
         )
@@ -121,10 +122,11 @@ class ArrayRemove(Func):
 
 def initial_invitations_search(portfolio):
     """Perform initial invitations search and get related DomainInvitation data based on the email."""
-    # Get DomainInvitation query for matching email
-    domain_invitations = DomainInvitation.objects.filter(email=OuterRef("email")).annotate(
-        domain_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField())
-    )
+    # Get DomainInvitation query for matching email and for the portfolio
+    domain_invitations = DomainInvitation.objects.filter(
+        email=OuterRef("email"),  # Check if email matches the OuterRef("email")
+        domain__domain_info__portfolio=portfolio,  # Check if the domain's portfolio matches the given portfolio
+    ).annotate(domain_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField()))
     # PortfolioInvitation query
     invitations = PortfolioInvitation.objects.filter(portfolio=portfolio)
     invitations = invitations.annotate(