diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 1b20caf2a..c5d4fa95d 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -478,7 +478,7 @@ class JsonServerFormatter(ServerFormatter): if not hasattr(record, "server_time"): record.server_time = self.formatTime(record, self.datefmt) - + log_entry = {"server_time": record.server_time, "level": record.levelname, "message": formatted_record} return json.dumps(log_entry) diff --git a/src/registrar/tests/test_emails.py b/src/registrar/tests/test_emails.py index 21ba06316..abbbb274f 100644 --- a/src/registrar/tests/test_emails.py +++ b/src/registrar/tests/test_emails.py @@ -60,20 +60,20 @@ class TestEmails(TestCase): # Assert that an email wasn't sent self.assertFalse(self.mock_client.send_email.called) - + @boto3_mocking.patching def test_email_with_cc(self): """Test sending email with cc works""" with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): send_templated_email( - "test content", - "test subject", - "doesnotexist@igorville.com", - context={"domain_request": self}, - bcc_address=None, - cc_addresses=["test_email1@example.com", "test_email2@example.com"] - ) - + "test content", + "test subject", + "doesnotexist@igorville.com", + context={"domain_request": self}, + bcc_address=None, + cc_addresses=["test_email1@example.com", "test_email2@example.com"], + ) + # check that an email was sent self.assertTrue(self.mock_client.send_email.called) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index d258dc472..6ca96f8c0 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -2,17 +2,14 @@ from unittest import skip from unittest.mock import MagicMock, ANY, patch from django.conf import settings -from django.test import override_settings from django.urls import reverse from django.contrib.auth import get_user_model from waffle.testutils import override_flag from api.tests.common import less_console_noise_decorator from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices -from registrar.utility.email import send_templated_email from .common import MockEppLib, MockSESClient, create_user # type: ignore from django_webtest import WebTest # type: ignore import boto3_mocking # type: ignore -from django.middleware.csrf import get_token from registrar.utility.errors import ( NameserverError, @@ -106,7 +103,6 @@ class TestWithDomainPermissions(TestWithUser): DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_deleted) DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_dns_needed) - self.role, _ = UserDomainRole.objects.get_or_create( user=self.user, domain=self.domain, role=UserDomainRole.Roles.MANAGER ) @@ -1995,7 +1991,7 @@ class TestDomainChangeNotifications(TestDomainOverview): AllowedEmail(email="doesnotexist@igorville.com"), ] AllowedEmail.objects.bulk_create(allowed_emails) - + def setUp(self): super().setUp() self.mock_client_class = MagicMock() @@ -2010,14 +2006,14 @@ class TestDomainChangeNotifications(TestDomainOverview): @less_console_noise_decorator def test_notification_on_org_name_change(self): """Test that an email is sent when the organization name is changed.""" - + self.domain_information.organization_name = "Town of Igorville" self.domain_information.address_line1 = "123 Main St" self.domain_information.city = "Igorville" self.domain_information.state_territory = "IL" self.domain_information.zipcode = "62052" self.domain_information.save() - + org_name_page = self.app.get(reverse("domain-org-name-address", kwargs={"pk": self.domain.id})) session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] @@ -2028,8 +2024,8 @@ class TestDomainChangeNotifications(TestDomainOverview): org_name_page.form.submit() # Check that an email was sent - self.assertTrue(self.mock_client.send_email.called) - + self.assertTrue(self.mock_client.send_email.called) + # Check email content # check the call sequence for the email _, kwargs = self.mock_client.send_email.call_args @@ -2048,7 +2044,7 @@ class TestDomainChangeNotifications(TestDomainOverview): @less_console_noise_decorator def test_no_notification_on_org_name_change_with_portfolio(self): """Test that an email is not sent on org name change when the domain is in a portfolio""" - + portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test org", creator=self.user) self.domain_information.organization_name = "Town of Igorville" @@ -2058,7 +2054,7 @@ class TestDomainChangeNotifications(TestDomainOverview): self.domain_information.zipcode = "62052" self.domain_information.portfolio = portfolio self.domain_information.save() - + org_name_page = self.app.get(reverse("domain-org-name-address", kwargs={"pk": self.domain.id})) session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] @@ -2069,13 +2065,13 @@ class TestDomainChangeNotifications(TestDomainOverview): org_name_page.form.submit() # Check that an email was not sent - self.assertFalse(self.mock_client.send_email.called) + self.assertFalse(self.mock_client.send_email.called) @boto3_mocking.patching @less_console_noise_decorator def test_notification_on_security_email_change(self): """Test that an email is sent when the security email is changed.""" - + security_email_page = self.app.get(reverse("domain-security-email", kwargs={"pk": self.domain.id})) session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] @@ -2085,8 +2081,8 @@ class TestDomainChangeNotifications(TestDomainOverview): with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): security_email_page.form.submit() - self.assertTrue(self.mock_client.send_email.called) - + self.assertTrue(self.mock_client.send_email.called) + _, kwargs = self.mock_client.send_email.call_args body = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"] @@ -2098,7 +2094,7 @@ class TestDomainChangeNotifications(TestDomainOverview): @less_console_noise_decorator def test_notification_on_dnssec_enable(self): """Test that an email is sent when DNSSEC is enabled.""" - + page = self.client.get(reverse("domain-dns-dnssec", kwargs={"pk": self.domain_multdsdata.id})) self.assertContains(page, "Disable DNSSEC") @@ -2118,8 +2114,8 @@ class TestDomainChangeNotifications(TestDomainOverview): self.assertContains(updated_page, "Enable DNSSEC") - self.assertTrue(self.mock_client.send_email.called) - + self.assertTrue(self.mock_client.send_email.called) + _, kwargs = self.mock_client.send_email.call_args body = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"] @@ -2131,7 +2127,7 @@ class TestDomainChangeNotifications(TestDomainOverview): @less_console_noise_decorator def test_notification_on_ds_data_change(self): """Test that an email is sent when DS data is changed.""" - + ds_data_page = self.app.get(reverse("domain-dns-dnssec-dsdata", kwargs={"pk": self.domain.id})) session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] @@ -2140,14 +2136,14 @@ class TestDomainChangeNotifications(TestDomainOverview): ds_data_page.forms[0]["form-0-algorithm"] = "13" ds_data_page.forms[0]["form-0-digest_type"] = "2" ds_data_page.forms[0]["form-0-digest"] = "1234567890ABCDEF1234567890ABCDEF1234567890ABCDEF1234567890ABCDEF" - + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): ds_data_page.forms[0].submit() # check that the email was sent - self.assertTrue(self.mock_client.send_email.called) - + self.assertTrue(self.mock_client.send_email.called) + # check some stuff about the email _, kwargs = self.mock_client.send_email.call_args body = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"] @@ -2160,12 +2156,12 @@ class TestDomainChangeNotifications(TestDomainOverview): @less_console_noise_decorator def test_notification_on_senior_official_change(self): """Test that an email is sent when the senior official information is changed.""" - + self.domain_information.senior_official = Contact.objects.create( first_name="Old", last_name="Official", title="Manager", email="old_official@example.com" ) self.domain_information.save() - + senior_official_page = self.app.get(reverse("domain-senior-official", kwargs={"pk": self.domain.id})) session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] @@ -2178,8 +2174,8 @@ class TestDomainChangeNotifications(TestDomainOverview): with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): senior_official_page.form.submit() - self.assertTrue(self.mock_client.send_email.called) - + self.assertTrue(self.mock_client.send_email.called) + _, kwargs = self.mock_client.send_email.call_args body = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"] @@ -2192,17 +2188,17 @@ class TestDomainChangeNotifications(TestDomainOverview): def test_no_notification_on_senior_official_when_portfolio(self): """Test that an email is not sent when the senior official information is changed and the domain is in a portfolio.""" - + self.domain_information.senior_official = Contact.objects.create( first_name="Old", last_name="Official", title="Manager", email="old_official@example.com" - ) - portfolio, _ =Portfolio.objects.get_or_create( + ) + portfolio, _ = Portfolio.objects.get_or_create( organization_name="portfolio", creator=self.user, ) self.domain_information.portfolio = portfolio self.domain_information.save() - + senior_official_page = self.app.get(reverse("domain-senior-official", kwargs={"pk": self.domain.id})) session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] @@ -2216,12 +2212,12 @@ class TestDomainChangeNotifications(TestDomainOverview): senior_official_page.form.submit() self.assertFalse(self.mock_client.send_email.called) - + @boto3_mocking.patching @less_console_noise_decorator def test_no_notification_when_dns_needed(self): """Test that an email is not sent when nameservers are changed while the state is DNS_NEEDED.""" - + nameservers_page = self.app.get(reverse("domain-dns-nameservers", kwargs={"pk": self.domain_dns_needed.id})) session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 35d8a2029..655c432ac 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -22,15 +22,15 @@ class EmailSendingError(RuntimeError): pass -def send_templated_email( +def send_templated_email( # noqa template_name: str, subject_template_name: str, - to_address: str="", - bcc_address: str="", + to_address: str = "", + bcc_address: str = "", context={}, attachment_file=None, wrap_email=False, - cc_addresses: list[str]=[], + cc_addresses: list[str] = [], ): """Send an email built from a template. @@ -58,7 +58,6 @@ def send_templated_email( if len(sendable_cc_addresses) < len(cc_addresses): logger.warning("Some CC'ed addresses were removed: %s.", blocked_cc_addresses) - template = get_template(template_name) email_body = template.render(context=context) @@ -127,6 +126,7 @@ def send_templated_email( except Exception as exc: raise EmailSendingError("Could not send SES email.") from exc + def _can_send_email(to_address, bcc_address): """Raises an EmailSendingError if we cannot send an email. Does nothing otherwise.""" @@ -144,15 +144,16 @@ def _can_send_email(to_address, bcc_address): if bcc_address and not AllowedEmail.is_allowed_email(bcc_address): raise EmailSendingError(message.format(bcc_address)) + def get_sendable_addresses(addresses: list[str]) -> tuple[list[str], list[str]]: """Checks whether a list of addresses can be sent to. - + Returns: a lists of all provided addresses that are ok to send to and a list of addresses that were blocked. Paramaters: - + addresses: a list of strings representing all addresses to be checked. - + raises: EmailSendingError if email sending is disabled """ diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 22ece989b..a3f9d153c 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -149,18 +149,18 @@ class DomainFormBaseView(DomainBaseView, FormMixin): logger.error("Could get domain_info. No domain info exists, or duplicates exist.") return current_domain_info - + def send_update_notification(self, form, force_send=False): - """Send a notification to all domain managers that an update has occured + """Send a notification to all domain managers that an update has occured for a single domain. Uses update_to_approved_domain.txt template. - + If there are no changes to the form, emails will NOT be sent unless force_send is set to True. """ # send notification email for changes to any of these forms form_label_dict = { - DomainSecurityEmailForm: "Security Email", + DomainSecurityEmailForm: "Security Email", DomainDnssecForm: "DNSSec", DomainDsdataFormset: "DS Data", DomainOrgNameAddressForm: "Org Name/Address", @@ -176,29 +176,35 @@ class DomainFormBaseView(DomainBaseView, FormMixin): if form.__class__ in form_label_dict: # these types of forms can cause notifications - should_notify=True + should_notify = True if form.__class__ in check_for_portfolio: # some forms shouldn't cause notifications if they are in a portfolio info = self.get_domain_info_from_domain() if not info or info.portfolio: - logger.info(f"Not notifying because of portfolio") - should_notify = False + should_notify = False else: # don't notify for any other types of forms - should_notify=False + should_notify = False logger.info(f"Not notifying for {form.__class__}") if (should_notify and form.has_changed()) or force_send: - context={ - "domain": self.object.name, - "user": self.request.user, - "date": date.today(), - "changes": form_label_dict[form.__class__] - } - self.email_domain_managers(self.object, "emails/update_to_approved_domain.txt", "emails/update_to_approved_domain_subject.txt", context) + context = { + "domain": self.object.name, + "user": self.request.user, + "date": date.today(), + "changes": form_label_dict[form.__class__], + } + self.email_domain_managers( + self.object, + "emails/update_to_approved_domain.txt", + "emails/update_to_approved_domain_subject.txt", + context, + ) else: - logger.info(f"Not notifying for {form.__class__}, form changes: {form.has_changed()}, force_send: {force_send}") + logger.info( + f"Not notifying for {form.__class__}, form changes: {form.has_changed()}, force_send: {force_send}" + ) - def email_domain_managers(self, domain_name, template: str, subject_template: str, context: any = {}): + def email_domain_managers(self, domain_name, template: str, subject_template: str, context={}): """Send a single email built from a template to all managers for a given domain. template_name and subject_template_name are relative to the same template @@ -214,20 +220,16 @@ class DomainFormBaseView(DomainBaseView, FormMixin): domain = Domain.objects.get(name=domain_name) except Domain.DoesNotExist: logger.warning( - "Could not send notification email for domain %s, unable to find matching domain object", - domain_name + "Could not send notification email for domain %s, unable to find matching domain object", domain_name ) - manager_pks = UserDomainRole.objects.filter(domain=domain.pk, role=UserDomainRole.Roles.MANAGER).values_list("user", flat=True) + manager_pks = UserDomainRole.objects.filter(domain=domain.pk, role=UserDomainRole.Roles.MANAGER).values_list( + "user", flat=True + ) emails = list(User.objects.filter(pk__in=manager_pks).values_list("email", flat=True)) logger.debug("attempting to send templated email to domain managers") try: - send_templated_email( - template, - subject_template, - context=context, - cc_addresses=emails - ) - except EmailSendingError as exc: + send_templated_email(template, subject_template, context=context, cc_addresses=emails) + except EmailSendingError: logger.warning( "Could not sent notification email to %s for domain %s", emails, @@ -492,8 +494,6 @@ class DomainNameserversView(DomainFormBaseView): This post method harmonizes using DomainBaseView and FormMixin """ - logger.info(f"POST request to DomainNameserversView") - self._get_domain(request) formset = self.get_form() @@ -502,14 +502,13 @@ class DomainNameserversView(DomainFormBaseView): return HttpResponseRedirect(url) if formset.is_valid(): - logger.info(f"Formset is valid") return self.form_valid(formset) else: return self.form_invalid(formset) def form_valid(self, formset): """The formset is valid, perform something with it.""" - + self.request.session["nameservers_form_domain"] = self.object # Set the nameservers from the formset @@ -800,7 +799,7 @@ class DomainSecurityEmailView(DomainFormBaseView): else: self.send_update_notification(form) messages.success(self.request, "The security email for this domain has been updated.") - + # superclass has the redirect return super().form_valid(form)