Fix remaining tests (hopefully)

This commit is contained in:
zandercymatics 2024-08-23 14:45:44 -06:00
parent 9f6d1324d9
commit 056df7151d
No known key found for this signature in database
GPG key ID: FF4636ABEC9682B7
9 changed files with 28 additions and 41 deletions

View file

@ -307,7 +307,6 @@ class DomainRequestAdminForm(forms.ModelForm):
return cleaned_data return cleaned_data
def _check_for_valid_rejection_reason(self, rejection_reason) -> bool: def _check_for_valid_rejection_reason(self, rejection_reason) -> bool:
""" """
Checks if the rejection_reason field is not none. Checks if the rejection_reason field is not none.
@ -1923,7 +1922,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin):
recipient = obj.creator recipient = obj.creator
elif not profile_flag and hasattr(obj, "submitter"): elif not profile_flag and hasattr(obj, "submitter"):
recipient = obj.submitter recipient = obj.submitter
else: else:
recipient = None recipient = None
# Displays a warning in admin when an email cannot be sent, # Displays a warning in admin when an email cannot be sent,
@ -1944,9 +1943,9 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin):
return super().save_model(request, obj, form, change) return super().save_model(request, obj, form, change)
def _check_for_valid_email(self, request, email): def _check_for_valid_email(self, request, email):
"""Certain emails are whitelisted in non-production environments, """Certain emails are whitelisted in non-production environments,
so we should display that information using this function. so we should display that information using this function.
""" """
allowed = models.AllowedEmail.is_allowed_email(email) allowed = models.AllowedEmail.is_allowed_email(email)
@ -3197,6 +3196,7 @@ class SuborganizationAdmin(ListHeaderAdmin, ImportExportModelAdmin):
extra_context = {"domain_requests": domain_requests, "domains": domains} extra_context = {"domain_requests": domain_requests, "domains": domains}
return super().change_view(request, object_id, form_url, extra_context) return super().change_view(request, object_id, form_url, extra_context)
class AllowedEmailAdmin(ListHeaderAdmin): class AllowedEmailAdmin(ListHeaderAdmin):
class Meta: class Meta:
model = models.AllowedEmail model = models.AllowedEmail
@ -3206,6 +3206,7 @@ class AllowedEmailAdmin(ListHeaderAdmin):
search_help_text = "Search by email." search_help_text = "Search by email."
ordering = ["email"] ordering = ["email"]
admin.site.unregister(LogEntry) # Unregister the default registration admin.site.unregister(LogEntry) # Unregister the default registration
admin.site.register(LogEntry, CustomLogEntryAdmin) admin.site.register(LogEntry, CustomLogEntryAdmin)

View file

@ -245,9 +245,7 @@ class UserFixture:
# Additional emails to add to the AllowedEmail whitelist. # Additional emails to add to the AllowedEmail whitelist.
# The format should be as follows: ["email@igorville.gov", "email2@igorville.gov"] # The format should be as follows: ["email@igorville.gov", "email2@igorville.gov"]
ADDITIONAL_ALLOWED_EMAILS = [ ADDITIONAL_ALLOWED_EMAILS = ["zander.adkinson@ecstech.com"]
"zander.adkinson@ecstech.com"
]
def load_users(cls, users, group_name, are_superusers=False): def load_users(cls, users, group_name, are_superusers=False):
logger.info(f"Going to load {len(users)} users in group {group_name}") logger.info(f"Going to load {len(users)} users in group {group_name}")
@ -290,7 +288,7 @@ class UserFixture:
first_name = user_data.get("first_name") first_name = user_data.get("first_name")
last_name = user_data.get("last_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 add email to whitelist for {first_name} {last_name}: No email exists.")
# Load additional emails # Load additional emails
for email in additional_emails: for email in additional_emails:
allowed_emails.append(AllowedEmail(email=email)) allowed_emails.append(AllowedEmail(email=email))

View file

