From 7cf8b8a82ea0a0d3df4885eaae45b1c4d0082dfc Mon Sep 17 00:00:00 2001
From: Matthew Spence <matthew.spence@cisa.dhs.gov>
Date: Fri, 22 Nov 2024 10:51:13 -0600
Subject: [PATCH 01/29] 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 @@ def _remove_client_hold(self):
             # 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 @@ def revert_client_hold(self, ignoreEPP=False):
     @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 @@ def deletedInEpp(self):
         # 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 @@ def _get_or_create_contact(self, contact: PublicContact):
                 )
 
                 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 <Matthewswspence@gmail.com>
Date: Tue, 26 Nov 2024 13:56:45 -0600
Subject: [PATCH 02/29] 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 @@ def nameservers(self, hosts: list[tuple[str, list]]):
 
         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 @@ def _delete_contacts(self):
         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 @@ def _get_or_create_contact(self, contact: PublicContact):
                 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 @@ def _delete_hosts_if_not_used(self, hostsToDelete: list[str]):
         """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 @@ def _delete_hosts_if_not_used(self, hostsToDelete: list[str]):
                 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 @@ def dummyInfoContactResultData(
         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 @@ def mockSend(self, _request, cleaned):
             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 @@ def mockUpdateHostCommands(self, _request, cleaned):
                 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 @@ def mockUpdateDomainCommands(self, _request, cleaned):
 
     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 @@ def mockInfoDomainCommands(self, _request, cleaned):
             "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 @@ def setUp(self):
         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 @@ def test_analyst_deletes_domain(self):
 
             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 @@ def test_deletion_is_unsuccessful(self):
             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 <Matthewswspence@gmail.com>
Date: Tue, 3 Dec 2024 15:04:25 -0600
Subject: [PATCH 03/29] 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 @@ def registered(cls, domain: str) -> bool:
         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 @@ def contacts(self) -> dict[str, str]:
             { 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 @@ def _remove_client_hold(self):
             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 @@ def _delete_domain(self):
         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 @@ def deletedInEpp(self):
         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 @@ def _get_or_create_contact(self, contact: PublicContact):
 
                 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 @@ def dummyInfoContactResultData(
             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 @@ def mockSend(self, _request, cleaned):
                 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 @@ def mockInfoDomainCommands(self, _request, cleaned):
 
         # 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 @@ def mockCreateContactCommands(self, _request, cleaned):
             # 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 @@ def setUp(self):
 
     def tearDown(self):
         Host.objects.all().delete()
+        PublicContact.objects.all().delete()
         Domain.objects.all().delete()
         super().tearDown()
 
@@ -2643,7 +2644,7 @@ def test_deletion_is_unsuccessful(self):
                     call(
                         commands.DeleteHost(name=common.HostObjSet(hosts=['ns1.sharedhost.com'])),
                         cleaned=True,
-                    )
+                    ),
                 ]
             )
             # Domain itself should not be deleted
@@ -2651,6 +2652,49 @@ def test_deletion_is_unsuccessful(self):
             # 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 <Matthewswspence@gmail.com>
Date: Wed, 4 Dec 2024 11:24:49 -0600
Subject: [PATCH 04/29] 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 @@ def get_help_text(cls, state) -> str:
             """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 @@ def _delete_subdomains(self):
         """
         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 @@ def mockSend(self, _request, cleaned):
             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 @@ def mockUpdateHostCommands(self, _request, cleaned):
                 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 @@ def test_user_removes_too_many_nameservers(self):
             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,
-                    ),
-                    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.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 @@ def test_deletion_is_unsuccessful(self):
             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 @@ def test_deletion_with_host_and_contacts(self):
         # 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 <Matthewswspence@gmail.com>
Date: Wed, 4 Dec 2024 12:50:28 -0600
Subject: [PATCH 05/29] 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 @@
     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 @@ def setUp(self):
     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 <Matthewswspence@gmail.com>
Date: Wed, 4 Dec 2024 13:37:23 -0600
Subject: [PATCH 06/29] 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 @@ def nameservers(self, hosts: list[tuple[str, list]]):
 
         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 @@ def _remove_client_hold(self):
             # 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 @@ def _delete_nonregistrant_contacts(self):
                 # 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 @@ def _get_or_create_contact(self, contact: PublicContact):
                 )
 
                 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 @@ def _delete_contact(self, contact_name: str, registry_id: str):
             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 @@ def _delete_hosts_if_not_used(self, hostsToDelete: list[str]):
                 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 @@ def mockUpdateHostCommands(self, _request, cleaned):
                 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 @@ def mockCreateContactCommands(self, _request, cleaned):
             # 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 @@ def test_deletion_is_unsuccessful(self):
             self.mockedSendFunction.assert_has_calls(
                 [
                     call(
-                        commands.DeleteHost(name='ns1.sharedhost.com'),
+                        commands.DeleteHost(name="ns1.sharedhost.com"),
                         cleaned=True,
                     ),
                 ]
@@ -2664,7 +2664,7 @@ def test_deletion_with_host_and_contacts(self):
             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 @@ def test_deletion_with_host_and_contacts(self):
         # 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 <Matthewswspence@gmail.com>
Date: Wed, 4 Dec 2024 16:28:33 -0600
Subject: [PATCH 07/29] 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 @@ def __delete__(self, obj):
             """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 @@ def _delete_subdomains(self):
             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 @@ def _delete_domain(self):
     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 @@ def deletedInEpp(self):
             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 @@ def mockUpdateHostCommands(self, _request, cleaned):
     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 @@ def setUp(self):
         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 @@ def test_deletion_is_unsuccessful(self):
             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 @@ def test_deletion_with_host_and_contacts(self):
             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 <Matthewswspence@gmail.com>
