Merge pull request #1200 from cisagov/dk/1016-nameservers-ui

Issue 1016 - Nameserver UI (includes issues 919 and 1104)

src/epplibwrapper/__init__.py

@@ -45,7 +45,7 @@ except NameError:
 # Attn: these imports should NOT be at the top of the file
 try:
     from .client import CLIENT, commands
-    from .errors import RegistryError, ErrorCode, CANNOT_CONTACT_REGISTRY, GENERIC_ERROR
+    from .errors import RegistryError, ErrorCode
     from epplib.models import common, info
     from epplib.responses import extensions
     from epplib import responses
@@ -61,6 +61,4 @@ __all__ = [
     "info",
     "ErrorCode",
     "RegistryError",
-    "CANNOT_CONTACT_REGISTRY",
-    "GENERIC_ERROR",
 ]

src/epplibwrapper/errors.py

@@ -1,8 +1,5 @@
 from enum import IntEnum
 
-CANNOT_CONTACT_REGISTRY = "Update failed. Cannot contact the registry."
-GENERIC_ERROR = "Value entered was wrong."
-
 
 class ErrorCode(IntEnum):
     """

src/registrar/assets/js/get-gov.js

@@ -231,41 +231,15 @@ function handleValidationClick(e) {
 
 
 /**
- * An IIFE that attaches a click handler for our dynamic nameservers form
- *
- * Only does something on a single page, but it should be fast enough to run
- * it everywhere.
+ * Prepare the namerservers and DS data forms delete buttons
+ * We will call this on the forms init, and also every time we add a form
+ * 
  */
-(function prepareNameserverForms() {
-  let serverForm = document.querySelectorAll(".server-form");
-  let container = document.querySelector("#form-container");
-  let addButton = document.querySelector("#add-nameserver-form");
-  let totalForms = document.querySelector("#id_form-TOTAL_FORMS");
-
-  let formNum = serverForm.length-1;
-  if (addButton)
-    addButton.addEventListener('click', addForm);
-
-  function addForm(e){
-      let newForm = serverForm[2].cloneNode(true);
-      let formNumberRegex = RegExp(`form-(\\d){1}-`,'g');
-      let formLabelRegex = RegExp(`Name server (\\d){1}`, 'g');
-      let formExampleRegex = RegExp(`ns(\\d){1}`, 'g');
-
-      formNum++;
-      newForm.innerHTML = newForm.innerHTML.replace(formNumberRegex, `form-${formNum}-`);
-      newForm.innerHTML = newForm.innerHTML.replace(formLabelRegex, `Name server ${formNum+1}`);
-      newForm.innerHTML = newForm.innerHTML.replace(formExampleRegex, `ns${formNum+1}`);
-      container.insertBefore(newForm, addButton);
-      newForm.querySelector("input").value = "";
-
-      totalForms.setAttribute('value', `${formNum+1}`);
-  }
-})();
-
-function prepareDeleteButtons() {
+function prepareDeleteButtons(formLabel) {
   let deleteButtons = document.querySelectorAll(".delete-record");
   let totalForms = document.querySelector("#id_form-TOTAL_FORMS");
+  let isNameserversForm = document.title.includes("DNS name servers |");
+  let addButton = document.querySelector("#add-form");
 
   // Loop through each delete button and attach the click event listener
   deleteButtons.forEach((deleteButton) => {
@@ -273,13 +247,15 @@ function prepareDeleteButtons() {
   });
 
   function removeForm(e){
-    let formToRemove = e.target.closest(".ds-record");
+    let formToRemove = e.target.closest(".repeatable-form");
     formToRemove.remove();
-    let forms = document.querySelectorAll(".ds-record");
+    let forms = document.querySelectorAll(".repeatable-form");
     totalForms.setAttribute('value', `${forms.length}`);
 
     let formNumberRegex = RegExp(`form-(\\d){1}-`, 'g');
-    let formLabelRegex = RegExp(`DS Data record (\\d){1}`, 'g');
+    let formLabelRegex = RegExp(`${formLabel} (\\d+){1}`, 'g');
+    // For the example on Nameservers
+    let formExampleRegex = RegExp(`ns(\\d+){1}`, 'g');
 
     forms.forEach((form, index) => {
       // Iterate over child nodes of the current element
@@ -294,48 +270,88 @@ function prepareDeleteButtons() {
         });
       });
 
-      Array.from(form.querySelectorAll('h2, legend')).forEach((node) => {
-        node.textContent = node.textContent.replace(formLabelRegex, `DS Data record ${index + 1}`);
+      // h2 and legend for DS form, label for nameservers  
+      Array.from(form.querySelectorAll('h2, legend, label, p')).forEach((node) => {
+        
+        // Ticket: 1192
+        // if (isNameserversForm && index <= 1 && !node.innerHTML.includes('*')) {
+        // // Create a new element
+        // const newElement = document.createElement('abbr');
+        // newElement.textContent = '*';
+        // // TODO: finish building abbr
+
+        // // Append the new element to the parent
+        // node.appendChild(newElement);
+        //   // Find the next sibling that is an input element
+        //   let nextInputElement = node.nextElementSibling;
+
+        //   while (nextInputElement) {
+        //     if (nextInputElement.tagName === 'INPUT') {
+        //       // Found the next input element
+        //       console.log(nextInputElement);
+        //       break;
+        //     }
+        //     nextInputElement = nextInputElement.nextElementSibling;
+        //   }
+        //   nextInputElement.required = true;
+        // }
+
+        // Ticket: 1192 - remove if
+        if (!(isNameserversForm && index <= 1)) {
+          node.textContent = node.textContent.replace(formLabelRegex, `${formLabel} ${index + 1}`);
+          node.textContent = node.textContent.replace(formExampleRegex, `ns${index + 1}`);
+        }
       });
+
+      // Display the add more button if we have less than 13 forms
+      if (isNameserversForm && forms.length <= 13) {
+        addButton.classList.remove("display-none")
+      }
     
     });
   }
 }
 
 /**
- * An IIFE that attaches a click handler for our dynamic DNSSEC forms
+ * An IIFE that attaches a click handler for our dynamic formsets
  *
+ * Only does something on a few pages, but it should be fast enough to run
+ * it everywhere.
  */
-(function prepareDNSSECForms() {
-  let serverForm = document.querySelectorAll(".ds-record");
+(function prepareFormsetsForms() {
+  let repeatableForm = document.querySelectorAll(".repeatable-form");
   let container = document.querySelector("#form-container");
-  let addButton = document.querySelector("#add-ds-form");
+  let addButton = document.querySelector("#add-form");
   let totalForms = document.querySelector("#id_form-TOTAL_FORMS");
+  let cloneIndex = 0;
+  let formLabel = '';
+  let isNameserversForm = document.title.includes("DNS name servers |");
+  if (isNameserversForm) {
+    cloneIndex = 2;
+    formLabel = "Name server";
+  } else if ((document.title.includes("DS Data |")) || (document.title.includes("Key Data |"))) {
+    formLabel = "DS Data record";
+  }
 
   // Attach click event listener on the delete buttons of the existing forms
-  prepareDeleteButtons();
+  prepareDeleteButtons(formLabel);
 
-  // Attack click event listener on the add button
   if (addButton)
     addButton.addEventListener('click', addForm);
 
-  /*
-   * Add a formset to the end of the form.
-   * For each element in the added formset, name the elements with the prefix,
-   * form-{#}-{element_name} where # is the index of the formset and element_name
-   * is the element's name.
-   * Additionally, update the form element's metadata, including totalForms' value.
-   */
   function addForm(e){
-      let forms = document.querySelectorAll(".ds-record");
+      let forms = document.querySelectorAll(".repeatable-form");
       let formNum = forms.length;
-      let newForm = serverForm[0].cloneNode(true);
+      let newForm = repeatableForm[cloneIndex].cloneNode(true);
       let formNumberRegex = RegExp(`form-(\\d){1}-`,'g');
-      let formLabelRegex = RegExp(`DS Data record (\\d){1}`, 'g');
+      let formLabelRegex = RegExp(`${formLabel} (\\d){1}`, 'g');
+      // For the eample on Nameservers
+      let formExampleRegex = RegExp(`ns(\\d){1}`, 'g');
 
       formNum++;
       newForm.innerHTML = newForm.innerHTML.replace(formNumberRegex, `form-${formNum-1}-`);
-      newForm.innerHTML = newForm.innerHTML.replace(formLabelRegex, `DS Data record ${formNum}`);
+      newForm.innerHTML = newForm.innerHTML.replace(formLabelRegex, `${formLabel} ${formNum}`);
+      newForm.innerHTML = newForm.innerHTML.replace(formExampleRegex, `ns${formNum}`);
       container.insertBefore(newForm, addButton);
 
       let inputs = newForm.querySelectorAll("input");
@@ -379,9 +395,13 @@ function prepareDeleteButtons() {
       totalForms.setAttribute('value', `${formNum}`);
 
       // Attach click event listener on the delete buttons of the new form
-      prepareDeleteButtons();
-  }
+      prepareDeleteButtons(formLabel);
 
+      // Hide the add more button if we have 13 forms
+      if (isNameserversForm && formNum == 13) {
+        addButton.classList.add("display-none")
+      }
+  }
 })();
 
 /**

src/registrar/assets/sass/_theme/_forms.scss

@@ -4,6 +4,10 @@
   margin-top: units(3);
 }
 
+.usa-form .usa-button.margin-bottom-075 {
+  margin-bottom: units(1.5);
+}
+
 .usa-form .usa-button.margin-top-1 {
   margin-top: units(1);
 }

src/registrar/forms/domain.py

@@ -5,8 +5,12 @@ from django.core.validators import MinValueValidator, MaxValueValidator, RegexVa
 from django.forms import formset_factory
 
 from phonenumber_field.widgets import RegionalPhoneNumberWidget
+from registrar.utility.errors import (
+    NameserverError,
+    NameserverErrorCodes as nsErrorCodes,
+)
 
-from ..models import Contact, DomainInformation
+from ..models import Contact, DomainInformation, Domain
 from .common import (
     ALGORITHM_CHOICES,
     DIGEST_TYPE_CHOICES,
@@ -19,16 +23,78 @@ class DomainAddUserForm(forms.Form):
     email = forms.EmailField(label="Email")
 
 
+class IPAddressField(forms.CharField):
+    def validate(self, value):
+        super().validate(value)  # Run the default CharField validation
+
+
 class DomainNameserverForm(forms.Form):
     """Form for changing nameservers."""
 
+    domain = forms.CharField(widget=forms.HiddenInput, required=False)
+
     server = forms.CharField(label="Name server", strip=True)
-    # when adding IPs to this form ensure they are stripped as well
+
+    ip = forms.CharField(label="IP Address (IPv4 or IPv6)", strip=True, required=False)
+
+    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.
+        # is_valid is typically called from view during a post
+        cleaned_data = super().clean()
+        self.clean_empty_strings(cleaned_data)
+        server = cleaned_data.get("server", "")
+        ip = cleaned_data.get("ip", None)
+        # remove ANY spaces in the ip field
+        ip = ip.replace(" ", "")
+        domain = cleaned_data.get("domain", "")
+
+        ip_list = self.extract_ip_list(ip)
+
+        if ip and not server and ip_list:
+            self.add_error("server", NameserverError(code=nsErrorCodes.MISSING_HOST))
+        elif server:
+            self.validate_nameserver_ip_combo(domain, server, ip_list)
+
+        return cleaned_data
+
+    def clean_empty_strings(self, cleaned_data):
+        ip = cleaned_data.get("ip", "")
+        if ip and len(ip.strip()) == 0:
+            cleaned_data["ip"] = None
+
+    def extract_ip_list(self, ip):
+        return [ip.strip() for ip in ip.split(",")] if ip else []
+
+    def validate_nameserver_ip_combo(self, domain, server, ip_list):
+        try:
+            Domain.checkHostIPCombo(domain, server, ip_list)
+        except NameserverError as e:
+            if e.code == nsErrorCodes.GLUE_RECORD_NOT_ALLOWED:
+                self.add_error(
+                    "server",
+                    NameserverError(
+                        code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED,
+                        nameserver=domain,
+                        ip=ip_list,
+                    ),
+                )
+            elif e.code == nsErrorCodes.MISSING_IP:
+                self.add_error(
+                    "ip",
+                    NameserverError(
+                        code=nsErrorCodes.MISSING_IP, nameserver=domain, ip=ip_list
+                    ),
+                )
+            else:
+                self.add_error("ip", str(e))
 
 
 NameserverFormset = formset_factory(
     DomainNameserverForm,
     extra=1,
+    max_num=13,
+    validate_max=True,
 )
 
 

src/registrar/models/domain.py

@@ -262,8 +262,11 @@ class Domain(TimeStampedModel, DomainHelper):
         """Creates the host object in the registry
         doesn't add the created host to the domain
         returns ErrorCode (int)"""
-        if addrs is not None:
-            addresses = [epp.Ip(addr=addr) for addr in addrs]
+        if addrs is not None and addrs != []:
+            addresses = [
+                epp.Ip(addr=addr, ip="v6" if self.is_ipv6(addr) else None)
+                for addr in addrs
+            ]
             request = commands.CreateHost(name=host, addrs=addresses)
         else:
             request = commands.CreateHost(name=host)
@@ -274,7 +277,7 @@ class Domain(TimeStampedModel, DomainHelper):
             return response.code
         except RegistryError as e:
             logger.error("Error _create_host, code was %s error was %s" % (e.code, e))
-            return e.code
+            raise e
 
     def _convert_list_to_dict(self, listToConvert: list[tuple[str, list]]):
         """converts a list of hosts into a dictionary
