Merge branch 'main' into za/850-epp-contact-get

src/registrar/admin.py

@@ -6,10 +6,13 @@ from django.contrib.auth.admin import UserAdmin as BaseUserAdmin
 from django.contrib.contenttypes.models import ContentType
 from django.http.response import HttpResponseRedirect
 from django.urls import reverse
+from epplibwrapper.errors import ErrorCode, RegistryError
+from registrar.models.domain import Domain
 from registrar.models.utility.admin_sort_fields import AdminSortFields
 from . import models
 from auditlog.models import LogEntry  # type: ignore
 from auditlog.admin import LogEntryAdmin  # type: ignore
+from django_fsm import TransitionNotAllowed  # type: ignore
 
 logger = logging.getLogger(__name__)
 
@@ -716,16 +719,61 @@ class DomainAdmin(ListHeaderAdmin):
         return super().response_change(request, obj)
 
     def do_delete_domain(self, request, obj):
+        if not isinstance(obj, Domain):
+            # Could be problematic if the type is similar,
+            # but not the same (same field/func names).
+            # We do not want to accidentally delete records.
+            self.message_user(request, "Object is not of type Domain", messages.ERROR)
+            return
+
         try:
-            obj.deleted()
+            obj.deletedInEpp()
             obj.save()
-        except Exception as err:
+        except RegistryError as err:
-            self.message_user(request, err, messages.ERROR)
+            # 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,
+            }
+
+            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)
+            self.message_user(
+                request, f"Error deleting this Domain: {message}", messages.ERROR
+            )
+        except TransitionNotAllowed:
+            if obj.state == Domain.State.DELETED:
+                self.message_user(
+                    request,
+                    "This domain is already deleted",
+                    messages.INFO,
+                )
+            else:
+                self.message_user(
+                    request,
+                    "Error deleting this Domain: "
+                    f"Can't switch from state '{obj.state}' to 'deleted'"
+                    ", must be either 'dns_needed' or 'on_hold'",
+                    messages.ERROR,
+                )
+        except Exception:
+            self.message_user(
+                request,
+                "Could not delete: An unspecified error occured",
+                messages.ERROR,
+            )
         else:
             self.message_user(
                 request,
-                ("Domain %s Should now be deleted " ". Thanks!") % obj.name,
+                ("Domain %s has been deleted. Thanks!") % obj.name,
             )
+
         return HttpResponseRedirect(".")
 
     def do_get_status(self, request, obj):

src/registrar/models/domain.py

@@ -2,7 +2,7 @@ from itertools import zip_longest
 import logging
 from datetime import date
 from string import digits
-from django_fsm import FSMField, transition  # type: ignore
+from django_fsm import FSMField, transition, TransitionNotAllowed  # type: ignore
 
 from django.db import models
 
