From 0f41d64e0e342098598b4023e98dccc60a09787b Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Thu, 1 Feb 2024 12:01:49 -0800 Subject: [PATCH 01/61] Update order of the column headers --- src/registrar/utility/csv_export.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index f9608f553..1ea9131e0 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -77,6 +77,8 @@ def parse_row(columns, domain_info: DomainInformation, security_emails_dict=None # create a dictionary of fields which can be included in output FIELDS = { "Domain name": domain.name, + "Status": domain.get_state_display(), + "Expiration date": domain.expiration_date, "Domain type": domain_type, "Agency": domain_info.federal_agency, "Organization name": domain_info.organization_name, @@ -85,8 +87,6 @@ def parse_row(columns, domain_info: DomainInformation, security_emails_dict=None "AO": domain_info.ao, # type: ignore "AO email": domain_info.authorizing_official.email if domain_info.authorizing_official else " ", "Security contact email": security_email, - "Status": domain.get_state_display(), - "Expiration date": domain.expiration_date, "Created at": domain.created_at, "First ready": domain.first_ready, "Deleted": domain.deleted, @@ -152,6 +152,8 @@ def export_data_type_to_csv(csv_file): # define columns to include in export columns = [ "Domain name", + "Status", + "Expiration date", "Domain type", "Agency", "Organization name", @@ -160,8 +162,6 @@ def export_data_type_to_csv(csv_file): "AO", "AO email", "Security contact email", - "Status", - "Expiration date", ] # Coalesce is used to replace federal_type of None with ZZZZZ sort_fields = [ 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 02/61] 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 03/61] 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 04/61] 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 a9302a8428a40a3126e74db7d1699e914d404132 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 6 Feb 2024 11:08:42 -0800 Subject: [PATCH 05/61] Get all emails for domain managers and matching header title to be dynamic --- src/registrar/utility/csv_export.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 1ea9131e0..c6e278254 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -17,7 +17,7 @@ logger = logging.getLogger(__name__) def write_header(writer, columns): """ Receives params from the parent methods and outputs a CSV with a header row. - Works with write_header as longas the same writer object is passed. + Works with write_header as long as the same writer object is passed. """ writer.writerow(columns) @@ -92,6 +92,13 @@ def parse_row(columns, domain_info: DomainInformation, security_emails_dict=None "Deleted": domain.deleted, } + # Get each domain managers email and add to list + dm_emails = [dm.email for dm in domain.permissions] + + # Matching header for domain managers to be dynamic + for i, dm_email in enumerate(dm_emails, start=1): + FIELDS[f"Domain Manager email {i}":dm_email] + row = [FIELDS.get(column, "") for column in columns] return row From 7f7e61ccc1d8a5200612bb4d516f707a932c11f5 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 6 Feb 2024 11:52:04 -0800 Subject: [PATCH 06/61] Add logic for domain manager title in header --- src/registrar/utility/csv_export.py | 30 ++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index c6e278254..9fe62bfcc 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -14,11 +14,16 @@ from registrar.models.public_contact import PublicContact logger = logging.getLogger(__name__) -def write_header(writer, columns): +def write_header(writer, columns, max_dm_count): """ Receives params from the parent methods and outputs a CSV with a header row. Works with write_header as long as the same writer object is passed. """ + + for i in range(1, max_dm_count + 1): + columns.append(f"Domain manager email {i}") + + writer.writerow("hello") writer.writerow(columns) @@ -134,12 +139,22 @@ def write_body( else: logger.warning("csv_export -> Domain was none for PublicContact") + # The maximum amount of domain managers an account has + # We get the max so we can set the column header accurately + max_dm_count = 0 + paginator_ran = False + # Reduce the memory overhead when performing the write operation paginator = Paginator(all_domain_infos, 1000) for page_num in paginator.page_range: page = paginator.page(page_num) rows = [] for domain_info in page.object_list: + # Get count of all the domain managers for an account + dm_count = len(domain_info.domain.permissions) + if dm_count > max_dm_count: + max_dm_count = dm_count + try: row = parse_row(columns, domain_info, security_emails_dict) rows.append(row) @@ -149,7 +164,12 @@ def write_body( logger.error("csv_export -> Error when parsing row, domain was None") continue + # We only want this to run once just for the column header + if paginator_ran is False: + write_header(writer, columns, max_dm_count) + writer.writerows(rows) + paginator_ran = True def export_data_type_to_csv(csv_file): @@ -184,7 +204,7 @@ def export_data_type_to_csv(csv_file): Domain.State.ON_HOLD, ], } - write_header(writer, columns) + # write_header(writer, columns) write_body(writer, columns, sort_fields, filter_condition) @@ -216,7 +236,7 @@ def export_data_full_to_csv(csv_file): Domain.State.ON_HOLD, ], } - write_header(writer, columns) + # write_header(writer, columns) write_body(writer, columns, sort_fields, filter_condition) @@ -249,7 +269,7 @@ def export_data_federal_to_csv(csv_file): Domain.State.ON_HOLD, ], } - write_header(writer, columns) + # write_header(writer, columns) write_body(writer, columns, sort_fields, filter_condition) @@ -317,6 +337,6 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): "domain__deleted__gte": start_date_formatted, } - write_header(writer, columns) + # write_header(writer, columns) write_body(writer, columns, sort_fields, filter_condition) write_body(writer, columns, sort_fields_for_deleted_domains, filter_condition_for_deleted_domains) 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 07/61] 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 08/61] 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 029f28aaf40ab72b1ab6c2066152629659a434bf Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 6 Feb 2024 13:18:05 -0800 Subject: [PATCH 09/61] Testing chanegs to trigger sandbox --- src/registrar/templates/home.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index c7a005f97..21cb1e301 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -10,7 +10,7 @@ {# the entire logged in page goes here #}
-

Manage your domains

+

Manage your domains - Test Trigger Here

Date: Tue, 6 Feb 2024 14:04:43 -0800 Subject: [PATCH 10/61] Test an open column title --- src/registrar/utility/csv_export.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index fcbce470d..64afd2d06 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -189,7 +189,14 @@ def export_data_type_to_csv(csv_file): "AO", "AO email", "Security contact email", + "Domain Manager email", ] + + # STUCK HERE + + # So the problem is we don't even have access to domains or a count here. + # We could pass it in, but it's messy. Maybe helper function? Seems repetitive + # Coalesce is used to replace federal_type of None with ZZZZZ sort_fields = [ "organization_type", 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 11/61] 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 12/61] 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 13/61] 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 14/61] 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 15/61] 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 16/61] 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 17/61] 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 18/61] 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 19/61] 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 20/61] 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 21/61] 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 22/61] 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 23/61] 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 24/61] 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 25/61] 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 26/61] 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 27/61] 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 28/61] 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 29/61] 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 30/61] 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 31/61] 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 32/61] 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 33/61] 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 34/61] 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 35/61] 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 36/61] 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 37/61] 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 39/61] 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 40/61] 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 8ed88aa137b5093b323cd1bc4102e7ee14d9af12 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 12 Feb 2024 10:02:55 -0800 Subject: [PATCH 41/61] Add conditionals for domain managers --- src/registrar/utility/csv_export.py | 53 ++++++++++++++++------------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 64afd2d06..fc3faae05 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -14,14 +14,14 @@ from registrar.utility.enums import DefaultEmail logger = logging.getLogger(__name__) -def write_header(writer, columns, max_dm_count): +def write_header(writer, columns, max_dm_count, get_domain_managers): """ Receives params from the parent methods and outputs a CSV with a header row. Works with write_header as long as the same writer object is passed. """ - - for i in range(1, max_dm_count + 1): - columns.append(f"Domain manager email {i}") + if get_domain_managers: + for i in range(1, max_dm_count + 1): + columns.append(f"Domain manager email {i}") writer.writerow("hello") writer.writerow(columns) @@ -48,7 +48,7 @@ def get_domain_infos(filter_condition, sort_fields): return domain_infos_cleaned -def parse_row(columns, domain_info: DomainInformation, security_emails_dict=None): +def parse_row(columns, domain_info: DomainInformation, get_domain_managers, security_emails_dict=None): """Given a set of columns, generate a new row from cleaned column data""" # Domain should never be none when parsing this information @@ -97,12 +97,13 @@ def parse_row(columns, domain_info: DomainInformation, security_emails_dict=None "Deleted": domain.deleted, } - # Get each domain managers email and add to list - dm_emails = [dm.email for dm in domain.permissions] + if get_domain_managers: + # Get each domain managers email and add to list + dm_emails = [dm.email for dm in domain.permissions] - # Matching header for domain managers to be dynamic - for i, dm_email in enumerate(dm_emails, start=1): - FIELDS[f"Domain Manager email {i}":dm_email] + # Matching header for domain managers to be dynamic + for i, dm_email in enumerate(dm_emails, start=1): + FIELDS[f"Domain Manager email {i}":dm_email] row = [FIELDS.get(column, "") for column in columns] return row @@ -113,12 +114,17 @@ def write_body( columns, sort_fields, filter_condition, + get_domain_managers=False, ): """ Receives params from the parent methods and outputs a CSV with fltered and sorted domains. Works with write_header as longas the same writer object is passed. """ + # We only want to write the domain manager information for export_thing_here so we have to make it conditional + + # Trying to make the domain managers logic conditional + # Get the domainInfos all_domain_infos = get_domain_infos(filter_condition, sort_fields) @@ -151,12 +157,12 @@ def write_body( rows = [] for domain_info in page.object_list: # Get count of all the domain managers for an account - dm_count = len(domain_info.domain.permissions) - if dm_count > max_dm_count: - max_dm_count = dm_count - + if get_domain_managers: + dm_count = len(domain_info.domain.permissions) + if dm_count > max_dm_count: + max_dm_count = dm_count try: - row = parse_row(columns, domain_info, security_emails_dict) + row = parse_row(columns, domain_info, security_emails_dict, get_domain_managers) rows.append(row) except ValueError: # This should not happen. If it does, just skip this row. @@ -165,8 +171,8 @@ def write_body( continue # We only want this to run once just for the column header - if paginator_ran is False: - write_header(writer, columns, max_dm_count) + if paginator_ran is False and "Domain name" in columns: + write_header(writer, columns, max_dm_count, get_domain_managers) writer.writerows(rows) paginator_ran = True @@ -189,14 +195,9 @@ def export_data_type_to_csv(csv_file): "AO", "AO email", "Security contact email", - "Domain Manager email", + # For domain manager we are pass it in as a parameter below in write_body ] - # STUCK HERE - - # So the problem is we don't even have access to domains or a count here. - # We could pass it in, but it's messy. Maybe helper function? Seems repetitive - # Coalesce is used to replace federal_type of None with ZZZZZ sort_fields = [ "organization_type", @@ -212,7 +213,7 @@ def export_data_type_to_csv(csv_file): ], } # write_header(writer, columns) - write_body(writer, columns, sort_fields, filter_condition) + write_body(writer, columns, sort_fields, filter_condition, True) def export_data_full_to_csv(csv_file): @@ -345,5 +346,9 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): } # write_header(writer, columns) + # Domains that got created write_body(writer, columns, sort_fields, filter_condition) + # Domains that got deleted + # Have a way to skip the header for this one + write_body(writer, columns, sort_fields_for_deleted_domains, filter_condition_for_deleted_domains) From ee76372a19ee90aed70cfa47745527037d99de26 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 12 Feb 2024 10:16:45 -0800 Subject: [PATCH 42/61] Add comment to jumpstart deploy --- src/registrar/utility/csv_export.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index fa252112d..9ffbb057c 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -19,11 +19,13 @@ def write_header(writer, columns, max_dm_count, get_domain_managers): Receives params from the parent methods and outputs a CSV with a header row. Works with write_header as long as the same writer object is passed. """ + + # If we have domain managers, set column title dynamically here if get_domain_managers: for i in range(1, max_dm_count + 1): columns.append(f"Domain manager email {i}") - writer.writerow("hello") + writer.writerow("hellotesting123") writer.writerow(columns) @@ -96,7 +98,7 @@ def parse_row(columns, domain_info: DomainInformation, get_domain_managers, secu "First ready": domain.first_ready, "Deleted": domain.deleted, } - + if get_domain_managers: # Get each domain managers email and add to list dm_emails = [dm.email for dm in domain.permissions] 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 43/61] 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 44/61] 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 45/61] 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 46/61] 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 f1f91669d7f64bd43b60e566ce504dc61b492339 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 12 Feb 2024 15:02:36 -0800 Subject: [PATCH 47/61] Fix logic --- src/registrar/utility/csv_export.py | 53 +++++++++++------------------ 1 file changed, 20 insertions(+), 33 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 9ffbb057c..242c997de 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -14,18 +14,12 @@ from registrar.utility.enums import DefaultEmail logger = logging.getLogger(__name__) -def write_header(writer, columns, max_dm_count, get_domain_managers): +def write_header(writer, columns): """ Receives params from the parent methods and outputs a CSV with a header row. Works with write_header as long as the same writer object is passed. """ - # If we have domain managers, set column title dynamically here - if get_domain_managers: - for i in range(1, max_dm_count + 1): - columns.append(f"Domain manager email {i}") - - writer.writerow("hellotesting123") writer.writerow(columns) @@ -50,7 +44,7 @@ def get_domain_infos(filter_condition, sort_fields): return domain_infos_cleaned -def parse_row(columns, domain_info: DomainInformation, get_domain_managers, security_emails_dict=None): +def parse_row(columns, domain_info: DomainInformation, security_emails_dict=None, get_domain_managers=False): """Given a set of columns, generate a new row from cleaned column data""" # Domain should never be none when parsing this information @@ -101,11 +95,11 @@ def parse_row(columns, domain_info: DomainInformation, get_domain_managers, secu if get_domain_managers: # Get each domain managers email and add to list - dm_emails = [dm.email for dm in domain.permissions] + dm_emails = [dm.user.email for dm in domain.permissions.all()] - # Matching header for domain managers to be dynamic + # This is the row fields for i, dm_email in enumerate(dm_emails, start=1): - FIELDS[f"Domain Manager email {i}":dm_email] + FIELDS[f"Domain manager email {i}"] = dm_email row = [FIELDS.get(column, "") for column in columns] return row @@ -117,6 +111,7 @@ def write_body( sort_fields, filter_condition, get_domain_managers=False, + should_write_header=True, ): """ Receives params from the parent methods and outputs a CSV with fltered and sorted domains. @@ -151,18 +146,19 @@ def write_body( # We get the max so we can set the column header accurately max_dm_count = 0 paginator_ran = False - # Reduce the memory overhead when performing the write operation paginator = Paginator(all_domain_infos, 1000) for page_num in paginator.page_range: page = paginator.page(page_num) rows = [] for domain_info in page.object_list: - # Get count of all the domain managers for an account if get_domain_managers: - dm_count = len(domain_info.domain.permissions) + dm_count = len(domain_info.domain.permissions.all()) if dm_count > max_dm_count: max_dm_count = dm_count + for i in range(1, max_dm_count + 1): + if f"Domain manager email {i}" not in columns: + columns.append(f"Domain manager email {i}") try: row = parse_row(columns, domain_info, security_emails_dict, get_domain_managers) rows.append(row) @@ -171,13 +167,12 @@ def write_body( # It indicates that DomainInformation.domain is None. logger.error("csv_export -> Error when parsing row, domain was None") continue + # We only want this to run once just for the column header + if paginator_ran is False and should_write_header: + write_header(writer, columns) - # We only want this to run once just for the column header - if paginator_ran is False and "Domain name" in columns: - write_header(writer, columns, max_dm_count, get_domain_managers) - - writer.writerows(rows) - paginator_ran = True + writer.writerows(rows) + paginator_ran = True def export_data_type_to_csv(csv_file): @@ -214,8 +209,7 @@ def export_data_type_to_csv(csv_file): Domain.State.ON_HOLD, ], } - # write_header(writer, columns) - write_body(writer, columns, sort_fields, filter_condition, True) + write_body(writer, columns, sort_fields, filter_condition, True, True) def export_data_full_to_csv(csv_file): @@ -246,8 +240,7 @@ def export_data_full_to_csv(csv_file): Domain.State.ON_HOLD, ], } - # write_header(writer, columns) - write_body(writer, columns, sort_fields, filter_condition) + write_body(writer, columns, sort_fields, filter_condition, False, True) def export_data_federal_to_csv(csv_file): @@ -279,8 +272,7 @@ def export_data_federal_to_csv(csv_file): Domain.State.ON_HOLD, ], } - # write_header(writer, columns) - write_body(writer, columns, sort_fields, filter_condition) + write_body(writer, columns, sort_fields, filter_condition, False, True) def get_default_start_date(): @@ -347,10 +339,5 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): "domain__deleted__gte": start_date_formatted, } - # write_header(writer, columns) - # Domains that got created - write_body(writer, columns, sort_fields, filter_condition) - # Domains that got deleted - # Have a way to skip the header for this one - - write_body(writer, columns, sort_fields_for_deleted_domains, filter_condition_for_deleted_domains) + write_body(writer, columns, sort_fields, filter_condition, False, True) + write_body(writer, columns, sort_fields_for_deleted_domains, filter_condition_for_deleted_domains, False, False) From 3c48fb9f7e54de29a603211bae8abe7faaa49041 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 12 Feb 2024 15:26:31 -0800 Subject: [PATCH 48/61] Refactor parts of the code --- src/registrar/utility/csv_export.py | 66 +++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 17 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 242c997de..1c8f365cf 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -105,6 +105,51 @@ def parse_row(columns, domain_info: DomainInformation, security_emails_dict=None return row +# def _check_domain_managers(domain_info, columns): +# max_dm_count = 0 + +# dm_count = len(domain_info.domain.permissions.all()) +# if dm_count > max_dm_count: +# max_dm_count = dm_count +# for i in range(1, max_dm_count + 1): +# if f"Domain manager email {i}" not in columns: +# columns.append(f"Domain manager email {i}") + +# return columns + + +def _get_security_emails(sec_contact_ids): + """ + Retrieve security contact emails for the given security contact IDs. + """ + security_emails_dict = {} + public_contacts = ( + PublicContact.objects.only("email", "domain__name") + .select_related("domain") + .filter(registry_id__in=sec_contact_ids) + ) + + # Populate a dictionary of domain names and their security contacts + for contact in public_contacts: + domain: Domain = contact.domain + if domain is not None and domain.name not in security_emails_dict: + security_emails_dict[domain.name] = contact.email + else: + logger.warning("csv_export -> Domain was none for PublicContact") + + return security_emails_dict + + +def update_columns_with_domain_managers(columns, max_dm_count): + """ + Update the columns list to include "Domain manager email" headers + based on the maximum domain manager count. + """ + for i in range(1, max_dm_count + 1): + if f"Domain manager email {i}" not in columns: + columns.append(f"Domain manager email {i}") + + def write_body( writer, columns, @@ -127,24 +172,12 @@ def write_body( # Store all security emails to avoid epp calls or excessive filters sec_contact_ids = all_domain_infos.values_list("domain__security_contact_registry_id", flat=True) - security_emails_dict = {} - public_contacts = ( - PublicContact.objects.only("email", "domain__name") - .select_related("domain") - .filter(registry_id__in=sec_contact_ids) - ) - # Populate a dictionary of domain names and their security contacts - for contact in public_contacts: - domain: Domain = contact.domain - if domain is not None and domain.name not in security_emails_dict: - security_emails_dict[domain.name] = contact.email - else: - logger.warning("csv_export -> Domain was none for PublicContact") + security_emails_dict = _get_security_emails(sec_contact_ids) # The maximum amount of domain managers an account has - # We get the max so we can set the column header accurately max_dm_count = 0 + # We get the max so we can set the column header accurately paginator_ran = False # Reduce the memory overhead when performing the write operation paginator = Paginator(all_domain_infos, 1000) @@ -153,12 +186,11 @@ def write_body( rows = [] for domain_info in page.object_list: if get_domain_managers: + # _check_domain_managers(domain_info, columns) dm_count = len(domain_info.domain.permissions.all()) if dm_count > max_dm_count: max_dm_count = dm_count - for i in range(1, max_dm_count + 1): - if f"Domain manager email {i}" not in columns: - columns.append(f"Domain manager email {i}") + update_columns_with_domain_managers(columns, max_dm_count) try: row = parse_row(columns, domain_info, security_emails_dict, get_domain_managers) rows.append(row) From 8ea135b5a4b8aff3baf15dfe21cf640506976e3e Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 12 Feb 2024 16:48:33 -0800 Subject: [PATCH 49/61] Update unit tests --- src/registrar/tests/test_reports.py | 84 +++++++++++++++++++++++++---- src/registrar/utility/csv_export.py | 14 ----- 2 files changed, 75 insertions(+), 23 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 630904218..3ea6b3695 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -7,13 +7,14 @@ from registrar.models.domain import Domain from registrar.models.public_contact import PublicContact from registrar.models.user import User from django.contrib.auth import get_user_model +from registrar.models.user_domain_role import UserDomainRole from registrar.tests.common import MockEppLib from registrar.utility.csv_export import ( - write_header, write_body, get_default_start_date, get_default_end_date, ) + from django.core.management import call_command from unittest.mock import MagicMock, call, mock_open, patch from api.views import get_current_federal, get_current_full @@ -336,11 +337,28 @@ class ExportDataTest(MockEppLib): federal_agency="Armed Forces Retirement Home", ) + meoward_user = get_user_model().objects.create( + username="meoward_username", first_name="first_meoward", last_name="last_meoward", email="meoward@rocks.com" + ) + + _, created = UserDomainRole.objects.get_or_create( + user=meoward_user, domain=self.domain_1, role=UserDomainRole.Roles.MANAGER + ) + + _, created = UserDomainRole.objects.get_or_create( + user=self.user, domain=self.domain_1, role=UserDomainRole.Roles.MANAGER + ) + + _, created = UserDomainRole.objects.get_or_create( + user=meoward_user, domain=self.domain_2, role=UserDomainRole.Roles.MANAGER + ) + def tearDown(self): PublicContact.objects.all().delete() Domain.objects.all().delete() DomainInformation.objects.all().delete() User.objects.all().delete() + UserDomainRole.objects.all().delete() super().tearDown() def test_export_domains_to_writer_security_emails(self): @@ -383,7 +401,6 @@ class ExportDataTest(MockEppLib): } self.maxDiff = None # Call the export functions - write_header(writer, columns) write_body(writer, columns, sort_fields, filter_condition) # Reset the CSV file's position to the beginning csv_file.seek(0) @@ -440,7 +457,6 @@ class ExportDataTest(MockEppLib): ], } # Call the export functions - write_header(writer, columns) write_body(writer, columns, sort_fields, filter_condition) # Reset the CSV file's position to the beginning csv_file.seek(0) @@ -489,7 +505,6 @@ class ExportDataTest(MockEppLib): ], } # Call the export functions - write_header(writer, columns) write_body(writer, columns, sort_fields, filter_condition) # Reset the CSV file's position to the beginning csv_file.seek(0) @@ -567,7 +582,6 @@ class ExportDataTest(MockEppLib): } # Call the export functions - write_header(writer, columns) write_body( writer, columns, @@ -575,10 +589,7 @@ class ExportDataTest(MockEppLib): filter_condition, ) write_body( - writer, - columns, - sort_fields_for_deleted_domains, - filter_conditions_for_deleted_domains, + writer, columns, sort_fields_for_deleted_domains, filter_conditions_for_deleted_domains, False, False ) # Reset the CSV file's position to the beginning @@ -606,6 +617,61 @@ class ExportDataTest(MockEppLib): self.assertEqual(csv_content, expected_content) + def test_export_domains_to_writer_domain_managers(self): + """Test that export_domains_to_writer returns the + expected domain managers""" + with less_console_noise(): + # Create a CSV file in memory + csv_file = StringIO() + writer = csv.writer(csv_file) + # Define columns, sort fields, and filter condition + + columns = [ + "Domain name", + "Status", + "Expiration date", + "Domain type", + "Agency", + "Organization name", + "City", + "State", + "AO", + "AO email", + "Security contact email", + ] + sort_fields = ["domain__name"] + filter_condition = { + "domain__state__in": [ + Domain.State.READY, + Domain.State.DNS_NEEDED, + Domain.State.ON_HOLD, + ], + } + self.maxDiff = None + # Call the export functions + write_body(writer, columns, sort_fields, filter_condition, True, True) + # Reset the CSV file's position to the beginning + csv_file.seek(0) + # Read the content into a variable + csv_content = csv_file.read() + # We expect READY domains, + # sorted alphabetially by domain name + expected_content = ( + "Domain name,Status,Expiration date,Domain type,Agency," + "Organization name,City,State,AO,AO email," + "Security contact email,Domain manager email 1,Domain manager email 2,\n" + "adomain10.gov,Ready,,Federal,Armed Forces Retirement Home,,,, , ,\n" + "adomain2.gov,Dns needed,,Interstate,,,,, , , ,meoward@rocks.com\n" + "cdomain1.gov,Ready,,Federal - Executive,World War I Centennial Commission,,," + ", , , ,meoward@rocks.com,info@example.com\n" + "ddomain3.gov,On hold,,Federal,Armed Forces Retirement Home,,,, , , ,,\n" + ) + # Normalize line endings and remove commas, + # spaces and leading/trailing whitespace + csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() + expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() + self.assertEqual(csv_content, expected_content) + class HelperFunctions(TestCase): """This asserts that 1=1. Its limited usefulness lies in making sure the helper methods stay healthy.""" diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 1c8f365cf..806e72952 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -105,19 +105,6 @@ def parse_row(columns, domain_info: DomainInformation, security_emails_dict=None return row -# def _check_domain_managers(domain_info, columns): -# max_dm_count = 0 - -# dm_count = len(domain_info.domain.permissions.all()) -# if dm_count > max_dm_count: -# max_dm_count = dm_count -# for i in range(1, max_dm_count + 1): -# if f"Domain manager email {i}" not in columns: -# columns.append(f"Domain manager email {i}") - -# return columns - - def _get_security_emails(sec_contact_ids): """ Retrieve security contact emails for the given security contact IDs. @@ -186,7 +173,6 @@ def write_body( rows = [] for domain_info in page.object_list: if get_domain_managers: - # _check_domain_managers(domain_info, columns) dm_count = len(domain_info.domain.permissions.all()) if dm_count > max_dm_count: max_dm_count = dm_count From d329ae6c8f1167f0100e832858e6c63ed17d489a Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 12 Feb 2024 17:46:24 -0800 Subject: [PATCH 50/61] Update comments --- src/registrar/templates/home.html | 2 +- src/registrar/utility/csv_export.py | 15 ++++++--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index 2f4f6d2e9..9dd093de9 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -13,7 +13,7 @@ {% block messages %} {% include "includes/form_messages.html" %} {% endblock %} -

    Manage your domains - TEST TEST 123

    +

    Manage your domains

    diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 806e72952..af4162489 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -97,7 +97,7 @@ def parse_row(columns, domain_info: DomainInformation, security_emails_dict=None # Get each domain managers email and add to list dm_emails = [dm.user.email for dm in domain.permissions.all()] - # This is the row fields + # Set up the "matching header" + row field data for i, dm_email in enumerate(dm_emails, start=1): FIELDS[f"Domain manager email {i}"] = dm_email @@ -129,7 +129,7 @@ def _get_security_emails(sec_contact_ids): def update_columns_with_domain_managers(columns, max_dm_count): """ - Update the columns list to include "Domain manager email" headers + Update the columns list to include "Domain manager email {#}" headers based on the maximum domain manager count. """ for i in range(1, max_dm_count + 1): @@ -148,13 +148,10 @@ def write_body( """ Receives params from the parent methods and outputs a CSV with fltered and sorted domains. Works with write_header as longas the same writer object is passed. + get_domain_managers: Conditional bc we only use domain manager info for export_data_full_to_csv + should_write_header: Conditional bc export_data_growth_to_csv calls write_body twice """ - # We only want to write the domain manager information for export_thing_here so we have to make it conditional - - # Trying to make the domain managers logic conditional - - # Get the domainInfos all_domain_infos = get_domain_infos(filter_condition, sort_fields) # Store all security emails to avoid epp calls or excessive filters @@ -163,8 +160,9 @@ def write_body( security_emails_dict = _get_security_emails(sec_contact_ids) # The maximum amount of domain managers an account has - max_dm_count = 0 # We get the max so we can set the column header accurately + max_dm_count = 0 + # Flag bc we don't want to set header every loop paginator_ran = False # Reduce the memory overhead when performing the write operation paginator = Paginator(all_domain_infos, 1000) @@ -185,7 +183,6 @@ def write_body( # It indicates that DomainInformation.domain is None. logger.error("csv_export -> Error when parsing row, domain was None") continue - # We only want this to run once just for the column header if paginator_ran is False and should_write_header: write_header(writer, columns) From 6e062e841fc9f085d6f05a9a6c9743a237551e52 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 13 Feb 2024 10:25:41 -0800 Subject: [PATCH 51/61] Update test comments --- src/registrar/tests/test_reports.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 3ea6b3695..386acb253 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -341,6 +341,7 @@ class ExportDataTest(MockEppLib): username="meoward_username", first_name="first_meoward", last_name="last_meoward", email="meoward@rocks.com" ) + # Test for more than 1 domain manager _, created = UserDomainRole.objects.get_or_create( user=meoward_user, domain=self.domain_1, role=UserDomainRole.Roles.MANAGER ) @@ -349,6 +350,7 @@ class ExportDataTest(MockEppLib): user=self.user, domain=self.domain_1, role=UserDomainRole.Roles.MANAGER ) + # Test for just 1 domain manager _, created = UserDomainRole.objects.get_or_create( user=meoward_user, domain=self.domain_2, role=UserDomainRole.Roles.MANAGER ) From a8857ef18e917b9b81b7eefe23921d3751ec4669 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 14 Feb 2024 13:09:25 -0800 Subject: [PATCH 52/61] Address refactor feedback --- src/registrar/utility/csv_export.py | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index af4162489..1c2b64f3d 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -133,8 +133,7 @@ def update_columns_with_domain_managers(columns, max_dm_count): based on the maximum domain manager count. """ for i in range(1, max_dm_count + 1): - if f"Domain manager email {i}" not in columns: - columns.append(f"Domain manager email {i}") + columns.append(f"Domain manager email {i}") def write_body( @@ -159,22 +158,19 @@ def write_body( security_emails_dict = _get_security_emails(sec_contact_ids) - # The maximum amount of domain managers an account has - # We get the max so we can set the column header accurately - max_dm_count = 0 - # Flag bc we don't want to set header every loop - paginator_ran = False # Reduce the memory overhead when performing the write operation paginator = Paginator(all_domain_infos, 1000) + + if get_domain_managers: + # We want to get the max amont of domain managers an + # account has to set the column header dynamically + max_dm_count = max(len(domain_info.domain.permissions.all()) for domain_info in all_domain_infos) + update_columns_with_domain_managers(columns, max_dm_count) + for page_num in paginator.page_range: page = paginator.page(page_num) rows = [] for domain_info in page.object_list: - if get_domain_managers: - dm_count = len(domain_info.domain.permissions.all()) - if dm_count > max_dm_count: - max_dm_count = dm_count - update_columns_with_domain_managers(columns, max_dm_count) try: row = parse_row(columns, domain_info, security_emails_dict, get_domain_managers) rows.append(row) @@ -183,11 +179,10 @@ def write_body( # It indicates that DomainInformation.domain is None. logger.error("csv_export -> Error when parsing row, domain was None") continue - if paginator_ran is False and should_write_header: + if should_write_header: write_header(writer, columns) writer.writerows(rows) - paginator_ran = True def export_data_type_to_csv(csv_file): @@ -222,6 +217,7 @@ def export_data_type_to_csv(csv_file): Domain.State.READY, Domain.State.DNS_NEEDED, Domain.State.ON_HOLD, + Domain.State.UNKNOWN, # REMOVE ], } write_body(writer, columns, sort_fields, filter_condition, True, True) From 2dbf9cfa84d7a1847d0e8a9567ee215d5e3be866 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 14 Feb 2024 13:33:41 -0800 Subject: [PATCH 53/61] Remove extra test line --- src/registrar/utility/csv_export.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 1c2b64f3d..85bc00e33 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -217,7 +217,6 @@ def export_data_type_to_csv(csv_file): Domain.State.READY, Domain.State.DNS_NEEDED, Domain.State.ON_HOLD, - Domain.State.UNKNOWN, # REMOVE ], } write_body(writer, columns, sort_fields, filter_condition, True, True) From 22cefd6ed8caf85f7709f029da0955b3ab470568 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 14 Feb 2024 13:45:33 -0800 Subject: [PATCH 54/61] Add parameter logic --- src/registrar/tests/test_reports.py | 28 ++++++++++++++++++++++------ src/registrar/utility/csv_export.py | 17 ++++++++++++----- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 386acb253..fa5acc96d 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -403,7 +403,10 @@ class ExportDataTest(MockEppLib): } self.maxDiff = None # Call the export functions - write_body(writer, columns, sort_fields, filter_condition) + write_body( + writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True + ) + # Reset the CSV file's position to the beginning csv_file.seek(0) # Read the content into a variable @@ -459,7 +462,9 @@ class ExportDataTest(MockEppLib): ], } # Call the export functions - write_body(writer, columns, sort_fields, filter_condition) + write_body( + writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True + ) # Reset the CSV file's position to the beginning csv_file.seek(0) # Read the content into a variable @@ -507,7 +512,9 @@ class ExportDataTest(MockEppLib): ], } # Call the export functions - write_body(writer, columns, sort_fields, filter_condition) + write_body( + writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True + ) # Reset the CSV file's position to the beginning csv_file.seek(0) # Read the content into a variable @@ -589,11 +596,17 @@ class ExportDataTest(MockEppLib): columns, sort_fields, filter_condition, + get_domain_managers=False, + should_write_header=True, ) write_body( - writer, columns, sort_fields_for_deleted_domains, filter_conditions_for_deleted_domains, False, False + writer, + columns, + sort_fields_for_deleted_domains, + filter_conditions_for_deleted_domains, + get_domain_managers=False, + should_write_header=False, ) - # Reset the CSV file's position to the beginning csv_file.seek(0) @@ -651,7 +664,10 @@ class ExportDataTest(MockEppLib): } self.maxDiff = None # Call the export functions - write_body(writer, columns, sort_fields, filter_condition, True, True) + write_body( + writer, columns, sort_fields, filter_condition, get_domain_managers=True, should_write_header=True + ) + # Reset the CSV file's position to the beginning csv_file.seek(0) # Read the content into a variable diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 85bc00e33..1e4895f80 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -219,7 +219,7 @@ def export_data_type_to_csv(csv_file): Domain.State.ON_HOLD, ], } - write_body(writer, columns, sort_fields, filter_condition, True, True) + write_body(writer, columns, sort_fields, filter_condition, get_domain_managers=True, should_write_header=True) def export_data_full_to_csv(csv_file): @@ -250,7 +250,7 @@ def export_data_full_to_csv(csv_file): Domain.State.ON_HOLD, ], } - write_body(writer, columns, sort_fields, filter_condition, False, True) + write_body(writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True) def export_data_federal_to_csv(csv_file): @@ -282,7 +282,7 @@ def export_data_federal_to_csv(csv_file): Domain.State.ON_HOLD, ], } - write_body(writer, columns, sort_fields, filter_condition, False, True) + write_body(writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True) def get_default_start_date(): @@ -349,5 +349,12 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): "domain__deleted__gte": start_date_formatted, } - write_body(writer, columns, sort_fields, filter_condition, False, True) - write_body(writer, columns, sort_fields_for_deleted_domains, filter_condition_for_deleted_domains, False, False) + write_body(writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True) + write_body( + writer, + columns, + sort_fields_for_deleted_domains, + filter_condition_for_deleted_domains, + get_domain_managers=False, + should_write_header=False, + ) From a66fb36432f3c028cfe5b92f342bd55973ee0103 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Thu, 15 Feb 2024 10:52:14 -0800 Subject: [PATCH 55/61] Update function naming and length check --- src/registrar/tests/test_reports.py | 16 ++++++++-------- src/registrar/utility/csv_export.py | 15 ++++++++------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index fa5acc96d..011c60b93 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -10,7 +10,7 @@ from django.contrib.auth import get_user_model from registrar.models.user_domain_role import UserDomainRole from registrar.tests.common import MockEppLib from registrar.utility.csv_export import ( - write_body, + write_csv, get_default_start_date, get_default_end_date, ) @@ -403,7 +403,7 @@ class ExportDataTest(MockEppLib): } self.maxDiff = None # Call the export functions - write_body( + write_csv( writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True ) @@ -427,7 +427,7 @@ class ExportDataTest(MockEppLib): expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() self.assertEqual(csv_content, expected_content) - def test_write_body(self): + def test_write_csv(self): """Test that write_body returns the existing domain, test that sort by domain name works, test that filter works""" @@ -462,7 +462,7 @@ class ExportDataTest(MockEppLib): ], } # Call the export functions - write_body( + write_csv( writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True ) # Reset the CSV file's position to the beginning @@ -512,7 +512,7 @@ class ExportDataTest(MockEppLib): ], } # Call the export functions - write_body( + write_csv( writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True ) # Reset the CSV file's position to the beginning @@ -591,7 +591,7 @@ class ExportDataTest(MockEppLib): } # Call the export functions - write_body( + write_csv( writer, columns, sort_fields, @@ -599,7 +599,7 @@ class ExportDataTest(MockEppLib): get_domain_managers=False, should_write_header=True, ) - write_body( + write_csv( writer, columns, sort_fields_for_deleted_domains, @@ -664,7 +664,7 @@ class ExportDataTest(MockEppLib): } self.maxDiff = None # Call the export functions - write_body( + write_csv( writer, columns, sort_fields, filter_condition, get_domain_managers=True, should_write_header=True ) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 1e4895f80..90e80f551 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -136,7 +136,7 @@ def update_columns_with_domain_managers(columns, max_dm_count): columns.append(f"Domain manager email {i}") -def write_body( +def write_csv( writer, columns, sort_fields, @@ -161,7 +161,7 @@ def write_body( # Reduce the memory overhead when performing the write operation paginator = Paginator(all_domain_infos, 1000) - if get_domain_managers: + if get_domain_managers and len(all_domain_infos) > 0: # We want to get the max amont of domain managers an # account has to set the column header dynamically max_dm_count = max(len(domain_info.domain.permissions.all()) for domain_info in all_domain_infos) @@ -179,6 +179,7 @@ def write_body( # It indicates that DomainInformation.domain is None. logger.error("csv_export -> Error when parsing row, domain was None") continue + if should_write_header: write_header(writer, columns) @@ -219,7 +220,7 @@ def export_data_type_to_csv(csv_file): Domain.State.ON_HOLD, ], } - write_body(writer, columns, sort_fields, filter_condition, get_domain_managers=True, should_write_header=True) + write_csv(writer, columns, sort_fields, filter_condition, get_domain_managers=True, should_write_header=True) def export_data_full_to_csv(csv_file): @@ -250,7 +251,7 @@ def export_data_full_to_csv(csv_file): Domain.State.ON_HOLD, ], } - write_body(writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True) + write_csv(writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True) def export_data_federal_to_csv(csv_file): @@ -282,7 +283,7 @@ def export_data_federal_to_csv(csv_file): Domain.State.ON_HOLD, ], } - write_body(writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True) + write_csv(writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True) def get_default_start_date(): @@ -349,8 +350,8 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): "domain__deleted__gte": start_date_formatted, } - write_body(writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True) - write_body( + write_csv(writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True) + write_csv( writer, columns, sort_fields_for_deleted_domains, 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 56/61] 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 0e23946cf85ad74b42b14e4c84d82a8b6aa27527 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 20 Feb 2024 10:45:49 -0700 Subject: [PATCH 57/61] Bug fix --- src/registrar/models/domain_information.py | 6 ++++++ src/registrar/models/utility/domain_helper.py | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index acaa330bb..b35f41ee4 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -255,6 +255,12 @@ class DomainInformation(TimeStampedModel): else: da_many_to_many_dict[field] = getattr(domain_application, field).all() + # This will not happen in normal code flow, but having some redundancy doesn't hurt. + # da_dict should not have "id" under any circumstances. + if "id" in da_dict: + logger.warning("create_from_da() -> Found attribute 'id' when trying to create") + da_dict.pop("id", None) + # Create a placeholder DomainInformation object domain_info = DomainInformation(**da_dict) diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 230b23e16..9e3559676 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -180,8 +180,8 @@ class DomainHelper: """ # Get a list of the existing fields on model_1 and model_2 - model_1_fields = set(field.name for field in model_1._meta.get_fields() if field != "id") - model_2_fields = set(field.name for field in model_2._meta.get_fields() if field != "id") + model_1_fields = set(field.name for field in model_1._meta.get_fields() if field.name != "id") + model_2_fields = set(field.name for field in model_2._meta.get_fields() if field.name != "id") # Get the fields that exist on both DomainApplication and DomainInformation common_fields = model_1_fields & model_2_fields From 0d23007fcd316f8c4093e7e5bbf9b99b84c5fa76 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 20 Feb 2024 11:13:09 -0700 Subject: [PATCH 58/61] Add some additional information --- src/registrar/models/domain_information.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index b35f41ee4..1a50efe2c 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -257,6 +257,8 @@ class DomainInformation(TimeStampedModel): # This will not happen in normal code flow, but having some redundancy doesn't hurt. # da_dict should not have "id" under any circumstances. + # If it does have it, then this indicates that common_fields is overzealous in the data + # that it is returning. Try looking in DomainHelper.get_common_fields. if "id" in da_dict: logger.warning("create_from_da() -> Found attribute 'id' when trying to create") da_dict.pop("id", 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 59/61] 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 60/61] 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 61/61] 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)