diff --git a/src/registrar/assets/src/js/getgov/form-nameservers.js b/src/registrar/assets/src/js/getgov/form-nameservers.js index 79b0b9750..4761b4377 100644 --- a/src/registrar/assets/src/js/getgov/form-nameservers.js +++ b/src/registrar/assets/src/js/getgov/form-nameservers.js @@ -389,9 +389,13 @@ export class NameserverForm { } removeFormErrors() { + // form errors have div id of form-errors. there can be multiple divs + // with same id, which is not syntactically correct, but is the case, + // so need to do below recursively let formErrorDiv = document.getElementById("form-errors"); - if (formErrorDiv) { + while (formErrorDiv) { formErrorDiv.remove(); + formErrorDiv = document.getElementById("form-errors"); } } diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index bf0dcf41f..53bb8e2eb 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -65,7 +65,12 @@ class DomainNameserverForm(forms.Form): domain = forms.CharField(widget=forms.HiddenInput, required=False) - server = forms.CharField(label="Name server", strip=True) + server = forms.CharField( + label="Name server", + strip=True, + required=True, + error_messages={"required": "At least two name servers are required."}, + ) ip = forms.CharField( label="IP address (IPv4 or IPv6)", @@ -76,13 +81,6 @@ class DomainNameserverForm(forms.Form): def __init__(self, *args, **kwargs): super(DomainNameserverForm, self).__init__(*args, **kwargs) - # add custom error messages - self.fields["server"].error_messages.update( - { - "required": "At least two name servers are required.", - } - ) - def clean(self): # clean is called from clean_forms, which is called from is_valid # after clean_fields. it is used to determine form level errors. @@ -187,15 +185,19 @@ class BaseNameserverFormset(forms.BaseFormSet): Check for duplicate entries in the formset. """ - if any(self.errors): # Skip validation if individual forms already have errors - return + # Check if there are at least two valid servers. Server field is required on each + # form in the formset. Need at least two valid forms, and need to manipulate + # error messages in the form, either adding or removing as appropriate - # Check if there are at least two valid servers valid_forms = [] # Track forms with valid "server" values empty_forms = [] # Track forms where "server" is empty + # forms with other types of errors are not counted in these two lists + + error_message = "At least two name servers are required." for form in self.forms: - if not form.is_valid(): # Ensure the form is valid before accessing cleaned_data + # skip if the form is not valid for a reason other than server not supplied + if not form.is_valid() and list(form.errors.get("server", [])) != [error_message]: continue server = form.cleaned_data.get("server", "").strip() @@ -204,16 +206,27 @@ class BaseNameserverFormset(forms.BaseFormSet): else: # If server is missing, track the form empty_forms.append(form) - # If there are fewer than 2 valid nameserver forms, add an error + # If there are fewer than 2 valid nameserver forms, make sure that one and only one + # error message for 'two name servers' if len(valid_forms) < 2: - error_message = "At least two name servers are required." - - # Add the error to each form that attempted to submit a nameserver + done = False for form in empty_forms: - form.add_error("server", error_message) + if list(form.errors.get("server", [])) == [error_message]: + form.errors.pop("server") + if not done: + form.add_error("server", error_message) + done = True - # Also associate the error with the formset itself - raise ValidationError(error_message) + else: + # If there are, remove the "At least two name servers are required" error from each form + # This will allow for successful submissions when the first or second entries are blanked + # but there are enough entries total + for form in self.forms: + if form.errors.get("server") == [error_message]: + form.errors.pop("server") + + if any(self.errors): # Skip validation if individual forms already have errors + return data = [] duplicates = [] diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 37480c7dd..cafdf49b0 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1449,7 +1449,7 @@ class MockEppLib(TestCase): ) infoDomainThreeHosts = fakedEppObject( - "my-nameserver.gov", + "threenameserversdomain.gov", cr_date=make_aware(datetime(2023, 5, 25, 19, 45, 35)), contacts=[], hosts=[ @@ -1460,7 +1460,7 @@ class MockEppLib(TestCase): ) infoDomainFourHosts = fakedEppObject( - "fournameserversDomain.gov", + "fournameserversdomain.gov", cr_date=make_aware(datetime(2023, 5, 25, 19, 45, 35)), contacts=[], hosts=[ diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 45eff8a9f..7148faa16 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -86,7 +86,7 @@ class TestWithDomainPermissions(TestWithUser): # that inherit this setUp self.domain_dnssec_none, _ = Domain.objects.get_or_create(name="dnssec-none.gov") self.domain_with_three_nameservers, _ = Domain.objects.get_or_create(name="threenameserversdomain.gov") - self.domain_with_four_nameservers, _ = Domain.objects.get_or_create(name="fournameserversDomain.gov") + self.domain_with_four_nameservers, _ = Domain.objects.get_or_create(name="fournameserversdomain.gov") self.domain_information, _ = DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain) @@ -1492,7 +1492,7 @@ class TestDomainNameservers(TestDomainOverview, MockEppLib): Uses self.app WebTest because we need to interact with forms. """ # initial nameservers page has one server with two ips - nameservers_page = self.app.get(reverse("domain-dns-nameservers", kwargs={"pk": self.domain_no_nameserver.id})) + nameservers_page = self.app.get(reverse("domain-dns-nameservers", kwargs={"domain_pk": self.domain_no_nameserver.id})) session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) # attempt to submit the form with only one nameserver, should error @@ -1739,72 +1739,88 @@ class TestDomainNameservers(TestDomainOverview, MockEppLib): """ nameserver1 = "" - nameserver2 = "ns2.my-nameserver.gov" - nameserver3 = "ns3.my-nameserver.gov" + nameserver2 = "ns2.threenameserversdomain.gov" + nameserver3 = "ns3.threenameserversdomain.gov" valid_ip = "" - valid_ip_2 = "128.0.0.2" - valid_ip_3 = "128.0.0.3" + valid_ip_2 = "128.8.8.1" + valid_ip_3 = "128.8.8.2" nameservers_page = self.app.get( - reverse("domain-dns-nameservers", kwargs={"pk": self.domain_with_three_nameservers.id}) + reverse("domain-dns-nameservers", kwargs={"domain_pk": self.domain_with_three_nameservers.id}) ) session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - print(nameservers_page.text) - print("TOTAL_FORMS:", nameservers_page.form["form-TOTAL_FORMS"].value) - print("INITIAL_FORMS:", nameservers_page.form["form-INITIAL_FORMS"].value) - print("MIN_NUM_FORMS:", nameservers_page.form["form-MIN_NUM_FORMS"].value) - print("MAX_NUM_FORMS:", nameservers_page.form["form-MAX_NUM_FORMS"].value) - print("Form fields:", nameservers_page.form.fields.keys()) - print("Deleted forms:", [key for key in nameservers_page.form.fields.keys() if "-DELETE" in key]) + + # webtest is not able to properly parse the form from nameservers_page, so manually + # inputting form data + form_data = { + "csrfmiddlewaretoken": nameservers_page.form["csrfmiddlewaretoken"].value, + "form-TOTAL_FORMS": "4", + "form-INITIAL_FORMS": "3", + "form-0-domain": "threenameserversdomain.gov", + "form-0-server": nameserver1, + "form-0-ip": valid_ip, + "form-1-domain": "threenameserversdomain.gov", + "form-1-server": nameserver2, + "form-1-ip": valid_ip_2, + "form-2-domain": "threenameserversdomain.gov", + "form-2-server": nameserver3, + "form-2-ip": valid_ip_3, + "form-3-domain": "threenameserversdomain.gov", + "form-3-server": "", + "form-3-ip": "", + } - nameservers_page.form["form-TOTAL_FORMS"] = "4" - nameservers_page.form["form-INITIAL_FORMS"] = "3" - nameservers_page.form["form-0-server"] = nameserver1 - # formset[0]['form-0-server'] = nameserver1 - # formset[0]['form-0-ip'] = valid_ip - # formset[1]['form-1-server'] = nameserver2 - # formset[1]['form-1-ip'] = valid_ip_2 - # formset[2]['form-2-server'] = nameserver3 - # formset[2]['form-2-ip'] = valid_ip_3 - nameservers_page.form["form-0-ip"] = valid_ip - nameservers_page.form.set("server", nameserver2, 1) - nameservers_page.form["form-1-ip"] = valid_ip_2 - nameservers_page.form["form-2-server"] = nameserver3 - nameservers_page.form["form-2-ip"] = valid_ip_3 - result = nameservers_page.form.submit() - # result = formset.submit() + result = self.app.post(reverse("domain-dns-nameservers", kwargs={"domain_pk": self.domain_with_three_nameservers.id}), form_data) # form submission was a successful post, response should be a 302 + self.assertEqual(result.status_code, 302) self.assertEqual( result["Location"], - reverse("domain-dns-nameservers", kwargs={"pk": self.domain_with_three_nameservers.id}), + reverse("domain-dns-nameservers", kwargs={"domain_pk": self.domain_with_three_nameservers.id}), ) + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) nameservers_page = result.follow() self.assertContains(nameservers_page, "The name servers for this domain have been updated") - nameserver1 = "ns1.my-nameserver.gov" + nameserver1 = "ns1.threenameserversdomain.gov" nameserver2 = "" - nameserver3 = "ns3.my-nameserver.gov" + nameserver3 = "ns3.threenameserversdomain.gov" valid_ip = "128.0.0.1" valid_ip_2 = "" valid_ip_3 = "128.0.0.3" session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - nameservers_page.form["form-0-server"] = nameserver1 - nameservers_page.form["form-0-ip"] = valid_ip - nameservers_page.form["form-1-server"] = nameserver2 - nameservers_page.form["form-1-ip"] = valid_ip_2 - nameservers_page.form["form-2-server"] = nameserver3 - nameservers_page.form["form-2-ip"] = valid_ip_3 - result = nameservers_page.form.submit() + + # webtest is not able to properly parse the form from nameservers_page, so manually + # inputting form data + form_data = { + "csrfmiddlewaretoken": nameservers_page.form["csrfmiddlewaretoken"].value, + "form-TOTAL_FORMS": "4", + "form-INITIAL_FORMS": "3", + "form-0-domain": "threenameserversdomain.gov", + "form-0-server": nameserver1, + "form-0-ip": valid_ip, + "form-1-domain": "threenameserversdomain.gov", + "form-1-server": nameserver2, + "form-1-ip": valid_ip_2, + "form-2-domain": "threenameserversdomain.gov", + "form-2-server": nameserver3, + "form-2-ip": valid_ip_3, + "form-3-domain": "threenameserversdomain.gov", + "form-3-server": "", + "form-3-ip": "", + } + + result = self.app.post(reverse("domain-dns-nameservers", kwargs={"domain_pk": self.domain_with_three_nameservers.id}), form_data) # form submission was a successful post, response should be a 302 self.assertEqual(result.status_code, 302) self.assertEqual( result["Location"], - reverse("domain-dns-nameservers", kwargs={"pk": self.domain_with_three_nameservers.id}), + reverse("domain-dns-nameservers", kwargs={"domain_pk": self.domain_with_three_nameservers.id}), ) self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) nameservers_page = result.follow() @@ -1834,20 +1850,28 @@ class TestDomainNameservers(TestDomainOverview, MockEppLib): session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + # webtest is not able to properly parse the form from nameservers_page, so manually + # inputting form data + form_data = { + "csrfmiddlewaretoken": nameservers_page.form["csrfmiddlewaretoken"].value, + "form-TOTAL_FORMS": "4", + "form-INITIAL_FORMS": "4", + "form-0-domain": "fournameserversdomain.gov", + "form-0-server": nameserver1, + "form-0-ip": valid_ip, + "form-1-domain": "fournameserversdomain.gov", + "form-1-server": nameserver2, + "form-1-ip": valid_ip_2, + "form-2-domain": "fournameserversdomain.gov", + "form-2-server": nameserver3, + "form-2-ip": valid_ip_3, + "form-3-domain": "fournameserversdomain.gov", + "form-3-server": nameserver4, + "form-3-ip": valid_ip_4, + } - # Minimal check to ensure the form is loaded correctly - self.assertEqual(nameservers_page.form["form-0-server"].value, "ns1.my-nameserver-1.com") - self.assertEqual(nameservers_page.form["form-3-server"].value, "ns1.explosive-chicken-nuggets.com") - - nameservers_page.form["form-0-server"] = nameserver1 - nameservers_page.form["form-0-ip"] = valid_ip - nameservers_page.form["form-1-server"] = nameserver2 - nameservers_page.form["form-1-ip"] = valid_ip_2 - nameservers_page.form["form-2-server"] = nameserver3 - nameservers_page.form["form-2-ip"] = valid_ip_3 - nameservers_page.form["form-3-server"] = nameserver4 - nameservers_page.form["form-3-ip"] = valid_ip_4 - result = nameservers_page.form.submit() + result = self.app.post(reverse("domain-dns-nameservers", kwargs={"domain_pk": self.domain_with_four_nameservers.id}), form_data) # form submission was a successful post, response should be a 302 self.assertEqual(result.status_code, 302) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 2ff334d34..3a083393e 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -889,7 +889,6 @@ class DomainNameserversView(DomainFormBaseView): """The initial value for the form (which is a formset here).""" nameservers = self.object.nameservers initial_data = [] - print(nameservers) if nameservers is not None: # Add existing nameservers as initial data initial_data.extend({"server": name, "ip": ",".join(ip)} for name, ip in nameservers) @@ -898,7 +897,6 @@ class DomainNameserversView(DomainFormBaseView): if len(initial_data) == 0: initial_data.append({}) - print(initial_data) return initial_data def get_success_url(self): @@ -917,13 +915,7 @@ class DomainNameserversView(DomainFormBaseView): formset = super().get_form(**kwargs) for i, form in enumerate(formset): - logger.debug(i) form.fields["server"].label += f" {i+1}" - if i < 2: - form.fields["server"].required = True - else: - form.fields["server"].required = False - form.fields["server"].label += " (optional)" form.fields["domain"].initial = self.object.name return formset