Battle of the linter, circa 2023

src/registrar/admin.py

@@ -4,19 +4,31 @@ from django.contrib.auth.admin import UserAdmin as BaseUserAdmin
 from django.contrib.contenttypes.models import ContentType
 from django.http.response import HttpResponseRedirect
 from django.urls import reverse
-
+from registrar.models.utility.admin_form_order_helper import AdminFormOrderHelper, SortingDictInterface # noqa
-from registrar.models.utility.admin_form_order_helper import AdminFormOrderHelper, SortingDictInterface
 from . import models
 
 logger = logging.getLogger(__name__)
+# The linter does not like the length of SortingDictInterface, so these are split here
+audited_admin_item_names = ["submitter", "authorizing_official",
+                            "investigator", "creator", "user"]
+audited_admin_orderby_names = ['first_name', 'last_name']
 
+contact_admin_item_names = ["domain", "requested_domain"]
+contact_admin_orderby_names = ["name"]
 # Used to keep track of how we want to order_by certain FKs
 foreignkey_orderby_dict: [SortingDictInterface] = [
-    #foreign_key - order_by
+    # foreign_key - order_by
-    SortingDictInterface(["submitter", "authorizing_official", "investigator", "creator", "user"], ['first_name', 'last_name']).sorting_dict,
+    SortingDictInterface(
-    SortingDictInterface(["domain", "requested_domain"], ["name"]).sorting_dict
+                        audited_admin_item_names,
+                        audited_admin_orderby_names
+                        ).sorting_dict,
+    SortingDictInterface(
+                        contact_admin_item_names,
+                        contact_admin_orderby_names
+                        ).sorting_dict
 ]
 
+
 class AuditedAdmin(admin.ModelAdmin):
     """Custom admin to make auditing easier."""
 
@@ -32,13 +44,11 @@ class AuditedAdmin(admin.ModelAdmin):
 
     def formfield_for_foreignkey(self, db_field, request, **kwargs):
         """Used to sort dropdown fields alphabetically but can be expanded upon"""
-        form_field = super(AuditedAdmin, self).formfield_for_foreignkey(db_field, request, **kwargs)
+        form_field = super().formfield_for_foreignkey(db_field, request, **kwargs)
         return form_field_order_helper(form_field, db_field)
 
 
-
 class ListHeaderAdmin(AuditedAdmin):
-
     """Custom admin to add a descriptive subheader to list views."""
 
     def changelist_view(self, request, extra_context=None):
@@ -182,7 +192,7 @@ class ContactAdmin(ListHeaderAdmin):
 
     def formfield_for_foreignkey(self, db_field, request, **kwargs):
         """Used to sort dropdown fields alphabetically but can be expanded upon"""
-        form_field = super(ContactAdmin, self).formfield_for_foreignkey(db_field, request, **kwargs)
+        form_field = super().formfield_for_foreignkey(db_field, request, **kwargs)
         return form_field_order_helper(form_field, db_field)
 
 
@@ -320,10 +330,15 @@ class DomainApplicationAdmin(ListHeaderAdmin):
             # Regular users can only view the specified fields
             return self.readonly_fields
 
+
+# For readability purposes, but can be replaced with a one liner
 def form_field_order_helper(form_field, db_field):
+    """A shorthand for AdminFormOrderHelper(foreignkey_orderby_dict).get_ordered_form_field(form_field, db_field)""" # noqa
+
     form = AdminFormOrderHelper(foreignkey_orderby_dict)
     return form.get_ordered_form_field(form_field, db_field)
 
+
 admin.site.register(models.User, MyUserAdmin)
 admin.site.register(models.UserDomainRole, AuditedAdmin)
 admin.site.register(models.Contact, ContactAdmin)

src/registrar/models/utility/admin_form_order_helper.py

@@ -1,9 +1,13 @@
 import logging
+from typing import Dict
 from django.forms import ModelChoiceField
+
 logger = logging.getLogger(__name__)
+
+
 class SortingDictInterface:
-    _model_list = {}
+    _model_list: Dict[type, type] = {}
-    _sort_list = []
+    _sort_list: list[type] = []
     sorting_dict = {}
 
     def __init__(self, model_list, sort_list):
