Rework delete from epp

src/registrar/models/domain.py

@@ -744,7 +744,12 @@ class Domain(TimeStampedModel, DomainHelper):
 
         successTotalNameservers = len(oldNameservers) - deleteCount + addToDomainCount
 
-        self._delete_hosts_if_not_used(hostsToDelete=deleted_values)
+        try:
+            self._delete_hosts_if_not_used(hostsToDelete=deleted_values)
+        except:
+            # in this case we don't care if there's an error, and it will be logged in the function.
+            pass
+
         if successTotalNameservers < 2:
             try:
                 self.dns_needed()
@@ -1032,19 +1037,28 @@ class Domain(TimeStampedModel, DomainHelper):
         RegistryErrors will be logged and raised. Additional 
         error handling should be provided by the caller.
         """
+        logger.info("Deleting contacts for %s", self.name)
         contacts = self._cache.get("contacts")
-        for contact in contacts:
-            self._delete_contact(contact)
+        logger.debug("Contacts to delete for %s inside _delete_contacts -> %s", self.name, contacts)
+        if contacts:
+            for contact in contacts:
+                self._delete_contact(contact)
+        
         
     def _delete_subdomains(self):
         """Subdomains of this domain should be deleted from the registry.
         Subdomains which are used by other domains (eg as a hostname) will
         not be deleted.
 
-        Supresses registry error, as registry can disallow delete for various reasons
+        raises:
+            RegistryError: if any subdomain cannot be deleted
         """
+        logger.info("Deleting nameservers for %s", self.name)
         nameservers = [n[0] for n in self.nameservers]
-        hostsToDelete = self.createDeleteHostList(nameservers)
+        logger.info("Nameservers found: %s", nameservers)
+        hostsToDelete, _ = self.createDeleteHostList(nameservers)
+        logger.debug("HostsToDelete from %s inside _delete_subdomains -> %s", self.name, hostsToDelete)
+
         self._delete_hosts_if_not_used(hostsToDelete)
 
     def _delete_domain(self):
@@ -1665,7 +1679,7 @@ class Domain(TimeStampedModel, DomainHelper):
                 raise e
     
     def _delete_contact(self, contact: PublicContact):
-        """Try to delete a contact. RegistryErrors will be logged.
+        """Try to delete a contact from the registry.
         
         raises:
             RegistryError: if the registry is unable to delete the contact
@@ -1790,7 +1804,6 @@ class Domain(TimeStampedModel, DomainHelper):
         """delete the host object in registry,
         will only delete the host object, if it's not being used by another domain
         Performs just the DeleteHost epp call
-        Supresses regstry error, as registry can disallow delete for various reasons
         Args:
             hostsToDelete (list[str])- list of nameserver/host names to remove
         Returns:
@@ -1808,6 +1821,8 @@ class Domain(TimeStampedModel, DomainHelper):
                 logger.info("Did not remove host %s because it is in use on another domain." % nameserver)
             else:
                 logger.error("Error _delete_hosts_if_not_used, code was %s error was %s" % (e.code, e))
