From 51307d3b12bd1c8c27aeb5600e768d4573135c55 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 18 Oct 2023 15:36:41 -0400 Subject: [PATCH] some error validation, light refactor of ip checking in model --- src/registrar/forms/domain.py | 9 ++++- src/registrar/models/domain.py | 6 +-- .../templates/domain_nameservers.html | 6 +-- src/registrar/views/domain.py | 37 ++++++++++++++++--- 4 files changed, 45 insertions(+), 13 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index d14ed41ba..b93950df0 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -26,7 +26,14 @@ class DomainNameserverForm(forms.Form): server = forms.CharField(label="Name server", strip=True) - ip = forms.CharField(label="IP address", strip=True, required=False) + ip = forms.CharField( + label="IP address", + strip=True, + required=False, + validators=[ + # TODO in progress + ], + ) NameserverFormset = formset_factory( diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 80bb1ab30..0b9139277 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -313,14 +313,14 @@ class Domain(TimeStampedModel, DomainHelper): NameserverError (if exception hit) Returns: None""" - if self.isSubdomain(nameserver) and (ip is None or ip == []): + if self.isSubdomain(nameserver) and (ip is None or ip == [] or ip != []): raise NameserverError(code=nsErrorCodes.MISSING_IP, nameserver=nameserver) - elif not self.isSubdomain(nameserver) and (ip is not None and ip != []): + elif not self.isSubdomain(nameserver) and (ip is not None and ip != [] and ip != ['']): raise NameserverError( code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED, nameserver=nameserver, ip=ip ) - elif ip is not None and ip != []: + elif ip is not None and ip != [] and ip != ['']: for addr in ip: logger.info(f"ip address {addr}") if not self._valid_ip_addr(addr): diff --git a/src/registrar/templates/domain_nameservers.html b/src/registrar/templates/domain_nameservers.html index 37fa1eb85..fffd4b8c0 100644 --- a/src/registrar/templates/domain_nameservers.html +++ b/src/registrar/templates/domain_nameservers.html @@ -16,7 +16,7 @@ {% include "includes/required_fields.html" %} -
+ {% csrf_token %} {{ formset.management_form }} @@ -26,7 +26,7 @@
{% with sublabel_text="Example: ns"|concat:forloop.counter|concat:".example.com" %} {% if forloop.counter <= 2 %} - {% with attr_required=True %} + {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %} {% input_with_errors form.server %} {% endwith %} {% else %} @@ -35,7 +35,7 @@ {% endwith %}
- {% with sublabel_text="Example: ns"|concat:forloop.counter|concat:".example.com" %} + {% with sublabel_text="Example: ns"|concat:forloop.counter|concat:".example.com" add_group_class="usa-form-group--unstyled-error" %} {% input_with_errors form.ip %} {% endwith %}
diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index dbf65fe6b..f8e144f7f 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -23,6 +23,7 @@ from registrar.models import ( UserDomainRole, ) from registrar.models.public_contact import PublicContact +from registrar.utility.errors import NameserverError from ..forms import ( ContactForm, @@ -222,20 +223,44 @@ class DomainNameserversView(DomainPermissionView, FormMixin): nameservers = [] for form in formset: try: + ip_string = form.cleaned_data["ip"] + # Split the string into a list using a comma as the delimiter + ip_list = ip_string.split(',') + # Remove any leading or trailing whitespace from each IP in the list + # this will return [''] if no ips have been entered, which is taken + # into account in the model in checkHostIPCombo + ip_list = [ip.strip() for ip in ip_list] + as_tuple = ( form.cleaned_data["server"], - [form.cleaned_data["ip"]] + ip_list, ) nameservers.append(as_tuple) except KeyError: # no server information in this field, skip it pass domain = self.get_object() - domain.nameservers = nameservers - - messages.success( - self.request, "The name servers for this domain have been updated." - ) + + try: + domain.nameservers = nameservers + except NameserverError as Err: + # TODO: move into literal + messages.error(self.request, 'Whoops, Nameservers Error') + # messages.error(self.request, GENERIC_ERROR) + logger.error(f"Nameservers error: {Err}") + # TODO: registry is not throwing an error when no connection + # TODO: merge 1103 and use literals + except RegistryError as Err: + if Err.is_connection_error(): + messages.error(self.request, 'CANNOT_CONTACT_REGISTRY') + logger.error(f"Registry connection error: {Err}") + else: + messages.error(self.request, 'GENERIC_ERROR') + logger.error(f"Registry error: {Err}") + else: + messages.success( + self.request, "The name servers for this domain have been updated." + ) # superclass has the redirect return super().form_valid(formset)