From 2891391382d9ca227f0d43960df851ea1d949b4a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 2 Feb 2024 15:33:42 -0700 Subject: [PATCH 01/55] Backend stuff --- src/registrar/admin.py | 21 +++++++++++++++++++++ src/registrar/models/domain.py | 19 +++++++++++-------- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 77d827d05..8e2f7af68 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1,3 +1,4 @@ +from datetime import date import logging from django import forms from django.db.models.functions import Concat @@ -1133,6 +1134,7 @@ class DomainAdmin(ListHeaderAdmin): "_edit_domain": self.do_edit_domain, "_delete_domain": self.do_delete_domain, "_get_status": self.do_get_status, + "_extend_expiration_date": self.do_extend_expiration_date } # Check which action button was pressed and call the corresponding function @@ -1143,6 +1145,25 @@ class DomainAdmin(ListHeaderAdmin): # If no matching action button is found, return the super method return super().response_change(request, obj) + def do_extend_expiration_date(self, request, obj): + if not isinstance(obj, Domain): + # Could be problematic if the type is similar, + # but not the same (same field/func names). + # We do not want to accidentally delete records. + self.message_user(request, "Object is not of type Domain", messages.ERROR) + return None + try: + obj.renew_domain(date_to_extend=date.today()) + except Exception as err: + self.message_user(request, err, messages.ERROR) + else: + updated_domain = Domain.objects.filter(id=obj).get() + self.message_user( + request, + f"Successfully extended expiration date to {updated_domain.registry_expiration_date}", + ) + return HttpResponseRedirect(".") + def do_delete_domain(self, request, obj): if not isinstance(obj, Domain): # Could be problematic if the type is similar, diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 27a8364bc..84f451f56 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -239,22 +239,25 @@ class Domain(TimeStampedModel, DomainHelper): To update the expiration date, use renew_domain method.""" raise NotImplementedError() - def renew_domain(self, length: int = 1, unit: epp.Unit = epp.Unit.YEAR): + def renew_domain(self, length: int = 1, date_to_extend = None, unit: epp.Unit = epp.Unit.YEAR): """ Renew the domain to a length and unit of time relative to the current expiration date. Default length and unit of time are 1 year. """ - # if no expiration date from registry, set to today - try: - cur_exp_date = self.registry_expiration_date - except KeyError: - logger.warning("current expiration date not set; setting to today") - cur_exp_date = date.today() + + # If no date is specified, grab the registry_expiration_date + if date_to_extend is None: + try: + date_to_extend = self.registry_expiration_date + except KeyError: + # if no expiration date from registry, set it to today + logger.warning("current expiration date not set; setting to today") + date_to_extend = date.today() # create RenewDomain request - request = commands.RenewDomain(name=self.name, cur_exp_date=cur_exp_date, period=epp.Period(length, unit)) + request = commands.RenewDomain(name=self.name, cur_exp_date=date_to_extend, period=epp.Period(length, unit)) try: # update expiration date in registry, and set the updated From b3401d8bfc492b1b5d09f7c62b028107df274d18 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 2 Feb 2024 15:39:51 -0700 Subject: [PATCH 02/55] Basic button --- src/registrar/admin.py | 3 ++- src/registrar/templates/django/admin/domain_change_form.html | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8e2f7af68..0571a1743 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1134,7 +1134,7 @@ class DomainAdmin(ListHeaderAdmin): "_edit_domain": self.do_edit_domain, "_delete_domain": self.do_delete_domain, "_get_status": self.do_get_status, - "_extend_expiration_date": self.do_extend_expiration_date + "_extend_expiration_date": self.do_extend_expiration_date, } # Check which action button was pressed and call the corresponding function @@ -1152,6 +1152,7 @@ class DomainAdmin(ListHeaderAdmin): # We do not want to accidentally delete records. self.message_user(request, "Object is not of type Domain", messages.ERROR) return None + try: obj.renew_domain(date_to_extend=date.today()) except Exception as err: diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index c4461d07f..bf2be1754 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -5,6 +5,7 @@
+
{% if original.state == original.State.READY %} From aff6aad1d493aaa978f77d9673618bc526c9bec2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 6 Feb 2024 10:40:24 -0700 Subject: [PATCH 03/55] Add some additional errors --- src/registrar/admin.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 0571a1743..74424b56a 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1156,7 +1156,21 @@ class DomainAdmin(ListHeaderAdmin): try: obj.renew_domain(date_to_extend=date.today()) except Exception as err: - self.message_user(request, err, messages.ERROR) + if err.code: + self.message_user( + request, + f"Error extending this domain: {err}", + messages.ERROR, + ) + elif err.is_connection_error(): + self.message_user( + request, + "Error connecting to the registry", + messages.ERROR, + ) + else: + # all other type error messages, display the error + self.message_user(request, err, messages.ERROR) else: updated_domain = Domain.objects.filter(id=obj).get() self.message_user( From 8d476d5a80ef3fa8e93a1653b21cda5fd2a76723 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 6 Feb 2024 12:58:44 -0700 Subject: [PATCH 04/55] Better logging --- src/registrar/admin.py | 8 +++----- src/registrar/models/domain.py | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 74424b56a..7c164ae27 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1147,15 +1147,12 @@ class DomainAdmin(ListHeaderAdmin): def do_extend_expiration_date(self, request, obj): if not isinstance(obj, Domain): - # Could be problematic if the type is similar, - # but not the same (same field/func names). - # We do not want to accidentally delete records. self.message_user(request, "Object is not of type Domain", messages.ERROR) return None try: obj.renew_domain(date_to_extend=date.today()) - except Exception as err: + except RegistryError as err: if err.code: self.message_user( request, @@ -1169,8 +1166,9 @@ class DomainAdmin(ListHeaderAdmin): messages.ERROR, ) else: - # all other type error messages, display the error self.message_user(request, err, messages.ERROR) + except Exception as err: + self.message_user(request, err, messages.ERROR) else: updated_domain = Domain.objects.filter(id=obj).get() self.message_user( diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 84f451f56..74461f4c9 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -255,7 +255,7 @@ class Domain(TimeStampedModel, DomainHelper): # if no expiration date from registry, set it to today logger.warning("current expiration date not set; setting to today") date_to_extend = date.today() - + print(f"This is the date to extend: {date_to_extend} vs registry {self.registry_expiration_date}") # create RenewDomain request request = commands.RenewDomain(name=self.name, cur_exp_date=date_to_extend, period=epp.Period(length, unit)) From b91572f9d301908dd55c8f4e7cf10640cddfe431 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 6 Feb 2024 13:06:56 -0700 Subject: [PATCH 05/55] Update domain.py --- src/registrar/models/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 74461f4c9..577786dd0 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -255,7 +255,7 @@ class Domain(TimeStampedModel, DomainHelper): # if no expiration date from registry, set it to today logger.warning("current expiration date not set; setting to today") date_to_extend = date.today() - print(f"This is the date to extend: {date_to_extend} vs registry {self.registry_expiration_date}") + logger.info(f"This is the date to extend: {date_to_extend} vs registry {self.registry_expiration_date}") # create RenewDomain request request = commands.RenewDomain(name=self.name, cur_exp_date=date_to_extend, period=epp.Period(length, unit)) From 0ab48468dc1f870383928cc422fd546e74b34dfd Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 6 Feb 2024 15:28:27 -0700 Subject: [PATCH 06/55] Import USWDS Experimental changes --- src/registrar/templates/admin/base_site.html | 3 ++- .../templates/django/admin/domain_change_form.html | 13 +++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/registrar/templates/admin/base_site.html b/src/registrar/templates/admin/base_site.html index c0884c912..da77c98d3 100644 --- a/src/registrar/templates/admin/base_site.html +++ b/src/registrar/templates/admin/base_site.html @@ -18,11 +18,12 @@ + + {% endblock %} {% block title %}{% if subtitle %}{{ subtitle }} | {% endif %}{{ title }} | {{ site_title|default:_('Django site admin') }}{% endblock %} - {% block extrastyle %}{{ block.super }} {% endblock %} diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index bf2be1754..c67d95235 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -6,6 +6,19 @@ + Disable DNSSEC +
+ {% include 'includes/modal.html' with modal_heading="Are you sure you want to disable DNSSEC?" modal_button=modal_button|safe %} +
{% if original.state == original.State.READY %} From f53df4f150f7df094c583995167128feeea5a99a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 7 Feb 2024 10:42:00 -0700 Subject: [PATCH 07/55] Add popup --- src/registrar/admin.py | 29 +++++++++-- src/registrar/assets/js/get-gov-admin.js | 23 +++++++++ src/registrar/assets/sass/_theme/_admin.scss | 6 +++ .../django/admin/domain_change_form.html | 48 +++++++++++++++++-- 4 files changed, 96 insertions(+), 10 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7c164ae27..ed5f143bb 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1136,7 +1136,7 @@ class DomainAdmin(ListHeaderAdmin): "_get_status": self.do_get_status, "_extend_expiration_date": self.do_extend_expiration_date, } - + print(f"this is the response! {request.POST}") # Check which action button was pressed and call the corresponding function for action, function in ACTION_FUNCTIONS.items(): if action in request.POST: @@ -1147,7 +1147,7 @@ class DomainAdmin(ListHeaderAdmin): def do_extend_expiration_date(self, request, obj): if not isinstance(obj, Domain): - self.message_user(request, "Object is not of type Domain", messages.ERROR) + self.message_user(request, "Object is not of type Domain.", messages.ERROR) return None try: @@ -1156,24 +1156,32 @@ class DomainAdmin(ListHeaderAdmin): if err.code: self.message_user( request, - f"Error extending this domain: {err}", + f"Error extending this domain: {err}.", messages.ERROR, ) elif err.is_connection_error(): self.message_user( request, - "Error connecting to the registry", + "Error connecting to the registry.", messages.ERROR, ) else: self.message_user(request, err, messages.ERROR) + except KeyError: + # In normal code flow, a keyerror can only occur when + # fresh data can't be pulled from the registry, and thus there is no cache. + self.message_user( + request, + "Error connecting to the registry. No expiration date was found.", + messages.ERROR, + ) except Exception as err: self.message_user(request, err, messages.ERROR) else: updated_domain = Domain.objects.filter(id=obj).get() self.message_user( request, - f"Successfully extended expiration date to {updated_domain.registry_expiration_date}", + f"Successfully extended expiration date to {updated_domain.registry_expiration_date}.", ) return HttpResponseRedirect(".") @@ -1328,6 +1336,17 @@ class DomainAdmin(ListHeaderAdmin): return True return super().has_change_permission(request, obj) + def changelist_view(self, request, extra_context=None): + extra_context = extra_context or {} + # Create HTML for the modal button + modal_button = ( + '' + ) + extra_context["modal_button"] = modal_button + return super().changelist_view(request, extra_context=extra_context) + class DraftDomainAdmin(ListHeaderAdmin): """Custom draft domain admin class.""" diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 866c7bd7d..007ded215 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -44,6 +44,29 @@ function openInNewTab(el, removeAttribute = false){ domainSubmitButton.addEventListener("mouseover", () => openInNewTab(domainFormElement, true)); domainSubmitButton.addEventListener("mouseout", () => openInNewTab(domainFormElement, false)); } + + let extendExpirationDateButton = document.getElementById("extend_expiration_date_button") + if (extendExpirationDateButton){ + extendExpirationDateButton.addEventListener("click", () => { + form = document.getElementById("domain_form") + /* + For some reason, Django admin has the propensity to ignore nested + inputs and delete nested form objects. + The workaround is to manually create an element as so, after the DOM + has been generated. + + Its not the most beautiful thing every, but it works. + */ + var input = document.createElement("input"); + input.type = "hidden"; + input.name = "_extend_expiration_date"; + // The value doesn't matter, just needs to be present + input.value = "1"; + // Add the hidden input to the form + form.appendChild(input); + form.submit(); + }) + } } prepareDjangoAdmin(); diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 760c4f13a..4c0a1f7cc 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -270,3 +270,9 @@ h1, h2, h3, margin: 0!important; } } + +// Button groups in /admin incorrectly have bullets. +// Remove that! +.usa-button-group .usa-button-group__item { + list-style-type: none; +} diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index c67d95235..bf8a0de2e 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -5,19 +5,57 @@
- + {{ modal_button }} Disable DNSSEC + > + Extend expiration date +
- {% include 'includes/modal.html' with modal_heading="Are you sure you want to disable DNSSEC?" modal_button=modal_button|safe %} +
+
+

+ Are you sure you want to extend this expiration date? +

+
+ +
+ +
+
    +
  • + +
    • +
  • + +
    • +
+
+
+
+ {# {% include 'includes/modal.html' with modal_heading="Are you sure you want to extend this expiration date?" modal_button=modal_button|safe %} #}
{% if original.state == original.State.READY %} From c9957bcd2c381a811179e9f4c4990635c3df4a4d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 7 Feb 2024 10:54:22 -0700 Subject: [PATCH 08/55] Hotfix --- src/registrar/admin.py | 2 +- src/registrar/models/domain.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ed5f143bb..de89e0150 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1136,7 +1136,7 @@ class DomainAdmin(ListHeaderAdmin): "_get_status": self.do_get_status, "_extend_expiration_date": self.do_extend_expiration_date, } - print(f"this is the response! {request.POST}") + # Check which action button was pressed and call the corresponding function for action, function in ACTION_FUNCTIONS.items(): if action in request.POST: diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 577786dd0..db17a29b4 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -246,7 +246,7 @@ class Domain(TimeStampedModel, DomainHelper): Default length and unit of time are 1 year. """ - + logger.info(f"This is the date to extend: {date_to_extend}") # If no date is specified, grab the registry_expiration_date if date_to_extend is None: try: @@ -255,7 +255,7 @@ class Domain(TimeStampedModel, DomainHelper): # if no expiration date from registry, set it to today logger.warning("current expiration date not set; setting to today") date_to_extend = date.today() - logger.info(f"This is the date to extend: {date_to_extend} vs registry {self.registry_expiration_date}") + # create RenewDomain request request = commands.RenewDomain(name=self.name, cur_exp_date=date_to_extend, period=epp.Period(length, unit)) From 68b6c8b46a5f479fb6e229d5b47ffb74da4aa233 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 7 Feb 2024 14:25:00 -0700 Subject: [PATCH 09/55] Update domain.py --- src/registrar/models/domain.py | 36 ++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index db17a29b4..be4c61301 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -239,25 +239,41 @@ class Domain(TimeStampedModel, DomainHelper): To update the expiration date, use renew_domain method.""" raise NotImplementedError() - def renew_domain(self, length: int = 1, date_to_extend = None, unit: epp.Unit = epp.Unit.YEAR): + def renew_domain(self, length: int = 1, unit: epp.Unit = epp.Unit.YEAR, extend_year_past_current_date = False): """ Renew the domain to a length and unit of time relative to the current expiration date. Default length and unit of time are 1 year. + + extend_past_current_date (bool): Specifies if the "desired" date + should exceed the present date + length. For instance, """ - logger.info(f"This is the date to extend: {date_to_extend}") + # If no date is specified, grab the registry_expiration_date - if date_to_extend is None: - try: - date_to_extend = self.registry_expiration_date - except KeyError: - # if no expiration date from registry, set it to today - logger.warning("current expiration date not set; setting to today") - date_to_extend = date.today() + try: + exp_date = self.registry_expiration_date + except KeyError: + # if no expiration date from registry, set it to today + logger.warning("current expiration date not set; setting to today") + exp_date = date.today() + + if extend_year_past_current_date: + # TODO - handle unit == month + expected_renewal_year = exp_date.year + length + current_year = date.today().year + if expected_renewal_year < current_year: + # Modify the length such that it will exceed the current year by the length + length = (current_year - exp_date.year) + length + # length = (current_year - expected_renewal_year) + length * 2 + elif expected_renewal_year == current_year: + # In the event that the expected renewal date will equal the current year, + # we need to apply double "length" for it shoot past the current date + # at the correct interval. + length = length * 2 # create RenewDomain request - request = commands.RenewDomain(name=self.name, cur_exp_date=date_to_extend, period=epp.Period(length, unit)) + request = commands.RenewDomain(name=self.name, cur_exp_date=exp_date, period=epp.Period(length, unit)) try: # update expiration date in registry, and set the updated From 942422b7bf150bc5548295e47d2a0527ceffe444 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 7 Feb 2024 15:23:49 -0700 Subject: [PATCH 10/55] Add logic to extend from current date --- src/registrar/admin.py | 34 +++++++++++++++++++++++++++++++++- src/registrar/models/domain.py | 5 +++-- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index de89e0150..d505f71d8 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3,6 +3,7 @@ import logging from django import forms from django.db.models.functions import Concat from django.http import HttpResponse +from dateutil.relativedelta import relativedelta from django.shortcuts import redirect from django_fsm import get_available_FIELD_transitions from django.contrib import admin, messages @@ -23,6 +24,9 @@ from auditlog.admin import LogEntryAdmin # type: ignore from django_fsm import TransitionNotAllowed # type: ignore from django.utils.safestring import mark_safe from django.utils.html import escape +from epplibwrapper import ( + common as epp, +) logger = logging.getLogger(__name__) @@ -1151,7 +1155,17 @@ class DomainAdmin(ListHeaderAdmin): return None try: - obj.renew_domain(date_to_extend=date.today()) + exp_date = obj.registry_expiration_date + except KeyError: + # if no expiration date from registry, set it to today + logger.warning("current expiration date not set; setting to today") + exp_date = date.today() + + desired_date = exp_date + relativedelta(years=1) + month_length = self._month_diff(desired_date, exp_date) + + try: + obj.renew_domain(length=month_length, unit=epp.Unit.MONTH) except RegistryError as err: if err.code: self.message_user( @@ -1185,6 +1199,24 @@ class DomainAdmin(ListHeaderAdmin): ) return HttpResponseRedirect(".") + def _month_diff(self, date_1, date_2): + """ + Calculate the difference in months between two dates using dateutil's relativedelta. + + :param date_1: The first date. + :param date_2: The second date. + :return: The difference in months as an integer. + """ + # Ensure date_1 is always the earlier date + start_date, end_date = sorted([date_1, date_2]) + + # Grab the delta between the two + rdelta = relativedelta(end_date, start_date) + + # Calculate total months as years * 12 + months + total_months = rdelta.years * 12 + rdelta.months + return total_months + def do_delete_domain(self, request, obj): if not isinstance(obj, Domain): # Could be problematic if the type is similar, diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index be4c61301..c72487761 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -257,10 +257,11 @@ class Domain(TimeStampedModel, DomainHelper): # if no expiration date from registry, set it to today logger.warning("current expiration date not set; setting to today") exp_date = date.today() - + """ if extend_year_past_current_date: # TODO - handle unit == month expected_renewal_year = exp_date.year + length + current_year = date.today().year if expected_renewal_year < current_year: # Modify the length such that it will exceed the current year by the length @@ -271,7 +272,7 @@ class Domain(TimeStampedModel, DomainHelper): # we need to apply double "length" for it shoot past the current date # at the correct interval. length = length * 2 - + """ # create RenewDomain request request = commands.RenewDomain(name=self.name, cur_exp_date=exp_date, period=epp.Period(length, unit)) From 1f1a537752f1ab3f72c9e2cfb582ac196548e762 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 7 Feb 2024 15:25:52 -0700 Subject: [PATCH 11/55] Fix bad logic --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index d505f71d8..13ecaeaab 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1161,7 +1161,7 @@ class DomainAdmin(ListHeaderAdmin): logger.warning("current expiration date not set; setting to today") exp_date = date.today() - desired_date = exp_date + relativedelta(years=1) + desired_date = date.today() + relativedelta(years=1) month_length = self._month_diff(desired_date, exp_date) try: From f46988ef675886da06abd23ae84ab110542c7461 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 7 Feb 2024 15:35:57 -0700 Subject: [PATCH 12/55] Add logging --- src/registrar/admin.py | 5 +- .../django/admin/domain_change_form.html | 82 ++++++++++--------- 2 files changed, 46 insertions(+), 41 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 13ecaeaab..fc3dd861b 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1162,9 +1162,11 @@ class DomainAdmin(ListHeaderAdmin): exp_date = date.today() desired_date = date.today() + relativedelta(years=1) - month_length = self._month_diff(desired_date, exp_date) + logger.info(f"do_extend_expiration_date -> exp {exp_date} des {desired_date}") + month_length = self._month_diff(exp_date, desired_date) try: + logger.info(f"do_extend_expiration_date -> month length: {month_length}") obj.renew_domain(length=month_length, unit=epp.Unit.MONTH) except RegistryError as err: if err.code: @@ -1212,6 +1214,7 @@ class DomainAdmin(ListHeaderAdmin): # Grab the delta between the two rdelta = relativedelta(end_date, start_date) + logger.info(f"rdelta is: {rdelta}, years {rdelta.years}, months {rdelta.months}") # Calculate total months as years * 12 + months total_months = rdelta.years * 12 + rdelta.months diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index bf8a0de2e..a07c514f3 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -13,48 +13,50 @@ > Extend expiration date -
-
-
-

