From 608eaaea25a04897bc2d87d99f6989883d5b40da Mon Sep 17 00:00:00 2001 From: Jon Roberts Date: Mon, 6 Mar 2023 12:24:45 -0500 Subject: [PATCH 1/9] removed save button --- src/registrar/templates/application_form.html | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/registrar/templates/application_form.html b/src/registrar/templates/application_form.html index 628916b83..da03b62e2 100644 --- a/src/registrar/templates/application_form.html +++ b/src/registrar/templates/application_form.html @@ -70,12 +70,18 @@ class="usa-button" >Submit your domain request {% endif %} + {% comment %} + Commenting this section as we are "removing" + the save progress button for now. + If confirmed to remove this, please reivist and + remove in application.py "if button == "save":" + {% endcomment %} {% endblock %} From ea9a2dbcb6fda40ae2844fd3dc4524eb3a1c4997 Mon Sep 17 00:00:00 2001 From: Jon Roberts Date: Mon, 6 Mar 2023 15:20:42 -0500 Subject: [PATCH 2/9] redo test and do data validation after each step --- src/registrar/tests/test_views.py | 199 ++++++++++++++++-------------- 1 file changed, 109 insertions(+), 90 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index b4fc3c1cb..1f9348941 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -139,19 +139,20 @@ class DomainApplicationTests(TestWithUser, WebTest): type_form = type_page.form type_form["organization_type-organization_type"] = "federal" - # test saving the page + ## test saving the page + #self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + #result = type_page.form.submit("submit_button", value="save") + ## should remain on the same page + #self.assertEquals(result["Location"], "/register/organization_type/") + + + # test next button and validate data self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - result = type_page.form.submit("submit_button", value="save") - # should remain on the same page - self.assertEquals(result["Location"], "/register/organization_type/") + type_result = type_page.form.submit() # should see results in db application = DomainApplication.objects.get() # there's only one self.assertEquals(application.organization_type, "federal") - # test next button - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - type_result = type_page.form.submit() - # the post request should return a redirect to the next form in # the application self.assertEquals(type_result.status_code, 302) @@ -165,19 +166,21 @@ class DomainApplicationTests(TestWithUser, WebTest): federal_form = federal_page.form federal_form["organization_federal-federal_type"] = "executive" - # test saving the page - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - result = federal_page.form.submit("submit_button", value="save") - # should remain on the same page - self.assertEquals(result["Location"], "/register/organization_federal/") - # should see results in db - application = DomainApplication.objects.get() # there's only one - self.assertEquals(application.federal_type, "executive") + ## test saving the page + #self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + #result = federal_page.form.submit("submit_button", value="save") + ## should remain on the same page + #self.assertEquals(result["Location"], "/register/organization_federal/") + ## should see results in db + # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) federal_result = federal_form.submit() - + application = DomainApplication.objects.get() # there's only one + self.assertEquals(application.federal_type, "executive") + # the post request should return a redirect to the next form in + # the application self.assertEquals(federal_result.status_code, 302) self.assertEquals(federal_result["Location"], "/register/organization_contact/") num_pages_tested += 1 @@ -199,12 +202,17 @@ class DomainApplicationTests(TestWithUser, WebTest): org_contact_form["organization_contact-zipcode"] = "10002" org_contact_form["organization_contact-urbanization"] = "URB Royal Oaks" - # test saving the page - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - result = org_contact_page.form.submit("submit_button", value="save") + ## test saving the page + #self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + #result = org_contact_page.form.submit("submit_button", value="save") # should remain on the same page - self.assertEquals(result["Location"], "/register/organization_contact/") + ##self.assertEquals(result["Location"], "/register/organization_contact/") # should see results in db + + + # test next button + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + org_contact_result = org_contact_form.submit() application = DomainApplication.objects.get() # there's only one self.assertEquals(application.organization_name, "Testorg") self.assertEquals(application.address_line1, "address 1") @@ -213,11 +221,8 @@ class DomainApplicationTests(TestWithUser, WebTest): self.assertEquals(application.state_territory, "NY") self.assertEquals(application.zipcode, "10002") self.assertEquals(application.urbanization, "URB Royal Oaks") - - # test next button - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - org_contact_result = org_contact_form.submit() - + # the post request should return a redirect to the next form in + # the application self.assertEquals(org_contact_result.status_code, 302) self.assertEquals( org_contact_result["Location"], "/register/authorizing_official/" @@ -236,10 +241,15 @@ class DomainApplicationTests(TestWithUser, WebTest): ao_form["authorizing_official-phone"] = "(201) 555 5555" # test saving the page - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - result = ao_page.form.submit("submit_button", value="save") + # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # result = ao_page.form.submit("submit_button", value="save") # should remain on the same page - self.assertEquals(result["Location"], "/register/authorizing_official/") + # self.assertEquals(result["Location"], "/register/authorizing_official/") + + + # test next button + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + ao_result = ao_form.submit() # should see results in db application = DomainApplication.objects.get() # there's only one self.assertEquals(application.authorizing_official.first_name, "Testy ATO") @@ -247,11 +257,8 @@ class DomainApplicationTests(TestWithUser, WebTest): self.assertEquals(application.authorizing_official.title, "Chief Tester") self.assertEquals(application.authorizing_official.email, "testy@town.com") self.assertEquals(application.authorizing_official.phone, "(201) 555 5555") - - # test next button - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - ao_result = ao_form.submit() - + # the post request should return a redirect to the next form in + # the application self.assertEquals(ao_result.status_code, 302) self.assertEquals(ao_result["Location"], "/register/current_sites/") num_pages_tested += 1 @@ -264,21 +271,23 @@ class DomainApplicationTests(TestWithUser, WebTest): current_sites_form["current_sites-0-website"] = "www.city.com" # test saving the page - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - result = current_sites_page.form.submit("submit_button", value="save") + # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # result = current_sites_page.form.submit("submit_button", value="save") # should remain on the same page - self.assertEquals(result["Location"], "/register/current_sites/") + # self.assertEquals(result["Location"], "/register/current_sites/") # should see results in db + + + # test next button + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + current_sites_result = current_sites_form.submit() application = DomainApplication.objects.get() # there's only one self.assertEquals( application.current_websites.filter(website="http://www.city.com").count(), 1, ) - - # test next button - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - current_sites_result = current_sites_form.submit() - + # the post request should return a redirect to the next form in + # the application self.assertEquals(current_sites_result.status_code, 302) self.assertEquals(current_sites_result["Location"], "/register/dotgov_domain/") num_pages_tested += 1 @@ -292,20 +301,21 @@ class DomainApplicationTests(TestWithUser, WebTest): dotgov_form["dotgov_domain-0-alternative_domain"] = "city1" # test saving the page - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - result = dotgov_page.form.submit("submit_button", value="save") + # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # result = dotgov_page.form.submit("submit_button", value="save") # should remain on the same page - self.assertEquals(result["Location"], "/register/dotgov_domain/") + # self.assertEquals(result["Location"], "/register/dotgov_domain/") + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + dotgov_result = dotgov_form.submit() # should see results in db application = DomainApplication.objects.get() # there's only one self.assertEquals(application.requested_domain.name, "city.gov") self.assertEquals( application.alternative_domains.filter(website="city1.gov").count(), 1 ) - - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - dotgov_result = dotgov_form.submit() - + # the post request should return a redirect to the next form in + # the application self.assertEquals(dotgov_result.status_code, 302) self.assertEquals(dotgov_result["Location"], "/register/purpose/") num_pages_tested += 1 @@ -318,18 +328,19 @@ class DomainApplicationTests(TestWithUser, WebTest): purpose_form["purpose-purpose"] = "For all kinds of things." # test saving the page - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - result = purpose_page.form.submit("submit_button", value="save") + # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # result = purpose_page.form.submit("submit_button", value="save") # should remain on the same page - self.assertEquals(result["Location"], "/register/purpose/") - # should see results in db - application = DomainApplication.objects.get() # there's only one - self.assertEquals(application.purpose, "For all kinds of things.") - + # self.assertEquals(result["Location"], "/register/purpose/") + # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) purpose_result = purpose_form.submit() - + # should see results in db + application = DomainApplication.objects.get() # there's only one + self.assertEquals(application.purpose, "For all kinds of things.") + # the post request should return a redirect to the next form in + # the application self.assertEquals(purpose_result.status_code, 302) self.assertEquals(purpose_result["Location"], "/register/your_contact/") num_pages_tested += 1 @@ -347,10 +358,15 @@ class DomainApplicationTests(TestWithUser, WebTest): your_contact_form["your_contact-phone"] = "(201) 555 5556" # test saving the page - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - result = your_contact_page.form.submit("submit_button", value="save") + # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # result = your_contact_page.form.submit("submit_button", value="save") # should remain on the same page - self.assertEquals(result["Location"], "/register/your_contact/") + # self.assertEquals(result["Location"], "/register/your_contact/") + + + # test next button + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + your_contact_result = your_contact_form.submit() # should see results in db application = DomainApplication.objects.get() # there's only one self.assertEquals(application.submitter.first_name, "Testy you") @@ -358,11 +374,8 @@ class DomainApplicationTests(TestWithUser, WebTest): self.assertEquals(application.submitter.title, "Admin Tester") self.assertEquals(application.submitter.email, "testy-admin@town.com") self.assertEquals(application.submitter.phone, "(201) 555 5556") - - # test next button - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - your_contact_result = your_contact_form.submit() - + # the post request should return a redirect to the next form in + # the application self.assertEquals(your_contact_result.status_code, 302) self.assertEquals(your_contact_result["Location"], "/register/other_contacts/") num_pages_tested += 1 @@ -380,10 +393,15 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form["other_contacts-0-phone"] = "(201) 555 5557" # test saving the page - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - result = other_contacts_page.form.submit("submit_button", value="save") + # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # result = other_contacts_page.form.submit("submit_button", value="save") # should remain on the same page - self.assertEquals(result["Location"], "/register/other_contacts/") + # self.assertEquals(result["Location"], "/register/other_contacts/") + + + # test next button + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + other_contacts_result = other_contacts_form.submit() # should see results in db application = DomainApplication.objects.get() # there's only one self.assertEquals( @@ -396,11 +414,8 @@ class DomainApplicationTests(TestWithUser, WebTest): ).count(), 1, ) - - # test next button - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - other_contacts_result = other_contacts_form.submit() - + # the post request should return a redirect to the next form in + # the application self.assertEquals(other_contacts_result.status_code, 302) self.assertEquals(other_contacts_result["Location"], "/register/anything_else/") num_pages_tested += 1 @@ -414,18 +429,20 @@ class DomainApplicationTests(TestWithUser, WebTest): anything_else_form["anything_else-anything_else"] = "Nothing else." # test saving the page - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - result = anything_else_page.form.submit("submit_button", value="save") + # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # result = anything_else_page.form.submit("submit_button", value="save") # should remain on the same page - self.assertEquals(result["Location"], "/register/anything_else/") - # should see results in db - application = DomainApplication.objects.get() # there's only one - self.assertEquals(application.anything_else, "Nothing else.") + # self.assertEquals(result["Location"], "/register/anything_else/") + # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) anything_else_result = anything_else_form.submit() - + # should see results in db + application = DomainApplication.objects.get() # there's only one + self.assertEquals(application.anything_else, "Nothing else.") + # the post request should return a redirect to the next form in + # the application self.assertEquals(anything_else_result.status_code, 302) self.assertEquals(anything_else_result["Location"], "/register/requirements/") num_pages_tested += 1 @@ -439,18 +456,20 @@ class DomainApplicationTests(TestWithUser, WebTest): requirements_form["requirements-is_policy_acknowledged"] = True # test saving the page - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - result = requirements_page.form.submit("submit_button", value="save") + # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # result = requirements_page.form.submit("submit_button", value="save") # should remain on the same page - self.assertEquals(result["Location"], "/register/requirements/") - # should see results in db - application = DomainApplication.objects.get() # there's only one - self.assertEquals(application.is_policy_acknowledged, True) + # self.assertEquals(result["Location"], "/register/requirements/") + # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) requirements_result = requirements_form.submit() - + # should see results in db + application = DomainApplication.objects.get() # there's only one + self.assertEquals(application.is_policy_acknowledged, True) + # the post request should return a redirect to the next form in + # the application self.assertEquals(requirements_result.status_code, 302) self.assertEquals(requirements_result["Location"], "/register/review/") num_pages_tested += 1 @@ -493,10 +512,10 @@ class DomainApplicationTests(TestWithUser, WebTest): self.assertContains(review_page, "Nothing else.") # test saving the page - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - result = review_page.form.submit("submit_button", value="save") + # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # result = review_page.form.submit("submit_button", value="save") # should remain on the same page - self.assertEquals(result["Location"], "/register/review/") + # self.assertEquals(result["Location"], "/register/review/") # final submission results in a redirect to the "finished" URL self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) From eada4e9e23ab95fa1cdf8271375102df9db1d151 Mon Sep 17 00:00:00 2001 From: Jon Roberts Date: Mon, 6 Mar 2023 15:29:12 -0500 Subject: [PATCH 3/9] cleaned up test, removing 'save progress' tests --- src/registrar/tests/test_views.py | 96 ++++--------------------------- 1 file changed, 10 insertions(+), 86 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 1f9348941..e0cdc0575 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -139,12 +139,6 @@ class DomainApplicationTests(TestWithUser, WebTest): type_form = type_page.form type_form["organization_type-organization_type"] = "federal" - ## test saving the page - #self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - #result = type_page.form.submit("submit_button", value="save") - ## should remain on the same page - #self.assertEquals(result["Location"], "/register/organization_type/") - # test next button and validate data self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) @@ -152,7 +146,6 @@ class DomainApplicationTests(TestWithUser, WebTest): # should see results in db application = DomainApplication.objects.get() # there's only one self.assertEquals(application.organization_type, "federal") - # the post request should return a redirect to the next form in # the application self.assertEquals(type_result.status_code, 302) @@ -166,17 +159,10 @@ class DomainApplicationTests(TestWithUser, WebTest): federal_form = federal_page.form federal_form["organization_federal-federal_type"] = "executive" - ## test saving the page - #self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - #result = federal_page.form.submit("submit_button", value="save") - ## should remain on the same page - #self.assertEquals(result["Location"], "/register/organization_federal/") - ## should see results in db - - # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) federal_result = federal_form.submit() + # validate that data from this step are being saved application = DomainApplication.objects.get() # there's only one self.assertEquals(application.federal_type, "executive") # the post request should return a redirect to the next form in @@ -201,18 +187,12 @@ class DomainApplicationTests(TestWithUser, WebTest): org_contact_form["organization_contact-state_territory"] = "NY" org_contact_form["organization_contact-zipcode"] = "10002" org_contact_form["organization_contact-urbanization"] = "URB Royal Oaks" - - ## test saving the page - #self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - #result = org_contact_page.form.submit("submit_button", value="save") - # should remain on the same page - ##self.assertEquals(result["Location"], "/register/organization_contact/") - # should see results in db # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) org_contact_result = org_contact_form.submit() + # validate that data from this step are being saved application = DomainApplication.objects.get() # there's only one self.assertEquals(application.organization_name, "Testorg") self.assertEquals(application.address_line1, "address 1") @@ -240,17 +220,11 @@ class DomainApplicationTests(TestWithUser, WebTest): ao_form["authorizing_official-email"] = "testy@town.com" ao_form["authorizing_official-phone"] = "(201) 555 5555" - # test saving the page - # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - # result = ao_page.form.submit("submit_button", value="save") - # should remain on the same page - # self.assertEquals(result["Location"], "/register/authorizing_official/") - # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) ao_result = ao_form.submit() - # should see results in db + # validate that data from this step are being saved application = DomainApplication.objects.get() # there's only one self.assertEquals(application.authorizing_official.first_name, "Testy ATO") self.assertEquals(application.authorizing_official.last_name, "Tester ATO") @@ -270,17 +244,10 @@ class DomainApplicationTests(TestWithUser, WebTest): current_sites_form = current_sites_page.form current_sites_form["current_sites-0-website"] = "www.city.com" - # test saving the page - # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - # result = current_sites_page.form.submit("submit_button", value="save") - # should remain on the same page - # self.assertEquals(result["Location"], "/register/current_sites/") - # should see results in db - - # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) current_sites_result = current_sites_form.submit() + # validate that data from this step are being saved application = DomainApplication.objects.get() # there's only one self.assertEquals( application.current_websites.filter(website="http://www.city.com").count(), @@ -300,15 +267,10 @@ class DomainApplicationTests(TestWithUser, WebTest): dotgov_form["dotgov_domain-requested_domain"] = "city" dotgov_form["dotgov_domain-0-alternative_domain"] = "city1" - # test saving the page - # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - # result = dotgov_page.form.submit("submit_button", value="save") - # should remain on the same page - # self.assertEquals(result["Location"], "/register/dotgov_domain/") self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) dotgov_result = dotgov_form.submit() - # should see results in db + # validate that data from this step are being saved application = DomainApplication.objects.get() # there's only one self.assertEquals(application.requested_domain.name, "city.gov") self.assertEquals( @@ -326,17 +288,11 @@ class DomainApplicationTests(TestWithUser, WebTest): purpose_page = dotgov_result.follow() purpose_form = purpose_page.form purpose_form["purpose-purpose"] = "For all kinds of things." - - # test saving the page - # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - # result = purpose_page.form.submit("submit_button", value="save") - # should remain on the same page - # self.assertEquals(result["Location"], "/register/purpose/") # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) purpose_result = purpose_form.submit() - # should see results in db + # validate that data from this step are being saved application = DomainApplication.objects.get() # there's only one self.assertEquals(application.purpose, "For all kinds of things.") # the post request should return a redirect to the next form in @@ -357,17 +313,11 @@ class DomainApplicationTests(TestWithUser, WebTest): your_contact_form["your_contact-email"] = "testy-admin@town.com" your_contact_form["your_contact-phone"] = "(201) 555 5556" - # test saving the page - # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - # result = your_contact_page.form.submit("submit_button", value="save") - # should remain on the same page - # self.assertEquals(result["Location"], "/register/your_contact/") - # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) your_contact_result = your_contact_form.submit() - # should see results in db + # validate that data from this step are being saved application = DomainApplication.objects.get() # there's only one self.assertEquals(application.submitter.first_name, "Testy you") self.assertEquals(application.submitter.last_name, "Tester you") @@ -392,17 +342,11 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form["other_contacts-0-email"] = "testy2@town.com" other_contacts_form["other_contacts-0-phone"] = "(201) 555 5557" - # test saving the page - # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - # result = other_contacts_page.form.submit("submit_button", value="save") - # should remain on the same page - # self.assertEquals(result["Location"], "/register/other_contacts/") - # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) other_contacts_result = other_contacts_form.submit() - # should see results in db + # validate that data from this step are being saved application = DomainApplication.objects.get() # there's only one self.assertEquals( application.other_contacts.filter( @@ -428,17 +372,10 @@ class DomainApplicationTests(TestWithUser, WebTest): anything_else_form["anything_else-anything_else"] = "Nothing else." - # test saving the page - # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - # result = anything_else_page.form.submit("submit_button", value="save") - # should remain on the same page - # self.assertEquals(result["Location"], "/register/anything_else/") - - # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) anything_else_result = anything_else_form.submit() - # should see results in db + # validate that data from this step are being saved application = DomainApplication.objects.get() # there's only one self.assertEquals(application.anything_else, "Nothing else.") # the post request should return a redirect to the next form in @@ -455,17 +392,10 @@ class DomainApplicationTests(TestWithUser, WebTest): requirements_form["requirements-is_policy_acknowledged"] = True - # test saving the page - # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - # result = requirements_page.form.submit("submit_button", value="save") - # should remain on the same page - # self.assertEquals(result["Location"], "/register/requirements/") - - # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) requirements_result = requirements_form.submit() - # should see results in db + # validate that data from this step are being saved application = DomainApplication.objects.get() # there's only one self.assertEquals(application.is_policy_acknowledged, True) # the post request should return a redirect to the next form in @@ -511,12 +441,6 @@ class DomainApplicationTests(TestWithUser, WebTest): self.assertContains(review_page, "(201) 555-5557") self.assertContains(review_page, "Nothing else.") - # test saving the page - # self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - # result = review_page.form.submit("submit_button", value="save") - # should remain on the same page - # self.assertEquals(result["Location"], "/register/review/") - # final submission results in a redirect to the "finished" URL self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) with less_console_noise(): From 26269648ceaa0973a51a324c4671e2f33ded7cfa Mon Sep 17 00:00:00 2001 From: Jon Roberts Date: Mon, 6 Mar 2023 16:12:09 -0500 Subject: [PATCH 4/9] lint --- src/registrar/tests/test_views.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index e0cdc0575..8f5842921 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -139,7 +139,6 @@ class DomainApplicationTests(TestWithUser, WebTest): type_form = type_page.form type_form["organization_type-organization_type"] = "federal" - # test next button and validate data self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) type_result = type_page.form.submit() @@ -187,7 +186,6 @@ class DomainApplicationTests(TestWithUser, WebTest): org_contact_form["organization_contact-state_territory"] = "NY" org_contact_form["organization_contact-zipcode"] = "10002" org_contact_form["organization_contact-urbanization"] = "URB Royal Oaks" - # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) @@ -220,7 +218,6 @@ class DomainApplicationTests(TestWithUser, WebTest): ao_form["authorizing_official-email"] = "testy@town.com" ao_form["authorizing_official-phone"] = "(201) 555 5555" - # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) ao_result = ao_form.submit() @@ -267,7 +264,6 @@ class DomainApplicationTests(TestWithUser, WebTest): dotgov_form["dotgov_domain-requested_domain"] = "city" dotgov_form["dotgov_domain-0-alternative_domain"] = "city1" - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) dotgov_result = dotgov_form.submit() # validate that data from this step are being saved @@ -288,7 +284,7 @@ class DomainApplicationTests(TestWithUser, WebTest): purpose_page = dotgov_result.follow() purpose_form = purpose_page.form purpose_form["purpose-purpose"] = "For all kinds of things." - + # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) purpose_result = purpose_form.submit() @@ -313,7 +309,6 @@ class DomainApplicationTests(TestWithUser, WebTest): your_contact_form["your_contact-email"] = "testy-admin@town.com" your_contact_form["your_contact-phone"] = "(201) 555 5556" - # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) your_contact_result = your_contact_form.submit() @@ -342,7 +337,6 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form["other_contacts-0-email"] = "testy2@town.com" other_contacts_form["other_contacts-0-phone"] = "(201) 555 5557" - # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) other_contacts_result = other_contacts_form.submit() From dee826c5e37fb84820341a6f4b2ad2c8b2df7b18 Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Mon, 6 Mar 2023 15:49:11 -0600 Subject: [PATCH 5/9] Remove UserProfile and add PublicContact --- .../decisions/0012-user-models.md | 2 +- .../0020-user-models-revisited-plus-whois.md | 103 ++++++++++++++++++ src/registrar/admin.py | 6 +- src/registrar/config/settings.py | 1 - src/registrar/fixtures.py | 3 - src/registrar/forms/edit_profile.py | 21 ---- src/registrar/models/__init__.py | 6 +- src/registrar/models/contact.py | 7 ++ src/registrar/models/public_contact.py | 49 +++++++++ src/registrar/models/user_profile.py | 28 ----- src/registrar/models/utility/address_model.py | 27 ----- src/registrar/views/profile.py | 21 ---- 12 files changed, 166 insertions(+), 108 deletions(-) create mode 100644 docs/architecture/decisions/0020-user-models-revisited-plus-whois.md delete mode 100644 src/registrar/forms/edit_profile.py create mode 100644 src/registrar/models/public_contact.py delete mode 100644 src/registrar/models/user_profile.py delete mode 100644 src/registrar/models/utility/address_model.py delete mode 100644 src/registrar/views/profile.py diff --git a/docs/architecture/decisions/0012-user-models.md b/docs/architecture/decisions/0012-user-models.md index 3e404106e..434dbb54a 100644 --- a/docs/architecture/decisions/0012-user-models.md +++ b/docs/architecture/decisions/0012-user-models.md @@ -4,7 +4,7 @@ Date: 2022-09-26 ## Status -Proposed +Superseded by [20. User models revisited, plus WHOIS](./0020-user-models-revisited-plus-whois.md) ## Context diff --git a/docs/architecture/decisions/0020-user-models-revisited-plus-whois.md b/docs/architecture/decisions/0020-user-models-revisited-plus-whois.md new file mode 100644 index 000000000..c35bd3507 --- /dev/null +++ b/docs/architecture/decisions/0020-user-models-revisited-plus-whois.md @@ -0,0 +1,103 @@ +# 20. User models revisited, plus WHOIS + +Date: 2022-03-01 + +## Status + +Accepted + +## Context + +In the process of thinking through the details of registry implementation and role-based access control, it has become clear that the registrar has 3 types of contacts: + +1. Those for the purpose of allowing CISA to verify the authenticity of a request. In other words, is the organization an eligible U.S.-based government and is the requestor duly authorized to make the request on behalf of their government? +1. Those for the purpose of managing a domain or its DNS configuration. + * There is ambiguous overlap remaining between use case 1 and 2. +1. Those for the purpose of publishing publicly in WHOIS. + +Additionally, there are two mental models of contacts that impact the permissions associated with them and how they can be updated: + +1. A contact represents a person: changes made in one part of the system will update in all parts of the system; people are not allowed to make updates unless they are authorized. +1. A contact represents information filled out on a sheet of paper: changes on one “copy” of the information will not update other “copies” of the information; people are allowed to make updates based on their authorization to access and edit the “sheet of paper”. + +## Decision + +To have a custom `User` model containing un-editable data derived from Login.gov and updated automatically each time a user logs in. In role-based access control, User is the model to which roles attach. + +To have a `Contact` model which stores name and contact data. The presence of a foreign key from Contact to User indicates that that contact data has been associated with a Login.gov user account. If a User is deleted, the foreign key column is set to null. + +User and Contact follow the “person” mental model. + +To have a `PublicContact` model which stores WHOIS data. Domains will be created with the following default values. + +PublicContact follows the “sheet of paper” mental model. + +### Registrant default values + +| Field | Value | +|---|---| +|name | CSD/CB – Attn: Cameron Dixon +|org | Cybersecurity and Infrastructure Security Agency +|street1 | CISA – NGR STOP 0645 +|street2 | 1110 N. Glebe Rd. +|city | Arlington +|sp | VA +|pc | 20598-0645 +|cc | US + +### Administrative default values + +| Field | Value | +|---|---| +|name | Program Manager +|org | Cybersecurity and Infrastructure Security Agency +|street1 | 4200 Wilson Blvd. +|city | Arlington +|sp | VA +|pc | 22201 +|cc | US +|voice | +1.8882820870 +|email | dotgov@cisa.dhs.gov + +### Technical default values + +Whether this contact will be created by default or not is yet to be determined. + +| Field | Value | +|---|---| +|name | Registry Customer Service +|org | Cybersecurity and Infrastructure Security Agency +|street1 | 4200 Wilson Blvd. +|city | Arlington +|sp | VA +|pc | 22201 +|cc | US +|voice | +1.8882820870 +|email | registrar@dotgov.gov + +### Security default values + +Whether this contact will be created by default or not is yet to be determined. + +The EPP “disclose tags” feature might be used to publish only the email address. + +| Field | Value | +|---|---| +|name | Registry Customer Service +|org | Cybersecurity and Infrastructure Security Agency +|street1 | 4200 Wilson Blvd. +|city | Arlington +|sp | VA +|pc | 22201 +|cc | US +|voice | +1.8882820870 +|email | registrar@dotgov.gov + + +## Consequences + +This has minimal impact on the code we’ve developed so far. + +By having PublicContact be an entirely separate model, it ensures that (for better or worse) WHOIS contact data must be updated separately from general Contacts. At present, CISA intends to allow registrants to edit only one contact: security, so this is a minor point of low impact. + +In a future state where CISA allows more to be published, it is easy to imagine a set of checkboxes on a contact update form: “[ ] publish this as my technical contact for example.gov”, etc. diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 19cf60729..b2d291ce5 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -22,18 +22,18 @@ class AuditedAdmin(admin.ModelAdmin): ) -class UserProfileInline(admin.StackedInline): +class UserContactInline(admin.StackedInline): """Edit a user's profile on the user page.""" - model = models.UserProfile + model = models.Contact class MyUserAdmin(UserAdmin): """Custom user admin class to use our inlines.""" - inlines = [UserProfileInline] + inlines = [UserContactInline] class HostIPInline(admin.StackedInline): diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index c2abd85d4..4043c6991 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -645,7 +645,6 @@ if DEBUG: NPLUSONE_RAISE = False NPLUSONE_WHITELIST = [ {"model": "admin.LogEntry", "field": "user"}, - {"model": "registrar.UserProfile"}, ] # insert the amazing django-debug-toolbar diff --git a/src/registrar/fixtures.py b/src/registrar/fixtures.py index 36c1733c5..9f54b20a7 100644 --- a/src/registrar/fixtures.py +++ b/src/registrar/fixtures.py @@ -4,7 +4,6 @@ from faker import Faker from registrar.models import ( User, - UserProfile, DomainApplication, Domain, Contact, @@ -57,8 +56,6 @@ class UserFixture: user.is_active = True user.save() logger.debug("User object created for %s" % admin["first_name"]) - UserProfile.objects.get_or_create(user=user) - logger.debug("Profile object created for %s" % admin["first_name"]) except Exception as e: logger.warning(e) logger.debug("All users loaded.") diff --git a/src/registrar/forms/edit_profile.py b/src/registrar/forms/edit_profile.py deleted file mode 100644 index d3819faa3..000000000 --- a/src/registrar/forms/edit_profile.py +++ /dev/null @@ -1,21 +0,0 @@ -from django import forms - -from ..models import UserProfile - - -class EditProfileForm(forms.ModelForm): - - """Custom form class for editing a UserProfile. - - We can add whatever fields we want to this form and customize how they - are displayed. The form is rendered into a template `profile.html` by a - view called `edit_profile` in `profile.py`. - """ - - display_name = forms.CharField( - widget=forms.TextInput(attrs={"class": "usa-input"}), label="Display Name" - ) - - class Meta: - model = UserProfile - fields = ["display_name"] diff --git a/src/registrar/models/__init__.py b/src/registrar/models/__init__.py index 1bb9dde84..9b7aba971 100644 --- a/src/registrar/models/__init__.py +++ b/src/registrar/models/__init__.py @@ -6,7 +6,7 @@ from .domain import Domain from .host_ip import HostIP from .host import Host from .nameserver import Nameserver -from .user_profile import UserProfile +from .public_contact import PublicContact from .user import User from .website import Website @@ -17,7 +17,7 @@ __all__ = [ "HostIP", "Host", "Nameserver", - "UserProfile", + "PublicContact", "User", "Website", ] @@ -28,6 +28,6 @@ auditlog.register(Domain) auditlog.register(HostIP) auditlog.register(Host) auditlog.register(Nameserver) -auditlog.register(UserProfile) +auditlog.register(PublicContact) auditlog.register(User) auditlog.register(Website) diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index 6f0b62ea8..d5d32a7ae 100644 --- a/src/registrar/models/contact.py +++ b/src/registrar/models/contact.py @@ -9,6 +9,13 @@ class Contact(TimeStampedModel): """Contact information follows a similar pattern for each contact.""" + user = models.OneToOneField( + "registrar.User", + null=True, + blank=True, + on_delete=models.SET_NULL, + ) + first_name = models.TextField( null=True, blank=True, diff --git a/src/registrar/models/public_contact.py b/src/registrar/models/public_contact.py new file mode 100644 index 000000000..9c9d627d2 --- /dev/null +++ b/src/registrar/models/public_contact.py @@ -0,0 +1,49 @@ +from django.db import models + +from .utility.time_stamped_model import TimeStampedModel + + +class PublicContact(TimeStampedModel): + """Contact information intended to be published in WHOIS.""" + + class ContactTypeChoices(models.TextChoices): + """These are the types of contacts accepted by the registry.""" + REGISTRANT = "registrant", "Registrant" + ADMINISTRATIVE = "administrative", "Administrative" + TECHNICAL = "technical", "Technical" + SECURITY = "security", "Security" + + contact_type = models.CharField(choices=ContactTypeChoices) + + # contact's full name + name = models.TextField(null=False) + # contact's organization (null ok) + org = models.TextField(null=True) + # contact's street + street1 = models.TextField(null=False) + # contact's street (null ok) + street2 = models.TextField(null=True) + # contact's street (null ok) + street3 = models.TextField(null=True) + # contact's city + city = models.TextField(null=False) + # contact's state or province + sp = models.TextField(null=False) + # contact's postal code + pc = models.TextField(null=False) + # contact's country code + cc = models.TextField(null=False) + # contact's email address + email = models.TextField(null=False) + # contact's phone number + # Must be in ITU.E164.2005 format + voice = models.TextField(null=False) + # contact's fax number (null ok) + # Must be in ITU.E164.2005 format + fax = models.TextField(null=True) + # contact's authorization code + # 16 characters minium + pw = models.TextField(null=False) + + def __str__(self): + return f"{self.name} <{self.email}>" \ No newline at end of file diff --git a/src/registrar/models/user_profile.py b/src/registrar/models/user_profile.py deleted file mode 100644 index 806124205..000000000 --- a/src/registrar/models/user_profile.py +++ /dev/null @@ -1,28 +0,0 @@ -from django.db import models - -from .utility.time_stamped_model import TimeStampedModel -from .utility.address_model import AddressModel - -from .contact import Contact - - -class UserProfile(Contact, TimeStampedModel, AddressModel): - - """User information, unrelated to their login/auth details.""" - - user = models.OneToOneField( - "registrar.User", - null=True, - blank=True, - on_delete=models.CASCADE, - ) - display_name = models.TextField() - - def __str__(self): - # use info stored in User rather than Contact, - # because Contact is user-editable while User - # pulls from identity-verified Login.gov - try: - return str(self.user) - except Exception: - return "Orphaned account" diff --git a/src/registrar/models/utility/address_model.py b/src/registrar/models/utility/address_model.py deleted file mode 100644 index c158ce085..000000000 --- a/src/registrar/models/utility/address_model.py +++ /dev/null @@ -1,27 +0,0 @@ -from django.db import models - - -class AddressModel(models.Model): - """ - An abstract base model that provides common fields - for postal addresses. - """ - - # contact's street (null ok) - street1 = models.TextField(blank=True) - # contact's street (null ok) - street2 = models.TextField(blank=True) - # contact's street (null ok) - street3 = models.TextField(blank=True) - # contact's city - city = models.TextField(blank=True) - # contact's state or province (null ok) - sp = models.TextField(blank=True) - # contact's postal code (null ok) - pc = models.TextField(blank=True) - # contact's country code - cc = models.TextField(blank=True) - - class Meta: - abstract = True - # don't put anything else here, it will be ignored diff --git a/src/registrar/views/profile.py b/src/registrar/views/profile.py deleted file mode 100644 index 3a1b416c3..000000000 --- a/src/registrar/views/profile.py +++ /dev/null @@ -1,21 +0,0 @@ -from django.shortcuts import redirect, render -from django.contrib.auth.decorators import login_required -from django.contrib import messages - -from registrar.forms import EditProfileForm - - -@login_required -def edit_profile(request): - """View for a profile editing page.""" - - if request.method == "POST": - # post to this view when changes are made - profile_form = EditProfileForm(request.POST, instance=request.user.userprofile) - if profile_form.is_valid(): - profile_form.save() - messages.success(request, "Your profile is updated successfully") - return redirect(to="edit-profile") - else: - profile_form = EditProfileForm(instance=request.user.userprofile) - return render(request, "profile.html", {"profile_form": profile_form}) From 7815d58c2c270a49774c54627a07831d836f24b1 Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Tue, 7 Mar 2023 10:56:49 -0600 Subject: [PATCH 6/9] Fix test failures --- src/registrar/config/urls.py | 1 - src/registrar/forms/__init__.py | 1 - .../migrations/0012_delete_userprofile.py | 17 +++++ .../0013_publiccontact_contact_user.py | 68 +++++++++++++++++++ src/registrar/models/public_contact.py | 5 +- src/registrar/tests/test_views.py | 4 -- src/registrar/views/__init__.py | 1 - 7 files changed, 88 insertions(+), 9 deletions(-) create mode 100644 src/registrar/migrations/0012_delete_userprofile.py create mode 100644 src/registrar/migrations/0013_publiccontact_contact_user.py diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 19bd176e5..287043c06 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -53,7 +53,6 @@ urlpatterns = [ name=views.ApplicationWizard.EDIT_URL_NAME, ), path("health/", views.health), - path("edit_profile/", views.edit_profile, name="edit-profile"), path("openid/", include("djangooidc.urls")), path("register/", include((application_urls, APPLICATION_NAMESPACE))), path("api/v1/available/", available, name="available"), diff --git a/src/registrar/forms/__init__.py b/src/registrar/forms/__init__.py index 954fff109..bd0426884 100644 --- a/src/registrar/forms/__init__.py +++ b/src/registrar/forms/__init__.py @@ -1,2 +1 @@ -from .edit_profile import * from .application_wizard import * diff --git a/src/registrar/migrations/0012_delete_userprofile.py b/src/registrar/migrations/0012_delete_userprofile.py new file mode 100644 index 000000000..b5bcebb95 --- /dev/null +++ b/src/registrar/migrations/0012_delete_userprofile.py @@ -0,0 +1,17 @@ +# Generated by Django 4.1.6 on 2023-03-07 14:31 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0011_remove_domainapplication_security_email"), + ] + + operations = [ + migrations.DeleteModel( + name="UserProfile", + ), + ] diff --git a/src/registrar/migrations/0013_publiccontact_contact_user.py b/src/registrar/migrations/0013_publiccontact_contact_user.py new file mode 100644 index 000000000..29a9385cd --- /dev/null +++ b/src/registrar/migrations/0013_publiccontact_contact_user.py @@ -0,0 +1,68 @@ +# Generated by Django 4.1.6 on 2023-03-07 14:31 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0012_delete_userprofile"), + ] + + operations = [ + migrations.CreateModel( + name="PublicContact", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("created_at", models.DateTimeField(auto_now_add=True)), + ("updated_at", models.DateTimeField(auto_now=True)), + ( + "contact_type", + models.CharField( + choices=[ + ("registrant", "Registrant"), + ("administrative", "Administrative"), + ("technical", "Technical"), + ("security", "Security"), + ], + max_length=14, + ), + ), + ("name", models.TextField()), + ("org", models.TextField(null=True)), + ("street1", models.TextField()), + ("street2", models.TextField(null=True)), + ("street3", models.TextField(null=True)), + ("city", models.TextField()), + ("sp", models.TextField()), + ("pc", models.TextField()), + ("cc", models.TextField()), + ("email", models.TextField()), + ("voice", models.TextField()), + ("fax", models.TextField(null=True)), + ("pw", models.TextField()), + ], + options={ + "abstract": False, + }, + ), + migrations.AddField( + model_name="contact", + name="user", + field=models.OneToOneField( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to=settings.AUTH_USER_MODEL, + ), + ), + ] diff --git a/src/registrar/models/public_contact.py b/src/registrar/models/public_contact.py index 9c9d627d2..c5517f8e0 100644 --- a/src/registrar/models/public_contact.py +++ b/src/registrar/models/public_contact.py @@ -8,12 +8,13 @@ class PublicContact(TimeStampedModel): class ContactTypeChoices(models.TextChoices): """These are the types of contacts accepted by the registry.""" + REGISTRANT = "registrant", "Registrant" ADMINISTRATIVE = "administrative", "Administrative" TECHNICAL = "technical", "Technical" SECURITY = "security", "Security" - contact_type = models.CharField(choices=ContactTypeChoices) + contact_type = models.CharField(max_length=14, choices=ContactTypeChoices.choices) # contact's full name name = models.TextField(null=False) @@ -46,4 +47,4 @@ class PublicContact(TimeStampedModel): pw = models.TextField(null=False) def __str__(self): - return f"{self.name} <{self.email}>" \ No newline at end of file + return f"{self.name} <{self.email}>" diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index b4fc3c1cb..5f68a3a42 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -83,10 +83,6 @@ class LoggedInTests(TestWithUser): self.assertContains(response, self.user.last_name) self.assertContains(response, self.user.email) - def test_edit_profile(self): - response = self.client.get("/edit_profile/") - self.assertContains(response, "Display Name") - def test_application_form_view(self): response = self.client.get("/register/", follow=True) self.assertContains( diff --git a/src/registrar/views/__init__.py b/src/registrar/views/__init__.py index 2952d1185..3941213ea 100644 --- a/src/registrar/views/__init__.py +++ b/src/registrar/views/__init__.py @@ -1,5 +1,4 @@ from .application import * from .health import * from .index import * -from .profile import * from .whoami import * From b65f1a63d34d10763914d0d096ae1741ff5a3aed Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Tue, 7 Mar 2023 11:07:20 -0600 Subject: [PATCH 7/9] Update post_save to work with Contact --- src/djangooidc/backends.py | 12 +- .../0014_user_phone_alter_contact_user.py | 27 +++++ src/registrar/models/user.py | 9 ++ src/registrar/signals.py | 48 ++++++-- src/registrar/tests/test_signals.py | 103 ++++++++++++++++++ 5 files changed, 180 insertions(+), 19 deletions(-) create mode 100644 src/registrar/migrations/0014_user_phone_alter_contact_user.py create mode 100644 src/registrar/tests/test_signals.py diff --git a/src/djangooidc/backends.py b/src/djangooidc/backends.py index 52cbc712b..ac65c89d4 100644 --- a/src/djangooidc/backends.py +++ b/src/djangooidc/backends.py @@ -28,18 +28,14 @@ class OpenIdConnectBackend(ModelBackend): UserModel = get_user_model() username = self.clean_username(kwargs["sub"]) - if "upn" in kwargs.keys(): - username = kwargs["upn"] # Some OP may actually choose to withhold some information, so we must # test if it is present openid_data = {"last_login": timezone.now()} - openid_data["first_name"] = kwargs.get("first_name", "") openid_data["first_name"] = kwargs.get("given_name", "") - openid_data["first_name"] = kwargs.get("christian_name", "") openid_data["last_name"] = kwargs.get("family_name", "") - openid_data["last_name"] = kwargs.get("last_name", "") openid_data["email"] = kwargs.get("email", "") + openid_data["phone"] = kwargs.get("phone", "") # Note that this could be accomplished in one try-except clause, but # instead we use get_or_create when creating unknown users since it has @@ -47,6 +43,7 @@ class OpenIdConnectBackend(ModelBackend): if getattr(settings, "OIDC_CREATE_UNKNOWN_USER", True): args = { UserModel.USERNAME_FIELD: username, + # defaults _will_ be updated, these are not fallbacks "defaults": openid_data, } user, created = UserModel.objects.update_or_create(**args) @@ -56,10 +53,7 @@ class OpenIdConnectBackend(ModelBackend): try: user = UserModel.objects.get_by_natural_key(username) except UserModel.DoesNotExist: - try: - user = UserModel.objects.get(email=kwargs["email"]) - except UserModel.DoesNotExist: - return None + return None return user def clean_username(self, username): diff --git a/src/registrar/migrations/0014_user_phone_alter_contact_user.py b/src/registrar/migrations/0014_user_phone_alter_contact_user.py new file mode 100644 index 000000000..452b2b9d5 --- /dev/null +++ b/src/registrar/migrations/0014_user_phone_alter_contact_user.py @@ -0,0 +1,27 @@ +# Generated by Django 4.1.6 on 2023-03-07 16:39 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import phonenumber_field.modelfields # type: ignore + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0013_publiccontact_contact_user"), + ] + + operations = [ + migrations.AddField( + model_name="user", + name="phone", + field=phonenumber_field.modelfields.PhoneNumberField( + blank=True, + db_index=True, + help_text="Phone", + max_length=128, + null=True, + region=None, + ), + ), + ] diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index acfd1769e..9fc726114 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -1,5 +1,7 @@ from django.contrib.auth.models import AbstractUser +from phonenumber_field.modelfields import PhoneNumberField # type: ignore + class User(AbstractUser): """ @@ -7,6 +9,13 @@ class User(AbstractUser): but can be customized later. """ + phone = PhoneNumberField( + null=True, + blank=True, + help_text="Phone", + db_index=True, + ) + def __str__(self): # this info is pulled from Login.gov if self.first_name or self.last_name: diff --git a/src/registrar/signals.py b/src/registrar/signals.py index 492f535d0..62c5873f0 100644 --- a/src/registrar/signals.py +++ b/src/registrar/signals.py @@ -5,7 +5,7 @@ from django.core.management import call_command from django.db.models.signals import post_save, post_migrate from django.dispatch import receiver -from .models import User, UserProfile +from .models import User, Contact logger = logging.getLogger(__name__) @@ -15,18 +15,46 @@ logger = logging.getLogger(__name__) def handle_profile(sender, instance, **kwargs): """Method for when a User is saved. - If the user is being created, then create a matching UserProfile. Otherwise - save an updated profile or create one if it doesn't exist. + A first time registrant may have been invited, so we'll search for a matching + Contact record, by email address, and associate them, if possible. + + A first time registrant may not have a matching Contact, so we'll create one, + copying the contact values we received from Login.gov in order to initialize it. + + During subsequent login, a User record may be updated with new data from Login.gov, + but in no case will we update contact values on an existing Contact record. """ - if kwargs.get("created", False): - UserProfile.objects.create(user=instance) + first_name = getattr(instance, "first_name", "") + last_name = getattr(instance, "last_name", "") + email = getattr(instance, "email", "") + phone = getattr(instance, "phone", "") + + is_new_user = kwargs.get("created", False) + + if is_new_user: + contacts = Contact.objects.filter(email=email) else: - # the user is not being created. - if hasattr(instance, "userprofile"): - instance.userprofile.save() - else: - UserProfile.objects.create(user=instance) + contacts = Contact.objects.filter(user=instance) + + if len(contacts) == 0: # no matching contact + Contact.objects.create( + user=instance, + first_name=first_name, + last_name=last_name, + email=email, + phone=phone, + ) + + if len(contacts) >= 1 and is_new_user: # a matching contact + contacts[0].user = instance + contacts[0].save() + + if len(contacts) > 1: # multiple matches + logger.warning( + "There are multiple Contacts with the same email address." + f" Picking #{contacts[0].id} for User #{instance.id}." + ) @receiver(post_migrate) diff --git a/src/registrar/tests/test_signals.py b/src/registrar/tests/test_signals.py new file mode 100644 index 000000000..ddc47e60d --- /dev/null +++ b/src/registrar/tests/test_signals.py @@ -0,0 +1,103 @@ +from django.test import TestCase +from django.contrib.auth import get_user_model + +from registrar.models import Contact + + +class TestUserPostSave(TestCase): + def setUp(self): + self.username = "test_user" + self.first_name = "First" + self.last_name = "Last" + self.email = "info@example.com" + self.phone = "202-555-0133" + + self.preferred_first_name = "One" + self.preferred_last_name = "Two" + self.preferred_email = "front_desk@example.com" + self.preferred_phone = "202-555-0134" + + def test_user_created_without_matching_contact(self): + """Expect 1 Contact containing data copied from User.""" + self.assertEquals(len(Contact.objects.all()), 0) + user = get_user_model().objects.create( + username=self.username, + first_name=self.first_name, + last_name=self.last_name, + email=self.email, + phone=self.phone, + ) + actual = Contact.objects.get(user=user) + self.assertEquals(actual.first_name, self.first_name) + self.assertEquals(actual.last_name, self.last_name) + self.assertEquals(actual.email, self.email) + self.assertEquals(actual.phone, self.phone) + + def test_user_created_with_matching_contact(self): + """Expect 1 Contact associated, but with no data copied from User.""" + self.assertEquals(len(Contact.objects.all()), 0) + Contact.objects.create( + first_name=self.preferred_first_name, + last_name=self.preferred_last_name, + email=self.email, # must be the same, to find the match! + phone=self.preferred_phone, + ) + user = get_user_model().objects.create( + username=self.username, + first_name=self.first_name, + last_name=self.last_name, + email=self.email, + ) + actual = Contact.objects.get(user=user) + self.assertEquals(actual.first_name, self.preferred_first_name) + self.assertEquals(actual.last_name, self.preferred_last_name) + self.assertEquals(actual.email, self.email) + self.assertEquals(actual.phone, self.preferred_phone) + + def test_user_updated_without_matching_contact(self): + """Expect 1 Contact containing data copied from User.""" + # create the user + self.assertEquals(len(Contact.objects.all()), 0) + user = get_user_model().objects.create( + username=self.username, first_name="", last_name="", email="", phone="" + ) + # delete the contact + Contact.objects.all().delete() + self.assertEquals(len(Contact.objects.all()), 0) + # modify the user + user.username = self.username + user.first_name = self.first_name + user.last_name = self.last_name + user.email = self.email + user.phone = self.phone + user.save() + # test + actual = Contact.objects.get(user=user) + self.assertEquals(actual.first_name, self.first_name) + self.assertEquals(actual.last_name, self.last_name) + self.assertEquals(actual.email, self.email) + self.assertEquals(actual.phone, self.phone) + + def test_user_updated_with_matching_contact(self): + """Expect 1 Contact associated, but with no data copied from User.""" + # create the user + self.assertEquals(len(Contact.objects.all()), 0) + user = get_user_model().objects.create( + username=self.username, + first_name=self.first_name, + last_name=self.last_name, + email=self.email, + phone=self.phone, + ) + # modify the user + user.first_name = self.preferred_first_name + user.last_name = self.preferred_last_name + user.email = self.preferred_email + user.phone = self.preferred_phone + user.save() + # test + actual = Contact.objects.get(user=user) + self.assertEquals(actual.first_name, self.first_name) + self.assertEquals(actual.last_name, self.last_name) + self.assertEquals(actual.email, self.email) + self.assertEquals(actual.phone, self.phone) From f4b40027e52ecbaa822e09a597143f026ed29594 Mon Sep 17 00:00:00 2001 From: Jon Roberts Date: Tue, 7 Mar 2023 14:00:56 -0500 Subject: [PATCH 8/9] removing parts as per Neil's comment --- src/registrar/templates/application_form.html | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/registrar/templates/application_form.html b/src/registrar/templates/application_form.html index da03b62e2..ccbdb4ffb 100644 --- a/src/registrar/templates/application_form.html +++ b/src/registrar/templates/application_form.html @@ -70,18 +70,6 @@ class="usa-button" >Submit your domain request {% endif %} - {% comment %} - Commenting this section as we are "removing" - the save progress button for now. - If confirmed to remove this, please reivist and - remove in application.py "if button == "save":" - - {% endcomment %} {% endblock %} From 3cd3869c934843c6bf65364bbb8c74a2fed21159 Mon Sep 17 00:00:00 2001 From: Michelle Rago <60157596+michelle-rago@users.noreply.github.com> Date: Wed, 8 Mar 2023 12:09:13 -0500 Subject: [PATCH 9/9] Mr/text updates (#444) * Updated error message text * Updated field labels, intro text * Tidy up thank you page * Removed reference to managing a domain * Update test language to match form language * Fix linting error (hopefully) * Fix linting and tests --------- Co-authored-by: Neil Martinsen-Burrell --- src/registrar/forms/application_wizard.py | 16 ++++++++++++---- src/registrar/templates/application_done.html | 4 ++-- .../templates/application_your_contact.html | 5 ++--- src/registrar/tests/test_forms.py | 14 ++++++++++---- src/registrar/tests/test_views.py | 2 +- 5 files changed, 27 insertions(+), 14 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index b8cc92a5d..43a59dfb0 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -226,8 +226,10 @@ class OrganizationElectionForm(RegistrarForm): is_election_board = self.cleaned_data["is_election_board"] if is_election_board is None: raise forms.ValidationError( - "Select “Yes” if you represent an election office. Select “No” if you" - " don’t.", + ( + "Select “Yes” if you represent an election office. Select “No” if" + " you don’t." + ), code="required", ) return is_election_board @@ -399,6 +401,12 @@ class CurrentSitesForm(RegistrarForm): website = forms.URLField( required=False, label="Public website", + error_messages={ + "invalid": ( + "Enter your organization's website in the required format, like" + " www.city.com." + ) + }, ) @@ -676,7 +684,7 @@ class NoOtherContactsForm(RegistrarForm): # label has to end in a space to get the label_suffix to show label=( "Please explain why there are no other employees from your organization" - " that we can contact." + " we can contact to help us assess your eligibility for a .gov domain." ), widget=forms.Textarea(), ) @@ -692,7 +700,7 @@ class AnythingElseForm(RegistrarForm): class RequirementsForm(RegistrarForm): is_policy_acknowledged = forms.BooleanField( - label=("I read and agree to the requirements for operating .gov domains."), + label="I read and agree to the requirements for operating .gov domains.", error_messages={ "required": ( "Check the box if you read and agree to the requirements for" diff --git a/src/registrar/templates/application_done.html b/src/registrar/templates/application_done.html index 4c94880fc..4b270c0ee 100644 --- a/src/registrar/templates/application_done.html +++ b/src/registrar/templates/application_done.html @@ -1,7 +1,7 @@ {% extends 'base.html' %} {% load static %} -{% block title %}Thank you for your domain request{% endblock %} +{% block title %}Thanks for your domain request!{% endblock %} {% block content %}
@@ -14,7 +14,7 @@ />

