From c6baf9c98bcc999e12de5c8ec3bbba044544f82f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 27 Sep 2023 14:45:33 -0600 Subject: [PATCH] Preliminary changes --- src/registrar/admin.py | 43 ++++++- src/registrar/models/domain.py | 47 +++++--- src/registrar/models/domain_application.py | 8 +- src/registrar/tests/common.py | 7 ++ src/registrar/tests/test_models_domain.py | 128 ++++++++++++++++++--- 5 files changed, 197 insertions(+), 36 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e99e767bd..6ec27a085 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -6,11 +6,12 @@ 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.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__) @@ -717,10 +718,46 @@ class DomainAdmin(ListHeaderAdmin): def do_delete_domain(self, request, obj): try: - obj.deleted() + obj.deletedInEpp() obj.save() - except Exception as err: + except RegistryError as err: + if err.is_connection_error(): + self.message_user( + request, + "Error connecting to the registry", + messages.ERROR, + ) + elif err.code == ErrorCode.OBJECT_STATUS_PROHIBITS_OPERATION: + self.message_user( + request, + "Error deleting this Domain: " + f"Cannot delete Domain when in status {obj.status}", + messages.ERROR, + ) + elif err.code == ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION: + self.message_user( + request, + "Error deleting this Domain: " + f" This subdomain is being used as a hostname on another domain", + messages.ERROR, + ) + elif err.code: + self.message_user( + request, + f"Error deleting this Domain: {err}", + messages.ERROR, + ) + else: + # all other type error messages, display the error + self.message_user(request, err, messages.ERROR) + except ValueError as err: self.message_user(request, err, messages.ERROR) + except TransitionNotAllowed + self.message_user( + request, + f"Error deleting this Domain: {err}", + messages.ERROR, + ) else: self.message_user( request, diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 2c7f8703c..827d26073 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -609,11 +609,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() @@ -658,7 +653,8 @@ 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) + response = registry.send(request, cleaned=True) + return response def __str__(self) -> str: return self.name @@ -773,6 +769,8 @@ class Domain(TimeStampedModel, DomainHelper): self.addAllDefaults() + + def addAllDefaults(self): security_contact = self.get_default_security_contact() security_contact.save() @@ -805,15 +803,34 @@ class Domain(TimeStampedModel, DomainHelper): # TODO -on the client hold ticket any additional error handling here @transition(field="state", source=State.ON_HOLD, target=State.DELETED) - def deleted(self): - """domain is deleted in epp but is saved in our database""" - # TODO Domains may not be deleted if: - # a child host is being used by - # another .gov domains. The host must be first removed - # and/or renamed before the parent domain may be deleted. - logger.info("pendingCreate()-> inside pending create") - self._delete_domain() - # TODO - delete ticket any additional error handling here + def deletedInEpp(self): + """domain is deleted in epp but is saved in our database. + Returns the request_code""" + valid_delete_states = [ + self.State.ON_HOLD, + self.State.DNS_NEEDED + ] + # Check that the domain contacts a valid status + if (self.state not in valid_delete_states): + raise ValueError( + f"Invalid domain state of {self.state}. Cannot delete." + ) + + 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 Exception as err: + logger.error( + f"Could not delete domain. An unspecified error occured: {err}" + ) + raise err + else: + self._invalidate_cache() @transition( field="state", diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index 7df51baf4..c9d32d978 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -598,7 +598,11 @@ class DomainApplication(TimeStampedModel): "emails/domain_request_withdrawn.txt", "emails/domain_request_withdrawn_subject.txt", ) - + + # TODO + #def delete(self, *args, **kwargs): + #super().delete(*args, **kwargs) + @transition( field="status", source=[IN_REVIEW, APPROVED], @@ -612,7 +616,7 @@ class DomainApplication(TimeStampedModel): (will cascade), and send an email notification.""" if self.status == self.APPROVED: - self.approved_domain.delete_request() + self.approved_domain.deletedInEpp() self.approved_domain.delete() self.approved_domain = None diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index fe41647f9..9fc85b76b 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -604,6 +604,13 @@ 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) == "fail.gov" + ): + raise RegistryError( + code=ErrorCode.OBJECT_STATUS_PROHIBITS_OPERATION + ) return MagicMock(res_data=[self.mockDataInfoHosts]) def setUp(self): diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 54045bb32..842f2e116 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -940,39 +940,95 @@ class TestAnalystLock(TestCase): raise -class TestAnalystDelete(TestCase): +class TestAnalystDelete(MockEppLib): """Rule: Analysts may delete a domain""" - def setUp(self): - """ - Background: - Given the analyst is logged in - And a domain exists in the registry - """ - pass + """ + Background: + Given the analyst is logged in + And a domain exists in the registry + """ + 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, + ) + ] + ) + # 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) - @skip("not implemented yet") def test_analyst_deletes_domain_idempotent(self): """ Scenario: Analyst tries to delete an already deleted domain Given `state` is already `DELETED` - When `domain.delete()` is called + When `domain.deletedInEpp()` is called Then `commands.DeleteDomain` is sent to the registry And Domain returns normally (without error) """ - 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, + ) + ] + ) + # 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) + + # Delete it again - monitoring for errors + try: + self.domain.deletedInEpp() + except Exception as err: + self.fail("deletedInEpp() threw an error") + raise err + + 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) - @skip("not implemented yet") def test_deletion_is_unsuccessful(self): """ Scenario: Domain deletion is unsuccessful @@ -980,4 +1036,44 @@ class TestAnalystDelete(TestCase): Then a user-friendly error message is returned for displaying on the web And `state` is not set to `DELETED` """ - raise + domain, _ = Domain.objects.get_or_create( + name="fail.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_STATUS_PROHIBITS_OPERATION + ) + # TODO - check UI for error + # Domain itself should not be deleted + self.assertNotEqual(domain, None) + # State should not have changed + self.assertEqual(domain.state, Domain.State.ON_HOLD) + + @skip("not implemented yet") + 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 + Then a user-friendly error message is returned for displaying on the web + And `state` is not set to `DELETED` + """ + self.domain.deletedInEpp() + self.mockedSendFunction.assert_has_calls( + [ + call( + commands.DeleteDomain(name="fake.gov", auth_info=None), + cleaned=True, + ) + ] + ) + # Domain should not be deleted + self.assertNotEqual(self.domain, None) + # Domain should have the right state + self.assertEqual(self.domain.state, "DELETED")