From 2a64230c9181c634dda8cc8f4758fa9cb05cd3cf Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 5 Jan 2024 11:30:52 -0500 Subject: [PATCH 001/100] getting started --- 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 5040649b6..65d19eae5 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -102,7 +102,7 @@ class RegistrarFormSet(forms.BaseFormSet): # when determining if related objects exist. threshold is 0 for most # relationships. if the relationship is related_name, we know that # there is already exactly 1 acceptable relationship (the one we are - # attempting to delete), so the threshold is 1 + # attempting to delete), so the threshold is 1 threshold = 1 if rel == related_name else 0 # Raise a KeyError if rel is not a defined field on the db_obj model From 13151c93686c25c985968af8ecdda59a4b2dec1c Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 5 Jan 2024 12:22:15 -0500 Subject: [PATCH 002/100] Fix the Add functionality on the Other Contacts formset --- src/registrar/assets/js/get-gov.js | 14 ++++++++++---- src/registrar/forms/application_wizard.py | 2 +- .../templates/application_other_contacts.html | 4 ++-- src/registrar/views/application.py | 4 ++++ 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 11ba49aa9..086ca5b91 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -331,10 +331,10 @@ function prepareDeleteButtons(formLabel) { * it everywhere. */ (function prepareFormsetsForms() { + let formIdentifier = "form" let repeatableForm = document.querySelectorAll(".repeatable-form"); let container = document.querySelector("#form-container"); let addButton = document.querySelector("#add-form"); - let totalForms = document.querySelector("#id_form-TOTAL_FORMS"); let cloneIndex = 0; let formLabel = ''; let isNameserversForm = document.title.includes("DNS name servers |"); @@ -343,7 +343,12 @@ function prepareDeleteButtons(formLabel) { formLabel = "Name server"; } else if ((document.title.includes("DS Data |")) || (document.title.includes("Key Data |"))) { formLabel = "DS Data record"; + } else if (document.title.includes("Other employees from your organization")) { + formLabel = "Organization contact"; + container = document.querySelector("#other-employees"); + formIdentifier = "other_contacts" } + let totalForms = document.querySelector(`#id_${formIdentifier}-TOTAL_FORMS`); // On load: Disable the add more button if we have 13 forms if (isNameserversForm && document.querySelectorAll(".repeatable-form").length == 13) { @@ -360,7 +365,7 @@ function prepareDeleteButtons(formLabel) { let forms = document.querySelectorAll(".repeatable-form"); let formNum = forms.length; let newForm = repeatableForm[cloneIndex].cloneNode(true); - let formNumberRegex = RegExp(`form-(\\d){1}-`,'g'); + let formNumberRegex = RegExp(`${formIdentifier}-(\\d){1}-`,'g'); let formLabelRegex = RegExp(`${formLabel} (\\d){1}`, 'g'); // For the eample on Nameservers let formExampleRegex = RegExp(`ns(\\d){1}`, 'g'); @@ -393,7 +398,8 @@ function prepareDeleteButtons(formLabel) { } formNum++; - newForm.innerHTML = newForm.innerHTML.replace(formNumberRegex, `form-${formNum-1}-`); + + newForm.innerHTML = newForm.innerHTML.replace(formNumberRegex, `${formIdentifier}-${formNum-1}-`); newForm.innerHTML = newForm.innerHTML.replace(formLabelRegex, `${formLabel} ${formNum}`); newForm.innerHTML = newForm.innerHTML.replace(formExampleRegex, `ns${formNum}`); container.insertBefore(newForm, addButton); @@ -402,7 +408,7 @@ function prepareDeleteButtons(formLabel) { // Reset the values of each input to blank inputs.forEach((input) => { input.classList.remove("usa-input--error"); - if (input.type === "text" || input.type === "number" || input.type === "password") { + if (input.type === "text" || input.type === "number" || input.type === "password" || input.type === "email" || input.type === "tel") { input.value = ""; // Set the value to an empty string } else if (input.type === "checkbox" || input.type === "radio") { diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 65d19eae5..ae7590c53 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -734,7 +734,7 @@ class BaseOtherContactsFormSet(RegistrarFormSet): OtherContactsFormSet = forms.formset_factory( OtherContactsForm, - extra=1, + extra=0, absolute_max=1500, # django default; use `max_num` to limit entries min_num=1, validate_min=True, diff --git a/src/registrar/templates/application_other_contacts.html b/src/registrar/templates/application_other_contacts.html index bee307dde..e71039e69 100644 --- a/src/registrar/templates/application_other_contacts.html +++ b/src/registrar/templates/application_other_contacts.html @@ -34,7 +34,7 @@ {{ forms.1.management_form }} {# forms.1 is a formset and this iterates over its forms #} {% for form in forms.1.forms %} -
+

