From f54276d82b729e16bcca82b58c0dc2250373102e Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Thu, 12 Jan 2023 09:07:07 -0500 Subject: [PATCH 1/4] Implement dynamic questions with formsets --- src/registrar/fixtures.py | 56 +++--- src/registrar/forms/application_wizard.py | 167 +++++++++++++----- ..._remove_userprofile_created_at_and_more.py | 48 +++++ src/registrar/models/contact.py | 4 +- src/registrar/models/user_profile.py | 2 +- src/registrar/models/website.py | 4 +- .../templates/application_dotgov_domain.html | 14 +- .../templates/application_other_contacts.html | 36 ++-- src/registrar/tests/test_views.py | 12 +- src/registrar/views/application.py | 11 +- 10 files changed, 249 insertions(+), 105 deletions(-) create mode 100644 src/registrar/migrations/0008_remove_userprofile_created_at_and_more.py diff --git a/src/registrar/fixtures.py b/src/registrar/fixtures.py index 58cfc633e..5b69e24f9 100644 --- a/src/registrar/fixtures.py +++ b/src/registrar/fixtures.py @@ -1,6 +1,5 @@ import logging import random -import string from faker import Faker from registrar.models import ( @@ -81,28 +80,29 @@ class DomainApplicationFixture: "status": "started", "organization_name": "Example - Finished but not Submitted", }, - { - "status": "started", - "organization_name": "Example - Just started", - "organization_type": "federal", - "federal_agency": None, - "federal_type": None, - "address_line1": None, - "address_line2": None, - "city": None, - "state_territory": None, - "zipcode": None, - "urbanization": None, - "purpose": None, - "security_email": None, - "anything_else": None, - "is_policy_acknowledged": None, - "authorizing_official": None, - "submitter": None, - "other_contacts": [], - "current_websites": [], - "alternative_domains": [], - }, + # an example of a more manual application + # { + # "status": "started", + # "organization_name": "Example - Just started", + # "organization_type": "federal", + # "federal_agency": None, + # "federal_type": None, + # "address_line1": None, + # "address_line2": None, + # "city": None, + # "state_territory": None, + # "zipcode": None, + # "urbanization": None, + # "purpose": None, + # "security_email": None, + # "anything_else": None, + # "is_policy_acknowledged": None, + # "authorizing_official": None, + # "submitter": None, + # "other_contacts": [], + # "current_websites": [], + # "alternative_domains": [], + # }, { "status": "submitted", "organization_name": "Example - Submitted but pending Investigation", @@ -121,12 +121,12 @@ class DomainApplicationFixture: "last_name": fake.last_name(), "title": fake.job(), "email": fake.ascii_safe_email(), - "phone": fake.phone_number(), + "phone": "201-555-5555", } @classmethod def fake_dot_gov(cls): - return "".join(random.choices(string.ascii_lowercase, k=16)) + ".gov" # nosec + return f"{fake.slug()}.gov" @classmethod def _set_non_foreign_key_fields(cls, da: DomainApplication, app: dict): @@ -199,7 +199,7 @@ class DomainApplicationFixture: if "other_contacts" in app: for contact in app["other_contacts"]: da.other_contacts.add(Contact.objects.get_or_create(**contact)[0]) - else: + elif not da.other_contacts.exists(): other_contacts = [ Contact.objects.create(**cls.fake_contact()) for _ in range(random.randint(0, 3)) # nosec @@ -211,7 +211,7 @@ class DomainApplicationFixture: da.current_websites.add( Website.objects.get_or_create(website=website)[0] ) - else: + elif not da.current_websites.exists(): current_websites = [ Website.objects.create(website=fake.uri()) for _ in range(random.randint(0, 3)) # nosec @@ -223,7 +223,7 @@ class DomainApplicationFixture: da.alternative_domains.add( Website.objects.get_or_create(website=domain)[0] ) - else: + elif not da.alternative_domains.exists(): alternative_domains = [ Website.objects.create(website=cls.fake_dot_gov()) for _ in range(random.randint(0, 3)) # nosec diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 32143cd66..69e51697f 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -1,12 +1,12 @@ from __future__ import annotations # allows forward references in annotations +from itertools import zip_longest import logging +from phonenumber_field.formfields import PhoneNumberField # type: ignore from django import forms from django.core.validators import RegexValidator from django.utils.safestring import mark_safe -from phonenumber_field.formfields import PhoneNumberField # type: ignore - from registrar.models import Contact, DomainApplication, Domain logger = logging.getLogger(__name__) @@ -57,6 +57,19 @@ class RegistrarForm(forms.Form): } # type: ignore +class RegistrarFormSet(forms.BaseFormSet): + """ + As with RegistrarForm, a common set of methods and configuration. + + Subclass this class to create new formsets. + """ + + def __init__(self, *args, **kwargs): + # save a reference to an application object + self.application = kwargs.pop("application", None) + super(RegistrarFormSet, self).__init__(*args, **kwargs) + + class OrganizationTypeForm(RegistrarForm): organization_type = forms.ChoiceField( required=True, @@ -335,6 +348,69 @@ class CurrentSitesForm(RegistrarForm): ) +class AlternativeDomainForm(RegistrarForm): + alternative_domain = forms.CharField( + required=False, + label="Alternative domain", + ) + + +class BaseAlternativeDomainFormSet(RegistrarFormSet): + def to_database(self, obj: DomainApplication): + if not self.is_valid(): + return + + obj.save() + query = obj.alternative_domains.order_by("created_at").all() # order matters + + # the use of `zip` pairs the forms in the formset with the + # related objects gotten from the database -- there should always be + # at least as many forms as database entries: extra forms means new + # entries, but fewer forms is _not_ the correct way to delete items + # (likely a client-side error or an attempt at data tampering) + + for db_obj, post_data in zip_longest(query, self.forms, fillvalue=None): + + cleaned = post_data.cleaned_data if post_data is not None else {} + domain = cleaned.get("alternative_domain", None) + + # matching database object exists, update it + if db_obj is not None and isinstance(domain, str): + entry_was_erased = domain.strip() == "" + if entry_was_erased: + db_obj.delete() + continue + try: + normalized = Domain.normalize(domain, "gov", blank=True) + except ValueError as e: + logger.debug(e) + continue + db_obj.website = normalized + db_obj.save() + + # no matching database object, create it + elif db_obj is None and domain is not None: + try: + normalized = Domain.normalize(domain, "gov", blank=True) + except ValueError as e: + logger.debug(e) + continue + obj.alternative_domains.create(website=normalized) + + @classmethod + def from_database(cls, obj): + query = obj.alternative_domains.order_by("created_at").all() # order matters + return [{"alternative_domain": domain.sld} for domain in query] + + +AlternativeDomainFormSet = forms.formset_factory( + AlternativeDomainForm, + extra=1, + absolute_max=1500, + formset=BaseAlternativeDomainFormSet, +) + + class DotGovDomainForm(RegistrarForm): def to_database(self, obj): if not self.is_valid(): @@ -353,12 +429,6 @@ class DotGovDomainForm(RegistrarForm): obj.save() obj.save() - normalized = Domain.normalize( - self.cleaned_data["alternative_domain"], "gov", blank=True - ) - if normalized: - # TODO: ability to update existing records - obj.alternative_domains.create(website=normalized) @classmethod def from_database(cls, obj): @@ -366,23 +436,9 @@ class DotGovDomainForm(RegistrarForm): requested_domain = getattr(obj, "requested_domain", None) if requested_domain is not None: values["requested_domain"] = requested_domain.sld - - alternative_domain = obj.alternative_domains.first() - if alternative_domain is not None: - values["alternative_domain"] = alternative_domain.sld - return values - requested_domain = forms.CharField( - label="What .gov domain do you want?", - ) - alternative_domain = forms.CharField( - required=False, - label=( - "Are there other domains you’d like if we can’t give you your first " - "choice? Entering alternative domains is optional." - ), - ) + requested_domain = forms.CharField(label="What .gov domain do you want?") def clean_requested_domain(self): """Requested domains need to be legal top-level domains, not subdomains. @@ -490,25 +546,6 @@ class YourContactForm(RegistrarForm): class OtherContactsForm(RegistrarForm): - def to_database(self, obj): - if not self.is_valid(): - return - obj.save() - - # TODO: ability to handle multiple contacts - contact = obj.other_contacts.filter(email=self.cleaned_data["email"]).first() - if contact is not None: - super().to_database(contact) - else: - contact = Contact() - super().to_database(contact) - obj.other_contacts.add(contact) - - @classmethod - def from_database(cls, obj): - other_contacts = obj.other_contacts.first() - return super().from_database(other_contacts) - first_name = forms.CharField( label="First name / given name", label_suffix=REQUIRED_SUFFIX, @@ -557,6 +594,52 @@ class OtherContactsForm(RegistrarForm): ) +class BaseOtherContactsFormSet(RegistrarFormSet): + def to_database(self, obj): + if not self.is_valid(): + return + obj.save() + + query = obj.other_contacts.order_by("created_at").all() + + # the use of `zip` pairs the forms in the formset with the + # related objects gotten from the database -- there should always be + # at least as many forms as database entries: extra forms means new + # entries, but fewer forms is _not_ the correct way to delete items + # (likely a client-side error or an attempt at data tampering) + + for db_obj, post_data in zip_longest(query, self.forms, fillvalue=None): + + cleaned = post_data.cleaned_data if post_data is not None else {} + + # matching database object exists, update it + if db_obj is not None and cleaned: + empty = (isinstance(v, str) and not v.strip() for v in cleaned.values()) + erased = all(empty) + if erased: + db_obj.delete() + continue + for key, value in cleaned.items(): + setattr(db_obj, key, value) + db_obj.save() + + # no matching database object, create it + elif db_obj is None and cleaned: + obj.other_contacts.create(**cleaned) + + @classmethod + def from_database(cls, obj): + return obj.other_contacts.order_by("created_at").values() # order matters + + +OtherContactsFormSet = forms.formset_factory( + OtherContactsForm, + extra=1, + absolute_max=1500, + formset=BaseOtherContactsFormSet, +) + + class SecurityEmailForm(RegistrarForm): security_email = forms.EmailField( required=False, diff --git a/src/registrar/migrations/0008_remove_userprofile_created_at_and_more.py b/src/registrar/migrations/0008_remove_userprofile_created_at_and_more.py new file mode 100644 index 000000000..56536a88b --- /dev/null +++ b/src/registrar/migrations/0008_remove_userprofile_created_at_and_more.py @@ -0,0 +1,48 @@ +# Generated by Django 4.1.5 on 2023-01-13 01:54 + +from django.db import migrations, models +import django.utils.timezone + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0007_domainapplication_more_organization_information_and_more"), + ] + + operations = [ + migrations.RemoveField( + model_name="userprofile", + name="created_at", + ), + migrations.RemoveField( + model_name="userprofile", + name="updated_at", + ), + migrations.AddField( + model_name="contact", + name="created_at", + field=models.DateTimeField( + auto_now_add=True, default=django.utils.timezone.now + ), + preserve_default=False, + ), + migrations.AddField( + model_name="contact", + name="updated_at", + field=models.DateTimeField(auto_now=True), + ), + migrations.AddField( + model_name="website", + name="created_at", + field=models.DateTimeField( + auto_now_add=True, default=django.utils.timezone.now + ), + preserve_default=False, + ), + migrations.AddField( + model_name="website", + name="updated_at", + field=models.DateTimeField(auto_now=True), + ), + ] diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index 0d3a7c389..6f0b62ea8 100644 --- a/src/registrar/models/contact.py +++ b/src/registrar/models/contact.py @@ -2,8 +2,10 @@ from django.db import models from phonenumber_field.modelfields import PhoneNumberField # type: ignore +from .utility.time_stamped_model import TimeStampedModel -class Contact(models.Model): + +class Contact(TimeStampedModel): """Contact information follows a similar pattern for each contact.""" diff --git a/src/registrar/models/user_profile.py b/src/registrar/models/user_profile.py index fd08c7821..806124205 100644 --- a/src/registrar/models/user_profile.py +++ b/src/registrar/models/user_profile.py @@ -6,7 +6,7 @@ from .utility.address_model import AddressModel from .contact import Contact -class UserProfile(TimeStampedModel, Contact, AddressModel): +class UserProfile(Contact, TimeStampedModel, AddressModel): """User information, unrelated to their login/auth details.""" diff --git a/src/registrar/models/website.py b/src/registrar/models/website.py index 5b4efb619..65d86ddf1 100644 --- a/src/registrar/models/website.py +++ b/src/registrar/models/website.py @@ -2,8 +2,10 @@ from django.apps import apps from django.core.exceptions import ValidationError from django.db import models +from .utility.time_stamped_model import TimeStampedModel -class Website(models.Model): + +class Website(TimeStampedModel): """Keep domain names in their own table so that applications can refer to many of them.""" diff --git a/src/registrar/templates/application_dotgov_domain.html b/src/registrar/templates/application_dotgov_domain.html index 4225764e5..744b525f7 100644 --- a/src/registrar/templates/application_dotgov_domain.html +++ b/src/registrar/templates/application_dotgov_domain.html @@ -1,4 +1,3 @@ - {% extends 'application_form.html' %} {% load widget_tweaks static%} @@ -75,8 +74,19 @@ .gov {% endif %} + {{ forms.1.management_form }} +

