diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 4a5306752..ff8952643 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -294,11 +294,12 @@ class Domain(TimeStampedModel, DomainHelper): return newDict def isSubdomain(self, nameserver): - return nameserver.find(self.name) != -1 + return self.name in nameserver def checkHostIPCombo(self, nameserver: str, ip: list): if self.isSubdomain(nameserver) and (ip is None or ip == []): - raise ValueError("Nameserver %s needs to have an ip address" % nameserver) + raise ValueError("Nameserver %s needs to have an " + "ip address because it is a subdomain" % nameserver) elif not self.isSubdomain(nameserver) and (ip is not None and ip != []): raise ValueError( "Nameserver %s cannot be linked " @@ -306,15 +307,15 @@ class Domain(TimeStampedModel, DomainHelper): ) return None - def getNameserverChanges(self, hosts: list[tuple[str]]): + def getNameserverChanges(self, hosts: list[tuple[str]]) -> tuple[list, list, dict, list]: """ calls self.nameserver, it should pull from cache but may result in an epp call returns tuple of four values as follows: - deleted_values: - updated_values: + deleted_values: list + updated_values: list new_values: dict - oldNameservers:""" + oldNameservers: list""" oldNameservers = self.nameservers previousHostDict = self._convert_list_to_dict(oldNameservers) @@ -417,7 +418,7 @@ class Domain(TimeStampedModel, DomainHelper): oldNameservers, ) = self.getNameserverChanges(hosts=hosts) - # TODO-848: Fix name here + # TODO-848: Fix name successTotalNameservers = self._loop_through( deleted_values, updated_values, new_values, oldNameservers ) @@ -1103,9 +1104,9 @@ class Domain(TimeStampedModel, DomainHelper): and len(old_ip_list) != 0 ): return ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY - - added_ip_list = set(ip_list) - set(old_ip_list) - removed_ip_list = set(old_ip_list) - set(ip_list) + + added_ip_list = set(ip_list).difference(old_ip_list) + removed_ip_list = set(old_ip_list).difference(ip_list) request = commands.UpdateHost( name=nameserver, diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 6f9c8aa56..d87757f26 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -631,8 +631,10 @@ class MockEppLib(TestCase): "ns2.nameserverwithip.gov", "ns3.nameserverwithip.gov", ], + addrs=["1.2.3", "2.3.4"], ) + # TODO-848: Fix naming extendedValues = False # TODO-848: Rename later - was getting complex err diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 7aaafe582..85968db01 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -544,15 +544,64 @@ class TestRegistrantNameservers(MockEppLib): name="my-nameserver.gov", state=Domain.State.DNS_NEEDED ) - def test_get_nameserver_changes(self): + def test_get_nameserver_changes_success_deleted_vals(self): + # 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": "ns3.my-nameserver.gov", "addrs": ["1.2.3"]}, ] newChanges = [ ("ns1.example.com",), + ] + ( + deleted_values, + updated_values, + new_values, + oldNameservers, + ) = self.domain.getNameserverChanges(newChanges) + + self.assertEqual(deleted_values, [("ns2.example.com", ["1.2.3"])]) + self.assertEqual(updated_values, []) + self.assertEqual(new_values, {}) + self.assertEqual( + oldNameservers, + { + 'ns1.example.com': None, 'ns2.example.com': ['1.2.3'] + }, + ) + + 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"]}, + ] + newChanges = [ ("ns3.my-nameserver.gov", ["1.2.4"]), + ] + ( + deleted_values, + updated_values, + new_values, + oldNameservers, + ) = self.domain.getNameserverChanges(newChanges) + + self.assertEqual(deleted_values, []) + self.assertEqual(updated_values, [("ns3.my-nameserver.gov", ["1.2.4"])]) + self.assertEqual(new_values, {}) + self.assertEqual( + oldNameservers, + { + "ns3.my-nameserver.gov": ["1.2.3"] + }, + ) + + def test_get_nameserver_changes_success_new_vals(self): + # Testing only creating no other changes + self.domain._cache["hosts"] = [ + {"name": "ns1.example.com", "addrs": None}, + ] + newChanges = [ + ("ns1.example.com",), ("ns4.example.com",), ] ( @@ -561,15 +610,19 @@ class TestRegistrantNameservers(MockEppLib): new_values, oldNameservers, ) = self.domain.getNameserverChanges(newChanges) - self.assertEqual(deleted_values, [("ns2.example.com", ["1.2.3"])]) - self.assertEqual(updated_values, [("ns3.my-nameserver.gov", ["1.2.4"])]) + + + print("deleted vals is ", deleted_values) + print("updated vals is ", updated_values) + print("new vals is ", new_values) + print("old nameserver is ", oldNameservers) + self.assertEqual(deleted_values, []) + self.assertEqual(updated_values, []) self.assertEqual(new_values, {"ns4.example.com": None}) self.assertEqual( oldNameservers, { "ns1.example.com": None, - "ns2.example.com": ["1.2.3"], - "ns3.my-nameserver.gov": ["1.2.3"], }, ) @@ -601,7 +654,8 @@ class TestRegistrantNameservers(MockEppLib): self.mockedSendFunction.assert_has_calls(expectedCalls) - # check that status is still NOT READY + # check that status is still NOT READY + # as you have less than 2 nameservers self.assertFalse(self.domain.is_active()) def test_user_adds_two_nameservers(self):