From c0f5dca8c1d7b514209556f2f85ed06acb0583d5 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 15 Jan 2025 14:50:59 -0500 Subject: [PATCH 1/9] add email to domain managers on domain invitation --- .../emails/domain_manager_notification.txt | 43 +++++++++++++++++++ .../domain_manager_notification_subject.txt | 1 + src/registrar/utility/email_invitations.py | 37 ++++++++++++++++ 3 files changed, 81 insertions(+) create mode 100644 src/registrar/templates/emails/domain_manager_notification.txt create mode 100644 src/registrar/templates/emails/domain_manager_notification_subject.txt diff --git a/src/registrar/templates/emails/domain_manager_notification.txt b/src/registrar/templates/emails/domain_manager_notification.txt new file mode 100644 index 000000000..aa8c6bf34 --- /dev/null +++ b/src/registrar/templates/emails/domain_manager_notification.txt @@ -0,0 +1,43 @@ +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} +Hi,{% if domain_manager and domain_manager.first_name %} {{ domain_manager.first_name }}.{% endif %} + +A domain manager was invited to {{ domain.name }}. +DOMAIN: {{ domain.name }} +INVITED BY: {{ requestor_email }} +INVITED ON: {{date}} +MANAGER INVITED: {{ invited_email_address }} + + +---------------------------------------------------------------- + + +NEXT STEPS + +The person who received the invitation will become a domain manager once they log in to the +.gov registrar. They'll need to access the registrar using a Login.gov account that's +associated with the invited email address. + +If you need to cancel this invitation or remove the domain manager (because they've already +logged in), you can do that by going to this domain in the .gov registrar . + + +WHY DID YOU RECEIVE THIS EMAIL? + +You’re listed as a domain manager for {{ domain.name }}, so you’ll receive a notification whenever +someone is invited to manage that domain. + +If you have questions or concerns, reach out to the person who sent the invitation or reply to this email. + + +THANK YOU +.Gov helps the public identify official, trusted information. Thank you for using a .gov domain. + +---------------------------------------------------------------- + +The .gov team +Contact us: +Learn about .gov + +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency +(CISA) +{% endautoescape %} diff --git a/src/registrar/templates/emails/domain_manager_notification_subject.txt b/src/registrar/templates/emails/domain_manager_notification_subject.txt new file mode 100644 index 000000000..0e9918de0 --- /dev/null +++ b/src/registrar/templates/emails/domain_manager_notification_subject.txt @@ -0,0 +1 @@ +A domain manager was invited to {{ domain.name }} \ No newline at end of file diff --git a/src/registrar/utility/email_invitations.py b/src/registrar/utility/email_invitations.py index 48c796340..fda901fba 100644 --- a/src/registrar/utility/email_invitations.py +++ b/src/registrar/utility/email_invitations.py @@ -1,6 +1,8 @@ +from datetime import date from django.conf import settings from registrar.models import DomainInvitation from registrar.models.domain import Domain +from registrar.models.user_domain_role import UserDomainRole from registrar.utility.errors import ( AlreadyDomainInvitedError, AlreadyDomainManagerError, @@ -41,6 +43,39 @@ def send_domain_invitation_email( send_invitation_email(email, requestor_email, domains, requested_user) + # send emails to domain managers + for domain in domains: + send_emails_to_domain_managers( + email=email, + requestor_email=requestor_email, + domain=domain, + requested_user=requested_user, + ) + + +def send_emails_to_domain_managers(email: str, requestor_email, domain: Domain, requested_user=None): + # Get each domain manager from list + user_domain_roles = UserDomainRole.objects.filter(domain=domain) + for user_domain_role in user_domain_roles: + # Send email to each domain manager + user = user_domain_role.user + try: + send_templated_email( + "emails/domain_manager_notification.txt", + "emails/domain_manager_notification_subject.txt", + to_address=user.email, + context={ + "domain": domain, + "requestor_email": requestor_email, + "invited_email_address": email, + "domain_manager": user, + "date": date.today(), + }, + ) + except EmailSendingError as err: + domain_names = ", ".join([domain.name for domain in domains]) + raise EmailSendingError(f"Could not send email invitation to {email} for domains: {domain_names}") from err + def normalize_domains(domains): """Ensures domains is always a list.""" @@ -69,6 +104,8 @@ def validate_invitation(email, domains, requestor, is_member_of_different_org): for domain in domains: validate_existing_invitation(email, domain) + # NOTE: should we also be validating against existing user_domain_roles + def check_outside_org_membership(email, requestor, is_member_of_different_org): """Raise an error if the email belongs to a different organization.""" From ca136c650c710a14a17b9f3fd8501fee48a36b23 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 15 Jan 2025 14:56:04 -0500 Subject: [PATCH 2/9] updated error message when EmailSendingError --- src/registrar/utility/email_invitations.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/registrar/utility/email_invitations.py b/src/registrar/utility/email_invitations.py index fda901fba..ba660499e 100644 --- a/src/registrar/utility/email_invitations.py +++ b/src/registrar/utility/email_invitations.py @@ -73,8 +73,7 @@ def send_emails_to_domain_managers(email: str, requestor_email, domain: Domain, }, ) except EmailSendingError as err: - domain_names = ", ".join([domain.name for domain in domains]) - raise EmailSendingError(f"Could not send email invitation to {email} for domains: {domain_names}") from err + raise EmailSendingError(f"Could not send email manager notification to {user.email} for domains: {domain.name}") from err def normalize_domains(domains): From 287638786dbdc4a47c405cfb3e10dc0c7a8115cb Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 15 Jan 2025 15:03:24 -0500 Subject: [PATCH 3/9] informational alerts on change forms --- src/registrar/admin.py | 4 +++- .../admin/domain_invitation_change_form.html | 14 ++++++++++++++ .../django/admin/user_domain_role_change_form.html | 14 ++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 src/registrar/templates/django/admin/domain_invitation_change_form.html create mode 100644 src/registrar/templates/django/admin/user_domain_role_change_form.html diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 1b6e2de6a..e89147b11 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1367,6 +1367,8 @@ class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): autocomplete_fields = ["user", "domain"] + change_form_template = "django/admin/user_domain_role_change_form.html" + # Fixes a bug where non-superusers are redirected to the main page def delete_view(self, request, object_id, extra_context=None): """Custom delete_view implementation that specifies redirect behaviour""" @@ -1500,7 +1502,7 @@ class DomainInvitationAdmin(BaseInvitationAdmin): autocomplete_fields = ["domain"] - change_form_template = "django/admin/email_clipboard_change_form.html" + change_form_template = "django/admin/domain_invitation_change_form.html" # Select domain invitations to change -> Domain invitations def changelist_view(self, request, extra_context=None): diff --git a/src/registrar/templates/django/admin/domain_invitation_change_form.html b/src/registrar/templates/django/admin/domain_invitation_change_form.html new file mode 100644 index 000000000..6ce6ed0d1 --- /dev/null +++ b/src/registrar/templates/django/admin/domain_invitation_change_form.html @@ -0,0 +1,14 @@ +{% extends 'django/admin/email_clipboard_change_form.html' %} +{% load custom_filters %} +{% load i18n static %} + +{% block content_subtitle %} +
+
+

+ If you add someone to a domain here, it will trigger emails to the invitee and all managers of the domain when you click "save." If you don't want to trigger those emails, use the User domain roles permissions table instead. +

+
+
+ {{ block.super }} +{% endblock %} diff --git a/src/registrar/templates/django/admin/user_domain_role_change_form.html b/src/registrar/templates/django/admin/user_domain_role_change_form.html new file mode 100644 index 000000000..200734ec1 --- /dev/null +++ b/src/registrar/templates/django/admin/user_domain_role_change_form.html @@ -0,0 +1,14 @@ +{% extends 'django/admin/email_clipboard_change_form.html' %} +{% load custom_filters %} +{% load i18n static %} + +{% block content_subtitle %} +
+
+

+ If you add someone to a domain here, it won't trigger any emails. To trigger emails, use the User Domain Role invitations table instead. +

+
+
+ {{ block.super }} +{% endblock %} From cb8494017acbc69c843f19edb727a0392338b3a3 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 15 Jan 2025 15:28:15 -0500 Subject: [PATCH 4/9] template tests --- .../admin/user_domain_role_change_form.html | 2 +- src/registrar/tests/test_admin.py | 54 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/registrar/templates/django/admin/user_domain_role_change_form.html b/src/registrar/templates/django/admin/user_domain_role_change_form.html index 200734ec1..d8c298bc1 100644 --- a/src/registrar/templates/django/admin/user_domain_role_change_form.html +++ b/src/registrar/templates/django/admin/user_domain_role_change_form.html @@ -6,7 +6,7 @@

- If you add someone to a domain here, it won't trigger any emails. To trigger emails, use the User Domain Role invitations table instead. + If you add someone to a domain here, it will not trigger any emails. To trigger emails, use the User Domain Role invitations table instead.

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 2a7a52a13..673057e20 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -165,6 +165,33 @@ class TestDomainInvitationAdmin(TestCase): response, "Domain invitations contain all individuals who have been invited to manage a .gov domain." ) self.assertContains(response, "Show more") + + @less_console_noise_decorator + def test_has_change_form_description(self): + """Tests if this model has a model description on the change form view""" + self.client.force_login(self.superuser) + + domain, _ = Domain.objects.get_or_create( + name="systemofadown.com" + ) + + domain_invitation, _ = DomainInvitation.objects.get_or_create( + email="toxicity@systemofadown.com", domain=domain + ) + + response = self.client.get( + "/admin/registrar/domaininvitation/{}/change/".format(domain_invitation.pk), + follow=True, + ) + + # Make sure that the page is loaded correctly + self.assertEqual(response.status_code, 200) + + # Test for a description snippet + self.assertContains( + response, + "If you add someone to a domain here, it will trigger emails to the invitee and all managers of the domain when you click", + ) @less_console_noise_decorator def test_get_filters(self): @@ -1956,6 +1983,33 @@ class TestUserDomainRoleAdmin(TestCase): response, "This table represents the managers who are assigned to each domain in the registrar" ) self.assertContains(response, "Show more") + + @less_console_noise_decorator + def test_has_change_form_description(self): + """Tests if this model has a model description on the change form view""" + self.client.force_login(self.superuser) + + domain, _ = Domain.objects.get_or_create( + name="systemofadown.com" + ) + + user_domain_role, _ = UserDomainRole.objects.get_or_create( + user=self.superuser, domain=domain, role=[UserDomainRole.Roles.MANAGER] + ) + + response = self.client.get( + "/admin/registrar/userdomainrole/{}/change/".format(user_domain_role.pk), + follow=True, + ) + + # Make sure that the page is loaded correctly + self.assertEqual(response.status_code, 200) + + # Test for a description snippet + self.assertContains( + response, + "If you add someone to a domain here, it will not trigger any emails.", + ) def test_domain_sortable(self): """Tests if the UserDomainrole sorts by domain correctly""" From 4a0dc40cee4e26d4f948afb4b76105a43ad35cc5 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 15 Jan 2025 16:00:03 -0500 Subject: [PATCH 5/9] fix mock_send_domain_email unit tests --- src/registrar/tests/test_views_domain.py | 43 +++++++++++++----------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index f13490312..a9de8d6e7 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -750,11 +750,12 @@ class TestDomainManagers(TestDomainOverview): response = self.client.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) self.assertContains(response, "Add a domain manager") - @boto3_mocking.patching @less_console_noise_decorator - def test_domain_user_add_form(self): + @patch("registrar.views.domain.send_domain_invitation_email") + def test_domain_user_add_form(self, mock_send_domain_email): """Adding an existing user works.""" get_user_model().objects.get_or_create(email="mayor@igorville.gov") + user = User.objects.filter(email="mayor@igorville.gov").first() add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] @@ -762,10 +763,11 @@ class TestDomainManagers(TestDomainOverview): self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - mock_client = MockSESClient() - with boto3_mocking.clients.handler_for("sesv2", mock_client): - with less_console_noise(): - success_result = add_page.form.submit() + success_result = add_page.form.submit() + + mock_send_domain_email.assert_called_once_with( + email="mayor@igorville.gov", requestor=self.user, domains=self.domain, is_member_of_different_org=None, requested_user=user + ) self.assertEqual(success_result.status_code, 302) self.assertEqual( @@ -974,13 +976,13 @@ class TestDomainManagers(TestDomainOverview): success_page = success_result.follow() self.assertContains(success_page, "Failed to send email.") - @boto3_mocking.patching @less_console_noise_decorator - def test_domain_invitation_created(self): + @patch("registrar.views.domain.send_domain_invitation_email") + def test_domain_invitation_created(self, mock_send_domain_email): """Add user on a nonexistent email creates an invitation. Adding a non-existent user sends an email as a side-effect, so mock - out the boto3 SES email sending here. + out send_domain_invitation_email here. """ # make sure there is no user with this email email_address = "mayor@igorville.gov" @@ -993,10 +995,11 @@ class TestDomainManagers(TestDomainOverview): add_page.form["email"] = email_address self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - mock_client = MockSESClient() - with boto3_mocking.clients.handler_for("sesv2", mock_client): - with less_console_noise(): - success_result = add_page.form.submit() + success_result = add_page.form.submit() + + mock_send_domain_email.assert_called_once_with( + email="mayor@igorville.gov", requestor=self.user, domains=self.domain, is_member_of_different_org=None + ) self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) success_page = success_result.follow() @@ -1005,13 +1008,13 @@ class TestDomainManagers(TestDomainOverview): self.assertContains(success_page, "Cancel") # link to cancel invitation self.assertTrue(DomainInvitation.objects.filter(email=email_address).exists()) - @boto3_mocking.patching @less_console_noise_decorator - def test_domain_invitation_created_for_caps_email(self): + @patch("registrar.views.domain.send_domain_invitation_email") + def test_domain_invitation_created_for_caps_email(self, mock_send_domain_email): """Add user on a nonexistent email with CAPS creates an invitation to lowercase email. Adding a non-existent user sends an email as a side-effect, so mock - out the boto3 SES email sending here. + out send_domain_invitation_email here. """ # make sure there is no user with this email email_address = "mayor@igorville.gov" @@ -1025,9 +1028,11 @@ class TestDomainManagers(TestDomainOverview): add_page.form["email"] = caps_email_address self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - mock_client = MockSESClient() - with boto3_mocking.clients.handler_for("sesv2", mock_client): - success_result = add_page.form.submit() + success_result = add_page.form.submit() + + mock_send_domain_email.assert_called_once_with( + email="mayor@igorville.gov", requestor=self.user, domains=self.domain, is_member_of_different_org=None + ) self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) success_page = success_result.follow() From c10a7738ca236a0c9572cde5a1cda8c6f8679359 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 15 Jan 2025 16:27:08 -0500 Subject: [PATCH 6/9] tests and lint --- src/registrar/tests/test_email_invitations.py | 303 ++++++++++++++++++ src/registrar/utility/email_invitations.py | 8 +- 2 files changed, 307 insertions(+), 4 deletions(-) create mode 100644 src/registrar/tests/test_email_invitations.py diff --git a/src/registrar/tests/test_email_invitations.py b/src/registrar/tests/test_email_invitations.py new file mode 100644 index 000000000..6a0423f4d --- /dev/null +++ b/src/registrar/tests/test_email_invitations.py @@ -0,0 +1,303 @@ +import unittest +from unittest.mock import patch, MagicMock +from datetime import date +from registrar.utility.email import EmailSendingError +from registrar.utility.email_invitations import send_domain_invitation_email + + +class DomainInvitationEmail(unittest.TestCase): + + @patch("registrar.utility.email_invitations.send_templated_email") + @patch("registrar.utility.email_invitations.UserDomainRole.objects.filter") + @patch("registrar.utility.email_invitations.validate_invitation") + @patch("registrar.utility.email_invitations.get_requestor_email") + @patch("registrar.utility.email_invitations.send_invitation_email") + @patch("registrar.utility.email_invitations.normalize_domains") + def test_send_domain_invitation_email( + self, + mock_normalize_domains, + mock_send_invitation_email, + mock_get_requestor_email, + mock_validate_invitation, + mock_user_domain_role_filter, + mock_send_templated_email, + ): + """Test sending domain invitation email for one domain. + Should also send emails to manager of that domain. + """ + # Setup + mock_domain = MagicMock(name="domain1") + mock_domain.name = "example.com" + mock_normalize_domains.return_value = [mock_domain] + + mock_requestor = MagicMock() + mock_requestor_email = "requestor@example.com" + mock_get_requestor_email.return_value = mock_requestor_email + + mock_user1 = MagicMock() + mock_user1.email = "manager1@example.com" + + mock_user_domain_role_filter.return_value = [MagicMock(user=mock_user1)] + + email = "invitee@example.com" + is_member_of_different_org = False + + # Call the function + send_domain_invitation_email( + email=email, + requestor=mock_requestor, + domains=mock_domain, + is_member_of_different_org=is_member_of_different_org, + ) + + # Assertions + mock_normalize_domains.assert_called_once_with(mock_domain) + mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain]) + mock_validate_invitation.assert_called_once_with( + email, [mock_domain], mock_requestor, is_member_of_different_org + ) + mock_send_invitation_email.assert_called_once_with(email, mock_requestor_email, [mock_domain], None) + mock_user_domain_role_filter.assert_called_once_with(domain=mock_domain) + mock_send_templated_email.assert_called_once_with( + "emails/domain_manager_notification.txt", + "emails/domain_manager_notification_subject.txt", + to_address=mock_user1.email, + context={ + "domain": mock_domain, + "requestor_email": mock_requestor_email, + "invited_email_address": email, + "domain_manager": mock_user1, + "date": date.today(), + }, + ) + + @patch("registrar.utility.email_invitations.send_templated_email") + @patch("registrar.utility.email_invitations.UserDomainRole.objects.filter") + @patch("registrar.utility.email_invitations.validate_invitation") + @patch("registrar.utility.email_invitations.get_requestor_email") + @patch("registrar.utility.email_invitations.send_invitation_email") + @patch("registrar.utility.email_invitations.normalize_domains") + def test_send_domain_invitation_email_multiple_domains( + self, + mock_normalize_domains, + mock_send_invitation_email, + mock_get_requestor_email, + mock_validate_invitation, + mock_user_domain_role_filter, + mock_send_templated_email, + ): + """Test sending domain invitation email for multiple domains. + Should also send emails to managers of each domain. + """ + # Setup + # Create multiple mock domains + mock_domain1 = MagicMock(name="domain1") + mock_domain1.name = "example.com" + mock_domain2 = MagicMock(name="domain2") + mock_domain2.name = "example.org" + + mock_normalize_domains.return_value = [mock_domain1, mock_domain2] + + mock_requestor = MagicMock() + mock_requestor_email = "requestor@example.com" + mock_get_requestor_email.return_value = mock_requestor_email + + mock_user1 = MagicMock() + mock_user1.email = "manager1@example.com" + mock_user2 = MagicMock() + mock_user2.email = "manager2@example.com" + + # Configure domain roles for each domain + def filter_side_effect(domain): + if domain == mock_domain1: + return [MagicMock(user=mock_user1)] + elif domain == mock_domain2: + return [MagicMock(user=mock_user2)] + return [] + + mock_user_domain_role_filter.side_effect = filter_side_effect + + email = "invitee@example.com" + is_member_of_different_org = False + + # Call the function + send_domain_invitation_email( + email=email, + requestor=mock_requestor, + domains=[mock_domain1, mock_domain2], + is_member_of_different_org=is_member_of_different_org, + ) + + # Assertions + mock_normalize_domains.assert_called_once_with([mock_domain1, mock_domain2]) + mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain1, mock_domain2]) + mock_validate_invitation.assert_called_once_with( + email, [mock_domain1, mock_domain2], mock_requestor, is_member_of_different_org + ) + mock_send_invitation_email.assert_called_once_with( + email, mock_requestor_email, [mock_domain1, mock_domain2], None + ) + + # Check that domain manager emails were sent for both domains + mock_user_domain_role_filter.assert_any_call(domain=mock_domain1) + mock_user_domain_role_filter.assert_any_call(domain=mock_domain2) + + mock_send_templated_email.assert_any_call( + "emails/domain_manager_notification.txt", + "emails/domain_manager_notification_subject.txt", + to_address=mock_user1.email, + context={ + "domain": mock_domain1, + "requestor_email": mock_requestor_email, + "invited_email_address": email, + "domain_manager": mock_user1, + "date": date.today(), + }, + ) + mock_send_templated_email.assert_any_call( + "emails/domain_manager_notification.txt", + "emails/domain_manager_notification_subject.txt", + to_address=mock_user2.email, + context={ + "domain": mock_domain2, + "requestor_email": mock_requestor_email, + "invited_email_address": email, + "domain_manager": mock_user2, + "date": date.today(), + }, + ) + + # Verify the total number of calls to send_templated_email + self.assertEqual(mock_send_templated_email.call_count, 2) + + @patch("registrar.utility.email_invitations.validate_invitation") + def test_send_domain_invitation_email_raises_invite_validation_exception(self, mock_validate_invitation): + """Test sending domain invitation email for one domain and assert exception + when invite validation fails. + """ + # Setup + mock_validate_invitation.side_effect = ValueError("Validation failed") + email = "invitee@example.com" + requestor = MagicMock() + domain = MagicMock() + + # Call and assert exception + with self.assertRaises(ValueError) as context: + send_domain_invitation_email(email, requestor, domain, is_member_of_different_org=False) + + self.assertEqual(str(context.exception), "Validation failed") + mock_validate_invitation.assert_called_once() + + @patch("registrar.utility.email_invitations.get_requestor_email") + def test_send_domain_invitation_email_raises_get_requestor_email_exception(self, mock_get_requestor_email): + """Test sending domain invitation email for one domain and assert exception + when get_requestor_email fails. + """ + # Setup + mock_get_requestor_email.side_effect = ValueError("Validation failed") + email = "invitee@example.com" + requestor = MagicMock() + domain = MagicMock() + + # Call and assert exception + with self.assertRaises(ValueError) as context: + send_domain_invitation_email(email, requestor, domain, is_member_of_different_org=False) + + self.assertEqual(str(context.exception), "Validation failed") + mock_get_requestor_email.assert_called_once() + + @patch("registrar.utility.email_invitations.validate_invitation") + @patch("registrar.utility.email_invitations.get_requestor_email") + @patch("registrar.utility.email_invitations.send_invitation_email") + @patch("registrar.utility.email_invitations.normalize_domains") + def test_send_domain_invitation_email_raises_sending_email_exception( + self, + mock_normalize_domains, + mock_send_invitation_email, + mock_get_requestor_email, + mock_validate_invitation, + ): + """Test sending domain invitation email for one domain and assert exception + when send_invitation_email fails. + """ + # Setup + mock_domain = MagicMock(name="domain1") + mock_domain.name = "example.com" + mock_normalize_domains.return_value = [mock_domain] + + mock_requestor = MagicMock() + mock_requestor_email = "requestor@example.com" + mock_get_requestor_email.return_value = mock_requestor_email + + mock_user1 = MagicMock() + mock_user1.email = "manager1@example.com" + + email = "invitee@example.com" + is_member_of_different_org = False + + mock_send_invitation_email.side_effect = EmailSendingError("Error sending email") + + # Call and assert exception + with self.assertRaises(EmailSendingError) as context: + send_domain_invitation_email( + email=email, + requestor=mock_requestor, + domains=mock_domain, + is_member_of_different_org=is_member_of_different_org, + ) + + # Assertions + mock_normalize_domains.assert_called_once_with(mock_domain) + mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain]) + mock_validate_invitation.assert_called_once_with( + email, [mock_domain], mock_requestor, is_member_of_different_org + ) + self.assertEqual(str(context.exception), "Error sending email") + + @patch("registrar.utility.email_invitations.send_emails_to_domain_managers") + @patch("registrar.utility.email_invitations.validate_invitation") + @patch("registrar.utility.email_invitations.get_requestor_email") + @patch("registrar.utility.email_invitations.send_invitation_email") + @patch("registrar.utility.email_invitations.normalize_domains") + def test_send_domain_invitation_email_manager_emails_send_mail_exception( + self, + mock_normalize_domains, + mock_send_invitation_email, + mock_get_requestor_email, + mock_validate_invitation, + mock_send_domain_manager_emails, + ): + """Test sending domain invitation email for one domain and assert exception + when send_emails_to_domain_managers fails. + """ + # Setup + mock_domain = MagicMock(name="domain1") + mock_domain.name = "example.com" + mock_normalize_domains.return_value = [mock_domain] + + mock_requestor = MagicMock() + mock_requestor_email = "requestor@example.com" + mock_get_requestor_email.return_value = mock_requestor_email + + email = "invitee@example.com" + is_member_of_different_org = False + + mock_send_domain_manager_emails.side_effect = EmailSendingError("Error sending email") + + # Call and assert exception + with self.assertRaises(EmailSendingError) as context: + send_domain_invitation_email( + email=email, + requestor=mock_requestor, + domains=mock_domain, + is_member_of_different_org=is_member_of_different_org, + ) + + # Assertions + mock_normalize_domains.assert_called_once_with(mock_domain) + mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain]) + mock_validate_invitation.assert_called_once_with( + email, [mock_domain], mock_requestor, is_member_of_different_org + ) + mock_send_invitation_email.assert_called_once_with(email, mock_requestor_email, [mock_domain], None) + self.assertEqual(str(context.exception), "Error sending email") diff --git a/src/registrar/utility/email_invitations.py b/src/registrar/utility/email_invitations.py index ba660499e..c2bf22c30 100644 --- a/src/registrar/utility/email_invitations.py +++ b/src/registrar/utility/email_invitations.py @@ -1,8 +1,6 @@ from datetime import date from django.conf import settings -from registrar.models import DomainInvitation -from registrar.models.domain import Domain -from registrar.models.user_domain_role import UserDomainRole +from registrar.models import Domain, DomainInvitation, UserDomainRole from registrar.utility.errors import ( AlreadyDomainInvitedError, AlreadyDomainManagerError, @@ -73,7 +71,9 @@ def send_emails_to_domain_managers(email: str, requestor_email, domain: Domain, }, ) except EmailSendingError as err: - raise EmailSendingError(f"Could not send email manager notification to {user.email} for domains: {domain.name}") from err + raise EmailSendingError( + f"Could not send email manager notification to {user.email} for domains: {domain.name}" + ) from err def normalize_domains(domains): From 13c9e1c1135e3bb0b1fa936614721c214a14df69 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 15 Jan 2025 16:35:04 -0500 Subject: [PATCH 7/9] lint --- src/registrar/tests/test_admin.py | 18 ++++++------------ src/registrar/tests/test_email_invitations.py | 8 ++++++++ src/registrar/tests/test_views_domain.py | 8 ++++++-- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 673057e20..1decf02dd 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -165,19 +165,15 @@ class TestDomainInvitationAdmin(TestCase): response, "Domain invitations contain all individuals who have been invited to manage a .gov domain." ) self.assertContains(response, "Show more") - + @less_console_noise_decorator def test_has_change_form_description(self): """Tests if this model has a model description on the change form view""" self.client.force_login(self.superuser) - domain, _ = Domain.objects.get_or_create( - name="systemofadown.com" - ) + domain, _ = Domain.objects.get_or_create(name="systemofadown.com") - domain_invitation, _ = DomainInvitation.objects.get_or_create( - email="toxicity@systemofadown.com", domain=domain - ) + domain_invitation, _ = DomainInvitation.objects.get_or_create(email="toxicity@systemofadown.com", domain=domain) response = self.client.get( "/admin/registrar/domaininvitation/{}/change/".format(domain_invitation.pk), @@ -190,7 +186,7 @@ class TestDomainInvitationAdmin(TestCase): # Test for a description snippet self.assertContains( response, - "If you add someone to a domain here, it will trigger emails to the invitee and all managers of the domain when you click", + "If you add someone to a domain here, it will trigger emails to the invitee and all managers of the domain", ) @less_console_noise_decorator @@ -1983,15 +1979,13 @@ class TestUserDomainRoleAdmin(TestCase): response, "This table represents the managers who are assigned to each domain in the registrar" ) self.assertContains(response, "Show more") - + @less_console_noise_decorator def test_has_change_form_description(self): """Tests if this model has a model description on the change form view""" self.client.force_login(self.superuser) - domain, _ = Domain.objects.get_or_create( - name="systemofadown.com" - ) + domain, _ = Domain.objects.get_or_create(name="systemofadown.com") user_domain_role, _ = UserDomainRole.objects.get_or_create( user=self.superuser, domain=domain, role=[UserDomainRole.Roles.MANAGER] diff --git a/src/registrar/tests/test_email_invitations.py b/src/registrar/tests/test_email_invitations.py index 6a0423f4d..87384d3be 100644 --- a/src/registrar/tests/test_email_invitations.py +++ b/src/registrar/tests/test_email_invitations.py @@ -4,9 +4,12 @@ from datetime import date from registrar.utility.email import EmailSendingError from registrar.utility.email_invitations import send_domain_invitation_email +from api.tests.common import less_console_noise_decorator + class DomainInvitationEmail(unittest.TestCase): + @less_console_noise_decorator @patch("registrar.utility.email_invitations.send_templated_email") @patch("registrar.utility.email_invitations.UserDomainRole.objects.filter") @patch("registrar.utility.email_invitations.validate_invitation") @@ -71,6 +74,7 @@ class DomainInvitationEmail(unittest.TestCase): }, ) + @less_console_noise_decorator @patch("registrar.utility.email_invitations.send_templated_email") @patch("registrar.utility.email_invitations.UserDomainRole.objects.filter") @patch("registrar.utility.email_invitations.validate_invitation") @@ -170,6 +174,7 @@ class DomainInvitationEmail(unittest.TestCase): # Verify the total number of calls to send_templated_email self.assertEqual(mock_send_templated_email.call_count, 2) + @less_console_noise_decorator @patch("registrar.utility.email_invitations.validate_invitation") def test_send_domain_invitation_email_raises_invite_validation_exception(self, mock_validate_invitation): """Test sending domain invitation email for one domain and assert exception @@ -188,6 +193,7 @@ class DomainInvitationEmail(unittest.TestCase): self.assertEqual(str(context.exception), "Validation failed") mock_validate_invitation.assert_called_once() + @less_console_noise_decorator @patch("registrar.utility.email_invitations.get_requestor_email") def test_send_domain_invitation_email_raises_get_requestor_email_exception(self, mock_get_requestor_email): """Test sending domain invitation email for one domain and assert exception @@ -206,6 +212,7 @@ class DomainInvitationEmail(unittest.TestCase): self.assertEqual(str(context.exception), "Validation failed") mock_get_requestor_email.assert_called_once() + @less_console_noise_decorator @patch("registrar.utility.email_invitations.validate_invitation") @patch("registrar.utility.email_invitations.get_requestor_email") @patch("registrar.utility.email_invitations.send_invitation_email") @@ -254,6 +261,7 @@ class DomainInvitationEmail(unittest.TestCase): ) self.assertEqual(str(context.exception), "Error sending email") + @less_console_noise_decorator @patch("registrar.utility.email_invitations.send_emails_to_domain_managers") @patch("registrar.utility.email_invitations.validate_invitation") @patch("registrar.utility.email_invitations.get_requestor_email") diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index a9de8d6e7..45758e502 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -9,7 +9,7 @@ from registrar.utility.email import EmailSendingError from waffle.testutils import override_flag from api.tests.common import less_console_noise_decorator from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices -from .common import MockEppLib, MockSESClient, create_user # type: ignore +from .common import MockEppLib, create_user # type: ignore from django_webtest import WebTest # type: ignore import boto3_mocking # type: ignore @@ -766,7 +766,11 @@ class TestDomainManagers(TestDomainOverview): success_result = add_page.form.submit() mock_send_domain_email.assert_called_once_with( - email="mayor@igorville.gov", requestor=self.user, domains=self.domain, is_member_of_different_org=None, requested_user=user + email="mayor@igorville.gov", + requestor=self.user, + domains=self.domain, + is_member_of_different_org=None, + requested_user=user, ) self.assertEqual(success_result.status_code, 302) From 3420cc3329cf884ed3b31a9cb0adc53625a24cee Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 15 Jan 2025 16:41:49 -0500 Subject: [PATCH 8/9] more linter --- src/registrar/utility/email_invitations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/utility/email_invitations.py b/src/registrar/utility/email_invitations.py index c2bf22c30..3653d4290 100644 --- a/src/registrar/utility/email_invitations.py +++ b/src/registrar/utility/email_invitations.py @@ -76,7 +76,7 @@ def send_emails_to_domain_managers(email: str, requestor_email, domain: Domain, ) from err -def normalize_domains(domains): +def normalize_domains(domains: Domain | list[Domain]) -> list[Domain]: """Ensures domains is always a list.""" return [domains] if isinstance(domains, Domain) else domains From 6905531061e48eb3f24b70f0d8c64982e9b3b34a Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 20 Jan 2025 07:34:00 -0500 Subject: [PATCH 9/9] fixing err message, and updating comment --- src/registrar/utility/email_invitations.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/registrar/utility/email_invitations.py b/src/registrar/utility/email_invitations.py index 3653d4290..25e9db0f3 100644 --- a/src/registrar/utility/email_invitations.py +++ b/src/registrar/utility/email_invitations.py @@ -52,6 +52,11 @@ def send_domain_invitation_email( def send_emails_to_domain_managers(email: str, requestor_email, domain: Domain, requested_user=None): + """ + Notifies all domain managers of the provided domain of a change + Raises: + EmailSendingError + """ # Get each domain manager from list user_domain_roles = UserDomainRole.objects.filter(domain=domain) for user_domain_role in user_domain_roles: @@ -72,7 +77,7 @@ def send_emails_to_domain_managers(email: str, requestor_email, domain: Domain, ) except EmailSendingError as err: raise EmailSendingError( - f"Could not send email manager notification to {user.email} for domains: {domain.name}" + f"Could not send email manager notification to {user.email} for domain: {domain.name}" ) from err