Refactor _update_delete_create_hosts function

src/registrar/models/domain.py

@@ -352,16 +352,17 @@ class Domain(TimeStampedModel, DomainHelper):
 
         return (deleted_values, updated_values, new_values, previousHostDict)
 
-    def _update_delete_create_hosts(
+    def _deleted_host_values(self, deleted_values) -> int:
-        self, deleted_values, updated_values, new_values, oldNameservers
-    ):
         successDeletedCount = 0
-        successCreatedCount = 0
+
         for hostTuple in deleted_values:
             deleted_response_code = self._delete_host(hostTuple[0])
             if deleted_response_code == ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY:
                 successDeletedCount += 1
 
+        return successDeletedCount
+
+    def _update_host_values(self, updated_values, oldNameservers):
         for hostTuple in updated_values:
             updated_response_code = self._update_host(
                 hostTuple[0], hostTuple[1], oldNameservers.get(hostTuple[0])
@@ -375,6 +376,8 @@ class Domain(TimeStampedModel, DomainHelper):
                     % (hostTuple[0], updated_response_code)
                 )
 
+    def _new_host_values(self, new_values) -> int:
+        successCreatedCount = 0
         for key, value in new_values.items():
             createdCode = self._create_host(
                 host=key, addrs=value
@@ -394,7 +397,24 @@ class Domain(TimeStampedModel, DomainHelper):
                         "Error adding nameserver, code was %s error was %s"
                         % (e.code, e)
                     )
-        return len(oldNameservers) - successDeletedCount + successCreatedCount
+        return successCreatedCount
+
+    # def _update_delete_create_hosts(
+    #     self, deleted_values, updated_values, new_values, oldNameservers
+    # ):
+
+    # for hostTuple in updated_values:
+    #     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)
+    #         )
 
     @nameservers.setter  # type: ignore
     def nameservers(self, hosts: list[tuple[str, list]]):
@@ -417,10 +437,20 @@ class Domain(TimeStampedModel, DomainHelper):
             oldNameservers,
         ) = self.getNameserverChanges(hosts=hosts)
 
-        successTotalNameservers = self._update_delete_create_hosts(
+        successDeletedCount = self._deleted_host_values(deleted_values)  # returns value
-            deleted_values, updated_values, new_values, oldNameservers
+        _ = self._update_host_values(
+            updated_values, oldNameservers
+        )  # returns nothing, just need to be run and errors
+        successCreatedCount = self._new_host_values(new_values)
+
+        successTotalNameservers = (
+            len(oldNameservers) - successDeletedCount + successCreatedCount
         )
 
+        # successTotalNameservers = self._update_delete_create_hosts(
+        #     deleted_values, updated_values, new_values, oldNameservers
+        # )
+
         if successTotalNameservers < 2:
             try:
                 print("DNS_NEEDED: We have less than 2 nameservers")

src/registrar/tests/test_models_domain.py

@@ -663,7 +663,7 @@ class TestRegistrantNameservers(MockEppLib):
         # Testing only deleting and no other changes
         self.domain._cache["hosts"] = [
             {"name": "ns1.example.com", "addrs": None},
-            {"name": "ns2.example.com", "addrs": ["1.2.3"]},
+            {"name": "ns2.example.com", "addrs": ["1.2.3.4"]},
         ]
         newChanges = [
             ("ns1.example.com",),
@@ -675,21 +675,21 @@ class TestRegistrantNameservers(MockEppLib):
             oldNameservers,
         ) = self.domain.getNameserverChanges(newChanges)
 
-        self.assertEqual(deleted_values, [("ns2.example.com", ["1.2.3"])])
+        self.assertEqual(deleted_values, [("ns2.example.com", ["1.2.3.4"])])
         self.assertEqual(updated_values, [])
         self.assertEqual(new_values, {})
         self.assertEqual(
             oldNameservers,
-            {"ns1.example.com": None, "ns2.example.com": ["1.2.3"]},
+            {"ns1.example.com": None, "ns2.example.com": ["1.2.3.4"]},
         )
 
     def test_get_nameserver_changes_success_updated_vals(self):
         # Testing only updating no other changes
         self.domain._cache["hosts"] = [
-            {"name": "ns3.my-nameserver.gov", "addrs": ["1.2.3"]},
+            {"name": "ns3.my-nameserver.gov", "addrs": ["1.2.3.4"]},
         ]
         newChanges = [
-            ("ns3.my-nameserver.gov", ["1.2.4"]),
+            ("ns3.my-nameserver.gov", ["1.2.4.5"]),
         ]
         (
             deleted_values,
@@ -699,11 +699,11 @@ class TestRegistrantNameservers(MockEppLib):
         ) = self.domain.getNameserverChanges(newChanges)
 
         self.assertEqual(deleted_values, [])
-        self.assertEqual(updated_values, [("ns3.my-nameserver.gov", ["1.2.4"])])
+        self.assertEqual(updated_values, [("ns3.my-nameserver.gov", ["1.2.4.5"])])
         self.assertEqual(new_values, {})
         self.assertEqual(
             oldNameservers,
-            {"ns3.my-nameserver.gov": ["1.2.3"]},
+            {"ns3.my-nameserver.gov": ["1.2.3.4"]},
         )
 
     def test_get_nameserver_changes_success_new_vals(self):