From 7cf8b8a82ea0a0d3df4885eaae45b1c4d0082dfc Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Fri, 22 Nov 2024 10:51:13 -0600 Subject: [PATCH 01/41] Delete contacts and subdomains on delete domain --- src/registrar/models/domain.py | 44 ++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 7fdc56971..03a969471 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1026,6 +1026,26 @@ class Domain(TimeStampedModel, DomainHelper): # if registry error occurs, log the error, and raise it as well logger.error(f"registry error removing client hold: {err}") raise (err) + + def _delete_contacts(self): + """Contacts associated with this domain will be deleted. + RegistryErrors will be logged and raised. Additional + error handling should be provided by the caller. + """ + contacts = self._cache.get("contacts") + for contact in contacts: + self._delete_contact(contact) + + def _delete_subdomains(self): + """Subdomains of this domain should be deleted from the registry. + Subdomains which are used by other domains (eg as a hostname) will + not be deleted. + + Supresses registry error, as registry can disallow delete for various reasons + """ + nameservers = [n[0] for n in self.nameservers] + hostsToDelete = self.createDeleteHostList(nameservers) + self._delete_hosts_if_not_used(hostsToDelete) def _delete_domain(self): """This domain should be deleted from the registry @@ -1431,6 +1451,8 @@ class Domain(TimeStampedModel, DomainHelper): @transition(field="state", source=[State.ON_HOLD, State.DNS_NEEDED], target=State.DELETED) def deletedInEpp(self): """Domain is deleted in epp but is saved in our database. + Subdomains will be deleted first if not in use by another domain. + Contacts for this domain will also be deleted. Error handling should be provided by the caller.""" # While we want to log errors, we want to preserve # that information when this function is called. @@ -1438,6 +1460,8 @@ class Domain(TimeStampedModel, DomainHelper): # as doing everything here would reduce reliablity. try: logger.info("deletedInEpp()-> inside _delete_domain") + self._delete_subdomains() + self._delete_contacts() self._delete_domain() self.deleted = timezone.now() except RegistryError as err: @@ -1639,6 +1663,26 @@ class Domain(TimeStampedModel, DomainHelper): ) raise e + + def _delete_contact(self, contact: PublicContact): + """Try to delete a contact. RegistryErrors will be logged. + + raises: + RegistryError: if the registry is unable to delete the contact + """ + logger.info("_delete_contact() -> Attempting to delete contact for %s from domain %s", contact.name, contact.domain) + try: + req = commands.DeletContact(id=contact.registry_id) + return registry.send(req, cleaned=True).res_data[0] + except RegistryError as error: + logger.error( + "Registry threw error when trying to delete contact id %s contact type is %s, error code is\n %s full error is %s", # noqa + contact.registry_id, + contact.contact_type, + error.code, + error, + ) + raise error def is_ipv6(self, ip: str): ip_addr = ipaddress.ip_address(ip) From 6891f5c8df34785be4452f81d0cf17a0f37bc754 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 26 Nov 2024 13:56:45 -0600 Subject: [PATCH 02/41] Rework delete from epp --- src/registrar/models/domain.py | 29 +++++++++---- src/registrar/tests/common.py | 30 +++++++++---- src/registrar/tests/test_models_domain.py | 51 ++++++++++++----------- 3 files changed, 70 insertions(+), 40 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 03a969471..37ce6c501 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -744,7 +744,12 @@ class Domain(TimeStampedModel, DomainHelper): successTotalNameservers = len(oldNameservers) - deleteCount + addToDomainCount - self._delete_hosts_if_not_used(hostsToDelete=deleted_values) + try: + self._delete_hosts_if_not_used(hostsToDelete=deleted_values) + except: + # in this case we don't care if there's an error, and it will be logged in the function. + pass + if successTotalNameservers < 2: try: self.dns_needed() @@ -1032,19 +1037,28 @@ class Domain(TimeStampedModel, DomainHelper): RegistryErrors will be logged and raised. Additional error handling should be provided by the caller. """ + logger.info("Deleting contacts for %s", self.name) contacts = self._cache.get("contacts") - for contact in contacts: - self._delete_contact(contact) + logger.debug("Contacts to delete for %s inside _delete_contacts -> %s", self.name, contacts) + if contacts: + for contact in contacts: + self._delete_contact(contact) + def _delete_subdomains(self): """Subdomains of this domain should be deleted from the registry. Subdomains which are used by other domains (eg as a hostname) will not be deleted. - Supresses registry error, as registry can disallow delete for various reasons + raises: + RegistryError: if any subdomain cannot be deleted """ + logger.info("Deleting nameservers for %s", self.name) nameservers = [n[0] for n in self.nameservers] - hostsToDelete = self.createDeleteHostList(nameservers) + logger.info("Nameservers found: %s", nameservers) + hostsToDelete, _ = self.createDeleteHostList(nameservers) + logger.debug("HostsToDelete from %s inside _delete_subdomains -> %s", self.name, hostsToDelete) + self._delete_hosts_if_not_used(hostsToDelete) def _delete_domain(self): @@ -1665,7 +1679,7 @@ class Domain(TimeStampedModel, DomainHelper): raise e def _delete_contact(self, contact: PublicContact): - """Try to delete a contact. RegistryErrors will be logged. + """Try to delete a contact from the registry. raises: RegistryError: if the registry is unable to delete the contact @@ -1790,7 +1804,6 @@ class Domain(TimeStampedModel, DomainHelper): """delete the host object in registry, will only delete the host object, if it's not being used by another domain Performs just the DeleteHost epp call - Supresses regstry error, as registry can disallow delete for various reasons Args: hostsToDelete (list[str])- list of nameserver/host names to remove Returns: @@ -1808,6 +1821,8 @@ class Domain(TimeStampedModel, DomainHelper): logger.info("Did not remove host %s because it is in use on another domain." % nameserver) else: logger.error("Error _delete_hosts_if_not_used, code was %s error was %s" % (e.code, e)) + + raise e def _fix_unknown_state(self, cleaned): """ diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 4edfbe680..3807534b2 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1279,6 +1279,15 @@ class MockEppLib(TestCase): hosts=["fake.host.com"], ) + infoDomainSharedHost = fakedEppObject( + "sharedHost.gov", + cr_date=make_aware(datetime(2023, 5, 25, 19, 45, 35)), + contacts=[], + hosts=[ + "ns1.sharedhost.com", + ], + ) + infoDomainThreeHosts = fakedEppObject( "my-nameserver.gov", cr_date=make_aware(datetime(2023, 5, 25, 19, 45, 35)), @@ -1496,10 +1505,7 @@ class MockEppLib(TestCase): case commands.UpdateHost: return self.mockUpdateHostCommands(_request, cleaned) case commands.DeleteHost: - return MagicMock( - res_data=[self.mockDataHostChange], - code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, - ) + return self.mockDeletHostCommands(_request, cleaned) case commands.CheckDomain: return self.mockCheckDomainCommand(_request, cleaned) case commands.DeleteDomain: @@ -1551,6 +1557,16 @@ class MockEppLib(TestCase): res_data=[self.mockDataHostChange], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, ) + + def mockDeletHostCommands(self, _request, cleaned): + hosts = getattr(_request, "name", None).hosts + for host in hosts: + if "sharedhost.com" in host: + raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) + return MagicMock( + res_data=[self.mockDataHostChange], + code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, + ) def mockUpdateDomainCommands(self, _request, cleaned): if getattr(_request, "name", None) == "dnssec-invalid.gov": @@ -1563,10 +1579,7 @@ class MockEppLib(TestCase): def mockDeleteDomainCommands(self, _request, cleaned): if getattr(_request, "name", None) == "failDelete.gov": - name = getattr(_request, "name", None) - fake_nameserver = "ns1.failDelete.gov" - if name in fake_nameserver: - raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) + raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) return None def mockRenewDomainCommand(self, _request, cleaned): @@ -1636,6 +1649,7 @@ class MockEppLib(TestCase): "subdomainwoip.gov": (self.mockDataInfoDomainSubdomainNoIP, None), "ddomain3.gov": (self.InfoDomainWithContacts, None), "igorville.gov": (self.InfoDomainWithContacts, None), + "sharingiscaring.gov": (self.infoDomainSharedHost, None), } # Retrieve the corresponding values from the dictionary diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index bbd1e3f54..f39c485c7 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -2585,6 +2585,7 @@ class TestAnalystDelete(MockEppLib): self.domain_on_hold, _ = Domain.objects.get_or_create(name="fake-on-hold.gov", state=Domain.State.ON_HOLD) def tearDown(self): + Host.objects.all().delete() Domain.objects.all().delete() super().tearDown() @@ -2597,39 +2598,39 @@ class TestAnalystDelete(MockEppLib): The deleted date is set. """ - with less_console_noise(): - # Put the domain in client hold - self.domain.place_client_hold() - # Delete it... - self.domain.deletedInEpp() - self.domain.save() - self.mockedSendFunction.assert_has_calls( - [ - call( - commands.DeleteDomain(name="fake.gov"), - cleaned=True, - ) - ] - ) - # Domain itself should not be deleted - self.assertNotEqual(self.domain, None) - # Domain should have the right state - self.assertEqual(self.domain.state, Domain.State.DELETED) - # Domain should have a deleted - self.assertNotEqual(self.domain.deleted, None) - # Cache should be invalidated - self.assertEqual(self.domain._cache, {}) + # with less_console_noise(): + # Put the domain in client hold + self.domain.place_client_hold() + # Delete it... + self.domain.deletedInEpp() + self.domain.save() + self.mockedSendFunction.assert_has_calls( + [ + call( + commands.DeleteDomain(name="fake.gov"), + cleaned=True, + ) + ] + ) + # Domain itself should not be deleted + self.assertNotEqual(self.domain, None) + # Domain should have the right state + self.assertEqual(self.domain.state, Domain.State.DELETED) + # Domain should have a deleted + self.assertNotEqual(self.domain.deleted, None) + # Cache should be invalidated + self.assertEqual(self.domain._cache, {}) def test_deletion_is_unsuccessful(self): """ Scenario: Domain deletion is unsuccessful - When a subdomain exists + When a subdomain exists that is in use by another domain Then a client error is returned of code 2305 And `state` is not set to `DELETED` """ with less_console_noise(): # Desired domain - domain, _ = Domain.objects.get_or_create(name="failDelete.gov", state=Domain.State.ON_HOLD) + domain, _ = Domain.objects.get_or_create(name="sharingiscaring.gov", state=Domain.State.ON_HOLD) # Put the domain in client hold domain.place_client_hold() # Delete it @@ -2640,7 +2641,7 @@ class TestAnalystDelete(MockEppLib): self.mockedSendFunction.assert_has_calls( [ call( - commands.DeleteDomain(name="failDelete.gov"), + commands.DeleteHost(name=common.HostObjSet(hosts=['ns1.sharedhost.com'])), cleaned=True, ) ] From b5e4f8b40c0bf062f203d7e6dc05e305d3b05e8b Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 3 Dec 2024 15:04:25 -0600 Subject: [PATCH 03/41] update deletion process and tests --- src/registrar/models/domain.py | 39 ++++++++++++------- src/registrar/tests/common.py | 13 +++++++ src/registrar/tests/test_models_domain.py | 46 ++++++++++++++++++++++- 3 files changed, 84 insertions(+), 14 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 37ce6c501..2f5524ab4 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -254,7 +254,7 @@ class Domain(TimeStampedModel, DomainHelper): return not cls.available(domain) @Cache - def contacts(self) -> dict[str, str]: + def registry_contacts(self) -> dict[str, str]: """ Get a dictionary of registry IDs for the contacts for this domain. @@ -263,7 +263,10 @@ class Domain(TimeStampedModel, DomainHelper): { PublicContact.ContactTypeChoices.REGISTRANT: "jd1234", PublicContact.ContactTypeChoices.ADMINISTRATIVE: "sh8013",...} """ - raise NotImplementedError() + if self._cache.get("contacts"): + return self._cache.get("contacts") + else: + return self._get_property("contacts") @Cache def creation_date(self) -> date: @@ -1032,17 +1035,19 @@ class Domain(TimeStampedModel, DomainHelper): logger.error(f"registry error removing client hold: {err}") raise (err) - def _delete_contacts(self): - """Contacts associated with this domain will be deleted. - RegistryErrors will be logged and raised. Additional - error handling should be provided by the caller. + def _delete_nonregistrant_contacts(self): + """Non-registrant contacts associated with this domain will be deleted. + RegistryErrors will be logged and raised. Error + handling should be provided by the caller. """ logger.info("Deleting contacts for %s", self.name) - contacts = self._cache.get("contacts") + contacts = self.registry_contacts logger.debug("Contacts to delete for %s inside _delete_contacts -> %s", self.name, contacts) if contacts: - for contact in contacts: - self._delete_contact(contact) + for contact, id in contacts.items(): + # registrants have to be deleted after the domain + if contact != PublicContact.ContactTypeChoices.REGISTRANT: + self._delete_contact(contact, id) def _delete_subdomains(self): @@ -1067,6 +1072,13 @@ class Domain(TimeStampedModel, DomainHelper): request = commands.DeleteDomain(name=self.name) registry.send(request, cleaned=True) + def _delete_domain_registrant(self): + """This domain's registrant should be deleted from the registry + may raises RegistryError, should be caught or handled correctly by caller""" + registrantID = self.registrant_contact.registry_id + request = commands.DeleteContact(id=registrantID) + registry.send(request, cleaned=True) + def __str__(self) -> str: return self.name @@ -1475,8 +1487,9 @@ class Domain(TimeStampedModel, DomainHelper): try: logger.info("deletedInEpp()-> inside _delete_domain") self._delete_subdomains() - self._delete_contacts() + self._delete_nonregistrant_contacts() self._delete_domain() + self._delete_domain_registrant() self.deleted = timezone.now() except RegistryError as err: logger.error(f"Could not delete domain. Registry returned error: {err}") @@ -1678,15 +1691,15 @@ class Domain(TimeStampedModel, DomainHelper): raise e - def _delete_contact(self, contact: PublicContact): + def _delete_contact(self, contact_name: str, registry_id: str): """Try to delete a contact from the registry. raises: RegistryError: if the registry is unable to delete the contact """ - logger.info("_delete_contact() -> Attempting to delete contact for %s from domain %s", contact.name, contact.domain) + logger.info("_delete_contact() -> Attempting to delete contact for %s from domain %s", contact_name, self.name) try: - req = commands.DeletContact(id=contact.registry_id) + req = commands.DeleteContact(id=registry_id) return registry.send(req, cleaned=True).res_data[0] except RegistryError as error: logger.error( diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 5bfa63462..ac444c8aa 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1229,6 +1229,7 @@ class MockEppLib(TestCase): common.Status(state="serverTransferProhibited", description="", lang="en"), common.Status(state="inactive", description="", lang="en"), ], + registrant="regContact", ex_date=date(2023, 5, 25), ) @@ -1610,6 +1611,8 @@ class MockEppLib(TestCase): return self.mockInfoContactCommands(_request, cleaned) case commands.CreateContact: return self.mockCreateContactCommands(_request, cleaned) + case commands.DeleteContact: + return self.mockDeleteContactCommands(_request, cleaned) case commands.UpdateDomain: return self.mockUpdateDomainCommands(_request, cleaned) case commands.CreateHost: @@ -1731,6 +1734,7 @@ class MockEppLib(TestCase): # Define a dictionary to map request names to data and extension values request_mappings = { + "fake.gov": (self.mockDataInfoDomain, None), "security.gov": (self.infoDomainNoContact, None), "dnssec-dsdata.gov": ( self.mockDataInfoDomain, @@ -1811,6 +1815,15 @@ class MockEppLib(TestCase): # mocks a contact error on creation raise ContactError(code=ContactErrorCodes.CONTACT_TYPE_NONE) return MagicMock(res_data=[self.mockDataInfoHosts]) + + def mockDeleteContactCommands(self, _request, cleaned): + if getattr(_request, "id", None) == "fail": + raise RegistryError(code=ErrorCode.OBJECT_EXISTS) + else: + return MagicMock( + res_data=[self.mockDataInfoContact], + code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, + ) def setUp(self): """mock epp send function as this will fail locally""" diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index f39c485c7..73691bb69 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -2586,6 +2586,7 @@ class TestAnalystDelete(MockEppLib): def tearDown(self): Host.objects.all().delete() + PublicContact.objects.all().delete() Domain.objects.all().delete() super().tearDown() @@ -2643,7 +2644,7 @@ class TestAnalystDelete(MockEppLib): call( commands.DeleteHost(name=common.HostObjSet(hosts=['ns1.sharedhost.com'])), cleaned=True, - ) + ), ] ) # Domain itself should not be deleted @@ -2651,6 +2652,49 @@ class TestAnalystDelete(MockEppLib): # State should not have changed self.assertEqual(domain.state, Domain.State.ON_HOLD) + def test_deletion_with_host_and_contacts(self): + """ + Scenario: Domain with related Host and Contacts is Deleted + When a contact and host exists that is tied to this domain + Then `commands.DeleteHost` is sent to the registry + Then `commands.DeleteContact` is sent to the registry + Then `commands.DeleteDomain` is sent to the registry + Then `commands.DeleteContact` is sent to the registry for the registrant contact + And `state` is set to `DELETED` + """ + # with less_console_noise(): + # Desired domain + domain, _ = Domain.objects.get_or_create(name="freeman.gov", state=Domain.State.ON_HOLD) + # Put the domain in client hold + domain.place_client_hold() + # Delete it + domain.deletedInEpp() + domain.save() + + # Check that the host and contacts are deleted, order doesn't matter + self.mockedSendFunction.assert_has_calls( + [ + call(commands.DeleteHost(name=common.HostObjSet(hosts=['fake.host.com'])), cleaned=True), + call(commands.DeleteContact(id="securityContact"), cleaned=True), + call(commands.DeleteContact(id="technicalContact"), cleaned=True), + call(commands.DeleteContact(id="adminContact"),cleaned=True,) + ], + any_order=True + ) + + # These calls need to be in order + self.mockedSendFunction.assert_has_calls( + [ + call(commands.DeleteDomain(name="freeman.gov"), cleaned=True), + call(commands.InfoContact(id="regContact"), cleaned=True), + call(commands.DeleteContact(id="regContact"), cleaned=True), + ], + ) + # Domain itself should not be deleted + self.assertNotEqual(domain, None) + # State should have changed + self.assertEqual(domain.state, Domain.State.DELETED) + def test_deletion_ready_fsm_failure(self): """ Scenario: Domain deletion is unsuccessful due to FSM rules From 27868a0aed8f1fe6fad6566788b281ca761dc163 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Dec 2024 11:24:49 -0600 Subject: [PATCH 04/41] minor fixes to tests --- src/registrar/models/domain.py | 10 ++-- src/registrar/tests/common.py | 11 ++-- src/registrar/tests/test_models_domain.py | 71 ++++++++++++----------- 3 files changed, 46 insertions(+), 46 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 2f5524ab4..9c954b073 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -161,12 +161,12 @@ class Domain(TimeStampedModel, DomainHelper): """Returns a help message for a desired state. If none is found, an empty string is returned""" help_texts = { # For now, unknown has the same message as DNS_NEEDED - cls.UNKNOWN: ("Before this domain can be used, " "you’ll need to add name server addresses."), - cls.DNS_NEEDED: ("Before this domain can be used, " "you’ll need to add name server addresses."), + cls.UNKNOWN: ("Before this domain can be used, " "you'll need to add name server addresses."), + cls.DNS_NEEDED: ("Before this domain can be used, " "you'll need to add name server addresses."), cls.READY: "This domain has name servers and is ready for use.", cls.ON_HOLD: ( "This domain is administratively paused, " - "so it can’t be edited and won’t resolve in DNS. " + "so it can't be edited and won't resolve in DNS. " "Contact help@get.gov for details." ), cls.DELETED: ("This domain has been removed and " "is no longer registered to your organization."), @@ -1060,11 +1060,11 @@ class Domain(TimeStampedModel, DomainHelper): """ logger.info("Deleting nameservers for %s", self.name) nameservers = [n[0] for n in self.nameservers] - logger.info("Nameservers found: %s", nameservers) hostsToDelete, _ = self.createDeleteHostList(nameservers) logger.debug("HostsToDelete from %s inside _delete_subdomains -> %s", self.name, hostsToDelete) - self._delete_hosts_if_not_used(hostsToDelete) + for objSet in hostsToDelete: + self._delete_hosts_if_not_used(objSet.hosts) def _delete_domain(self): """This domain should be deleted from the registry diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index ac444c8aa..72a315e9b 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1620,7 +1620,7 @@ class MockEppLib(TestCase): case commands.UpdateHost: return self.mockUpdateHostCommands(_request, cleaned) case commands.DeleteHost: - return self.mockDeletHostCommands(_request, cleaned) + return self.mockDeleteHostCommands(_request, cleaned) case commands.CheckDomain: return self.mockCheckDomainCommand(_request, cleaned) case commands.DeleteDomain: @@ -1673,11 +1673,10 @@ class MockEppLib(TestCase): code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, ) - def mockDeletHostCommands(self, _request, cleaned): - hosts = getattr(_request, "name", None).hosts - for host in hosts: - if "sharedhost.com" in host: - raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) + def mockDeleteHostCommands(self, _request, cleaned): + host = getattr(_request, "name", None) + if "sharedhost.com" in host: + raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) return MagicMock( res_data=[self.mockDataHostChange], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 73691bb69..b013c7811 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -1422,40 +1422,41 @@ class TestRegistrantNameservers(MockEppLib): And `domain.is_active` returns False """ - with less_console_noise(): - self.domainWithThreeNS.nameservers = [(self.nameserver1,)] - expectedCalls = [ - call( - commands.InfoDomain(name=self.domainWithThreeNS.name, auth_info=None), - cleaned=True, + # with less_console_noise(): + self.domainWithThreeNS.nameservers = [(self.nameserver1,)] + expectedCalls = [ + call( + commands.InfoDomain(name=self.domainWithThreeNS.name, auth_info=None), + cleaned=True, + ), + call(commands.InfoHost(name="ns1.my-nameserver-1.com"), cleaned=True), + call(commands.InfoHost(name="ns1.my-nameserver-2.com"), cleaned=True), + call(commands.InfoHost(name="ns1.cats-are-superior3.com"), cleaned=True), + call(commands.DeleteHost(name="ns1.my-nameserver-2.com"), cleaned=True), + call( + commands.UpdateDomain( + name=self.domainWithThreeNS.name, + add=[], + rem=[ + common.HostObjSet( + hosts=[ + "ns1.my-nameserver-2.com", + "ns1.cats-are-superior3.com", + ] + ), + ], + nsset=None, + keyset=None, + registrant=None, + auth_info=None, ), - call(commands.InfoHost(name="ns1.my-nameserver-1.com"), cleaned=True), - call(commands.InfoHost(name="ns1.my-nameserver-2.com"), cleaned=True), - call(commands.InfoHost(name="ns1.cats-are-superior3.com"), cleaned=True), - call(commands.DeleteHost(name="ns1.my-nameserver-2.com"), cleaned=True), - call( - commands.UpdateDomain( - name=self.domainWithThreeNS.name, - add=[], - rem=[ - common.HostObjSet( - hosts=[ - "ns1.my-nameserver-2.com", - "ns1.cats-are-superior3.com", - ] - ), - ], - nsset=None, - keyset=None, - registrant=None, - auth_info=None, - ), - cleaned=True, - ), - call(commands.DeleteHost(name="ns1.cats-are-superior3.com"), cleaned=True), - ] - self.mockedSendFunction.assert_has_calls(expectedCalls, any_order=True) - self.assertFalse(self.domainWithThreeNS.is_active()) + cleaned=True, + ), + call(commands.DeleteHost(name="ns1.cats-are-superior3.com"), cleaned=True), + ] + + self.mockedSendFunction.assert_has_calls(expectedCalls, any_order=True) + self.assertFalse(self.domainWithThreeNS.is_active()) def test_user_replaces_nameservers(self): """ @@ -2642,7 +2643,7 @@ class TestAnalystDelete(MockEppLib): self.mockedSendFunction.assert_has_calls( [ call( - commands.DeleteHost(name=common.HostObjSet(hosts=['ns1.sharedhost.com'])), + commands.DeleteHost(name='ns1.sharedhost.com'), cleaned=True, ), ] @@ -2674,7 +2675,7 @@ class TestAnalystDelete(MockEppLib): # Check that the host and contacts are deleted, order doesn't matter self.mockedSendFunction.assert_has_calls( [ - call(commands.DeleteHost(name=common.HostObjSet(hosts=['fake.host.com'])), cleaned=True), + call(commands.DeleteHost(name='fake.host.com'), cleaned=True), call(commands.DeleteContact(id="securityContact"), cleaned=True), call(commands.DeleteContact(id="technicalContact"), cleaned=True), call(commands.DeleteContact(id="adminContact"),cleaned=True,) From 9437b732c8a475d3b5216ca9c805942e3507b586 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Dec 2024 12:50:28 -0600 Subject: [PATCH 05/41] minor test fix --- src/registrar/tests/test_admin_domain.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index f02b59a91..ee275741c 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -16,6 +16,7 @@ from registrar.models import ( Host, Portfolio, ) +from registrar.models.public_contact import PublicContact from registrar.models.user_domain_role import UserDomainRole from .common import ( MockSESClient, @@ -59,6 +60,7 @@ class TestDomainAdminAsStaff(MockEppLib): def tearDown(self): super().tearDown() Host.objects.all().delete() + PublicContact.objects.all().delete() Domain.objects.all().delete() DomainInformation.objects.all().delete() DomainRequest.objects.all().delete() From 89253a1626f9e4cf6a24f0d8ed4ef203822a8e30 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Dec 2024 13:37:23 -0600 Subject: [PATCH 06/41] linter fixes --- src/registrar/models/domain.py | 25 ++++++++--------------- src/registrar/tests/common.py | 10 ++++----- src/registrar/tests/test_models_domain.py | 13 +++++++----- 3 files changed, 22 insertions(+), 26 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 9c954b073..64d29a21a 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -747,11 +747,7 @@ class Domain(TimeStampedModel, DomainHelper): successTotalNameservers = len(oldNameservers) - deleteCount + addToDomainCount - try: - self._delete_hosts_if_not_used(hostsToDelete=deleted_values) - except: - # in this case we don't care if there's an error, and it will be logged in the function. - pass + self._delete_hosts_if_not_used(hostsToDelete=deleted_values) if successTotalNameservers < 2: try: @@ -1034,10 +1030,10 @@ class Domain(TimeStampedModel, DomainHelper): # if registry error occurs, log the error, and raise it as well logger.error(f"registry error removing client hold: {err}") raise (err) - + def _delete_nonregistrant_contacts(self): """Non-registrant contacts associated with this domain will be deleted. - RegistryErrors will be logged and raised. Error + RegistryErrors will be logged and raised. Error handling should be provided by the caller. """ logger.info("Deleting contacts for %s", self.name) @@ -1048,8 +1044,7 @@ class Domain(TimeStampedModel, DomainHelper): # registrants have to be deleted after the domain if contact != PublicContact.ContactTypeChoices.REGISTRANT: self._delete_contact(contact, id) - - + def _delete_subdomains(self): """Subdomains of this domain should be deleted from the registry. Subdomains which are used by other domains (eg as a hostname) will @@ -1690,10 +1685,10 @@ class Domain(TimeStampedModel, DomainHelper): ) raise e - + def _delete_contact(self, contact_name: str, registry_id: str): """Try to delete a contact from the registry. - + raises: RegistryError: if the registry is unable to delete the contact """ @@ -1703,10 +1698,8 @@ class Domain(TimeStampedModel, DomainHelper): return registry.send(req, cleaned=True).res_data[0] except RegistryError as error: logger.error( - "Registry threw error when trying to delete contact id %s contact type is %s, error code is\n %s full error is %s", # noqa - contact.registry_id, - contact.contact_type, - error.code, + "Registry threw error when trying to delete contact %s, error: %s", # noqa + contact_name, error, ) raise error @@ -1834,7 +1827,7 @@ class Domain(TimeStampedModel, DomainHelper): logger.info("Did not remove host %s because it is in use on another domain." % nameserver) else: logger.error("Error _delete_hosts_if_not_used, code was %s error was %s" % (e.code, e)) - + raise e def _fix_unknown_state(self, cleaned): diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 72a315e9b..79c262cb9 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1672,7 +1672,7 @@ class MockEppLib(TestCase): res_data=[self.mockDataHostChange], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, ) - + def mockDeleteHostCommands(self, _request, cleaned): host = getattr(_request, "name", None) if "sharedhost.com" in host: @@ -1814,15 +1814,15 @@ class MockEppLib(TestCase): # mocks a contact error on creation raise ContactError(code=ContactErrorCodes.CONTACT_TYPE_NONE) return MagicMock(res_data=[self.mockDataInfoHosts]) - + def mockDeleteContactCommands(self, _request, cleaned): if getattr(_request, "id", None) == "fail": raise RegistryError(code=ErrorCode.OBJECT_EXISTS) else: return MagicMock( - res_data=[self.mockDataInfoContact], - code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, - ) + res_data=[self.mockDataInfoContact], + code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, + ) def setUp(self): """mock epp send function as this will fail locally""" diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index b013c7811..e381a06fe 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -2643,7 +2643,7 @@ class TestAnalystDelete(MockEppLib): self.mockedSendFunction.assert_has_calls( [ call( - commands.DeleteHost(name='ns1.sharedhost.com'), + commands.DeleteHost(name="ns1.sharedhost.com"), cleaned=True, ), ] @@ -2664,7 +2664,7 @@ class TestAnalystDelete(MockEppLib): And `state` is set to `DELETED` """ # with less_console_noise(): - # Desired domain + # Desired domain domain, _ = Domain.objects.get_or_create(name="freeman.gov", state=Domain.State.ON_HOLD) # Put the domain in client hold domain.place_client_hold() @@ -2675,12 +2675,15 @@ class TestAnalystDelete(MockEppLib): # Check that the host and contacts are deleted, order doesn't matter self.mockedSendFunction.assert_has_calls( [ - call(commands.DeleteHost(name='fake.host.com'), cleaned=True), + call(commands.DeleteHost(name="fake.host.com"), cleaned=True), call(commands.DeleteContact(id="securityContact"), cleaned=True), call(commands.DeleteContact(id="technicalContact"), cleaned=True), - call(commands.DeleteContact(id="adminContact"),cleaned=True,) + call( + commands.DeleteContact(id="adminContact"), + cleaned=True, + ), ], - any_order=True + any_order=True, ) # These calls need to be in order From f25bb9be055835866c004a827e7241ef0485c1cd Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Dec 2024 16:28:33 -0600 Subject: [PATCH 07/41] include hostname in error messages for shared hosts --- src/registrar/models/domain.py | 23 ++++++- src/registrar/tests/common.py | 1 + src/registrar/tests/test_models_domain.py | 79 +++++++++++------------ 3 files changed, 59 insertions(+), 44 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 64d29a21a..61cc539b0 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -231,6 +231,14 @@ class Domain(TimeStampedModel, DomainHelper): """Called during delete. Example: `del domain.registrant`.""" super().__delete__(obj) + def save( + self, force_insert=False, force_update=False, using=None, update_fields=None + ): + # If the domain is deleted we don't want the expiration date to be set + if self.state == self.State.DELETED and self.expiration_date: + self.expiration_date = None + super().save(force_insert, force_update, using, update_fields) + @classmethod def available(cls, domain: str) -> bool: """Check if a domain is available. @@ -1054,6 +1062,13 @@ class Domain(TimeStampedModel, DomainHelper): RegistryError: if any subdomain cannot be deleted """ logger.info("Deleting nameservers for %s", self.name) + # check if any nameservers are in use by another domain + hosts = Host.objects.filter(name__regex=r'.+{}'.format(self.name)) + for host in hosts: + if host.domain != self: + logger.error("Host %s in use by another domain: %s", host.name, host.domain) + raise RegistryError("Host in use by another domain: {}".format(host.domain), code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) + nameservers = [n[0] for n in self.nameservers] hostsToDelete, _ = self.createDeleteHostList(nameservers) logger.debug("HostsToDelete from %s inside _delete_subdomains -> %s", self.name, hostsToDelete) @@ -1070,9 +1085,10 @@ class Domain(TimeStampedModel, DomainHelper): def _delete_domain_registrant(self): """This domain's registrant should be deleted from the registry may raises RegistryError, should be caught or handled correctly by caller""" - registrantID = self.registrant_contact.registry_id - request = commands.DeleteContact(id=registrantID) - registry.send(request, cleaned=True) + if self.registrant_contact: + registrantID = self.registrant_contact.registry_id + request = commands.DeleteContact(id=registrantID) + registry.send(request, cleaned=True) def __str__(self) -> str: return self.name @@ -1486,6 +1502,7 @@ class Domain(TimeStampedModel, DomainHelper): self._delete_domain() self._delete_domain_registrant() self.deleted = timezone.now() + self.expiration_date = None except RegistryError as err: logger.error(f"Could not delete domain. Registry returned error: {err}") raise err diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 79c262cb9..16fa58104 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1676,6 +1676,7 @@ class MockEppLib(TestCase): def mockDeleteHostCommands(self, _request, cleaned): host = getattr(_request, "name", None) if "sharedhost.com" in host: + print("raising registry error") raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) return MagicMock( res_data=[self.mockDataHostChange], diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index e381a06fe..8dfb764e3 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -2584,6 +2584,7 @@ class TestAnalystDelete(MockEppLib): super().setUp() self.domain, _ = Domain.objects.get_or_create(name="fake.gov", state=Domain.State.READY) self.domain_on_hold, _ = Domain.objects.get_or_create(name="fake-on-hold.gov", state=Domain.State.ON_HOLD) + Host.objects.create(name="ns1.sharingiscaring.gov", domain=self.domain_on_hold) def tearDown(self): Host.objects.all().delete() @@ -2639,15 +2640,9 @@ class TestAnalystDelete(MockEppLib): with self.assertRaises(RegistryError) as err: domain.deletedInEpp() domain.save() + self.assertTrue(err.is_client_error() and err.code == ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) - self.mockedSendFunction.assert_has_calls( - [ - call( - commands.DeleteHost(name="ns1.sharedhost.com"), - cleaned=True, - ), - ] - ) + self.assertEqual(err.msg, "Host in use by another domain: fake-on-hold.gov") # Domain itself should not be deleted self.assertNotEqual(domain, None) # State should not have changed @@ -2663,41 +2658,43 @@ class TestAnalystDelete(MockEppLib): Then `commands.DeleteContact` is sent to the registry for the registrant contact And `state` is set to `DELETED` """ - # with less_console_noise(): - # Desired domain - domain, _ = Domain.objects.get_or_create(name="freeman.gov", state=Domain.State.ON_HOLD) - # Put the domain in client hold - domain.place_client_hold() - # Delete it - domain.deletedInEpp() - domain.save() + with less_console_noise(): + # Desired domain + domain, _ = Domain.objects.get_or_create(name="freeman.gov", state=Domain.State.ON_HOLD) + # Put the domain in client hold + domain.place_client_hold() + # Delete it + domain.deletedInEpp() + domain.save() - # Check that the host and contacts are deleted, order doesn't matter - self.mockedSendFunction.assert_has_calls( - [ - call(commands.DeleteHost(name="fake.host.com"), cleaned=True), - call(commands.DeleteContact(id="securityContact"), cleaned=True), - call(commands.DeleteContact(id="technicalContact"), cleaned=True), - call( - commands.DeleteContact(id="adminContact"), - cleaned=True, - ), - ], - any_order=True, - ) + # Check that the host and contacts are deleted, order doesn't matter + self.mockedSendFunction.assert_has_calls( + [ + call(commands.DeleteHost(name="fake.host.com"), cleaned=True), + call(commands.DeleteContact(id="securityContact"), cleaned=True), + call(commands.DeleteContact(id="technicalContact"), cleaned=True), + call( + commands.DeleteContact(id="adminContact"), + cleaned=True, + ), + ], + any_order=True, + ) + actual_calls = self.mockedSendFunction.call_args_list + print("actual_calls", actual_calls) - # These calls need to be in order - self.mockedSendFunction.assert_has_calls( - [ - call(commands.DeleteDomain(name="freeman.gov"), cleaned=True), - call(commands.InfoContact(id="regContact"), cleaned=True), - call(commands.DeleteContact(id="regContact"), cleaned=True), - ], - ) - # Domain itself should not be deleted - self.assertNotEqual(domain, None) - # State should have changed - self.assertEqual(domain.state, Domain.State.DELETED) + # These calls need to be in order + self.mockedSendFunction.assert_has_calls( + [ + call(commands.DeleteDomain(name="freeman.gov"), cleaned=True), + call(commands.InfoContact(id="regContact"), cleaned=True), + call(commands.DeleteContact(id="regContact"), cleaned=True), + ], + ) + # Domain itself should not be deleted + self.assertNotEqual(domain, None) + # State should have changed + self.assertEqual(domain.state, Domain.State.DELETED) def test_deletion_ready_fsm_failure(self): """ From dad42264bf6293765c107dac17861bec078b7357 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Dec 2024 16:32:17 -0600 Subject: [PATCH 08/41] add back in less console noise decorator --- src/registrar/tests/test_models_domain.py | 110 +++++++++++----------- 1 file changed, 55 insertions(+), 55 deletions(-) diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 8dfb764e3..8fd2b5411 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -1422,41 +1422,41 @@ class TestRegistrantNameservers(MockEppLib): And `domain.is_active` returns False """ - # with less_console_noise(): - self.domainWithThreeNS.nameservers = [(self.nameserver1,)] - expectedCalls = [ - call( - commands.InfoDomain(name=self.domainWithThreeNS.name, auth_info=None), - cleaned=True, - ), - call(commands.InfoHost(name="ns1.my-nameserver-1.com"), cleaned=True), - call(commands.InfoHost(name="ns1.my-nameserver-2.com"), cleaned=True), - call(commands.InfoHost(name="ns1.cats-are-superior3.com"), cleaned=True), - call(commands.DeleteHost(name="ns1.my-nameserver-2.com"), cleaned=True), - call( - commands.UpdateDomain( - name=self.domainWithThreeNS.name, - add=[], - rem=[ - common.HostObjSet( - hosts=[ - "ns1.my-nameserver-2.com", - "ns1.cats-are-superior3.com", - ] - ), - ], - nsset=None, - keyset=None, - registrant=None, - auth_info=None, + with less_console_noise(): + self.domainWithThreeNS.nameservers = [(self.nameserver1,)] + expectedCalls = [ + call( + commands.InfoDomain(name=self.domainWithThreeNS.name, auth_info=None), + cleaned=True, ), - cleaned=True, - ), - call(commands.DeleteHost(name="ns1.cats-are-superior3.com"), cleaned=True), - ] + call(commands.InfoHost(name="ns1.my-nameserver-1.com"), cleaned=True), + call(commands.InfoHost(name="ns1.my-nameserver-2.com"), cleaned=True), + call(commands.InfoHost(name="ns1.cats-are-superior3.com"), cleaned=True), + call(commands.DeleteHost(name="ns1.my-nameserver-2.com"), cleaned=True), + call( + commands.UpdateDomain( + name=self.domainWithThreeNS.name, + add=[], + rem=[ + common.HostObjSet( + hosts=[ + "ns1.my-nameserver-2.com", + "ns1.cats-are-superior3.com", + ] + ), + ], + nsset=None, + keyset=None, + registrant=None, + auth_info=None, + ), + cleaned=True, + ), + call(commands.DeleteHost(name="ns1.cats-are-superior3.com"), cleaned=True), + ] - self.mockedSendFunction.assert_has_calls(expectedCalls, any_order=True) - self.assertFalse(self.domainWithThreeNS.is_active()) + self.mockedSendFunction.assert_has_calls(expectedCalls, any_order=True) + self.assertFalse(self.domainWithThreeNS.is_active()) def test_user_replaces_nameservers(self): """ @@ -2601,28 +2601,28 @@ class TestAnalystDelete(MockEppLib): The deleted date is set. """ - # with less_console_noise(): - # Put the domain in client hold - self.domain.place_client_hold() - # Delete it... - self.domain.deletedInEpp() - self.domain.save() - self.mockedSendFunction.assert_has_calls( - [ - call( - commands.DeleteDomain(name="fake.gov"), - cleaned=True, - ) - ] - ) - # Domain itself should not be deleted - self.assertNotEqual(self.domain, None) - # Domain should have the right state - self.assertEqual(self.domain.state, Domain.State.DELETED) - # Domain should have a deleted - self.assertNotEqual(self.domain.deleted, None) - # Cache should be invalidated - self.assertEqual(self.domain._cache, {}) + with less_console_noise(): + # Put the domain in client hold + self.domain.place_client_hold() + # Delete it... + self.domain.deletedInEpp() + self.domain.save() + self.mockedSendFunction.assert_has_calls( + [ + call( + commands.DeleteDomain(name="fake.gov"), + cleaned=True, + ) + ] + ) + # Domain itself should not be deleted + self.assertNotEqual(self.domain, None) + # Domain should have the right state + self.assertEqual(self.domain.state, Domain.State.DELETED) + # Domain should have a deleted + self.assertNotEqual(self.domain.deleted, None) + # Cache should be invalidated + self.assertEqual(self.domain._cache, {}) def test_deletion_is_unsuccessful(self): """ From 6fdb763c0249bdc46684a0d8d2e3928a442fae43 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Dec 2024 17:10:45 -0600 Subject: [PATCH 09/41] admin fix --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 40d4befb5..042666619 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1570,7 +1570,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): "is_policy_acknowledged", ] - # For each filter_horizontal, init in admin js initFilterHorizontalWidget + # For each filter_horizontal, init in admin js extendFilterHorizontalWidgets # to activate the edit/delete/view buttons filter_horizontal = ("other_contacts",) From e8fdf0c5d376b2b94647ff782d59adfdf6d957f5 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 5 Dec 2024 10:16:40 -0600 Subject: [PATCH 10/41] revert accidental admin change --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 042666619..40d4befb5 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1570,7 +1570,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): "is_policy_acknowledged", ] - # For each filter_horizontal, init in admin js extendFilterHorizontalWidgets + # For each filter_horizontal, init in admin js initFilterHorizontalWidget # to activate the edit/delete/view buttons filter_horizontal = ("other_contacts",) From 3f79b562bd9db55af9eb5aac5bf08c3aca61a962 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 5 Dec 2024 10:58:12 -0600 Subject: [PATCH 11/41] temp test changes --- src/registrar/models/domain.py | 6 +++--- src/registrar/tests/test_views.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 61cc539b0..6ca3676f7 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -161,12 +161,12 @@ class Domain(TimeStampedModel, DomainHelper): """Returns a help message for a desired state. If none is found, an empty string is returned""" help_texts = { # For now, unknown has the same message as DNS_NEEDED - cls.UNKNOWN: ("Before this domain can be used, " "you'll need to add name server addresses."), - cls.DNS_NEEDED: ("Before this domain can be used, " "you'll need to add name server addresses."), + cls.UNKNOWN: ("Before this domain can be used, " "you’ll need to add name server addresses."), + cls.DNS_NEEDED: ("Before this domain can be used, " "you’ll need to add name server addresses."), cls.READY: "This domain has name servers and is ready for use.", cls.ON_HOLD: ( "This domain is administratively paused, " - "so it can't be edited and won't resolve in DNS. " + "so it can’t be edited and won’t resolve in DNS. " "Contact help@get.gov for details." ), cls.DELETED: ("This domain has been removed and " "is no longer registered to your organization."), diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index f46e417be..3c1f1959e 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -169,7 +169,7 @@ class HomeTests(TestWithUser): self.assertContains(response, "You don't have any registered domains.") self.assertContains(response, "Why don't I see my domain when I sign in to the registrar?") - @less_console_noise_decorator + # @less_console_noise_decorator def test_state_help_text(self): """Tests if each domain state has help text""" From 2e841711e112cf0d1482dd42e19d839d86cfbbac Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 5 Dec 2024 11:30:16 -0600 Subject: [PATCH 12/41] fix a test --- src/registrar/models/domain.py | 2 +- src/registrar/tests/test_views.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 6ca3676f7..661e958e6 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1154,7 +1154,7 @@ class Domain(TimeStampedModel, DomainHelper): Returns True if expired, False otherwise. """ if self.expiration_date is None: - return True + return self.state != self.State.DELETED now = timezone.now().date() return self.expiration_date < now diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 3c1f1959e..f46e417be 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -169,7 +169,7 @@ class HomeTests(TestWithUser): self.assertContains(response, "You don't have any registered domains.") self.assertContains(response, "Why don't I see my domain when I sign in to the registrar?") - # @less_console_noise_decorator + @less_console_noise_decorator def test_state_help_text(self): """Tests if each domain state has help text""" From aaaa4f21d238e1e46b0010741cf7be55a7a41822 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 5 Dec 2024 12:50:25 -0600 Subject: [PATCH 13/41] fix broken test --- src/registrar/models/domain.py | 13 +++++++------ src/registrar/tests/test_models_domain.py | 2 +- src/registrar/tests/test_reports.py | 8 ++++---- src/registrar/utility/csv_export.py | 3 ++- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 661e958e6..348ccf3ad 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -231,9 +231,7 @@ class Domain(TimeStampedModel, DomainHelper): """Called during delete. Example: `del domain.registrant`.""" super().__delete__(obj) - def save( - self, force_insert=False, force_update=False, using=None, update_fields=None - ): + def save(self, force_insert=False, force_update=False, using=None, update_fields=None): # If the domain is deleted we don't want the expiration date to be set if self.state == self.State.DELETED and self.expiration_date: self.expiration_date = None @@ -1063,12 +1061,15 @@ class Domain(TimeStampedModel, DomainHelper): """ logger.info("Deleting nameservers for %s", self.name) # check if any nameservers are in use by another domain - hosts = Host.objects.filter(name__regex=r'.+{}'.format(self.name)) + hosts = Host.objects.filter(name__regex=r".+{}".format(self.name)) for host in hosts: if host.domain != self: logger.error("Host %s in use by another domain: %s", host.name, host.domain) - raise RegistryError("Host in use by another domain: {}".format(host.domain), code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) - + raise RegistryError( + "Host in use by another domain: {}".format(host.domain), + code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, + ) + nameservers = [n[0] for n in self.nameservers] hostsToDelete, _ = self.createDeleteHostList(nameservers) logger.debug("HostsToDelete from %s inside _delete_subdomains -> %s", self.name, hostsToDelete) diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 8fd2b5411..e5df19d82 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -2640,7 +2640,7 @@ class TestAnalystDelete(MockEppLib): with self.assertRaises(RegistryError) as err: domain.deletedInEpp() domain.save() - + self.assertTrue(err.is_client_error() and err.code == ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) self.assertEqual(err.msg, "Host in use by another domain: fake-on-hold.gov") # Domain itself should not be deleted diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 377216aa4..0c3fad51a 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -880,18 +880,18 @@ class MemberExportTest(MockDbForIndividualTests, MockEppLib): "Email,Organization admin,Invited by,Joined date,Last active,Domain requests," "Member management,Domain management,Number of domains,Domains\n" # Content + "big_lebowski@dude.co,False,help@get.gov,2022-04-01,Invalid date,None,Viewer,True,1,cdomain1.gov\n" + "cozy_staffuser@igorville.gov,True,help@get.gov,2022-04-01,2024-02-01,Viewer,Viewer,False,0,\n" + "icy_superuser@igorville.gov,True,help@get.gov,2022-04-01,2024-02-01,Viewer Requester,Manager,False,0,\n" "meoward@rocks.com,False,big_lebowski@dude.co,2022-04-01,Invalid date,None," 'Manager,True,2,"adomain2.gov,cdomain1.gov"\n' - "big_lebowski@dude.co,False,help@get.gov,2022-04-01,Invalid date,None,Viewer,True,1,cdomain1.gov\n" - "tired_sleepy@igorville.gov,False,System,2022-04-01,Invalid date,Viewer,None,False,0,\n" - "icy_superuser@igorville.gov,True,help@get.gov,2022-04-01,2024-02-01,Viewer Requester,Manager,False,0,\n" - "cozy_staffuser@igorville.gov,True,help@get.gov,2022-04-01,2024-02-01,Viewer,Viewer,False,0,\n" "nonexistentmember_1@igorville.gov,False,help@get.gov,Unretrieved,Invited,None,Manager,False,0,\n" "nonexistentmember_2@igorville.gov,False,help@get.gov,Unretrieved,Invited,None,Viewer,False,0,\n" "nonexistentmember_3@igorville.gov,False,help@get.gov,Unretrieved,Invited,Viewer,None,False,0,\n" "nonexistentmember_4@igorville.gov,True,help@get.gov,Unretrieved," "Invited,Viewer Requester,Manager,False,0,\n" "nonexistentmember_5@igorville.gov,True,help@get.gov,Unretrieved,Invited,Viewer,Viewer,False,0,\n" + "tired_sleepy@igorville.gov,False,System,2022-04-01,Invalid date,Viewer,None,False,0,\n" ) # Normalize line endings and remove commas, # spaces and leading/trailing whitespace diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index a03e51de5..48a5f9e2d 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -415,7 +415,8 @@ class MemberExport(BaseExport): .values(*shared_columns) ) - return convert_queryset_to_dict(permissions.union(invitations), is_model=False) + members = permissions.union(invitations).order_by("email_display") + return convert_queryset_to_dict(members, is_model=False) @classmethod def get_invited_by_query(cls, object_id_query): From 8b473d5e1846d80d4495ff5b375a2136c8b14f53 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 5 Dec 2024 13:55:32 -0600 Subject: [PATCH 14/41] add error message to registry errors --- src/epplibwrapper/errors.py | 3 ++- src/registrar/admin.py | 5 ++--- src/registrar/models/domain.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/epplibwrapper/errors.py b/src/epplibwrapper/errors.py index 2b7bdd255..4ded1e5a7 100644 --- a/src/epplibwrapper/errors.py +++ b/src/epplibwrapper/errors.py @@ -62,9 +62,10 @@ class RegistryError(Exception): - 2501 - 2502 Something malicious or abusive may have occurred """ - def __init__(self, *args, code=None, **kwargs): + def __init__(self, *args, code=None, msg=None,**kwargs): super().__init__(*args, **kwargs) self.code = code + self.msg = msg def should_retry(self): return self.code == ErrorCode.COMMAND_FAILED diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 40d4befb5..6bafbab08 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2916,18 +2916,17 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): except RegistryError as err: # Using variables to get past the linter message1 = f"Cannot delete Domain when in state {obj.state}" - message2 = "This subdomain is being used as a hostname on another domain" # Human-readable mappings of ErrorCodes. Can be expanded. error_messages = { # noqa on these items as black wants to reformat to an invalid length ErrorCode.OBJECT_STATUS_PROHIBITS_OPERATION: message1, - ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION: message2, + ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION: err.msg, } message = "Cannot connect to the registry" if not err.is_connection_error(): # If nothing is found, will default to returned err - message = error_messages.get(err.code, err) + message = error_messages[err.code] self.message_user(request, f"Error deleting this Domain: {message}", messages.ERROR) except TransitionNotAllowed: if obj.state == Domain.State.DELETED: diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 348ccf3ad..f4922bfdd 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1066,7 +1066,7 @@ class Domain(TimeStampedModel, DomainHelper): if host.domain != self: logger.error("Host %s in use by another domain: %s", host.name, host.domain) raise RegistryError( - "Host in use by another domain: {}".format(host.domain), + msg="Host in use by another domain: {}".format(host.domain), code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, ) From 3dbafb52207d2c64af201a736f53e510b123b5c8 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 5 Dec 2024 14:21:02 -0600 Subject: [PATCH 15/41] up log level --- ops/manifests/manifest-ms.yaml | 2 +- src/epplibwrapper/errors.py | 4 ++-- src/registrar/admin.py | 1 + src/registrar/models/domain.py | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/ops/manifests/manifest-ms.yaml b/ops/manifests/manifest-ms.yaml index 153ee5f08..ac46f5d92 100644 --- a/ops/manifests/manifest-ms.yaml +++ b/ops/manifests/manifest-ms.yaml @@ -20,7 +20,7 @@ applications: # Tell Django where it is being hosted DJANGO_BASE_URL: https://getgov-ms.app.cloud.gov # Tell Django how much stuff to log - DJANGO_LOG_LEVEL: INFO + DJANGO_LOG_LEVEL: DEBUG # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/src/epplibwrapper/errors.py b/src/epplibwrapper/errors.py index 4ded1e5a7..d30ae93ea 100644 --- a/src/epplibwrapper/errors.py +++ b/src/epplibwrapper/errors.py @@ -62,10 +62,10 @@ class RegistryError(Exception): - 2501 - 2502 Something malicious or abusive may have occurred """ - def __init__(self, *args, code=None, msg=None,**kwargs): + def __init__(self, *args, code=None, note=None,**kwargs): super().__init__(*args, **kwargs) self.code = code - self.msg = msg + self.note = note def should_retry(self): return self.code == ErrorCode.COMMAND_FAILED diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 6bafbab08..81e4772e5 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2916,6 +2916,7 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): except RegistryError as err: # Using variables to get past the linter message1 = f"Cannot delete Domain when in state {obj.state}" + message2 = f"This subdomain is being used as a hostname on another domain: {err.note}" # Human-readable mappings of ErrorCodes. Can be expanded. error_messages = { # noqa on these items as black wants to reformat to an invalid length diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index f4922bfdd..e3a2c910a 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1066,8 +1066,8 @@ class Domain(TimeStampedModel, DomainHelper): if host.domain != self: logger.error("Host %s in use by another domain: %s", host.name, host.domain) raise RegistryError( - msg="Host in use by another domain: {}".format(host.domain), code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, + note=host.domain, ) nameservers = [n[0] for n in self.nameservers] From a9710dafde51eb48b09327f9ac6a786861c56b46 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 5 Dec 2024 14:50:49 -0600 Subject: [PATCH 16/41] more debugging --- src/epplibwrapper/errors.py | 1 + src/registrar/admin.py | 2 +- src/registrar/models/domain.py | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/epplibwrapper/errors.py b/src/epplibwrapper/errors.py index d30ae93ea..0f6ee2722 100644 --- a/src/epplibwrapper/errors.py +++ b/src/epplibwrapper/errors.py @@ -65,6 +65,7 @@ class RegistryError(Exception): def __init__(self, *args, code=None, note=None,**kwargs): super().__init__(*args, **kwargs) self.code = code + # note is a string that can be used to provide additional context self.note = note def should_retry(self): diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 81e4772e5..d26566c63 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2921,7 +2921,7 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): error_messages = { # noqa on these items as black wants to reformat to an invalid length ErrorCode.OBJECT_STATUS_PROHIBITS_OPERATION: message1, - ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION: err.msg, + ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION: message2, } message = "Cannot connect to the registry" diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index e3a2c910a..19c4f6a8d 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1505,7 +1505,7 @@ class Domain(TimeStampedModel, DomainHelper): self.deleted = timezone.now() self.expiration_date = None except RegistryError as err: - logger.error(f"Could not delete domain. Registry returned error: {err}") + logger.error(f"Could not delete domain. Registry returned error: {err}. Additional context: {err.note}") raise err except TransitionNotAllowed as err: logger.error("Could not delete domain. FSM failure: {err}") From 5e7823a6ecd3390703691063a84134363720afd3 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 5 Dec 2024 15:10:16 -0600 Subject: [PATCH 17/41] more debugging --- src/registrar/models/domain.py | 2 +- src/registrar/tests/common.py | 2 +- src/registrar/tests/test_admin_domain.py | 51 +++++++++++++++++++++++- 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 19c4f6a8d..6596232f6 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1486,7 +1486,7 @@ class Domain(TimeStampedModel, DomainHelper): self._remove_client_hold() # TODO -on the client hold ticket any additional error handling here - @transition(field="state", source=[State.ON_HOLD, State.DNS_NEEDED], target=State.DELETED) + @transition(field="state", source=[State.ON_HOLD, State.DNS_NEEDED, State.UNKNOWN], target=State.DELETED) def deletedInEpp(self): """Domain is deleted in epp but is saved in our database. Subdomains will be deleted first if not in use by another domain. diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 16fa58104..0f7923083 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1677,7 +1677,7 @@ class MockEppLib(TestCase): host = getattr(_request, "name", None) if "sharedhost.com" in host: print("raising registry error") - raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) + raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, note="otherdomain.gov") return MagicMock( res_data=[self.mockDataHostChange], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index ee275741c..57961605d 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -172,7 +172,7 @@ class TestDomainAdminAsStaff(MockEppLib): @less_console_noise_decorator def test_deletion_is_successful(self): """ - Scenario: Domain deletion is unsuccessful + Scenario: Domain deletion is successful When the domain is deleted Then a user-friendly success message is returned for displaying on the web And `state` is set to `DELETED` @@ -223,6 +223,55 @@ class TestDomainAdminAsStaff(MockEppLib): self.assertEqual(domain.state, Domain.State.DELETED) + # @less_console_noise_decorator + def test_deletion_is_unsuccessful(self): + """ + Scenario: Domain deletion is unsuccessful + When the domain is deleted and has shared subdomains + Then a user-friendly success message is returned for displaying on the web + And `state` is not set to `DELETED` + """ + domain, _ = Domain.objects.get_or_create(name="sharingiscaring.gov", state=Domain.State.ON_HOLD) + # Put in client hold + domain.place_client_hold() + # Ensure everything is displaying correctly + response = self.client.get( + "/admin/registrar/domain/{}/change/".format(domain.pk), + follow=True, + ) + self.assertEqual(response.status_code, 200) + self.assertContains(response, domain.name) + self.assertContains(response, "Remove from registry") + + # The contents of the modal should exist before and after the post. + # Check for the header + self.assertContains(response, "Are you sure you want to remove this domain from the registry?") + + # Check for some of its body + self.assertContains(response, "When a domain is removed from the registry:") + + # Check for some of the button content + self.assertContains(response, "Yes, remove from registry") + + # Test the info dialog + request = self.factory.post( + "/admin/registrar/domain/{}/change/".format(domain.pk), + {"_delete_domain": "Remove from registry", "name": domain.name}, + follow=True, + ) + request.user = self.client + with patch("django.contrib.messages.add_message") as mock_add_message: + self.admin.do_delete_domain(request, domain) + mock_add_message.assert_called_once_with( + request, + messages.ERROR, + "Error deleting this Domain: This subdomain is being used as a hostname on another domain: otherdomain.gov", + extra_tags="", + fail_silently=False, + ) + + self.assertEqual(domain.state, Domain.State.ON_HOLD) + @less_console_noise_decorator def test_deletion_ready_fsm_failure(self): """ From e87c4f78f111553ab0d185d9e242a2714703ee26 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Mon, 9 Dec 2024 13:34:34 -0600 Subject: [PATCH 18/41] use update function to delete hosts --- src/registrar/models/domain.py | 5 +++-- src/registrar/tests/test_admin_domain.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 6596232f6..c768838d5 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1074,8 +1074,9 @@ class Domain(TimeStampedModel, DomainHelper): hostsToDelete, _ = self.createDeleteHostList(nameservers) logger.debug("HostsToDelete from %s inside _delete_subdomains -> %s", self.name, hostsToDelete) - for objSet in hostsToDelete: - self._delete_hosts_if_not_used(objSet.hosts) + self.addAndRemoveHostsFromDomain(None, hostsToDelete=nameservers) + # for objSet in hostsToDelete: + # self._delete_hosts_if_not_used(objSet.hosts) def _delete_domain(self): """This domain should be deleted from the registry diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index 57961605d..aed4795a6 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -228,7 +228,7 @@ class TestDomainAdminAsStaff(MockEppLib): """ Scenario: Domain deletion is unsuccessful When the domain is deleted and has shared subdomains - Then a user-friendly success message is returned for displaying on the web + Then a user-friendly error message is returned for displaying on the web And `state` is not set to `DELETED` """ domain, _ = Domain.objects.get_or_create(name="sharingiscaring.gov", state=Domain.State.ON_HOLD) From 2730047588e8f75a02ac825b470f9a3130474a0c Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 10 Dec 2024 13:41:48 -0500 Subject: [PATCH 19/41] domain information changes done --- src/registrar/admin.py | 151 +++++++++++++++++- .../getgov-admin/domain-information-form.js | 12 +- .../helpers-portfolio-dynamic-fields.js | 24 +-- 3 files changed, 161 insertions(+), 26 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 0e8e4847a..7adb7e3ed 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -220,6 +220,14 @@ class DomainInformationAdminForm(forms.ModelForm): fields = "__all__" widgets = { "other_contacts": NoAutocompleteFilteredSelectMultiple("other_contacts", False), + "portfolio": AutocompleteSelectWithPlaceholder( + DomainRequest._meta.get_field("portfolio"), admin.site, attrs={"data-placeholder": "---------"} + ), + "sub_organization": AutocompleteSelectWithPlaceholder( + DomainRequest._meta.get_field("sub_organization"), + admin.site, + attrs={"data-placeholder": "---------", "ajax-url": "get-suborganization-list-json"}, + ), } @@ -1523,6 +1531,71 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): orderable_fk_fields = [("domain", "name")] + # Define methods to display fields from the related portfolio + def portfolio_senior_official(self, obj) -> Optional[SeniorOfficial]: + return obj.portfolio.senior_official if obj.portfolio and obj.portfolio.senior_official else None + + portfolio_senior_official.short_description = "Senior official" # type: ignore + + def portfolio_organization_type(self, obj): + return ( + DomainRequest.OrganizationChoices.get_org_label(obj.portfolio.organization_type) + if obj.portfolio and obj.portfolio.organization_type + else "-" + ) + + portfolio_organization_type.short_description = "Organization type" # type: ignore + + def portfolio_federal_type(self, obj): + return ( + BranchChoices.get_branch_label(obj.portfolio.federal_type) + if obj.portfolio and obj.portfolio.federal_type + else "-" + ) + + portfolio_federal_type.short_description = "Federal type" # type: ignore + + def portfolio_organization_name(self, obj): + return obj.portfolio.organization_name if obj.portfolio else "" + + portfolio_organization_name.short_description = "Organization name" # type: ignore + + def portfolio_federal_agency(self, obj): + return obj.portfolio.federal_agency if obj.portfolio else "" + + portfolio_federal_agency.short_description = "Federal agency" # type: ignore + + def portfolio_state_territory(self, obj): + return obj.portfolio.state_territory if obj.portfolio else "" + + portfolio_state_territory.short_description = "State, territory, or military post" # type: ignore + + def portfolio_address_line1(self, obj): + return obj.portfolio.address_line1 if obj.portfolio else "" + + portfolio_address_line1.short_description = "Address line 1" # type: ignore + + def portfolio_address_line2(self, obj): + return obj.portfolio.address_line2 if obj.portfolio else "" + + portfolio_address_line2.short_description = "Address line 2" # type: ignore + + def portfolio_city(self, obj): + return obj.portfolio.city if obj.portfolio else "" + + portfolio_city.short_description = "City" # type: ignore + + def portfolio_zipcode(self, obj): + return obj.portfolio.zipcode if obj.portfolio else "" + + portfolio_zipcode.short_description = "Zip code" # type: ignore + + def portfolio_urbanization(self, obj): + return obj.portfolio.urbanization if obj.portfolio else "" + + portfolio_urbanization.short_description = "Urbanization" # type: ignore + + # Filters list_filter = [GenericOrgFilter] @@ -1537,16 +1610,36 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): None, { "fields": [ - "portfolio", - "sub_organization", - "creator", "domain_request", "notes", ] }, ), + ( + "Requested by", + { + "fields": [ + "portfolio", + "sub_organization", + "creator", + ] + }, + ), (".gov domain", {"fields": ["domain"]}), - ("Contacts", {"fields": ["senior_official", "other_contacts", "no_other_contacts_rationale"]}), + ( + "Contacts", + { + "fields": [ + "senior_official", + "portfolio_senior_official", + "other_contacts", + "no_other_contacts_rationale", + "cisa_representative_first_name", + "cisa_representative_last_name", + "cisa_representative_email", + ] + }, + ), ("Background info", {"fields": ["anything_else"]}), ( "Type of organization", @@ -1595,10 +1688,58 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): ], }, ), + # the below three sections are for portfolio fields + ( + "Type of organization", + { + "fields": [ + "portfolio_organization_type", + "portfolio_federal_type", + ] + }, + ), + ( + "Organization name and mailing address", + { + "fields": [ + "portfolio_organization_name", + "portfolio_federal_agency", + ] + }, + ), + ( + "Show details", + { + "classes": ["collapse--dgfieldset"], + "description": "Extends organization name and mailing address", + "fields": [ + "portfolio_state_territory", + "portfolio_address_line1", + "portfolio_address_line2", + "portfolio_city", + "portfolio_zipcode", + "portfolio_urbanization", + ], + }, + ), ] # Readonly fields for analysts and superusers - readonly_fields = ("other_contacts", "is_election_board") + readonly_fields = ( + "portfolio_senior_official", + "portfolio_organization_type", + "portfolio_federal_type", + "portfolio_organization_name", + "portfolio_federal_agency", + "portfolio_state_territory", + "portfolio_address_line1", + "portfolio_address_line2", + "portfolio_city", + "portfolio_zipcode", + "portfolio_urbanization", + "other_contacts", + "is_election_board" + ) # Read only that we'll leverage for CISA Analysts analyst_readonly_fields = [ diff --git a/src/registrar/assets/src/js/getgov-admin/domain-information-form.js b/src/registrar/assets/src/js/getgov-admin/domain-information-form.js index 8139c752f..072b73720 100644 --- a/src/registrar/assets/src/js/getgov-admin/domain-information-form.js +++ b/src/registrar/assets/src/js/getgov-admin/domain-information-form.js @@ -1,17 +1,11 @@ -import { handleSuborganizationFields } from './helpers-portfolio-dynamic-fields.js'; +import { handlePortfolioSelection } from './helpers-portfolio-dynamic-fields.js'; /** - * A function for dynamic DomainInformation fields + * A function for dynamic DomainRequest fields */ export function initDynamicDomainInformationFields(){ const domainInformationPage = document.getElementById("domaininformation_form"); if (domainInformationPage) { - handleSuborganizationFields(); - } - - // DomainInformation is embedded inside domain so this should fire there too - const domainPage = document.getElementById("domain_form"); - if (domainPage) { - handleSuborganizationFields(portfolioDropdownSelector="#id_domain_info-0-portfolio", suborgDropdownSelector="#id_domain_info-0-sub_organization"); + handlePortfolioSelection(); } } diff --git a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js index 39f30b87f..ca4c4b44c 100644 --- a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js +++ b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js @@ -24,13 +24,13 @@ export function handleSuborganizationFields( function toggleSuborganizationFields() { if (portfolioDropdown.val() && !suborganizationDropdown.val()) { - showElement(requestedSuborgField); - showElement(suborgCity); - showElement(suborgStateTerritory); + if (requestedSuborgField) showElement(requestedSuborgField); + if (suborgCity) showElement(suborgCity); + if (suborgStateTerritory) showElement(suborgStateTerritory); }else { - hideElement(requestedSuborgField); - hideElement(suborgCity); - hideElement(suborgStateTerritory); + if (requestedSuborgField) hideElement(requestedSuborgField); + if (suborgCity) hideElement(suborgCity); + if (suborgStateTerritory) hideElement(suborgStateTerritory); } } @@ -504,14 +504,14 @@ export function handlePortfolioSelection() { if (portfolio_id && !suborganization_id) { // Show suborganization request fields - showElement(requestedSuborganizationField); - showElement(suborganizationCity); - showElement(suborganizationStateTerritory); + if (requestedSuborganizationField) showElement(requestedSuborganizationField); + if (suborganizationCity) showElement(suborganizationCity); + if (suborganizationStateTerritory) showElement(suborganizationStateTerritory); } else { // Hide suborganization request fields if suborganization is selected - hideElement(requestedSuborganizationField); - hideElement(suborganizationCity); - hideElement(suborganizationStateTerritory); + if (requestedSuborganizationField) hideElement(requestedSuborganizationField); + if (suborganizationCity) hideElement(suborganizationCity); + if (suborganizationStateTerritory) hideElement(suborganizationStateTerritory); } } From 787506989742024344ebbb526e8c61ccbae8b223 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 10 Dec 2024 15:37:18 -0500 Subject: [PATCH 20/41] fixed rest of domain information --- src/registrar/admin.py | 4 ++-- .../django/admin/domain_information_change_form.html | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7adb7e3ed..30b49f17e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -221,10 +221,10 @@ class DomainInformationAdminForm(forms.ModelForm): widgets = { "other_contacts": NoAutocompleteFilteredSelectMultiple("other_contacts", False), "portfolio": AutocompleteSelectWithPlaceholder( - DomainRequest._meta.get_field("portfolio"), admin.site, attrs={"data-placeholder": "---------"} + DomainInformation._meta.get_field("portfolio"), admin.site, attrs={"data-placeholder": "---------"} ), "sub_organization": AutocompleteSelectWithPlaceholder( - DomainRequest._meta.get_field("sub_organization"), + DomainInformation._meta.get_field("sub_organization"), admin.site, attrs={"data-placeholder": "---------", "ajax-url": "get-suborganization-list-json"}, ), diff --git a/src/registrar/templates/django/admin/domain_information_change_form.html b/src/registrar/templates/django/admin/domain_information_change_form.html index c5b0d54b8..487fd97e1 100644 --- a/src/registrar/templates/django/admin/domain_information_change_form.html +++ b/src/registrar/templates/django/admin/domain_information_change_form.html @@ -1,6 +1,13 @@ {% extends 'admin/change_form.html' %} {% load i18n static %} +{% block content %} + {% comment %} Stores the json endpoint in a url for easier access {% endcomment %} + {% url 'get-portfolio-json' as url %} + + {{ block.super }} +{% endblock content %} + {% block field_sets %} {% for fieldset in adminform %} {% comment %} From 42c62c2ba0830fd0a79c6f777a01006f915022dc Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 10 Dec 2024 16:29:21 -0500 Subject: [PATCH 21/41] changes to domain form --- src/registrar/admin.py | 73 +++++++++++++++++++ .../assets/src/js/getgov-admin/domain-form.js | 14 ++++ .../getgov-admin/domain-information-form.js | 3 +- .../helpers-portfolio-dynamic-fields.js | 17 +++-- .../assets/src/js/getgov-admin/main.js | 2 + .../django/admin/domain_change_form.html | 7 ++ 6 files changed, 108 insertions(+), 8 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 30b49f17e..76c935f96 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -239,6 +239,14 @@ class DomainInformationInlineForm(forms.ModelForm): fields = "__all__" widgets = { "other_contacts": NoAutocompleteFilteredSelectMultiple("other_contacts", False), + "portfolio": AutocompleteSelectWithPlaceholder( + DomainInformation._meta.get_field("portfolio"), admin.site, attrs={"data-placeholder": "---------"} + ), + "sub_organization": AutocompleteSelectWithPlaceholder( + DomainInformation._meta.get_field("sub_organization"), + admin.site, + attrs={"data-placeholder": "---------", "ajax-url": "get-suborganization-list-json"}, + ), } @@ -2716,7 +2724,72 @@ class DomainInformationInline(admin.StackedInline): template = "django/admin/includes/domain_info_inline_stacked.html" model = models.DomainInformation + # Define methods to display fields from the related portfolio + def portfolio_senior_official(self, obj) -> Optional[SeniorOfficial]: + return obj.portfolio.senior_official if obj.portfolio and obj.portfolio.senior_official else None + + portfolio_senior_official.short_description = "Senior official" # type: ignore + + def portfolio_organization_type(self, obj): + return ( + DomainRequest.OrganizationChoices.get_org_label(obj.portfolio.organization_type) + if obj.portfolio and obj.portfolio.organization_type + else "-" + ) + + portfolio_organization_type.short_description = "Organization type" # type: ignore + + def portfolio_federal_type(self, obj): + return ( + BranchChoices.get_branch_label(obj.portfolio.federal_type) + if obj.portfolio and obj.portfolio.federal_type + else "-" + ) + + portfolio_federal_type.short_description = "Federal type" # type: ignore + + def portfolio_organization_name(self, obj): + return obj.portfolio.organization_name if obj.portfolio else "" + + portfolio_organization_name.short_description = "Organization name" # type: ignore + + def portfolio_federal_agency(self, obj): + return obj.portfolio.federal_agency if obj.portfolio else "" + + portfolio_federal_agency.short_description = "Federal agency" # type: ignore + + def portfolio_state_territory(self, obj): + return obj.portfolio.state_territory if obj.portfolio else "" + + portfolio_state_territory.short_description = "State, territory, or military post" # type: ignore + + def portfolio_address_line1(self, obj): + return obj.portfolio.address_line1 if obj.portfolio else "" + + portfolio_address_line1.short_description = "Address line 1" # type: ignore + + def portfolio_address_line2(self, obj): + return obj.portfolio.address_line2 if obj.portfolio else "" + + portfolio_address_line2.short_description = "Address line 2" # type: ignore + + def portfolio_city(self, obj): + return obj.portfolio.city if obj.portfolio else "" + + portfolio_city.short_description = "City" # type: ignore + + def portfolio_zipcode(self, obj): + return obj.portfolio.zipcode if obj.portfolio else "" + + portfolio_zipcode.short_description = "Zip code" # type: ignore + + def portfolio_urbanization(self, obj): + return obj.portfolio.urbanization if obj.portfolio else "" + + portfolio_urbanization.short_description = "Urbanization" # type: ignore + fieldsets = copy.deepcopy(list(DomainInformationAdmin.fieldsets)) + readonly_fields = copy.deepcopy(DomainInformationAdmin.readonly_fields) analyst_readonly_fields = copy.deepcopy(DomainInformationAdmin.analyst_readonly_fields) autocomplete_fields = copy.deepcopy(DomainInformationAdmin.autocomplete_fields) diff --git a/src/registrar/assets/src/js/getgov-admin/domain-form.js b/src/registrar/assets/src/js/getgov-admin/domain-form.js index 474e2822c..8c5ab5b1c 100644 --- a/src/registrar/assets/src/js/getgov-admin/domain-form.js +++ b/src/registrar/assets/src/js/getgov-admin/domain-form.js @@ -1,3 +1,5 @@ +import { handlePortfolioSelection } from './helpers-portfolio-dynamic-fields.js'; + /** * A function that appends target="_blank" to the domain_form buttons */ @@ -28,3 +30,15 @@ export function initDomainFormTargetBlankButtons() { domainSubmitButton.addEventListener("mouseout", () => openInNewTab(domainFormElement, false)); } } + +/** + * A function for dynamic Domain fields +*/ +export function initDynamicDomainFields(){ + const domainPage = document.getElementById("domain_form"); + if (domainPage) { + console.log("handling domain page"); + handlePortfolioSelection("#id_domain_info-0-portfolio", + "#id_domain_info-0-sub_organization"); + } +} diff --git a/src/registrar/assets/src/js/getgov-admin/domain-information-form.js b/src/registrar/assets/src/js/getgov-admin/domain-information-form.js index 072b73720..6c79bd32a 100644 --- a/src/registrar/assets/src/js/getgov-admin/domain-information-form.js +++ b/src/registrar/assets/src/js/getgov-admin/domain-information-form.js @@ -1,11 +1,12 @@ import { handlePortfolioSelection } from './helpers-portfolio-dynamic-fields.js'; /** - * A function for dynamic DomainRequest fields + * A function for dynamic DomainInformation fields */ export function initDynamicDomainInformationFields(){ const domainInformationPage = document.getElementById("domaininformation_form"); if (domainInformationPage) { + console.log("handling domain information page"); handlePortfolioSelection(); } } diff --git a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js index ca4c4b44c..64813df86 100644 --- a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js +++ b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js @@ -48,10 +48,13 @@ export function handleSuborganizationFields( * * IMPORTANT NOTE: The logic in this method is paired dynamicPortfolioFields */ -export function handlePortfolioSelection() { +export function handlePortfolioSelection( + portfolioDropdownSelector="#id_portfolio", + suborgDropdownSelector="#id_sub_organization" +) { // These dropdown are select2 fields so they must be interacted with via jquery - const portfolioDropdown = django.jQuery("#id_portfolio"); - const suborganizationDropdown = django.jQuery("#id_sub_organization"); + const portfolioDropdown = django.jQuery(portfolioDropdownSelector); + const suborganizationDropdown = django.jQuery(suborgDropdownSelector); const suborganizationField = document.querySelector(".field-sub_organization"); const requestedSuborganizationField = document.querySelector(".field-requested_suborganization"); const suborganizationCity = document.querySelector(".field-suborganization_city"); @@ -440,8 +443,8 @@ export function handlePortfolioSelection() { showElement(portfolioSeniorOfficialField); // Hide fields not applicable when a portfolio is selected - hideElement(otherEmployeesField); - hideElement(noOtherContactsRationaleField); + if (otherEmployeesField) hideElement(otherEmployeesField); + if (noOtherContactsRationaleField) hideElement(noOtherContactsRationaleField); hideElement(cisaRepresentativeFirstNameField); hideElement(cisaRepresentativeLastNameField); hideElement(cisaRepresentativeEmailField); @@ -463,8 +466,8 @@ export function handlePortfolioSelection() { // Show fields that are relevant when no portfolio is selected showElement(seniorOfficialField); hideElement(portfolioSeniorOfficialField); - showElement(otherEmployeesField); - showElement(noOtherContactsRationaleField); + if (otherEmployeesField) showElement(otherEmployeesField); + if (noOtherContactsRationaleField) showElement(noOtherContactsRationaleField); showElement(cisaRepresentativeFirstNameField); showElement(cisaRepresentativeLastNameField); showElement(cisaRepresentativeEmailField); diff --git a/src/registrar/assets/src/js/getgov-admin/main.js b/src/registrar/assets/src/js/getgov-admin/main.js index ec9aeeedf..64be572b2 100644 --- a/src/registrar/assets/src/js/getgov-admin/main.js +++ b/src/registrar/assets/src/js/getgov-admin/main.js @@ -14,6 +14,7 @@ import { import { initDomainFormTargetBlankButtons } from './domain-form.js'; import { initDynamicPortfolioFields } from './portfolio-form.js'; import { initDynamicDomainInformationFields } from './domain-information-form.js'; +import { initDynamicDomainFields } from './domain-form.js'; // General initModals(); @@ -33,6 +34,7 @@ initDynamicDomainRequestFields(); // Domain initDomainFormTargetBlankButtons(); +initDynamicDomainFields(); // Portfolio initDynamicPortfolioFields(); diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index 662328660..7aa0034b9 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -1,6 +1,13 @@ {% extends 'admin/change_form.html' %} {% load i18n static %} +{% block content %} + {% comment %} Stores the json endpoint in a url for easier access {% endcomment %} + {% url 'get-portfolio-json' as url %} + + {{ block.super }} +{% endblock content %} + {% block field_sets %}
From 41c148b15e2518e29e197a9581b06a88c4234aa0 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 10 Dec 2024 17:59:38 -0500 Subject: [PATCH 22/41] init refactor of portfolio form js --- .../helpers-portfolio-dynamic-fields.js | 66 ++-- .../src/js/getgov-admin/portfolio-form.js | 297 +++++++++--------- 2 files changed, 186 insertions(+), 177 deletions(-) diff --git a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js index 64813df86..96c4290e8 100644 --- a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js +++ b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js @@ -4,41 +4,41 @@ import { hideElement, showElement } from './helpers-admin.js'; * Helper function that handles business logic for the suborganization field. * Can be used anywhere the suborganization dropdown exists */ -export function handleSuborganizationFields( - portfolioDropdownSelector="#id_portfolio", - suborgDropdownSelector="#id_sub_organization", - requestedSuborgFieldSelector=".field-requested_suborganization", - suborgCitySelector=".field-suborganization_city", - suborgStateTerritorySelector=".field-suborganization_state_territory" -) { - // These dropdown are select2 fields so they must be interacted with via jquery - const portfolioDropdown = django.jQuery(portfolioDropdownSelector) - const suborganizationDropdown = django.jQuery(suborgDropdownSelector) - const requestedSuborgField = document.querySelector(requestedSuborgFieldSelector); - const suborgCity = document.querySelector(suborgCitySelector); - const suborgStateTerritory = document.querySelector(suborgStateTerritorySelector); - if (!suborganizationDropdown || !requestedSuborgField || !suborgCity || !suborgStateTerritory) { - console.error("Requested suborg fields not found."); - return; - } +// export function handleSuborganizationFields( +// portfolioDropdownSelector="#id_portfolio", +// suborgDropdownSelector="#id_sub_organization", +// requestedSuborgFieldSelector=".field-requested_suborganization", +// suborgCitySelector=".field-suborganization_city", +// suborgStateTerritorySelector=".field-suborganization_state_territory" +// ) { +// // These dropdown are select2 fields so they must be interacted with via jquery +// const portfolioDropdown = django.jQuery(portfolioDropdownSelector) +// const suborganizationDropdown = django.jQuery(suborgDropdownSelector) +// const requestedSuborgField = document.querySelector(requestedSuborgFieldSelector); +// const suborgCity = document.querySelector(suborgCitySelector); +// const suborgStateTerritory = document.querySelector(suborgStateTerritorySelector); +// if (!suborganizationDropdown || !requestedSuborgField || !suborgCity || !suborgStateTerritory) { +// console.error("Requested suborg fields not found."); +// return; +// } - function toggleSuborganizationFields() { - if (portfolioDropdown.val() && !suborganizationDropdown.val()) { - if (requestedSuborgField) showElement(requestedSuborgField); - if (suborgCity) showElement(suborgCity); - if (suborgStateTerritory) showElement(suborgStateTerritory); - }else { - if (requestedSuborgField) hideElement(requestedSuborgField); - if (suborgCity) hideElement(suborgCity); - if (suborgStateTerritory) hideElement(suborgStateTerritory); - } - } +// function toggleSuborganizationFields() { +// if (portfolioDropdown.val() && !suborganizationDropdown.val()) { +// if (requestedSuborgField) showElement(requestedSuborgField); +// if (suborgCity) showElement(suborgCity); +// if (suborgStateTerritory) showElement(suborgStateTerritory); +// }else { +// if (requestedSuborgField) hideElement(requestedSuborgField); +// if (suborgCity) hideElement(suborgCity); +// if (suborgStateTerritory) hideElement(suborgStateTerritory); +// } +// } - // Run the function once on page startup, then attach an event listener - toggleSuborganizationFields(); - suborganizationDropdown.on("change", toggleSuborganizationFields); - portfolioDropdown.on("change", toggleSuborganizationFields); -} +// // Run the function once on page startup, then attach an event listener +// toggleSuborganizationFields(); +// suborganizationDropdown.on("change", toggleSuborganizationFields); +// portfolioDropdown.on("change", toggleSuborganizationFields); +// } /** diff --git a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js index f001bf39b..ada162681 100644 --- a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js +++ b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js @@ -4,69 +4,87 @@ import { hideElement, showElement } from './helpers-admin.js'; * A function for dynamically changing some fields on the portfolio admin model * IMPORTANT NOTE: The logic in this function is paired handlePortfolioSelection and should be refactored once we solidify our requirements. */ -export function initDynamicPortfolioFields(){ +function handlePortfolioFields(){ - // the federal agency change listener fires on page load, which we don't want. - var isInitialPageLoad = true + let isPageLoading = true + let seniorOfficialContactList = document.querySelector(".field-senior_official .dja-address-contact-list"); + const federalAgency = document.querySelector(".field-federal_agency"); + // $ symbolically denotes that this is using jQuery + let $federalAgency = django.jQuery("#id_federal_agency"); + let organizationType = document.getElementById("id_organization_type"); + let readonlyOrganizationType = document.querySelector(".field-organization_type .readonly"); + let organizationName = document.querySelector(".field-organization_name"); + let federalType = document.querySelector(".field-federal_type"); + let urbanization = document.querySelector(".field-urbanization"); + let stateTerritory = document.getElementById("id_state_territory"); + let $seniorOfficial = django.jQuery("#id_senior_official"); + let readonlySeniorOfficial = document.querySelector(".field-senior_official .readonly"); - // This is the additional information that exists beneath the SO element. - var contactList = document.querySelector(".field-senior_official .dja-address-contact-list"); - const federalAgencyContainer = document.querySelector(".field-federal_agency"); - document.addEventListener('DOMContentLoaded', function() { - - let isPortfolioPage = document.getElementById("portfolio_form"); - if (!isPortfolioPage) { - return; - } - - // $ symbolically denotes that this is using jQuery - let $federalAgency = django.jQuery("#id_federal_agency"); - let organizationType = document.getElementById("id_organization_type"); - let readonlyOrganizationType = document.querySelector(".field-organization_type .readonly"); - - let organizationNameContainer = document.querySelector(".field-organization_name"); - let federalType = document.querySelector(".field-federal_type"); - - if ($federalAgency && (organizationType || readonlyOrganizationType)) { - // Attach the change event listener - $federalAgency.on("change", function() { - handleFederalAgencyChange($federalAgency, organizationType, readonlyOrganizationType, organizationNameContainer, federalType); + function getFederalTypeFromAgency(agency) { + let federalPortfolioApi = document.getElementById("federal_and_portfolio_types_from_agency_json_url").value; + return fetch(`${federalPortfolioApi}?&agency_name=${agency}`) + .then(response => { + const statusCode = response.status; + return response.json().then(data => ({ statusCode, data })); + }) + .then(({ statusCode, data }) => { + if (data.error) { + console.error("Error in AJAX call: " + data.error); + return; + } + return data.federal_type + }) + .catch(error => { + console.error("Error fetching federal and portfolio types: ", error); + return null }); - } - - // Handle dynamically hiding the urbanization field - let urbanizationField = document.querySelector(".field-urbanization"); - let stateTerritory = document.getElementById("id_state_territory"); - if (urbanizationField && stateTerritory) { - // Execute this function once on load - handleStateTerritoryChange(stateTerritory, urbanizationField); + } - // Attach the change event listener for state/territory - stateTerritory.addEventListener("change", function() { - handleStateTerritoryChange(stateTerritory, urbanizationField); + function getSeniorOfficialFromAgency(agency, seniorOfficialAddUrl) { + let seniorOfficialApi = document.getElementById("senior_official_from_agency_json_url").value; + return fetch(`${seniorOfficialApi}?agency_name=${agency}`) + .then(response => { + const statusCode = response.status; + return response.json().then(data => ({ statusCode, data })); + }) + .then(({ statusCode, data }) => { + if (data.error) { + if (statusCode === 404) { + + if ($seniorOfficial && $seniorOfficial.length > 0) { + $seniorOfficial.val("").trigger("change"); + } else { + // Show the "create one now" text if this field is none in readonly mode. + readonlySeniorOfficial.innerHTML = `No senior official found. Create one now.`; + } + + console.warn("Record not found: " + data.error); + } else { + console.error("Error in AJAX call: " + data.error); + } + return null; + } else { + return data; + } + }) + .catch(error => { + console.error("Error fetching senior official: ", error) + return null; }); - } - - // Handle hiding the organization name field when the organization_type is federal. - // Run this first one page load, then secondly on a change event. - handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType); - organizationType.addEventListener("change", function() { - handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType); - }); - }); - + } + function handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType) { if (organizationType && organizationNameContainer) { let selectedValue = organizationType.value; if (selectedValue === "federal") { hideElement(organizationNameContainer); - showElement(federalAgencyContainer); + showElement(federalAgency); if (federalType) { showElement(federalType); } } else { showElement(organizationNameContainer); - hideElement(federalAgencyContainer); + hideElement(federalAgency); if (federalType) { hideElement(federalType); } @@ -75,106 +93,62 @@ export function initDynamicPortfolioFields(){ } function handleFederalAgencyChange(federalAgency, organizationType, readonlyOrganizationType, organizationNameContainer, federalType) { - // Don't do anything on page load - if (isInitialPageLoad) { - isInitialPageLoad = false; - return; - } + if (!isPageLoading) { - // Set the org type to federal if an agency is selected - let selectedText = federalAgency.find("option:selected").text(); - - // There isn't a federal senior official associated with null records - if (!selectedText) { - return; - } - - let organizationTypeValue = organizationType ? organizationType.value : readonlyOrganizationType.innerText.toLowerCase(); - if (selectedText !== "Non-Federal Agency") { - if (organizationTypeValue !== "federal") { - if (organizationType){ - organizationType.value = "federal"; - }else { - readonlyOrganizationType.innerText = "Federal" - } - } - }else { - if (organizationTypeValue === "federal") { - if (organizationType){ - organizationType.value = ""; - }else { - readonlyOrganizationType.innerText = "-" - } - } - } - - handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType); - - // Determine if any changes are necessary to the display of portfolio type or federal type - // based on changes to the Federal Agency - let federalPortfolioApi = document.getElementById("federal_and_portfolio_types_from_agency_json_url").value; - fetch(`${federalPortfolioApi}?&agency_name=${selectedText}`) - .then(response => { - const statusCode = response.status; - return response.json().then(data => ({ statusCode, data })); - }) - .then(({ statusCode, data }) => { - if (data.error) { - console.error("Error in AJAX call: " + data.error); + let selectedFederalAgency = federalAgency.find("option:selected").text(); + // There isn't a federal senior official associated with null records + if (!selectedFederalAgency) { return; } - updateReadOnly(data.federal_type, '.field-federal_type'); - }) - .catch(error => console.error("Error fetching federal and portfolio types: ", error)); - // Hide the contactList initially. - // If we can update the contact information, it'll be shown again. - hideElement(contactList.parentElement); - - let seniorOfficialAddUrl = document.getElementById("senior-official-add-url").value; - let $seniorOfficial = django.jQuery("#id_senior_official"); - let readonlySeniorOfficial = document.querySelector(".field-senior_official .readonly"); - let seniorOfficialApi = document.getElementById("senior_official_from_agency_json_url").value; - fetch(`${seniorOfficialApi}?agency_name=${selectedText}`) - .then(response => { - const statusCode = response.status; - return response.json().then(data => ({ statusCode, data })); - }) - .then(({ statusCode, data }) => { - if (data.error) { - // Clear the field if the SO doesn't exist. - if (statusCode === 404) { - if ($seniorOfficial && $seniorOfficial.length > 0) { - $seniorOfficial.val("").trigger("change"); + let organizationTypeValue = organizationType ? organizationType.value : readonlyOrganizationType.innerText.toLowerCase(); + if (selectedFederalAgency !== "Non-Federal Agency") { + if (organizationTypeValue !== "federal") { + if (organizationType){ + organizationType.value = "federal"; }else { - // Show the "create one now" text if this field is none in readonly mode. - readonlySeniorOfficial.innerHTML = `No senior official found. Create one now.`; + readonlyOrganizationType.innerText = "Federal" + } + } + } else { + if (organizationTypeValue === "federal") { + if (organizationType){ + organizationType.value = ""; + }else { + readonlyOrganizationType.innerText = "-" } - console.warn("Record not found: " + data.error); - }else { - console.error("Error in AJAX call: " + data.error); } - return; } - // Update the "contact details" blurb beneath senior official - updateContactInfo(data); - showElement(contactList.parentElement); + handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType); + + // Determine if any changes are necessary to the display of portfolio type or federal type + // based on changes to the Federal Agency + getFederalTypeFromAgency(selectedFederalAgency).then((federalType) => updateReadOnly(federalType, '.field-federal_type')); - // Get the associated senior official with this federal agency - let seniorOfficialId = data.id; - let seniorOfficialName = [data.first_name, data.last_name].join(" "); - if ($seniorOfficial && $seniorOfficial.length > 0) { - // If the senior official is a dropdown field, edit that - updateSeniorOfficialDropdown($seniorOfficial, seniorOfficialId, seniorOfficialName); - }else { - if (readonlySeniorOfficial) { - let seniorOfficialLink = `${seniorOfficialName}` - readonlySeniorOfficial.innerHTML = seniorOfficialName ? seniorOfficialLink : "-"; + hideElement(seniorOfficialContactList.parentElement); + let seniorOfficialAddUrl = document.getElementById("senior-official-add-url").value; + getSeniorOfficialFromAgency(selectedFederalAgency, seniorOfficialAddUrl).then((data) => { + // Update the "contact details" blurb beneath senior official + updateContactInfo(data); + showElement(seniorOfficialContactList.parentElement); + // Get the associated senior official with this federal agency + let seniorOfficialId = data.id; + let seniorOfficialName = [data.first_name, data.last_name].join(" "); + if ($seniorOfficial && $seniorOfficial.length > 0) { + // If the senior official is a dropdown field, edit that + updateSeniorOfficialDropdown($seniorOfficial, seniorOfficialId, seniorOfficialName); + }else { + if (readonlySeniorOfficial) { + let seniorOfficialLink = `${seniorOfficialName}` + readonlySeniorOfficial.innerHTML = seniorOfficialName ? seniorOfficialLink : "-"; + } } - } - }) - .catch(error => console.error("Error fetching senior official: ", error)); + }); + + } else { + isPageLoading = false; + } } @@ -184,7 +158,6 @@ export function initDynamicPortfolioFields(){ dropdown.val("").trigger("change"); return; } - // Add the senior official to the dropdown. // This format supports select2 - if we decide to convert this field in the future. if (dropdown.find(`option[value='${seniorOfficialId}']`).length) { @@ -227,11 +200,11 @@ export function initDynamicPortfolioFields(){ } function updateContactInfo(data) { - if (!contactList) return; + if (!seniorOfficialContactList) return; - const titleSpan = contactList.querySelector(".contact_info_title"); - const emailSpan = contactList.querySelector(".contact_info_email"); - const phoneSpan = contactList.querySelector(".contact_info_phone"); + const titleSpan = seniorOfficialContactList.querySelector(".contact_info_title"); + const emailSpan = seniorOfficialContactList.querySelector(".contact_info_email"); + const phoneSpan = seniorOfficialContactList.querySelector(".contact_info_phone"); if (titleSpan) { titleSpan.textContent = data.title || "None"; @@ -239,10 +212,10 @@ export function initDynamicPortfolioFields(){ // Update the email field and the content for the clipboard if (emailSpan) { - let copyButton = contactList.querySelector(".admin-icon-group"); + let copyButton = seniorOfficialContactList.querySelector(".admin-icon-group"); emailSpan.textContent = data.email || "None"; if (data.email) { - const clipboardInput = contactList.querySelector(".admin-icon-group input"); + const clipboardInput = seniorOfficialContactList.querySelector(".admin-icon-group input"); if (clipboardInput) { clipboardInput.value = data.email; }; @@ -256,4 +229,40 @@ export function initDynamicPortfolioFields(){ phoneSpan.textContent = data.phone || "None"; }; } + + function initializePortfolioSettings() { + if (urbanization && stateTerritory) { + handleStateTerritoryChange(stateTerritory, urbanization); + } + handleOrganizationTypeChange(organizationType, organizationName, federalType); + } + + function setEventListeners() { + if ($federalAgency && (organizationType || readonlyOrganizationType)) { + $federalAgency.on("change", function() { + handleFederalAgencyChange($federalAgency, organizationType, readonlyOrganizationType, organizationName, federalType); + }); + } + if (urbanization && stateTerritory) { + stateTerritory.addEventListener("change", function() { + handleStateTerritoryChange(stateTerritory, urbanization); + }); + } + organizationType.addEventListener("change", function() { + handleOrganizationTypeChange(organizationType, organizationName, federalType); + }); + } + + // Run initial setup functions + initializePortfolioSettings(); + setEventListeners(); +} + +export function initPortfolioFields() { + document.addEventListener('DOMContentLoaded', function() { + let isPortfolioPage = document.getElementById("portfolio_form"); + if (isPortfolioPage) { + handlePortfolioFields(); + } + }); } From 1a9b6717584b189a7082f0522e405907ab8b0a4f Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 11 Dec 2024 10:11:05 -0600 Subject: [PATCH 23/41] consolidate delete domain function --- src/epplibwrapper/errors.py | 2 +- src/registrar/models/domain.py | 118 +++++--------- src/registrar/tests/test_admin_domain.py | 2 +- src/registrar/tests/test_models_domain.py | 182 ++++++++++++---------- 4 files changed, 143 insertions(+), 161 deletions(-) diff --git a/src/epplibwrapper/errors.py b/src/epplibwrapper/errors.py index 0f6ee2722..78272ff0a 100644 --- a/src/epplibwrapper/errors.py +++ b/src/epplibwrapper/errors.py @@ -62,7 +62,7 @@ class RegistryError(Exception): - 2501 - 2502 Something malicious or abusive may have occurred """ - def __init__(self, *args, code=None, note=None,**kwargs): + def __init__(self, *args, code=None, note="",**kwargs): super().__init__(*args, **kwargs) self.code = code # note is a string that can be used to provide additional context diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index c768838d5..c1357c83c 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -269,10 +269,7 @@ class Domain(TimeStampedModel, DomainHelper): { PublicContact.ContactTypeChoices.REGISTRANT: "jd1234", PublicContact.ContactTypeChoices.ADMINISTRATIVE: "sh8013",...} """ - if self._cache.get("contacts"): - return self._cache.get("contacts") - else: - return self._get_property("contacts") + raise NotImplementedError() @Cache def creation_date(self) -> date: @@ -1036,61 +1033,51 @@ class Domain(TimeStampedModel, DomainHelper): # if registry error occurs, log the error, and raise it as well logger.error(f"registry error removing client hold: {err}") raise (err) - - def _delete_nonregistrant_contacts(self): - """Non-registrant contacts associated with this domain will be deleted. - RegistryErrors will be logged and raised. Error - handling should be provided by the caller. - """ - logger.info("Deleting contacts for %s", self.name) - contacts = self.registry_contacts - logger.debug("Contacts to delete for %s inside _delete_contacts -> %s", self.name, contacts) - if contacts: - for contact, id in contacts.items(): - # registrants have to be deleted after the domain - if contact != PublicContact.ContactTypeChoices.REGISTRANT: - self._delete_contact(contact, id) - - def _delete_subdomains(self): - """Subdomains of this domain should be deleted from the registry. - Subdomains which are used by other domains (eg as a hostname) will - not be deleted. - - raises: - RegistryError: if any subdomain cannot be deleted - """ - logger.info("Deleting nameservers for %s", self.name) - # check if any nameservers are in use by another domain - hosts = Host.objects.filter(name__regex=r".+{}".format(self.name)) - for host in hosts: - if host.domain != self: - logger.error("Host %s in use by another domain: %s", host.name, host.domain) - raise RegistryError( - code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, - note=host.domain, - ) - - nameservers = [n[0] for n in self.nameservers] - hostsToDelete, _ = self.createDeleteHostList(nameservers) - logger.debug("HostsToDelete from %s inside _delete_subdomains -> %s", self.name, hostsToDelete) - - self.addAndRemoveHostsFromDomain(None, hostsToDelete=nameservers) - # for objSet in hostsToDelete: - # self._delete_hosts_if_not_used(objSet.hosts) - + def _delete_domain(self): """This domain should be deleted from the registry may raises RegistryError, should be caught or handled correctly by caller""" + + logger.info("Deleting subdomains for %s", self.name) + # check if any subdomains are in use by another domain + hosts = Host.objects.filter(name__regex=r".+{}".format(self.name)) + logger.debug("Checking if any subdomains are in use by another domain") + for host in hosts: + if host.domain != self: + logger.error("Unable to delete host: %s is in use by another domain: %s", host.name, host.domain) + raise RegistryError( + code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, + note=f"Host {host.name} is in use by {host.domain}", + ) + logger.debug("No subdomains are in use by another domain") + + nameservers = [host.name for host in hosts] + hosts = self.createDeleteHostList(hostsToDelete=nameservers) + response_code = self.addAndRemoveHostsFromDomain(hostsToAdd=None, hostsToDelete=hosts) + if response_code != ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY: + raise RegistryError(code=response_code) + + logger.debug("Deleting subordinate hosts for %s", self.name) + self._delete_hosts_if_not_used(nameservers) + + logger.debug("Deleting non-registrant contacts for %s", self.name) + contacts = PublicContact.objects.filter(domain=self) + for contact in contacts: + if contact.contact_type != PublicContact.ContactTypeChoices.REGISTRANT: + logger.debug("removing contact %s from domain %s", contact.registry_id, self.name) + self._update_domain_with_contact(contact, rem=True) + logger.debug("deleting contact %s from registry", contact.registry_id) + request = commands.DeleteContact(contact.registry_id) + registry.send(request, cleaned=True) + + logger.info("Deleting domain %s", self.name) request = commands.DeleteDomain(name=self.name) registry.send(request, cleaned=True) - def _delete_domain_registrant(self): - """This domain's registrant should be deleted from the registry - may raises RegistryError, should be caught or handled correctly by caller""" - if self.registrant_contact: - registrantID = self.registrant_contact.registry_id - request = commands.DeleteContact(id=registrantID) - registry.send(request, cleaned=True) + logger.debug("Deleting registrant contact for %s", self.name) + registrant_id = self.registrant_contact.registry_id + deleteRegistrant = commands.DeleteContact(id=registrant_id) + registry.send(deleteRegistrant, cleaned=True) def __str__(self) -> str: return self.name @@ -1487,7 +1474,7 @@ class Domain(TimeStampedModel, DomainHelper): self._remove_client_hold() # TODO -on the client hold ticket any additional error handling here - @transition(field="state", source=[State.ON_HOLD, State.DNS_NEEDED, State.UNKNOWN], target=State.DELETED) + @transition(field="state", source=[State.ON_HOLD, State.DNS_NEEDED], target=State.DELETED) def deletedInEpp(self): """Domain is deleted in epp but is saved in our database. Subdomains will be deleted first if not in use by another domain. @@ -1499,14 +1486,11 @@ class Domain(TimeStampedModel, DomainHelper): # as doing everything here would reduce reliablity. try: logger.info("deletedInEpp()-> inside _delete_domain") - self._delete_subdomains() - self._delete_nonregistrant_contacts() self._delete_domain() - self._delete_domain_registrant() self.deleted = timezone.now() self.expiration_date = None except RegistryError as err: - logger.error(f"Could not delete domain. Registry returned error: {err}. Additional context: {err.note}") + logger.error(f"Could not delete domain. Registry returned error: {err}. {err.note}") raise err except TransitionNotAllowed as err: logger.error("Could not delete domain. FSM failure: {err}") @@ -1705,24 +1689,6 @@ class Domain(TimeStampedModel, DomainHelper): raise e - def _delete_contact(self, contact_name: str, registry_id: str): - """Try to delete a contact from the registry. - - raises: - RegistryError: if the registry is unable to delete the contact - """ - logger.info("_delete_contact() -> Attempting to delete contact for %s from domain %s", contact_name, self.name) - try: - req = commands.DeleteContact(id=registry_id) - return registry.send(req, cleaned=True).res_data[0] - except RegistryError as error: - logger.error( - "Registry threw error when trying to delete contact %s, error: %s", # noqa - contact_name, - error, - ) - raise error - def is_ipv6(self, ip: str): ip_addr = ipaddress.ip_address(ip) return ip_addr.version == 6 diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index aed4795a6..57961605d 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -228,7 +228,7 @@ class TestDomainAdminAsStaff(MockEppLib): """ Scenario: Domain deletion is unsuccessful When the domain is deleted and has shared subdomains - Then a user-friendly error message is returned for displaying on the web + Then a user-friendly success message is returned for displaying on the web And `state` is not set to `DELETED` """ domain, _ = Domain.objects.get_or_create(name="sharingiscaring.gov", state=Domain.State.ON_HOLD) diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index e5df19d82..70ef4cdde 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -9,6 +9,7 @@ from django.db.utils import IntegrityError from unittest.mock import MagicMock, patch, call import datetime from django.utils.timezone import make_aware +from api.tests.common import less_console_noise_decorator from registrar.models import Domain, Host, HostIP from unittest import skip @@ -2592,6 +2593,7 @@ class TestAnalystDelete(MockEppLib): Domain.objects.all().delete() super().tearDown() + @less_console_noise_decorator def test_analyst_deletes_domain(self): """ Scenario: Analyst permanently deletes a domain @@ -2601,29 +2603,29 @@ class TestAnalystDelete(MockEppLib): The deleted date is set. """ - with less_console_noise(): - # Put the domain in client hold - self.domain.place_client_hold() - # Delete it... - self.domain.deletedInEpp() - self.domain.save() - self.mockedSendFunction.assert_has_calls( - [ - call( - commands.DeleteDomain(name="fake.gov"), - cleaned=True, - ) - ] - ) - # Domain itself should not be deleted - self.assertNotEqual(self.domain, None) - # Domain should have the right state - self.assertEqual(self.domain.state, Domain.State.DELETED) - # Domain should have a deleted - self.assertNotEqual(self.domain.deleted, None) - # Cache should be invalidated - self.assertEqual(self.domain._cache, {}) + # Put the domain in client hold + self.domain.place_client_hold() + # Delete it... + self.domain.deletedInEpp() + self.domain.save() + self.mockedSendFunction.assert_has_calls( + [ + call( + commands.DeleteDomain(name="fake.gov"), + cleaned=True, + ) + ] + ) + # Domain itself should not be deleted + self.assertNotEqual(self.domain, None) + # Domain should have the right state + self.assertEqual(self.domain.state, Domain.State.DELETED) + # Domain should have a deleted + self.assertNotEqual(self.domain.deleted, None) + # Cache should be invalidated + self.assertEqual(self.domain._cache, {}) + # @less_console_noise_decorator def test_deletion_is_unsuccessful(self): """ Scenario: Domain deletion is unsuccessful @@ -2631,23 +2633,23 @@ class TestAnalystDelete(MockEppLib): Then a client error is returned of code 2305 And `state` is not set to `DELETED` """ - with less_console_noise(): - # Desired domain - domain, _ = Domain.objects.get_or_create(name="sharingiscaring.gov", state=Domain.State.ON_HOLD) - # Put the domain in client hold - domain.place_client_hold() - # Delete it - with self.assertRaises(RegistryError) as err: - domain.deletedInEpp() - domain.save() + # Desired domain + domain, _ = Domain.objects.get_or_create(name="sharingiscaring.gov", state=Domain.State.ON_HOLD) + # Put the domain in client hold + domain.place_client_hold() + # Delete it + with self.assertRaises(RegistryError) as err: + domain.deletedInEpp() + domain.save() - self.assertTrue(err.is_client_error() and err.code == ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) - self.assertEqual(err.msg, "Host in use by another domain: fake-on-hold.gov") - # Domain itself should not be deleted - self.assertNotEqual(domain, None) - # State should not have changed - self.assertEqual(domain.state, Domain.State.ON_HOLD) + self.assertTrue(err.is_client_error() and err.code == ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) + self.assertEqual(err.msg, "Host in use by another domain: fake-on-hold.gov") + # Domain itself should not be deleted + self.assertNotEqual(domain, None) + # State should not have changed + self.assertEqual(domain.state, Domain.State.ON_HOLD) + # @less_console_noise_decorator def test_deletion_with_host_and_contacts(self): """ Scenario: Domain with related Host and Contacts is Deleted @@ -2658,44 +2660,59 @@ class TestAnalystDelete(MockEppLib): Then `commands.DeleteContact` is sent to the registry for the registrant contact And `state` is set to `DELETED` """ - with less_console_noise(): - # Desired domain - domain, _ = Domain.objects.get_or_create(name="freeman.gov", state=Domain.State.ON_HOLD) - # Put the domain in client hold - domain.place_client_hold() - # Delete it - domain.deletedInEpp() - domain.save() + # Desired domain + domain, _ = Domain.objects.get_or_create(name="freeman.gov", state=Domain.State.ON_HOLD) + # Put the domain in client hold + domain.place_client_hold() + # Delete it + domain.deletedInEpp() + domain.save() - # Check that the host and contacts are deleted, order doesn't matter - self.mockedSendFunction.assert_has_calls( - [ - call(commands.DeleteHost(name="fake.host.com"), cleaned=True), - call(commands.DeleteContact(id="securityContact"), cleaned=True), - call(commands.DeleteContact(id="technicalContact"), cleaned=True), - call( - commands.DeleteContact(id="adminContact"), - cleaned=True, + # Check that the host and contacts are deleted, order doesn't matter + self.mockedSendFunction.assert_has_calls( + [ + call( + commands.UpdateDomain( + name="freeman.gov", + add=[ + common.Status( + state=Domain.Status.CLIENT_HOLD, + description="", + lang="en", + ) + ], + rem=[], + nsset=None, + keyset=None, + registrant=None, + auth_info=None, ), - ], - any_order=True, - ) - actual_calls = self.mockedSendFunction.call_args_list - print("actual_calls", actual_calls) + cleaned=True, + ), + call(commands.DeleteHost(name="fake.host.com"), cleaned=True), + call(commands.DeleteContact(id="securityContact"), cleaned=True), + call(commands.DeleteContact(id="technicalContact"), cleaned=True), + call(commands.DeleteContact(id="adminContact"), cleaned=True), + ], + any_order=True, + ) + actual_calls = self.mockedSendFunction.call_args_list + print("actual_calls", actual_calls) - # These calls need to be in order - self.mockedSendFunction.assert_has_calls( - [ - call(commands.DeleteDomain(name="freeman.gov"), cleaned=True), - call(commands.InfoContact(id="regContact"), cleaned=True), - call(commands.DeleteContact(id="regContact"), cleaned=True), - ], - ) - # Domain itself should not be deleted - self.assertNotEqual(domain, None) - # State should have changed - self.assertEqual(domain.state, Domain.State.DELETED) + # These calls need to be in order + self.mockedSendFunction.assert_has_calls( + [ + call(commands.DeleteDomain(name="freeman.gov"), cleaned=True), + call(commands.InfoContact(id="regContact"), cleaned=True), + call(commands.DeleteContact(id="regContact"), cleaned=True), + ], + ) + # Domain itself should not be deleted + self.assertNotEqual(domain, None) + # State should have changed + self.assertEqual(domain.state, Domain.State.DELETED) + # @less_console_noise_decorator def test_deletion_ready_fsm_failure(self): """ Scenario: Domain deletion is unsuccessful due to FSM rules @@ -2707,15 +2724,14 @@ class TestAnalystDelete(MockEppLib): The deleted date is still null. """ - with less_console_noise(): - self.assertEqual(self.domain.state, Domain.State.READY) - with self.assertRaises(TransitionNotAllowed) as err: - self.domain.deletedInEpp() - self.domain.save() - self.assertTrue(err.is_client_error() and err.code == ErrorCode.OBJECT_STATUS_PROHIBITS_OPERATION) - # Domain should not be deleted - self.assertNotEqual(self.domain, None) - # Domain should have the right state - self.assertEqual(self.domain.state, Domain.State.READY) - # deleted should be null - self.assertEqual(self.domain.deleted, None) + self.assertEqual(self.domain.state, Domain.State.READY) + with self.assertRaises(TransitionNotAllowed) as err: + self.domain.deletedInEpp() + self.domain.save() + self.assertTrue(err.is_client_error() and err.code == ErrorCode.OBJECT_STATUS_PROHIBITS_OPERATION) + # Domain should not be deleted + self.assertNotEqual(self.domain, None) + # Domain should have the right state + self.assertEqual(self.domain.state, Domain.State.READY) + # deleted should be null + self.assertEqual(self.domain.deleted, None) From 7f0dc4bfca0bbc98f157d8904c80e930bcefac7f Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 11 Dec 2024 11:29:20 -0500 Subject: [PATCH 24/41] updated handling of error from lookup api, plus some variable naming --- .../src/js/getgov-admin/portfolio-form.js | 81 ++++++++++--------- 1 file changed, 43 insertions(+), 38 deletions(-) diff --git a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js index ada162681..5c34c8fce 100644 --- a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js +++ b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js @@ -7,18 +7,21 @@ import { hideElement, showElement } from './helpers-admin.js'; function handlePortfolioFields(){ let isPageLoading = true - let seniorOfficialContactList = document.querySelector(".field-senior_official .dja-address-contact-list"); - const federalAgency = document.querySelector(".field-federal_agency"); // $ symbolically denotes that this is using jQuery - let $federalAgency = django.jQuery("#id_federal_agency"); + const $seniorOfficial = django.jQuery("#id_senior_official"); + const seniorOfficialField = document.querySelector(".field-senior_official"); + const seniorOfficialAddress = seniorOfficialField.querySelector(".dja-address-contact-list"); + const seniorOfficialReadonly = seniorOfficialField.querySelector(".readonly"); + const $federalAgency = django.jQuery("#id_federal_agency"); + const federalAgencyField = document.querySelector(".field-federal_agency"); let organizationType = document.getElementById("id_organization_type"); let readonlyOrganizationType = document.querySelector(".field-organization_type .readonly"); let organizationName = document.querySelector(".field-organization_name"); let federalType = document.querySelector(".field-federal_type"); let urbanization = document.querySelector(".field-urbanization"); let stateTerritory = document.getElementById("id_state_territory"); - let $seniorOfficial = django.jQuery("#id_senior_official"); - let readonlySeniorOfficial = document.querySelector(".field-senior_official .readonly"); + + const seniorOfficialAddUrl = document.getElementById("senior-official-add-url").value; function getFederalTypeFromAgency(agency) { let federalPortfolioApi = document.getElementById("federal_and_portfolio_types_from_agency_json_url").value; @@ -40,8 +43,9 @@ function handlePortfolioFields(){ }); } - function getSeniorOfficialFromAgency(agency, seniorOfficialAddUrl) { - let seniorOfficialApi = document.getElementById("senior_official_from_agency_json_url").value; + function getSeniorOfficialFromAgency(agency) { + const seniorOfficialApi = document.getElementById("senior_official_from_agency_json_url").value; + return fetch(`${seniorOfficialApi}?agency_name=${agency}`) .then(response => { const statusCode = response.status; @@ -49,27 +53,15 @@ function handlePortfolioFields(){ }) .then(({ statusCode, data }) => { if (data.error) { - if (statusCode === 404) { - - if ($seniorOfficial && $seniorOfficial.length > 0) { - $seniorOfficial.val("").trigger("change"); - } else { - // Show the "create one now" text if this field is none in readonly mode. - readonlySeniorOfficial.innerHTML = `No senior official found. Create one now.`; - } - - console.warn("Record not found: " + data.error); - } else { - console.error("Error in AJAX call: " + data.error); - } - return null; + // Throw an error with status code and message + throw { statusCode, message: data.error }; } else { return data; } }) .catch(error => { - console.error("Error fetching senior official: ", error) - return null; + console.error("Error fetching senior official: ", error); + throw error; // Re-throw for external handling }); } @@ -78,13 +70,13 @@ function handlePortfolioFields(){ let selectedValue = organizationType.value; if (selectedValue === "federal") { hideElement(organizationNameContainer); - showElement(federalAgency); + showElement(federalAgencyField); if (federalType) { showElement(federalType); } } else { showElement(organizationNameContainer); - hideElement(federalAgency); + hideElement(federalAgencyField); if (federalType) { hideElement(federalType); } @@ -126,12 +118,12 @@ function handlePortfolioFields(){ // based on changes to the Federal Agency getFederalTypeFromAgency(selectedFederalAgency).then((federalType) => updateReadOnly(federalType, '.field-federal_type')); - hideElement(seniorOfficialContactList.parentElement); - let seniorOfficialAddUrl = document.getElementById("senior-official-add-url").value; - getSeniorOfficialFromAgency(selectedFederalAgency, seniorOfficialAddUrl).then((data) => { + hideElement(seniorOfficialAddress.parentElement); + + getSeniorOfficialFromAgency(selectedFederalAgency).then((data) => { // Update the "contact details" blurb beneath senior official updateContactInfo(data); - showElement(seniorOfficialContactList.parentElement); + showElement(seniorOfficialAddress.parentElement); // Get the associated senior official with this federal agency let seniorOfficialId = data.id; let seniorOfficialName = [data.first_name, data.last_name].join(" "); @@ -139,11 +131,24 @@ function handlePortfolioFields(){ // If the senior official is a dropdown field, edit that updateSeniorOfficialDropdown($seniorOfficial, seniorOfficialId, seniorOfficialName); }else { - if (readonlySeniorOfficial) { + if (seniorOfficialReadonly) { let seniorOfficialLink = `${seniorOfficialName}` - readonlySeniorOfficial.innerHTML = seniorOfficialName ? seniorOfficialLink : "-"; + seniorOfficialReadonly.innerHTML = seniorOfficialName ? seniorOfficialLink : "-"; } } + }) + .catch(error => { + if (error.statusCode === 404) { + // Handle "not found" senior official + if ($seniorOfficial && $seniorOfficial.length > 0) { + $seniorOfficial.val("").trigger("change"); + } else { + seniorOfficialReadonly.innerHTML = `No senior official found. Create one now.`; + } + } else { + // Handle other errors + console.error("An error occurred:", error.message); + } }); } else { @@ -200,11 +205,11 @@ function handlePortfolioFields(){ } function updateContactInfo(data) { - if (!seniorOfficialContactList) return; + if (!seniorOfficialAddress) return; - const titleSpan = seniorOfficialContactList.querySelector(".contact_info_title"); - const emailSpan = seniorOfficialContactList.querySelector(".contact_info_email"); - const phoneSpan = seniorOfficialContactList.querySelector(".contact_info_phone"); + const titleSpan = seniorOfficialAddress.querySelector(".contact_info_title"); + const emailSpan = seniorOfficialAddress.querySelector(".contact_info_email"); + const phoneSpan = seniorOfficialAddress.querySelector(".contact_info_phone"); if (titleSpan) { titleSpan.textContent = data.title || "None"; @@ -212,10 +217,10 @@ function handlePortfolioFields(){ // Update the email field and the content for the clipboard if (emailSpan) { - let copyButton = seniorOfficialContactList.querySelector(".admin-icon-group"); + let copyButton = seniorOfficialAddress.querySelector(".admin-icon-group"); emailSpan.textContent = data.email || "None"; if (data.email) { - const clipboardInput = seniorOfficialContactList.querySelector(".admin-icon-group input"); + const clipboardInput = seniorOfficialAddress.querySelector(".admin-icon-group input"); if (clipboardInput) { clipboardInput.value = data.email; }; @@ -258,7 +263,7 @@ function handlePortfolioFields(){ setEventListeners(); } -export function initPortfolioFields() { +export function initDynamicPortfolioFields() { document.addEventListener('DOMContentLoaded', function() { let isPortfolioPage = document.getElementById("portfolio_form"); if (isPortfolioPage) { From b85bfd7244f9ac791dc8557ba421e404027d0305 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 11 Dec 2024 12:02:44 -0500 Subject: [PATCH 25/41] wip variable name changes, and removing params from methods, and inheriting from global consts --- .../src/js/getgov-admin/portfolio-form.js | 61 ++++++++++--------- 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js index 5c34c8fce..b1c85a527 100644 --- a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js +++ b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js @@ -14,13 +14,14 @@ function handlePortfolioFields(){ const seniorOfficialReadonly = seniorOfficialField.querySelector(".readonly"); const $federalAgency = django.jQuery("#id_federal_agency"); const federalAgencyField = document.querySelector(".field-federal_agency"); - let organizationType = document.getElementById("id_organization_type"); - let readonlyOrganizationType = document.querySelector(".field-organization_type .readonly"); - let organizationName = document.querySelector(".field-organization_name"); - let federalType = document.querySelector(".field-federal_type"); - let urbanization = document.querySelector(".field-urbanization"); - let stateTerritory = document.getElementById("id_state_territory"); - + const organizationTypeField = document.querySelector(".field-organization_type"); + const organizationTypeReadonly = organizationTypeField.querySelector(".readonly"); + const organizationTypeDropdown = document.getElementById("id_organization_type"); + const organizationNameField = document.querySelector(".field-organization_name"); + const federalTypeField = document.querySelector(".field-federal_type"); + const urbanizationField = document.querySelector(".field-urbanization"); + const stateTerritoryDropdown = document.getElementById("id_state_territory"); + // consts for different urls const seniorOfficialAddUrl = document.getElementById("senior-official-add-url").value; function getFederalTypeFromAgency(agency) { @@ -65,26 +66,26 @@ function handlePortfolioFields(){ }); } - function handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType) { - if (organizationType && organizationNameContainer) { - let selectedValue = organizationType.value; + function handleOrganizationTypeChange() { + if (organizationTypeDropdown && organizationNameField) { + let selectedValue = organizationTypeDropdown.value; if (selectedValue === "federal") { - hideElement(organizationNameContainer); + hideElement(organizationNameField); showElement(federalAgencyField); - if (federalType) { - showElement(federalType); + if (federalTypeField) { + showElement(federalTypeField); } } else { - showElement(organizationNameContainer); + showElement(organizationNameField); hideElement(federalAgencyField); - if (federalType) { - hideElement(federalType); + if (federalTypeField) { + hideElement(federalTypeField); } } } } - function handleFederalAgencyChange(federalAgency, organizationType, readonlyOrganizationType, organizationNameContainer, federalType) { + function handleFederalAgencyChange(federalAgency, organizationType, readonlyOrganizationType) { if (!isPageLoading) { let selectedFederalAgency = federalAgency.find("option:selected").text(); @@ -112,7 +113,7 @@ function handlePortfolioFields(){ } } - handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType); + handleOrganizationTypeChange(); // Determine if any changes are necessary to the display of portfolio type or federal type // based on changes to the Federal Agency @@ -175,8 +176,8 @@ function handlePortfolioFields(){ } } - function handleStateTerritoryChange(stateTerritory, urbanizationField) { - let selectedValue = stateTerritory.value; + function handleStateTerritoryChange() { + let selectedValue = stateTerritoryDropdown.value; if (selectedValue === "PR") { showElement(urbanizationField) } else { @@ -236,25 +237,25 @@ function handlePortfolioFields(){ } function initializePortfolioSettings() { - if (urbanization && stateTerritory) { - handleStateTerritoryChange(stateTerritory, urbanization); + if (urbanizationField && stateTerritoryDropdown) { + handleStateTerritoryChange(); } - handleOrganizationTypeChange(organizationType, organizationName, federalType); + handleOrganizationTypeChange(); } function setEventListeners() { - if ($federalAgency && (organizationType || readonlyOrganizationType)) { + if ($federalAgency && (organizationTypeDropdown || organizationTypeReadonly)) { $federalAgency.on("change", function() { - handleFederalAgencyChange($federalAgency, organizationType, readonlyOrganizationType, organizationName, federalType); + handleFederalAgencyChange($federalAgency, organizationTypeDropdown, organizationTypeReadonly); }); } - if (urbanization && stateTerritory) { - stateTerritory.addEventListener("change", function() { - handleStateTerritoryChange(stateTerritory, urbanization); + if (urbanizationField && stateTerritoryDropdown) { + stateTerritoryDropdown.addEventListener("change", function() { + handleStateTerritoryChange(); }); } - organizationType.addEventListener("change", function() { - handleOrganizationTypeChange(organizationType, organizationName, federalType); + organizationTypeDropdown.addEventListener("change", function() { + handleOrganizationTypeChange(); }); } From ee142a40964df37f460dce0503c93efc4c6d6f1a Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 11 Dec 2024 12:36:15 -0500 Subject: [PATCH 26/41] removed some more params --- .../assets/src/js/getgov-admin/portfolio-form.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js index b1c85a527..92f57712b 100644 --- a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js +++ b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js @@ -85,7 +85,7 @@ function handlePortfolioFields(){ } } - function handleFederalAgencyChange(federalAgency, organizationType, readonlyOrganizationType) { + function handleFederalAgencyChange() { if (!isPageLoading) { let selectedFederalAgency = federalAgency.find("option:selected").text(); @@ -99,7 +99,7 @@ function handlePortfolioFields(){ if (organizationTypeValue !== "federal") { if (organizationType){ organizationType.value = "federal"; - }else { + } else { readonlyOrganizationType.innerText = "Federal" } } @@ -107,7 +107,7 @@ function handlePortfolioFields(){ if (organizationTypeValue === "federal") { if (organizationType){ organizationType.value = ""; - }else { + } else { readonlyOrganizationType.innerText = "-" } } @@ -131,7 +131,7 @@ function handlePortfolioFields(){ if ($seniorOfficial && $seniorOfficial.length > 0) { // If the senior official is a dropdown field, edit that updateSeniorOfficialDropdown($seniorOfficial, seniorOfficialId, seniorOfficialName); - }else { + } else { if (seniorOfficialReadonly) { let seniorOfficialLink = `${seniorOfficialName}` seniorOfficialReadonly.innerHTML = seniorOfficialName ? seniorOfficialLink : "-"; @@ -246,7 +246,7 @@ function handlePortfolioFields(){ function setEventListeners() { if ($federalAgency && (organizationTypeDropdown || organizationTypeReadonly)) { $federalAgency.on("change", function() { - handleFederalAgencyChange($federalAgency, organizationTypeDropdown, organizationTypeReadonly); + handleFederalAgencyChange(); }); } if (urbanizationField && stateTerritoryDropdown) { From 058d42f0140975b8915d5b6f9efe59d6c99cc97c Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 11 Dec 2024 12:51:02 -0500 Subject: [PATCH 27/41] fix misnamed references --- .../src/js/getgov-admin/portfolio-form.js | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js index 92f57712b..9422c95f6 100644 --- a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js +++ b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js @@ -12,7 +12,7 @@ function handlePortfolioFields(){ const seniorOfficialField = document.querySelector(".field-senior_official"); const seniorOfficialAddress = seniorOfficialField.querySelector(".dja-address-contact-list"); const seniorOfficialReadonly = seniorOfficialField.querySelector(".readonly"); - const $federalAgency = django.jQuery("#id_federal_agency"); + const $federalAgencyDropdown = django.jQuery("#id_federal_agency"); const federalAgencyField = document.querySelector(".field-federal_agency"); const organizationTypeField = document.querySelector(".field-organization_type"); const organizationTypeReadonly = organizationTypeField.querySelector(".readonly"); @@ -88,39 +88,39 @@ function handlePortfolioFields(){ function handleFederalAgencyChange() { if (!isPageLoading) { - let selectedFederalAgency = federalAgency.find("option:selected").text(); - // There isn't a federal senior official associated with null records + let selectedFederalAgency = $federalAgencyDropdown.find("option:selected").text(); if (!selectedFederalAgency) { return; } - let organizationTypeValue = organizationType ? organizationType.value : readonlyOrganizationType.innerText.toLowerCase(); + // 1. Handle organization type + let organizationTypeValue = organizationTypeDropdown ? organizationTypeDropdown.value : organizationTypeReadonly.innerText.toLowerCase(); if (selectedFederalAgency !== "Non-Federal Agency") { if (organizationTypeValue !== "federal") { - if (organizationType){ - organizationType.value = "federal"; + if (organizationTypeDropdown){ + organizationTypeDropdown.value = "federal"; } else { - readonlyOrganizationType.innerText = "Federal" + organizationTypeReadonly.innerText = "Federal" } } } else { if (organizationTypeValue === "federal") { - if (organizationType){ - organizationType.value = ""; + if (organizationTypeDropdown){ + organizationTypeDropdown.value = ""; } else { - readonlyOrganizationType.innerText = "-" + organizationTypeReadonly.innerText = "-" } } } + // 2. Handle organization type change side effects handleOrganizationTypeChange(); - // Determine if any changes are necessary to the display of portfolio type or federal type - // based on changes to the Federal Agency + // 3. Handle federal type getFederalTypeFromAgency(selectedFederalAgency).then((federalType) => updateReadOnly(federalType, '.field-federal_type')); + // 4. Handle senior official hideElement(seniorOfficialAddress.parentElement); - getSeniorOfficialFromAgency(selectedFederalAgency).then((data) => { // Update the "contact details" blurb beneath senior official updateContactInfo(data); @@ -244,8 +244,8 @@ function handlePortfolioFields(){ } function setEventListeners() { - if ($federalAgency && (organizationTypeDropdown || organizationTypeReadonly)) { - $federalAgency.on("change", function() { + if ($federalAgencyDropdown && (organizationTypeDropdown || organizationTypeReadonly)) { + $federalAgencyDropdown.on("change", function() { handleFederalAgencyChange(); }); } From 591cc0ee7546eddcb6ee3ac76b12f6ad506bdd09 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 11 Dec 2024 13:04:18 -0500 Subject: [PATCH 28/41] clean up JS and add comments --- .../helpers-portfolio-dynamic-fields.js | 43 +------------- .../src/js/getgov-admin/portfolio-form.js | 56 ++++++++++++++++--- 2 files changed, 48 insertions(+), 51 deletions(-) diff --git a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js index 96c4290e8..0e5946c23 100644 --- a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js +++ b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js @@ -1,52 +1,11 @@ import { hideElement, showElement } from './helpers-admin.js'; -/** - * Helper function that handles business logic for the suborganization field. - * Can be used anywhere the suborganization dropdown exists -*/ -// export function handleSuborganizationFields( -// portfolioDropdownSelector="#id_portfolio", -// suborgDropdownSelector="#id_sub_organization", -// requestedSuborgFieldSelector=".field-requested_suborganization", -// suborgCitySelector=".field-suborganization_city", -// suborgStateTerritorySelector=".field-suborganization_state_territory" -// ) { -// // These dropdown are select2 fields so they must be interacted with via jquery -// const portfolioDropdown = django.jQuery(portfolioDropdownSelector) -// const suborganizationDropdown = django.jQuery(suborgDropdownSelector) -// const requestedSuborgField = document.querySelector(requestedSuborgFieldSelector); -// const suborgCity = document.querySelector(suborgCitySelector); -// const suborgStateTerritory = document.querySelector(suborgStateTerritorySelector); -// if (!suborganizationDropdown || !requestedSuborgField || !suborgCity || !suborgStateTerritory) { -// console.error("Requested suborg fields not found."); -// return; -// } - -// function toggleSuborganizationFields() { -// if (portfolioDropdown.val() && !suborganizationDropdown.val()) { -// if (requestedSuborgField) showElement(requestedSuborgField); -// if (suborgCity) showElement(suborgCity); -// if (suborgStateTerritory) showElement(suborgStateTerritory); -// }else { -// if (requestedSuborgField) hideElement(requestedSuborgField); -// if (suborgCity) hideElement(suborgCity); -// if (suborgStateTerritory) hideElement(suborgStateTerritory); -// } -// } - -// // Run the function once on page startup, then attach an event listener -// toggleSuborganizationFields(); -// suborganizationDropdown.on("change", toggleSuborganizationFields); -// portfolioDropdown.on("change", toggleSuborganizationFields); -// } - - /** * * This function handles the portfolio selection as well as display of * portfolio-related fields in the DomainRequest Form. * - * IMPORTANT NOTE: The logic in this method is paired dynamicPortfolioFields + * IMPORTANT NOTE: The business logic in this method is based on dynamicPortfolioFields */ export function handlePortfolioSelection( portfolioDropdownSelector="#id_portfolio", diff --git a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js index 9422c95f6..1d73e2bd5 100644 --- a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js +++ b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js @@ -2,7 +2,7 @@ import { hideElement, showElement } from './helpers-admin.js'; /** * A function for dynamically changing some fields on the portfolio admin model - * IMPORTANT NOTE: The logic in this function is paired handlePortfolioSelection and should be refactored once we solidify our requirements. + * IMPORTANT NOTE: The business logic in this function is related to handlePortfolioSelection */ function handlePortfolioFields(){ @@ -21,9 +21,15 @@ function handlePortfolioFields(){ const federalTypeField = document.querySelector(".field-federal_type"); const urbanizationField = document.querySelector(".field-urbanization"); const stateTerritoryDropdown = document.getElementById("id_state_territory"); - // consts for different urls const seniorOfficialAddUrl = document.getElementById("senior-official-add-url").value; + /** + * Fetches federal type data based on a selected agency using an AJAX call. + * + * @param {string} agency + * @returns {Promise} - A promise that resolves to the portfolio data object if successful, + * or null if there was an error. + */ function getFederalTypeFromAgency(agency) { let federalPortfolioApi = document.getElementById("federal_and_portfolio_types_from_agency_json_url").value; return fetch(`${federalPortfolioApi}?&agency_name=${agency}`) @@ -44,6 +50,13 @@ function handlePortfolioFields(){ }); } + /** + * Fetches senior official contact data based on a selected agency using an AJAX call. + * + * @param {string} agency + * @returns {Promise} - A promise that resolves to the portfolio data object if successful, + * or null if there was an error. + */ function getSeniorOfficialFromAgency(agency) { const seniorOfficialApi = document.getElementById("senior_official_from_agency_json_url").value; @@ -66,6 +79,12 @@ function handlePortfolioFields(){ }); } + /** + * Handles the side effects of change on the organization type field + * + * 1. If selection is federal, hide org name, show federal agency, show federal type if applicable + * 2. else show org name, hide federal agency, hide federal type if applicable + */ function handleOrganizationTypeChange() { if (organizationTypeDropdown && organizationNameField) { let selectedValue = organizationTypeDropdown.value; @@ -85,6 +104,14 @@ function handlePortfolioFields(){ } } + /** + * Handles the side effects of change on the federal agency field + * + * 1. handle org type dropdown or readonly + * 2. call handleOrganizationTypeChange + * 3. call getFederalTypeFromAgency and update federal type + * 4. call getSeniorOfficialFromAgency and update the SO fieldset + */ function handleFederalAgencyChange() { if (!isPageLoading) { @@ -123,7 +150,7 @@ function handlePortfolioFields(){ hideElement(seniorOfficialAddress.parentElement); getSeniorOfficialFromAgency(selectedFederalAgency).then((data) => { // Update the "contact details" blurb beneath senior official - updateContactInfo(data); + updateSeniorOfficialContactInfo(data); showElement(seniorOfficialAddress.parentElement); // Get the associated senior official with this federal agency let seniorOfficialId = data.id; @@ -158,6 +185,9 @@ function handlePortfolioFields(){ } + /** + * Helper for updating federal type field + */ function updateSeniorOfficialDropdown(dropdown, seniorOfficialId, seniorOfficialName) { if (!seniorOfficialId || !seniorOfficialName || !seniorOfficialName.trim()){ // Clear the field if the SO doesn't exist @@ -176,6 +206,9 @@ function handlePortfolioFields(){ } } + /** + * Handle urbanization + */ function handleStateTerritoryChange() { let selectedValue = stateTerritoryDropdown.value; if (selectedValue === "PR") { @@ -186,11 +219,7 @@ function handlePortfolioFields(){ } /** - * Utility that selects a div from the DOM using selectorString, - * and updates a div within that div which has class of 'readonly' - * so that the text of the div is updated to updateText - * @param {*} updateText - * @param {*} selectorString + * Helper for updating senior official dropdown */ function updateReadOnly(updateText, selectorString) { // find the div by selectorString @@ -205,7 +234,10 @@ function handlePortfolioFields(){ } } - function updateContactInfo(data) { + /** + * Helper for updating senior official contact info + */ + function updateSeniorOfficialContactInfo(data) { if (!seniorOfficialAddress) return; const titleSpan = seniorOfficialAddress.querySelector(".contact_info_title"); @@ -236,6 +268,9 @@ function handlePortfolioFields(){ }; } + /** + * Initializes necessary data and display configurations for the portfolio fields. + */ function initializePortfolioSettings() { if (urbanizationField && stateTerritoryDropdown) { handleStateTerritoryChange(); @@ -243,6 +278,9 @@ function handlePortfolioFields(){ handleOrganizationTypeChange(); } + /** + * Sets event listeners for key UI elements. + */ function setEventListeners() { if ($federalAgencyDropdown && (organizationTypeDropdown || organizationTypeReadonly)) { $federalAgencyDropdown.on("change", function() { From f599d4fb67d8edf70bf410d1139d8b83df412eb5 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 11 Dec 2024 13:09:17 -0500 Subject: [PATCH 29/41] lint --- src/registrar/admin.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 76c935f96..f7fd4991c 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1603,7 +1603,6 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): portfolio_urbanization.short_description = "Urbanization" # type: ignore - # Filters list_filter = [GenericOrgFilter] @@ -1635,7 +1634,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): ), (".gov domain", {"fields": ["domain"]}), ( - "Contacts", + "Contacts", { "fields": [ "senior_official", @@ -1746,7 +1745,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): "portfolio_zipcode", "portfolio_urbanization", "other_contacts", - "is_election_board" + "is_election_board", ) # Read only that we'll leverage for CISA Analysts From bc6c756aab85f6efcf0de2afaa08091d093fccd1 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 11 Dec 2024 13:36:54 -0500 Subject: [PATCH 30/41] Fix unit tests --- src/registrar/tests/test_admin.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 8307163c6..a259e5bef 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -853,9 +853,9 @@ class TestDomainInformationAdmin(TestCase): self.test_helper.assert_response_contains_distinct_values(response, expected_other_employees_fields) # Test for the copy link - # We expect 3 in the form + 2 from the js module copy-to-clipboard.js + # We expect 4 in the form + 2 from the js module copy-to-clipboard.js # that gets pulled in the test in django.contrib.staticfiles.finders.FileSystemFinder - self.assertContains(response, "copy-to-clipboard", count=5) + self.assertContains(response, "copy-to-clipboard", count=6) # cleanup this test domain_info.delete() @@ -871,6 +871,17 @@ class TestDomainInformationAdmin(TestCase): readonly_fields = self.admin.get_readonly_fields(request) expected_fields = [ + "portfolio_senior_official", + "portfolio_organization_type", + "portfolio_federal_type", + "portfolio_organization_name", + "portfolio_federal_agency", + "portfolio_state_territory", + "portfolio_address_line1", + "portfolio_address_line2", + "portfolio_city", + "portfolio_zipcode", + "portfolio_urbanization", "other_contacts", "is_election_board", "federal_agency", From e0eb70abe2e89dac7bdbe69dc389bb8fbaad1374 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 11 Dec 2024 14:12:42 -0500 Subject: [PATCH 31/41] working _delete_domain --- src/registrar/models/domain.py | 56 ++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index c1357c83c..37cc0d2b7 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1038,28 +1038,46 @@ class Domain(TimeStampedModel, DomainHelper): """This domain should be deleted from the registry may raises RegistryError, should be caught or handled correctly by caller""" - logger.info("Deleting subdomains for %s", self.name) - # check if any subdomains are in use by another domain - hosts = Host.objects.filter(name__regex=r".+{}".format(self.name)) - logger.debug("Checking if any subdomains are in use by another domain") - for host in hosts: - if host.domain != self: - logger.error("Unable to delete host: %s is in use by another domain: %s", host.name, host.domain) - raise RegistryError( - code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, - note=f"Host {host.name} is in use by {host.domain}", - ) - logger.debug("No subdomains are in use by another domain") + # logger.info("Deleting subdomains for %s", self.name) + # # check if any subdomains are in use by another domain + # hosts = Host.objects.filter(name__regex=r".+{}".format(self.name)) + # logger.debug("Checking if any subdomains are in use by another domain") + # for host in hosts: + # if host.domain != self: + # logger.error("Unable to delete host: %s is in use by another domain: %s", host.name, host.domain) + # raise RegistryError( + # code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, + # note=f"Host {host.name} is in use by {host.domain}", + # ) + # logger.debug("No subdomains are in use by another domain") - nameservers = [host.name for host in hosts] - hosts = self.createDeleteHostList(hostsToDelete=nameservers) - response_code = self.addAndRemoveHostsFromDomain(hostsToAdd=None, hostsToDelete=hosts) - if response_code != ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY: - raise RegistryError(code=response_code) + # nameservers = [host.name for host in hosts] + # hosts = self.createDeleteHostList(hostsToDelete=nameservers) + # response_code = self.addAndRemoveHostsFromDomain(hostsToAdd=[], hostsToDelete=hosts) + # if response_code != ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY: + # raise RegistryError(code=response_code) - logger.debug("Deleting subordinate hosts for %s", self.name) - self._delete_hosts_if_not_used(nameservers) + # logger.debug("Deleting subordinate hosts for %s", self.name) + # self._delete_hosts_if_not_used(nameservers) + ( + deleted_values, + updated_values, + new_values, + oldNameservers, + ) = self.getNameserverChanges(hosts=[]) + + _ = self._update_host_values(updated_values, oldNameservers) # returns nothing, just need to be run and errors + addToDomainList, addToDomainCount = self.createNewHostList(new_values) + deleteHostList, deleteCount = self.createDeleteHostList(deleted_values) + responseCode = self.addAndRemoveHostsFromDomain(hostsToAdd=addToDomainList, hostsToDelete=deleteHostList) + + # if unable to update domain raise error and stop + if responseCode != ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY: + raise NameserverError(code=nsErrorCodes.BAD_DATA) + + self._delete_hosts_if_not_used(hostsToDelete=deleted_values) + logger.debug("Deleting non-registrant contacts for %s", self.name) contacts = PublicContact.objects.filter(domain=self) for contact in contacts: From 30c15fa817394d98fb911fd1805f2926827a4bf2 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 11 Dec 2024 14:38:14 -0500 Subject: [PATCH 32/41] js cleanup --- .../assets/src/js/getgov-admin/domain-form.js | 1 - .../src/js/getgov-admin/portfolio-form.js | 53 ++++++++----------- 2 files changed, 23 insertions(+), 31 deletions(-) diff --git a/src/registrar/assets/src/js/getgov-admin/domain-form.js b/src/registrar/assets/src/js/getgov-admin/domain-form.js index 8c5ab5b1c..5335bd0d3 100644 --- a/src/registrar/assets/src/js/getgov-admin/domain-form.js +++ b/src/registrar/assets/src/js/getgov-admin/domain-form.js @@ -37,7 +37,6 @@ export function initDomainFormTargetBlankButtons() { export function initDynamicDomainFields(){ const domainPage = document.getElementById("domain_form"); if (domainPage) { - console.log("handling domain page"); handlePortfolioSelection("#id_domain_info-0-portfolio", "#id_domain_info-0-sub_organization"); } diff --git a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js index 1d73e2bd5..74729c2b2 100644 --- a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js +++ b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js @@ -8,7 +8,7 @@ function handlePortfolioFields(){ let isPageLoading = true // $ symbolically denotes that this is using jQuery - const $seniorOfficial = django.jQuery("#id_senior_official"); + const $seniorOfficialDropdown = django.jQuery("#id_senior_official"); const seniorOfficialField = document.querySelector(".field-senior_official"); const seniorOfficialAddress = seniorOfficialField.querySelector(".dja-address-contact-list"); const seniorOfficialReadonly = seniorOfficialField.querySelector(".readonly"); @@ -22,6 +22,8 @@ function handlePortfolioFields(){ const urbanizationField = document.querySelector(".field-urbanization"); const stateTerritoryDropdown = document.getElementById("id_state_territory"); const seniorOfficialAddUrl = document.getElementById("senior-official-add-url").value; + const seniorOfficialApi = document.getElementById("senior_official_from_agency_json_url").value; + const federalPortfolioApi = document.getElementById("federal_and_portfolio_types_from_agency_json_url").value; /** * Fetches federal type data based on a selected agency using an AJAX call. @@ -31,7 +33,6 @@ function handlePortfolioFields(){ * or null if there was an error. */ function getFederalTypeFromAgency(agency) { - let federalPortfolioApi = document.getElementById("federal_and_portfolio_types_from_agency_json_url").value; return fetch(`${federalPortfolioApi}?&agency_name=${agency}`) .then(response => { const statusCode = response.status; @@ -58,8 +59,6 @@ function handlePortfolioFields(){ * or null if there was an error. */ function getSeniorOfficialFromAgency(agency) { - const seniorOfficialApi = document.getElementById("senior_official_from_agency_json_url").value; - return fetch(`${seniorOfficialApi}?agency_name=${agency}`) .then(response => { const statusCode = response.status; @@ -148,16 +147,16 @@ function handlePortfolioFields(){ // 4. Handle senior official hideElement(seniorOfficialAddress.parentElement); - getSeniorOfficialFromAgency(selectedFederalAgency).then((data) => { + getSeniorOfficialFromAgency(selectedFederalAgency).then((senior_official) => { // Update the "contact details" blurb beneath senior official - updateSeniorOfficialContactInfo(data); + updateSeniorOfficialContactInfo(senior_official); showElement(seniorOfficialAddress.parentElement); // Get the associated senior official with this federal agency - let seniorOfficialId = data.id; - let seniorOfficialName = [data.first_name, data.last_name].join(" "); - if ($seniorOfficial && $seniorOfficial.length > 0) { + let seniorOfficialId = senior_official.id; + let seniorOfficialName = [senior_official.first_name, senior_official.last_name].join(" "); + if ($seniorOfficialDropdown && $seniorOfficialDropdown.length > 0) { // If the senior official is a dropdown field, edit that - updateSeniorOfficialDropdown($seniorOfficial, seniorOfficialId, seniorOfficialName); + updateSeniorOfficialDropdown(seniorOfficialId, seniorOfficialName); } else { if (seniorOfficialReadonly) { let seniorOfficialLink = `${seniorOfficialName}` @@ -168,8 +167,8 @@ function handlePortfolioFields(){ .catch(error => { if (error.statusCode === 404) { // Handle "not found" senior official - if ($seniorOfficial && $seniorOfficial.length > 0) { - $seniorOfficial.val("").trigger("change"); + if ($seniorOfficialDropdown && $seniorOfficialDropdown.length > 0) { + $seniorOfficialDropdown.val("").trigger("change"); } else { seniorOfficialReadonly.innerHTML = `No senior official found. Create one now.`; } @@ -177,32 +176,30 @@ function handlePortfolioFields(){ // Handle other errors console.error("An error occurred:", error.message); } - }); - + }); } else { isPageLoading = false; } - } /** * Helper for updating federal type field */ - function updateSeniorOfficialDropdown(dropdown, seniorOfficialId, seniorOfficialName) { + function updateSeniorOfficialDropdown(seniorOfficialId, seniorOfficialName) { if (!seniorOfficialId || !seniorOfficialName || !seniorOfficialName.trim()){ // Clear the field if the SO doesn't exist - dropdown.val("").trigger("change"); + $seniorOfficialDropdown.val("").trigger("change"); return; } // Add the senior official to the dropdown. // This format supports select2 - if we decide to convert this field in the future. - if (dropdown.find(`option[value='${seniorOfficialId}']`).length) { + if ($seniorOfficialDropdown.find(`option[value='${seniorOfficialId}']`).length) { // Select the value that is associated with the current Senior Official. - dropdown.val(seniorOfficialId).trigger("change"); + $seniorOfficialDropdown.val(seniorOfficialId).trigger("change"); } else { // Create a DOM Option that matches the desired Senior Official. Then append it and select it. let userOption = new Option(seniorOfficialName, seniorOfficialId, true, true); - dropdown.append(userOption).trigger("change"); + $seniorOfficialDropdown.append(userOption).trigger("change"); } } @@ -237,34 +234,30 @@ function handlePortfolioFields(){ /** * Helper for updating senior official contact info */ - function updateSeniorOfficialContactInfo(data) { + function updateSeniorOfficialContactInfo(senior_official) { if (!seniorOfficialAddress) return; - const titleSpan = seniorOfficialAddress.querySelector(".contact_info_title"); const emailSpan = seniorOfficialAddress.querySelector(".contact_info_email"); const phoneSpan = seniorOfficialAddress.querySelector(".contact_info_phone"); - if (titleSpan) { - titleSpan.textContent = data.title || "None"; + titleSpan.textContent = senior_official.title || "None"; }; - // Update the email field and the content for the clipboard if (emailSpan) { let copyButton = seniorOfficialAddress.querySelector(".admin-icon-group"); - emailSpan.textContent = data.email || "None"; - if (data.email) { + emailSpan.textContent = senior_official.email || "None"; + if (senior_official.email) { const clipboardInput = seniorOfficialAddress.querySelector(".admin-icon-group input"); if (clipboardInput) { - clipboardInput.value = data.email; + clipboardInput.value = senior_official.email; }; showElement(copyButton); }else { hideElement(copyButton); } } - if (phoneSpan) { - phoneSpan.textContent = data.phone || "None"; + phoneSpan.textContent = senior_official.phone || "None"; }; } From 533e57722fba896d6d6728accb59bb4560cbc6e7 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 11 Dec 2024 14:30:58 -0600 Subject: [PATCH 33/41] refactor delete to use same logic as nameserver setter --- src/registrar/models/domain.py | 37 +++++++++++++--------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 37cc0d2b7..a3b5c3c58 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1038,27 +1038,18 @@ class Domain(TimeStampedModel, DomainHelper): """This domain should be deleted from the registry may raises RegistryError, should be caught or handled correctly by caller""" - # logger.info("Deleting subdomains for %s", self.name) - # # check if any subdomains are in use by another domain - # hosts = Host.objects.filter(name__regex=r".+{}".format(self.name)) - # logger.debug("Checking if any subdomains are in use by another domain") - # for host in hosts: - # if host.domain != self: - # logger.error("Unable to delete host: %s is in use by another domain: %s", host.name, host.domain) - # raise RegistryError( - # code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, - # note=f"Host {host.name} is in use by {host.domain}", - # ) - # logger.debug("No subdomains are in use by another domain") - - # nameservers = [host.name for host in hosts] - # hosts = self.createDeleteHostList(hostsToDelete=nameservers) - # response_code = self.addAndRemoveHostsFromDomain(hostsToAdd=[], hostsToDelete=hosts) - # if response_code != ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY: - # raise RegistryError(code=response_code) - - # logger.debug("Deleting subordinate hosts for %s", self.name) - # self._delete_hosts_if_not_used(nameservers) + logger.info("Deleting subdomains for %s", self.name) + # check if any subdomains are in use by another domain + hosts = Host.objects.filter(name__regex=r".+{}".format(self.name)) + logger.debug("Checking if any subdomains are in use by another domain") + for host in hosts: + if host.domain != self: + logger.error("Unable to delete host: %s is in use by another domain: %s", host.name, host.domain) + raise RegistryError( + code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, + note=f"Host {host.name} is in use by {host.domain}", + ) + logger.debug("No subdomains are in use by another domain") ( deleted_values, @@ -1068,8 +1059,8 @@ class Domain(TimeStampedModel, DomainHelper): ) = self.getNameserverChanges(hosts=[]) _ = self._update_host_values(updated_values, oldNameservers) # returns nothing, just need to be run and errors - addToDomainList, addToDomainCount = self.createNewHostList(new_values) - deleteHostList, deleteCount = self.createDeleteHostList(deleted_values) + addToDomainList, _ = self.createNewHostList(new_values) + deleteHostList, _ = self.createDeleteHostList(deleted_values) responseCode = self.addAndRemoveHostsFromDomain(hostsToAdd=addToDomainList, hostsToDelete=deleteHostList) # if unable to update domain raise error and stop From 51da456f02ba72f09238808129ebc3186571bd7d Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 12 Dec 2024 13:45:36 -0600 Subject: [PATCH 34/41] fix tests --- src/registrar/models/domain.py | 4 + src/registrar/tests/common.py | 2 +- src/registrar/tests/test_admin_domain.py | 2 +- src/registrar/tests/test_models_domain.py | 127 ++++++++++++++++------ 4 files changed, 97 insertions(+), 38 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index a3b5c3c58..245c82f7f 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1071,6 +1071,7 @@ class Domain(TimeStampedModel, DomainHelper): logger.debug("Deleting non-registrant contacts for %s", self.name) contacts = PublicContact.objects.filter(domain=self) + logger.debug("contacts %s", contacts) for contact in contacts: if contact.contact_type != PublicContact.ContactTypeChoices.REGISTRANT: logger.debug("removing contact %s from domain %s", contact.registry_id, self.name) @@ -1085,6 +1086,9 @@ class Domain(TimeStampedModel, DomainHelper): logger.debug("Deleting registrant contact for %s", self.name) registrant_id = self.registrant_contact.registry_id + logger.debug("setting default registrant contact") + self._add_registrant_to_existing_domain(self.get_default_registrant_contact()) + logger.debug("deleting registrant contact %s from registry", registrant_id) deleteRegistrant = commands.DeleteContact(id=registrant_id) registry.send(deleteRegistrant, cleaned=True) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 0f7923083..392d5b248 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1677,7 +1677,7 @@ class MockEppLib(TestCase): host = getattr(_request, "name", None) if "sharedhost.com" in host: print("raising registry error") - raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, note="otherdomain.gov") + raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, note="ns1.sharedhost.com") return MagicMock( res_data=[self.mockDataHostChange], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index 57961605d..f3ff49abf 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -265,7 +265,7 @@ class TestDomainAdminAsStaff(MockEppLib): mock_add_message.assert_called_once_with( request, messages.ERROR, - "Error deleting this Domain: This subdomain is being used as a hostname on another domain: otherdomain.gov", + "Error deleting this Domain: This subdomain is being used as a hostname on another domain: ns1.sharedhost.com", extra_tags="", fail_silently=False, ) diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 70ef4cdde..d6a24e2bd 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -2584,8 +2584,24 @@ class TestAnalystDelete(MockEppLib): """ super().setUp() self.domain, _ = Domain.objects.get_or_create(name="fake.gov", state=Domain.State.READY) + self.domain_with_contacts, _ = Domain.objects.get_or_create(name="freeman.gov", state=Domain.State.READY) self.domain_on_hold, _ = Domain.objects.get_or_create(name="fake-on-hold.gov", state=Domain.State.ON_HOLD) Host.objects.create(name="ns1.sharingiscaring.gov", domain=self.domain_on_hold) + PublicContact.objects.create( + registry_id="regContact", + contact_type=PublicContact.ContactTypeChoices.REGISTRANT, + domain=self.domain_with_contacts, + ) + PublicContact.objects.create( + registry_id="adminContact", + contact_type=PublicContact.ContactTypeChoices.ADMINISTRATIVE, + domain=self.domain_with_contacts, + ) + PublicContact.objects.create( + registry_id="techContact", + contact_type=PublicContact.ContactTypeChoices.TECHNICAL, + domain=self.domain_with_contacts, + ) def tearDown(self): Host.objects.all().delete() @@ -2642,8 +2658,8 @@ class TestAnalystDelete(MockEppLib): domain.deletedInEpp() domain.save() - self.assertTrue(err.is_client_error() and err.code == ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) - self.assertEqual(err.msg, "Host in use by another domain: fake-on-hold.gov") + self.assertTrue(err.code == ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) + self.assertEqual(err.msg, "Host ns1.sharingiscaring.gov is in use by: fake-on-hold.gov") # Domain itself should not be deleted self.assertNotEqual(domain, None) # State should not have changed @@ -2654,33 +2670,22 @@ class TestAnalystDelete(MockEppLib): """ Scenario: Domain with related Host and Contacts is Deleted When a contact and host exists that is tied to this domain - Then `commands.DeleteHost` is sent to the registry - Then `commands.DeleteContact` is sent to the registry - Then `commands.DeleteDomain` is sent to the registry - Then `commands.DeleteContact` is sent to the registry for the registrant contact + Then all the needed commands are sent to the registry And `state` is set to `DELETED` """ - # Desired domain - domain, _ = Domain.objects.get_or_create(name="freeman.gov", state=Domain.State.ON_HOLD) # Put the domain in client hold - domain.place_client_hold() + self.domain_with_contacts.place_client_hold() # Delete it - domain.deletedInEpp() - domain.save() + self.domain_with_contacts.deletedInEpp() + self.domain_with_contacts.save() - # Check that the host and contacts are deleted, order doesn't matter + # Check that the host and contacts are deleted self.mockedSendFunction.assert_has_calls( [ call( commands.UpdateDomain( - name="freeman.gov", - add=[ - common.Status( - state=Domain.Status.CLIENT_HOLD, - description="", - lang="en", - ) - ], + name='freeman.gov', + add=[common.Status(state=Domain.Status.CLIENT_HOLD, description='', lang='en')], rem=[], nsset=None, keyset=None, @@ -2689,28 +2694,78 @@ class TestAnalystDelete(MockEppLib): ), cleaned=True, ), - call(commands.DeleteHost(name="fake.host.com"), cleaned=True), - call(commands.DeleteContact(id="securityContact"), cleaned=True), - call(commands.DeleteContact(id="technicalContact"), cleaned=True), - call(commands.DeleteContact(id="adminContact"), cleaned=True), + call( + commands.InfoDomain(name='freeman.gov', auth_info=None), + cleaned=True, + ), + call( + commands.InfoHost(name='fake.host.com'), + cleaned=True, + ), + call( + commands.UpdateDomain( + name='freeman.gov', + add=[], + rem=[common.HostObjSet(hosts=['fake.host.com'])], + nsset=None, + keyset=None, + registrant=None, + auth_info=None, + ), + cleaned=True, + ), + call( + commands.DeleteHost(name='fake.host.com'), + cleaned=True, + ), + call( + commands.UpdateDomain( + name='freeman.gov', + add=[], + rem=[common.DomainContact(contact='adminContact', type='admin')], + nsset=None, + keyset=None, + registrant=None, + auth_info=None, + ), + cleaned=True, + ), + call( + commands.DeleteContact(id='adminContact'), + cleaned=True, + ), + call( + commands.UpdateDomain( + name='freeman.gov', + add=[], + rem=[common.DomainContact(contact='techContact', type='tech')], + nsset=None, + keyset=None, + registrant=None, + auth_info=None, + ), + cleaned=True, + ), + call( + commands.DeleteContact(id='techContact'), + cleaned=True, + ), + call( + commands.DeleteDomain(name='freeman.gov'), + cleaned=True, + ), + call( + commands.DeleteContact(id='regContact'), + cleaned=True, + ), ], any_order=True, ) - actual_calls = self.mockedSendFunction.call_args_list - print("actual_calls", actual_calls) - # These calls need to be in order - self.mockedSendFunction.assert_has_calls( - [ - call(commands.DeleteDomain(name="freeman.gov"), cleaned=True), - call(commands.InfoContact(id="regContact"), cleaned=True), - call(commands.DeleteContact(id="regContact"), cleaned=True), - ], - ) # Domain itself should not be deleted - self.assertNotEqual(domain, None) + self.assertNotEqual(self.domain_with_contacts, None) # State should have changed - self.assertEqual(domain.state, Domain.State.DELETED) + self.assertEqual(self.domain_with_contacts.state, Domain.State.DELETED) # @less_console_noise_decorator def test_deletion_ready_fsm_failure(self): From 1fb8b222c496c3f6310eee10b72398aee85aae79 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 12 Dec 2024 15:31:40 -0600 Subject: [PATCH 35/41] pushing changes to pull on another device --- src/registrar/models/domain.py | 8 -------- src/registrar/tests/test_models_domain.py | 4 ---- 2 files changed, 12 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 245c82f7f..b2626bbe1 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1084,14 +1084,6 @@ class Domain(TimeStampedModel, DomainHelper): request = commands.DeleteDomain(name=self.name) registry.send(request, cleaned=True) - logger.debug("Deleting registrant contact for %s", self.name) - registrant_id = self.registrant_contact.registry_id - logger.debug("setting default registrant contact") - self._add_registrant_to_existing_domain(self.get_default_registrant_contact()) - logger.debug("deleting registrant contact %s from registry", registrant_id) - deleteRegistrant = commands.DeleteContact(id=registrant_id) - registry.send(deleteRegistrant, cleaned=True) - def __str__(self) -> str: return self.name diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index d6a24e2bd..9e54a6e5f 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -2754,10 +2754,6 @@ class TestAnalystDelete(MockEppLib): commands.DeleteDomain(name='freeman.gov'), cleaned=True, ), - call( - commands.DeleteContact(id='regContact'), - cleaned=True, - ), ], any_order=True, ) From 63c2b7907f1a2a7f858eab480644f37ff78329c8 Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Thu, 12 Dec 2024 15:33:50 -0600 Subject: [PATCH 36/41] clean up --- src/registrar/models/domain.py | 5 ----- src/registrar/tests/test_models_domain.py | 6 +++--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index b2626bbe1..191b6f143 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1041,7 +1041,6 @@ class Domain(TimeStampedModel, DomainHelper): logger.info("Deleting subdomains for %s", self.name) # check if any subdomains are in use by another domain hosts = Host.objects.filter(name__regex=r".+{}".format(self.name)) - logger.debug("Checking if any subdomains are in use by another domain") for host in hosts: if host.domain != self: logger.error("Unable to delete host: %s is in use by another domain: %s", host.name, host.domain) @@ -1049,7 +1048,6 @@ class Domain(TimeStampedModel, DomainHelper): code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, note=f"Host {host.name} is in use by {host.domain}", ) - logger.debug("No subdomains are in use by another domain") ( deleted_values, @@ -1071,12 +1069,9 @@ class Domain(TimeStampedModel, DomainHelper): logger.debug("Deleting non-registrant contacts for %s", self.name) contacts = PublicContact.objects.filter(domain=self) - logger.debug("contacts %s", contacts) for contact in contacts: if contact.contact_type != PublicContact.ContactTypeChoices.REGISTRANT: - logger.debug("removing contact %s from domain %s", contact.registry_id, self.name) self._update_domain_with_contact(contact, rem=True) - logger.debug("deleting contact %s from registry", contact.registry_id) request = commands.DeleteContact(contact.registry_id) registry.send(request, cleaned=True) diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 9e54a6e5f..115351c20 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -2641,7 +2641,7 @@ class TestAnalystDelete(MockEppLib): # Cache should be invalidated self.assertEqual(self.domain._cache, {}) - # @less_console_noise_decorator + @less_console_noise_decorator def test_deletion_is_unsuccessful(self): """ Scenario: Domain deletion is unsuccessful @@ -2665,7 +2665,7 @@ class TestAnalystDelete(MockEppLib): # State should not have changed self.assertEqual(domain.state, Domain.State.ON_HOLD) - # @less_console_noise_decorator + @less_console_noise_decorator def test_deletion_with_host_and_contacts(self): """ Scenario: Domain with related Host and Contacts is Deleted @@ -2763,7 +2763,7 @@ class TestAnalystDelete(MockEppLib): # State should have changed self.assertEqual(self.domain_with_contacts.state, Domain.State.DELETED) - # @less_console_noise_decorator + @less_console_noise_decorator def test_deletion_ready_fsm_failure(self): """ Scenario: Domain deletion is unsuccessful due to FSM rules From 21007ce869bc6b737e8aa77d76ce3c7e42a9a0ef Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Fri, 13 Dec 2024 15:23:15 -0600 Subject: [PATCH 37/41] review changes --- src/registrar/admin.py | 2 +- src/registrar/models/domain.py | 9 ++++++++- src/registrar/tests/common.py | 1 - 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index d26566c63..5a9118549 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2927,7 +2927,7 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): message = "Cannot connect to the registry" if not err.is_connection_error(): # If nothing is found, will default to returned err - message = error_messages[err.code] + message = error_messages.get(err.code, err) self.message_user(request, f"Error deleting this Domain: {message}", messages.ERROR) except TransitionNotAllowed: if obj.state == Domain.State.DELETED: diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 191b6f143..8c290a8a6 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -750,7 +750,12 @@ class Domain(TimeStampedModel, DomainHelper): successTotalNameservers = len(oldNameservers) - deleteCount + addToDomainCount - self._delete_hosts_if_not_used(hostsToDelete=deleted_values) + try: + self._delete_hosts_if_not_used(hostsToDelete=deleted_values) + except: + # the error will be logged in the erring function and we don't + # need this part to succeed in order to continue.s + pass if successTotalNameservers < 2: try: @@ -1065,6 +1070,8 @@ class Domain(TimeStampedModel, DomainHelper): if responseCode != ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY: raise NameserverError(code=nsErrorCodes.BAD_DATA) + # addAndRemoveHostsFromDomain removes the hosts from the domain object, + # but we still need to delete the object themselves self._delete_hosts_if_not_used(hostsToDelete=deleted_values) logger.debug("Deleting non-registrant contacts for %s", self.name) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 392d5b248..c379b1c26 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1676,7 +1676,6 @@ class MockEppLib(TestCase): def mockDeleteHostCommands(self, _request, cleaned): host = getattr(_request, "name", None) if "sharedhost.com" in host: - print("raising registry error") raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, note="ns1.sharedhost.com") return MagicMock( res_data=[self.mockDataHostChange], From 32789faec6b93e471528fab147474e0836a084dc Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Fri, 13 Dec 2024 15:27:57 -0600 Subject: [PATCH 38/41] review changes and linting --- src/epplibwrapper/errors.py | 2 +- src/registrar/models/domain.py | 4 +- src/registrar/tests/test_models_domain.py | 72 +++++++++++++---------- 3 files changed, 45 insertions(+), 33 deletions(-) diff --git a/src/epplibwrapper/errors.py b/src/epplibwrapper/errors.py index 78272ff0a..95db40ab8 100644 --- a/src/epplibwrapper/errors.py +++ b/src/epplibwrapper/errors.py @@ -62,7 +62,7 @@ class RegistryError(Exception): - 2501 - 2502 Something malicious or abusive may have occurred """ - def __init__(self, *args, code=None, note="",**kwargs): + def __init__(self, *args, code=None, note="", **kwargs): super().__init__(*args, **kwargs) self.code = code # note is a string that can be used to provide additional context diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 191b6f143..80dd79f70 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1033,7 +1033,7 @@ class Domain(TimeStampedModel, DomainHelper): # if registry error occurs, log the error, and raise it as well logger.error(f"registry error removing client hold: {err}") raise (err) - + def _delete_domain(self): """This domain should be deleted from the registry may raises RegistryError, should be caught or handled correctly by caller""" @@ -1048,7 +1048,7 @@ class Domain(TimeStampedModel, DomainHelper): code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, note=f"Host {host.name} is in use by {host.domain}", ) - + ( deleted_values, updated_values, diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 115351c20..1aa08ffe4 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -2684,8 +2684,8 @@ class TestAnalystDelete(MockEppLib): [ call( commands.UpdateDomain( - name='freeman.gov', - add=[common.Status(state=Domain.Status.CLIENT_HOLD, description='', lang='en')], + name="freeman.gov", + add=[common.Status(state=Domain.Status.CLIENT_HOLD, description="", lang="en")], rem=[], nsset=None, keyset=None, @@ -2694,19 +2694,43 @@ class TestAnalystDelete(MockEppLib): ), cleaned=True, ), + ] + ) + self.mockedSendFunction.assert_has_calls( + [ call( - commands.InfoDomain(name='freeman.gov', auth_info=None), + commands.InfoDomain(name="freeman.gov", auth_info=None), cleaned=True, ), call( - commands.InfoHost(name='fake.host.com'), + commands.InfoHost(name="fake.host.com"), cleaned=True, ), call( commands.UpdateDomain( - name='freeman.gov', + name="freeman.gov", add=[], - rem=[common.HostObjSet(hosts=['fake.host.com'])], + rem=[common.HostObjSet(hosts=["fake.host.com"])], + nsset=None, + keyset=None, + registrant=None, + auth_info=None, + ), + cleaned=True, + ), + ] + ) + self.mockedSendFunction.assert_has_calls( + [ + call( + commands.DeleteHost(name="fake.host.com"), + cleaned=True, + ), + call( + commands.UpdateDomain( + name="freeman.gov", + add=[], + rem=[common.DomainContact(contact="adminContact", type="admin")], nsset=None, keyset=None, registrant=None, @@ -2715,14 +2739,14 @@ class TestAnalystDelete(MockEppLib): cleaned=True, ), call( - commands.DeleteHost(name='fake.host.com'), + commands.DeleteContact(id="adminContact"), cleaned=True, ), call( commands.UpdateDomain( - name='freeman.gov', + name="freeman.gov", add=[], - rem=[common.DomainContact(contact='adminContact', type='admin')], + rem=[common.DomainContact(contact="techContact", type="tech")], nsset=None, keyset=None, registrant=None, @@ -2731,32 +2755,20 @@ class TestAnalystDelete(MockEppLib): cleaned=True, ), call( - commands.DeleteContact(id='adminContact'), - cleaned=True, - ), - call( - commands.UpdateDomain( - name='freeman.gov', - add=[], - rem=[common.DomainContact(contact='techContact', type='tech')], - nsset=None, - keyset=None, - registrant=None, - auth_info=None, - ), - cleaned=True, - ), - call( - commands.DeleteContact(id='techContact'), - cleaned=True, - ), - call( - commands.DeleteDomain(name='freeman.gov'), + commands.DeleteContact(id="techContact"), cleaned=True, ), ], any_order=True, ) + self.mockedSendFunction.assert_has_calls( + [ + call( + commands.DeleteDomain(name="freeman.gov"), + cleaned=True, + ), + ], + ) # Domain itself should not be deleted self.assertNotEqual(self.domain_with_contacts, None) From bb2072debce7a86cae74791d6139f3638b527c3a Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Fri, 13 Dec 2024 16:35:29 -0600 Subject: [PATCH 39/41] log exception in nameserver setter. --- src/registrar/models/domain.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index c3ed8cada..f67002e4f 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -752,10 +752,9 @@ class Domain(TimeStampedModel, DomainHelper): try: self._delete_hosts_if_not_used(hostsToDelete=deleted_values) - except: - # the error will be logged in the erring function and we don't - # need this part to succeed in order to continue.s - pass + except Exception as e: + # we don't need this part to succeed in order to continue. + logger.error("Failed to delete nameserver hosts: %s", e) if successTotalNameservers < 2: try: From 0178d5749952762841438d8c244392ab852870a4 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Mon, 16 Dec 2024 10:28:55 -0600 Subject: [PATCH 40/41] linter fixes --- src/registrar/models/domain.py | 6 +++--- src/registrar/tests/test_admin_domain.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 9aca9b5c3..f99de3d45 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -712,7 +712,7 @@ class Domain(TimeStampedModel, DomainHelper): raise e @nameservers.setter # type: ignore - def nameservers(self, hosts: list[tuple[str, list]]): + def nameservers(self, hosts: list[tuple[str, list]]): # noqa """Host should be a tuple of type str, str,... where the elements are Fully qualified host name, addresses associated with the host example: [(ns1.okay.gov, [127.0.0.1, others ips])]""" @@ -749,7 +749,7 @@ class Domain(TimeStampedModel, DomainHelper): successTotalNameservers = len(oldNameservers) - deleteCount + addToDomainCount - try: + try: self._delete_hosts_if_not_used(hostsToDelete=deleted_values) except Exception as e: # we don't need this part to succeed in order to continue. @@ -1068,7 +1068,7 @@ class Domain(TimeStampedModel, DomainHelper): if responseCode != ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY: raise NameserverError(code=nsErrorCodes.BAD_DATA) - # addAndRemoveHostsFromDomain removes the hosts from the domain object, + # addAndRemoveHostsFromDomain removes the hosts from the domain object, # but we still need to delete the object themselves self._delete_hosts_if_not_used(hostsToDelete=deleted_values) diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index 6eb934091..8a487ea2b 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -265,7 +265,7 @@ class TestDomainAdminAsStaff(MockEppLib): mock_add_message.assert_called_once_with( request, messages.ERROR, - "Error deleting this Domain: This subdomain is being used as a hostname on another domain: ns1.sharedhost.com", + "Error deleting this Domain: This subdomain is being used as a hostname on another domain: ns1.sharedhost.com", # noqa extra_tags="", fail_silently=False, ) From f039f315e0b41840249f7c3678449c221e20b2e8 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Mon, 16 Dec 2024 10:35:34 -0600 Subject: [PATCH 41/41] linter fixes --- src/registrar/models/domain.py | 2 +- src/registrar/tests/test_admin_domain.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index f99de3d45..19e96719f 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -712,7 +712,7 @@ class Domain(TimeStampedModel, DomainHelper): raise e @nameservers.setter # type: ignore - def nameservers(self, hosts: list[tuple[str, list]]): # noqa + def nameservers(self, hosts: list[tuple[str, list]]): # noqa """Host should be a tuple of type str, str,... where the elements are Fully qualified host name, addresses associated with the host example: [(ns1.okay.gov, [127.0.0.1, others ips])]""" diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index 8a487ea2b..072bc1f7f 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -265,7 +265,7 @@ class TestDomainAdminAsStaff(MockEppLib): mock_add_message.assert_called_once_with( request, messages.ERROR, - "Error deleting this Domain: This subdomain is being used as a hostname on another domain: ns1.sharedhost.com", # noqa + "Error deleting this Domain: This subdomain is being used as a hostname on another domain: ns1.sharedhost.com", # noqa extra_tags="", fail_silently=False, )