diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 96b8aaa33..e38393a5a 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -4,13 +4,13 @@ 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_sort_fields import AdminSortFields from . import models logger = logging.getLogger(__name__) -class AuditedAdmin(admin.ModelAdmin): - +class AuditedAdmin(admin.ModelAdmin, AdminSortFields): """Custom admin to make auditing easier.""" def history_view(self, request, object_id, extra_context=None): @@ -23,9 +23,13 @@ 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().formfield_for_foreignkey(db_field, request, **kwargs) + return self.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): @@ -162,7 +166,6 @@ class DomainAdmin(ListHeaderAdmin): class ContactAdmin(ListHeaderAdmin): - """Custom contact admin class to add search.""" search_fields = ["email", "first_name", "last_name"] diff --git a/src/registrar/models/utility/admin_form_order_helper.py b/src/registrar/models/utility/admin_form_order_helper.py new file mode 100644 index 000000000..acc26db11 --- /dev/null +++ b/src/registrar/models/utility/admin_form_order_helper.py @@ -0,0 +1,63 @@ +import logging +from typing import Dict +from django.forms import ModelChoiceField + +logger = logging.getLogger(__name__) + + +class SortingDict: + """Stores a sorting dictionary object""" + + _sorting_dict: Dict[type, type] = {} + + def __init__(self, model_list, sort_list): + self._sorting_dict = { + "dropDownSelected": self.convert_list_to_dict(model_list), + "sortBy": sort_list, + } + + # Used in __init__ for model_list for performance reasons + def convert_list_to_dict(self, value_list): + """Used internally to convert model_list to a dictionary""" + return {item: item for item in value_list} + + def get_dict(self): + """Grabs the associated dictionary item, + has two fields: 'dropDownSelected': model_list and 'sortBy': sort_list""" + # This should never happen so we need to log this + if self._sorting_dict is None: + raise ValueError("_sorting_dict was None") + return self._sorting_dict + + +class AdminFormOrderHelper: + """A helper class to order a dropdown field in Django Admin, + takes the fields you want to order by as an array""" + + # Used to keep track of how we want to order_by certain FKs + _sorting_list: list[SortingDict] = [] + + def __init__(self, sort: list[SortingDict]): + self._sorting_list = 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""" + _order_by_list = [] + + for item in self._sorting_list: + item_dict = item.get_dict() + drop_down_selected = item_dict.get("dropDownSelected") + sort_by = item_dict.get("sortBy") + + if db_field.name in drop_down_selected: + _order_by_list = sort_by + # Exit loop when order_by_list is found + break + + # Only order if we choose to do so + # noqa for the linter... reduces readability otherwise + if _order_by_list is not None and _order_by_list != []: # noqa + form_field.queryset = form_field.queryset.order_by(*_order_by_list) + + return form_field diff --git a/src/registrar/models/utility/admin_sort_fields.py b/src/registrar/models/utility/admin_sort_fields.py new file mode 100644 index 000000000..8037c6df0 --- /dev/null +++ b/src/registrar/models/utility/admin_sort_fields.py @@ -0,0 +1,27 @@ +from registrar.models.utility.admin_form_order_helper import ( + AdminFormOrderHelper, + SortingDict, +) + + +class AdminSortFields: + # Used to keep track of how we want to order_by certain FKs + foreignkey_orderby_dict: list[SortingDict] = [ + # foreign_key - order_by + # Handles fields that are sorted by 'first_name / last_name + SortingDict( + ["submitter", "authorizing_official", "investigator", "creator", "user"], + ["first_name", "last_name"], + ), + # Handles fields that are sorted by 'name' + SortingDict(["domain", "requested_domain"], ["name"]), + SortingDict(["domain_application"], ["requested_domain__name"]), + ] + + # For readability purposes, but can be replaced with a one liner + def form_field_order_helper(self, form_field, db_field): + """A shorthand for AdminFormOrderHelper(foreignkey_orderby_dict) + .get_ordered_form_field(form_field, db_field)""" + + form = AdminFormOrderHelper(self.foreignkey_orderby_dict) + return form.get_ordered_form_field(form_field, db_field) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 4359fc454..c4a2772b0 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -2,13 +2,26 @@ import os import logging from contextlib import contextmanager +import random +from string import ascii_uppercase from unittest.mock import Mock 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, + DomainInformation, + Domain, +) + +logger = logging.getLogger(__name__) def get_handlers(): @@ -88,6 +101,308 @@ class MockSESClient(Mock): self.EMAILS_SENT.append({"args": args, "kwargs": kwargs}) +class AuditedAdminMockData: + """Creates simple data mocks for AuditedAdminTest. + Can likely be more generalized, but the primary purpose of this class is to simplify + mock data creation, especially for lists of items, + by making the assumption that for most use cases we don't have to worry about + data 'accuracy' ('testy 2' is not an accurate first_name for example), we just care about + implementing some kind of patterning, especially with lists of items. + + Two variables are used across multiple functions: + + *item_name* - Used in patterning. Will be appended en masse to multiple str fields, + like first_name. For example, item_name 'egg' will return a user object of: + + first_name: 'egg first_name:user', + last_name: 'egg last_name:user', + username: 'egg username:user' + + where 'user' is the short_hand + + *short_hand* - Used in patterning. Certain fields will have ':{shorthand}' appended to it, + as a way to optionally include metadata in the str itself. Can be further expanded on. + Came from a bug where different querysets used in testing would effectively be 'anonymized', wherein + it would only display a list of types, but not include the variable name. + """ # noqa + + # Constants for different domain object types + INFORMATION = "information" + APPLICATION = "application" + INVITATION = "invitation" + + def dummy_user(self, item_name, short_hand): + """Creates a dummy user object, + but with a shorthand and support for multiple""" + user = User.objects.get_or_create( + first_name="{} first_name:{}".format(item_name, short_hand), + last_name="{} last_name:{}".format(item_name, short_hand), + username="{} username:{}".format(item_name, short_hand), + )[0] + return user + + def dummy_contact(self, item_name, short_hand): + """Creates a dummy contact object""" + contact = Contact.objects.get_or_create( + first_name="{} first_name:{}".format(item_name, short_hand), + last_name="{} last_name:{}".format(item_name, short_hand), + title="{} title:{}".format(item_name, short_hand), + email="{}testy@town.com".format(item_name), + phone="(555) 555 5555", + )[0] + return contact + + def dummy_draft_domain(self, item_name, prebuilt=False): + """ + Creates a dummy DraftDomain object + Args: + item_name (str): Value for 'name' in a DraftDomain object. + prebuilt (boolean): Determines return type. + Returns: + DraftDomain: Where name = 'item_name'. If prebuilt = True, then + name will be "city{}.gov".format(item_name). + """ + if prebuilt: + item_name = "city{}.gov".format(item_name) + return DraftDomain.objects.get_or_create(name=item_name)[0] + + def dummy_domain(self, item_name, prebuilt=False): + """ + Creates a dummy domain object + Args: + item_name (str): Value for 'name' in a Domain object. + prebuilt (boolean): Determines return type. + Returns: + Domain: Where name = 'item_name'. If prebuilt = True, then + domain name will be "city{}.gov".format(item_name). + """ + if prebuilt: + item_name = "city{}.gov".format(item_name) + return Domain.objects.get_or_create(name=item_name)[0] + + def dummy_website(self, item_name): + """ + Creates a dummy website object + Args: + item_name (str): Value for 'website' in a Website object. + Returns: + Website: Where website = 'item_name'. + """ + return Website.objects.get_or_create(website=item_name)[0] + + def dummy_alt(self, item_name): + """ + Creates a dummy website object for alternates + Args: + item_name (str): Value for 'website' in a Website object. + Returns: + Website: Where website = "cityalt{}.gov".format(item_name). + """ + return self.dummy_website(item_name="cityalt{}.gov".format(item_name)) + + def dummy_current(self, item_name): + """ + Creates a dummy website object for current + Args: + item_name (str): Value for 'website' in a Website object. + prebuilt (boolean): Determines return type. + Returns: + Website: Where website = "city{}.gov".format(item_name) + """ + return self.dummy_website(item_name="city{}.com".format(item_name)) + + def get_common_domain_arg_dictionary( + self, + item_name, + org_type="federal", + federal_type="executive", + purpose="Purpose of the site", + ): + """ + Generates a generic argument dict for most domains + Args: + item_name (str): A shared str value appended to first_name, last_name, + organization_name, address_line1, address_line2, + title, email, and username. + + org_type (str - optional): Sets a domains org_type + + federal_type (str - optional): Sets a domains federal_type + + purpose (str - optional): Sets a domains purpose + Returns: + Dictionary: { + organization_type: str, + federal_type: str, + purpose: str, + organization_name: str = "{} organization".format(item_name), + address_line1: str = "{} address_line1".format(item_name), + address_line2: str = "{} address_line2".format(item_name), + is_policy_acknowledged: boolean = True, + state_territory: str = "NY", + zipcode: str = "10002", + type_of_work: str = "e-Government", + anything_else: str = "There is more", + authorizing_official: Contact = self.dummy_contact(item_name, "authorizing_official"), + submitter: Contact = self.dummy_contact(item_name, "submitter"), + creator: User = self.dummy_user(item_name, "creator"), + } + """ # noqa + common_args = dict( + organization_type=org_type, + federal_type=federal_type, + purpose=purpose, + organization_name="{} organization".format(item_name), + address_line1="{} address_line1".format(item_name), + address_line2="{} address_line2".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 + + def dummy_kwarg_boilerplate( + self, + domain_type, + item_name, + status=DomainApplication.STARTED, + org_type="federal", + federal_type="executive", + purpose="Purpose of the site", + ): + """ + Returns a prebuilt kwarg dictionary for DomainApplication, + DomainInformation, or DomainInvitation. + Args: + domain_type (str): is either 'application', 'information', + or 'invitation'. + + item_name (str): A shared str value appended to first_name, last_name, + organization_name, address_line1, address_line2, + title, email, and username. + + status (str - optional): Defines the status for DomainApplication, + e.g. DomainApplication.STARTED + + org_type (str - optional): Sets a domains org_type + + federal_type (str - optional): Sets a domains federal_type + + purpose (str - optional): Sets a domains purpose + Returns: + dict: Returns a dictionary structurally consistent with the expected input + of either DomainApplication, DomainInvitation, or DomainInformation + based on the 'domain_type' field. + """ # noqa + common_args = self.get_common_domain_arg_dictionary( + item_name, org_type, federal_type, purpose + ) + full_arg_dict = None + match domain_type: + case self.APPLICATION: + full_arg_dict = dict( + **common_args, + requested_domain=self.dummy_draft_domain(item_name), + investigator=self.dummy_user(item_name, "investigator"), + status=status, + ) + case self.INFORMATION: + domain_app = self.create_full_dummy_domain_application(item_name) + full_arg_dict = dict( + **common_args, + domain=self.dummy_domain(item_name, True), + domain_application=domain_app, + ) + case self.INVITATION: + full_arg_dict = dict( + email="test_mail@mail.com", + domain=self.dummy_domain(item_name, True), + status=DomainInvitation.INVITED, + ) + return full_arg_dict + + def create_full_dummy_domain_application( + self, item_name, status=DomainApplication.STARTED + ): + """Creates a dummy domain application object""" + domain_application_kwargs = self.dummy_kwarg_boilerplate( + self.APPLICATION, item_name, status + ) + application = DomainApplication.objects.get_or_create( + **domain_application_kwargs + )[0] + return application + + def create_full_dummy_domain_information( + self, item_name, status=DomainApplication.STARTED + ): + """Creates a dummy domain information object""" + domain_application_kwargs = self.dummy_kwarg_boilerplate( + self.INFORMATION, item_name, status + ) + application = DomainInformation.objects.get_or_create( + **domain_application_kwargs + )[0] + return application + + def create_full_dummy_domain_invitation( + self, item_name, status=DomainApplication.STARTED + ): + """Creates a dummy domain invitation object""" + domain_application_kwargs = self.dummy_kwarg_boilerplate( + self.INVITATION, item_name, status + ) + application = DomainInvitation.objects.get_or_create( + **domain_application_kwargs + )[0] + + return application + + def create_full_dummy_domain_object( + self, + domain_type, + item_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( + item_name, status + ) + case self.INVITATION: + application = self.create_full_dummy_domain_invitation( + item_name, status + ) + case self.INFORMATION: + application = self.create_full_dummy_domain_information( + item_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(item_name, "other") + application.other_contacts.add(other) + if has_current_website and domain_type == self.APPLICATION: + current = self.dummy_current(item_name) + application.current_websites.add(current) + if has_alternative_gov_domain and domain_type == self.APPLICATION: + alt = self.dummy_alt(item_name) + application.alternative_domains.add(alt) + + return application + + def mock_user(): """A simple user.""" user_kwargs = dict( @@ -142,15 +457,15 @@ 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", - last_name="Tester you", + first_name="Testy2", + last_name="Tester2", title="Admin Tester", email="mayor@igorville.gov", phone="(555) 555 5556", ) other, _ = Contact.objects.get_or_create( - first_name="Testy2", - last_name="Tester2", + first_name="Testy", + last_name="Tester", title="Another Tester", email="testy2@town.com", phone="(555) 555 5557", @@ -188,3 +503,18 @@ def completed_application( application.alternative_domains.add(alt) return application + + +def multiple_unalphabetical_domain_objects( + domain_type=AuditedAdminMockData.APPLICATION, +): + """Returns a list of generic domain objects for testing purposes""" + applications = [] + list_of_letters = list(ascii_uppercase) + random.shuffle(list_of_letters) + + mock = AuditedAdminMockData() + for object_name in list_of_letters: + application = mock.create_full_dummy_domain_object(domain_type, object_name) + applications.append(application) + return applications diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 5f78eac3c..a27dcb741 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1,13 +1,34 @@ from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite -from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin, MyUserAdmin -from registrar.models import DomainApplication, DomainInformation, User -from .common import completed_application, mock_user, create_superuser, create_user + +from registrar.admin import ( + DomainApplicationAdmin, + ListHeaderAdmin, + MyUserAdmin, + AuditedAdmin, +) +from registrar.models import ( + DomainApplication, + DomainInformation, + User, + DomainInvitation, +) +from .common import ( + completed_application, + mock_user, + create_superuser, + create_user, + multiple_unalphabetical_domain_objects, +) + from django.contrib.auth import get_user_model from django.conf import settings from unittest.mock import MagicMock import boto3_mocking # type: ignore +import logging + +logger = logging.getLogger(__name__) class TestDomainApplicationAdmin(TestCase): @@ -394,3 +415,224 @@ class MyUserAdminTest(TestCase): def tearDown(self): User.objects.all().delete() + + +class AuditedAdminTest(TestCase): + def setUp(self): + self.site = AdminSite() + 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)) + + ordered_list = list( + obj_to_sort.get_queryset(request) + .order_by(*formatted_sort_fields) + .values_list(*formatted_sort_fields) + ) + + return ordered_list + + def test_alphabetically_sorted_fk_fields_domain_application(self): + tested_fields = [ + DomainApplication.authorizing_official.field, + DomainApplication.submitter.field, + DomainApplication.investigator.field, + DomainApplication.creator.field, + DomainApplication.requested_domain.field, + ] + + # Creates multiple domain applications - review status does not matter + applications = multiple_unalphabetical_domain_objects("application") + + # Create a mock request + request = self.factory.post( + "/admin/registrar/domainapplication/{}/change/".format(applications[0].pk) + ) + + model_admin = AuditedAdmin(DomainApplication, 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 == DomainApplication.requested_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 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 = [ + DomainInformation.authorizing_official.field, + DomainInformation.submitter.field, + DomainInformation.creator.field, + (DomainInformation.domain.field, ["name"]), + (DomainInformation.domain_application.field, ["requested_domain__name"]), + ] + # Creates multiple domain applications - 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: + isOtherOrderfield: bool = isinstance(field, tuple) + field_obj = None + if isOtherOrderfield: + sorted_fields = field[1] + field_obj = field[0] + else: + sorted_fields = ["first_name", "last_name"] + field_obj = field + # 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_obj.name, *sorted_fields + ) + current_sort_order = list( + model_admin.formfield_for_foreignkey(field_obj, 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 obj in current_sort_order: + last = None + if not isOtherOrderfield: + first = obj.first_name + last = obj.last_name + elif field_obj == DomainInformation.domain.field: + first = obj.name + elif field_obj == DomainInformation.domain_application.field: + first = obj.requested_domain.name + + name_tuple = self.coerced_fk_field_helper( + first, last, field_obj.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_obj.name), + ) + + def test_alphabetically_sorted_fk_fields_domain_invitation(self): + tested_fields = [DomainInvitation.domain.field] + + # Creates multiple domain applications - 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 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 coerced_fk_field_helper( + self, first_name, last_name, field_name, queryset_shorthand + ): + """Handles edge cases for test cases""" + if first_name is None: + raise ValueError("Invalid value for first_name, must be defined") + + returned_tuple = (first_name, last_name) + # Handles edge case for names - structured strangely + if last_name is None: + return (first_name,) + + if first_name.split(queryset_shorthand)[1] == field_name: + return returned_tuple + else: + return None + + def tearDown(self): + DomainInformation.objects.all().delete() + DomainApplication.objects.all().delete() + DomainInvitation.objects.all().delete()