@@ -592,11 +592,6 @@ class Domain(TimeStampedModel, DomainHelper):
         """
         return self.state == self.State.READY
 
-    def delete_request(self):
-        """Delete from host. Possibly a duplicate of _delete_host?"""
-        # TODO fix in ticket #901
-        pass
-
     def transfer(self):
         """Going somewhere. Not implemented."""
         raise NotImplementedError()
@@ -641,7 +636,7 @@ class Domain(TimeStampedModel, DomainHelper):
         """This domain should be deleted from the registry
         may raises RegistryError, should be caught or handled correctly by caller"""
         request = commands.DeleteDomain(name=self.name)
-        registry.send(request)
+        registry.send(request, cleaned=True)
 
     def __str__(self) -> str:
         return self.name
@@ -998,16 +993,32 @@ class Domain(TimeStampedModel, DomainHelper):
         self._remove_client_hold()
         # TODO -on the client hold ticket any additional error handling here
 
-    @transition(field="state", source=State.ON_HOLD, target=State.DELETED)
+    @transition(
-    def deleted(self):
+        field="state", source=[State.ON_HOLD, State.DNS_NEEDED], target=State.DELETED
-        """domain is deleted in epp but is saved in our database"""
+    )
-        # TODO Domains may not be deleted if:
+    def deletedInEpp(self):
-        #  a child host is being used by
+        """Domain is deleted in epp but is saved in our database.
-        # another .gov domains.  The host must be first removed
+        Error handling should be provided by the caller."""
-        # and/or renamed before the parent domain may be deleted.
+        # While we want to log errors, we want to preserve
-        logger.info("pendingCreate()-> inside pending create")
+        # that information when this function is called.
-        self._delete_domain()
+        # Human-readable errors are introduced at the admin.py level,
-        # TODO - delete ticket any additional error handling here
+        # as doing everything here would reduce reliablity.
+        try:
+            logger.info("deletedInEpp()-> inside _delete_domain")
+            self._delete_domain()
+        except RegistryError as err:
+            logger.error(f"Could not delete domain. Registry returned error: {err}")
+            raise err
+        except TransitionNotAllowed as err:
+            logger.error("Could not delete domain. FSM failure: {err}")
+            raise err
+        except Exception as err:
+            logger.error(
+                f"Could not delete domain. An unspecified error occured: {err}"
+            )
+            raise err
+        else:
+            self._invalidate_cache()
 
     @transition(
         field="state",

src/registrar/models/domain_application.py

@@ -6,6 +6,7 @@ import logging
 from django.apps import apps
 from django.db import models
 from django_fsm import FSMField, transition  # type: ignore
+from registrar.models.domain import Domain
 
 from .utility.time_stamped_model import TimeStampedModel
 from ..utility.email import send_templated_email, EmailSendingError
@@ -610,9 +611,11 @@ class DomainApplication(TimeStampedModel):
 
         As side effects this will delete the domain and domain_information
         (will cascade), and send an email notification."""
-
         if self.status == self.APPROVED:
-            self.approved_domain.delete_request()
+            domain_state = self.approved_domain.state
+            # Only reject if it exists on EPP
+            if domain_state != Domain.State.UNKNOWN:
+                self.approved_domain.deletedInEpp()
             self.approved_domain.delete()
             self.approved_domain = None
 
