From a1fe3aaccab532170bbac7c2683fc3a63d694fe0 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 17 Dec 2023 18:14:08 -0700 Subject: [PATCH 01/17] Finally fixed! --- src/registrar/forms/application_wizard.py | 83 ++++++++++++++++++++++- 1 file changed, 80 insertions(+), 3 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 5310c4610..faf005d71 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -15,6 +15,7 @@ from registrar.templatetags.url_helpers import public_site_url from registrar.utility import errors logger = logging.getLogger(__name__) +from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper class RegistrarForm(forms.Form): @@ -262,7 +263,7 @@ class OrganizationContactForm(RegistrarForm): validators=[ RegexValidator( "^[0-9]{5}(?:-[0-9]{4})?$|^$", - message="Enter a zip code in the required format, like 12345 or 12345-6789.", + message="Enter a zip code in the form of 12345 or 12345-6789.", ) ], ) @@ -557,6 +558,7 @@ class YourContactForm(RegistrarForm): class OtherContactsForm(RegistrarForm): first_name = forms.CharField( + # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="First name / given name", error_messages={"required": "Enter the first name / given name of this contact."}, ) @@ -565,10 +567,12 @@ class OtherContactsForm(RegistrarForm): label="Middle name (optional)", ) last_name = forms.CharField( + # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="Last name / family name", error_messages={"required": "Enter the last name / family name of this contact."}, ) title = forms.CharField( + # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="Title or role in your organization", error_messages={ "required": ( @@ -577,27 +581,100 @@ class OtherContactsForm(RegistrarForm): }, ) email = forms.EmailField( + # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="Email", error_messages={"invalid": ("Enter an email address in the required format, like name@example.com.")}, ) phone = PhoneNumberField( + # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="Phone", error_messages={"required": "Enter a phone number for this contact."}, ) + + + + # Override clean in order to correct validation logic + def clean(self): + cleaned = self.cleaned_data # super().clean() + TerminalHelper.print_debug(f"""{TerminalColors.MAGENTA} CLEANING... + FIELDS: + {self.fields} + CLEANED: + {cleaned}{TerminalColors.ENDC}""") + + form_is_empty = all(v is None or v == '' for v in cleaned.values()) + # if not form_is_empty: #TODO: add check for "marked for deleteion" when implementing button + #Validation Logic + # if not self.cleaned_data["first_name"]: + # self.add_error('first_name', "Enter the first name / given name of this contact.") + # if not self.cleaned_data["last_name"]: + # self.add_error('last_name', "Enter the last name / family name of this contact.") + # if not self.cleaned_data["title"]: + # self.add_error('title', "Enter the title or role in your organization of this contact (e.g., Chief Information Officer).") + if form_is_empty: + TerminalHelper.print_debug(f"""{TerminalColors.MAGENTA} ***** BLANK FORM DETECTED ******* + {TerminalColors.ENDC}""") + + # clear any errors raised by the form fields + # (before this clean() method is run, each field + # performs its own clean, which could result in + # errors that we wish to ignore at this point) + # + # NOTE: we cannot just clear() the errors list. + # That causes problems. + for field in self.fields: + if field in self.errors: + del self.errors[field] + return cleaned + + + # # Always return a value to use as the new cleaned data, even if + # # this method didn't change it. + # return data + + # for field in self.fields.values(): + # isBlank = field is None or field == '' + # TerminalHelper.print_debug(f"""{TerminalColors.YELLOW} {field} is blank = {isBlank} {TerminalColors.ENDC}""") + + + # TerminalHelper.print_debug(f"""{TerminalColors.YELLOW} {field.required} {TerminalColors.ENDC}""") + # return None + # return super().clean() + + # def _should_delete_form(self, form): + # TerminalHelper.print_debug(f"{TerminalColors.MAGENTA} SHOULD DELETE FORM?...{TerminalColors.ENDC}") + # """Return whether or not the form was marked for deletion.""" + # return all(field is None or field == '' for field in self.fields.values()) class BaseOtherContactsFormSet(RegistrarFormSet): JOIN = "other_contacts" + # def get_deletion_widget(self): + # delete_button = '' + # return delete_button + + # # if form.cleaned_data.get(forms.formsets.DELETION_FIELD_NAME): + # # return True # marked for delete + # # fields = ('name', 'question', 'amount', 'measure', 'comment') + # print(form.cleaned_data) + # if not any(form.cleaned_data): + # return True + # return False + def should_delete(self, cleaned): - empty = (isinstance(v, str) and not v.strip() for v in cleaned.values()) + # TerminalHelper.print_debug(f"{TerminalColors.MAGENTA} SHOULD DELETE OTHER CONTACTS?... {cleaned.values()}{TerminalColors.ENDC}") + empty = (isinstance(v, str) and (v.strip() == "" or v == None) for v in cleaned.values()) return all(empty) + def to_database(self, obj: DomainApplication): + TerminalHelper.print_debug(f"{TerminalColors.OKCYAN} TO DATABASE {TerminalColors.ENDC}") self._to_database(obj, self.JOIN, self.should_delete, self.pre_update, self.pre_create) @classmethod def from_database(cls, obj): + TerminalHelper.print_debug(f"{TerminalColors.OKCYAN} FROM DATABASE {TerminalColors.ENDC}") return super().from_database(obj, cls.JOIN, cls.on_fetch) @@ -606,6 +683,7 @@ OtherContactsFormSet = forms.formset_factory( extra=1, absolute_max=1500, # django default; use `max_num` to limit entries formset=BaseOtherContactsFormSet, + # can_delete=True #TODO: use when implementing delete button? ) @@ -634,7 +712,6 @@ class AnythingElseForm(RegistrarForm): ], ) - class RequirementsForm(RegistrarForm): is_policy_acknowledged = forms.BooleanField( label="I read and agree to the requirements for operating .gov domains.", From 6d4bd3d592eaeeed72c7f0424237c6d2da180d96 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 17 Dec 2023 19:17:50 -0700 Subject: [PATCH 02/17] Cleaned up dead experiments and fixed CORS error --- src/registrar/forms/application_wizard.py | 63 +---------------------- 1 file changed, 1 insertion(+), 62 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index faf005d71..5aaa738e7 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -15,7 +15,6 @@ from registrar.templatetags.url_helpers import public_site_url from registrar.utility import errors logger = logging.getLogger(__name__) -from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper class RegistrarForm(forms.Form): @@ -558,7 +557,6 @@ class YourContactForm(RegistrarForm): class OtherContactsForm(RegistrarForm): first_name = forms.CharField( - # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="First name / given name", error_messages={"required": "Enter the first name / given name of this contact."}, ) @@ -567,12 +565,10 @@ class OtherContactsForm(RegistrarForm): label="Middle name (optional)", ) last_name = forms.CharField( - # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="Last name / family name", error_messages={"required": "Enter the last name / family name of this contact."}, ) title = forms.CharField( - # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="Title or role in your organization", error_messages={ "required": ( @@ -581,40 +577,19 @@ class OtherContactsForm(RegistrarForm): }, ) email = forms.EmailField( - # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="Email", error_messages={"invalid": ("Enter an email address in the required format, like name@example.com.")}, ) phone = PhoneNumberField( - # required=False, #is required, but validate in clean() instead to allow for custom form deletion condition label="Phone", error_messages={"required": "Enter a phone number for this contact."}, ) - - # Override clean in order to correct validation logic def clean(self): - cleaned = self.cleaned_data # super().clean() - TerminalHelper.print_debug(f"""{TerminalColors.MAGENTA} CLEANING... - FIELDS: - {self.fields} - CLEANED: - {cleaned}{TerminalColors.ENDC}""") - + cleaned = super().clean() #self.cleaned_data <---Why does this cause a CORS error? form_is_empty = all(v is None or v == '' for v in cleaned.values()) - # if not form_is_empty: #TODO: add check for "marked for deleteion" when implementing button - #Validation Logic - # if not self.cleaned_data["first_name"]: - # self.add_error('first_name', "Enter the first name / given name of this contact.") - # if not self.cleaned_data["last_name"]: - # self.add_error('last_name', "Enter the last name / family name of this contact.") - # if not self.cleaned_data["title"]: - # self.add_error('title', "Enter the title or role in your organization of this contact (e.g., Chief Information Officer).") if form_is_empty: - TerminalHelper.print_debug(f"""{TerminalColors.MAGENTA} ***** BLANK FORM DETECTED ******* - {TerminalColors.ENDC}""") - # clear any errors raised by the form fields # (before this clean() method is run, each field # performs its own clean, which could result in @@ -627,54 +602,19 @@ class OtherContactsForm(RegistrarForm): del self.errors[field] return cleaned - - # # Always return a value to use as the new cleaned data, even if - # # this method didn't change it. - # return data - - # for field in self.fields.values(): - # isBlank = field is None or field == '' - # TerminalHelper.print_debug(f"""{TerminalColors.YELLOW} {field} is blank = {isBlank} {TerminalColors.ENDC}""") - - - # TerminalHelper.print_debug(f"""{TerminalColors.YELLOW} {field.required} {TerminalColors.ENDC}""") - # return None - # return super().clean() - - # def _should_delete_form(self, form): - # TerminalHelper.print_debug(f"{TerminalColors.MAGENTA} SHOULD DELETE FORM?...{TerminalColors.ENDC}") - # """Return whether or not the form was marked for deletion.""" - # return all(field is None or field == '' for field in self.fields.values()) - class BaseOtherContactsFormSet(RegistrarFormSet): JOIN = "other_contacts" - # def get_deletion_widget(self): - # delete_button = '' - # return delete_button - - # # if form.cleaned_data.get(forms.formsets.DELETION_FIELD_NAME): - # # return True # marked for delete - # # fields = ('name', 'question', 'amount', 'measure', 'comment') - # print(form.cleaned_data) - # if not any(form.cleaned_data): - # return True - # return False - def should_delete(self, cleaned): - # TerminalHelper.print_debug(f"{TerminalColors.MAGENTA} SHOULD DELETE OTHER CONTACTS?... {cleaned.values()}{TerminalColors.ENDC}") empty = (isinstance(v, str) and (v.strip() == "" or v == None) for v in cleaned.values()) return all(empty) - def to_database(self, obj: DomainApplication): - TerminalHelper.print_debug(f"{TerminalColors.OKCYAN} TO DATABASE {TerminalColors.ENDC}") self._to_database(obj, self.JOIN, self.should_delete, self.pre_update, self.pre_create) @classmethod def from_database(cls, obj): - TerminalHelper.print_debug(f"{TerminalColors.OKCYAN} FROM DATABASE {TerminalColors.ENDC}") return super().from_database(obj, cls.JOIN, cls.on_fetch) @@ -683,7 +623,6 @@ OtherContactsFormSet = forms.formset_factory( extra=1, absolute_max=1500, # django default; use `max_num` to limit entries formset=BaseOtherContactsFormSet, - # can_delete=True #TODO: use when implementing delete button? ) From 1035130978be56ced2c7aa5c5f521dc64149d91a Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 17 Dec 2023 19:31:01 -0700 Subject: [PATCH 03/17] Linted and minor comment update --- src/registrar/forms/application_wizard.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 5aaa738e7..b45ee5f68 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -586,9 +586,9 @@ class OtherContactsForm(RegistrarForm): ) # Override clean in order to correct validation logic - def clean(self): - cleaned = super().clean() #self.cleaned_data <---Why does this cause a CORS error? - form_is_empty = all(v is None or v == '' for v in cleaned.values()) + def clean(self): + cleaned = super().clean() + form_is_empty = all(v is None or v == "" for v in cleaned.values()) if form_is_empty: # clear any errors raised by the form fields # (before this clean() method is run, each field @@ -651,6 +651,7 @@ class AnythingElseForm(RegistrarForm): ], ) + class RequirementsForm(RegistrarForm): is_policy_acknowledged = forms.BooleanField( label="I read and agree to the requirements for operating .gov domains.", From 93b978abd3849d5e9d8339bba4642da3d625733a Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 17 Dec 2023 19:31:10 -0700 Subject: [PATCH 04/17] Updated comment --- src/registrar/forms/application_wizard.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index b45ee5f68..c0cd6e5b4 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -587,6 +587,7 @@ class OtherContactsForm(RegistrarForm): # Override clean in order to correct validation logic def clean(self): + # NOTE: using self.cleaned_data directly apparently causes a CORS error cleaned = super().clean() form_is_empty = all(v is None or v == "" for v in cleaned.values()) if form_is_empty: From 515fa2c105b992f875b0fe8cf3670f3739309476 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 20 Dec 2023 15:01:54 -0700 Subject: [PATCH 05/17] unit test progress --- src/registrar/tests/test_views.py | 118 ++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 57fa03f52..5f071692d 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -729,6 +729,124 @@ class DomainApplicationTests(TestWithUser, WebTest): actual_url_slug = no_contacts_page.request.path.split("/")[-2] self.assertEqual(expected_url_slug, actual_url_slug) + def test_application_delete_other_contact(self): + """Other contacts can be deleted after being saved to database.""" + # Populate the databse with a domain application that + # has 1 "other contact" assigned to it + ao, _ = Contact.objects.get_or_create( + first_name="Testy", + last_name="Tester", + title="Chief Tester", + email="testy@town.com", + phone="(555) 555 5555", + ) + domain, _ = DraftDomain.objects.get_or_create(name="fakeSite.gov") + you, _ = Contact.objects.get_or_create( + first_name="Testy you", + last_name="Tester you", + title="Admin Tester", + email="testy-admin@town.com", + phone="(555) 555 5556", + ) + other, _ = Contact.objects.get_or_create( + first_name="Testy2", + last_name="Tester2", + title="Another Tester", + email="testy2@town.com", + phone="(555) 555 5557", + ) + application, _ = DomainApplication.objects.get_or_create( + organization_type="federal", + federal_type="executive", + purpose="Purpose of the site", + anything_else="No", + is_policy_acknowledged=True, + organization_name="Testorg", + address_line1="address 1", + state_territory="NY", + zipcode="10002", + authorizing_official=ao, + requested_domain=domain, + submitter=you, + creator=self.user, + ) + application.other_contacts.add(other) + + # prime the form by visiting /edit + url = reverse("edit-application", kwargs={"id": application.pk}) + response = self.client.get(url) + + url = reverse("application:other_contacts") + other_contacts_page = self.client.get(url, follow=True) + + # ====== METHOD 2 -- prime form + # other_contacts_page = self.app.get(reverse("application:other_contacts")) + # session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + # # Fill in the other contact form + # other_contacts_form = other_contacts_page.forms[0] + # other_contacts_form["other_contacts-0-first_name"] = "Testy2" + # other_contacts_form["other_contacts-0-last_name"] = "Tester2" + # other_contacts_form["other_contacts-0-title"] = "Another Tester" + # other_contacts_form["other_contacts-0-email"] = "testy2@town.com" + # other_contacts_form["other_contacts-0-phone"] = "(201) 555 5557" + + # # for f in other_contacts_form.fields: + # # if not "submit" in f: + # # print(f) + # # print(other_contacts_form[f].value) + + # # Submit the form + # other_contacts_result = other_contacts_form.submit() + # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + # # validate that data from this step are being saved + # application = DomainApplication.objects.get() # there's only one + # self.assertEqual( + # application.other_contacts.count(), + # 1, + # ) + # # Verify user is taken to "anything else" page + # self.assertEqual(other_contacts_result.status_code, 302) + # self.assertEqual(other_contacts_result["Location"], "/register/anything_else/") + + # # Go back to the previous step + # other_contacts_page = self.app.get(reverse("application:other_contacts")) + + # clear the form + other_contacts_form = other_contacts_page.forms[0] + other_contacts_form["other_contacts-0-first_name"] = "" + other_contacts_form["other_contacts-0-middle_name"] = "" + other_contacts_form["other_contacts-0-last_name"] = "" + other_contacts_form["other_contacts-0-title"] = "" + other_contacts_form["other_contacts-0-email"] = "" + other_contacts_form["other_contacts-0-phone"] = "" + + for f in other_contacts_form.fields: + if not "submit" in f: + print(f) + print(other_contacts_form[f].value) + + # Submit the now empty form + result = other_contacts_form.submit() + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + # Verify that the contact we saved earlier has been removed from the database + application = DomainApplication.objects.get() # There are no contacts anymore + self.assertEqual( + application.other_contacts.count(), + 0, + ) + + # Verify that on submit, user is advanced to "no contacts" page + no_contacts_page = result.follow() + expected_url_slug = str(Step.NO_OTHER_CONTACTS) + actual_url_slug = no_contacts_page.request.path.split("/")[-2] + self.assertEqual(expected_url_slug, actual_url_slug) + + + def test_application_about_your_organiztion_interstate(self): """Special districts have to answer an additional question.""" type_page = self.app.get(reverse("application:")).follow() From e1a214b74ba355c90071e87b401184c018cba455 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 21 Dec 2023 15:22:16 -0700 Subject: [PATCH 06/17] Unit test experiments --- src/registrar/tests/test_views.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 5f071692d..1cdd27c4b 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -33,7 +33,7 @@ from registrar.models import ( UserDomainRole, User, ) -from registrar.views.application import ApplicationWizard, Step +from registrar.views.application import ApplicationStatus, ApplicationWizard, Step from .common import less_console_noise @@ -741,6 +741,7 @@ class DomainApplicationTests(TestWithUser, WebTest): phone="(555) 555 5555", ) domain, _ = DraftDomain.objects.get_or_create(name="fakeSite.gov") + current, _ = Website.objects.get_or_create(website="city.com") you, _ = Contact.objects.get_or_create( first_name="Testy you", last_name="Tester you", @@ -769,14 +770,23 @@ class DomainApplicationTests(TestWithUser, WebTest): requested_domain=domain, submitter=you, creator=self.user, + status="started" ) application.other_contacts.add(other) + application.current_websites.add(current) + + # django-webtest does not handle cookie-based sessions well because it keeps + # resetting the session key on each new request, thus destroying the concept + # of a "session". We are going to do it manually, saving the session ID here + # and then setting the cookie on each request. + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] # prime the form by visiting /edit url = reverse("edit-application", kwargs={"id": application.pk}) - response = self.client.get(url) + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) url = reverse("application:other_contacts") + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) other_contacts_page = self.client.get(url, follow=True) # ====== METHOD 2 -- prime form From 8f3f327eef0845db8c4a9b6dddf846accaa9eef7 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 21 Dec 2023 16:28:32 -0700 Subject: [PATCH 07/17] Unit test is fixed --- src/registrar/tests/test_views.py | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 1cdd27c4b..1347f51fe 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -740,8 +740,6 @@ class DomainApplicationTests(TestWithUser, WebTest): email="testy@town.com", phone="(555) 555 5555", ) - domain, _ = DraftDomain.objects.get_or_create(name="fakeSite.gov") - current, _ = Website.objects.get_or_create(website="city.com") you, _ = Contact.objects.get_or_create( first_name="Testy you", last_name="Tester you", @@ -767,27 +765,25 @@ class DomainApplicationTests(TestWithUser, WebTest): state_territory="NY", zipcode="10002", authorizing_official=ao, - requested_domain=domain, submitter=you, creator=self.user, status="started" ) application.other_contacts.add(other) - application.current_websites.add(current) + + # prime the form by visiting /edit + edit_app_page = self.app.get(reverse("edit-application", kwargs={"id": application.pk})) # django-webtest does not handle cookie-based sessions well because it keeps # resetting the session key on each new request, thus destroying the concept # of a "session". We are going to do it manually, saving the session ID here # and then setting the cookie on each request. session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] - - # prime the form by visiting /edit - url = reverse("edit-application", kwargs={"id": application.pk}) self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - url = reverse("application:other_contacts") + other_contacts_page = self.app.get(reverse("application:other_contacts")) self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - other_contacts_page = self.client.get(url, follow=True) + # ====== METHOD 2 -- prime form # other_contacts_page = self.app.get(reverse("application:other_contacts")) @@ -824,8 +820,22 @@ class DomainApplicationTests(TestWithUser, WebTest): # # Go back to the previous step # other_contacts_page = self.app.get(reverse("application:other_contacts")) - # clear the form + ##### ^ The commented out method doesn't work because it creates a duplicate application entry #### + other_contacts_form = other_contacts_page.forms[0] + + # DEBUG print statements + for f in other_contacts_form.fields: + if not "submit" in f: + print(f) + print(other_contacts_form[f].value) + + # Minimal check to ensure the form is loaded with data (if this part of + # the application doesn't work, we should be equipped with other unit + # tests to flag it) + self.assertEqual(other_contacts_form["other_contacts-0-first_name"].value, "Testy2") + + # clear the form other_contacts_form["other_contacts-0-first_name"] = "" other_contacts_form["other_contacts-0-middle_name"] = "" other_contacts_form["other_contacts-0-last_name"] = "" @@ -833,6 +843,7 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form["other_contacts-0-email"] = "" other_contacts_form["other_contacts-0-phone"] = "" + # DEBUG print statements for f in other_contacts_form.fields: if not "submit" in f: print(f) From a4293b4aaccb56c60f38275818788b1632263a56 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 21 Dec 2023 20:24:48 -0700 Subject: [PATCH 08/17] Linting & cleanup --- src/registrar/forms/application_wizard.py | 2 +- src/registrar/tests/test_views.py | 55 +++-------------------- 2 files changed, 8 insertions(+), 49 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index c0cd6e5b4..ac84a2f9f 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -608,7 +608,7 @@ class BaseOtherContactsFormSet(RegistrarFormSet): JOIN = "other_contacts" def should_delete(self, cleaned): - empty = (isinstance(v, str) and (v.strip() == "" or v == None) for v in cleaned.values()) + empty = (isinstance(v, str) and (v.strip() == "" or v is None) for v in cleaned.values()) return all(empty) def to_database(self, obj: DomainApplication): diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 1347f51fe..7301cc681 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -33,7 +33,7 @@ from registrar.models import ( UserDomainRole, User, ) -from registrar.views.application import ApplicationStatus, ApplicationWizard, Step +from registrar.views.application import ApplicationWizard, Step from .common import less_console_noise @@ -767,13 +767,12 @@ class DomainApplicationTests(TestWithUser, WebTest): authorizing_official=ao, submitter=you, creator=self.user, - status="started" + status="started", ) application.other_contacts.add(other) - # prime the form by visiting /edit - edit_app_page = self.app.get(reverse("edit-application", kwargs={"id": application.pk})) + self.app.get(reverse("edit-application", kwargs={"id": application.pk})) # django-webtest does not handle cookie-based sessions well because it keeps # resetting the session key on each new request, thus destroying the concept # of a "session". We are going to do it manually, saving the session ID here @@ -784,53 +783,15 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_page = self.app.get(reverse("application:other_contacts")) self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - - # ====== METHOD 2 -- prime form - # other_contacts_page = self.app.get(reverse("application:other_contacts")) - # session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] - # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - - # # Fill in the other contact form - # other_contacts_form = other_contacts_page.forms[0] - # other_contacts_form["other_contacts-0-first_name"] = "Testy2" - # other_contacts_form["other_contacts-0-last_name"] = "Tester2" - # other_contacts_form["other_contacts-0-title"] = "Another Tester" - # other_contacts_form["other_contacts-0-email"] = "testy2@town.com" - # other_contacts_form["other_contacts-0-phone"] = "(201) 555 5557" - - # # for f in other_contacts_form.fields: - # # if not "submit" in f: - # # print(f) - # # print(other_contacts_form[f].value) - - # # Submit the form - # other_contacts_result = other_contacts_form.submit() - # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - - # # validate that data from this step are being saved - # application = DomainApplication.objects.get() # there's only one - # self.assertEqual( - # application.other_contacts.count(), - # 1, - # ) - # # Verify user is taken to "anything else" page - # self.assertEqual(other_contacts_result.status_code, 302) - # self.assertEqual(other_contacts_result["Location"], "/register/anything_else/") - - # # Go back to the previous step - # other_contacts_page = self.app.get(reverse("application:other_contacts")) - - ##### ^ The commented out method doesn't work because it creates a duplicate application entry #### - other_contacts_form = other_contacts_page.forms[0] # DEBUG print statements for f in other_contacts_form.fields: - if not "submit" in f: + if "submit" not in f: print(f) print(other_contacts_form[f].value) - - # Minimal check to ensure the form is loaded with data (if this part of + + # Minimal check to ensure the form is loaded with data (if this part of # the application doesn't work, we should be equipped with other unit # tests to flag it) self.assertEqual(other_contacts_form["other_contacts-0-first_name"].value, "Testy2") @@ -845,7 +806,7 @@ class DomainApplicationTests(TestWithUser, WebTest): # DEBUG print statements for f in other_contacts_form.fields: - if not "submit" in f: + if "submit" not in f: print(f) print(other_contacts_form[f].value) @@ -866,8 +827,6 @@ class DomainApplicationTests(TestWithUser, WebTest): actual_url_slug = no_contacts_page.request.path.split("/")[-2] self.assertEqual(expected_url_slug, actual_url_slug) - - def test_application_about_your_organiztion_interstate(self): """Special districts have to answer an additional question.""" type_page = self.app.get(reverse("application:")).follow() From 7087434d7601bc5be270fa37534bcb55cda20c9c Mon Sep 17 00:00:00 2001 From: CuriousX Date: Thu, 28 Dec 2023 22:32:25 -0700 Subject: [PATCH 09/17] Update src/registrar/tests/test_views.py Co-authored-by: zandercymatics <141044360+zandercymatics@users.noreply.github.com> --- src/registrar/tests/test_views.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 7301cc681..17a40d517 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -785,11 +785,6 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form = other_contacts_page.forms[0] - # DEBUG print statements - for f in other_contacts_form.fields: - if "submit" not in f: - print(f) - print(other_contacts_form[f].value) # Minimal check to ensure the form is loaded with data (if this part of # the application doesn't work, we should be equipped with other unit From ac3afe436cef15e4de7f3f2420f44ef262fba251 Mon Sep 17 00:00:00 2001 From: CuriousX Date: Thu, 28 Dec 2023 22:32:35 -0700 Subject: [PATCH 10/17] Update src/registrar/tests/test_views.py Co-authored-by: zandercymatics <141044360+zandercymatics@users.noreply.github.com> --- src/registrar/tests/test_views.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 17a40d517..7ae925b27 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -799,11 +799,6 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form["other_contacts-0-email"] = "" other_contacts_form["other_contacts-0-phone"] = "" - # DEBUG print statements - for f in other_contacts_form.fields: - if "submit" not in f: - print(f) - print(other_contacts_form[f].value) # Submit the now empty form result = other_contacts_form.submit() From affc35398d9d54c95729216476fd4d657c9677b5 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Fri, 29 Dec 2023 02:11:19 -0700 Subject: [PATCH 11/17] Fixed (with a bandaid) --- src/registrar/forms/application_wizard.py | 54 +++++++++++++++++++++-- src/registrar/tests/test_forms.py | 4 +- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index ac84a2f9f..b2abc155d 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -9,6 +9,7 @@ from django.core.validators import RegexValidator, MaxLengthValidator from django.utils.safestring import mark_safe from api.views import DOMAIN_API_MESSAGES +from registrar.management.commands.utility.terminal_helper import TerminalColors from registrar.models import Contact, DomainApplication, DraftDomain, Domain from registrar.templatetags.url_helpers import public_site_url @@ -262,7 +263,7 @@ class OrganizationContactForm(RegistrarForm): validators=[ RegexValidator( "^[0-9]{5}(?:-[0-9]{4})?$|^$", - message="Enter a zip code in the form of 12345 or 12345-6789.", + message="Enter a zip code in the required format, like 12345 or 12345-6789.", ) ], ) @@ -585,11 +586,52 @@ class OtherContactsForm(RegistrarForm): error_messages={"required": "Enter a phone number for this contact."}, ) + # Override clean in order to correct validation logic def clean(self): # NOTE: using self.cleaned_data directly apparently causes a CORS error cleaned = super().clean() - form_is_empty = all(v is None or v == "" for v in cleaned.values()) + + logger.info(f""" + {TerminalColors.MAGENTA}form data: + {TerminalColors.OKBLUE}{self.data} + + {TerminalColors.MAGENTA}form cleaned: + {TerminalColors.OKBLUE}{cleaned} + + + {self.data.items} + {TerminalColors.ENDC} + + """) + + # for f in self.fields: + # logger.info(f""" + # {TerminalColors.YELLOW}{f} + # {self.data.get(f)} + # {TerminalColors.ENDC} + # """) + + form_is_empty = all(v is None or v == "" for v in cleaned.values()) + + # NOTE: Phone number and email do NOT show up in cleaned values. + # I have spent hours tyring to figure out why, but have no idea... + # so for now we will grab their values from the raw data... + for i in self.data: + if 'phone' in i or 'email' in i: + field_value = self.data.get(i) + logger.info(f""" + {TerminalColors.YELLOW}{i} + {self.data.get(i)} + {TerminalColors.ENDC} + """) + form_is_empty = field_value == "" or field_value is None + logger.info(f""" + {TerminalColors.OKCYAN}empty? {form_is_empty} + {TerminalColors.ENDC} + """) + + if form_is_empty: # clear any errors raised by the form fields # (before this clean() method is run, each field @@ -599,8 +641,14 @@ class OtherContactsForm(RegistrarForm): # NOTE: we cannot just clear() the errors list. # That causes problems. for field in self.fields: - if field in self.errors: + if field in self.errors: # and field in cleaned + logger.info(f""" + {TerminalColors.FAIL}removing {field} + {TerminalColors.ENDC} + """) del self.errors[field] + + return cleaned diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index 00bb7ce61..9b6c3d6dd 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -216,7 +216,7 @@ class TestFormValidation(MockEppLib): def test_other_contact_email_invalid(self): """must be a valid email address.""" - form = OtherContactsForm(data={"email": "boss@boss"}) + form = OtherContactsForm(data={"email": "splendid@boss"}) self.assertEqual( form.errors["email"], ["Enter an email address in the required format, like name@example.com."], @@ -224,7 +224,7 @@ class TestFormValidation(MockEppLib): def test_other_contact_phone_invalid(self): """Must be a valid phone number.""" - form = OtherContactsForm(data={"phone": "boss@boss"}) + form = OtherContactsForm(data={"phone": "super@boss"}) self.assertTrue(form.errors["phone"][0].startswith("Enter a valid phone number ")) def test_requirements_form_blank(self): From 0fea0c2f94b4814727bb8df800357665fc080c86 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Fri, 29 Dec 2023 02:13:40 -0700 Subject: [PATCH 12/17] Remove print statements --- src/registrar/forms/application_wizard.py | 38 ++--------------------- 1 file changed, 3 insertions(+), 35 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index b2abc155d..b55fa62d6 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -591,27 +591,6 @@ class OtherContactsForm(RegistrarForm): def clean(self): # NOTE: using self.cleaned_data directly apparently causes a CORS error cleaned = super().clean() - - logger.info(f""" - {TerminalColors.MAGENTA}form data: - {TerminalColors.OKBLUE}{self.data} - - {TerminalColors.MAGENTA}form cleaned: - {TerminalColors.OKBLUE}{cleaned} - - - {self.data.items} - {TerminalColors.ENDC} - - """) - - # for f in self.fields: - # logger.info(f""" - # {TerminalColors.YELLOW}{f} - # {self.data.get(f)} - # {TerminalColors.ENDC} - # """) - form_is_empty = all(v is None or v == "" for v in cleaned.values()) # NOTE: Phone number and email do NOT show up in cleaned values. @@ -619,17 +598,10 @@ class OtherContactsForm(RegistrarForm): # so for now we will grab their values from the raw data... for i in self.data: if 'phone' in i or 'email' in i: + # check if it has data field_value = self.data.get(i) - logger.info(f""" - {TerminalColors.YELLOW}{i} - {self.data.get(i)} - {TerminalColors.ENDC} - """) + # update the bool on whether the form is actually empty form_is_empty = field_value == "" or field_value is None - logger.info(f""" - {TerminalColors.OKCYAN}empty? {form_is_empty} - {TerminalColors.ENDC} - """) if form_is_empty: @@ -641,11 +613,7 @@ class OtherContactsForm(RegistrarForm): # NOTE: we cannot just clear() the errors list. # That causes problems. for field in self.fields: - if field in self.errors: # and field in cleaned - logger.info(f""" - {TerminalColors.FAIL}removing {field} - {TerminalColors.ENDC} - """) + if field in self.errors: del self.errors[field] From ea043bf5045484852f1635a3852505054115eeec Mon Sep 17 00:00:00 2001 From: CocoByte Date: Fri, 29 Dec 2023 02:16:39 -0700 Subject: [PATCH 13/17] linted --- src/registrar/forms/application_wizard.py | 10 +++------- src/registrar/tests/test_views.py | 2 -- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index b55fa62d6..7a79d0677 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -9,7 +9,6 @@ from django.core.validators import RegexValidator, MaxLengthValidator from django.utils.safestring import mark_safe from api.views import DOMAIN_API_MESSAGES -from registrar.management.commands.utility.terminal_helper import TerminalColors from registrar.models import Contact, DomainApplication, DraftDomain, Domain from registrar.templatetags.url_helpers import public_site_url @@ -586,24 +585,22 @@ class OtherContactsForm(RegistrarForm): error_messages={"required": "Enter a phone number for this contact."}, ) - # Override clean in order to correct validation logic def clean(self): # NOTE: using self.cleaned_data directly apparently causes a CORS error cleaned = super().clean() - form_is_empty = all(v is None or v == "" for v in cleaned.values()) - + form_is_empty = all(v is None or v == "" for v in cleaned.values()) + # NOTE: Phone number and email do NOT show up in cleaned values. # I have spent hours tyring to figure out why, but have no idea... # so for now we will grab their values from the raw data... for i in self.data: - if 'phone' in i or 'email' in i: + if "phone" in i or "email" in i: # check if it has data field_value = self.data.get(i) # update the bool on whether the form is actually empty form_is_empty = field_value == "" or field_value is None - if form_is_empty: # clear any errors raised by the form fields # (before this clean() method is run, each field @@ -616,7 +613,6 @@ class OtherContactsForm(RegistrarForm): if field in self.errors: del self.errors[field] - return cleaned diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 7ae925b27..c465373dd 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -785,7 +785,6 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form = other_contacts_page.forms[0] - # Minimal check to ensure the form is loaded with data (if this part of # the application doesn't work, we should be equipped with other unit # tests to flag it) @@ -799,7 +798,6 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form["other_contacts-0-email"] = "" other_contacts_form["other_contacts-0-phone"] = "" - # Submit the now empty form result = other_contacts_form.submit() self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) From 0ac1989a04a587cb1b608551542e219b71a6223e Mon Sep 17 00:00:00 2001 From: CocoByte Date: Fri, 29 Dec 2023 11:41:36 -0700 Subject: [PATCH 14/17] Clarification in comments for Unit Test bandaid --- src/registrar/forms/application_wizard.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 7a79d0677..4da0c9700 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -591,7 +591,9 @@ class OtherContactsForm(RegistrarForm): cleaned = super().clean() form_is_empty = all(v is None or v == "" for v in cleaned.values()) - # NOTE: Phone number and email do NOT show up in cleaned values. + # ==== UNIT TEST BANDAID ==== + # NOTE: Phone number and email do NOT show up in cleaned values + # This is for UNIT TESTS only. The page itself loads cleaned data just fine. # I have spent hours tyring to figure out why, but have no idea... # so for now we will grab their values from the raw data... for i in self.data: From 06a2e39c123348523329fe012437ee0e5333260b Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 29 Dec 2023 15:37:31 -0500 Subject: [PATCH 15/17] updated clean in form --- src/registrar/forms/application_wizard.py | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 7a79d0677..06cbe81ca 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -587,19 +587,12 @@ class OtherContactsForm(RegistrarForm): # Override clean in order to correct validation logic def clean(self): - # NOTE: using self.cleaned_data directly apparently causes a CORS error - cleaned = super().clean() - form_is_empty = all(v is None or v == "" for v in cleaned.values()) - # NOTE: Phone number and email do NOT show up in cleaned values. - # I have spent hours tyring to figure out why, but have no idea... - # so for now we will grab their values from the raw data... - for i in self.data: - if "phone" in i or "email" in i: - # check if it has data - field_value = self.data.get(i) - # update the bool on whether the form is actually empty - form_is_empty = field_value == "" or field_value is None + form_is_empty = True + for name, field in self.fields.items(): + value = field.widget.value_from_datadict(self.data, self.files, self.add_prefix(name)) + if value is not None and value != "": + form_is_empty = False if form_is_empty: # clear any errors raised by the form fields @@ -613,7 +606,7 @@ class OtherContactsForm(RegistrarForm): if field in self.errors: del self.errors[field] - return cleaned + return self.cleaned_data class BaseOtherContactsFormSet(RegistrarFormSet): From e2cb8ee2d8e20f2999b788d25d5a0622a0fecfc3 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 29 Dec 2023 15:45:12 -0500 Subject: [PATCH 16/17] added comments to clean --- src/registrar/forms/application_wizard.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 06cbe81ca..39829d2b2 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -585,12 +585,21 @@ class OtherContactsForm(RegistrarForm): error_messages={"required": "Enter a phone number for this contact."}, ) - # Override clean in order to correct validation logic def clean(self): + """ + This method overrides the default behavior for forms. + This cleans the form after field validation has already taken place. + In this override, allow for a form which is empty to be considered + valid even though certain required fields have not passed field + validation + """ + # Set form_is_empty to True initially form_is_empty = True for name, field in self.fields.items(): + # get the value of the field from the widget value = field.widget.value_from_datadict(self.data, self.files, self.add_prefix(name)) + # if any field in the submitted form is not empty, set form_is_empty to False if value is not None and value != "": form_is_empty = False From 5df9c0f83dd08ec7081a3b1a091e8012a1882f63 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 29 Dec 2023 15:24:50 -0700 Subject: [PATCH 17/17] Run black linter --- src/registrar/forms/application_wizard.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 39829d2b2..59c3e25c7 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -590,7 +590,7 @@ class OtherContactsForm(RegistrarForm): This method overrides the default behavior for forms. This cleans the form after field validation has already taken place. In this override, allow for a form which is empty to be considered - valid even though certain required fields have not passed field + valid even though certain required fields have not passed field validation """