@@ -293,14 +296,16 @@ class Domain(TimeStampedModel, DomainHelper):
                 newDict[tup[0]] = tup[1]
         return newDict
 
-    def isSubdomain(self, nameserver: str):
+    @classmethod
+    def isSubdomain(cls, name: str, nameserver: str):
         """Returns boolean if the domain name is found in the argument passed"""
         subdomain_pattern = r"([\w-]+\.)*"
-        full_pattern = subdomain_pattern + self.name
+        full_pattern = subdomain_pattern + name
         regex = re.compile(full_pattern)
         return bool(regex.match(nameserver))
 
-    def checkHostIPCombo(self, nameserver: str, ip: list[str]):
+    @classmethod
+    def checkHostIPCombo(cls, name: str, nameserver: str, ip: list[str]):
         """Checks the parameters past for a valid combination
         raises error if:
             - nameserver is a subdomain but is missing ip
@@ -314,22 +319,23 @@ class Domain(TimeStampedModel, DomainHelper):
             NameserverError (if exception hit)
         Returns:
             None"""
-        if self.isSubdomain(nameserver) and (ip is None or ip == []):
+        if cls.isSubdomain(name, nameserver) and (ip is None or ip == []):
             raise NameserverError(code=nsErrorCodes.MISSING_IP, nameserver=nameserver)
 
