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):