@ -35,21 +35,17 @@ class AllowedEmail(TimeStampedModel):
# Check if there's a '+' in the local part # Check if there's a '+' in the local part
if "+" in local: if "+" in local:
base_local = local.split("+")[0] base_local = local.split("+")[0]
base_email_exists = cls.objects.filter( base_email_exists = cls.objects.filter(Q(email__iexact=f"{base_local}@{domain}")).exists()
Q(email__iexact=f"{base_local}@{domain}")
).exists()
# Given an example email, such as "joe.smoe+1@igorville.com" # Given an example email, such as "joe.smoe+1@igorville.com"
# The full regex statement will be: "^joe.smoe\\+\\d+@igorville.com$" # The full regex statement will be: "^joe.smoe\\+\\d+@igorville.com$"
pattern = f'^{re.escape(base_local)}\\+\\d+@{re.escape(domain)}$' pattern = f"^{re.escape(base_local)}\\+\\d+@{re.escape(domain)}$"
return base_email_exists and re.match(pattern, email) return base_email_exists and re.match(pattern, email)
else: else:
# Edge case, the +1 record exists but the base does not, # Edge case, the +1 record exists but the base does not,
# and the record we are checking is the base record. # and the record we are checking is the base record.
pattern = f'^{re.escape(local)}\\+\\d+@{re.escape(domain)}$' pattern = f"^{re.escape(local)}\\+\\d+@{re.escape(domain)}$"
plus_email_exists = cls.objects.filter( plus_email_exists = cls.objects.filter(Q(email__iregex=pattern)).exists()
Q(email__iregex=pattern)
).exists()
return plus_email_exists return plus_email_exists
def __str__(self): def __str__(self):

View file

@ -580,14 +580,8 @@ class DomainRequest(TimeStampedModel):
@classmethod @classmethod
def get_statuses_that_send_emails(cls): def get_statuses_that_send_emails(cls):
"""Returns a list of statuses that send an email to the user""" """Returns a list of statuses that send an email to the user"""
excluded_statuses = [ excluded_statuses = [cls.DomainRequestStatus.INELIGIBLE, cls.DomainRequestStatus.IN_REVIEW]
cls.DomainRequestStatus.INELIGIBLE, return [status for status in cls.DomainRequestStatus if status not in excluded_statuses]
cls.DomainRequestStatus.IN_REVIEW
]
return [
status for status in cls.DomainRequestStatus
if status not in excluded_statuses
]
def sync_organization_type(self): def sync_organization_type(self):
""" """

View file

@ -56,7 +56,8 @@ class TestDomainRequestAdmin(MockEppLib):
@classmethod @classmethod
def tearDownClass(cls): def tearDownClass(cls):
super().tearDownClass() super().tearDownClass()
AllowedEmail.objects.all.delete()
@classmethod @classmethod
def setUpClass(self): def setUpClass(self):
super().setUpClass() super().setUpClass()
@ -74,6 +75,8 @@ class TestDomainRequestAdmin(MockEppLib):
model=DomainRequest, model=DomainRequest,
) )
self.mock_client = MockSESClient() self.mock_client = MockSESClient()
allowed_emails = [AllowedEmail(email="mayor@igorville.gov"), AllowedEmail(email="help@get.gov")]
AllowedEmail.objects.bulk_create(allowed_emails)
def tearDown(self): def tearDown(self):
super().tearDown() super().tearDown()
@ -604,8 +607,8 @@ class TestDomainRequestAdmin(MockEppLib):
): ):
"""Helper method for the email test cases. """Helper method for the email test cases.
email_index is the index of the email in mock_client.""" email_index is the index of the email in mock_client."""
allowed_email, _ = AllowedEmail.objects.get_or_create(email=email_address) AllowedEmail.objects.get_or_create(email=email_address)
allowed_bcc_email, _ = AllowedEmail.objects.get_or_create(email=bcc_email_address) AllowedEmail.objects.get_or_create(email=bcc_email_address)
with less_console_noise(): with less_console_noise():
# Access the arguments passed to send_email # Access the arguments passed to send_email
call_args = self.mock_client.EMAILS_SENT call_args = self.mock_client.EMAILS_SENT
@ -632,9 +635,6 @@ class TestDomainRequestAdmin(MockEppLib):
if bcc_email_address: if bcc_email_address:
bcc_email = kwargs["Destination"]["BccAddresses"][0] bcc_email = kwargs["Destination"]["BccAddresses"][0]
self.assertEqual(bcc_email, bcc_email_address) self.assertEqual(bcc_email, bcc_email_address)
allowed_email.delete()
allowed_bcc_email.delete()
@override_settings(IS_PRODUCTION=True) @override_settings(IS_PRODUCTION=True)
@less_console_noise_decorator @less_console_noise_decorator

View file

