From 0e5978011dec06809c982396fd981666227da2c8 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 17 Oct 2023 14:23:42 -0400 Subject: [PATCH 1/2] Error handling on the post for security contact --- src/epplibwrapper/__init__.py | 4 ++- src/epplibwrapper/errors.py | 3 ++ src/registrar/models/domain.py | 2 +- src/registrar/tests/common.py | 16 +++++++++ src/registrar/tests/test_views.py | 60 +++++++++++++++++++++++++++++++ src/registrar/views/domain.py | 30 +++++++++++++--- 6 files changed, 108 insertions(+), 7 deletions(-) diff --git a/src/epplibwrapper/__init__.py b/src/epplibwrapper/__init__.py index dd6664a3a..d0138d73c 100644 --- a/src/epplibwrapper/__init__.py +++ b/src/epplibwrapper/__init__.py @@ -45,7 +45,7 @@ except NameError: # Attn: these imports should NOT be at the top of the file try: from .client import CLIENT, commands - from .errors import RegistryError, ErrorCode + from .errors import RegistryError, ErrorCode, CANNOT_CONTACT_REGISTRY, GENERIC_ERROR from epplib.models import common, info from epplib.responses import extensions from epplib import responses @@ -61,4 +61,6 @@ __all__ = [ "info", "ErrorCode", "RegistryError", + "CANNOT_CONTACT_REGISTRY", + "GENERIC_ERROR", ] diff --git a/src/epplibwrapper/errors.py b/src/epplibwrapper/errors.py index d34ed5e91..dba5f328c 100644 --- a/src/epplibwrapper/errors.py +++ b/src/epplibwrapper/errors.py @@ -1,5 +1,8 @@ from enum import IntEnum +CANNOT_CONTACT_REGISTRY = "Update failed. Cannot contact the registry." +GENERIC_ERROR = "Value entered was wrong." + class ErrorCode(IntEnum): """ diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index d6dd5e287..fa3ff443c 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -701,7 +701,7 @@ class Domain(TimeStampedModel, DomainHelper): and errorCode != ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY ): # TODO- ticket #433 look here for error handling - raise Exception("Unable to add contact to registry") + raise RegistryError(code=errorCode) # contact doesn't exist on the domain yet logger.info("_set_singleton_contact()-> contact has been added to the registry") diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index b8fea7f93..803c2f069 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -32,6 +32,8 @@ from epplibwrapper import ( ErrorCode, ) +from registrar.models.utility.contact_error import ContactError, ContactErrorCodes + logger = logging.getLogger(__name__) @@ -794,6 +796,20 @@ 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.CreateContact) + and getattr(_request, "email", None) == "test@failCreate.gov" + ): + # use this for when a contact is being updated + # mocks a registry error on creation + raise RegistryError(code=None) + elif ( + isinstance(_request, commands.CreateContact) + and getattr(_request, "email", None) == "test@contactError.gov" + ): + # use this for when a contact is being updated + # mocks a registry error on creation + raise ContactError(code=ContactErrorCodes.CONTACT_TYPE_NONE) elif isinstance(_request, commands.CreateHost): return MagicMock( res_data=[self.mockDataHostChange], diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 2194b42db..bda23546b 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1490,6 +1490,66 @@ class TestDomainDetail(TestWithDomainPermissions, WebTest, MockEppLib): success_page, "The security email for this domain has been updated" ) + def test_security_email_form_messages(self): + """ + Test against the success and error messages that are defined in the view + """ + p = "adminpass" + self.client.login(username="superuser", password=p) + + form_data_registry_error = { + "security_email": "test@failCreate.gov", + } + + form_data_contact_error = { + "security_email": "test@contactError.gov", + } + + form_data_success = { + "security_email": "test@something.gov", + } + + test_cases = [ + ( + "RegistryError", + form_data_registry_error, + "Update failed. Cannot contact the registry.", + ), + ("ContactError", form_data_contact_error, "Value entered was wrong."), + ( + "RegistrySuccess", + form_data_success, + "The security email for this domain has been updated.", + ), + # Add more test cases with different scenarios here + ] + + for test_name, data, expected_message in test_cases: + response = self.client.post( + reverse("domain-security-email", kwargs={"pk": self.domain.id}), + data=data, + follow=True, + ) + + # Check the response status code, content, or any other relevant assertions + self.assertEqual(response.status_code, 200) + + # Check if the expected message tag is set + if test_name == "RegistryError" or test_name == "ContactError": + message_tag = "error" + elif test_name == "RegistrySuccess": + message_tag = "success" + else: + # Handle other cases if needed + message_tag = "info" # Change to the appropriate default + + # Check the message tag + messages = list(response.context["messages"]) + self.assertEqual(len(messages), 1) + message = messages[0] + self.assertEqual(message.tags, message_tag) + self.assertEqual(message.message, expected_message) + def test_domain_overview_blocked_for_ineligible_user(self): """We could easily duplicate this test for all domain management views, but a single url test should be solid enough since all domain diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 4ea3d2fbc..e993a7c1a 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -22,6 +22,7 @@ from registrar.models import ( UserDomainRole, ) from registrar.models.public_contact import PublicContact +from registrar.models.utility.contact_error import ContactError from ..forms import ( ContactForm, @@ -30,6 +31,13 @@ from ..forms import ( DomainSecurityEmailForm, NameserverFormset, ) + +from epplibwrapper import ( + RegistryError, + CANNOT_CONTACT_REGISTRY, + GENERIC_ERROR, +) + from ..utility.email import send_templated_email, EmailSendingError from .utility import DomainPermissionView, DomainInvitationPermissionDeleteView @@ -310,15 +318,27 @@ class DomainSecurityEmailView(DomainPermissionView, FormMixin): # If no default is created for security_contact, # then we cannot connect to the registry. if contact is None: - messages.error(self.request, "Update failed. Cannot contact the registry.") + messages.error(self.request, CANNOT_CONTACT_REGISTRY) return redirect(self.get_success_url()) contact.email = new_email - contact.save() - messages.success( - self.request, "The security email for this domain has been updated." - ) + try: + contact.save() + except RegistryError as Err: + if Err.is_connection_error(): + messages.error(self.request, CANNOT_CONTACT_REGISTRY) + logger.error(f"Registry connection error: {Err}") + else: + messages.error(self.request, GENERIC_ERROR) + logger.error(f"Registry error: {Err}") + except ContactError as Err: + messages.error(self.request, GENERIC_ERROR) + logger.error(f"Generic registry error: {Err}") + else: + messages.success( + self.request, "The security email for this domain has been updated." + ) # superclass has the redirect return redirect(self.get_success_url()) From 7ed916a86489c295b50e6f1cda06ac4e7bc7c6e1 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 17 Oct 2023 14:56:50 -0400 Subject: [PATCH 2/2] lint --- src/registrar/tests/common.py | 42 +++++++++++++++---------------- src/registrar/tests/test_views.py | 1 + 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index f73082ab4..a9f38db03 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -795,28 +795,8 @@ class MockEppLib(TestCase): return self.mockInfoContactCommands(_request, cleaned) elif isinstance(_request, commands.UpdateDomain): return self.mockUpdateDomainCommands(_request, cleaned) - elif ( - isinstance(_request, commands.CreateContact) - and getattr(_request, "id", None) == "fail" - and self.mockedSendFunction.call_count == 3 - ): - # 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.CreateContact) - and getattr(_request, "email", None) == "test@failCreate.gov" - ): - # use this for when a contact is being updated - # mocks a registry error on creation - raise RegistryError(code=None) - elif ( - isinstance(_request, commands.CreateContact) - and getattr(_request, "email", None) == "test@contactError.gov" - ): - # use this for when a contact is being updated - # mocks a registry error on creation - raise ContactError(code=ContactErrorCodes.CONTACT_TYPE_NONE) + elif isinstance(_request, commands.CreateContact): + return self.mockCreateContactCommands(_request, cleaned) elif isinstance(_request, commands.CreateHost): return MagicMock( res_data=[self.mockDataHostChange], @@ -913,6 +893,24 @@ class MockEppLib(TestCase): return MagicMock(res_data=[mocked_result]) + def mockCreateContactCommands(self, _request, cleaned): + if ( + getattr(_request, "id", None) == "fail" + and self.mockedSendFunction.call_count == 3 + ): + # use this for when a contact is being updated + # sets the second send() to fail + raise RegistryError(code=ErrorCode.OBJECT_EXISTS) + elif getattr(_request, "email", None) == "test@failCreate.gov": + # use this for when a contact is being updated + # mocks a registry error on creation + raise RegistryError(code=None) + elif getattr(_request, "email", None) == "test@contactError.gov": + # use this for when a contact is being updated + # mocks a contact error on creation + raise ContactError(code=ContactErrorCodes.CONTACT_TYPE_NONE) + return MagicMock(res_data=[self.mockDataInfoHosts]) + def setUp(self): """mock epp send function as this will fail locally""" self.mockSendPatch = patch("registrar.models.domain.registry.send") diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index ce901626b..0e8f895af 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1626,6 +1626,7 @@ class TestDomainSecurityEmail(TestDomainOverview): response = self.client.get(reverse("domain", kwargs={"pk": self.domain.id})) self.assertEqual(response.status_code, 403) + class TestDomainDNSSEC(TestDomainOverview): """MockEPPLib is already inherited."""