From cf92d4d7e9a758f5112b64d6cf4237cabafb21cc Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 22 Dec 2023 14:42:35 -0700 Subject: [PATCH] Test case refactor --- src/registrar/fixtures_applications.py | 2 +- src/registrar/models/domain_application.py | 8 +- src/registrar/tests/test_models.py | 107 +++++++++++++++------ src/registrar/tests/test_models_domain.py | 5 +- 4 files changed, 86 insertions(+), 36 deletions(-) diff --git a/src/registrar/fixtures_applications.py b/src/registrar/fixtures_applications.py index ae82ddd8b..92094b876 100644 --- a/src/registrar/fixtures_applications.py +++ b/src/registrar/fixtures_applications.py @@ -221,5 +221,5 @@ class DomainFixture(DomainApplicationFixture): # We don't want fixtures sending out real emails to # fake email addresses, so we just skip that and log it instead - application.approve(do_fake_send_email=True) + application.approve(send_email=False) application.save() diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index 16b24cf40..8fca5918b 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -561,7 +561,7 @@ class DomainApplication(TimeStampedModel): return not self.approved_domain.is_active() return True - def _send_status_update_email(self, new_status, email_template, email_template_subject, do_fake_send_email=False): + def _send_status_update_email(self, new_status, email_template, email_template_subject, send_email=True): """Send a status update email to the submitter. The email goes to the email address that the submitter gave as their @@ -573,7 +573,7 @@ class DomainApplication(TimeStampedModel): logger.warning(f"Cannot send {new_status} email, no submitter email address.") return None - if do_fake_send_email: + if not send_email: logger.info(f"Email was not sent. Would send {new_status} email: {self.submitter.email}") return None @@ -676,7 +676,7 @@ class DomainApplication(TimeStampedModel): ], target=ApplicationStatus.APPROVED, ) - def approve(self, do_fake_send_email=False): + def approve(self, send_email=True): """Approve an application that has been submitted. This has substantial side-effects because it creates another database @@ -705,7 +705,7 @@ class DomainApplication(TimeStampedModel): "application approved", "emails/status_change_approved.txt", "emails/status_change_approved_subject.txt", - do_fake_send_email, + send_email, ) @transition( diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 332f2b3ff..4d75ce6a3 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -19,8 +19,8 @@ from registrar.models.transition_domain import TransitionDomain # type: ignore from .common import MockSESClient, less_console_noise, completed_application from django_fsm import TransitionNotAllowed -boto3_mocking.clients.register_handler("sesv2", MockSESClient) +boto3_mocking.clients.register_handler("sesv2", MockSESClient) # The DomainApplication submit method has a side effect of sending an email # with AWS SES, so mock that out in all of these test cases @@ -52,6 +52,12 @@ class TestDomainApplication(TestCase): status=DomainApplication.ApplicationStatus.INELIGIBLE, name="ineligible.gov" ) + self.mock_client = MockSESClient() + + def tearDown(self): + super().tearDown() + self.mock_client.EMAILS_SENT.clear() + def assertNotRaises(self, exception_type): """Helper method for testing allowed transitions.""" return self.assertRaises(Exception, None, exception_type) @@ -130,8 +136,8 @@ class TestDomainApplication(TestCase): user, _ = User.objects.get_or_create(username="testy") application = DomainApplication.objects.create(creator=user) - mock_client = MockSESClient - with boto3_mocking.clients.handler_for("sesv2", mock_client): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): with self.assertRaises(ValueError): # can't submit an application with a null domain name @@ -143,8 +149,8 @@ class TestDomainApplication(TestCase): application = DomainApplication.objects.create(creator=user, requested_domain=site) # no submitter email so this emits a log warning - mock_client = MockSESClient - with boto3_mocking.clients.handler_for("sesv2", mock_client): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): application.submit() self.assertEqual(application.status, application.ApplicationStatus.SUBMITTED) @@ -161,7 +167,8 @@ class TestDomainApplication(TestCase): ) application.save() - with boto3_mocking.clients.handler_for("sesv2", MockSESClient): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): application.submit() @@ -187,8 +194,9 @@ class TestDomainApplication(TestCase): (self.action_needed_application, TransitionNotAllowed), (self.withdrawn_application, TransitionNotAllowed), ] - - with boto3_mocking.clients.handler_for("sesv2", MockSESClient): + + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): for application, exception_type in test_cases: with self.subTest(application=application, exception_type=exception_type): @@ -208,7 +216,8 @@ class TestDomainApplication(TestCase): (self.ineligible_application, TransitionNotAllowed), ] - with boto3_mocking.clients.handler_for("sesv2", MockSESClient): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): for application, exception_type in test_cases: with self.subTest(application=application, exception_type=exception_type): @@ -227,7 +236,8 @@ class TestDomainApplication(TestCase): (self.ineligible_application, TransitionNotAllowed), ] - with boto3_mocking.clients.handler_for("sesv2", MockSESClient): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): for application, exception_type in test_cases: with self.subTest(application=application, exception_type=exception_type): @@ -246,7 +256,8 @@ class TestDomainApplication(TestCase): (self.withdrawn_application, TransitionNotAllowed), ] - with boto3_mocking.clients.handler_for("sesv2", MockSESClient): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): for application, exception_type in test_cases: with self.subTest(application=application, exception_type=exception_type): @@ -264,7 +275,8 @@ class TestDomainApplication(TestCase): (self.ineligible_application, TransitionNotAllowed), ] - with boto3_mocking.clients.handler_for("sesv2", MockSESClient): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): for application, exception_type in test_cases: with self.subTest(application=application, exception_type=exception_type): @@ -284,7 +296,8 @@ class TestDomainApplication(TestCase): (self.withdrawn_application, TransitionNotAllowed), ] - with boto3_mocking.clients.handler_for("sesv2", MockSESClient): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): for application, exception_type in test_cases: with self.subTest(application=application, exception_type=exception_type): @@ -302,7 +315,8 @@ class TestDomainApplication(TestCase): (self.rejected_application, TransitionNotAllowed), ] - with boto3_mocking.clients.handler_for("sesv2", MockSESClient): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): for application, exception_type in test_cases: with self.subTest(application=application, exception_type=exception_type): @@ -310,6 +324,19 @@ class TestDomainApplication(TestCase): application.approve() except TransitionNotAllowed: self.fail("TransitionNotAllowed was raised, but it was not expected.") + + def test_approved_skips_sending_email(self): + """ + Test that calling .approve with send_email=False doesn't actually send + an email + """ + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): + with less_console_noise(): + self.submitted_application.approve(send_email=False) + + # Assert that no emails were sent + self.assertEqual(len(self.mock_client.EMAILS_SENT), 0) def test_approved_transition_not_allowed(self): """ @@ -322,7 +349,8 @@ class TestDomainApplication(TestCase): (self.ineligible_application, TransitionNotAllowed), ] - with boto3_mocking.clients.handler_for("sesv2", MockSESClient): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): for application, exception_type in test_cases: with self.subTest(application=application, exception_type=exception_type): @@ -339,7 +367,8 @@ class TestDomainApplication(TestCase): (self.action_needed_application, TransitionNotAllowed), ] - with boto3_mocking.clients.handler_for("sesv2", MockSESClient): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): for application, exception_type in test_cases: with self.subTest(application=application, exception_type=exception_type): @@ -360,7 +389,8 @@ class TestDomainApplication(TestCase): (self.ineligible_application, TransitionNotAllowed), ] - with boto3_mocking.clients.handler_for("sesv2", MockSESClient): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): for application, exception_type in test_cases: with self.subTest(application=application, exception_type=exception_type): @@ -377,7 +407,8 @@ class TestDomainApplication(TestCase): (self.approved_application, TransitionNotAllowed), ] - with boto3_mocking.clients.handler_for("sesv2", MockSESClient): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): for application, exception_type in test_cases: with self.subTest(application=application, exception_type=exception_type): @@ -398,7 +429,8 @@ class TestDomainApplication(TestCase): (self.ineligible_application, TransitionNotAllowed), ] - with boto3_mocking.clients.handler_for("sesv2", MockSESClient): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): for application, exception_type in test_cases: with self.subTest(application=application, exception_type=exception_type): @@ -416,7 +448,8 @@ class TestDomainApplication(TestCase): (self.rejected_application, TransitionNotAllowed), ] - with boto3_mocking.clients.handler_for("sesv2", MockSESClient): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): for application, exception_type in test_cases: with self.subTest(application=application, exception_type=exception_type): @@ -436,7 +469,8 @@ class TestDomainApplication(TestCase): (self.ineligible_application, TransitionNotAllowed), ] - with boto3_mocking.clients.handler_for("sesv2", MockSESClient): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): for application, exception_type in test_cases: with self.subTest(application=application, exception_type=exception_type): @@ -455,7 +489,8 @@ class TestDomainApplication(TestCase): def custom_is_active(self): return True # Override to return True - with boto3_mocking.clients.handler_for("sesv2", MockSESClient): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): # Use patch to temporarily replace is_active with the custom implementation with patch.object(Domain, "is_active", custom_is_active): @@ -475,7 +510,8 @@ class TestDomainApplication(TestCase): def custom_is_active(self): return True # Override to return True - with boto3_mocking.clients.handler_for("sesv2", MockSESClient): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): # Use patch to temporarily replace is_active with the custom implementation with patch.object(Domain, "is_active", custom_is_active): @@ -485,17 +521,24 @@ class TestDomainApplication(TestCase): class TestPermissions(TestCase): - """Test the User-Domain-Role connection.""" + def setUp(self): + super().setUp() + self.mock_client = MockSESClient() + + def tearDown(self): + super().tearDown() + self.mock_client.EMAILS_SENT.clear() + @boto3_mocking.patching def test_approval_creates_role(self): draft_domain, _ = DraftDomain.objects.get_or_create(name="igorville.gov") user, _ = User.objects.get_or_create() application = DomainApplication.objects.create(creator=user, requested_domain=draft_domain) - mock_client = MagicMock() - with boto3_mocking.clients.handler_for("sesv2", mock_client): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): # skip using the submit method application.status = DomainApplication.ApplicationStatus.SUBMITTED @@ -510,14 +553,22 @@ class TestDomainInfo(TestCase): """Test creation of Domain Information when approved.""" + def setUp(self): + super().setUp() + self.mock_client = MockSESClient() + + def tearDown(self): + super().tearDown() + self.mock_client.EMAILS_SENT.clear() + @boto3_mocking.patching def test_approval_creates_info(self): draft_domain, _ = DraftDomain.objects.get_or_create(name="igorville.gov") user, _ = User.objects.get_or_create() application = DomainApplication.objects.create(creator=user, requested_domain=draft_domain) - mock_client = MagicMock() - with boto3_mocking.clients.handler_for("sesv2", mock_client): + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): # skip using the submit method application.status = DomainApplication.ApplicationStatus.SUBMITTED diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 07d45bbdc..b76e2000c 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -29,7 +29,7 @@ from epplibwrapper import ( RegistryError, ErrorCode, ) -from .common import MockEppLib, less_console_noise +from .common import MockEppLib, MockSESClient, less_console_noise import logging import boto3_mocking # type: ignore @@ -263,8 +263,7 @@ class TestDomainCreation(MockEppLib): user, _ = User.objects.get_or_create() application = DomainApplication.objects.create(creator=user, requested_domain=draft_domain) - mock_client = MagicMock() - with boto3_mocking.clients.handler_for("sesv2", mock_client): + with boto3_mocking.clients.handler_for("sesv2", MockSESClient): with less_console_noise(): # skip using the submit method application.status = DomainApplication.ApplicationStatus.SUBMITTED