From 95927753aa377e3baaad05d5a096baea1613250c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 8 Aug 2023 14:42:08 -0600 Subject: [PATCH 01/27] Progress save Needs some code cleanup --- src/registrar/admin.py | 23 ++++++++++++++++++++++ src/registrar/models/domain_information.py | 3 --- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 182543c19..ee5195fc7 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -22,6 +22,29 @@ class AuditedAdmin(admin.ModelAdmin): object_id=object_id, ) ) + + + def formfield_for_foreignkey(self, db_field, request, **kwargs): + """Used to sort dropdown fields alphabetically but can be expanded upon""" + # Determines what we want to sort by, ex: by name + order_by_list = [] + if db_field.name == "submitter" or db_field.name == "authorizing_official" or db_field.name == "creator" or db_field.name == "investigator": + order_by_list = ['first_name', 'last_name'] + elif db_field.name == "requested_domain": + order_by_list = ['name'] + + return self.formfield_order_helper(order_by_list, db_field, request, **kwargs) + + def formfield_order_helper(self, order_by_list, db_field, request, **kwargs): + """A helper function to order a dropdown field in Django Admin, takes the fields you want to order by as an array""" + formfield = super(AuditedAdmin, self).formfield_for_foreignkey(db_field, request, **kwargs) + # Only order if we choose to do so + if order_by_list: + formfield.queryset = formfield.queryset.order_by(*order_by_list) + + return formfield + + class ListHeaderAdmin(AuditedAdmin): diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index b12039e73..084bd0515 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -244,6 +244,3 @@ class DomainInformation(TimeStampedModel): domain_info.domain = domain domain_info.save() return domain_info - - class Meta: - verbose_name_plural = "Domain Information" From e2ae261cfa53c42bfa2bb4b4dd9ac17a67c104fb Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 9 Aug 2023 08:47:09 -0600 Subject: [PATCH 02/27] Formatting changes --- src/registrar/admin.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ee5195fc7..94d7a4108 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -22,17 +22,16 @@ class AuditedAdmin(admin.ModelAdmin): object_id=object_id, ) ) - def formfield_for_foreignkey(self, db_field, request, **kwargs): """Used to sort dropdown fields alphabetically but can be expanded upon""" # Determines what we want to sort by, ex: by name order_by_list = [] - if db_field.name == "submitter" or db_field.name == "authorizing_official" or db_field.name == "creator" or db_field.name == "investigator": + if db_field.name == "submitter" or db_field.name == "authorizing_official" or db_field.name == "creator": order_by_list = ['first_name', 'last_name'] elif db_field.name == "requested_domain": order_by_list = ['name'] - + return self.formfield_order_helper(order_by_list, db_field, request, **kwargs) def formfield_order_helper(self, order_by_list, db_field, request, **kwargs): @@ -42,9 +41,7 @@ class AuditedAdmin(admin.ModelAdmin): if order_by_list: formfield.queryset = formfield.queryset.order_by(*order_by_list) - return formfield - - + return formfield class ListHeaderAdmin(AuditedAdmin): From 6a04704cfb4064305f8e60bfedd7295781b0d3ce Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 10 Aug 2023 09:10:30 -0600 Subject: [PATCH 03/27] (Somewhat) Generalized logic and expanded it to all dropdowns --- src/registrar/admin.py | 55 +++++++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 17 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 94d7a4108..d2073c516 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -8,9 +8,20 @@ from . import models logger = logging.getLogger(__name__) +# Used to keep track of how we want to order_by certain FKs +foreignkey_orderby_dict = { + # foreign_key # order_by + "submitter" : ['first_name', 'last_name'], + "authorizing_official" : ['first_name', 'last_name'], + "investigator" : ['first_name', 'last_name'], + "creator" : ['first_name', 'last_name'], + "user" : ['first_name', 'last_name'], + "domain" : ['name'], + "requested_domain" : ['name'], + "domain_application" : ['id'], +} class AuditedAdmin(admin.ModelAdmin): - """Custom admin to make auditing easier.""" def history_view(self, request, object_id, extra_context=None): @@ -22,26 +33,19 @@ class AuditedAdmin(admin.ModelAdmin): object_id=object_id, ) ) - + def formfield_for_foreignkey(self, db_field, request, **kwargs): """Used to sort dropdown fields alphabetically but can be expanded upon""" - # Determines what we want to sort by, ex: by name order_by_list = [] - if db_field.name == "submitter" or db_field.name == "authorizing_official" or db_field.name == "creator": - order_by_list = ['first_name', 'last_name'] - elif db_field.name == "requested_domain": - order_by_list = ['name'] - return self.formfield_order_helper(order_by_list, db_field, request, **kwargs) + # Determines what we want to sort by, ex: by name + if db_field.name in foreignkey_orderby_dict: + order_by_list = foreignkey_orderby_dict.get(db_field.name) + + form_field = super(AuditedAdmin, self).formfield_for_foreignkey(db_field, request, **kwargs) + return formfield_order_helper(form_field, order_by_list) - def formfield_order_helper(self, order_by_list, db_field, request, **kwargs): - """A helper function to order a dropdown field in Django Admin, takes the fields you want to order by as an array""" - formfield = super(AuditedAdmin, self).formfield_for_foreignkey(db_field, request, **kwargs) - # Only order if we choose to do so - if order_by_list: - formfield.queryset = formfield.queryset.order_by(*order_by_list) - - return formfield + class ListHeaderAdmin(AuditedAdmin): @@ -182,12 +186,21 @@ 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""" + order_by_list = [] + # Determines what we want to sort by, ex: by name + if db_field.name in foreignkey_orderby_dict: + order_by_list = foreignkey_orderby_dict.get(db_field.name) + + form_field = super(ContactAdmin, self).formfield_for_foreignkey(db_field, request, **kwargs) + return formfield_order_helper(form_field, order_by_list) + class DomainApplicationAdmin(ListHeaderAdmin): @@ -324,6 +337,14 @@ class DomainApplicationAdmin(ListHeaderAdmin): return self.readonly_fields +def formfield_order_helper(form_field, order_by_list): + """A helper function to order a dropdown field in Django Admin, takes the fields you want to order by as an array""" + # Only order if we choose to do so + if order_by_list: + form_field.queryset = form_field.queryset.order_by(*order_by_list) + + return form_field + admin.site.register(models.User, MyUserAdmin) admin.site.register(models.UserDomainRole, AuditedAdmin) admin.site.register(models.Contact, ContactAdmin) From 94738ca06f100bd0b870d9070ef52a11f0302905 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 10 Aug 2023 14:56:16 -0600 Subject: [PATCH 04/27] Moved sort logic in a class / generalized --- src/registrar/admin.py | 46 ++++++------------- .../models/utility/admin_form_order_helper.py | 41 +++++++++++++++++ 2 files changed, 54 insertions(+), 33 deletions(-) create mode 100644 src/registrar/models/utility/admin_form_order_helper.py diff --git a/src/registrar/admin.py b/src/registrar/admin.py index d2073c516..3d568f441 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -4,22 +4,19 @@ 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 from . import models logger = logging.getLogger(__name__) # Used to keep track of how we want to order_by certain FKs -foreignkey_orderby_dict = { - # foreign_key # order_by - "submitter" : ['first_name', 'last_name'], - "authorizing_official" : ['first_name', 'last_name'], - "investigator" : ['first_name', 'last_name'], - "creator" : ['first_name', 'last_name'], - "user" : ['first_name', 'last_name'], - "domain" : ['name'], - "requested_domain" : ['name'], - "domain_application" : ['id'], -} +foreignkey_orderby_dict: [SortingDictInterface] = [ + #foreign_key - order_by + SortingDictInterface(["submitter", "authorizing_official", "investigator", "creator", "user"], ['first_name', 'last_name']).sorting_dict, + SortingDictInterface(["domain", "requested_domain"], ["name"]).sorting_dict, + SortingDictInterface(["domain_application"], ['id']).sorting_dict +] class AuditedAdmin(admin.ModelAdmin): """Custom admin to make auditing easier.""" @@ -36,16 +33,9 @@ class AuditedAdmin(admin.ModelAdmin): def formfield_for_foreignkey(self, db_field, request, **kwargs): """Used to sort dropdown fields alphabetically but can be expanded upon""" - order_by_list = [] - - # Determines what we want to sort by, ex: by name - if db_field.name in foreignkey_orderby_dict: - order_by_list = foreignkey_orderby_dict.get(db_field.name) - form_field = super(AuditedAdmin, self).formfield_for_foreignkey(db_field, request, **kwargs) - return formfield_order_helper(form_field, order_by_list) + return form_field_order_helper(form_field, db_field) - class ListHeaderAdmin(AuditedAdmin): @@ -193,13 +183,8 @@ class ContactAdmin(ListHeaderAdmin): def formfield_for_foreignkey(self, db_field, request, **kwargs): """Used to sort dropdown fields alphabetically but can be expanded upon""" - order_by_list = [] - # Determines what we want to sort by, ex: by name - if db_field.name in foreignkey_orderby_dict: - order_by_list = foreignkey_orderby_dict.get(db_field.name) - form_field = super(ContactAdmin, self).formfield_for_foreignkey(db_field, request, **kwargs) - return formfield_order_helper(form_field, order_by_list) + return form_field_order_helper(form_field, db_field) class DomainApplicationAdmin(ListHeaderAdmin): @@ -336,14 +321,9 @@ class DomainApplicationAdmin(ListHeaderAdmin): # Regular users can only view the specified fields return self.readonly_fields - -def formfield_order_helper(form_field, order_by_list): - """A helper function to order a dropdown field in Django Admin, takes the fields you want to order by as an array""" - # Only order if we choose to do so - if order_by_list: - form_field.queryset = form_field.queryset.order_by(*order_by_list) - - return form_field +def form_field_order_helper(form_field, db_field): + 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) 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..4ce56f97b --- /dev/null +++ b/src/registrar/models/utility/admin_form_order_helper.py @@ -0,0 +1,41 @@ +import logging +from django.forms import ModelChoiceField +logger = logging.getLogger(__name__) +class SortingDictInterface: + _model_list = {} + _sort_list = [] + sorting_dict = {} + + def __init__(self, model_list, sort_list): + self.sorting_dict = { + "dropDownSelected": model_list, + "sortBy": sort_list + } + + +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_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""" + _order_by_list = [] + + for item in self._sorting_dict: + drop_down_selected = item.get("dropDownSelected") + 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: + form_field.queryset = form_field.queryset.order_by(*_order_by_list) + + return form_field + + From d99260f70af0e6ba6c1885bf93f5fb91070e4635 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 11 Aug 2023 12:41:45 -0600 Subject: [PATCH 05/27] Uncommited changes for tests Test cases still a WIP - unsure as to why these two querysets are ordered differently. Otherwise the sorting logic is there --- src/registrar/admin.py | 3 +- src/registrar/tests/common.py | 78 ++++++++++++++++++++++++++++++- src/registrar/tests/test_admin.py | 49 +++++++++++++++++-- 3 files changed, 123 insertions(+), 7 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 3d568f441..34a14486f 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -14,8 +14,7 @@ logger = logging.getLogger(__name__) foreignkey_orderby_dict: [SortingDictInterface] = [ #foreign_key - order_by SortingDictInterface(["submitter", "authorizing_official", "investigator", "creator", "user"], ['first_name', 'last_name']).sorting_dict, - SortingDictInterface(["domain", "requested_domain"], ["name"]).sorting_dict, - SortingDictInterface(["domain_application"], ['id']).sorting_dict + SortingDictInterface(["domain", "requested_domain"], ["name"]).sorting_dict ] class AuditedAdmin(admin.ModelAdmin): diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 4359fc454..a8ee22a03 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -2,6 +2,8 @@ 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 @@ -149,8 +151,8 @@ def completed_application( 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 +190,75 @@ def completed_application( application.alternative_domains.add(alt) return application + +def multiple_completed_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,): + applications = [] + list_of_letters = list(ascii_uppercase) + random.shuffle(list_of_letters) + for x in list_of_letters: + if not user: + user = get_user_model().objects.create(username="username{}".format(x)) + ao, _ = Contact.objects.get_or_create( + first_name="{} Testy".format(x), + last_name="{} Tester".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="{} Testy you".format(x), + last_name="{} Tester you".format(x), + title="{} Admin Tester".format(x), + email="mayor@igorville.gov", + phone="(555) 555 5556", + ) + other, _ = Contact.objects.get_or_create( + first_name="{} Testy".format(x), + last_name="{} Tester".format(x), + title="{} Another Tester".format(x), + email="{}testy2@town.com".format(x), + phone="(555) 555 5557", + ) + 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, + ) + 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( + **domain_application_kwargs + ) + + 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 \ No newline at end of file diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 3c07e7608..54b98fda8 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1,15 +1,17 @@ from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite -from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin, MyUserAdmin +from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin, MyUserAdmin, AuditedAdmin from registrar.models import DomainApplication, DomainInformation, User -from .common import completed_application, mock_user, create_superuser, create_user +from registrar.models.contact import Contact +from .common import completed_application, mock_user, create_superuser, create_user, multiple_completed_applications 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): def setUp(self): self.site = AdminSite() @@ -367,3 +369,44 @@ 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") + self.superuser = create_superuser() + self.factory.post + + def test_alphabetically_sorted_fk_fields(self): + mock_client = MagicMock() + + #tested_fields = [{"name": "submitter"}, {"name": "authorizing_official"}, {"name": "investigator"}, {"name": "creator"}, {"name": "user"}] + tested_fields = [DomainApplication.authorizing_official.field, DomainApplication.submitter.field, DomainApplication.investigator.field, DomainApplication.creator.field] + with boto3_mocking.clients.handler_for("sesv2", mock_client): + # Create a sample application - review status does not matter + applications = multiple_completed_applications(status=DomainApplication.IN_REVIEW) + # Create a mock request + request = self.factory.post( + "/admin/registrar/domainapplication/{}/change/".format(applications[0].pk) + ) + + model_admin = AuditedAdmin(DomainApplication, self.site) + + for field in tested_fields: + desired_order = model_admin.get_queryset(request).order_by("{}__first_name".format(field.name)) + current_sort_order = model_admin.formfield_for_foreignkey(field, request).queryset + + self.assertEqual(desired_order, current_sort_order, "{} is not ordered alphabetically".format(field.name)) + # Is initalized in alphabetical order + + + for x in model_admin.get_queryset(request).all(): + logger.debug(x.authorizing_official) + + + def tearDown(self): + DomainInformation.objects.all().delete() + DomainApplication.objects.all().delete() + User.objects.all().delete() + self.superuser.delete() From 9df58515e38929ec860a8356f4ee27b4c1b1a0ad Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 11 Aug 2023 12:42:16 -0600 Subject: [PATCH 06/27] Update test_admin.py --- src/registrar/tests/test_admin.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 54b98fda8..f9ab53863 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -398,11 +398,6 @@ class AuditedAdminTest(TestCase): current_sort_order = model_admin.formfield_for_foreignkey(field, request).queryset self.assertEqual(desired_order, current_sort_order, "{} is not ordered alphabetically".format(field.name)) - # Is initalized in alphabetical order - - - for x in model_admin.get_queryset(request).all(): - logger.debug(x.authorizing_official) def tearDown(self): From c7cba16e3c252981887412a459b09bba132a8a43 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Sun, 13 Aug 2023 21:34:00 -0600 Subject: [PATCH 07/27] Test case --- src/registrar/tests/common.py | 28 +++++++---- src/registrar/tests/test_admin.py | 82 ++++++++++++++++++++++--------- 2 files changed, 76 insertions(+), 34 deletions(-) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index a8ee22a03..def9793f3 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -191,7 +191,7 @@ def completed_application( return application -def multiple_completed_applications(has_other_contacts=True, +def multiple_completed_applications_for_alphabetical_test(has_other_contacts=True, has_current_website=True, has_alternative_gov_domain=True, has_type_of_work=True, @@ -202,11 +202,14 @@ def multiple_completed_applications(has_other_contacts=True, list_of_letters = list(ascii_uppercase) random.shuffle(list_of_letters) for x in list_of_letters: - if not user: - user = get_user_model().objects.create(username="username{}".format(x)) + 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="{} Testy".format(x), - last_name="{} Tester".format(x), + 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", @@ -215,19 +218,24 @@ def multiple_completed_applications(has_other_contacts=True, 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="{} Testy you".format(x), - last_name="{} Tester you".format(x), + 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="{} Testy".format(x), - last_name="{} Tester".format(x), + 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", @@ -243,6 +251,7 @@ def multiple_completed_applications(has_other_contacts=True, submitter=you, creator=user, status=status, + investigator=inv ) if has_type_of_work: domain_application_kwargs["type_of_work"] = "e-Government" @@ -260,5 +269,4 @@ def multiple_completed_applications(has_other_contacts=True, if has_alternative_gov_domain: application.alternative_domains.add(alt) applications.append(application) - return applications \ No newline at end of file diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index f9ab53863..f971ed6c0 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -3,7 +3,7 @@ from django.contrib.admin.sites import AdminSite from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin, 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_completed_applications +from .common import completed_application, mock_user, create_superuser, create_user, multiple_completed_applications_for_alphabetical_test from django.contrib.auth import get_user_model from django.conf import settings @@ -375,33 +375,67 @@ class AuditedAdminTest(TestCase): self.site = AdminSite() self.factory = RequestFactory() self.client = Client(HTTP_HOST="localhost:8080") - self.superuser = create_superuser() - self.factory.post - def test_alphabetically_sorted_fk_fields(self): - mock_client = MagicMock() - - #tested_fields = [{"name": "submitter"}, {"name": "authorizing_official"}, {"name": "investigator"}, {"name": "creator"}, {"name": "user"}] + def test_alphabetically_sorted_fk_fields_domain_application(self): tested_fields = [DomainApplication.authorizing_official.field, DomainApplication.submitter.field, DomainApplication.investigator.field, DomainApplication.creator.field] - with boto3_mocking.clients.handler_for("sesv2", mock_client): - # Create a sample application - review status does not matter - applications = multiple_completed_applications(status=DomainApplication.IN_REVIEW) - # Create a mock request - request = self.factory.post( - "/admin/registrar/domainapplication/{}/change/".format(applications[0].pk) - ) - - model_admin = AuditedAdmin(DomainApplication, self.site) - - for field in tested_fields: - desired_order = model_admin.get_queryset(request).order_by("{}__first_name".format(field.name)) - current_sort_order = model_admin.formfield_for_foreignkey(field, request).queryset - self.assertEqual(desired_order, current_sort_order, "{} is not ordered alphabetically".format(field.name)) - + # Create a sample application - review status does not matter + applications = multiple_completed_applications_for_alphabetical_test(status=DomainApplication.IN_REVIEW) + # Create a mock request + request = self.factory.post( + "/admin/registrar/domainapplication/{}/change/".format(applications[0].pk) + ) + + model_admin = AuditedAdmin(DomainApplication, self.site) + + # Typically we wouldnt 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) + 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)) + logger.debug(desired_order) + current_sort_order: Contact = list(model_admin.formfield_for_foreignkey(field, request).queryset) + 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_name = contact.first_name + last_name = contact.last_name + + match field.name: + 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)) + 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)) + logger.debug("current: {}".format(current_sort_order_coerced_type)) + + 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): + if(first_name and first_name.split(queryset_shorthand)[1] == field_name): + return (first_name, last_name) + else: + return None + def tearDown(self): DomainInformation.objects.all().delete() DomainApplication.objects.all().delete() - User.objects.all().delete() - self.superuser.delete() From 0b2faac625afa5a30a981b3878b9f9c3341eddb0 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Sun, 13 Aug 2023 21:34:42 -0600 Subject: [PATCH 08/27] Removed logger.debug --- src/registrar/tests/test_admin.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index f971ed6c0..79492b01e 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -395,10 +395,9 @@ class AuditedAdminTest(TestCase): 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)) - logger.debug(desired_order) + 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, request).queryset) + current_sort_order_coerced_type = [] # This is necessary as .queryset and get_queryset return lists of different types/structures. @@ -424,7 +423,6 @@ class AuditedAdminTest(TestCase): 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)) - logger.debug("current: {}".format(current_sort_order_coerced_type)) self.assertEqual(desired_order, current_sort_order_coerced_type, "{} is not ordered alphabetically".format(field.name)) From 4fa9e786886342ae14b8a2c5fa0d1102aa6a32db Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 14 Aug 2023 08:59:22 -0600 Subject: [PATCH 09/27] Battle of the linter, circa 2023 --- src/registrar/admin.py | 35 +++++--- .../models/utility/admin_form_order_helper.py | 23 ++--- src/registrar/tests/common.py | 15 ++-- src/registrar/tests/test_admin.py | 88 +++++++++++-------- 4 files changed, 98 insertions(+), 63 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 34a14486f..9729e018c 100644 --- a/src/registrar/admin.py +++ b/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 +from registrar.models.utility.admin_form_order_helper import AdminFormOrderHelper, SortingDictInterface # noqa 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 - SortingDictInterface(["submitter", "authorizing_official", "investigator", "creator", "user"], ['first_name', 'last_name']).sorting_dict, - SortingDictInterface(["domain", "requested_domain"], ["name"]).sorting_dict + # foreign_key - order_by + SortingDictInterface( + 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.""" @@ -29,16 +41,14 @@ class AuditedAdmin(admin.ModelAdmin): object_id=object_id, ) ) - + 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) diff --git a/src/registrar/models/utility/admin_form_order_helper.py b/src/registrar/models/utility/admin_form_order_helper.py index 4ce56f97b..3004ec288 100644 --- a/src/registrar/models/utility/admin_form_order_helper.py +++ b/src/registrar/models/utility/admin_form_order_helper.py @@ -1,30 +1,35 @@ import logging +from typing import Dict from django.forms import ModelChoiceField + logger = logging.getLogger(__name__) + + class SortingDictInterface: - _model_list = {} - _sort_list = [] + _model_list: Dict[type, type] = {} + _sort_list: list[type] = [] sorting_dict = {} def __init__(self, model_list, sort_list): self.sorting_dict = { "dropDownSelected": model_list, - "sortBy": sort_list + "sortBy": sort_list } 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: drop_down_selected = item.get("dropDownSelected") sort_by = item.get("sortBy") @@ -37,5 +42,3 @@ class AdminFormOrderHelper(): 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 def9793f3..9597065b7 100644 --- a/src/registrar/tests/common.py +++ b/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", - last_name="Tester you", + first_name="Testy2", + last_name="Tester2", title="Admin Tester", email="mayor@igorville.gov", phone="(555) 555 5556", @@ -191,17 +191,20 @@ 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) - for x in 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), @@ -269,4 +272,4 @@ def multiple_completed_applications_for_alphabetical_test(has_other_contacts=Tru if has_alternative_gov_domain: application.alternative_domains.add(alt) applications.append(application) - return applications \ No newline at end of file + return applications diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 79492b01e..6575f3a4b 100644 --- a/src/registrar/tests/test_admin.py +++ b/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,63 +380,74 @@ 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( "/admin/registrar/domainapplication/{}/change/".format(applications[0].pk) ) - + 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) - last_name_field = "{}__last_name".format(field.name) + field_name = field[0].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)) - current_sort_order: Contact = list(model_admin.formfield_for_foreignkey(field, request).queryset) + # 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 = 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 - last_name = contact.last_name + first = contact.first_name + last = contact.last_name - match field.name: - 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)) - 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)) + name_tuple = self.coerced_fk_field_helper(first, last, field[1], ':') + 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)) - # 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): - if(first_name and first_name.split(queryset_shorthand)[1] == 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 + if(first_name and first_name.split(queryset_shorthand)[1] == field_name): # noqa return (first_name, last_name) else: return None - + def tearDown(self): DomainInformation.objects.all().delete() DomainApplication.objects.all().delete() From 5fd842233a1af814838217bfd6e481408db21afd Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 14 Aug 2023 09:13:22 -0600 Subject: [PATCH 10/27] Linting stuff --- src/registrar/models/utility/admin_form_order_helper.py | 4 ++-- src/registrar/tests/test_admin.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/models/utility/admin_form_order_helper.py b/src/registrar/models/utility/admin_form_order_helper.py index 3004ec288..e2d70efd9 100644 --- a/src/registrar/models/utility/admin_form_order_helper.py +++ b/src/registrar/models/utility/admin_form_order_helper.py @@ -8,7 +8,7 @@ logger = logging.getLogger(__name__) class SortingDictInterface: _model_list: Dict[type, type] = {} _sort_list: list[type] = [] - sorting_dict = {} + sorting_dict: Dict[type, type] = {} def __init__(self, model_list, sort_list): self.sorting_dict = { @@ -21,7 +21,7 @@ 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 # Used to keep track of how we want to order_by certain FKs - _sorting_dict: [SortingDictInterface] = [...] + _sorting_dict: [SortingDictInterface] = [...] # noqa def __init__(self, sort): self._sorting_dict = sort diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 6575f3a4b..be62d7c1e 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -436,7 +436,7 @@ class AuditedAdminTest(TestCase): self.assertEqual(desired_order, current_sort_order_coerced_type, - "{} is not ordered alphabetically".format(field_name)) + "{} is not ordered alphabetically".format(field.name)) # I originally spent some time trying to fully # generalize this to replace the match/arg fields, From 23405cd145deb682876f45e7b31c8549e0b203a8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 14 Aug 2023 09:47:16 -0600 Subject: [PATCH 11/27] Lint --- src/registrar/models/utility/admin_form_order_helper.py | 2 +- src/registrar/tests/test_admin.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/utility/admin_form_order_helper.py b/src/registrar/models/utility/admin_form_order_helper.py index e2d70efd9..aaa0cdce4 100644 --- a/src/registrar/models/utility/admin_form_order_helper.py +++ b/src/registrar/models/utility/admin_form_order_helper.py @@ -21,7 +21,7 @@ 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 # Used to keep track of how we want to order_by certain FKs - _sorting_dict: [SortingDictInterface] = [...] # noqa + _sorting_dict: list[SortingDictInterface] = [...] # noqa def __init__(self, sort): self._sorting_dict = sort diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index be62d7c1e..6575f3a4b 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -436,7 +436,7 @@ class AuditedAdminTest(TestCase): self.assertEqual(desired_order, current_sort_order_coerced_type, - "{} is not ordered alphabetically".format(field.name)) + "{} is not ordered alphabetically".format(field_name)) # I originally spent some time trying to fully # generalize this to replace the match/arg fields, From 5b78b665a7aeaec03ad2aa562af6eb47f146c068 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 14 Aug 2023 09:57:02 -0600 Subject: [PATCH 12/27] Another lint change --- src/registrar/models/utility/admin_form_order_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/utility/admin_form_order_helper.py b/src/registrar/models/utility/admin_form_order_helper.py index aaa0cdce4..d29951cf7 100644 --- a/src/registrar/models/utility/admin_form_order_helper.py +++ b/src/registrar/models/utility/admin_form_order_helper.py @@ -21,7 +21,7 @@ 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 # Used to keep track of how we want to order_by certain FKs - _sorting_dict: list[SortingDictInterface] = [...] # noqa + _sorting_dict: list[SortingDictInterface] = [] # noqa def __init__(self, sort): self._sorting_dict = sort From 738b75a7df41904c4d87537b8e0579d0c9e53373 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 16 Aug 2023 12:32:17 -0600 Subject: [PATCH 13/27] Updated test cases / common.py helpers Expanded some test cases and added some common.py helpers for future tests --- src/registrar/admin.py | 2 +- .../models/utility/admin_form_order_helper.py | 5 +- src/registrar/tests/common.py | 252 ++++++++++++------ src/registrar/tests/test_admin.py | 170 ++++++++++-- 4 files changed, 322 insertions(+), 107 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 9729e018c..124553c31 100644 --- a/src/registrar/admin.py +++ b/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, diff --git a/src/registrar/models/utility/admin_form_order_helper.py b/src/registrar/models/utility/admin_form_order_helper.py index d29951cf7..8957262b3 100644 --- a/src/registrar/models/utility/admin_form_order_helper.py +++ b/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 diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 9597065b7..ef4428177 100644 --- a/src/registrar/tests/common.py +++ b/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( - 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, -): +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) - 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( - **domain_application_kwargs - ) - - 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) + 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 6575f3a4b..490442f00 100644 --- a/src/registrar/tests/test_admin.py +++ b/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) - # 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') + DomainApplication.authorizing_official.field, + DomainApplication.submitter.field, + DomainApplication.investigator.field, + DomainApplication.creator.field, ] # 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 = list( - model_admin.get_queryset(request).order_by( - first_name_field, last_name_field).values_list( - first_name_field, last_name_field) - ) + 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[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)) + 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.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() From f31258d2b0274d54de4fd42d09db146df27e9bae Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 16 Aug 2023 14:26:17 -0600 Subject: [PATCH 14/27] Linter things / Rachid suggestions --- src/registrar/admin.py | 18 +-- .../models/utility/admin_form_order_helper.py | 10 +- src/registrar/tests/common.py | 123 ++++++++++++------ src/registrar/tests/test_admin.py | 79 ++++++++--- 4 files changed, 157 insertions(+), 73 deletions(-) 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 From b389c9f831afb6f500dae71bcf01ff8857ba018e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 16 Aug 2023 14:32:11 -0600 Subject: [PATCH 15/27] Fixed unintentional model change Not sure how this snuck in --- src/registrar/models/domain_information.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index 084bd0515..438567003 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -244,3 +244,6 @@ class DomainInformation(TimeStampedModel): domain_info.domain = domain domain_info.save() return domain_info + + class Meta: + verbose_name_plural = "Domain Information" \ No newline at end of file From 13718371034080cc5e24b350c00ca96662ff79b0 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 16 Aug 2023 15:05:54 -0600 Subject: [PATCH 16/27] Update admin_form_order_helper.py --- .../models/utility/admin_form_order_helper.py | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/registrar/models/utility/admin_form_order_helper.py b/src/registrar/models/utility/admin_form_order_helper.py index ff32ead93..2057a2276 100644 --- a/src/registrar/models/utility/admin_form_order_helper.py +++ b/src/registrar/models/utility/admin_form_order_helper.py @@ -10,9 +10,20 @@ class SortingDictInterface: _sort_list: list[type] = [] sorting_dict: Dict[type, type] = {} + # _model_list and _sort_list can be + # any length, and will be called multiple times. + # We want the perf advantage of a dictionary, + # while making creating new SortingDictInterface + # items pretty straight forward and easy (aka as a list) + def convert_list_to_dict(self, value_list): + dictionary: Dict[type, type] = {} + for item in value_list: + dictionary[item] = item + return dictionary + def __init__(self, model_list, sort_list): self.sorting_dict = { - "dropDownSelected": model_list, + "dropDownSelected": self.convert_list_to_dict(model_list), "sortBy": sort_list } @@ -33,11 +44,9 @@ 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") + drop_down_selected = item["dropDownSelected"] + sort_by = item["sortBy"] + if db_field.name in drop_down_selected: _order_by_list = sort_by break From ed26035763f87239b168065a49c6523545027744 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 17 Aug 2023 09:54:16 -0600 Subject: [PATCH 17/27] Fixe some linting issues / comments --- src/registrar/admin.py | 29 +++++++-------- .../models/utility/admin_form_order_helper.py | 35 +++++++++++-------- 2 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index d19231755..66215733c 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -4,29 +4,26 @@ 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, SortingDict +# Split up for the linter +from registrar.models.utility.admin_form_order_helper import SortingDict 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'] -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] = [ +foreignkey_orderby_dict: list[SortingDict] = [ # foreign_key - order_by - SortingDictInterface( - audited_admin_item_names, - audited_admin_orderby_names - ).sorting_dict, + # 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' - SortingDictInterface( - special_audited_admin_item_names, - special_audited_admin_orderby_names - ).sorting_dict + SortingDict( + ["domain", "requested_domain"], + ["name"] + ) ] diff --git a/src/registrar/models/utility/admin_form_order_helper.py b/src/registrar/models/utility/admin_form_order_helper.py index 2057a2276..fc16f5a31 100644 --- a/src/registrar/models/utility/admin_form_order_helper.py +++ b/src/registrar/models/utility/admin_form_order_helper.py @@ -5,16 +5,14 @@ from django.forms import ModelChoiceField logger = logging.getLogger(__name__) -class SortingDictInterface: - _model_list: Dict[type, type] = {} - _sort_list: list[type] = [] - sorting_dict: Dict[type, type] = {} +class SortingDict: + _sorting_dict: Dict[type, type] = {} - # _model_list and _sort_list can be - # any length, and will be called multiple times. - # We want the perf advantage of a dictionary, - # while making creating new SortingDictInterface - # items pretty straight forward and easy (aka as a list) + # model_list can be will be called multiple times. + # Not super necessary, but it'd be nice + # to have the perf advantage of a dictionary, + # while minimizing typing when + # adding a new SortingDict (input as a list) def convert_list_to_dict(self, value_list): dictionary: Dict[type, type] = {} for item in value_list: @@ -22,30 +20,37 @@ class SortingDictInterface: return dictionary def __init__(self, model_list, sort_list): - self.sorting_dict = { + self._sorting_dict = { "dropDownSelected": self.convert_list_to_dict(model_list), "sortBy": sort_list } + def get_dict(self): + # 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_dict: list[SortingDictInterface] = [] + _sorting_list: list[SortingDict] = [] def __init__(self, sort): - self._sorting_dict = sort + 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_dict: - drop_down_selected = item["dropDownSelected"] - sort_by = item["sortBy"] + 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 From 080a70e613109f9188c03aeab26b48bfbfd7f868 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 17 Aug 2023 11:24:35 -0600 Subject: [PATCH 18/27] Why must the linter lint? --- src/registrar/admin.py | 10 +++++----- src/registrar/models/domain_information.py | 2 +- .../models/utility/admin_form_order_helper.py | 6 +++++- src/registrar/tests/test_admin.py | 3 ++- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 66215733c..8c9f8512c 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -4,7 +4,7 @@ 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, SortingDict +from registrar.models.utility.admin_form_order_helper import AdminFormOrderHelper # Split up for the linter from registrar.models.utility.admin_form_order_helper import SortingDict from . import models @@ -16,13 +16,13 @@ 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'] + ["submitter", "authorizing_official", "investigator", "creator", "user"], + ['first_name', 'last_name'] ), # Handles fields that are sorted by 'name' SortingDict( - ["domain", "requested_domain"], - ["name"] + ["domain", "requested_domain"], + ["name"] ) ] diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index 438567003..b12039e73 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -246,4 +246,4 @@ class DomainInformation(TimeStampedModel): return domain_info class Meta: - verbose_name_plural = "Domain Information" \ No newline at end of file + verbose_name_plural = "Domain Information" diff --git a/src/registrar/models/utility/admin_form_order_helper.py b/src/registrar/models/utility/admin_form_order_helper.py index fc16f5a31..e017db66f 100644 --- a/src/registrar/models/utility/admin_form_order_helper.py +++ b/src/registrar/models/utility/admin_form_order_helper.py @@ -6,6 +6,7 @@ logger = logging.getLogger(__name__) class SortingDict: + """Stores a sorting dictionary object""" _sorting_dict: Dict[type, type] = {} # model_list can be will be called multiple times. @@ -14,6 +15,7 @@ class SortingDict: # while minimizing typing when # adding a new SortingDict (input as a list) def convert_list_to_dict(self, value_list): + """Used internally to convert model_list to a dictionary""" dictionary: Dict[type, type] = {} for item in value_list: dictionary[item] = item @@ -26,6 +28,8 @@ class SortingDict: } 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") @@ -39,7 +43,7 @@ class AdminFormOrderHelper(): # Used to keep track of how we want to order_by certain FKs _sorting_list: list[SortingDict] = [] - def __init__(self, sort): + def __init__(self, sort: list[SortingDict]): self._sorting_list = sort def get_ordered_form_field(self, form_field, db_field) -> (ModelChoiceField | None): diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index cd66c9bbd..5978d2159 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -583,6 +583,7 @@ class AuditedAdminTest(TestCase): 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') @@ -591,7 +592,7 @@ class AuditedAdminTest(TestCase): if last_name is None: return (first_name,) - if(first_name.split(queryset_shorthand)[1] == field_name): + if (first_name.split(queryset_shorthand)[1] == field_name): return returned_tuple else: return None From 40b5d7ec5c41552ddfe5e2fc2218ada17c9b60d1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 17 Aug 2023 11:41:07 -0600 Subject: [PATCH 19/27] Black formatting --- src/registrar/admin.py | 9 +- .../models/utility/admin_form_order_helper.py | 7 +- src/registrar/tests/common.py | 94 ++++++++----------- src/registrar/tests/test_admin.py | 85 +++++++---------- 4 files changed, 82 insertions(+), 113 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8c9f8512c..7ff3d8652 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -5,6 +5,7 @@ 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 + # Split up for the linter from registrar.models.utility.admin_form_order_helper import SortingDict from . import models @@ -17,13 +18,10 @@ foreignkey_orderby_dict: list[SortingDict] = [ # Handles fields that are sorted by 'first_name / last_name SortingDict( ["submitter", "authorizing_official", "investigator", "creator", "user"], - ['first_name', 'last_name'] + ["first_name", "last_name"], ), # Handles fields that are sorted by 'name' - SortingDict( - ["domain", "requested_domain"], - ["name"] - ) + SortingDict(["domain", "requested_domain"], ["name"]), ] @@ -184,6 +182,7 @@ 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." diff --git a/src/registrar/models/utility/admin_form_order_helper.py b/src/registrar/models/utility/admin_form_order_helper.py index e017db66f..28e653830 100644 --- a/src/registrar/models/utility/admin_form_order_helper.py +++ b/src/registrar/models/utility/admin_form_order_helper.py @@ -7,6 +7,7 @@ logger = logging.getLogger(__name__) class SortingDict: """Stores a sorting dictionary object""" + _sorting_dict: Dict[type, type] = {} # model_list can be will be called multiple times. @@ -24,7 +25,7 @@ class SortingDict: def __init__(self, model_list, sort_list): self._sorting_dict = { "dropDownSelected": self.convert_list_to_dict(model_list), - "sortBy": sort_list + "sortBy": sort_list, } def get_dict(self): @@ -36,7 +37,7 @@ class SortingDict: return self._sorting_dict -class AdminFormOrderHelper(): +class AdminFormOrderHelper: """A helper class to order a dropdown field in Django Admin, takes the fields you want to order by as an array""" @@ -46,7 +47,7 @@ class AdminFormOrderHelper(): def __init__(self, sort: list[SortingDict]): self._sorting_list = sort - def get_ordered_form_field(self, form_field, db_field) -> (ModelChoiceField | None): + 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 = [] diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index b92fcf351..f838220e8 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -11,6 +11,7 @@ from django.conf import settings from django.contrib.auth import get_user_model, login from registrar.models import Contact, DraftDomain, Website, DomainApplication + # For the linter from registrar.models import DomainInvitation, User, DomainInformation, Domain @@ -118,7 +119,7 @@ class AuditedAdminMockData: 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) + username="{} username:{}".format(item_name, shorthand), )[0] return user @@ -129,7 +130,7 @@ class AuditedAdminMockData: last_name="{} Last:{}".format(item_name, shorthand), title="{} title:{}".format(item_name, shorthand), email="{}testy@town.com".format(item_name), - phone="(555) 555 5555" + phone="(555) 555 5555", )[0] return contact @@ -143,9 +144,9 @@ class AuditedAdminMockData: 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""" @@ -156,7 +157,7 @@ class AuditedAdminMockData: item_name, org_type="federal", federal_type="executive", - purpose="Purpose of the site" + purpose="Purpose of the site", ): """Generates a generic argument list for most domains""" common_args = dict( @@ -169,11 +170,11 @@ class AuditedAdminMockData: is_policy_acknowledged=True, state_territory="NY", zipcode="10002", - type_of_work='e-Government', + 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'), + 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 @@ -185,14 +186,11 @@ class AuditedAdminMockData: status, org_type="federal", federal_type="executive", - purpose="Purpose of the site" + 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 + item_name, org_type, federal_type, purpose ) full_arg_list = None match domain_type: @@ -200,7 +198,7 @@ class AuditedAdminMockData: full_arg_list = dict( **common_args, requested_domain=self.dummy_draft_domain(item_name), - investigator=self.dummy_user(item_name, 'investigator'), + investigator=self.dummy_user(item_name, "investigator"), status=status, ) case self.INFORMATION: @@ -208,62 +206,50 @@ class AuditedAdminMockData: full_arg_list = dict( **common_args, domain=self.dummy_domain(item_name), - domain_application=domain_app + 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 + status=DomainInvitation.INVITED, ) return full_arg_list def create_full_dummy_domain_application( - self, - object_name, - status=DomainApplication.STARTED + self, object_name, status=DomainApplication.STARTED ): """Creates a dummy domain application object""" domain_application_kwargs = self.dummy_kwarg_boilerplate( - self.APPLICATION, - object_name, - status - ) + self.APPLICATION, object_name, status + ) application = DomainApplication.objects.get_or_create( - **domain_application_kwargs - )[0] + **domain_application_kwargs + )[0] return application def create_full_dummy_domain_information( - self, - object_name, - status=DomainApplication.STARTED + self, object_name, status=DomainApplication.STARTED ): """Creates a dummy domain information object""" domain_application_kwargs = self.dummy_kwarg_boilerplate( - self.INFORMATION, - object_name, - status - ) + self.INFORMATION, object_name, status + ) application = DomainInformation.objects.get_or_create( - **domain_application_kwargs - )[0] + **domain_application_kwargs + )[0] return application def create_full_dummy_domain_invitation( - self, - object_name, - status=DomainApplication.STARTED + self, object_name, status=DomainApplication.STARTED ): """Creates a dummy domain invitation object""" domain_application_kwargs = self.dummy_kwarg_boilerplate( - self.INVITATION, - object_name, - status - ) + self.INVITATION, object_name, status + ) application = DomainInvitation.objects.get_or_create( - **domain_application_kwargs - )[0] + **domain_application_kwargs + )[0] return application @@ -274,28 +260,28 @@ class AuditedAdminMockData: has_other_contacts=True, has_current_website=True, has_alternative_gov_domain=True, - status=DomainApplication.STARTED + 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 - ) + object_name, status + ) case self.INVITATION: application = self.create_full_dummy_domain_invitation( - object_name, status - ) + object_name, status + ) case self.INFORMATION: application = self.create_full_dummy_domain_information( - object_name, status - ) + 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') + 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) @@ -410,7 +396,7 @@ def completed_application( def multiple_unalphabetical_domain_objects( - domain_type=AuditedAdminMockData.APPLICATION + domain_type=AuditedAdminMockData.APPLICATION, ): """Returns a list of generic domain objects for testing purposes""" applications = [] diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 5978d2159..461b7d217 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1,12 +1,14 @@ from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite 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 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 @@ -385,11 +387,7 @@ class AuditedAdminTest(TestCase): self.client = Client(HTTP_HOST="localhost:8080") def order_by_desired_field_helper( - self, - obj_to_sort: AuditedAdmin, - request, - field_name, - *obj_names + self, obj_to_sort: AuditedAdmin, request, field_name, *obj_names ): formatted_sort_fields = [] for obj in obj_names: @@ -397,10 +395,10 @@ class AuditedAdminTest(TestCase): # 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) - ) + obj_to_sort.get_queryset(request) + .order_by(*formatted_sort_fields) + .values_list(*formatted_sort_fields) + ) return ordered_list @@ -408,11 +406,11 @@ class AuditedAdminTest(TestCase): # is it worth the time investment to do so? 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.authorizing_official.field, + DomainApplication.submitter.field, + DomainApplication.investigator.field, + DomainApplication.creator.field, + ] # Creates multiple domain applications - review status does not matter applications = multiple_unalphabetical_domain_objects("application") @@ -428,18 +426,13 @@ class AuditedAdminTest(TestCase): # but both fields are of a fixed length. # For test case purposes, this should be performant. 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" - ) + model_admin, request, field.name, "first_name", "last_name" + ) current_sort_order: Contact = list( model_admin.formfield_for_foreignkey(field, request).queryset - ) + ) # Conforms to the same object structure as desired_order current_sort_order_coerced_type = [] @@ -451,23 +444,23 @@ class AuditedAdminTest(TestCase): first = contact.first_name last = contact.last_name - name_tuple = self.coerced_fk_field_helper(first, last, field.name, ':') + 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 - ] + DomainInformation.authorizing_official.field, + DomainInformation.submitter.field, + DomainInformation.domain.field, + DomainInformation.creator.field, + ] # Creates multiple domain applications - review status does not matter applications = multiple_unalphabetical_domain_objects("information") @@ -484,19 +477,16 @@ 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 == DomainInformation.domain.field) - if (isNamefield): + 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 - ) + model_admin, request, field.name, *sorted_fields + ) current_sort_order = list( model_admin.formfield_for_foreignkey(field, request).queryset ) @@ -515,14 +505,14 @@ class AuditedAdminTest(TestCase): first = contact.name last = None - name_tuple = self.coerced_fk_field_helper(first, last, field.name, ':') + 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) + "{} is not ordered alphabetically".format(field.name), ) def test_alphabetically_sorted_fk_fields_domain_invitation(self): @@ -547,10 +537,7 @@ class AuditedAdminTest(TestCase): # 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 + model_admin, request, field.name, *sorted_fields ) current_sort_order = list( model_admin.formfield_for_foreignkey(field, request).queryset @@ -566,33 +553,29 @@ class AuditedAdminTest(TestCase): first = contact.name last = None - name_tuple = self.coerced_fk_field_helper(first, last, field.name, ':') + 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) + "{} is not ordered alphabetically".format(field.name), ) def coerced_fk_field_helper( - self, - first_name, - last_name, - field_name, - queryset_shorthand + 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') + 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): + if first_name.split(queryset_shorthand)[1] == field_name: return returned_tuple else: return None From aef9df5296e1ffb388d1869d7a0ff7048b44b367 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 17 Aug 2023 14:55:21 -0600 Subject: [PATCH 20/27] PR changes --- src/registrar/admin.py | 5 +- .../models/utility/admin_form_order_helper.py | 21 ++---- src/registrar/tests/common.py | 75 ++++++++++++------- src/registrar/tests/test_admin.py | 17 +---- 4 files changed, 60 insertions(+), 58 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7ff3d8652..ed16e25b5 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -4,10 +4,7 @@ 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 - -# Split up for the linter -from registrar.models.utility.admin_form_order_helper import SortingDict +from registrar.models.utility.admin_form_order_helper import AdminFormOrderHelper, SortingDict # noqa from . import models logger = logging.getLogger(__name__) diff --git a/src/registrar/models/utility/admin_form_order_helper.py b/src/registrar/models/utility/admin_form_order_helper.py index 28e653830..fe081fc9b 100644 --- a/src/registrar/models/utility/admin_form_order_helper.py +++ b/src/registrar/models/utility/admin_form_order_helper.py @@ -10,24 +10,17 @@ class SortingDict: _sorting_dict: Dict[type, type] = {} - # model_list can be will be called multiple times. - # Not super necessary, but it'd be nice - # to have the perf advantage of a dictionary, - # while minimizing typing when - # adding a new SortingDict (input as a list) - def convert_list_to_dict(self, value_list): - """Used internally to convert model_list to a dictionary""" - dictionary: Dict[type, type] = {} - for item in value_list: - dictionary[item] = item - return dictionary - 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""" @@ -59,10 +52,12 @@ class AdminFormOrderHelper: 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 - if _order_by_list is not None: + # 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/tests/common.py b/src/registrar/tests/common.py index f838220e8..0ff255bca 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -10,10 +10,7 @@ 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 - -# For the linter -from registrar.models import DomainInvitation, User, DomainInformation, Domain +from registrar.models import Contact, DraftDomain, Website, DomainApplication, DomainInvitation, User, DomainInformation, Domain # noqa logger = logging.getLogger(__name__) @@ -96,39 +93,51 @@ class MockSESClient(Mock): class AuditedAdminMockData: - """Creates simple data mocks for AuditedAdminTest""" + """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 string 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 string 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" - # 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): + 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:{}".format(item_name, shorthand), - last_name="{} Last:{}".format(item_name, shorthand), - username="{} username:{}".format(item_name, shorthand), + 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, shorthand): + def dummy_contact(self, item_name, short_hand): """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), + 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] @@ -164,9 +173,9 @@ class AuditedAdminMockData: 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), + 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", @@ -178,7 +187,6 @@ class AuditedAdminMockData: ) return common_args - # This can be boiled down more, though for our purposes this is OK def dummy_kwarg_boilerplate( self, domain_type, @@ -188,7 +196,18 @@ class AuditedAdminMockData: federal_type="executive", purpose="Purpose of the site", ): - """Returns kwargs for different domain object types""" + """ + A helper function that returns premade kwargs for easily creating different domain object types. + There is a decent amount of boilerplate associated with + creating new domain objects (such as domain_application, or domain_information), + so for test case purposes, we can make some assumptions and utilize that to simplify + the object creation process. + + *domain_type* uses constants. Used to identify what kind of 'Domain' object you'd like to make. + + In more detail: domain_type specifies what kind of domain object you'd like to create, i.e. + domain_application (APPLICATION), or domain_information (INFORMATION). + """ common_args = self.get_common_domain_arg_dictionary( item_name, org_type, federal_type, purpose ) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 461b7d217..8f039d9bf 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1,16 +1,10 @@ from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite -from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin +# noqa is used on all three of these as the linter doesn't like the length of this line +from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin, MyUserAdmin, AuditedAdmin # noqa +from registrar.models import DomainApplication, DomainInformation, User, Contact, DomainInvitation # noqa +from .common import completed_application, mock_user, create_superuser, create_user, multiple_unalphabetical_domain_objects # noqa -# 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 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 @@ -393,7 +387,6 @@ class AuditedAdminTest(TestCase): 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) @@ -402,8 +395,6 @@ class AuditedAdminTest(TestCase): 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 = [ DomainApplication.authorizing_official.field, From 9ed83a51da8bb049db885f28b3fbb9fdaf870357 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 17 Aug 2023 15:02:42 -0600 Subject: [PATCH 21/27] Linter changes --- src/registrar/tests/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 0ff255bca..e23384822 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -207,7 +207,7 @@ class AuditedAdminMockData: In more detail: domain_type specifies what kind of domain object you'd like to create, i.e. domain_application (APPLICATION), or domain_information (INFORMATION). - """ + """ # noqa common_args = self.get_common_domain_arg_dictionary( item_name, org_type, federal_type, purpose ) From 2db74e6b007e667bffe8077b0bafd4730bb0d6c9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 17 Aug 2023 15:32:19 -0600 Subject: [PATCH 22/27] Black changes - would not pass otherwise --- src/registrar/admin.py | 5 +++- .../models/utility/admin_form_order_helper.py | 2 +- src/registrar/tests/common.py | 15 ++++++++--- src/registrar/tests/test_admin.py | 25 ++++++++++++++++--- 4 files changed, 38 insertions(+), 9 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ed16e25b5..63b57138b 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -4,7 +4,10 @@ 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, SortingDict # noqa +from registrar.models.utility.admin_form_order_helper import ( + AdminFormOrderHelper, + SortingDict, +) from . import models logger = logging.getLogger(__name__) diff --git a/src/registrar/models/utility/admin_form_order_helper.py b/src/registrar/models/utility/admin_form_order_helper.py index fe081fc9b..acc26db11 100644 --- a/src/registrar/models/utility/admin_form_order_helper.py +++ b/src/registrar/models/utility/admin_form_order_helper.py @@ -57,7 +57,7 @@ class AdminFormOrderHelper: # 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 + 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/tests/common.py b/src/registrar/tests/common.py index e23384822..49c4d567f 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -10,7 +10,16 @@ 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, DomainInformation, Domain # noqa +from registrar.models import ( + Contact, + DraftDomain, + Website, + DomainApplication, + DomainInvitation, + User, + DomainInformation, + Domain, +) logger = logging.getLogger(__name__) @@ -115,7 +124,7 @@ class AuditedAdminMockData: as a way to optionally include metadata in the string 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 + """ # noqa # Constants for different domain object types INFORMATION = "information" @@ -207,7 +216,7 @@ class AuditedAdminMockData: In more detail: domain_type specifies what kind of domain object you'd like to create, i.e. domain_application (APPLICATION), or domain_information (INFORMATION). - """ # noqa + """ # noqa common_args = self.get_common_domain_arg_dictionary( item_name, org_type, federal_type, purpose ) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 8f039d9bf..a1c8486e7 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1,9 +1,26 @@ from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite -# noqa is used on all three of these as the linter doesn't like the length of this line -from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin, MyUserAdmin, AuditedAdmin # noqa -from registrar.models import DomainApplication, DomainInformation, User, Contact, DomainInvitation # noqa -from .common import completed_application, mock_user, create_superuser, create_user, multiple_unalphabetical_domain_objects # noqa + +from registrar.admin import ( + DomainApplicationAdmin, + ListHeaderAdmin, + MyUserAdmin, + AuditedAdmin, +) +from registrar.models import ( + DomainApplication, + DomainInformation, + User, + Contact, + 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 3beb23262bed8044e1a0d8669507771936dc1893 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 18 Aug 2023 08:51:04 -0600 Subject: [PATCH 23/27] Sort on domain_application --- src/registrar/admin.py | 2 + src/registrar/tests/common.py | 26 +++---- src/registrar/tests/test_admin.py | 119 +++++++++++++++++------------- 3 files changed, 84 insertions(+), 63 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 63b57138b..a2c8e0c09 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -4,6 +4,7 @@ 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.domain_application import DomainApplication from registrar.models.utility.admin_form_order_helper import ( AdminFormOrderHelper, SortingDict, @@ -22,6 +23,7 @@ foreignkey_orderby_dict: list[SortingDict] = [ ), # Handles fields that are sorted by 'name' SortingDict(["domain", "requested_domain"], ["name"]), + SortingDict(['domain_application'], ['requested_domain__name']), ] diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 49c4d567f..ea22303ea 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -245,11 +245,11 @@ class AuditedAdminMockData: return full_arg_list def create_full_dummy_domain_application( - self, object_name, status=DomainApplication.STARTED + self, item_name, status=DomainApplication.STARTED ): """Creates a dummy domain application object""" domain_application_kwargs = self.dummy_kwarg_boilerplate( - self.APPLICATION, object_name, status + self.APPLICATION, item_name, status ) application = DomainApplication.objects.get_or_create( **domain_application_kwargs @@ -257,11 +257,11 @@ class AuditedAdminMockData: return application def create_full_dummy_domain_information( - self, object_name, status=DomainApplication.STARTED + self, item_name, status=DomainApplication.STARTED ): """Creates a dummy domain information object""" domain_application_kwargs = self.dummy_kwarg_boilerplate( - self.INFORMATION, object_name, status + self.INFORMATION, item_name, status ) application = DomainInformation.objects.get_or_create( **domain_application_kwargs @@ -269,11 +269,11 @@ class AuditedAdminMockData: return application def create_full_dummy_domain_invitation( - self, object_name, status=DomainApplication.STARTED + self, item_name, status=DomainApplication.STARTED ): """Creates a dummy domain invitation object""" domain_application_kwargs = self.dummy_kwarg_boilerplate( - self.INVITATION, object_name, status + self.INVITATION, item_name, status ) application = DomainInvitation.objects.get_or_create( **domain_application_kwargs @@ -284,7 +284,7 @@ class AuditedAdminMockData: def create_full_dummy_domain_object( self, domain_type, - object_name, + item_name, has_other_contacts=True, has_current_website=True, has_alternative_gov_domain=True, @@ -295,27 +295,27 @@ class AuditedAdminMockData: match domain_type: case self.APPLICATION: application = self.create_full_dummy_domain_application( - object_name, status + item_name, status ) case self.INVITATION: application = self.create_full_dummy_domain_invitation( - object_name, status + item_name, status ) case self.INFORMATION: application = self.create_full_dummy_domain_information( - object_name, status + 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(object_name, "other") + 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(object_name) + 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(object_name) + alt = self.dummy_alt(item_name) application.alternative_domains.add(alt) return application diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index a1c8486e7..f6ef7feeb 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -418,6 +418,7 @@ class AuditedAdminTest(TestCase): DomainApplication.submitter.field, DomainApplication.investigator.field, DomainApplication.creator.field, + DomainApplication.requested_domain.field, ] # Creates multiple domain applications - review status does not matter @@ -430,62 +431,13 @@ class AuditedAdminTest(TestCase): model_admin = AuditedAdmin(DomainApplication, self.site) - # 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: - # 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" - ) - current_sort_order: Contact = 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.first_name - last = contact.last_name - - 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), - ) - - def test_alphabetically_sorted_fk_fields_domain_information(self): - tested_fields = [ - DomainInformation.authorizing_official.field, - DomainInformation.submitter.field, - DomainInformation.domain.field, - DomainInformation.creator.field, - ] - - # 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: - isNamefield: bool = field == DomainInformation.domain.field + isNamefield: bool = field == DomainApplication.requested_domain.field if isNamefield: sorted_fields = ["name"] else: @@ -523,6 +475,73 @@ class AuditedAdminTest(TestCase): "{} 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: + if not isOtherOrderfield: + first = obj.first_name + last = obj.last_name + elif field_obj == DomainInformation.domain.field: + first = obj.name + last = None + elif field_obj.name == DomainInformation.domain_application.field: + first = obj.requested_domain + last = None + + name_tuple = self.coerced_fk_field_helper(first, last, field_obj.name, ":") + if name_tuple is not None: + logger.debug(name_tuple) + 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] From 0978c91450159ca5a55b920b922f8a11f16921f8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 18 Aug 2023 09:15:12 -0600 Subject: [PATCH 24/27] Test fix --- src/registrar/admin.py | 3 +-- src/registrar/tests/test_admin.py | 18 ++++++++---------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index a2c8e0c09..c1267343e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -4,7 +4,6 @@ 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.domain_application import DomainApplication from registrar.models.utility.admin_form_order_helper import ( AdminFormOrderHelper, SortingDict, @@ -23,7 +22,7 @@ foreignkey_orderby_dict: list[SortingDict] = [ ), # Handles fields that are sorted by 'name' SortingDict(["domain", "requested_domain"], ["name"]), - SortingDict(['domain_application'], ['requested_domain__name']), + SortingDict(["domain_application"], ["requested_domain__name"]), ] diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index f6ef7feeb..8a288eb15 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -11,7 +11,6 @@ from registrar.models import ( DomainApplication, DomainInformation, User, - Contact, DomainInvitation, ) from .common import ( @@ -431,7 +430,6 @@ class AuditedAdminTest(TestCase): 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. @@ -480,8 +478,8 @@ class AuditedAdminTest(TestCase): DomainInformation.authorizing_official.field, DomainInformation.submitter.field, DomainInformation.creator.field, - (DomainInformation.domain.field, ['name']), - (DomainInformation.domain_application.field, ['requested_domain__name']), + (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") @@ -521,19 +519,19 @@ class AuditedAdminTest(TestCase): # 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 - last = None - elif field_obj.name == DomainInformation.domain_application.field: - first = obj.requested_domain - last = None + elif field_obj == DomainInformation.domain_application.field: + first = obj.requested_domain.name - name_tuple = self.coerced_fk_field_helper(first, last, field_obj.name, ":") + name_tuple = self.coerced_fk_field_helper( + first, last, field_obj.name, ":" + ) if name_tuple is not None: - logger.debug(name_tuple) current_sort_order_coerced_type.append(name_tuple) self.assertEqual( From 74571069c3378205fbe9b091c7fd744859980bf2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 18 Aug 2023 15:50:44 -0600 Subject: [PATCH 25/27] AdminSortFields class --- src/registrar/admin.py | 31 ++----------------- .../models/utility/admin_sort_fields.py | 27 ++++++++++++++++ 2 files changed, 30 insertions(+), 28 deletions(-) create mode 100644 src/registrar/models/utility/admin_sort_fields.py diff --git a/src/registrar/admin.py b/src/registrar/admin.py index c1267343e..0fa019a13 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -4,29 +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_form_order_helper import ( - AdminFormOrderHelper, - SortingDict, -) +from registrar.models.utility.admin_sort_fields import AdminSortFields from . import models logger = logging.getLogger(__name__) -# 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"]), -] - -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): @@ -42,7 +26,7 @@ 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 form_field_order_helper(form_field, db_field) + return self.form_field_order_helper(form_field, db_field) class ListHeaderAdmin(AuditedAdmin): @@ -323,15 +307,6 @@ class DomainApplicationAdmin(ListHeaderAdmin): 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)""" - - 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) 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) From d8ca745172168ba99ceb61e9b911406288c726d3 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 22 Aug 2023 08:31:26 -0600 Subject: [PATCH 26/27] Comment changes for common.py --- src/registrar/tests/common.py | 144 ++++++++++++++++++++++++++-------- 1 file changed, 113 insertions(+), 31 deletions(-) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index ea22303ea..fd99b56ea 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -111,7 +111,7 @@ class AuditedAdminMockData: Two variables are used across multiple functions: - *item_name* - Used in patterning. Will be appended en masse to multiple string fields, + *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', @@ -121,7 +121,7 @@ class AuditedAdminMockData: 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 string itself. Can be further expanded on. + 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 @@ -152,23 +152,64 @@ class AuditedAdminMockData: )[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_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): - """Creates a dummy domain object""" - return Domain.objects.get_or_create(name="city{}.gov".format(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""" - return Website.objects.get_or_create(website="cityalt{}.gov".format(item_name))[ - 0 - ] + """ + 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""" - return Website.objects.get_or_create(website="city{}.com".format(item_name))[0] + """ + 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, @@ -177,7 +218,36 @@ class AuditedAdminMockData: federal_type="executive", purpose="Purpose of the site", ): - """Generates a generic argument list for most domains""" + """ + 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, @@ -200,30 +270,42 @@ class AuditedAdminMockData: self, domain_type, item_name, - status, + status=DomainApplication.STARTED, org_type="federal", federal_type="executive", purpose="Purpose of the site", ): """ - A helper function that returns premade kwargs for easily creating different domain object types. - There is a decent amount of boilerplate associated with - creating new domain objects (such as domain_application, or domain_information), - so for test case purposes, we can make some assumptions and utilize that to simplify - the object creation process. + Returns a prebuilt kwarg dictionary for DomainApplication, + DomainInformation, or DomainInvitation. + Args: + domain_type (str): is either 'application', 'information', + or 'invitation'. - *domain_type* uses constants. Used to identify what kind of 'Domain' object you'd like to make. + item_name (str): A shared str value appended to first_name, last_name, + organization_name, address_line1, address_line2, + title, email, and username. - In more detail: domain_type specifies what kind of domain object you'd like to create, i.e. - domain_application (APPLICATION), or domain_information (INFORMATION). + 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_list = None + full_arg_dict = None match domain_type: case self.APPLICATION: - full_arg_list = dict( + full_arg_dict = dict( **common_args, requested_domain=self.dummy_draft_domain(item_name), investigator=self.dummy_user(item_name, "investigator"), @@ -231,18 +313,18 @@ class AuditedAdminMockData: ) case self.INFORMATION: domain_app = self.create_full_dummy_domain_application(item_name) - full_arg_list = dict( + full_arg_dict = dict( **common_args, - domain=self.dummy_domain(item_name), + domain=self.dummy_domain(item_name, True), domain_application=domain_app, ) case self.INVITATION: - full_arg_list = dict( + full_arg_dict = dict( email="test_mail@mail.com", - domain=self.dummy_domain(item_name), + domain=self.dummy_domain(item_name, True), status=DomainInvitation.INVITED, ) - return full_arg_list + return full_arg_dict def create_full_dummy_domain_application( self, item_name, status=DomainApplication.STARTED From 54fe5384be6fcaa1beb7c6355b4f5bf73419b1b8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 22 Aug 2023 08:34:38 -0600 Subject: [PATCH 27/27] Linter --- src/registrar/tests/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index fd99b56ea..c4a2772b0 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -247,7 +247,7 @@ class AuditedAdminMockData: submitter: Contact = self.dummy_contact(item_name, "submitter"), creator: User = self.dummy_user(item_name, "creator"), } - """ # noqa + """ # noqa common_args = dict( organization_type=org_type, federal_type=federal_type,