@@ -638,7 +641,10 @@ class DomainApplication(TimeStampedModel):
         and domain_information (will cascade) when they exist."""
 
         if self.status == self.APPROVED:
-            self.approved_domain.delete_request()
+            domain_state = self.approved_domain.state
+            # Only reject if it exists on EPP
+            if domain_state != Domain.State.UNKNOWN:
+                self.approved_domain.deletedInEpp()
             self.approved_domain.delete()
             self.approved_domain = None
 

src/registrar/templates/django/admin/domain_change_form.html

@@ -15,7 +15,9 @@
         {% endif %}
         <input id="manageDomainSubmitButton" type="submit" value="Manage Domain" name="_edit_domain">
         <input type="submit" value="get status" name="_get_status">
-        <input type="submit" value="EPP Delete Domain" name="_delete_domain">
+        {% if original.state != original.State.DELETED %}
+        <input type="submit" value="Delete Domain in Registry" name="_delete_domain">
+        {% endif %}
     </div>
     {{ block.super }}
 {% endblock %}

src/registrar/tests/common.py

@@ -709,6 +709,16 @@ class MockEppLib(TestCase):
             # use this for when a contact is being updated
             # sets the second send() to fail
             raise RegistryError(code=ErrorCode.OBJECT_EXISTS)
+        elif (
+            isinstance(_request, commands.DeleteDomain)
+            and 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
+                )
         return MagicMock(res_data=[self.mockDataInfoHosts])
 
     def setUp(self):

src/registrar/tests/test_admin.py

@@ -49,6 +49,7 @@ class TestDomainAdmin(MockEppLib):
         self.client = Client(HTTP_HOST="localhost:8080")
         self.superuser = create_superuser()
         self.staffuser = create_user()
+        self.factory = RequestFactory()
         super().setUp()
 
     def test_place_and_remove_hold(self):
@@ -87,6 +88,155 @@ class TestDomainAdmin(MockEppLib):
         self.assertContains(response, "Place hold")
         self.assertNotContains(response, "Remove hold")
 
+    def test_deletion_is_successful(self):
+        """
+        Scenario: Domain deletion is unsuccessful
+            When the domain is deleted
+            Then a user-friendly success message is returned for displaying on the web
+            And `state` is et to `DELETED`
+        """
+        domain = create_ready_domain()
+        # Put in client hold
+        domain.place_client_hold()
+        p = "userpass"
+        self.client.login(username="staffuser", password=p)
+
+        # 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, "Delete Domain in Registry")
+
+        # Test the info dialog
+        request = self.factory.post(
+            "/admin/registrar/domain/{}/change/".format(domain.pk),
+            {"_delete_domain": "Delete Domain in 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.INFO,
+                "Domain city.gov has been deleted. Thanks!",
+                extra_tags="",
+                fail_silently=False,
+            )
+
+        self.assertEqual(domain.state, Domain.State.DELETED)
+
+    def test_deletion_ready_fsm_failure(self):
+        """
+        Scenario: Domain deletion is unsuccessful
+            When an error is returned from epplibwrapper
+            Then a user-friendly error message is returned for displaying on the web
+            And `state` is not set to `DELETED`
+        """
+        domain = create_ready_domain()
+        p = "userpass"
+        self.client.login(username="staffuser", password=p)
+
+        # 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, "Delete Domain in Registry")
+
+        # Test the error
+        request = self.factory.post(
+            "/admin/registrar/domain/{}/change/".format(domain.pk),
+            {"_delete_domain": "Delete Domain in 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: "
+                "Can't switch from state 'ready' to 'deleted'"
+                ", must be either 'dns_needed' or 'on_hold'",
+                extra_tags="",
+                fail_silently=False,
+            )
+
+        self.assertEqual(domain.state, Domain.State.READY)
+
+    def test_analyst_deletes_domain_idempotent(self):
+        """
+        Scenario: Analyst tries to delete an already deleted domain
+            Given `state` is already `DELETED`
+            When `domain.deletedInEpp()` is called
+            Then `commands.DeleteDomain` is sent to the registry
+            And Domain returns normally without an error dialog
+        """
+        domain = create_ready_domain()
+        # Put in client hold
+        domain.place_client_hold()
+        p = "userpass"
+        self.client.login(username="staffuser", password=p)
+
+        # 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, "Delete Domain in Registry")
+
+        # Test the info dialog
+        request = self.factory.post(
+            "/admin/registrar/domain/{}/change/".format(domain.pk),
+            {"_delete_domain": "Delete Domain in Registry", "name": domain.name},
+            follow=True,
+        )
+        request.user = self.client
+
+        # Delete it once
+        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.INFO,
+                "Domain city.gov has been deleted. Thanks!",
+                extra_tags="",
+                fail_silently=False,
+            )
+
+        self.assertEqual(domain.state, Domain.State.DELETED)
+
+        # Try to delete it again
+        # Test the info dialog
+        request = self.factory.post(
+            "/admin/registrar/domain/{}/change/".format(domain.pk),
+            {"_delete_domain": "Delete Domain in 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.INFO,
+                "This domain is already deleted",
+                extra_tags="",
+                fail_silently=False,
+            )
+
+        self.assertEqual(domain.state, Domain.State.DELETED)
+
     @skip("Waiting on epp lib to implement")
     def test_place_and_remove_hold_epp(self):
         raise
@@ -138,8 +288,9 @@ class TestDomainApplicationAdminForm(TestCase):
         )
 
 
-class TestDomainApplicationAdmin(TestCase):
+class TestDomainApplicationAdmin(MockEppLib):
     def setUp(self):
+        super().setUp()
         self.site = AdminSite()
         self.factory = RequestFactory()
         self.admin = DomainApplicationAdmin(
@@ -690,6 +841,7 @@ class TestDomainApplicationAdmin(TestCase):
             domain_information.refresh_from_db()
 
     def tearDown(self):
+        super().tearDown()
         Domain.objects.all().delete()
         DomainInformation.objects.all().delete()
         DomainApplication.objects.all().delete()

src/registrar/tests/test_models_domain.py

@@ -16,7 +16,7 @@ from registrar.models.draft_domain import DraftDomain
 from registrar.models.public_contact import PublicContact
 from registrar.models.user import User
 from .common import MockEppLib
-
+from django_fsm import TransitionNotAllowed  # type: ignore
 from epplibwrapper import (
     commands,
     common,
@@ -1255,7 +1255,7 @@ class TestAnalystLock(TestCase):
         raise
 
 
-class TestAnalystDelete(TestCase):
+class TestAnalystDelete(MockEppLib):
     """Rule: Analysts may delete a domain"""
 
     def setUp(self):
@@ -1264,35 +1264,99 @@ class TestAnalystDelete(TestCase):
             Given the analyst is logged in
             And a domain exists in the registry
         """