@ -15,7 +15,7 @@ import boto3_mocking # type: ignore
class TestEmails(TestCase): class TestEmails(TestCase):
@classmethod @classmethod
def setUpClass(cls): def setUpClass(cls):
super().setUpClass() super().setUpClass()
@ -29,7 +29,7 @@ class TestEmails(TestCase):
AllowedEmail(email="recipient@example.com"), AllowedEmail(email="recipient@example.com"),
] ]
AllowedEmail.objects.bulk_create(allowed_emails) AllowedEmail.objects.bulk_create(allowed_emails)
@classmethod @classmethod
def tearDownClass(cls): def tearDownClass(cls):
super().tearDownClass() super().tearDownClass()
@ -38,7 +38,7 @@ class TestEmails(TestCase):
def setUp(self): def setUp(self):
self.mock_client_class = MagicMock() self.mock_client_class = MagicMock()
self.mock_client = self.mock_client_class.return_value self.mock_client = self.mock_client_class.return_value
def tearDown(self): def tearDown(self):
super().tearDown() super().tearDown()

View file

@ -2360,7 +2360,7 @@ class TestAllowedEmail(TestCase):
def test_plus_email_not_in_whitelist(self): def test_plus_email_not_in_whitelist(self):
"""Test for a +1 email not defined in the whitelist""" """Test for a +1 email not defined in the whitelist"""
# This email should not be allowed. # This email should not be allowed.
# Checks that we do more than just a regex check on the record. # Checks that we do more than just a regex check on the record.
plus_email_allowed = AllowedEmail.is_allowed_email(self.plus_email) plus_email_allowed = AllowedEmail.is_allowed_email(self.plus_email)
self.assertFalse(plus_email_allowed) self.assertFalse(plus_email_allowed)
@ -2426,8 +2426,8 @@ class TestAllowedEmail(TestCase):
# TODO: We need a small test for domain request admin # 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. # We also need a basic test in test_emails based off the mocked is_allowed_email value.
# This will be simpler # This will be simpler
# def test_email_in_whitelist_in_prod(self): # def test_email_in_whitelist_in_prod(self):
# """Tests that the whitelist does nothing when we are in production""" # """Tests that the whitelist does nothing when we are in production"""
# allowed_email = AllowedEmail.objects.create(email=self.email) # allowed_email = AllowedEmail.objects.create(email=self.email)
# self.assertEqual(allowed_email.is_allowed_email(), True) # self.assertEqual(allowed_email.is_allowed_email(), True)

View file

@ -355,7 +355,7 @@ class TestDomainManagers(TestDomainOverview):
AllowedEmail(email="testy2@town.com"), AllowedEmail(email="testy2@town.com"),
] ]
AllowedEmail.objects.bulk_create(allowed_emails) AllowedEmail.objects.bulk_create(allowed_emails)
@classmethod @classmethod
def tearDownClass(cls): def tearDownClass(cls):
super().tearDownClass() super().tearDownClass()

View file

@ -28,7 +28,7 @@ def send_templated_email(
to_address: str, to_address: str,
bcc_address="", bcc_address="",
context={}, context={},
attachment_file = None, attachment_file=None,
wrap_email=False, wrap_email=False,
): ):
"""Send an email built from a template to one email address. """Send an email built from a template to one email address.
@ -40,8 +40,6 @@ def send_templated_email(
Raises EmailSendingError if SES client could not be accessed Raises EmailSendingError if SES client could not be accessed
""" """
if not settings.IS_PRODUCTION: # type: ignore if not settings.IS_PRODUCTION: # type: ignore
if flag_is_active(None, "disable_email_sending"): # type: ignore 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'." message = "Could not send email. Email sending is disabled due to flag 'disable_email_sending'."
@ -49,7 +47,7 @@ def send_templated_email(
else: else:
# Raise an email sending error if these doesn't exist within our whitelist. # Raise an email sending error if these doesn't exist within our whitelist.
# If these emails don't exist, this function can handle that elsewhere. # If these emails don't exist, this function can handle that elsewhere.
AllowedEmail = apps.get_model('registrar', 'AllowedEmail') AllowedEmail = apps.get_model("registrar", "AllowedEmail")
message = "Could not send email. The email '{}' does not exist within the whitelist." message = "Could not send email. The email '{}' does not exist within the whitelist."
if to_address and not AllowedEmail.is_allowed_email(to_address): if to_address and not AllowedEmail.is_allowed_email(to_address):
raise EmailSendingError(message.format(to_address)) raise EmailSendingError(message.format(to_address))