Thank you

-

Thank you for your domain request. We'll email a copy of your request to you, +

We'll email a copy of your request to you, your authorizing official, and any contacts you added.

Next steps in this process

diff --git a/src/registrar/templates/application_your_contact.html b/src/registrar/templates/application_your_contact.html index ffbf10f8a..1016ebc71 100644 --- a/src/registrar/templates/application_your_contact.html +++ b/src/registrar/templates/application_your_contact.html @@ -2,8 +2,7 @@ {% load field_helpers %} {% block form_instructions %} -

We’ll use the following information to contact you about your domain request and, - once your request is approved, about managing your domain.

+

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

If you’d like us to use a different name, email, or phone number you can make those changes below. Changing your contact information here won’t affect your login.gov @@ -35,4 +34,4 @@ {% endwith %} -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index a388bd9a3..7e3cfdf8a 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -29,7 +29,13 @@ class TestFormValidation(TestCase): def test_website_invalid(self): form = CurrentSitesForm(data={"website": "nah"}) - self.assertEqual(form.errors["website"], ["Enter a valid URL."]) + self.assertEqual( + form.errors["website"], + [ + "Enter your organization's" + " website in the required format, like www.city.com." + ], + ) def test_website_valid(self): form = CurrentSitesForm(data={"website": "hyphens-rule.gov.uk"}) @@ -83,7 +89,7 @@ class TestFormValidation(TestCase): """Must be a valid phone number.""" form = AuthorizingOfficialForm(data={"phone": "boss@boss"}) self.assertTrue( - form.errors["phone"][0].startswith("Enter a valid phone number") + form.errors["phone"][0].startswith("Enter a valid phone number ") ) def test_your_contact_email_invalid(self): @@ -98,7 +104,7 @@ class TestFormValidation(TestCase): """Must be a valid phone number.""" form = YourContactForm(data={"phone": "boss@boss"}) self.assertTrue( - form.errors["phone"][0].startswith("Enter a valid phone number") + form.errors["phone"][0].startswith("Enter a valid phone number ") ) def test_other_contact_email_invalid(self): @@ -113,7 +119,7 @@ class TestFormValidation(TestCase): """Must be a valid phone number.""" form = OtherContactsForm(data={"phone": "boss@boss"}) self.assertTrue( - form.errors["phone"][0].startswith("Enter a valid phone number") + form.errors["phone"][0].startswith("Enter a valid phone number ") ) def test_requirements_form_blank(self): diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 86a05a5ff..b1cc9c8d1 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -445,7 +445,7 @@ class DomainApplicationTests(TestWithUser, WebTest): self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) with less_console_noise(): final_result = review_result.follow() - self.assertContains(final_result, "Thank you for your domain request") + self.assertContains(final_result, "Thanks for your domain request!") # check that any new pages are added to this test self.assertEqual(num_pages, num_pages_tested)