Date: Wed, 4 Dec 2024 16:32:17 -0600
Subject: [PATCH 08/29] 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 @@ def test_user_removes_too_many_nameservers(self):
             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,
+                ),
+                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,
                 ),
-                cleaned=True,
-            ),
-            call(commands.DeleteHost(name="ns1.cats-are-superior3.com"), 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 @@ def test_analyst_deletes_domain(self):
 
             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 <Matthewswspence@gmail.com>
Date: Wed, 4 Dec 2024 17:10:45 -0600
Subject: [PATCH 09/29] 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 <Matthewswspence@gmail.com>
Date: Thu, 5 Dec 2024 10:16:40 -0600
Subject: [PATCH 10/29] 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 <Matthewswspence@gmail.com>
Date: Thu, 5 Dec 2024 10:58:12 -0600
Subject: [PATCH 11/29] 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 @@ def get_help_text(cls, state) -> str:
             """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 @@ def test_empty_domain_table(self):
         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 <Matthewswspence@gmail.com>
Date: Thu, 5 Dec 2024 11:30:16 -0600
Subject: [PATCH 12/29] 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 @@ def is_expired(self):
         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 @@ def test_empty_domain_table(self):
         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 <Matthewswspence@gmail.com>
Date: Thu, 5 Dec 2024 12:50:25 -0600
Subject: [PATCH 13/29] 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 @@ def __delete__(self, obj):
             """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 @@ def _delete_subdomains(self):
         """
         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 @@ def test_deletion_is_unsuccessful(self):
             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 @@ def test_member_export(self):
             "Email,Organization admin,Invited by,Joined date,Last active,Domain requests,"
             "Member management,Domain management,Number of domains,Domains\n"
             # Content
-            "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"
+            "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'
             "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 @@ def get_model_annotation_dict(cls, request=None, **kwargs):
             .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 <Matthewswspence@gmail.com>
Date: Thu, 5 Dec 2024 13:55:32 -0600
Subject: [PATCH 14/29] 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 @@ def do_delete_domain(self, request, obj):
         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 @@ def _delete_subdomains(self):
             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 <Matthewswspence@gmail.com>
Date: Thu, 5 Dec 2024 14:21:02 -0600
Subject: [PATCH 15/29] 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 @@ def do_delete_domain(self, request, obj):
         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 @@ def _delete_subdomains(self):
             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 <Matthewswspence@gmail.com>
Date: Thu, 5 Dec 2024 14:50:49 -0600
Subject: [PATCH 16/29] 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 @@ def do_delete_domain(self, request, obj):
             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 @@ def deletedInEpp(self):
             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 <Matthewswspence@gmail.com>
Date: Thu, 5 Dec 2024 15:10:16 -0600
Subject: [PATCH 17/29] 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 @@ def revert_client_hold(self, ignoreEPP=False):
             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 @@ 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)
+            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 @@ def test_analyst_can_see_inline_domain_information_in_domain_change_form(self):
     @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 @@ def test_deletion_is_successful(self):
 
         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 <Matthewswspence@gmail.com>
Date: Mon, 9 Dec 2024 13:34:34 -0600
Subject: [PATCH 18/29] 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 @@ def _delete_subdomains(self):
         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 @@ 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
+            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 1a9b6717584b189a7082f0522e405907ab8b0a4f Mon Sep 17 00:00:00 2001
From: matthewswspence <Matthewswspence@gmail.com>
Date: Wed, 11 Dec 2024 10:11:05 -0600
Subject: [PATCH 19/29] consolidate delete domain function

---
 src/epplibwrapper/errors.py               |   2 +-
 src/registrar/models/domain.py            | 110 +++++--------
 src/registrar/tests/test_admin_domain.py  |   2 +-
 src/registrar/tests/test_models_domain.py | 188 ++++++++++++----------
 4 files changed, 142 insertions(+), 160 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 @@ def registry_contacts(self) -> dict[str, str]:
             { 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 @@ def _remove_client_hold(self):
             # 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"""
 
-    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
+        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("Host %s in use by another domain: %s", host.name, host.domain)
+                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=host.domain,
+                    note=f"Host {host.name} is in use by {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.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 @@ def revert_client_hold(self, ignoreEPP=False):
             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 @@ def deletedInEpp(self):
         # 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 @@ def _get_or_create_contact(self, contact: PublicContact):
 
                 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 @@ def test_deletion_is_unsuccessful(self):
         """
         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 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 @@ def tearDown(self):
         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 @@ def test_analyst_deletes_domain(self):
 
             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 @@ def test_deletion_is_unsuccessful(self):
             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 @@ def test_deletion_with_host_and_contacts(self):
             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="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,
+        # 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.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)
-
-            # 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)
+                    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)
 
+    # @less_console_noise_decorator
     def test_deletion_ready_fsm_failure(self):
         """
         Scenario: Domain deletion is unsuccessful due to FSM rules
@@ -2707,15 +2724,14 @@ def test_deletion_ready_fsm_failure(self):
 
             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 e0eb70abe2e89dac7bdbe69dc389bb8fbaad1374 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 11 Dec 2024 14:12:42 -0500
Subject: [PATCH 20/29] working _delete_domain

---
 src/registrar/models/domain.py | 58 ++++++++++++++++++++++------------
 1 file changed, 38 insertions(+), 20 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 @@ 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.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.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 533e57722fba896d6d6728accb59bb4560cbc6e7 Mon Sep 17 00:00:00 2001
From: matthewswspence <Matthewswspence@gmail.com>
Date: Wed, 11 Dec 2024 14:30:58 -0600
Subject: [PATCH 21/29] 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 @@ 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=[], 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 @@ def _delete_domain(self):
         ) = self.getNameserverChanges(hosts=[])
 
         _ = self._update_host_values(updated_values, oldNameservers)  # returns nothing, just need to be run and errors
