Updated test cases / common.py helpers

Expanded some test cases and added some common.py helpers for future tests

src/registrar/admin.py

@@ -16,7 +16,7 @@ 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] = [
+foreignkey_orderby_dict: list[SortingDictInterface] = [
     # foreign_key - order_by
     SortingDictInterface(
                         audited_admin_item_names,

src/registrar/models/utility/admin_form_order_helper.py

@@ -31,14 +31,17 @@ class AdminFormOrderHelper():
         _order_by_list = []
 
         for item in self._sorting_dict:
+            # Used to disable black as this is a false positive
+            # fmt: off
             drop_down_selected = item.get("dropDownSelected")
+            # fmt: on
             sort_by = item.get("sortBy")
             if db_field.name in drop_down_selected:
                 _order_by_list = sort_by
                 break
 
         # Only order if we choose to do so
-        if _order_by_list:
+        if not _order_by_list is None:
             form_field.queryset = form_field.queryset.order_by(*_order_by_list)
 
         return form_field

src/registrar/tests/common.py

@@ -10,8 +10,13 @@ from typing import List, Dict
 from django.conf import settings
 from django.contrib.auth import get_user_model, login
 
-from registrar.models import Contact, DraftDomain, Website, DomainApplication, User
+from registrar.models import Contact, DraftDomain, Website, DomainApplication, DomainInvitation, User
+import logging
+from registrar.models.domain import Domain
 
+from registrar.models.domain_information import DomainInformation
+
+logger = logging.getLogger(__name__)
 
 def get_handlers():
     """Obtain pointers to all StreamHandlers."""
@@ -90,6 +95,170 @@ class MockSESClient(Mock):
         self.EMAILS_SENT.append({"args": args, "kwargs": kwargs})
 
 
+class AuditedAdminMockData:
+    """Creates simple data mocks for AuditedAdminTest"""
+
+    # Constants for different domain object types
+    INFORMATION = "information"
+    APPLICATION = "application"
+    INVITATION = "invitation"
+
+    # These all can likely be generalized more if necessary, particulary with shorthands.
+    # These are kept basic for now.
+    # As for why we have shorthands to begin with:
+    # .queryset returns a list of all objects of the same type,
+    # rather than by seperating out those fields.
+    # For such scenarios, the shorthand allows us to not only id a field,
+    # but append additional information to it.
+    # This is useful for that scenario and outside it for identifying if values swapped unexpectedly
+    def dummy_user(self, item_name, shorthand):
+        """Creates a dummy user object,
+        but with a shorthand and support for multiple"""
+        user = User.objects.get_or_create(
+            first_name="{} First:{}".format(item_name, shorthand),
+            last_name="{} Last:{}".format(item_name, shorthand),
+            username="{} username:{}".format(item_name, shorthand)
+        )[0]
+        return user
+
+    def dummy_contact(self, item_name, shorthand):
+        """Creates a dummy contact object"""
+        contact = Contact.objects.get_or_create(
+            first_name="{} First:{}".format(item_name, shorthand),
+            last_name="{} Last:{}".format(item_name, shorthand),
+            title="{} title:{}".format(item_name, shorthand),
+            email="{}testy@town.com".format(item_name),
+            phone="(555) 555 5555"
+        )[0]
+        return contact
+
+    def dummy_draft_domain(self,item_name):
+        """Creates a dummy draft domain object"""
+        return DraftDomain.objects.get_or_create(name="city{}.gov".format(item_name))[0]
+
+    def dummy_domain(self,item_name):
+        """Creates a dummy domain object"""
+        return Domain.objects.get_or_create(name="city{}.gov".format(item_name))[0]
+
+    def dummy_alt(self, item_name):
+        """Creates a dummy website object for alternates"""
+        return Website.objects.get_or_create(website="cityalt{}.gov".format(item_name))[0]
+
+    def dummy_current(self, item_name):
+        """Creates a dummy website object for current"""
+        return Website.objects.get_or_create(website="city{}.com".format(item_name))[0]
+
+    def get_common_domain_arg_dictionary(self, item_name, org_type="federal", federal_type="executive", purpose="Purpose of the site"):
+        """Generates a generic argument list for most domains"""
+        common_args = dict(
+            organization_type=org_type,
+            federal_type=federal_type,
+            purpose=purpose,
+            organization_name="{} Testorg".format(item_name),
+            address_line1="{} address 1".format(item_name),
+            address_line2="{} address 2".format(item_name),
+            is_policy_acknowledged=True,
+            state_territory="NY",
+            zipcode="10002",
+            type_of_work = 'e-Government',
+            anything_else = "There is more",
+            authorizing_official = self.dummy_contact(item_name, 'authorizing_official'),
+            submitter = self.dummy_contact(item_name, 'submitter'),
+            creator = self.dummy_user(item_name, 'creator'),
+        )
+        return common_args
+
+    # This can be boiled down more, though for our purposes this is OK
+    def dummy_kwarg_boilerplate(self, domain_type, item_name, status, org_type="federal", federal_type="executive", purpose="Purpose of the site"):
+        """Returns kwargs for different domain object types"""
+        common_args = self.get_common_domain_arg_dictionary(item_name, org_type, federal_type, purpose)
+        full_arg_list = None
+        match domain_type:
+            case self.APPLICATION:
+                full_arg_list = dict(
+                    **common_args,
+                    requested_domain = self.dummy_draft_domain(item_name),
+                    investigator = self.dummy_user(item_name, 'investigator'),
+                    status = status,
+                )
+            case self.INFORMATION:
+                full_arg_list = dict(
+                    **common_args,
+                    domain = self.dummy_domain(item_name),
+                    domain_application = self.create_full_dummy_domain_application(item_name)
+                )
+            case self.INVITATION:
+                full_arg_list = dict(
+                    email = "test_mail@mail.com",
+                    domain = self.dummy_domain(item_name),
+                    status = DomainInvitation.INVITED
+                )
+        return full_arg_list
+
+    def create_full_dummy_domain_application(
+        self,
+        object_name,
+        status=DomainApplication.STARTED
+    ):
+        """Creates a dummy domain application object"""
+        domain_application_kwargs = self.dummy_kwarg_boilerplate(self.APPLICATION, object_name, status)
+        application = DomainApplication.objects.get_or_create(**domain_application_kwargs)[0]
+        return application
+
+    def create_full_dummy_domain_information(
+        self,
+        object_name,
+        status=DomainApplication.STARTED
+    ):
+        """Creates a dummy domain information object"""
+        domain_application_kwargs = self.dummy_kwarg_boilerplate(self.INFORMATION, object_name, status)
+        application = DomainInformation.objects.get_or_create(**domain_application_kwargs)[0]
+        return application
+
+    def create_full_dummy_domain_invitation(
+        self,
+        object_name,
+        status=DomainApplication.STARTED
+    ):
+        """Creates a dummy domain invitation object"""
+        domain_application_kwargs = self.dummy_kwarg_boilerplate(self.INVITATION, object_name, status)
+        application = DomainInvitation.objects.get_or_create(**domain_application_kwargs)[0]
+
+        return application
+
+    def create_full_dummy_domain_object(
+        self,
+        domain_type,
+        object_name,
+        has_other_contacts=True,
+        has_current_website=True,
+        has_alternative_gov_domain=True,
+        status=DomainApplication.STARTED
+    ):
+        """A helper to create a dummy domain application object"""
+        application = None
+        match domain_type:
+            case self.APPLICATION:
+                application = self.create_full_dummy_domain_application(object_name, status)
+            case self.INVITATION:
+                application = self.create_full_dummy_domain_invitation(object_name, status)
+            case self.INFORMATION:
+                application = self.create_full_dummy_domain_information(object_name, status)
+            case _:
+                raise ValueError("Invalid domain_type, must conform to given constants")
+
+        if has_other_contacts and domain_type != self.INVITATION:
+            other = self.dummy_contact(object_name, 'other')
+            application.other_contacts.add(other)
+        if has_current_website and domain_type == self.APPLICATION:
+            current = self.dummy_current(object_name)
+            application.current_websites.add(current)
+        if has_alternative_gov_domain and domain_type == self.APPLICATION:
+            alt = self.dummy_alt(object_name)
+            application.alternative_domains.add(alt)
+
+        return application
+
 def mock_user():
     """A simple user."""
     user_kwargs = dict(
@@ -192,84 +361,15 @@ def completed_application(
     return application
 
 
-def multiple_unalphabetical_applications(
+def multiple_unalphabetical_domain_objects(domain_type = AuditedAdminMockData.APPLICATION):
-    has_other_contacts=True,
+    """Returns a list of generic domain objects for testing purposes"""
-    has_current_website=True,
-    has_alternative_gov_domain=True,
-    has_type_of_work=True,
-    has_anything_else=True,
-    status=DomainApplication.STARTED,
-    user=False,
-):
     applications = []
     list_of_letters = list(ascii_uppercase)
     random.shuffle(list_of_letters)
-    for x in list_of_letters:
-        user = get_user_model().objects.create(
-            first_name="{} First:cre".format(x),
-            last_name="{} Last:cre".format(x),
-            username="{} username:cre".format(x)
-        )
-        ao, _ = Contact.objects.get_or_create(
-            first_name="{} First:ao".format(x),
-            last_name="{} Last:ao".format(x),
-            title="{} Chief Tester".format(x),
-            email="testy@town.com",
-            phone="(555) 555 5555",
-        )
-        domain, _ = DraftDomain.objects.get_or_create(name="city{}.gov".format(x))
-        alt, _ = Website.objects.get_or_create(website="cityalt{}.gov".format(x))
-        current, _ = Website.objects.get_or_create(website="city{}.com".format(x))
-        you, _ = Contact.objects.get_or_create(
-            first_name="{} First:you".format(x),
-            last_name="{} Last:you".format(x),
-            title="{} Admin Tester".format(x),
-            email="mayor@igorville.gov",
-            phone="(555) 555 5556",
-        )
-        other, _ = Contact.objects.get_or_create(
-            first_name="{} First:other".format(x),
-            last_name="{} Last:other".format(x),
-            title="{} Another Tester".format(x),
-            email="{}testy2@town.com".format(x),
-            phone="(555) 555 5557",
-        )
-        inv, _ = User.objects.get_or_create(
-            first_name="{} First:inv".format(x),
-            last_name="{} Last:inv".format(x),
-            username="{} username:inv".format(x)
-        )
-        domain_application_kwargs = dict(
-            organization_type="federal",
-            federal_type="executive",
-            purpose="Purpose of the site",
-            is_policy_acknowledged=True,
-            organization_name="{}Testorg".format(x),
-            address_line1="address 1",
-            address_line2="address 2",
-            state_territory="NY",
-            zipcode="10002",
-            authorizing_official=ao,
-            requested_domain=domain,
-            submitter=you,
-            creator=user,
-            status=status,
-            investigator=inv
-        )
-        if has_type_of_work:
-            domain_application_kwargs["type_of_work"] = "e-Government"
-        if has_anything_else:
-            domain_application_kwargs["anything_else"] = "There is more"
 
-        application, _ = DomainApplication.objects.get_or_create(
+    mock = AuditedAdminMockData()
-            **domain_application_kwargs
+    for object_name in list_of_letters:
-        )
+        application = mock.create_full_dummy_domain_object(domain_type, object_name)
-
-        if has_other_contacts:
-            application.other_contacts.add(other)
-        if has_current_website:
-            application.current_websites.add(current)
-        if has_alternative_gov_domain:
-            application.alternative_domains.add(alt)
         applications.append(application)
     return applications
+

src/registrar/tests/test_admin.py

@@ -1,9 +1,14 @@
 from django.test import TestCase, RequestFactory, Client
 from django.contrib.admin.sites import AdminSite
-from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin, MyUserAdmin, AuditedAdmin # noqa
+from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin
+# Need to split these up due to the linter
+from registrar.admin import MyUserAdmin, AuditedAdmin
 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_unalphabetical_applications # noqa
+from registrar.models.domain_invitation import DomainInvitation
+from .common import completed_application, mock_user, create_superuser, create_user
+# Need to split these up due to the linter
+from .common import multiple_unalphabetical_domain_objects
 from django.contrib.auth import get_user_model
 
 from django.conf import settings
@@ -379,20 +384,32 @@ class AuditedAdminTest(TestCase):
         self.factory = RequestFactory()
         self.client = Client(HTTP_HOST="localhost:8080")
 
+    def order_by_desired_field_helper(self, obj_to_sort: AuditedAdmin, request, field_name, *obj_names):
+        formatted_sort_fields = []
+        for obj in obj_names:
+            formatted_sort_fields.append("{}__{}".format(field_name, obj))
+
+        # Not really a fan of how this looks, but as the linter demands...
+        ordered_list = list(
+            obj_to_sort.get_queryset(request).order_by(
+                                    *formatted_sort_fields).values_list(
+                                    *formatted_sort_fields)
+            )
+
+        return ordered_list
+
+    # Q: These three tests can be generalized into an object,
+    # is it worth the time investment to do so?
     def test_alphabetically_sorted_fk_fields_domain_application(self):
         tested_fields = [
-                        # field (0) - field shorthand (1)
+                        DomainApplication.authorizing_official.field,
-                        # the 'field shorthand' is used for type coercion.
+                        DomainApplication.submitter.field,
-                        # It is only used here to reduce boilerplate,
+                        DomainApplication.investigator.field,
-                        # but keep in mind that it is not needed outside this class.
+                        DomainApplication.creator.field,
-                        (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_unalphabetical_applications()
+        applications = multiple_unalphabetical_domain_objects("application")
 
         # Create a mock request
         request = self.factory.post(
@@ -405,19 +422,11 @@ class AuditedAdminTest(TestCase):
         # but both fields are of a fixed length.
         # For test case purposes, this should be performant.
         for field in tested_fields:
-            field_name = field[0].name
-            first_name_field = "{}__first_name".format(field_name)
-            last_name_field = "{}__last_name".format(field_name)
 
             # We want both of these to be lists, as it is richer test wise.
-            # Not really a fan of how this looks, but as the linter demands...
+            desired_order = self.order_by_desired_field_helper(model_admin, request, field.name, "first_name", "last_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)
-                )
             current_sort_order: Contact = list(
-                model_admin.formfield_for_foreignkey(field[0], request).queryset
+                model_admin.formfield_for_foreignkey(field, request).queryset
                 )
 
             # Conforms to the same object structure as desired_order
@@ -430,24 +439,127 @@ class AuditedAdminTest(TestCase):
                 first = contact.first_name
                 last = contact.last_name
 
-                name_tuple = self.coerced_fk_field_helper(first, last, field[1], ':')
+                name_tuple = self.coerced_fk_field_helper(first, last, field.name, ':')
                 if name_tuple:
                     current_sort_order_coerced_type.append((first, last))
 
             self.assertEqual(desired_order,
                             current_sort_order_coerced_type,
-                             "{} is not ordered alphabetically".format(field_name))
+                            "{} is not ordered alphabetically".format(field.name))
+
+    def test_alphabetically_sorted_fk_fields_domain_information(self):
+        tested_fields = [
+                        DomainInformation.authorizing_official.field,
+                        DomainInformation.submitter.field,
+                        DomainInformation.domain.field,
+                        DomainInformation.creator.field
+                        ]
+
+        # Create a sample application - review status does not matter
+        applications = multiple_unalphabetical_domain_objects("information")
+
+        # Create a mock request
+        request = self.factory.post(
+            "/admin/registrar/domaininformation/{}/change/".format(applications[0].pk)
+        )
+
+        model_admin = AuditedAdmin(DomainInformation, self.site)
+
+        sorted_fields = []
+        # 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:
+            isNamefield: bool = (field == DomainInformation.domain.field)
+            if(isNamefield):
+                sorted_fields = ["name"]
+            else:
+                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
+                    last = None
+
+                name_tuple = self.coerced_fk_field_helper(first, last, field.name, ':')
+                if not name_tuple is 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_invitation(self):
+        tested_fields = [DomainInvitation.domain.field]
+
+        # Create a sample application - review status does not matter
+        applications = multiple_unalphabetical_domain_objects("invitation")
+
+        # Create a mock request
+        request = self.factory.post(
+            "/admin/registrar/domaininvitation/{}/change/".format(applications[0].pk)
+        )
+
+        model_admin = AuditedAdmin(DomainInvitation, self.site)
+
+        sorted_fields = []
+        # 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:
+            sorted_fields = ["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:
+                first = contact.name
+                last = None
+
+                name_tuple = self.coerced_fk_field_helper(first, last, field.name, ':')
+                if not name_tuple is 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))
 
-    # I originally spent some time trying to fully
-    # generalize this to replace the match/arg fields,
-    # but I think for this specific use case
-    # 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
+        returned_tuple = (first_name, last_name)
+        # Handles edge case for names - structured strangely
+        if last_name is None:
+            return (first_name,)
+
         if(first_name and first_name.split(queryset_shorthand)[1] == field_name): # noqa
-            return (first_name, last_name)
+            return returned_tuple
         else:
             return None
 
     def tearDown(self):
         DomainInformation.objects.all().delete()
         DomainApplication.objects.all().delete()
+        DomainInvitation.objects.all().delete()