- Are you sure you want to extend this expiration date? -

-
- -
- -
-
    -
  • - -
    • -
  • - -
    • -
+ {% if original.state != original.State.DELETED %} +
+
+
+

+ Are you sure you want to extend this expiration date? +

+
+ +
+ +
+
    +
  • + +
    • +
  • + +
    • +
+
-
+ {% endif %} {# {% include 'includes/modal.html' with modal_heading="Are you sure you want to extend this expiration date?" modal_button=modal_button|safe %} #}
From 6c909dcc64846941877dbcc5d21c5ad4c9237b48 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 7 Feb 2024 15:39:29 -0700 Subject: [PATCH 13/55] Update admin.py --- src/registrar/admin.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index fc3dd861b..c17e4e514 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1163,7 +1163,11 @@ class DomainAdmin(ListHeaderAdmin): desired_date = date.today() + relativedelta(years=1) logger.info(f"do_extend_expiration_date -> exp {exp_date} des {desired_date}") - month_length = self._month_diff(exp_date, desired_date) + + # Get the difference in months between the expiration date, and the + # desired date (today + 1). Then, add one year to that. + one_year = 12 + month_length = self._month_diff(exp_date, desired_date) + one_year try: logger.info(f"do_extend_expiration_date -> month length: {month_length}") From 7b878c2195c003ce74007f8eacb51d5b51646e7d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 7 Feb 2024 15:54:38 -0700 Subject: [PATCH 14/55] Temp changes --- src/registrar/admin.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index c17e4e514..bef82cdfd 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1166,12 +1166,23 @@ class DomainAdmin(ListHeaderAdmin): # Get the difference in months between the expiration date, and the # desired date (today + 1). Then, add one year to that. + # TODO - error: Periods for domain registrations must be specified in years.??? one_year = 12 month_length = self._month_diff(exp_date, desired_date) + one_year try: logger.info(f"do_extend_expiration_date -> month length: {month_length}") - obj.renew_domain(length=month_length, unit=epp.Unit.MONTH) + # TODO why cant I specify months + #obj.renew_domain(length=month_length, unit=epp.Unit.MONTH) + years = month_length/12 + if years >= 1: + obj.renew_domain(length=month_length/12) + else: + self.message_user( + request, + f"Error extending this domain: Can't extend date by 0 years.", + messages.ERROR, + ) except RegistryError as err: if err.code: self.message_user( From 8700a05fbf6a55089fb5792c603a9b34bb606e8b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 7 Feb 2024 16:02:57 -0700 Subject: [PATCH 15/55] Change years to int Temp changes because I cant pass in months specifically --- src/registrar/admin.py | 2 +- .../django/admin/domain_change_form.html | 84 +++++++++---------- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index bef82cdfd..de8e11606 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1174,7 +1174,7 @@ class DomainAdmin(ListHeaderAdmin): logger.info(f"do_extend_expiration_date -> month length: {month_length}") # TODO why cant I specify months #obj.renew_domain(length=month_length, unit=epp.Unit.MONTH) - years = month_length/12 + years = int(month_length/12) if years >= 1: obj.renew_domain(length=month_length/12) else: diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index a07c514f3..8ee7605a5 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -6,6 +6,7 @@ {{ modal_button }} + {% if original.state != original.State.DELETED %} Extend expiration date - {% if original.state != original.State.DELETED %} -
-
-
-

- Are you sure you want to extend this expiration date? -

-
- -
- -
-
    -
  • - -
    • -
  • - -
    • -
-
+
+
+
+

+ Are you sure you want to extend this expiration date? +

+
+ +
+ +
+
    +
  • + +
    • +
  • + +
    • +
- {% endif %} +
+ {% endif %} {# {% include 'includes/modal.html' with modal_heading="Are you sure you want to extend this expiration date?" modal_button=modal_button|safe %} #}
From 83269447aed7a18b122f851ff421bbe38fa8bbcd Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 7 Feb 2024 16:10:45 -0700 Subject: [PATCH 16/55] Update admin.py --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index de8e11606..b266c0c57 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1176,7 +1176,7 @@ class DomainAdmin(ListHeaderAdmin): #obj.renew_domain(length=month_length, unit=epp.Unit.MONTH) years = int(month_length/12) if years >= 1: - obj.renew_domain(length=month_length/12) + obj.renew_domain(length=years) else: self.message_user( request, From abdc8fa7fdbd726db62ab7a015e56e90f8b9c97a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Feb 2024 09:08:51 -0700 Subject: [PATCH 17/55] Change to years --- src/registrar/admin.py | 13 ++++++++----- .../templates/django/admin/domain_change_form.html | 4 ++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index b266c0c57..5b756f689 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1167,14 +1167,17 @@ class DomainAdmin(ListHeaderAdmin): # Get the difference in months between the expiration date, and the # desired date (today + 1). Then, add one year to that. # TODO - error: Periods for domain registrations must be specified in years.??? - one_year = 12 - month_length = self._month_diff(exp_date, desired_date) + one_year + # one_year = 12 + # month_length = self._month_diff(exp_date, desired_date) + one_year + years = 1 + if desired_date > exp_date: + years = (desired_date.year - exp_date.year) try: - logger.info(f"do_extend_expiration_date -> month length: {month_length}") + # logger.info(f"do_extend_expiration_date -> month length: {month_length}") + logger.info(f"do_extend_expiration_date -> years {years}") # TODO why cant I specify months #obj.renew_domain(length=month_length, unit=epp.Unit.MONTH) - years = int(month_length/12) if years >= 1: obj.renew_domain(length=years) else: @@ -1209,7 +1212,7 @@ class DomainAdmin(ListHeaderAdmin): except Exception as err: self.message_user(request, err, messages.ERROR) else: - updated_domain = Domain.objects.filter(id=obj).get() + updated_domain = Domain.objects.filter(id=obj.id).get() self.message_user( request, f"Successfully extended expiration date to {updated_domain.registry_expiration_date}.", diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index 8ee7605a5..2dddc4dcb 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -60,6 +60,10 @@ {# {% include 'includes/modal.html' with modal_heading="Are you sure you want to extend this expiration date?" modal_button=modal_button|safe %} #}
+ {% if original.state != original.State.DELETED %} + + {% endif %} + | {% if original.state == original.State.READY %} {% elif original.state == original.State.ON_HOLD %} From 65b7091a8fc230f2c1070b9cf63647eb2dc8cd9b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Feb 2024 10:38:28 -0700 Subject: [PATCH 18/55] Add USWDS classes for spacing, etc --- src/registrar/assets/sass/_theme/_admin.scss | 6 ++ .../django/admin/domain_change_form.html | 93 +++++-------------- 2 files changed, 28 insertions(+), 71 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 4c0a1f7cc..098e9bb81 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -271,6 +271,12 @@ h1, h2, h3, } } +@include at-media(mobile){ + .button-list-mobile { + display: contents; + } +} + // Button groups in /admin incorrectly have bullets. // Remove that! .usa-button-group .usa-button-group__item { diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index 2dddc4dcb..6420f8b5f 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -2,79 +2,30 @@ {% load i18n static %} {% block field_sets %} -
- - - {{ modal_button }} - {% if original.state != original.State.DELETED %} - - Extend expiration date - -
-
-
-

- Are you sure you want to extend this expiration date? -

-
- -
- -
-
    -
  • - -
    • -
  • - -
    • -