-        elif not self.isSubdomain(nameserver) and (ip is not None and ip != []):
+        elif not cls.isSubdomain(name, nameserver) and (ip is not None and ip != []):
             raise NameserverError(
                 code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED, nameserver=nameserver, ip=ip
             )
         elif ip is not None and ip != []:
             for addr in ip:
-                if not self._valid_ip_addr(addr):
+                if not cls._valid_ip_addr(addr):
                     raise NameserverError(
                         code=nsErrorCodes.INVALID_IP, nameserver=nameserver, ip=ip
                     )
         return None
 
-    def _valid_ip_addr(self, ipToTest: str):
+    @classmethod
+    def _valid_ip_addr(cls, ipToTest: str):
         """returns boolean if valid ip address string
         We currently only accept v4 or v6 ips
         returns:
@@ -382,7 +388,9 @@ class Domain(TimeStampedModel, DomainHelper):
                 if newHostDict[prevHost] is not None and set(
                     newHostDict[prevHost]
                 ) != set(addrs):
-                    self.checkHostIPCombo(nameserver=prevHost, ip=newHostDict[prevHost])
+                    self.__class__.checkHostIPCombo(
+                        name=self.name, nameserver=prevHost, ip=newHostDict[prevHost]
+                    )
                     updated_values.append((prevHost, newHostDict[prevHost]))
 
         new_values = {
@@ -392,7 +400,9 @@ class Domain(TimeStampedModel, DomainHelper):
         }
 
         for nameserver, ip in new_values.items():
-            self.checkHostIPCombo(nameserver=nameserver, ip=ip)
+            self.__class__.checkHostIPCombo(
+                name=self.name, nameserver=nameserver, ip=ip
+            )
 
         return (deleted_values, updated_values, new_values, previousHostDict)
 
@@ -567,7 +577,11 @@ class Domain(TimeStampedModel, DomainHelper):
         if len(hosts) > 13:
             raise NameserverError(code=nsErrorCodes.TOO_MANY_HOSTS)
 
-        if self.state not in [self.State.DNS_NEEDED, self.State.READY]:
+        if self.state not in [
+            self.State.DNS_NEEDED,
+            self.State.READY,
+            self.State.UNKNOWN,
+        ]:
             raise ActionNotAllowed("Nameservers can not be " "set in the current state")
 
         logger.info("Setting nameservers")
@@ -1351,7 +1365,7 @@ class Domain(TimeStampedModel, DomainHelper):
 
     @transition(
         field="state",
-        source=[State.DNS_NEEDED],
+        source=[State.DNS_NEEDED, State.READY],
         target=State.READY,
         # conditions=[dns_not_needed]
     )
@@ -1514,7 +1528,7 @@ class Domain(TimeStampedModel, DomainHelper):
             data = registry.send(req, cleaned=True).res_data[0]
             host = {
                 "name": name,
-                "addrs": getattr(data, "addrs", ...),
+                "addrs": [item.addr for item in getattr(data, "addrs", [])],
                 "cr_date": getattr(data, "cr_date", ...),
                 "statuses": getattr(data, "statuses", ...),
                 "tr_date": getattr(data, "tr_date", ...),
@@ -1539,10 +1553,9 @@ class Domain(TimeStampedModel, DomainHelper):
             return []
 
         for ip_addr in ip_list:
-            if self.is_ipv6(ip_addr):
-                edited_ip_list.append(epp.Ip(addr=ip_addr, ip="v6"))
-            else:  # default ip addr is v4
-                edited_ip_list.append(epp.Ip(addr=ip_addr))
+            edited_ip_list.append(
+                epp.Ip(addr=ip_addr, ip="v6" if self.is_ipv6(ip_addr) else None)
+            )
 
         return edited_ip_list
 
@@ -1580,7 +1593,7 @@ class Domain(TimeStampedModel, DomainHelper):
             return response.code
         except RegistryError as e:
             logger.error("Error _update_host, code was %s error was %s" % (e.code, e))
-            return e.code
+            raise e
 
     def addAndRemoveHostsFromDomain(
         self, hostsToAdd: list[str], hostsToDelete: list[str]

src/registrar/templates/domain_dsdata.html

@@ -29,7 +29,7 @@
     {{ formset.management_form }}
 
     {% for form in formset %}
-    <fieldset class="ds-record">
+    <fieldset class="repeatable-form">
 
       <legend class="sr-only">DS Data record {{forloop.counter}}</legend>
 
@@ -74,7 +74,7 @@
     </fieldset>
     {% endfor %}
 
-    <button type="button" class="usa-button usa-button--unstyled display-block margin-bottom-2" id="add-ds-form">
+    <button type="button" class="usa-button usa-button--unstyled display-block margin-bottom-2" id="add-form">
       <svg class="usa-icon" aria-hidden="true" focusable="false" role="img" width="24" height="24">
         <use xlink:href="{%static 'img/sprite.svg'%}#add_circle"></use>
       </svg><span class="margin-left-05">Add new record</span>

src/registrar/templates/domain_nameservers.html

@@ -11,40 +11,79 @@
 
   <h1>DNS name servers</h1>
 
-  <p>Before your domain can be used we'll need information about your domain
-  name servers.</p>
+  <p>Before your domain can be used we’ll need information about your domain name servers. Name server records indicate which DNS server is authoritative for your domain.</p>
+
+  <p>Add a name server record by entering the address (e.g., ns1.nameserver.com) in the name server fields below. You must add at least two name servers (13 max).</p>
+
+  <div class="usa-alert usa-alert--slim usa-alert--info">
+    <div class="usa-alert__body">
+      <p class="margin-top-0">Add an IP address only when your name server's address includes your domain name (e.g., if your domain name is "example.gov" and your name server is "ns1.example.gov,” then an IP address is required.) To add multiple IP addresses, separate them with commas.</p>
+      <p class="margin-bottom-0">This step is uncommon unless you self-host your DNS or use custom addresses for your nameserver.</p>
+    </div>
+  </div>
 
   {% include "includes/required_fields.html" %}
 
-  <form class="usa-form usa-form--large" method="post" novalidate id="form-container">
+  <form class="usa-form usa-form--extra-large" method="post" novalidate id="form-container">
     {% csrf_token %}
     {{ formset.management_form }}
 
     {% for form in formset %}
-    <div class="server-form">
-    {% with sublabel_text="Example: ns"|concat:forloop.counter|concat:".example.com" %}
-      {% if forloop.counter <= 2 %}
-        {% with attr_required=True %}
-          {% input_with_errors form.server %}
-        {% endwith %}
-      {% else %}
-        {% input_with_errors form.server %}
-      {% endif %}
-    {% endwith %}
+    <div class="repeatable-form">
+      <div class="grid-row grid-gap-2 flex-end">
+        <div class="tablet:grid-col-5">
+          {{ form.domain }}
+          {% with sublabel_text="Example: ns"|concat:forloop.counter|concat:".example.com" %}
+            {% if forloop.counter <= 2 %}
+              {% with attr_required=True add_group_class="usa-form-group--unstyled-error" %}
+                {% input_with_errors form.server %}
+              {% endwith %}
+            {% else %}
+              {% input_with_errors form.server %}
+            {% endif %}
+          {% endwith %}
+        </div>
+        <div class="tablet:grid-col-5">
+          {% with sublabel_text="Example: 86.124.49.54 or 2001:db8::1234:5678" add_group_class="usa-form-group--unstyled-error" %}
+            {% input_with_errors form.ip %}
+          {% endwith %}
+        </div>
+        <div class="tablet:grid-col-2">
+          {% comment %} TODO: remove this if for 1192 {% endcomment %}
+          {% if forloop.counter > 2 %}
+            <button type="button" class="usa-button usa-button--unstyled display-block delete-record margin-bottom-075">
+              <svg class="usa-icon" aria-hidden="true" focusable="false" role="img" width="24" height="24">
+                <use xlink:href="{%static 'img/sprite.svg'%}#delete"></use>
+              </svg><span class="margin-left-05">Delete</span>
+            </button>
+          {% endif %}
+        </div>
+      </div>
     </div>
     {% endfor %}
 
-    <button type="button" class="usa-button usa-button--unstyled display-block" id="add-nameserver-form">
+    <button type="button" class="usa-button usa-button--unstyled display-block" id="add-form">
       <svg class="usa-icon" aria-hidden="true" focusable="false" role="img" width="24" height="24">
         <use xlink:href="{%static 'img/sprite.svg'%}#add_circle"></use>
       </svg><span class="margin-left-05">Add another name server</span>
     </button>
 
-    <button
-          type="submit"
-          class="usa-button"
-      >Save
-    </button>
-  </form>
+    {% comment %} Work around USWDS' button margins to add some spacing between the submit and the 'add more'
+    This solution still works when we remove the 'add more' at 13 forms {% endcomment %}
+    <div class="margin-top-2">
+      <button
+            type="submit"
+            class="usa-button"
+        >Save
+      </button>
 
+      <button
+            type="submit"
+            class="usa-button usa-button--outline"
+            name="btn-cancel-click"
+            aria-label="Reset the data in the Name Server form to the registry state (undo changes)"
+        >Cancel
+      </button>
+    </div>
+  </form>
 {% endblock %}  {# domain_content #}

src/registrar/tests/common.py

@@ -756,7 +756,7 @@ class MockEppLib(TestCase):
     mockDataInfoHosts = fakedEppObject(
         "lastPw",
         cr_date=datetime.datetime(2023, 8, 25, 19, 45, 35),
-        addrs=["1.2.3.4", "2.3.4.5"],
+        addrs=[common.Ip(addr="1.2.3.4"), common.Ip(addr="2.3.4.5")],
     )
 
     mockDataHostChange = fakedEppObject(
@@ -813,7 +813,7 @@ class MockEppLib(TestCase):
             "ns2.nameserverwithip.gov",
             "ns3.nameserverwithip.gov",
         ],
-        addrs=["1.2.3.4", "2.3.4.5"],
+        addrs=[common.Ip(addr="1.2.3.4"), common.Ip(addr="2.3.4.5")],
     )
 
     justNameserver = fakedEppObject(

src/registrar/tests/test_models_domain.py

@@ -107,7 +107,7 @@ class TestDomainCache(MockEppLib):
         }
         expectedHostsDict = {
             "name": self.mockDataInfoDomain.hosts[0],
-            "addrs": self.mockDataInfoHosts.addrs,
+            "addrs": [item.addr for item in self.mockDataInfoHosts.addrs],
             "cr_date": self.mockDataInfoHosts.cr_date,
         }
 

src/registrar/tests/test_nameserver_error.py

@@ -10,10 +10,7 @@ class TestNameserverError(TestCase):
     def test_with_no_ip(self):
         """Test NameserverError when no ip address is passed"""
         nameserver = "nameserver val"
-        expected = (
-            f"Nameserver {nameserver} needs to have an "
-            "IP address because it is a subdomain"
-        )
+        expected = "Using your domain for a name server requires an IP address"
 
         nsException = NameserverError(
             code=nsErrorCodes.MISSING_IP, nameserver=nameserver
@@ -38,7 +35,7 @@ class TestNameserverError(TestCase):
         ip = "ip val"
         nameserver = "nameserver val"
 
-        expected = f"Nameserver {nameserver} has an invalid IP address: {ip}"
+        expected = f"{nameserver}: Enter an IP address in the required format."
         nsException = NameserverError(
             code=nsErrorCodes.INVALID_IP, nameserver=nameserver, ip=ip
         )

src/registrar/tests/test_views.py

@@ -10,6 +10,10 @@ from .common import MockEppLib, completed_application  # type: ignore
 from django_webtest import WebTest  # type: ignore
 import boto3_mocking  # type: ignore
 
+from registrar.utility.errors import (
+    NameserverError,
+    NameserverErrorCodes,
+)
 
 from registrar.models import (
     DomainApplication,
@@ -1442,20 +1446,165 @@ class TestDomainNameservers(TestDomainOverview):
         )
         self.assertContains(page, "DNS name servers")
 
-    @skip("Broken by adding registry connection fix in ticket 848")
-    def test_domain_nameservers_form(self):
-        """Can change domain's nameservers.
+    def test_domain_nameservers_form_submit_one_nameserver(self):
+        """Nameserver form submitted with one nameserver throws error.
 
         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.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
+        # regarding required fields
         with less_console_noise():  # swallow log warning message
             result = nameservers_page.form.submit()
-        # form submission was a post, response should be a redirect
+        # form submission was a post with an error, response should be a 200
+        # error text appears twice, once at the top of the page, once around
+        # the required field.  form requires a minimum of 2 name servers
+        self.assertContains(result, "This field is required.", count=2, status_code=200)
+
+    def test_domain_nameservers_form_submit_subdomain_missing_ip(self):
+        """Nameserver form catches missing ip error on subdomain.
+
+        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.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 without two hosts, both subdomains,
+        # only one has ips
+        nameservers_page.form["form-1-server"] = "ns2.igorville.gov"
+        with less_console_noise():  # swallow log warning message
+            result = nameservers_page.form.submit()
+        # form submission was a post with an error, response should be a 200
+        # error text appears twice, once at the top of the page, once around
+        # the required field.  subdomain missing an ip
+        self.assertContains(
+            result,
+            str(NameserverError(code=NameserverErrorCodes.MISSING_IP)),
+            count=2,
+            status_code=200,
+        )
+
+    def test_domain_nameservers_form_submit_missing_host(self):
+        """Nameserver form catches error when host is missing.
+
+        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.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 without two hosts, both subdomains,
+        # only one has ips
+        nameservers_page.form["form-1-ip"] = "127.0.0.1"
+        with less_console_noise():  # swallow log warning message
+            result = nameservers_page.form.submit()
+        # form submission was a post with an error, response should be a 200
+        # error text appears twice, once at the top of the page, once around
+        # the required field.  nameserver has ip but missing host
+        self.assertContains(
+            result,
+            str(NameserverError(code=NameserverErrorCodes.MISSING_HOST)),
+            count=2,
+            status_code=200,
+        )
+
+    def test_domain_nameservers_form_submit_glue_record_not_allowed(self):
+        """Nameserver form catches error when IP is present
+        but host not subdomain.
+
+        Uses self.app WebTest because we need to interact with forms.
+        """
+        nameserver1 = "ns1.igorville.gov"
+        nameserver2 = "ns2.igorville.com"
+        valid_ip = "127.0.0.1"
+        # initial nameservers page has one server with two ips
+        nameservers_page = self.app.get(
+            reverse("domain-dns-nameservers", kwargs={"pk": self.domain.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 without two hosts, both subdomains,
+        # only one has ips
+        nameservers_page.form["form-0-server"] = nameserver1
+        nameservers_page.form["form-1-server"] = nameserver2
+        nameservers_page.form["form-1-ip"] = valid_ip
+        with less_console_noise():  # swallow log warning message
+            result = nameservers_page.form.submit()
+        # form submission was a post with an error, response should be a 200
+        # error text appears twice, once at the top of the page, once around
+        # the required field.  nameserver has ip but missing host
+        self.assertContains(
+            result,
+            str(NameserverError(code=NameserverErrorCodes.GLUE_RECORD_NOT_ALLOWED)),
+            count=2,
+            status_code=200,
+        )
+
+    def test_domain_nameservers_form_submit_invalid_ip(self):
+        """Nameserver form catches invalid IP on submission.
+
+        Uses self.app WebTest because we need to interact with forms.
+        """
+        nameserver = "ns2.igorville.gov"
+        invalid_ip = "123"
+        # initial nameservers page has one server with two ips
+        nameservers_page = self.app.get(
+            reverse("domain-dns-nameservers", kwargs={"pk": self.domain.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 without two hosts, both subdomains,
+        # only one has ips
+        nameservers_page.form["form-1-server"] = nameserver
+        nameservers_page.form["form-1-ip"] = invalid_ip
+        with less_console_noise():  # swallow log warning message
+            result = nameservers_page.form.submit()
+        # form submission was a post with an error, response should be a 200
+        # error text appears twice, once at the top of the page, once around
+        # the required field.  nameserver has ip but missing host
+        self.assertContains(
+            result,
+            str(
+                NameserverError(
+                    code=NameserverErrorCodes.INVALID_IP, nameserver=nameserver
+                )
+            ),
+            count=2,
+            status_code=200,
+        )
+
+    def test_domain_nameservers_form_submits_successfully(self):
+        """Nameserver form submits successfully with valid input.
+
+        Uses self.app WebTest because we need to interact with forms.
+        """
+        nameserver1 = "ns1.igorville.gov"
+        nameserver2 = "ns2.igorville.gov"
+        invalid_ip = "127.0.0.1"
+        # initial nameservers page has one server with two ips
+        nameservers_page = self.app.get(
+            reverse("domain-dns-nameservers", kwargs={"pk": self.domain.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 without two hosts, both subdomains,
+        # only one has ips
+        nameservers_page.form["form-0-server"] = nameserver1
+        nameservers_page.form["form-1-server"] = nameserver2
+        nameservers_page.form["form-1-ip"] = invalid_ip
+        with less_console_noise():  # swallow log warning message
+            result = nameservers_page.form.submit()
+        # form submission was a successful post, response should be a 302
         self.assertEqual(result.status_code, 302)
         self.assertEqual(
             result["Location"],
@@ -1465,9 +1614,8 @@ class TestDomainNameservers(TestDomainOverview):
         page = result.follow()
         self.assertContains(page, "The name servers for this domain have been updated")
 
-    @skip("Broken by adding registry connection fix in ticket 848")
     def test_domain_nameservers_form_invalid(self):
-        """Can change domain's nameservers.
+        """Nameserver form does not submit with invalid data.
 
         Uses self.app WebTest because we need to interact with forms.
         """
