Merge pull request #2442 from cisagov/za/fix-portfolio-500-error

Bug fix: 500 error when adding a "senior official" to portfolio

src/registrar/admin.py

@@ -19,7 +19,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
@@ -448,8 +448,9 @@ class AdminSortFields:
     sort_mapping = {
         # == Contact == #
         "other_contacts": (Contact, _name_sort),
-        "senior_official": (Contact, _name_sort),
         "submitter": (Contact, _name_sort),
+        # == Senior Official == #
+        "senior_official": (SeniorOfficial, _name_sort),
         # == User == #
         "creator": (User, _name_sort),
         "user": (User, _name_sort),

src/registrar/tests/test_admin.py

@@ -44,6 +44,7 @@ from registrar.models import (
     UserGroup,
     TransitionDomain,
 )
+from registrar.models.senior_official import SeniorOfficial
 from registrar.models.user_domain_role import UserDomainRole
 from registrar.models.verified_by_staff import VerifiedByStaff
 from .common import (
@@ -935,6 +936,32 @@ class TestDomainRequestAdmin(MockEppLib):
         )
         self.mock_client = MockSESClient()
 
+    def test_domain_request_senior_official_is_alphabetically_sorted(self):
+        """Tests if the senior offical dropdown is alphanetically sorted in the django admin display"""
+
+        SeniorOfficial.objects.get_or_create(first_name="mary", last_name="joe", title="some other guy")
+        SeniorOfficial.objects.get_or_create(first_name="alex", last_name="smoe", title="some guy")
+        SeniorOfficial.objects.get_or_create(first_name="Zoup", last_name="Soup", title="title")
+
+        contact, _ = Contact.objects.get_or_create(user=self.staffuser)
+        domain_request = completed_domain_request(submitter=contact, name="city1.gov")
+        request = self.factory.post("/admin/registrar/domainrequest/{}/change/".format(domain_request.pk))
+        model_admin = AuditedAdmin(DomainRequest, self.site)
+
+        # Get the queryset that would be returned for the list
+        senior_offical_queryset = model_admin.formfield_for_foreignkey(
+            DomainInformation.senior_official.field, request
+        ).queryset
+
+        # Make the list we're comparing on a bit prettier display-wise. Optional step.
+        current_sort_order = []
+        for official in senior_offical_queryset:
+            current_sort_order.append(f"{official.first_name} {official.last_name}")
+
+        expected_sort_order = ["alex smoe", "mary joe", "Zoup Soup"]
+
+        self.assertEqual(current_sort_order, expected_sort_order)
+
     @less_console_noise_decorator
     def test_has_model_description(self):
         """Tests if this model has a model description on the table view"""
@@ -2732,6 +2759,7 @@ class TestDomainRequestAdmin(MockEppLib):
         User.objects.all().delete()
         Contact.objects.all().delete()
         Website.objects.all().delete()
+        SeniorOfficial.objects.all().delete()
         self.mock_client.EMAILS_SENT.clear()
 
 
@@ -2913,6 +2941,38 @@ class TestDomainInformationAdmin(TestCase):
         Domain.objects.all().delete()
         Contact.objects.all().delete()
         User.objects.all().delete()
+        SeniorOfficial.objects.all().delete()
+
+    def test_domain_information_senior_official_is_alphabetically_sorted(self):
+        """Tests if the senior offical dropdown is alphanetically sorted in the django admin display"""
+
+        SeniorOfficial.objects.get_or_create(first_name="mary", last_name="joe", title="some other guy")
+        SeniorOfficial.objects.get_or_create(first_name="alex", last_name="smoe", title="some guy")
+        SeniorOfficial.objects.get_or_create(first_name="Zoup", last_name="Soup", title="title")
+
+        contact, _ = Contact.objects.get_or_create(user=self.staffuser)
+        domain_request = completed_domain_request(
+            submitter=contact, name="city1244.gov", status=DomainRequest.DomainRequestStatus.IN_REVIEW
+        )
+        domain_request.approve()
+
+        domain_info = DomainInformation.objects.get(domain_request=domain_request)
+        request = self.factory.post("/admin/registrar/domaininformation/{}/change/".format(domain_info.pk))
+        model_admin = AuditedAdmin(DomainInformation, self.site)
+
+        # Get the queryset that would be returned for the list
+        senior_offical_queryset = model_admin.formfield_for_foreignkey(
+            DomainInformation.senior_official.field, request
+        ).queryset
+
+        # Make the list we're comparing on a bit prettier display-wise. Optional step.
+        current_sort_order = []
+        for official in senior_offical_queryset:
+            current_sort_order.append(f"{official.first_name} {official.last_name}")
+
+        expected_sort_order = ["alex smoe", "mary joe", "Zoup Soup"]
+
+        self.assertEqual(current_sort_order, expected_sort_order)
 
     @less_console_noise_decorator
     def test_admin_can_see_cisa_region_federal(self):
@@ -3667,6 +3727,7 @@ class AuditedAdminTest(TestCase):
         self.site = AdminSite()
         self.factory = RequestFactory()
         self.client = Client(HTTP_HOST="localhost:8080")
+        self.staffuser = create_user()
 
     def order_by_desired_field_helper(self, obj_to_sort: AuditedAdmin, request, field_name, *obj_names):
         with less_console_noise():
@@ -3718,7 +3779,9 @@ class AuditedAdminTest(TestCase):
     def test_alphabetically_sorted_fk_fields_domain_request(self):
         with less_console_noise():
             tested_fields = [
-                DomainRequest.senior_official.field,
+                # Senior offical is commented out for now - this is alphabetized
+                # and this test does not accurately reflect that.
+                # DomainRequest.senior_official.field,
                 DomainRequest.submitter.field,
                 # DomainRequest.investigator.field,
                 DomainRequest.creator.field,
@@ -3776,7 +3839,9 @@ class AuditedAdminTest(TestCase):
     def test_alphabetically_sorted_fk_fields_domain_information(self):
         with less_console_noise():
             tested_fields = [
-                DomainInformation.senior_official.field,
+                # Senior offical is commented out for now - this is alphabetized
+                # and this test does not accurately reflect that.
+                # DomainInformation.senior_official.field,
                 DomainInformation.submitter.field,
                 # DomainInformation.creator.field,
                 (DomainInformation.domain.field, ["name"]),
@@ -3809,7 +3874,6 @@ class AuditedAdminTest(TestCase):
 
                 # Conforms to the same object structure as desired_order
                 current_sort_order_coerced_type = []
-
                 # This is necessary as .queryset and get_queryset
                 # return lists of different types/structures.
                 # We need to parse this data and coerce them into the same type.
@@ -3886,7 +3950,8 @@ class AuditedAdminTest(TestCase):
         if last_name is None:
             return (first_name,)
 
-        if first_name.split(queryset_shorthand)[1] == field_name:
+        split_name = first_name.split(queryset_shorthand)
+        if len(split_name) == 2 and split_name[1] == field_name:
             return returned_tuple
         else:
             return None