@@ -14,15 +18,16 @@ class SortingDictInterface:
 
 
 class AdminFormOrderHelper():
-    """A helper class to order a dropdown field in Django Admin, takes the fields you want to order by as an array"""
+    """A helper class to order a dropdown field in Django Admin, takes the fields you want to order by as an array""" # noqa
+
     # Used to keep track of how we want to order_by certain FKs
-    _sorting_dict: [SortingDictInterface] = []
+    _sorting_dict: [SortingDictInterface] = [...]
 
     def __init__(self, sort):
         self._sorting_dict = sort
 
     def get_ordered_form_field(self, form_field, db_field) -> (ModelChoiceField | None):
-        """Orders the queryset for a ModelChoiceField based on the order_by_dict dictionary"""
+        """Orders the queryset for a ModelChoiceField based on the order_by_dict dictionary""" # noqa
         _order_by_list = []
 
         for item in self._sorting_dict:
@@ -37,5 +42,3 @@ class AdminFormOrderHelper():
             form_field.queryset = form_field.queryset.order_by(*_order_by_list)
 
         return form_field
-    
-

src/registrar/tests/common.py

@@ -144,8 +144,8 @@ def completed_application(
     alt, _ = Website.objects.get_or_create(website="city1.gov")
     current, _ = Website.objects.get_or_create(website="city.com")
     you, _ = Contact.objects.get_or_create(
-        first_name="Testy you",
+        first_name="Testy2",
-        last_name="Tester you",
+        last_name="Tester2",
         title="Admin Tester",
         email="mayor@igorville.gov",
         phone="(555) 555 5556",
@@ -191,13 +191,16 @@ def completed_application(
 
     return application
 
-def multiple_completed_applications_for_alphabetical_test(has_other_contacts=True,
+
+def multiple_unalphabetical_applications(
+    has_other_contacts=True,
     has_current_website=True,
     has_alternative_gov_domain=True,
     has_type_of_work=True,
     has_anything_else=True,
     status=DomainApplication.STARTED,
-    user=False,):
+    user=False,
+):
     applications = []
     list_of_letters = list(ascii_uppercase)
     random.shuffle(list_of_letters)

src/registrar/tests/test_admin.py

@@ -1,9 +1,9 @@
 from django.test import TestCase, RequestFactory, Client
 from django.contrib.admin.sites import AdminSite
-from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin, MyUserAdmin, AuditedAdmin
+from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin, MyUserAdmin, AuditedAdmin # noqa
 from registrar.models import DomainApplication, DomainInformation, User
 from registrar.models.contact import Contact
-from .common import completed_application, mock_user, create_superuser, create_user, multiple_completed_applications_for_alphabetical_test
+from .common import completed_application, mock_user, create_superuser, create_user, multiple_unalphabetical_applications # noqa
 from django.contrib.auth import get_user_model
 
 from django.conf import settings
@@ -12,6 +12,8 @@ import boto3_mocking  # type: ignore
 import logging
 
 logger = logging.getLogger(__name__)
+
+
 class TestDomainApplicationAdmin(TestCase):
     def setUp(self):
         self.site = AdminSite()
@@ -370,6 +372,7 @@ class MyUserAdminTest(TestCase):
     def tearDown(self):
         User.objects.all().delete()
 
+
 class AuditedAdminTest(TestCase):
     def setUp(self):
         self.site = AdminSite()
@@ -377,10 +380,19 @@ class AuditedAdminTest(TestCase):
         self.client = Client(HTTP_HOST="localhost:8080")
 
     def test_alphabetically_sorted_fk_fields_domain_application(self):
-        tested_fields = [DomainApplication.authorizing_official.field, DomainApplication.submitter.field, DomainApplication.investigator.field, DomainApplication.creator.field]
+        tested_fields = [
+                        # field (0) - field shorthand (1)
+                        # the 'field shorthand' is used for type coercion.
+                        # It is only used here to reduce boilerplate,
+                        # but keep in mind that it is not needed outside this class.
+                        (DomainApplication.authorizing_official.field, 'ao'),
+                        (DomainApplication.submitter.field, 'you'),
+                        (DomainApplication.investigator.field, 'inv'),
+                        (DomainApplication.creator.field, 'cre')
+                        ]
 
         # Create a sample application - review status does not matter
-        applications = multiple_completed_applications_for_alphabetical_test(status=DomainApplication.IN_REVIEW)
+        applications = multiple_unalphabetical_applications()
 
         # Create a mock request
         request = self.factory.post(
@@ -389,47 +401,49 @@ class AuditedAdminTest(TestCase):
 
         model_admin = AuditedAdmin(DomainApplication, self.site)
 
-        # Typically we wouldnt want two nested for fields, but both fields are of a fixed length.
+        # Typically we wouldn't want two nested for fields,
+        # but both fields are of a fixed length.
         # For test case purposes, this should be performant.
         for field in tested_fields:
-            first_name_field = "{}__first_name".format(field.name)
+            field_name = field[0].name
-            last_name_field = "{}__last_name".format(field.name)
+            first_name_field = "{}__first_name".format(field_name)
+            last_name_field = "{}__last_name".format(field_name)
 
-            desired_order = list(model_admin.get_queryset(request).order_by(first_name_field, last_name_field).values_list(first_name_field, last_name_field))
+            # We want both of these to be lists, as it is richer test wise.
-            current_sort_order: Contact = list(model_admin.formfield_for_foreignkey(field, request).queryset)
+            # Not really a fan of how this looks, but as the linter demands...
+            desired_order = list(
+                model_admin.get_queryset(request).order_by(
+                                        first_name_field, last_name_field).values_list(
+                                        first_name_field, last_name_field)
+                )
+            current_sort_order: Contact = list(
+                model_admin.formfield_for_foreignkey(field[0], 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.
+            # 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:
-                first_name = contact.first_name
+                first = contact.first_name
-                last_name = contact.last_name
+                last = contact.last_name
 
-                match field.name:
+                name_tuple = self.coerced_fk_field_helper(first, last, field[1], ':')
-                    case DomainApplication.authorizing_official.field.name:
-                        name_tuple = self.coerced_fk_field_helper(first_name, last_name, 'ao', ':')
                 if name_tuple:
-                            current_sort_order_coerced_type.append((first_name, last_name))
+                    current_sort_order_coerced_type.append((first, last))
-                    case DomainApplication.submitter.field.name:
-                        name_tuple = self.coerced_fk_field_helper(first_name, last_name, 'you', ':')
-                        if name_tuple:
-                            current_sort_order_coerced_type.append((first_name, last_name))
-                    case DomainApplication.investigator.field.name:
-                        name_tuple = self.coerced_fk_field_helper(first_name, last_name, 'inv', ':')
-                        if name_tuple:
-                            current_sort_order_coerced_type.append((first_name, last_name))
-                    case DomainApplication.creator.field.name:
-                        name_tuple = self.coerced_fk_field_helper(first_name, last_name, 'cre', ':')
-                        if name_tuple:
-                            current_sort_order_coerced_type.append((first_name, last_name))
 
-            self.assertEqual(desired_order, current_sort_order_coerced_type, "{} is not ordered alphabetically".format(field.name))
+            self.assertEqual(desired_order,
+                             current_sort_order_coerced_type,
+                             "{} is not ordered alphabetically".format(field_name))
 
-    # I originally spent some time trying to fully generalize this to replace the match/arg fields, 
+    # I originally spent some time trying to fully
-    # but I think for this specific use-case its not necessary since it'll only be used here
+    # generalize this to replace the match/arg fields,
-    def coerced_fk_field_helper(self, first_name, last_name, field_name, queryset_shorthand):
+    # but I think for this specific use case
-        if(first_name and first_name.split(queryset_shorthand)[1] == field_name):
+    # its not necessary since it'll only be used here.
+    def coerced_fk_field_helper(self, first_name, last_name, field_name, queryset_shorthand): # noqa
+        if(first_name and first_name.split(queryset_shorthand)[1] == field_name): # noqa
             return (first_name, last_name)
         else:
             return None