Fix unit tests and linting

src/registrar/admin.py

@@ -13,7 +13,7 @@ from django.contrib.contenttypes.models import ContentType
 from django.http.response import HttpResponseRedirect
 from django.urls import reverse
 from epplibwrapper.errors import ErrorCode, RegistryError
-from registrar.models import (Contact, Domain, DomainApplication, DraftDomain, User, Website)
+from registrar.models import Contact, Domain, DomainApplication, DraftDomain, User, Website
 from registrar.utility import csv_export
 from registrar.views.utility.mixins import OrderableFieldsMixin
 from django.contrib.admin.views.main import ORDER_VAR
@@ -272,6 +272,14 @@ class UserContactInline(admin.StackedInline):
 class UserAdmin(BaseUserAdmin):
     """Custom user admin class to use our inlines."""
 
+    class Meta:
+        """Contains meta information about this class"""
+
+        model = models.User
+        fields = "__all__"
+
+    _meta = Meta()
+
     inlines = [UserContactInline]
 
     list_display = (
@@ -371,16 +379,16 @@ class UserAdmin(BaseUserAdmin):
 
         # If we aren't given a request to modify, we shouldn't try to
         if request is None or not hasattr(request, "GET"):
-            return queryset, use_distinct 
+            return queryset, use_distinct
-        
+
         # Otherwise, lets modify it!
         request_get = request.GET
 
         # The request defines model name and field name.
         # For instance, model_name could be "DomainApplication"
         # and field_name could be "investigator".
-        model_name = request_get.get('model_name', None)
+        model_name = request_get.get("model_name", None)
-        field_name = request_get.get('field_name', None)
+        field_name = request_get.get("field_name", None)
 
         # Make sure we're only modifying requests from these models.
         models_to_target = {"domainapplication"}
@@ -814,21 +822,15 @@ class DomainApplicationAdmin(ListHeaderAdmin):
             privileged_users = (
                 DomainApplication.objects.select_related("investigator")
                 .filter(investigator__is_staff=True)
-                .order_by(
+                .order_by("investigator__first_name", "investigator__last_name", "investigator__email")
-                    "investigator__first_name", 
-                    "investigator__last_name", 
-                    "investigator__email"
-                )
             )
 
             # Annotate the full name and return a values list that lookups can use
             privileged_users_annotated = privileged_users.annotate(
                 full_name=Coalesce(
-                    Concat(
+                    Concat("investigator__first_name", Value(" "), "investigator__last_name", output_field=CharField()),
-                        "investigator__first_name", Value(" "), "investigator__last_name", output_field=CharField()
-                    ),
                     "investigator__email",
-                    output_field=CharField()
+                    output_field=CharField(),
                 )
             ).values_list("investigator__id", "full_name")
 
@@ -932,24 +934,19 @@ class DomainApplicationAdmin(ListHeaderAdmin):
         "anything_else",
         "is_policy_acknowledged",
     ]
-    autocomplete_fields = ["approved_domain", "requested_domain", "submitter", "creator", "authorizing_official", "investigator"]
+    autocomplete_fields = [
+        "approved_domain",
+        "requested_domain",
+        "submitter",
+        "creator",
+        "authorizing_official",
+        "investigator",
+    ]
     filter_horizontal = ("current_websites", "alternative_domains", "other_contacts")
 
     # Table ordering
     ordering = ["requested_domain__name"]
 
-    # lists in filter_horizontal are not sorted properly, sort them
-    # by website
-    def formfield_for_manytomany(self, db_field, request, **kwargs):
-        logger.info(f"timing formfield_for_manytomany -> {db_field.name}")
-        with Timer() as t:
-            return super().formfield_for_manytomany(db_field, request, **kwargs)
-
-    def formfield_for_foreignkey(self, db_field, request, **kwargs):
-        logger.info(f"timing formfield_for_foreignkey -> {db_field.name}")
-        with Timer() as t:
-            return super().formfield_for_foreignkey(db_field, request, **kwargs)
-
     # Trigger action when a fieldset is changed
     def save_model(self, request, obj, form, change):
         if obj and obj.creator.status != models.User.RESTRICTED:

src/registrar/tests/common.py

@@ -231,6 +231,7 @@ class AuditedAdminMockData:
             first_name="{} first_name:{}".format(item_name, short_hand),
             last_name="{} last_name:{}".format(item_name, short_hand),
             username="{} username:{}".format(item_name + str(uuid.uuid4())[:8], short_hand),
+            is_staff=True,
         )[0]
         return user
 

src/registrar/tests/test_admin.py