+            
+            raise e
 
     def _fix_unknown_state(self, cleaned):
         """

src/registrar/tests/common.py

@@ -1279,6 +1279,15 @@ class MockEppLib(TestCase):
         hosts=["fake.host.com"],
     )
 
+    infoDomainSharedHost = fakedEppObject(
+        "sharedHost.gov",
+        cr_date=make_aware(datetime(2023, 5, 25, 19, 45, 35)),
+        contacts=[],
+        hosts=[
+            "ns1.sharedhost.com",
+        ],
+    )
+
     infoDomainThreeHosts = fakedEppObject(
         "my-nameserver.gov",
         cr_date=make_aware(datetime(2023, 5, 25, 19, 45, 35)),
@@ -1496,10 +1505,7 @@ class MockEppLib(TestCase):
             case commands.UpdateHost:
                 return self.mockUpdateHostCommands(_request, cleaned)
             case commands.DeleteHost:
-                return MagicMock(
-                    res_data=[self.mockDataHostChange],
-                    code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY,
-                )
+                return self.mockDeletHostCommands(_request, cleaned)
             case commands.CheckDomain:
                 return self.mockCheckDomainCommand(_request, cleaned)
             case commands.DeleteDomain:
@@ -1551,6 +1557,16 @@ class MockEppLib(TestCase):
                 res_data=[self.mockDataHostChange],
                 code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY,
             )
+        
+    def mockDeletHostCommands(self, _request, cleaned):
+        hosts = getattr(_request, "name", None).hosts
+        for host in hosts:
+            if "sharedhost.com" in host:
+                raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) 
+        return MagicMock(
+            res_data=[self.mockDataHostChange],
+            code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY,
+        )
 
     def mockUpdateDomainCommands(self, _request, cleaned):
         if getattr(_request, "name", None) == "dnssec-invalid.gov":
@@ -1563,10 +1579,7 @@ class MockEppLib(TestCase):
 
     def mockDeleteDomainCommands(self, _request, cleaned):
         if getattr(_request, "name", None) == "failDelete.gov":
-            name = getattr(_request, "name", None)
-            fake_nameserver = "ns1.failDelete.gov"
-            if name in fake_nameserver:
-                raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION)
+            raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION)
         return None
 
     def mockRenewDomainCommand(self, _request, cleaned):
@@ -1636,6 +1649,7 @@ class MockEppLib(TestCase):
             "subdomainwoip.gov": (self.mockDataInfoDomainSubdomainNoIP, None),
             "ddomain3.gov": (self.InfoDomainWithContacts, None),
             "igorville.gov": (self.InfoDomainWithContacts, None),
+            "sharingiscaring.gov": (self.infoDomainSharedHost, None),
         }
 
         # Retrieve the corresponding values from the dictionary

src/registrar/tests/test_models_domain.py

@@ -2585,6 +2585,7 @@ class TestAnalystDelete(MockEppLib):
         self.domain_on_hold, _ = Domain.objects.get_or_create(name="fake-on-hold.gov", state=Domain.State.ON_HOLD)
 
     def tearDown(self):
+        Host.objects.all().delete()
         Domain.objects.all().delete()
         super().tearDown()
 
@@ -2597,39 +2598,39 @@ class TestAnalystDelete(MockEppLib):
 
             The deleted date is set.
         """
-        with less_console_noise():
-            # Put the domain in client hold
-            self.domain.place_client_hold()
-            # Delete it...
-            self.domain.deletedInEpp()
-            self.domain.save()
-            self.mockedSendFunction.assert_has_calls(
-                [
-                    call(
-                        commands.DeleteDomain(name="fake.gov"),
-                        cleaned=True,
-                    )
-                ]
-            )
-            # Domain itself should not be deleted
-            self.assertNotEqual(self.domain, None)
-            # Domain should have the right state
-            self.assertEqual(self.domain.state, Domain.State.DELETED)
-            # Domain should have a deleted
-            self.assertNotEqual(self.domain.deleted, None)
-            # Cache should be invalidated
-            self.assertEqual(self.domain._cache, {})
+        # with less_console_noise():
+        # Put the domain in client hold
+        self.domain.place_client_hold()
+        # Delete it...
+        self.domain.deletedInEpp()
+        self.domain.save()
+        self.mockedSendFunction.assert_has_calls(
+            [
+                call(
+                    commands.DeleteDomain(name="fake.gov"),
+                    cleaned=True,
+                )
+            ]
+        )
+        # Domain itself should not be deleted
+        self.assertNotEqual(self.domain, None)
+        # Domain should have the right state
+        self.assertEqual(self.domain.state, Domain.State.DELETED)
+        # Domain should have a deleted
+        self.assertNotEqual(self.domain.deleted, None)
+        # Cache should be invalidated
+        self.assertEqual(self.domain._cache, {})
 
     def test_deletion_is_unsuccessful(self):
         """
         Scenario: Domain deletion is unsuccessful
-            When a subdomain exists
+            When a subdomain exists that is in use by another domain
             Then a client error is returned of code 2305
             And `state` is not set to `DELETED`
         """
         with less_console_noise():
             # Desired domain
-            domain, _ = Domain.objects.get_or_create(name="failDelete.gov", state=Domain.State.ON_HOLD)
+            domain, _ = Domain.objects.get_or_create(name="sharingiscaring.gov", state=Domain.State.ON_HOLD)
             # Put the domain in client hold
             domain.place_client_hold()
             # Delete it
@@ -2640,7 +2641,7 @@ class TestAnalystDelete(MockEppLib):
             self.mockedSendFunction.assert_has_calls(
                 [
                     call(
-                        commands.DeleteDomain(name="failDelete.gov"),
+                        commands.DeleteHost(name=common.HostObjSet(hosts=['ns1.sharedhost.com'])),
                         cleaned=True,
                     )
                 ]