Organization contact {{ forloop.counter }} (optional)

 @@ -62,7 +62,7 @@
{% endfor %} - + + + {% if forms.1.can_delete %} {{ form.DELETE }} From e90474cab1a0fb1303b7c21fe7adc40df47f02a8 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 8 Jan 2024 06:33:26 -0500 Subject: [PATCH 007/100] remove errors from being returned if forms are deleted, js to hide deleted forms on page load --- src/registrar/assets/js/get-gov.js | 22 +++++++++++++++++++++- src/registrar/forms/application_wizard.py | 21 +++++++++++++++++---- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 85b427901..8f598b3f1 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -463,9 +463,26 @@ function prepareDeleteButtons(formLabel) { } }); - +} +/** + * On form load, hide deleted forms, ie. those forms with hidden input of class 'deletion' + * with value='on' + */ +function hideDeletedForms() { + let hiddenDeleteButtonsWithValueOn = document.querySelectorAll('input[type="hidden"].deletion[value="on"]'); + + // Iterating over the NodeList of hidden inputs + hiddenDeleteButtonsWithValueOn.forEach(function(hiddenInput) { + // Finding the closest parent element with class "repeatable-form" for each hidden input + var repeatableFormToHide = hiddenInput.closest('.repeatable-form'); + // Checking if a matching parent element is found for each hidden input + if (repeatableFormToHide) { + // Setting the display property to "none" for each matching parent element + repeatableFormToHide.style.display = 'none'; + } + }); } /** @@ -500,6 +517,9 @@ function prepareDeleteButtons(formLabel) { addButton.setAttribute("disabled", "true"); } + // Hide forms which have previously been deleted + hideDeletedForms() + // Attach click event listener on the delete buttons of the existing forms prepareDeleteButtons(formLabel); diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 85f69f0f3..63ad48a95 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -780,12 +780,12 @@ 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 - validation + In this override, allow for a form which is empty, or one marked for + deletion to be considered valid even though certain required fields have + not passed field validation """ - if self.form_data_marked_for_deletion: + if self.form_data_marked_for_deletion or self.cleaned_data["DELETE"]: # clear any errors raised by the form fields # (before this clean() method is run, each field # performs its own clean, which could result in @@ -810,6 +810,19 @@ class OtherContactsForm(RegistrarForm): class BaseOtherContactsFormSet(RegistrarFormSet): + """ + FormSet for Other Contacts + + There are two conditions by which a form in the formset can be marked for deletion. + One is if the user clicks 'DELETE' button, and this is submitted in the form. The + other is if the YesNo form, which is submitted with this formset, is set to No; in + this case, all forms in formset are marked for deletion. Both of these conditions + must co-exist. + Also, other_contacts have db relationships to multiple db objects. When attempting + to delete an other_contact from an application, those db relationships must be + tested and handled; this is configured with REVERSE_JOINS, which is an array of + strings representing the relationships between contact model and other models. + """ JOIN = "other_contacts" REVERSE_JOINS = [ "user", From b00149f2aec9d738d53bd3f71295c54ec308a0da Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 8 Jan 2024 12:08:49 -0500 Subject: [PATCH 008/100] non_form_errors raised to the template and displayed to users --- src/registrar/forms/application_wizard.py | 31 +++++++++++++++++-- src/registrar/templates/application_form.html | 1 + .../templates/includes/non_form_errors.html | 9 ++++++ src/registrar/views/application.py | 5 +++ 4 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 src/registrar/templates/includes/non_form_errors.html diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 63ad48a95..ec86a656f 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -188,6 +188,7 @@ class RegistrarFormSet(forms.BaseFormSet): """Return the number of forms that are required in this FormSet.""" logger.info("in initial_form_count") if self.is_bound: + logger.info(f"initial form count = {self.management_form.cleaned_data[INITIAL_FORM_COUNT]}") return self.management_form.cleaned_data[INITIAL_FORM_COUNT] else: # Use the length of the initial data if it's there, 0 otherwise. @@ -785,6 +786,15 @@ class OtherContactsForm(RegistrarForm): 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 + if self.form_data_marked_for_deletion or self.cleaned_data["DELETE"]: # clear any errors raised by the form fields # (before this clean() method is run, each field @@ -795,6 +805,7 @@ class OtherContactsForm(RegistrarForm): # That causes problems. for field in self.fields: if field in self.errors: + logger.info(f"deleting error {self.errors[field]}") del self.errors[field] # return empty object with only 'delete' attribute defined. # this will prevent _to_database from creating an empty @@ -848,9 +859,25 @@ class BaseOtherContactsFormSet(RegistrarFormSet): self.forms[index].use_required_attribute = True def should_delete(self, cleaned): - empty = (isinstance(v, str) and (v.strip() == "" or v is None) for v in cleaned.values()) - return all(empty) or self.formset_data_marked_for_deletion or cleaned.get("DELETE", False) + # empty = (isinstance(v, str) and (v.strip() == "" or v is None) for v in cleaned.values()) + # empty forms should throw errors + return self.formset_data_marked_for_deletion or cleaned.get("DELETE", False) + def non_form_errors(self): + """ + Method to override non_form_errors. + If minimum number of contacts is not submitted, customize the error message + that is returned.""" + # Get the default non_form_errors + errors = super().non_form_errors() + + # Check if the default error message is present + if 'Please submit at least 1 form.' in errors: + # Replace the default message with the custom message + errors = ['Please submit at least 1 contact.'] + + return errors + def pre_create(self, db_obj, cleaned): """Code to run before an item in the formset is created in the database.""" # remove DELETE from cleaned diff --git a/src/registrar/templates/application_form.html b/src/registrar/templates/application_form.html index c34ddf5bc..4d00076cb 100644 --- a/src/registrar/templates/application_form.html +++ b/src/registrar/templates/application_form.html @@ -43,6 +43,7 @@ {% for inner in outer.forms %} {% include "includes/form_errors.html" with form=inner %} {% endfor %} + {% include "includes/non_form_errors.html" with form=outer %} {% else %} {% include "includes/form_errors.html" with form=outer %} {% endif %} diff --git a/src/registrar/templates/includes/non_form_errors.html b/src/registrar/templates/includes/non_form_errors.html new file mode 100644 index 000000000..5c33904a3 --- /dev/null +++ b/src/registrar/templates/includes/non_form_errors.html @@ -0,0 +1,9 @@ +{% if form.errors %} + {% for error in form.non_form_errors %} +
+
+ {{ error|escape }} +
+
+ {% endfor %} +{% endif %} \ No newline at end of file diff --git a/src/registrar/views/application.py b/src/registrar/views/application.py index 1525021e4..82836f7af 100644 --- a/src/registrar/views/application.py +++ b/src/registrar/views/application.py @@ -493,6 +493,11 @@ class OtherContacts(ApplicationWizard): template_name = "application_other_contacts.html" forms = [forms.OtherContactsYesNoForm, forms.OtherContactsFormSet, forms.NoOtherContactsForm] + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + logger.info(context) + return context + def is_valid(self, forms: list) -> bool: """Overrides default behavior defined in ApplicationWizard. Depending on value in other_contacts_yes_no_form, marks forms in From 7c6e8c891aa80bb19a01e4877ec8c2f1547a1f65 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 8 Jan 2024 14:35:21 -0500 Subject: [PATCH 009/100] added debugging (to be removed later), fixed required attributes not showing on new forms in formset --- src/registrar/forms/application_wizard.py | 92 ++++++++++++++++++++--- src/registrar/views/application.py | 7 ++ 2 files changed, 89 insertions(+), 10 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index ec86a656f..b50a7b179 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -1,10 +1,12 @@ from __future__ import annotations # allows forward references in annotations from itertools import zip_longest import logging +import copy from typing import Callable from phonenumber_field.formfields import PhoneNumberField # type: ignore from django import forms +from django.forms.utils import ErrorDict from django.core.validators import RegexValidator, MaxLengthValidator from django.utils.safestring import mark_safe from django.db.models.fields.related import ForeignObjectRel, OneToOneField @@ -75,6 +77,7 @@ class RegistrarFormSet(forms.BaseFormSet): # if you opt to fill it out, you must fill it out _right_) for index in range(self.initial_form_count()): self.forms[index].use_required_attribute = True + self.totalPass = 0 def should_delete(self, cleaned): """Should this entry be deleted from the database?""" @@ -104,7 +107,13 @@ class RegistrarFormSet(forms.BaseFormSet): Clean all of self.data and populate self._errors and self._non_form_errors. """ - logger.info("in full_clean") + thisPass = 0 + if (self.totalPass): + thisPass = self.totalPass + self.totalPass += 1 + else: + self.totalPass = 0 + logger.info(f"({thisPass}) in full_clean") self._errors = [] self._non_form_errors = self.error_class() empty_forms_count = 0 @@ -112,7 +121,7 @@ class RegistrarFormSet(forms.BaseFormSet): if not self.is_bound: # Stop further processing. return - logger.info("about to test management form ") + logger.info(f"({thisPass}) about to test management form ") if not self.management_form.is_valid(): error = forms.ValidationError( self.error_messages['missing_management_form'], @@ -126,11 +135,12 @@ class RegistrarFormSet(forms.BaseFormSet): ) self._non_form_errors.append(error) - logger.info("about to test forms in self.forms") + logger.info(f"({thisPass}) about to test forms in self.forms") for i, form in enumerate(self.forms): - logger.info(f"checking form {i}") + logger.info(f"({thisPass}) checking form {i}") # Empty forms are unchanged forms beyond those with initial data. if not form.has_changed() and i >= self.initial_form_count(): + logger.info(f"({thisPass}) empty forms count increase condition found") empty_forms_count += 1 # Accessing errors calls full_clean() if necessary. # _should_delete_form() requires cleaned_data. @@ -138,9 +148,9 @@ class RegistrarFormSet(forms.BaseFormSet): if self.can_delete and self._should_delete_form(form): continue self._errors.append(form_errors) - logger.info("at the end of for loop processing") + logger.info(f"({thisPass}) at the end of for loop processing") try: - logger.info("about to test validate max and min") + logger.info(f"({thisPass}) about to test validate max and min") if (self.validate_max and self.total_form_count() - len(self.deleted_forms) > self.max_num) or \ self.management_form.cleaned_data[TOTAL_FORM_COUNT] > self.absolute_max: @@ -149,7 +159,7 @@ class RegistrarFormSet(forms.BaseFormSet): "Please submit at most %d forms.", self.max_num) % self.max_num, code='too_many_forms', ) - logger.info("between validate max and validate min") + logger.info(f"({thisPass}) between validate max and validate min") if (self.validate_min and self.total_form_count() - len(self.deleted_forms) - empty_forms_count < self.min_num): raise forms.ValidationError(ngettext( @@ -157,10 +167,10 @@ class RegistrarFormSet(forms.BaseFormSet): "Please submit at least %d forms.", self.min_num) % self.min_num, code='too_few_forms') # Give self.clean() a chance to do cross-form validation. - logger.info("about to call clean on formset") + logger.info(f"({thisPass}) about to call clean on formset") self.clean() except forms.ValidationError as e: - logger.info(f"hit an exception {e}") + logger.info(f"({thisPass}) hit an exception {e}") self._non_form_errors = self.error_class(e.error_list) def total_form_count(self): @@ -202,6 +212,7 @@ class RegistrarFormSet(forms.BaseFormSet): # Accessing errors triggers a full clean the first time only. logger.info("before self.errors") self.errors + logger.info(f"self.errors = {self.errors}") # List comprehension ensures is_valid() is called for all forms. # Forms due to be deleted shouldn't cause the formset to be invalid. logger.info("before all isvalid") @@ -773,6 +784,7 @@ class OtherContactsForm(RegistrarForm): def __init__(self, *args, **kwargs): self.form_data_marked_for_deletion = False super().__init__(*args, **kwargs) + self.empty_permitted=False def mark_form_for_deletion(self): self.form_data_marked_for_deletion = True @@ -814,6 +826,65 @@ class OtherContactsForm(RegistrarForm): return self.cleaned_data + def full_clean(self): + logger.info("in form full_clean()") + logger.info(self.fields) + self._errors = ErrorDict() + if not self.is_bound: # Stop further processing. + logger.info("not bound") + return + self.cleaned_data = {} + # If the form is permitted to be empty, and none of the form data has + # changed from the initial data, short circuit any validation. + if self.empty_permitted and not self.has_changed(): + logger.info("empty permitted and has not changed") + return + + self._clean_fields() + self._clean_form() + self._post_clean() + + # need to remove below before merge + def _clean_fields(self): + for name, field in self.fields.items(): + # value_from_datadict() gets the data from the data dictionaries. + # Each widget type knows how to retrieve its own data, because some + # widgets split data over several HTML fields. + if field.disabled: + value = self.get_initial_for_field(field, name) + else: + value = field.widget.value_from_datadict(self.data, self.files, self.add_prefix(name)) + try: + if isinstance(field, forms.FileField): + initial = self.get_initial_for_field(field, name) + value = field.clean(value, initial) + else: + value = field.clean(value) + self.cleaned_data[name] = value + if hasattr(self, 'clean_%s' % name): + value = getattr(self, 'clean_%s' % name)() + self.cleaned_data[name] = value + except forms.ValidationError as e: + self.add_error(name, e) + + # need to remove below before merge + def _clean_form(self): + try: + cleaned_data = self.clean() + except forms.ValidationError as e: + self.add_error(None, e) + else: + if cleaned_data is not None: + self.cleaned_data = cleaned_data + + # need to remove below before merge + def _post_clean(self): + """ + An internal hook for performing additional cleaning after form cleaning + is complete. Used for model validation in model forms. + """ + pass + def is_valid(self): val = super().is_valid() logger.info(f"othercontactsform validation yields: {val}") @@ -857,6 +928,7 @@ class BaseOtherContactsFormSet(RegistrarFormSet): # in the formset plus those that have data already. for index in range(max(self.initial_form_count(), 1)): self.forms[index].use_required_attribute = True + self.totalPass = 0 def should_delete(self, cleaned): # empty = (isinstance(v, str) and (v.strip() == "" or v is None) for v in cleaned.values()) @@ -908,7 +980,7 @@ class BaseOtherContactsFormSet(RegistrarFormSet): number of other contacts when contacts marked for deletion""" if self.formset_data_marked_for_deletion: self.validate_min = False - logger.info("in is_valid()") + logger.info("in FormSet is_valid()") val = super().is_valid() logger.info(f"formset validation yields: {val}") return val diff --git a/src/registrar/views/application.py b/src/registrar/views/application.py index 82836f7af..6c1125651 100644 --- a/src/registrar/views/application.py +++ b/src/registrar/views/application.py @@ -508,6 +508,13 @@ class OtherContacts(ApplicationWizard): other_contacts_forms = forms[1] no_other_contacts_form = forms[2] + # set all the required other_contact fields as necessary since new forms + # were added through javascript + for form in forms[1].forms: + for field_name, field in form.fields.items(): + if field.required: + field.widget.attrs['required'] = 'required' + all_forms_valid = True # test first for yes_no_form validity if other_contacts_yes_no_form.is_valid(): From 02456e92971528e5ac93a978916f13a33e357f83 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Jan 2024 14:20:49 -0700 Subject: [PATCH 010/100] Update views.py --- src/api/views.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/api/views.py b/src/api/views.py index a7dd7600a..e40924708 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -88,10 +88,17 @@ def available(request, domain=""): """ domain = request.GET.get("domain", "") DraftDomain = apps.get_model("registrar.DraftDomain") + if domain is None or domain.strip() == "": + # TODO - change this... should it be the regular required? + return JsonResponse({"available": False, "code": "invalid", "message": "This field is required"}) # validate that the given domain could be a domain name and fail early if # not. if not (DraftDomain.string_could_be_domain(domain) or DraftDomain.string_could_be_domain(domain + ".gov")): - return JsonResponse({"available": False, "code": "invalid", "message": DOMAIN_API_MESSAGES["invalid"]}) + print(f"What is the domain at this point? {domain}") + if "." in domain: + return JsonResponse({"available": False, "code": "invalid", "message": DOMAIN_API_MESSAGES["extra_dots"]}) + else: + return JsonResponse({"available": False, "code": "invalid", "message": DOMAIN_API_MESSAGES["invalid"]}) # a domain is available if it is NOT in the list of current domains try: if check_domain_available(domain): From 79519f956683df011ef62eb5c70bc56041a63e9b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 8 Jan 2024 16:33:41 -0500 Subject: [PATCH 011/100] Handle fieldset headers in JS delete and add, fix the page jump on delete first form through JS, remove form level errors from template and form --- src/registrar/assets/js/get-gov.js | 121 +++++++----------- src/registrar/forms/application_wizard.py | 20 +-- src/registrar/templates/application_form.html | 1 - .../templates/includes/non_form_errors.html | 9 -- 4 files changed, 52 insertions(+), 99 deletions(-) delete mode 100644 src/registrar/templates/includes/non_form_errors.html diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 8f598b3f1..f4f770c84 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -307,7 +307,7 @@ function removeForm(e, formLabel, isNameserversForm, addButton, formIdentifier){ }); } -function markForm(e){ +function markForm(e, formLabel){ let totalShownForms = document.querySelectorAll(`.repeatable-form:not([style*="display: none"])`).length; console.log("markForm start: " + totalShownForms) @@ -334,84 +334,50 @@ function markForm(e){ // Set display to 'none' formToRemove.style.display = 'none'; + + + // Get all hidden fieldsets + const hiddenFieldset = document.querySelector('.repeatable-form[style="display: none;"]'); + let targetFieldset = null; + + // Loop. If a hidden fieldset does not have any sibling out of all the previous siblings that's visible: + // There is no previous sibling that does not have display none + if (hiddenFieldset && !hiddenFieldset.previousElementSibling.matches('.repeatable-form:not([style="display: none;"])')) { + let currentSibling = hiddenFieldset.nextElementSibling; + + // Iterate through siblings until a visible fieldset is found + while (currentSibling) { + if (currentSibling.matches(':not([style="display: none;"])')) { + targetFieldset = currentSibling; + break; + } + + currentSibling = currentSibling.nextElementSibling; + } + } + + if (targetFieldset) { + // Apply your logic or styles to the targetFieldset + targetFieldset.querySelector('h2').style.marginTop = '1rem'; // Example style + } + // update headers on shown forms console.log("markForm end: " + totalShownForms) } - // let shownForms = document.querySelectorAll(".repeatable-form"); - // totalForms.setAttribute('value', `${forms.length}`); + let shownForms = document.querySelectorAll(`.repeatable-form:not([style*="display: none"])`); // let formNumberRegex = RegExp(`form-(\\d){1}-`, 'g'); - // let formLabelRegex = RegExp(`${formLabel} (\\d+){1}`, 'g'); - // // For the example on Nameservers - // let formExampleRegex = RegExp(`ns(\\d+){1}`, 'g'); + let formLabelRegex = RegExp(`${formLabel} (\\d+){1}`, 'g'); - // forms.forEach((form, index) => { - // // Iterate over child nodes of the current element - // Array.from(form.querySelectorAll('label, input, select')).forEach((node) => { - // // Iterate through the attributes of the current node - // Array.from(node.attributes).forEach((attr) => { - // // Check if the attribute value matches the regex - // if (formNumberRegex.test(attr.value)) { - // // Replace the attribute value with the updated value - // attr.value = attr.value.replace(formNumberRegex, `form-${index}-`); - // } - // }); - // }); - - // // h2 and legend for DS form, label for nameservers - // Array.from(form.querySelectorAll('h2, legend, label, p')).forEach((node) => { - - // // If the node is a nameserver label, one of the first 2 which was previously 3 and up (not required) - // // inject the USWDS required markup and make sure the INPUT is required - // if (isNameserversForm && index <= 1 && node.innerHTML.includes('server') && !node.innerHTML.includes('*')) { - // // Create a new element - // const newElement = document.createElement('abbr'); - // newElement.textContent = '*'; - // newElement.setAttribute("title", "required"); - // newElement.classList.add("usa-hint", "usa-hint--required"); - - // // Append the new element to the label - // node.appendChild(newElement); - // // Find the next sibling that is an input element - // let nextInputElement = node.nextElementSibling; - - // while (nextInputElement) { - // if (nextInputElement.tagName === 'INPUT') { - // // Found the next input element - // nextInputElement.setAttribute("required", "") - // break; - // } - // nextInputElement = nextInputElement.nextElementSibling; - // } - // nextInputElement.required = true; - // } - - // let innerSpan = node.querySelector('span') - // if (innerSpan) { - // innerSpan.textContent = innerSpan.textContent.replace(formLabelRegex, `${formLabel} ${index + 1}`); - // } else { - // node.textContent = node.textContent.replace(formLabelRegex, `${formLabel} ${index + 1}`); - // node.textContent = node.textContent.replace(formExampleRegex, `ns${index + 1}`); - // } - // }); - - // // Display the add more button if we have less than 13 forms - // if (isNameserversForm && forms.length <= 13) { - // console.log('remove disabled'); - // addButton.removeAttribute("disabled"); - // } - - // if (isNameserversForm && forms.length < 3) { - // // Hide the delete buttons on the remaining nameservers - // Array.from(form.querySelectorAll('.delete-record')).forEach((deleteButton) => { - // deleteButton.setAttribute("disabled", "true"); - // }); - // } - - // }); + shownForms.forEach((form, index) => { + // Iterate over child nodes of the current element + Array.from(form.querySelectorAll('h2')).forEach((node) => { + node.textContent = node.textContent.replace(formLabelRegex, `${formLabel} ${index + 1}`); + }); + }); } function prepareNewDeleteButton(btn, formLabel) { @@ -425,7 +391,9 @@ function prepareNewDeleteButton(btn, formLabel) { if (isOtherContactsForm) { // We will mark the forms for deletion - btn.addEventListener('click', markForm); + btn.addEventListener('click', function(e) { + markForm(e, formLabel); + }); } else { // We will remove the forms and re-order the formset btn.addEventListener('click', function(e) { @@ -454,7 +422,9 @@ function prepareDeleteButtons(formLabel) { deleteButtons.forEach((deleteButton) => { if (isOtherContactsForm) { // We will mark the forms for deletion - deleteButton.addEventListener('click', markForm); + deleteButton.addEventListener('click', function(e) { + markForm(e, formLabel); + }); } else { // We will remove the forms and re-order the formset deleteButton.addEventListener('click', function(e) { @@ -565,7 +535,12 @@ function hideDeletedForms() { formNum++; newForm.innerHTML = newForm.innerHTML.replace(formNumberRegex, `${formIdentifier}-${formNum-1}-`); - newForm.innerHTML = newForm.innerHTML.replace(formLabelRegex, `${formLabel} ${formNum}`); + if (isOtherContactsForm) { + let totalShownForms = document.querySelectorAll(`.repeatable-form:not([style*="display: none"])`).length; + newForm.innerHTML = newForm.innerHTML.replace(formLabelRegex, `${formLabel} ${totalShownForms + 1}`); + } else { + newForm.innerHTML = newForm.innerHTML.replace(formLabelRegex, `${formLabel} ${formNum}`); + } newForm.innerHTML = newForm.innerHTML.replace(formExampleRegex, `ns${formNum}`); container.insertBefore(newForm, addButton); diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index b50a7b179..dfb727743 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -774,7 +774,10 @@ class OtherContactsForm(RegistrarForm): ) email = forms.EmailField( label="Email", - error_messages={"invalid": ("Enter an email address in the required format, like name@example.com.")}, + error_messages={ + "required": ("Enter an email address in the required format, like name@example.com."), + "invalid": ("Enter an email address in the required format, like name@example.com.") + }, ) phone = PhoneNumberField( label="Phone", @@ -934,21 +937,6 @@ class BaseOtherContactsFormSet(RegistrarFormSet): # empty = (isinstance(v, str) and (v.strip() == "" or v is None) for v in cleaned.values()) # empty forms should throw errors return self.formset_data_marked_for_deletion or cleaned.get("DELETE", False) - - def non_form_errors(self): - """ - Method to override non_form_errors. - If minimum number of contacts is not submitted, customize the error message - that is returned.""" - # Get the default non_form_errors - errors = super().non_form_errors() - - # Check if the default error message is present - if 'Please submit at least 1 form.' in errors: - # Replace the default message with the custom message - errors = ['Please submit at least 1 contact.'] - - return errors def pre_create(self, db_obj, cleaned): """Code to run before an item in the formset is created in the database.""" diff --git a/src/registrar/templates/application_form.html b/src/registrar/templates/application_form.html index 4d00076cb..c34ddf5bc 100644 --- a/src/registrar/templates/application_form.html +++ b/src/registrar/templates/application_form.html @@ -43,7 +43,6 @@ {% for inner in outer.forms %} {% include "includes/form_errors.html" with form=inner %} {% endfor %} - {% include "includes/non_form_errors.html" with form=outer %} {% else %} {% include "includes/form_errors.html" with form=outer %} {% endif %} diff --git a/src/registrar/templates/includes/non_form_errors.html b/src/registrar/templates/includes/non_form_errors.html deleted file mode 100644 index 5c33904a3..000000000 --- a/src/registrar/templates/includes/non_form_errors.html +++ /dev/null @@ -1,9 +0,0 @@ -{% if form.errors %} - {% for error in form.non_form_errors %} -
-
- {{ error|escape }} -
-
- {% endfor %} -{% endif %} \ No newline at end of file From 080d59c0b5d9d01e362d8ee4945267a7ed9a9844 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 8 Jan 2024 16:59:34 -0500 Subject: [PATCH 012/100] updated comments and removed extraneous code and logging from application_wizard --- src/registrar/forms/application_wizard.py | 231 ++-------------------- 1 file changed, 17 insertions(+), 214 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index dfb727743..6e6ffa6ac 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -77,7 +77,6 @@ class RegistrarFormSet(forms.BaseFormSet): # if you opt to fill it out, you must fill it out _right_) for index in range(self.initial_form_count()): self.forms[index].use_required_attribute = True - self.totalPass = 0 def should_delete(self, cleaned): """Should this entry be deleted from the database?""" @@ -102,127 +101,6 @@ class RegistrarFormSet(forms.BaseFormSet): """ raise NotImplementedError - def full_clean(self): - """ - Clean all of self.data and populate self._errors and - self._non_form_errors. - """ - thisPass = 0 - if (self.totalPass): - thisPass = self.totalPass - self.totalPass += 1 - else: - self.totalPass = 0 - logger.info(f"({thisPass}) in full_clean") - self._errors = [] - self._non_form_errors = self.error_class() - empty_forms_count = 0 - - if not self.is_bound: # Stop further processing. - return - - logger.info(f"({thisPass}) about to test management form ") - if not self.management_form.is_valid(): - error = forms.ValidationError( - self.error_messages['missing_management_form'], - params={ - 'field_names': ', '.join( - self.management_form.add_prefix(field_name) - for field_name in self.management_form.errors - ), - }, - code='missing_management_form', - ) - self._non_form_errors.append(error) - - logger.info(f"({thisPass}) about to test forms in self.forms") - for i, form in enumerate(self.forms): - logger.info(f"({thisPass}) checking form {i}") - # Empty forms are unchanged forms beyond those with initial data. - if not form.has_changed() and i >= self.initial_form_count(): - logger.info(f"({thisPass}) empty forms count increase condition found") - empty_forms_count += 1 - # Accessing errors calls full_clean() if necessary. - # _should_delete_form() requires cleaned_data. - form_errors = form.errors - if self.can_delete and self._should_delete_form(form): - continue - self._errors.append(form_errors) - logger.info(f"({thisPass}) at the end of for loop processing") - try: - logger.info(f"({thisPass}) about to test validate max and min") - if (self.validate_max and - self.total_form_count() - len(self.deleted_forms) > self.max_num) or \ - self.management_form.cleaned_data[TOTAL_FORM_COUNT] > self.absolute_max: - raise forms.ValidationError(ngettext( - "Please submit at most %d form.", - "Please submit at most %d forms.", self.max_num) % self.max_num, - code='too_many_forms', - ) - logger.info(f"({thisPass}) between validate max and validate min") - if (self.validate_min and - self.total_form_count() - len(self.deleted_forms) - empty_forms_count < self.min_num): - raise forms.ValidationError(ngettext( - "Please submit at least %d form.", - "Please submit at least %d forms.", self.min_num) % self.min_num, - code='too_few_forms') - # Give self.clean() a chance to do cross-form validation. - logger.info(f"({thisPass}) about to call clean on formset") - self.clean() - except forms.ValidationError as e: - logger.info(f"({thisPass}) hit an exception {e}") - self._non_form_errors = self.error_class(e.error_list) - - def total_form_count(self): - """Return the total number of forms in this FormSet.""" - logger.info("in total_form_count") - if self.is_bound: - logger.info("is_bound") - # return absolute_max if it is lower than the actual total form - # count in the data; this is DoS protection to prevent clients - # from forcing the server to instantiate arbitrary numbers of - # forms - return min(self.management_form.cleaned_data[TOTAL_FORM_COUNT], self.absolute_max) - else: - initial_forms = self.initial_form_count() - total_forms = max(initial_forms, self.min_num) + self.extra - # Allow all existing related objects/inlines to be displayed, - # but don't allow extra beyond max_num. - if initial_forms > self.max_num >= 0: - total_forms = initial_forms - elif total_forms > self.max_num >= 0: - total_forms = self.max_num - return total_forms - - def initial_form_count(self): - """Return the number of forms that are required in this FormSet.""" - logger.info("in initial_form_count") - if self.is_bound: - logger.info(f"initial form count = {self.management_form.cleaned_data[INITIAL_FORM_COUNT]}") - return self.management_form.cleaned_data[INITIAL_FORM_COUNT] - else: - # Use the length of the initial data if it's there, 0 otherwise. - initial_forms = len(self.initial) if self.initial else 0 - return initial_forms - - def is_valid(self): - """Return True if every form in self.forms is valid.""" - if not self.is_bound: - return False - # Accessing errors triggers a full clean the first time only. - logger.info("before self.errors") - self.errors - logger.info(f"self.errors = {self.errors}") - # List comprehension ensures is_valid() is called for all forms. - # Forms due to be deleted shouldn't cause the formset to be invalid. - logger.info("before all isvalid") - forms_valid = all([ - form.is_valid() for form in self.forms - if not (self.can_delete and self._should_delete_form(form)) - ]) - logger.info(f"forms_valid = {forms_valid}") - return forms_valid and not self.non_form_errors() - def test_if_more_than_one_join(self, db_obj, rel, related_name): """Helper for finding whether an object is joined more than once.""" # threshold is the number of related objects that are acceptable @@ -268,7 +146,6 @@ class RegistrarFormSet(forms.BaseFormSet): """ if not self.is_valid(): return - logger.info(obj) obj.save() query = getattr(obj, join).order_by("created_at").all() # order matters @@ -290,9 +167,6 @@ class RegistrarFormSet(forms.BaseFormSet): for db_obj, post_data in zip_longest(query, self.forms, fillvalue=None): cleaned = post_data.cleaned_data if post_data is not None else {} - logger.info(post_data) - logger.info(cleaned) - logger.info(db_obj) # matching database object exists, update it if db_obj is not None and cleaned: if should_delete(cleaned): @@ -310,11 +184,7 @@ class RegistrarFormSet(forms.BaseFormSet): # no matching database object, create it # make sure not to create a database object if cleaned has 'delete' attribute elif db_obj is None and cleaned and not cleaned.get("DELETE", False): - logger.info(cleaned.get("DELETE",False)) - logger.info("about to pre_create") kwargs = pre_create(db_obj, cleaned) - logger.info("after pre_create") - logger.info(kwargs) getattr(obj, join).create(**kwargs) @classmethod @@ -785,6 +655,14 @@ class OtherContactsForm(RegistrarForm): ) def __init__(self, *args, **kwargs): + """ + Override the __init__ method for RegistrarForm. + Set form_data_marked_for_deletion to false. + Empty_permitted set to False, as this is overridden in certain circumstances by + Django's BaseFormSet, and results in empty forms being allowed and field level + errors not appropriately raised. This works with code in the view which appropriately + displays required attributes on fields. + """ self.form_data_marked_for_deletion = False super().__init__(*args, **kwargs) self.empty_permitted=False @@ -796,20 +674,10 @@ 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, or one marked for - deletion to be considered valid even though certain required fields have + In this override, allow for a form which is deleted by user or marked for + deletion by formset 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 - if self.form_data_marked_for_deletion or self.cleaned_data["DELETE"]: # clear any errors raised by the form fields # (before this clean() method is run, each field @@ -828,70 +696,6 @@ class OtherContactsForm(RegistrarForm): return {"DELETE": True} return self.cleaned_data - - def full_clean(self): - logger.info("in form full_clean()") - logger.info(self.fields) - self._errors = ErrorDict() - if not self.is_bound: # Stop further processing. - logger.info("not bound") - return - self.cleaned_data = {} - # If the form is permitted to be empty, and none of the form data has - # changed from the initial data, short circuit any validation. - if self.empty_permitted and not self.has_changed(): - logger.info("empty permitted and has not changed") - return - - self._clean_fields() - self._clean_form() - self._post_clean() - - # need to remove below before merge - def _clean_fields(self): - for name, field in self.fields.items(): - # value_from_datadict() gets the data from the data dictionaries. - # Each widget type knows how to retrieve its own data, because some - # widgets split data over several HTML fields. - if field.disabled: - value = self.get_initial_for_field(field, name) - else: - value = field.widget.value_from_datadict(self.data, self.files, self.add_prefix(name)) - try: - if isinstance(field, forms.FileField): - initial = self.get_initial_for_field(field, name) - value = field.clean(value, initial) - else: - value = field.clean(value) - self.cleaned_data[name] = value - if hasattr(self, 'clean_%s' % name): - value = getattr(self, 'clean_%s' % name)() - self.cleaned_data[name] = value - except forms.ValidationError as e: - self.add_error(name, e) - - # need to remove below before merge - def _clean_form(self): - try: - cleaned_data = self.clean() - except forms.ValidationError as e: - self.add_error(None, e) - else: - if cleaned_data is not None: - self.cleaned_data = cleaned_data - - # need to remove below before merge - def _post_clean(self): - """ - An internal hook for performing additional cleaning after form cleaning - is complete. Used for model validation in model forms. - """ - pass - - def is_valid(self): - val = super().is_valid() - logger.info(f"othercontactsform validation yields: {val}") - return val class BaseOtherContactsFormSet(RegistrarFormSet): @@ -923,6 +727,9 @@ class BaseOtherContactsFormSet(RegistrarFormSet): return forms.HiddenInput(attrs={"class": "deletion"}) def __init__(self, *args, **kwargs): + """ + Override __init__ for RegistrarFormSet. + """ self.formset_data_marked_for_deletion = False self.application = kwargs.pop("application", None) super(RegistrarFormSet, self).__init__(*args, **kwargs) @@ -931,11 +738,11 @@ class BaseOtherContactsFormSet(RegistrarFormSet): # in the formset plus those that have data already. for index in range(max(self.initial_form_count(), 1)): self.forms[index].use_required_attribute = True - self.totalPass = 0 def should_delete(self, cleaned): - # empty = (isinstance(v, str) and (v.strip() == "" or v is None) for v in cleaned.values()) - # empty forms should throw errors + """ + Implements should_delete method from BaseFormSet. + """ return self.formset_data_marked_for_deletion or cleaned.get("DELETE", False) def pre_create(self, db_obj, cleaned): @@ -946,7 +753,6 @@ class BaseOtherContactsFormSet(RegistrarFormSet): return cleaned def to_database(self, obj: DomainApplication): - logger.info("in to_database for BaseOtherContactsFormSet") self._to_database(obj, self.JOIN, self.REVERSE_JOINS, self.should_delete, self.pre_update, self.pre_create) @classmethod @@ -968,10 +774,7 @@ class BaseOtherContactsFormSet(RegistrarFormSet): number of other contacts when contacts marked for deletion""" if self.formset_data_marked_for_deletion: self.validate_min = False - logger.info("in FormSet is_valid()") - val = super().is_valid() - logger.info(f"formset validation yields: {val}") - return val + return super().is_valid() OtherContactsFormSet = forms.formset_factory( From fd1bd09f6c74f5c9567fd79ca28202c973df0a7f Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 8 Jan 2024 17:08:53 -0500 Subject: [PATCH 013/100] removed extraneous logging and code from views/application.py and application_wizard.py --- src/registrar/forms/application_wizard.py | 7 ------- src/registrar/views/application.py | 17 +---------------- 2 files changed, 1 insertion(+), 23 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 6e6ffa6ac..c85d357c1 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -1,12 +1,10 @@ from __future__ import annotations # allows forward references in annotations from itertools import zip_longest import logging -import copy from typing import Callable from phonenumber_field.formfields import PhoneNumberField # type: ignore from django import forms -from django.forms.utils import ErrorDict from django.core.validators import RegexValidator, MaxLengthValidator from django.utils.safestring import mark_safe from django.db.models.fields.related import ForeignObjectRel, OneToOneField @@ -16,13 +14,9 @@ from api.views import DOMAIN_API_MESSAGES from registrar.models import Contact, DomainApplication, DraftDomain, Domain from registrar.templatetags.url_helpers import public_site_url from registrar.utility import errors -from django.utils.translation import gettext_lazy as _, ngettext logger = logging.getLogger(__name__) -TOTAL_FORM_COUNT = 'TOTAL_FORMS' -INITIAL_FORM_COUNT = 'INITIAL_FORMS' - class RegistrarForm(forms.Form): """ A common set of methods and configuration. @@ -688,7 +682,6 @@ class OtherContactsForm(RegistrarForm): # That causes problems. for field in self.fields: if field in self.errors: - logger.info(f"deleting error {self.errors[field]}") del self.errors[field] # return empty object with only 'delete' attribute defined. # this will prevent _to_database from creating an empty diff --git a/src/registrar/views/application.py b/src/registrar/views/application.py index 6c1125651..881590c4f 100644 --- a/src/registrar/views/application.py +++ b/src/registrar/views/application.py @@ -277,7 +277,6 @@ class ApplicationWizard(ApplicationWizardPermissionView, TemplateView): for form in forms: data = form.from_database(self.application) if self.has_pk() else None if use_post: - logger.info("about to instantiate form ") instantiated.append(form(self.request.POST, **kwargs)) elif use_db: instantiated.append(form(data, **kwargs)) @@ -371,10 +370,6 @@ class ApplicationWizard(ApplicationWizardPermissionView, TemplateView): def post(self, request, *args, **kwargs) -> HttpResponse: """This method handles POST requests.""" - - # Log the keys and values of request.POST - for key, value in request.POST.items(): - logger.info("Key: %s, Value: %s", key, value) # which button did the user press? button: str = request.POST.get("submit_button", "") @@ -390,7 +385,6 @@ class ApplicationWizard(ApplicationWizardPermissionView, TemplateView): return self.goto(self.steps.first) forms = self.get_forms(use_post=True) - logger.info("after geting forms") if self.is_valid(forms): # always save progress self.save(forms) @@ -492,11 +486,6 @@ class YourContact(ApplicationWizard): class OtherContacts(ApplicationWizard): template_name = "application_other_contacts.html" forms = [forms.OtherContactsYesNoForm, forms.OtherContactsFormSet, forms.NoOtherContactsForm] - - def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - logger.info(context) - return context def is_valid(self, forms: list) -> bool: """Overrides default behavior defined in ApplicationWizard. @@ -511,7 +500,7 @@ class OtherContacts(ApplicationWizard): # set all the required other_contact fields as necessary since new forms # were added through javascript for form in forms[1].forms: - for field_name, field in form.fields.items(): + for _, field in form.fields.items(): if field.required: field.widget.attrs['required'] = 'required' @@ -520,14 +509,10 @@ class OtherContacts(ApplicationWizard): if other_contacts_yes_no_form.is_valid(): # test for has_contacts if other_contacts_yes_no_form.cleaned_data.get("has_other_contacts"): - logger.info("has other contacts") # mark the no_other_contacts_form for deletion no_other_contacts_form.mark_form_for_deletion() - logger.info("after marking for deletion") # test that the other_contacts_forms and no_other_contacts_forms are valid all_forms_valid = all(form.is_valid() for form in forms[1:]) - logger.info("after checking forms for validity") - logger.info(f"all forms valid = {all_forms_valid}") else: # mark the other_contacts_forms formset for deletion other_contacts_forms.mark_formset_for_deletion() From 39f594f4e4eb1c7b6667eea5c559cb08debb7409 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 8 Jan 2024 17:28:09 -0500 Subject: [PATCH 014/100] Clean up and comment JS --- src/registrar/assets/js/get-gov.js | 64 ++++++++++++++++-------------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index f4f770c84..177b771e4 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -229,6 +229,10 @@ function handleValidationClick(e) { } })(); +/** + * Delete method for formsets that diff in the view and delete in the model (Nameservers, DS Data) + * + */ function removeForm(e, formLabel, isNameserversForm, addButton, formIdentifier){ let totalForms = document.querySelector(`#id_${formIdentifier}-TOTAL_FORMS`); let formToRemove = e.target.closest(".repeatable-form"); @@ -293,7 +297,6 @@ function removeForm(e, formLabel, isNameserversForm, addButton, formIdentifier){ // Display the add more button if we have less than 13 forms if (isNameserversForm && forms.length <= 13) { - console.log('remove disabled'); addButton.removeAttribute("disabled"); } @@ -307,22 +310,24 @@ function removeForm(e, formLabel, isNameserversForm, addButton, formIdentifier){ }); } +/** + * Delete method for formsets using the DJANGO DELETE widget (Other Contacts) + * + */ function markForm(e, formLabel){ + // Unlike removeForm, we only work with the visible forms when using DJANGO's DELETE widget let totalShownForms = document.querySelectorAll(`.repeatable-form:not([style*="display: none"])`).length; - console.log("markForm start: " + totalShownForms) - if (totalShownForms == 1) { // toggle the radio buttons let radioButton = document.querySelector('input[name="other_contacts-has_other_contacts"][value="False"]'); radioButton.checked = true; - // Trigger the change event let event = new Event('change'); radioButton.dispatchEvent(event); } else { - // Grab the hidden delete input and CHECK it + // Grab the hidden delete input and assign a value DJANGO will look for let formToRemove = e.target.closest(".repeatable-form"); if (formToRemove) { let deleteInput = formToRemove.querySelector('input[class="deletion"]'); @@ -334,44 +339,34 @@ function markForm(e, formLabel){ // Set display to 'none' formToRemove.style.display = 'none'; - - - // Get all hidden fieldsets + // + // This next block is a hack to fix a page jump when a fielset is set to display none at the start of the formset but still takes + // a bit of space in the DOM, causing the content to jump down a bit + // + // Get the first hidden fieldset const hiddenFieldset = document.querySelector('.repeatable-form[style="display: none;"]'); let targetFieldset = null; - - // Loop. If a hidden fieldset does not have any sibling out of all the previous siblings that's visible: - // There is no previous sibling that does not have display none + // If that first hidden fieldset does not have any sibling out of all the previous siblings that's visible, get the next visible fieldset if (hiddenFieldset && !hiddenFieldset.previousElementSibling.matches('.repeatable-form:not([style="display: none;"])')) { let currentSibling = hiddenFieldset.nextElementSibling; - // Iterate through siblings until a visible fieldset is found while (currentSibling) { if (currentSibling.matches(':not([style="display: none;"])')) { targetFieldset = currentSibling; break; } - currentSibling = currentSibling.nextElementSibling; } } - if (targetFieldset) { - // Apply your logic or styles to the targetFieldset - targetFieldset.querySelector('h2').style.marginTop = '1rem'; // Example style + // Account for the space the hidden fieldsets at the top of the formset are occupying in the DOM + targetFieldset.querySelector('h2').style.marginTop = '1rem'; } - - // update headers on shown forms - console.log("markForm end: " + totalShownForms) - } - + // Update h2s on the visible forms only. We won't worry about the forms' identifiers let shownForms = document.querySelectorAll(`.repeatable-form:not([style*="display: none"])`); - - // let formNumberRegex = RegExp(`form-(\\d){1}-`, 'g'); let formLabelRegex = RegExp(`${formLabel} (\\d+){1}`, 'g'); - shownForms.forEach((form, index) => { // Iterate over child nodes of the current element Array.from(form.querySelectorAll('h2')).forEach((node) => { @@ -380,6 +375,11 @@ function markForm(e, formLabel){ }); } +/** + * Prepare the namerservers, DS data and Other Contacts formsets' delete button + * for the last added form. We call this from the Add function + * + */ function prepareNewDeleteButton(btn, formLabel) { let formIdentifier = "form" let isNameserversForm = document.title.includes("DNS name servers |"); @@ -403,8 +403,8 @@ function prepareNewDeleteButton(btn, formLabel) { } /** - * Prepare the namerservers and DS data forms delete buttons - * We will call this on the forms init, and also every time we add a form + * Prepare the namerservers, DS data and Other Contacts formsets' delete buttons + * We will call this on the forms init * */ function prepareDeleteButtons(formLabel) { @@ -417,7 +417,6 @@ function prepareDeleteButtons(formLabel) { formIdentifier = "other_contacts"; } - // Loop through each delete button and attach the click event listener deleteButtons.forEach((deleteButton) => { if (isOtherContactsForm) { @@ -432,10 +431,10 @@ function prepareDeleteButtons(formLabel) { }); } }); - } /** + * DJANGO formset's DELETE widget * On form load, hide deleted forms, ie. those forms with hidden input of class 'deletion' * with value='on' */ @@ -470,11 +469,14 @@ function hideDeletedForms() { let formLabel = ''; let isNameserversForm = document.title.includes("DNS name servers |"); let isOtherContactsForm = document.title.includes("Other employees from your organization"); + // The Nameservers form st features 2 required and 11 optionals if (isNameserversForm) { cloneIndex = 2; formLabel = "Name server"; - } else if ((document.title.includes("DS Data |")) || (document.title.includes("Key Data |"))) { + // DNSSEC: DS Data + } else if (document.title.includes("DS Data |")) { formLabel = "DS Data record"; + // The Other Contacts form } else if (isOtherContactsForm) { formLabel = "Organization contact"; container = document.querySelector("#other-employees"); @@ -535,6 +537,9 @@ function hideDeletedForms() { formNum++; newForm.innerHTML = newForm.innerHTML.replace(formNumberRegex, `${formIdentifier}-${formNum-1}-`); + // For the other contacts form, we need to update the fieldset headers based on what's visible vs hidden, + // since the form on the backend employs Django's DELETE widget. For the other formsets, we delete the form + // in JS (completely remove from teh DOM) so we update the headers/labels based on total number of forms. if (isOtherContactsForm) { let totalShownForms = document.querySelectorAll(`.repeatable-form:not([style*="display: none"])`).length; newForm.innerHTML = newForm.innerHTML.replace(formLabelRegex, `${formLabel} ${totalShownForms + 1}`); @@ -633,6 +638,7 @@ function hideDeletedForms() { } })(); +// A generic display none/block toggle function that takes an integer param to indicate how the elements toggle function toggleTwoDomElements(ele1, ele2, index) { let element1 = document.getElementById(ele1); let element2 = document.getElementById(ele2); From 93c1f066f762ce609fbef73202409baf3478883e Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 8 Jan 2024 18:15:21 -0500 Subject: [PATCH 015/100] Fix typo in comment --- src/registrar/assets/js/get-gov.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 177b771e4..e0eb191ef 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -469,7 +469,7 @@ function hideDeletedForms() { let formLabel = ''; let isNameserversForm = document.title.includes("DNS name servers |"); let isOtherContactsForm = document.title.includes("Other employees from your organization"); - // The Nameservers form st features 2 required and 11 optionals + // The Nameservers formset features 2 required and 11 optionals if (isNameserversForm) { cloneIndex = 2; formLabel = "Name server"; From fffa489bb8917252becf794bc701fd1da7a7af9f Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 8 Jan 2024 19:14:03 -0500 Subject: [PATCH 016/100] WIP test_application_delete_other_contact --- src/registrar/tests/test_views.py | 47 ++++++++++++++++++------------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 38ab9b96b..7b1fb2e5c 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -957,7 +957,7 @@ class DomainApplicationTests(TestWithUser, WebTest): def test_submitting_no_other_contacts_rationale_removes_reference_other_contacts_when_joined(self): """When a user submits the Other Contacts form with no other contacts selected, the application's other contacts references get removed for other contacts that exist and are joined to other objects""" - # Populate the databse with a domain application that + # Populate the database with a domain application that # has 1 "other contact" assigned to it # We'll do it from scratch so we can reuse the other contact ao, _ = Contact.objects.get_or_create( @@ -1079,11 +1079,14 @@ class DomainApplicationTests(TestWithUser, WebTest): # Assert that it is returned, ie the contacts form is required self.assertContains(response, "Enter the first name / given name of this contact.") - @skip("Repurpose when working on ticket 903") 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 + """Other contacts can be deleted after being saved to database. + + This formset uses the DJANGO DELETE widget. We'll test that by setting 2 contacts on an application, + loading the form and marking one contact up for deletion.""" + # Populate the database with a domain application that # has 1 "other contact" assigned to it + # We'll do it from scratch so we can reuse the other contact ao, _ = Contact.objects.get_or_create( first_name="Testy", last_name="Tester", @@ -1105,6 +1108,13 @@ class DomainApplicationTests(TestWithUser, WebTest): email="testy2@town.com", phone="(555) 555 5557", ) + other2, _ = Contact.objects.get_or_create( + first_name="Testy3", + last_name="Tester3", + title="Another Tester", + email="testy3@town.com", + phone="(555) 555 5557", + ) application, _ = DomainApplication.objects.get_or_create( organization_type="federal", federal_type="executive", @@ -1121,6 +1131,7 @@ class DomainApplicationTests(TestWithUser, WebTest): status="started", ) application.other_contacts.add(other) + application.other_contacts.add(other2) # prime the form by visiting /edit self.app.get(reverse("edit-application", kwargs={"id": application.pk})) @@ -1135,36 +1146,34 @@ class DomainApplicationTests(TestWithUser, WebTest): self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) 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) self.assertEqual(other_contacts_form["other_contacts-0-first_name"].value, "Testy2") + self.assertEqual(other_contacts_form["other_contacts-1-first_name"].value, "Testy3") # 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"] = "" - other_contacts_form["other_contacts-0-title"] = "" - other_contacts_form["other_contacts-0-email"] = "" - other_contacts_form["other_contacts-0-phone"] = "" + other_contacts_form["other_contacts-0-DELETE"].value = "on" + - # Submit the now empty form - result = other_contacts_form.submit() + # Submit the form + other_contacts_form.submit() self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + print(other_contacts_page.content.decode('utf-8')) + # 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, + 1, ) - - # 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) + + Contact.objects.all().delete() + DomainApplication.objects.all().delete() def test_application_about_your_organiztion_interstate(self): """Special districts have to answer an additional question.""" From eb7498b28e6d53c11d81cdeee3449d2620b816af Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 9 Jan 2024 00:12:27 -0500 Subject: [PATCH 017/100] Fixed test_delete_other_contact, wip test_delete_other_contact_sets_visible_empty_form_as_required_after_failed_submit --- src/registrar/tests/test_views.py | 125 ++++++++++++++++++++++++----- src/registrar/views/application.py | 4 + 2 files changed, 108 insertions(+), 21 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 7b1fb2e5c..ec4a51b98 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -5,6 +5,8 @@ from django.conf import settings from django.test import Client, TestCase from django.urls import reverse from django.contrib.auth import get_user_model + +from registrar.forms.application_wizard import OtherContactsFormSet from .common import MockEppLib, MockSESClient, completed_application, create_user # type: ignore from django_webtest import WebTest # type: ignore import boto3_mocking # type: ignore @@ -1079,7 +1081,7 @@ class DomainApplicationTests(TestWithUser, WebTest): # Assert that it is returned, ie the contacts form is required self.assertContains(response, "Enter the first name / given name of this contact.") - def test_application_delete_other_contact(self): + def test_delete_other_contact(self): """Other contacts can be deleted after being saved to database. This formset uses the DJANGO DELETE widget. We'll test that by setting 2 contacts on an application, @@ -1092,28 +1094,28 @@ class DomainApplicationTests(TestWithUser, WebTest): last_name="Tester", title="Chief Tester", email="testy@town.com", - phone="(555) 555 5555", + phone="(201) 555 5555", ) 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", + phone="(201) 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", + phone="(201) 555 5557", ) other2, _ = Contact.objects.get_or_create( first_name="Testy3", last_name="Tester3", title="Another Tester", email="testy3@town.com", - phone="(555) 555 5557", + phone="(201) 555 5557", ) application, _ = DomainApplication.objects.get_or_create( organization_type="federal", @@ -1147,33 +1149,114 @@ 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) + # Minimal check to ensure the form is loaded with both other contacts self.assertEqual(other_contacts_form["other_contacts-0-first_name"].value, "Testy2") self.assertEqual(other_contacts_form["other_contacts-1-first_name"].value, "Testy3") - # clear the form - other_contacts_form["other_contacts-0-DELETE"].value = "on" - + # Mark the first dude for deletion + other_contacts_form.set("other_contacts-0-DELETE", "on") # Submit the form other_contacts_form.submit() self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + # Verify that the first dude was deleted + application = DomainApplication.objects.get() + self.assertEqual(application.other_contacts.count(), 1) + self.assertEqual(application.other_contacts.first().first_name, "Testy3") - print(other_contacts_page.content.decode('utf-8')) + def test_delete_other_contact_sets_visible_empty_form_as_required_after_failed_submit(self): + """When you + 1. add an empty contact, + 2. delete existing contacts, + 3. then submit, + The forms on page reload shows all the required fields and their errors.""" - # 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(), - 1, + # Populate the database with a domain application that + # has 1 "other contact" assigned to it + # We'll do it from scratch so we can reuse the other contact + ao, _ = Contact.objects.get_or_create( + first_name="Testy", + last_name="Tester", + title="Chief Tester", + email="testy@town.com", + phone="(201) 555 5555", ) + you, _ = Contact.objects.get_or_create( + first_name="Testy you", + last_name="Tester you", + title="Admin Tester", + email="testy-admin@town.com", + phone="(201) 555 5556", + ) + other, _ = Contact.objects.get_or_create( + first_name="Testy2", + last_name="Tester2", + title="Another Tester", + email="testy2@town.com", + phone="(201) 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, + submitter=you, + creator=self.user, + status="started", + ) + application.other_contacts.add(other) + + # prime the form by visiting /edit + 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] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + other_contacts_page = self.app.get(reverse("application:other_contacts")) + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + other_contacts_form = other_contacts_page.forms[0] + + # Minimal check to ensure the form is loaded + self.assertEqual(other_contacts_form["other_contacts-0-first_name"].value, "Testy2") + + # # Create an instance of the formset + # formset = OtherContactsFormSet() + + # # Check that there is initially one form in the formset + # self.assertEqual(len(formset.forms), 1) + + # # Simulate adding a form by increasing the 'extra' parameter + # formset.extra += 2 + # formset = OtherContactsFormSet() + + # # Check that there are now two forms in the formset + # self.assertEqual(len(formset.forms), 2) + + + + # # # Mark the first dude for deletion + # # other_contacts_form.set("other_contacts-0-DELETE", "on") + + # # # Submit the form + # # other_contacts_form.submit() + # # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + # # # Verify that the first dude was deleted + # # application = DomainApplication.objects.get() + # # # self.assertEqual(application.other_contacts.count(), 1) + # # # self.assertEqual(application.other_contacts.first().first_name, "Testy3") - Contact.objects.all().delete() - DomainApplication.objects.all().delete() def test_application_about_your_organiztion_interstate(self): """Special districts have to answer an additional question.""" diff --git a/src/registrar/views/application.py b/src/registrar/views/application.py index 881590c4f..9659b0873 100644 --- a/src/registrar/views/application.py +++ b/src/registrar/views/application.py @@ -370,6 +370,10 @@ class ApplicationWizard(ApplicationWizardPermissionView, TemplateView): def post(self, request, *args, **kwargs) -> HttpResponse: """This method handles POST requests.""" + + # Log the keys and values of request.POST + for key, value in request.POST.items(): + logger.info("Key: %s, Value: %s", key, value) # which button did the user press? button: str = request.POST.get("submit_button", "") From f2b12e01d09ede57a17bc9543a2e2bc5e51b9f8d Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 9 Jan 2024 13:03:23 -0500 Subject: [PATCH 018/100] Fix DELETE getter in form's clean, add test_delete_other_contact_does_not_allow_zero_contacts, cleanup --- src/registrar/forms/application_wizard.py | 2 +- src/registrar/tests/test_views.py | 127 +++++++++++++++++++--- src/registrar/views/application.py | 4 - 3 files changed, 111 insertions(+), 22 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index c85d357c1..e2946b3b7 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -672,7 +672,7 @@ class OtherContactsForm(RegistrarForm): deletion by formset to be considered valid even though certain required fields have not passed field validation """ - if self.form_data_marked_for_deletion or self.cleaned_data["DELETE"]: + if self.form_data_marked_for_deletion or self.cleaned_data.get("DELETE"): # clear any errors raised by the form fields # (before this clean() method is run, each field # performs its own clean, which could result in diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index ec4a51b98..ba2cfe345 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1086,8 +1086,8 @@ class DomainApplicationTests(TestWithUser, WebTest): This formset uses the DJANGO DELETE widget. We'll test that by setting 2 contacts on an application, loading the form and marking one contact up for deletion.""" - # Populate the database with a domain application that - # has 1 "other contact" assigned to it + # Populate the database with a domain application that + # has 2 "other contact" assigned to it # We'll do it from scratch so we can reuse the other contact ao, _ = Contact.objects.get_or_create( first_name="Testy", @@ -1164,9 +1164,81 @@ class DomainApplicationTests(TestWithUser, WebTest): application = DomainApplication.objects.get() self.assertEqual(application.other_contacts.count(), 1) self.assertEqual(application.other_contacts.first().first_name, "Testy3") + + def test_delete_other_contact_does_not_allow_zero_contacts(self): + """Delete Other Contact does not allow submission with zero contacts.""" + # Populate the database with a domain application that + # has 1 "other contact" assigned to it + # We'll do it from scratch so we can reuse the other contact + ao, _ = Contact.objects.get_or_create( + first_name="Testy", + last_name="Tester", + title="Chief Tester", + email="testy@town.com", + phone="(201) 555 5555", + ) + you, _ = Contact.objects.get_or_create( + first_name="Testy you", + last_name="Tester you", + title="Admin Tester", + email="testy-admin@town.com", + phone="(201) 555 5556", + ) + other, _ = Contact.objects.get_or_create( + first_name="Testy2", + last_name="Tester2", + title="Another Tester", + email="testy2@town.com", + phone="(201) 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, + submitter=you, + creator=self.user, + status="started", + ) + application.other_contacts.add(other) + + # prime the form by visiting /edit + 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] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + other_contacts_page = self.app.get(reverse("application:other_contacts")) + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + other_contacts_form = other_contacts_page.forms[0] + + # Minimal check to ensure the form is loaded + self.assertEqual(other_contacts_form["other_contacts-0-first_name"].value, "Testy2") + + # Mark the first dude for deletion + other_contacts_form.set("other_contacts-0-DELETE", "on") + + # Submit the form + other_contacts_form.submit() + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + # Verify that the contact was not deleted + application = DomainApplication.objects.get() + self.assertEqual(application.other_contacts.count(), 1) + self.assertEqual(application.other_contacts.first().first_name, "Testy2") def test_delete_other_contact_sets_visible_empty_form_as_required_after_failed_submit(self): - """When you + """When you: 1. add an empty contact, 2. delete existing contacts, 3. then submit, @@ -1227,26 +1299,47 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form = other_contacts_page.forms[0] + + # other_contacts_form["other_contacts-has_other_contacts"] = "True" + # other_contacts_form.set("other_contacts-0-first_name", "") + # other_contacts_page = other_contacts_page.forms[0].submit() + # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + # Print the content to inspect the HTML + # print(other_contacts_page.content.decode('utf-8')) + + # other_contacts_form = other_contacts_page.forms[0] + + # Access the "Add another contact" button and click it + # other_contacts_page = other_contacts_page.click('#add-form', index=0) + # Minimal check to ensure the form is loaded self.assertEqual(other_contacts_form["other_contacts-0-first_name"].value, "Testy2") - # # Create an instance of the formset - # formset = OtherContactsFormSet() - - # # Check that there is initially one form in the formset - # self.assertEqual(len(formset.forms), 1) - - # # Simulate adding a form by increasing the 'extra' parameter - # formset.extra += 2 - # formset = OtherContactsFormSet() + # Get the formset from the response context + formset = other_contacts_page.context['forms'][1] # Replace with the actual context variable name - # # Check that there are now two forms in the formset - # self.assertEqual(len(formset.forms), 2) + # Check the initial number of forms in the formset + initial_form_count = formset.total_form_count() - + print(f'initial_form_count {initial_form_count}') - # # # Mark the first dude for deletion - # # other_contacts_form.set("other_contacts-0-DELETE", "on") + # Add a new form to the formset data + formset_data = formset.management_form.initial + formset_data['TOTAL_FORMS'] = initial_form_count + 1 # Increase the total form count + + print(f"initial_form_count {formset_data['TOTAL_FORMS']}") + + formset.extra = 1 + + other_contacts_form_0 = formset[0] + other_contacts_form_1 = formset[1] + + print(other_contacts_page.content.decode('utf-8')) + + other_contacts_form_1.set("other_contacts-1-first_name", "Rachid") + + # self.assertEqual(other_contacts_form["other_contacts-1-first_name"].value, "") # # # Submit the form # # other_contacts_form.submit() diff --git a/src/registrar/views/application.py b/src/registrar/views/application.py index 9659b0873..881590c4f 100644 --- a/src/registrar/views/application.py +++ b/src/registrar/views/application.py @@ -370,10 +370,6 @@ class ApplicationWizard(ApplicationWizardPermissionView, TemplateView): def post(self, request, *args, **kwargs) -> HttpResponse: """This method handles POST requests.""" - - # Log the keys and values of request.POST - for key, value in request.POST.items(): - logger.info("Key: %s, Value: %s", key, value) # which button did the user press? button: str = request.POST.get("submit_button", "") From 3895a04879e46f349c1ff6058fb046910fba28a1 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 9 Jan 2024 13:17:43 -0500 Subject: [PATCH 019/100] cleanup and lint --- src/registrar/forms/application_wizard.py | 26 ++++++----- src/registrar/tests/test_views.py | 55 +++++++++++------------ src/registrar/views/application.py | 6 +-- 3 files changed, 43 insertions(+), 44 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index e2946b3b7..f7febead7 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -17,6 +17,7 @@ from registrar.utility import errors logger = logging.getLogger(__name__) + class RegistrarForm(forms.Form): """ A common set of methods and configuration. @@ -94,14 +95,14 @@ class RegistrarFormSet(forms.BaseFormSet): Hint: Subclass should call `self._to_database(...)`. """ raise NotImplementedError - + def test_if_more_than_one_join(self, db_obj, rel, related_name): """Helper for finding whether an object is joined more than once.""" # threshold is the number of related objects that are acceptable # when determining if related objects exist. threshold is 0 for most # relationships. if the relationship is related_name, we know that # there is already exactly 1 acceptable relationship (the one we are - # attempting to delete), so the threshold is 1 + # attempting to delete), so the threshold is 1 threshold = 1 if rel == related_name else 0 # Raise a KeyError if rel is not a defined field on the db_obj model @@ -640,7 +641,7 @@ class OtherContactsForm(RegistrarForm): label="Email", error_messages={ "required": ("Enter an email address in the required format, like name@example.com."), - "invalid": ("Enter an email address in the required format, like name@example.com.") + "invalid": ("Enter an email address in the required format, like name@example.com."), }, ) phone = PhoneNumberField( @@ -659,7 +660,7 @@ class OtherContactsForm(RegistrarForm): """ self.form_data_marked_for_deletion = False super().__init__(*args, **kwargs) - self.empty_permitted=False + self.empty_permitted = False def mark_form_for_deletion(self): self.form_data_marked_for_deletion = True @@ -668,8 +669,8 @@ 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 deleted by user or marked for - deletion by formset to be considered valid even though certain required fields have + In this override, allow for a form which is deleted by user or marked for + deletion by formset to be considered valid even though certain required fields have not passed field validation """ if self.form_data_marked_for_deletion or self.cleaned_data.get("DELETE"): @@ -694,9 +695,9 @@ class OtherContactsForm(RegistrarForm): class BaseOtherContactsFormSet(RegistrarFormSet): """ FormSet for Other Contacts - + There are two conditions by which a form in the formset can be marked for deletion. - One is if the user clicks 'DELETE' button, and this is submitted in the form. The + One is if the user clicks 'DELETE' button, and this is submitted in the form. The other is if the YesNo form, which is submitted with this formset, is set to No; in this case, all forms in formset are marked for deletion. Both of these conditions must co-exist. @@ -705,6 +706,7 @@ class BaseOtherContactsFormSet(RegistrarFormSet): tested and handled; this is configured with REVERSE_JOINS, which is an array of strings representing the relationships between contact model and other models. """ + JOIN = "other_contacts" REVERSE_JOINS = [ "user", @@ -718,7 +720,7 @@ class BaseOtherContactsFormSet(RegistrarFormSet): def get_deletion_widget(self): return forms.HiddenInput(attrs={"class": "deletion"}) - + def __init__(self, *args, **kwargs): """ Override __init__ for RegistrarFormSet. @@ -737,14 +739,14 @@ class BaseOtherContactsFormSet(RegistrarFormSet): Implements should_delete method from BaseFormSet. """ return self.formset_data_marked_for_deletion or cleaned.get("DELETE", False) - + def pre_create(self, db_obj, cleaned): """Code to run before an item in the formset is created in the database.""" # remove DELETE from cleaned if "DELETE" in cleaned: - cleaned.pop('DELETE') + cleaned.pop("DELETE") return cleaned - + def to_database(self, obj: DomainApplication): self._to_database(obj, self.JOIN, self.REVERSE_JOINS, self.should_delete, self.pre_update, self.pre_create) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index ba2cfe345..4f36debaa 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -6,7 +6,6 @@ from django.test import Client, TestCase from django.urls import reverse from django.contrib.auth import get_user_model -from registrar.forms.application_wizard import OtherContactsFormSet from .common import MockEppLib, MockSESClient, completed_application, create_user # type: ignore from django_webtest import WebTest # type: ignore import boto3_mocking # type: ignore @@ -1083,7 +1082,7 @@ class DomainApplicationTests(TestWithUser, WebTest): def test_delete_other_contact(self): """Other contacts can be deleted after being saved to database. - + This formset uses the DJANGO DELETE widget. We'll test that by setting 2 contacts on an application, loading the form and marking one contact up for deletion.""" # Populate the database with a domain application that @@ -1148,7 +1147,7 @@ class DomainApplicationTests(TestWithUser, WebTest): self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) other_contacts_form = other_contacts_page.forms[0] - + # Minimal check to ensure the form is loaded with both other contacts self.assertEqual(other_contacts_form["other_contacts-0-first_name"].value, "Testy2") self.assertEqual(other_contacts_form["other_contacts-1-first_name"].value, "Testy3") @@ -1159,12 +1158,12 @@ class DomainApplicationTests(TestWithUser, WebTest): # Submit the form other_contacts_form.submit() self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - + # Verify that the first dude was deleted application = DomainApplication.objects.get() self.assertEqual(application.other_contacts.count(), 1) self.assertEqual(application.other_contacts.first().first_name, "Testy3") - + def test_delete_other_contact_does_not_allow_zero_contacts(self): """Delete Other Contact does not allow submission with zero contacts.""" # Populate the database with a domain application that @@ -1221,7 +1220,7 @@ class DomainApplicationTests(TestWithUser, WebTest): self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) other_contacts_form = other_contacts_page.forms[0] - + # Minimal check to ensure the form is loaded self.assertEqual(other_contacts_form["other_contacts-0-first_name"].value, "Testy2") @@ -1231,19 +1230,20 @@ class DomainApplicationTests(TestWithUser, WebTest): # Submit the form other_contacts_form.submit() self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - + # Verify that the contact was not deleted application = DomainApplication.objects.get() self.assertEqual(application.other_contacts.count(), 1) self.assertEqual(application.other_contacts.first().first_name, "Testy2") + @skip("Can't figure out how to make this work") def test_delete_other_contact_sets_visible_empty_form_as_required_after_failed_submit(self): """When you: 1. add an empty contact, 2. delete existing contacts, - 3. then submit, + 3. then submit, The forms on page reload shows all the required fields and their errors.""" - + # Populate the database with a domain application that # has 1 "other contact" assigned to it # We'll do it from scratch so we can reuse the other contact @@ -1298,58 +1298,55 @@ class DomainApplicationTests(TestWithUser, WebTest): self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) other_contacts_form = other_contacts_page.forms[0] - - + # other_contacts_form["other_contacts-has_other_contacts"] = "True" # other_contacts_form.set("other_contacts-0-first_name", "") # other_contacts_page = other_contacts_page.forms[0].submit() # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - + # Print the content to inspect the HTML # print(other_contacts_page.content.decode('utf-8')) - + # other_contacts_form = other_contacts_page.forms[0] # Access the "Add another contact" button and click it # other_contacts_page = other_contacts_page.click('#add-form', index=0) - + # Minimal check to ensure the form is loaded self.assertEqual(other_contacts_form["other_contacts-0-first_name"].value, "Testy2") - + # Get the formset from the response context - formset = other_contacts_page.context['forms'][1] # Replace with the actual context variable name + formset = other_contacts_page.context["forms"][1] # Replace with the actual context variable name # Check the initial number of forms in the formset initial_form_count = formset.total_form_count() - - print(f'initial_form_count {initial_form_count}') + + print(f"initial_form_count {initial_form_count}") # Add a new form to the formset data formset_data = formset.management_form.initial - formset_data['TOTAL_FORMS'] = initial_form_count + 1 # Increase the total form count - + formset_data["TOTAL_FORMS"] = initial_form_count + 1 # Increase the total form count + print(f"initial_form_count {formset_data['TOTAL_FORMS']}") - + formset.extra = 1 - - other_contacts_form_0 = formset[0] + other_contacts_form_1 = formset[1] - - print(other_contacts_page.content.decode('utf-8')) - + + print(other_contacts_page.content.decode("utf-8")) + other_contacts_form_1.set("other_contacts-1-first_name", "Rachid") - + # self.assertEqual(other_contacts_form["other_contacts-1-first_name"].value, "") # # # Submit the form # # other_contacts_form.submit() # # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - + # # # Verify that the first dude was deleted # # application = DomainApplication.objects.get() # # # self.assertEqual(application.other_contacts.count(), 1) # # # self.assertEqual(application.other_contacts.first().first_name, "Testy3") - def test_application_about_your_organiztion_interstate(self): """Special districts have to answer an additional question.""" diff --git a/src/registrar/views/application.py b/src/registrar/views/application.py index 881590c4f..4102935f8 100644 --- a/src/registrar/views/application.py +++ b/src/registrar/views/application.py @@ -486,7 +486,7 @@ class YourContact(ApplicationWizard): class OtherContacts(ApplicationWizard): template_name = "application_other_contacts.html" forms = [forms.OtherContactsYesNoForm, forms.OtherContactsFormSet, forms.NoOtherContactsForm] - + def is_valid(self, forms: list) -> bool: """Overrides default behavior defined in ApplicationWizard. Depending on value in other_contacts_yes_no_form, marks forms in @@ -500,9 +500,9 @@ class OtherContacts(ApplicationWizard): # set all the required other_contact fields as necessary since new forms # were added through javascript for form in forms[1].forms: - for _, field in form.fields.items(): + for field_item, field in form.fields.items(): if field.required: - field.widget.attrs['required'] = 'required' + field.widget.attrs["required"] = "required" all_forms_valid = True # test first for yes_no_form validity From 89431b111db153f098bd2b946ab6988ea101e75f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 11:41:55 -0700 Subject: [PATCH 020/100] Unify error messages under one banner --- src/api/views.py | 33 ++---- src/registrar/forms/application_wizard.py | 28 +---- src/registrar/models/utility/domain_helper.py | 102 ++++++++++++++++-- src/registrar/utility/errors.py | 2 + 4 files changed, 107 insertions(+), 58 deletions(-) diff --git a/src/api/views.py b/src/api/views.py index e40924708..4bec29b80 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -1,7 +1,7 @@ """Internal API views""" from django.apps import apps from django.views.decorators.http import require_http_methods -from django.http import HttpResponse, JsonResponse +from django.http import HttpResponse from django.utils.safestring import mark_safe from registrar.templatetags.url_helpers import public_site_url @@ -71,6 +71,7 @@ def check_domain_available(domain): a match. If check fails, throws a RegistryError. """ Domain = apps.get_model("registrar.Domain") + if domain.endswith(".gov"): return Domain.available(domain) else: @@ -86,29 +87,15 @@ def available(request, domain=""): Response is a JSON dictionary with the key "available" and value true or false. """ + Domain = apps.get_model("registrar.Domain") domain = request.GET.get("domain", "") - DraftDomain = apps.get_model("registrar.DraftDomain") - if domain is None or domain.strip() == "": - # TODO - change this... should it be the regular required? - return JsonResponse({"available": False, "code": "invalid", "message": "This field is required"}) - # validate that the given domain could be a domain name and fail early if - # not. - if not (DraftDomain.string_could_be_domain(domain) or DraftDomain.string_could_be_domain(domain + ".gov")): - print(f"What is the domain at this point? {domain}") - if "." in domain: - return JsonResponse({"available": False, "code": "invalid", "message": DOMAIN_API_MESSAGES["extra_dots"]}) - else: - return JsonResponse({"available": False, "code": "invalid", "message": DOMAIN_API_MESSAGES["invalid"]}) - # a domain is available if it is NOT in the list of current domains - try: - if check_domain_available(domain): - return JsonResponse({"available": True, "code": "success", "message": DOMAIN_API_MESSAGES["success"]}) - else: - return JsonResponse( - {"available": False, "code": "unavailable", "message": DOMAIN_API_MESSAGES["unavailable"]} - ) - except Exception: - return JsonResponse({"available": False, "code": "error", "message": DOMAIN_API_MESSAGES["error"]}) + + json_response = Domain.validate_and_handle_errors( + domain=domain, + error_return_type="JSON_RESPONSE", + display_success=True, + ) + return json_response @require_http_methods(["GET"]) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index fcf6bda7a..00f832d59 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -384,17 +384,8 @@ CurrentSitesFormSet = forms.formset_factory( class AlternativeDomainForm(RegistrarForm): def clean_alternative_domain(self): """Validation code for domain names.""" - try: - requested = self.cleaned_data.get("alternative_domain", None) - validated = DraftDomain.validate(requested, blank_ok=True) - except errors.ExtraDotsError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["extra_dots"], code="extra_dots") - except errors.DomainUnavailableError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["unavailable"], code="unavailable") - except errors.RegistrySystemError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["error"], code="error") - except ValueError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["invalid"], code="invalid") + requested = self.cleaned_data.get("alternative_domain", None) + validated = DraftDomain.validate_and_handle_errors(requested, "FORM_VALIDATION_ERROR") return validated alternative_domain = forms.CharField( @@ -469,19 +460,8 @@ class DotGovDomainForm(RegistrarForm): def clean_requested_domain(self): """Validation code for domain names.""" - try: - requested = self.cleaned_data.get("requested_domain", None) - validated = DraftDomain.validate(requested) - except errors.BlankValueError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["required"], code="required") - except errors.ExtraDotsError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["extra_dots"], code="extra_dots") - except errors.DomainUnavailableError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["unavailable"], code="unavailable") - except errors.RegistrySystemError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["error"], code="error") - except ValueError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["invalid"], code="invalid") + requested = self.cleaned_data.get("requested_domain", None) + validated = DraftDomain.validate_and_handle_errors(requested, "FORM_VALIDATION_ERROR") return validated requested_domain = forms.CharField(label="What .gov domain do you want?") diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index e43661b1d..7993d0f90 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -1,9 +1,16 @@ +from enum import Enum import re -from api.views import check_domain_available +from django import forms +from django.http import JsonResponse + +from api.views import DOMAIN_API_MESSAGES, check_domain_available from registrar.utility import errors from epplibwrapper.errors import RegistryError +class ValidationErrorReturnType(Enum): + JSON_RESPONSE = "JSON_RESPONSE" + FORM_VALIDATION_ERROR = "FORM_VALIDATION_ERROR" class DomainHelper: """Utility functions and constants for domain names.""" @@ -28,16 +35,10 @@ class DomainHelper: if domain is None: raise errors.BlankValueError() if not isinstance(domain, str): - raise ValueError("Domain name must be a string") - domain = domain.lower().strip() - if domain == "" and not blank_ok: - raise errors.BlankValueError() - if domain.endswith(".gov"): - domain = domain[:-4] - if "." in domain: - raise errors.ExtraDotsError() - if not DomainHelper.string_could_be_domain(domain + ".gov"): - raise ValueError() + raise errors.InvalidDomainError("Domain name must be a string") + + domain = DomainHelper._parse_domain_string(domain, blank_ok) + try: if not check_domain_available(domain): raise errors.DomainUnavailableError() @@ -45,6 +46,85 @@ class DomainHelper: raise errors.RegistrySystemError() from err return domain + @staticmethod + def _parse_domain_string(domain: str, blank_ok) -> str: + """Parses '.gov' out of the domain_name string, and does some validation on it""" + domain = domain.lower().strip() + + if domain == "" and not blank_ok: + raise errors.BlankValueError() + + if domain.endswith(".gov"): + domain = domain[:-4] + + if "." in domain: + raise errors.ExtraDotsError() + + if not DomainHelper.string_could_be_domain(domain + ".gov"): + raise errors.InvalidDomainError() + + @classmethod + def validate_and_handle_errors(cls, domain: str, error_return_type: str, display_success: bool = False): + """Runs validate() and catches possible exceptions.""" + try: + validated = cls.validate(domain) + except errors.BlankValueError: + return DomainHelper._return_form_error_or_json_response( + error_return_type, code="required" + ) + except errors.ExtraDotsError: + return DomainHelper._return_form_error_or_json_response( + error_return_type, code="extra_dots" + ) + except errors.DomainUnavailableError: + return DomainHelper._return_form_error_or_json_response( + error_return_type, code="unavailable" + ) + except errors.RegistrySystemError: + return DomainHelper._return_form_error_or_json_response( + error_return_type, code="error" + ) + except errors.InvalidDomainError: + return DomainHelper._return_form_error_or_json_response( + error_return_type, code="invalid" + ) + except Exception: + return DomainHelper._return_form_error_or_json_response( + error_return_type, code="error" + ) + else: + if display_success: + return DomainHelper._return_form_error_or_json_response( + error_return_type, code="success", available=True + ) + else: + return validated + + @staticmethod + def _return_form_error_or_json_response(return_type, code, available=False): + print(f"What is the code? {code}") + if return_type == "JSON_RESPONSE": + print("in the return context") + return JsonResponse( + {"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]} + ) + + if return_type == "FORM_VALIDATION_ERROR": + raise forms.ValidationError(DOMAIN_API_MESSAGES[code], code=code) + + # Why is this not working?? + """ + match return_type: + case ValidationErrorReturnType.FORM_VALIDATION_ERROR: + raise forms.ValidationError(DOMAIN_API_MESSAGES[code], code=code) + case ValidationErrorReturnType.JSON_RESPONSE: + return JsonResponse( + {"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]} + ) + case _: + raise ValueError("Invalid return type specified") + """ + @classmethod def sld(cls, domain: str): """ diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 455419236..199997cc2 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -16,6 +16,8 @@ class DomainUnavailableError(ValueError): class RegistrySystemError(ValueError): pass +class InvalidDomainError(ValueError): + pass class ActionNotAllowed(Exception): """User accessed an action that is not From a7fa332d36827f8bfc97b90bba493477e11e58d1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 12:17:33 -0700 Subject: [PATCH 021/100] Update domain_helper.py --- src/registrar/models/utility/domain_helper.py | 27 ++++++------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 7993d0f90..018087f7b 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -37,18 +37,6 @@ class DomainHelper: if not isinstance(domain, str): raise errors.InvalidDomainError("Domain name must be a string") - domain = DomainHelper._parse_domain_string(domain, blank_ok) - - try: - if not check_domain_available(domain): - raise errors.DomainUnavailableError() - except RegistryError as err: - raise errors.RegistrySystemError() from err - return domain - - @staticmethod - def _parse_domain_string(domain: str, blank_ok) -> str: - """Parses '.gov' out of the domain_name string, and does some validation on it""" domain = domain.lower().strip() if domain == "" and not blank_ok: @@ -63,6 +51,13 @@ class DomainHelper: if not DomainHelper.string_could_be_domain(domain + ".gov"): raise errors.InvalidDomainError() + try: + if not check_domain_available(domain): + raise errors.DomainUnavailableError() + except RegistryError as err: + raise errors.RegistrySystemError() from err + return domain + @classmethod def validate_and_handle_errors(cls, domain: str, error_return_type: str, display_success: bool = False): """Runs validate() and catches possible exceptions.""" @@ -88,10 +83,6 @@ class DomainHelper: return DomainHelper._return_form_error_or_json_response( error_return_type, code="invalid" ) - except Exception: - return DomainHelper._return_form_error_or_json_response( - error_return_type, code="error" - ) else: if display_success: return DomainHelper._return_form_error_or_json_response( @@ -102,11 +93,9 @@ class DomainHelper: @staticmethod def _return_form_error_or_json_response(return_type, code, available=False): - print(f"What is the code? {code}") if return_type == "JSON_RESPONSE": - print("in the return context") return JsonResponse( - {"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]} + {"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]} ) if return_type == "FORM_VALIDATION_ERROR": From 5b2eeee54771528df541bfe49a6e475de699ffbf Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 12:27:38 -0700 Subject: [PATCH 022/100] Add enums.py --- .../utility/extra_transition_domain_helper.py | 4 ++- .../commands/utility/terminal_helper.py | 21 +-------------- src/registrar/models/utility/domain_helper.py | 9 ++----- src/registrar/utility/enums.py | 27 +++++++++++++++++++ 4 files changed, 33 insertions(+), 28 deletions(-) create mode 100644 src/registrar/utility/enums.py diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index 54f68d5c8..755c9b98a 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -11,6 +11,7 @@ import os import sys from typing import Dict, List from django.core.paginator import Paginator +from registrar.utility.enums import LogCode from registrar.models.transition_domain import TransitionDomain from registrar.management.commands.utility.load_organization_error import ( LoadOrganizationError, @@ -28,7 +29,8 @@ from .epp_data_containers import ( ) from .transition_domain_arguments import TransitionDomainArguments -from .terminal_helper import TerminalColors, TerminalHelper, LogCode +from .terminal_helper import TerminalColors, TerminalHelper + logger = logging.getLogger(__name__) diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index 85bfc8193..3ae9ff3cd 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -1,29 +1,10 @@ -from enum import Enum import logging import sys from typing import List - +from registrar.utility.enums import LogCode logger = logging.getLogger(__name__) -class LogCode(Enum): - """Stores the desired log severity - - Overview of error codes: - - 1 ERROR - - 2 WARNING - - 3 INFO - - 4 DEBUG - - 5 DEFAULT - """ - - ERROR = 1 - WARNING = 2 - INFO = 3 - DEBUG = 4 - DEFAULT = 5 - - class TerminalColors: """Colors for terminal outputs (makes reading the logs WAY easier)""" diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 018087f7b..28eaa391e 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -8,9 +8,6 @@ from api.views import DOMAIN_API_MESSAGES, check_domain_available from registrar.utility import errors from epplibwrapper.errors import RegistryError -class ValidationErrorReturnType(Enum): - JSON_RESPONSE = "JSON_RESPONSE" - FORM_VALIDATION_ERROR = "FORM_VALIDATION_ERROR" class DomainHelper: """Utility functions and constants for domain names.""" @@ -102,17 +99,15 @@ class DomainHelper: raise forms.ValidationError(DOMAIN_API_MESSAGES[code], code=code) # Why is this not working?? - """ match return_type: - case ValidationErrorReturnType.FORM_VALIDATION_ERROR: + case ValidationErrorReturnType.FORM_VALIDATION_ERROR.value: raise forms.ValidationError(DOMAIN_API_MESSAGES[code], code=code) - case ValidationErrorReturnType.JSON_RESPONSE: + case ValidationErrorReturnType.JSON_RESPONSE.value: return JsonResponse( {"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]} ) case _: raise ValueError("Invalid return type specified") - """ @classmethod def sld(cls, domain: str): diff --git a/src/registrar/utility/enums.py b/src/registrar/utility/enums.py new file mode 100644 index 000000000..64411f33e --- /dev/null +++ b/src/registrar/utility/enums.py @@ -0,0 +1,27 @@ +"""Used for holding various enums""" + +from enum import Enum + + +class ValidationErrorReturnType(Enum): + """Determines the return value of the validate_and_handle_errors class""" + JSON_RESPONSE = "JSON_RESPONSE" + FORM_VALIDATION_ERROR = "FORM_VALIDATION_ERROR" + + +class LogCode(Enum): + """Stores the desired log severity + + Overview of error codes: + - 1 ERROR + - 2 WARNING + - 3 INFO + - 4 DEBUG + - 5 DEFAULT + """ + + ERROR = 1 + WARNING = 2 + INFO = 3 + DEBUG = 4 + DEFAULT = 5 \ No newline at end of file From 91ed4a598c73e330ae2dff5159a135a91a144316 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 12:30:23 -0700 Subject: [PATCH 023/100] Hook up enum --- src/api/views.py | 3 ++- src/registrar/forms/application_wizard.py | 5 +++-- src/registrar/models/utility/domain_helper.py | 18 +++++------------- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/api/views.py b/src/api/views.py index 4bec29b80..b89e2629d 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -5,6 +5,7 @@ from django.http import HttpResponse from django.utils.safestring import mark_safe from registrar.templatetags.url_helpers import public_site_url +from registrar.utility.enums import ValidationErrorReturnType from registrar.utility.errors import GenericError, GenericErrorCodes import requests @@ -92,7 +93,7 @@ def available(request, domain=""): json_response = Domain.validate_and_handle_errors( domain=domain, - error_return_type="JSON_RESPONSE", + error_return_type=ValidationErrorReturnType.JSON_RESPONSE, display_success=True, ) return json_response diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 00f832d59..aa583a10c 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -13,6 +13,7 @@ from api.views import DOMAIN_API_MESSAGES from registrar.models import Contact, DomainApplication, DraftDomain, Domain from registrar.templatetags.url_helpers import public_site_url from registrar.utility import errors +from registrar.utility.enums import ValidationErrorReturnType logger = logging.getLogger(__name__) @@ -385,7 +386,7 @@ class AlternativeDomainForm(RegistrarForm): def clean_alternative_domain(self): """Validation code for domain names.""" requested = self.cleaned_data.get("alternative_domain", None) - validated = DraftDomain.validate_and_handle_errors(requested, "FORM_VALIDATION_ERROR") + validated = DraftDomain.validate_and_handle_errors(requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR) return validated alternative_domain = forms.CharField( @@ -461,7 +462,7 @@ class DotGovDomainForm(RegistrarForm): def clean_requested_domain(self): """Validation code for domain names.""" requested = self.cleaned_data.get("requested_domain", None) - validated = DraftDomain.validate_and_handle_errors(requested, "FORM_VALIDATION_ERROR") + validated = DraftDomain.validate_and_handle_errors(requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR) return validated requested_domain = forms.CharField(label="What .gov domain do you want?") diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 28eaa391e..cf2369567 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -7,6 +7,7 @@ from django.http import JsonResponse from api.views import DOMAIN_API_MESSAGES, check_domain_available from registrar.utility import errors from epplibwrapper.errors import RegistryError +from registrar.utility.enums import ValidationErrorReturnType class DomainHelper: @@ -56,7 +57,7 @@ class DomainHelper: return domain @classmethod - def validate_and_handle_errors(cls, domain: str, error_return_type: str, display_success: bool = False): + def validate_and_handle_errors(cls, domain, error_return_type, display_success = False): """Runs validate() and catches possible exceptions.""" try: validated = cls.validate(domain) @@ -89,20 +90,11 @@ class DomainHelper: return validated @staticmethod - def _return_form_error_or_json_response(return_type, code, available=False): - if return_type == "JSON_RESPONSE": - return JsonResponse( - {"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]} - ) - - if return_type == "FORM_VALIDATION_ERROR": - raise forms.ValidationError(DOMAIN_API_MESSAGES[code], code=code) - - # Why is this not working?? + def _return_form_error_or_json_response(return_type: ValidationErrorReturnType, code, available=False): match return_type: - case ValidationErrorReturnType.FORM_VALIDATION_ERROR.value: + case ValidationErrorReturnType.FORM_VALIDATION_ERROR: raise forms.ValidationError(DOMAIN_API_MESSAGES[code], code=code) - case ValidationErrorReturnType.JSON_RESPONSE.value: + case ValidationErrorReturnType.JSON_RESPONSE: return JsonResponse( {"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]} ) From 7e6a1c4188b1e64282aafe747ee644156615e390 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 9 Jan 2024 14:45:13 -0500 Subject: [PATCH 024/100] fixed test_delete_other_contact_sets_visible_empty_form_as_required_after_failed_submit --- src/registrar/tests/test_views.py | 54 ++++++------------------------- 1 file changed, 10 insertions(+), 44 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 4f36debaa..97b864e22 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1236,7 +1236,7 @@ class DomainApplicationTests(TestWithUser, WebTest): self.assertEqual(application.other_contacts.count(), 1) self.assertEqual(application.other_contacts.first().first_name, "Testy2") - @skip("Can't figure out how to make this work") + # @skip("Can't figure out how to make this work") def test_delete_other_contact_sets_visible_empty_form_as_required_after_failed_submit(self): """When you: 1. add an empty contact, @@ -1299,54 +1299,20 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form = other_contacts_page.forms[0] - # other_contacts_form["other_contacts-has_other_contacts"] = "True" - # other_contacts_form.set("other_contacts-0-first_name", "") - # other_contacts_page = other_contacts_page.forms[0].submit() - # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - - # Print the content to inspect the HTML - # print(other_contacts_page.content.decode('utf-8')) - - # other_contacts_form = other_contacts_page.forms[0] - - # Access the "Add another contact" button and click it - # other_contacts_page = other_contacts_page.click('#add-form', index=0) - # Minimal check to ensure the form is loaded self.assertEqual(other_contacts_form["other_contacts-0-first_name"].value, "Testy2") - # Get the formset from the response context - formset = other_contacts_page.context["forms"][1] # Replace with the actual context variable name + # Set total forms to 2 indicating an additional formset was added. + # Submit no data though for the second formset. + # Set the first formset to be deleted. + other_contacts_form["other_contacts-TOTAL_FORMS"] = "2" + other_contacts_form.set("other_contacts-0-DELETE", "on") - # Check the initial number of forms in the formset - initial_form_count = formset.total_form_count() + response = other_contacts_form.submit() - print(f"initial_form_count {initial_form_count}") - - # Add a new form to the formset data - formset_data = formset.management_form.initial - formset_data["TOTAL_FORMS"] = initial_form_count + 1 # Increase the total form count - - print(f"initial_form_count {formset_data['TOTAL_FORMS']}") - - formset.extra = 1 - - other_contacts_form_1 = formset[1] - - print(other_contacts_page.content.decode("utf-8")) - - other_contacts_form_1.set("other_contacts-1-first_name", "Rachid") - - # self.assertEqual(other_contacts_form["other_contacts-1-first_name"].value, "") - - # # # Submit the form - # # other_contacts_form.submit() - # # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - - # # # Verify that the first dude was deleted - # # application = DomainApplication.objects.get() - # # # self.assertEqual(application.other_contacts.count(), 1) - # # # self.assertEqual(application.other_contacts.first().first_name, "Testy3") + # Assert that the response presents errors to the user, including to + # Enter the first name ... + self.assertContains(response, "Enter the first name / given name of this contact.") def test_application_about_your_organiztion_interstate(self): """Special districts have to answer an additional question.""" From d48572e1933e09750cc8fe81a4bbff2b63cdfbd2 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 9 Jan 2024 15:08:11 -0500 Subject: [PATCH 025/100] formatting --- src/registrar/tests/test_views.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index f86819c18..eb254580a 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1236,7 +1236,6 @@ class DomainApplicationTests(TestWithUser, WebTest): self.assertEqual(application.other_contacts.count(), 1) self.assertEqual(application.other_contacts.first().first_name, "Testy2") - # @skip("Can't figure out how to make this work") def test_delete_other_contact_sets_visible_empty_form_as_required_after_failed_submit(self): """When you: 1. add an empty contact, From ab6cbb93c645f011b7bbcadf16cfa11ed3f33caf Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 13:31:32 -0700 Subject: [PATCH 026/100] Code cleanup --- src/registrar/forms/application_wizard.py | 7 +- .../commands/utility/terminal_helper.py | 1 + src/registrar/models/utility/domain_helper.py | 72 +++++++++++++------ src/registrar/utility/enums.py | 3 +- src/registrar/utility/errors.py | 2 + 5 files changed, 59 insertions(+), 26 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index aa583a10c..2eb359984 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -8,11 +8,8 @@ from django import forms from django.core.validators import RegexValidator, MaxLengthValidator from django.utils.safestring import mark_safe -from api.views import DOMAIN_API_MESSAGES - from registrar.models import Contact, DomainApplication, DraftDomain, Domain from registrar.templatetags.url_helpers import public_site_url -from registrar.utility import errors from registrar.utility.enums import ValidationErrorReturnType logger = logging.getLogger(__name__) @@ -386,7 +383,9 @@ class AlternativeDomainForm(RegistrarForm): def clean_alternative_domain(self): """Validation code for domain names.""" requested = self.cleaned_data.get("alternative_domain", None) - validated = DraftDomain.validate_and_handle_errors(requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR) + validated = DraftDomain.validate_and_handle_errors( + requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR, prevent_blank=False + ) return validated alternative_domain = forms.CharField( diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index 3ae9ff3cd..c24bd9616 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -2,6 +2,7 @@ import logging import sys from typing import List from registrar.utility.enums import LogCode + logger = logging.getLogger(__name__) diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index cf2369567..eb174d814 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -57,30 +57,44 @@ class DomainHelper: return domain @classmethod - def validate_and_handle_errors(cls, domain, error_return_type, display_success = False): - """Runs validate() and catches possible exceptions.""" + def validate_and_handle_errors(cls, domain, error_return_type, prevent_blank=True, display_success=False): + """ + Validates the provided domain and handles any validation errors. + + This method attempts to validate the domain using the `validate` method. If validation fails, + it catches the exception and returns an appropriate error response. The type of the error response + (JSON response or form validation error) is determined by the `error_return_type` parameter. + + If validation is successful and `display_success` is True, it returns a success response. + Otherwise, it returns the validated domain. + + Args: + domain (str): The domain to validate. + error_return_type (ValidationErrorReturnType): The type of error response to return if validation fails. + prevent_blank (bool, optional): Whether to return an exception if the input is blank. Defaults to True. + display_success (bool, optional): Whether to return a success response if validation is successful. Defaults to False. + + Returns: + The error response if validation fails, + the success response if validation is successful and `display_success` is True, + or the validated domain otherwise. + """ # noqa + try: validated = cls.validate(domain) except errors.BlankValueError: - return DomainHelper._return_form_error_or_json_response( - error_return_type, code="required" - ) + if not prevent_blank: + return DomainHelper._return_form_error_or_json_response(error_return_type, code="required") + else: + return validated except errors.ExtraDotsError: - return DomainHelper._return_form_error_or_json_response( - error_return_type, code="extra_dots" - ) + return DomainHelper._return_form_error_or_json_response(error_return_type, code="extra_dots") except errors.DomainUnavailableError: - return DomainHelper._return_form_error_or_json_response( - error_return_type, code="unavailable" - ) + return DomainHelper._return_form_error_or_json_response(error_return_type, code="unavailable") except errors.RegistrySystemError: - return DomainHelper._return_form_error_or_json_response( - error_return_type, code="error" - ) + return DomainHelper._return_form_error_or_json_response(error_return_type, code="error") except errors.InvalidDomainError: - return DomainHelper._return_form_error_or_json_response( - error_return_type, code="invalid" - ) + return DomainHelper._return_form_error_or_json_response(error_return_type, code="invalid") else: if display_success: return DomainHelper._return_form_error_or_json_response( @@ -88,16 +102,32 @@ class DomainHelper: ) else: return validated - + @staticmethod def _return_form_error_or_json_response(return_type: ValidationErrorReturnType, code, available=False): + """ + Returns an error response based on the `return_type`. + + If `return_type` is `FORM_VALIDATION_ERROR`, raises a form validation error. + If `return_type` is `JSON_RESPONSE`, returns a JSON response with 'available', 'code', and 'message' fields. + If `return_type` is neither, raises a ValueError. + + Args: + return_type (ValidationErrorReturnType): The type of error response. + code (str): The error code for the error message. + available (bool, optional): Availability, only used for JSON responses. Defaults to False. + + Returns: + A JSON response or a form validation error. + + Raises: + ValueError: If `return_type` is neither `FORM_VALIDATION_ERROR` nor `JSON_RESPONSE`. + """ # noqa match return_type: case ValidationErrorReturnType.FORM_VALIDATION_ERROR: raise forms.ValidationError(DOMAIN_API_MESSAGES[code], code=code) case ValidationErrorReturnType.JSON_RESPONSE: - return JsonResponse( - {"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]} - ) + return JsonResponse({"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]}) case _: raise ValueError("Invalid return type specified") diff --git a/src/registrar/utility/enums.py b/src/registrar/utility/enums.py index 64411f33e..6aa4f4044 100644 --- a/src/registrar/utility/enums.py +++ b/src/registrar/utility/enums.py @@ -5,6 +5,7 @@ from enum import Enum class ValidationErrorReturnType(Enum): """Determines the return value of the validate_and_handle_errors class""" + JSON_RESPONSE = "JSON_RESPONSE" FORM_VALIDATION_ERROR = "FORM_VALIDATION_ERROR" @@ -24,4 +25,4 @@ class LogCode(Enum): WARNING = 2 INFO = 3 DEBUG = 4 - DEFAULT = 5 \ No newline at end of file + DEFAULT = 5 diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 199997cc2..bac18d076 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -16,9 +16,11 @@ class DomainUnavailableError(ValueError): class RegistrySystemError(ValueError): pass + class InvalidDomainError(ValueError): pass + class ActionNotAllowed(Exception): """User accessed an action that is not allowed by the current state""" From 6b6aed8f2406e7fa1ed433c41a64fac90922cb62 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 13:36:09 -0700 Subject: [PATCH 027/100] Fix blanks --- src/registrar/forms/application_wizard.py | 2 +- src/registrar/models/utility/domain_helper.py | 11 ++++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 2eb359984..22335c1c8 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -384,7 +384,7 @@ class AlternativeDomainForm(RegistrarForm): """Validation code for domain names.""" requested = self.cleaned_data.get("alternative_domain", None) validated = DraftDomain.validate_and_handle_errors( - requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR, prevent_blank=False + requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR, blank_ok=True ) return validated diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index eb174d814..e4d3c094c 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -57,7 +57,7 @@ class DomainHelper: return domain @classmethod - def validate_and_handle_errors(cls, domain, error_return_type, prevent_blank=True, display_success=False): + def validate_and_handle_errors(cls, domain, error_return_type, blank_ok=False, display_success=False): """ Validates the provided domain and handles any validation errors. @@ -71,7 +71,7 @@ class DomainHelper: Args: domain (str): The domain to validate. error_return_type (ValidationErrorReturnType): The type of error response to return if validation fails. - prevent_blank (bool, optional): Whether to return an exception if the input is blank. Defaults to True. + blank_ok (bool, optional): Whether to return an exception if the input is blank. Defaults to False. display_success (bool, optional): Whether to return a success response if validation is successful. Defaults to False. Returns: @@ -81,12 +81,9 @@ class DomainHelper: """ # noqa try: - validated = cls.validate(domain) + validated = cls.validate(domain, blank_ok) except errors.BlankValueError: - if not prevent_blank: - return DomainHelper._return_form_error_or_json_response(error_return_type, code="required") - else: - return validated + return DomainHelper._return_form_error_or_json_response(error_return_type, code="required") except errors.ExtraDotsError: return DomainHelper._return_form_error_or_json_response(error_return_type, code="extra_dots") except errors.DomainUnavailableError: From ce361cdd3b6a9d682227ff4fba96300fcc1695c0 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 14:07:41 -0700 Subject: [PATCH 028/100] Return tuple and do error mapping --- src/api/views.py | 5 +- src/registrar/forms/application_wizard.py | 4 +- src/registrar/models/utility/domain_helper.py | 46 +++++++++---------- 3 files changed, 25 insertions(+), 30 deletions(-) diff --git a/src/api/views.py b/src/api/views.py index b89e2629d..a98bd88a9 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -91,10 +91,9 @@ def available(request, domain=""): Domain = apps.get_model("registrar.Domain") domain = request.GET.get("domain", "") - json_response = Domain.validate_and_handle_errors( + _, json_response = Domain.validate_and_handle_errors( domain=domain, - error_return_type=ValidationErrorReturnType.JSON_RESPONSE, - display_success=True, + error_return_type=ValidationErrorReturnType.JSON_RESPONSE, ) return json_response diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 22335c1c8..acd1d1cfc 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -383,7 +383,7 @@ class AlternativeDomainForm(RegistrarForm): def clean_alternative_domain(self): """Validation code for domain names.""" requested = self.cleaned_data.get("alternative_domain", None) - validated = DraftDomain.validate_and_handle_errors( + validated, _ = DraftDomain.validate_and_handle_errors( requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR, blank_ok=True ) return validated @@ -461,7 +461,7 @@ class DotGovDomainForm(RegistrarForm): def clean_requested_domain(self): """Validation code for domain names.""" requested = self.cleaned_data.get("requested_domain", None) - validated = DraftDomain.validate_and_handle_errors(requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR) + validated, _ = DraftDomain.validate_and_handle_errors(requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR) return validated requested_domain = forms.CharField(label="What .gov domain do you want?") diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index e4d3c094c..df7aa2b7e 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -57,7 +57,7 @@ class DomainHelper: return domain @classmethod - def validate_and_handle_errors(cls, domain, error_return_type, blank_ok=False, display_success=False): + def validate_and_handle_errors(cls, domain, error_return_type, blank_ok=False): """ Validates the provided domain and handles any validation errors. @@ -65,40 +65,36 @@ class DomainHelper: it catches the exception and returns an appropriate error response. The type of the error response (JSON response or form validation error) is determined by the `error_return_type` parameter. - If validation is successful and `display_success` is True, it returns a success response. - Otherwise, it returns the validated domain. Args: domain (str): The domain to validate. error_return_type (ValidationErrorReturnType): The type of error response to return if validation fails. blank_ok (bool, optional): Whether to return an exception if the input is blank. Defaults to False. - display_success (bool, optional): Whether to return a success response if validation is successful. Defaults to False. - Returns: - The error response if validation fails, - the success response if validation is successful and `display_success` is True, - or the validated domain otherwise. + A tuple of the validated domain name, and the response """ # noqa - + error_map = { + errors.BlankValueError: "required", + errors.ExtraDotsError: "extra_dots", + errors.DomainUnavailableError: "unavailable", + errors.RegistrySystemError: "error", + errors.InvalidDomainError: "invalid", + } + validated = None + response = None try: validated = cls.validate(domain, blank_ok) - except errors.BlankValueError: - return DomainHelper._return_form_error_or_json_response(error_return_type, code="required") - except errors.ExtraDotsError: - return DomainHelper._return_form_error_or_json_response(error_return_type, code="extra_dots") - except errors.DomainUnavailableError: - return DomainHelper._return_form_error_or_json_response(error_return_type, code="unavailable") - except errors.RegistrySystemError: - return DomainHelper._return_form_error_or_json_response(error_return_type, code="error") - except errors.InvalidDomainError: - return DomainHelper._return_form_error_or_json_response(error_return_type, code="invalid") + # Get a list of each possible exception, and the code to return + except tuple(error_map.keys()) as error: + # For each exception, determine which code should be returned + response = DomainHelper._return_form_error_or_json_response( + error_return_type, code=error_map.get(type(error)) + ) else: - if display_success: - return DomainHelper._return_form_error_or_json_response( - error_return_type, code="success", available=True - ) - else: - return validated + response = DomainHelper._return_form_error_or_json_response( + error_return_type, code="success", available=True + ) + return (validated, response) @staticmethod def _return_form_error_or_json_response(return_type: ValidationErrorReturnType, code, available=False): From 05ebe98adb98501130c541ce42fcaa935e3679c3 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 14:18:32 -0700 Subject: [PATCH 029/100] Update domain_helper.py --- src/registrar/models/utility/domain_helper.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index df7aa2b7e..78e27477b 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -91,9 +91,10 @@ class DomainHelper: error_return_type, code=error_map.get(type(error)) ) else: - response = DomainHelper._return_form_error_or_json_response( - error_return_type, code="success", available=True - ) + if error_return_type != ValidationErrorReturnType.FORM_VALIDATION_ERROR: + response = DomainHelper._return_form_error_or_json_response( + error_return_type, code="success", available=True + ) return (validated, response) @staticmethod From 45e994ea05e9f2a92d835ecd42a16b1a1d037e30 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 14:22:50 -0700 Subject: [PATCH 030/100] Linting --- src/api/views.py | 2 +- src/registrar/forms/application_wizard.py | 4 +++- src/registrar/models/utility/domain_helper.py | 1 - 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/api/views.py b/src/api/views.py index a98bd88a9..24960ff1c 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -92,7 +92,7 @@ def available(request, domain=""): domain = request.GET.get("domain", "") _, json_response = Domain.validate_and_handle_errors( - domain=domain, + domain=domain, error_return_type=ValidationErrorReturnType.JSON_RESPONSE, ) return json_response diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index acd1d1cfc..c29225ca6 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -461,7 +461,9 @@ class DotGovDomainForm(RegistrarForm): def clean_requested_domain(self): """Validation code for domain names.""" requested = self.cleaned_data.get("requested_domain", None) - validated, _ = DraftDomain.validate_and_handle_errors(requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR) + validated, _ = DraftDomain.validate_and_handle_errors( + requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR + ) return validated requested_domain = forms.CharField(label="What .gov domain do you want?") diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 78e27477b..58629f213 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -1,4 +1,3 @@ -from enum import Enum import re from django import forms From 2c4ad262b41c91f402f664576b3b422a591066de Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 9 Jan 2024 16:26:28 -0500 Subject: [PATCH 031/100] Change layout architecture of legend + delete button to fix accessibility issue (legend needs to be direct child of fieldset) --- src/registrar/assets/js/get-gov.js | 24 -------------- src/registrar/assets/sass/_theme/_base.scss | 4 +++ src/registrar/assets/sass/_theme/_forms.scss | 7 ++++ .../templates/application_other_contacts.html | 33 ++++++++----------- 4 files changed, 25 insertions(+), 43 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index e0eb191ef..1dd7f6bc9 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -338,30 +338,6 @@ function markForm(e, formLabel){ // Set display to 'none' formToRemove.style.display = 'none'; - - // - // This next block is a hack to fix a page jump when a fielset is set to display none at the start of the formset but still takes - // a bit of space in the DOM, causing the content to jump down a bit - // - // Get the first hidden fieldset - const hiddenFieldset = document.querySelector('.repeatable-form[style="display: none;"]'); - let targetFieldset = null; - // If that first hidden fieldset does not have any sibling out of all the previous siblings that's visible, get the next visible fieldset - if (hiddenFieldset && !hiddenFieldset.previousElementSibling.matches('.repeatable-form:not([style="display: none;"])')) { - let currentSibling = hiddenFieldset.nextElementSibling; - // Iterate through siblings until a visible fieldset is found - while (currentSibling) { - if (currentSibling.matches(':not([style="display: none;"])')) { - targetFieldset = currentSibling; - break; - } - currentSibling = currentSibling.nextElementSibling; - } - } - if (targetFieldset) { - // Account for the space the hidden fieldsets at the top of the formset are occupying in the DOM - targetFieldset.querySelector('h2').style.marginTop = '1rem'; - } } // Update h2s on the visible forms only. We won't worry about the forms' identifiers diff --git a/src/registrar/assets/sass/_theme/_base.scss b/src/registrar/assets/sass/_theme/_base.scss index 1d936a255..b6d13cee3 100644 --- a/src/registrar/assets/sass/_theme/_base.scss +++ b/src/registrar/assets/sass/_theme/_base.scss @@ -4,6 +4,10 @@ .sr-only { @include sr-only; } + +.clear-both { + clear: both; +} * { -webkit-font-smoothing: antialiased; diff --git a/src/registrar/assets/sass/_theme/_forms.scss b/src/registrar/assets/sass/_theme/_forms.scss index d0bfbee67..29ad30530 100644 --- a/src/registrar/assets/sass/_theme/_forms.scss +++ b/src/registrar/assets/sass/_theme/_forms.scss @@ -31,3 +31,10 @@ padding-left: 0; border-left: none; } + +legend.float-left-tablet + button.float-right-tablet { + margin-top: .5rem; + @include at-media('tablet') { + margin-top: 1rem; + } +} \ No newline at end of file diff --git a/src/registrar/templates/application_other_contacts.html b/src/registrar/templates/application_other_contacts.html index 0b4c34ae6..a19df86c9 100644 --- a/src/registrar/templates/application_other_contacts.html +++ b/src/registrar/templates/application_other_contacts.html @@ -34,31 +34,26 @@ {{ forms.1.management_form }} {# forms.1 is a formset and this iterates over its forms #} {% for form in forms.1.forms %} -
+
-
+ +

Organization contact {{ forloop.counter }}

+ -
- -

Organization contact {{ forloop.counter }}

- -
+ -
- -
- -
{% if forms.1.can_delete %} {{ form.DELETE }} {% endif %} - {% input_with_errors form.first_name %} +
+ {% input_with_errors form.first_name %} +
{% input_with_errors form.middle_name %} @@ -71,11 +66,11 @@ affecting the margin of this block. The wrapper div is a temporary workaround. {% endcomment %}
- {% input_with_errors form.email %} + {% input_with_errors form.email %}
{% with add_class="usa-input--medium" %} - {% input_with_errors form.phone %} + {% input_with_errors form.phone %} {% endwith %}
From 7045e5dcadf1efd206854af4f381391fde50c9ae Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 14:40:53 -0700 Subject: [PATCH 032/100] Exclude blank fields --- src/registrar/assets/js/get-gov.js | 9 +++++++++ src/registrar/templates/application_dotgov_domain.html | 8 +++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index a2a99e104..92b6a1e46 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -134,10 +134,19 @@ function _checkDomainAvailability(el) { const callback = (response) => { toggleInputValidity(el, (response && response.available), msg=response.message); announce(el.id, response.message); + + // Determines if we ignore the field if it is just blank + ignore_blank = el.classList.contains("blank-ok") if (el.validity.valid) { el.classList.add('usa-input--success'); // use of `parentElement` due to .gov inputs being wrapped in www/.gov decoration inlineToast(el.parentElement, el.id, SUCCESS, response.message); + } else if (ignore_blank && response.code == "required"){ + // Visually remove the error + error = "usa-input--error" + if (el.classList.contains(error)){ + el.classList.remove(error) + } } else { inlineToast(el.parentElement, el.id, ERROR, response.message); } diff --git a/src/registrar/templates/application_dotgov_domain.html b/src/registrar/templates/application_dotgov_domain.html index bd3c4a473..ab5504264 100644 --- a/src/registrar/templates/application_dotgov_domain.html +++ b/src/registrar/templates/application_dotgov_domain.html @@ -73,9 +73,11 @@ {# attr_validate / validate="domain" invokes code in get-gov.js #} {# attr_auto_validate likewise triggers behavior in get-gov.js #} {% with append_gov=True attr_validate="domain" attr_auto_validate=True %} - {% for form in forms.1 %} - {% input_with_errors form.alternative_domain %} - {% endfor %} + {% with add_class="blank-ok" %} + {% for form in forms.1 %} + {% input_with_errors form.alternative_domain %} + {% endfor %} + {% endwith %} {% endwith %} {% endwith %} From 5cdbafeeb68a546b50357ace4c732541f7f330d0 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 14:55:18 -0700 Subject: [PATCH 033/100] Improve comments --- src/registrar/models/utility/domain_helper.py | 31 +++++++++++++------ src/registrar/utility/errors.py | 2 ++ 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 58629f213..6c85cb884 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -58,20 +58,21 @@ class DomainHelper: @classmethod def validate_and_handle_errors(cls, domain, error_return_type, blank_ok=False): """ - Validates the provided domain and handles any validation errors. - - This method attempts to validate the domain using the `validate` method. If validation fails, - it catches the exception and returns an appropriate error response. The type of the error response - (JSON response or form validation error) is determined by the `error_return_type` parameter. + Validates a domain and returns an appropriate response based on the validation result. + This method uses the `validate` method to validate the domain. If validation fails, it catches the exception, + maps it to a corresponding error code, and returns a response based on the `error_return_type` parameter. Args: domain (str): The domain to validate. - error_return_type (ValidationErrorReturnType): The type of error response to return if validation fails. - blank_ok (bool, optional): Whether to return an exception if the input is blank. Defaults to False. + error_return_type (ValidationErrorReturnType): Determines the type of response (JSON or form validation error). + blank_ok (bool, optional): If True, blank input does not raise an exception. Defaults to False. + Returns: - A tuple of the validated domain name, and the response + tuple: The validated domain (or None if validation failed), and the response (success or error). """ # noqa + + # Map each exception to a corresponding error code error_map = { errors.BlankValueError: "required", errors.ExtraDotsError: "extra_dots", @@ -79,21 +80,31 @@ class DomainHelper: errors.RegistrySystemError: "error", errors.InvalidDomainError: "invalid", } + validated = None response = None + try: + # Attempt to validate the domain validated = cls.validate(domain, blank_ok) + # Get a list of each possible exception, and the code to return except tuple(error_map.keys()) as error: - # For each exception, determine which code should be returned + # If an error is caught, get its type + error_type = type(error) + + # Generate the response based on the error code and return type response = DomainHelper._return_form_error_or_json_response( - error_return_type, code=error_map.get(type(error)) + error_return_type, code=error_map.get(error_type) ) else: + # For form validation, we do not need to display the success message if error_return_type != ValidationErrorReturnType.FORM_VALIDATION_ERROR: response = DomainHelper._return_form_error_or_json_response( error_return_type, code="success", available=True ) + + # Return the validated domain and the response (either error or success) return (validated, response) @staticmethod diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index bac18d076..21158e58a 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -18,6 +18,8 @@ class RegistrySystemError(ValueError): class InvalidDomainError(ValueError): + """Error class for situations where an invalid domain is supplied""" + pass From 1b292ce640a64e66d3603bea8bd64acad70322e8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 15:01:41 -0700 Subject: [PATCH 034/100] Name refactor --- src/api/views.py | 4 ++-- src/registrar/forms/application_wizard.py | 9 +++++--- src/registrar/models/utility/domain_helper.py | 22 +++++++++---------- src/registrar/utility/enums.py | 2 +- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/api/views.py b/src/api/views.py index 24960ff1c..9096c41ea 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -5,7 +5,7 @@ from django.http import HttpResponse from django.utils.safestring import mark_safe from registrar.templatetags.url_helpers import public_site_url -from registrar.utility.enums import ValidationErrorReturnType +from registrar.utility.enums import ValidationReturnType from registrar.utility.errors import GenericError, GenericErrorCodes import requests @@ -93,7 +93,7 @@ def available(request, domain=""): _, json_response = Domain.validate_and_handle_errors( domain=domain, - error_return_type=ValidationErrorReturnType.JSON_RESPONSE, + return_type=ValidationReturnType.JSON_RESPONSE, ) return json_response diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index c29225ca6..e318ee5aa 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -10,7 +10,7 @@ from django.utils.safestring import mark_safe from registrar.models import Contact, DomainApplication, DraftDomain, Domain from registrar.templatetags.url_helpers import public_site_url -from registrar.utility.enums import ValidationErrorReturnType +from registrar.utility.enums import ValidationReturnType logger = logging.getLogger(__name__) @@ -384,7 +384,9 @@ class AlternativeDomainForm(RegistrarForm): """Validation code for domain names.""" requested = self.cleaned_data.get("alternative_domain", None) validated, _ = DraftDomain.validate_and_handle_errors( - requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR, blank_ok=True + domain=requested, + return_type=ValidationReturnType.FORM_VALIDATION_ERROR, + blank_ok=True, ) return validated @@ -462,7 +464,8 @@ class DotGovDomainForm(RegistrarForm): """Validation code for domain names.""" requested = self.cleaned_data.get("requested_domain", None) validated, _ = DraftDomain.validate_and_handle_errors( - requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR + domain=requested, + return_type=ValidationReturnType.FORM_VALIDATION_ERROR, ) return validated diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 6c85cb884..5647fe49e 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -6,7 +6,7 @@ from django.http import JsonResponse from api.views import DOMAIN_API_MESSAGES, check_domain_available from registrar.utility import errors from epplibwrapper.errors import RegistryError -from registrar.utility.enums import ValidationErrorReturnType +from registrar.utility.enums import ValidationReturnType class DomainHelper: @@ -56,16 +56,16 @@ class DomainHelper: return domain @classmethod - def validate_and_handle_errors(cls, domain, error_return_type, blank_ok=False): + def validate_and_handle_errors(cls, domain, return_type, blank_ok=False): """ Validates a domain and returns an appropriate response based on the validation result. This method uses the `validate` method to validate the domain. If validation fails, it catches the exception, - maps it to a corresponding error code, and returns a response based on the `error_return_type` parameter. + maps it to a corresponding error code, and returns a response based on the `return_type` parameter. Args: domain (str): The domain to validate. - error_return_type (ValidationErrorReturnType): Determines the type of response (JSON or form validation error). + return_type (ValidationReturnType): Determines the type of response (JSON or form validation error). blank_ok (bool, optional): If True, blank input does not raise an exception. Defaults to False. Returns: @@ -95,20 +95,20 @@ class DomainHelper: # Generate the response based on the error code and return type response = DomainHelper._return_form_error_or_json_response( - error_return_type, code=error_map.get(error_type) + return_type, code=error_map.get(error_type) ) else: # For form validation, we do not need to display the success message - if error_return_type != ValidationErrorReturnType.FORM_VALIDATION_ERROR: + if return_type != ValidationReturnType.FORM_VALIDATION_ERROR: response = DomainHelper._return_form_error_or_json_response( - error_return_type, code="success", available=True + return_type, code="success", available=True ) # Return the validated domain and the response (either error or success) return (validated, response) @staticmethod - def _return_form_error_or_json_response(return_type: ValidationErrorReturnType, code, available=False): + def _return_form_error_or_json_response(return_type: ValidationReturnType, code, available=False): """ Returns an error response based on the `return_type`. @@ -117,7 +117,7 @@ class DomainHelper: If `return_type` is neither, raises a ValueError. Args: - return_type (ValidationErrorReturnType): The type of error response. + return_type (ValidationReturnType): The type of error response. code (str): The error code for the error message. available (bool, optional): Availability, only used for JSON responses. Defaults to False. @@ -128,9 +128,9 @@ class DomainHelper: ValueError: If `return_type` is neither `FORM_VALIDATION_ERROR` nor `JSON_RESPONSE`. """ # noqa match return_type: - case ValidationErrorReturnType.FORM_VALIDATION_ERROR: + case ValidationReturnType.FORM_VALIDATION_ERROR: raise forms.ValidationError(DOMAIN_API_MESSAGES[code], code=code) - case ValidationErrorReturnType.JSON_RESPONSE: + case ValidationReturnType.JSON_RESPONSE: return JsonResponse({"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]}) case _: raise ValueError("Invalid return type specified") diff --git a/src/registrar/utility/enums.py b/src/registrar/utility/enums.py index 6aa4f4044..51f6523c5 100644 --- a/src/registrar/utility/enums.py +++ b/src/registrar/utility/enums.py @@ -3,7 +3,7 @@ from enum import Enum -class ValidationErrorReturnType(Enum): +class ValidationReturnType(Enum): """Determines the return value of the validate_and_handle_errors class""" JSON_RESPONSE = "JSON_RESPONSE" From cdf46044d7b57d697ba1204109a9a03d2ac89ba1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 11:06:40 -0700 Subject: [PATCH 035/100] Unit tests --- src/registrar/tests/test_forms.py | 109 ++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index e0afb2d71..0187a7636 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -1,8 +1,12 @@ """Test form validation requirements.""" +import json from django.test import TestCase, RequestFactory +from django.urls import reverse +from api.views import available from registrar.forms.application_wizard import ( + AlternativeDomainForm, CurrentSitesForm, DotGovDomainForm, AuthorizingOfficialForm, @@ -23,6 +27,7 @@ from django.contrib.auth import get_user_model class TestFormValidation(MockEppLib): def setUp(self): super().setUp() + self.API_BASE_PATH = "/api/v1/available/?domain=" self.user = get_user_model().objects.create(username="username") self.factory = RequestFactory() @@ -74,6 +79,110 @@ class TestFormValidation(MockEppLib): ["Enter the .gov domain you want without any periods."], ) + def test_requested_domain_errors_consistent(self): + """Tests if the errors on submit and with the check availability buttons are consistent + for requested_domains + """ + test_cases = [ + # extra_dots + ("top-level-agency.com", "Enter the .gov domain you want without any periods."), + # invalid + ( + "underscores_forever", + "Enter a domain using only letters, numbers, " "or hyphens (though we don't recommend using hyphens).", + ), + # required + ("", "Enter the .gov domain you want. Don’t include “www” or “.gov.”"), + # unavailable + ( + "whitehouse.gov", + "That domain isn’t available. Read more about " + "choosing your .gov domain.", + ), + ] + + for domain, expected_error in test_cases: + with self.subTest(domain=domain, error=expected_error): + form = DotGovDomainForm(data={"requested_domain": domain}) + + form_error = list(form.errors["requested_domain"]) + + # Ensure the form returns what we expect + self.assertEqual( + form_error, + [expected_error], + ) + + request = self.factory.get(self.API_BASE_PATH + domain) + request.user = self.user + response = available(request, domain=domain) + + # Ensure that we're getting the right kind of response + self.assertContains(response, "available") + + response_object = json.loads(response.content) + + json_error = response_object["message"] + # Test if the message is what we expect + self.assertEqual(json_error, expected_error) + + # While its implied, + # for good measure, test if the two objects are equal anyway + self.assertEqual([json_error], form_error) + + def test_alternate_domain_errors_consistent(self): + """Tests if the errors on submit and with the check availability buttons are consistent + for alternative_domains + """ + test_cases = [ + # extra_dots + ("top-level-agency.com", "Enter the .gov domain you want without any periods."), + # invalid + ( + "underscores_forever", + "Enter a domain using only letters, numbers, " "or hyphens (though we don't recommend using hyphens).", + ), + # required + ("", "Enter the .gov domain you want. Don’t include “www” or “.gov.”"), + # unavailable + ( + "whitehouse.gov", + "That domain isn’t available. Read more about " + "choosing your .gov domain.", + ), + ] + + for domain, expected_error in test_cases: + with self.subTest(domain=domain, error=expected_error): + form = AlternativeDomainForm(data={"alternative_domain": domain}) + + form_error = list(form.errors["alternative_domain"]) + + # Ensure the form returns what we expect + self.assertEqual( + form_error, + [expected_error], + ) + + request = self.factory.get(self.API_BASE_PATH + domain) + request.user = self.user + response = available(request, domain=domain) + + # Ensure that we're getting the right kind of response + self.assertContains(response, "available") + + response_object = json.loads(response.content) + + json_error = response_object["message"] + # Test if the message is what we expect + self.assertEqual(json_error, expected_error) + + # While its implied, + # for good measure, test if the two objects are equal anyway + self.assertEqual([json_error], form_error) + def test_requested_domain_two_dots_invalid(self): """don't accept domains that are subdomains""" form = DotGovDomainForm(data={"requested_domain": "sub.top-level-agency.gov"}) From 3dc72997c0e6857b50f14e2390bf1417cb268267 Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Wed, 10 Jan 2024 12:26:18 -0600 Subject: [PATCH 036/100] Updated tribe label --- 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 2802b1893..394007211 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -170,7 +170,7 @@ class TribalGovernmentForm(RegistrarForm): ) tribe_name = forms.CharField( - label="What is the name of the tribe you represent?", + label="Name of tribe", error_messages={"required": "Enter the tribe you represent."}, ) From 7b56b82fc5dd3d71b043b68e3f9eae869f8008e4 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 10 Jan 2024 13:47:34 -0500 Subject: [PATCH 037/100] Teaks to the layout of the Tribal Gov template --- .../application_tribal_government.html | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/registrar/templates/application_tribal_government.html b/src/registrar/templates/application_tribal_government.html index bdca60907..b7fde3278 100644 --- a/src/registrar/templates/application_tribal_government.html +++ b/src/registrar/templates/application_tribal_government.html @@ -1,24 +1,24 @@ {% extends 'application_form.html' %} {% load field_helpers %} +{% block form_instructions %} +

To help us determine your eligibility for a .gov domain, we need to know more about your tribal government.

+{% endblock %} {% block form_fields %} - {% with sublabel_text="Please include the entire name of your tribe as recognized by the Bureau of Indian Affairs." %} - {% with link_text="Bureau of Indian Affairs" %} - {% with link_href="https://www.federalregister.gov/documents/2023/01/12/2023-00504/indian-entities-recognized-by-and-eligible-to-receive-services-from-the-united-states-bureau-of" %} - {% with external_link="true" target_blank="true" %} - {% input_with_errors forms.0.tribe_name %} - {% endwith %} - {% endwith %} - {% endwith %} +

What is the name of the tribe you represent?

+

Please include the full name of your tribe as recognized by the Bureau of Indian Affairs.

+ + {% with external_link="true" target_blank="true" %} + {% input_with_errors forms.0.tribe_name %} {% endwith %}
-

Is your organization a federally-recognized tribe or a state-recognized tribe? Check all that apply. - *

+

Is your organization a federally-recognized tribe or a state-recognized tribe?

 +

Check all that apply. *

{% input_with_errors forms.0.federally_recognized_tribe %} {% input_with_errors forms.0.state_recognized_tribe %}
From 659fff322e38a3e766a4a6318b524ae46dca5a24 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 12:04:13 -0700 Subject: [PATCH 038/100] Add unit tests --- src/registrar/forms/application_wizard.py | 14 ++++++++++---- src/registrar/models/utility/domain_helper.py | 8 ++------ src/registrar/tests/test_forms.py | 10 ++++++---- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index e318ee5aa..34bfb1731 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -2,6 +2,7 @@ from __future__ import annotations # allows forward references in annotations from itertools import zip_longest import logging from typing import Callable +from api.views import DOMAIN_API_MESSAGES from phonenumber_field.formfields import PhoneNumberField # type: ignore from django import forms @@ -384,8 +385,8 @@ class AlternativeDomainForm(RegistrarForm): """Validation code for domain names.""" requested = self.cleaned_data.get("alternative_domain", None) validated, _ = DraftDomain.validate_and_handle_errors( - domain=requested, - return_type=ValidationReturnType.FORM_VALIDATION_ERROR, + domain=requested, + return_type=ValidationReturnType.FORM_VALIDATION_ERROR, blank_ok=True, ) return validated @@ -464,12 +465,17 @@ class DotGovDomainForm(RegistrarForm): """Validation code for domain names.""" requested = self.cleaned_data.get("requested_domain", None) validated, _ = DraftDomain.validate_and_handle_errors( - domain=requested, + domain=requested, return_type=ValidationReturnType.FORM_VALIDATION_ERROR, ) return validated - requested_domain = forms.CharField(label="What .gov domain do you want?") + requested_domain = forms.CharField( + label="What .gov domain do you want?", + error_messages={ + "required": DOMAIN_API_MESSAGES["required"], + }, + ) class PurposeForm(RegistrarForm): diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 5647fe49e..818473f2f 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -94,15 +94,11 @@ class DomainHelper: error_type = type(error) # Generate the response based on the error code and return type - response = DomainHelper._return_form_error_or_json_response( - return_type, code=error_map.get(error_type) - ) + response = DomainHelper._return_form_error_or_json_response(return_type, code=error_map.get(error_type)) else: # For form validation, we do not need to display the success message if return_type != ValidationReturnType.FORM_VALIDATION_ERROR: - response = DomainHelper._return_form_error_or_json_response( - return_type, code="success", available=True - ) + response = DomainHelper._return_form_error_or_json_response(return_type, code="success", available=True) # Return the validated domain and the response (either error or success) return (validated, response) diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index 0187a7636..a49257b75 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -2,7 +2,6 @@ import json from django.test import TestCase, RequestFactory -from django.urls import reverse from api.views import available from registrar.forms.application_wizard import ( @@ -92,7 +91,12 @@ class TestFormValidation(MockEppLib): "Enter a domain using only letters, numbers, " "or hyphens (though we don't recommend using hyphens).", ), # required - ("", "Enter the .gov domain you want. Don’t include “www” or “.gov.”"), + ( + "", + "Enter the .gov domain you want. Don’t include “www” or “.gov.”" + " For example, if you want www.city.gov, you would enter “city”" + " (without the quotes).", + ), # unavailable ( "whitehouse.gov", @@ -143,8 +147,6 @@ class TestFormValidation(MockEppLib): "underscores_forever", "Enter a domain using only letters, numbers, " "or hyphens (though we don't recommend using hyphens).", ), - # required - ("", "Enter the .gov domain you want. Don’t include “www” or “.gov.”"), # unavailable ( "whitehouse.gov", From cca3c5a23ce74e8d1f10010a4218fab8d16b55a4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 12:25:56 -0700 Subject: [PATCH 039/100] Update test_forms.py --- src/registrar/tests/test_forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index a49257b75..e6913a48d 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -152,7 +152,7 @@ class TestFormValidation(MockEppLib): "whitehouse.gov", "That domain isn’t available. Read more about " - "choosing your .gov domain.", + "choosing your .gov domain..", ), ] From be8fb61c43b45149e4f000fe1637bcf1a3306c31 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 12:32:53 -0700 Subject: [PATCH 040/100] Add period --- src/registrar/tests/test_forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index e6913a48d..2f464046f 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -102,7 +102,7 @@ class TestFormValidation(MockEppLib): "whitehouse.gov", "That domain isn’t available. Read more about " - "choosing your .gov domain.", + "choosing your .gov domain.", ), ] From f0027629168fe2e1dec5a88e7f8bed957acbe65c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 12:36:29 -0700 Subject: [PATCH 041/100] Fix periods --- src/registrar/tests/test_forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index 2f464046f..7b42311d4 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -152,7 +152,7 @@ class TestFormValidation(MockEppLib): "whitehouse.gov", "That domain isn’t available. Read more about " - "choosing your .gov domain..", + "choosing your .gov domain.", ), ] From 6d1819fd402c99138cc70fc9d218737702ace336 Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Wed, 10 Jan 2024 16:15:39 -0600 Subject: [PATCH 042/100] Update "election" step --- src/registrar/templates/application_org_election.html | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/application_org_election.html b/src/registrar/templates/application_org_election.html index 04c8f2657..b2ef462b5 100644 --- a/src/registrar/templates/application_org_election.html +++ b/src/registrar/templates/application_org_election.html @@ -2,9 +2,11 @@ {% load field_helpers %} {% block form_instructions %} -

Is your organization an election office?

+ -

An election office is a government entity whose primary responsibility is overseeing elections and/or conducting voter registration.

+

An election office is a government entity whose primary responsibility is overseeing elections and/or conducting voter registration. If your organization is an election office, we'll prioritize your request.

+ +

Is your organization an election office?

Answer “yes” only if the main purpose of your organization is to serve as an election office.

From 72db5d1d897b626b74bf98824e087b219092187e Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Wed, 10 Jan 2024 16:24:39 -0600 Subject: [PATCH 043/100] Updates to "Org mailing address" page --- .../templates/application_org_contact.html | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/registrar/templates/application_org_contact.html b/src/registrar/templates/application_org_contact.html index f5f773647..01b55d03d 100644 --- a/src/registrar/templates/application_org_contact.html +++ b/src/registrar/templates/application_org_contact.html @@ -2,15 +2,12 @@ {% load field_helpers %} {% block form_instructions %} -

- What is the name and mailing address of your organization? -

+

If your domain request is approved, the name of your organization and your city/state will be listed in .gov’s public data.

-

Enter the name of the organization you represent. Your organization might be part - of a larger entity. If so, enter information about your part of the larger entity.

+

What is the name and mailing address of the organization you represent?

+ +

Your organization might be part of a larger entity. If so, enter the name of your part of the larger entity.

-

If your domain request is approved, the name of your organization will be publicly - listed as the domain registrant.

{% endblock %} @@ -43,4 +40,4 @@ {% input_with_errors forms.0.urbanization %}
-{% endblock %} \ No newline at end of file +{% endblock %} From de2b073edccb360c1b68c5e0af6b01ccfd334b8d Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 10 Jan 2024 19:52:37 -0500 Subject: [PATCH 044/100] Fix label on no_other_contacts_rationale --- src/registrar/forms/application_wizard.py | 4 +--- src/registrar/templates/application_other_contacts.html | 3 +++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 955344966..d0eeb32a6 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -791,9 +791,7 @@ class NoOtherContactsForm(RegistrarForm): required=True, # label has to end in a space to get the label_suffix to show label=( - "You don’t need to provide names of other employees now, but it may " - "slow down our assessment of your eligibility. Describe why there are " - "no other employees who can help verify your request." + "No other employees rationale" ), widget=forms.Textarea(), validators=[ diff --git a/src/registrar/templates/application_other_contacts.html b/src/registrar/templates/application_other_contacts.html index a19df86c9..351fba0fc 100644 --- a/src/registrar/templates/application_other_contacts.html +++ b/src/registrar/templates/application_other_contacts.html @@ -88,6 +88,9 @@

No other employees from your organization?

 +

You don't need to provide names of other employees now, but it may + slow down our assessment of your eligibility. Describe why there are + no other employees who can help verify your request.

{% with attr_maxlength=1000 add_label_class="usa-sr-only" %} {% input_with_errors forms.2.no_other_contacts_rationale %} {% endwith %} From aeb7665acac24b874cdccb707977bb88edbad7e2 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 10 Jan 2024 19:58:48 -0500 Subject: [PATCH 045/100] minor formatting --- src/registrar/forms/application_wizard.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index c29a5f1a1..315798c59 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -789,9 +789,7 @@ class NoOtherContactsForm(RegistrarForm): no_other_contacts_rationale = forms.CharField( required=True, # label has to end in a space to get the label_suffix to show - label=( - "No other employees rationale" - ), + label=("No other employees rationale"), widget=forms.Textarea(), validators=[ MaxLengthValidator( From c226babfdfef05278dab5a4bad48a06f16017d3e Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 10 Jan 2024 20:00:33 -0500 Subject: [PATCH 046/100] Add end line --- src/registrar/assets/sass/_theme/_forms.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/assets/sass/_theme/_forms.scss b/src/registrar/assets/sass/_theme/_forms.scss index 29ad30530..94407f88d 100644 --- a/src/registrar/assets/sass/_theme/_forms.scss +++ b/src/registrar/assets/sass/_theme/_forms.scss @@ -37,4 +37,4 @@ legend.float-left-tablet + button.float-right-tablet { @include at-media('tablet') { margin-top: 1rem; } -} \ No newline at end of file +} From a213264fff5158c0e5d59db950eaffd36488c3a8 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 11 Jan 2024 14:53:28 -0500 Subject: [PATCH 047/100] modified to_database for handling edits in domain application AO and Submitter --- src/registrar/forms/application_wizard.py | 60 ++++++++++++++++++++++- 1 file changed, 58 insertions(+), 2 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 315798c59..4c5baf55d 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -34,6 +34,34 @@ class RegistrarForm(forms.Form): self.application = kwargs.pop("application", None) super(RegistrarForm, self).__init__(*args, **kwargs) + def has_more_than_one_join(self, db_obj, rel, related_name): + """Helper for finding whether an object is joined more than once.""" + # threshold is the number of related objects that are acceptable + # when determining if related objects exist. threshold is 0 for most + # relationships. if the relationship is related_name, we know that + # there is already exactly 1 acceptable relationship (the one we are + # attempting to delete), so the threshold is 1 + threshold = 1 if rel == related_name else 0 + + # Raise a KeyError if rel is not a defined field on the db_obj model + # This will help catch any errors in reverse_join config on forms + if rel not in [field.name for field in db_obj._meta.get_fields()]: + raise KeyError(f"{rel} is not a defined field on the {db_obj._meta.model_name} model.") + + # if attr rel in db_obj is not None, then test if reference object(s) exist + if getattr(db_obj, rel) is not None: + field = db_obj._meta.get_field(rel) + if isinstance(field, OneToOneField): + # if the rel field is a OneToOne field, then we have already + # determined that the object exists (is not None) + return True + elif isinstance(field, ForeignObjectRel): + # if the rel field is a ManyToOne or ManyToMany, then we need + # to determine if the count of related objects is greater than + # the threshold + return getattr(db_obj, rel).count() > threshold + return False + def to_database(self, obj: DomainApplication | Contact): """ Adds this form's cleaned data to `obj` and saves `obj`. @@ -351,13 +379,27 @@ class AboutYourOrganizationForm(RegistrarForm): class AuthorizingOfficialForm(RegistrarForm): + JOIN = "authorizing_official" + REVERSE_JOINS = [ + "user", + "authorizing_official", + "submitted_applications", + "contact_applications", + "information_authorizing_official", + "submitted_applications_information", + "contact_applications_information", + ] + def to_database(self, obj): if not self.is_valid(): return contact = getattr(obj, "authorizing_official", None) - if contact is not None: + if contact is not None and not any(self.has_more_than_one_join(contact, rel, "authorizing_official") for rel in self.REVERSE_JOINS): + # if contact exists in the database and is not joined to other entities super().to_database(contact) else: + # no contact exists OR contact exists which is joined also to other entities; + # in either case, create a new contact and update it contact = Contact() super().to_database(contact) obj.authorizing_official = contact @@ -549,13 +591,27 @@ class PurposeForm(RegistrarForm): class YourContactForm(RegistrarForm): + JOIN = "submitter" + REVERSE_JOINS = [ + "user", + "authorizing_official", + "submitted_applications", + "contact_applications", + "information_authorizing_official", + "submitted_applications_information", + "contact_applications_information", + ] + def to_database(self, obj): if not self.is_valid(): return contact = getattr(obj, "submitter", None) - if contact is not None: + if contact is not None and not any(self.has_more_than_one_join(contact, rel, "submitted_applications") for rel in self.REVERSE_JOINS): + # if contact exists in the database and is not joined to other entities super().to_database(contact) else: + # no contact exists OR contact exists which is joined also to other entities; + # in either case, create a new contact and update it contact = Contact() super().to_database(contact) obj.submitter = contact From 31d80cc91df8950699a2807dff34350e937c1cd6 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 11 Jan 2024 15:49:29 -0500 Subject: [PATCH 048/100] Refactor JS tests for page titles to test against classes instead --- src/registrar/assets/js/get-gov.js | 21 +++++++++---------- .../templates/application_other_contacts.html | 2 +- src/registrar/templates/domain_dsdata.html | 2 +- .../templates/domain_nameservers.html | 2 +- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 1dd7f6bc9..68e8af69c 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -358,14 +358,12 @@ function markForm(e, formLabel){ */ function prepareNewDeleteButton(btn, formLabel) { let formIdentifier = "form" - let isNameserversForm = document.title.includes("DNS name servers |"); - let isOtherContactsForm = document.title.includes("Other employees from your organization"); + let isNameserversForm = document.querySelector(".nameservers-form"); + let isOtherContactsForm = document.querySelector(".other-contacts-form"); let addButton = document.querySelector("#add-form"); + if (isOtherContactsForm) { formIdentifier = "other_contacts"; - } - - if (isOtherContactsForm) { // We will mark the forms for deletion btn.addEventListener('click', function(e) { markForm(e, formLabel); @@ -386,8 +384,8 @@ function prepareNewDeleteButton(btn, formLabel) { function prepareDeleteButtons(formLabel) { let formIdentifier = "form" let deleteButtons = document.querySelectorAll(".delete-record"); - let isNameserversForm = document.title.includes("DNS name servers |"); - let isOtherContactsForm = document.title.includes("Other employees from your organization"); + let isNameserversForm = document.querySelector(".nameservers-form"); + let isOtherContactsForm = document.querySelector(".other-contacts-form"); let addButton = document.querySelector("#add-form"); if (isOtherContactsForm) { formIdentifier = "other_contacts"; @@ -443,15 +441,16 @@ function hideDeletedForms() { let addButton = document.querySelector("#add-form"); let cloneIndex = 0; let formLabel = ''; - let isNameserversForm = document.title.includes("DNS name servers |"); - let isOtherContactsForm = document.title.includes("Other employees from your organization"); + let isNameserversForm = document.querySelector(".nameservers-form"); + let isOtherContactsForm = document.querySelector(".other-contacts-form"); + let isDsDataForm = document.querySelector(".ds-data-form"); // The Nameservers formset features 2 required and 11 optionals if (isNameserversForm) { cloneIndex = 2; formLabel = "Name server"; // DNSSEC: DS Data - } else if (document.title.includes("DS Data |")) { - formLabel = "DS Data record"; + } else if (isDsDataForm) { + formLabel = "DS data record"; // The Other Contacts form } else if (isOtherContactsForm) { formLabel = "Organization contact"; diff --git a/src/registrar/templates/application_other_contacts.html b/src/registrar/templates/application_other_contacts.html index 351fba0fc..b14458bcc 100644 --- a/src/registrar/templates/application_other_contacts.html +++ b/src/registrar/templates/application_other_contacts.html @@ -29,7 +29,7 @@
-
+
{% include "includes/required_fields.html" %} {{ forms.1.management_form }} {# forms.1 is a formset and this iterates over its forms #} diff --git a/src/registrar/templates/domain_dsdata.html b/src/registrar/templates/domain_dsdata.html index 1ec4c1f93..b62ad7ec5 100644 --- a/src/registrar/templates/domain_dsdata.html +++ b/src/registrar/templates/domain_dsdata.html @@ -24,7 +24,7 @@ {% include "includes/required_fields.html" %} -
+ {% csrf_token %} {{ formset.management_form }} diff --git a/src/registrar/templates/domain_nameservers.html b/src/registrar/templates/domain_nameservers.html index 15b810193..d60be2de8 100644 --- a/src/registrar/templates/domain_nameservers.html +++ b/src/registrar/templates/domain_nameservers.html @@ -24,7 +24,7 @@ {% include "includes/required_fields.html" %} - + {% csrf_token %} {{ formset.management_form }} From 272f5e1544bb797b3d00ac99dd7207ba6aee6b9e Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Thu, 11 Jan 2024 16:50:19 -0600 Subject: [PATCH 049/100] Updated BIA link --- src/registrar/templates/application_tribal_government.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/application_tribal_government.html b/src/registrar/templates/application_tribal_government.html index b7fde3278..3e79a4524 100644 --- a/src/registrar/templates/application_tribal_government.html +++ b/src/registrar/templates/application_tribal_government.html @@ -8,7 +8,7 @@ {% block form_fields %}

What is the name of the tribe you represent?

-

Please include the full name of your tribe as recognized by the Bureau of Indian Affairs.

+

Please include the full name of your tribe as recognized by the Bureau of Indian Affairs.

{% with external_link="true" target_blank="true" %} {% input_with_errors forms.0.tribe_name %} From 579a4ebb38fa52177cfd1f533ae0c8394527144d Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Thu, 11 Jan 2024 17:02:12 -0600 Subject: [PATCH 050/100] Updates to "about your org" page --- .../templates/application_about_your_organization.html | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/registrar/templates/application_about_your_organization.html b/src/registrar/templates/application_about_your_organization.html index 0d384b4f5..01820ac70 100644 --- a/src/registrar/templates/application_about_your_organization.html +++ b/src/registrar/templates/application_about_your_organization.html @@ -2,14 +2,16 @@ {% load field_helpers %} {% block form_instructions %} -

We’d like to know more about your organization. Include the following in your response:

+

To help us determine your eligibility for a .gov domain, we need to know more about your organization. For example:

  • The type of work your organization does
    • -
  • How your organization is a government organization that is independent of a state government
    • -
  • Include links to authorizing legislation, applicable bylaws or charter, or other documentation to support your claims.
    • +
  • How your organization operates independently from a state government
    • +
  • A description of the specialized, essential services you offer (if applicable)
    • +
  • Links to authorizing legislation, applicable bylaws or charter, or other documentation to support your claims

+

What can you tell us about your organization?

{% endblock %} {% block form_required_fields_help_text %} @@ -20,4 +22,4 @@ {% with attr_maxlength=1000 add_label_class="usa-sr-only" %} {% input_with_errors forms.0.about_your_organization %} {% endwith %} -{% endblock %} \ No newline at end of file +{% endblock %} From 9e95bc270636a9f062d1ac649ba7b5ec52ade929 Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Thu, 11 Jan 2024 17:07:33 -0600 Subject: [PATCH 051/100] Update last paragraph on AO page --- src/registrar/templates/application_authorizing_official.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/application_authorizing_official.html b/src/registrar/templates/application_authorizing_official.html index 3e33ab34e..068457373 100644 --- a/src/registrar/templates/application_authorizing_official.html +++ b/src/registrar/templates/application_authorizing_official.html @@ -14,7 +14,7 @@ {% include "includes/ao_example.html" %}
-

We typically don’t reach out to the authorizing official, but if contact is necessary, our practice is to coordinate first with you, the requestor. Read more about who can serve as an authorizing official.

+

We typically don’t reach out to the authorizing official, but if contact is necessary, our practice is to coordinate with you, the requestor, first.

{% endblock %} From 4dae5ee23e10e407f2c64f34120727aca0fab00b Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 12 Jan 2024 08:02:42 -0500 Subject: [PATCH 052/100] handled edit for domain application --- src/registrar/forms/application_wizard.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 4c5baf55d..4f4d7fab5 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -200,8 +200,14 @@ class RegistrarFormSet(forms.BaseFormSet): # If there are no other relationships, delete the object db_obj.delete() else: - pre_update(db_obj, cleaned) - db_obj.save() + if any(self.has_more_than_one_join(db_obj, rel, related_name) for rel in reverse_joins): + # create a new db_obj and disconnect existing one + getattr(db_obj, related_name).remove(self.application) + kwargs = pre_create(db_obj, cleaned) + getattr(obj, join).create(**kwargs) + else: + pre_update(db_obj, cleaned) + db_obj.save() # no matching database object, create it # make sure not to create a database object if cleaned has 'delete' attribute From 42a233a31ed0381525f0523728447c6aae4f5201 Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Fri, 12 Jan 2024 13:45:36 -0600 Subject: [PATCH 053/100] Updates to "current websites" page --- src/registrar/templates/application_current_sites.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/templates/application_current_sites.html b/src/registrar/templates/application_current_sites.html index 67343aee9..7ce357d86 100644 --- a/src/registrar/templates/application_current_sites.html +++ b/src/registrar/templates/application_current_sites.html @@ -2,9 +2,9 @@ {% load static field_helpers %} {% block form_instructions %} -

Enter your organization’s current public website, if you have one. For example, - www.city.com. We can better evaluate your domain request if we know about domains -you’re already using. If you already have any .gov domains please include them. This question is optional.

+

We can better evaluate your domain request if we know about domains you’re already using.

+

What are the current websites for your organization?

+

Enter your organization’s current public websites. If you already have a .gov domain, include that in your list. This question is optional.

{% endblock %} {% block form_required_fields_help_text %} From 9106fbb807388b7a5f45408b06bc00cd6e8751da Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 12 Jan 2024 15:29:52 -0500 Subject: [PATCH 054/100] fixed domain information authorizing official --- src/registrar/forms/domain.py | 69 +++++++++++++++++++++++++++++++++++ src/registrar/views/domain.py | 1 + 2 files changed, 70 insertions(+) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 17616df4b..865a5bee4 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -1,9 +1,13 @@ """Forms for domain management.""" +import logging + from django import forms from django.core.validators import MinValueValidator, MaxValueValidator, RegexValidator from django.forms import formset_factory +from django.db.models.fields.related import ForeignObjectRel, OneToOneField + from phonenumber_field.widgets import RegionalPhoneNumberWidget from registrar.utility.errors import ( NameserverError, @@ -22,6 +26,7 @@ from .common import ( import re +logger = logging.getLogger(__name__) class DomainAddUserForm(forms.Form): """Form for adding a user to a domain.""" @@ -209,6 +214,16 @@ class ContactForm(forms.ModelForm): class AuthorizingOfficialContactForm(ContactForm): """Form for updating authorizing official contacts.""" + JOIN = "authorizing_official" + REVERSE_JOINS = [ + "user", + "authorizing_official", + "submitted_applications", + "contact_applications", + "information_authorizing_official", + "submitted_applications_information", + "contact_applications_information", + ] def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -230,6 +245,60 @@ class AuthorizingOfficialContactForm(ContactForm): self.fields["email"].error_messages = { "required": "Enter an email address in the required format, like name@example.com." } + self.domainInfo = None + + def setDomainInfo(self, domainInfo): + self.domainInfo = domainInfo + + def save(self, commit=True): + logger.info(f"in save: {self.instance}") + logger.info(f"{self.instance.__class__.__name__}") + logger.info(f"{self.instance.id}") + logger.info(f"self.fields => {self.fields}") + logger.info(f"domain info: {self.instance.information_authorizing_official}") + + # get db object + db_ao = Contact.objects.get(id=self.instance.id) + logger.info(f"db_ao.information_authorizing_official {db_ao.information_authorizing_official}") + if self.domainInfo and any(self.has_more_than_one_join(db_ao, rel, "information_authorizing_official") for rel in self.REVERSE_JOINS): + logger.info(f"domain info => {self.domainInfo}") + logger.info(f"authorizing official id => {self.domainInfo.authorizing_official.id}") + contact = Contact() + for name, value in self.cleaned_data.items(): + setattr(contact, name, value) + contact.save() + self.domainInfo.authorizing_official = contact + self.domainInfo.save() + else: + super().save() + + def has_more_than_one_join(self, db_obj, rel, related_name): + """Helper for finding whether an object is joined more than once.""" + # threshold is the number of related objects that are acceptable + # when determining if related objects exist. threshold is 0 for most + # relationships. if the relationship is related_name, we know that + # there is already exactly 1 acceptable relationship (the one we are + # attempting to delete), so the threshold is 1 + threshold = 1 if rel == related_name else 0 + + # Raise a KeyError if rel is not a defined field on the db_obj model + # This will help catch any errors in reverse_join config on forms + if rel not in [field.name for field in db_obj._meta.get_fields()]: + raise KeyError(f"{rel} is not a defined field on the {db_obj._meta.model_name} model.") + + # if attr rel in db_obj is not None, then test if reference object(s) exist + if getattr(db_obj, rel) is not None: + field = db_obj._meta.get_field(rel) + if isinstance(field, OneToOneField): + # if the rel field is a OneToOne field, then we have already + # determined that the object exists (is not None) + return True + elif isinstance(field, ForeignObjectRel): + # if the rel field is a ManyToOne or ManyToMany, then we need + # to determine if the count of related objects is greater than + # the threshold + return getattr(db_obj, rel).count() > threshold + return False class DomainSecurityEmailForm(forms.Form): diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 2cd12eb37..aa37b15b0 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -222,6 +222,7 @@ class DomainAuthorizingOfficialView(DomainFormBaseView): def form_valid(self, form): """The form is valid, save the authorizing official.""" + form.setDomainInfo(self.object.domain_info) form.save() messages.success(self.request, "The authorizing official for this domain has been updated.") From ab05da9c2db9e85f24d9692a3532a682200295e7 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 12 Jan 2024 16:27:09 -0500 Subject: [PATCH 055/100] moved has_more_than_one_join to contact model --- src/registrar/forms/application_wizard.py | 66 ++--------------------- src/registrar/forms/domain.py | 32 +---------- src/registrar/models/contact.py | 28 ++++++++++ 3 files changed, 34 insertions(+), 92 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 4f4d7fab5..a1d454f74 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -7,7 +7,7 @@ from phonenumber_field.formfields import PhoneNumberField # type: ignore from django import forms from django.core.validators import RegexValidator, MaxLengthValidator from django.utils.safestring import mark_safe -from django.db.models.fields.related import ForeignObjectRel, OneToOneField +from django.db.models.fields.related import ForeignObjectRel from api.views import DOMAIN_API_MESSAGES @@ -33,34 +33,6 @@ class RegistrarForm(forms.Form): # save a reference to an application object self.application = kwargs.pop("application", None) super(RegistrarForm, self).__init__(*args, **kwargs) - - def has_more_than_one_join(self, db_obj, rel, related_name): - """Helper for finding whether an object is joined more than once.""" - # threshold is the number of related objects that are acceptable - # when determining if related objects exist. threshold is 0 for most - # relationships. if the relationship is related_name, we know that - # there is already exactly 1 acceptable relationship (the one we are - # attempting to delete), so the threshold is 1 - threshold = 1 if rel == related_name else 0 - - # Raise a KeyError if rel is not a defined field on the db_obj model - # This will help catch any errors in reverse_join config on forms - if rel not in [field.name for field in db_obj._meta.get_fields()]: - raise KeyError(f"{rel} is not a defined field on the {db_obj._meta.model_name} model.") - - # if attr rel in db_obj is not None, then test if reference object(s) exist - if getattr(db_obj, rel) is not None: - field = db_obj._meta.get_field(rel) - if isinstance(field, OneToOneField): - # if the rel field is a OneToOne field, then we have already - # determined that the object exists (is not None) - return True - elif isinstance(field, ForeignObjectRel): - # if the rel field is a ManyToOne or ManyToMany, then we need - # to determine if the count of related objects is greater than - # the threshold - return getattr(db_obj, rel).count() > threshold - return False def to_database(self, obj: DomainApplication | Contact): """ @@ -124,34 +96,6 @@ class RegistrarFormSet(forms.BaseFormSet): """ raise NotImplementedError - def has_more_than_one_join(self, db_obj, rel, related_name): - """Helper for finding whether an object is joined more than once.""" - # threshold is the number of related objects that are acceptable - # when determining if related objects exist. threshold is 0 for most - # relationships. if the relationship is related_name, we know that - # there is already exactly 1 acceptable relationship (the one we are - # attempting to delete), so the threshold is 1 - threshold = 1 if rel == related_name else 0 - - # Raise a KeyError if rel is not a defined field on the db_obj model - # This will help catch any errors in reverse_join config on forms - if rel not in [field.name for field in db_obj._meta.get_fields()]: - raise KeyError(f"{rel} is not a defined field on the {db_obj._meta.model_name} model.") - - # if attr rel in db_obj is not None, then test if reference object(s) exist - if getattr(db_obj, rel) is not None: - field = db_obj._meta.get_field(rel) - if isinstance(field, OneToOneField): - # if the rel field is a OneToOne field, then we have already - # determined that the object exists (is not None) - return True - elif isinstance(field, ForeignObjectRel): - # if the rel field is a ManyToOne or ManyToMany, then we need - # to determine if the count of related objects is greater than - # the threshold - return getattr(db_obj, rel).count() > threshold - return False - def _to_database( self, obj: DomainApplication, @@ -193,14 +137,14 @@ class RegistrarFormSet(forms.BaseFormSet): # matching database object exists, update it if db_obj is not None and cleaned: if should_delete(cleaned): - if any(self.has_more_than_one_join(db_obj, rel, related_name) for rel in reverse_joins): + if hasattr(db_obj, "has_more_than_one_join") and any(db_obj.has_more_than_one_join(rel, related_name) for rel in reverse_joins): # Remove the specific relationship without deleting the object getattr(db_obj, related_name).remove(self.application) else: # If there are no other relationships, delete the object db_obj.delete() else: - if any(self.has_more_than_one_join(db_obj, rel, related_name) for rel in reverse_joins): + if hasattr(db_obj, "has_more_than_one_join") and any(db_obj.has_more_than_one_join(rel, related_name) for rel in reverse_joins): # create a new db_obj and disconnect existing one getattr(db_obj, related_name).remove(self.application) kwargs = pre_create(db_obj, cleaned) @@ -400,7 +344,7 @@ class AuthorizingOfficialForm(RegistrarForm): if not self.is_valid(): return contact = getattr(obj, "authorizing_official", None) - if contact is not None and not any(self.has_more_than_one_join(contact, rel, "authorizing_official") for rel in self.REVERSE_JOINS): + if contact is not None and not any(contact.has_more_than_one_join(rel, "authorizing_official") for rel in self.REVERSE_JOINS): # if contact exists in the database and is not joined to other entities super().to_database(contact) else: @@ -612,7 +556,7 @@ class YourContactForm(RegistrarForm): if not self.is_valid(): return contact = getattr(obj, "submitter", None) - if contact is not None and not any(self.has_more_than_one_join(contact, rel, "submitted_applications") for rel in self.REVERSE_JOINS): + if contact is not None and not any(contact.has_more_than_one_join(rel, "submitted_applications") for rel in self.REVERSE_JOINS): # if contact exists in the database and is not joined to other entities super().to_database(contact) else: diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 865a5bee4..b15bc379e 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -6,8 +6,6 @@ from django import forms from django.core.validators import MinValueValidator, MaxValueValidator, RegexValidator from django.forms import formset_factory -from django.db.models.fields.related import ForeignObjectRel, OneToOneField - from phonenumber_field.widgets import RegionalPhoneNumberWidget from registrar.utility.errors import ( NameserverError, @@ -260,7 +258,7 @@ class AuthorizingOfficialContactForm(ContactForm): # get db object db_ao = Contact.objects.get(id=self.instance.id) logger.info(f"db_ao.information_authorizing_official {db_ao.information_authorizing_official}") - if self.domainInfo and any(self.has_more_than_one_join(db_ao, rel, "information_authorizing_official") for rel in self.REVERSE_JOINS): + if self.domainInfo and any(db_ao.has_more_than_one_join(rel, "information_authorizing_official") for rel in self.REVERSE_JOINS): logger.info(f"domain info => {self.domainInfo}") logger.info(f"authorizing official id => {self.domainInfo.authorizing_official.id}") contact = Contact() @@ -272,34 +270,6 @@ class AuthorizingOfficialContactForm(ContactForm): else: super().save() - def has_more_than_one_join(self, db_obj, rel, related_name): - """Helper for finding whether an object is joined more than once.""" - # threshold is the number of related objects that are acceptable - # when determining if related objects exist. threshold is 0 for most - # relationships. if the relationship is related_name, we know that - # there is already exactly 1 acceptable relationship (the one we are - # attempting to delete), so the threshold is 1 - threshold = 1 if rel == related_name else 0 - - # Raise a KeyError if rel is not a defined field on the db_obj model - # This will help catch any errors in reverse_join config on forms - if rel not in [field.name for field in db_obj._meta.get_fields()]: - raise KeyError(f"{rel} is not a defined field on the {db_obj._meta.model_name} model.") - - # if attr rel in db_obj is not None, then test if reference object(s) exist - if getattr(db_obj, rel) is not None: - field = db_obj._meta.get_field(rel) - if isinstance(field, OneToOneField): - # if the rel field is a OneToOne field, then we have already - # determined that the object exists (is not None) - return True - elif isinstance(field, ForeignObjectRel): - # if the rel field is a ManyToOne or ManyToMany, then we need - # to determine if the count of related objects is greater than - # the threshold - return getattr(db_obj, rel).count() > threshold - return False - class DomainSecurityEmailForm(forms.Form): """Form for adding or editing a security email to a domain.""" diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index 06cf83887..4352e0a16 100644 --- a/src/registrar/models/contact.py +++ b/src/registrar/models/contact.py @@ -54,6 +54,34 @@ class Contact(TimeStampedModel): db_index=True, ) + def has_more_than_one_join(self, rel, related_name): + """Helper for finding whether an object is joined more than once.""" + # threshold is the number of related objects that are acceptable + # when determining if related objects exist. threshold is 0 for most + # relationships. if the relationship is related_name, we know that + # there is already exactly 1 acceptable relationship (the one we are + # attempting to delete), so the threshold is 1 + threshold = 1 if rel == related_name else 0 + + # Raise a KeyError if rel is not a defined field on the db_obj model + # This will help catch any errors in reverse_join config on forms + if rel not in [field.name for field in self._meta.get_fields()]: + raise KeyError(f"{rel} is not a defined field on the {self._meta.model_name} model.") + + # if attr rel in db_obj is not None, then test if reference object(s) exist + if getattr(self, rel) is not None: + field = self._meta.get_field(rel) + if isinstance(field, models.OneToOneField): + # if the rel field is a OneToOne field, then we have already + # determined that the object exists (is not None) + return True + elif isinstance(field, models.ForeignObjectRel): + # if the rel field is a ManyToOne or ManyToMany, then we need + # to determine if the count of related objects is greater than + # the threshold + return getattr(self, rel).count() > threshold + return False + def get_formatted_name(self): """Returns the contact's name in Western order.""" names = [n for n in [self.first_name, self.middle_name, self.last_name] if n] From 4ab1549bfde288f399de856801c88be58287a8e3 Mon Sep 17 00:00:00 2001 From: Michelle Rago <60157596+michelle-rago@users.noreply.github.com> Date: Fri, 12 Jan 2024 16:27:55 -0500 Subject: [PATCH 056/100] removed an extra space --- .../templates/application_about_your_organization.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/application_about_your_organization.html b/src/registrar/templates/application_about_your_organization.html index 01820ac70..02e2e2c4f 100644 --- a/src/registrar/templates/application_about_your_organization.html +++ b/src/registrar/templates/application_about_your_organization.html @@ -6,7 +6,7 @@
  • The type of work your organization does
    • -
  • How your organization operates independently from a state government
    • +
  • How your organization operates independently from a state government
  • A description of the specialized, essential services you offer (if applicable)
  • Links to authorizing legislation, applicable bylaws or charter, or other documentation to support your claims
From 73d65e695432229f3956233e83a4c5f140be31fe Mon Sep 17 00:00:00 2001 From: Michelle Rago <60157596+michelle-rago@users.noreply.github.com> Date: Fri, 12 Jan 2024 16:30:50 -0500 Subject: [PATCH 057/100] Removed "domain" from "We can better evaluate your domain request..." --- src/registrar/templates/application_current_sites.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/application_current_sites.html b/src/registrar/templates/application_current_sites.html index 7ce357d86..debadcfe2 100644 --- a/src/registrar/templates/application_current_sites.html +++ b/src/registrar/templates/application_current_sites.html @@ -2,7 +2,7 @@ {% load static field_helpers %} {% block form_instructions %} -

We can better evaluate your domain request if we know about domains you’re already using.

+

We can better evaluate your request if we know about domains you’re already using.

What are the current websites for your organization?

Enter your organization’s current public websites. If you already have a .gov domain, include that in your list. This question is optional.

{% endblock %} From 092024659372790a7afaecea1023b9edf045c904 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 12 Jan 2024 17:51:02 -0500 Subject: [PATCH 058/100] changed parameters for has_more_than_one_join to pass array of reverse_joins rather than individual join --- src/registrar/forms/application_wizard.py | 6 +++--- src/registrar/models/contact.py | 22 ++++++++++++++-------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index a1d454f74..3c1f0cc9c 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -137,14 +137,14 @@ class RegistrarFormSet(forms.BaseFormSet): # matching database object exists, update it if db_obj is not None and cleaned: if should_delete(cleaned): - if hasattr(db_obj, "has_more_than_one_join") and any(db_obj.has_more_than_one_join(rel, related_name) for rel in reverse_joins): + if hasattr(db_obj, "has_more_than_one_join") and db_obj.has_more_than_one_join(reverse_joins, related_name): # Remove the specific relationship without deleting the object getattr(db_obj, related_name).remove(self.application) else: # If there are no other relationships, delete the object db_obj.delete() else: - if hasattr(db_obj, "has_more_than_one_join") and any(db_obj.has_more_than_one_join(rel, related_name) for rel in reverse_joins): + if hasattr(db_obj, "has_more_than_one_join") and db_obj.has_more_than_one_join(reverse_joins, related_name): # create a new db_obj and disconnect existing one getattr(db_obj, related_name).remove(self.application) kwargs = pre_create(db_obj, cleaned) @@ -344,7 +344,7 @@ class AuthorizingOfficialForm(RegistrarForm): if not self.is_valid(): return contact = getattr(obj, "authorizing_official", None) - if contact is not None and not any(contact.has_more_than_one_join(rel, "authorizing_official") for rel in self.REVERSE_JOINS): + if contact is not None and not contact.has_more_than_one_join(self.REVERSE_JOINS, "authorizing_official"): # if contact exists in the database and is not joined to other entities super().to_database(contact) else: diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index 4352e0a16..483752c56 100644 --- a/src/registrar/models/contact.py +++ b/src/registrar/models/contact.py @@ -54,23 +54,29 @@ class Contact(TimeStampedModel): db_index=True, ) - def has_more_than_one_join(self, rel, related_name): + def has_more_than_one_join(self, all_relations, expected_relation): + """Helper for finding whether an object is joined more than once. + all_relations is the list of all_relations to be checked for existing joins. + expected_relation is the one relation with one expected join""" + return any(self._has_more_than_one_join_per_relation(rel, expected_relation) for rel in all_relations) + + def _has_more_than_one_join_per_relation(self, relation, expected_relation): """Helper for finding whether an object is joined more than once.""" # threshold is the number of related objects that are acceptable # when determining if related objects exist. threshold is 0 for most - # relationships. if the relationship is related_name, we know that + # relationships. if the relationship is expected_relation, we know that # there is already exactly 1 acceptable relationship (the one we are # attempting to delete), so the threshold is 1 - threshold = 1 if rel == related_name else 0 + threshold = 1 if relation == expected_relation else 0 # Raise a KeyError if rel is not a defined field on the db_obj model # This will help catch any errors in reverse_join config on forms - if rel not in [field.name for field in self._meta.get_fields()]: - raise KeyError(f"{rel} is not a defined field on the {self._meta.model_name} model.") + if relation not in [field.name for field in self._meta.get_fields()]: + raise KeyError(f"{relation} is not a defined field on the {self._meta.model_name} model.") # if attr rel in db_obj is not None, then test if reference object(s) exist - if getattr(self, rel) is not None: - field = self._meta.get_field(rel) + if getattr(self, relation) is not None: + field = self._meta.get_field(relation) if isinstance(field, models.OneToOneField): # if the rel field is a OneToOne field, then we have already # determined that the object exists (is not None) @@ -79,7 +85,7 @@ class Contact(TimeStampedModel): # if the rel field is a ManyToOne or ManyToMany, then we need # to determine if the count of related objects is greater than # the threshold - return getattr(self, rel).count() > threshold + return getattr(self, relation).count() > threshold return False def get_formatted_name(self): From 579b890996e8f99379d02084c054891f48d23fe7 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 12 Jan 2024 17:54:30 -0500 Subject: [PATCH 059/100] finished changing parameters for has_more_than_one_join to pass array of reverse_joins rather than individual join --- src/registrar/forms/application_wizard.py | 2 +- src/registrar/forms/domain.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 3c1f0cc9c..fba1e6da1 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -556,7 +556,7 @@ class YourContactForm(RegistrarForm): if not self.is_valid(): return contact = getattr(obj, "submitter", None) - if contact is not None and not any(contact.has_more_than_one_join(rel, "submitted_applications") for rel in self.REVERSE_JOINS): + if contact is not None and not contact.has_more_than_one_join(self.REVERSE_JOINS, "submitted_applications"): # if contact exists in the database and is not joined to other entities super().to_database(contact) else: diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index b15bc379e..9b28af2d6 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -258,7 +258,7 @@ class AuthorizingOfficialContactForm(ContactForm): # get db object db_ao = Contact.objects.get(id=self.instance.id) logger.info(f"db_ao.information_authorizing_official {db_ao.information_authorizing_official}") - if self.domainInfo and any(db_ao.has_more_than_one_join(rel, "information_authorizing_official") for rel in self.REVERSE_JOINS): + if self.domainInfo and db_ao.has_more_than_one_join(self.REVERSE_JOINS, "information_authorizing_official"): logger.info(f"domain info => {self.domainInfo}") logger.info(f"authorizing official id => {self.domainInfo.authorizing_official.id}") contact = Contact() From bec3c96e0dc6455a3997e76156d56b2bb6d9e096 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 12 Jan 2024 17:57:42 -0500 Subject: [PATCH 060/100] Layout tweaks --- src/registrar/forms/application_wizard.py | 2 +- src/registrar/templates/application_other_contacts.html | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 315798c59..bf62769f8 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -609,7 +609,7 @@ class OtherContactsYesNoForm(RegistrarForm): self.fields["has_other_contacts"] = forms.TypedChoiceField( coerce=lambda x: x.lower() == "true" if x is not None else None, # coerce strings to bool, excepting None - choices=((True, "Yes, I can name other employees."), (False, "No (We’ll ask you to explain why).")), + choices=((True, "Yes, I can name other employees."), (False, "No. (We’ll ask you to explain why.)")), initial=initial_value, widget=forms.RadioSelect, ) diff --git a/src/registrar/templates/application_other_contacts.html b/src/registrar/templates/application_other_contacts.html index b14458bcc..900134c0a 100644 --- a/src/registrar/templates/application_other_contacts.html +++ b/src/registrar/templates/application_other_contacts.html @@ -84,9 +84,9 @@
-
+
-

No other employees from your organization?

+

No other employees from your organization?

You don't need to provide names of other employees now, but it may slow down our assessment of your eligibility. Describe why there are From 73b0b33ee895d9c1455f02fb9d490fd8a6586fa4 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 12 Jan 2024 18:02:25 -0500 Subject: [PATCH 061/100] moved reverse_joins definition centrally to contact model --- src/registrar/forms/application_wizard.py | 45 ++++------------------- src/registrar/forms/domain.py | 11 +----- src/registrar/models/contact.py | 15 ++++++-- 3 files changed, 21 insertions(+), 50 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index fba1e6da1..cbdb29579 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -100,7 +100,6 @@ class RegistrarFormSet(forms.BaseFormSet): self, obj: DomainApplication, join: str, - reverse_joins: list, should_delete: Callable, pre_update: Callable, pre_create: Callable, @@ -137,14 +136,14 @@ class RegistrarFormSet(forms.BaseFormSet): # matching database object exists, update it if db_obj is not None and cleaned: if should_delete(cleaned): - if hasattr(db_obj, "has_more_than_one_join") and db_obj.has_more_than_one_join(reverse_joins, related_name): + if hasattr(db_obj, "has_more_than_one_join") and db_obj.has_more_than_one_join(related_name): # Remove the specific relationship without deleting the object getattr(db_obj, related_name).remove(self.application) else: # If there are no other relationships, delete the object db_obj.delete() else: - if hasattr(db_obj, "has_more_than_one_join") and db_obj.has_more_than_one_join(reverse_joins, related_name): + if hasattr(db_obj, "has_more_than_one_join") and db_obj.has_more_than_one_join(related_name): # create a new db_obj and disconnect existing one getattr(db_obj, related_name).remove(self.application) kwargs = pre_create(db_obj, cleaned) @@ -330,21 +329,12 @@ class AboutYourOrganizationForm(RegistrarForm): class AuthorizingOfficialForm(RegistrarForm): JOIN = "authorizing_official" - REVERSE_JOINS = [ - "user", - "authorizing_official", - "submitted_applications", - "contact_applications", - "information_authorizing_official", - "submitted_applications_information", - "contact_applications_information", - ] def to_database(self, obj): if not self.is_valid(): return contact = getattr(obj, "authorizing_official", None) - if contact is not None and not contact.has_more_than_one_join(self.REVERSE_JOINS, "authorizing_official"): + if contact is not None and not contact.has_more_than_one_join("authorizing_official"): # if contact exists in the database and is not joined to other entities super().to_database(contact) else: @@ -403,7 +393,7 @@ class BaseCurrentSitesFormSet(RegistrarFormSet): def to_database(self, obj: DomainApplication): # If we want to test against multiple joins for a website object, replace the empty array # and change the JOIN in the models to allow for reverse references - self._to_database(obj, self.JOIN, [], self.should_delete, self.pre_update, self.pre_create) + self._to_database(obj, self.JOIN, self.should_delete, self.pre_update, self.pre_create) @classmethod def from_database(cls, obj): @@ -462,7 +452,7 @@ class BaseAlternativeDomainFormSet(RegistrarFormSet): def to_database(self, obj: DomainApplication): # If we want to test against multiple joins for a website object, replace the empty array and # change the JOIN in the models to allow for reverse references - self._to_database(obj, self.JOIN, [], self.should_delete, self.pre_update, self.pre_create) + self._to_database(obj, self.JOIN, self.should_delete, self.pre_update, self.pre_create) @classmethod def on_fetch(cls, query): @@ -542,21 +532,12 @@ class PurposeForm(RegistrarForm): class YourContactForm(RegistrarForm): JOIN = "submitter" - REVERSE_JOINS = [ - "user", - "authorizing_official", - "submitted_applications", - "contact_applications", - "information_authorizing_official", - "submitted_applications_information", - "contact_applications_information", - ] def to_database(self, obj): if not self.is_valid(): return contact = getattr(obj, "submitter", None) - if contact is not None and not contact.has_more_than_one_join(self.REVERSE_JOINS, "submitted_applications"): + if contact is not None and not contact.has_more_than_one_join("submitted_applications"): # if contact exists in the database and is not joined to other entities super().to_database(contact) else: @@ -711,20 +692,10 @@ class BaseOtherContactsFormSet(RegistrarFormSet): must co-exist. Also, other_contacts have db relationships to multiple db objects. When attempting to delete an other_contact from an application, those db relationships must be - tested and handled; this is configured with REVERSE_JOINS, which is an array of - strings representing the relationships between contact model and other models. + tested and handled. """ JOIN = "other_contacts" - REVERSE_JOINS = [ - "user", - "authorizing_official", - "submitted_applications", - "contact_applications", - "information_authorizing_official", - "submitted_applications_information", - "contact_applications_information", - ] def get_deletion_widget(self): return forms.HiddenInput(attrs={"class": "deletion"}) @@ -756,7 +727,7 @@ class BaseOtherContactsFormSet(RegistrarFormSet): return cleaned def to_database(self, obj: DomainApplication): - self._to_database(obj, self.JOIN, self.REVERSE_JOINS, self.should_delete, self.pre_update, self.pre_create) + self._to_database(obj, self.JOIN, self.should_delete, self.pre_update, self.pre_create) @classmethod def from_database(cls, obj): diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 9b28af2d6..4db714b5c 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -213,15 +213,6 @@ class ContactForm(forms.ModelForm): class AuthorizingOfficialContactForm(ContactForm): """Form for updating authorizing official contacts.""" JOIN = "authorizing_official" - REVERSE_JOINS = [ - "user", - "authorizing_official", - "submitted_applications", - "contact_applications", - "information_authorizing_official", - "submitted_applications_information", - "contact_applications_information", - ] def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -258,7 +249,7 @@ class AuthorizingOfficialContactForm(ContactForm): # get db object db_ao = Contact.objects.get(id=self.instance.id) logger.info(f"db_ao.information_authorizing_official {db_ao.information_authorizing_official}") - if self.domainInfo and db_ao.has_more_than_one_join(self.REVERSE_JOINS, "information_authorizing_official"): + if self.domainInfo and db_ao.has_more_than_one_join("information_authorizing_official"): logger.info(f"domain info => {self.domainInfo}") logger.info(f"authorizing official id => {self.domainInfo.authorizing_official.id}") contact = Contact() diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index 483752c56..02f13114f 100644 --- a/src/registrar/models/contact.py +++ b/src/registrar/models/contact.py @@ -54,10 +54,19 @@ class Contact(TimeStampedModel): db_index=True, ) - def has_more_than_one_join(self, all_relations, expected_relation): + def has_more_than_one_join(self, expected_relation): """Helper for finding whether an object is joined more than once. - all_relations is the list of all_relations to be checked for existing joins. expected_relation is the one relation with one expected join""" + # all_relations is the list of all_relations (from contact) to be checked for existing joins + all_relations = [ + "user", + "authorizing_official", + "submitted_applications", + "contact_applications", + "information_authorizing_official", + "submitted_applications_information", + "contact_applications_information", + ] return any(self._has_more_than_one_join_per_relation(rel, expected_relation) for rel in all_relations) def _has_more_than_one_join_per_relation(self, relation, expected_relation): @@ -70,7 +79,7 @@ class Contact(TimeStampedModel): threshold = 1 if relation == expected_relation else 0 # Raise a KeyError if rel is not a defined field on the db_obj model - # This will help catch any errors in reverse_join config on forms + # This will help catch any errors in relation passed. if relation not in [field.name for field in self._meta.get_fields()]: raise KeyError(f"{relation} is not a defined field on the {self._meta.model_name} model.") From 946baf05e954587698a57027540c523cbba16d4f Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 12 Jan 2024 18:07:02 -0500 Subject: [PATCH 062/100] formatted for linter --- src/registrar/forms/application_wizard.py | 4 ++-- src/registrar/forms/domain.py | 2 ++ src/registrar/models/contact.py | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index cbdb29579..a1842c911 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -33,7 +33,7 @@ class RegistrarForm(forms.Form): # save a reference to an application object self.application = kwargs.pop("application", None) super(RegistrarForm, self).__init__(*args, **kwargs) - + def to_database(self, obj: DomainApplication | Contact): """ Adds this form's cleaned data to `obj` and saves `obj`. @@ -329,7 +329,7 @@ class AboutYourOrganizationForm(RegistrarForm): class AuthorizingOfficialForm(RegistrarForm): JOIN = "authorizing_official" - + def to_database(self, obj): if not self.is_valid(): return diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 4db714b5c..eabfba6b0 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -26,6 +26,7 @@ import re logger = logging.getLogger(__name__) + class DomainAddUserForm(forms.Form): """Form for adding a user to a domain.""" @@ -212,6 +213,7 @@ class ContactForm(forms.ModelForm): class AuthorizingOfficialContactForm(ContactForm): """Form for updating authorizing official contacts.""" + JOIN = "authorizing_official" def __init__(self, *args, **kwargs): diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index 02f13114f..d5d673b32 100644 --- a/src/registrar/models/contact.py +++ b/src/registrar/models/contact.py @@ -68,7 +68,7 @@ class Contact(TimeStampedModel): "contact_applications_information", ] return any(self._has_more_than_one_join_per_relation(rel, expected_relation) for rel in all_relations) - + def _has_more_than_one_join_per_relation(self, relation, expected_relation): """Helper for finding whether an object is joined more than once.""" # threshold is the number of related objects that are acceptable @@ -96,7 +96,7 @@ class Contact(TimeStampedModel): # the threshold return getattr(self, relation).count() > threshold return False - + def get_formatted_name(self): """Returns the contact's name in Western order.""" names = [n for n in [self.first_name, self.middle_name, self.last_name] if n] From d493fe293be0fa87091bb14d0a208c38b61bf3c8 Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Tue, 16 Jan 2024 09:36:19 -0600 Subject: [PATCH 063/100] Updates to ".gov domain" page --- .../templates/application_dotgov_domain.html | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/registrar/templates/application_dotgov_domain.html b/src/registrar/templates/application_dotgov_domain.html index bd3c4a473..e21b679bb 100644 --- a/src/registrar/templates/application_dotgov_domain.html +++ b/src/registrar/templates/application_dotgov_domain.html @@ -2,24 +2,22 @@ {% load static field_helpers url_helpers %} {% block form_instructions %} -

Before requesting a .gov domain, please make sure it - meets our naming requirements. Your domain name must: +

Before requesting a .gov domain, please make sure it meets our naming requirements. Your domain name must:

  • Be available
    • -
  • Be unique
  • Relate to your organization’s name, location, and/or services
  • Be clear to the general public. Your domain name must not be easily confused with other organizations.

+

Names that uniquely apply to your organization are likely to be approved over names that could also apply to other organizations. In most instances, this requires including your state’s two-letter abbreviation.

+ +

Requests for your organization’s initials or an abbreviated name might not be approved, but we encourage you to request the name you want.

+

Note that only federal agencies can request generic terms like vote.gov.

-

We’ll try to give you the domain you want. We first need to make sure your request - meets our requirements. We’ll work with you to find the best domain for your - organization.

-

Domain examples for your type of organization

{% include "includes/domain_example.html" %} @@ -87,6 +85,4 @@
-

If you’re not sure this is the domain you want, that’s - okay. You can change it later.

{% endblock %} From 2ccca5c5065ac1bc3b194900f964fb6c08d42a13 Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Tue, 16 Jan 2024 09:48:21 -0600 Subject: [PATCH 064/100] Update domain examples --- .../templates/includes/domain_example.html | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/registrar/templates/includes/domain_example.html b/src/registrar/templates/includes/domain_example.html index 3b34b8e80..5058cf701 100644 --- a/src/registrar/templates/includes/domain_example.html +++ b/src/registrar/templates/includes/domain_example.html @@ -40,8 +40,7 @@
  • AmericanSamoa.gov
  • Colorado.gov
    • -
  • Georgia.gov
    • -
  • AmericanSamoa.gov
    • +
  • MN.gov
  • Guam.gov
@@ -55,45 +54,49 @@ {% elif organization_type == 'county' %} -

Most county .gov domains must include the two-letter state abbreviation or the full state name. County names that aren’t shared by any other city, county, parish, town, borough, village or equivalent in the U.S., at the time a domain is granted, can be requested without referring to the state. Counties can include “county” in their domain to distinguish it from other places with similar names. We use the Census Bureau’s National Places Gazetteer Files to determine if county names are unique.

+

Most county .gov domains must include the two-letter state abbreviation or the full state name. County names that aren’t shared by any other city, county, parish, town, borough, village or equivalent in the U.S. (at the time a domain is granted) don’t have to refer to their state in their domain name. Counties can include “county” in their domain to distinguish it from other places with similar names.

+ +

We use the Census Bureau’s National Places Gazetteer Files to determine if county names are unique.

Examples:

    -
  • AdamsCountyMS.gov
    • -
  • Erie.gov
    • +
  • LACounty.gov
  • LivingstonParishLA.gov
  • MitchellCountyNC.gov
    • +
  • MiamiDade.gov
{% elif organization_type == 'city' %}

Most city domains must include the two-letter state abbreviation or clearly spell out the state name. Using phrases like “City of” or “Town of” is optional.

Cities that meet one of the criteria below don’t have to refer to their state in the domain name.

    -
  • City names that are not shared by any other U.S. city, town, or village can be requested without referring to the state. We use the Census Bureau’s National Places Gazetteer Files to determine if names are unique.
    • -
  • Certain cities are so well-known that they may not require a state reference to communicate location. We use the list of U.S. “dateline cities” in the Associated Press Stylebook to make this determination.
    • -
  • The 50 largest cities, as measured by population according to the Census Bureau, can have .gov domain names that don’t refer to their state.
    • +
  • The city name is not shared by any other U.S. city, town, village, or county. We use the Census Bureau’s National Places Gazetteer Files to determine if names are unique.
    • +
  • The city is so well known that it doesn’t need a state reference to communicate location. We use the list of U.S. “dateline cities” in the Associated Press Stylebook as part of our decision.
    • +
  • It’s one of the 150 largest cities by population, according to the Census Bureau.

Examples:

  • CityofEudoraKS.gov
    • -
  • Pocatello.gov
  • WallaWallaWA.gov
    • +
  • Pocatello.gov

{% elif organization_type == 'special_district' %} -

Domain names must represent your organization or institutional name, not solely the services you provide. It also needs to include your two-letter state abbreviation or clearly spell out the state name unless county or city exceptions apply.

+

Domain names must represent your organization or institutional name, not solely the services you provide. It also needs to include your two-letter state abbreviation or clearly spell out the state name.

Examples:

    -
  • ElectionsShelbyTN.gov
  • GlacierViewFire.gov
    • -
  • HVcoVote.gov
  • TechshareTX.gov
  • UtahTrust.gov
{% elif organization_type == 'school_district' %}

Domain names must represent your organization or institutional name.

-

Example: mckinneyISDTX.gov

+

Examples:

+
    +
  • mckinneyISDTX.gov
    • +
  • BooneCSDIA.gov
    • +
{%endif %} From 6bd16fe9cb2c57fd70bc9d02d6d7f9e2d73336f9 Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Tue, 16 Jan 2024 09:51:46 -0600 Subject: [PATCH 065/100] Update census link --- src/registrar/templates/includes/domain_example.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/includes/domain_example.html b/src/registrar/templates/includes/domain_example.html index 5058cf701..b378d4ed5 100644 --- a/src/registrar/templates/includes/domain_example.html +++ b/src/registrar/templates/includes/domain_example.html @@ -71,7 +71,7 @@
  • The city name is not shared by any other U.S. city, town, village, or county. We use the Census Bureau’s National Places Gazetteer Files to determine if names are unique.
  • The city is so well known that it doesn’t need a state reference to communicate location. We use the list of U.S. “dateline cities” in the Associated Press Stylebook as part of our decision.
    • -
  • It’s one of the 150 largest cities by population, according to the Census Bureau.
    • +
  • It’s one of the 150 largest cities by population, according to the Census Bureau.

Examples:

    From 00dc7cacbab15eb6ae732643b0ce59ea14b80e85 Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Tue, 16 Jan 2024 11:45:18 -0600 Subject: [PATCH 066/100] Updates to purpose page --- src/registrar/templates/application_purpose.html | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/registrar/templates/application_purpose.html b/src/registrar/templates/application_purpose.html index 8747a34c7..d71d068de 100644 --- a/src/registrar/templates/application_purpose.html +++ b/src/registrar/templates/application_purpose.html @@ -2,14 +2,10 @@ {% load field_helpers url_helpers %} {% block form_instructions %} -

    .Gov domain names are for use on the internet. Don’t register a .gov to simply reserve a -domain name or for mainly internal use.

    - -

    Describe the reason for your domain request. Explain how you plan to use this domain. -Who is your intended audience? Will you use it for a website and/or email? Are you moving -your website from another top-level domain (like .com or .org)? -Read about activities that are prohibited on .gov domains.

    - +

    .Gov domains are intended for public use. Domains will not be given to organizations that only want to reserve a domain name (defensive registration) or that only intend to use the domain internally (as for an intranet).

    +

    Read about activities that are prohibited on .gov domains.

    +

    What is the purpose of your requested domain?

    +

    Describe how you’ll use your .gov domain. Will it be used for a website, email, or something else?

    {% endblock %} {% block form_required_fields_help_text %} From 6a7a23f63ab522148ed0769a9efb139c76b33830 Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Tue, 16 Jan 2024 11:51:49 -0600 Subject: [PATCH 067/100] Updates to your contact info page --- src/registrar/templates/application_your_contact.html | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/registrar/templates/application_your_contact.html b/src/registrar/templates/application_your_contact.html index 9456bbbb3..080b84eb6 100644 --- a/src/registrar/templates/application_your_contact.html +++ b/src/registrar/templates/application_your_contact.html @@ -2,14 +2,11 @@ {% load field_helpers %} {% block form_instructions %} -

    We’ll use this information to contact you about your domain request.

    +

    While reviewing your domain request, we may need to reach out with questions. We’ll also email you when we complete our review.

    -

    If you’d like us to use a different name, email, or phone number you can make those - changes below. Changing your contact information here won’t affect your Login.gov - account information.

    +

    What contact information should we use to reach you?

    -

    The contact information you provide here won’t be public and will only be used to - support your domain request.

    +

    Your contact information won’t be made public and will be used only for .gov purposes. The information you provide here won't impact your Login.gov account information.

    {% endblock %} From 76ec2b030a67320389e015a504a39ae3eb980d52 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 16 Jan 2024 15:20:35 -0500 Subject: [PATCH 068/100] wrote test code --- src/registrar/models/contact.py | 3 +- src/registrar/tests/test_models.py | 13 + src/registrar/tests/test_views.py | 492 +++++++++++++++++++++++++++++ 3 files changed, 507 insertions(+), 1 deletion(-) diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index d5d673b32..f2e8c7bfe 100644 --- a/src/registrar/models/contact.py +++ b/src/registrar/models/contact.py @@ -89,7 +89,8 @@ class Contact(TimeStampedModel): if isinstance(field, models.OneToOneField): # if the rel field is a OneToOne field, then we have already # determined that the object exists (is not None) - return True + # so return True unless the relation being tested is the expected_relation + return True if relation != expected_relation else False elif isinstance(field, models.ForeignObjectRel): # if the rel field is a ManyToOne or ManyToMany, then we need # to determine if the count of related objects is greater than diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 672b8f465..e28e56b00 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -695,8 +695,12 @@ class TestContact(TestCase): self.user, _ = User.objects.get_or_create(email=self.email, first_name="Jeff", last_name="Lebowski") self.contact, _ = Contact.objects.get_or_create(user=self.user) + self.contact_as_ao, _ = Contact.objects.get_or_create(email="newguy@igorville.gov") + self.application = DomainApplication.objects.create(creator=self.user, authorizing_official=self.contact_as_ao) + def tearDown(self): super().tearDown() + DomainApplication.objects.all().delete() Contact.objects.all().delete() User.objects.all().delete() @@ -770,3 +774,12 @@ class TestContact(TestCase): # Updating the contact's email does not propagate self.assertEqual(self.invalid_contact.email, "joey.baloney@diaperville.com") self.assertEqual(self.invalid_user.email, "intern@igorville.gov") + + def test_has_more_than_one_join(self): + """Test the Contact model method, has_more_than_one_join""" + # test for a contact which has one user defined + self.assertFalse(self.contact.has_more_than_one_join("user")) + self.assertTrue(self.contact.has_more_than_one_join("authorizing_official")) + # test for a contact which is assigned as an authorizing official on an application + self.assertFalse(self.contact_as_ao.has_more_than_one_join("authorizing_official")) + self.assertTrue(self.contact_as_ao.has_more_than_one_join("submitted_applications")) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index eb254580a..a2fc40377 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1313,6 +1313,440 @@ class DomainApplicationTests(TestWithUser, WebTest): # Enter the first name ... self.assertContains(response, "Enter the first name / given name of this contact.") + def test_edit_other_contact_in_place(self): + """When you: + 1. edit an existing contact which is not joined to another model, + 2. then submit, + The application is linked to the existing contact, and the existing contact updated.""" + + # Populate the database with a domain application that + # has 1 "other contact" assigned to it + # We'll do it from scratch + ao, _ = Contact.objects.get_or_create( + first_name="Testy", + last_name="Tester", + title="Chief Tester", + email="testy@town.com", + phone="(201) 555 5555", + ) + you, _ = Contact.objects.get_or_create( + first_name="Testy you", + last_name="Tester you", + title="Admin Tester", + email="testy-admin@town.com", + phone="(201) 555 5556", + ) + other, _ = Contact.objects.get_or_create( + first_name="Testy2", + last_name="Tester2", + title="Another Tester", + email="testy2@town.com", + phone="(201) 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, + submitter=you, + creator=self.user, + status="started", + ) + application.other_contacts.add(other) + + # other_contact_pk is the initial pk of the other contact. set it before update + # to be able to verify after update that the same contact object is in place + other_contact_pk = other.id + + # prime the form by visiting /edit + 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] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + other_contacts_page = self.app.get(reverse("application:other_contacts")) + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + other_contacts_form = other_contacts_page.forms[0] + + # Minimal check to ensure the form is loaded + self.assertEqual(other_contacts_form["other_contacts-0-first_name"].value, "Testy2") + + # update the first name of the contact + other_contacts_form["other_contacts-0-first_name"] = "Testy3" + + # Submit the updated form + other_contacts_form.submit() + + application.refresh_from_db() + + # assert that the Other Contact is updated "in place" + other_contact = application.other_contacts.all()[0] + self.assertEquals(other_contact_pk, other_contact.id) + self.assertEquals("Testy3", other_contact.first_name) + + def test_edit_other_contact_creates_new(self): + """When you: + 1. edit an existing contact which IS joined to another model, + 2. then submit, + The application is linked to a new contact, and the new contact is updated.""" + + # Populate the database with a domain application that + # has 1 "other contact" assigned to it, the other contact is also + # the authorizing official initially + # We'll do it from scratch + ao, _ = Contact.objects.get_or_create( + first_name="Testy", + last_name="Tester", + title="Chief Tester", + email="testy@town.com", + phone="(201) 555 5555", + ) + you, _ = Contact.objects.get_or_create( + first_name="Testy you", + last_name="Tester you", + title="Admin Tester", + email="testy-admin@town.com", + phone="(201) 555 5556", + ) + 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, + submitter=you, + creator=self.user, + status="started", + ) + application.other_contacts.add(ao) + + # other_contact_pk is the initial pk of the other contact. set it before update + # to be able to verify after update that the ao contact is still in place + # and not updated, and that the new contact has a new id + other_contact_pk = ao.id + + # prime the form by visiting /edit + 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] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + other_contacts_page = self.app.get(reverse("application:other_contacts")) + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + other_contacts_form = other_contacts_page.forms[0] + + # Minimal check to ensure the form is loaded + self.assertEqual(other_contacts_form["other_contacts-0-first_name"].value, "Testy") + + # update the first name of the contact + other_contacts_form["other_contacts-0-first_name"] = "Testy2" + + # Submit the updated form + other_contacts_form.submit() + + application.refresh_from_db() + + # assert that other contact info is updated, and that a new Contact + # is created for the other contact + other_contact = application.other_contacts.all()[0] + self.assertNotEquals(other_contact_pk, other_contact.id) + self.assertEquals("Testy2", other_contact.first_name) + # assert that the authorizing official is not updated + authorizing_official = application.authorizing_official + self.assertEquals("Testy", authorizing_official.first_name) + + def test_edit_authorizing_official_in_place(self): + """When you: + 1. edit an authorizing official which is not joined to another model, + 2. then submit, + The application is linked to the existing ao, and the ao updated.""" + + # Populate the database with a domain application that + # has an authorizing_official (ao) + # We'll do it from scratch + ao, _ = Contact.objects.get_or_create( + first_name="Testy", + last_name="Tester", + title="Chief Tester", + email="testy@town.com", + phone="(201) 555 5555", + ) + 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, + creator=self.user, + status="started", + ) + + # ao_pk is the initial pk of the Authorizing Official. set it before update + # to be able to verify after update that the same Contact object is in place + ao_pk = ao.id + + # prime the form by visiting /edit + 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] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + ao_page = self.app.get(reverse("application:authorizing_official")) + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + ao_form = ao_page.forms[0] + + # Minimal check to ensure the form is loaded + self.assertEqual(ao_form["authorizing_official-first_name"].value, "Testy") + + # update the first name of the contact + ao_form["authorizing_official-first_name"] = "Testy2" + + # Submit the updated form + ao_form.submit() + + application.refresh_from_db() + + # assert AO is updated "in place" + updated_ao = application.authorizing_official + self.assertEquals(ao_pk, updated_ao.id) + self.assertEquals("Testy2", updated_ao.first_name) + + def test_edit_authorizing_official_creates_new(self): + """When you: + 1. edit an existing authorizing official which IS joined to another model, + 2. then submit, + The application is linked to a new Contact, and the new Contact is updated.""" + + # Populate the database with a domain application that + # has authorizing official assigned to it, the authorizing offical is also + # an other contact initially + # We'll do it from scratch + ao, _ = Contact.objects.get_or_create( + first_name="Testy", + last_name="Tester", + title="Chief Tester", + email="testy@town.com", + phone="(201) 555 5555", + ) + 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, + creator=self.user, + status="started", + ) + application.other_contacts.add(ao) + + # ao_pk is the initial pk of the authorizing official. set it before update + # to be able to verify after update that the other contact is still in place + # and not updated, and that the new ao has a new id + ao_pk = ao.id + + # prime the form by visiting /edit + 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] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + ao_page = self.app.get(reverse("application:authorizing_official")) + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + ao_form = ao_page.forms[0] + + # Minimal check to ensure the form is loaded + self.assertEqual(ao_form["authorizing_official-first_name"].value, "Testy") + + # update the first name of the contact + ao_form["authorizing_official-first_name"] = "Testy2" + + # Submit the updated form + ao_form.submit() + + application.refresh_from_db() + + # assert that the other contact is not updated + other_contacts = application.other_contacts.all() + other_contact = other_contacts[0] + self.assertEquals(ao_pk, other_contact.id) + self.assertEquals("Testy", other_contact.first_name) + # assert that the authorizing official is updated + authorizing_official = application.authorizing_official + self.assertEquals("Testy2", authorizing_official.first_name) + + def test_edit_submitter_in_place(self): + """When you: + 1. edit a submitter (your contact) which is not joined to another model, + 2. then submit, + The application is linked to the existing submitter, and the submitter updated.""" + + # Populate the database with a domain application that + # has a submitter + # We'll do it from scratch + you, _ = Contact.objects.get_or_create( + first_name="Testy", + last_name="Tester", + title="Chief Tester", + email="testy@town.com", + phone="(201) 555 5555", + ) + 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", + submitter=you, + creator=self.user, + status="started", + ) + + # submitter_pk is the initial pk of the submitter. set it before update + # to be able to verify after update that the same contact object is in place + submitter_pk = you.id + + # prime the form by visiting /edit + 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] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + your_contact_page = self.app.get(reverse("application:your_contact")) + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + your_contact_form = your_contact_page.forms[0] + + # Minimal check to ensure the form is loaded + self.assertEqual(your_contact_form["your_contact-first_name"].value, "Testy") + + # update the first name of the contact + your_contact_form["your_contact-first_name"] = "Testy2" + + # Submit the updated form + your_contact_form.submit() + + application.refresh_from_db() + + updated_submitter = application.submitter + self.assertEquals(submitter_pk, updated_submitter.id) + self.assertEquals("Testy2", updated_submitter.first_name) + + def test_edit_submitter_creates_new(self): + """When you: + 1. edit an existing your contact which IS joined to another model, + 2. then submit, + The application is linked to a new Contact, and the new Contact is updated.""" + + # Populate the database with a domain application that + # has submitter assigned to it, the submitter is also + # an other contact initially + # We'll do it from scratch + submitter, _ = Contact.objects.get_or_create( + first_name="Testy", + last_name="Tester", + title="Chief Tester", + email="testy@town.com", + phone="(201) 555 5555", + ) + 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", + submitter=submitter, + creator=self.user, + status="started", + ) + application.other_contacts.add(submitter) + + # submitter_pk is the initial pk of the your contact. set it before update + # to be able to verify after update that the other contact is still in place + # and not updated, and that the new submitter has a new id + submitter_pk = submitter.id + + # prime the form by visiting /edit + 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] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + your_contact_page = self.app.get(reverse("application:your_contact")) + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + your_contact_form = your_contact_page.forms[0] + + # Minimal check to ensure the form is loaded + self.assertEqual(your_contact_form["your_contact-first_name"].value, "Testy") + + # update the first name of the contact + your_contact_form["your_contact-first_name"] = "Testy2" + + # Submit the updated form + your_contact_form.submit() + + application.refresh_from_db() + + # assert that the other contact is not updated + other_contacts = application.other_contacts.all() + other_contact = other_contacts[0] + self.assertEquals(submitter_pk, other_contact.id) + self.assertEquals("Testy", other_contact.first_name) + # assert that the submitter is updated + submitter = application.submitter + self.assertEquals("Testy2", submitter.first_name) + def test_application_about_your_organiztion_interstate(self): """Special districts have to answer an additional question.""" intro_page = self.app.get(reverse("application:")) @@ -2636,6 +3070,64 @@ class TestDomainAuthorizingOfficial(TestDomainOverview): page = self.app.get(reverse("domain-authorizing-official", kwargs={"pk": self.domain.id})) self.assertContains(page, "Testy") + def test_domain_edit_authorizing_official_in_place(self): + """When editing an authorizing official for domain information and AO is not + joined to any other objects""" + self.domain_information.authorizing_official = Contact( + first_name="Testy", last_name="Tester", title="CIO", email="nobody@igorville.gov" + ) + self.domain_information.authorizing_official.save() + self.domain_information.save() + ao_page = self.app.get(reverse("domain-authorizing-official", kwargs={"pk": self.domain.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + ao_form = ao_page.forms[0] + self.assertEqual(ao_form["first_name"].value, "Testy") + ao_form["first_name"] = "Testy2" + # ao_pk is the initial pk of the authorizing official. set it before update + # to be able to verify after update that the same contact object is in place + ao_pk = self.domain_information.authorizing_official.id + result = ao_form.submit() + + # refresh domain information + self.domain_information.refresh_from_db() + self.assertEqual("Testy2", self.domain_information.authorizing_official.first_name) + self.assertEqual(ao_pk, self.domain_information.authorizing_official.id) + + def test_domain_edit_authorizing_official_creates_new(self): + """When editing an authorizing official for domain information and AO IS + joined to another object""" + # set AO and Other Contact to the same Contact object + self.domain_information.authorizing_official = Contact( + first_name="Testy", last_name="Tester", title="CIO", email="nobody@igorville.gov" + ) + self.domain_information.authorizing_official.save() + self.domain_information.save() + self.domain_information.other_contacts.add(self.domain_information.authorizing_official) + self.domain_information.save() + # load the Authorizing Official in the web form + ao_page = self.app.get(reverse("domain-authorizing-official", kwargs={"pk": self.domain.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + ao_form = ao_page.forms[0] + # verify the first name is "Testy" and then change it to "Testy2" + self.assertEqual(ao_form["first_name"].value, "Testy") + ao_form["first_name"] = "Testy2" + # ao_pk is the initial pk of the authorizing official. set it before update + # to be able to verify after update that the same contact object is in place + ao_pk = self.domain_information.authorizing_official.id + ao_form.submit() + + # refresh domain information + self.domain_information.refresh_from_db() + # assert that AO information is updated, and that the AO is a new Contact + self.assertEqual("Testy2", self.domain_information.authorizing_official.first_name) + self.assertNotEqual(ao_pk, self.domain_information.authorizing_official.id) + # assert that the Other Contact information is not updated and that the Other Contact + # is the original Contact object + other_contact = self.domain_information.other_contacts.all()[0] + self.assertEqual("Testy", other_contact.first_name) + self.assertEqual(ao_pk, other_contact.id) class TestDomainOrganization(TestDomainOverview): def test_domain_org_name_address(self): From 3d1dab93c2ee69cdd12a793d85a658013f9b6b06 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 16 Jan 2024 15:29:24 -0500 Subject: [PATCH 069/100] Change required message on yes/no form --- src/registrar/forms/application_wizard.py | 3 +++ src/registrar/templates/application_other_contacts.html | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index bf62769f8..33cebd273 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -612,6 +612,9 @@ class OtherContactsYesNoForm(RegistrarForm): choices=((True, "Yes, I can name other employees."), (False, "No. (We’ll ask you to explain why.)")), initial=initial_value, widget=forms.RadioSelect, + error_messages={ + 'required': 'This question is required.', + } ) diff --git a/src/registrar/templates/application_other_contacts.html b/src/registrar/templates/application_other_contacts.html index 900134c0a..c8810edce 100644 --- a/src/registrar/templates/application_other_contacts.html +++ b/src/registrar/templates/application_other_contacts.html @@ -51,7 +51,7 @@ {{ form.DELETE }} {% endif %} -
    +
    {% input_with_errors form.first_name %}
    From adea22fe9f3f694f6c83b080e5a58c3775e04716 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 16 Jan 2024 15:30:23 -0500 Subject: [PATCH 070/100] linting --- src/registrar/tests/test_views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index a2fc40377..4e2fab261 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -3087,7 +3087,7 @@ class TestDomainAuthorizingOfficial(TestDomainOverview): # ao_pk is the initial pk of the authorizing official. set it before update # to be able to verify after update that the same contact object is in place ao_pk = self.domain_information.authorizing_official.id - result = ao_form.submit() + ao_form.submit() # refresh domain information self.domain_information.refresh_from_db() @@ -3129,6 +3129,7 @@ class TestDomainAuthorizingOfficial(TestDomainOverview): self.assertEqual("Testy", other_contact.first_name) self.assertEqual(ao_pk, other_contact.id) + class TestDomainOrganization(TestDomainOverview): def test_domain_org_name_address(self): """Can load domain's org name and mailing address page.""" From f01afadee25930b98d98f8252e9de32e7c48b001 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 16 Jan 2024 15:42:36 -0500 Subject: [PATCH 071/100] lint --- src/registrar/forms/application_wizard.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 33cebd273..6f1fe0cce 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -613,8 +613,8 @@ class OtherContactsYesNoForm(RegistrarForm): initial=initial_value, widget=forms.RadioSelect, error_messages={ - 'required': 'This question is required.', - } + "required": "This question is required.", + }, ) From b5681041942f462c4accccf1e572898cc4ad55f7 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 16 Jan 2024 17:06:20 -0500 Subject: [PATCH 072/100] updated comments in code for readability; cleaned up debugging messages --- src/registrar/forms/domain.py | 22 +++++++++------------- src/registrar/views/domain.py | 3 +++ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index eabfba6b0..030fac86d 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -1,7 +1,5 @@ """Forms for domain management.""" -import logging - from django import forms from django.core.validators import MinValueValidator, MaxValueValidator, RegexValidator from django.forms import formset_factory @@ -24,8 +22,6 @@ from .common import ( import re -logger = logging.getLogger(__name__) - class DomainAddUserForm(forms.Form): """Form for adding a user to a domain.""" @@ -239,21 +235,21 @@ class AuthorizingOfficialContactForm(ContactForm): self.domainInfo = None def setDomainInfo(self, domainInfo): + """Set the domain information for the form. + The form instance is associated with the contact itself. In order to access the associated + domain information object, this needs to be set in the form by the view.""" self.domainInfo = domainInfo def save(self, commit=True): - logger.info(f"in save: {self.instance}") - logger.info(f"{self.instance.__class__.__name__}") - logger.info(f"{self.instance.id}") - logger.info(f"self.fields => {self.fields}") - logger.info(f"domain info: {self.instance.information_authorizing_official}") + """Override the save() method of the BaseModelForm.""" - # get db object + # Get the Contact object from the db for the Authorizing Official db_ao = Contact.objects.get(id=self.instance.id) - logger.info(f"db_ao.information_authorizing_official {db_ao.information_authorizing_official}") if self.domainInfo and db_ao.has_more_than_one_join("information_authorizing_official"): - logger.info(f"domain info => {self.domainInfo}") - logger.info(f"authorizing official id => {self.domainInfo.authorizing_official.id}") + # Handle the case where the domain information object is available and the AO Contact + # has more than one joined object. + # In this case, create a new Contact, and update the new Contact with form data. + # Then associate with domain information object as the authorizing_official contact = Contact() for name, value in self.cleaned_data.items(): setattr(contact, name, value) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index aa37b15b0..b5ba25a89 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -222,6 +222,9 @@ class DomainAuthorizingOfficialView(DomainFormBaseView): def form_valid(self, form): """The form is valid, save the authorizing official.""" + # Set the domain information in the form so that it can be accessible + # to associate a new Contact as authorizing official, if new Contact is needed + # in the save() method form.setDomainInfo(self.object.domain_info) form.save() From 62596e6f04405277108f4474439320b2d02b48ac Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Tue, 16 Jan 2024 17:00:55 -0600 Subject: [PATCH 073/100] Update to "anything else" page --- src/registrar/templates/application_anything_else.html | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/registrar/templates/application_anything_else.html b/src/registrar/templates/application_anything_else.html index f69b7e70e..ae76856f5 100644 --- a/src/registrar/templates/application_anything_else.html +++ b/src/registrar/templates/application_anything_else.html @@ -2,7 +2,9 @@ {% load field_helpers %} {% block form_instructions %} -

    Is there anything else you'd like us to know about your domain request? This question is optional.

    +

    Is there anything else you'd like us to know about your domain request?

    + +

    This question is optional.

    {% endblock %} {% block form_required_fields_help_text %} From 153faf43da7be940b4fdb59cc01e5dbf8584a596 Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Tue, 16 Jan 2024 17:11:00 -0600 Subject: [PATCH 074/100] Updates to requirements page --- .../templates/application_requirements.html | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/registrar/templates/application_requirements.html b/src/registrar/templates/application_requirements.html index c1600d523..d06f5de40 100644 --- a/src/registrar/templates/application_requirements.html +++ b/src/registrar/templates/application_requirements.html @@ -3,51 +3,55 @@ {% block form_instructions %}

    Please read this page. Check the box at the bottom to show that you agree to the requirements for operating .gov domains.

    -

    The .gov domain space exists to support a broad diversity of government missions. Generally, we don’t examine how government organizations use their domains. However, misuse of a .gov domain can reflect upon the integrity of the entire .gov space. There are categories of misuse that are statutorily prohibited or abusive in nature.

    +

    The .gov domain space exists to support a broad diversity of government missions. Generally, we don’t review or audit how government organizations use their registered domains. However, misuse of a .gov domain can reflect upon the integrity of the entire .gov space. There are categories of misuse that are statutorily prohibited or abusive in nature.

    -

    What you can’t do with .gov domains

    +

    What you can’t do with a .gov domain

    Commercial purposes

    -

    .Gov domains must not be used for commercial purposes, such as advertising that benefits private individuals or entities.

    +

    .A .gov domain must not be used for commercial purposes, such as advertising that benefits private individuals or entities.

    Political campaigns

    -

    .Gov domains must not be used for political campaigns.

    +

    A .gov domain must not be used for political campaign purposes, such as the website for a candidate seeking elected office.

    Illegal content

    -

    .Gov domains must not be used to distribute or promote material whose distribution violates applicable law.

    +

    A .gov domain must not be used to distribute or promote material whose distribution violates applicable law.

    Malicious cyber activity

    -

    .Gov is a trusted and safe space. .Gov domains must not distribute malware, host - open redirects, or otherwise engage in malicious cyber activity.

    +

    A .gov domain must not distribute malware, host open redirects, or engage in malicious cyber activity.

    What .gov domain registrants must do

    Keep your contact information updated

    -

    .Gov domain registrants must maintain accurate contact information in the .gov registrar.

    +

    .Gov domain registrants must maintain accurate contact information in the .gov registrar. You will be asked to verify it as part of the renewal process.

    Be responsive if we contact you

    -

    Registrants should respond promptly to communications about potential violations to these requirements.

    +

    .Gov domain registrants must respond promptly to communications about potential violations to these requirements.

    -

    Failure to comply with these requirements could result in domain suspension or termination

    +

    Failure to comply could result in domain suspension or termination

    -

    We may need to suspend or terminate a domain registration for violations. When we discover a violation, we’ll make reasonable efforts to contact a registrant, including: +

    We may need to suspend or terminate a domain registration for violations. When we discover a violation, we’ll make reasonable efforts to contact a registrant, including emails or phone calls to:

      -
    • Emails to domain contacts
      • -
    • Phone calls to domain contacts
      • -
    • Email or phone call to the authorizing official
      • -
    • Emails or phone calls to the government organization, a parent organization, - or affiliated entities
      • +
    • Domain contacts
      • +
    • The authorizing official
      • +
    • The government organization, a parent organization, or affiliated entities

    -

    We understand the critical importance of the availability of .gov domains. Suspending or terminating a .gov domain is reserved for prolonged, unresolved, serious violations where the registrant is non-responsive. We'll make extensive efforts to contact registrants and to identify potential solutions. We'll make reasonable accommodations for remediation timelines based on the severity of the issue.

    +

    We understand the critical importance of availability for a .gov domain. Suspending or terminating a .gov domain is reserved for prolonged, unresolved, serious violations where the registrant is non-responsive. We'll make extensive efforts to contact registrants and to identify potential solutions. We'll make reasonable accommodations for remediation timelines based on the severity of the issue.

    + + + +

    .Gov domains are registered for a one-year period. To renew your domain, you'll be asked to verify your organization’s eligibility and your contact information.

    + +

    Though a domain may expire, it will not be automatically put on hold or deleted. We’ll make extensive efforts to contact your organization before holding or deleting a domain.

    + {% endblock %} {% block form_required_fields_help_text %} From fc1c7f030010454a838d41f40a006e40b8e7ea03 Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Tue, 16 Jan 2024 17:32:57 -0600 Subject: [PATCH 075/100] Modify text - .gov domain --- src/registrar/templates/application_dotgov_domain.html | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/registrar/templates/application_dotgov_domain.html b/src/registrar/templates/application_dotgov_domain.html index e21b679bb..98737839a 100644 --- a/src/registrar/templates/application_dotgov_domain.html +++ b/src/registrar/templates/application_dotgov_domain.html @@ -39,10 +39,7 @@

    What .gov domain do you want?

    -

    After you enter your domain, we’ll make sure it’s - available and that it meets some of our naming requirements. If your domain passes - these initial checks, we’ll verify that it meets all of our requirements once you - complete and submit the rest of this form.

    +

    After you enter your domain, we’ll make sure it’s available and that it meets some of our naming requirements. If your domain passes these initial checks, we’ll verify that it meets all our requirements after you complete the rest of this form.

    {% with attr_aria_describedby="domain_instructions domain_instructions2" %} {# attr_validate / validate="domain" invokes code in get-gov.js #} From e0aa7b6ff8f787046417f2b43d60f39881dca3da Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Tue, 16 Jan 2024 17:45:46 -0600 Subject: [PATCH 076/100] Fix h2 heading --- src/registrar/templates/application_requirements.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/application_requirements.html b/src/registrar/templates/application_requirements.html index d06f5de40..fa42031b2 100644 --- a/src/registrar/templates/application_requirements.html +++ b/src/registrar/templates/application_requirements.html @@ -46,7 +46,7 @@

    We understand the critical importance of availability for a .gov domain. Suspending or terminating a .gov domain is reserved for prolonged, unresolved, serious violations where the registrant is non-responsive. We'll make extensive efforts to contact registrants and to identify potential solutions. We'll make reasonable accommodations for remediation timelines based on the severity of the issue.

    - +

    Domain renewal

    .Gov domains are registered for a one-year period. To renew your domain, you'll be asked to verify your organization’s eligibility and your contact information.

    From 7851672dbc241e14e7dffc77ca8c92a32a44e6df Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 17 Jan 2024 13:30:36 -0500 Subject: [PATCH 077/100] small refactors --- src/registrar/models/contact.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index f2e8c7bfe..d8c94398b 100644 --- a/src/registrar/models/contact.py +++ b/src/registrar/models/contact.py @@ -54,19 +54,14 @@ class Contact(TimeStampedModel): db_index=True, ) + def _get_all_relations(self): + return [f.name for f in self._meta.get_fields() if f.is_relation] + def has_more_than_one_join(self, expected_relation): """Helper for finding whether an object is joined more than once. expected_relation is the one relation with one expected join""" # all_relations is the list of all_relations (from contact) to be checked for existing joins - all_relations = [ - "user", - "authorizing_official", - "submitted_applications", - "contact_applications", - "information_authorizing_official", - "submitted_applications_information", - "contact_applications_information", - ] + all_relations = self._get_all_relations() return any(self._has_more_than_one_join_per_relation(rel, expected_relation) for rel in all_relations) def _has_more_than_one_join_per_relation(self, relation, expected_relation): @@ -90,7 +85,8 @@ class Contact(TimeStampedModel): # if the rel field is a OneToOne field, then we have already # determined that the object exists (is not None) # so return True unless the relation being tested is the expected_relation - return True if relation != expected_relation else False + is_not_expected_relation = relation != expected_relation + return is_not_expected_relation elif isinstance(field, models.ForeignObjectRel): # if the rel field is a ManyToOne or ManyToMany, then we need # to determine if the count of related objects is greater than From 70dfccbb2e2c0302b2bd544da481a85a77379a1d Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 17 Jan 2024 13:39:33 -0500 Subject: [PATCH 078/100] small refactors and linting --- src/registrar/forms/domain.py | 7 ++----- src/registrar/models/contact.py | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 030fac86d..e29a2bb6f 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -250,11 +250,8 @@ class AuthorizingOfficialContactForm(ContactForm): # has more than one joined object. # In this case, create a new Contact, and update the new Contact with form data. # Then associate with domain information object as the authorizing_official - contact = Contact() - for name, value in self.cleaned_data.items(): - setattr(contact, name, value) - contact.save() - self.domainInfo.authorizing_official = contact + data = dict(self.cleaned_data.items()) + self.domainInfo.authorizing_official = Contact.objects.create(**data) self.domainInfo.save() else: super().save() diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index d8c94398b..00f27ae56 100644 --- a/src/registrar/models/contact.py +++ b/src/registrar/models/contact.py @@ -56,7 +56,7 @@ class Contact(TimeStampedModel): def _get_all_relations(self): return [f.name for f in self._meta.get_fields() if f.is_relation] - + def has_more_than_one_join(self, expected_relation): """Helper for finding whether an object is joined more than once. expected_relation is the one relation with one expected join""" From af656eb5a3eaeadb2b9328293a9af10879dd263f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 17 Jan 2024 11:58:49 -0700 Subject: [PATCH 079/100] Remove duplicate form errors --- src/registrar/assets/js/get-gov.js | 52 +++++++++++++++++++ .../templates/application_dotgov_domain.html | 3 +- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 92b6a1e46..e016165ae 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -236,8 +236,60 @@ function handleValidationClick(e) { for(const button of activatesValidation) { button.addEventListener('click', handleValidationClick); } + + + // Add event listener to the "Check availability" button + const checkAvailabilityButton = document.getElementById('check-availability-button'); + if (checkAvailabilityButton) { + const targetId = checkAvailabilityButton.getAttribute('validate-for'); + const checkAvailabilityInput = document.getElementById(targetId); + checkAvailabilityButton.addEventListener('click', function() { + removeFormErrors(checkAvailabilityInput); + }); + } + + // Add event listener to the alternate domains input + const alternateDomainsInputs = document.querySelectorAll('[auto-validate]'); + if (alternateDomainsInputs) { + for (const domainInput of alternateDomainsInputs){ + // Only apply this logic to alternate domains input + if (domainInput.classList.contains('alternate-domain-input')){ + domainInput.addEventListener('input', function() { + removeFormErrors(domainInput); + }); + } + } + } })(); +/** + * Removes form errors surrounding a form input + */ +function removeFormErrors(input){ + // Remove error message + const errorMessage = document.getElementById(`${input.id}__error-message`); + if (errorMessage) { + errorMessage.remove(); + } + + // Remove error classes + if (input.classList.contains('usa-input--error')) { + input.classList.remove('usa-input--error'); + } + + const label = document.querySelector(`label[for="${input.id}"]`); + if (label) { + label.classList.remove('usa-label--error'); + + // Remove error classes from parent div + const parentDiv = label.parentElement; + if (parentDiv) { + parentDiv.classList.remove('usa-form-group--error'); + } + } + + +} /** * Prepare the namerservers and DS data forms delete buttons diff --git a/src/registrar/templates/application_dotgov_domain.html b/src/registrar/templates/application_dotgov_domain.html index ab5504264..25724c010 100644 --- a/src/registrar/templates/application_dotgov_domain.html +++ b/src/registrar/templates/application_dotgov_domain.html @@ -53,6 +53,7 @@ {% endwith %} {% endwith %}