-
-
-
- {% endif %} - {# {% include 'includes/modal.html' with modal_heading="Are you sure you want to extend this expiration date?" modal_button=modal_button|safe %} #} +
+
+ + {# todo: avoid this #} +   +
-
- {% if original.state != original.State.DELETED %} - - {% endif %} - | - {% if original.state == original.State.READY %} - - {% elif original.state == original.State.ON_HOLD %} - - {% endif %} - {% if original.state == original.State.READY or original.state == original.State.ON_HOLD %} - | - {% endif %} - {% if original.state != original.State.DELETED %} - +
+ {% if original.state != original.State.DELETED %} + + {% endif %} + | + {% if original.state == original.State.READY %} + + {% elif original.state == original.State.ON_HOLD %} + + {% endif %} + {% if original.state == original.State.READY or original.state == original.State.ON_HOLD %} + | + {% endif %} + {% if original.state != original.State.DELETED %} + {% endif %} +
{{ block.super }} {% endblock %} From 612f967c6fea2867cd08b2d874a6100ce5b667de Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Feb 2024 10:48:09 -0700 Subject: [PATCH 19/55] Desktop/mobile styling --- src/registrar/assets/sass/_theme/_admin.scss | 6 ++++++ .../templates/django/admin/domain_change_form.html | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 098e9bb81..7432da46d 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -271,6 +271,12 @@ h1, h2, h3, } } +@include at-media(desktop){ + .button-list-mobile { + display: block; + } +} + @include at-media(mobile){ .button-list-mobile { display: contents; diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index 6420f8b5f..8c0093d60 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -7,7 +7,7 @@ {# todo: avoid this #}   - +
{% if original.state != original.State.DELETED %} From df70a179e883d0a9aca842c7569887bb69e6d734 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Feb 2024 11:43:56 -0700 Subject: [PATCH 20/55] Add conditional "confirm" logic --- src/registrar/assets/js/get-gov-admin.js | 33 +++++++++---------- src/registrar/assets/sass/_theme/_admin.scss | 5 +++ .../django/admin/domain_change_form.html | 6 +++- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 007ded215..8005c68cd 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -44,27 +44,24 @@ function openInNewTab(el, removeAttribute = false){ domainSubmitButton.addEventListener("mouseover", () => openInNewTab(domainFormElement, true)); domainSubmitButton.addEventListener("mouseout", () => openInNewTab(domainFormElement, false)); } + + let extendExpirationDateButton = document.getElementById("extend-expiration-button") + let confirmationButtons = document.querySelector(".admin-confirmation-buttons") + let cancelExpirationButton = document.getElementById("cancel-extend-button") + if (extendExpirationDateButton && confirmationButtons && cancelExpirationButton){ - let extendExpirationDateButton = document.getElementById("extend_expiration_date_button") - if (extendExpirationDateButton){ + // Tie logic to the extend button to show confirmation options extendExpirationDateButton.addEventListener("click", () => { - form = document.getElementById("domain_form") - /* - For some reason, Django admin has the propensity to ignore nested - inputs and delete nested form objects. - The workaround is to manually create an element as so, after the DOM - has been generated. + extendExpirationDateButton.hidden = true + console.log("these are the buttons: ") + console.log(confirmationButtons) + confirmationButtons.hidden = false + }) - Its not the most beautiful thing every, but it works. - */ - var input = document.createElement("input"); - input.type = "hidden"; - input.name = "_extend_expiration_date"; - // The value doesn't matter, just needs to be present - input.value = "1"; - // Add the hidden input to the form - form.appendChild(input); - form.submit(); + // Tie logic to the cancel button to hide the confirmation options + cancelExpirationButton.addEventListener("click", () => { + confirmationButtons.hidden = true + extendExpirationDateButton.hidden = false }) } } diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 7432da46d..98f3a7835 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -283,6 +283,11 @@ h1, h2, h3, } } +.usa-button{ + font-size: 14px; + font-weight: normal; +} + // Button groups in /admin incorrectly have bullets. // Remove that! .usa-button-group .usa-button-group__item { diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index 8c0093d60..1a21f4a90 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -11,7 +11,11 @@
{% if original.state != original.State.DELETED %} - + + {% endif %} | {% if original.state == original.State.READY %} From 37430c552f0a54c46e4cf6130c05ec3eecdc1da5 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Feb 2024 11:54:52 -0700 Subject: [PATCH 21/55] Change button styling on /admin --- src/registrar/assets/sass/_theme/_admin.scss | 19 ++++++------------- .../django/admin/domain_change_form.html | 4 ++-- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 98f3a7835..e9c13b895 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -271,21 +271,14 @@ h1, h2, h3, } } -@include at-media(desktop){ - .button-list-mobile { - display: block; - } -} - -@include at-media(mobile){ - .button-list-mobile { - display: contents; - } -} - -.usa-button{ +.usa-button.admin-button{ font-size: 14px; font-weight: normal; + background-color: var(--button-bg); +} + +.usa-button--secondary.admin-button{ + background-color: var(--delete-button-bg); } // Button groups in /admin incorrectly have bullets. diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index 1a21f4a90..d45122422 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -12,8 +12,8 @@
{% if original.state != original.State.DELETED %} {% endif %} From efa8b7d0ef9adf2d3c37efe7f716e2a71585cab1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Feb 2024 12:21:16 -0700 Subject: [PATCH 22/55] Use default admin input --- src/registrar/assets/sass/_theme/_admin.scss | 14 +++----------- .../templates/django/admin/domain_change_form.html | 8 +++++--- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index e9c13b895..2ed0594dd 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -271,18 +271,10 @@ h1, h2, h3, } } -.usa-button.admin-button{ - font-size: 14px; - font-weight: normal; - background-color: var(--button-bg); -} - -.usa-button--secondary.admin-button{ +.cancel-extend-button{ background-color: var(--delete-button-bg); } -// Button groups in /admin incorrectly have bullets. -// Remove that! -.usa-button-group .usa-button-group__item { - list-style-type: none; +.admin-confirm-button{ + text-transform: none; } diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index d45122422..0835c6680 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -12,12 +12,14 @@
{% if original.state != original.State.DELETED %} - {% endif %} | + {% endif %} {% if original.state == original.State.READY %} {% elif original.state == original.State.ON_HOLD %} From 591f67f6ecdbed3de3cd2cfdac80a5851ccaaca9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Feb 2024 12:33:53 -0700 Subject: [PATCH 23/55] QOL Changes --- src/registrar/assets/js/get-gov-admin.js | 8 +++----- src/registrar/assets/sass/_theme/_admin.scss | 4 ++-- .../templates/django/admin/domain_change_form.html | 2 +- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 8005c68cd..fa429ba7f 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -41,20 +41,18 @@ function openInNewTab(el, removeAttribute = false){ let domainFormElement = document.getElementById("domain_form"); let domainSubmitButton = document.getElementById("manageDomainSubmitButton"); if(domainSubmitButton && domainFormElement){ - domainSubmitButton.addEventListener("mouseover", () => openInNewTab(domainFormElement, true)); - domainSubmitButton.addEventListener("mouseout", () => openInNewTab(domainFormElement, false)); + domainSubmitButton.addEventListener("mouseover", () => openInNewTab(domainFormElement, true)); + domainSubmitButton.addEventListener("mouseout", () => openInNewTab(domainFormElement, false)); } let extendExpirationDateButton = document.getElementById("extend-expiration-button") let confirmationButtons = document.querySelector(".admin-confirmation-buttons") - let cancelExpirationButton = document.getElementById("cancel-extend-button") + let cancelExpirationButton = document.querySelector(".cancel-extend-button") if (extendExpirationDateButton && confirmationButtons && cancelExpirationButton){ // Tie logic to the extend button to show confirmation options extendExpirationDateButton.addEventListener("click", () => { extendExpirationDateButton.hidden = true - console.log("these are the buttons: ") - console.log(confirmationButtons) confirmationButtons.hidden = false }) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 2ed0594dd..f3bd0bbd8 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -275,6 +275,6 @@ h1, h2, h3, background-color: var(--delete-button-bg); } -.admin-confirm-button{ - text-transform: none; +input.admin-confirm-button{ + text-transform: none !important; } diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index 0835c6680..2b4461a49 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -15,7 +15,7 @@ {# todo: avoid this #}   - + | From b06babd6f00cac606691d889050a49187e475624 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Feb 2024 13:40:02 -0700 Subject: [PATCH 24/55] Code cleanup --- src/registrar/admin.py | 97 ++++++-------------- src/registrar/assets/sass/_theme/_admin.scss | 2 +- src/registrar/models/domain.py | 20 +--- 3 files changed, 31 insertions(+), 88 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 5b756f689..72d587855 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -24,9 +24,7 @@ from auditlog.admin import LogEntryAdmin # type: ignore from django_fsm import TransitionNotAllowed # type: ignore from django.utils.safestring import mark_safe from django.utils.html import escape -from epplibwrapper import ( - common as epp, -) + logger = logging.getLogger(__name__) @@ -1150,10 +1148,17 @@ class DomainAdmin(ListHeaderAdmin): return super().response_change(request, obj) def do_extend_expiration_date(self, request, obj): + """Extends a domains expiration date by one year from the current date""" + + # Make sure we're dealing with a Domain if not isinstance(obj, Domain): self.message_user(request, "Object is not of type Domain.", messages.ERROR) return None + # Get the date we want to update to + desired_date = date.today() + relativedelta(years=1) + + # Grab the current expiration date try: exp_date = obj.registry_expiration_date except KeyError: @@ -1161,46 +1166,29 @@ class DomainAdmin(ListHeaderAdmin): logger.warning("current expiration date not set; setting to today") exp_date = date.today() - desired_date = date.today() + relativedelta(years=1) - logger.info(f"do_extend_expiration_date -> exp {exp_date} des {desired_date}") - - # Get the difference in months between the expiration date, and the - # desired date (today + 1). Then, add one year to that. - # TODO - error: Periods for domain registrations must be specified in years.??? - # one_year = 12 - # month_length = self._month_diff(exp_date, desired_date) + one_year + # If the expiration date is super old (2020, for example), we need to + # "catch up" to the current year, so we add the difference. + # If both years match, then lets just proceed as normal. years = 1 if desired_date > exp_date: - years = (desired_date.year - exp_date.year) + year_difference = relativedelta(desired_date.year, exp_date.year).years + years = year_difference + # Renew the domain. try: - # logger.info(f"do_extend_expiration_date -> month length: {month_length}") - logger.info(f"do_extend_expiration_date -> years {years}") - # TODO why cant I specify months - #obj.renew_domain(length=month_length, unit=epp.Unit.MONTH) - if years >= 1: - obj.renew_domain(length=years) - else: - self.message_user( - request, - f"Error extending this domain: Can't extend date by 0 years.", - messages.ERROR, - ) + obj.renew_domain(length=years) + updated_domain = Domain.objects.filter(id=obj.id).get() + self.message_user( + request, + f"Successfully extended expiration date to {updated_domain.registry_expiration_date}.", + ) except RegistryError as err: - if err.code: - self.message_user( - request, - f"Error extending this domain: {err}.", - messages.ERROR, - ) - elif err.is_connection_error(): - self.message_user( - request, - "Error connecting to the registry.", - messages.ERROR, - ) + if err.is_connection_error(): + error_message = "Error connecting to the registry." else: - self.message_user(request, err, messages.ERROR) + error_message = f"Error extending this domain: {err}." + + self.message_user(request, error_message, messages.ERROR) except KeyError: # In normal code flow, a keyerror can only occur when # fresh data can't be pulled from the registry, and thus there is no cache. @@ -1210,34 +1198,11 @@ class DomainAdmin(ListHeaderAdmin): messages.ERROR, ) except Exception as err: - self.message_user(request, err, messages.ERROR) - else: - updated_domain = Domain.objects.filter(id=obj.id).get() - self.message_user( - request, - f"Successfully extended expiration date to {updated_domain.registry_expiration_date}.", - ) + logger.error(err, stack_info=True) + self.message_user(request, "Could not delete: An unspecified error occured", messages.ERROR) + return HttpResponseRedirect(".") - def _month_diff(self, date_1, date_2): - """ - Calculate the difference in months between two dates using dateutil's relativedelta. - - :param date_1: The first date. - :param date_2: The second date. - :return: The difference in months as an integer. - """ - # Ensure date_1 is always the earlier date - start_date, end_date = sorted([date_1, date_2]) - - # Grab the delta between the two - rdelta = relativedelta(end_date, start_date) - logger.info(f"rdelta is: {rdelta}, years {rdelta.years}, months {rdelta.months}") - - # Calculate total months as years * 12 + months - total_months = rdelta.years * 12 + rdelta.months - return total_months - def do_delete_domain(self, request, obj): if not isinstance(obj, Domain): # Could be problematic if the type is similar, @@ -1392,11 +1357,7 @@ class DomainAdmin(ListHeaderAdmin): def changelist_view(self, request, extra_context=None): extra_context = extra_context or {} # Create HTML for the modal button - modal_button = ( - '' - ) + modal_button = '' extra_context["modal_button"] = modal_button return super().changelist_view(request, extra_context=extra_context) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index f3bd0bbd8..4e4c15dd9 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -272,7 +272,7 @@ h1, h2, h3, } .cancel-extend-button{ - background-color: var(--delete-button-bg); + background-color: var(--delete-button-bg) !important; } input.admin-confirm-button{ diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index c72487761..dbde2be10 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -239,15 +239,12 @@ class Domain(TimeStampedModel, DomainHelper): To update the expiration date, use renew_domain method.""" raise NotImplementedError() - def renew_domain(self, length: int = 1, unit: epp.Unit = epp.Unit.YEAR, extend_year_past_current_date = False): + def renew_domain(self, length: int = 1, unit: epp.Unit = epp.Unit.YEAR): """ Renew the domain to a length and unit of time relative to the current expiration date. Default length and unit of time are 1 year. - - extend_past_current_date (bool): Specifies if the "desired" date - should exceed the present date + length. For instance, """ # If no date is specified, grab the registry_expiration_date @@ -257,22 +254,7 @@ class Domain(TimeStampedModel, DomainHelper): # if no expiration date from registry, set it to today logger.warning("current expiration date not set; setting to today") exp_date = date.today() - """ - if extend_year_past_current_date: - # TODO - handle unit == month - expected_renewal_year = exp_date.year + length - current_year = date.today().year - if expected_renewal_year < current_year: - # Modify the length such that it will exceed the current year by the length - length = (current_year - exp_date.year) + length - # length = (current_year - expected_renewal_year) + length * 2 - elif expected_renewal_year == current_year: - # In the event that the expected renewal date will equal the current year, - # we need to apply double "length" for it shoot past the current date - # at the correct interval. - length = length * 2 - """ # create RenewDomain request request = commands.RenewDomain(name=self.name, cur_exp_date=exp_date, period=epp.Period(length, unit)) From 773d6e0cee6f777be401e50de19d8372905a967e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Feb 2024 10:06:46 -0700 Subject: [PATCH 25/55] Unit test --- src/registrar/admin.py | 2 +- src/registrar/tests/test_admin.py | 64 ++++++++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 72d587855..10d23be15 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1171,7 +1171,7 @@ class DomainAdmin(ListHeaderAdmin): # If both years match, then lets just proceed as normal. years = 1 if desired_date > exp_date: - year_difference = relativedelta(desired_date.year, exp_date.year).years + year_difference = desired_date.year - exp_date.year years = year_difference # Renew the domain. diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index ad616d6b0..082ad9b2b 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1,6 +1,8 @@ +from datetime import date from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite from contextlib import ExitStack +from django_webtest import WebTest # type: ignore from django.contrib import messages from django.urls import reverse from registrar.admin import ( @@ -35,7 +37,7 @@ from .common import ( ) from django.contrib.sessions.backends.db import SessionStore from django.contrib.auth import get_user_model -from unittest.mock import patch +from unittest.mock import call, patch from unittest import skip from django.conf import settings @@ -45,7 +47,8 @@ import logging logger = logging.getLogger(__name__) -class TestDomainAdmin(MockEppLib): +class TestDomainAdmin(MockEppLib, WebTest): + csrf_checks = False def setUp(self): self.site = AdminSite() self.admin = DomainAdmin(model=Domain, admin_site=self.site) @@ -53,8 +56,65 @@ class TestDomainAdmin(MockEppLib): self.superuser = create_superuser() self.staffuser = create_user() self.factory = RequestFactory() + self.app.set_user(self.superuser.username) + self.client.force_login(self.superuser) super().setUp() + def test_extend_expiration_date_button(self): + """ + Tests if extend_expiration_date button sends the right epp command + """ + + # Create a ready domain with a preset expiration date + domain, _ = Domain.objects.get_or_create(name="fake.gov", state=Domain.State.READY) + + response = self.app.get(reverse("admin:registrar_domain_change", args=[domain.pk])) + + # Make sure that the page is loading as expected + self.assertEqual(response.status_code, 200) + self.assertContains(response, domain.name) + self.assertContains(response, "Extend expiration date") + + # Grab the form to submit + form = response.forms["domain_form"] + + with patch("django.contrib.messages.add_message") as mock_add_message: + with patch("registrar.models.Domain.renew_domain") as renew_mock: + # Submit the form + response = form.submit("_extend_expiration_date") + + # Follow the response + response = response.follow() + + # We need to use date.today() here, as it is not trivial + # to mock "date.today()". To do so requires libraries like freezegun, + # or convoluted workarounds. + extension_length = (date.today().year + 1) - 2023 + + # Assert that it is calling the function + renew_mock.assert_has_calls([call(length=extension_length)], any_order=False) + self.assertEqual(renew_mock.call_count, 1) + + # Assert that everything on the page looks correct + self.assertEqual(response.status_code, 200) + self.assertContains(response, domain.name) + self.assertContains(response, "Extend expiration date") + + # Ensure the message we recieve is in line with what we expect + expected_message = f"Successfully extended expiration date to 2024-01-01." + #self.assertContains(response, expected_message) + expected_call = call( + response, + messages.INFO, + expected_message, + extra_tags="", + fail_silently=False, + ) + mock_add_message.assert_has_calls(expected_call, 1) + # Assert that the domain was updated correctly + expected_date = date(year=2025, month=1, day=1) + self.assertEqual(domain.expiration_date, expected_date) + def test_short_org_name_in_domains_list(self): """ Make sure the short name is displaying in admin on the list page From 3189b50baaa443b2e8c65abca03725fe7be6e818 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Feb 2024 10:13:10 -0700 Subject: [PATCH 26/55] Fix unit test --- src/registrar/admin.py | 3 +-- src/registrar/tests/test_admin.py | 13 +++++-------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 10d23be15..3dc0d0e56 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1177,10 +1177,9 @@ class DomainAdmin(ListHeaderAdmin): # Renew the domain. try: obj.renew_domain(length=years) - updated_domain = Domain.objects.filter(id=obj.id).get() self.message_user( request, - f"Successfully extended expiration date to {updated_domain.registry_expiration_date}.", + f"Successfully extended expiration date.", ) except RegistryError as err: if err.is_connection_error(): diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 082ad9b2b..4da7de85a 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -37,7 +37,7 @@ from .common import ( ) from django.contrib.sessions.backends.db import SessionStore from django.contrib.auth import get_user_model -from unittest.mock import call, patch +from unittest.mock import ANY, call, patch from unittest import skip from django.conf import settings @@ -101,19 +101,16 @@ class TestDomainAdmin(MockEppLib, WebTest): self.assertContains(response, "Extend expiration date") # Ensure the message we recieve is in line with what we expect - expected_message = f"Successfully extended expiration date to 2024-01-01." - #self.assertContains(response, expected_message) + expected_message = f"Successfully extended expiration date." expected_call = call( - response, + # The WGSI request doesn't need to be tested + ANY, messages.INFO, expected_message, extra_tags="", fail_silently=False, ) - mock_add_message.assert_has_calls(expected_call, 1) - # Assert that the domain was updated correctly - expected_date = date(year=2025, month=1, day=1) - self.assertEqual(domain.expiration_date, expected_date) + mock_add_message.assert_has_calls([expected_call], 1) def test_short_org_name_in_domains_list(self): """ From d18baa773ca3c3ec129d0b2150ab239483dfce1c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Feb 2024 10:18:08 -0700 Subject: [PATCH 27/55] Update admin.py --- src/registrar/admin.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 3dc0d0e56..e77df1ce5 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1169,8 +1169,9 @@ class DomainAdmin(ListHeaderAdmin): # If the expiration date is super old (2020, for example), we need to # "catch up" to the current year, so we add the difference. # If both years match, then lets just proceed as normal. + calculated_exp_date = exp_date + relativedelta(years=1) years = 1 - if desired_date > exp_date: + if desired_date > calculated_exp_date: year_difference = desired_date.year - exp_date.year years = year_difference From a1769b50c660fa046f7c8d20ee88a8c9d0212db4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Feb 2024 11:50:05 -0700 Subject: [PATCH 28/55] Use modal --- src/registrar/assets/js/get-gov-admin.js | 45 ++++++++----- src/registrar/assets/sass/_theme/_admin.scss | 7 ++ .../django/admin/domain_change_form.html | 66 ++++++++++++++++++- 3 files changed, 99 insertions(+), 19 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index fa429ba7f..29aa9ce03 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -23,6 +23,33 @@ function openInNewTab(el, removeAttribute = false){ // <<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>> // Initialization code. +/** An IIFE for pages in DjangoAdmin that use modals. + * Dja strips out form elements, and modals generate their content outside + * of the current form scope, so we need to "inject" these inputs. +*/ +(function (){ + function createPhantomModalFormButtons(){ + let submitButtons = document.querySelectorAll('.usa-modal button[type="submit"]'); + form = document.querySelector("form") + submitButtons.forEach((button) => { + + let input = document.createElement("input"); + input.type = "submit"; + input.name = button.name; + input.value = button.value; + input.style.display = "none" + + // Add the hidden input to the form + form.appendChild(input); + button.addEventListener("click", () => { + console.log("clicking") + input.click(); + }) + }) + } + + createPhantomModalFormButtons(); +})(); /** An IIFE for pages in DjangoAdmin which may need custom JS implementation. * Currently only appends target="_blank" to the domain_form object, * but this can be expanded. @@ -44,24 +71,6 @@ function openInNewTab(el, removeAttribute = false){ domainSubmitButton.addEventListener("mouseover", () => openInNewTab(domainFormElement, true)); domainSubmitButton.addEventListener("mouseout", () => openInNewTab(domainFormElement, false)); } - - let extendExpirationDateButton = document.getElementById("extend-expiration-button") - let confirmationButtons = document.querySelector(".admin-confirmation-buttons") - let cancelExpirationButton = document.querySelector(".cancel-extend-button") - if (extendExpirationDateButton && confirmationButtons && cancelExpirationButton){ - - // Tie logic to the extend button to show confirmation options - extendExpirationDateButton.addEventListener("click", () => { - extendExpirationDateButton.hidden = true - confirmationButtons.hidden = false - }) - - // Tie logic to the cancel button to hide the confirmation options - cancelExpirationButton.addEventListener("click", () => { - confirmationButtons.hidden = true - extendExpirationDateButton.hidden = false - }) - } } prepareDjangoAdmin(); diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 4e4c15dd9..bee4f1466 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -278,3 +278,10 @@ h1, h2, h3, input.admin-confirm-button{ text-transform: none !important; } + + +// Button groups in /admin incorrectly have bullets. +// Remove that! +.usa-modal__footer .usa-button-group__item{ + list-style-type: none; +} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index 2b4461a49..28ae01aec 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -17,7 +17,14 @@   - + + Extend expiration date + | {% endif %} {% if original.state == original.State.READY %} @@ -35,3 +42,60 @@
{{ block.super }} {% endblock %} + +{% block submit_buttons_bottom %} +
+
+
+

+ Are you sure you want to extend this expiration date? +

+
+ +
+ +
+
    +
  • + +
    • +
  • + +
    • +
+
+
+ +
+
+{{ block.super }} +{% endblock %} \ No newline at end of file From bfc82bda0149fabb9ee4bf2b213748b6c98ea2eb Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Feb 2024 11:55:47 -0700 Subject: [PATCH 29/55] Add "the" --- src/registrar/admin.py | 2 +- src/registrar/tests/test_admin.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e77df1ce5..7439217a1 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1180,7 +1180,7 @@ class DomainAdmin(ListHeaderAdmin): obj.renew_domain(length=years) self.message_user( request, - f"Successfully extended expiration date.", + f"Successfully extended the expiration date.", ) except RegistryError as err: if err.is_connection_error(): diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 4da7de85a..6cc6f96ea 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -101,7 +101,7 @@ class TestDomainAdmin(MockEppLib, WebTest): self.assertContains(response, "Extend expiration date") # Ensure the message we recieve is in line with what we expect - expected_message = f"Successfully extended expiration date." + expected_message = f"Successfully extended the expiration date." expected_call = call( # The WGSI request doesn't need to be tested ANY, From 10e8317e3c31096f6236634c46cf4ab69fb008db Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Feb 2024 15:08:08 -0700 Subject: [PATCH 30/55] Test cases, black linting --- src/registrar/admin.py | 14 +++- src/registrar/tests/common.py | 25 ++++-- src/registrar/tests/test_admin.py | 126 ++++++++++++++++++++++++++++-- 3 files changed, 149 insertions(+), 16 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7439217a1..0e86e7764 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1156,7 +1156,7 @@ class DomainAdmin(ListHeaderAdmin): return None # Get the date we want to update to - desired_date = date.today() + relativedelta(years=1) + desired_date = self._get_current_date() + relativedelta(years=1) # Grab the current expiration date try: @@ -1164,7 +1164,7 @@ class DomainAdmin(ListHeaderAdmin): except KeyError: # if no expiration date from registry, set it to today logger.warning("current expiration date not set; setting to today") - exp_date = date.today() + exp_date = self._get_current_date() # If the expiration date is super old (2020, for example), we need to # "catch up" to the current year, so we add the difference. @@ -1178,9 +1178,10 @@ class DomainAdmin(ListHeaderAdmin): # Renew the domain. try: obj.renew_domain(length=years) + self.message_user( request, - f"Successfully extended the expiration date.", + "Successfully extended the expiration date.", ) except RegistryError as err: if err.is_connection_error(): @@ -1203,6 +1204,13 @@ class DomainAdmin(ListHeaderAdmin): return HttpResponseRedirect(".") + # Workaround for unit tests, as we cannot mock date directly. + # it is immutable. Rather than dealing with a convoluted workaround, + # lets wrap this in a function. + def _get_current_date(self): + """Gets the current date""" + return date.today() + def do_delete_domain(self, request, obj): if not isinstance(obj, Domain): # Could be problematic if the type is similar, diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 023e5319e..e3bb2adc9 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -920,6 +920,11 @@ class MockEppLib(TestCase): ex_date=datetime.date(2023, 5, 25), ) + mockButtonRenewedDomainExpDate = fakedEppObject( + "fakefuture.gov", + ex_date=datetime.date(2025, 5, 25), + ) + mockDnsNeededRenewedDomainExpDate = fakedEppObject( "fakeneeded.gov", ex_date=datetime.date(2023, 2, 15), @@ -1031,6 +1036,7 @@ class MockEppLib(TestCase): return None def mockRenewDomainCommand(self, _request, cleaned): + print(f"What is the request at this time? {_request}") if getattr(_request, "name", None) == "fake-error.gov": raise RegistryError(code=ErrorCode.PARAMETER_VALUE_RANGE_ERROR) elif getattr(_request, "name", None) == "waterbutpurple.gov": @@ -1048,11 +1054,20 @@ class MockEppLib(TestCase): res_data=[self.mockMaximumRenewedDomainExpDate], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, ) - else: - return MagicMock( - res_data=[self.mockRenewedDomainExpDate], - code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, - ) + elif getattr(_request, "name", None) == "fake.gov": + period = getattr(_request, "period", None) + extension_period = getattr(period, "length", None) + + if extension_period == 2: + return MagicMock( + res_data=[self.mockButtonRenewedDomainExpDate], + code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, + ) + else: + return MagicMock( + res_data=[self.mockRenewedDomainExpDate], + code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, + ) def mockInfoDomainCommands(self, _request, cleaned): request_name = getattr(_request, "name", None) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 6cc6f96ea..f5a60011c 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -49,6 +49,7 @@ logger = logging.getLogger(__name__) class TestDomainAdmin(MockEppLib, WebTest): csrf_checks = False + def setUp(self): self.site = AdminSite() self.admin = DomainAdmin(model=Domain, admin_site=self.site) @@ -61,6 +62,56 @@ class TestDomainAdmin(MockEppLib, WebTest): super().setUp() def test_extend_expiration_date_button(self): + """ + Tests if extend_expiration_date button extends correctly + """ + + # Create a ready domain with a preset expiration date + domain, _ = Domain.objects.get_or_create(name="fake.gov", state=Domain.State.READY) + + response = self.app.get(reverse("admin:registrar_domain_change", args=[domain.pk])) + + # Make sure that the page is loading as expected + self.assertEqual(response.status_code, 200) + self.assertContains(response, domain.name) + self.assertContains(response, "Extend expiration date") + + # Grab the form to submit + form = response.forms["domain_form"] + + with patch("django.contrib.messages.add_message") as mock_add_message: + # Submit the form + response = form.submit("_extend_expiration_date") + + # Follow the response + response = response.follow() + + # refresh_from_db() does not work for objects with protected=True. + # https://github.com/viewflow/django-fsm/issues/89 + new_domain = Domain.objects.get(id=domain.id) + + # Check that the current expiration date is what we expect + self.assertEqual(new_domain.expiration_date, date(2025, 5, 25)) + + # Assert that everything on the page looks correct + self.assertEqual(response.status_code, 200) + self.assertContains(response, domain.name) + self.assertContains(response, "Extend expiration date") + + # Ensure the message we recieve is in line with what we expect + expected_message = "Successfully extended the expiration date." + expected_call = call( + # The WGSI request doesn't need to be tested + ANY, + messages.INFO, + expected_message, + extra_tags="", + fail_silently=False, + ) + mock_add_message.assert_has_calls([expected_call], 1) + + @patch("registrar.admin.DomainAdmin._get_current_date", return_value=date(2024, 1, 1)) + def test_extend_expiration_date_button_epp(self, mock_date_today): """ Tests if extend_expiration_date button sends the right epp command """ @@ -74,7 +125,7 @@ class TestDomainAdmin(MockEppLib, WebTest): self.assertEqual(response.status_code, 200) self.assertContains(response, domain.name) self.assertContains(response, "Extend expiration date") - + # Grab the form to submit form = response.forms["domain_form"] @@ -85,14 +136,18 @@ class TestDomainAdmin(MockEppLib, WebTest): # Follow the response response = response.follow() - - # We need to use date.today() here, as it is not trivial - # to mock "date.today()". To do so requires libraries like freezegun, - # or convoluted workarounds. - extension_length = (date.today().year + 1) - 2023 - # Assert that it is calling the function + # This value is based off of the current year - the expiration date. + # We "freeze" time to 2024, so 2024 - 2023 will always result in an + # "extension" of 2, as that will be one year of extension from that date. + extension_length = 2 + + # Assert that it is calling the function with the right extension length. + # We only need to test the value that EPP sends, as we can assume the other + # test cases cover the "renew" function. renew_mock.assert_has_calls([call(length=extension_length)], any_order=False) + + # We should not make duplicate calls self.assertEqual(renew_mock.call_count, 1) # Assert that everything on the page looks correct @@ -101,7 +156,62 @@ class TestDomainAdmin(MockEppLib, WebTest): self.assertContains(response, "Extend expiration date") # Ensure the message we recieve is in line with what we expect - expected_message = f"Successfully extended the expiration date." + expected_message = "Successfully extended the expiration date." + expected_call = call( + # The WGSI request doesn't need to be tested + ANY, + messages.INFO, + expected_message, + extra_tags="", + fail_silently=False, + ) + mock_add_message.assert_has_calls([expected_call], 1) + + @patch("registrar.admin.DomainAdmin._get_current_date", return_value=date(2023, 1, 1)) + def test_extend_expiration_date_button_date_matches_epp(self, mock_date_today): + """ + Tests if extend_expiration_date button sends the right epp command + when the current year matches the expiration date + """ + + # Create a ready domain with a preset expiration date + domain, _ = Domain.objects.get_or_create(name="fake.gov", state=Domain.State.READY) + + response = self.app.get(reverse("admin:registrar_domain_change", args=[domain.pk])) + + # Make sure that the page is loading as expected + self.assertEqual(response.status_code, 200) + self.assertContains(response, domain.name) + self.assertContains(response, "Extend expiration date") + + # Grab the form to submit + form = response.forms["domain_form"] + + with patch("django.contrib.messages.add_message") as mock_add_message: + with patch("registrar.models.Domain.renew_domain") as renew_mock: + # Submit the form + response = form.submit("_extend_expiration_date") + + # Follow the response + response = response.follow() + + extension_length = 1 + + # Assert that it is calling the function with the right extension length. + # We only need to test the value that EPP sends, as we can assume the other + # test cases cover the "renew" function. + renew_mock.assert_has_calls([call(length=extension_length)], any_order=False) + + # We should not make duplicate calls + self.assertEqual(renew_mock.call_count, 1) + + # Assert that everything on the page looks correct + self.assertEqual(response.status_code, 200) + self.assertContains(response, domain.name) + self.assertContains(response, "Extend expiration date") + + # Ensure the message we recieve is in line with what we expect + expected_message = "Successfully extended the expiration date." expected_call = call( # The WGSI request doesn't need to be tested ANY, From d989d9581a70884fa9fad2ea9f5b2dfb4713e4d6 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Feb 2024 15:35:07 -0700 Subject: [PATCH 31/55] Minor code cleanup --- src/registrar/admin.py | 25 ++-- .../django/admin/domain_change_form.html | 115 ++++++++++-------- 2 files changed, 71 insertions(+), 69 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 0e86e7764..393cdf23d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2,19 +2,17 @@ from datetime import date import logging from django import forms from django.db.models.functions import Concat -from django.http import HttpResponse -from dateutil.relativedelta import relativedelta +from django.http import HttpResponse, HttpResponseRedirect from django.shortcuts import redirect from django_fsm import get_available_FIELD_transitions from django.contrib import admin, messages from django.contrib.auth.admin import UserAdmin as BaseUserAdmin from django.contrib.auth.models import Group from django.contrib.contenttypes.models import ContentType -from django.http.response import HttpResponseRedirect from django.urls import reverse +from dateutil.relativedelta import relativedelta # type: ignore from epplibwrapper.errors import ErrorCode, RegistryError -from registrar.models.domain import Domain -from registrar.models.user import User +from registrar.models import Domain, User from registrar.utility import csv_export from registrar.views.utility.mixins import OrderableFieldsMixin from django.contrib.admin.views.main import ORDER_VAR @@ -1170,15 +1168,14 @@ class DomainAdmin(ListHeaderAdmin): # "catch up" to the current year, so we add the difference. # If both years match, then lets just proceed as normal. calculated_exp_date = exp_date + relativedelta(years=1) - years = 1 - if desired_date > calculated_exp_date: - year_difference = desired_date.year - exp_date.year - years = year_difference + + year_difference = desired_date.year - exp_date.year + # Max probably isn't needed here (no code flow), but it guards against negative and 0. + years = max(1, year_difference) if desired_date > calculated_exp_date else 1 # Renew the domain. try: obj.renew_domain(length=years) - self.message_user( request, "Successfully extended the expiration date.", @@ -1188,7 +1185,6 @@ class DomainAdmin(ListHeaderAdmin): error_message = "Error connecting to the registry." else: error_message = f"Error extending this domain: {err}." - self.message_user(request, error_message, messages.ERROR) except KeyError: # In normal code flow, a keyerror can only occur when @@ -1362,13 +1358,6 @@ class DomainAdmin(ListHeaderAdmin): return True return super().has_change_permission(request, obj) - def changelist_view(self, request, extra_context=None): - extra_context = extra_context or {} - # Create HTML for the modal button - modal_button = '' - extra_context["modal_button"] = modal_button - return super().changelist_view(request, extra_context=extra_context) - class DraftDomainAdmin(ListHeaderAdmin): """Custom draft domain admin class.""" diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index 28ae01aec..fbb3380a7 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -44,58 +44,71 @@ {% endblock %} {% block submit_buttons_bottom %} -
-
-
-

- Are you sure you want to extend this expiration date? -

-
- -
- -
-
    -
  • - -
    • -
  • - -
    • -
-
-
- + {% comment %} + Modals behave very weirdly in django admin. + They tend to "strip out" any injected form elements, leaving only the main form. + In addition, USWDS handles modals by first destroying the element, then repopulating it toward the end of the page. + In effect, this means that the modal is not, and cannot, be surrounded by any form element at compile time. + + The current workaround for this is to use javascript to inject a hidden input, and bind submit of that + element to the click of the confirmation button within this modal. + + This is controlled by the class `dja-form-placeholder` on the button. + + In addition, the modal element MUST be placed low in the DOM. The script loads slower on DJA than on other portions + of the application, so this means that it will briefly "populate", causing unintended visual effects. + {% endcomment %} +
+
+
+

+ Are you sure you want to extend this expiration date? +

+
+ +
+ +
+
    +
  • + +
    • +
  • + +
    • +
+ +
+
{{ block.super }} {% endblock %} \ No newline at end of file From 2765f46d3ae7fe65d99bcbb013e2e19e27850a22 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Feb 2024 15:41:21 -0700 Subject: [PATCH 32/55] Update domain_change_form.html --- src/registrar/templates/django/admin/domain_change_form.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index fbb3380a7..d0fd46800 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -61,13 +61,13 @@

- Are you sure you want to extend this expiration date? + Are you sure you want to extend the expiration date?

-

+ This will extend the expiration date by one year. +

+

+ Domain: {{ original.name }} +
+ New expiration date: {{ extended_expiration_date }} + {{test}} +

+

+ This action cannot be undone.

@@ -83,7 +92,7 @@ class="usa-button dja-form-placeholder" name="_extend_expiration_date" > - Confirm + Yes, extend date
  • From 757c31da3a5c59c12c986fa7952abcd05e641957 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Feb 2024 09:34:15 -0700 Subject: [PATCH 34/55] Add mini spacer --- src/registrar/assets/sass/_theme/_admin.scss | 4 ++++ .../templates/django/admin/domain_change_form.html | 12 +++--------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index bee4f1466..d238c7c54 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -183,6 +183,10 @@ h1, h2, h3, .submit-row div.spacer { flex-grow: 1; } +.submit-row .mini-spacer{ + margin-left: 2px; + margin-right: 2px; +} .submit-row span { margin-top: units(1); } diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index b48f04e60..8388d8768 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -5,18 +5,12 @@
    - {# todo: avoid this #} -   - + {# Dja has margin styles defined on inputs as is. Lets work with it, rather than fight it. #} + +
    {% if original.state != original.State.DELETED %} - Date: Mon, 12 Feb 2024 10:14:01 -0700 Subject: [PATCH 35/55] Split calculated years into a function --- src/registrar/admin.py | 64 ++++++++++++------- src/registrar/assets/sass/_theme/_admin.scss | 23 ++++--- .../django/admin/domain_change_form.html | 11 ++-- 3 files changed, 61 insertions(+), 37 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 292ecf01f..1f96571a5 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1079,9 +1079,15 @@ class DomainAdmin(ListHeaderAdmin): def changeform_view(self, request, object_id=None, form_url="", extra_context=None): if extra_context is None: extra_context = {} - # Pass in what the an extended expiration date would be - # for the expiration date modal - extra_context["extended_expiration_date"] = date.today() + relativedelta(years=1) + + # Pass in what the an extended expiration date would be for the expiration date modal + if object_id is not None: + domain = Domain.objects.get(pk=object_id) + years_to_extend_by = self._get_calculated_years_for_exp_date(domain) + extra_context["extended_expiration_date"] = date.today() + relativedelta(years=years_to_extend_by) + else: + extra_context["extended_expiration_date"] = None + return super().changeform_view(request, object_id, form_url, extra_context) def export_data_type(self, request): @@ -1161,26 +1167,7 @@ class DomainAdmin(ListHeaderAdmin): self.message_user(request, "Object is not of type Domain.", messages.ERROR) return None - # Get the date we want to update to - desired_date = self._get_current_date() + relativedelta(years=1) - - # Grab the current expiration date - try: - exp_date = obj.registry_expiration_date - except KeyError: - # if no expiration date from registry, set it to today - logger.warning("current expiration date not set; setting to today") - exp_date = self._get_current_date() - - # If the expiration date is super old (2020, for example), we need to - # "catch up" to the current year, so we add the difference. - # If both years match, then lets just proceed as normal. - calculated_exp_date = exp_date + relativedelta(years=1) - - year_difference = desired_date.year - exp_date.year - # Max probably isn't needed here (no code flow), but it guards against negative and 0. - years = max(1, year_difference) if desired_date > calculated_exp_date else 1 - + years = self._get_calculated_years_for_exp_date(obj) # Renew the domain. try: obj.renew_domain(length=years) @@ -1208,6 +1195,37 @@ class DomainAdmin(ListHeaderAdmin): return HttpResponseRedirect(".") + def _get_calculated_years_for_exp_date(self, obj, extension_period: int = 1): + """Given the current date, an extension period, and a registry_expiration_date + on the domain object, calculate the number of years needed to extend the + current expiration date by the extension period. + """ + # Get the date we want to update to + desired_date = self._get_current_date() + relativedelta(years=extension_period) + + # Grab the current expiration date + try: + exp_date = obj.registry_expiration_date + except KeyError: + # if no expiration date from registry, set it to today + logger.warning("current expiration date not set; setting to today") + exp_date = self._get_current_date() + + # If the expiration date is super old (2020, for example), we need to + # "catch up" to the current year, so we add the difference. + # If both years match, then lets just proceed as normal. + calculated_exp_date = exp_date + relativedelta(years=extension_period) + + year_difference = desired_date.year - exp_date.year + + years = extension_period + if desired_date > calculated_exp_date: + # Max probably isn't needed here (no code flow), but it guards against negative and 0. + # In both of those cases, we just want to extend by the extension_period. + years = max(extension_period, year_difference) + + return years + # Workaround for unit tests, as we cannot mock date directly. # it is immutable. Rather than dealing with a convoluted workaround, # lets wrap this in a function. diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index d238c7c54..ccbbeaa15 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -275,17 +275,24 @@ h1, h2, h3, } } -.cancel-extend-button{ - background-color: var(--delete-button-bg) !important; +input.admin-confirm-button { + text-transform: none; } -input.admin-confirm-button{ - text-transform: none !important; -} - - // Button groups in /admin incorrectly have bullets. // Remove that! -.usa-modal__footer .usa-button-group__item{ +.usa-modal__footer .usa-button-group__item { list-style-type: none; +} + +@include at-media(tablet) { + .button-list-mobile { + display: contents !important; + } +} + +@include at-media(mobile) { + .button-list-mobile { + display: contents !important; + } } \ No newline at end of file diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index 8388d8768..57e073f17 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -66,15 +66,14 @@

    This will extend the expiration date by one year. -

    -

    - Domain: {{ original.name }}
    - New expiration date: {{ extended_expiration_date }} - {{test}} + This action cannot be undone.

    - This action cannot be undone. + Domain: {{ original.name }} +
    + New expiration date: {{ extended_expiration_date }} + {{test}}

    From 96b2d6bb971830ef8e98d6cea045fe5cb48d0115 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Feb 2024 11:25:40 -0700 Subject: [PATCH 36/55] Content changes --- src/registrar/admin.py | 1 + src/registrar/assets/sass/_theme/_admin.scss | 10 +++------- .../templates/django/admin/domain_change_form.html | 10 ++++++---- src/registrar/tests/common.py | 4 +--- src/registrar/tests/test_admin.py | 8 +++++++- 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 1f96571a5..9e38618fa 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1168,6 +1168,7 @@ class DomainAdmin(ListHeaderAdmin): return None years = self._get_calculated_years_for_exp_date(obj) + # Renew the domain. try: obj.renew_domain(length=years) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index ccbbeaa15..f0ac5eb80 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -285,14 +285,10 @@ input.admin-confirm-button { list-style-type: none; } -@include at-media(tablet) { +// USWDS media checks are overzealous in this situation, +// we should manually define this behaviour. +@media (max-width: 768px) { .button-list-mobile { display: contents !important; } } - -@include at-media(mobile) { - .button-list-mobile { - display: contents !important; - } -} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index 57e073f17..67c5ac291 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -66,13 +66,15 @@

    This will extend the expiration date by one year. -
    + {# Acts as a
    #} +

    This action cannot be undone.

    - Domain: {{ original.name }} -
    - New expiration date: {{ extended_expiration_date }} + Domain: {{ original.name }} + {# Acts as a
    #} +

    + New expiration date: {{ extended_expiration_date }} {{test}}

    diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index e3bb2adc9..4bd7d6543 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -921,7 +921,7 @@ class MockEppLib(TestCase): ) mockButtonRenewedDomainExpDate = fakedEppObject( - "fakefuture.gov", + "fake.gov", ex_date=datetime.date(2025, 5, 25), ) @@ -1036,7 +1036,6 @@ class MockEppLib(TestCase): return None def mockRenewDomainCommand(self, _request, cleaned): - print(f"What is the request at this time? {_request}") if getattr(_request, "name", None) == "fake-error.gov": raise RegistryError(code=ErrorCode.PARAMETER_VALUE_RANGE_ERROR) elif getattr(_request, "name", None) == "waterbutpurple.gov": @@ -1057,7 +1056,6 @@ class MockEppLib(TestCase): elif getattr(_request, "name", None) == "fake.gov": period = getattr(_request, "period", None) extension_period = getattr(period, "length", None) - if extension_period == 2: return MagicMock( res_data=[self.mockButtonRenewedDomainExpDate], diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index f5a60011c..80c0f7b39 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -61,7 +61,9 @@ class TestDomainAdmin(MockEppLib, WebTest): self.client.force_login(self.superuser) super().setUp() - def test_extend_expiration_date_button(self): + @skip("TODO for another ticket. This test case is grabbing old db data.") + @patch("registrar.admin.DomainAdmin._get_current_date", return_value=date(2024, 1, 1)) + def test_extend_expiration_date_button(self, mock_date_today): """ Tests if extend_expiration_date button extends correctly """ @@ -71,6 +73,10 @@ class TestDomainAdmin(MockEppLib, WebTest): response = self.app.get(reverse("admin:registrar_domain_change", args=[domain.pk])) + # Make sure the ex date is what we expect it to be + domain_ex_date = Domain.objects.get(id=domain.id).expiration_date + self.assertEqual(domain_ex_date, date(2023, 5, 25)) + # Make sure that the page is loading as expected self.assertEqual(response.status_code, 200) self.assertContains(response, domain.name) From 5e0a1a6269917f4dbc658f2b4d050c773f1afbe4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Feb 2024 11:28:35 -0700 Subject: [PATCH 37/55] Update test_admin.py --- src/registrar/tests/test_admin.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 80c0f7b39..c8bd15d7e 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -48,6 +48,9 @@ logger = logging.getLogger(__name__) class TestDomainAdmin(MockEppLib, WebTest): + + # csrf checks do not work with WebTest. + # We disable them here. TODO for another ticket. csrf_checks = False def setUp(self): From 5de2df18723a7104b598b51a0fd036b91a330ce6 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Feb 2024 12:44:15 -0700 Subject: [PATCH 38/55] Linting --- src/registrar/admin.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 9e38618fa..306662403 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1077,6 +1077,7 @@ class DomainAdmin(ListHeaderAdmin): ordering = ["name"] def changeform_view(self, request, object_id=None, form_url="", extra_context=None): + """Custom changeform implementation to pass in context information""" if extra_context is None: extra_context = {} @@ -1198,7 +1199,7 @@ class DomainAdmin(ListHeaderAdmin): def _get_calculated_years_for_exp_date(self, obj, extension_period: int = 1): """Given the current date, an extension period, and a registry_expiration_date - on the domain object, calculate the number of years needed to extend the + on the domain object, calculate the number of years needed to extend the current expiration date by the extension period. """ # Get the date we want to update to From 9fa5ba8209b5db2ddb4525bdf0edeb41a94e14ea Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Feb 2024 12:49:46 -0700 Subject: [PATCH 39/55] Linting 2 --- src/registrar/tests/test_admin.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index c8bd15d7e..e8b4284ef 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -48,7 +48,6 @@ logger = logging.getLogger(__name__) class TestDomainAdmin(MockEppLib, WebTest): - # csrf checks do not work with WebTest. # We disable them here. TODO for another ticket. csrf_checks = False From a51949da0b98b15134312839238b381b85fc6095 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 13 Feb 2024 13:57:15 -0700 Subject: [PATCH 40/55] Improve performance --- src/registrar/admin.py | 231 +++++++++++++++++++++++++---------------- 1 file changed, 141 insertions(+), 90 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 4034bf35b..8f3f75105 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1,6 +1,8 @@ import logging +import time from django import forms -from django.db.models.functions import Concat +from django.db.models.functions import Concat, Coalesce +from django.db.models import Value, CharField from django.http import HttpResponse from django.shortcuts import redirect from django_fsm import get_available_FIELD_transitions @@ -12,6 +14,7 @@ from django.http.response import HttpResponseRedirect from django.urls import reverse from epplibwrapper.errors import ErrorCode, RegistryError from registrar.models.domain import Domain +from registrar.models.domain_application import DomainApplication from registrar.models.user import User from registrar.utility import csv_export from registrar.views.utility.mixins import OrderableFieldsMixin @@ -707,6 +710,17 @@ class DomainInformationAdmin(ListHeaderAdmin): return readonly_fields # Read-only fields for analysts +class Timer: + def __enter__(self): + self.start = time.time() + return self # This allows usage of the instance within the with block + + def __exit__(self, *args): + self.end = time.time() + self.duration = self.end - self.start + logger.info(f"Execution time: {self.duration} seconds") + + class DomainApplicationAdminForm(forms.ModelForm): """Custom form to limit transitions to available transitions""" @@ -753,15 +767,41 @@ class DomainApplicationAdmin(ListHeaderAdmin): """Lookup reimplementation, gets users of is_staff. Returns a list of tuples consisting of (user.id, user) """ - privileged_users = User.objects.filter(is_staff=True).order_by("first_name", "last_name", "email") - return [(user.id, user) for user in privileged_users] + logger.info("timing lookups") + with Timer() as t: + + # Select all investigators that are staff, then order by name and email + privileged_users = ( + DomainApplication.objects.select_related("investigator") + .filter(investigator__is_staff=True) + .order_by( + "investigator__first_name", + "investigator__last_name", + "investigator__email" + ) + ) + + # Annotate the full name and return a values list that lookups can use + privileged_users_annotated = privileged_users.annotate( + full_name=Coalesce( + Concat( + "investigator__first_name", Value(" "), "investigator__last_name", output_field=CharField() + ), + "investigator__email", + output_field=CharField() + ) + ).values_list("investigator__id", "full_name") + + return privileged_users_annotated def queryset(self, request, queryset): """Custom queryset implementation, filters by investigator""" - if self.value() is None: - return queryset - else: - return queryset.filter(investigator__id__exact=self.value()) + logger.info("timing queryset") + with Timer() as t: + if self.value() is None: + return queryset + else: + return queryset.filter(investigator__id__exact=self.value()) # Columns list_display = [ @@ -863,77 +903,83 @@ class DomainApplicationAdmin(ListHeaderAdmin): # lists in filter_horizontal are not sorted properly, sort them # by website def formfield_for_manytomany(self, db_field, request, **kwargs): - if db_field.name in ("current_websites", "alternative_domains"): - kwargs["queryset"] = models.Website.objects.all().order_by("website") # Sort websites - return super().formfield_for_manytomany(db_field, request, **kwargs) + logger.info("timing formfield_for_manytomany") + with Timer() as t: + if db_field.name in ("current_websites", "alternative_domains"): + kwargs["queryset"] = models.Website.objects.all().order_by("website") # Sort websites + return super().formfield_for_manytomany(db_field, request, **kwargs) def formfield_for_foreignkey(self, db_field, request, **kwargs): - # Removes invalid investigator options from the investigator dropdown - if db_field.name == "investigator": - kwargs["queryset"] = User.objects.filter(is_staff=True) - return db_field.formfield(**kwargs) - return super().formfield_for_foreignkey(db_field, request, **kwargs) + logger.info("timing formfield_for_foreignkey") + with Timer() as t: + # Removes invalid investigator options from the investigator dropdown + if db_field.name == "investigator": + kwargs["queryset"] = User.objects.filter(is_staff=True) + return db_field.formfield(**kwargs) + return super().formfield_for_foreignkey(db_field, request, **kwargs) # Trigger action when a fieldset is changed def save_model(self, request, obj, form, change): - if obj and obj.creator.status != models.User.RESTRICTED: - if change: # Check if the application is being edited - # Get the original application from the database - original_obj = models.DomainApplication.objects.get(pk=obj.pk) + logger.info("timing save_model") + with Timer() as t: + if obj and obj.creator.status != models.User.RESTRICTED: + if change: # Check if the application is being edited + # Get the original application from the database + original_obj = models.DomainApplication.objects.get(pk=obj.pk) - if ( - obj - and original_obj.status == models.DomainApplication.ApplicationStatus.APPROVED - and obj.status != models.DomainApplication.ApplicationStatus.APPROVED - and not obj.domain_is_not_active() - ): - # If an admin tried to set an approved application to - # another status and the related domain is already - # active, shortcut the action and throw a friendly - # error message. This action would still not go through - # shortcut or not as the rules are duplicated on the model, - # but the error would be an ugly Django error screen. + if ( + obj + and original_obj.status == models.DomainApplication.ApplicationStatus.APPROVED + and obj.status != models.DomainApplication.ApplicationStatus.APPROVED + and not obj.domain_is_not_active() + ): + # If an admin tried to set an approved application to + # another status and the related domain is already + # active, shortcut the action and throw a friendly + # error message. This action would still not go through + # shortcut or not as the rules are duplicated on the model, + # but the error would be an ugly Django error screen. - # Clear the success message - messages.set_level(request, messages.ERROR) + # Clear the success message + messages.set_level(request, messages.ERROR) - messages.error( - request, - "This action is not permitted. The domain is already active.", - ) + messages.error( + request, + "This action is not permitted. The domain is already active.", + ) - else: - if obj.status != original_obj.status: - status_method_mapping = { - models.DomainApplication.ApplicationStatus.STARTED: None, - models.DomainApplication.ApplicationStatus.SUBMITTED: obj.submit, - models.DomainApplication.ApplicationStatus.IN_REVIEW: obj.in_review, - models.DomainApplication.ApplicationStatus.ACTION_NEEDED: obj.action_needed, - models.DomainApplication.ApplicationStatus.APPROVED: obj.approve, - models.DomainApplication.ApplicationStatus.WITHDRAWN: obj.withdraw, - models.DomainApplication.ApplicationStatus.REJECTED: obj.reject, - models.DomainApplication.ApplicationStatus.INELIGIBLE: (obj.reject_with_prejudice), - } - selected_method = status_method_mapping.get(obj.status) - if selected_method is None: - logger.warning("Unknown status selected in django admin") - else: - # This is an fsm in model which will throw an error if the - # transition condition is violated, so we roll back the - # status to what it was before the admin user changed it and - # let the fsm method set it. - obj.status = original_obj.status - selected_method() + else: + if obj.status != original_obj.status: + status_method_mapping = { + models.DomainApplication.ApplicationStatus.STARTED: None, + models.DomainApplication.ApplicationStatus.SUBMITTED: obj.submit, + models.DomainApplication.ApplicationStatus.IN_REVIEW: obj.in_review, + models.DomainApplication.ApplicationStatus.ACTION_NEEDED: obj.action_needed, + models.DomainApplication.ApplicationStatus.APPROVED: obj.approve, + models.DomainApplication.ApplicationStatus.WITHDRAWN: obj.withdraw, + models.DomainApplication.ApplicationStatus.REJECTED: obj.reject, + models.DomainApplication.ApplicationStatus.INELIGIBLE: (obj.reject_with_prejudice), + } + selected_method = status_method_mapping.get(obj.status) + if selected_method is None: + logger.warning("Unknown status selected in django admin") + else: + # This is an fsm in model which will throw an error if the + # transition condition is violated, so we roll back the + # status to what it was before the admin user changed it and + # let the fsm method set it. + obj.status = original_obj.status + selected_method() - super().save_model(request, obj, form, change) - else: - # Clear the success message - messages.set_level(request, messages.ERROR) + super().save_model(request, obj, form, change) + else: + # Clear the success message + messages.set_level(request, messages.ERROR) - messages.error( - request, - "This action is not permitted for applications with a restricted creator.", - ) + messages.error( + request, + "This action is not permitted for applications with a restricted creator.", + ) def get_readonly_fields(self, request, obj=None): """Set the read-only state on form elements. @@ -941,36 +987,41 @@ class DomainApplicationAdmin(ListHeaderAdmin): admin user permissions and the application creator's status, so we'll use the baseline readonly_fields and extend it as needed. """ + logger.info("timing get_readonly_fields") + with Timer() as t: + readonly_fields = list(self.readonly_fields) - readonly_fields = list(self.readonly_fields) + # Check if the creator is restricted + if obj and obj.creator.status == models.User.RESTRICTED: + # For fields like CharField, IntegerField, etc., the widget used is + # straightforward and the readonly_fields list can control their behavior + readonly_fields.extend([field.name for field in self.model._meta.fields]) + # Add the multi-select fields to readonly_fields: + # Complex fields like ManyToManyField require special handling + readonly_fields.extend(["current_websites", "other_contacts", "alternative_domains"]) - # Check if the creator is restricted - if obj and obj.creator.status == models.User.RESTRICTED: - # For fields like CharField, IntegerField, etc., the widget used is - # straightforward and the readonly_fields list can control their behavior - readonly_fields.extend([field.name for field in self.model._meta.fields]) - # Add the multi-select fields to readonly_fields: - # Complex fields like ManyToManyField require special handling - readonly_fields.extend(["current_websites", "other_contacts", "alternative_domains"]) - - if request.user.has_perm("registrar.full_access_permission"): + if request.user.has_perm("registrar.full_access_permission"): + return readonly_fields + # Return restrictive Read-only fields for analysts and + # users who might not belong to groups + readonly_fields.extend([field for field in self.analyst_readonly_fields]) return readonly_fields - # Return restrictive Read-only fields for analysts and - # users who might not belong to groups - readonly_fields.extend([field for field in self.analyst_readonly_fields]) - return readonly_fields def display_restricted_warning(self, request, obj): - if obj and obj.creator.status == models.User.RESTRICTED: - messages.warning( - request, - "Cannot edit an application with a restricted creator.", - ) + logger.info("timing display_restricted_warning") + with Timer() as t: + if obj and obj.creator.status == models.User.RESTRICTED: + messages.warning( + request, + "Cannot edit an application with a restricted creator.", + ) def change_view(self, request, object_id, form_url="", extra_context=None): - obj = self.get_object(request, object_id) - self.display_restricted_warning(request, obj) - return super().change_view(request, object_id, form_url, extra_context) + logger.info("timing change_view") + with Timer() as t: + obj = self.get_object(request, object_id) + self.display_restricted_warning(request, obj) + return super().change_view(request, object_id, form_url, extra_context) class TransitionDomainAdmin(ListHeaderAdmin): From ee41a8ac73d1cbf5fd873ff645fc4d07d75e256b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 13 Feb 2024 14:50:41 -0700 Subject: [PATCH 41/55] Add some timing --- src/registrar/admin.py | 84 +++++++++++++++++++++++------------------- 1 file changed, 47 insertions(+), 37 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8f3f75105..dda3b0c3c 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -205,17 +205,21 @@ class ListHeaderAdmin(AuditedAdmin, OrderableFieldsMixin): Reference: https://code.djangoproject.com/ticket/31975 """ - return MultiFieldSortableChangeList + logger.info("timing get_changelist") + with Timer() as t: + return MultiFieldSortableChangeList def changelist_view(self, request, extra_context=None): - if extra_context is None: - extra_context = {} - # Get the filtered values - filters = self.get_filters(request) - # Pass the filtered values to the template context - extra_context["filters"] = filters - extra_context["search_query"] = request.GET.get("q", "") # Assuming the search query parameter is 'q' - return super().changelist_view(request, extra_context=extra_context) + logger.info("timing changelist_view") + with Timer() as t: + if extra_context is None: + extra_context = {} + # Get the filtered values + filters = self.get_filters(request) + # Pass the filtered values to the template context + extra_context["filters"] = filters + extra_context["search_query"] = request.GET.get("q", "") # Assuming the search query parameter is 'q' + return super().changelist_view(request, extra_context=extra_context) def get_filters(self, request): """Retrieve the current set of parameters being used to filter the table @@ -224,39 +228,40 @@ class ListHeaderAdmin(AuditedAdmin, OrderableFieldsMixin): parameter_value: string} TODO: convert investigator id to investigator username """ + logger.info("timing get_filters") + with Timer() as t: + filters = [] + # Retrieve the filter parameters + for param in request.GET.keys(): + # Exclude the default search parameter 'q' + if param != "q" and param != "o": + parameter_name = param.replace("__exact", "").replace("_type", "").replace("__id", " id") - filters = [] - # Retrieve the filter parameters - for param in request.GET.keys(): - # Exclude the default search parameter 'q' - if param != "q" and param != "o": - parameter_name = param.replace("__exact", "").replace("_type", "").replace("__id", " id") - - if parameter_name == "investigator id": - # Retrieves the corresponding contact from Users - id_value = request.GET.get(param) - try: - contact = models.User.objects.get(id=id_value) - investigator_name = contact.first_name + " " + contact.last_name + if parameter_name == "investigator id": + # Retrieves the corresponding contact from Users + id_value = request.GET.get(param) + try: + contact = models.User.objects.get(id=id_value) + investigator_name = contact.first_name + " " + contact.last_name + filters.append( + { + "parameter_name": "investigator", + "parameter_value": investigator_name, + } + ) + except models.User.DoesNotExist: + pass + else: + # For other parameter names, append a dictionary with the original + # parameter_name and the corresponding parameter_value filters.append( { - "parameter_name": "investigator", - "parameter_value": investigator_name, + "parameter_name": parameter_name, + "parameter_value": request.GET.get(param), } ) - except models.User.DoesNotExist: - pass - else: - # For other parameter names, append a dictionary with the original - # parameter_name and the corresponding parameter_value - filters.append( - { - "parameter_name": parameter_name, - "parameter_value": request.GET.get(param), - } - ) - return filters + return filters class UserContactInline(admin.StackedInline): @@ -803,6 +808,11 @@ class DomainApplicationAdmin(ListHeaderAdmin): else: return queryset.filter(investigator__id__exact=self.value()) + def __new__(self, *args, **kwargs): + logger.info("timing __new__") + with Timer() as t: + return super().__new__(self, *args, **kwargs) + # Columns list_display = [ "requested_domain", @@ -905,7 +915,7 @@ class DomainApplicationAdmin(ListHeaderAdmin): def formfield_for_manytomany(self, db_field, request, **kwargs): logger.info("timing formfield_for_manytomany") with Timer() as t: - if db_field.name in ("current_websites", "alternative_domains"): + if db_field.name in {"current_websites", "alternative_domains"}: kwargs["queryset"] = models.Website.objects.all().order_by("website") # Sort websites return super().formfield_for_manytomany(db_field, request, **kwargs) From f55d5ef93428462f549ba9bf3630efb173e19e8f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 14 Feb 2024 08:16:26 -0700 Subject: [PATCH 42/55] Logging --- src/registrar/admin.py | 66 ++++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index dda3b0c3c..18b664498 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -56,44 +56,46 @@ class MultiFieldSortableChangeList(admin.views.main.ChangeList): Mostly identical to the base implementation, except that now it can return a list of order_field objects rather than just one. """ - params = self.params - ordering = list(self.model_admin.get_ordering(request) or self._get_default_ordering()) + logger.info("timing get_ordering") + with Timer() as t: + params = self.params + ordering = list(self.model_admin.get_ordering(request) or self._get_default_ordering()) - if ORDER_VAR in params: - # Clear ordering and used params - ordering = [] + if ORDER_VAR in params: + # Clear ordering and used params + ordering = [] - order_params = params[ORDER_VAR].split(".") - for p in order_params: - try: - none, pfx, idx = p.rpartition("-") - field_name = self.list_display[int(idx)] + order_params = params[ORDER_VAR].split(".") + for p in order_params: + try: + none, pfx, idx = p.rpartition("-") + field_name = self.list_display[int(idx)] - order_fields = self.get_ordering_field(field_name) + order_fields = self.get_ordering_field(field_name) - if isinstance(order_fields, list): - for order_field in order_fields: - if order_field: - ordering.append(pfx + order_field) - else: - ordering.append(pfx + order_fields) + if isinstance(order_fields, list): + for order_field in order_fields: + if order_field: + ordering.append(pfx + order_field) + else: + ordering.append(pfx + order_fields) - except (IndexError, ValueError): - continue # Invalid ordering specified, skip it. + except (IndexError, ValueError): + continue # Invalid ordering specified, skip it. - # Add the given query's ordering fields, if any. - ordering.extend(queryset.query.order_by) + # Add the given query's ordering fields, if any. + ordering.extend(queryset.query.order_by) - # Ensure that the primary key is systematically present in the list of - # ordering fields so we can guarantee a deterministic order across all - # database backends. - pk_name = self.lookup_opts.pk.name - if not (set(ordering) & set(["pk", "-pk", pk_name, "-" + pk_name])): - # The two sets do not intersect, meaning the pk isn't present. So - # we add it. - ordering.append("-pk") + # Ensure that the primary key is systematically present in the list of + # ordering fields so we can guarantee a deterministic order across all + # database backends. + pk_name = self.lookup_opts.pk.name + if not (set(ordering) & set(["pk", "-pk", pk_name, "-" + pk_name])): + # The two sets do not intersect, meaning the pk isn't present. So + # we add it. + ordering.append("-pk") - return ordering + return ordering class CustomLogEntryAdmin(LogEntryAdmin): @@ -913,14 +915,14 @@ class DomainApplicationAdmin(ListHeaderAdmin): # lists in filter_horizontal are not sorted properly, sort them # by website def formfield_for_manytomany(self, db_field, request, **kwargs): - logger.info("timing formfield_for_manytomany") + logger.info(f"timing formfield_for_manytomany -> {db_field.name}") with Timer() as t: if db_field.name in {"current_websites", "alternative_domains"}: kwargs["queryset"] = models.Website.objects.all().order_by("website") # Sort websites return super().formfield_for_manytomany(db_field, request, **kwargs) def formfield_for_foreignkey(self, db_field, request, **kwargs): - logger.info("timing formfield_for_foreignkey") + logger.info(f"timing formfield_for_foreignkey -> {db_field.name}") with Timer() as t: # Removes invalid investigator options from the investigator dropdown if db_field.name == "investigator": From 63438f0193c659e75f1b1e7abfa8ea7485667123 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 14 Feb 2024 11:16:13 -0700 Subject: [PATCH 43/55] AdminSortFields refactor --- src/registrar/admin.py | 71 ++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 37 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 18b664498..16fb1b1d1 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -13,9 +13,12 @@ from django.contrib.contenttypes.models import ContentType from django.http.response import HttpResponseRedirect from django.urls import reverse from epplibwrapper.errors import ErrorCode, RegistryError +from registrar.models.contact import Contact from registrar.models.domain import Domain from registrar.models.domain_application import DomainApplication +from registrar.models.draft_domain import DraftDomain from registrar.models.user import User +from registrar.models.website import Website from registrar.utility import csv_export from registrar.views.utility.mixins import OrderableFieldsMixin from django.contrib.admin.views.main import ORDER_VAR @@ -123,41 +126,39 @@ class CustomLogEntryAdmin(LogEntryAdmin): class AdminSortFields: - def get_queryset(db_field): + _name_sort = Concat("first_name", "last_name", "email") + # Define a mapping of field names to model querysets and sort expressions + sort_mapping = { + "other_contacts": (Contact, _name_sort), + "authorizing_official": (Contact, _name_sort), + "submitter": (Contact, _name_sort), + "current_websites": (Website, "website"), + "alternative_domains": (Website, "website"), + "creator": (User, _name_sort), + "user": (User, _name_sort), + "investigator": (User, _name_sort), + "domain": (Domain, "name"), + "approved_domain": (Domain, "name"), + "requested_domain": (DraftDomain, "name"), + "domain_application": (DomainApplication, "requested_domain__name"), + } + + @classmethod + def get_queryset(cls, db_field): """This is a helper function for formfield_for_manytomany and formfield_for_foreignkey""" - # customize sorting - if db_field.name in ( - "other_contacts", - "authorizing_official", - "submitter", - ): - # Sort contacts by first_name, then last_name, then email - return models.Contact.objects.all().order_by(Concat("first_name", "last_name", "email")) - elif db_field.name in ("current_websites", "alternative_domains"): - # sort web sites - return models.Website.objects.all().order_by("website") - elif db_field.name in ( - "creator", - "user", - "investigator", - ): - # Sort users by first_name, then last_name, then email - return models.User.objects.all().order_by(Concat("first_name", "last_name", "email")) - elif db_field.name in ( - "domain", - "approved_domain", - ): - # Sort domains by name - return models.Domain.objects.all().order_by("name") - elif db_field.name in ("requested_domain",): - # Sort draft domains by name - return models.DraftDomain.objects.all().order_by("name") - elif db_field.name in ("domain_application",): - # Sort domain applications by name - return models.DomainApplication.objects.all().order_by("requested_domain__name") - else: + queryset_info = cls.sort_mapping.get(db_field.name, None) + if queryset_info is None: return None + model, order_by = queryset_info + match db_field.name: + case "investigator": + # We should only return users who are staff + return model.objects.filter(is_staff=True).order_by(order_by) + case _: + # If no case is defined, return the default + return model.objects.order_by(order_by) + class AuditedAdmin(admin.ModelAdmin): """Custom admin to make auditing easier.""" @@ -922,12 +923,8 @@ class DomainApplicationAdmin(ListHeaderAdmin): return super().formfield_for_manytomany(db_field, request, **kwargs) def formfield_for_foreignkey(self, db_field, request, **kwargs): - logger.info(f"timing formfield_for_foreignkey -> {db_field.name}") with Timer() as t: - # Removes invalid investigator options from the investigator dropdown - if db_field.name == "investigator": - kwargs["queryset"] = User.objects.filter(is_staff=True) - return db_field.formfield(**kwargs) + print(f"This is the db_field: {db_field}") return super().formfield_for_foreignkey(db_field, request, **kwargs) # Trigger action when a fieldset is changed From 6a0587d5fe0936e939a5bf82f880cf7a77341076 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 14 Feb 2024 13:22:10 -0700 Subject: [PATCH 44/55] Test autocomplete dropdown --- src/registrar/admin.py | 376 +++++++++---------- src/registrar/assets/sass/_theme/_admin.scss | 4 + 2 files changed, 183 insertions(+), 197 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 16fb1b1d1..18f1970da 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1,5 +1,6 @@ import logging import time +from django.db import transaction from django import forms from django.db.models.functions import Concat, Coalesce from django.db.models import Value, CharField @@ -59,46 +60,44 @@ class MultiFieldSortableChangeList(admin.views.main.ChangeList): Mostly identical to the base implementation, except that now it can return a list of order_field objects rather than just one. """ - logger.info("timing get_ordering") - with Timer() as t: - params = self.params - ordering = list(self.model_admin.get_ordering(request) or self._get_default_ordering()) + params = self.params + ordering = list(self.model_admin.get_ordering(request) or self._get_default_ordering()) - if ORDER_VAR in params: - # Clear ordering and used params - ordering = [] + if ORDER_VAR in params: + # Clear ordering and used params + ordering = [] - order_params = params[ORDER_VAR].split(".") - for p in order_params: - try: - none, pfx, idx = p.rpartition("-") - field_name = self.list_display[int(idx)] + order_params = params[ORDER_VAR].split(".") + for p in order_params: + try: + none, pfx, idx = p.rpartition("-") + field_name = self.list_display[int(idx)] - order_fields = self.get_ordering_field(field_name) + order_fields = self.get_ordering_field(field_name) - if isinstance(order_fields, list): - for order_field in order_fields: - if order_field: - ordering.append(pfx + order_field) - else: - ordering.append(pfx + order_fields) + if isinstance(order_fields, list): + for order_field in order_fields: + if order_field: + ordering.append(pfx + order_field) + else: + ordering.append(pfx + order_fields) - except (IndexError, ValueError): - continue # Invalid ordering specified, skip it. + except (IndexError, ValueError): + continue # Invalid ordering specified, skip it. - # Add the given query's ordering fields, if any. - ordering.extend(queryset.query.order_by) + # Add the given query's ordering fields, if any. + ordering.extend(queryset.query.order_by) - # Ensure that the primary key is systematically present in the list of - # ordering fields so we can guarantee a deterministic order across all - # database backends. - pk_name = self.lookup_opts.pk.name - if not (set(ordering) & set(["pk", "-pk", pk_name, "-" + pk_name])): - # The two sets do not intersect, meaning the pk isn't present. So - # we add it. - ordering.append("-pk") + # Ensure that the primary key is systematically present in the list of + # ordering fields so we can guarantee a deterministic order across all + # database backends. + pk_name = self.lookup_opts.pk.name + if not (set(ordering) & set(["pk", "-pk", pk_name, "-" + pk_name])): + # The two sets do not intersect, meaning the pk isn't present. So + # we add it. + ordering.append("-pk") - return ordering + return ordering class CustomLogEntryAdmin(LogEntryAdmin): @@ -129,18 +128,24 @@ class AdminSortFields: _name_sort = Concat("first_name", "last_name", "email") # Define a mapping of field names to model querysets and sort expressions sort_mapping = { + # == Contact == # "other_contacts": (Contact, _name_sort), "authorizing_official": (Contact, _name_sort), "submitter": (Contact, _name_sort), - "current_websites": (Website, "website"), - "alternative_domains": (Website, "website"), + # == User == # "creator": (User, _name_sort), "user": (User, _name_sort), "investigator": (User, _name_sort), + # == Website == # + "current_websites": (Website, "website"), + "alternative_domains": (Website, "website"), + # == DraftDomain == # + "requested_domain": (DraftDomain, "name"), + # == DomainApplication == # + "domain_application": (DomainApplication, "requested_domain__name"), + # == Domain == # "domain": (Domain, "name"), "approved_domain": (Domain, "name"), - "requested_domain": (DraftDomain, "name"), - "domain_application": (DomainApplication, "requested_domain__name"), } @classmethod @@ -155,6 +160,9 @@ class AdminSortFields: case "investigator": # We should only return users who are staff return model.objects.filter(is_staff=True).order_by(order_by) + case "submitter": + db_field_model = db_field.model + db_field_model.objects.select_related("submitter") case _: # If no case is defined, return the default return model.objects.order_by(order_by) @@ -208,21 +216,17 @@ class ListHeaderAdmin(AuditedAdmin, OrderableFieldsMixin): Reference: https://code.djangoproject.com/ticket/31975 """ - logger.info("timing get_changelist") - with Timer() as t: - return MultiFieldSortableChangeList + return MultiFieldSortableChangeList def changelist_view(self, request, extra_context=None): - logger.info("timing changelist_view") - with Timer() as t: - if extra_context is None: - extra_context = {} - # Get the filtered values - filters = self.get_filters(request) - # Pass the filtered values to the template context - extra_context["filters"] = filters - extra_context["search_query"] = request.GET.get("q", "") # Assuming the search query parameter is 'q' - return super().changelist_view(request, extra_context=extra_context) + if extra_context is None: + extra_context = {} + # Get the filtered values + filters = self.get_filters(request) + # Pass the filtered values to the template context + extra_context["filters"] = filters + extra_context["search_query"] = request.GET.get("q", "") # Assuming the search query parameter is 'q' + return super().changelist_view(request, extra_context=extra_context) def get_filters(self, request): """Retrieve the current set of parameters being used to filter the table @@ -231,40 +235,38 @@ class ListHeaderAdmin(AuditedAdmin, OrderableFieldsMixin): parameter_value: string} TODO: convert investigator id to investigator username """ - logger.info("timing get_filters") - with Timer() as t: - filters = [] - # Retrieve the filter parameters - for param in request.GET.keys(): - # Exclude the default search parameter 'q' - if param != "q" and param != "o": - parameter_name = param.replace("__exact", "").replace("_type", "").replace("__id", " id") + filters = [] + # Retrieve the filter parameters + for param in request.GET.keys(): + # Exclude the default search parameter 'q' + if param != "q" and param != "o": + parameter_name = param.replace("__exact", "").replace("_type", "").replace("__id", " id") - if parameter_name == "investigator id": - # Retrieves the corresponding contact from Users - id_value = request.GET.get(param) - try: - contact = models.User.objects.get(id=id_value) - investigator_name = contact.first_name + " " + contact.last_name + if parameter_name == "investigator id": + # Retrieves the corresponding contact from Users + id_value = request.GET.get(param) + try: + contact = models.User.objects.get(id=id_value) + investigator_name = contact.first_name + " " + contact.last_name - filters.append( - { - "parameter_name": "investigator", - "parameter_value": investigator_name, - } - ) - except models.User.DoesNotExist: - pass - else: - # For other parameter names, append a dictionary with the original - # parameter_name and the corresponding parameter_value filters.append( { - "parameter_name": parameter_name, - "parameter_value": request.GET.get(param), + "parameter_name": "investigator", + "parameter_value": investigator_name, } ) - return filters + except models.User.DoesNotExist: + pass + else: + # For other parameter names, append a dictionary with the original + # parameter_name and the corresponding parameter_value + filters.append( + { + "parameter_name": parameter_name, + "parameter_value": request.GET.get(param), + } + ) + return filters class UserContactInline(admin.StackedInline): @@ -775,46 +777,36 @@ class DomainApplicationAdmin(ListHeaderAdmin): """Lookup reimplementation, gets users of is_staff. Returns a list of tuples consisting of (user.id, user) """ - logger.info("timing lookups") - with Timer() as t: - - # Select all investigators that are staff, then order by name and email - privileged_users = ( - DomainApplication.objects.select_related("investigator") - .filter(investigator__is_staff=True) - .order_by( - "investigator__first_name", - "investigator__last_name", - "investigator__email" - ) + # Select all investigators that are staff, then order by name and email + privileged_users = ( + DomainApplication.objects.select_related("investigator") + .filter(investigator__is_staff=True) + .order_by( + "investigator__first_name", + "investigator__last_name", + "investigator__email" ) + ) - # Annotate the full name and return a values list that lookups can use - privileged_users_annotated = privileged_users.annotate( - full_name=Coalesce( - Concat( - "investigator__first_name", Value(" "), "investigator__last_name", output_field=CharField() - ), - "investigator__email", - output_field=CharField() - ) - ).values_list("investigator__id", "full_name") + # Annotate the full name and return a values list that lookups can use + privileged_users_annotated = privileged_users.annotate( + full_name=Coalesce( + Concat( + "investigator__first_name", Value(" "), "investigator__last_name", output_field=CharField() + ), + "investigator__email", + output_field=CharField() + ) + ).values_list("investigator__id", "full_name") - return privileged_users_annotated + return privileged_users_annotated def queryset(self, request, queryset): """Custom queryset implementation, filters by investigator""" - logger.info("timing queryset") - with Timer() as t: - if self.value() is None: - return queryset - else: - return queryset.filter(investigator__id__exact=self.value()) - - def __new__(self, *args, **kwargs): - logger.info("timing __new__") - with Timer() as t: - return super().__new__(self, *args, **kwargs) + if self.value() is None: + return queryset + else: + return queryset.filter(investigator__id__exact=self.value()) # Columns list_display = [ @@ -907,7 +899,7 @@ class DomainApplicationAdmin(ListHeaderAdmin): "anything_else", "is_policy_acknowledged", ] - + autocomplete_fields = ["submitter"] filter_horizontal = ("current_websites", "alternative_domains", "other_contacts") # Table ordering @@ -918,77 +910,73 @@ class DomainApplicationAdmin(ListHeaderAdmin): def formfield_for_manytomany(self, db_field, request, **kwargs): logger.info(f"timing formfield_for_manytomany -> {db_field.name}") with Timer() as t: - if db_field.name in {"current_websites", "alternative_domains"}: - kwargs["queryset"] = models.Website.objects.all().order_by("website") # Sort websites return super().formfield_for_manytomany(db_field, request, **kwargs) def formfield_for_foreignkey(self, db_field, request, **kwargs): + logger.info(f"timing formfield_for_foreignkey -> {db_field.name}") with Timer() as t: - print(f"This is the db_field: {db_field}") return super().formfield_for_foreignkey(db_field, request, **kwargs) # Trigger action when a fieldset is changed def save_model(self, request, obj, form, change): - logger.info("timing save_model") - with Timer() as t: - if obj and obj.creator.status != models.User.RESTRICTED: - if change: # Check if the application is being edited - # Get the original application from the database - original_obj = models.DomainApplication.objects.get(pk=obj.pk) + if obj and obj.creator.status != models.User.RESTRICTED: + if change: # Check if the application is being edited + # Get the original application from the database + original_obj = models.DomainApplication.objects.get(pk=obj.pk) - if ( - obj - and original_obj.status == models.DomainApplication.ApplicationStatus.APPROVED - and obj.status != models.DomainApplication.ApplicationStatus.APPROVED - and not obj.domain_is_not_active() - ): - # If an admin tried to set an approved application to - # another status and the related domain is already - # active, shortcut the action and throw a friendly - # error message. This action would still not go through - # shortcut or not as the rules are duplicated on the model, - # but the error would be an ugly Django error screen. + if ( + obj + and original_obj.status == models.DomainApplication.ApplicationStatus.APPROVED + and obj.status != models.DomainApplication.ApplicationStatus.APPROVED + and not obj.domain_is_not_active() + ): + # If an admin tried to set an approved application to + # another status and the related domain is already + # active, shortcut the action and throw a friendly + # error message. This action would still not go through + # shortcut or not as the rules are duplicated on the model, + # but the error would be an ugly Django error screen. - # Clear the success message - messages.set_level(request, messages.ERROR) + # Clear the success message + messages.set_level(request, messages.ERROR) - messages.error( - request, - "This action is not permitted. The domain is already active.", - ) + messages.error( + request, + "This action is not permitted. The domain is already active.", + ) - else: - if obj.status != original_obj.status: - status_method_mapping = { - models.DomainApplication.ApplicationStatus.STARTED: None, - models.DomainApplication.ApplicationStatus.SUBMITTED: obj.submit, - models.DomainApplication.ApplicationStatus.IN_REVIEW: obj.in_review, - models.DomainApplication.ApplicationStatus.ACTION_NEEDED: obj.action_needed, - models.DomainApplication.ApplicationStatus.APPROVED: obj.approve, - models.DomainApplication.ApplicationStatus.WITHDRAWN: obj.withdraw, - models.DomainApplication.ApplicationStatus.REJECTED: obj.reject, - models.DomainApplication.ApplicationStatus.INELIGIBLE: (obj.reject_with_prejudice), - } - selected_method = status_method_mapping.get(obj.status) - if selected_method is None: - logger.warning("Unknown status selected in django admin") - else: - # This is an fsm in model which will throw an error if the - # transition condition is violated, so we roll back the - # status to what it was before the admin user changed it and - # let the fsm method set it. - obj.status = original_obj.status - selected_method() + else: + if obj.status != original_obj.status: + status_method_mapping = { + models.DomainApplication.ApplicationStatus.STARTED: None, + models.DomainApplication.ApplicationStatus.SUBMITTED: obj.submit, + models.DomainApplication.ApplicationStatus.IN_REVIEW: obj.in_review, + models.DomainApplication.ApplicationStatus.ACTION_NEEDED: obj.action_needed, + models.DomainApplication.ApplicationStatus.APPROVED: obj.approve, + models.DomainApplication.ApplicationStatus.WITHDRAWN: obj.withdraw, + models.DomainApplication.ApplicationStatus.REJECTED: obj.reject, + models.DomainApplication.ApplicationStatus.INELIGIBLE: (obj.reject_with_prejudice), + } + selected_method = status_method_mapping.get(obj.status) + if selected_method is None: + logger.warning("Unknown status selected in django admin") + else: + # This is an fsm in model which will throw an error if the + # transition condition is violated, so we roll back the + # status to what it was before the admin user changed it and + # let the fsm method set it. + obj.status = original_obj.status + selected_method() - super().save_model(request, obj, form, change) - else: - # Clear the success message - messages.set_level(request, messages.ERROR) + super().save_model(request, obj, form, change) + else: + # Clear the success message + messages.set_level(request, messages.ERROR) - messages.error( - request, - "This action is not permitted for applications with a restricted creator.", - ) + messages.error( + request, + "This action is not permitted for applications with a restricted creator.", + ) def get_readonly_fields(self, request, obj=None): """Set the read-only state on form elements. @@ -996,41 +984,35 @@ class DomainApplicationAdmin(ListHeaderAdmin): admin user permissions and the application creator's status, so we'll use the baseline readonly_fields and extend it as needed. """ - logger.info("timing get_readonly_fields") - with Timer() as t: - readonly_fields = list(self.readonly_fields) + readonly_fields = list(self.readonly_fields) - # Check if the creator is restricted - if obj and obj.creator.status == models.User.RESTRICTED: - # For fields like CharField, IntegerField, etc., the widget used is - # straightforward and the readonly_fields list can control their behavior - readonly_fields.extend([field.name for field in self.model._meta.fields]) - # Add the multi-select fields to readonly_fields: - # Complex fields like ManyToManyField require special handling - readonly_fields.extend(["current_websites", "other_contacts", "alternative_domains"]) + # Check if the creator is restricted + if obj and obj.creator.status == models.User.RESTRICTED: + # For fields like CharField, IntegerField, etc., the widget used is + # straightforward and the readonly_fields list can control their behavior + readonly_fields.extend([field.name for field in self.model._meta.fields]) + # Add the multi-select fields to readonly_fields: + # Complex fields like ManyToManyField require special handling + readonly_fields.extend(["current_websites", "other_contacts", "alternative_domains"]) - if request.user.has_perm("registrar.full_access_permission"): - return readonly_fields - # Return restrictive Read-only fields for analysts and - # users who might not belong to groups - readonly_fields.extend([field for field in self.analyst_readonly_fields]) + if request.user.has_perm("registrar.full_access_permission"): return readonly_fields + # Return restrictive Read-only fields for analysts and + # users who might not belong to groups + readonly_fields.extend([field for field in self.analyst_readonly_fields]) + return readonly_fields def display_restricted_warning(self, request, obj): - logger.info("timing display_restricted_warning") - with Timer() as t: - if obj and obj.creator.status == models.User.RESTRICTED: - messages.warning( - request, - "Cannot edit an application with a restricted creator.", - ) + if obj and obj.creator.status == models.User.RESTRICTED: + messages.warning( + request, + "Cannot edit an application with a restricted creator.", + ) def change_view(self, request, object_id, form_url="", extra_context=None): - logger.info("timing change_view") - with Timer() as t: - obj = self.get_object(request, object_id) - self.display_restricted_warning(request, obj) - return super().change_view(request, object_id, form_url, extra_context) + obj = self.get_object(request, object_id) + self.display_restricted_warning(request, obj) + return super().change_view(request, object_id, form_url, extra_context) class TransitionDomainAdmin(ListHeaderAdmin): diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 760c4f13a..bc0a99067 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -270,3 +270,7 @@ h1, h2, h3, margin: 0!important; } } + +.select2-dropdown { + display: inline-grid !important; +} \ No newline at end of file From 9cb0f7ec62632dd23e01013b51a7f457fd6b3aae Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 14 Feb 2024 13:43:38 -0700 Subject: [PATCH 45/55] Test add autocomplete --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 18f1970da..e4e7c919f 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -899,7 +899,7 @@ class DomainApplicationAdmin(ListHeaderAdmin): "anything_else", "is_policy_acknowledged", ] - autocomplete_fields = ["submitter"] + autocomplete_fields = ["approved_domain", "requested_domain", "submitter", "creator", "authorizing_official", "investigator"] filter_horizontal = ("current_websites", "alternative_domains", "other_contacts") # Table ordering From 397e71463ef1a64a6328d8e68d95e08719cdac4d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 14 Feb 2024 14:37:06 -0700 Subject: [PATCH 46/55] Add ordering --- src/registrar/admin.py | 14 +++++++++----- src/registrar/assets/sass/_theme/_admin.scss | 4 ++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e4e7c919f..5c82a42f4 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -160,9 +160,6 @@ class AdminSortFields: case "investigator": # We should only return users who are staff return model.objects.filter(is_staff=True).order_by(order_by) - case "submitter": - db_field_model = db_field.model - db_field_model.objects.select_related("submitter") case _: # If no case is defined, return the default return model.objects.order_by(order_by) @@ -275,7 +272,7 @@ class UserContactInline(admin.StackedInline): model = models.Contact -class MyUserAdmin(BaseUserAdmin): +class UserAdmin(BaseUserAdmin): """Custom user admin class to use our inlines.""" inlines = [UserContactInline] @@ -430,6 +427,9 @@ class ContactAdmin(ListHeaderAdmin): "contact", "email", ] + # this ordering effects the ordering of results + # in autocomplete_fields for user + ordering = ["first_name", "last_name", "email"] # We name the custom prop 'contact' because linter # is not allowing a short_description attr on it @@ -1342,6 +1342,10 @@ class DraftDomainAdmin(ListHeaderAdmin): search_fields = ["name"] search_help_text = "Search by draft domain name." + # this ordering effects the ordering of results + # in autocomplete_fields for user + ordering = ["name"] + class VerifiedByStaffAdmin(ListHeaderAdmin): list_display = ("email", "requestor", "truncated_notes", "created_at") @@ -1368,7 +1372,7 @@ class VerifiedByStaffAdmin(ListHeaderAdmin): admin.site.unregister(LogEntry) # Unregister the default registration admin.site.register(LogEntry, CustomLogEntryAdmin) -admin.site.register(models.User, MyUserAdmin) +admin.site.register(models.User, UserAdmin) # Unregister the built-in Group model admin.site.unregister(Group) # Register UserGroup diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index bc0a99067..f08531095 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -271,6 +271,10 @@ h1, h2, h3, } } +.select2-selection__clear { + display: none; +} + .select2-dropdown { display: inline-grid !important; } \ No newline at end of file From 46d6022aed77eb81a530d4cd05a257de6b9657e4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 14 Feb 2024 15:29:10 -0700 Subject: [PATCH 47/55] Test removing concat --- src/registrar/admin.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 5c82a42f4..d4ff2e5dc 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -125,7 +125,7 @@ class CustomLogEntryAdmin(LogEntryAdmin): class AdminSortFields: - _name_sort = Concat("first_name", "last_name", "email") + _name_sort = ["first_name", "last_name", "email"] # Define a mapping of field names to model querysets and sort expressions sort_mapping = { # == Contact == # @@ -159,10 +159,13 @@ class AdminSortFields: match db_field.name: case "investigator": # We should only return users who are staff - return model.objects.filter(is_staff=True).order_by(order_by) + return model.objects.filter(is_staff=True).order_by(*order_by) case _: # If no case is defined, return the default - return model.objects.order_by(order_by) + if isinstance(order_by, list) or isinstance(order_by, tuple): + return model.objects.order_by(*order_by) + else: + return model.objects.order_by(order_by) class AuditedAdmin(admin.ModelAdmin): From 3672aaa401a8af69b1fd6b7b72578a698952d1c0 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 15 Feb 2024 09:45:18 -0700 Subject: [PATCH 48/55] Fix investigator edge case --- src/registrar/admin.py | 44 +++++++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index d4ff2e5dc..e27ce24d1 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1,6 +1,5 @@ import logging import time -from django.db import transaction from django import forms from django.db.models.functions import Concat, Coalesce from django.db.models import Value, CharField @@ -14,12 +13,7 @@ from django.contrib.contenttypes.models import ContentType from django.http.response import HttpResponseRedirect from django.urls import reverse from epplibwrapper.errors import ErrorCode, RegistryError -from registrar.models.contact import Contact -from registrar.models.domain import Domain -from registrar.models.domain_application import DomainApplication -from registrar.models.draft_domain import DraftDomain -from registrar.models.user import User -from registrar.models.website import Website +from registrar.models import (Contact, Domain, DomainApplication, DraftDomain, User, Website) from registrar.utility import csv_export from registrar.views.utility.mixins import OrderableFieldsMixin from django.contrib.admin.views.main import ORDER_VAR @@ -366,6 +360,42 @@ class UserAdmin(BaseUserAdmin): # in autocomplete_fields for user ordering = ["first_name", "last_name", "email"] + def get_search_results(self, request, queryset, search_term): + """ + Override for get_search_results. This affects any upstream model using autocomplete_fields, + such as DomainApplication. This is because autocomplete_fields uses an API call to fetch data, + and this fetch comes from this method. + """ + # Custom filtering logic + queryset, use_distinct = super().get_search_results(request, queryset, search_term) + + # If we aren't given a request to modify, we shouldn't try to + if request is None or not hasattr(request, "GET"): + return queryset, use_distinct + + # Otherwise, lets modify it! + request_get = request.GET + + # The request defines model name and field name. + # For instance, model_name could be "DomainApplication" + # and field_name could be "investigator". + model_name = request_get.get('model_name', None) + field_name = request_get.get('field_name', None) + + # Make sure we're only modifying requests from these models. + models_to_target = {"domainapplication"} + if model_name in models_to_target: + # Define rules per field + match field_name: + case "investigator": + # We should not display investigators who don't have a staff role + queryset = queryset.filter(is_staff=True) + case _: + # In the default case, do nothing + pass + + return queryset, use_distinct + # Let's define First group # (which should in theory be the ONLY group) def group(self, obj): From c5f27769becceaf3b9bc3770ba9a01c2d39f6614 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 15 Feb 2024 13:25:49 -0700 Subject: [PATCH 49/55] Fix unit tests and linting --- src/registrar/admin.py | 51 ++++++------- src/registrar/tests/common.py | 1 + src/registrar/tests/test_admin.py | 119 ++++++++++++++++++++---------- 3 files changed, 107 insertions(+), 64 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e27ce24d1..a71014c40 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -13,7 +13,7 @@ from django.contrib.contenttypes.models import ContentType from django.http.response import HttpResponseRedirect from django.urls import reverse from epplibwrapper.errors import ErrorCode, RegistryError -from registrar.models import (Contact, Domain, DomainApplication, DraftDomain, User, Website) +from registrar.models import Contact, Domain, DomainApplication, DraftDomain, User, Website from registrar.utility import csv_export from registrar.views.utility.mixins import OrderableFieldsMixin from django.contrib.admin.views.main import ORDER_VAR @@ -272,6 +272,14 @@ class UserContactInline(admin.StackedInline): class UserAdmin(BaseUserAdmin): """Custom user admin class to use our inlines.""" + class Meta: + """Contains meta information about this class""" + + model = models.User + fields = "__all__" + + _meta = Meta() + inlines = [UserContactInline] list_display = ( @@ -371,16 +379,16 @@ class UserAdmin(BaseUserAdmin): # If we aren't given a request to modify, we shouldn't try to if request is None or not hasattr(request, "GET"): - return queryset, use_distinct - + return queryset, use_distinct + # Otherwise, lets modify it! request_get = request.GET # The request defines model name and field name. # For instance, model_name could be "DomainApplication" # and field_name could be "investigator". - model_name = request_get.get('model_name', None) - field_name = request_get.get('field_name', None) + model_name = request_get.get("model_name", None) + field_name = request_get.get("field_name", None) # Make sure we're only modifying requests from these models. models_to_target = {"domainapplication"} @@ -814,21 +822,15 @@ class DomainApplicationAdmin(ListHeaderAdmin): privileged_users = ( DomainApplication.objects.select_related("investigator") .filter(investigator__is_staff=True) - .order_by( - "investigator__first_name", - "investigator__last_name", - "investigator__email" - ) + .order_by("investigator__first_name", "investigator__last_name", "investigator__email") ) # Annotate the full name and return a values list that lookups can use privileged_users_annotated = privileged_users.annotate( full_name=Coalesce( - Concat( - "investigator__first_name", Value(" "), "investigator__last_name", output_field=CharField() - ), + Concat("investigator__first_name", Value(" "), "investigator__last_name", output_field=CharField()), "investigator__email", - output_field=CharField() + output_field=CharField(), ) ).values_list("investigator__id", "full_name") @@ -932,24 +934,19 @@ class DomainApplicationAdmin(ListHeaderAdmin): "anything_else", "is_policy_acknowledged", ] - autocomplete_fields = ["approved_domain", "requested_domain", "submitter", "creator", "authorizing_official", "investigator"] + autocomplete_fields = [ + "approved_domain", + "requested_domain", + "submitter", + "creator", + "authorizing_official", + "investigator", + ] filter_horizontal = ("current_websites", "alternative_domains", "other_contacts") # Table ordering ordering = ["requested_domain__name"] - # lists in filter_horizontal are not sorted properly, sort them - # by website - def formfield_for_manytomany(self, db_field, request, **kwargs): - logger.info(f"timing formfield_for_manytomany -> {db_field.name}") - with Timer() as t: - return super().formfield_for_manytomany(db_field, request, **kwargs) - - def formfield_for_foreignkey(self, db_field, request, **kwargs): - logger.info(f"timing formfield_for_foreignkey -> {db_field.name}") - with Timer() as t: - return super().formfield_for_foreignkey(db_field, request, **kwargs) - # Trigger action when a fieldset is changed def save_model(self, request, obj, form, change): if obj and obj.creator.status != models.User.RESTRICTED: diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 2865bf5c5..8d2385c23 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -231,6 +231,7 @@ class AuditedAdminMockData: first_name="{} first_name:{}".format(item_name, short_hand), last_name="{} last_name:{}".format(item_name, short_hand), username="{} username:{}".format(item_name + str(uuid.uuid4())[:8], short_hand), + is_staff=True, )[0] return user diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index f90b18584..85853cba2 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -9,7 +9,7 @@ from registrar.admin import ( DomainApplicationAdminForm, DomainInvitationAdmin, ListHeaderAdmin, - MyUserAdmin, + UserAdmin, AuditedAdmin, ContactAdmin, DomainInformationAdmin, @@ -941,8 +941,17 @@ class TestDomainApplicationAdmin(MockEppLib): investigator_field = DomainApplication._meta.get_field("investigator") # We should only be displaying staff users, in alphabetical order - expected_dropdown = list(User.objects.filter(is_staff=True)) - current_dropdown = list(self.admin.formfield_for_foreignkey(investigator_field, request).queryset) + sorted_fields = ["first_name", "last_name", "email"] + expected_dropdown = list(User.objects.filter(is_staff=True).order_by(*sorted_fields)) + + # Grab the current dropdown. We do an API call to autocomplete to get this info. + application_queryset = self.admin.formfield_for_foreignkey(investigator_field, request).queryset + user_request = self.factory.post( + "/admin/autocomplete/?app_label=registrar&model_name=domainapplication&field_name=investigator" + ) + user_admin = UserAdmin(User, self.site) + user_queryset = user_admin.get_search_results(user_request, application_queryset, None)[0] + current_dropdown = list(user_queryset) self.assertEqual(expected_dropdown, current_dropdown) @@ -1335,10 +1344,10 @@ class ListHeaderAdminTest(TestCase): User.objects.all().delete() -class MyUserAdminTest(TestCase): +class UserAdminTest(TestCase): def setUp(self): admin_site = AdminSite() - self.admin = MyUserAdmin(model=get_user_model(), admin_site=admin_site) + self.admin = UserAdmin(model=get_user_model(), admin_site=admin_site) def test_list_display_without_username(self): request = self.client.request().wsgi_request @@ -1360,7 +1369,7 @@ class MyUserAdminTest(TestCase): request = self.client.request().wsgi_request request.user = create_superuser() fieldsets = self.admin.get_fieldsets(request) - expected_fieldsets = super(MyUserAdmin, self.admin).get_fieldsets(request) + expected_fieldsets = super(UserAdmin, self.admin).get_fieldsets(request) self.assertEqual(fieldsets, expected_fieldsets) def test_get_fieldsets_cisa_analyst(self): @@ -1396,11 +1405,46 @@ class AuditedAdminTest(TestCase): return ordered_list + def test_alphabetically_sorted_domain_application_investigator(self): + """Tests if the investigator field is alphabetically sorted by mimicking + the call event flow""" + # Creates multiple domain applications - review status does not matter + applications = multiple_unalphabetical_domain_objects("application") + + # Create a mock request + application_request = self.factory.post( + "/admin/registrar/domainapplication/{}/change/".format(applications[0].pk) + ) + + # Get the formfield data from the application page + application_admin = AuditedAdmin(DomainApplication, self.site) + field = DomainApplication.investigator.field + application_queryset = application_admin.formfield_for_foreignkey(field, application_request).queryset + + request = self.factory.post( + "/admin/autocomplete/?app_label=registrar&model_name=domainapplication&field_name=investigator" + ) + + sorted_fields = ["first_name", "last_name", "email"] + desired_sort_order = list(User.objects.filter(is_staff=True).order_by(*sorted_fields)) + + # Grab the data returned from get search results + admin = UserAdmin(User, self.site) + search_queryset = admin.get_search_results(request, application_queryset, None)[0] + current_sort_order = list(search_queryset) + + self.assertEqual( + desired_sort_order, + current_sort_order, + "Investigator is not ordered alphabetically", + ) + + # This test case should be refactored in general, as it is too overly specific and engineered def test_alphabetically_sorted_fk_fields_domain_application(self): tested_fields = [ DomainApplication.authorizing_official.field, DomainApplication.submitter.field, - DomainApplication.investigator.field, + # DomainApplication.investigator.field, DomainApplication.creator.field, DomainApplication.requested_domain.field, ] @@ -1418,39 +1462,40 @@ class AuditedAdminTest(TestCase): # but both fields are of a fixed length. # For test case purposes, this should be performant. for field in tested_fields: - isNamefield: bool = field == DomainApplication.requested_domain.field - if isNamefield: - sorted_fields = ["name"] - else: - sorted_fields = ["first_name", "last_name"] - # We want both of these to be lists, as it is richer test wise. - - desired_order = self.order_by_desired_field_helper(model_admin, request, field.name, *sorted_fields) - current_sort_order = list(model_admin.formfield_for_foreignkey(field, request).queryset) - - # Conforms to the same object structure as desired_order - current_sort_order_coerced_type = [] - - # This is necessary as .queryset and get_queryset - # return lists of different types/structures. - # We need to parse this data and coerce them into the same type. - for contact in current_sort_order: - if not isNamefield: - first = contact.first_name - last = contact.last_name + with self.subTest(field=field): + isNamefield: bool = field == DomainApplication.requested_domain.field + if isNamefield: + sorted_fields = ["name"] else: - first = contact.name - last = None + sorted_fields = ["first_name", "last_name"] + # We want both of these to be lists, as it is richer test wise. - name_tuple = self.coerced_fk_field_helper(first, last, field.name, ":") - if name_tuple is not None: - current_sort_order_coerced_type.append(name_tuple) + 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) - self.assertEqual( - desired_order, - current_sort_order_coerced_type, - "{} is not ordered alphabetically".format(field.name), - ) + # Conforms to the same object structure as desired_order + current_sort_order_coerced_type = [] + + # This is necessary as .queryset and get_queryset + # return lists of different types/structures. + # We need to parse this data and coerce them into the same type. + for contact in current_sort_order: + if not isNamefield: + first = contact.first_name + last = contact.last_name + else: + first = contact.name + last = None + + name_tuple = self.coerced_fk_field_helper(first, last, field.name, ":") + if name_tuple is not None: + current_sort_order_coerced_type.append(name_tuple) + + self.assertEqual( + desired_order, + current_sort_order_coerced_type, + "{} is not ordered alphabetically".format(field.name), + ) def test_alphabetically_sorted_fk_fields_domain_information(self): tested_fields = [ From 2d1982c252dda463d5de5aa51a1bad26dfde9e5b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 15 Feb 2024 13:50:48 -0700 Subject: [PATCH 50/55] Code cleanup --- src/registrar/admin.py | 33 +++++++++-------- src/registrar/assets/sass/_theme/_admin.scss | 2 ++ .../models/utility/generic_helper.py | 36 +++++++++++++++++++ 3 files changed, 57 insertions(+), 14 deletions(-) create mode 100644 src/registrar/models/utility/generic_helper.py diff --git a/src/registrar/admin.py b/src/registrar/admin.py index a71014c40..27acc0cea 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1,5 +1,5 @@ import logging -import time + from django import forms from django.db.models.functions import Concat, Coalesce from django.db.models import Value, CharField @@ -120,7 +120,11 @@ class CustomLogEntryAdmin(LogEntryAdmin): class AdminSortFields: _name_sort = ["first_name", "last_name", "email"] - # Define a mapping of field names to model querysets and sort expressions + + # Define a mapping of field names to model querysets and sort expressions. + # A dictionary is used for specificity, but the downside is some degree of repetition. + # To eliminate this, this list can be generated dynamically but the readability of that + # is impacted. sort_mapping = { # == Contact == # "other_contacts": (Contact, _name_sort), @@ -149,10 +153,12 @@ class AdminSortFields: if queryset_info is None: return None + # Grab the model we want to order, and grab how we want to order it model, order_by = queryset_info match db_field.name: case "investigator": - # We should only return users who are staff + # We should only return users who are staff. + # Currently a fallback. Consider removing this if it is not needed. return model.objects.filter(is_staff=True).order_by(*order_by) case _: # If no case is defined, return the default @@ -178,9 +184,14 @@ class AuditedAdmin(admin.ModelAdmin): def formfield_for_manytomany(self, db_field, request, **kwargs): """customize the behavior of formfields with manytomany relationships. the customized behavior includes sorting of objects in lists as well as customizing helper text""" + + # Define a queryset. Note that in the super of this, + # a new queryset will only be generated if one does not exist. + # Thus, the order in which we define queryset matters. queryset = AdminSortFields.get_queryset(db_field) if queryset: kwargs["queryset"] = queryset + formfield = super().formfield_for_manytomany(db_field, request, **kwargs) # customize the help text for all formfields for manytomany formfield.help_text = ( @@ -192,9 +203,14 @@ class AuditedAdmin(admin.ModelAdmin): def formfield_for_foreignkey(self, db_field, request, **kwargs): """customize the behavior of formfields with foreign key relationships. this will customize the behavior of selects. customized behavior includes sorting of objects in list""" + + # Define a queryset. Note that in the super of this, + # a new queryset will only be generated if one does not exist. + # Thus, the order in which we define queryset matters. queryset = AdminSortFields.get_queryset(db_field) if queryset: kwargs["queryset"] = queryset + return super().formfield_for_foreignkey(db_field, request, **kwargs) @@ -761,17 +777,6 @@ class DomainInformationAdmin(ListHeaderAdmin): return readonly_fields # Read-only fields for analysts -class Timer: - def __enter__(self): - self.start = time.time() - return self # This allows usage of the instance within the with block - - def __exit__(self, *args): - self.end = time.time() - self.duration = self.end - self.start - logger.info(f"Execution time: {self.duration} seconds") - - class DomainApplicationAdminForm(forms.ModelForm): """Custom form to limit transitions to available transitions""" diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index f08531095..93a0d7338 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -271,10 +271,12 @@ h1, h2, h3, } } +// Hides the "clear" button on autocomplete, as we already have one to use .select2-selection__clear { display: none; } +// Fixes a display issue where the list was entirely white, or had too much whitespace .select2-dropdown { display: inline-grid !important; } \ No newline at end of file diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py new file mode 100644 index 000000000..8fdd3eb7a --- /dev/null +++ b/src/registrar/models/utility/generic_helper.py @@ -0,0 +1,36 @@ +"""This file contains general purpose helpers that don't belong in any specific location""" +import time +import logging + + +logger = logging.getLogger(__name__) + + +class Timer: + """ + This class is used to measure execution time for performance profiling. + __enter__ and __exit__ is used such that you can wrap any code you want + around a with statement. After this exits, logger.info will print + the execution time in seconds. + + Note that this class does not account for general randomness as more + robust libraries do, so there is some tiny amount of latency involved + in using this, but it is minimal enough that for most applications it is not + noticable. + + Usage: + with Timer(): + ...some code + """ + + def __enter__(self): + """Starts the timer""" + self.start = time.time() + # This allows usage of the instance within the with block + return self + + def __exit__(self, *args): + """Ends the timer and logs what happened""" + self.end = time.time() + self.duration = self.end - self.start + logger.info(f"Execution time: {self.duration} seconds") From 133c435f033d8ce75e8a42509c10e6b7030fa7c3 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 15 Feb 2024 14:04:36 -0700 Subject: [PATCH 51/55] Linting, fix test --- src/registrar/models/utility/generic_helper.py | 1 + src/registrar/tests/test_admin.py | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 8fdd3eb7a..01d4e6b33 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -1,4 +1,5 @@ """This file contains general purpose helpers that don't belong in any specific location""" + import time import logging diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 85853cba2..ce2b40122 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -985,7 +985,13 @@ class TestDomainApplicationAdmin(MockEppLib): self.client.login(username="staffuser", password=p) request = RequestFactory().get("/") - expected_list = list(User.objects.filter(is_staff=True).order_by("first_name", "last_name", "email")) + # These names have metadata embedded in them. :investigator implicitly tests if + # these are actually from the attribute "investigator". + expected_list = [ + "AGuy AGuy last_name:investigator", + "FinalGuy FinalGuy last_name:investigator", + "SomeGuy first_name:investigator SomeGuy last_name:investigator", + ] # Get the actual sorted list of investigators from the lookups method actual_list = [item for _, item in self.admin.InvestigatorFilter.lookups(self, request, self.admin)] From 38d6285ce681afe232da04fe01415f8bbf99fc6d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 16 Feb 2024 13:09:20 -0700 Subject: [PATCH 52/55] UI hotfix for popups --- src/registrar/admin.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 306662403..f81e21416 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1085,7 +1085,12 @@ class DomainAdmin(ListHeaderAdmin): if object_id is not None: domain = Domain.objects.get(pk=object_id) years_to_extend_by = self._get_calculated_years_for_exp_date(domain) - extra_context["extended_expiration_date"] = date.today() + relativedelta(years=years_to_extend_by) + curr_exp_date = domain.registry_expiration_date + if curr_exp_date < date.today(): + extra_context["extended_expiration_date"] = date.today() + relativedelta(years=years_to_extend_by) + else: + new_date = domain.registry_expiration_date + relativedelta(years=years_to_extend_by) + extra_context["extended_expiration_date"] = new_date else: extra_context["extended_expiration_date"] = None From 92e8877cec3f3a6110d05f28636f9d91b2840f42 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 20 Feb 2024 13:23:57 -0700 Subject: [PATCH 53/55] Update _admin.scss --- src/registrar/assets/sass/_theme/_admin.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 93a0d7338..06a737d62 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -279,4 +279,4 @@ h1, h2, h3, // Fixes a display issue where the list was entirely white, or had too much whitespace .select2-dropdown { display: inline-grid !important; -} \ No newline at end of file +} From 1a088c4b35c4a32a1884133eb81e13c3f7d30fea Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 22 Feb 2024 09:03:35 -0700 Subject: [PATCH 54/55] Readd my user --- src/registrar/admin.py | 14 ++++++-------- src/registrar/tests/test_admin.py | 10 +++++----- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index f1680c76a..fc7edf1ca 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -158,10 +158,8 @@ class AdminSortFields: match db_field.name: case "investigator": # We should only return users who are staff. - # Currently a fallback. Consider removing this if it is not needed. return model.objects.filter(is_staff=True).order_by(*order_by) case _: - # If no case is defined, return the default if isinstance(order_by, list) or isinstance(order_by, tuple): return model.objects.order_by(*order_by) else: @@ -201,8 +199,8 @@ class AuditedAdmin(admin.ModelAdmin): return formfield def formfield_for_foreignkey(self, db_field, request, **kwargs): - """customize the behavior of formfields with foreign key relationships. this will customize - the behavior of selects. customized behavior includes sorting of objects in list""" + """Customize the behavior of formfields with foreign key relationships. This will customize + the behavior of selects. Customized behavior includes sorting of objects in list.""" # Define a queryset. Note that in the super of this, # a new queryset will only be generated if one does not exist. @@ -285,7 +283,7 @@ class UserContactInline(admin.StackedInline): model = models.Contact -class UserAdmin(BaseUserAdmin): +class MyUserAdmin(BaseUserAdmin): """Custom user admin class to use our inlines.""" class Meta: @@ -1095,8 +1093,8 @@ class DomainInformationInline(admin.StackedInline): return formfield def formfield_for_foreignkey(self, db_field, request, **kwargs): - """customize the behavior of formfields with foreign key relationships. this will customize - the behavior of selects. customized behavior includes sorting of objects in list""" + """Customize the behavior of formfields with foreign key relationships. This will customize + the behavior of selects. Customized behavior includes sorting of objects in list.""" queryset = AdminSortFields.get_queryset(db_field) if queryset: kwargs["queryset"] = queryset @@ -1406,7 +1404,7 @@ class VerifiedByStaffAdmin(ListHeaderAdmin): admin.site.unregister(LogEntry) # Unregister the default registration admin.site.register(LogEntry, CustomLogEntryAdmin) -admin.site.register(models.User, UserAdmin) +admin.site.register(models.User, MyUserAdmin) # Unregister the built-in Group model admin.site.unregister(Group) # Register UserGroup diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index ce2b40122..4f432728b 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -9,7 +9,7 @@ from registrar.admin import ( DomainApplicationAdminForm, DomainInvitationAdmin, ListHeaderAdmin, - UserAdmin, + MyUserAdmin, AuditedAdmin, ContactAdmin, DomainInformationAdmin, @@ -949,7 +949,7 @@ class TestDomainApplicationAdmin(MockEppLib): user_request = self.factory.post( "/admin/autocomplete/?app_label=registrar&model_name=domainapplication&field_name=investigator" ) - user_admin = UserAdmin(User, self.site) + user_admin = MyUserAdmin(User, self.site) user_queryset = user_admin.get_search_results(user_request, application_queryset, None)[0] current_dropdown = list(user_queryset) @@ -1350,10 +1350,10 @@ class ListHeaderAdminTest(TestCase): User.objects.all().delete() -class UserAdminTest(TestCase): +class MyUserAdminTest(TestCase): def setUp(self): admin_site = AdminSite() - self.admin = UserAdmin(model=get_user_model(), admin_site=admin_site) + self.admin = MyUserAdmin(model=get_user_model(), admin_site=admin_site) def test_list_display_without_username(self): request = self.client.request().wsgi_request @@ -1375,7 +1375,7 @@ class UserAdminTest(TestCase): request = self.client.request().wsgi_request request.user = create_superuser() fieldsets = self.admin.get_fieldsets(request) - expected_fieldsets = super(UserAdmin, self.admin).get_fieldsets(request) + expected_fieldsets = super(MyUserAdmin, self.admin).get_fieldsets(request) self.assertEqual(fieldsets, expected_fieldsets) def test_get_fieldsets_cisa_analyst(self): From 31c24446da46c8f0a7b3cd4e9e679322909d4f17 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 22 Feb 2024 09:06:42 -0700 Subject: [PATCH 55/55] Update test_admin.py --- src/registrar/tests/test_admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 4f432728b..6f1460144 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1435,7 +1435,7 @@ class AuditedAdminTest(TestCase): desired_sort_order = list(User.objects.filter(is_staff=True).order_by(*sorted_fields)) # Grab the data returned from get search results - admin = UserAdmin(User, self.site) + admin = MyUserAdmin(User, self.site) search_queryset = admin.get_search_results(request, application_queryset, None)[0] current_sort_order = list(search_queryset)