From bc75ac454098dfbe9f9fcbe5b55e637f6287ec4f Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 28 Aug 2024 08:42:28 -0600
Subject: [PATCH] Cleanup
---
src/registrar/admin.py | 5 +++--
src/registrar/fixtures_users.py | 9 +++------
.../includes/descriptions/allowed_email_description.html | 2 +-
src/registrar/tests/test_models.py | 8 --------
src/registrar/utility/email.py | 9 +++++++--
5 files changed, 14 insertions(+), 19 deletions(-)
diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 8b40d8229..cda31ebf9 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -1969,6 +1969,8 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin):
so we should display that information using this function.
"""
+
+ # TODO 2574: remove lines 1977-1978 (refactor as needed)
profile_flag = flag_is_active(request, "profile_feature")
if profile_flag and hasattr(obj, "creator"):
recipient = obj.creator
@@ -1977,8 +1979,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin):
else:
recipient = None
- # Displays a warning in admin when an email cannot be sent,
- # Or a success message if it was.
+ # Displays a warning in admin when an email cannot be sent
if recipient and recipient.email:
email = recipient.email
allowed = models.AllowedEmail.is_allowed_email(email)
diff --git a/src/registrar/fixtures_users.py b/src/registrar/fixtures_users.py
index fb786a1e3..e7a71af5d 100644
--- a/src/registrar/fixtures_users.py
+++ b/src/registrar/fixtures_users.py
@@ -244,7 +244,6 @@ class UserFixture:
]
# Additional emails to add to the AllowedEmail whitelist.
- # The format should be as follows: ["email@igorville.gov", "email2@igorville.gov"]
ADDITIONAL_ALLOWED_EMAILS: list[str] = []
def load_users(cls, users, group_name, are_superusers=False):
@@ -277,9 +276,8 @@ class UserFixture:
if additional_emails:
logger.info(f"Going to load {len(additional_emails)} additional allowed emails")
- allowed_emails = []
-
# Load user emails
+ allowed_emails = []
for user_data in users:
user_email = user_data.get("email")
if user_email and user_email not in allowed_emails:
@@ -287,11 +285,10 @@ class UserFixture:
else:
first_name = user_data.get("first_name")
last_name = user_data.get("last_name")
- logger.warning(f"Could add email to whitelist for {first_name} {last_name}: No email exists.")
+ logger.warning(f"Could not add email to whitelist for {first_name} {last_name}.")
# Load additional emails
- for email in additional_emails:
- allowed_emails.append(AllowedEmail(email=email))
+ allowed_emails.extend(additional_emails)
if allowed_emails:
AllowedEmail.objects.bulk_create(allowed_emails)
diff --git a/src/registrar/templates/django/admin/includes/descriptions/allowed_email_description.html b/src/registrar/templates/django/admin/includes/descriptions/allowed_email_description.html
index 4bac06437..5ec5a4906 100644
--- a/src/registrar/templates/django/admin/includes/descriptions/allowed_email_description.html
+++ b/src/registrar/templates/django/admin/includes/descriptions/allowed_email_description.html
@@ -3,4 +3,4 @@
If an email is sent out and the email does not exist within this table (or is not a subset of it),
then no email will be sent.
-If this table is populated in a production environment, no change will occur as it will simply be ignored.
\ No newline at end of file
+If this table is populated in a production environment, no change will occur as it will simply be ignored.
diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py
index 8ee5dac3d..b2c4452f6 100644
--- a/src/registrar/tests/test_models.py
+++ b/src/registrar/tests/test_models.py
@@ -2423,11 +2423,3 @@ class TestAllowedEmail(TestCase):
# For good measure, also check the other plus email
regular_plus_email = AllowedEmail.is_allowed_email(self.plus_email)
self.assertFalse(regular_plus_email)
-
- # TODO: We need a small test for domain request admin
- # We also need a basic test in test_emails based off the mocked is_allowed_email value.
- # This will be simpler
- # def test_email_in_whitelist_in_prod(self):
- # """Tests that the whitelist does nothing when we are in production"""
- # allowed_email = AllowedEmail.objects.create(email=self.email)
- # self.assertEqual(allowed_email.is_allowed_email(), True)
diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py
index 533bd9e99..3d32213a0 100644
--- a/src/registrar/utility/email.py
+++ b/src/registrar/utility/email.py
@@ -41,6 +41,9 @@ def send_templated_email(
"""
if not settings.IS_PRODUCTION: # type: ignore
+ # Split into a function: C901 'send_templated_email' is too complex.
+ # Raises an error if we cannot send an email (due to restrictions).
+ # Does nothing otherwise.
_can_send_email(to_address, bcc_address)
template = get_template(template_name)
@@ -63,7 +66,7 @@ def send_templated_email(
)
logger.info(f"An email was sent! Template name: {template_name} to {to_address}")
except Exception as exc:
- logger.debug("An email was unable to send! Could not access the SES client.")
+ logger.debug("E-mail unable to send! Could not access the SES client.")
raise EmailSendingError("Could not access the SES client.") from exc
destination = {"ToAddresses": [to_address]}
@@ -103,7 +106,8 @@ def send_templated_email(
def _can_send_email(to_address, bcc_address):
- """Raises an error if we cannot send an error"""
+ """Raises an EmailSendingError if we cannot send an email. Does nothing otherwise."""
+
if flag_is_active(None, "disable_email_sending"): # type: ignore
message = "Could not send email. Email sending is disabled due to flag 'disable_email_sending'."
raise EmailSendingError(message)
@@ -114,6 +118,7 @@ def _can_send_email(to_address, bcc_address):
message = "Could not send email. The email '{}' does not exist within the whitelist."
if to_address and not AllowedEmail.is_allowed_email(to_address):
raise EmailSendingError(message.format(to_address))
+
if bcc_address and not AllowedEmail.is_allowed_email(bcc_address):
raise EmailSendingError(message.format(bcc_address))