Django admin portfolio members section

src/registrar/admin.py

@@ -2867,11 +2867,7 @@ class PortfolioAdmin(ListHeaderAdmin):
     fieldsets = [
         # created_on is the created_at field, and portfolio_type is f"{organization_type} - {federal_type}"
         (None, {"fields": ["portfolio_type", "organization_name", "creator", "created_on", "notes"]}),
-        # TODO - uncomment in #2521
-        # ("Portfolio members", {
-        #     "classes": ("collapse", "closed"),
-        #     "fields": ["administrators", "members"]}
-        # ),
+        ("Portfolio members", {"fields": ["display_admins", "display_members"]}),
         ("Portfolio domains", {"classes": ("collapse", "closed"), "fields": ["domains", "domain_requests"]}),
         ("Type of organization", {"fields": ["organization_type", "federal_type"]}),
         (
@@ -2925,8 +2921,94 @@ class PortfolioAdmin(ListHeaderAdmin):
         "domain_requests",
         "suborganizations",
         "portfolio_type",
+        # Django admin doesn't allow methods to be directly listed in fieldsets. We can
+        # display the custom methods display_admins amd display_members in the admin form if
+        # they are readonly.
+        "display_admins",
+        "display_members",
     ]
 
+    def display_admins(self, obj):
+        """Get joined users who are Admin, unpack and return an HTML block.
+
+        'DJA readonly can't handle querysets, so we need to unpack and return html here.
+        Alternatively, we could return querysets in context but that would limit where this
+        data would display in a custom change form without extensive template customization.
+
+        Will be used in the field_readonly block"""
+        admins = [user for user in obj.user.all() if "Admin" in user.portfolio_role_summary]
+        if not admins:
+            return format_html("<p>No admins found.</p>")
+
+        admin_details = ""
+        for portfolio_admin in admins:
+            change_url = reverse("admin:registrar_user_change", args=[portfolio_admin.pk])
+            admin_details += "<address class='margin-bottom-2 dja-address-contact-list'>"
+            admin_details += f'<a href="{change_url}">{portfolio_admin}</a><br>'
+            admin_details += f"{portfolio_admin.title}<br>"
+            admin_details += f"{portfolio_admin.email}"
+            admin_details += "<div class='admin-icon-group admin-icon-group__clipboard-link'>"
+            admin_details += f"<input aria-hidden='true' class='display-none' value='{portfolio_admin.email}'>"
+            admin_details += (
+                "<button class='usa-button usa-button--unstyled padding-right-1 usa-button--icon"
+                + "button--clipboard copy-to-clipboard text-no-underline' type='button'>"
+            )
+            admin_details += "<svg class='usa-icon'>"
+            admin_details += "<use aria-hidden='true' xlink:href='/public/img/sprite.svg#content_copy'></use>"
+            admin_details += "</svg>"
+            admin_details += "<span class='padding-left-05'>Copy</span>"
+            admin_details += "</button>"
+            admin_details += "</div><br>"
+            admin_details += f"{portfolio_admin.phone}"
+            admin_details += "</address>"
+        return format_html(admin_details)
+
+    display_admins.short_description = "Administrators"  # type: ignore
+
+    def display_members(self, obj):
+        """Get joined users who have roles/perms that are not Admin, unpack and return an HTML block.
+
+        DJA readonly can't handle querysets, so we need to unpack and return html here.
+        Alternatively, we could return querysets in context but that would limit where this
+        data would display in a custom change form without extensive template customization.
+
+        Will be used in the after_help_text block."""
+        members = [user for user in obj.user.all() if "Admin" not in user.portfolio_role_summary]
+        if not members:
+            return format_html("<p>No members found.</p>")
+
+        member_details = (
+            "<table><thead><tr><th>Name</th><th>Title</th><th>Email</th>"
+            + "<th>Phone</th><th>Roles</th></tr></thead><tbody>"
+        )
+        for member in members:
+            full_name = (
+                f"{member.first_name} {member.middle_name} {member.last_name}"
+                if member.middle_name
+                else f"{member.first_name} {member.last_name}"
+            )
+            member_details += "<tr>"
+            member_details += f"<td>{full_name}</td>"
+            member_details += f"<td>{member.title}</td>"
+            member_details += f"<td>{member.email}</td>"
+            member_details += f"<td>{member.phone}</td>"
+            member_details += "<td>"
+            for role in member.portfolio_role_summary:
+                member_details += f"<span class='usa-tag'>{role}</span> "
+            member_details += "</td></tr>"
+        member_details += "</tbody></table>"
+        return format_html(member_details)
+
+    display_members.short_description = "Members"  # type: ignore
+
+    def display_members_summary(self, obj):
+        """Will be passed as context and used in the field_readonly block."""
+        members = [user for user in obj.user.all() if "Admin" not in user.portfolio_role_summary]
+        if not members:
+            return {}
+
+        return self.get_field_links_as_list(members, "user", separator=", ")
+
     def federal_type(self, obj: models.Portfolio):
         """Returns the federal_type field"""
         return BranchChoices.get_branch_label(obj.federal_type) if obj.federal_type else "-"
@@ -2977,7 +3059,7 @@ class PortfolioAdmin(ListHeaderAdmin):
     ]
 
     def get_field_links_as_list(
-        self, queryset, model_name, attribute_name=None, link_info_attribute=None, seperator=None
+        self, queryset, model_name, attribute_name=None, link_info_attribute=None, separator=None
     ):
         """
         Generate HTML links for items in a queryset, using a specified attribute for link text.
@@ -3009,14 +3091,14 @@ class PortfolioAdmin(ListHeaderAdmin):
                 if link_info_attribute:
                     link += f" ({self.value_of_attribute(item, link_info_attribute)})"
 
-                if seperator:
+                if separator:
                     links.append(link)
                 else:
                     links.append(f"<li>{link}</li>")
 
-        # If no seperator is specified, just return an unordered list.
-        if seperator:
-            return format_html(seperator.join(links)) if links else "-"
+        # If no separator is specified, just return an unordered list.
+        if separator:
+            return format_html(separator.join(links)) if links else "-"
         else:
             links = "".join(links)
             return format_html(f'<ul class="add-list-reset">{links}</ul>') if links else "-"
@@ -3059,8 +3141,12 @@ class PortfolioAdmin(ListHeaderAdmin):
         return readonly_fields
 
     def change_view(self, request, object_id, form_url="", extra_context=None):
-        """Add related suborganizations and domain groups"""
-        extra_context = {"skip_additional_contact_info": True}
+        """Add related suborganizations and domain groups.
+        Add the summary for the portfolio members field (list of members that link to change_forms)."""
+        obj = self.get_object(request, object_id)
+        extra_context = extra_context or {}
+        extra_context["skip_additional_contact_info"] = True
+        extra_context["display_members_summary"] = self.display_members_summary(obj)
         return super().change_view(request, object_id, form_url, extra_context)
 
     def save_model(self, request, obj, form, change):

src/registrar/models/user.py

@@ -293,6 +293,49 @@ class User(AbstractUser):
     def has_edit_suborganization(self):
         return self._has_portfolio_permission(UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION)
 
+    def has_edit_requests(self):
+        return self._has_portfolio_permission(UserPortfolioPermissionChoices.EDIT_REQUESTS)
+
+    @property
+    def portfolio_role_summary(self):
+        """Returns a list of roles based on the user's permissions."""
+        roles = []
+
+        # Define the conditions and their corresponding roles
+        conditions_roles = [
+            (self.has_edit_suborganization(), ["Admin"]),
+            (
+                self.has_view_all_domains_permission()
+                and self.has_domain_requests_portfolio_permission()
+                and self.has_edit_requests(),
+                ["View-only admin", "Domain requestor"],
+            ),
+            (
+                self.has_view_all_domains_permission() and self.has_domain_requests_portfolio_permission(),
+                ["View-only admin"],
+            ),
+            (
+                self.has_base_portfolio_permission()
+                and self.has_edit_requests()
+                and self.has_domains_portfolio_permission(),
+                ["Member", "Domain requestor", "Domain manager"],
+            ),
+            (self.has_base_portfolio_permission() and self.has_edit_requests(), ["Member", "Domain requestor"]),
+            (
+                self.has_base_portfolio_permission() and self.has_domains_portfolio_permission(),
+                ["Member", "Domain manager"],
+            ),
+            (self.has_base_portfolio_permission(), ["Member"]),
+        ]
+
+        # Evaluate conditions and add roles
+        for condition, role_list in conditions_roles:
+            if condition:
+                roles.extend(role_list)
+                break
+
+        return roles
+
     @classmethod
     def needs_identity_verification(cls, email, uuid):
         """A method used by our oidc classes to test whether a user needs email/uuid verification

src/registrar/templates/django/admin/includes/detail_table_fieldset.html

@@ -137,6 +137,10 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
             {% endfor %}
             {% endwith %}
         </div>
+    {% elif field.field.name == "display_admins" %}
+        <div class="readonly">{{ field.contents|safe }}</div>
+    {% elif field.field.name == "display_members" %}
+        <div class="readonly">{{ display_members_summary }}</div>
     {% else %}
         <div class="readonly">{{ field.contents }}</div>
     {% endif %}
@@ -240,6 +244,13 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
             </details>
         {% endif %}
         {% endwith %}
+    {% elif field.field.name == "display_members" %}
+        <details class="margin-top-1 dja-detail-table" aria-role="button" open>
+            <summary class="padding-1 padding-left-0 dja-details-summary">Details</summary>
+            <div class="grid-container margin-left-0 padding-left-0 padding-right-0 dja-details-contents">
+                {{ field.contents|safe }}
+            </div>
+        </details>
     {% elif field.field.name == "state_territory" %}
         <div class="flex-container margin-top-2">
             <span>

src/registrar/templates/django/admin/portfolio_change_form.html

@@ -14,8 +14,7 @@
         This is a placeholder for now.
 
         Disclaimer:
-        When extending the fieldset view - *make a new one* that extends from detail_table_fieldset.
-        For instance, "portfolio_fieldset.html".
+        When extending the fieldset view consider whether you need to make a new one that extends from detail_table_fieldset.
         detail_table_fieldset is used on multiple admin pages, so a change there can have unintended consequences.
         {% endcomment %}
         {% include "django/admin/includes/detail_table_fieldset.html" with original_object=original %}

src/registrar/tests/test_admin.py

@@ -45,6 +45,7 @@ from registrar.models import (
 from registrar.models.portfolio_invitation import PortfolioInvitation
 from registrar.models.senior_official import SeniorOfficial
 from registrar.models.user_domain_role import UserDomainRole
+from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices
 from registrar.models.verified_by_staff import VerifiedByStaff
 from .common import (
     MockDbForSharedTests,
@@ -2066,6 +2067,7 @@ class TestPortfolioAdmin(TestCase):
         DomainRequest.objects.all().delete()
         Domain.objects.all().delete()
         Portfolio.objects.all().delete()
+        User.objects.all().delete()
 
     @less_console_noise_decorator
     def test_created_on_display(self):
@@ -2121,3 +2123,81 @@ class TestPortfolioAdmin(TestCase):
         self.assertIn("request1.gov", domain_requests)
         self.assertIn("request2.gov", domain_requests)
         self.assertIn('<ul class="add-list-reset">', domain_requests)
+
+    @less_console_noise_decorator
+    def test_portfolio_members_display(self):
+        """Tests the custom portfolio members field, admin and member sections"""
+        admin_user_1 = User.objects.create(
+            username="testuser1",
+            first_name="Gerald",
+            last_name="Meoward",
+            title="Captain",
+            email="meaoward@gov.gov",
+            portfolio=self.portfolio,
+            portfolio_roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
+        )
+
+        admin_user_2 = User.objects.create(
+            username="testuser2",
+            first_name="Arnold",
+            last_name="Poopy",
+            title="Major",
+            email="poopy@gov.gov",
+            portfolio=self.portfolio,
+            portfolio_roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
+        )
+
+        admin_user_3 = User.objects.create(
+            username="testuser3",
+            first_name="Mad",
+            last_name="Max",
+            title="Road warrior",
+            email="madmax@gov.gov",
+            portfolio=self.portfolio,
+            portfolio_roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
+        )
+
+        admin_user_4 = User.objects.create(
+            username="testuser4",
+            first_name="Agent",
+            last_name="Smith",
+            title="Program",
+            email="thematrix@gov.gov",
+            portfolio=self.portfolio,
+            portfolio_additional_permissions=[
+                UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
+                UserPortfolioPermissionChoices.EDIT_REQUESTS,
+            ],
+        )
+
+        display_admins = self.admin.display_admins(self.portfolio)
+
+        self.assertIn(
+            f'<a href="/admin/registrar/user/{admin_user_1.pk}/change/">Gerald Meoward meaoward@gov.gov</a>',
+            display_admins,
+        )
+        self.assertIn("Captain", display_admins)
+        self.assertIn(
+            f'<a href="/admin/registrar/user/{admin_user_2.pk}/change/">Arnold Poopy poopy@gov.gov</a>', display_admins
+        )
+        self.assertIn("Major", display_admins)
+
+        display_members_summary = self.admin.display_members_summary(self.portfolio)
+
+        self.assertIn(
+            f'<a href="/admin/registrar/user/{admin_user_3.pk}/change/">Mad Max madmax@gov.gov</a>',
+            display_members_summary,
+        )
+        self.assertIn(
+            f'<a href="/admin/registrar/user/{admin_user_4.pk}/change/">Agent Smith thematrix@gov.gov</a>',
+            display_members_summary,
+        )
+
+        display_members = self.admin.display_members(self.portfolio)
+
+        self.assertIn("Mad Max", display_members)
+        self.assertIn("<span class='usa-tag'>Member</span>", display_members)
+        self.assertIn("Road warrior", display_members)
+        self.assertIn("Agent Smith", display_members)
+        self.assertIn("<span class='usa-tag'>Domain requestor</span>", display_members)
+        self.assertIn("Program", display_members)

src/registrar/tests/test_models.py

@@ -1201,6 +1201,61 @@ class TestUser(TestCase):
         User.objects.all().delete()
         UserDomainRole.objects.all().delete()
 
+    @patch.object(User, "has_edit_suborganization", return_value=True)
+    def test_portfolio_role_summary_admin(self, mock_edit_suborganization):
+        # Test if the user is recognized as an Admin
+        self.assertEqual(self.user.portfolio_role_summary, ["Admin"])
+
+    @patch.multiple(
+        User,
+        has_view_all_domains_permission=lambda self: True,
+        has_domain_requests_portfolio_permission=lambda self: True,
+        has_edit_requests=lambda self: True,
+    )
+    def test_portfolio_role_summary_view_only_admin_and_domain_requestor(self):
+        # Test if the user has both 'View-only admin' and 'Domain requestor' roles
+        self.assertEqual(self.user.portfolio_role_summary, ["View-only admin", "Domain requestor"])
+
+    @patch.multiple(
+        User,
+        has_view_all_domains_permission=lambda self: True,
+        has_domain_requests_portfolio_permission=lambda self: True,
+    )
+    def test_portfolio_role_summary_view_only_admin(self):
+        # Test if the user is recognized as a View-only admin
+        self.assertEqual(self.user.portfolio_role_summary, ["View-only admin"])
+
+    @patch.multiple(
+        User,
+        has_base_portfolio_permission=lambda self: True,
+        has_edit_requests=lambda self: True,
+        has_domains_portfolio_permission=lambda self: True,
+    )
+    def test_portfolio_role_summary_member_domain_requestor_domain_manager(self):
+        # Test if the user has 'Member', 'Domain requestor', and 'Domain manager' roles
+        self.assertEqual(self.user.portfolio_role_summary, ["Member", "Domain requestor", "Domain manager"])
+
+    @patch.multiple(User, has_base_portfolio_permission=lambda self: True, has_edit_requests=lambda self: True)
+    def test_portfolio_role_summary_member_domain_requestor(self):
+        # Test if the user has 'Member' and 'Domain requestor' roles
+        self.assertEqual(self.user.portfolio_role_summary, ["Member", "Domain requestor"])
+
+    @patch.multiple(
+        User, has_base_portfolio_permission=lambda self: True, has_domains_portfolio_permission=lambda self: True
+    )
+    def test_portfolio_role_summary_member_domain_manager(self):
+        # Test if the user has 'Member' and 'Domain manager' roles
+        self.assertEqual(self.user.portfolio_role_summary, ["Member", "Domain manager"])
+
+    @patch.multiple(User, has_base_portfolio_permission=lambda self: True)
+    def test_portfolio_role_summary_member(self):
+        # Test if the user is recognized as a Member
+        self.assertEqual(self.user.portfolio_role_summary, ["Member"])
+
+    def test_portfolio_role_summary_empty(self):
+        # Test if the user has no roles
+        self.assertEqual(self.user.portfolio_role_summary, [])
+
     @less_console_noise_decorator
     def test_check_transition_domains_without_domains_on_login(self):
         """A user's on_each_login callback does not check transition domains.