From 5f0b342986ad918c512fd8a863e6283b161d5d0c Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Tue, 13 Aug 2024 14:10:50 -0600
Subject: [PATCH] Further cleanup

---
 src/registrar/admin.py            | 21 +++++++--------------
 src/registrar/models/portfolio.py |  8 ++++++++
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index abaa5ff54..afe23c86d 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -2839,10 +2839,11 @@ class PortfolioAdmin(ListHeaderAdmin):
     change_form_template = "django/admin/portfolio_change_form.html"
     fieldsets = [
         (None, {"fields": ["portfolio_type", "organization_name", "creator", "created_at", "notes"]}),
-        ("Portfolio members", {
-            "classes": ("collapse", "closed"),
-            "fields": ["administrators", "members"]}
-        ),
+        # TODO - uncomment in #2521
+        # ("Portfolio members", {
+        #     "classes": ("collapse", "closed"),
+        #     "fields": ["administrators", "members"]}
+        # ),
         ("Portfolio domains", {
             "classes": ("collapse", "closed"),
             "fields": ["domains", "domain_requests"]}
@@ -2882,16 +2883,8 @@ class PortfolioAdmin(ListHeaderAdmin):
         "portfolio_type",
     ]
 
-    # TODO - this returns None when empty rather than - for some reason
     def portfolio_type(self, obj: models.Portfolio):
-        """Returns a concat of organization type and federal agency,
-        seperated by a dash (-)"""
-        org_choices = DomainRequest.OrganizationChoices
-        org_type = org_choices.get_org_label(obj.organization_type)
-        if obj.organization_type == org_choices.FEDERAL and obj.federal_agency:
-            return " - ".join([org_type, obj.federal_agency.agency])
-        else:
-            return org_type
+        return obj.portfolio_type if obj.portfolio_type else "-"
 
     portfolio_type.short_description = "Portfolio type"
 
@@ -2953,7 +2946,7 @@ class PortfolioAdmin(ListHeaderAdmin):
             if item_display_value:
                 change_url = reverse(f"admin:registrar_{model_name}_change", args=[item.pk])
                 links.append(f'<a href="{change_url}">{escape(item_display_value)}</a>')
-        return format_html(seperator.join(links))
+        return format_html(seperator.join(links)) if links else "-"
 
     def change_view(self, request, object_id, form_url="", extra_context=None):
         """Add related suborganizations and domain groups"""
diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py
index 7e5ee980d..77df3be31 100644
--- a/src/registrar/models/portfolio.py
+++ b/src/registrar/models/portfolio.py
@@ -121,6 +121,14 @@ class Portfolio(TimeStampedModel):
     def __str__(self) -> str:
         return str(self.organization_name)
 
+    @property
+    def portfolio_type(self):
+        org_type = self.OrganizationChoices.get_org_label(self.organization_type)
+        if self.organization_type == self.OrganizationChoices.FEDERAL and self.federal_agency:
+            return " - ".join([org_type, self.federal_agency.agency])
+        else:
+            return org_type
+
     # == Getters for domains == #
     def get_domains(self):
         """Returns all DomainInformations associated with this portfolio"""