Cleanup

src/registrar/admin.py

@@ -2835,25 +2835,35 @@ class VerifiedByStaffAdmin(ListHeaderAdmin):
         super().save_model(request, obj, form, change)
 
 
-
 class PortfolioAdmin(ListHeaderAdmin):
-    class SuborganizationInline(admin.StackedInline):
-        """"""
-        model = models.Suborganization
-
     change_form_template = "django/admin/portfolio_change_form.html"
-    #inlines = [SuborganizationInline, UserInline]
     fieldsets = [
-        # TODO - this will need to be reworked
-        #(None, {"fields": ["organization_name", "federal_agency", "creator", "created_at", "notes"]}),
         (None, {"fields": ["portfolio_type", "organization_name", "creator", "created_at", "notes"]}),
-        ("Portfolio members", {"fields": ["administrators", "members"]}),
+        ("Portfolio members", {
-        ("Portfolio domains", {"fields": ["domains", "domain_requests"]}),
+            "classes": ("collapse", "closed"),
+            "fields": ["administrators", "members"]}
+        ),
+        ("Portfolio domains", {
+            "classes": ("collapse", "closed"),
+            "fields": ["domains", "domain_requests"]}
+        ),
         ("Type of organization", {"fields": ["organization_type", "federal_type"]}),
-        ("Organization name and mailing address", {"fields": ["federal_agency", "state_territory", "address_line1", "address_line2", "city", "zipcode", "urbanization"]}),
+        (
+            "Organization name and mailing address",
+            {
+                "fields": [
+                    "federal_agency",
+                    "state_territory",
+                    "address_line1",
+                    "address_line2",
+                    "city",
+                    "zipcode",
+                    "urbanization",
+                ]
+            },
+        ),
         ("Suborganizations", {"fields": ["suborganizations"]}),
         ("Senior official", {"fields": ["senior_official"]}),
-        ("Other", {"fields": ["security_contact_email"]}),
     ]
 
     # NOTE: use add_fieldsets to modify that page
@@ -2863,7 +2873,7 @@ class PortfolioAdmin(ListHeaderAdmin):
     readonly_fields = [
         "created_at",
         # Custom fields such as these must be defined as readonly.
-        "administrators", 
+        "administrators",
         "members",
         "domains",
         "domain_requests",
@@ -2872,41 +2882,55 @@ 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
+
     portfolio_type.short_description = "Portfolio type"
 
     def suborganizations(self, obj: models.Portfolio):
+        """Returns a comma seperated list of links for each related suborg"""
         queryset = obj.get_suborganizations()
         sep = '<div class="display-block margin-top-1"></div>'
         return self.get_links_csv(queryset, "suborganization", seperator=sep)
+
     suborganizations.short_description = "Suborganizations"
 
     def domains(self, obj: models.Portfolio):
+        """Returns a comma seperated list of links for each related domain"""
         queryset = obj.get_domains()
         sep = '<div class="display-block margin-top-1"></div>'
         return self.get_links_csv(queryset, "domaininformation", seperator=sep)
+
     domains.short_description = "Domains"
-    
+
     def domain_requests(self, obj: models.Portfolio):
+        """Returns a comma seperated list of links for each related domain request"""
         queryset = obj.get_domain_requests()
         sep = '<div class="display-block margin-top-1"></div>'
         return self.get_links_csv(queryset, "domainrequest", seperator=sep)
+
     domain_requests.short_description = "Domain requests"
 
     def administrators(self, obj: models.Portfolio):
+        """Returns a comma seperated list of links for each related administrator"""
         queryset = obj.get_administrators()
         return self.get_links_csv(queryset, "user", "get_full_name")
+
     administrators.short_description = "Administrators"
 
     def members(self, obj: models.Portfolio):
+        """Returns a comma seperated list of links for each related member"""
         queryset = obj.get_members()
         return self.get_links_csv(queryset, "user", "get_full_name")
+
     members.short_description = "Members"
 
     # Creates select2 fields (with search bars)

src/registrar/migrations/0118_portfolio_federal_type_alter_portfolio_creator_and_more.py

@@ -1,5 +1,6 @@
-# Generated by Django 4.2.10 on 2024-08-13 16:31
+# Generated by Django 4.2.10 on 2024-08-13 20:01
 
+from django.conf import settings
 from django.db import migrations, models
 import django.db.models.deletion
 
@@ -21,6 +22,28 @@ class Migration(migrations.Migration):
                 null=True,
             ),
         ),
+        migrations.AlterField(
+            model_name="portfolio",
+            name="creator",
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.PROTECT,
+                related_name="created_portfolios",
+                to=settings.AUTH_USER_MODEL,
+                verbose_name="Portfolio creator",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="portfolio",
+            name="organization_name",
+            field=models.CharField(blank=True, null=True, verbose_name="Portfolio organization"),
+        ),
+        migrations.AlterField(
+            model_name="portfolio",
+            name="senior_official",
+            field=models.ForeignKey(
+                blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, to="registrar.seniorofficial"
+            ),
+        ),
         migrations.AlterField(
             model_name="suborganization",
             name="portfolio",

src/registrar/models/portfolio.py

@@ -23,11 +23,26 @@ class Portfolio(TimeStampedModel):
     creator = models.ForeignKey(
         "registrar.User",
         on_delete=models.PROTECT,
-        help_text="Associated user",
+        verbose_name="Portfolio creator",
         related_name="created_portfolios",
         unique=False,
     )
 
+    # Q for reviewers: shouldn't this be a required field?
+    organization_name = models.CharField(
+        null=True,
+        blank=True,
+        verbose_name="Portfolio organization",
+    )
+
+    organization_type = models.CharField(
+        max_length=255,
+        choices=OrganizationChoices.choices,
+        null=True,
+        blank=True,
+        help_text="Type of organization",
+    )
+
     notes = models.TextField(
         null=True,
         blank=True,
@@ -51,25 +66,11 @@ class Portfolio(TimeStampedModel):
     senior_official = models.ForeignKey(
         "registrar.SeniorOfficial",
         on_delete=models.PROTECT,
-        help_text="Associated senior official",
         unique=False,
         null=True,
         blank=True,
     )
 
-    organization_type = models.CharField(
-        max_length=255,
-        choices=OrganizationChoices.choices,
-        null=True,
-        blank=True,
-        help_text="Type of organization",
-    )
-
-    organization_name = models.CharField(
-        null=True,
-        blank=True,
-    )
-
     address_line1 = models.CharField(
         null=True,
         blank=True,
@@ -118,7 +119,7 @@ class Portfolio(TimeStampedModel):
     )
 
     def __str__(self) -> str:
-        return f"{self.organization_name}"
+        return str(self.organization_name)
 
     # == Getters for domains == #
     def get_domains(self):

src/registrar/models/senior_official.py

@@ -54,6 +54,9 @@ class SeniorOfficial(TimeStampedModel):
         names = [n for n in [self.first_name, self.last_name] if n]
         return " ".join(names) if names else "Unknown"
 
+    def has_contact_info(self):
+        return bool(self.title or self.email or self.phone)
+
     def __str__(self):
         if self.first_name or self.last_name:
             return self.get_formatted_name()

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

@@ -9,8 +9,8 @@
         {% else %}
             None
         {% endif %}
+        </br>
     {% endif %}
-    </br>
 
     {% if user.has_contact_info %}
         {# Title #}