@@ -9,7 +9,7 @@ from registrar.admin import (
     DomainApplicationAdminForm,
     DomainInvitationAdmin,
     ListHeaderAdmin,
-    MyUserAdmin,
+    UserAdmin,
     AuditedAdmin,
     ContactAdmin,
     DomainInformationAdmin,
@@ -941,8 +941,17 @@ class TestDomainApplicationAdmin(MockEppLib):
         investigator_field = DomainApplication._meta.get_field("investigator")
 
         # We should only be displaying staff users, in alphabetical order
-        expected_dropdown = list(User.objects.filter(is_staff=True))
+        sorted_fields = ["first_name", "last_name", "email"]
-        current_dropdown = list(self.admin.formfield_for_foreignkey(investigator_field, request).queryset)
+        expected_dropdown = list(User.objects.filter(is_staff=True).order_by(*sorted_fields))
+
+        # Grab the current dropdown. We do an API call to autocomplete to get this info.
+        application_queryset = self.admin.formfield_for_foreignkey(investigator_field, request).queryset
+        user_request = self.factory.post(
+            "/admin/autocomplete/?app_label=registrar&model_name=domainapplication&field_name=investigator"
+        )
+        user_admin = UserAdmin(User, self.site)
+        user_queryset = user_admin.get_search_results(user_request, application_queryset, None)[0]
+        current_dropdown = list(user_queryset)
 
         self.assertEqual(expected_dropdown, current_dropdown)
 
@@ -1335,10 +1344,10 @@ class ListHeaderAdminTest(TestCase):
         User.objects.all().delete()
 
 
-class MyUserAdminTest(TestCase):
+class UserAdminTest(TestCase):
     def setUp(self):
         admin_site = AdminSite()
-        self.admin = MyUserAdmin(model=get_user_model(), admin_site=admin_site)
+        self.admin = UserAdmin(model=get_user_model(), admin_site=admin_site)
 
     def test_list_display_without_username(self):
         request = self.client.request().wsgi_request
@@ -1360,7 +1369,7 @@ class MyUserAdminTest(TestCase):
         request = self.client.request().wsgi_request
         request.user = create_superuser()
         fieldsets = self.admin.get_fieldsets(request)
-        expected_fieldsets = super(MyUserAdmin, self.admin).get_fieldsets(request)
+        expected_fieldsets = super(UserAdmin, self.admin).get_fieldsets(request)
         self.assertEqual(fieldsets, expected_fieldsets)
 
     def test_get_fieldsets_cisa_analyst(self):
@@ -1396,11 +1405,46 @@ class AuditedAdminTest(TestCase):
 
         return ordered_list
 
+    def test_alphabetically_sorted_domain_application_investigator(self):
+        """Tests if the investigator field is alphabetically sorted by mimicking
+        the call event flow"""
+        # Creates multiple domain applications - review status does not matter
+        applications = multiple_unalphabetical_domain_objects("application")
+
+        # Create a mock request
+        application_request = self.factory.post(
+            "/admin/registrar/domainapplication/{}/change/".format(applications[0].pk)
+        )
+
+        # Get the formfield data from the application page
+        application_admin = AuditedAdmin(DomainApplication, self.site)
+        field = DomainApplication.investigator.field
+        application_queryset = application_admin.formfield_for_foreignkey(field, application_request).queryset
+
+        request = self.factory.post(
+            "/admin/autocomplete/?app_label=registrar&model_name=domainapplication&field_name=investigator"
+        )
+
+        sorted_fields = ["first_name", "last_name", "email"]
+        desired_sort_order = list(User.objects.filter(is_staff=True).order_by(*sorted_fields))
+
+        # Grab the data returned from get search results
+        admin = UserAdmin(User, self.site)
+        search_queryset = admin.get_search_results(request, application_queryset, None)[0]
+        current_sort_order = list(search_queryset)
+
+        self.assertEqual(
+            desired_sort_order,
+            current_sort_order,
+            "Investigator is not ordered alphabetically",
+        )
+
+    # This test case should be refactored in general, as it is too overly specific and engineered
     def test_alphabetically_sorted_fk_fields_domain_application(self):
         tested_fields = [
             DomainApplication.authorizing_official.field,
             DomainApplication.submitter.field,
-            DomainApplication.investigator.field,
+            # DomainApplication.investigator.field,
             DomainApplication.creator.field,
             DomainApplication.requested_domain.field,
         ]
@@ -1418,39 +1462,40 @@ class AuditedAdminTest(TestCase):
         # but both fields are of a fixed length.
         # For test case purposes, this should be performant.
         for field in tested_fields:
-            isNamefield: bool = field == DomainApplication.requested_domain.field
+            with self.subTest(field=field):
-            if isNamefield:
+                isNamefield: bool = field == DomainApplication.requested_domain.field
-                sorted_fields = ["name"]
+                if isNamefield:
-            else:
+                    sorted_fields = ["name"]
-                sorted_fields = ["first_name", "last_name"]
-            # We want both of these to be lists, as it is richer test wise.
-
-            desired_order = self.order_by_desired_field_helper(model_admin, request, field.name, *sorted_fields)
-            current_sort_order = list(model_admin.formfield_for_foreignkey(field, request).queryset)
-
-            # 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.
-            for contact in current_sort_order:
-                if not isNamefield:
-                    first = contact.first_name
-                    last = contact.last_name
                 else:
-                    first = contact.name
+                    sorted_fields = ["first_name", "last_name"]
-                    last = None
+                # We want both of these to be lists, as it is richer test wise.
 
-                name_tuple = self.coerced_fk_field_helper(first, last, field.name, ":")
+                desired_order = self.order_by_desired_field_helper(model_admin, request, field.name, *sorted_fields)
-                if name_tuple is not None:
+                current_sort_order = list(model_admin.formfield_for_foreignkey(field, request).queryset)
-                    current_sort_order_coerced_type.append(name_tuple)
 
-            self.assertEqual(
+                # Conforms to the same object structure as desired_order
-                desired_order,
+                current_sort_order_coerced_type = []
-                current_sort_order_coerced_type,
+
-                "{} is not ordered alphabetically".format(field.name),
+                # 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.
+                for contact in current_sort_order:
+                    if not isNamefield:
+                        first = contact.first_name
+                        last = contact.last_name
+                    else:
+                        first = contact.name
+                        last = None
+
+                    name_tuple = self.coerced_fk_field_helper(first, last, field.name, ":")
+                    if name_tuple is not None:
+                        current_sort_order_coerced_type.append(name_tuple)
+
+                self.assertEqual(
+                    desired_order,
+                    current_sort_order_coerced_type,
+                    "{} is not ordered alphabetically".format(field.name),
+                )
 
     def test_alphabetically_sorted_fk_fields_domain_information(self):
         tested_fields = [