From 7cf8b8a82ea0a0d3df4885eaae45b1c4d0082dfc Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Fri, 22 Nov 2024 10:51:13 -0600 Subject: [PATCH 01/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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 a940fa673d9e90776e11b9dff7863ca555dc72b2 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Wed, 11 Dec 2024 15:01:11 -0800 Subject: [PATCH 34/44] Update domain request confirmation page content for org model --- .../templates/domain_request_done.html | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/registrar/templates/domain_request_done.html b/src/registrar/templates/domain_request_done.html index 0d38309d8..9b1b109a1 100644 --- a/src/registrar/templates/domain_request_done.html +++ b/src/registrar/templates/domain_request_done.html @@ -26,13 +26,17 @@

Next steps in this process

We’ll review your request. This review period can take 30 business days. Due to the volume of requests, the wait time is longer than usual. We appreciate your patience.

- -

During our review we’ll verify that:

-
    -
  • Your organization is eligible for a .gov domain.
    • -
  • You work at the organization and/or can make requests on its behalf.
    • -
  • Your requested domain meets our naming requirements.
    • -
+ + {% if has_organization_feature_flag %} +

During our review we’ll verify that your requested domain meets our naming requirements.

+ {% else %} +

During our review we’ll verify that:

+
    +
  • Your organization is eligible for a .gov domain.
    • +
  • You work at the organization and/or can make requests on its behalf.
    • +
  • Your requested domain meets our naming requirements.
    • +
+ {% endif %}

We’ll email you if we have questions. We’ll also email you as soon as we complete our review. You can check the status of your request at any time on the registrar.

From 51da456f02ba72f09238808129ebc3186571bd7d Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 12 Dec 2024 13:45:36 -0600 Subject: [PATCH 35/44] 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 36/44] 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 37/44] 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 38/44] 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 39/44] 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 40/44] 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 41/44] 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 42/44] 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, ) From 91740c9026b2c514279bce812a6d12c8346faaab Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Mon, 16 Dec 2024 10:20:08 -0800 Subject: [PATCH 43/44] Apply suggestions from code review Add suggested commas Co-authored-by: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> --- src/registrar/templates/domain_request_done.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/domain_request_done.html b/src/registrar/templates/domain_request_done.html index 9b1b109a1..17eea9854 100644 --- a/src/registrar/templates/domain_request_done.html +++ b/src/registrar/templates/domain_request_done.html @@ -28,9 +28,9 @@

We’ll review your request. This review period can take 30 business days. Due to the volume of requests, the wait time is longer than usual. We appreciate your patience.

{% if has_organization_feature_flag %} -

During our review we’ll verify that your requested domain meets our naming requirements.

+

During our review, we’ll verify that your requested domain meets our naming requirements.

{% else %} -

During our review we’ll verify that:

+

During our review, we’ll verify that:

  • Your organization is eligible for a .gov domain.
  • You work at the organization and/or can make requests on its behalf.
    • From 1558358bce381ffc7dd219b7caa7abc6f811f3e2 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Mon, 16 Dec 2024 11:24:20 -0800 Subject: [PATCH 44/44] Update domain request content --- src/registrar/templates/emails/submission_confirmation.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/emails/submission_confirmation.txt b/src/registrar/templates/emails/submission_confirmation.txt index ef9736a9d..aa1c207ce 100644 --- a/src/registrar/templates/emails/submission_confirmation.txt +++ b/src/registrar/templates/emails/submission_confirmation.txt @@ -12,12 +12,12 @@ STATUS: Submitted NEXT STEPS We’ll review your request. This review period can take 30 business days. Due to the volume of requests, the wait time is longer than usual. We appreciate your patience. -During our review we’ll verify that: +During our review, we’ll verify that: - Your organization is eligible for a .gov domain - You work at the organization and/or can make requests on its behalf - Your requested domain meets our naming requirements -We’ll email you if we have questions. We’ll also email you as soon as we complete our review. You can check the status of your request at any time on the registrar homepage. +We’ll email you if we have questions. We’ll also email you as soon as we complete our review. You can check the status of your request at any time on the registrar. NEED TO MAKE CHANGES?