-        pass
+        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
+        )
+
+    def tearDown(self):
+        Domain.objects.all().delete()
+        super().tearDown()
 
-    @skip("not implemented yet")
     def test_analyst_deletes_domain(self):
         """
         Scenario: Analyst permanently deletes a domain
-            When `domain.delete()` is called
+            When `domain.deletedInEpp()` is called
             Then `commands.DeleteDomain` is sent to the registry
             And `state` is set to `DELETED`
         """
-        raise
+        # Put the domain in client hold
+        self.domain.place_client_hold()
+        # Delete it...
+        self.domain.deletedInEpp()
+        self.mockedSendFunction.assert_has_calls(
+            [
+                call(
+                    commands.DeleteDomain(name="fake.gov"),
+                    cleaned=True,
+                )
+            ]
+        )
 
-    @skip("not implemented yet")
+        # Domain itself should not be deleted
-    def test_analyst_deletes_domain_idempotent(self):
+        self.assertNotEqual(self.domain, None)
-        """
+
-        Scenario: Analyst tries to delete an already deleted domain
+        # Domain should have the right state
-            Given `state` is already `DELETED`
+        self.assertEqual(self.domain.state, Domain.State.DELETED)
-            When `domain.delete()` is called
+
-            Then `commands.DeleteDomain` is sent to the registry
+        # Cache should be invalidated
-            And Domain returns normally (without error)
+        self.assertEqual(self.domain._cache, {})
-        """
-        raise
 
-    @skip("not implemented yet")
     def test_deletion_is_unsuccessful(self):
         """
         Scenario: Domain deletion is unsuccessful
-            When an error is returned from epplibwrapper
+            When a subdomain exists
-            Then a user-friendly error message is returned for displaying on the web
+            Then a client error is returned of code 2305
             And `state` is not set to `DELETED`
         """
-        raise
+        # Desired domain
+        domain, _ = Domain.objects.get_or_create(
+            name="failDelete.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()
+            self.assertTrue(
+                err.is_client_error()
+                and err.code == ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION
+            )
+        self.mockedSendFunction.assert_has_calls(
+            [
+                call(
+                    commands.DeleteDomain(name="failDelete.gov"),
+                    cleaned=True,
+                )
+            ]
+        )
+
+        # Domain itself should not be deleted
+        self.assertNotEqual(domain, None)
+        # State should not have changed
+        self.assertEqual(domain.state, Domain.State.ON_HOLD)
+
+    def test_deletion_ready_fsm_failure(self):
+        """
+        Scenario: Domain deletion is unsuccessful due to FSM rules
+            Given state is 'ready'
+            When `domain.deletedInEpp()` is called
+            and domain is of `state` is `READY`
+            Then an FSM error is returned
+            And `state` is not set to `DELETED`
+        """
+        self.assertEqual(self.domain.state, Domain.State.READY)
+        with self.assertRaises(TransitionNotAllowed) as err:
+            self.domain.deletedInEpp()
+            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)