Are there other domains you’d like if we can’t give you your first choice? Entering alternative domains is optional.

- diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 378341295..247232896 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -283,7 +283,7 @@ class DomainApplicationTests(TestWithUser, WebTest): dotgov_page = current_sites_result.follow() dotgov_form = dotgov_page.form dotgov_form["dotgov_domain-requested_domain"] = "city" - dotgov_form["dotgov_domain-alternative_domain"] = "city1" + dotgov_form["dotgov_domain-0-alternative_domain"] = "city1" # test saving the page self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) @@ -367,11 +367,11 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_page = your_contact_result.follow() other_contacts_form = other_contacts_page.form - other_contacts_form["other_contacts-first_name"] = "Testy2" - other_contacts_form["other_contacts-last_name"] = "Tester2" - other_contacts_form["other_contacts-title"] = "Another Tester" - other_contacts_form["other_contacts-email"] = "testy2@town.com" - other_contacts_form["other_contacts-phone"] = "(201) 555 5557" + other_contacts_form["other_contacts-0-first_name"] = "Testy2" + other_contacts_form["other_contacts-0-last_name"] = "Tester2" + other_contacts_form["other_contacts-0-title"] = "Another Tester" + other_contacts_form["other_contacts-0-email"] = "testy2@town.com" + other_contacts_form["other_contacts-0-phone"] = "(201) 555 5557" # test saving the page self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) diff --git a/src/registrar/views/application.py b/src/registrar/views/application.py index a53fb51cf..e210eb6e7 100644 --- a/src/registrar/views/application.py +++ b/src/registrar/views/application.py @@ -155,8 +155,9 @@ class ApplicationWizard(LoginRequiredMixin, TemplateView): @storage.deleter def storage(self): - del self.request.session[self.prefix] - self.request.session.modified = True + if self.prefix in self.request.session: + del self.request.session[self.prefix] + self.request.session.modified = True def done(self): """Called when the user clicks the submit button, if all forms are valid.""" @@ -193,7 +194,9 @@ class ApplicationWizard(LoginRequiredMixin, TemplateView): # if user visited via an "edit" url, associate the id of the # application they are trying to edit to this wizard instance + # and remove any prior wizard data from their session if current_url == self.EDIT_URL_NAME and "id" in kwargs: + del self.storage self.storage["application_id"] = kwargs["id"] # if accessing this class directly, redirect to the first step @@ -380,7 +383,7 @@ class CurrentSites(ApplicationWizard): class DotgovDomain(ApplicationWizard): template_name = "application_dotgov_domain.html" - forms = [forms.DotGovDomainForm] + forms = [forms.DotGovDomainForm, forms.AlternativeDomainFormSet] class Purpose(ApplicationWizard): @@ -395,7 +398,7 @@ class YourContact(ApplicationWizard): class OtherContacts(ApplicationWizard): template_name = "application_other_contacts.html" - forms = [forms.OtherContactsForm] + forms = [forms.OtherContactsFormSet] class SecurityEmail(ApplicationWizard): From f6c70f88a9fe9ee9a4d8b3fc15fdf5a9edd882b4 Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Fri, 13 Jan 2023 14:43:13 -0500 Subject: [PATCH 2/4] Add more code comments Co-authored-by: Neil MartinsenBurrell --- src/registrar/forms/application_wizard.py | 2 +- src/registrar/templates/application_other_contacts.html | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 69e51697f..625b2179d 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -374,7 +374,7 @@ class BaseAlternativeDomainFormSet(RegistrarFormSet): cleaned = post_data.cleaned_data if post_data is not None else {} domain = cleaned.get("alternative_domain", None) - # matching database object exists, update it + # matching database object exists, update or delete it if db_obj is not None and isinstance(domain, str): entry_was_erased = domain.strip() == "" if entry_was_erased: diff --git a/src/registrar/templates/application_other_contacts.html b/src/registrar/templates/application_other_contacts.html index cbc3887a2..1b3ec65f4 100644 --- a/src/registrar/templates/application_other_contacts.html +++ b/src/registrar/templates/application_other_contacts.html @@ -11,6 +11,7 @@ {% csrf_token %} {{ forms.0.management_form }} + {# forms.0 is a formset and this iterates over its forms #} {% for form in forms.0.forms %}
From 0f87d9ea9aa3d31ebf48d8bedf037292de769072 Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Tue, 17 Jan 2023 09:52:23 -0600 Subject: [PATCH 3/4] Respond to PR feedback --- src/api/views.py | 5 +- src/registrar/fixtures.py | 46 +-- src/registrar/forms/application_wizard.py | 315 ++++++++++-------- src/registrar/models/domain.py | 79 ++--- .../templates/application_anything_else.html | 3 +- .../application_authorizing_official.html | 3 +- .../templates/application_current_sites.html | 20 +- .../templates/application_dotgov_domain.html | 78 +---- src/registrar/templates/application_form.html | 36 +- .../templates/application_org_contact.html | 2 +- .../templates/application_org_election.html | 1 - .../templates/application_org_federal.html | 1 - .../templates/application_org_type.html | 1 - .../templates/application_other_contacts.html | 4 +- .../templates/application_purpose.html | 11 +- .../templates/application_requirements.html | 1 - .../templates/application_review.html | 3 +- .../templates/application_security_email.html | 3 +- .../templates/application_sidebar.html | 2 +- .../templates/application_type_of_work.html | 9 +- .../templates/application_your_contact.html | 3 +- src/registrar/templates/home.html | 1 - .../templates/includes/form_errors.html | 18 + .../templates/includes/input_with_errors.html | 8 + src/registrar/templates/whoami.html | 1 - .../templatetags/dynamic_question_tags.py | 6 + src/registrar/templatetags/field_helpers.py | 10 +- src/registrar/tests/test_forms.py | 22 +- src/registrar/tests/test_views.py | 43 ++- src/registrar/utility/errors.py | 10 + src/registrar/views/application.py | 2 +- 31 files changed, 392 insertions(+), 355 deletions(-) create mode 100644 src/registrar/templates/includes/form_errors.html create mode 100644 src/registrar/utility/errors.py diff --git a/src/api/views.py b/src/api/views.py index ab9a151d6..dcda09c69 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -1,6 +1,6 @@ """Internal API views""" - +from django.apps import apps from django.core.exceptions import BadRequest from django.views.decorators.http import require_http_methods from django.http import JsonResponse @@ -11,7 +11,6 @@ import requests from cachetools.func import ttl_cache -from registrar.models import Domain DOMAIN_FILE_URL = ( "https://raw.githubusercontent.com/cisagov/dotgov-data/main/current-full.csv" @@ -27,6 +26,7 @@ def _domains(): Fetch a file from DOMAIN_FILE_URL, parse the CSV for the domain, lowercase everything and return the list. """ + Domain = apps.get_model("registrar.Domain") # 5 second timeout file_contents = requests.get(DOMAIN_FILE_URL, timeout=5).text domains = set() @@ -65,6 +65,7 @@ def available(request, domain=""): Response is a JSON dictionary with the key "available" and value true or false. """ + Domain = apps.get_model("registrar.Domain") # validate that the given domain could be a domain name and fail early if # not. if not ( diff --git a/src/registrar/fixtures.py b/src/registrar/fixtures.py index 5b69e24f9..4c66bcd2e 100644 --- a/src/registrar/fixtures.py +++ b/src/registrar/fixtures.py @@ -75,34 +75,34 @@ class DomainApplicationFixture: # any fields not specified here will be filled in with fake data or defaults # NOTE BENE: each fixture must have `organization_name` for uniqueness! + # Here is a more complete example as a template: + # { + # "status": "started", + # "organization_name": "Example - Just started", + # "organization_type": "federal", + # "federal_agency": None, + # "federal_type": None, + # "address_line1": None, + # "address_line2": None, + # "city": None, + # "state_territory": None, + # "zipcode": None, + # "urbanization": None, + # "purpose": None, + # "security_email": None, + # "anything_else": None, + # "is_policy_acknowledged": None, + # "authorizing_official": None, + # "submitter": None, + # "other_contacts": [], + # "current_websites": [], + # "alternative_domains": [], + # }, DA = [ { "status": "started", "organization_name": "Example - Finished but not Submitted", }, - # an example of a more manual application - # { - # "status": "started", - # "organization_name": "Example - Just started", - # "organization_type": "federal", - # "federal_agency": None, - # "federal_type": None, - # "address_line1": None, - # "address_line2": None, - # "city": None, - # "state_territory": None, - # "zipcode": None, - # "urbanization": None, - # "purpose": None, - # "security_email": None, - # "anything_else": None, - # "is_policy_acknowledged": None, - # "authorizing_official": None, - # "submitter": None, - # "other_contacts": [], - # "current_websites": [], - # "alternative_domains": [], - # }, { "status": "submitted", "organization_name": "Example - Submitted but pending Investigation", diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 625b2179d..34950d719 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -1,6 +1,7 @@ from __future__ import annotations # allows forward references in annotations from itertools import zip_longest import logging +from typing import Callable from phonenumber_field.formfields import PhoneNumberField # type: ignore from django import forms @@ -8,6 +9,7 @@ from django.core.validators import RegexValidator from django.utils.safestring import mark_safe from registrar.models import Contact, DomainApplication, Domain +from registrar.utility import errors logger = logging.getLogger(__name__) @@ -69,6 +71,83 @@ class RegistrarFormSet(forms.BaseFormSet): self.application = kwargs.pop("application", None) super(RegistrarFormSet, self).__init__(*args, **kwargs) + def should_delete(self, cleaned): + """Should this entry be deleted from the database?""" + raise NotImplementedError + + def pre_update(self, db_obj, cleaned): + """Code to run before an item in the formset is saved.""" + for key, value in cleaned.items(): + setattr(db_obj, key, value) + + def pre_create(self, db_obj, cleaned): + """Code to run before an item in the formset is created in the database.""" + return cleaned + + def to_database(self, obj: DomainApplication): + """ + Adds this form's cleaned data to `obj` and saves `obj`. + + Does nothing if form is not valid. + + Hint: Subclass should call `self._to_database(...)`. + """ + raise NotImplementedError + + def _to_database( + self, + obj: DomainApplication, + join: str, + should_delete: Callable, + pre_update: Callable, + pre_create: Callable, + ): + """ + Performs the actual work of saving. + + Has hooks such as `should_delete` and `pre_update` by which the + subclass can control behavior. Add more hooks whenever needed. + """ + if not self.is_valid(): + return + obj.save() + + query = getattr(obj, join).order_by("created_at").all() # order matters + + # the use of `zip` pairs the forms in the formset with the + # related objects gotten from the database -- there should always be + # at least as many forms as database entries: extra forms means new + # entries, but fewer forms is _not_ the correct way to delete items + # (likely a client-side error or an attempt at data tampering) + + for db_obj, post_data in zip_longest(query, self.forms, fillvalue=None): + + cleaned = post_data.cleaned_data if post_data is not None else {} + + # matching database object exists, update it + if db_obj is not None and cleaned: + if should_delete(cleaned): + db_obj.delete() + continue + else: + pre_update(db_obj, cleaned) + db_obj.save() + + # no matching database object, create it + elif db_obj is None and cleaned: + kwargs = pre_create(db_obj, cleaned) + getattr(obj, join).create(**kwargs) + + @classmethod + def on_fetch(cls, query): + """Code to run when fetching formset's objects from the database.""" + return query.values() + + @classmethod + def from_database(cls, obj: DomainApplication, join: str, on_fetch: Callable): + """Returns a dict of form field values gotten from `obj`.""" + return on_fetch(getattr(obj, join).order_by("created_at")) # order matters + class OrganizationTypeForm(RegistrarForm): organization_type = forms.ChoiceField( @@ -299,53 +378,35 @@ class AuthorizingOfficialForm(RegistrarForm): class CurrentSitesForm(RegistrarForm): - def to_database(self, obj): - if not self.is_valid(): - return - obj.save() - normalized = Domain.normalize(self.cleaned_data["current_site"], blank=True) - if normalized: - # TODO: ability to update existing records - obj.current_websites.create(website=normalized) + website = forms.URLField( + required=False, + label="Public website", + ) + + +class BaseCurrentSitesFormSet(RegistrarFormSet): + JOIN = "current_websites" + + def should_delete(self, cleaned): + website = cleaned.get("website", "") + return website.strip() == "" + + def to_database(self, obj: DomainApplication): + self._to_database( + obj, self.JOIN, self.should_delete, self.pre_update, self.pre_create + ) @classmethod def from_database(cls, obj): - current_website = obj.current_websites.first() - if current_website is not None: - return {"current_site": current_website.website} - else: - return {} + return super().from_database(obj, cls.JOIN, cls.on_fetch) - current_site = forms.CharField( - required=False, - label=( - "Enter your organization’s website in the required format, like" - " www.city.com." - ), - ) - def clean_current_site(self): - """This field should be a legal domain name.""" - inputted_site = self.cleaned_data["current_site"] - if not inputted_site: - # empty string is fine - return inputted_site - - # something has been inputted - - if inputted_site.startswith("http://") or inputted_site.startswith("https://"): - # strip of the protocol that the pasted from their web browser - inputted_site = inputted_site.split("//", 1)[1] - - if Domain.string_could_be_domain(inputted_site): - return inputted_site - else: - # string could not be a domain - raise forms.ValidationError( - "Enter your organization’s website in the required format, like" - " www.city.com.", - code="invalid", - ) +CurrentSitesFormSet = forms.formset_factory( + CurrentSitesForm, + extra=1, + absolute_max=1500, # django default; use `max_num` to limit entries + formset=BaseCurrentSitesFormSet, +) class AlternativeDomainForm(RegistrarForm): @@ -354,59 +415,67 @@ class AlternativeDomainForm(RegistrarForm): label="Alternative domain", ) + def clean_alternative_domain(self): + """Validation code for domain names.""" + try: + requested = self.cleaned_data.get("alternative_domain", None) + validated = Domain.validate(requested, blank_ok=True) + except errors.ExtraDotsError: + raise forms.ValidationError( + "Please enter a domain without any periods.", + code="invalid", + ) + except errors.DomainUnavailableError: + raise forms.ValidationError( + "ERROR MESSAGE GOES HERE", + code="invalid", + ) + except ValueError: + raise forms.ValidationError( + "Please enter a valid domain name using only letters, " + "numbers, and hyphens", + code="invalid", + ) + return validated + class BaseAlternativeDomainFormSet(RegistrarFormSet): + JOIN = "alternative_domains" + + def should_delete(self, cleaned): + domain = cleaned.get("alternative_domain", "") + return domain.strip() == "" + + def pre_update(self, db_obj, cleaned): + domain = cleaned.get("alternative_domain", None) + if domain is not None: + db_obj.website = f"{domain}.gov" + + def pre_create(self, db_obj, cleaned): + domain = cleaned.get("alternative_domain", None) + if domain is not None: + return {"website": f"{domain}.gov"} + else: + return {} + def to_database(self, obj: DomainApplication): - if not self.is_valid(): - return + self._to_database( + obj, self.JOIN, self.should_delete, self.pre_update, self.pre_create + ) - obj.save() - query = obj.alternative_domains.order_by("created_at").all() # order matters - - # the use of `zip` pairs the forms in the formset with the - # related objects gotten from the database -- there should always be - # at least as many forms as database entries: extra forms means new - # entries, but fewer forms is _not_ the correct way to delete items - # (likely a client-side error or an attempt at data tampering) - - for db_obj, post_data in zip_longest(query, self.forms, fillvalue=None): - - cleaned = post_data.cleaned_data if post_data is not None else {} - domain = cleaned.get("alternative_domain", None) - - # matching database object exists, update or delete it - if db_obj is not None and isinstance(domain, str): - entry_was_erased = domain.strip() == "" - if entry_was_erased: - db_obj.delete() - continue - try: - normalized = Domain.normalize(domain, "gov", blank=True) - except ValueError as e: - logger.debug(e) - continue - db_obj.website = normalized - db_obj.save() - - # no matching database object, create it - elif db_obj is None and domain is not None: - try: - normalized = Domain.normalize(domain, "gov", blank=True) - except ValueError as e: - logger.debug(e) - continue - obj.alternative_domains.create(website=normalized) + @classmethod + def on_fetch(cls, query): + return [{"alternative_domain": domain.sld} for domain in query] @classmethod def from_database(cls, obj): - query = obj.alternative_domains.order_by("created_at").all() # order matters - return [{"alternative_domain": domain.sld} for domain in query] + return super().from_database(obj, cls.JOIN, cls.on_fetch) AlternativeDomainFormSet = forms.formset_factory( AlternativeDomainForm, extra=1, - absolute_max=1500, + absolute_max=1500, # django default; use `max_num` to limit entries formset=BaseAlternativeDomainFormSet, ) @@ -415,16 +484,14 @@ class DotGovDomainForm(RegistrarForm): def to_database(self, obj): if not self.is_valid(): return - normalized = Domain.normalize( - self.cleaned_data["requested_domain"], "gov", blank=True - ) - if normalized: + domain = self.cleaned_data.get("requested_domain", None) + if domain: requested_domain = getattr(obj, "requested_domain", None) if requested_domain is not None: - requested_domain.name = normalized + requested_domain.name = f"{domain}.gov" requested_domain.save() else: - requested_domain = Domain.objects.create(name=normalized) + requested_domain = Domain.objects.create(name=f"{domain}.gov") obj.requested_domain = requested_domain obj.save() @@ -438,16 +505,12 @@ class DotGovDomainForm(RegistrarForm): values["requested_domain"] = requested_domain.sld return values - requested_domain = forms.CharField(label="What .gov domain do you want?") - def clean_requested_domain(self): - """Requested domains need to be legal top-level domains, not subdomains. - - If they end with `.gov`, then we can reasonably take that off. If they have - any other dots in them, raise an error. - """ - requested = self.cleaned_data["requested_domain"] - if not requested: + """Validation code for domain names.""" + try: + requested = self.cleaned_data.get("requested_domain", None) + validated = Domain.validate(requested) + except errors.BlankValueError: # none or empty string raise forms.ValidationError( "Enter the .gov domain you want. Don’t include “www” or “.gov.” For" @@ -455,20 +518,25 @@ class DotGovDomainForm(RegistrarForm): " the quotes).", code="invalid", ) - if requested.endswith(".gov"): - requested = requested[:-4] - if "." in requested: + except errors.ExtraDotsError: raise forms.ValidationError( "Enter the .gov domain you want without any periods.", code="invalid", ) - if not Domain.string_could_be_domain(requested + ".gov"): + except errors.DomainUnavailableError: + raise forms.ValidationError( + "ERROR MESSAGE GOES HERE", + code="invalid", + ) + except ValueError: raise forms.ValidationError( "Enter a domain using only letters, " "numbers, or hyphens (though we don't recommend using hyphens).", code="invalid", ) - return requested + return validated + + requested_domain = forms.CharField(label="What .gov domain do you want?") class PurposeForm(RegistrarForm): @@ -595,47 +663,26 @@ class OtherContactsForm(RegistrarForm): class BaseOtherContactsFormSet(RegistrarFormSet): - def to_database(self, obj): - if not self.is_valid(): - return - obj.save() + JOIN = "other_contacts" - query = obj.other_contacts.order_by("created_at").all() + def should_delete(self, cleaned): + empty = (isinstance(v, str) and not v.strip() for v in cleaned.values()) + return all(empty) - # the use of `zip` pairs the forms in the formset with the - # related objects gotten from the database -- there should always be - # at least as many forms as database entries: extra forms means new - # entries, but fewer forms is _not_ the correct way to delete items - # (likely a client-side error or an attempt at data tampering) - - for db_obj, post_data in zip_longest(query, self.forms, fillvalue=None): - - cleaned = post_data.cleaned_data if post_data is not None else {} - - # matching database object exists, update it - if db_obj is not None and cleaned: - empty = (isinstance(v, str) and not v.strip() for v in cleaned.values()) - erased = all(empty) - if erased: - db_obj.delete() - continue - for key, value in cleaned.items(): - setattr(db_obj, key, value) - db_obj.save() - - # no matching database object, create it - elif db_obj is None and cleaned: - obj.other_contacts.create(**cleaned) + def to_database(self, obj: DomainApplication): + self._to_database( + obj, self.JOIN, self.should_delete, self.pre_update, self.pre_create + ) @classmethod def from_database(cls, obj): - return obj.other_contacts.order_by("created_at").values() # order matters + return super().from_database(obj, cls.JOIN, cls.on_fetch) OtherContactsFormSet = forms.formset_factory( OtherContactsForm, extra=1, - absolute_max=1500, + absolute_max=1500, # django default; use `max_num` to limit entries formset=BaseOtherContactsFormSet, ) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 29126c98f..e05f2fda4 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -6,7 +6,9 @@ from django.core.exceptions import ValidationError from django.db import models from django_fsm import FSMField, transition # type: ignore +from api.views import in_domains from epp.mock_epp import domain_info, domain_check +from registrar.utility import errors from .utility.time_stamped_model import TimeStampedModel @@ -92,60 +94,6 @@ class Domain(TimeStampedModel): # begin or end with a hyphen, followed by a TLD of 2-6 alphabetic characters DOMAIN_REGEX = re.compile(r"^(?!-)[A-Za-z0-9-]{1,63}(? str: # noqa: C901 - """Return `domain` in form `.`. - - Raises ValueError if string cannot be normalized. - - This does not guarantee the returned string is a valid domain name. - - Set `blank` to True to allow empty strings. - """ - if blank and len(domain.strip()) == 0: - return "" - cleaned = domain.lower() - # starts with https or http - if cleaned.startswith("https://"): - cleaned = cleaned[8:] - if cleaned.startswith("http://"): - cleaned = cleaned[7:] - # has url parts - if "/" in cleaned: - cleaned = cleaned.split("/")[0] - # has query parts - if "?" in cleaned: - cleaned = cleaned.split("?")[0] - # has fragments - if "#" in cleaned: - cleaned = cleaned.split("#")[0] - # replace disallowed chars - re.sub(r"^[^A-Za-z0-9.-]+", "", cleaned) - - parts = cleaned.split(".") - # has subdomains or invalid repetitions - if cleaned.count(".") > 0: - # remove invalid repetitions - while parts[-1] == parts[-2]: - parts.pop() - # remove subdomains - parts = parts[-2:] - hasTLD = len(parts) == 2 - if hasTLD: - # set correct tld - if tld is not None: - parts[-1] = tld - else: - # add tld - if tld is not None: - parts.append(tld) - else: - raise ValueError("You must specify a tld for %s" % domain) - - cleaned = ".".join(parts) - - return cleaned - @classmethod def string_could_be_domain(cls, domain: str | None) -> bool: """Return True if the string could be a domain name, otherwise False.""" @@ -153,6 +101,29 @@ class Domain(TimeStampedModel): return False return bool(cls.DOMAIN_REGEX.match(domain)) + @classmethod + def validate(cls, domain: str | None, blank_ok=False) -> str: + """Attempt to determine if a domain name could be requested.""" + if domain is None: + raise errors.BlankValueError() + if not isinstance(domain, str): + raise ValueError("Domain name must be a string") + domain = domain.lower().strip() + if domain == "": + if blank_ok: + return domain + else: + raise errors.BlankValueError() + if domain.endswith(".gov"): + domain = domain[:-4] + if "." in domain: + raise errors.ExtraDotsError() + if not Domain.string_could_be_domain(domain + ".gov"): + raise ValueError() + if in_domains(domain): + raise errors.DomainUnavailableError() + return domain + @classmethod def available(cls, domain: str) -> bool: """Check if a domain is available. diff --git a/src/registrar/templates/application_anything_else.html b/src/registrar/templates/application_anything_else.html index 1394ea093..3893101d6 100644 --- a/src/registrar/templates/application_anything_else.html +++ b/src/registrar/templates/application_anything_else.html @@ -1,4 +1,3 @@ - {% extends 'application_form.html' %} {% load widget_tweaks %} @@ -6,7 +5,7 @@

Is there anything else we should know about your domain request?

- +
{% csrf_token %} diff --git a/src/registrar/templates/application_authorizing_official.html b/src/registrar/templates/application_authorizing_official.html index f5c18b2ce..79badd433 100644 --- a/src/registrar/templates/application_authorizing_official.html +++ b/src/registrar/templates/application_authorizing_official.html @@ -1,4 +1,3 @@ - {% extends 'application_form.html' %} {% load widget_tweaks %} {% load static %} @@ -43,7 +42,7 @@ {% include "includes/required_fields.html" %} - + {% csrf_token %}
diff --git a/src/registrar/templates/application_current_sites.html b/src/registrar/templates/application_current_sites.html index c72a7391f..2d858353f 100644 --- a/src/registrar/templates/application_current_sites.html +++ b/src/registrar/templates/application_current_sites.html @@ -1,14 +1,28 @@ - {% extends 'application_form.html' %} {% load widget_tweaks field_helpers %} {% load static %} {% block form_content %} - + {% csrf_token %} +
+ {{ forms.0.management_form }} + {# TODO: aria-describedby to associate these instructions with the input! #} +

+ Enter your organization’s public website, if you have one. For example, www.city.com. +

+ {% for form in forms.0 %} + {% input_with_errors form.website %} + {% endfor %} - {% input_with_errors forms.0.current_site %} + +
+
{{ block.super }} diff --git a/src/registrar/templates/application_dotgov_domain.html b/src/registrar/templates/application_dotgov_domain.html index 744b525f7..607bee459 100644 --- a/src/registrar/templates/application_dotgov_domain.html +++ b/src/registrar/templates/application_dotgov_domain.html @@ -1,8 +1,9 @@ {% extends 'application_form.html' %} -{% load widget_tweaks static%} +{% load widget_tweaks field_helpers static %} {% block form_content %} -

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

+

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

  • Be available
  • Be unique
  • @@ -11,83 +12,38 @@

-

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

+

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

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

Here are a few domain examples for your type of organization.

-
+
{% include "includes/domain_example__city.html" %}
+
-

What .gov domain do you want?

-

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

+

What .gov domain do you want?

+ {# TODO: aria-describedby to associate these instructions with the input! #} +

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

-

This question is required.

{% csrf_token %} - - {% if forms.0.requested_domain.errors %} -
- {% for error in forms.0.requested_domain.errors %} - - {{ error }} - - {% endfor %} -
- www. - {{ forms.0.requested_domain|add_class:"usa-input usa-input--error"|attr:"aria-describedby:domain_instructions"|attr:"aria-invalid:true" }} - .gov -
-
- {% else %} -
- www. - {{ forms.0.requested_domain|add_class:"usa-input"|attr:"aria-describedby:domain_instructions" }} - .gov -
- {% endif %} - + {% input_with_errors forms.0.requested_domain www_gov=True %} +

Alternative domains

- {% if forms.0.alternative_domain.errors %} -
- {{ forms.0.alternative_domain|add_label_class:"usa-label usa-label--error" }} - {% for error in forms.0.alternative_domain.errors %} - - {{ error }} - - {% endfor %} -
- www. - { forms.0.alternative_domain|add_class:"usa-input usa-input--error"|attr:"aria-describedby:domain_instructions"|attr:"aria-invalid:true" }} - .gov -
-
- {% else %} - {{ forms.0.alternative_domain|add_label_class:"usa-label" }} -
- www. - {{ forms.0.alternative_domain|add_class:"usa-input"|attr:"aria-describedby:domain_instructions" }} - .gov -
- {% endif %} {{ forms.1.management_form }} -

Are there other domains you’d like if we can’t give you your first choice? Entering alternative domains is optional.

+ {# TODO: aria-describedby to associate these instructions with the input! #} +

Are there other domains you’d like if we can’t give you your first choice? Entering alternative domains is optional.

- {% for form in forms.1 %} - {{ form.alternative_domain|add_label_class:"usa-label" }} -
- www. - {{ form.alternative_domain|add_class:"usa-input"|attr:"aria-describedby:alt_domain_instructions" }} - .gov -
- {% endfor %} + {% for form in forms.1 %} + {% input_with_errors form.alternative_domain www_gov=True %} + {% endfor %} diff --git a/src/registrar/templates/application_form.html b/src/registrar/templates/application_form.html index c1ae605c6..f67efb071 100644 --- a/src/registrar/templates/application_form.html +++ b/src/registrar/templates/application_form.html @@ -1,5 +1,5 @@ {% extends 'base.html' %} -{% load static widget_tweaks namespaced_urls %} +{% load static widget_tweaks dynamic_question_tags namespaced_urls %} {% block title %}Apply for a .gov domain – {{form_titles|get_item:steps.current}}{% endblock %} {% block content %} @@ -12,30 +12,26 @@
{% if steps.prev %} - Previous step + Previous step {% endif %} - {% for form in forms %} - {% if form.errors %} - {% for error in form.non_field_errors %} -
-
- {{ error|escape }} -
-
- {% endfor %} - {% for field in form %} - {% for error in field.errors %} -
-
- {{ error|escape }} -
-
- {% endfor %} + {% comment %} + to make sense of this loop, consider that + a context variable of `forms` contains all + the forms for this page; each of these + may be itself a formset and contain additional + forms, hence `forms.forms` + {% endcomment %} + {% for outer in forms %} + {% if outer|isformset %} + {% for inner in outer.forms %} + {% include "includes/form_errors.html" with form=inner %} {% endfor %} + {% else %} + {% include "includes/form_errors.html" with form=outer %} {% endif %} {% endfor %} diff --git a/src/registrar/templates/application_org_contact.html b/src/registrar/templates/application_org_contact.html index 324ffb602..cf3090c20 100644 --- a/src/registrar/templates/application_org_contact.html +++ b/src/registrar/templates/application_org_contact.html @@ -1,4 +1,3 @@ - {% extends 'application_form.html' %} {% load widget_tweaks field_helpers %} @@ -16,6 +15,7 @@ {% include "includes/required_fields.html" %}
+ {% csrf_token %} diff --git a/src/registrar/templates/application_org_election.html b/src/registrar/templates/application_org_election.html index e92b0bc8a..1b7b92994 100644 --- a/src/registrar/templates/application_org_election.html +++ b/src/registrar/templates/application_org_election.html @@ -1,4 +1,3 @@ - {% extends 'application_form.html' %} {% load widget_tweaks %} {% load dynamic_question_tags %} diff --git a/src/registrar/templates/application_org_federal.html b/src/registrar/templates/application_org_federal.html index 2c54a1356..26d84a864 100644 --- a/src/registrar/templates/application_org_federal.html +++ b/src/registrar/templates/application_org_federal.html @@ -1,4 +1,3 @@ - {% extends 'application_form.html' %} {% load widget_tweaks %} {% load dynamic_question_tags %} diff --git a/src/registrar/templates/application_org_type.html b/src/registrar/templates/application_org_type.html index e20f0ed58..f74b4059d 100644 --- a/src/registrar/templates/application_org_type.html +++ b/src/registrar/templates/application_org_type.html @@ -1,4 +1,3 @@ - {% extends 'application_form.html' %} {% load widget_tweaks %} {% load dynamic_question_tags %} diff --git a/src/registrar/templates/application_other_contacts.html b/src/registrar/templates/application_other_contacts.html index 1b3ec65f4..834dd9805 100644 --- a/src/registrar/templates/application_other_contacts.html +++ b/src/registrar/templates/application_other_contacts.html @@ -8,7 +8,7 @@

We’d like to contact other employees with administrative or technical responsibilities in your organization. For example, they could be involved in managing your organization or its technical infrastructure. This information will help us assess your eligibility and understand the purpose of the .gov domain. These contacts should be in addition to you and your authorizing official.

{% include "includes/required_fields.html" %} - + {% csrf_token %} {{ forms.0.management_form }} {# forms.0 is a formset and this iterates over its forms #} @@ -28,7 +28,7 @@
diff --git a/src/registrar/templates/application_purpose.html b/src/registrar/templates/application_purpose.html index 2a6f922ef..c2ed0a8ba 100644 --- a/src/registrar/templates/application_purpose.html +++ b/src/registrar/templates/application_purpose.html @@ -1,14 +1,13 @@ - {% extends 'application_form.html' %} {% load widget_tweaks %} {% block form_content %} -

.Gov domain names are intended for use on the internet. They should be registered with an intent to deploy services, not simply to reserve a name. .Gov domains should not be registered for primarily internal use.

- -

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

- -

This question is required.

+
+

.Gov domain names are intended for use on the internet. They should be registered with an intent to deploy services, not simply to reserve a name. .Gov domains should not be registered for primarily internal use.

+

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

+

This question is required.

+
diff --git a/src/registrar/templates/application_requirements.html b/src/registrar/templates/application_requirements.html index d313b901d..b44ea2eee 100644 --- a/src/registrar/templates/application_requirements.html +++ b/src/registrar/templates/application_requirements.html @@ -1,4 +1,3 @@ - {% extends 'application_form.html' %} {% load widget_tweaks %} diff --git a/src/registrar/templates/application_review.html b/src/registrar/templates/application_review.html index 2c8bf874c..ffbd42c59 100644 --- a/src/registrar/templates/application_review.html +++ b/src/registrar/templates/application_review.html @@ -1,10 +1,9 @@ - {% extends 'application_form.html' %} {% load static widget_tweaks namespaced_urls %} {% block form_content %} - + {% csrf_token %} {% for step in steps.all|slice:":-1" %} diff --git a/src/registrar/templates/application_security_email.html b/src/registrar/templates/application_security_email.html index 36595ad38..f909a024b 100644 --- a/src/registrar/templates/application_security_email.html +++ b/src/registrar/templates/application_security_email.html @@ -1,4 +1,3 @@ - {% extends 'application_form.html' %} {% load widget_tweaks %} {% load static %} @@ -7,7 +6,7 @@

We strongly recommend that you provide a security email. This email will allow the public to report observed or suspected security issues on your domain. Security emails are made public. We recommend using an alias, like security@<domain.gov>.

- + {% csrf_token %} {% if forms.0.security_email.errors %} diff --git a/src/registrar/templates/application_sidebar.html b/src/registrar/templates/application_sidebar.html index 4224a3f6b..7e5417669 100644 --- a/src/registrar/templates/application_sidebar.html +++ b/src/registrar/templates/application_sidebar.html @@ -13,7 +13,7 @@ {% else %}
  • - {% endfor %} - {{ field|add_class:"usa-input--error usa-textarea usa-character-count__field"|attr:"aria-describedby:instructions"|attr:"maxlength=500"|attr:"aria-invalid:true" }} + {{ field|add_class:"usa-input--error usa-textarea usa-character-count__field"|attr:"maxlength=500"|attr:"aria-invalid:true" }}
  • {% else %} {{ field|add_label_class:"usa-label" }} - {{ field|add_class:"usa-textarea usa-character-count__field"|attr:"aria-describedby:instructions"|attr:"maxlength=500" }} + {{ field|add_class:"usa-textarea usa-character-count__field"|attr:"maxlength=500" }} {% endif %} {% endwith %} You can enter up to 500 characters @@ -42,11 +41,11 @@ {{ error }} {% endfor %} - {{ field|add_class:"usa-input--error usa-textarea usa-character-count__field"|attr:"aria-describedby:instructions"|attr:"maxlength=500"|attr:"aria-invalid:true" }} + {{ field|add_class:"usa-input--error usa-textarea usa-character-count__field"|attr:"maxlength=500"|attr:"aria-invalid:true" }}
    {% else %} {{ field|add_label_class:"usa-label" }} - {{ field|add_class:"usa-textarea usa-character-count__field"|attr:"aria-describedby:instructions"|attr:"maxlength=500" }} + {{ field|add_class:"usa-textarea usa-character-count__field"|attr:"maxlength=500" }} {% endif %} {% endwith %} You can enter up to 500 characters diff --git a/src/registrar/templates/application_your_contact.html b/src/registrar/templates/application_your_contact.html index b3cab07d0..6acec24d4 100644 --- a/src/registrar/templates/application_your_contact.html +++ b/src/registrar/templates/application_your_contact.html @@ -1,4 +1,3 @@ - {% extends 'application_form.html' %} {% load widget_tweaks %} {% load static %} @@ -16,7 +15,7 @@ {% include "includes/required_fields.html" %} - + {% csrf_token %}
    diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index 86e8c957f..9b2531733 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -1,4 +1,3 @@ - {% extends 'base.html' %} {% block title %} Hello {% endblock %} diff --git a/src/registrar/templates/includes/form_errors.html b/src/registrar/templates/includes/form_errors.html new file mode 100644 index 000000000..f5e9a8791 --- /dev/null +++ b/src/registrar/templates/includes/form_errors.html @@ -0,0 +1,18 @@ +{% if form.errors %} + {% for error in form.non_field_errors %} +
    +
    + {{ error|escape }} +
    +
    + {% endfor %} + {% for field in form %} + {% for error in field.errors %} +
    +
    + {{ error|escape }} +
    +
    + {% endfor %} + {% endfor %} +{% endif %} \ No newline at end of file diff --git a/src/registrar/templates/includes/input_with_errors.html b/src/registrar/templates/includes/input_with_errors.html index a1662c40b..4859c8617 100644 --- a/src/registrar/templates/includes/input_with_errors.html +++ b/src/registrar/templates/includes/input_with_errors.html @@ -21,9 +21,17 @@ error messages, if necessary.
    {% else %} {{ field|add_label_class:"usa-label" }} + {% if www_gov %} +
    + www. + {% endif %} {% if required %} {{ field|add_class:input_class|attr:"required" }} {% else %} {{ field|add_class:input_class }} {% endif %} + {% if www_gov %} + .gov +
    + {% endif %} {% endif %} diff --git a/src/registrar/templates/whoami.html b/src/registrar/templates/whoami.html index 12aa0a4fc..69c4d9672 100644 --- a/src/registrar/templates/whoami.html +++ b/src/registrar/templates/whoami.html @@ -1,4 +1,3 @@ - {% extends 'base.html' %} {% block title %} Hello {% endblock %} diff --git a/src/registrar/templatetags/dynamic_question_tags.py b/src/registrar/templatetags/dynamic_question_tags.py index 452f28b21..b6bb44136 100644 --- a/src/registrar/templatetags/dynamic_question_tags.py +++ b/src/registrar/templatetags/dynamic_question_tags.py @@ -1,9 +1,15 @@ from django import template +from django.forms import BaseFormSet from django.utils.html import format_html register = template.Library() +@register.filter +def isformset(value): + return isinstance(value, BaseFormSet) + + @register.simple_tag def radio_buttons_by_value(boundfield): """ diff --git a/src/registrar/templatetags/field_helpers.py b/src/registrar/templatetags/field_helpers.py index 789488e5b..ce3b6fed3 100644 --- a/src/registrar/templatetags/field_helpers.py +++ b/src/registrar/templatetags/field_helpers.py @@ -5,7 +5,7 @@ from django import template register = template.Library() -def _field_context(field, input_class, add_class, required=False): +def _field_context(field, input_class, add_class, *, required=False, www_gov=False): """Helper to construct template context. input_class is the CSS class to use on the input element, add_class @@ -17,17 +17,19 @@ def _field_context(field, input_class, add_class, required=False): context = {"field": field, "input_class": input_class} if required: context["required"] = True + if www_gov: + context["www_gov"] = True return context @register.inclusion_tag("includes/input_with_errors.html") -def input_with_errors(field, add_class=None): +def input_with_errors(field, add_class=None, www_gov=False): """Make an input field along with error handling. field is a form field instance. add_class is a string of additional classes (space separated) to add to "usa-input" on the field. """ - return _field_context(field, "usa-input", add_class) + return _field_context(field, "usa-input", add_class, www_gov=www_gov) @register.inclusion_tag("includes/input_with_errors.html") @@ -37,4 +39,4 @@ def select_with_errors(field, add_class=None, required=False): field is a form field instance. add_class is a string of additional classes (space separated) to add to "usa-select" on the field. """ - return _field_context(field, "usa-select", add_class, required) + return _field_context(field, "usa-select", add_class, required=required) diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index a48918b5c..2352714da 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -27,24 +27,18 @@ class TestFormValidation(TestCase): form = OrganizationContactForm(data={"zipcode": zipcode}) self.assertNotIn("zipcode", form.errors) - def test_current_site_invalid(self): - form = CurrentSitesForm(data={"current_site": "nah"}) - self.assertEqual( - form.errors["current_site"], - [ - "Enter your organization’s website in the required format, like" - " www.city.com." - ], - ) + def test_website_invalid(self): + form = CurrentSitesForm(data={"website": "nah"}) + self.assertEqual(form.errors["website"], ["Enter a valid URL."]) - def test_current_site_valid(self): - form = CurrentSitesForm(data={"current_site": "hyphens-rule.gov.uk"}) + def test_website_valid(self): + form = CurrentSitesForm(data={"website": "hyphens-rule.gov.uk"}) self.assertEqual(len(form.errors), 0) - def test_current_site_scheme_valid(self): - form = CurrentSitesForm(data={"current_site": "http://hyphens-rule.gov.uk"}) + def test_website_scheme_valid(self): + form = CurrentSitesForm(data={"website": "http://hyphens-rule.gov.uk"}) self.assertEqual(len(form.errors), 0) - form = CurrentSitesForm(data={"current_site": "https://hyphens-rule.gov.uk"}) + form = CurrentSitesForm(data={"website": "https://hyphens-rule.gov.uk"}) self.assertEqual(len(form.errors), 0) def test_requested_domain_valid(self): diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 247232896..1bcdcd7c1 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -256,7 +256,7 @@ class DomainApplicationTests(TestWithUser, WebTest): self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) current_sites_page = ao_result.follow() current_sites_form = current_sites_page.form - current_sites_form["current_sites-current_site"] = "www.city.com" + current_sites_form["current_sites-0-website"] = "www.city.com" # test saving the page self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) @@ -266,7 +266,8 @@ class DomainApplicationTests(TestWithUser, WebTest): # should see results in db application = DomainApplication.objects.get() # there's only one self.assertEquals( - application.current_websites.filter(website="city.com").count(), 1 + application.current_websites.filter(website="http://www.city.com").count(), + 1, ) # test next button @@ -742,12 +743,6 @@ class DomainApplicationTests(TestWithUser, WebTest): def test_application_ao_dynamic_text(self): type_page = self.app.get(reverse("application:")).follow() - # 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] - # ---- TYPE PAGE ---- type_form = type_page.form type_form["organization_type-organization_type"] = "federal" @@ -804,6 +799,38 @@ class DomainApplicationTests(TestWithUser, WebTest): ao_page = election_page.click(str(self.TITLES["authorizing_official"]), index=0) self.assertContains(ao_page, "Domain requests from cities") + def test_application_formsets(self): + """Users are able to add more than one of some fields.""" + current_sites_page = self.app.get(reverse("application:current_sites")) + # 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] + + # fill in the form field + current_sites_form = current_sites_page.form + self.assertIn("current_sites-0-website", current_sites_form.fields) + self.assertNotIn("current_sites-1-website", current_sites_form.fields) + current_sites_form["current_sites-0-website"] = "https://example.com" + + # click "Add another" + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + current_sites_result = current_sites_form.submit("submit_button", value="save") + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + current_sites_form = current_sites_result.follow().form + + # verify that there are two form fields + value = current_sites_form["current_sites-0-website"].value + self.assertEqual(value, "https://example.com") + self.assertIn("current_sites-1-website", current_sites_form.fields) + # and it is correctly referenced in the ManyToOne relationship + application = DomainApplication.objects.get() # there's only one + self.assertEquals( + application.current_websites.filter(website="https://example.com").count(), + 1, + ) + @skip("WIP") def test_application_edit_restore(self): """ diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py new file mode 100644 index 000000000..3b17a17c7 --- /dev/null +++ b/src/registrar/utility/errors.py @@ -0,0 +1,10 @@ +class BlankValueError(ValueError): + pass + + +class ExtraDotsError(ValueError): + pass + + +class DomainUnavailableError(ValueError): + pass diff --git a/src/registrar/views/application.py b/src/registrar/views/application.py index e210eb6e7..f46071861 100644 --- a/src/registrar/views/application.py +++ b/src/registrar/views/application.py @@ -378,7 +378,7 @@ class AuthorizingOfficial(ApplicationWizard): class CurrentSites(ApplicationWizard): template_name = "application_current_sites.html" - forms = [forms.CurrentSitesForm] + forms = [forms.CurrentSitesFormSet] class DotgovDomain(ApplicationWizard): From 6a7dbbab950c02ef42c59eaae4b10f4ede35d3ea Mon Sep 17 00:00:00 2001 From: Seamus Johnston Date: Tue, 24 Jan 2023 10:20:44 -0600 Subject: [PATCH 4/4] Fix rebase errors --- src/registrar/tests/test_views.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 1bcdcd7c1..165f26a49 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -743,6 +743,12 @@ class DomainApplicationTests(TestWithUser, WebTest): def test_application_ao_dynamic_text(self): type_page = self.app.get(reverse("application:")).follow() + # 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] + # ---- TYPE PAGE ---- type_form = type_page.form type_form["organization_type-organization_type"] = "federal"