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/43] 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/43] 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/43] (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/43] 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/43] 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/43] 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/43] 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/43] 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/43] 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/43] 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/43] 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/43] 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 765502f322b0de8c5dac96926d7b60674567bc74 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Mon, 14 Aug 2023 14:48:16 -0400 Subject: [PATCH 13/43] Edit user fictures to add optional emails --- docs/developer/README.md | 2 ++ src/registrar/fixtures.py | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/docs/developer/README.md b/docs/developer/README.md index b6938298c..9ef62bba9 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -84,6 +84,7 @@ The endpoint /admin can be used to view and manage site content, including but n ``` 5. In the browser, navigate to /admin. To verify that all is working correctly, under "domain applications" you should see fake domains with various fake statuses. +6. Add an optional email key/value pair ### Adding an Analyst to /admin Analysts are a variant of the admin role with limited permissions. The process for adding an Analyst is much the same as adding an admin: @@ -105,6 +106,7 @@ Analysts are a variant of the admin role with limited permissions. The process f ``` 5. In the browser, navigate to /admin. To verify that all is working correctly, verify that you can only see a sub-section of the modules and some are set to view-only. +6. Add an optional email key/value pair Do note that if you wish to have both an analyst and admin account, append `-Analyst` to your first and last name, or use a completely different first/last name to avoid confusion. Example: `Bob-Analyst` ## Adding to CODEOWNERS (optional) diff --git a/src/registrar/fixtures.py b/src/registrar/fixtures.py index 2c94a1eb4..2291001de 100644 --- a/src/registrar/fixtures.py +++ b/src/registrar/fixtures.py @@ -79,6 +79,7 @@ class UserFixture: "username": "319c490d-453b-43d9-bc4d-7d6cd8ff6844", "first_name": "Rachid-Analyst", "last_name": "Mrad-Analyst", + "email": "rachid.mrad@gmail.com", }, { "username": "b6a15987-5c88-4e26-8de2-ca71a0bdb2cd", @@ -129,6 +130,8 @@ class UserFixture: user.is_superuser = True user.first_name = admin["first_name"] user.last_name = admin["last_name"] + if "email" in admin.keys(): + user.email = admin["email"] user.is_staff = True user.is_active = True user.save() @@ -146,6 +149,8 @@ class UserFixture: user.is_superuser = False user.first_name = staff["first_name"] user.last_name = staff["last_name"] + if "email" in admin.keys(): + user.email = admin["email"] user.is_staff = True user.is_active = True From 5e24e3473db83d419e7221318cd1cacdef3de43c Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Wed, 16 Aug 2023 14:05:52 -0400 Subject: [PATCH 14/43] lint --- src/registrar/fixtures.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/fixtures.py b/src/registrar/fixtures.py index 2291001de..0b1b8926d 100644 --- a/src/registrar/fixtures.py +++ b/src/registrar/fixtures.py @@ -130,7 +130,7 @@ class UserFixture: user.is_superuser = True user.first_name = admin["first_name"] user.last_name = admin["last_name"] - if "email" in admin.keys(): + if "email" in admin.keys(): user.email = admin["email"] user.is_staff = True user.is_active = True @@ -149,7 +149,7 @@ class UserFixture: user.is_superuser = False user.first_name = staff["first_name"] user.last_name = staff["last_name"] - if "email" in admin.keys(): + if "email" in admin.keys(): user.email = admin["email"] user.is_staff = True user.is_active = True 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 15/43] 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 16/43] 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 17/43] 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 18/43] 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 a0f50465f8604822b7df10a411bcca49242b32d5 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 16 Aug 2023 15:30:59 -0600 Subject: [PATCH 19/43] Update home.html --- src/registrar/templates/home.html | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index 792beec43..a1a54376a 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -43,13 +43,13 @@ {{ domain.application_status|title }} - Manage {{ domain.name }} @@ -115,14 +115,16 @@ aria-live="polite" > {% else %} -

You don't have any active domain requests right now

+

You don't have any active domain requests right now

{% endif %}

Start a new domain request

+ {# Note: Reimplement this later after MVP #} + {% else %} {# not user.is_authenticated #} From f4b4c2e0e31df0bd025b460c223a5e019cfbe936 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 16 Aug 2023 15:34:56 -0600 Subject: [PATCH 20/43] Merge was a bit funky, fixing --- src/registrar/templates/home.html | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index a24ae97d6..6daaf35ba 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -126,6 +126,7 @@

Archived domains

You don't have any archived domains

+ --> - + + --> {% else %} {# not user.is_authenticated #} From e384b03d3610c1e6fc9b3bf9daf6235fd2138a23 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 17 Aug 2023 07:34:12 -0600 Subject: [PATCH 21/43] Domain Request Button --- src/registrar/templates/home.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index 6daaf35ba..b2f784ebd 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -116,8 +116,8 @@ > {% else %}

You don't have any active domain requests right now

- {% endif %}

Start a new domain request

+ {% endif %} {# Note: Reimplement this later after MVP #} From 16c70d555c04d2cc3636193b69cdfda528028f1b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 17 Aug 2023 08:38:09 -0600 Subject: [PATCH 22/43] Verbiage change --- src/registrar/templates/home.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index b2f784ebd..86fefef4a 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -120,7 +120,7 @@ {% endif %} - {# Note: Reimplement this later after MVP #} + {# Note: Reimplement this after MVP #}