From a1fe3aaccab532170bbac7c2683fc3a63d694fe0 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 17 Dec 2023 18:14:08 -0700 Subject: [PATCH 01/42] Finally fixed! --- src/registrar/forms/application_wizard.py | 83 ++++++++++++++++++++++- 1 file changed, 80 insertions(+), 3 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 5310c4610..faf005d71 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -15,6 +15,7 @@ from registrar.templatetags.url_helpers import public_site_url from registrar.utility import errors logger = logging.getLogger(__name__) +from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper class RegistrarForm(forms.Form): @@ -262,7 +263,7 @@ class OrganizationContactForm(RegistrarForm): validators=[ RegexValidator( "^[0-9]{5}(?:-[0-9]{4})?$|^$", - message="Enter a zip code in the required format, like 12345 or 12345-6789.", + message="Enter a zip code in the form of 12345 or 12345-6789.", ) ], ) @@ -557,6 +558,7 @@ class YourContactForm(RegistrarForm): class OtherContactsForm(RegistrarForm): first_name = forms.CharField( + # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="First name / given name", error_messages={"required": "Enter the first name / given name of this contact."}, ) @@ -565,10 +567,12 @@ class OtherContactsForm(RegistrarForm): label="Middle name (optional)", ) last_name = forms.CharField( + # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="Last name / family name", error_messages={"required": "Enter the last name / family name of this contact."}, ) title = forms.CharField( + # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="Title or role in your organization", error_messages={ "required": ( @@ -577,27 +581,100 @@ class OtherContactsForm(RegistrarForm): }, ) email = forms.EmailField( + # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="Email", error_messages={"invalid": ("Enter an email address in the required format, like name@example.com.")}, ) phone = PhoneNumberField( + # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="Phone", error_messages={"required": "Enter a phone number for this contact."}, ) + + + + # Override clean in order to correct validation logic + def clean(self): + cleaned = self.cleaned_data # super().clean() + TerminalHelper.print_debug(f"""{TerminalColors.MAGENTA} CLEANING... + FIELDS: + {self.fields} + CLEANED: + {cleaned}{TerminalColors.ENDC}""") + + form_is_empty = all(v is None or v == '' for v in cleaned.values()) + # if not form_is_empty: #TODO: add check for "marked for deleteion" when implementing button + #Validation Logic + # if not self.cleaned_data["first_name"]: + # self.add_error('first_name', "Enter the first name / given name of this contact.") + # if not self.cleaned_data["last_name"]: + # self.add_error('last_name', "Enter the last name / family name of this contact.") + # if not self.cleaned_data["title"]: + # self.add_error('title', "Enter the title or role in your organization of this contact (e.g., Chief Information Officer).") + if form_is_empty: + TerminalHelper.print_debug(f"""{TerminalColors.MAGENTA} ***** BLANK FORM DETECTED ******* + {TerminalColors.ENDC}""") + + # clear any errors raised by the form fields + # (before this clean() method is run, each field + # performs its own clean, which could result in + # errors that we wish to ignore at this point) + # + # NOTE: we cannot just clear() the errors list. + # That causes problems. + for field in self.fields: + if field in self.errors: + del self.errors[field] + return cleaned + + + # # Always return a value to use as the new cleaned data, even if + # # this method didn't change it. + # return data + + # for field in self.fields.values(): + # isBlank = field is None or field == '' + # TerminalHelper.print_debug(f"""{TerminalColors.YELLOW} {field} is blank = {isBlank} {TerminalColors.ENDC}""") + + + # TerminalHelper.print_debug(f"""{TerminalColors.YELLOW} {field.required} {TerminalColors.ENDC}""") + # return None + # return super().clean() + + # def _should_delete_form(self, form): + # TerminalHelper.print_debug(f"{TerminalColors.MAGENTA} SHOULD DELETE FORM?...{TerminalColors.ENDC}") + # """Return whether or not the form was marked for deletion.""" + # return all(field is None or field == '' for field in self.fields.values()) class BaseOtherContactsFormSet(RegistrarFormSet): JOIN = "other_contacts" + # def get_deletion_widget(self): + # delete_button = '' + # return delete_button + + # # if form.cleaned_data.get(forms.formsets.DELETION_FIELD_NAME): + # # return True # marked for delete + # # fields = ('name', 'question', 'amount', 'measure', 'comment') + # print(form.cleaned_data) + # if not any(form.cleaned_data): + # return True + # return False + def should_delete(self, cleaned): - empty = (isinstance(v, str) and not v.strip() for v in cleaned.values()) + # TerminalHelper.print_debug(f"{TerminalColors.MAGENTA} SHOULD DELETE OTHER CONTACTS?... {cleaned.values()}{TerminalColors.ENDC}") + empty = (isinstance(v, str) and (v.strip() == "" or v == None) for v in cleaned.values()) return all(empty) + def to_database(self, obj: DomainApplication): + TerminalHelper.print_debug(f"{TerminalColors.OKCYAN} TO DATABASE {TerminalColors.ENDC}") self._to_database(obj, self.JOIN, self.should_delete, self.pre_update, self.pre_create) @classmethod def from_database(cls, obj): + TerminalHelper.print_debug(f"{TerminalColors.OKCYAN} FROM DATABASE {TerminalColors.ENDC}") return super().from_database(obj, cls.JOIN, cls.on_fetch) @@ -606,6 +683,7 @@ OtherContactsFormSet = forms.formset_factory( extra=1, absolute_max=1500, # django default; use `max_num` to limit entries formset=BaseOtherContactsFormSet, + # can_delete=True #TODO: use when implementing delete button? ) @@ -634,7 +712,6 @@ class AnythingElseForm(RegistrarForm): ], ) - class RequirementsForm(RegistrarForm): is_policy_acknowledged = forms.BooleanField( label="I read and agree to the requirements for operating .gov domains.", From 6d4bd3d592eaeeed72c7f0424237c6d2da180d96 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 17 Dec 2023 19:17:50 -0700 Subject: [PATCH 02/42] Cleaned up dead experiments and fixed CORS error --- src/registrar/forms/application_wizard.py | 63 +---------------------- 1 file changed, 1 insertion(+), 62 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index faf005d71..5aaa738e7 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -15,7 +15,6 @@ from registrar.templatetags.url_helpers import public_site_url from registrar.utility import errors logger = logging.getLogger(__name__) -from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper class RegistrarForm(forms.Form): @@ -558,7 +557,6 @@ class YourContactForm(RegistrarForm): class OtherContactsForm(RegistrarForm): first_name = forms.CharField( - # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="First name / given name", error_messages={"required": "Enter the first name / given name of this contact."}, ) @@ -567,12 +565,10 @@ class OtherContactsForm(RegistrarForm): label="Middle name (optional)", ) last_name = forms.CharField( - # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="Last name / family name", error_messages={"required": "Enter the last name / family name of this contact."}, ) title = forms.CharField( - # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="Title or role in your organization", error_messages={ "required": ( @@ -581,40 +577,19 @@ class OtherContactsForm(RegistrarForm): }, ) email = forms.EmailField( - # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="Email", error_messages={"invalid": ("Enter an email address in the required format, like name@example.com.")}, ) phone = PhoneNumberField( - # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="Phone", error_messages={"required": "Enter a phone number for this contact."}, ) - - # Override clean in order to correct validation logic def clean(self): - cleaned = self.cleaned_data # super().clean() - TerminalHelper.print_debug(f"""{TerminalColors.MAGENTA} CLEANING... - FIELDS: - {self.fields} - CLEANED: - {cleaned}{TerminalColors.ENDC}""") - + cleaned = super().clean() #self.cleaned_data <---Why does this cause a CORS error? form_is_empty = all(v is None or v == '' for v in cleaned.values()) - # if not form_is_empty: #TODO: add check for "marked for deleteion" when implementing button - #Validation Logic - # if not self.cleaned_data["first_name"]: - # self.add_error('first_name', "Enter the first name / given name of this contact.") - # if not self.cleaned_data["last_name"]: - # self.add_error('last_name', "Enter the last name / family name of this contact.") - # if not self.cleaned_data["title"]: - # self.add_error('title', "Enter the title or role in your organization of this contact (e.g., Chief Information Officer).") if form_is_empty: - TerminalHelper.print_debug(f"""{TerminalColors.MAGENTA} ***** BLANK FORM DETECTED ******* - {TerminalColors.ENDC}""") - # clear any errors raised by the form fields # (before this clean() method is run, each field # performs its own clean, which could result in @@ -627,54 +602,19 @@ class OtherContactsForm(RegistrarForm): del self.errors[field] return cleaned - - # # Always return a value to use as the new cleaned data, even if - # # this method didn't change it. - # return data - - # for field in self.fields.values(): - # isBlank = field is None or field == '' - # TerminalHelper.print_debug(f"""{TerminalColors.YELLOW} {field} is blank = {isBlank} {TerminalColors.ENDC}""") - - - # TerminalHelper.print_debug(f"""{TerminalColors.YELLOW} {field.required} {TerminalColors.ENDC}""") - # return None - # return super().clean() - - # def _should_delete_form(self, form): - # TerminalHelper.print_debug(f"{TerminalColors.MAGENTA} SHOULD DELETE FORM?...{TerminalColors.ENDC}") - # """Return whether or not the form was marked for deletion.""" - # return all(field is None or field == '' for field in self.fields.values()) - class BaseOtherContactsFormSet(RegistrarFormSet): JOIN = "other_contacts" - # def get_deletion_widget(self): - # delete_button = '' - # return delete_button - - # # if form.cleaned_data.get(forms.formsets.DELETION_FIELD_NAME): - # # return True # marked for delete - # # fields = ('name', 'question', 'amount', 'measure', 'comment') - # print(form.cleaned_data) - # if not any(form.cleaned_data): - # return True - # return False - def should_delete(self, cleaned): - # TerminalHelper.print_debug(f"{TerminalColors.MAGENTA} SHOULD DELETE OTHER CONTACTS?... {cleaned.values()}{TerminalColors.ENDC}") empty = (isinstance(v, str) and (v.strip() == "" or v == None) for v in cleaned.values()) return all(empty) - def to_database(self, obj: DomainApplication): - TerminalHelper.print_debug(f"{TerminalColors.OKCYAN} TO DATABASE {TerminalColors.ENDC}") self._to_database(obj, self.JOIN, self.should_delete, self.pre_update, self.pre_create) @classmethod def from_database(cls, obj): - TerminalHelper.print_debug(f"{TerminalColors.OKCYAN} FROM DATABASE {TerminalColors.ENDC}") return super().from_database(obj, cls.JOIN, cls.on_fetch) @@ -683,7 +623,6 @@ OtherContactsFormSet = forms.formset_factory( extra=1, absolute_max=1500, # django default; use `max_num` to limit entries formset=BaseOtherContactsFormSet, - # can_delete=True #TODO: use when implementing delete button? ) From 1035130978be56ced2c7aa5c5f521dc64149d91a Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 17 Dec 2023 19:31:01 -0700 Subject: [PATCH 03/42] Linted and minor comment update --- src/registrar/forms/application_wizard.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 5aaa738e7..b45ee5f68 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -586,9 +586,9 @@ class OtherContactsForm(RegistrarForm): ) # Override clean in order to correct validation logic - def clean(self): - cleaned = super().clean() #self.cleaned_data <---Why does this cause a CORS error? - form_is_empty = all(v is None or v == '' for v in cleaned.values()) + def clean(self): + cleaned = super().clean() + form_is_empty = all(v is None or v == "" for v in cleaned.values()) if form_is_empty: # clear any errors raised by the form fields # (before this clean() method is run, each field @@ -651,6 +651,7 @@ class AnythingElseForm(RegistrarForm): ], ) + class RequirementsForm(RegistrarForm): is_policy_acknowledged = forms.BooleanField( label="I read and agree to the requirements for operating .gov domains.", From 93b978abd3849d5e9d8339bba4642da3d625733a Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 17 Dec 2023 19:31:10 -0700 Subject: [PATCH 04/42] Updated comment --- src/registrar/forms/application_wizard.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index b45ee5f68..c0cd6e5b4 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -587,6 +587,7 @@ class OtherContactsForm(RegistrarForm): # Override clean in order to correct validation logic def clean(self): + # NOTE: using self.cleaned_data directly apparently causes a CORS error cleaned = super().clean() form_is_empty = all(v is None or v == "" for v in cleaned.values()) if form_is_empty: From c2536d63005452d09a96d13829b27ac751d4e61f Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 19 Dec 2023 11:55:34 -0500 Subject: [PATCH 05/42] WIP urls, view, template, JS connecting tge template submission to view, rough-in the export method --- src/registrar/admin.py | 5 +- src/registrar/assets/js/get-gov-admin.js | 51 +++++++++++++ src/registrar/config/settings.py | 2 +- src/registrar/config/urls.py | 6 ++ src/registrar/forms/__init__.py | 1 + src/registrar/forms/admin.py | 13 ++++ .../migrations/0057_domain_deleted_at.py | 17 +++++ src/registrar/models/domain.py | 9 +++ .../templates/admin/export_data.html | 18 +++++ src/registrar/templates/admin/index.html | 20 +++++ src/registrar/templates/export_data.html | 19 +++++ src/registrar/utility/csv_export.py | 76 +++++++++++++++++-- src/registrar/views/admin_views.py | 74 ++++++++++++++++++ 13 files changed, 302 insertions(+), 9 deletions(-) create mode 100644 src/registrar/forms/admin.py create mode 100644 src/registrar/migrations/0057_domain_deleted_at.py create mode 100644 src/registrar/templates/admin/export_data.html create mode 100644 src/registrar/templates/admin/index.html create mode 100644 src/registrar/templates/export_data.html create mode 100644 src/registrar/views/admin_views.py diff --git a/src/registrar/admin.py b/src/registrar/admin.py index def7c64b1..9a8a655c1 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -795,6 +795,9 @@ class DomainAdmin(ListHeaderAdmin): "name", "organization_type", "state", + "created_at", + "deleted_at", + "expiration_date", ] def organization_type(self, obj): @@ -809,7 +812,7 @@ class DomainAdmin(ListHeaderAdmin): search_help_text = "Search by domain name." change_form_template = "django/admin/domain_change_form.html" change_list_template = "django/admin/domain_change_list.html" - readonly_fields = ["state", "expiration_date"] + readonly_fields = ["state", "expiration_date", "deleted_at"] def export_data_type(self, request): # match the CSV example with all the fields diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 53eeb22a3..dcdeeb106 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -275,3 +275,54 @@ function enableRelatedWidgetButtons(changeLink, deleteLink, viewLink, elementPk, viewLink.setAttribute('href', viewLink.getAttribute('data-href-template').replace('__fk__', elementPk)); viewLink.setAttribute('title', viewLink.getAttribute('title-template').replace('selected item', elementText)); } + +// function performDataLookup(e) { +// e.preventDefault(); // Prevent the default form submission + +// console.log('Form submitted!'); + + +// var form = document.getElementById("exportDataForm"); +// var formData = new FormData(form); + +// // Perform an AJAX request to fetch data +// fetch('/admin/', { +// method: 'POST', +// body: formData, +// }) +// .then(response => { +// if (!response.ok) { +// console.log(response); +// console.log(`HTTP error! Status: ${response.status}`); +// throw new Error(`HTTP error! Status: ${response.status}`); +// } +// return response.json(); +// }) +// .then(data => { +// // Handle the data (update the result div, for example) +// document.getElementById("dataResult").innerText = JSON.stringify(data); +// }) +// .catch(error => console.error('Error:', error)); +// } + + (function (){ + + document.getElementById('exportLink').addEventListener('click', function(event) { + event.preventDefault(); // Prevent the default link behavior + + // Get the selected start and end dates + var startDate = document.getElementById('start').value; + var endDate = document.getElementById('end').value; + + var exportUrl = document.getElementById('exportLink').dataset.exportUrl; + + // Build the URL with parameters + exportUrl += "?start_date=" + startDate + "&end_date=" + endDate; + + // Redirect to the export URL + window.location.href = exportUrl; + }); + + + // document.getElementById('exportDataForm').addEventListener('submit', performDataLookup); +})(); \ No newline at end of file diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index c99daf72b..317cb9375 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -210,7 +210,7 @@ STATICFILES_DIRS = [ TEMPLATES = [ { "BACKEND": "django.template.backends.django.DjangoTemplates", - "DIRS": [BASE_DIR / "registrar" / "templates"], + # "DIRS": [BASE_DIR / "registrar" / "templates"], # look for templates inside installed apps # required by django-debug-toolbar "APP_DIRS": True, diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index d30c85ce9..af44fc237 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -9,6 +9,10 @@ from django.urls import include, path from django.views.generic import RedirectView from registrar import views +# from registrar.views.admin_views import export_data +from registrar.views.admin_views import ExportData + + from registrar.views.application import Step from registrar.views.utility import always_404 from api.views import available, get_current_federal, get_current_full @@ -50,6 +54,8 @@ urlpatterns = [ RedirectView.as_view(pattern_name="logout", permanent=False), ), path("admin/", admin.site.urls), + # path('export_data/', export_data, name='admin_export_data'), + path('export_data/', ExportData.as_view(), name='admin_export_data'), path( "application//edit/", views.ApplicationWizard.as_view(), diff --git a/src/registrar/forms/__init__.py b/src/registrar/forms/__init__.py index 914db375c..a1c34e777 100644 --- a/src/registrar/forms/__init__.py +++ b/src/registrar/forms/__init__.py @@ -10,3 +10,4 @@ from .domain import ( DomainDsdataFormset, DomainDsdataForm, ) +from .admin import DataExportForm diff --git a/src/registrar/forms/admin.py b/src/registrar/forms/admin.py new file mode 100644 index 000000000..78d359743 --- /dev/null +++ b/src/registrar/forms/admin.py @@ -0,0 +1,13 @@ +from django import forms + +class DataExportForm(forms.Form): + # start_date = forms.DateField(label='Start date', widget=forms.DateInput(attrs={'type': 'date'})) + # end_date = forms.DateField(label='End date', widget=forms.DateInput(attrs={'type': 'date'})) + + security_email = forms.EmailField( + label="Security email (optional)", + required=False, + error_messages={ + "invalid": 'dsas', + }, + ) \ No newline at end of file diff --git a/src/registrar/migrations/0057_domain_deleted_at.py b/src/registrar/migrations/0057_domain_deleted_at.py new file mode 100644 index 000000000..e93068945 --- /dev/null +++ b/src/registrar/migrations/0057_domain_deleted_at.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.7 on 2023-12-19 05:42 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0056_alter_domain_state_alter_domainapplication_status_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="domain", + name="deleted_at", + field=models.DateField(editable=False, help_text="Deleted at date", null=True), + ), + ] diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 44cb45433..25c60ca2a 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -33,6 +33,7 @@ from django.db.models import DateField from .utility.domain_field import DomainField from .utility.domain_helper import DomainHelper from .utility.time_stamped_model import TimeStampedModel +from django.utils import timezone from .public_contact import PublicContact @@ -959,6 +960,12 @@ class Domain(TimeStampedModel, DomainHelper): null=True, help_text=("Duplication of registry's expiration date saved for ease of reporting"), ) + + deleted_at = DateField( + null=True, + editable=False, + help_text="Deleted at date", + ) def isActive(self): return self.state == Domain.State.CREATED @@ -1279,6 +1286,8 @@ class Domain(TimeStampedModel, DomainHelper): try: logger.info("deletedInEpp()-> inside _delete_domain") self._delete_domain() + self.deleted_at = timezone.now() + self.save() except RegistryError as err: logger.error(f"Could not delete domain. Registry returned error: {err}") raise err diff --git a/src/registrar/templates/admin/export_data.html b/src/registrar/templates/admin/export_data.html new file mode 100644 index 000000000..589284c1b --- /dev/null +++ b/src/registrar/templates/admin/export_data.html @@ -0,0 +1,18 @@ + +
+ {% csrf_token %} + + + {% for field in form %} +
+ {{ field.label_tag }} + {{ field }} + {% if field.errors %} +
{{ field.errors|join:", " }}
+ {% endif %} +
+ {% endfor %} + + + +
\ No newline at end of file diff --git a/src/registrar/templates/admin/index.html b/src/registrar/templates/admin/index.html new file mode 100644 index 000000000..82c881a9e --- /dev/null +++ b/src/registrar/templates/admin/index.html @@ -0,0 +1,20 @@ +{% extends "admin/index.html" %} + +{% block content %} +
+ {% include "admin/app_list.html" with app_list=app_list show_changelinks=True %} +
+

Welcome to the Custom Admin Homepage!

+ + {% comment %} {% include "export_data.html" %} {% endcomment %} + + + + + + + Export + +
+
+{% endblock %} \ No newline at end of file diff --git a/src/registrar/templates/export_data.html b/src/registrar/templates/export_data.html new file mode 100644 index 000000000..69b00c744 --- /dev/null +++ b/src/registrar/templates/export_data.html @@ -0,0 +1,19 @@ +{% load static field_helpers%} + +
+ {% csrf_token %} + + + {% for field in form %} +
+ {{ field.label_tag }} + {{ field }} + {% if field.errors %} +
{{ field.errors|join:", " }}
+ {% endif %} +
+ {% endfor %} + + + +
\ No newline at end of file diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 64136c3a5..5ceb49cd2 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -1,4 +1,5 @@ import csv +from datetime import datetime from registrar.models.domain import Domain from registrar.models.domain_information import DomainInformation from registrar.models.public_contact import PublicContact @@ -10,9 +11,18 @@ def export_domains_to_writer(writer, columns, sort_fields, filter_condition): # write columns headers to writer writer.writerow(columns) - domainInfos = DomainInformation.objects.filter(**filter_condition).order_by(*sort_fields) + + print(f"filter_condition {filter_condition}") + if 'domain__created_at__gt' in filter_condition: + + domainInfos = DomainInformation.objects.filter(domain__state=Domain.State.DELETED).order_by("domain__deleted_at") + print(f"filtering by deleted {domainInfos}") + else: + domainInfos = DomainInformation.objects.filter(**filter_condition).order_by(*sort_fields) + for domainInfo in domainInfos: security_contacts = domainInfo.domain.contacts.filter(contact_type=PublicContact.ContactTypeChoices.SECURITY) + print(f"regular filtering {domainInfos}") # For linter ao = " " if domainInfo.authorizing_official: @@ -31,9 +41,11 @@ def export_domains_to_writer(writer, columns, sort_fields, filter_condition): "State": domainInfo.state_territory, "AO": ao, "AO email": domainInfo.authorizing_official.email if domainInfo.authorizing_official else " ", - "Security Contact Email": security_contacts[0].email if security_contacts else " ", + "Security contact email": security_contacts[0].email if security_contacts else " ", "Status": domainInfo.domain.state, - "Expiration Date": domainInfo.domain.expiration_date, + "Expiration date": domainInfo.domain.expiration_date, + "Created at": domainInfo.domain.created_at, + "Deleted at": domainInfo.domain.deleted_at, } writer.writerow([FIELDS.get(column, "") for column in columns]) @@ -50,9 +62,9 @@ def export_data_type_to_csv(csv_file): "State", "AO", "AO email", - "Security Contact Email", + "Security contact email", "Status", - "Expiration Date", + "Expiration date", ] # Coalesce is used to replace federal_type of None with ZZZZZ sort_fields = [ @@ -81,7 +93,7 @@ def export_data_full_to_csv(csv_file): "Organization name", "City", "State", - "Security Contact Email", + "Security contact email", ] # Coalesce is used to replace federal_type of None with ZZZZZ sort_fields = [ @@ -110,7 +122,7 @@ def export_data_federal_to_csv(csv_file): "Organization name", "City", "State", - "Security Contact Email", + "Security contact email", ] # Coalesce is used to replace federal_type of None with ZZZZZ sort_fields = [ @@ -128,3 +140,53 @@ def export_data_federal_to_csv(csv_file): ], } export_domains_to_writer(writer, columns, sort_fields, filter_condition) + +def export_data_growth_to_csv(csv_file, start_date, end_date): + + print(f'start_date {start_date}') + print(f'end_date {end_date}') + + # Check if start_date is not empty before using strptime + if start_date: + start_date_formatted = datetime.strptime(start_date, "%Y-%m-%d") + print(f'start_date_formatted {start_date_formatted}') + else: + # Handle the case where start_date is missing or empty + print('ON NO') + start_date_formatted = None # Replace with appropriate handling + + if end_date: + end_date_formatted = datetime.strptime(end_date, "%Y-%m-%d") + print(f'start_date_formatted {end_date_formatted}') + else: + # Handle the case where start_date is missing or empty + print('ON NO') + end_date_formatted = None # Replace with appropriate handling + + writer = csv.writer(csv_file) + # define columns to include in export + columns = [ + "Domain name", + "Domain type", + "Agency", + "Organization name", + "City", + "State", + "Security contact email", + "Created at", + "Expiration date", + ] + # Coalesce is used to replace federal_type of None with ZZZZZ + sort_fields = [ + "created_at", + "domain__name", + ] + filter_condition = { + "domain__state__in": [ + Domain.State.UNKNOWN, + Domain.State.DELETED, + ], + "domain__expiration_date__lt": end_date_formatted, + "domain__created_at__gt": start_date_formatted, + } + export_domains_to_writer(writer, columns, sort_fields, filter_condition) diff --git a/src/registrar/views/admin_views.py b/src/registrar/views/admin_views.py new file mode 100644 index 000000000..6c6aa6616 --- /dev/null +++ b/src/registrar/views/admin_views.py @@ -0,0 +1,74 @@ +"""Admin-related views.""" + +from django.http import HttpResponse, JsonResponse +from django.views import View +from django.views.decorators.csrf import csrf_exempt +from django.shortcuts import render + +from registrar.utility import csv_export +from ..forms import DataExportForm +from django.views.generic import TemplateView + +from registrar.models import ( + Domain, + DomainApplication, + DomainInvitation, + DomainInformation, + UserDomainRole, +) +import logging + + +logger = logging.getLogger(__name__) + +def export_data(self): + """CSV download""" + print('VIEW') + # Federal only + response = HttpResponse(content_type="text/csv") + response["Content-Disposition"] = 'attachment; filename="current-federal.csv"' + csv_export.export_data_growth_to_csv(response) + return response + +class ExportData(View): + + def get(self, request, *args, **kwargs): + # Get start_date and end_date from the request's GET parameters + start_date = request.GET.get('start_date', '') + end_date = request.GET.get('end_date', '') + + print(start_date) + print(end_date) + # Do something with start_date and end_date, e.g., include in the CSV export logic + + # # Federal only + response = HttpResponse(content_type="text/csv") + response["Content-Disposition"] = f'attachment; filename="growth-from-{start_date}-to-{end_date}.csv"' + csv_export.export_data_growth_to_csv(response, start_date, end_date) + + + # response = HttpResponse(content_type="text/csv") + # response["Content-Disposition"] = 'attachment; filename="current-federal.csv"' + # csv_export.export_data_growth_to_csv(response) + + return response + + +# class ExportData(TemplateView): +# """Django form""" + +# template_name = "export_data.html" +# form_class = DataExportForm + +# def form_valid(self, form): +# print('Form is valid') +# # Form is valid, perform data export logic here +# return JsonResponse({'message': 'Data exported successfully!'}, content_type='application/json') + +# def form_invalid(self, form): +# print('Form is invalid') +# # Form is invalid, return error response +# return JsonResponse({'error': 'Invalid form data'}, status=400, content_type='application/json') + + + \ No newline at end of file From e6b541c59d9f5b6943148d07fd129039344fea1b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 20 Dec 2023 00:23:57 -0500 Subject: [PATCH 06/42] WIP - working on 2 solutions, a JS/link based one and a django form based one --- src/registrar/config/urls.py | 3 +- src/registrar/forms/admin.py | 12 +----- .../templates/admin/export_data.html | 18 --------- src/registrar/templates/admin/index.html | 9 ++++- src/registrar/templates/export_data.html | 6 ++- src/registrar/utility/csv_export.py | 23 ++++++++---- src/registrar/views/admin_views.py | 37 ++++++------------- 7 files changed, 42 insertions(+), 66 deletions(-) delete mode 100644 src/registrar/templates/admin/export_data.html diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index af44fc237..edfe7619e 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -53,9 +53,8 @@ urlpatterns = [ "admin/logout/", RedirectView.as_view(pattern_name="logout", permanent=False), ), - path("admin/", admin.site.urls), - # path('export_data/', export_data, name='admin_export_data'), path('export_data/', ExportData.as_view(), name='admin_export_data'), + path("admin/", admin.site.urls), path( "application//edit/", views.ApplicationWizard.as_view(), diff --git a/src/registrar/forms/admin.py b/src/registrar/forms/admin.py index 78d359743..f0015027d 100644 --- a/src/registrar/forms/admin.py +++ b/src/registrar/forms/admin.py @@ -1,13 +1,5 @@ from django import forms class DataExportForm(forms.Form): - # start_date = forms.DateField(label='Start date', widget=forms.DateInput(attrs={'type': 'date'})) - # end_date = forms.DateField(label='End date', widget=forms.DateInput(attrs={'type': 'date'})) - - security_email = forms.EmailField( - label="Security email (optional)", - required=False, - error_messages={ - "invalid": 'dsas', - }, - ) \ No newline at end of file + start_date = forms.DateField(label='Start date', widget=forms.DateInput(attrs={'type': 'date'})) + end_date = forms.DateField(label='End date', widget=forms.DateInput(attrs={'type': 'date'})) diff --git a/src/registrar/templates/admin/export_data.html b/src/registrar/templates/admin/export_data.html deleted file mode 100644 index 589284c1b..000000000 --- a/src/registrar/templates/admin/export_data.html +++ /dev/null @@ -1,18 +0,0 @@ - -
- {% csrf_token %} - - - {% for field in form %} -
- {{ field.label_tag }} - {{ field }} - {% if field.errors %} -
{{ field.errors|join:", " }}
- {% endif %} -
- {% endfor %} - - - -
\ No newline at end of file diff --git a/src/registrar/templates/admin/index.html b/src/registrar/templates/admin/index.html index 82c881a9e..f74da2891 100644 --- a/src/registrar/templates/admin/index.html +++ b/src/registrar/templates/admin/index.html @@ -6,13 +6,20 @@

Welcome to the Custom Admin Homepage!

- {% comment %} {% include "export_data.html" %} {% endcomment %} + {% comment %} + Inputs of type date suck for accessibility. + We'll need to replace those guys with a django form once we figure out how to hook one onto this page. + The challenge is in the path definition in urls. Itdoes NOT like admin/export_data/ + + {% include "export_data.html" %} + {% endcomment %} + {% comment %} TODO: add a aria label or something {% endcomment %} Export
diff --git a/src/registrar/templates/export_data.html b/src/registrar/templates/export_data.html index 69b00c744..df6111c0d 100644 --- a/src/registrar/templates/export_data.html +++ b/src/registrar/templates/export_data.html @@ -1,8 +1,8 @@ -{% load static field_helpers%}
{% csrf_token %} +

The context test 1: {{ test }}

{% for field in form %}
@@ -14,6 +14,8 @@
{% endfor %} +

The context test 2: {{ test }}

- + +
\ No newline at end of file diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 5ceb49cd2..563983e3c 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -5,6 +5,7 @@ from registrar.models.domain_information import DomainInformation from registrar.models.public_contact import PublicContact from django.db.models import Value from django.db.models.functions import Coalesce +from itertools import chain def export_domains_to_writer(writer, columns, sort_fields, filter_condition): @@ -13,14 +14,20 @@ def export_domains_to_writer(writer, columns, sort_fields, filter_condition): print(f"filter_condition {filter_condition}") + domainInfos = DomainInformation.objects.filter(**filter_condition).order_by(*sort_fields) + if 'domain__created_at__gt' in filter_condition: - domainInfos = DomainInformation.objects.filter(domain__state=Domain.State.DELETED).order_by("domain__deleted_at") + deleted_domainInfos = DomainInformation.objects.filter(domain__state=Domain.State.DELETED).order_by("domain__deleted_at") print(f"filtering by deleted {domainInfos}") + + # Combine the two querysets into a single iterable + all_domainInfos = list(chain(domainInfos, deleted_domainInfos)) else: - domainInfos = DomainInformation.objects.filter(**filter_condition).order_by(*sort_fields) + all_domainInfos = list(domainInfos) + - for domainInfo in domainInfos: + for domainInfo in all_domainInfos: security_contacts = domainInfo.domain.contacts.filter(contact_type=PublicContact.ContactTypeChoices.SECURITY) print(f"regular filtering {domainInfos}") # For linter @@ -153,14 +160,16 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): else: # Handle the case where start_date is missing or empty print('ON NO') + # TODO: use Nov 1 2023 start_date_formatted = None # Replace with appropriate handling if end_date: end_date_formatted = datetime.strptime(end_date, "%Y-%m-%d") - print(f'start_date_formatted {end_date_formatted}') + print(f'end_date_formatted {end_date_formatted}') else: # Handle the case where start_date is missing or empty print('ON NO') + # TODO: use now end_date_formatted = None # Replace with appropriate handling writer = csv.writer(csv_file) @@ -172,11 +181,11 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): "Organization name", "City", "State", - "Security contact email", + "Status", "Created at", + "Deleted at", "Expiration date", ] - # Coalesce is used to replace federal_type of None with ZZZZZ sort_fields = [ "created_at", "domain__name", @@ -186,7 +195,7 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): Domain.State.UNKNOWN, Domain.State.DELETED, ], - "domain__expiration_date__lt": end_date_formatted, + "domain__created_at__lt": end_date_formatted, "domain__created_at__gt": start_date_formatted, } export_domains_to_writer(writer, columns, sort_fields, filter_condition) diff --git a/src/registrar/views/admin_views.py b/src/registrar/views/admin_views.py index 6c6aa6616..83e49954f 100644 --- a/src/registrar/views/admin_views.py +++ b/src/registrar/views/admin_views.py @@ -21,17 +21,20 @@ import logging logger = logging.getLogger(__name__) -def export_data(self): - """CSV download""" - print('VIEW') - # Federal only - response = HttpResponse(content_type="text/csv") - response["Content-Disposition"] = 'attachment; filename="current-federal.csv"' - csv_export.export_data_growth_to_csv(response) - return response class ExportData(View): + template_name = "admin/index.html" + form_class = DataExportForm + + + def get_context_data(self, **kwargs): + print('VIE VIE VIE') + context = super().get_context_data(**kwargs) + context['form'] = self.form_class() + context['test'] = 'testing the context' + return context + def get(self, request, *args, **kwargs): # Get start_date and end_date from the request's GET parameters start_date = request.GET.get('start_date', '') @@ -52,23 +55,5 @@ class ExportData(View): # csv_export.export_data_growth_to_csv(response) return response - - -# class ExportData(TemplateView): -# """Django form""" - -# template_name = "export_data.html" -# form_class = DataExportForm - -# def form_valid(self, form): -# print('Form is valid') -# # Form is valid, perform data export logic here -# return JsonResponse({'message': 'Data exported successfully!'}, content_type='application/json') -# def form_invalid(self, form): -# print('Form is invalid') -# # Form is invalid, return error response -# return JsonResponse({'error': 'Invalid form data'}, status=400, content_type='application/json') - - \ No newline at end of file From 35fda9d124dd3b43e8991684419aa1e143519b1b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 20 Dec 2023 00:26:03 -0500 Subject: [PATCH 07/42] Review for ticket #999 --- src/registrar/forms/__init__.py | 1 - src/registrar/forms/admin.py | 5 ----- src/registrar/templates/admin/index.html | 2 -- src/registrar/templates/export_data.html | 21 --------------------- src/registrar/views/admin_views.py | 5 ----- 5 files changed, 34 deletions(-) delete mode 100644 src/registrar/forms/admin.py delete mode 100644 src/registrar/templates/export_data.html diff --git a/src/registrar/forms/__init__.py b/src/registrar/forms/__init__.py index a1c34e777..914db375c 100644 --- a/src/registrar/forms/__init__.py +++ b/src/registrar/forms/__init__.py @@ -10,4 +10,3 @@ from .domain import ( DomainDsdataFormset, DomainDsdataForm, ) -from .admin import DataExportForm diff --git a/src/registrar/forms/admin.py b/src/registrar/forms/admin.py deleted file mode 100644 index f0015027d..000000000 --- a/src/registrar/forms/admin.py +++ /dev/null @@ -1,5 +0,0 @@ -from django import forms - -class DataExportForm(forms.Form): - start_date = forms.DateField(label='Start date', widget=forms.DateInput(attrs={'type': 'date'})) - end_date = forms.DateField(label='End date', widget=forms.DateInput(attrs={'type': 'date'})) diff --git a/src/registrar/templates/admin/index.html b/src/registrar/templates/admin/index.html index f74da2891..9052ddb44 100644 --- a/src/registrar/templates/admin/index.html +++ b/src/registrar/templates/admin/index.html @@ -10,8 +10,6 @@ Inputs of type date suck for accessibility. We'll need to replace those guys with a django form once we figure out how to hook one onto this page. The challenge is in the path definition in urls. Itdoes NOT like admin/export_data/ - - {% include "export_data.html" %} {% endcomment %} diff --git a/src/registrar/templates/export_data.html b/src/registrar/templates/export_data.html deleted file mode 100644 index df6111c0d..000000000 --- a/src/registrar/templates/export_data.html +++ /dev/null @@ -1,21 +0,0 @@ - -
- {% csrf_token %} - -

The context test 1: {{ test }}

- - {% for field in form %} -
- {{ field.label_tag }} - {{ field }} - {% if field.errors %} -
{{ field.errors|join:", " }}
- {% endif %} -
- {% endfor %} - -

The context test 2: {{ test }}

- - - -
\ No newline at end of file diff --git a/src/registrar/views/admin_views.py b/src/registrar/views/admin_views.py index 83e49954f..4c7aa3340 100644 --- a/src/registrar/views/admin_views.py +++ b/src/registrar/views/admin_views.py @@ -6,7 +6,6 @@ from django.views.decorators.csrf import csrf_exempt from django.shortcuts import render from registrar.utility import csv_export -from ..forms import DataExportForm from django.views.generic import TemplateView from registrar.models import ( @@ -24,10 +23,6 @@ logger = logging.getLogger(__name__) class ExportData(View): - template_name = "admin/index.html" - form_class = DataExportForm - - def get_context_data(self, **kwargs): print('VIE VIE VIE') context = super().get_context_data(**kwargs) From aac5cd698c1433ce6ca866ca653aa50ea98a0c62 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 20 Dec 2023 11:18:51 -0500 Subject: [PATCH 08/42] Clean up debugging code and commented out experiments --- src/registrar/assets/js/get-gov-admin.js | 62 +++++++----------------- src/registrar/templates/admin/index.html | 5 +- src/registrar/utility/csv_export.py | 32 +++++------- src/registrar/views/admin_views.py | 31 ++---------- 4 files changed, 38 insertions(+), 92 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index dcdeeb106..f5bcd3b26 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -276,53 +276,27 @@ function enableRelatedWidgetButtons(changeLink, deleteLink, viewLink, elementPk, viewLink.setAttribute('title', viewLink.getAttribute('title-template').replace('selected item', elementText)); } -// function performDataLookup(e) { -// e.preventDefault(); // Prevent the default form submission +/** An IIFE for admin in DjangoAdmin to listen to clicks on the growth report export button, + * attach the seleted start and end dates to a url that'll trigger the view, and finally + * redirect to that url. +*/ +(function (){ -// console.log('Form submitted!'); + let exportGrowthReportButton = document.getElementById('exportLink'); + if (exportGrowthReportButton) { + exportGrowthReportButton.addEventListener('click', function() { + // Get the selected start and end dates + let startDate = document.getElementById('start').value; + let endDate = document.getElementById('end').value; + let exportUrl = document.getElementById('exportLink').dataset.exportUrl; -// var form = document.getElementById("exportDataForm"); -// var formData = new FormData(form); - -// // Perform an AJAX request to fetch data -// fetch('/admin/', { -// method: 'POST', -// body: formData, -// }) -// .then(response => { -// if (!response.ok) { -// console.log(response); -// console.log(`HTTP error! Status: ${response.status}`); -// throw new Error(`HTTP error! Status: ${response.status}`); -// } -// return response.json(); -// }) -// .then(data => { -// // Handle the data (update the result div, for example) -// document.getElementById("dataResult").innerText = JSON.stringify(data); -// }) -// .catch(error => console.error('Error:', error)); -// } - - (function (){ - - document.getElementById('exportLink').addEventListener('click', function(event) { - event.preventDefault(); // Prevent the default link behavior + // Build the URL with parameters + exportUrl += "?start_date=" + startDate + "&end_date=" + endDate; - // Get the selected start and end dates - var startDate = document.getElementById('start').value; - var endDate = document.getElementById('end').value; - - var exportUrl = document.getElementById('exportLink').dataset.exportUrl; + // Redirect to the export URL + window.location.href = exportUrl; + }); + } - // Build the URL with parameters - exportUrl += "?start_date=" + startDate + "&end_date=" + endDate; - - // Redirect to the export URL - window.location.href = exportUrl; - }); - - - // document.getElementById('exportDataForm').addEventListener('submit', performDataLookup); })(); \ No newline at end of file diff --git a/src/registrar/templates/admin/index.html b/src/registrar/templates/admin/index.html index 9052ddb44..495dbc4f9 100644 --- a/src/registrar/templates/admin/index.html +++ b/src/registrar/templates/admin/index.html @@ -10,6 +10,8 @@ Inputs of type date suck for accessibility. We'll need to replace those guys with a django form once we figure out how to hook one onto this page. The challenge is in the path definition in urls. Itdoes NOT like admin/export_data/ + + See the commit "Review for ticket #999" {% endcomment %} @@ -17,8 +19,7 @@ - {% comment %} TODO: add a aria label or something {% endcomment %} - Export + diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 563983e3c..50b67909b 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -1,4 +1,5 @@ import csv +import logging from datetime import datetime from registrar.models.domain import Domain from registrar.models.domain_information import DomainInformation @@ -7,20 +8,20 @@ from django.db.models import Value from django.db.models.functions import Coalesce from itertools import chain +logger = logging.getLogger(__name__) def export_domains_to_writer(writer, columns, sort_fields, filter_condition): # write columns headers to writer writer.writerow(columns) - - print(f"filter_condition {filter_condition}") + # Get the domainInfos domainInfos = DomainInformation.objects.filter(**filter_condition).order_by(*sort_fields) + # domain__created_at__gt is in filter_conditions. This means that we're querrying for the growth report and + # need to fetch the domainInfos for the deleted domains. This is an OR situation so we can' combine the filters + # in one query which would be an AND operation. if 'domain__created_at__gt' in filter_condition: - - deleted_domainInfos = DomainInformation.objects.filter(domain__state=Domain.State.DELETED).order_by("domain__deleted_at") - print(f"filtering by deleted {domainInfos}") - + deleted_domainInfos = DomainInformation.objects.filter(domain__state=Domain.State.DELETED).order_by("domain__deleted_at") # Combine the two querysets into a single iterable all_domainInfos = list(chain(domainInfos, deleted_domainInfos)) else: @@ -150,27 +151,20 @@ def export_data_federal_to_csv(csv_file): def export_data_growth_to_csv(csv_file, start_date, end_date): - print(f'start_date {start_date}') - print(f'end_date {end_date}') - - # Check if start_date is not empty before using strptime if start_date: start_date_formatted = datetime.strptime(start_date, "%Y-%m-%d") - print(f'start_date_formatted {start_date_formatted}') else: # Handle the case where start_date is missing or empty - print('ON NO') - # TODO: use Nov 1 2023 - start_date_formatted = None # Replace with appropriate handling + # Default to a date that's prior to our first deployment + logger.error(f"Error fetching the start date, will default to 12023/1/1") + start_date_formatted = datetime(2023, 11, 1) # Replace with appropriate handling if end_date: end_date_formatted = datetime.strptime(end_date, "%Y-%m-%d") - print(f'end_date_formatted {end_date_formatted}') else: - # Handle the case where start_date is missing or empty - print('ON NO') - # TODO: use now - end_date_formatted = None # Replace with appropriate handling + # Handle the case where end_date is missing or empty + logger.error(f"Error fetching the end date, will default to now()") + end_date_formatted = datetime.now() # Replace with appropriate handling writer = csv.writer(csv_file) # define columns to include in export diff --git a/src/registrar/views/admin_views.py b/src/registrar/views/admin_views.py index 4c7aa3340..22792a002 100644 --- a/src/registrar/views/admin_views.py +++ b/src/registrar/views/admin_views.py @@ -1,54 +1,31 @@ """Admin-related views.""" -from django.http import HttpResponse, JsonResponse +from django.http import HttpResponse from django.views import View from django.views.decorators.csrf import csrf_exempt from django.shortcuts import render from registrar.utility import csv_export -from django.views.generic import TemplateView -from registrar.models import ( - Domain, - DomainApplication, - DomainInvitation, - DomainInformation, - UserDomainRole, -) import logging - logger = logging.getLogger(__name__) class ExportData(View): - def get_context_data(self, **kwargs): - print('VIE VIE VIE') - context = super().get_context_data(**kwargs) - context['form'] = self.form_class() - context['test'] = 'testing the context' - return context - def get(self, request, *args, **kwargs): # Get start_date and end_date from the request's GET parameters + # #999: not needed if we switch to django forms start_date = request.GET.get('start_date', '') end_date = request.GET.get('end_date', '') - - print(start_date) - print(end_date) - # Do something with start_date and end_date, e.g., include in the CSV export logic - # # Federal only response = HttpResponse(content_type="text/csv") response["Content-Disposition"] = f'attachment; filename="growth-from-{start_date}-to-{end_date}.csv"' + # For #999: set export_data_growth_to_csv to return the resulting queryset, which we can then use + # in context to display this data in the template. csv_export.export_data_growth_to_csv(response, start_date, end_date) - - # response = HttpResponse(content_type="text/csv") - # response["Content-Disposition"] = 'attachment; filename="current-federal.csv"' - # csv_export.export_data_growth_to_csv(response) - return response \ No newline at end of file From 31031d054de507859d32cf620da770eb052dfeea Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 20 Dec 2023 15:20:53 -0500 Subject: [PATCH 09/42] Started on unit tests, started on fix for issue with deleted domains filters, broken, wip --- .../tests/data/fake_current_federal.csv | 2 +- .../tests/data/fake_current_full.csv | 2 +- src/registrar/tests/test_reports.py | 160 +++++++++++++++++- src/registrar/utility/csv_export.py | 34 ++-- 4 files changed, 175 insertions(+), 23 deletions(-) diff --git a/src/registrar/tests/data/fake_current_federal.csv b/src/registrar/tests/data/fake_current_federal.csv index 33f679e9e..df4fe871f 100644 --- a/src/registrar/tests/data/fake_current_federal.csv +++ b/src/registrar/tests/data/fake_current_federal.csv @@ -1,3 +1,3 @@ -Domain name,Domain type,Agency,Organization name,City,State,Security Contact Email +Domain name,Domain type,Agency,Organization name,City,State,Security contact email cdomain1.gov,Federal - Executive,World War I Centennial Commission,,,, ddomain3.gov,Federal,Armed Forces Retirement Home,,,, \ No newline at end of file diff --git a/src/registrar/tests/data/fake_current_full.csv b/src/registrar/tests/data/fake_current_full.csv index 43eefc271..9fef96c60 100644 --- a/src/registrar/tests/data/fake_current_full.csv +++ b/src/registrar/tests/data/fake_current_full.csv @@ -1,4 +1,4 @@ -Domain name,Domain type,Agency,Organization name,City,State,Security Contact Email +Domain name,Domain type,Agency,Organization name,City,State,Security contact email cdomain1.gov,Federal - Executive,World War I Centennial Commission,,,, ddomain3.gov,Federal,Armed Forces Retirement Home,,,, adomain2.gov,Interstate,,,,, \ No newline at end of file diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 112b2ba34..c13570c52 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -14,7 +14,8 @@ from django.conf import settings from botocore.exceptions import ClientError import boto3_mocking from registrar.utility.s3_bucket import S3ClientError, S3ClientErrorCodes # type: ignore - +from datetime import datetime, timedelta +from django.utils import timezone class CsvReportsTest(TestCase): """Tests to determine if we are uploading our reports correctly""" @@ -75,7 +76,7 @@ class CsvReportsTest(TestCase): mock_client = MagicMock() fake_open = mock_open() expected_file_content = [ - call("Domain name,Domain type,Agency,Organization name,City,State,Security Contact Email\r\n"), + call("Domain name,Domain type,Agency,Organization name,City,State,Security contact email\r\n"), call("cdomain1.gov,Federal - Executive,World War I Centennial Commission,,,, \r\n"), call("ddomain3.gov,Federal,Armed Forces Retirement Home,,,, \r\n"), ] @@ -94,7 +95,7 @@ class CsvReportsTest(TestCase): mock_client = MagicMock() fake_open = mock_open() expected_file_content = [ - call("Domain name,Domain type,Agency,Organization name,City,State,Security Contact Email\r\n"), + call("Domain name,Domain type,Agency,Organization name,City,State,Security contact email\r\n"), call("cdomain1.gov,Federal - Executive,World War I Centennial Commission,,,, \r\n"), call("ddomain3.gov,Federal,Armed Forces Retirement Home,,,, \r\n"), call("adomain2.gov,Interstate,,,,, \r\n"), @@ -175,7 +176,7 @@ class CsvReportsTest(TestCase): # Check that the response contains what we expect expected_file_content = ( - "Domain name,Domain type,Agency,Organization name,City,State,Security Contact Email\n" + "Domain name,Domain type,Agency,Organization name,City,State,Security contact email\n" "cdomain1.gov,Federal - Executive,World War I Centennial Commission,,,,\n" "ddomain3.gov,Federal,Armed Forces Retirement Home,,,," ).encode() @@ -207,7 +208,7 @@ class CsvReportsTest(TestCase): # Check that the response contains what we expect expected_file_content = ( - "Domain name,Domain type,Agency,Organization name,City,State,Security Contact Email\n" + "Domain name,Domain type,Agency,Organization name,City,State,Security contact email\n" "cdomain1.gov,Federal - Executive,World War I Centennial Commission,,,,\n" "ddomain3.gov,Federal,Armed Forces Retirement Home,,,,\n" "adomain2.gov,Interstate,,,,," @@ -231,6 +232,8 @@ class ExportDataTest(TestCase): self.domain_3, _ = Domain.objects.get_or_create(name="ddomain3.gov", state=Domain.State.ON_HOLD) self.domain_4, _ = Domain.objects.get_or_create(name="bdomain4.gov", state=Domain.State.UNKNOWN) self.domain_4, _ = Domain.objects.get_or_create(name="bdomain4.gov", state=Domain.State.UNKNOWN) + self.domain_5, _ = Domain.objects.get_or_create(name="bdomain5.gov", state=Domain.State.DELETED, deleted_at=datetime(2023, 11, 1)) + self.domain_6, _ = Domain.objects.get_or_create(name="bdomain6.gov", state=Domain.State.DELETED, deleted_at=datetime(1980, 10, 16)) self.domain_information_1, _ = DomainInformation.objects.get_or_create( creator=self.user, @@ -256,6 +259,18 @@ class ExportDataTest(TestCase): organization_type="federal", federal_agency="Armed Forces Retirement Home", ) + self.domain_information_5, _ = DomainInformation.objects.get_or_create( + creator=self.user, + domain=self.domain_5, + organization_type="federal", + federal_agency="Armed Forces Retirement Home", + ) + self.domain_information_6, _ = DomainInformation.objects.get_or_create( + creator=self.user, + domain=self.domain_6, + organization_type="federal", + federal_agency="Armed Forces Retirement Home", + ) def tearDown(self): Domain.objects.all().delete() @@ -285,7 +300,7 @@ class ExportDataTest(TestCase): "Submitter title", "Submitter email", "Submitter phone", - "Security Contact Email", + "Security contact email", "Status", ] sort_fields = ["domain__name"] @@ -311,7 +326,7 @@ class ExportDataTest(TestCase): expected_content = ( "Domain name,Domain type,Agency,Organization name,City,State,AO," "AO email,Submitter,Submitter title,Submitter email,Submitter phone," - "Security Contact Email,Status\n" + "Security contact email,Status\n" "adomain2.gov,Interstate,dnsneeded\n" "cdomain1.gov,Federal - Executive,World War I Centennial Commission,ready\n" "ddomain3.gov,Federal,Armed Forces Retirement Home,onhold\n" @@ -338,7 +353,7 @@ class ExportDataTest(TestCase): "Organization name", "City", "State", - "Security Contact Email", + "Security contact email", ] sort_fields = ["domain__name", "federal_agency", "organization_type"] filter_condition = { @@ -364,7 +379,7 @@ class ExportDataTest(TestCase): # sorted alphabetially by domain name expected_content = ( "Domain name,Domain type,Agency,Organization name,City," - "State,Security Contact Email\n" + "State,Security contact email\n" "cdomain1.gov,Federal - Executive,World War I Centennial Commission\n" "ddomain3.gov,Federal,Armed Forces Retirement Home\n" ) @@ -375,3 +390,130 @@ class ExportDataTest(TestCase): expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() self.assertEqual(csv_content, expected_content) + + def test_export_domains_to_writer_with_date_filter_pulls_domains_in_range(self): + """Test that domains that are READY and in range are pulled when the growth report conditions + are applied to export_domains_to_writer.""" + # Create a CSV file in memory + csv_file = StringIO() + writer = csv.writer(csv_file) + + # Define columns, sort fields, and filter condition + columns = [ + "Domain name", + "Domain type", + "Agency", + "Organization name", + "City", + "State", + "Status", + "Deleted at", + "Expiration date", + ] + sort_fields = ["created_at","domain__name",] + filter_condition = { + "domain__state__in": [ + Domain.State.READY, + ], + "domain__created_at__lt": timezone.make_aware(datetime.now() + timedelta(days=1)), + "domain__created_at__gt": timezone.make_aware(datetime.now() - timedelta(days=1)), + } + filter_conditions_for_additional_domains = { + "domain__state__in": [ + Domain.State.DELETED, + ], + "domain__deleted_at__lt": timezone.make_aware(datetime.now() + timedelta(days=1)), + "domain__deleted_at__gt": timezone.make_aware(datetime.now() - timedelta(days=1)), + } + + # Call the export function + export_domains_to_writer(writer, columns, sort_fields, filter_condition) + + # Reset the CSV file's position to the beginning + csv_file.seek(0) + + # Read the content into a variable + csv_content = csv_file.read() + + print(f'csv_content {csv_content}') + + # We expect READY domains, + # federal only + # sorted alphabetially by domain name + expected_content = ( + "Domain name,Domain type,Agency,Organization name,City," + "State,Status,Deleted at,Expiration date\n" + "cdomain1.gov,Federal-Executive,World War I Centennial Commission,ready,\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) + + def test_export_domains_to_writer_with_date_filter_pulls_appropriate_deleted_domains(self): + """When domain__created_at__gt is in filters, we know it's a growth report + and we need to fetch the domainInfos for the deleted domains that are within + the date range. However, deleted domains that were deleted at a date outside + the range do not get pulled.""" + # Create a CSV file in memory + csv_file = StringIO() + writer = csv.writer(csv_file) + + # Define columns, sort fields, and filter condition + columns = [ + "Domain name", + "Domain type", + "Agency", + "Organization name", + "City", + "State", + "Status", + "Deleted at", + "Expiration date", + ] + sort_fields = ["created_at","domain__name",] + filter_condition = { + "domain__state__in": [ + Domain.State.READY, + ], + "domain__created_at__lt": timezone.make_aware(datetime(2023, 10, 1)), + "domain__created_at__gt": timezone.make_aware(datetime(2023, 12, 1)), + } + filter_conditions_for_additional_domains = { + "domain__state__in": [ + Domain.State.DELETED, + ], + "domain__deleted_at__lt": timezone.make_aware(datetime(2023, 10, 1)), + "domain__deleted_at__gt": timezone.make_aware(datetime(2023, 12, 1)), + } + + # Call the export function + export_domains_to_writer(writer, columns, sort_fields, filter_condition, filter_conditions_for_additional_domains) + + # Reset the CSV file's position to the beginning + csv_file.seek(0) + + # Read the content into a variable + csv_content = csv_file.read() + + print(f'csv_content {csv_content}') + + # We expect READY domains, + # federal only + # sorted alphabetially by domain name + expected_content = ( + "Domain name,Domain type,Agency,Organization name,City," + "State,Status,Deleted at,Expiration date\n" + "bdomain5.gov,Federal,Armed Forces Retirement Home,deleted,2023-11-01,\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) + diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 50b67909b..d5b133daf 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -7,20 +7,25 @@ from registrar.models.public_contact import PublicContact from django.db.models import Value from django.db.models.functions import Coalesce from itertools import chain +from django.utils import timezone logger = logging.getLogger(__name__) -def export_domains_to_writer(writer, columns, sort_fields, filter_condition): +def export_domains_to_writer(writer, columns, sort_fields, filter_condition, filter_condition_for_additional_domains=None): # write columns headers to writer writer.writerow(columns) + + logger.info('export_domains_to_writer') + logger.info(filter_condition) + logger.info(filter_condition_for_additional_domains) # Get the domainInfos domainInfos = DomainInformation.objects.filter(**filter_condition).order_by(*sort_fields) - # domain__created_at__gt is in filter_conditions. This means that we're querrying for the growth report and - # need to fetch the domainInfos for the deleted domains. This is an OR situation so we can' combine the filters - # in one query which would be an AND operation. - if 'domain__created_at__gt' in filter_condition: + # Condition is true for export_data_growth_to_csv. This is an OR situation so we can' combine the filters + # in one query. + if filter_condition_for_additional_domains is not None and 'domain__deleted_at__lt' in filter_condition_for_additional_domains: + logger.info("Fetching deleted domains") deleted_domainInfos = DomainInformation.objects.filter(domain__state=Domain.State.DELETED).order_by("domain__deleted_at") # Combine the two querysets into a single iterable all_domainInfos = list(chain(domainInfos, deleted_domainInfos)) @@ -30,7 +35,6 @@ def export_domains_to_writer(writer, columns, sort_fields, filter_condition): for domainInfo in all_domainInfos: security_contacts = domainInfo.domain.contacts.filter(contact_type=PublicContact.ContactTypeChoices.SECURITY) - print(f"regular filtering {domainInfos}") # For linter ao = " " if domainInfo.authorizing_official: @@ -152,19 +156,19 @@ def export_data_federal_to_csv(csv_file): def export_data_growth_to_csv(csv_file, start_date, end_date): if start_date: - start_date_formatted = datetime.strptime(start_date, "%Y-%m-%d") + start_date_formatted = timezone.make_aware(datetime.strptime(start_date, "%Y-%m-%d")) else: # Handle the case where start_date is missing or empty # Default to a date that's prior to our first deployment logger.error(f"Error fetching the start date, will default to 12023/1/1") - start_date_formatted = datetime(2023, 11, 1) # Replace with appropriate handling + start_date_formatted = timezone.make_aware(datetime(2023, 11, 1)) # Replace with appropriate handling if end_date: - end_date_formatted = datetime.strptime(end_date, "%Y-%m-%d") + end_date_formatted = timezone.make_aware(datetime.strptime(end_date, "%Y-%m-%d")) else: # Handle the case where end_date is missing or empty logger.error(f"Error fetching the end date, will default to now()") - end_date_formatted = datetime.now() # Replace with appropriate handling + end_date_formatted = timezone.make_aware(datetime.now()) # Replace with appropriate handling writer = csv.writer(csv_file) # define columns to include in export @@ -186,10 +190,16 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): ] filter_condition = { "domain__state__in": [ - Domain.State.UNKNOWN, + Domain.State.READY, + ], + "domain__created_at__lt": end_date_formatted, + "domain__created_at__gt": start_date_formatted, + } + filter_condition_for_additional_domains = { + "domain__state__in": [ Domain.State.DELETED, ], "domain__created_at__lt": end_date_formatted, "domain__created_at__gt": start_date_formatted, } - export_domains_to_writer(writer, columns, sort_fields, filter_condition) + export_domains_to_writer(writer, columns, sort_fields, filter_condition, filter_condition_for_additional_domains) From 515fa2c105b992f875b0fe8cf3670f3739309476 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 20 Dec 2023 15:01:54 -0700 Subject: [PATCH 10/42] unit test progress --- src/registrar/tests/test_views.py | 118 ++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 57fa03f52..5f071692d 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -729,6 +729,124 @@ class DomainApplicationTests(TestWithUser, WebTest): actual_url_slug = no_contacts_page.request.path.split("/")[-2] self.assertEqual(expected_url_slug, actual_url_slug) + def test_application_delete_other_contact(self): + """Other contacts can be deleted after being saved to database.""" + # Populate the databse with a domain application that + # has 1 "other contact" assigned to it + ao, _ = Contact.objects.get_or_create( + first_name="Testy", + last_name="Tester", + title="Chief Tester", + email="testy@town.com", + phone="(555) 555 5555", + ) + domain, _ = DraftDomain.objects.get_or_create(name="fakeSite.gov") + you, _ = Contact.objects.get_or_create( + first_name="Testy you", + last_name="Tester you", + title="Admin Tester", + email="testy-admin@town.com", + phone="(555) 555 5556", + ) + other, _ = Contact.objects.get_or_create( + first_name="Testy2", + last_name="Tester2", + title="Another Tester", + email="testy2@town.com", + phone="(555) 555 5557", + ) + application, _ = DomainApplication.objects.get_or_create( + organization_type="federal", + federal_type="executive", + purpose="Purpose of the site", + anything_else="No", + is_policy_acknowledged=True, + organization_name="Testorg", + address_line1="address 1", + state_territory="NY", + zipcode="10002", + authorizing_official=ao, + requested_domain=domain, + submitter=you, + creator=self.user, + ) + application.other_contacts.add(other) + + # prime the form by visiting /edit + url = reverse("edit-application", kwargs={"id": application.pk}) + response = self.client.get(url) + + url = reverse("application:other_contacts") + other_contacts_page = self.client.get(url, follow=True) + + # ====== METHOD 2 -- prime form + # other_contacts_page = self.app.get(reverse("application:other_contacts")) + # session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + # # Fill in the other contact form + # other_contacts_form = other_contacts_page.forms[0] + # other_contacts_form["other_contacts-0-first_name"] = "Testy2" + # other_contacts_form["other_contacts-0-last_name"] = "Tester2" + # other_contacts_form["other_contacts-0-title"] = "Another Tester" + # other_contacts_form["other_contacts-0-email"] = "testy2@town.com" + # other_contacts_form["other_contacts-0-phone"] = "(201) 555 5557" + + # # for f in other_contacts_form.fields: + # # if not "submit" in f: + # # print(f) + # # print(other_contacts_form[f].value) + + # # Submit the form + # other_contacts_result = other_contacts_form.submit() + # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + # # validate that data from this step are being saved + # application = DomainApplication.objects.get() # there's only one + # self.assertEqual( + # application.other_contacts.count(), + # 1, + # ) + # # Verify user is taken to "anything else" page + # self.assertEqual(other_contacts_result.status_code, 302) + # self.assertEqual(other_contacts_result["Location"], "/register/anything_else/") + + # # Go back to the previous step + # other_contacts_page = self.app.get(reverse("application:other_contacts")) + + # clear the form + other_contacts_form = other_contacts_page.forms[0] + other_contacts_form["other_contacts-0-first_name"] = "" + other_contacts_form["other_contacts-0-middle_name"] = "" + other_contacts_form["other_contacts-0-last_name"] = "" + other_contacts_form["other_contacts-0-title"] = "" + other_contacts_form["other_contacts-0-email"] = "" + other_contacts_form["other_contacts-0-phone"] = "" + + for f in other_contacts_form.fields: + if not "submit" in f: + print(f) + print(other_contacts_form[f].value) + + # Submit the now empty form + result = other_contacts_form.submit() + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + # Verify that the contact we saved earlier has been removed from the database + application = DomainApplication.objects.get() # There are no contacts anymore + self.assertEqual( + application.other_contacts.count(), + 0, + ) + + # Verify that on submit, user is advanced to "no contacts" page + no_contacts_page = result.follow() + expected_url_slug = str(Step.NO_OTHER_CONTACTS) + actual_url_slug = no_contacts_page.request.path.split("/")[-2] + self.assertEqual(expected_url_slug, actual_url_slug) + + + def test_application_about_your_organiztion_interstate(self): """Special districts have to answer an additional question.""" type_page = self.app.get(reverse("application:")).follow() From cb16f5eb96a23667c23d3c1eff20b58896aeaccb Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 20 Dec 2023 21:54:13 -0500 Subject: [PATCH 11/42] Code cleanup, layout cleanup, unit tests --- src/registrar/assets/js/get-gov-admin.js | 9 +- src/registrar/templates/admin/index.html | 16 ++- src/registrar/tests/test_admin_views.py | 52 ++++++++ src/registrar/tests/test_reports.py | 154 +++++++++++------------ src/registrar/utility/csv_export.py | 149 +++++++++++++--------- 5 files changed, 229 insertions(+), 151 deletions(-) create mode 100644 src/registrar/tests/test_admin_views.py diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index f5bcd3b26..ce0725a63 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -282,13 +282,20 @@ function enableRelatedWidgetButtons(changeLink, deleteLink, viewLink, elementPk, */ (function (){ + // Get the current date in the format YYYY-MM-DD + var currentDate = new Date().toISOString().split('T')[0]; + + // Default the value of the end date input field to the current date + let endDateInput =document.getElementById('end'); + endDateInput.value = currentDate; + let exportGrowthReportButton = document.getElementById('exportLink'); if (exportGrowthReportButton) { exportGrowthReportButton.addEventListener('click', function() { // Get the selected start and end dates let startDate = document.getElementById('start').value; - let endDate = document.getElementById('end').value; + let endDate = endDateInput.value; let exportUrl = document.getElementById('exportLink').dataset.exportUrl; // Build the URL with parameters diff --git a/src/registrar/templates/admin/index.html b/src/registrar/templates/admin/index.html index 495dbc4f9..a98a09696 100644 --- a/src/registrar/templates/admin/index.html +++ b/src/registrar/templates/admin/index.html @@ -14,12 +14,18 @@ See the commit "Review for ticket #999" {% endcomment %} - - - - +
+
+ + +
+
+ + +
- + +
diff --git a/src/registrar/tests/test_admin_views.py b/src/registrar/tests/test_admin_views.py new file mode 100644 index 000000000..6336ed139 --- /dev/null +++ b/src/registrar/tests/test_admin_views.py @@ -0,0 +1,52 @@ +from django.test import TestCase, Client +from django.urls import reverse +from registrar.tests.common import create_superuser +from registrar.views.admin_views import ExportData + + +class TestViews(TestCase): + def setUp(self): + self.client = Client(HTTP_HOST="localhost:8080") + self.superuser = create_superuser() + + def test_export_data_view(self): + + self.client.force_login(self.superuser) + + # Reverse the URL for the admin index page + admin_index_url = reverse("admin:index") + + # Make a GET request to the admin index page + response = self.client.get(admin_index_url) + + print(f'response1 {response}') + + # Assert that the response status code is 200 (OK) + self.assertEqual(response.status_code, 200) + + # Ensure that the start_date and end_date are set + start_date = "2023-01-01" + end_date = "2023-12-31" + + # Construct the URL for the export data view with start_date and end_date parameters: + # This stuff is currently done in JS + export_data_url = reverse("admin_export_data") + f"?start_date={start_date}&end_date={end_date}" + + # Make a GET request to the export data page + response = self.client.get(export_data_url) + + print(response) + + # Assert that the response status code is 200 (OK) or the expected status code + self.assertEqual(response.status_code, 200) + + # Assert that the content type is CSV + self.assertEqual(response["Content-Type"], "text/csv") + + # Check if the filename in the Content-Disposition header matches the expected pattern + expected_filename = f'growth-from-{start_date}-to-{end_date}.csv' + self.assertIn(f'attachment; filename="{expected_filename}"', response["Content-Disposition"]) + + + + \ No newline at end of file diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index c13570c52..2264adf28 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -6,7 +6,7 @@ from registrar.models.domain_information import DomainInformation from registrar.models.domain import Domain from registrar.models.user import User from django.contrib.auth import get_user_model -from registrar.utility.csv_export import export_domains_to_writer +from registrar.utility.csv_export import export_domains_to_writer, get_default_start_date, get_default_end_date, export_data_growth_to_csv 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 @@ -14,7 +14,7 @@ from django.conf import settings from botocore.exceptions import ClientError import boto3_mocking from registrar.utility.s3_bucket import S3ClientError, S3ClientErrorCodes # type: ignore -from datetime import datetime, timedelta +from datetime import date, datetime, timedelta from django.utils import timezone class CsvReportsTest(TestCase): @@ -232,9 +232,13 @@ class ExportDataTest(TestCase): self.domain_3, _ = Domain.objects.get_or_create(name="ddomain3.gov", state=Domain.State.ON_HOLD) self.domain_4, _ = Domain.objects.get_or_create(name="bdomain4.gov", state=Domain.State.UNKNOWN) self.domain_4, _ = Domain.objects.get_or_create(name="bdomain4.gov", state=Domain.State.UNKNOWN) - self.domain_5, _ = Domain.objects.get_or_create(name="bdomain5.gov", state=Domain.State.DELETED, deleted_at=datetime(2023, 11, 1)) - self.domain_6, _ = Domain.objects.get_or_create(name="bdomain6.gov", state=Domain.State.DELETED, deleted_at=datetime(1980, 10, 16)) - + self.domain_5, _ = Domain.objects.get_or_create(name="bdomain5.gov", state=Domain.State.DELETED, deleted_at=timezone.make_aware(datetime(2023, 11, 1))) + self.domain_6, _ = Domain.objects.get_or_create(name="bdomain6.gov", state=Domain.State.DELETED, deleted_at=timezone.make_aware(datetime(1980, 10, 16))) + self.domain_7, _ = Domain.objects.get_or_create(name="xdomain7.gov", state=Domain.State.DELETED, deleted_at=timezone.now()) + self.domain_8, _ = Domain.objects.get_or_create(name="sdomain8.gov", state=Domain.State.DELETED, deleted_at=timezone.now()) + # We use timezone.make_aware to sync to server time a datetime object with the current date (using date.today()) and a specific time (using datetime.min.time()). + self.domain_9, _ = Domain.objects.get_or_create(name="zdomain9.gov", state=Domain.State.DELETED, deleted_at=timezone.make_aware(datetime.combine(date.today() - timedelta(days=1), datetime.min.time()))) + self.domain_information_1, _ = DomainInformation.objects.get_or_create( creator=self.user, domain=self.domain_1, @@ -271,6 +275,24 @@ class ExportDataTest(TestCase): organization_type="federal", federal_agency="Armed Forces Retirement Home", ) + self.domain_information_7, _ = DomainInformation.objects.get_or_create( + creator=self.user, + domain=self.domain_7, + organization_type="federal", + federal_agency="Armed Forces Retirement Home", + ) + self.domain_information_8, _ = DomainInformation.objects.get_or_create( + creator=self.user, + domain=self.domain_8, + organization_type="federal", + federal_agency="Armed Forces Retirement Home", + ) + self.domain_information_9, _ = DomainInformation.objects.get_or_create( + creator=self.user, + domain=self.domain_9, + organization_type="federal", + federal_agency="Armed Forces Retirement Home", + ) def tearDown(self): Domain.objects.all().delete() @@ -392,11 +414,23 @@ class ExportDataTest(TestCase): self.assertEqual(csv_content, expected_content) def test_export_domains_to_writer_with_date_filter_pulls_domains_in_range(self): - """Test that domains that are READY and in range are pulled when the growth report conditions - are applied to export_domains_to_writer.""" + """Test that domains that are + 1. READY and their created_at dates are in range + 2. DELETED and their deleted_at dates are in range + are pulled when the growth report conditions are applied to export_domains_to_writed. + Test that ready domains display first and deleted second, sorted according to + specified keys. + + We considered testing export_data_growth_to_csv which calls export_domains_to_writer + and would have been easy to set up, but expected_content would contain created_at dates + which are hard to mock.""" + # Create a CSV file in memory csv_file = StringIO() writer = csv.writer(csv_file) + # We use timezone.make_aware to sync to server time a datetime object with the current date (using date.today()) and a specific time (using datetime.min.time()). + end_date = timezone.make_aware(datetime.combine(date.today() + timedelta(days=2), datetime.min.time())) + start_date = timezone.make_aware(datetime.combine(date.today() - timedelta(days=2), datetime.min.time())) # Define columns, sort fields, and filter condition columns = [ @@ -407,43 +441,46 @@ class ExportDataTest(TestCase): "City", "State", "Status", - "Deleted at", "Expiration date", ] sort_fields = ["created_at","domain__name",] + sort_fields_for_additional_domains = [ + "domain__deleted_at", + "domain__name", + ] filter_condition = { "domain__state__in": [ Domain.State.READY, ], - "domain__created_at__lt": timezone.make_aware(datetime.now() + timedelta(days=1)), - "domain__created_at__gt": timezone.make_aware(datetime.now() - timedelta(days=1)), + "domain__created_at__lt": end_date, + "domain__created_at__gt": start_date, } filter_conditions_for_additional_domains = { "domain__state__in": [ Domain.State.DELETED, ], - "domain__deleted_at__lt": timezone.make_aware(datetime.now() + timedelta(days=1)), - "domain__deleted_at__gt": timezone.make_aware(datetime.now() - timedelta(days=1)), + "domain__deleted_at__lt": end_date, + "domain__deleted_at__gt": start_date, } # Call the export function - export_domains_to_writer(writer, columns, sort_fields, filter_condition) + export_domains_to_writer(writer, columns, sort_fields, filter_condition, sort_fields_for_additional_domains, filter_conditions_for_additional_domains) # Reset the CSV file's position to the beginning csv_file.seek(0) # Read the content into a variable csv_content = csv_file.read() - - print(f'csv_content {csv_content}') - - # We expect READY domains, - # federal only - # sorted alphabetially by domain name + + # We expect READY domains first, created between today-2 and today+2, sorted by created_at then name + # and DELETED domains deleted between today-2 and today+2, sorted by deleted_at then name expected_content = ( "Domain name,Domain type,Agency,Organization name,City," - "State,Status,Deleted at,Expiration date\n" - "cdomain1.gov,Federal-Executive,World War I Centennial Commission,ready,\n" + "State,Status,Expiration date\n" + "cdomain1.gov,Federal-Executive,World War I Centennial Commission,ready\n" + "zdomain9.gov,Federal,Armed Forces Retirement Home,,,,deleted,\n" + "sdomain8.gov,Federal,Armed Forces Retirement Home,,,,deleted,\n" + "xdomain7.gov,Federal,Armed Forces Retirement Home,,,,deleted,\n" ) # Normalize line endings and remove commas, @@ -453,67 +490,16 @@ class ExportDataTest(TestCase): self.assertEqual(csv_content, expected_content) - def test_export_domains_to_writer_with_date_filter_pulls_appropriate_deleted_domains(self): - """When domain__created_at__gt is in filters, we know it's a growth report - and we need to fetch the domainInfos for the deleted domains that are within - the date range. However, deleted domains that were deleted at a date outside - the range do not get pulled.""" - # Create a CSV file in memory - csv_file = StringIO() - writer = csv.writer(csv_file) - - # Define columns, sort fields, and filter condition - columns = [ - "Domain name", - "Domain type", - "Agency", - "Organization name", - "City", - "State", - "Status", - "Deleted at", - "Expiration date", - ] - sort_fields = ["created_at","domain__name",] - filter_condition = { - "domain__state__in": [ - Domain.State.READY, - ], - "domain__created_at__lt": timezone.make_aware(datetime(2023, 10, 1)), - "domain__created_at__gt": timezone.make_aware(datetime(2023, 12, 1)), - } - filter_conditions_for_additional_domains = { - "domain__state__in": [ - Domain.State.DELETED, - ], - "domain__deleted_at__lt": timezone.make_aware(datetime(2023, 10, 1)), - "domain__deleted_at__gt": timezone.make_aware(datetime(2023, 12, 1)), - } - - # Call the export function - export_domains_to_writer(writer, columns, sort_fields, filter_condition, filter_conditions_for_additional_domains) - - # Reset the CSV file's position to the beginning - csv_file.seek(0) - - # Read the content into a variable - csv_content = csv_file.read() - - print(f'csv_content {csv_content}') - - # We expect READY domains, - # federal only - # sorted alphabetially by domain name - expected_content = ( - "Domain name,Domain type,Agency,Organization name,City," - "State,Status,Deleted at,Expiration date\n" - "bdomain5.gov,Federal,Armed Forces Retirement Home,deleted,2023-11-01,\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.""" + + def test_get_default_start_date(self): + expected_date = timezone.make_aware(datetime(2023, 11, 1)) + actual_date = get_default_start_date() + self.assertEqual(actual_date, expected_date) + def test_get_default_end_date(self): + # Note: You may need to mock timezone.now() for accurate testing + expected_date = timezone.now() + actual_date = get_default_end_date() + self.assertEqual(actual_date.date(), expected_date.date()) \ No newline at end of file diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index d5b133daf..43c532d73 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -1,6 +1,6 @@ import csv import logging -from datetime import datetime +from datetime import date, datetime from registrar.models.domain import Domain from registrar.models.domain_information import DomainInformation from registrar.models.public_contact import PublicContact @@ -11,58 +11,66 @@ from django.utils import timezone logger = logging.getLogger(__name__) -def export_domains_to_writer(writer, columns, sort_fields, filter_condition, filter_condition_for_additional_domains=None): +def get_domain_infos(filter_condition, sort_fields): + domain_infos = DomainInformation.objects.filter(**filter_condition).order_by(*sort_fields) + return domain_infos + +def write_row(writer, columns, domain_info): + security_contacts = domain_info.domain.contacts.filter(contact_type=PublicContact.ContactTypeChoices.SECURITY) + # For linter + ao = " " + if domain_info.authorizing_official: + first_name = domain_info.authorizing_official.first_name or "" + last_name = domain_info.authorizing_official.last_name or "" + ao = first_name + " " + last_name + # create a dictionary of fields which can be included in output + FIELDS = { + "Domain name": domain_info.domain.name, + "Domain type": domain_info.get_organization_type_display() + " - " + domain_info.get_federal_type_display() + if domain_info.federal_type + else domain_info.get_organization_type_display(), + "Agency": domain_info.federal_agency, + "Organization name": domain_info.organization_name, + "City": domain_info.city, + "State": domain_info.state_territory, + "AO": ao, + "AO email": domain_info.authorizing_official.email if domain_info.authorizing_official else " ", + "Security contact email": security_contacts[0].email if security_contacts else " ", + "Status": domain_info.domain.state, + "Expiration date": domain_info.domain.expiration_date, + "Created at": domain_info.domain.created_at, + "Deleted at": domain_info.domain.deleted_at, + } + writer.writerow([FIELDS.get(column, "") for column in columns]) + +def export_domains_to_writer(writer, columns, sort_fields, filter_condition, sort_fields_for_additional_domains=None, filter_condition_for_additional_domains=None): + """ + Receives params from the parent methods and outputs a CSV with fltered and sorted domains. + The 'additional' params enable us to concatenate 2 different filtered lists. + """ # write columns headers to writer writer.writerow(columns) - - logger.info('export_domains_to_writer') - logger.info(filter_condition) - logger.info(filter_condition_for_additional_domains) # Get the domainInfos - domainInfos = DomainInformation.objects.filter(**filter_condition).order_by(*sort_fields) + domainInfos = get_domain_infos(filter_condition, sort_fields) # Condition is true for export_data_growth_to_csv. This is an OR situation so we can' combine the filters # in one query. if filter_condition_for_additional_domains is not None and 'domain__deleted_at__lt' in filter_condition_for_additional_domains: - logger.info("Fetching deleted domains") - deleted_domainInfos = DomainInformation.objects.filter(domain__state=Domain.State.DELETED).order_by("domain__deleted_at") + # Get the deleted domain infos + deleted_domainInfos = get_domain_infos(filter_condition_for_additional_domains, sort_fields_for_additional_domains) # Combine the two querysets into a single iterable all_domainInfos = list(chain(domainInfos, deleted_domainInfos)) else: all_domainInfos = list(domainInfos) - - for domainInfo in all_domainInfos: - security_contacts = domainInfo.domain.contacts.filter(contact_type=PublicContact.ContactTypeChoices.SECURITY) - # For linter - ao = " " - if domainInfo.authorizing_official: - first_name = domainInfo.authorizing_official.first_name or "" - last_name = domainInfo.authorizing_official.last_name or "" - ao = first_name + " " + last_name - # create a dictionary of fields which can be included in output - FIELDS = { - "Domain name": domainInfo.domain.name, - "Domain type": domainInfo.get_organization_type_display() + " - " + domainInfo.get_federal_type_display() - if domainInfo.federal_type - else domainInfo.get_organization_type_display(), - "Agency": domainInfo.federal_agency, - "Organization name": domainInfo.organization_name, - "City": domainInfo.city, - "State": domainInfo.state_territory, - "AO": ao, - "AO email": domainInfo.authorizing_official.email if domainInfo.authorizing_official else " ", - "Security contact email": security_contacts[0].email if security_contacts else " ", - "Status": domainInfo.domain.state, - "Expiration date": domainInfo.domain.expiration_date, - "Created at": domainInfo.domain.created_at, - "Deleted at": domainInfo.domain.deleted_at, - } - writer.writerow([FIELDS.get(column, "") for column in columns]) - + # Write rows to CSV + for domain_info in all_domainInfos: + write_row(writer, columns, domain_info) def export_data_type_to_csv(csv_file): + """All domains report with extra columns""" + writer = csv.writer(csv_file) # define columns to include in export columns = [ @@ -94,8 +102,9 @@ def export_data_type_to_csv(csv_file): } export_domains_to_writer(writer, columns, sort_fields, filter_condition) - def export_data_full_to_csv(csv_file): + """All domains report""" + writer = csv.writer(csv_file) # define columns to include in export columns = [ @@ -125,6 +134,8 @@ def export_data_full_to_csv(csv_file): def export_data_federal_to_csv(csv_file): + """Federal domains report""" + writer = csv.writer(csv_file) # define columns to include in export columns = [ @@ -152,25 +163,38 @@ def export_data_federal_to_csv(csv_file): ], } export_domains_to_writer(writer, columns, sort_fields, filter_condition) - + +def get_default_start_date(): + # Default to a date that's prior to our first deployment + return timezone.make_aware(datetime(2023, 11, 1)) + +def get_default_end_date(): + # Default to now() + return timezone.now() + def export_data_growth_to_csv(csv_file, start_date, end_date): + """ + Growth report: + Receive start and end dates from the view, parse them. + Request from export_domains_to_writer READY domains that are created between + the start and end dates, as well as DELETED domains that are deleted between + the start and end dates. Specify sort params for both lists. + """ - if start_date: - start_date_formatted = timezone.make_aware(datetime.strptime(start_date, "%Y-%m-%d")) - else: - # Handle the case where start_date is missing or empty - # Default to a date that's prior to our first deployment - logger.error(f"Error fetching the start date, will default to 12023/1/1") - start_date_formatted = timezone.make_aware(datetime(2023, 11, 1)) # Replace with appropriate handling + start_date_formatted = ( + timezone.make_aware(datetime.strptime(start_date, "%Y-%m-%d")) + if start_date + else get_default_start_date() + ) + + end_date_formatted = ( + timezone.make_aware(datetime.strptime(end_date, "%Y-%m-%d")) + if end_date + else get_default_end_date() + ) - if end_date: - end_date_formatted = timezone.make_aware(datetime.strptime(end_date, "%Y-%m-%d")) - else: - # Handle the case where end_date is missing or empty - logger.error(f"Error fetching the end date, will default to now()") - end_date_formatted = timezone.make_aware(datetime.now()) # Replace with appropriate handling - writer = csv.writer(csv_file) + # define columns to include in export columns = [ "Domain name", @@ -189,17 +213,20 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): "domain__name", ] filter_condition = { - "domain__state__in": [ - Domain.State.READY, - ], + "domain__state__in": [Domain.State.READY], "domain__created_at__lt": end_date_formatted, "domain__created_at__gt": start_date_formatted, } + + # We also want domains deleted between sar and end dates, sorted + sort_fields_for_additional_domains = [ + "domain__deleted_at", + "domain__name", + ] filter_condition_for_additional_domains = { - "domain__state__in": [ - Domain.State.DELETED, - ], + "domain__state__in": [Domain.State.DELETED], "domain__created_at__lt": end_date_formatted, "domain__created_at__gt": start_date_formatted, } - export_domains_to_writer(writer, columns, sort_fields, filter_condition, filter_condition_for_additional_domains) + + export_domains_to_writer(writer, columns, sort_fields, filter_condition, sort_fields_for_additional_domains, filter_condition_for_additional_domains) From 4b38c4abc83cfed92fbfe5efd38871c05d9c01a1 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 21 Dec 2023 12:22:03 -0500 Subject: [PATCH 12/42] Add ready_at column and test date range against it for READY domains (growth report) --- src/registrar/config/settings.py | 1 - ...0057_domain_deleted_at_domain_ready_at.py} | 9 +++++- src/registrar/models/domain.py | 8 ++++- src/registrar/models/domain_application.py | 2 ++ src/registrar/tests/test_models_domain.py | 18 ++++++++++++ src/registrar/tests/test_reports.py | 29 ++++++++++++++----- src/registrar/utility/csv_export.py | 6 ++-- 7 files changed, 60 insertions(+), 13 deletions(-) rename src/registrar/migrations/{0057_domain_deleted_at.py => 0057_domain_deleted_at_domain_ready_at.py} (57%) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 317cb9375..bc46c60ba 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -210,7 +210,6 @@ STATICFILES_DIRS = [ TEMPLATES = [ { "BACKEND": "django.template.backends.django.DjangoTemplates", - # "DIRS": [BASE_DIR / "registrar" / "templates"], # look for templates inside installed apps # required by django-debug-toolbar "APP_DIRS": True, diff --git a/src/registrar/migrations/0057_domain_deleted_at.py b/src/registrar/migrations/0057_domain_deleted_at_domain_ready_at.py similarity index 57% rename from src/registrar/migrations/0057_domain_deleted_at.py rename to src/registrar/migrations/0057_domain_deleted_at_domain_ready_at.py index e93068945..400fddc3a 100644 --- a/src/registrar/migrations/0057_domain_deleted_at.py +++ b/src/registrar/migrations/0057_domain_deleted_at_domain_ready_at.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.7 on 2023-12-19 05:42 +# Generated by Django 4.2.7 on 2023-12-21 17:12 from django.db import migrations, models @@ -14,4 +14,11 @@ class Migration(migrations.Migration): name="deleted_at", field=models.DateField(editable=False, help_text="Deleted at date", null=True), ), + migrations.AddField( + model_name="domain", + name="ready_at", + field=models.DateField( + editable=False, help_text="The last time this domain moved into the READY state", null=True + ), + ), ] diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 25c60ca2a..3b347b7cd 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -966,6 +966,12 @@ class Domain(TimeStampedModel, DomainHelper): editable=False, help_text="Deleted at date", ) + + ready_at = DateField( + null=True, + editable=False, + help_text="The last time this domain moved into the READY state", + ) def isActive(self): return self.state == Domain.State.CREATED @@ -1287,7 +1293,6 @@ class Domain(TimeStampedModel, DomainHelper): logger.info("deletedInEpp()-> inside _delete_domain") self._delete_domain() self.deleted_at = timezone.now() - self.save() except RegistryError as err: logger.error(f"Could not delete domain. Registry returned error: {err}") raise err @@ -1331,6 +1336,7 @@ class Domain(TimeStampedModel, DomainHelper): """ logger.info("Changing to ready state") logger.info("able to transition to ready state") + self.ready_at = timezone.now() @transition( field="state", diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index 12eda4caf..1b32bce9b 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -711,6 +711,7 @@ class DomainApplication(TimeStampedModel): # Only reject if it exists on EPP if domain_state != Domain.State.UNKNOWN: self.approved_domain.deletedInEpp() + self.approved_domain.save() self.approved_domain.delete() self.approved_domain = None @@ -740,6 +741,7 @@ class DomainApplication(TimeStampedModel): # Only reject if it exists on EPP if domain_state != Domain.State.UNKNOWN: self.approved_domain.deletedInEpp() + self.approved_domain.save() self.approved_domain.delete() self.approved_domain = None diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 39f63c942..fcb527014 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -1112,6 +1112,7 @@ class TestRegistrantNameservers(MockEppLib): Then `commands.CreateHost` and `commands.UpdateDomain` is sent to the registry And `domain.is_active` returns False + And domain.ready_at is null """ # set 1 nameserver @@ -1137,6 +1138,8 @@ class TestRegistrantNameservers(MockEppLib): # check that status is still NOT READY # as you have less than 2 nameservers self.assertFalse(self.domain.is_active()) + + self.assertEqual(self.domain.ready_at, None) def test_user_adds_two_nameservers(self): """ @@ -1146,6 +1149,7 @@ class TestRegistrantNameservers(MockEppLib): Then `commands.CreateHost` and `commands.UpdateDomain` is sent to the registry And `domain.is_active` returns True + And domain.ready_at is not null """ # set 2 nameservers @@ -1176,6 +1180,7 @@ class TestRegistrantNameservers(MockEppLib): self.assertEqual(4, self.mockedSendFunction.call_count) # check that status is READY self.assertTrue(self.domain.is_active()) + self.assertNotEqual(self.domain.ready_at, None) def test_user_adds_too_many_nameservers(self): """ @@ -2248,11 +2253,14 @@ class TestAnalystDelete(MockEppLib): When `domain.deletedInEpp()` is called Then `commands.DeleteDomain` is sent to the registry And `state` is set to `DELETED` + + The deleted_at date is set. """ # Put the domain in client hold self.domain.place_client_hold() # Delete it... self.domain.deletedInEpp() + self.domain.save() self.mockedSendFunction.assert_has_calls( [ call( @@ -2267,6 +2275,9 @@ class TestAnalystDelete(MockEppLib): # Domain should have the right state self.assertEqual(self.domain.state, Domain.State.DELETED) + + # Domain should have a deleted_at + self.assertNotEqual(self.domain.deleted_at, None) # Cache should be invalidated self.assertEqual(self.domain._cache, {}) @@ -2286,6 +2297,7 @@ class TestAnalystDelete(MockEppLib): # Delete it with self.assertRaises(RegistryError) as err: domain.deletedInEpp() + domain.save() self.assertTrue(err.is_client_error() and err.code == ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) self.mockedSendFunction.assert_has_calls( [ @@ -2309,12 +2321,18 @@ class TestAnalystDelete(MockEppLib): and domain is of `state` is `READY` Then an FSM error is returned And `state` is not set to `DELETED` + + The deleted_at date is still null. """ self.assertEqual(self.domain.state, Domain.State.READY) with self.assertRaises(TransitionNotAllowed) as err: self.domain.deletedInEpp() + self.domain.save() self.assertTrue(err.is_client_error() and err.code == ErrorCode.OBJECT_STATUS_PROHIBITS_OPERATION) # Domain should not be deleted self.assertNotEqual(self.domain, None) # Domain should have the right state self.assertEqual(self.domain.state, Domain.State.READY) + + # deleted_at should be null + self.assertEqual(self.domain.deleted_at, None) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 2264adf28..a9f28b8c8 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -227,7 +227,7 @@ class ExportDataTest(TestCase): username=username, first_name=first_name, last_name=last_name, email=email ) - self.domain_1, _ = Domain.objects.get_or_create(name="cdomain1.gov", state=Domain.State.READY) + self.domain_1, _ = Domain.objects.get_or_create(name="cdomain1.gov", state=Domain.State.READY, ready_at=timezone.now()) self.domain_2, _ = Domain.objects.get_or_create(name="adomain2.gov", state=Domain.State.DNS_NEEDED) self.domain_3, _ = Domain.objects.get_or_create(name="ddomain3.gov", state=Domain.State.ON_HOLD) self.domain_4, _ = Domain.objects.get_or_create(name="bdomain4.gov", state=Domain.State.UNKNOWN) @@ -237,7 +237,10 @@ class ExportDataTest(TestCase): self.domain_7, _ = Domain.objects.get_or_create(name="xdomain7.gov", state=Domain.State.DELETED, deleted_at=timezone.now()) self.domain_8, _ = Domain.objects.get_or_create(name="sdomain8.gov", state=Domain.State.DELETED, deleted_at=timezone.now()) # We use timezone.make_aware to sync to server time a datetime object with the current date (using date.today()) and a specific time (using datetime.min.time()). + # Deleted yesterday self.domain_9, _ = Domain.objects.get_or_create(name="zdomain9.gov", state=Domain.State.DELETED, deleted_at=timezone.make_aware(datetime.combine(date.today() - timedelta(days=1), datetime.min.time()))) + # ready tomorrow + self.domain_10, _ = Domain.objects.get_or_create(name="adomain10.gov", state=Domain.State.READY, ready_at=timezone.make_aware(datetime.combine(date.today() + timedelta(days=1), datetime.min.time()))) self.domain_information_1, _ = DomainInformation.objects.get_or_create( creator=self.user, @@ -293,6 +296,12 @@ class ExportDataTest(TestCase): organization_type="federal", federal_agency="Armed Forces Retirement Home", ) + self.domain_information_10, _ = DomainInformation.objects.get_or_create( + creator=self.user, + domain=self.domain_10, + organization_type="federal", + federal_agency="Armed Forces Retirement Home", + ) def tearDown(self): Domain.objects.all().delete() @@ -349,6 +358,7 @@ class ExportDataTest(TestCase): "Domain name,Domain type,Agency,Organization name,City,State,AO," "AO email,Submitter,Submitter title,Submitter email,Submitter phone," "Security contact email,Status\n" + "adomain10.gov,Federal,Armed Forces Retirement Home,ready\n" "adomain2.gov,Interstate,dnsneeded\n" "cdomain1.gov,Federal - Executive,World War I Centennial Commission,ready\n" "ddomain3.gov,Federal,Armed Forces Retirement Home,onhold\n" @@ -402,6 +412,7 @@ class ExportDataTest(TestCase): expected_content = ( "Domain name,Domain type,Agency,Organization name,City," "State,Security contact email\n" + "adomain10.gov,Federal,Armed Forces Retirement Home\n" "cdomain1.gov,Federal - Executive,World War I Centennial Commission\n" "ddomain3.gov,Federal,Armed Forces Retirement Home\n" ) @@ -415,15 +426,16 @@ class ExportDataTest(TestCase): def test_export_domains_to_writer_with_date_filter_pulls_domains_in_range(self): """Test that domains that are - 1. READY and their created_at dates are in range + 1. READY and their ready_at dates are in range 2. DELETED and their deleted_at dates are in range are pulled when the growth report conditions are applied to export_domains_to_writed. - Test that ready domains display first and deleted second, sorted according to - specified keys. + Test that ready domains are sorted by ready_at/deleted_at dates first, names second. We considered testing export_data_growth_to_csv which calls export_domains_to_writer and would have been easy to set up, but expected_content would contain created_at dates - which are hard to mock.""" + which are hard to mock. + + TODO: Simplify is created_at is not needed for the report.""" # Create a CSV file in memory csv_file = StringIO() @@ -452,8 +464,8 @@ class ExportDataTest(TestCase): "domain__state__in": [ Domain.State.READY, ], - "domain__created_at__lt": end_date, - "domain__created_at__gt": start_date, + "domain__ready_at__lt": end_date, + "domain__ready_at__gt": start_date, } filter_conditions_for_additional_domains = { "domain__state__in": [ @@ -477,7 +489,8 @@ class ExportDataTest(TestCase): expected_content = ( "Domain name,Domain type,Agency,Organization name,City," "State,Status,Expiration date\n" - "cdomain1.gov,Federal-Executive,World War I Centennial Commission,ready\n" + "cdomain1.gov,Federal-Executive,World War I Centennial Commission,,,,ready,\n" + "adomain10.gov,Federal,Armed Forces Retirement Home,,,,ready,\n" "zdomain9.gov,Federal,Armed Forces Retirement Home,,,,deleted,\n" "sdomain8.gov,Federal,Armed Forces Retirement Home,,,,deleted,\n" "xdomain7.gov,Federal,Armed Forces Retirement Home,,,,deleted,\n" diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 43c532d73..45b3abd39 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -39,6 +39,7 @@ def write_row(writer, columns, domain_info): "Status": domain_info.domain.state, "Expiration date": domain_info.domain.expiration_date, "Created at": domain_info.domain.created_at, + "Ready at": domain_info.domain.ready_at, "Deleted at": domain_info.domain.deleted_at, } writer.writerow([FIELDS.get(column, "") for column in columns]) @@ -205,6 +206,7 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): "State", "Status", "Created at", + "Ready at", "Deleted at", "Expiration date", ] @@ -214,8 +216,8 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): ] filter_condition = { "domain__state__in": [Domain.State.READY], - "domain__created_at__lt": end_date_formatted, - "domain__created_at__gt": start_date_formatted, + "domain__ready_at__lt": end_date_formatted, + "domain__ready_at__gt": start_date_formatted, } # We also want domains deleted between sar and end dates, sorted From ff32a020224ff83527cd4cf85500078b9f597058 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 21 Dec 2023 12:46:26 -0500 Subject: [PATCH 13/42] lint --- src/registrar/config/urls.py | 3 +- src/registrar/models/domain.py | 4 +- src/registrar/tests/test_admin_views.py | 16 ++--- src/registrar/tests/test_models_domain.py | 10 +-- src/registrar/tests/test_reports.py | 84 ++++++++++++++++------- src/registrar/utility/csv_export.py | 73 +++++++++++++------- src/registrar/views/admin_views.py | 13 ++-- 7 files changed, 126 insertions(+), 77 deletions(-) diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index edfe7619e..16b44ec85 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -9,6 +9,7 @@ from django.urls import include, path from django.views.generic import RedirectView from registrar import views + # from registrar.views.admin_views import export_data from registrar.views.admin_views import ExportData @@ -53,7 +54,7 @@ urlpatterns = [ "admin/logout/", RedirectView.as_view(pattern_name="logout", permanent=False), ), - path('export_data/', ExportData.as_view(), name='admin_export_data'), + path("export_data/", ExportData.as_view(), name="admin_export_data"), path("admin/", admin.site.urls), path( "application//edit/", diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 3b347b7cd..1c9a9f1ad 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -960,13 +960,13 @@ class Domain(TimeStampedModel, DomainHelper): null=True, help_text=("Duplication of registry's expiration date saved for ease of reporting"), ) - + deleted_at = DateField( null=True, editable=False, help_text="Deleted at date", ) - + ready_at = DateField( null=True, editable=False, diff --git a/src/registrar/tests/test_admin_views.py b/src/registrar/tests/test_admin_views.py index 6336ed139..b55fc2ddd 100644 --- a/src/registrar/tests/test_admin_views.py +++ b/src/registrar/tests/test_admin_views.py @@ -1,7 +1,6 @@ from django.test import TestCase, Client from django.urls import reverse from registrar.tests.common import create_superuser -from registrar.views.admin_views import ExportData class TestViews(TestCase): @@ -10,16 +9,15 @@ class TestViews(TestCase): self.superuser = create_superuser() def test_export_data_view(self): - self.client.force_login(self.superuser) - + # Reverse the URL for the admin index page admin_index_url = reverse("admin:index") # Make a GET request to the admin index page response = self.client.get(admin_index_url) - - print(f'response1 {response}') + + print(f"response1 {response}") # Assert that the response status code is 200 (OK) self.assertEqual(response.status_code, 200) @@ -39,14 +37,10 @@ class TestViews(TestCase): # Assert that the response status code is 200 (OK) or the expected status code self.assertEqual(response.status_code, 200) - + # Assert that the content type is CSV self.assertEqual(response["Content-Type"], "text/csv") # Check if the filename in the Content-Disposition header matches the expected pattern - expected_filename = f'growth-from-{start_date}-to-{end_date}.csv' + expected_filename = f"growth-from-{start_date}-to-{end_date}.csv" self.assertIn(f'attachment; filename="{expected_filename}"', response["Content-Disposition"]) - - - - \ No newline at end of file diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index fcb527014..c84001569 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -1138,7 +1138,7 @@ class TestRegistrantNameservers(MockEppLib): # check that status is still NOT READY # as you have less than 2 nameservers self.assertFalse(self.domain.is_active()) - + self.assertEqual(self.domain.ready_at, None) def test_user_adds_two_nameservers(self): @@ -2253,7 +2253,7 @@ class TestAnalystDelete(MockEppLib): When `domain.deletedInEpp()` is called Then `commands.DeleteDomain` is sent to the registry And `state` is set to `DELETED` - + The deleted_at date is set. """ # Put the domain in client hold @@ -2275,7 +2275,7 @@ class TestAnalystDelete(MockEppLib): # Domain should have the right state self.assertEqual(self.domain.state, Domain.State.DELETED) - + # Domain should have a deleted_at self.assertNotEqual(self.domain.deleted_at, None) @@ -2321,7 +2321,7 @@ class TestAnalystDelete(MockEppLib): and domain is of `state` is `READY` Then an FSM error is returned And `state` is not set to `DELETED` - + The deleted_at date is still null. """ self.assertEqual(self.domain.state, Domain.State.READY) @@ -2333,6 +2333,6 @@ class TestAnalystDelete(MockEppLib): self.assertNotEqual(self.domain, None) # Domain should have the right state self.assertEqual(self.domain.state, Domain.State.READY) - + # deleted_at should be null self.assertEqual(self.domain.deleted_at, None) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index a9f28b8c8..6b24da906 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -6,7 +6,11 @@ from registrar.models.domain_information import DomainInformation from registrar.models.domain import Domain from registrar.models.user import User from django.contrib.auth import get_user_model -from registrar.utility.csv_export import export_domains_to_writer, get_default_start_date, get_default_end_date, export_data_growth_to_csv +from registrar.utility.csv_export import ( + export_domains_to_writer, + 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 @@ -17,6 +21,7 @@ from registrar.utility.s3_bucket import S3ClientError, S3ClientErrorCodes # typ from datetime import date, datetime, timedelta from django.utils import timezone + class CsvReportsTest(TestCase): """Tests to determine if we are uploading our reports correctly""" @@ -227,21 +232,40 @@ class ExportDataTest(TestCase): username=username, first_name=first_name, last_name=last_name, email=email ) - self.domain_1, _ = Domain.objects.get_or_create(name="cdomain1.gov", state=Domain.State.READY, ready_at=timezone.now()) + self.domain_1, _ = Domain.objects.get_or_create( + name="cdomain1.gov", state=Domain.State.READY, ready_at=timezone.now() + ) self.domain_2, _ = Domain.objects.get_or_create(name="adomain2.gov", state=Domain.State.DNS_NEEDED) self.domain_3, _ = Domain.objects.get_or_create(name="ddomain3.gov", state=Domain.State.ON_HOLD) self.domain_4, _ = Domain.objects.get_or_create(name="bdomain4.gov", state=Domain.State.UNKNOWN) self.domain_4, _ = Domain.objects.get_or_create(name="bdomain4.gov", state=Domain.State.UNKNOWN) - self.domain_5, _ = Domain.objects.get_or_create(name="bdomain5.gov", state=Domain.State.DELETED, deleted_at=timezone.make_aware(datetime(2023, 11, 1))) - self.domain_6, _ = Domain.objects.get_or_create(name="bdomain6.gov", state=Domain.State.DELETED, deleted_at=timezone.make_aware(datetime(1980, 10, 16))) - self.domain_7, _ = Domain.objects.get_or_create(name="xdomain7.gov", state=Domain.State.DELETED, deleted_at=timezone.now()) - self.domain_8, _ = Domain.objects.get_or_create(name="sdomain8.gov", state=Domain.State.DELETED, deleted_at=timezone.now()) - # We use timezone.make_aware to sync to server time a datetime object with the current date (using date.today()) and a specific time (using datetime.min.time()). + self.domain_5, _ = Domain.objects.get_or_create( + name="bdomain5.gov", state=Domain.State.DELETED, deleted_at=timezone.make_aware(datetime(2023, 11, 1)) + ) + self.domain_6, _ = Domain.objects.get_or_create( + name="bdomain6.gov", state=Domain.State.DELETED, deleted_at=timezone.make_aware(datetime(1980, 10, 16)) + ) + self.domain_7, _ = Domain.objects.get_or_create( + name="xdomain7.gov", state=Domain.State.DELETED, deleted_at=timezone.now() + ) + self.domain_8, _ = Domain.objects.get_or_create( + name="sdomain8.gov", state=Domain.State.DELETED, deleted_at=timezone.now() + ) + # We use timezone.make_aware to sync to server time a datetime object with the current date (using date.today()) + # and a specific time (using datetime.min.time()). # Deleted yesterday - self.domain_9, _ = Domain.objects.get_or_create(name="zdomain9.gov", state=Domain.State.DELETED, deleted_at=timezone.make_aware(datetime.combine(date.today() - timedelta(days=1), datetime.min.time()))) + self.domain_9, _ = Domain.objects.get_or_create( + name="zdomain9.gov", + state=Domain.State.DELETED, + deleted_at=timezone.make_aware(datetime.combine(date.today() - timedelta(days=1), datetime.min.time())), + ) # ready tomorrow - self.domain_10, _ = Domain.objects.get_or_create(name="adomain10.gov", state=Domain.State.READY, ready_at=timezone.make_aware(datetime.combine(date.today() + timedelta(days=1), datetime.min.time()))) - + self.domain_10, _ = Domain.objects.get_or_create( + name="adomain10.gov", + state=Domain.State.READY, + ready_at=timezone.make_aware(datetime.combine(date.today() + timedelta(days=1), datetime.min.time())), + ) + self.domain_information_1, _ = DomainInformation.objects.get_or_create( creator=self.user, domain=self.domain_1, @@ -423,24 +447,25 @@ class ExportDataTest(TestCase): expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() self.assertEqual(csv_content, expected_content) - + def test_export_domains_to_writer_with_date_filter_pulls_domains_in_range(self): - """Test that domains that are + """Test that domains that are 1. READY and their ready_at dates are in range 2. DELETED and their deleted_at dates are in range are pulled when the growth report conditions are applied to export_domains_to_writed. Test that ready domains are sorted by ready_at/deleted_at dates first, names second. - + We considered testing export_data_growth_to_csv which calls export_domains_to_writer and would have been easy to set up, but expected_content would contain created_at dates which are hard to mock. - + TODO: Simplify is created_at is not needed for the report.""" - + # Create a CSV file in memory csv_file = StringIO() writer = csv.writer(csv_file) - # We use timezone.make_aware to sync to server time a datetime object with the current date (using date.today()) and a specific time (using datetime.min.time()). + # We use timezone.make_aware to sync to server time a datetime object with the current date (using date.today()) + # and a specific time (using datetime.min.time()). end_date = timezone.make_aware(datetime.combine(date.today() + timedelta(days=2), datetime.min.time())) start_date = timezone.make_aware(datetime.combine(date.today() - timedelta(days=2), datetime.min.time())) @@ -455,7 +480,10 @@ class ExportDataTest(TestCase): "Status", "Expiration date", ] - sort_fields = ["created_at","domain__name",] + sort_fields = [ + "created_at", + "domain__name", + ] sort_fields_for_additional_domains = [ "domain__deleted_at", "domain__name", @@ -476,15 +504,22 @@ class ExportDataTest(TestCase): } # Call the export function - export_domains_to_writer(writer, columns, sort_fields, filter_condition, sort_fields_for_additional_domains, filter_conditions_for_additional_domains) + export_domains_to_writer( + writer, + columns, + sort_fields, + filter_condition, + sort_fields_for_additional_domains, + filter_conditions_for_additional_domains, + ) # 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 first, created between today-2 and today+2, sorted by created_at then name + + # We expect READY domains first, created between today-2 and today+2, sorted by created_at then name # and DELETED domains deleted between today-2 and today+2, sorted by deleted_at then name expected_content = ( "Domain name,Domain type,Agency,Organization name,City," @@ -500,12 +535,13 @@ class ExportDataTest(TestCase): # 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.""" - + def test_get_default_start_date(self): expected_date = timezone.make_aware(datetime(2023, 11, 1)) actual_date = get_default_start_date() @@ -515,4 +551,4 @@ class HelperFunctions(TestCase): # Note: You may need to mock timezone.now() for accurate testing expected_date = timezone.now() actual_date = get_default_end_date() - self.assertEqual(actual_date.date(), expected_date.date()) \ No newline at end of file + self.assertEqual(actual_date.date(), expected_date.date()) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 45b3abd39..fd220b891 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -1,6 +1,6 @@ import csv import logging -from datetime import date, datetime +from datetime import datetime from registrar.models.domain import Domain from registrar.models.domain_information import DomainInformation from registrar.models.public_contact import PublicContact @@ -11,10 +11,12 @@ from django.utils import timezone logger = logging.getLogger(__name__) + def get_domain_infos(filter_condition, sort_fields): domain_infos = DomainInformation.objects.filter(**filter_condition).order_by(*sort_fields) return domain_infos + def write_row(writer, columns, domain_info): security_contacts = domain_info.domain.contacts.filter(contact_type=PublicContact.ContactTypeChoices.SECURITY) # For linter @@ -44,34 +46,48 @@ def write_row(writer, columns, domain_info): } writer.writerow([FIELDS.get(column, "") for column in columns]) -def export_domains_to_writer(writer, columns, sort_fields, filter_condition, sort_fields_for_additional_domains=None, filter_condition_for_additional_domains=None): + +def export_domains_to_writer( + writer, + columns, + sort_fields, + filter_condition, + sort_fields_for_additional_domains=None, + filter_condition_for_additional_domains=None, +): """ - Receives params from the parent methods and outputs a CSV with fltered and sorted domains. - The 'additional' params enable us to concatenate 2 different filtered lists. + Receives params from the parent methods and outputs a CSV with fltered and sorted domains. + The 'additional' params enable us to concatenate 2 different filtered lists. """ # write columns headers to writer writer.writerow(columns) - # Get the domainInfos + # Get the domainInfos domainInfos = get_domain_infos(filter_condition, sort_fields) - + # Condition is true for export_data_growth_to_csv. This is an OR situation so we can' combine the filters - # in one query. - if filter_condition_for_additional_domains is not None and 'domain__deleted_at__lt' in filter_condition_for_additional_domains: + # in one query. + if ( + filter_condition_for_additional_domains is not None + and "domain__deleted_at__lt" in filter_condition_for_additional_domains + ): # Get the deleted domain infos - deleted_domainInfos = get_domain_infos(filter_condition_for_additional_domains, sort_fields_for_additional_domains) + deleted_domainInfos = get_domain_infos( + filter_condition_for_additional_domains, sort_fields_for_additional_domains + ) # Combine the two querysets into a single iterable all_domainInfos = list(chain(domainInfos, deleted_domainInfos)) else: all_domainInfos = list(domainInfos) - + # Write rows to CSV for domain_info in all_domainInfos: write_row(writer, columns, domain_info) + def export_data_type_to_csv(csv_file): """All domains report with extra columns""" - + writer = csv.writer(csv_file) # define columns to include in export columns = [ @@ -103,9 +119,10 @@ def export_data_type_to_csv(csv_file): } export_domains_to_writer(writer, columns, sort_fields, filter_condition) + def export_data_full_to_csv(csv_file): """All domains report""" - + writer = csv.writer(csv_file) # define columns to include in export columns = [ @@ -136,7 +153,7 @@ def export_data_full_to_csv(csv_file): def export_data_federal_to_csv(csv_file): """Federal domains report""" - + writer = csv.writer(csv_file) # define columns to include in export columns = [ @@ -165,14 +182,17 @@ def export_data_federal_to_csv(csv_file): } export_domains_to_writer(writer, columns, sort_fields, filter_condition) + def get_default_start_date(): # Default to a date that's prior to our first deployment return timezone.make_aware(datetime(2023, 11, 1)) + def get_default_end_date(): # Default to now() return timezone.now() + def export_data_growth_to_csv(csv_file, start_date, end_date): """ Growth report: @@ -181,21 +201,17 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): the start and end dates, as well as DELETED domains that are deleted between the start and end dates. Specify sort params for both lists. """ - + start_date_formatted = ( - timezone.make_aware(datetime.strptime(start_date, "%Y-%m-%d")) - if start_date - else get_default_start_date() + timezone.make_aware(datetime.strptime(start_date, "%Y-%m-%d")) if start_date else get_default_start_date() ) end_date_formatted = ( - timezone.make_aware(datetime.strptime(end_date, "%Y-%m-%d")) - if end_date - else get_default_end_date() + timezone.make_aware(datetime.strptime(end_date, "%Y-%m-%d")) if end_date else get_default_end_date() ) - + writer = csv.writer(csv_file) - + # define columns to include in export columns = [ "Domain name", @@ -219,7 +235,7 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): "domain__ready_at__lt": end_date_formatted, "domain__ready_at__gt": start_date_formatted, } - + # We also want domains deleted between sar and end dates, sorted sort_fields_for_additional_domains = [ "domain__deleted_at", @@ -230,5 +246,12 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): "domain__created_at__lt": end_date_formatted, "domain__created_at__gt": start_date_formatted, } - - export_domains_to_writer(writer, columns, sort_fields, filter_condition, sort_fields_for_additional_domains, filter_condition_for_additional_domains) + + export_domains_to_writer( + writer, + columns, + sort_fields, + filter_condition, + sort_fields_for_additional_domains, + filter_condition_for_additional_domains, + ) diff --git a/src/registrar/views/admin_views.py b/src/registrar/views/admin_views.py index 22792a002..ddccc3a50 100644 --- a/src/registrar/views/admin_views.py +++ b/src/registrar/views/admin_views.py @@ -2,8 +2,6 @@ from django.http import HttpResponse from django.views import View -from django.views.decorators.csrf import csrf_exempt -from django.shortcuts import render from registrar.utility import csv_export @@ -13,19 +11,16 @@ logger = logging.getLogger(__name__) class ExportData(View): - def get(self, request, *args, **kwargs): # Get start_date and end_date from the request's GET parameters # #999: not needed if we switch to django forms - start_date = request.GET.get('start_date', '') - end_date = request.GET.get('end_date', '') + start_date = request.GET.get("start_date", "") + end_date = request.GET.get("end_date", "") response = HttpResponse(content_type="text/csv") response["Content-Disposition"] = f'attachment; filename="growth-from-{start_date}-to-{end_date}.csv"' # For #999: set export_data_growth_to_csv to return the resulting queryset, which we can then use - # in context to display this data in the template. + # in context to display this data in the template. csv_export.export_data_growth_to_csv(response, start_date, end_date) - - return response - \ No newline at end of file + return response From 1be00962ac326ada7a9acc9b6063e5834c452370 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 21 Dec 2023 12:49:52 -0500 Subject: [PATCH 14/42] trailing line --- src/registrar/assets/js/get-gov-admin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index ce0725a63..68e7829a9 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -306,4 +306,4 @@ function enableRelatedWidgetButtons(changeLink, deleteLink, viewLink, elementPk, }); } -})(); \ No newline at end of file +})(); From 92f17c437fd7b13fadf06bb40ee1043a28b3851b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 21 Dec 2023 14:48:54 -0500 Subject: [PATCH 15/42] Change ready_at to first_ready_at and make a unit test for it --- ...omain_deleted_at_domain_first_ready_at.py} | 4 +-- src/registrar/models/domain.py | 8 +++-- src/registrar/tests/test_models_domain.py | 36 ++++++++++++++++--- 3 files changed, 40 insertions(+), 8 deletions(-) rename src/registrar/migrations/{0057_domain_deleted_at_domain_ready_at.py => 0057_domain_deleted_at_domain_first_ready_at.py} (88%) diff --git a/src/registrar/migrations/0057_domain_deleted_at_domain_ready_at.py b/src/registrar/migrations/0057_domain_deleted_at_domain_first_ready_at.py similarity index 88% rename from src/registrar/migrations/0057_domain_deleted_at_domain_ready_at.py rename to src/registrar/migrations/0057_domain_deleted_at_domain_first_ready_at.py index 400fddc3a..77c565886 100644 --- a/src/registrar/migrations/0057_domain_deleted_at_domain_ready_at.py +++ b/src/registrar/migrations/0057_domain_deleted_at_domain_first_ready_at.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.7 on 2023-12-21 17:12 +# Generated by Django 4.2.7 on 2023-12-21 19:28 from django.db import migrations, models @@ -16,7 +16,7 @@ class Migration(migrations.Migration): ), migrations.AddField( model_name="domain", - name="ready_at", + name="first_ready_at", field=models.DateField( editable=False, help_text="The last time this domain moved into the READY state", null=True ), diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 1c9a9f1ad..e02978937 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -967,7 +967,7 @@ class Domain(TimeStampedModel, DomainHelper): help_text="Deleted at date", ) - ready_at = DateField( + first_ready_at = DateField( null=True, editable=False, help_text="The last time this domain moved into the READY state", @@ -1336,7 +1336,11 @@ class Domain(TimeStampedModel, DomainHelper): """ logger.info("Changing to ready state") logger.info("able to transition to ready state") - self.ready_at = timezone.now() + # if self.first_ready_at is not None, this means that his + # domain wasr READY, then not READY, then is READY again. + # We do not want to overwrite first_ready_at. + if self.first_ready_at is None: + self.first_ready_at = timezone.now() @transition( field="state", diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index c84001569..95f8ca2a5 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -385,6 +385,34 @@ class TestDomainStatuses(MockEppLib): """Domain 'revert_client_hold' method causes the registry to change statuses""" raise + def test_first_ready_at(self): + """ + first_ready_at is set when a domain is first transitioned to READY. It does not get overwritten + in case the domain gets out of and back into READY. + """ + domain, _ = Domain.objects.get_or_create(name="pig-knuckles.gov", state=Domain.State.DNS_NEEDED) + self.assertEqual(domain.first_ready_at, None) + + domain.ready() + + # check that status is READY + self.assertTrue(domain.is_active()) + self.assertNotEqual(domain.first_ready_at, None) + + # Capture the value of first_ready_at + first_ready_at = domain.first_ready_at + + # change domain status + domain.dns_needed() + self.assertFalse(domain.is_active()) + + # change back to READY + domain.ready() + self.assertTrue(domain.is_active()) + + # assert that the value of first_ready_at has not changed + self.assertEqual(domain.first_ready_at, first_ready_at) + def tearDown(self) -> None: PublicContact.objects.all().delete() Domain.objects.all().delete() @@ -1112,7 +1140,7 @@ class TestRegistrantNameservers(MockEppLib): Then `commands.CreateHost` and `commands.UpdateDomain` is sent to the registry And `domain.is_active` returns False - And domain.ready_at is null + And domain.first_ready_at is null """ # set 1 nameserver @@ -1139,7 +1167,7 @@ class TestRegistrantNameservers(MockEppLib): # as you have less than 2 nameservers self.assertFalse(self.domain.is_active()) - self.assertEqual(self.domain.ready_at, None) + self.assertEqual(self.domain.first_ready_at, None) def test_user_adds_two_nameservers(self): """ @@ -1149,7 +1177,7 @@ class TestRegistrantNameservers(MockEppLib): Then `commands.CreateHost` and `commands.UpdateDomain` is sent to the registry And `domain.is_active` returns True - And domain.ready_at is not null + And domain.first_ready_at is not null """ # set 2 nameservers @@ -1180,7 +1208,7 @@ class TestRegistrantNameservers(MockEppLib): self.assertEqual(4, self.mockedSendFunction.call_count) # check that status is READY self.assertTrue(self.domain.is_active()) - self.assertNotEqual(self.domain.ready_at, None) + self.assertNotEqual(self.domain.first_ready_at, None) def test_user_adds_too_many_nameservers(self): """ From fff987399827a54f3702e9385fd6a4494c9ecac8 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 21 Dec 2023 15:11:31 -0500 Subject: [PATCH 16/42] firx readt_at in unit tests --- src/registrar/tests/test_reports.py | 12 ++++++------ src/registrar/utility/csv_export.py | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 6b24da906..a1cbf1a18 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -233,7 +233,7 @@ class ExportDataTest(TestCase): ) self.domain_1, _ = Domain.objects.get_or_create( - name="cdomain1.gov", state=Domain.State.READY, ready_at=timezone.now() + name="cdomain1.gov", state=Domain.State.READY, first_ready_at=timezone.now() ) self.domain_2, _ = Domain.objects.get_or_create(name="adomain2.gov", state=Domain.State.DNS_NEEDED) self.domain_3, _ = Domain.objects.get_or_create(name="ddomain3.gov", state=Domain.State.ON_HOLD) @@ -263,7 +263,7 @@ class ExportDataTest(TestCase): self.domain_10, _ = Domain.objects.get_or_create( name="adomain10.gov", state=Domain.State.READY, - ready_at=timezone.make_aware(datetime.combine(date.today() + timedelta(days=1), datetime.min.time())), + first_ready_at=timezone.make_aware(datetime.combine(date.today() + timedelta(days=1), datetime.min.time())), ) self.domain_information_1, _ = DomainInformation.objects.get_or_create( @@ -450,10 +450,10 @@ class ExportDataTest(TestCase): def test_export_domains_to_writer_with_date_filter_pulls_domains_in_range(self): """Test that domains that are - 1. READY and their ready_at dates are in range + 1. READY and their first_ready_at dates are in range 2. DELETED and their deleted_at dates are in range are pulled when the growth report conditions are applied to export_domains_to_writed. - Test that ready domains are sorted by ready_at/deleted_at dates first, names second. + Test that ready domains are sorted by first_ready_at/deleted_at dates first, names second. We considered testing export_data_growth_to_csv which calls export_domains_to_writer and would have been easy to set up, but expected_content would contain created_at dates @@ -492,8 +492,8 @@ class ExportDataTest(TestCase): "domain__state__in": [ Domain.State.READY, ], - "domain__ready_at__lt": end_date, - "domain__ready_at__gt": start_date, + "domain__first_ready_at__lt": end_date, + "domain__first_ready_at__gt": start_date, } filter_conditions_for_additional_domains = { "domain__state__in": [ diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index fd220b891..7f078362c 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -41,7 +41,7 @@ def write_row(writer, columns, domain_info): "Status": domain_info.domain.state, "Expiration date": domain_info.domain.expiration_date, "Created at": domain_info.domain.created_at, - "Ready at": domain_info.domain.ready_at, + "Ready at": domain_info.domain.first_ready_at, "Deleted at": domain_info.domain.deleted_at, } writer.writerow([FIELDS.get(column, "") for column in columns]) @@ -232,8 +232,8 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): ] filter_condition = { "domain__state__in": [Domain.State.READY], - "domain__ready_at__lt": end_date_formatted, - "domain__ready_at__gt": start_date_formatted, + "domain__first_ready_at__lt": end_date_formatted, + "domain__first_ready_at__gt": start_date_formatted, } # We also want domains deleted between sar and end dates, sorted From 2f3a06f7fa867ad573fe411f54a03272c07cc050 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 21 Dec 2023 15:24:14 -0500 Subject: [PATCH 17/42] Add first_ready_at as a readonly field on Domain admin --- 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 9a8a655c1..8f379b676 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -812,7 +812,7 @@ class DomainAdmin(ListHeaderAdmin): search_help_text = "Search by domain name." change_form_template = "django/admin/domain_change_form.html" change_list_template = "django/admin/domain_change_list.html" - readonly_fields = ["state", "expiration_date", "deleted_at"] + readonly_fields = ["state", "expiration_date", "first_ready_at", "deleted_at"] def export_data_type(self, request): # match the CSV example with all the fields From 2b6f2e977e11c34996befd91cd240de20198db9e Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 21 Dec 2023 16:04:30 -0500 Subject: [PATCH 18/42] Fix some issues with deleted_at and first_ready_at in csv_export, add those new date columns to Domain table in admin --- src/registrar/admin.py | 1 + src/registrar/tests/test_reports.py | 8 ++++---- src/registrar/utility/csv_export.py | 19 ++++++++----------- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8f379b676..3c1063145 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -796,6 +796,7 @@ class DomainAdmin(ListHeaderAdmin): "organization_type", "state", "created_at", + "first_ready_at", "deleted_at", "expiration_date", ] diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index a1cbf1a18..23cc2caf8 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -492,15 +492,15 @@ class ExportDataTest(TestCase): "domain__state__in": [ Domain.State.READY, ], - "domain__first_ready_at__lt": end_date, - "domain__first_ready_at__gt": start_date, + "domain__first_ready_at__lte": end_date, + "domain__first_ready_at__gte": start_date, } filter_conditions_for_additional_domains = { "domain__state__in": [ Domain.State.DELETED, ], - "domain__deleted_at__lt": end_date, - "domain__deleted_at__gt": start_date, + "domain__deleted_at__lte": end_date, + "domain__deleted_at__gte": start_date, } # Call the export function diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 7f078362c..dad9e94c8 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -41,7 +41,7 @@ def write_row(writer, columns, domain_info): "Status": domain_info.domain.state, "Expiration date": domain_info.domain.expiration_date, "Created at": domain_info.domain.created_at, - "Ready at": domain_info.domain.first_ready_at, + "First ready at": domain_info.domain.first_ready_at, "Deleted at": domain_info.domain.deleted_at, } writer.writerow([FIELDS.get(column, "") for column in columns]) @@ -67,10 +67,7 @@ def export_domains_to_writer( # Condition is true for export_data_growth_to_csv. This is an OR situation so we can' combine the filters # in one query. - if ( - filter_condition_for_additional_domains is not None - and "domain__deleted_at__lt" in filter_condition_for_additional_domains - ): + if filter_condition_for_additional_domains is not None: # Get the deleted domain infos deleted_domainInfos = get_domain_infos( filter_condition_for_additional_domains, sort_fields_for_additional_domains @@ -222,18 +219,18 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): "State", "Status", "Created at", - "Ready at", + "First ready at", "Deleted at", "Expiration date", ] sort_fields = [ - "created_at", + "domain__first_ready_at", "domain__name", ] filter_condition = { "domain__state__in": [Domain.State.READY], - "domain__first_ready_at__lt": end_date_formatted, - "domain__first_ready_at__gt": start_date_formatted, + "domain__first_ready_at__lte": end_date_formatted, + "domain__first_ready_at__gte": start_date_formatted, } # We also want domains deleted between sar and end dates, sorted @@ -243,8 +240,8 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): ] filter_condition_for_additional_domains = { "domain__state__in": [Domain.State.DELETED], - "domain__created_at__lt": end_date_formatted, - "domain__created_at__gt": start_date_formatted, + "domain__deleted_at__lte": end_date_formatted, + "domain__deleted_at__gte": start_date_formatted, } export_domains_to_writer( From 49324fcf668aeb3ddcaf7a804c1bce3c5f53a486 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 21 Dec 2023 16:29:02 -0500 Subject: [PATCH 19/42] set start date to now, use display for State --- src/registrar/assets/js/get-gov-admin.js | 6 +++++- src/registrar/tests/test_reports.py | 18 +++++++++--------- src/registrar/utility/csv_export.py | 2 +- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 68e7829a9..cdbbc83ee 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -284,6 +284,10 @@ function enableRelatedWidgetButtons(changeLink, deleteLink, viewLink, elementPk, // Get the current date in the format YYYY-MM-DD var currentDate = new Date().toISOString().split('T')[0]; + + // Default the value of the start date input field to the current date + let startDateInput =document.getElementById('start'); + startDateInput.value = currentDate; // Default the value of the end date input field to the current date let endDateInput =document.getElementById('end'); @@ -294,7 +298,7 @@ function enableRelatedWidgetButtons(changeLink, deleteLink, viewLink, elementPk, if (exportGrowthReportButton) { exportGrowthReportButton.addEventListener('click', function() { // Get the selected start and end dates - let startDate = document.getElementById('start').value; + let startDate = startDateInput.value; let endDate = endDateInput.value; let exportUrl = document.getElementById('exportLink').dataset.exportUrl; diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 23cc2caf8..c55d776cc 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -382,10 +382,10 @@ class ExportDataTest(TestCase): "Domain name,Domain type,Agency,Organization name,City,State,AO," "AO email,Submitter,Submitter title,Submitter email,Submitter phone," "Security contact email,Status\n" - "adomain10.gov,Federal,Armed Forces Retirement Home,ready\n" - "adomain2.gov,Interstate,dnsneeded\n" - "cdomain1.gov,Federal - Executive,World War I Centennial Commission,ready\n" - "ddomain3.gov,Federal,Armed Forces Retirement Home,onhold\n" + "adomain10.gov,Federal,Armed Forces Retirement Home,Ready\n" + "adomain2.gov,Interstate,Dns needed\n" + "cdomain1.gov,Federal - Executive,World War I Centennial Commission,Ready\n" + "ddomain3.gov,Federal,Armed Forces Retirement Home,On hold\n" ) # Normalize line endings and remove commas, @@ -524,11 +524,11 @@ class ExportDataTest(TestCase): expected_content = ( "Domain name,Domain type,Agency,Organization name,City," "State,Status,Expiration date\n" - "cdomain1.gov,Federal-Executive,World War I Centennial Commission,,,,ready,\n" - "adomain10.gov,Federal,Armed Forces Retirement Home,,,,ready,\n" - "zdomain9.gov,Federal,Armed Forces Retirement Home,,,,deleted,\n" - "sdomain8.gov,Federal,Armed Forces Retirement Home,,,,deleted,\n" - "xdomain7.gov,Federal,Armed Forces Retirement Home,,,,deleted,\n" + "cdomain1.gov,Federal-Executive,World War I Centennial Commission,,,,Ready,\n" + "adomain10.gov,Federal,Armed Forces Retirement Home,,,,Ready,\n" + "zdomain9.gov,Federal,Armed Forces Retirement Home,,,,Deleted,\n" + "sdomain8.gov,Federal,Armed Forces Retirement Home,,,,Deleted,\n" + "xdomain7.gov,Federal,Armed Forces Retirement Home,,,,Deleted,\n" ) # Normalize line endings and remove commas, diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index dad9e94c8..9e8739a94 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -38,7 +38,7 @@ def write_row(writer, columns, domain_info): "AO": ao, "AO email": domain_info.authorizing_official.email if domain_info.authorizing_official else " ", "Security contact email": security_contacts[0].email if security_contacts else " ", - "Status": domain_info.domain.state, + "Status": domain_info.domain.get_state_display(), "Expiration date": domain_info.domain.expiration_date, "Created at": domain_info.domain.created_at, "First ready at": domain_info.domain.first_ready_at, From e1a214b74ba355c90071e87b401184c018cba455 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 21 Dec 2023 15:22:16 -0700 Subject: [PATCH 20/42] Unit test experiments --- src/registrar/tests/test_views.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 5f071692d..1cdd27c4b 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -33,7 +33,7 @@ from registrar.models import ( UserDomainRole, User, ) -from registrar.views.application import ApplicationWizard, Step +from registrar.views.application import ApplicationStatus, ApplicationWizard, Step from .common import less_console_noise @@ -741,6 +741,7 @@ class DomainApplicationTests(TestWithUser, WebTest): phone="(555) 555 5555", ) domain, _ = DraftDomain.objects.get_or_create(name="fakeSite.gov") + current, _ = Website.objects.get_or_create(website="city.com") you, _ = Contact.objects.get_or_create( first_name="Testy you", last_name="Tester you", @@ -769,14 +770,23 @@ class DomainApplicationTests(TestWithUser, WebTest): requested_domain=domain, submitter=you, creator=self.user, + status="started" ) application.other_contacts.add(other) + application.current_websites.add(current) + + # django-webtest does not handle cookie-based sessions well because it keeps + # resetting the session key on each new request, thus destroying the concept + # of a "session". We are going to do it manually, saving the session ID here + # and then setting the cookie on each request. + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] # prime the form by visiting /edit url = reverse("edit-application", kwargs={"id": application.pk}) - response = self.client.get(url) + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) url = reverse("application:other_contacts") + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) other_contacts_page = self.client.get(url, follow=True) # ====== METHOD 2 -- prime form From 8f3f327eef0845db8c4a9b6dddf846accaa9eef7 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 21 Dec 2023 16:28:32 -0700 Subject: [PATCH 21/42] Unit test is fixed --- src/registrar/tests/test_views.py | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 1cdd27c4b..1347f51fe 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -740,8 +740,6 @@ class DomainApplicationTests(TestWithUser, WebTest): email="testy@town.com", phone="(555) 555 5555", ) - domain, _ = DraftDomain.objects.get_or_create(name="fakeSite.gov") - current, _ = Website.objects.get_or_create(website="city.com") you, _ = Contact.objects.get_or_create( first_name="Testy you", last_name="Tester you", @@ -767,27 +765,25 @@ class DomainApplicationTests(TestWithUser, WebTest): state_territory="NY", zipcode="10002", authorizing_official=ao, - requested_domain=domain, submitter=you, creator=self.user, status="started" ) application.other_contacts.add(other) - application.current_websites.add(current) + + # prime the form by visiting /edit + edit_app_page = self.app.get(reverse("edit-application", kwargs={"id": application.pk})) # django-webtest does not handle cookie-based sessions well because it keeps # resetting the session key on each new request, thus destroying the concept # of a "session". We are going to do it manually, saving the session ID here # and then setting the cookie on each request. session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] - - # prime the form by visiting /edit - url = reverse("edit-application", kwargs={"id": application.pk}) self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - url = reverse("application:other_contacts") + other_contacts_page = self.app.get(reverse("application:other_contacts")) self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - other_contacts_page = self.client.get(url, follow=True) + # ====== METHOD 2 -- prime form # other_contacts_page = self.app.get(reverse("application:other_contacts")) @@ -824,8 +820,22 @@ class DomainApplicationTests(TestWithUser, WebTest): # # Go back to the previous step # other_contacts_page = self.app.get(reverse("application:other_contacts")) - # clear the form + ##### ^ The commented out method doesn't work because it creates a duplicate application entry #### + other_contacts_form = other_contacts_page.forms[0] + + # DEBUG print statements + for f in other_contacts_form.fields: + if not "submit" in f: + print(f) + print(other_contacts_form[f].value) + + # Minimal check to ensure the form is loaded with data (if this part of + # the application doesn't work, we should be equipped with other unit + # tests to flag it) + self.assertEqual(other_contacts_form["other_contacts-0-first_name"].value, "Testy2") + + # clear the form other_contacts_form["other_contacts-0-first_name"] = "" other_contacts_form["other_contacts-0-middle_name"] = "" other_contacts_form["other_contacts-0-last_name"] = "" @@ -833,6 +843,7 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form["other_contacts-0-email"] = "" other_contacts_form["other_contacts-0-phone"] = "" + # DEBUG print statements for f in other_contacts_form.fields: if not "submit" in f: print(f) From a4293b4aaccb56c60f38275818788b1632263a56 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 21 Dec 2023 20:24:48 -0700 Subject: [PATCH 22/42] Linting & cleanup --- src/registrar/forms/application_wizard.py | 2 +- src/registrar/tests/test_views.py | 55 +++-------------------- 2 files changed, 8 insertions(+), 49 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index c0cd6e5b4..ac84a2f9f 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -608,7 +608,7 @@ class BaseOtherContactsFormSet(RegistrarFormSet): JOIN = "other_contacts" def should_delete(self, cleaned): - empty = (isinstance(v, str) and (v.strip() == "" or v == None) for v in cleaned.values()) + empty = (isinstance(v, str) and (v.strip() == "" or v is None) for v in cleaned.values()) return all(empty) def to_database(self, obj: DomainApplication): diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 1347f51fe..7301cc681 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -33,7 +33,7 @@ from registrar.models import ( UserDomainRole, User, ) -from registrar.views.application import ApplicationStatus, ApplicationWizard, Step +from registrar.views.application import ApplicationWizard, Step from .common import less_console_noise @@ -767,13 +767,12 @@ class DomainApplicationTests(TestWithUser, WebTest): authorizing_official=ao, submitter=you, creator=self.user, - status="started" + status="started", ) application.other_contacts.add(other) - # prime the form by visiting /edit - edit_app_page = self.app.get(reverse("edit-application", kwargs={"id": application.pk})) + self.app.get(reverse("edit-application", kwargs={"id": application.pk})) # django-webtest does not handle cookie-based sessions well because it keeps # resetting the session key on each new request, thus destroying the concept # of a "session". We are going to do it manually, saving the session ID here @@ -784,53 +783,15 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_page = self.app.get(reverse("application:other_contacts")) self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - - # ====== METHOD 2 -- prime form - # other_contacts_page = self.app.get(reverse("application:other_contacts")) - # session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] - # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - - # # Fill in the other contact form - # other_contacts_form = other_contacts_page.forms[0] - # other_contacts_form["other_contacts-0-first_name"] = "Testy2" - # other_contacts_form["other_contacts-0-last_name"] = "Tester2" - # other_contacts_form["other_contacts-0-title"] = "Another Tester" - # other_contacts_form["other_contacts-0-email"] = "testy2@town.com" - # other_contacts_form["other_contacts-0-phone"] = "(201) 555 5557" - - # # for f in other_contacts_form.fields: - # # if not "submit" in f: - # # print(f) - # # print(other_contacts_form[f].value) - - # # Submit the form - # other_contacts_result = other_contacts_form.submit() - # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - - # # validate that data from this step are being saved - # application = DomainApplication.objects.get() # there's only one - # self.assertEqual( - # application.other_contacts.count(), - # 1, - # ) - # # Verify user is taken to "anything else" page - # self.assertEqual(other_contacts_result.status_code, 302) - # self.assertEqual(other_contacts_result["Location"], "/register/anything_else/") - - # # Go back to the previous step - # other_contacts_page = self.app.get(reverse("application:other_contacts")) - - ##### ^ The commented out method doesn't work because it creates a duplicate application entry #### - other_contacts_form = other_contacts_page.forms[0] # DEBUG print statements for f in other_contacts_form.fields: - if not "submit" in f: + if "submit" not in f: print(f) print(other_contacts_form[f].value) - - # Minimal check to ensure the form is loaded with data (if this part of + + # Minimal check to ensure the form is loaded with data (if this part of # the application doesn't work, we should be equipped with other unit # tests to flag it) self.assertEqual(other_contacts_form["other_contacts-0-first_name"].value, "Testy2") @@ -845,7 +806,7 @@ class DomainApplicationTests(TestWithUser, WebTest): # DEBUG print statements for f in other_contacts_form.fields: - if not "submit" in f: + if "submit" not in f: print(f) print(other_contacts_form[f].value) @@ -866,8 +827,6 @@ class DomainApplicationTests(TestWithUser, WebTest): actual_url_slug = no_contacts_page.request.path.split("/")[-2] self.assertEqual(expected_url_slug, actual_url_slug) - - def test_application_about_your_organiztion_interstate(self): """Special districts have to answer an additional question.""" type_page = self.app.get(reverse("application:")).follow() From 9a78f235d503f1b2ca189e72651c88d6da35e188 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 27 Dec 2023 15:23:02 -0500 Subject: [PATCH 23/42] Refactor csv_export methods to reuse (call multiple times from the wrapper methods) instead of expand one method --- src/registrar/tests/test_reports.py | 41 +++++++++++--------- src/registrar/utility/csv_export.py | 59 ++++++++++++----------------- 2 files changed, 49 insertions(+), 51 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index c55d776cc..30f4273ea 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -7,7 +7,8 @@ from registrar.models.domain import Domain from registrar.models.user import User from django.contrib.auth import get_user_model from registrar.utility.csv_export import ( - export_domains_to_writer, + write_header, + write_body, get_default_start_date, get_default_end_date, ) @@ -41,7 +42,6 @@ class CsvReportsTest(TestCase): self.domain_2, _ = Domain.objects.get_or_create(name="adomain2.gov", state=Domain.State.DNS_NEEDED) self.domain_3, _ = Domain.objects.get_or_create(name="ddomain3.gov", state=Domain.State.ON_HOLD) self.domain_4, _ = Domain.objects.get_or_create(name="bdomain4.gov", state=Domain.State.UNKNOWN) - self.domain_4, _ = Domain.objects.get_or_create(name="bdomain4.gov", state=Domain.State.UNKNOWN) self.domain_information_1, _ = DomainInformation.objects.get_or_create( creator=self.user, @@ -333,8 +333,8 @@ class ExportDataTest(TestCase): User.objects.all().delete() super().tearDown() - def test_export_domains_to_writer(self): - """Test that export_domains_to_writer returns the + def test_write_body(self): + """Test that write_body returns the existing domain, test that sort by domain name works, test that filter works""" # Create a CSV file in memory @@ -367,8 +367,9 @@ class ExportDataTest(TestCase): ], } - # Call the export function - export_domains_to_writer(writer, columns, sort_fields, filter_condition) + # 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) @@ -395,7 +396,7 @@ class ExportDataTest(TestCase): self.assertEqual(csv_content, expected_content) - def test_export_domains_to_writer_additional(self): + def test_write_body_additional(self): """An additional test for filters and multi-column sort""" # Create a CSV file in memory csv_file = StringIO() @@ -421,8 +422,9 @@ class ExportDataTest(TestCase): ], } - # Call the export function - export_domains_to_writer(writer, columns, sort_fields, filter_condition) + # 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) @@ -448,14 +450,14 @@ class ExportDataTest(TestCase): self.assertEqual(csv_content, expected_content) - def test_export_domains_to_writer_with_date_filter_pulls_domains_in_range(self): + def test_write_body_with_date_filter_pulls_domains_in_range(self): """Test that domains that are 1. READY and their first_ready_at dates are in range 2. DELETED and their deleted_at dates are in range are pulled when the growth report conditions are applied to export_domains_to_writed. Test that ready domains are sorted by first_ready_at/deleted_at dates first, names second. - We considered testing export_data_growth_to_csv which calls export_domains_to_writer + We considered testing export_data_growth_to_csv which calls write_body and would have been easy to set up, but expected_content would contain created_at dates which are hard to mock. @@ -484,7 +486,7 @@ class ExportDataTest(TestCase): "created_at", "domain__name", ] - sort_fields_for_additional_domains = [ + sort_fields_for_deleted_domains = [ "domain__deleted_at", "domain__name", ] @@ -495,7 +497,7 @@ class ExportDataTest(TestCase): "domain__first_ready_at__lte": end_date, "domain__first_ready_at__gte": start_date, } - filter_conditions_for_additional_domains = { + filter_conditions_for_deleted_domains = { "domain__state__in": [ Domain.State.DELETED, ], @@ -503,14 +505,19 @@ class ExportDataTest(TestCase): "domain__deleted_at__gte": start_date, } - # Call the export function - export_domains_to_writer( + # Call the export functions + write_header(writer, columns) + write_body( writer, columns, sort_fields, filter_condition, - sort_fields_for_additional_domains, - filter_conditions_for_additional_domains, + ) + write_body( + writer, + columns, + sort_fields_for_deleted_domains, + filter_conditions_for_deleted_domains, ) # Reset the CSV file's position to the beginning diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 9e8739a94..0ed943613 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -6,12 +6,19 @@ from registrar.models.domain_information import DomainInformation from registrar.models.public_contact import PublicContact from django.db.models import Value from django.db.models.functions import Coalesce -from itertools import chain from django.utils import timezone 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. + """ + writer.writerow(columns) + + def get_domain_infos(filter_condition, sort_fields): domain_infos = DomainInformation.objects.filter(**filter_condition).order_by(*sort_fields) return domain_infos @@ -47,38 +54,24 @@ def write_row(writer, columns, domain_info): writer.writerow([FIELDS.get(column, "") for column in columns]) -def export_domains_to_writer( +def write_body( writer, columns, sort_fields, filter_condition, - sort_fields_for_additional_domains=None, - filter_condition_for_additional_domains=None, ): """ Receives params from the parent methods and outputs a CSV with fltered and sorted domains. - The 'additional' params enable us to concatenate 2 different filtered lists. + Works with write_header as longas the same writer object is passed. """ - # write columns headers to writer - writer.writerow(columns) # Get the domainInfos - domainInfos = get_domain_infos(filter_condition, sort_fields) + domain_infos = get_domain_infos(filter_condition, sort_fields) - # Condition is true for export_data_growth_to_csv. This is an OR situation so we can' combine the filters - # in one query. - if filter_condition_for_additional_domains is not None: - # Get the deleted domain infos - deleted_domainInfos = get_domain_infos( - filter_condition_for_additional_domains, sort_fields_for_additional_domains - ) - # Combine the two querysets into a single iterable - all_domainInfos = list(chain(domainInfos, deleted_domainInfos)) - else: - all_domainInfos = list(domainInfos) + all_domain_infos = list(domain_infos) # Write rows to CSV - for domain_info in all_domainInfos: + for domain_info in all_domain_infos: write_row(writer, columns, domain_info) @@ -114,7 +107,8 @@ def export_data_type_to_csv(csv_file): Domain.State.ON_HOLD, ], } - export_domains_to_writer(writer, columns, sort_fields, filter_condition) + write_header(writer, columns) + write_body(writer, columns, sort_fields, filter_condition) def export_data_full_to_csv(csv_file): @@ -145,7 +139,8 @@ def export_data_full_to_csv(csv_file): Domain.State.ON_HOLD, ], } - export_domains_to_writer(writer, columns, sort_fields, filter_condition) + write_header(writer, columns) + write_body(writer, columns, sort_fields, filter_condition) def export_data_federal_to_csv(csv_file): @@ -177,7 +172,8 @@ def export_data_federal_to_csv(csv_file): Domain.State.ON_HOLD, ], } - export_domains_to_writer(writer, columns, sort_fields, filter_condition) + write_header(writer, columns) + write_body(writer, columns, sort_fields, filter_condition) def get_default_start_date(): @@ -194,7 +190,7 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): """ Growth report: Receive start and end dates from the view, parse them. - Request from export_domains_to_writer READY domains that are created between + Request from write_body READY domains that are created between the start and end dates, as well as DELETED domains that are deleted between the start and end dates. Specify sort params for both lists. """ @@ -234,21 +230,16 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): } # We also want domains deleted between sar and end dates, sorted - sort_fields_for_additional_domains = [ + sort_fields_for_deleted_domains = [ "domain__deleted_at", "domain__name", ] - filter_condition_for_additional_domains = { + filter_condition_for_deleted_domains = { "domain__state__in": [Domain.State.DELETED], "domain__deleted_at__lte": end_date_formatted, "domain__deleted_at__gte": start_date_formatted, } - export_domains_to_writer( - writer, - columns, - sort_fields, - filter_condition, - sort_fields_for_additional_domains, - filter_condition_for_additional_domains, - ) + 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 407fe6e9459e5884be86e56a889cc825e9146132 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 27 Dec 2023 15:30:37 -0500 Subject: [PATCH 24/42] Resolve migrations --- ..._at.py => 0058_domain_deleted_at_domain_first_ready_at.py} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename src/registrar/migrations/{0057_domain_deleted_at_domain_first_ready_at.py => 0058_domain_deleted_at_domain_first_ready_at.py} (81%) diff --git a/src/registrar/migrations/0057_domain_deleted_at_domain_first_ready_at.py b/src/registrar/migrations/0058_domain_deleted_at_domain_first_ready_at.py similarity index 81% rename from src/registrar/migrations/0057_domain_deleted_at_domain_first_ready_at.py rename to src/registrar/migrations/0058_domain_deleted_at_domain_first_ready_at.py index 77c565886..c27d30be9 100644 --- a/src/registrar/migrations/0057_domain_deleted_at_domain_first_ready_at.py +++ b/src/registrar/migrations/0058_domain_deleted_at_domain_first_ready_at.py @@ -1,11 +1,11 @@ -# Generated by Django 4.2.7 on 2023-12-21 19:28 +# Generated by Django 4.2.7 on 2023-12-27 20:29 from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ("registrar", "0056_alter_domain_state_alter_domainapplication_status_and_more"), + ("registrar", "0057_domainapplication_submission_date"), ] operations = [ From 0951faa9986e46490114d6f4b0e2c3ec04804f0b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 27 Dec 2023 15:48:39 -0500 Subject: [PATCH 25/42] linter --- src/registrar/models/domain.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index b0bf745c3..cde80fa58 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -34,7 +34,6 @@ from django.db.models import DateField from .utility.domain_field import DomainField from .utility.domain_helper import DomainHelper from .utility.time_stamped_model import TimeStampedModel -from django.utils import timezone from .public_contact import PublicContact From e211a59304100e7dd7e529f5f2396875f8d57732 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 27 Dec 2023 19:33:48 -0500 Subject: [PATCH 26/42] Typo fixes and cleanup --- src/registrar/config/urls.py | 1 - src/registrar/models/domain.py | 4 ++-- src/registrar/templates/admin/index.html | 2 +- src/registrar/tests/test_admin_views.py | 4 ---- 4 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 16b44ec85..607bf5f61 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -10,7 +10,6 @@ from django.views.generic import RedirectView from registrar import views -# from registrar.views.admin_views import export_data from registrar.views.admin_views import ExportData diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index cde80fa58..30ea87266 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1354,8 +1354,8 @@ class Domain(TimeStampedModel, DomainHelper): """ logger.info("Changing to ready state") logger.info("able to transition to ready state") - # if self.first_ready_at is not None, this means that his - # domain wasr READY, then not READY, then is READY again. + # if self.first_ready_at is not None, this means that this + # domain was READY, then not READY, then is READY again. # We do not want to overwrite first_ready_at. if self.first_ready_at is None: self.first_ready_at = timezone.now() diff --git a/src/registrar/templates/admin/index.html b/src/registrar/templates/admin/index.html index a98a09696..ff251e53b 100644 --- a/src/registrar/templates/admin/index.html +++ b/src/registrar/templates/admin/index.html @@ -4,7 +4,7 @@
{% include "admin/app_list.html" with app_list=app_list show_changelinks=True %}
-

Welcome to the Custom Admin Homepage!

+

Domain growth report

{% comment %} Inputs of type date suck for accessibility. diff --git a/src/registrar/tests/test_admin_views.py b/src/registrar/tests/test_admin_views.py index b55fc2ddd..d31fcbcbb 100644 --- a/src/registrar/tests/test_admin_views.py +++ b/src/registrar/tests/test_admin_views.py @@ -17,8 +17,6 @@ class TestViews(TestCase): # Make a GET request to the admin index page response = self.client.get(admin_index_url) - print(f"response1 {response}") - # Assert that the response status code is 200 (OK) self.assertEqual(response.status_code, 200) @@ -33,8 +31,6 @@ class TestViews(TestCase): # Make a GET request to the export data page response = self.client.get(export_data_url) - print(response) - # Assert that the response status code is 200 (OK) or the expected status code self.assertEqual(response.status_code, 200) From a0260aa1ab5195f31616be22edd8ac4b29f143c7 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 27 Dec 2023 20:13:42 -0500 Subject: [PATCH 27/42] make the header for the new custom section on admin index bold --- src/registrar/assets/sass/_theme/_admin.scss | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 3afc81a35..39cefe75b 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -136,7 +136,8 @@ html[data-theme="dark"] { } #branding h1, -h1, h2, h3 { +h1, h2, h3, +.module h2 { font-weight: font-weight('bold'); } From 49aaa3fb5df98ee0b4593b5498761e5ceccd46ab Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 28 Dec 2023 17:56:13 -0500 Subject: [PATCH 28/42] Move expiration before creation date in Domain Admin --- 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 c3d55f0d1..94086e012 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -887,10 +887,10 @@ class DomainAdmin(ListHeaderAdmin): "name", "organization_type", "state", + "expiration_date", "created_at", "first_ready_at", "deleted_at", - "expiration_date", ] # this ordering effects the ordering of results From 0ce04007ffe8e34951b3634777560517f4a38bed Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 28 Dec 2023 18:16:59 -0500 Subject: [PATCH 29/42] resolve migration conflict --- ..._at.py => 0059_domain_deleted_at_domain_first_ready_at.py} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename src/registrar/migrations/{0058_domain_deleted_at_domain_first_ready_at.py => 0059_domain_deleted_at_domain_first_ready_at.py} (84%) diff --git a/src/registrar/migrations/0058_domain_deleted_at_domain_first_ready_at.py b/src/registrar/migrations/0059_domain_deleted_at_domain_first_ready_at.py similarity index 84% rename from src/registrar/migrations/0058_domain_deleted_at_domain_first_ready_at.py rename to src/registrar/migrations/0059_domain_deleted_at_domain_first_ready_at.py index c27d30be9..ebbd291a8 100644 --- a/src/registrar/migrations/0058_domain_deleted_at_domain_first_ready_at.py +++ b/src/registrar/migrations/0059_domain_deleted_at_domain_first_ready_at.py @@ -1,11 +1,11 @@ -# Generated by Django 4.2.7 on 2023-12-27 20:29 +# Generated by Django 4.2.7 on 2023-12-28 23:16 from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ("registrar", "0057_domainapplication_submission_date"), + ("registrar", "0058_alter_domaininformation_options"), ] operations = [ From 7087434d7601bc5be270fa37534bcb55cda20c9c Mon Sep 17 00:00:00 2001 From: CuriousX Date: Thu, 28 Dec 2023 22:32:25 -0700 Subject: [PATCH 30/42] Update src/registrar/tests/test_views.py Co-authored-by: zandercymatics <141044360+zandercymatics@users.noreply.github.com> --- src/registrar/tests/test_views.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 7301cc681..17a40d517 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -785,11 +785,6 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form = other_contacts_page.forms[0] - # DEBUG print statements - for f in other_contacts_form.fields: - if "submit" not in f: - print(f) - print(other_contacts_form[f].value) # Minimal check to ensure the form is loaded with data (if this part of # the application doesn't work, we should be equipped with other unit From ac3afe436cef15e4de7f3f2420f44ef262fba251 Mon Sep 17 00:00:00 2001 From: CuriousX Date: Thu, 28 Dec 2023 22:32:35 -0700 Subject: [PATCH 31/42] Update src/registrar/tests/test_views.py Co-authored-by: zandercymatics <141044360+zandercymatics@users.noreply.github.com> --- src/registrar/tests/test_views.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 17a40d517..7ae925b27 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -799,11 +799,6 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form["other_contacts-0-email"] = "" other_contacts_form["other_contacts-0-phone"] = "" - # DEBUG print statements - for f in other_contacts_form.fields: - if "submit" not in f: - print(f) - print(other_contacts_form[f].value) # Submit the now empty form result = other_contacts_form.submit() From affc35398d9d54c95729216476fd4d657c9677b5 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Fri, 29 Dec 2023 02:11:19 -0700 Subject: [PATCH 32/42] Fixed (with a bandaid) --- src/registrar/forms/application_wizard.py | 54 +++++++++++++++++++++-- src/registrar/tests/test_forms.py | 4 +- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index ac84a2f9f..b2abc155d 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -9,6 +9,7 @@ from django.core.validators import RegexValidator, MaxLengthValidator from django.utils.safestring import mark_safe from api.views import DOMAIN_API_MESSAGES +from registrar.management.commands.utility.terminal_helper import TerminalColors from registrar.models import Contact, DomainApplication, DraftDomain, Domain from registrar.templatetags.url_helpers import public_site_url @@ -262,7 +263,7 @@ class OrganizationContactForm(RegistrarForm): validators=[ RegexValidator( "^[0-9]{5}(?:-[0-9]{4})?$|^$", - message="Enter a zip code in the form of 12345 or 12345-6789.", + message="Enter a zip code in the required format, like 12345 or 12345-6789.", ) ], ) @@ -585,11 +586,52 @@ class OtherContactsForm(RegistrarForm): error_messages={"required": "Enter a phone number for this contact."}, ) + # Override clean in order to correct validation logic def clean(self): # NOTE: using self.cleaned_data directly apparently causes a CORS error cleaned = super().clean() - form_is_empty = all(v is None or v == "" for v in cleaned.values()) + + logger.info(f""" + {TerminalColors.MAGENTA}form data: + {TerminalColors.OKBLUE}{self.data} + + {TerminalColors.MAGENTA}form cleaned: + {TerminalColors.OKBLUE}{cleaned} + + + {self.data.items} + {TerminalColors.ENDC} + + """) + + # for f in self.fields: + # logger.info(f""" + # {TerminalColors.YELLOW}{f} + # {self.data.get(f)} + # {TerminalColors.ENDC} + # """) + + form_is_empty = all(v is None or v == "" for v in cleaned.values()) + + # NOTE: Phone number and email do NOT show up in cleaned values. + # I have spent hours tyring to figure out why, but have no idea... + # so for now we will grab their values from the raw data... + for i in self.data: + if 'phone' in i or 'email' in i: + field_value = self.data.get(i) + logger.info(f""" + {TerminalColors.YELLOW}{i} + {self.data.get(i)} + {TerminalColors.ENDC} + """) + form_is_empty = field_value == "" or field_value is None + logger.info(f""" + {TerminalColors.OKCYAN}empty? {form_is_empty} + {TerminalColors.ENDC} + """) + + if form_is_empty: # clear any errors raised by the form fields # (before this clean() method is run, each field @@ -599,8 +641,14 @@ class OtherContactsForm(RegistrarForm): # NOTE: we cannot just clear() the errors list. # That causes problems. for field in self.fields: - if field in self.errors: + if field in self.errors: # and field in cleaned + logger.info(f""" + {TerminalColors.FAIL}removing {field} + {TerminalColors.ENDC} + """) del self.errors[field] + + return cleaned diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index 00bb7ce61..9b6c3d6dd 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -216,7 +216,7 @@ class TestFormValidation(MockEppLib): def test_other_contact_email_invalid(self): """must be a valid email address.""" - form = OtherContactsForm(data={"email": "boss@boss"}) + form = OtherContactsForm(data={"email": "splendid@boss"}) self.assertEqual( form.errors["email"], ["Enter an email address in the required format, like name@example.com."], @@ -224,7 +224,7 @@ class TestFormValidation(MockEppLib): def test_other_contact_phone_invalid(self): """Must be a valid phone number.""" - form = OtherContactsForm(data={"phone": "boss@boss"}) + form = OtherContactsForm(data={"phone": "super@boss"}) self.assertTrue(form.errors["phone"][0].startswith("Enter a valid phone number ")) def test_requirements_form_blank(self): From 0fea0c2f94b4814727bb8df800357665fc080c86 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Fri, 29 Dec 2023 02:13:40 -0700 Subject: [PATCH 33/42] Remove print statements --- src/registrar/forms/application_wizard.py | 38 ++--------------------- 1 file changed, 3 insertions(+), 35 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index b2abc155d..b55fa62d6 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -591,27 +591,6 @@ class OtherContactsForm(RegistrarForm): def clean(self): # NOTE: using self.cleaned_data directly apparently causes a CORS error cleaned = super().clean() - - logger.info(f""" - {TerminalColors.MAGENTA}form data: - {TerminalColors.OKBLUE}{self.data} - - {TerminalColors.MAGENTA}form cleaned: - {TerminalColors.OKBLUE}{cleaned} - - - {self.data.items} - {TerminalColors.ENDC} - - """) - - # for f in self.fields: - # logger.info(f""" - # {TerminalColors.YELLOW}{f} - # {self.data.get(f)} - # {TerminalColors.ENDC} - # """) - form_is_empty = all(v is None or v == "" for v in cleaned.values()) # NOTE: Phone number and email do NOT show up in cleaned values. @@ -619,17 +598,10 @@ class OtherContactsForm(RegistrarForm): # so for now we will grab their values from the raw data... for i in self.data: if 'phone' in i or 'email' in i: + # check if it has data field_value = self.data.get(i) - logger.info(f""" - {TerminalColors.YELLOW}{i} - {self.data.get(i)} - {TerminalColors.ENDC} - """) + # update the bool on whether the form is actually empty form_is_empty = field_value == "" or field_value is None - logger.info(f""" - {TerminalColors.OKCYAN}empty? {form_is_empty} - {TerminalColors.ENDC} - """) if form_is_empty: @@ -641,11 +613,7 @@ class OtherContactsForm(RegistrarForm): # NOTE: we cannot just clear() the errors list. # That causes problems. for field in self.fields: - if field in self.errors: # and field in cleaned - logger.info(f""" - {TerminalColors.FAIL}removing {field} - {TerminalColors.ENDC} - """) + if field in self.errors: del self.errors[field] From ea043bf5045484852f1635a3852505054115eeec Mon Sep 17 00:00:00 2001 From: CocoByte Date: Fri, 29 Dec 2023 02:16:39 -0700 Subject: [PATCH 34/42] linted --- src/registrar/forms/application_wizard.py | 10 +++------- src/registrar/tests/test_views.py | 2 -- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index b55fa62d6..7a79d0677 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -9,7 +9,6 @@ from django.core.validators import RegexValidator, MaxLengthValidator from django.utils.safestring import mark_safe from api.views import DOMAIN_API_MESSAGES -from registrar.management.commands.utility.terminal_helper import TerminalColors from registrar.models import Contact, DomainApplication, DraftDomain, Domain from registrar.templatetags.url_helpers import public_site_url @@ -586,24 +585,22 @@ class OtherContactsForm(RegistrarForm): error_messages={"required": "Enter a phone number for this contact."}, ) - # Override clean in order to correct validation logic def clean(self): # NOTE: using self.cleaned_data directly apparently causes a CORS error cleaned = super().clean() - form_is_empty = all(v is None or v == "" for v in cleaned.values()) - + form_is_empty = all(v is None or v == "" for v in cleaned.values()) + # NOTE: Phone number and email do NOT show up in cleaned values. # I have spent hours tyring to figure out why, but have no idea... # so for now we will grab their values from the raw data... for i in self.data: - if 'phone' in i or 'email' in i: + if "phone" in i or "email" in i: # check if it has data field_value = self.data.get(i) # update the bool on whether the form is actually empty form_is_empty = field_value == "" or field_value is None - if form_is_empty: # clear any errors raised by the form fields # (before this clean() method is run, each field @@ -616,7 +613,6 @@ class OtherContactsForm(RegistrarForm): if field in self.errors: del self.errors[field] - return cleaned diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 7ae925b27..c465373dd 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -785,7 +785,6 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form = other_contacts_page.forms[0] - # Minimal check to ensure the form is loaded with data (if this part of # the application doesn't work, we should be equipped with other unit # tests to flag it) @@ -799,7 +798,6 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form["other_contacts-0-email"] = "" other_contacts_form["other_contacts-0-phone"] = "" - # Submit the now empty form result = other_contacts_form.submit() self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) From 72edec836292fb811354b5a09889771e05bb46e3 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 29 Dec 2023 12:05:49 -0500 Subject: [PATCH 35/42] change first_ready_at and deleted_at to first_ready and deleted --- src/registrar/admin.py | 6 ++-- ...0059_domain_deleted_domain_first_ready.py} | 4 +-- src/registrar/models/domain.py | 14 ++++---- src/registrar/tests/test_models_domain.py | 36 +++++++++---------- src/registrar/tests/test_reports.py | 32 ++++++++--------- src/registrar/utility/csv_export.py | 22 ++++++------ 6 files changed, 57 insertions(+), 57 deletions(-) rename src/registrar/migrations/{0059_domain_deleted_at_domain_first_ready_at.py => 0059_domain_deleted_domain_first_ready.py} (90%) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ecf4aa522..c0d21f60e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -985,8 +985,8 @@ class DomainAdmin(ListHeaderAdmin): "state", "expiration_date", "created_at", - "first_ready_at", - "deleted_at", + "first_ready", + "deleted", ] # this ordering effects the ordering of results @@ -1005,7 +1005,7 @@ class DomainAdmin(ListHeaderAdmin): search_help_text = "Search by domain name." change_form_template = "django/admin/domain_change_form.html" change_list_template = "django/admin/domain_change_list.html" - readonly_fields = ["state", "expiration_date", "first_ready_at", "deleted_at"] + readonly_fields = ["state", "expiration_date", "first_ready", "deleted"] # Table ordering ordering = ["name"] diff --git a/src/registrar/migrations/0059_domain_deleted_at_domain_first_ready_at.py b/src/registrar/migrations/0059_domain_deleted_domain_first_ready.py similarity index 90% rename from src/registrar/migrations/0059_domain_deleted_at_domain_first_ready_at.py rename to src/registrar/migrations/0059_domain_deleted_domain_first_ready.py index ebbd291a8..f061c3669 100644 --- a/src/registrar/migrations/0059_domain_deleted_at_domain_first_ready_at.py +++ b/src/registrar/migrations/0059_domain_deleted_domain_first_ready.py @@ -11,12 +11,12 @@ class Migration(migrations.Migration): operations = [ migrations.AddField( model_name="domain", - name="deleted_at", + name="deleted", field=models.DateField(editable=False, help_text="Deleted at date", null=True), ), migrations.AddField( model_name="domain", - name="first_ready_at", + name="first_ready", field=models.DateField( editable=False, help_text="The last time this domain moved into the READY state", null=True ), diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 30ea87266..a32f8d386 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -969,13 +969,13 @@ class Domain(TimeStampedModel, DomainHelper): help_text=("Duplication of registry's expiration date saved for ease of reporting"), ) - deleted_at = DateField( + deleted = DateField( null=True, editable=False, help_text="Deleted at date", ) - first_ready_at = DateField( + first_ready = DateField( null=True, editable=False, help_text="The last time this domain moved into the READY state", @@ -1310,7 +1310,7 @@ class Domain(TimeStampedModel, DomainHelper): try: logger.info("deletedInEpp()-> inside _delete_domain") self._delete_domain() - self.deleted_at = timezone.now() + self.deleted = timezone.now() except RegistryError as err: logger.error(f"Could not delete domain. Registry returned error: {err}") raise err @@ -1354,11 +1354,11 @@ class Domain(TimeStampedModel, DomainHelper): """ logger.info("Changing to ready state") logger.info("able to transition to ready state") - # if self.first_ready_at is not None, this means that this + # if self.first_ready is not None, this means that this # domain was READY, then not READY, then is READY again. - # We do not want to overwrite first_ready_at. - if self.first_ready_at is None: - self.first_ready_at = timezone.now() + # We do not want to overwrite first_ready. + if self.first_ready is None: + self.first_ready = timezone.now() @transition( field="state", diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 01df1a6bd..3eb372d6c 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -385,22 +385,22 @@ class TestDomainStatuses(MockEppLib): """Domain 'revert_client_hold' method causes the registry to change statuses""" raise - def test_first_ready_at(self): + def test_first_ready(self): """ - first_ready_at is set when a domain is first transitioned to READY. It does not get overwritten + first_ready is set when a domain is first transitioned to READY. It does not get overwritten in case the domain gets out of and back into READY. """ domain, _ = Domain.objects.get_or_create(name="pig-knuckles.gov", state=Domain.State.DNS_NEEDED) - self.assertEqual(domain.first_ready_at, None) + self.assertEqual(domain.first_ready, None) domain.ready() # check that status is READY self.assertTrue(domain.is_active()) - self.assertNotEqual(domain.first_ready_at, None) + self.assertNotEqual(domain.first_ready, None) - # Capture the value of first_ready_at - first_ready_at = domain.first_ready_at + # Capture the value of first_ready + first_ready = domain.first_ready # change domain status domain.dns_needed() @@ -410,8 +410,8 @@ class TestDomainStatuses(MockEppLib): domain.ready() self.assertTrue(domain.is_active()) - # assert that the value of first_ready_at has not changed - self.assertEqual(domain.first_ready_at, first_ready_at) + # assert that the value of first_ready has not changed + self.assertEqual(domain.first_ready, first_ready) def tearDown(self) -> None: PublicContact.objects.all().delete() @@ -1139,7 +1139,7 @@ class TestRegistrantNameservers(MockEppLib): Then `commands.CreateHost` and `commands.UpdateDomain` is sent to the registry And `domain.is_active` returns False - And domain.first_ready_at is null + And domain.first_ready is null """ # set 1 nameserver @@ -1166,7 +1166,7 @@ class TestRegistrantNameservers(MockEppLib): # as you have less than 2 nameservers self.assertFalse(self.domain.is_active()) - self.assertEqual(self.domain.first_ready_at, None) + self.assertEqual(self.domain.first_ready, None) def test_user_adds_two_nameservers(self): """ @@ -1176,7 +1176,7 @@ class TestRegistrantNameservers(MockEppLib): Then `commands.CreateHost` and `commands.UpdateDomain` is sent to the registry And `domain.is_active` returns True - And domain.first_ready_at is not null + And domain.first_ready is not null """ # set 2 nameservers @@ -1207,7 +1207,7 @@ class TestRegistrantNameservers(MockEppLib): self.assertEqual(4, self.mockedSendFunction.call_count) # check that status is READY self.assertTrue(self.domain.is_active()) - self.assertNotEqual(self.domain.first_ready_at, None) + self.assertNotEqual(self.domain.first_ready, None) def test_user_adds_too_many_nameservers(self): """ @@ -2331,7 +2331,7 @@ class TestAnalystDelete(MockEppLib): Then `commands.DeleteDomain` is sent to the registry And `state` is set to `DELETED` - The deleted_at date is set. + The deleted date is set. """ # Put the domain in client hold self.domain.place_client_hold() @@ -2353,8 +2353,8 @@ class TestAnalystDelete(MockEppLib): # Domain should have the right state self.assertEqual(self.domain.state, Domain.State.DELETED) - # Domain should have a deleted_at - self.assertNotEqual(self.domain.deleted_at, None) + # Domain should have a deleted + self.assertNotEqual(self.domain.deleted, None) # Cache should be invalidated self.assertEqual(self.domain._cache, {}) @@ -2399,7 +2399,7 @@ class TestAnalystDelete(MockEppLib): Then an FSM error is returned And `state` is not set to `DELETED` - The deleted_at date is still null. + The deleted date is still null. """ self.assertEqual(self.domain.state, Domain.State.READY) with self.assertRaises(TransitionNotAllowed) as err: @@ -2411,5 +2411,5 @@ class TestAnalystDelete(MockEppLib): # Domain should have the right state self.assertEqual(self.domain.state, Domain.State.READY) - # deleted_at should be null - self.assertEqual(self.domain.deleted_at, None) + # deleted should be null + self.assertEqual(self.domain.deleted, None) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index d1bba9e46..85e24ce33 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -232,23 +232,23 @@ class ExportDataTest(TestCase): ) self.domain_1, _ = Domain.objects.get_or_create( - name="cdomain1.gov", state=Domain.State.READY, first_ready_at=timezone.now() + name="cdomain1.gov", state=Domain.State.READY, first_ready=timezone.now() ) self.domain_2, _ = Domain.objects.get_or_create(name="adomain2.gov", state=Domain.State.DNS_NEEDED) self.domain_3, _ = Domain.objects.get_or_create(name="ddomain3.gov", state=Domain.State.ON_HOLD) self.domain_4, _ = Domain.objects.get_or_create(name="bdomain4.gov", state=Domain.State.UNKNOWN) self.domain_4, _ = Domain.objects.get_or_create(name="bdomain4.gov", state=Domain.State.UNKNOWN) self.domain_5, _ = Domain.objects.get_or_create( - name="bdomain5.gov", state=Domain.State.DELETED, deleted_at=timezone.make_aware(datetime(2023, 11, 1)) + name="bdomain5.gov", state=Domain.State.DELETED, deleted=timezone.make_aware(datetime(2023, 11, 1)) ) self.domain_6, _ = Domain.objects.get_or_create( - name="bdomain6.gov", state=Domain.State.DELETED, deleted_at=timezone.make_aware(datetime(1980, 10, 16)) + name="bdomain6.gov", state=Domain.State.DELETED, deleted=timezone.make_aware(datetime(1980, 10, 16)) ) self.domain_7, _ = Domain.objects.get_or_create( - name="xdomain7.gov", state=Domain.State.DELETED, deleted_at=timezone.now() + name="xdomain7.gov", state=Domain.State.DELETED, deleted=timezone.now() ) self.domain_8, _ = Domain.objects.get_or_create( - name="sdomain8.gov", state=Domain.State.DELETED, deleted_at=timezone.now() + name="sdomain8.gov", state=Domain.State.DELETED, deleted=timezone.now() ) # We use timezone.make_aware to sync to server time a datetime object with the current date (using date.today()) # and a specific time (using datetime.min.time()). @@ -256,13 +256,13 @@ class ExportDataTest(TestCase): self.domain_9, _ = Domain.objects.get_or_create( name="zdomain9.gov", state=Domain.State.DELETED, - deleted_at=timezone.make_aware(datetime.combine(date.today() - timedelta(days=1), datetime.min.time())), + deleted=timezone.make_aware(datetime.combine(date.today() - timedelta(days=1), datetime.min.time())), ) # ready tomorrow self.domain_10, _ = Domain.objects.get_or_create( name="adomain10.gov", state=Domain.State.READY, - first_ready_at=timezone.make_aware(datetime.combine(date.today() + timedelta(days=1), datetime.min.time())), + first_ready=timezone.make_aware(datetime.combine(date.today() + timedelta(days=1), datetime.min.time())), ) self.domain_information_1, _ = DomainInformation.objects.get_or_create( @@ -451,10 +451,10 @@ class ExportDataTest(TestCase): def test_write_body_with_date_filter_pulls_domains_in_range(self): """Test that domains that are - 1. READY and their first_ready_at dates are in range - 2. DELETED and their deleted_at dates are in range + 1. READY and their first_ready dates are in range + 2. DELETED and their deleted dates are in range are pulled when the growth report conditions are applied to export_domains_to_writed. - Test that ready domains are sorted by first_ready_at/deleted_at dates first, names second. + Test that ready domains are sorted by first_ready/deleted dates first, names second. We considered testing export_data_growth_to_csv which calls write_body and would have been easy to set up, but expected_content would contain created_at dates @@ -486,22 +486,22 @@ class ExportDataTest(TestCase): "domain__name", ] sort_fields_for_deleted_domains = [ - "domain__deleted_at", + "domain__deleted", "domain__name", ] filter_condition = { "domain__state__in": [ Domain.State.READY, ], - "domain__first_ready_at__lte": end_date, - "domain__first_ready_at__gte": start_date, + "domain__first_ready__lte": end_date, + "domain__first_ready__gte": start_date, } filter_conditions_for_deleted_domains = { "domain__state__in": [ Domain.State.DELETED, ], - "domain__deleted_at__lte": end_date, - "domain__deleted_at__gte": start_date, + "domain__deleted__lte": end_date, + "domain__deleted__gte": start_date, } # Call the export functions @@ -526,7 +526,7 @@ class ExportDataTest(TestCase): csv_content = csv_file.read() # We expect READY domains first, created between today-2 and today+2, sorted by created_at then name - # and DELETED domains deleted between today-2 and today+2, sorted by deleted_at then name + # and DELETED domains deleted between today-2 and today+2, sorted by deleted then name expected_content = ( "Domain name,Domain type,Agency,Organization name,City," "State,Status,Expiration date\n" diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 0ed943613..4c46ee3a3 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -48,8 +48,8 @@ def write_row(writer, columns, domain_info): "Status": domain_info.domain.get_state_display(), "Expiration date": domain_info.domain.expiration_date, "Created at": domain_info.domain.created_at, - "First ready at": domain_info.domain.first_ready_at, - "Deleted at": domain_info.domain.deleted_at, + "First ready": domain_info.domain.first_ready, + "Deleted": domain_info.domain.deleted, } writer.writerow([FIELDS.get(column, "") for column in columns]) @@ -214,30 +214,30 @@ def export_data_growth_to_csv(csv_file, start_date, end_date): "City", "State", "Status", - "Created at", - "First ready at", - "Deleted at", "Expiration date", + "Created at", + "First ready", + "Deleted", ] sort_fields = [ - "domain__first_ready_at", + "domain__first_ready", "domain__name", ] filter_condition = { "domain__state__in": [Domain.State.READY], - "domain__first_ready_at__lte": end_date_formatted, - "domain__first_ready_at__gte": start_date_formatted, + "domain__first_ready__lte": end_date_formatted, + "domain__first_ready__gte": start_date_formatted, } # We also want domains deleted between sar and end dates, sorted sort_fields_for_deleted_domains = [ - "domain__deleted_at", + "domain__deleted", "domain__name", ] filter_condition_for_deleted_domains = { "domain__state__in": [Domain.State.DELETED], - "domain__deleted_at__lte": end_date_formatted, - "domain__deleted_at__gte": start_date_formatted, + "domain__deleted__lte": end_date_formatted, + "domain__deleted__gte": start_date_formatted, } write_header(writer, columns) From 0ac1989a04a587cb1b608551542e219b71a6223e Mon Sep 17 00:00:00 2001 From: CocoByte Date: Fri, 29 Dec 2023 11:41:36 -0700 Subject: [PATCH 36/42] Clarification in comments for Unit Test bandaid --- src/registrar/forms/application_wizard.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 7a79d0677..4da0c9700 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -591,7 +591,9 @@ class OtherContactsForm(RegistrarForm): cleaned = super().clean() form_is_empty = all(v is None or v == "" for v in cleaned.values()) - # NOTE: Phone number and email do NOT show up in cleaned values. + # ==== UNIT TEST BANDAID ==== + # NOTE: Phone number and email do NOT show up in cleaned values + # This is for UNIT TESTS only. The page itself loads cleaned data just fine. # I have spent hours tyring to figure out why, but have no idea... # so for now we will grab their values from the raw data... for i in self.data: From 06a2e39c123348523329fe012437ee0e5333260b Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 29 Dec 2023 15:37:31 -0500 Subject: [PATCH 37/42] updated clean in form --- src/registrar/forms/application_wizard.py | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 7a79d0677..06cbe81ca 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -587,19 +587,12 @@ class OtherContactsForm(RegistrarForm): # Override clean in order to correct validation logic def clean(self): - # NOTE: using self.cleaned_data directly apparently causes a CORS error - cleaned = super().clean() - form_is_empty = all(v is None or v == "" for v in cleaned.values()) - # NOTE: Phone number and email do NOT show up in cleaned values. - # I have spent hours tyring to figure out why, but have no idea... - # so for now we will grab their values from the raw data... - for i in self.data: - if "phone" in i or "email" in i: - # check if it has data - field_value = self.data.get(i) - # update the bool on whether the form is actually empty - form_is_empty = field_value == "" or field_value is None + form_is_empty = True + for name, field in self.fields.items(): + value = field.widget.value_from_datadict(self.data, self.files, self.add_prefix(name)) + if value is not None and value != "": + form_is_empty = False if form_is_empty: # clear any errors raised by the form fields @@ -613,7 +606,7 @@ class OtherContactsForm(RegistrarForm): if field in self.errors: del self.errors[field] - return cleaned + return self.cleaned_data class BaseOtherContactsFormSet(RegistrarFormSet): From e2cb8ee2d8e20f2999b788d25d5a0622a0fecfc3 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 29 Dec 2023 15:45:12 -0500 Subject: [PATCH 38/42] added comments to clean --- src/registrar/forms/application_wizard.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 06cbe81ca..39829d2b2 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -585,12 +585,21 @@ class OtherContactsForm(RegistrarForm): error_messages={"required": "Enter a phone number for this contact."}, ) - # Override clean in order to correct validation logic def clean(self): + """ + This method overrides the default behavior for forms. + This cleans the form after field validation has already taken place. + In this override, allow for a form which is empty to be considered + valid even though certain required fields have not passed field + validation + """ + # Set form_is_empty to True initially form_is_empty = True for name, field in self.fields.items(): + # get the value of the field from the widget value = field.widget.value_from_datadict(self.data, self.files, self.add_prefix(name)) + # if any field in the submitted form is not empty, set form_is_empty to False if value is not None and value != "": form_is_empty = False From 573083719c53c09387983dc95c4e58dfa42b158f Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 29 Dec 2023 16:58:33 -0500 Subject: [PATCH 39/42] tweak report name and ui --- src/registrar/assets/sass/_theme/_admin.scss | 6 ++++++ src/registrar/templates/admin/index.html | 5 +++-- src/registrar/views/admin_views.py | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 39cefe75b..a3d631243 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -141,6 +141,12 @@ h1, h2, h3, font-weight: font-weight('bold'); } +.module h3 { + padding: 0; + color: var(--primary); + margin: units(2) 0 units(1) 0; +} + .change-list { .usa-table--striped tbody tr:nth-child(odd) td, .usa-table--striped tbody tr:nth-child(odd) th, diff --git a/src/registrar/templates/admin/index.html b/src/registrar/templates/admin/index.html index ff251e53b..04601ef32 100644 --- a/src/registrar/templates/admin/index.html +++ b/src/registrar/templates/admin/index.html @@ -4,7 +4,8 @@
{% include "admin/app_list.html" with app_list=app_list show_changelinks=True %}
-

Domain growth report

+

Reports

+

Domain growth report

{% comment %} Inputs of type date suck for accessibility. @@ -14,7 +15,7 @@ See the commit "Review for ticket #999" {% endcomment %} -
+
diff --git a/src/registrar/views/admin_views.py b/src/registrar/views/admin_views.py index ddccc3a50..f7164663b 100644 --- a/src/registrar/views/admin_views.py +++ b/src/registrar/views/admin_views.py @@ -18,7 +18,7 @@ class ExportData(View): end_date = request.GET.get("end_date", "") response = HttpResponse(content_type="text/csv") - response["Content-Disposition"] = f'attachment; filename="growth-from-{start_date}-to-{end_date}.csv"' + response["Content-Disposition"] = f'attachment; filename="domain-growth-report-{start_date}-to-{end_date}.csv"' # For #999: set export_data_growth_to_csv to return the resulting queryset, which we can then use # in context to display this data in the template. csv_export.export_data_growth_to_csv(response, start_date, end_date) From 5df9c0f83dd08ec7081a3b1a091e8012a1882f63 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 29 Dec 2023 15:24:50 -0700 Subject: [PATCH 40/42] Run black linter --- src/registrar/forms/application_wizard.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 39829d2b2..59c3e25c7 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -590,7 +590,7 @@ class OtherContactsForm(RegistrarForm): This method overrides the default behavior for forms. This cleans the form after field validation has already taken place. In this override, allow for a form which is empty to be considered - valid even though certain required fields have not passed field + valid even though certain required fields have not passed field validation """ From 6819d934475728e25ee1604f12e82c27d05b6a60 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 29 Dec 2023 17:45:54 -0500 Subject: [PATCH 41/42] fix migrations --- ...rst_ready.py => 0060_domain_deleted_domain_first_ready.py} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename src/registrar/migrations/{0059_domain_deleted_domain_first_ready.py => 0060_domain_deleted_domain_first_ready.py} (84%) diff --git a/src/registrar/migrations/0059_domain_deleted_domain_first_ready.py b/src/registrar/migrations/0060_domain_deleted_domain_first_ready.py similarity index 84% rename from src/registrar/migrations/0059_domain_deleted_domain_first_ready.py rename to src/registrar/migrations/0060_domain_deleted_domain_first_ready.py index f061c3669..e4caa1525 100644 --- a/src/registrar/migrations/0059_domain_deleted_domain_first_ready.py +++ b/src/registrar/migrations/0060_domain_deleted_domain_first_ready.py @@ -1,11 +1,11 @@ -# Generated by Django 4.2.7 on 2023-12-28 23:16 +# Generated by Django 4.2.7 on 2023-12-29 22:44 from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ("registrar", "0058_alter_domaininformation_options"), + ("registrar", "0059_delete_nameserver"), ] operations = [ From 530fa9ec8be815bc047f20adc74867ecdab1a749 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 29 Dec 2023 23:18:29 -0500 Subject: [PATCH 42/42] fix view test after changing export file name --- src/registrar/tests/test_admin_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_admin_views.py b/src/registrar/tests/test_admin_views.py index d31fcbcbb..aa150d55c 100644 --- a/src/registrar/tests/test_admin_views.py +++ b/src/registrar/tests/test_admin_views.py @@ -38,5 +38,5 @@ class TestViews(TestCase): self.assertEqual(response["Content-Type"], "text/csv") # Check if the filename in the Content-Disposition header matches the expected pattern - expected_filename = f"growth-from-{start_date}-to-{end_date}.csv" + expected_filename = f"domain-growth-report-{start_date}-to-{end_date}.csv" self.assertIn(f'attachment; filename="{expected_filename}"', response["Content-Disposition"])