_is_dns_needed not working

src/registrar/models/domain.py

@@ -318,6 +318,8 @@ class Domain(TimeStampedModel, DomainHelper):
                 deleted_values.append((prevHost,addrs))
             #if the host exists in both, check if the addresses changed
             else:
+                #TODO - host is being updated when previous was None and new is an empty list
+                #add check here
                 if newHostDict[prevHost] != addrs: 
                     updated_values.append((prevHost,newHostDict[prevHost]))
 
@@ -370,8 +372,9 @@ class Domain(TimeStampedModel, DomainHelper):
                 successDeletedCount += 1 
 
         for hostTuple in updated_values:
-            updated_response_code = self._updated_host(hostTuple[0], hostTuple[1], oldNameservers.get(hostTuple[0]))
-
+            updated_response_code = self._update_host(hostTuple[0], hostTuple[1], oldNameservers.get(hostTuple[0]))
+            if updated_response_code not in [ ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, ErrorCode.OBJECT_EXISTS]:
+                logger.warning("Could not update host %s. Error code was: %s " % (hostTuple[0], updated_response_code))
         for key, value in new_values.items():
             print("HELLO THERE KEY, VALUE PAIR")
             print(key)
@@ -1052,10 +1055,10 @@ class Domain(TimeStampedModel, DomainHelper):
     def _convert_ips(self, ip_list: list[str]):
         edited_ip_list = []
         for ip_addr in ip_list:
-            if is_ipv6:
-                edited_ip_list.append(command.Ip(addr=ip_addr, ip="v6"))
+            if self.is_ipv6():
+                edited_ip_list.append(epp.Ip(addr=ip_addr, ip="v6"))
             else: # default ip addr is v4
-                edited_ip_list.append(command.Ip(addr=ip_addr))
+                edited_ip_list.append(epp.Ip(addr=ip_addr))
         return edited_ip_list
 
     def _update_host(self, nameserver: str, ip_list: list[str], old_ip_list: list[str]):
@@ -1074,13 +1077,23 @@ class Domain(TimeStampedModel, DomainHelper):
 
     def _delete_host(self, nameserver: str):
         try:
-            request = commands.DeleteHost(name=nameserver)
-            response = registry.send(request, cleaned=True)
-            logger.info("_delete_host()-> sending req as %s" % request)
+            updateReq = commands.UpdateDomain(
+                    name=self.name, rem=[epp.HostObjSet([nameserver])]
+                )
+            response=registry.send(updateReq, cleaned=True)
+        
+            logger.info("_delete_host()-> sending update domain req as %s" % updateReq)
+            
+            deleteHostReq = commands.DeleteHost(name=nameserver)
+            response = registry.send(deleteHostReq, cleaned=True)
+            logger.info("_delete_host()-> sending delete host req as %s" % deleteHostReq)
             return response.code
         except RegistryError as e:
-            logger.error("Error _delete_host, code was %s error was %s" % (e.code, e))
-            return e.code
+            if e.code==ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION:
+                logger.info("Did not remove host %s because it is in use on another domain." % nameserver)
+            else:
+                logger.error("Error _delete_host, code was %s error was %s" % (e.code, e))
+                return e.code
             
     def _fetch_cache(self, fetch_hosts=False, fetch_contacts=False):
         """Contact registry for info about a domain."""

src/registrar/tests/common.py

@@ -587,7 +587,7 @@ class MockEppLib(TestCase):
     mockDataInfoHosts = fakedEppObject(
         "lastPw", cr_date=datetime.datetime(2023, 8, 25, 19, 45, 35), addrs=["1.2.3", "2.3.4"]
     )
-    mockDataCreateHost  =fakedEppObject(
+    mockDataHostChange  =fakedEppObject(
         "lastPw", cr_date=datetime.datetime(2023, 8, 25, 19, 45, 35)
     )
 
@@ -611,7 +611,11 @@ class MockEppLib(TestCase):
             # sets the second send() to fail
             raise RegistryError(code=ErrorCode.OBJECT_EXISTS)
         elif (isinstance(_request, commands.CreateHost)):
-            return MagicMock(res_data=[self.mockDataCreateHost], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY)
+            return MagicMock(res_data=[self.mockDataHostChange], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY)
+        elif (isinstance(_request, commands.UpdateHost)):
+            return MagicMock(res_data=[self.mockDataHostChange], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY)
+        elif (isinstance(_request, commands.DeleteHost)):
+            return MagicMock(res_data=[self.mockDataHostChange], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY)
         return MagicMock(res_data=[self.mockDataInfoHosts])
 
     def setUp(self):

src/registrar/tests/test_models_domain.py

@@ -533,6 +533,10 @@ class TestRegistrantNameservers(MockEppLib):
             And the registrant is the admin on a domain
         """
         super().setUp()
+        self.nameserver1 = "ns1.my-nameserver-1.com"
+        self.nameserver2 = "ns1.my-nameserver-2.com"
+        self.nameserver3 = "ns1.cats-are-superior3.com"
+
         self.domain, _ = Domain.objects.get_or_create(name="my-nameserver.gov", state=Domain.State.DNS_NEEDED)
 
     def test_get_nameserver_changes(self):
@@ -549,10 +553,10 @@ class TestRegistrantNameservers(MockEppLib):
         ]
         newChanges=[("ns1.example.com",),("ns3.example.com",["1.2.4"]),("ns4.example.com",)]
         deleted_values,updated_values,new_values, oldNameservers=self.domain.getNameserverChanges(newChanges)
-
+        print(oldNameservers)
         self.assertEqual(deleted_values, [('ns2.example.com', ['1.2.3'])])
         self.assertEqual(updated_values, [('ns3.example.com', ['1.2.4'])])
-        self.assertEqual(new_values, {'ns4.example.com'})
+        self.assertEqual(new_values, {'ns4.example.com':None})
         self.assertEqual(oldNameservers, {'ns1.example.com': None, 'ns2.example.com': ['1.2.3'], 'ns3.example.com': ['1.2.3']})
 
     def test_user_adds_one_nameserver(self):
@@ -595,15 +599,14 @@ class TestRegistrantNameservers(MockEppLib):
         """
 
         # set 2 nameservers
-        nameserver1 = "ns1.my-nameserver-1.com"
-        nameserver2 = "ns1.my-nameserver-2.com"
-        self.domain.nameservers = [(nameserver1,), (nameserver2,)]  
+        
+        self.domain.nameservers = [(self.nameserver1,), (self.nameserver2,)]  
 
         # when you create a host, you also have to update at same time
-        created_host1 = commands.CreateHost(nameserver1)
+        created_host1 = commands.CreateHost(self.nameserver1)
         update_domain_with_created1 = commands.UpdateDomain(name=self.domain.name, add=[common.HostObjSet([created_host1.name])])
 
-        created_host2 = commands.CreateHost(nameserver2)
+        created_host2 = commands.CreateHost(self.nameserver2)
         update_domain_with_created2 = commands.UpdateDomain(name=self.domain.name, add=[common.HostObjSet([created_host2.name])])
 
         # checking if commands were sent (commands have to be sent in order)
@@ -664,6 +667,14 @@ class TestRegistrantNameservers(MockEppLib):
                 to the registry
             And `domain.is_active` returns True
         """
+        #Given the domain has 3 nameservers
+        self.domain.nameservers = [(self.nameserver1,), (self.nameserver2,),(self.nameserver3,)]  
+
+        #now remove one
+        self.domain.nameservers = [(self.nameserver1,), (self.nameserver2,)]
+
+        #assert updatedomain called
+        #assert call deletehost?
         raise
 
     @skip("not implemented yet")