From 37189a5ff8900ee11e8831808b8592b577040775 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Thu, 4 Jan 2024 14:54:42 -0600 Subject: [PATCH 01/11] Use submission_date in application emails --- .../templates/emails/status_change_action_needed.txt | 2 +- src/registrar/templates/emails/status_change_approved.txt | 2 +- src/registrar/templates/emails/status_change_in_review.txt | 2 +- src/registrar/templates/emails/status_change_rejected.txt | 4 ++-- src/registrar/templates/emails/submission_confirmation.txt | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/registrar/templates/emails/status_change_action_needed.txt b/src/registrar/templates/emails/status_change_action_needed.txt index bce47f070..27886115d 100644 --- a/src/registrar/templates/emails/status_change_action_needed.txt +++ b/src/registrar/templates/emails/status_change_action_needed.txt @@ -4,7 +4,7 @@ Hi {{ application.submitter.first_name }}. We've identified an action needed to complete the review of your .gov domain request. DOMAIN REQUESTED: {{ application.requested_domain.name }} -REQUEST RECEIVED ON: {{ application.updated_at|date }} +REQUEST RECEIVED ON: {{ application.submission_date|date }} REQUEST #: {{ application.id }} STATUS: Action needed diff --git a/src/registrar/templates/emails/status_change_approved.txt b/src/registrar/templates/emails/status_change_approved.txt index 25abe6e69..e16b3e3e7 100644 --- a/src/registrar/templates/emails/status_change_approved.txt +++ b/src/registrar/templates/emails/status_change_approved.txt @@ -4,7 +4,7 @@ Hi {{ application.submitter.first_name }}. Congratulations! Your .gov domain request has been approved. DOMAIN REQUESTED: {{ application.requested_domain.name }} -REQUEST RECEIVED ON: {{ application.updated_at|date }} +REQUEST RECEIVED ON: {{ application.submission_date|date }} REQUEST #: {{ application.id }} STATUS: In review diff --git a/src/registrar/templates/emails/status_change_in_review.txt b/src/registrar/templates/emails/status_change_in_review.txt index b45b0d244..d013eb473 100644 --- a/src/registrar/templates/emails/status_change_in_review.txt +++ b/src/registrar/templates/emails/status_change_in_review.txt @@ -4,7 +4,7 @@ Hi {{ application.submitter.first_name }}. Your .gov domain request is being reviewed. DOMAIN REQUESTED: {{ application.requested_domain.name }} -REQUEST RECEIVED ON: {{ application.updated_at|date }} +REQUEST RECEIVED ON: {{ application.submission_date|date }} REQUEST #: {{ application.id }} STATUS: In review diff --git a/src/registrar/templates/emails/status_change_rejected.txt b/src/registrar/templates/emails/status_change_rejected.txt index 772050a7b..1b177573c 100644 --- a/src/registrar/templates/emails/status_change_rejected.txt +++ b/src/registrar/templates/emails/status_change_rejected.txt @@ -4,7 +4,7 @@ Hi {{ application.submitter.first_name }}. Your .gov domain request has been rejected. DOMAIN REQUESTED: {{ application.requested_domain.name }} -REQUEST RECEIVED ON: {{ application.updated_at|date }} +REQUEST RECEIVED ON: {{ application.submission_date|date }} REQUEST #: {{ application.id }} STATUS: Rejected @@ -29,4 +29,4 @@ requesting a .gov domain. The .gov team Contact us: Visit -{% endautoescape %} \ No newline at end of file +{% endautoescape %} diff --git a/src/registrar/templates/emails/submission_confirmation.txt b/src/registrar/templates/emails/submission_confirmation.txt index 2b0c0e86e..b25704052 100644 --- a/src/registrar/templates/emails/submission_confirmation.txt +++ b/src/registrar/templates/emails/submission_confirmation.txt @@ -4,7 +4,7 @@ Hi {{ application.submitter.first_name }}. We received your .gov domain request. DOMAIN REQUESTED: {{ application.requested_domain.name }} -REQUEST RECEIVED ON: {{ application.updated_at|date }} +REQUEST RECEIVED ON: {{ application.submission_date|date }} REQUEST #: {{ application.id }} STATUS: Received From fd4204a415336d963af1223087c03dff6f848930 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 8 Jan 2024 11:38:38 -0500 Subject: [PATCH 02/11] Tweak required on no other contacts --- src/registrar/templates/application_other_contacts.html | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/registrar/templates/application_other_contacts.html b/src/registrar/templates/application_other_contacts.html index bee307dde..0c2f59768 100644 --- a/src/registrar/templates/application_other_contacts.html +++ b/src/registrar/templates/application_other_contacts.html @@ -70,12 +70,11 @@
- {% include "includes/required_fields.html" %}

