From 095f7ca56f7de0ee9b16722ae032f4339d63d825 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 2 Oct 2023 14:02:07 -0700 Subject: [PATCH] Refactor _update_delete_create_hosts function --- src/registrar/models/domain.py | 44 +++++++++++++++++++---- src/registrar/tests/test_models_domain.py | 14 ++++---- 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index abec7a04a..57944a35e 100644 --- a/src/registrar/models/domain.py +++ b/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( - self, deleted_values, updated_values, new_values, oldNameservers - ): + def _deleted_host_values(self, deleted_values) -> int: 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( - deleted_values, updated_values, new_values, oldNameservers + successDeletedCount = self._deleted_host_values(deleted_values) # returns value + _ = 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") diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 4b8b3020d..c0fcc2fb6 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/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):