-        addToDomainList, addToDomainCount = self.createNewHostList(new_values)
-        deleteHostList, deleteCount = self.createDeleteHostList(deleted_values)
+        addToDomainList, _ = self.createNewHostList(new_values)
+        deleteHostList, _ = self.createDeleteHostList(deleted_values)
         responseCode = self.addAndRemoveHostsFromDomain(hostsToAdd=addToDomainList, hostsToDelete=deleteHostList)
 
         # if unable to update domain raise error and stop

From 51da456f02ba72f09238808129ebc3186571bd7d Mon Sep 17 00:00:00 2001
From: matthewswspence <Matthewswspence@gmail.com>
Date: Thu, 12 Dec 2024 13:45:36 -0600
Subject: [PATCH 22/29] 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 @@ def _delete_domain(self):
 
         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 @@ def _delete_domain(self):
 
         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 @@ 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="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 @@ def test_deletion_is_unsuccessful(self):
             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 @@ def setUp(self):
         """
         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 @@ def test_deletion_is_unsuccessful(self):
             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 @@ 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
+            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 @@ def test_deletion_with_host_and_contacts(self):
                     ),
                     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 <Matthewswspence@gmail.com>
Date: Thu, 12 Dec 2024 15:31:40 -0600
Subject: [PATCH 23/29] 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 @@ def _delete_domain(self):
         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 @@ def test_deletion_with_host_and_contacts(self):
                     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 <matthew.spence@cisa.dhs.gov>
Date: Thu, 12 Dec 2024 15:33:50 -0600
Subject: [PATCH 24/29] 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 @@ def _delete_domain(self):
         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 @@ def _delete_domain(self):
                     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 @@ def _delete_domain(self):
 
         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 @@ def test_analyst_deletes_domain(self):
         # 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 @@ def test_deletion_is_unsuccessful(self):
         # 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 @@ def test_deletion_with_host_and_contacts(self):
         # 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 <matthew.spence@cisa.dhs.gov>
Date: Fri, 13 Dec 2024 15:23:15 -0600
Subject: [PATCH 25/29] 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 @@ def do_delete_domain(self, request, obj):
             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 @@ def nameservers(self, hosts: list[tuple[str, list]]):
 
         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 @@ def _delete_domain(self):
         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 @@ def mockUpdateHostCommands(self, _request, cleaned):
     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 <Matthewswspence@gmail.com>
Date: Fri, 13 Dec 2024 15:27:57 -0600
Subject: [PATCH 26/29] review changes and linting

---
 src/epplibwrapper/errors.py               |  2 +-
 src/registrar/models/domain.py            |  4 +--
 src/registrar/tests/test_models_domain.py | 42 +++++++++++++++--------
 3 files changed, 30 insertions(+), 18 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 @@ def _remove_client_hold(self):
             # 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 @@ def _delete_domain(self):
                     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 @@ def test_deletion_with_host_and_contacts(self):
             [
                 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,23 @@ def test_deletion_with_host_and_contacts(self):
                     ),
                     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,
@@ -2714,15 +2718,19 @@ def test_deletion_with_host_and_contacts(self):
                     ),
                     cleaned=True,
                 ),
+            ]
+        )
+        self.mockedSendFunction.assert_has_calls(
+            [
                 call(
-                    commands.DeleteHost(name='fake.host.com'),
+                    commands.DeleteHost(name="fake.host.com"),
                     cleaned=True,
                 ),
                 call(
                     commands.UpdateDomain(
-                        name='freeman.gov',
+                        name="freeman.gov",
                         add=[],
-                        rem=[common.DomainContact(contact='adminContact', type='admin')],
+                        rem=[common.DomainContact(contact="adminContact", type="admin")],
                         nsset=None,
                         keyset=None,
                         registrant=None,
@@ -2731,14 +2739,14 @@ def test_deletion_with_host_and_contacts(self):
                     cleaned=True,
                 ),
                 call(
-                    commands.DeleteContact(id='adminContact'),
+                    commands.DeleteContact(id="adminContact"),
                     cleaned=True,
                 ),
                 call(
                     commands.UpdateDomain(
-                        name='freeman.gov',
+                        name="freeman.gov",
                         add=[],
-                        rem=[common.DomainContact(contact='techContact', type='tech')],
+                        rem=[common.DomainContact(contact="techContact", type="tech")],
                         nsset=None,
                         keyset=None,
                         registrant=None,
@@ -2747,15 +2755,19 @@ def test_deletion_with_host_and_contacts(self):
                     cleaned=True,
                 ),
                 call(
-                    commands.DeleteContact(id='techContact'),
+                    commands.DeleteContact(id="techContact"),
                     cleaned=True,
                 ),
+            ],
+            any_order=True,
+        )
+        self.mockedSendFunction.assert_has_calls(
+            [
                 call(
-                    commands.DeleteDomain(name='freeman.gov'),
+                    commands.DeleteDomain(name="freeman.gov"),
                     cleaned=True,
                 ),
             ],
-            any_order=True,
         )
 
         # Domain itself should not be deleted

From bb2072debce7a86cae74791d6139f3638b527c3a Mon Sep 17 00:00:00 2001
From: Matthew Spence <matthew.spence@cisa.dhs.gov>
Date: Fri, 13 Dec 2024 16:35:29 -0600
Subject: [PATCH 27/29] 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 @@ def nameservers(self, hosts: list[tuple[str, list]]):
 
         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 <Matthewswspence@gmail.com>
Date: Mon, 16 Dec 2024 10:28:55 -0600
Subject: [PATCH 28/29] 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 @@ def dnssecdata(self, _dnssecdata: Optional[extensions.DNSSECExtension]):
             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 @@ def nameservers(self, hosts: list[tuple[str, list]]):
 
         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 @@ def _delete_domain(self):
         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 @@ def test_deletion_is_unsuccessful(self):
             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 <Matthewswspence@gmail.com>
Date: Mon, 16 Dec 2024 10:35:34 -0600
Subject: [PATCH 29/29] 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 @@ def dnssecdata(self, _dnssecdata: Optional[extensions.DNSSECExtension]):
             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 @@ def test_deletion_is_unsuccessful(self):
             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,
             )