No other employees from your organization?

 - {% with attr_maxlength=1000 %} + {% with attr_maxlength=1000 add_label_class="usa-sr-only" %} {% input_with_errors forms.2.no_other_contacts_rationale %} {% endwith %}
From 93c1f066f762ce609fbef73202409baf3478883e Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 8 Jan 2024 18:15:21 -0500 Subject: [PATCH 03/11] 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 04/11] 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 05/11] 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 e2af9ac15319ac006e82059502110feeb3c1b5ca Mon Sep 17 00:00:00 2001 From: Michelle Rago <60157596+michelle-rago@users.noreply.github.com> Date: Tue, 9 Jan 2024 09:19:51 -0500 Subject: [PATCH 06/11] Text updates to success and error messages (#1593) * Text updates to error messages * Error message text updates * Error message text updates * Update to success message text for name server update * Update to DS data removal warning text * Success message text updates * typo fix * Text updates for success messages * Update 403 text * Update 500 error text * Using example.com instead of city.com * Putting the period outside the link * Updated name server minimum error text * Trying to fix python errors * Update name server minimum text * Update errors.py * Trying to fix python errors * Update errors.py * Making error messages agree * Minor text change * Fix black error message formatting * Fixed tests and reformatted * One last fix? --------- Co-authored-by: Neil Martinsen-Burrell --- src/api/views.py | 2 +- src/djangooidc/tests/test_views.py | 2 +- src/registrar/admin.py | 2 +- src/registrar/forms/application_wizard.py | 11 +++++++---- src/registrar/forms/domain.py | 2 +- src/registrar/templates/403.html | 6 +++--- src/registrar/templates/500.html | 4 ++-- src/registrar/templates/domain_dsdata.html | 2 +- src/registrar/tests/test_forms.py | 8 ++++---- src/registrar/tests/test_nameserver_error.py | 4 ++-- src/registrar/tests/test_views.py | 4 ++-- src/registrar/utility/errors.py | 20 +++++++++++--------- src/registrar/views/domain.py | 11 +++++------ 13 files changed, 41 insertions(+), 37 deletions(-) diff --git a/src/api/views.py b/src/api/views.py index a7dd7600a..3071712a7 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -29,7 +29,7 @@ DOMAIN_API_MESSAGES = { "unavailable": mark_safe( # nosec "That domain isn’t available. " "" - "Read more about choosing your .gov domain.".format(public_site_url("domains/choosing")) + "Read more about choosing your .gov domain.".format(public_site_url("domains/choosing")) ), "invalid": "Enter a domain using only letters, numbers, or hyphens (though we don't recommend using hyphens).", "success": "That domain is available! We’ll try to give you the domain you want, \ diff --git a/src/djangooidc/tests/test_views.py b/src/djangooidc/tests/test_views.py index da12f4fd5..282e91e1f 100644 --- a/src/djangooidc/tests/test_views.py +++ b/src/djangooidc/tests/test_views.py @@ -51,7 +51,7 @@ class ViewsTest(TestCase): # assert self.assertEqual(response.status_code, 500) self.assertTemplateUsed(response, "500.html") - self.assertIn("server error", response.content.decode("utf-8")) + self.assertIn("Server error", response.content.decode("utf-8")) def test_login_callback_reads_next(self, mock_client): # setup diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 364ae81f6..18eb4119c 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1251,7 +1251,7 @@ admin.site.register(models.DomainInformation, DomainInformationAdmin) admin.site.register(models.Domain, DomainAdmin) admin.site.register(models.DraftDomain, DraftDomainAdmin) # Host and HostIP removed from django admin because changes in admin -# do not propogate to registry and logic not applied +# do not propagate to registry and logic not applied # admin.site.register(models.Host, MyHostAdmin) admin.site.register(models.Website, WebsiteAdmin) admin.site.register(models.PublicContact, AuditedAdmin) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index fcf6bda7a..2802b1893 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -262,7 +262,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.", ) ], ) @@ -353,7 +353,7 @@ class CurrentSitesForm(RegistrarForm): required=False, label="Public website", error_messages={ - "invalid": ("Enter your organization's current website in the required format, like www.city.com.") + "invalid": ("Enter your organization's current website in the required format, like example.com.") }, ) @@ -543,7 +543,7 @@ class YourContactForm(RegistrarForm): ) phone = PhoneNumberField( label="Phone", - error_messages={"required": "Enter your phone number."}, + error_messages={"invalid": "Enter a valid 10-digit phone number.", "required": "Enter your phone number."}, ) @@ -574,7 +574,10 @@ class OtherContactsForm(RegistrarForm): ) phone = PhoneNumberField( label="Phone", - error_messages={"required": "Enter a phone number for this contact."}, + error_messages={ + "invalid": "Enter a valid 10-digit phone number.", + "required": "Enter a phone number for this contact.", + }, ) def clean(self): diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 5977449c3..17616df4b 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -59,7 +59,7 @@ class DomainNameserverForm(forms.Form): # add custom error messages self.fields["server"].error_messages.update( { - "required": "A minimum of 2 name servers are required.", + "required": "At least two name servers are required.", } ) diff --git a/src/registrar/templates/403.html b/src/registrar/templates/403.html index 91652d807..660e5d34c 100644 --- a/src/registrar/templates/403.html +++ b/src/registrar/templates/403.html @@ -9,10 +9,10 @@

- {% translate "You do not have the right permissions to view this page." %} + {% translate "You're not authorized to view this page." %}

- {% translate "Status 403" %} + {% translate "403 error" %}

@@ -23,7 +23,7 @@ {% endif %}

You must be an authorized user and need to be signed in to view this page. - Try logging in again. + Try signing in again.

If you'd like help with this error contact us. diff --git a/src/registrar/templates/500.html b/src/registrar/templates/500.html index 3c95900b2..dfbd90142 100644 --- a/src/registrar/templates/500.html +++ b/src/registrar/templates/500.html @@ -9,10 +9,10 @@

- {% translate "We're having some trouble" %} + {% translate "We're having some trouble." %}

- {% translate "Status 500 – server error" %} + {% translate "500 error" %}

{% if friendly_message %}

{{ friendly_message }}

diff --git a/src/registrar/templates/domain_dsdata.html b/src/registrar/templates/domain_dsdata.html index 15343413b..1ec4c1f93 100644 --- a/src/registrar/templates/domain_dsdata.html +++ b/src/registrar/templates/domain_dsdata.html @@ -114,7 +114,7 @@ aria-describedby="Your DNSSEC records will be deleted from the registry." data-force-action > - {% include 'includes/modal.html' with cancel_button_resets_ds_form=True modal_heading="Warning: You are about to remove all DS records on your domain" modal_description="To fully disable DNSSEC: In addition to removing your DS records here you’ll also need to delete the DS records at your DNS host. To avoid causing your domain to appear offline you should wait to delete your DS records at your DNS host until the Time to Live (TTL) expires. This is often less than 24 hours, but confirm with your provider." modal_button=modal_button|safe %} + {% include 'includes/modal.html' with cancel_button_resets_ds_form=True modal_heading="Warning: You are about to remove all DS records on your domain." modal_description="To fully disable DNSSEC: In addition to removing your DS records here, you’ll need to delete the DS records at your DNS host. To avoid causing your domain to appear offline, you should wait to delete your DS records at your DNS host until the Time to Live (TTL) expires. This is often less than 24 hours, but confirm with your provider." modal_button=modal_button|safe %}
{% endblock %} {# domain_content #} diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index e0afb2d71..3a8d63f37 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -30,7 +30,7 @@ class TestFormValidation(MockEppLib): form = OrganizationContactForm(data={"zipcode": "nah"}) self.assertEqual( form.errors["zipcode"], - ["Enter a zip code in the required format, like 12345 or 12345-6789."], + ["Enter a zip code in the form of 12345 or 12345-6789."], ) def test_org_contact_zip_valid(self): @@ -42,7 +42,7 @@ class TestFormValidation(MockEppLib): form = CurrentSitesForm(data={"website": "nah"}) self.assertEqual( form.errors["website"], - ["Enter your organization's current website in the required format, like www.city.com."], + ["Enter your organization's current website in the required format, like example.com."], ) def test_website_valid(self): @@ -207,7 +207,7 @@ class TestFormValidation(MockEppLib): def test_your_contact_phone_invalid(self): """Must be a valid phone number.""" form = YourContactForm(data={"phone": "boss@boss"}) - self.assertTrue(form.errors["phone"][0].startswith("Enter a valid phone number ")) + self.assertTrue(form.errors["phone"][0].startswith("Enter a valid 10-digit phone number.")) def test_other_contact_email_invalid(self): """must be a valid email address.""" @@ -220,7 +220,7 @@ class TestFormValidation(MockEppLib): def test_other_contact_phone_invalid(self): """Must be a valid phone number.""" form = OtherContactsForm(data={"phone": "super@boss"}) - self.assertTrue(form.errors["phone"][0].startswith("Enter a valid phone number ")) + self.assertTrue(form.errors["phone"][0].startswith("Enter a valid 10-digit phone number.")) def test_requirements_form_blank(self): """Requirements box unchecked is an error.""" diff --git a/src/registrar/tests/test_nameserver_error.py b/src/registrar/tests/test_nameserver_error.py index 78c6f2669..be9a26f6f 100644 --- a/src/registrar/tests/test_nameserver_error.py +++ b/src/registrar/tests/test_nameserver_error.py @@ -10,7 +10,7 @@ class TestNameserverError(TestCase): def test_with_no_ip(self): """Test NameserverError when no ip address is passed""" nameserver = "nameserver val" - expected = "Using your domain for a name server requires an IP address" + expected = "Using your domain for a name server requires an IP address." nsException = NameserverError(code=nsErrorCodes.MISSING_IP, nameserver=nameserver) self.assertEqual(nsException.message, expected) @@ -20,7 +20,7 @@ class TestNameserverError(TestCase): """Test NameserverError when no ip address and no nameserver is passed""" nameserver = "nameserver val" - expected = "Too many hosts provided, you may not have more than 13 nameservers." + expected = "You can't have more than 13 nameservers." nsException = NameserverError(code=nsErrorCodes.TOO_MANY_HOSTS, nameserver=nameserver) self.assertEqual(nsException.message, expected) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 1419d34f2..8d55462cb 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1974,7 +1974,7 @@ class TestDomainNameservers(TestDomainOverview): # the required field. form requires a minimum of 2 name servers self.assertContains( result, - "A minimum of 2 name servers are required.", + "At least two name servers are required.", count=2, status_code=200, ) @@ -2215,7 +2215,7 @@ class TestDomainNameservers(TestDomainOverview): # once around each required field. self.assertContains( result, - "A minimum of 2 name servers are required.", + "At least two name servers are required.", count=4, status_code=200, ) diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 455419236..ab08172ce 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -44,8 +44,8 @@ class GenericError(Exception): _error_mapping = { GenericErrorCodes.CANNOT_CONTACT_REGISTRY: ( - "We’re experiencing a system connection error. Please wait a few minutes " - "and try again. If you continue to receive this error after a few tries, " + "We’re experiencing a system error. Please wait a few minutes " + "and try again. If you continue to get this error, " "contact help@get.gov." ), GenericErrorCodes.GENERIC_ERROR: ("Value entered was wrong."), @@ -97,13 +97,15 @@ class NameserverError(Exception): """ _error_mapping = { - NameserverErrorCodes.MISSING_IP: ("Using your domain for a name server requires an IP address"), + NameserverErrorCodes.MISSING_IP: ("Using your domain for a name server requires an IP address."), NameserverErrorCodes.GLUE_RECORD_NOT_ALLOWED: ("Name server address does not match domain name"), NameserverErrorCodes.INVALID_IP: ("{}: Enter an IP address in the required format."), - NameserverErrorCodes.TOO_MANY_HOSTS: ("Too many hosts provided, you may not have more than 13 nameservers."), - NameserverErrorCodes.MISSING_HOST: ("Name server must be provided to enter IP address."), + NameserverErrorCodes.TOO_MANY_HOSTS: ("You can't have more than 13 nameservers."), + NameserverErrorCodes.MISSING_HOST: ("You must provide a name server to enter an IP address."), NameserverErrorCodes.INVALID_HOST: ("Enter a name server in the required format, like ns1.example.com"), - NameserverErrorCodes.DUPLICATE_HOST: ("Remove duplicate entry"), + NameserverErrorCodes.DUPLICATE_HOST: ( + "You already entered this name server address. Name server addresses must be unique." + ), NameserverErrorCodes.BAD_DATA: ( "There’s something wrong with the name server information you provided. " "If you need help email us at help@get.gov." @@ -156,8 +158,8 @@ class DsDataError(Exception): ), DsDataErrorCodes.INVALID_DIGEST_SHA1: ("SHA-1 digest must be exactly 40 characters."), DsDataErrorCodes.INVALID_DIGEST_SHA256: ("SHA-256 digest must be exactly 64 characters."), - DsDataErrorCodes.INVALID_DIGEST_CHARS: ("Digest must contain only alphanumeric characters [0-9,a-f]."), - DsDataErrorCodes.INVALID_KEYTAG_SIZE: ("Key tag must be less than 65535"), + DsDataErrorCodes.INVALID_DIGEST_CHARS: ("Digest must contain only alphanumeric characters (0-9, a-f)."), + DsDataErrorCodes.INVALID_KEYTAG_SIZE: ("Key tag must be less than 65535."), } def __init__(self, *args, code=None, **kwargs): @@ -187,7 +189,7 @@ class SecurityEmailError(Exception): """ _error_mapping = { - SecurityEmailErrorCodes.BAD_DATA: ("Enter an email address in the required format, like name@example.com.") + SecurityEmailErrorCodes.BAD_DATA: ("Enter an email address in the required format, " "like name@example.com."), } def __init__(self, *args, code=None, **kwargs): diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 59b206993..2cd12eb37 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -196,7 +196,7 @@ class DomainOrgNameAddressView(DomainFormBaseView): """The form is valid, save the organization name and mailing address.""" form.save() - messages.success(self.request, "The organization information has been updated.") + messages.success(self.request, "The organization information for this domain has been updated.") # superclass has the redirect return super().form_valid(form) @@ -348,9 +348,8 @@ class DomainNameserversView(DomainFormBaseView): messages.success( self.request, "The name servers for this domain have been updated. " - "Keep in mind that DNS changes may take some time to " - "propagate across the internet. It can take anywhere " - "from a few minutes to 48 hours for your changes to take place.", + "Note that DNS changes could take anywhere from a few minutes to " + "48 hours to propagate across the internet.", ) # superclass has the redirect @@ -549,7 +548,7 @@ class DomainYourContactInformationView(DomainFormBaseView): # Post to DB using values from the form form.save() - messages.success(self.request, "Your contact information for this domain has been updated.") + messages.success(self.request, "Your contact information has been updated.") # superclass has the redirect return super().form_valid(form) @@ -686,7 +685,7 @@ class DomainAddUserView(DomainFormBaseView): ) else: if add_success: - messages.success(self.request, f"Invited {email} to this domain.") + messages.success(self.request, f"{email} has been invited to this domain.") def _make_invitation(self, email_address: str, requester: User): """Make a Domain invitation for this email and redirect with a message.""" From f2b12e01d09ede57a17bc9543a2e2bc5e51b9f8d Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 9 Jan 2024 13:03:23 -0500 Subject: [PATCH 07/11] 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 08/11] 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 7e6a1c4188b1e64282aafe747ee644156615e390 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 9 Jan 2024 14:45:13 -0500 Subject: [PATCH 09/11] 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 10/11] 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 2c4ad262b41c91f402f664576b3b422a591066de Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 9 Jan 2024 16:26:28 -0500 Subject: [PATCH 11/11] 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 %}