diff --git a/src/registrar/admin.py b/src/registrar/admin.py index d3530101d..2151d6f3c 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3193,9 +3193,6 @@ class PortfolioAdmin(ListHeaderAdmin): return self.add_fieldsets return super().get_fieldsets(request, obj) - # TODO - I think the solution to this may just be to set this as editable. - # Then in the underlying page override, we need a fake readonly display that - # is toggled on or off. def get_readonly_fields(self, request, obj=None): """Set the read-only state on form elements. We have 2 conditions that determine which fields are read-only: @@ -3241,7 +3238,6 @@ class PortfolioAdmin(ListHeaderAdmin): return super().change_view(request, object_id, form_url, extra_context) def save_model(self, request, obj: Portfolio, form, change): - if hasattr(obj, "creator") is False: # ---- update creator ---- # Set the creator field to the current admin user @@ -3255,8 +3251,8 @@ class PortfolioAdmin(ListHeaderAdmin): if is_federal and obj.organization_name is None: obj.organization_name = obj.federal_agency.agency - # TODO - this should be handled almost entirely in javascript - # Handle the federal agency and senior official fields + # Set the senior official field to the senior official on the federal agency + # when federal - otherwise, clear the field. if obj.organization_type == obj.OrganizationChoices.FEDERAL: if obj.federal_agency: if obj.federal_agency.so_federal_agency.exists(): @@ -3269,7 +3265,6 @@ class PortfolioAdmin(ListHeaderAdmin): obj.senior_official = None obj.federal_agency = FederalAgency.objects.filter(agency="Non-Federal Agency").first() - super().save_model(request, obj, form, change) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 9c5e3b582..2677462df 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2103,6 +2103,66 @@ class TestPortfolioAdmin(TestCase): display_members = self.admin.display_members(self.portfolio) self.assertIn(f'2 members', display_members) + @less_console_noise_decorator + def test_senior_official_readonly_for_federal_org(self): + """Test that senior_official field is readonly for federal organizations""" + request = self.factory.get("/") + request.user = self.superuser + + # Create a federal portfolio + portfolio = Portfolio.objects.create( + organization_name="Test Federal Org", + organization_type=DomainRequest.OrganizationChoices.FEDERAL, + creator=self.superuser, + ) + + readonly_fields = self.admin.get_readonly_fields(request, portfolio) + self.assertIn("senior_official", readonly_fields) + + # Change to non-federal org + portfolio.organization_type = DomainRequest.OrganizationChoices.CITY + readonly_fields = self.admin.get_readonly_fields(request, portfolio) + self.assertNotIn("senior_official", readonly_fields) + + @less_console_noise_decorator + def test_senior_official_auto_assignment(self): + """Test automatic senior official assignment based on organization type and federal agency""" + request = self.factory.get("/") + request.user = self.superuser + + # Create a federal agency with a senior official + federal_agency = FederalAgency.objects.create(agency="Test Agency") + senior_official = SeniorOfficial.objects.create( + first_name="Test", + last_name="Official", + title="Some guy", + email="test@example.gov", + federal_agency=federal_agency, + ) + + # Create a federal portfolio + portfolio = Portfolio.objects.create( + organization_name="Test Federal Org", + organization_type=DomainRequest.OrganizationChoices.FEDERAL, + creator=self.superuser, + ) + + # Test that the federal org gets senior official from agency when federal + portfolio.federal_agency = federal_agency + self.admin.save_model(request, portfolio, form=None, change=False) + self.assertEqual(portfolio.senior_official, senior_official) + + # Test non-federal org clears senior official when not city + portfolio.organization_type = DomainRequest.OrganizationChoices.CITY + self.admin.save_model(request, portfolio, form=None, change=True) + self.assertIsNone(portfolio.senior_official) + self.assertEqual(portfolio.federal_agency.agency, "Non-Federal Agency") + + # Cleanup + senior_official.delete() + federal_agency.delete() + portfolio.delete() + class TestTransferUser(WebTest): """User transfer custom admin page"""