@@ -1482,9 +1630,9 @@ class TestDomainNameservers(TestDomainOverview):
         with less_console_noise():  # swallow logged warning message
             result = nameservers_page.form.submit()
         # form submission was a post with an error, response should be a 200
-        # error text appears twice, once at the top of the page, once around
-        # the field.
-        self.assertContains(result, "This field is required", count=2, status_code=200)
+        # error text appears four times, twice at the top of the page,
+        # once around each required field.
+        self.assertContains(result, "This field is required", count=4, status_code=200)
 
 
 class TestDomainAuthorizingOfficial(TestDomainOverview):

src/registrar/utility/errors.py

@@ -20,6 +20,41 @@ class ActionNotAllowed(Exception):
     pass
 
 
+class GenericErrorCodes(IntEnum):
+    """Used across the registrar for
+    error mapping.
+    Overview of generic error codes:
+        - 1 GENERIC_ERROR a generic value error
+        - 2 CANNOT_CONTACT_REGISTRY a connection error w registry
+    """
+
+    GENERIC_ERROR = 1
+    CANNOT_CONTACT_REGISTRY = 2
+
+
+class GenericError(Exception):
+    """
+    GenericError class used to raise exceptions across
+    the registrar
+    """
+
+    _error_mapping = {
+        GenericErrorCodes.CANNOT_CONTACT_REGISTRY: (
+            "Update failed. Cannot contact the registry."
+        ),
+        GenericErrorCodes.GENERIC_ERROR: ("Value entered was wrong."),
+    }
+
+    def __init__(self, *args, code=None, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.code = code
+        if self.code in self._error_mapping:
+            self.message = self._error_mapping.get(self.code)
+
+    def __str__(self):
+        return f"{self.message}"
+
+
 class NameserverErrorCodes(IntEnum):
     """Used in the NameserverError class for
     error mapping.
@@ -29,6 +64,8 @@ class NameserverErrorCodes(IntEnum):
                                     value but is not a subdomain
         - 3 INVALID_IP  invalid ip address format or invalid version
         - 4 TOO_MANY_HOSTS  more than the max allowed host values
+        - 5 UNABLE_TO_UPDATE_DOMAIN unable to update the domain
+        - 6 MISSING_HOST host is missing for a nameserver
     """
 
     MISSING_IP = 1
@@ -36,6 +73,7 @@ class NameserverErrorCodes(IntEnum):
     INVALID_IP = 3
     TOO_MANY_HOSTS = 4
     UNABLE_TO_UPDATE_DOMAIN = 5
+    MISSING_HOST = 6
 
 
 class NameserverError(Exception):
@@ -45,11 +83,15 @@ class NameserverError(Exception):
     """
 
     _error_mapping = {
-        NameserverErrorCodes.MISSING_IP: "Nameserver {} needs to have an "
-        "IP address because it is a subdomain",
-        NameserverErrorCodes.GLUE_RECORD_NOT_ALLOWED: "Nameserver {} cannot be linked "
-        "because it is not a subdomain",
-        NameserverErrorCodes.INVALID_IP: "Nameserver {} has an invalid IP address: {}",
+        NameserverErrorCodes.MISSING_IP: (
+            "Using your domain for a name server requires an IP address"
+        ),
+        NameserverErrorCodes.GLUE_RECORD_NOT_ALLOWED: (
+            "Name server address does not match domain name"
+        ),
+        NameserverErrorCodes.INVALID_IP: (
+            "{}: Enter an IP address in the required format."
+        ),
         NameserverErrorCodes.TOO_MANY_HOSTS: (
             "Too many hosts provided, you may not have more than 13 nameservers."
         ),
@@ -57,6 +99,9 @@ class NameserverError(Exception):
             "Unable to update domain, changes were not applied."
             "Check logs as a Registry Error is the likely cause"
         ),
+        NameserverErrorCodes.MISSING_HOST: (
+            "Name server must be provided to enter IP address."
+        ),
     }
 
     def __init__(self, *args, code=None, nameserver=None, ip=None, **kwargs):
@@ -65,7 +110,7 @@ class NameserverError(Exception):
         if self.code in self._error_mapping:
             self.message = self._error_mapping.get(self.code)
             if nameserver is not None and ip is not None:
-                self.message = self.message.format(str(nameserver), str(ip))
+                self.message = self.message.format(str(nameserver))
             elif nameserver is not None:
                 self.message = self.message.format(str(nameserver))
             elif ip is not None:

src/registrar/views/domain.py

@@ -23,6 +23,12 @@ from registrar.models import (
     UserDomainRole,
 )
 from registrar.models.public_contact import PublicContact
+from registrar.utility.errors import (
+    GenericError,
+    GenericErrorCodes,
+    NameserverError,
+    NameserverErrorCodes as nsErrorCodes,
+)
 from registrar.models.utility.contact_error import ContactError
 
 from ..forms import (
@@ -40,8 +46,6 @@ from epplibwrapper import (
     common,
     extensions,
     RegistryError,
-    CANNOT_CONTACT_REGISTRY,
-    GENERIC_ERROR,
 )
 
 from ..utility.email import send_templated_email, EmailSendingError
@@ -215,6 +219,7 @@ class DomainNameserversView(DomainFormBaseView):
 
     template_name = "domain_nameservers.html"
     form_class = NameserverFormset
+    model = Domain
 
     def get_initial(self):
         """The initial value for the form (which is a formset here)."""
@@ -223,7 +228,9 @@ class DomainNameserversView(DomainFormBaseView):
 
         if nameservers is not None:
             # Add existing nameservers as initial data
-            initial_data.extend({"server": name} for name, *ip in nameservers)
+            initial_data.extend(
+                {"server": name, "ip": ",".join(ip)} for name, ip in nameservers
+            )
 
         # Ensure at least 3 fields, filled or empty
         while len(initial_data) < 2:
@@ -252,25 +259,82 @@ class DomainNameserversView(DomainFormBaseView):
                 form.fields["server"].required = True
             else:
                 form.fields["server"].required = False
+            form.fields["domain"].initial = self.object.name
         return formset
 
+    def post(self, request, *args, **kwargs):
+        """Form submission posts to this view.
+
+        This post method harmonizes using DomainBaseView and FormMixin
+        """
+        self._get_domain(request)
+        formset = self.get_form()
+
+        if "btn-cancel-click" in request.POST:
+            url = self.get_success_url()
+            return HttpResponseRedirect(url)
+
+        if formset.is_valid():
+            return self.form_valid(formset)
+        else:
+            return self.form_invalid(formset)
+
     def form_valid(self, formset):
         """The formset is valid, perform something with it."""
 
+        self.request.session["nameservers_form_domain"] = self.object
+
         # Set the nameservers from the formset
         nameservers = []
         for form in formset:
             try:
-                as_tuple = (form.cleaned_data["server"],)
+                ip_string = form.cleaned_data["ip"]
+                # ip_string will be None or a string of IP addresses
+                # comma-separated
+                ip_list = []
+                if ip_string:
+                    # Split the string into a list using a comma as the delimiter
+                    ip_list = ip_string.split(",")
+
+                as_tuple = (
+                    form.cleaned_data["server"],
+                    ip_list,
+                )
                 nameservers.append(as_tuple)
             except KeyError:
                 # no server information in this field, skip it
                 pass
-        self.object.nameservers = nameservers
 
-        messages.success(
-            self.request, "The name servers for this domain have been updated."
-        )
+        try:
+            self.object.nameservers = nameservers
+        except NameserverError as Err:
+            # NamserverErrors *should* be caught in form; if reached here,
+            # there was an uncaught error in submission (through EPP)
+            messages.error(
+                self.request, NameserverError(code=nsErrorCodes.UNABLE_TO_UPDATE_DOMAIN)
+            )
+            logger.error(f"Nameservers error: {Err}")
+        # TODO: registry is not throwing an error when no connection
+        except RegistryError as Err:
+            if Err.is_connection_error():
+                messages.error(
+                    self.request,
+                    GenericError(code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY),
+                )
+                logger.error(f"Registry connection error: {Err}")
+            else:
+                messages.error(
+                    self.request, GenericError(code=GenericErrorCodes.GENERIC_ERROR)
+                )
+                logger.error(f"Registry error: {Err}")
+        else:
+            messages.success(
+                self.request,
+                "The name servers for this domain have been updated. "
+                "Keep in mind that DNS changes may take some time to "
+                "propagate across the internet. It can take anywhere "
+                "from a few minutes to 48 hours for your changes to take place.",
+            )
 
         # superclass has the redirect
         return super().form_valid(formset)
@@ -431,10 +495,15 @@ class DomainDsDataView(DomainFormBaseView):
             self.object.dnssecdata = dnssecdata
         except RegistryError as err:
             if err.is_connection_error():
-                messages.error(self.request, CANNOT_CONTACT_REGISTRY)
+                messages.error(
+                    self.request,
+                    GenericError(code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY),
+                )
                 logger.error(f"Registry connection error: {err}")
             else:
-                messages.error(self.request, GENERIC_ERROR)
+                messages.error(
+                    self.request, GenericError(code=GenericErrorCodes.GENERIC_ERROR)
+                )
                 logger.error(f"Registry error: {err}")
             return self.form_invalid(formset)
         else:
@@ -510,7 +579,10 @@ class DomainSecurityEmailView(DomainFormBaseView):
         # If no default is created for security_contact,
         # then we cannot connect to the registry.
         if contact is None:
-            messages.error(self.request, CANNOT_CONTACT_REGISTRY)
+            messages.error(
+                self.request,
+                GenericError(code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY),
+            )
             return redirect(self.get_success_url())
 
         contact.email = new_email
@@ -519,13 +591,20 @@ class DomainSecurityEmailView(DomainFormBaseView):
             contact.save()
         except RegistryError as Err:
             if Err.is_connection_error():
-                messages.error(self.request, CANNOT_CONTACT_REGISTRY)
+                messages.error(
+                    self.request,
+                    GenericError(code=GenericErrorCodes.CANNOT_CONTACT_REGISTRY),
+                )
                 logger.error(f"Registry connection error: {Err}")
             else:
-                messages.error(self.request, GENERIC_ERROR)
+                messages.error(
+                    self.request, GenericError(code=GenericErrorCodes.GENERIC_ERROR)
+                )
                 logger.error(f"Registry error: {Err}")
         except ContactError as Err:
-            messages.error(self.request, GENERIC_ERROR)
+            messages.error(
+                self.request, GenericError(code=GenericErrorCodes.GENERIC_ERROR)
+            )
             logger.error(f"Generic registry error: {Err}")
         else:
             messages.success(