fixed bugs in ip list handling, reformatted clean function in forms, removed custom field type for ip address in nameserverform

src/registrar/forms/domain.py

@@ -37,64 +37,58 @@ class DomainNameserverForm(forms.Form):
 
     server = forms.CharField(label="Name server", strip=True)
 
-    ip = IPAddressField(
+    ip = forms.CharField(label="IP Address (IPv4 or IPv6)", strip=True, required=False)
-        label="IP Address (IPv4 or IPv6)",
-        strip=True,
-        required=False,
-        # validators=[
-        #     django.core.validators.validate_ipv46_address
-        # ],
-    )
 
     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", "")
+        ip = cleaned_data.get("ip", None)
         domain = cleaned_data.get("domain", "")
-        print(f"clean is called on {domain} {server}")
 
-        # make sure there's a nameserver if an ip is passed
+        ip_list = self.extract_ip_list(ip)
-        if ip:
-            ip_list = [ip.strip() for ip in ip.split(",")]
-            if not server and len(ip_list) > 0:
-                # If 'server' is empty, disallow 'ip' input
-                self.add_error(
-                    "server", NameserverError(code=nsErrorCodes.MISSING_HOST)
-                )
 
-        # if there's a nameserver and an ip, validate nameserver/ip combo
+        if ip and not server and ip_list:
-
+            self.add_error("server", NameserverError(code=nsErrorCodes.MISSING_HOST))
-        if server:
+        elif server:
-            if ip:
+            self.validate_nameserver_ip_combo(domain, server, ip_list)
-                ip_list = [ip.strip() for ip in ip.split(",")]
-            else:
-                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))
 
         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,

src/registrar/models/domain.py

@@ -260,7 +260,7 @@ 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:
+        if addrs is not None and addrs != []:
             addresses = [epp.Ip(addr=addr) for addr in addrs]
             request = commands.CreateHost(name=host, addrs=addresses)
         else:
@@ -314,16 +314,14 @@ class Domain(TimeStampedModel, DomainHelper):
             NameserverError (if exception hit)
         Returns:
             None"""
-        if cls.isSubdomain(name, nameserver) and (ip is None or ip == [] or ip == [""]):
+        if cls.isSubdomain(name, nameserver) and (ip is None or ip == []):
             raise NameserverError(code=nsErrorCodes.MISSING_IP, nameserver=nameserver)
 
-        elif not cls.isSubdomain(name, nameserver) and (
+        elif not cls.isSubdomain(name, nameserver) and (ip is not None and ip != []):
-            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 != [] and ip != [""]:
+        elif ip is not None and ip != []:
             for addr in ip:
                 if not cls._valid_ip_addr(addr):
                     raise NameserverError(

src/registrar/tests/test_nameserver_error.py

@@ -35,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/utility/errors.py

@@ -87,7 +87,9 @@ class NameserverError(Exception):
         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.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."
         ),

src/registrar/views/domain.py

@@ -295,8 +295,12 @@ class DomainNameserversView(DomainFormBaseView):
         for form in formset:
             try:
                 ip_string = form.cleaned_data["ip"]
-                # Split the string into a list using a comma as the delimiter
+                # ip_string will be None or a string of IP addresses
-                ip_list = ip_string.split(",")
+                # comma-separated
+                ip_list = []
+                if ip_string:
+                    # 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