diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 124553c31..d19231755 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -13,8 +13,8 @@ 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"] +special_audited_admin_item_names = ["domain", "requested_domain"] +special_audited_admin_orderby_names = ["name"] # Used to keep track of how we want to order_by certain FKs foreignkey_orderby_dict: list[SortingDictInterface] = [ # foreign_key - order_by @@ -22,9 +22,10 @@ foreignkey_orderby_dict: list[SortingDictInterface] = [ audited_admin_item_names, audited_admin_orderby_names ).sorting_dict, + # Handles fields that are sorted by 'name' SortingDictInterface( - contact_admin_item_names, - contact_admin_orderby_names + special_audited_admin_item_names, + special_audited_admin_orderby_names ).sorting_dict ] @@ -186,15 +187,9 @@ class DomainAdmin(ListHeaderAdmin): class ContactAdmin(ListHeaderAdmin): """Custom contact admin class to add search.""" - search_fields = ["email", "first_name", "last_name"] search_help_text = "Search by firstname, lastname or email." - 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 form_field_order_helper(form_field, db_field) - class DomainApplicationAdmin(ListHeaderAdmin): @@ -333,7 +328,8 @@ class DomainApplicationAdmin(ListHeaderAdmin): # 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 + """A shorthand for AdminFormOrderHelper(foreignkey_orderby_dict) + .get_ordered_form_field(form_field, db_field)""" form = AdminFormOrderHelper(foreignkey_orderby_dict) return form.get_ordered_form_field(form_field, db_field) diff --git a/src/registrar/models/utility/admin_form_order_helper.py b/src/registrar/models/utility/admin_form_order_helper.py index 8957262b3..ff32ead93 100644 --- a/src/registrar/models/utility/admin_form_order_helper.py +++ b/src/registrar/models/utility/admin_form_order_helper.py @@ -18,16 +18,18 @@ 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""" # noqa + """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_dict: list[SortingDictInterface] = [] # noqa + _sorting_dict: list[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""" # noqa + """Orders the queryset for a ModelChoiceField + based on the order_by_dict dictionary""" _order_by_list = [] for item in self._sorting_dict: @@ -41,7 +43,7 @@ class AdminFormOrderHelper(): break # Only order if we choose to do so - if not _order_by_list is None: + if _order_by_list is not None: form_field.queryset = form_field.queryset.order_by(*_order_by_list) return form_field diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index ef4428177..b92fcf351 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -10,14 +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, DomainInvitation, User -import logging -from registrar.models.domain import Domain - -from registrar.models.domain_information import DomainInformation +from registrar.models import Contact, DraftDomain, Website, DomainApplication +# For the linter +from registrar.models import DomainInvitation, User, DomainInformation, Domain logger = logging.getLogger(__name__) + def get_handlers(): """Obtain pointers to all StreamHandlers.""" handlers = {} @@ -103,14 +102,16 @@ class AuditedAdminMockData: APPLICATION = "application" INVITATION = "invitation" - # These all can likely be generalized more if necessary, particulary with shorthands. + # 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 + # 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""" @@ -132,23 +133,31 @@ class AuditedAdminMockData: )[0] return contact - def dummy_draft_domain(self,item_name): + 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): + 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] + 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"): + 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, @@ -160,38 +169,52 @@ class AuditedAdminMockData: 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'), + 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"): + 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) + 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, + 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_list = dict( **common_args, - domain = self.dummy_domain(item_name), - domain_application = self.create_full_dummy_domain_application(item_name) + domain=self.dummy_domain(item_name), + domain_application=domain_app ) case self.INVITATION: full_arg_list = dict( - email = "test_mail@mail.com", - domain = self.dummy_domain(item_name), - status = DomainInvitation.INVITED + email="test_mail@mail.com", + domain=self.dummy_domain(item_name), + status=DomainInvitation.INVITED ) return full_arg_list @@ -201,8 +224,14 @@ class AuditedAdminMockData: 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] + 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( @@ -211,8 +240,14 @@ class AuditedAdminMockData: 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] + 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( @@ -221,8 +256,14 @@ class AuditedAdminMockData: 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] + domain_application_kwargs = self.dummy_kwarg_boilerplate( + self.INVITATION, + object_name, + status + ) + application = DomainInvitation.objects.get_or_create( + **domain_application_kwargs + )[0] return application @@ -239,11 +280,17 @@ class AuditedAdminMockData: application = None match domain_type: case self.APPLICATION: - application = self.create_full_dummy_domain_application(object_name, status) + application = self.create_full_dummy_domain_application( + object_name, status + ) case self.INVITATION: - application = self.create_full_dummy_domain_invitation(object_name, status) + application = self.create_full_dummy_domain_invitation( + object_name, status + ) case self.INFORMATION: - application = self.create_full_dummy_domain_information(object_name, status) + application = self.create_full_dummy_domain_information( + object_name, status + ) case _: raise ValueError("Invalid domain_type, must conform to given constants") @@ -259,6 +306,7 @@ class AuditedAdminMockData: return application + def mock_user(): """A simple user.""" user_kwargs = dict( @@ -361,7 +409,9 @@ def completed_application( return application -def multiple_unalphabetical_domain_objects(domain_type = AuditedAdminMockData.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) @@ -372,4 +422,3 @@ def multiple_unalphabetical_domain_objects(domain_type = AuditedAdminMockData.AP 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 490442f00..cd66c9bbd 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -384,7 +384,13 @@ 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): + 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)) @@ -408,7 +414,7 @@ class AuditedAdminTest(TestCase): DomainApplication.creator.field, ] - # Create a sample application - review status does not matter + # Creates multiple domain applications - review status does not matter applications = multiple_unalphabetical_domain_objects("application") # Create a mock request @@ -424,7 +430,13 @@ class AuditedAdminTest(TestCase): for field in tested_fields: # 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, "first_name", "last_name") + desired_order = self.order_by_desired_field_helper( + model_admin, + request, + field.name, + "first_name", + "last_name" + ) current_sort_order: Contact = list( model_admin.formfield_for_foreignkey(field, request).queryset ) @@ -443,9 +455,11 @@ class AuditedAdminTest(TestCase): 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)) + 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 = [ @@ -455,7 +469,7 @@ class AuditedAdminTest(TestCase): DomainInformation.creator.field ] - # Create a sample application - review status does not matter + # Creates multiple domain applications - review status does not matter applications = multiple_unalphabetical_domain_objects("information") # Create a mock request @@ -471,13 +485,18 @@ class AuditedAdminTest(TestCase): # For test case purposes, this should be performant. for field in tested_fields: isNamefield: bool = (field == DomainInformation.domain.field) - if(isNamefield): + 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) + 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 ) @@ -497,17 +516,19 @@ class AuditedAdminTest(TestCase): last = None name_tuple = self.coerced_fk_field_helper(first, last, field.name, ':') - if not name_tuple is None: + 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)) + 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 + # Creates multiple domain applications - review status does not matter applications = multiple_unalphabetical_domain_objects("invitation") # Create a mock request @@ -525,7 +546,12 @@ class AuditedAdminTest(TestCase): 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) + 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 ) @@ -541,20 +567,31 @@ class AuditedAdminTest(TestCase): last = None name_tuple = self.coerced_fk_field_helper(first, last, field.name, ':') - if not name_tuple is None: + 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)) + 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 + ): + if first_name is None: + raise ValueError('Invalid value for first_name, must be defined') - 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 + if(first_name.split(queryset_shorthand)[1] == field_name): return returned_tuple else: return None