Fix pre-existing bug with sortfields

The AdminSortFields helper is incorrectly sorting the senior_official contact object.

Added a workaround as this ultimately needs a refactor

src/registrar/admin.py

@@ -22,7 +22,7 @@ from epplibwrapper.errors import ErrorCode, RegistryError
 from registrar.models.user_domain_role import UserDomainRole
 from waffle.admin import FlagAdmin
 from waffle.models import Sample, Switch
-from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website
+from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website, SeniorOfficial
 from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes
 from registrar.views.utility.mixins import OrderableFieldsMixin
 from django.contrib.admin.views.main import ORDER_VAR
@@ -493,6 +493,8 @@ class CustomLogEntryAdmin(LogEntryAdmin):
     #     return super().change_view(request, object_id, form_url, extra_context=extra_context)
 
 
+# TODO - this should be refactored. This is shared among every class that inherits this,
+# and it breaks the senior_official field because it exists both as model "Contact" and "SeniorOfficial".
 class AdminSortFields:
     _name_sort = ["first_name", "last_name", "email"]
 
@@ -504,6 +506,8 @@ class AdminSortFields:
         # == Contact == #
         "other_contacts": (Contact, _name_sort),
         "submitter": (Contact, _name_sort),
+        # == Senior Official == #
+        "senior_official": (SeniorOfficial, _name_sort),
         # == User == #
         "creator": (User, _name_sort),
         "user": (User, _name_sort),
@@ -553,15 +557,16 @@ class AuditedAdmin(admin.ModelAdmin):
             )
         )
 
-    def formfield_for_manytomany(self, db_field, request, **kwargs):
+    def formfield_for_manytomany(self, db_field, request, use_admin_sort_fields=True, **kwargs):
         """customize the behavior of formfields with manytomany relationships.  the customized
         behavior includes sorting of objects in lists as well as customizing helper text"""
 
         # Define a queryset. Note that in the super of this,
         # a new queryset will only be generated if one does not exist.
         # Thus, the order in which we define queryset matters.
+
         queryset = AdminSortFields.get_queryset(db_field)
-        if queryset:
+        if queryset and use_admin_sort_fields:
             kwargs["queryset"] = queryset
 
         formfield = super().formfield_for_manytomany(db_field, request, **kwargs)
@@ -572,7 +577,7 @@ class AuditedAdmin(admin.ModelAdmin):
         )
         return formfield
 
-    def formfield_for_foreignkey(self, db_field, request, **kwargs):
+    def formfield_for_foreignkey(self, db_field, request, use_admin_sort_fields=True, **kwargs):
         """Customize the behavior of formfields with foreign key relationships. This will customize
         the behavior of selects. Customized behavior includes sorting of objects in list."""
 
@@ -580,7 +585,7 @@ class AuditedAdmin(admin.ModelAdmin):
         # a new queryset will only be generated if one does not exist.
         # Thus, the order in which we define queryset matters.
         queryset = AdminSortFields.get_queryset(db_field)
-        if queryset:
+        if queryset and use_admin_sort_fields:
             kwargs["queryset"] = queryset
 
         return super().formfield_for_foreignkey(db_field, request, **kwargs)
@@ -1542,6 +1547,16 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin):
         # Get the filtered values
         return super().changelist_view(request, extra_context=extra_context)
 
