diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index eabbfcd74..ba4db0ef9 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -17,6 +17,7 @@ from epplibwrapper import ( ErrorCode, ) from registrar.utility.errors import ( + ActionNotAllowed, NameserverError, NameserverErrorCodes as nsErrorCodes, ) @@ -433,6 +434,10 @@ class Domain(TimeStampedModel, DomainHelper): if len(hosts) > 13: raise NameserverError(code=nsErrorCodes.TOO_MANY_HOSTS) + + if self.state not in [self.State.DNS_NEEDED, self.State.READY]: + raise ActionNotAllowed("Nameservers can not be " "set in the current state") + logger.info("Setting nameservers") logger.info(hosts) @@ -465,9 +470,6 @@ class Domain(TimeStampedModel, DomainHelper): ) elif successTotalNameservers >= 2 and successTotalNameservers <= 13: try: - print( - "READY/SAVE: We are in happy path where btwen 2 and 13 inclusive ns" - ) self.ready() self.save() except Exception as err: diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index a444522c1..b0bd701aa 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -727,17 +727,11 @@ class MockEppLib(TestCase): ], ) - # Bc we have multiple tests utilizing 3 nameservers, - # easier to set a flag for it - threeNS = False - def _getattrInfoDomain(self, _request): if getattr(_request, "name", None) == "security.gov": return MagicMock(res_data=[self.infoDomainNoContact]) elif getattr(_request, "name", None) == "my-nameserver.gov": - if self.threeNS: - return MagicMock(res_data=[self.infoDomainThreeHosts]) - elif self.mockedSendFunction.call_count == 5: + if self.mockedSendFunction.call_count == 5: return MagicMock(res_data=[self.infoDomainTwoHosts]) else: return MagicMock(res_data=[self.infoDomainNoHost]) @@ -747,8 +741,8 @@ class MockEppLib(TestCase): return MagicMock(res_data=[self.infoDomainCheckHostIPCombo]) elif getattr(_request, "name", None) == "freeman.gov": return MagicMock(res_data=[self.InfoDomainWithContacts]) - # elif getattr(_request, "name", None) == "failednameserver.gov": - # return MagicMock(res_data=[self.infoDomainUpdateFail]) + elif getattr(_request, "name", None) == "threenameserversDomain.gov": + return MagicMock(res_data=[self.infoDomainThreeHosts]) return MagicMock(res_data=[self.mockDataInfoDomain]) def mockSend(self, _request, cleaned): diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index c1107a590..5cdd3c79d 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -16,7 +16,7 @@ from registrar.models.domain_information import DomainInformation from registrar.models.draft_domain import DraftDomain from registrar.models.public_contact import PublicContact from registrar.models.user import User -from registrar.utility.errors import NameserverError +from registrar.utility.errors import ActionNotAllowed, NameserverError from .common import MockEppLib from django_fsm import TransitionNotAllowed # type: ignore from epplibwrapper import ( @@ -862,6 +862,9 @@ class TestRegistrantNameservers(MockEppLib): self.domain, _ = Domain.objects.get_or_create( name="my-nameserver.gov", state=Domain.State.DNS_NEEDED ) + self.domainWithThreeNS, _ = Domain.objects.get_or_create( + name="threenameserversDomain.gov", state=Domain.State.READY + ) def test_get_nameserver_changes_success_deleted_vals(self): # Testing only deleting and no other changes @@ -1063,14 +1066,13 @@ class TestRegistrantNameservers(MockEppLib): """ # Mock is set to return 3 nameservers on infodomain - self.threeNS = True - self.domain.nameservers = [(self.nameserver1,), (self.nameserver2,)] + self.domainWithThreeNS.nameservers = [(self.nameserver1,), (self.nameserver2,)] expectedCalls = [ # calls info domain, and info on all hosts # to get past values # then removes the single host and updates domain call( - commands.InfoDomain(name="my-nameserver.gov", auth_info=None), + commands.InfoDomain(name=self.domainWithThreeNS.name, auth_info=None), cleaned=True, ), call(commands.InfoHost(name="ns1.my-nameserver-1.com"), cleaned=True), @@ -1078,7 +1080,7 @@ class TestRegistrantNameservers(MockEppLib): call(commands.InfoHost(name="ns1.cats-are-superior3.com"), cleaned=True), call( commands.UpdateDomain( - name="my-nameserver.gov", + name=self.domainWithThreeNS.name, add=[], rem=[common.HostObjSet(hosts=["ns1.cats-are-superior3.com"])], nsset=None, @@ -1104,12 +1106,11 @@ class TestRegistrantNameservers(MockEppLib): And `domain.is_active` returns False """ - self.threeNS = True - self.domain.ready() - self.domain.nameservers = [(self.nameserver1,)] + + self.domainWithThreeNS.nameservers = [(self.nameserver1,)] expectedCalls = [ call( - commands.InfoDomain(name="my-nameserver.gov", auth_info=None), + commands.InfoDomain(name=self.domainWithThreeNS.name, auth_info=None), cleaned=True, ), call(commands.InfoHost(name="ns1.my-nameserver-1.com"), cleaned=True), @@ -1117,7 +1118,7 @@ class TestRegistrantNameservers(MockEppLib): call(commands.InfoHost(name="ns1.cats-are-superior3.com"), cleaned=True), call( commands.UpdateDomain( - name="my-nameserver.gov", + name=self.domainWithThreeNS.name, add=[], rem=[common.HostObjSet(hosts=["ns1.my-nameserver-2.com"])], nsset=None, @@ -1130,7 +1131,7 @@ class TestRegistrantNameservers(MockEppLib): call(commands.DeleteHost(name="ns1.my-nameserver-2.com"), cleaned=True), call( commands.UpdateDomain( - name="my-nameserver.gov", + name=self.domainWithThreeNS.name, add=[], rem=[common.HostObjSet(hosts=["ns1.cats-are-superior3.com"])], nsset=None, @@ -1156,9 +1157,8 @@ class TestRegistrantNameservers(MockEppLib): And `commands.UpdateDomain` is sent to add #4 and #5 plus remove #2 and #3 And `commands.DeleteHost` is sent to delete #2 and #3 """ - self.threeNS = True - self.domain.ready() - self.domain.nameservers = [ + + self.domainWithThreeNS.nameservers = [ (self.nameserver1,), ("ns1.cats-are-superior1.com",), ("ns1.cats-are-superior2.com",), @@ -1166,7 +1166,7 @@ class TestRegistrantNameservers(MockEppLib): expectedCalls = [ call( - commands.InfoDomain(name="my-nameserver.gov", auth_info=None), + commands.InfoDomain(name=self.domainWithThreeNS.name, auth_info=None), cleaned=True, ), call(commands.InfoHost(name="ns1.my-nameserver-1.com"), cleaned=True), @@ -1174,7 +1174,7 @@ class TestRegistrantNameservers(MockEppLib): call(commands.InfoHost(name="ns1.cats-are-superior3.com"), cleaned=True), call( commands.UpdateDomain( - name="my-nameserver.gov", + name=self.domainWithThreeNS.name, add=[], rem=[common.HostObjSet(hosts=["ns1.my-nameserver-2.com"])], nsset=None, @@ -1191,7 +1191,7 @@ class TestRegistrantNameservers(MockEppLib): ), call( commands.UpdateDomain( - name="my-nameserver.gov", + name=self.domainWithThreeNS.name, add=[common.HostObjSet(hosts=["ns1.cats-are-superior1.com"])], rem=[], nsset=None, @@ -1207,7 +1207,7 @@ class TestRegistrantNameservers(MockEppLib): ), call( commands.UpdateDomain( - name="my-nameserver.gov", + name=self.domainWithThreeNS.name, add=[common.HostObjSet(hosts=["ns1.cats-are-superior2.com"])], rem=[], nsset=None, @@ -1312,14 +1312,9 @@ class TestRegistrantNameservers(MockEppLib): to the registry twice with identical data Then no errors are raised in Domain """ - # implementation note: this requires seeing what happens when these are actually - # sent like this, and then implementing appropriate mocks for any errors the - # registry normally sends in this case - - self.threeNS = True # Checking that it doesn't create or update even if out of order - self.domain.nameservers = [ + self.domainWithThreeNS.nameservers = [ (self.nameserver3,), (self.nameserver1,), (self.nameserver2,), @@ -1327,7 +1322,7 @@ class TestRegistrantNameservers(MockEppLib): expectedCalls = [ call( - commands.InfoDomain(name="my-nameserver.gov", auth_info=None), + commands.InfoDomain(name=self.domainWithThreeNS.name, auth_info=None), cleaned=True, ), call(commands.InfoHost(name="ns1.my-nameserver-1.com"), cleaned=True), @@ -1371,6 +1366,15 @@ class TestRegistrantNameservers(MockEppLib): ("ns2.nameserversubdomain.gov", ["2.3.4"]), ] + def test_setting_not_allowed(self): + """Scenario: A domain state is not Ready or DNS Needed + then setting nameservers is not allowed""" + domain, _ = Domain.objects.get_or_create( + name="onholdDomain.gov", state=Domain.State.ON_HOLD + ) + with self.assertRaises(ActionNotAllowed): + domain.nameservers = [self.nameserver1, self.nameserver2] + @skip("not implemented yet") def test_update_is_unsuccessful(self): """ @@ -1391,7 +1395,6 @@ class TestRegistrantNameservers(MockEppLib): domain.nameservers = [("ns1.failednameserver.gov", ["4.5.6"])] def tearDown(self): - self.threeNS = False Domain.objects.all().delete() return super().tearDown() diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 98c2e6667..5eb92750e 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -13,6 +13,13 @@ class DomainUnavailableError(ValueError): pass +class ActionNotAllowed(Exception): + """User accessed an action that is not + allowed by the current state""" + + pass + + class NameserverErrorCodes(IntEnum): """Used in the NameserverError class for error mapping.