+    def formfield_for_foreignkey(self, db_field, request, **kwargs):
+        """Customize the behavior of formfields with foreign key relationships. This will customize
+        the behavior of selects. Customized behavior includes sorting of objects in list."""
+        # Remove this check on senior_official if this underlying model changes from
+        # "Contact" to "SeniorOfficial" or if we refactor AdminSortFields.
+        # Removing this will cause the list on django admin to return SeniorOffical
+        # objects rather than Contact objects.
+        use_sort = db_field.name != "senior_official"
+        return super().formfield_for_foreignkey(db_field, request, use_admin_sort_fields=use_sort, **kwargs)
+
 
 class DomainRequestResource(FsmModelResource):
     """defines how each field in the referenced model should be mapped to the corresponding fields in the
@@ -2209,6 +2224,16 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin):
 
         return None
 
+    def formfield_for_foreignkey(self, db_field, request, **kwargs):
+        """Customize the behavior of formfields with foreign key relationships. This will customize
+        the behavior of selects. Customized behavior includes sorting of objects in list."""
+        # Remove this check on senior_official if this underlying model changes from
+        # "Contact" to "SeniorOfficial" or if we refactor AdminSortFields.
+        # Removing this will cause the list on django admin to return SeniorOffical
+        # objects rather than Contact objects.
+        use_sort = db_field.name != "senior_official"
+        return super().formfield_for_foreignkey(db_field, request, use_admin_sort_fields=use_sort, **kwargs)
+
 
 class TransitionDomainAdmin(ListHeaderAdmin):
     """Custom transition domain admin class."""
@@ -2258,6 +2283,7 @@ class DomainInformationInline(admin.StackedInline):
     def formfield_for_manytomany(self, db_field, request, **kwargs):
         """customize the behavior of formfields with manytomany relationships.  the customized
         behavior includes sorting of objects in lists as well as customizing helper text"""
+
         queryset = AdminSortFields.get_queryset(db_field)
         if queryset:
             kwargs["queryset"] = queryset
@@ -2272,8 +2298,12 @@ class DomainInformationInline(admin.StackedInline):
     def formfield_for_foreignkey(self, db_field, request, **kwargs):
         """Customize the behavior of formfields with foreign key relationships. This will customize
         the behavior of selects. Customized behavior includes sorting of objects in list."""
+        # Remove this check on senior_official if this underlying model changes from
+        # "Contact" to "SeniorOfficial" or if we refactor AdminSortFields.
+        # Removing this will cause the list on django admin to return SeniorOffical
+        # objects rather than Contact objects.
         queryset = AdminSortFields.get_queryset(db_field)
-        if queryset:
+        if queryset and db_field.name != "senior_official":
             kwargs["queryset"] = queryset
         return super().formfield_for_foreignkey(db_field, request, **kwargs)
 

src/registrar/forms/domain.py

@@ -326,7 +326,7 @@ class SeniorOfficialContactForm(ContactForm):
     def __init__(self, disable_fields=False, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
-        if self.instance:
+        if self.instance and self.instance.id:
             self.fields["full_name"].initial = self.instance.get_formatted_name()
 
         # Overriding bc phone not required in this form

src/registrar/forms/portfolio.py

@@ -87,5 +87,5 @@ class PortfolioSeniorOfficialForm(forms.ModelForm):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        if self.instance:
+        if self.instance and self.instance.id:
             self.fields["full_name"].initial = self.instance.get_formatted_name()

src/registrar/templates/includes/senior_official.html

@@ -44,4 +44,4 @@
     {% if form.email.value is not None %}
         {% include "includes/input_read_only.html" with field=form.email %}
     {% endif %}
-{% endif %}
+{% endif %}

src/registrar/tests/test_views_portfolio.py

@@ -41,7 +41,7 @@ class TestPortfolio(WebTest):
     @less_console_noise_decorator
     @override_flag("organization_feature", active=True)
     def test_portfolio_senior_official(self):
-        """Tests the senior official page on portfolio"""
+        """Tests that the senior official page on portfolio contains the content we expect"""
         self.app.set_user(self.user.username)
 
         so = SeniorOfficial.objects.create(
@@ -63,6 +63,7 @@ class TestPortfolio(WebTest):
         self.assertContains(so_portfolio_page, "Saturn Enceladus")
         self.assertContains(so_portfolio_page, "Planet/Moon")
         self.assertContains(so_portfolio_page, "spacedivision@igorville.com")
+        self.assertNotContains(so_portfolio_page, "Save")
 
         self.portfolio.delete()
         so.delete()