From 9c090ae5fe640767da48cb297eadf56e640ef0bd Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 10 Sep 2024 15:37:04 -0500 Subject: [PATCH 01/49] send notification on nameserver changes --- src/registrar/fixtures_users.py | 2 ++ src/registrar/views/domain.py | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/registrar/fixtures_users.py b/src/registrar/fixtures_users.py index 1b8eda9ab..a1ad0ecf7 100644 --- a/src/registrar/fixtures_users.py +++ b/src/registrar/fixtures_users.py @@ -39,6 +39,7 @@ class UserFixture: "username": "be17c826-e200-4999-9389-2ded48c43691", "first_name": "Matthew", "last_name": "Spence", + "email": "mspence1845@gmail.com" }, { "username": "5f283494-31bd-49b5-b024-a7e7cae00848", @@ -155,6 +156,7 @@ class UserFixture: "username": "d6bf296b-fac5-47ff-9c12-f88ccc5c1b99", "first_name": "Matthew-Analyst", "last_name": "Spence-Analyst", + "email": "mspence1845+1@gmail.com" }, { "username": "319c490d-453b-43d9-bc4d-7d6cd8ff6844", diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 003f8dd0d..1abdb10e5 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -441,6 +441,9 @@ class DomainNameserversView(DomainFormBaseView): # no server information in this field, skip it pass + old_nameservers = self.object.nameservers + should_notify = old_nameservers and old_nameservers != nameservers + try: self.object.nameservers = nameservers except NameserverError as Err: @@ -467,6 +470,30 @@ class DomainNameserversView(DomainFormBaseView): "48 hours to propagate across the internet.", ) + # if the nameservers where changed, send notification to domain managers. + if should_notify: + managers = UserDomainRole.objects.filter(domain=self.object.name, role=UserDomainRole.Roles.MANAGER) + emails = list(managers.values_list("user", flat=True).values_list("email", flat=True)) + to_addresses=', '.join(emails) + + try: + send_templated_email( + "templateName", + "Subject Template Name", + to_address=to_addresses, + context={ + "nameservers": nameservers, + "domain": self.object, + }, + ) + except EmailSendingError as exc: + logger.warn( + "Could not sent notification email to %s for domain %s", + to_addresses, + self.object, + exc_info=True, + ) + # superclass has the redirect return super().form_valid(formset) From 3475a76899b07cee707b355ada8765d3c8289cd3 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 12 Sep 2024 14:39:29 -0500 Subject: [PATCH 02/49] test on nameservers --- src/registrar/utility/email.py | 33 ++++++++++++++++++++++++++++++++- src/registrar/views/domain.py | 24 ++++-------------------- 2 files changed, 36 insertions(+), 21 deletions(-) diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 1fe1be596..39bf9df5b 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -10,6 +10,7 @@ from django.template.loader import get_template from email.mime.application import MIMEApplication from email.mime.multipart import MIMEMultipart from email.mime.text import MIMEText +from registrar.models.user_domain_role import UserDomainRole from waffle import flag_is_active @@ -31,7 +32,7 @@ def send_templated_email( attachment_file=None, wrap_email=False, ): - """Send an email built from a template to one email address. + """Send an email built from a template. template_name and subject_template_name are relative to the same template context as Django's HTML templates. context gives additional information @@ -164,3 +165,33 @@ def send_email_with_attachment(sender, recipient, subject, body, attachment_file response = ses_client.send_raw_email(Source=sender, Destinations=[recipient], RawMessage={"Data": msg.as_string()}) return response + +def email_domain_managers(domain, template: str, subject_template: str, context: any = {}): + """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 + context as Django's HTML templates. context gives additional information + that the template may use. + + context is a dictionary containing any information needed to fill in values + in the provided template, exactly the same as with send_templated_email. + + Will log a warning if the email fails to send for any reason, but will not raise an error. + """ + managers = UserDomainRole.objects.filter(domain=domain, role=UserDomainRole.Roles.MANAGER) + emails = list(managers.values_list("user", flat=True).values_list("email", flat=True)) + + try: + send_templated_email( + template, + subject_template, + to_address=emails, + context=context, + ) + except EmailSendingError as exc: + logger.warn( + "Could not sent notification email to %s for domain %s", + emails, + domain, + exc_info=True, + ) \ No newline at end of file diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 1abdb10e5..1ac19ec44 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -57,7 +57,7 @@ from epplibwrapper import ( RegistryError, ) -from ..utility.email import send_templated_email, EmailSendingError +from ..utility.email import send_templated_email, EmailSendingError, email_domain_managers from .utility import DomainPermissionView, DomainInvitationPermissionDeleteView from waffle.decorators import waffle_flag @@ -472,27 +472,11 @@ class DomainNameserversView(DomainFormBaseView): # if the nameservers where changed, send notification to domain managers. if should_notify: - managers = UserDomainRole.objects.filter(domain=self.object.name, role=UserDomainRole.Roles.MANAGER) - emails = list(managers.values_list("user", flat=True).values_list("email", flat=True)) - to_addresses=', '.join(emails) - - try: - send_templated_email( - "templateName", - "Subject Template Name", - to_address=to_addresses, - context={ + context={ "nameservers": nameservers, "domain": self.object, - }, - ) - except EmailSendingError as exc: - logger.warn( - "Could not sent notification email to %s for domain %s", - to_addresses, - self.object, - exc_info=True, - ) + } + email_domain_managers(self.object.name, "template", "subject", context) # superclass has the redirect return super().form_valid(formset) From 6c605566a23c5da32fd2168815f6ad7e71d4a9df Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 12 Sep 2024 14:55:44 -0500 Subject: [PATCH 03/49] add temp email template and subject --- src/registrar/templates/emails/domain_change_notification.txt | 1 + .../templates/emails/domain_change_notification_subject.txt | 0 src/registrar/views/domain.py | 3 +-- 3 files changed, 2 insertions(+), 2 deletions(-) create mode 100644 src/registrar/templates/emails/domain_change_notification.txt create mode 100644 src/registrar/templates/emails/domain_change_notification_subject.txt diff --git a/src/registrar/templates/emails/domain_change_notification.txt b/src/registrar/templates/emails/domain_change_notification.txt new file mode 100644 index 000000000..b3c750257 --- /dev/null +++ b/src/registrar/templates/emails/domain_change_notification.txt @@ -0,0 +1 @@ +There has been a change to {{ domain }} \ No newline at end of file diff --git a/src/registrar/templates/emails/domain_change_notification_subject.txt b/src/registrar/templates/emails/domain_change_notification_subject.txt new file mode 100644 index 000000000..e69de29bb diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 1ac19ec44..aba504c41 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -473,10 +473,9 @@ class DomainNameserversView(DomainFormBaseView): # if the nameservers where changed, send notification to domain managers. if should_notify: context={ - "nameservers": nameservers, "domain": self.object, } - email_domain_managers(self.object.name, "template", "subject", context) + email_domain_managers(self.object.name, "emails/domain_change_notification.txt", "emails.domain_change_notification_subject.txt", context) # superclass has the redirect return super().form_valid(formset) From 1b7408aebc7c8f1fdcd734c794a5e479c429f415 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 12 Sep 2024 15:12:38 -0500 Subject: [PATCH 04/49] debug logs --- .../templates/emails/domain_change_notification_subject.txt | 1 + src/registrar/utility/email.py | 2 +- src/registrar/views/domain.py | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/registrar/templates/emails/domain_change_notification_subject.txt b/src/registrar/templates/emails/domain_change_notification_subject.txt index e69de29bb..d3f6fbedb 100644 --- a/src/registrar/templates/emails/domain_change_notification_subject.txt +++ b/src/registrar/templates/emails/domain_change_notification_subject.txt @@ -0,0 +1 @@ +Change Notification \ No newline at end of file diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 39bf9df5b..4a53661cf 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -180,7 +180,7 @@ def email_domain_managers(domain, template: str, subject_template: str, context: """ managers = UserDomainRole.objects.filter(domain=domain, role=UserDomainRole.Roles.MANAGER) emails = list(managers.values_list("user", flat=True).values_list("email", flat=True)) - + logger.debug("attempting to send templated email to domain managers") try: send_templated_email( template, diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index aba504c41..603fbbab5 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -472,6 +472,7 @@ class DomainNameserversView(DomainFormBaseView): # if the nameservers where changed, send notification to domain managers. if should_notify: + logger.debug("Sending email to domain managers") context={ "domain": self.object, } From 9609db5a53cbb8ca37d18815cedd98547f7de764 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 12 Sep 2024 15:32:49 -0500 Subject: [PATCH 05/49] MOAR LOGS --- src/registrar/views/domain.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 603fbbab5..97fa22d88 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -417,6 +417,8 @@ class DomainNameserversView(DomainFormBaseView): def form_valid(self, formset): """The formset is valid, perform something with it.""" + + logger.debug("------ Form is valid -------") self.request.session["nameservers_form_domain"] = self.object @@ -442,8 +444,9 @@ class DomainNameserversView(DomainFormBaseView): pass old_nameservers = self.object.nameservers + logger.debug("nameservers", nameservers) should_notify = old_nameservers and old_nameservers != nameservers - + logger.debug("should_notify", should_notify) try: self.object.nameservers = nameservers except NameserverError as Err: From 0ffff70cb4c52c45df20f5223a406d78bb46b8b8 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 12 Sep 2024 15:53:14 -0500 Subject: [PATCH 06/49] MOAR DEBUG LOGS --- src/registrar/views/domain.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 97fa22d88..6fbb0fe08 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -403,6 +403,9 @@ class DomainNameserversView(DomainFormBaseView): This post method harmonizes using DomainBaseView and FormMixin """ + + logger.debug("Posted to Namservers View") + self._get_domain(request) formset = self.get_form() From d6996bd189ae50ebfe6480ca79a5c5757b3d92d9 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Fri, 13 Sep 2024 10:07:53 -0500 Subject: [PATCH 07/49] more debug messages --- src/registrar/views/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 6fbb0fe08..1a87f185d 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -421,7 +421,7 @@ class DomainNameserversView(DomainFormBaseView): def form_valid(self, formset): """The formset is valid, perform something with it.""" - logger.debug("------ Form is valid -------") + logger.debug("------ Nameserver Form is valid -------") self.request.session["nameservers_form_domain"] = self.object From 579a8ac8aa7e129fa5eec90de18030f166a87224 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Fri, 13 Sep 2024 10:41:32 -0500 Subject: [PATCH 08/49] change to info level --- src/registrar/views/domain.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 1a87f185d..cc6816121 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -404,7 +404,7 @@ class DomainNameserversView(DomainFormBaseView): This post method harmonizes using DomainBaseView and FormMixin """ - logger.debug("Posted to Namservers View") + logger.info("Posted to Namservers View") self._get_domain(request) formset = self.get_form() @@ -421,7 +421,7 @@ class DomainNameserversView(DomainFormBaseView): def form_valid(self, formset): """The formset is valid, perform something with it.""" - logger.debug("------ Nameserver Form is valid -------") + logger.info("------ Nameserver Form is valid -------") self.request.session["nameservers_form_domain"] = self.object @@ -447,9 +447,9 @@ class DomainNameserversView(DomainFormBaseView): pass old_nameservers = self.object.nameservers - logger.debug("nameservers", nameservers) + logger.info("nameservers", nameservers) should_notify = old_nameservers and old_nameservers != nameservers - logger.debug("should_notify", should_notify) + logger.info("should_notify", should_notify) try: self.object.nameservers = nameservers except NameserverError as Err: @@ -478,7 +478,7 @@ class DomainNameserversView(DomainFormBaseView): # if the nameservers where changed, send notification to domain managers. if should_notify: - logger.debug("Sending email to domain managers") + logger.info("Sending email to domain managers") context={ "domain": self.object, } From 9bbcb98071f5c6a9878d989c30f84f5bfe7d0f1e Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 17 Sep 2024 14:22:58 -0500 Subject: [PATCH 09/49] tested up to email sending --- src/registrar/utility/email.py | 79 ++++++++++++++++++---------------- src/registrar/views/domain.py | 69 ++++++++++++++++++++++++++--- 2 files changed, 106 insertions(+), 42 deletions(-) diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 4a53661cf..c1082f50d 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -10,7 +10,6 @@ from django.template.loader import get_template from email.mime.application import MIMEApplication from email.mime.multipart import MIMEMultipart from email.mime.text import MIMEText -from registrar.models.user_domain_role import UserDomainRole from waffle import flag_is_active @@ -26,14 +25,20 @@ class EmailSendingError(RuntimeError): def send_templated_email( template_name: str, subject_template_name: str, - to_address: str, - bcc_address="", + to_address: str="", + bcc_address: str="", context={}, attachment_file=None, wrap_email=False, + cc_addresses: list[str]=[], ): """Send an email built from a template. + to can be either a string representing a single address or a + list of strings for multi-recipient emails. + + bcc currently only supports a single address. + template_name and subject_template_name are relative to the same template context as Django's HTML templates. context gives additional information that the template may use. @@ -46,6 +51,8 @@ def send_templated_email( # Raises an error if we cannot send an email (due to restrictions). # Does nothing otherwise. _can_send_email(to_address, bcc_address) + sendable_cc_addresses = get_sendable_addresses(cc_addresses) + template = get_template(template_name) email_body = template.render(context=context) @@ -70,9 +77,18 @@ def send_templated_email( 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]} + destination = {} + if to_address: + destination["ToAddresses"] = [to_address] if bcc_address: destination["BccAddresses"] = [bcc_address] + if cc_addresses: + destination["CcAddresses"] = sendable_cc_addresses + + # make sure we don't try and send an email to nowhere + if not destination: + message = "E-mail unable to send, no valid recipients provided." + raise EmailSendingError(message) try: if not attachment_file: @@ -105,7 +121,6 @@ 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.""" @@ -123,6 +138,28 @@ 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]) -> 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 + + Paramaters: + + addresses: a list of strings representing all addresses to be checked. + + raises: + EmailSendingError if email sending is disabled + """ + + 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) + else: + AllowedEmail = apps.get_model("registrar", "AllowedEmail") + allowed_emails = [address for address in addresses if (address and AllowedEmail.is_allowed_email(address))] + + return allowed_emails + def wrap_text_and_preserve_paragraphs(text, width): """ @@ -164,34 +201,4 @@ def send_email_with_attachment(sender, recipient, subject, body, attachment_file msg.attach(attachment_part) response = ses_client.send_raw_email(Source=sender, Destinations=[recipient], RawMessage={"Data": msg.as_string()}) - return response - -def email_domain_managers(domain, template: str, subject_template: str, context: any = {}): - """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 - context as Django's HTML templates. context gives additional information - that the template may use. - - context is a dictionary containing any information needed to fill in values - in the provided template, exactly the same as with send_templated_email. - - Will log a warning if the email fails to send for any reason, but will not raise an error. - """ - managers = UserDomainRole.objects.filter(domain=domain, role=UserDomainRole.Roles.MANAGER) - emails = list(managers.values_list("user", flat=True).values_list("email", flat=True)) - logger.debug("attempting to send templated email to domain managers") - try: - send_templated_email( - template, - subject_template, - to_address=emails, - context=context, - ) - except EmailSendingError as exc: - logger.warn( - "Could not sent notification email to %s for domain %s", - emails, - domain, - exc_info=True, - ) \ No newline at end of file + return response \ No newline at end of file diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index cc6816121..6b3b6095f 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -57,7 +57,7 @@ from epplibwrapper import ( RegistryError, ) -from ..utility.email import send_templated_email, EmailSendingError, email_domain_managers +from ..utility.email import send_templated_email, EmailSendingError from .utility import DomainPermissionView, DomainInvitationPermissionDeleteView from waffle.decorators import waffle_flag @@ -149,6 +149,43 @@ class DomainFormBaseView(DomainBaseView, FormMixin): logger.error("Could get domain_info. No domain info exists, or duplicates exist.") return current_domain_info + + def email_domain_managers(self, domain_name, template: str, subject_template: str, context: any = {}): + """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 + context as Django's HTML templates. context gives additional information + that the template may use. + + context is a dictionary containing any information needed to fill in values + in the provided template, exactly the same as with send_templated_email. + + Will log a warning if the email fails to send for any reason, but will not raise an error. + """ + try: + domain = Domain.objects.get(name=domain_name) + except Domain.DoesNotExist: + logger.warn( + "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) + 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: + logger.warn( + "Could not sent notification email to %s for domain %s", + emails, + domain_name, + exc_info=True, + ) class DomainView(DomainBaseView): @@ -225,6 +262,13 @@ class DomainOrgNameAddressView(DomainFormBaseView): def form_valid(self, form): """The form is valid, save the organization name and mailing address.""" + if form.has_changed(): + logger.info("Sending email to domain managers") + context={ + "domain": self.object, + } + self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) + form.save() messages.success(self.request, "The organization information for this domain has been updated.") @@ -325,6 +369,14 @@ class DomainSeniorOfficialView(DomainFormBaseView): # Set the domain information in the form so that it can be accessible # to associate a new Contact, if a new Contact is needed # in the save() method + if form.has_changed(): + logger.info("Sending email to domain managers") + context={ + "domain": self.object, + } + self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) + + form.set_domain_info(self.object.domain_info) form.save() @@ -447,10 +499,15 @@ class DomainNameserversView(DomainFormBaseView): pass old_nameservers = self.object.nameservers - logger.info("nameservers", nameservers) - should_notify = old_nameservers and old_nameservers != nameservers - logger.info("should_notify", should_notify) + logger.info("nameservers %s", nameservers) + logger.info("old nameservers: %s", old_nameservers) + + logger.info("State: %s", self.object.state) + + # if there are existing + logger.info("has changed? %s", formset.has_changed()) try: + # logger.info("skipping actual assignment of nameservers") self.object.nameservers = nameservers except NameserverError as Err: # NamserverErrors *should* be caught in form; if reached here, @@ -477,12 +534,12 @@ class DomainNameserversView(DomainFormBaseView): ) # if the nameservers where changed, send notification to domain managers. - if should_notify: + if formset.has_changed(): logger.info("Sending email to domain managers") context={ "domain": self.object, } - email_domain_managers(self.object.name, "emails/domain_change_notification.txt", "emails.domain_change_notification_subject.txt", context) + self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) # superclass has the redirect return super().form_valid(formset) From b8d697ebf04879030b3c08f41398bfe441d05d09 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 18 Sep 2024 14:33:44 -0500 Subject: [PATCH 10/49] another stab at email sending --- ops/manifests/manifest-ms.yaml | 2 +- src/registrar/tests/test_emails.py | 16 ++++++++ src/registrar/tests/test_views_domain.py | 48 ++++++++++++++++++++++++ src/registrar/utility/email.py | 13 +++++-- 4 files changed, 74 insertions(+), 5 deletions(-) diff --git a/ops/manifests/manifest-ms.yaml b/ops/manifests/manifest-ms.yaml index 153ee5f08..ac46f5d92 100644 --- a/ops/manifests/manifest-ms.yaml +++ b/ops/manifests/manifest-ms.yaml @@ -20,7 +20,7 @@ applications: # Tell Django where it is being hosted DJANGO_BASE_URL: https://getgov-ms.app.cloud.gov # Tell Django how much stuff to log - DJANGO_LOG_LEVEL: INFO + DJANGO_LOG_LEVEL: DEBUG # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/src/registrar/tests/test_emails.py b/src/registrar/tests/test_emails.py index e699d9b75..e798a0e8f 100644 --- a/src/registrar/tests/test_emails.py +++ b/src/registrar/tests/test_emails.py @@ -60,6 +60,22 @@ 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=["test_email1@example.com", "test_email2@example.com"] + ) + + # check that an email was sent + self.assertTrue(self.mock_client.send_email.called) @boto3_mocking.patching @less_console_noise_decorator diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index b096527f9..a4a9ecf96 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -2,6 +2,7 @@ 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 @@ -10,6 +11,7 @@ from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices 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, @@ -1973,3 +1975,49 @@ class TestDomainDNSSEC(TestDomainOverview): self.assertContains( result, str(DsDataError(code=DsDataErrorCodes.INVALID_DIGEST_SHA256)), count=2, status_code=200 ) + + +# class TestDomainChangeNotifications(TestDomainOverview): +# """Test email notifications on updates to domain information""" + +# @classmethod +# def setUpClass(cls): +# super().setUpClass() +# allowed_emails = [ +# AllowedEmail(email="info@example.com"), +# ] +# AllowedEmail.objects.bulk_create(allowed_emails) + +# @classmethod +# def tearDownClass(cls): +# super().tearDownClass() +# AllowedEmail.objects.all().delete() + +# def test_notification_email_sent_on_org_name_change(self): +# """Test that an email is sent when the organization name is changed.""" +# with patch('registrar.utility.email.boto3.client') as mock_boto3_client: +# mock_ses_client = mock_boto3_client.return_value + +# self.domain_information.organization_name = "Town of Igorville" +# 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] + +# org_name_page.form["organization_name"] = "Not igorville" + +# self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) +# success_result_page = org_name_page.form.submit() + +# # Check that the page loads successfully +# self.assertEqual(success_result_page.status_code, 200) +# self.assertContains(success_result_page, "Not igorville") + +# # Check that an email was sent +# mock_ses_client.send_email.assert_called_once() + +# # Check email content +# call_kwargs = mock_ses_client.send_email.call_args[1] +# self.assertEqual(call_kwargs['FromEmailAddress'], settings.DEFAULT_FROM_EMAIL) +# self.assertIn('Domain information updated', call_kwargs['Content']['Simple']['Subject']['Data']) +# self.assertIn('City of Igorville', call_kwargs['Content']['Simple']['Body']['Text']['Data']) \ No newline at end of file diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index c1082f50d..63d347cae 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -51,7 +51,10 @@ def send_templated_email( # Raises an error if we cannot send an email (due to restrictions). # Does nothing otherwise. _can_send_email(to_address, bcc_address) - sendable_cc_addresses = get_sendable_addresses(cc_addresses) + sendable_cc_addresses, blocked_cc_addresses = get_sendable_addresses(cc_addresses) + + 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) @@ -107,6 +110,7 @@ def send_templated_email( }, }, ) + logger.info("Email sent to %s, bcc %s, cc %s", to_address, bcc_address, cc_addresses) else: ses_client = boto3.client( "ses", @@ -138,10 +142,10 @@ 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]) -> list[str]: +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 + Returns: a lists of all provided addresses that are ok to send to and a list of addresses that were blocked. Paramaters: @@ -157,8 +161,9 @@ def get_sendable_addresses(addresses: list[str]) -> list[str]: else: AllowedEmail = apps.get_model("registrar", "AllowedEmail") allowed_emails = [address for address in addresses if (address and AllowedEmail.is_allowed_email(address))] + blocked_emails = [address for address in addresses if (address and not AllowedEmail.is_allowed_email(address))] - return allowed_emails + return allowed_emails, blocked_emails def wrap_text_and_preserve_paragraphs(text, width): From f254e0441fdea44207e16441dd48e143436c260e Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 19 Sep 2024 15:10:00 -0500 Subject: [PATCH 11/49] add email sending to all required forms --- src/registrar/views/domain.py | 37 +++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 6b3b6095f..1d693d6e6 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -526,6 +526,14 @@ class DomainNameserversView(DomainFormBaseView): messages.error(self.request, NameserverError(code=nsErrorCodes.BAD_DATA)) logger.error(f"Registry error: {Err}") else: + if form.has_changed(): + logger.info("Sending email to domain managers") + context={ + "domain": self.object, + } + self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) + + messages.success( self.request, "The name servers for this domain have been updated. " @@ -533,14 +541,6 @@ class DomainNameserversView(DomainFormBaseView): "48 hours to propagate across the internet.", ) - # if the nameservers where changed, send notification to domain managers. - if formset.has_changed(): - logger.info("Sending email to domain managers") - context={ - "domain": self.object, - } - self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) - # superclass has the redirect return super().form_valid(formset) @@ -586,6 +586,13 @@ class DomainDNSSECView(DomainFormBaseView): errmsg = "Error removing existing DNSSEC record(s)." logger.error(errmsg + ": " + err) messages.error(self.request, errmsg) + else: + if form.has_changed(): + logger.info("Sending email to domain managers") + context={ + "domain": self.object, + } + self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) return self.form_valid(form) @@ -710,6 +717,13 @@ class DomainDsDataView(DomainFormBaseView): logger.error(f"Registry error: {err}") return self.form_invalid(formset) else: + if form.has_changed(): + logger.info("Sending email to domain managers") + context={ + "domain": self.object, + } + self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) + messages.success(self.request, "The DS data records for this domain have been updated.") # superclass has the redirect return super().form_valid(formset) @@ -808,6 +822,13 @@ class DomainSecurityEmailView(DomainFormBaseView): messages.error(self.request, SecurityEmailError(code=SecurityEmailErrorCodes.BAD_DATA)) logger.error(f"Generic registry error: {Err}") else: + if form.has_changed(): + logger.info("Sending email to domain managers") + context={ + "domain": self.object, + } + self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) + messages.success(self.request, "The security email for this domain has been updated.") # superclass has the redirect From 4ce28fd18baa4c9afe4bfb54d568541b4916240a Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 19 Sep 2024 15:34:50 -0500 Subject: [PATCH 12/49] test simpler way to organize which emails to send --- src/registrar/views/domain.py | 93 ++++++++++++++++++++--------------- 1 file changed, 54 insertions(+), 39 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 1d693d6e6..8973ec8ec 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -110,6 +110,22 @@ class DomainFormBaseView(DomainBaseView, FormMixin): implementations of post, form_valid and form_invalid. """ + # send notification email for changes to any of these forms + notify_on_change = ( + DomainSecurityEmailForm, + DomainDnssecForm, + DomainDsdataFormset, + ) + + # forms of these types should not send notifications if they're part of a portfolio/Organization + notify_unless_portfolio = ( + DomainOrgNameAddressForm, + SeniorOfficialContactForm + ) + + def should_notify(self, form) -> bool: + return isinstance(form, self.notify_on_change) or isinstance(form, self.notify_unless_portfolio) + def post(self, request, *args, **kwargs): """Form submission posts to this view. @@ -126,6 +142,13 @@ class DomainFormBaseView(DomainBaseView, FormMixin): # updates session cache with domain self._update_session_with_domain() + if self.should_notify(form): + logger.info("Sending email to domain managers") + context={ + "domain": self.object, + } + self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) + # superclass has the redirect return super().form_valid(form) @@ -262,13 +285,6 @@ class DomainOrgNameAddressView(DomainFormBaseView): def form_valid(self, form): """The form is valid, save the organization name and mailing address.""" - if form.has_changed(): - logger.info("Sending email to domain managers") - context={ - "domain": self.object, - } - self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) - form.save() messages.success(self.request, "The organization information for this domain has been updated.") @@ -369,12 +385,12 @@ class DomainSeniorOfficialView(DomainFormBaseView): # Set the domain information in the form so that it can be accessible # to associate a new Contact, if a new Contact is needed # in the save() method - if form.has_changed(): - logger.info("Sending email to domain managers") - context={ - "domain": self.object, - } - self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) + # if form.has_changed(): + # logger.info("Sending email to domain managers") + # context={ + # "domain": self.object, + # } + # self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) form.set_domain_info(self.object.domain_info) @@ -526,13 +542,12 @@ class DomainNameserversView(DomainFormBaseView): messages.error(self.request, NameserverError(code=nsErrorCodes.BAD_DATA)) logger.error(f"Registry error: {Err}") else: - if form.has_changed(): - logger.info("Sending email to domain managers") - context={ - "domain": self.object, - } - self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) - + # if form.has_changed(): + # logger.info("Sending email to domain managers") + # context={ + # "domain": self.object, + # } + # self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) messages.success( self.request, @@ -586,13 +601,13 @@ class DomainDNSSECView(DomainFormBaseView): errmsg = "Error removing existing DNSSEC record(s)." logger.error(errmsg + ": " + err) messages.error(self.request, errmsg) - else: - if form.has_changed(): - logger.info("Sending email to domain managers") - context={ - "domain": self.object, - } - self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) + # else: + # if form.has_changed(): + # logger.info("Sending email to domain managers") + # context={ + # "domain": self.object, + # } + # self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) return self.form_valid(form) @@ -717,12 +732,12 @@ class DomainDsDataView(DomainFormBaseView): logger.error(f"Registry error: {err}") return self.form_invalid(formset) else: - if form.has_changed(): - logger.info("Sending email to domain managers") - context={ - "domain": self.object, - } - self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) + # if formset.has_changed(): + # logger.info("Sending email to domain managers") + # context={ + # "domain": self.object, + # } + # self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) messages.success(self.request, "The DS data records for this domain have been updated.") # superclass has the redirect @@ -822,12 +837,12 @@ class DomainSecurityEmailView(DomainFormBaseView): messages.error(self.request, SecurityEmailError(code=SecurityEmailErrorCodes.BAD_DATA)) logger.error(f"Generic registry error: {Err}") else: - if form.has_changed(): - logger.info("Sending email to domain managers") - context={ - "domain": self.object, - } - self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) + # if form.has_changed(): + # logger.info("Sending email to domain managers") + # context={ + # "domain": self.object, + # } + # self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) messages.success(self.request, "The security email for this domain has been updated.") From 2f270cded85e3fdef4089e91fc2c1dc8b7536868 Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Thu, 19 Sep 2024 16:05:15 -0500 Subject: [PATCH 13/49] cleanup and add email content --- ops/manifests/manifest-ms.yaml | 2 +- src/registrar/fixtures_users.py | 2 -- .../emails/domain_change_notification.txt | 1 - .../domain_change_notification_subject.txt | 1 - .../emails/update_to_approved_domain.txt | 23 +++++++++++++++++++ .../update_to_approved_domain_subject.txt | 1 + src/registrar/views/domain.py | 4 ++++ 7 files changed, 29 insertions(+), 5 deletions(-) delete mode 100644 src/registrar/templates/emails/domain_change_notification.txt delete mode 100644 src/registrar/templates/emails/domain_change_notification_subject.txt create mode 100644 src/registrar/templates/emails/update_to_approved_domain.txt create mode 100644 src/registrar/templates/emails/update_to_approved_domain_subject.txt diff --git a/ops/manifests/manifest-ms.yaml b/ops/manifests/manifest-ms.yaml index ac46f5d92..153ee5f08 100644 --- a/ops/manifests/manifest-ms.yaml +++ b/ops/manifests/manifest-ms.yaml @@ -20,7 +20,7 @@ applications: # Tell Django where it is being hosted DJANGO_BASE_URL: https://getgov-ms.app.cloud.gov # Tell Django how much stuff to log - DJANGO_LOG_LEVEL: DEBUG + DJANGO_LOG_LEVEL: INFO # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/src/registrar/fixtures_users.py b/src/registrar/fixtures_users.py index a1ad0ecf7..1b8eda9ab 100644 --- a/src/registrar/fixtures_users.py +++ b/src/registrar/fixtures_users.py @@ -39,7 +39,6 @@ class UserFixture: "username": "be17c826-e200-4999-9389-2ded48c43691", "first_name": "Matthew", "last_name": "Spence", - "email": "mspence1845@gmail.com" }, { "username": "5f283494-31bd-49b5-b024-a7e7cae00848", @@ -156,7 +155,6 @@ class UserFixture: "username": "d6bf296b-fac5-47ff-9c12-f88ccc5c1b99", "first_name": "Matthew-Analyst", "last_name": "Spence-Analyst", - "email": "mspence1845+1@gmail.com" }, { "username": "319c490d-453b-43d9-bc4d-7d6cd8ff6844", diff --git a/src/registrar/templates/emails/domain_change_notification.txt b/src/registrar/templates/emails/domain_change_notification.txt deleted file mode 100644 index b3c750257..000000000 --- a/src/registrar/templates/emails/domain_change_notification.txt +++ /dev/null @@ -1 +0,0 @@ -There has been a change to {{ domain }} \ No newline at end of file diff --git a/src/registrar/templates/emails/domain_change_notification_subject.txt b/src/registrar/templates/emails/domain_change_notification_subject.txt deleted file mode 100644 index d3f6fbedb..000000000 --- a/src/registrar/templates/emails/domain_change_notification_subject.txt +++ /dev/null @@ -1 +0,0 @@ -Change Notification \ No newline at end of file diff --git a/src/registrar/templates/emails/update_to_approved_domain.txt b/src/registrar/templates/emails/update_to_approved_domain.txt new file mode 100644 index 000000000..93ab4819f --- /dev/null +++ b/src/registrar/templates/emails/update_to_approved_domain.txt @@ -0,0 +1,23 @@ +Hi, +An update was made to a domain you manage. +DOMAIN: {{domain.gov}} +UPDATED BY: {{user}} +UPDATED ON: {{date}} +INFORMATION UPDATED: {{changes}} +You can view this update in the .gov registrar . + + +Get help with managing your .gov domain . + +---------------------------------------------------------------- +WHY DID YOU RECEIVE THIS EMAIL? +You’re listed as a domain manager for $domain.gov, so you’ll receive a notification whenever changes are made to that domain. +If you have questions or concerns, reach out to the person who made the change 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 \ No newline at end of file diff --git a/src/registrar/templates/emails/update_to_approved_domain_subject.txt b/src/registrar/templates/emails/update_to_approved_domain_subject.txt new file mode 100644 index 000000000..cf4c9a14c --- /dev/null +++ b/src/registrar/templates/emails/update_to_approved_domain_subject.txt @@ -0,0 +1 @@ +An update was made to {{domain}} \ No newline at end of file diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 8973ec8ec..895d58090 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -5,6 +5,7 @@ authorized users can see information on a domain, every view here should inherit from `DomainPermissionView` (or DomainInvitationPermissionDeleteView). """ +from datetime import date import logging from django.contrib import messages @@ -146,6 +147,9 @@ class DomainFormBaseView(DomainBaseView, FormMixin): logger.info("Sending email to domain managers") context={ "domain": self.object, + "user": self.request.user, + "date": date.today(), + "changes": form.changed_data } self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) From e0025dfa4a952ca51747606a1656c49c758bafe7 Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Thu, 19 Sep 2024 16:18:11 -0500 Subject: [PATCH 14/49] more cleanup --- src/registrar/utility/email.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 63d347cae..35d8a2029 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -34,16 +34,18 @@ def send_templated_email( ): """Send an email built from a template. - to can be either a string representing a single address or a - list of strings for multi-recipient emails. + to_address and bcc_address currently only supports a single address. - bcc currently only supports a single address. + cc_address is a list and can contain many addresses. Emails not in the + whitelist (if applicable) will be filtered out before sending. template_name and subject_template_name are relative to the same template context as Django's HTML templates. context gives additional information that the template may use. - Raises EmailSendingError if SES client could not be accessed + Raises EmailSendingError if: + SES client could not be accessed + No valid recipient addresses are provided """ if not settings.IS_PRODUCTION: # type: ignore @@ -160,8 +162,13 @@ def get_sendable_addresses(addresses: list[str]) -> tuple[list[str], list[str]]: raise EmailSendingError(message) else: AllowedEmail = apps.get_model("registrar", "AllowedEmail") - allowed_emails = [address for address in addresses if (address and AllowedEmail.is_allowed_email(address))] - blocked_emails = [address for address in addresses if (address and not AllowedEmail.is_allowed_email(address))] + allowed_emails = [] + blocked_emails = [] + for address in addresses: + if AllowedEmail.is_allowed_email(address): + allowed_emails.append(address) + else: + blocked_emails.append(address) return allowed_emails, blocked_emails @@ -206,4 +213,4 @@ def send_email_with_attachment(sender, recipient, subject, body, attachment_file msg.attach(attachment_part) response = ses_client.send_raw_email(Source=sender, Destinations=[recipient], RawMessage={"Data": msg.as_string()}) - return response \ No newline at end of file + return response From 68b8c7de41b7b6c2bed83aa95006afe252162f21 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 19 Sep 2024 16:41:27 -0500 Subject: [PATCH 15/49] fix email template/subject name --- src/registrar/views/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 895d58090..bd5d60756 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -151,7 +151,7 @@ class DomainFormBaseView(DomainBaseView, FormMixin): "date": date.today(), "changes": form.changed_data } - self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) + self.email_domain_managers(self.object, "emails/update_to_approved_domain.txt", "emails/update_to_approved_domain_subject.txt", context) # superclass has the redirect return super().form_valid(form) From 58d42156ba1d0ba14951985826e7abf7c9b2e26c Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Thu, 19 Sep 2024 16:57:56 -0500 Subject: [PATCH 16/49] add super call for form valid on security email form --- src/registrar/views/domain.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index bd5d60756..fc7be8fa7 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -143,6 +143,7 @@ class DomainFormBaseView(DomainBaseView, FormMixin): # updates session cache with domain self._update_session_with_domain() + logger.info("Valid form has changed? %s", form.has_changed()) if self.should_notify(form): logger.info("Sending email to domain managers") context={ @@ -849,6 +850,9 @@ class DomainSecurityEmailView(DomainFormBaseView): # self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) messages.success(self.request, "The security email for this domain has been updated.") + + # superclass has the redirect + return super().form_valid(form) # superclass has the redirect return redirect(self.get_success_url()) From 63eea77e802a12f9502d17bd4b22b048a370b3b4 Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Fri, 20 Sep 2024 13:11:37 -0500 Subject: [PATCH 17/49] another refactor --- src/registrar/views/domain.py | 145 ++++++++++++++++------------------ 1 file changed, 69 insertions(+), 76 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 1ab8af816..5d91aadb3 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -109,22 +109,6 @@ class DomainFormBaseView(DomainBaseView, FormMixin): implementations of post, form_valid and form_invalid. """ - # send notification email for changes to any of these forms - notify_on_change = ( - DomainSecurityEmailForm, - DomainDnssecForm, - DomainDsdataFormset, - ) - - # forms of these types should not send notifications if they're part of a portfolio/Organization - notify_unless_portfolio = ( - DomainOrgNameAddressForm, - SeniorOfficialContactForm - ) - - def should_notify(self, form) -> bool: - return isinstance(form, self.notify_on_change) or isinstance(form, self.notify_unless_portfolio) - def post(self, request, *args, **kwargs): """Form submission posts to this view. @@ -141,17 +125,6 @@ class DomainFormBaseView(DomainBaseView, FormMixin): # updates session cache with domain self._update_session_with_domain() - logger.info("Valid form has changed? %s", form.has_changed()) - if self.should_notify(form): - logger.info("Sending email to domain managers") - context={ - "domain": self.object, - "user": self.request.user, - "date": date.today(), - "changes": form.changed_data - } - self.email_domain_managers(self.object, "emails/update_to_approved_domain.txt", "emails/update_to_approved_domain_subject.txt", context) - # superclass has the redirect return super().form_valid(form) @@ -176,6 +149,63 @@ class DomainFormBaseView(DomainBaseView, FormMixin): return current_domain_info + def send_update_notification(self, form, is_formset: bool=False): + """Send a notification to all domain managers that an update has occured + for a single domain. Uses update_to_approved_domain.txt template. + + Checks for changes, and does nothing if the form has not changed. + + Formsets have to be handled in a special way, so use is_formset to indicate + whether the value passed into form is actually a formset. + """ + + # send notification email for changes to any of these forms + notify_on_change = ( + DomainSecurityEmailForm, + DomainDnssecForm, + DomainDsdataFormset, + ) + + # forms of these types should not send notifications if they're part of a portfolio/Organization + notify_unless_in_portfolio = ( + DomainOrgNameAddressForm, + SeniorOfficialContactForm + ) + + if isinstance(form, notify_on_change): + # always notify for these forms + should_notify=True + elif isinstance(form, notify_unless_in_portfolio): + # for these forms, only notify if the domain is not in a portfolio + info: DomainInformation = self.get_domain_info_from_domain() + if not info or info.portfolio: + should_notify = False + else: + should_notify=True + else: + # don't notify for any other types of forms + should_notify=False + + if should_notify and form.has_changed: + logger.info("Sending email to domain managers") + + changes = self._get_changes_from_formset(form) if is_formset else form.changed_data + + context={ + "domain": self.object.name, + "user": self.request.user, + "date": date.today(), + "changes": str(changes).strip("'") # django templates auto-escape quotes + } + self.email_domain_managers(self.object, "emails/update_to_approved_domain.txt", "emails/update_to_approved_domain_subject.txt", context) + + def _get_changes_from_formset(self, formset): + changes = set() + for form in formset: + changes.update(form.get_changes) + + return list(changes) + def email_domain_managers(self, domain_name, template: str, subject_template: str, context: any = {}): """Send a single email built from a template to all managers for a given domain. @@ -191,7 +221,7 @@ class DomainFormBaseView(DomainBaseView, FormMixin): try: domain = Domain.objects.get(name=domain_name) except Domain.DoesNotExist: - logger.warn( + logger.warning( "Could not send notification email for domain %s, unable to find matching domain object", domain_name ) @@ -206,7 +236,7 @@ class DomainFormBaseView(DomainBaseView, FormMixin): cc_addresses=emails ) except EmailSendingError as exc: - logger.warn( + logger.warning( "Could not sent notification email to %s for domain %s", emails, domain_name, @@ -290,6 +320,8 @@ class DomainOrgNameAddressView(DomainFormBaseView): """The form is valid, save the organization name and mailing address.""" form.save() + self.send_update_notification(form) + messages.success(self.request, "The organization information for this domain has been updated.") # superclass has the redirect @@ -387,18 +419,12 @@ class DomainSeniorOfficialView(DomainFormBaseView): # Set the domain information in the form so that it can be accessible # to associate a new Contact, if a new Contact is needed - # in the save() method - # if form.has_changed(): - # logger.info("Sending email to domain managers") - # context={ - # "domain": self.object, - # } - # self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) - - + # in the save() methodS form.set_domain_info(self.object.domain_info) form.save() + self.send_update_notification(form) + messages.success(self.request, "The senior official for this domain has been updated.") # superclass has the redirect @@ -516,17 +542,7 @@ class DomainNameserversView(DomainFormBaseView): except KeyError: # no server information in this field, skip it pass - - old_nameservers = self.object.nameservers - logger.info("nameservers %s", nameservers) - logger.info("old nameservers: %s", old_nameservers) - - logger.info("State: %s", self.object.state) - - # if there are existing - logger.info("has changed? %s", formset.has_changed()) try: - # logger.info("skipping actual assignment of nameservers") self.object.nameservers = nameservers except NameserverError as Err: # NamserverErrors *should* be caught in form; if reached here, @@ -545,13 +561,7 @@ class DomainNameserversView(DomainFormBaseView): messages.error(self.request, NameserverError(code=nsErrorCodes.BAD_DATA)) logger.error(f"Registry error: {Err}") else: - # if form.has_changed(): - # logger.info("Sending email to domain managers") - # context={ - # "domain": self.object, - # } - # self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) - + self.send_update_notification(formset, is_formset=True) messages.success( self.request, "The name servers for this domain have been updated. " @@ -604,14 +614,8 @@ class DomainDNSSECView(DomainFormBaseView): errmsg = "Error removing existing DNSSEC record(s)." logger.error(errmsg + ": " + err) messages.error(self.request, errmsg) - # else: - # if form.has_changed(): - # logger.info("Sending email to domain managers") - # context={ - # "domain": self.object, - # } - # self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) - + else: + self.send_update_notification(form) return self.form_valid(form) @@ -735,12 +739,7 @@ class DomainDsDataView(DomainFormBaseView): logger.error(f"Registry error: {err}") return self.form_invalid(formset) else: - # if formset.has_changed(): - # logger.info("Sending email to domain managers") - # context={ - # "domain": self.object, - # } - # self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) + self.send_update_notification(formset, is_formset=True) messages.success(self.request, "The DS data records for this domain have been updated.") # superclass has the redirect @@ -808,13 +807,7 @@ class DomainSecurityEmailView(DomainFormBaseView): messages.error(self.request, SecurityEmailError(code=SecurityEmailErrorCodes.BAD_DATA)) logger.error(f"Generic registry error: {Err}") else: - # if form.has_changed(): - # logger.info("Sending email to domain managers") - # context={ - # "domain": self.object, - # } - # self.email_domain_managers(self.object, "emails/domain_change_notification.txt", "emails/domain_change_notification_subject.txt", context) - + self.send_update_notification(form) messages.success(self.request, "The security email for this domain has been updated.") # superclass has the redirect From 07c1060f6e98eaa324a42a03530af74011733057 Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Fri, 20 Sep 2024 13:19:29 -0500 Subject: [PATCH 18/49] fix minor bug in logger formatting --- src/registrar/config/settings.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 03d9e38c6..1b20caf2a 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -475,6 +475,10 @@ class JsonServerFormatter(ServerFormatter): def format(self, record): formatted_record = super().format(record) + + 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) From cb7005611a344c69dc48f196c230406b6d18a109 Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Fri, 20 Sep 2024 13:28:45 -0500 Subject: [PATCH 19/49] typo fix --- src/registrar/views/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 5d91aadb3..89a1338e6 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -202,7 +202,7 @@ class DomainFormBaseView(DomainBaseView, FormMixin): def _get_changes_from_formset(self, formset): changes = set() for form in formset: - changes.update(form.get_changes) + changes.update(form.changed_data) return list(changes) From 5dbc356d711d1163c9e611b754b24509afcc0211 Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Fri, 20 Sep 2024 13:53:43 -0500 Subject: [PATCH 20/49] map forms to labels --- src/registrar/views/domain.py | 53 +++++++++++++---------------------- 1 file changed, 20 insertions(+), 33 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 89a1338e6..47f72d0f8 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -149,39 +149,35 @@ class DomainFormBaseView(DomainBaseView, FormMixin): return current_domain_info - def send_update_notification(self, form, is_formset: bool=False): + def send_update_notification(self, form): """Send a notification to all domain managers that an update has occured for a single domain. Uses update_to_approved_domain.txt template. Checks for changes, and does nothing if the form has not changed. - - Formsets have to be handled in a special way, so use is_formset to indicate - whether the value passed into form is actually a formset. """ # send notification email for changes to any of these forms - notify_on_change = ( - DomainSecurityEmailForm, - DomainDnssecForm, - DomainDsdataFormset, - ) + form_label_dict = { + DomainSecurityEmailForm: "Security Email", + DomainDnssecForm: "DNSSec", + DomainDsdataFormset: "DS Data", + DomainOrgNameAddressForm: "Org Name/Address", + SeniorOfficialContactForm: "Senior Official", + } # forms of these types should not send notifications if they're part of a portfolio/Organization - notify_unless_in_portfolio = ( + check_for_portfolio = { DomainOrgNameAddressForm, - SeniorOfficialContactForm - ) + SeniorOfficialContactForm, + } - if isinstance(form, notify_on_change): - # always notify for these forms + if form.__class__ in form_label_dict: should_notify=True - elif isinstance(form, notify_unless_in_portfolio): - # for these forms, only notify if the domain is not in a portfolio - info: DomainInformation = self.get_domain_info_from_domain() - if not info or info.portfolio: - should_notify = False - else: - should_notify=True + if form.__class__ in check_for_portfolio: + # check for portfolio + info = self.get_domain_info_from_domain() + if not info or info.portfolio: + should_notify = False else: # don't notify for any other types of forms should_notify=False @@ -189,22 +185,13 @@ class DomainFormBaseView(DomainBaseView, FormMixin): if should_notify and form.has_changed: logger.info("Sending email to domain managers") - changes = self._get_changes_from_formset(form) if is_formset else form.changed_data - context={ "domain": self.object.name, "user": self.request.user, "date": date.today(), - "changes": str(changes).strip("'") # django templates auto-escape quotes + "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) - - def _get_changes_from_formset(self, formset): - changes = set() - for form in formset: - changes.update(form.changed_data) - - return list(changes) def email_domain_managers(self, domain_name, template: str, subject_template: str, context: any = {}): """Send a single email built from a template to all managers for a given domain. @@ -561,7 +548,7 @@ class DomainNameserversView(DomainFormBaseView): messages.error(self.request, NameserverError(code=nsErrorCodes.BAD_DATA)) logger.error(f"Registry error: {Err}") else: - self.send_update_notification(formset, is_formset=True) + self.send_update_notification(formset) messages.success( self.request, "The name servers for this domain have been updated. " @@ -739,7 +726,7 @@ class DomainDsDataView(DomainFormBaseView): logger.error(f"Registry error: {err}") return self.form_invalid(formset) else: - self.send_update_notification(formset, is_formset=True) + self.send_update_notification(formset) messages.success(self.request, "The DS data records for this domain have been updated.") # superclass has the redirect From ccbefe209c408c1dba14179c327921c234315989 Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Tue, 24 Sep 2024 10:11:55 -0500 Subject: [PATCH 21/49] Minor refactor fo update notification and formatting changes --- .../emails/update_to_approved_domain.txt | 9 ++++++--- src/registrar/views/domain.py | 16 +++++++++------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/registrar/templates/emails/update_to_approved_domain.txt b/src/registrar/templates/emails/update_to_approved_domain.txt index 93ab4819f..bc0950808 100644 --- a/src/registrar/templates/emails/update_to_approved_domain.txt +++ b/src/registrar/templates/emails/update_to_approved_domain.txt @@ -1,6 +1,8 @@ +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} + Hi, An update was made to a domain you manage. -DOMAIN: {{domain.gov}} +DOMAIN: {{domain}} UPDATED BY: {{user}} UPDATED ON: {{date}} INFORMATION UPDATED: {{changes}} @@ -11,7 +13,7 @@ Get help with managing your .gov domain -Learn about .gov \ No newline at end of file +Learn about .gov +{% endautoescape %} \ No newline at end of file diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 47f72d0f8..04b740f6e 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -149,11 +149,12 @@ class DomainFormBaseView(DomainBaseView, FormMixin): return current_domain_info - def send_update_notification(self, form): + def send_update_notification(self, form, force_send=False): """Send a notification to all domain managers that an update has occured for a single domain. Uses update_to_approved_domain.txt template. - Checks for changes, and does nothing if the form has not changed. + 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 @@ -172,9 +173,10 @@ class DomainFormBaseView(DomainBaseView, FormMixin): } if form.__class__ in form_label_dict: + # these types of forms can cause notifications should_notify=True if form.__class__ in check_for_portfolio: - # 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: should_notify = False @@ -182,7 +184,7 @@ class DomainFormBaseView(DomainBaseView, FormMixin): # don't notify for any other types of forms should_notify=False - if should_notify and form.has_changed: + if (should_notify and form.has_changed()) or force_send: logger.info("Sending email to domain managers") context={ @@ -305,10 +307,10 @@ class DomainOrgNameAddressView(DomainFormBaseView): def form_valid(self, form): """The form is valid, save the organization name and mailing address.""" - form.save() - self.send_update_notification(form) + form.save() + messages.success(self.request, "The organization information for this domain has been updated.") # superclass has the redirect @@ -602,7 +604,7 @@ class DomainDNSSECView(DomainFormBaseView): logger.error(errmsg + ": " + err) messages.error(self.request, errmsg) else: - self.send_update_notification(form) + self.send_update_notification(form, force_send=True) return self.form_valid(form) From 89f19bf80718b1f913182edf1c7ad60379f05307 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 24 Sep 2024 10:23:51 -0500 Subject: [PATCH 22/49] uncomment test --- src/registrar/tests/test_views_domain.py | 69 ++++++++++++------------ 1 file changed, 34 insertions(+), 35 deletions(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index f0aea8588..109b80913 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -1968,47 +1968,46 @@ class TestDomainDNSSEC(TestDomainOverview): ) -# class TestDomainChangeNotifications(TestDomainOverview): -# """Test email notifications on updates to domain information""" +class TestDomainChangeNotifications(TestDomainOverview): + """Test email notifications on updates to domain information""" -# @classmethod -# def setUpClass(cls): -# super().setUpClass() -# allowed_emails = [ -# AllowedEmail(email="info@example.com"), -# ] -# AllowedEmail.objects.bulk_create(allowed_emails) + @classmethod + def setUpClass(cls): + super().setUpClass() + allowed_emails = [ + AllowedEmail(email="info@example.com"), + ] + AllowedEmail.objects.bulk_create(allowed_emails) -# @classmethod -# def tearDownClass(cls): -# super().tearDownClass() -# AllowedEmail.objects.all().delete() + @classmethod + def tearDownClass(cls): + super().tearDownClass() + AllowedEmail.objects.all().delete() -# def test_notification_email_sent_on_org_name_change(self): -# """Test that an email is sent when the organization name is changed.""" -# with patch('registrar.utility.email.boto3.client') as mock_boto3_client: -# mock_ses_client = mock_boto3_client.return_value + def test_notification_email_sent_on_org_name_change(self): + """Test that an email is sent when the organization name is changed.""" + with patch('registrar.utility.email.boto3.client') as mock_boto3_client: + mock_ses_client = mock_boto3_client.return_value -# self.domain_information.organization_name = "Town of Igorville" -# self.domain_information.save() + self.domain_information.organization_name = "Town of Igorville" + 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] + 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] -# org_name_page.form["organization_name"] = "Not igorville" + org_name_page.form["organization_name"] = "Not igorville" -# self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) -# success_result_page = org_name_page.form.submit() + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + success_result_page = org_name_page.form.submit() + # Check that the page loads successfully + self.assertEqual(success_result_page.status_code, 200) + self.assertContains(success_result_page, "Not igorville") -# # Check that the page loads successfully -# self.assertEqual(success_result_page.status_code, 200) -# self.assertContains(success_result_page, "Not igorville") - -# # Check that an email was sent -# mock_ses_client.send_email.assert_called_once() + # Check that an email was sent + mock_ses_client.send_email.assert_called_once() -# # Check email content -# call_kwargs = mock_ses_client.send_email.call_args[1] -# self.assertEqual(call_kwargs['FromEmailAddress'], settings.DEFAULT_FROM_EMAIL) -# self.assertIn('Domain information updated', call_kwargs['Content']['Simple']['Subject']['Data']) -# self.assertIn('City of Igorville', call_kwargs['Content']['Simple']['Body']['Text']['Data']) \ No newline at end of file + # Check email content + call_kwargs = mock_ses_client.send_email.call_args[1] + self.assertEqual(call_kwargs['FromEmailAddress'], settings.DEFAULT_FROM_EMAIL) + self.assertIn('DOMAIN: Igorville.gov', call_kwargs['Content']['Simple']['Subject']['Data']) + self.assertIn('INFORMATION UPDATED: Org Name/Address', call_kwargs['Content']['Simple']['Body']['Text']['Data']) \ No newline at end of file From 9de6831591f657d80487aeddb54bd2f4cebbda07 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 24 Sep 2024 10:48:23 -0500 Subject: [PATCH 23/49] tests --- src/registrar/tests/test_views_domain.py | 49 +++++++++++++++--------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 109b80913..43ea9bdd4 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -1978,36 +1978,47 @@ class TestDomainChangeNotifications(TestDomainOverview): AllowedEmail(email="info@example.com"), ] AllowedEmail.objects.bulk_create(allowed_emails) + + def setUp(self): + super().setUp() + self.mock_client_class = MagicMock() + self.mock_client = self.mock_client_class.return_value @classmethod def tearDownClass(cls): super().tearDownClass() AllowedEmail.objects.all().delete() + @boto3_mocking.patching def test_notification_email_sent_on_org_name_change(self): """Test that an email is sent when the organization name is changed.""" - with patch('registrar.utility.email.boto3.client') as mock_boto3_client: - mock_ses_client = mock_boto3_client.return_value - self.domain_information.organization_name = "Town of Igorville" - 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] + self.domain_information.organization_name = "Town of Igorville" + 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] - org_name_page.form["organization_name"] = "Not igorville" + org_name_page.form["organization_name"] = "Not igorville" - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): success_result_page = org_name_page.form.submit() # Check that the page loads successfully - self.assertEqual(success_result_page.status_code, 200) - self.assertContains(success_result_page, "Not igorville") + self.assertEqual(success_result_page.status_code, 200) + self.assertContains(success_result_page, "Not igorville") - # Check that an email was sent - mock_ses_client.send_email.assert_called_once() - - # Check email content - call_kwargs = mock_ses_client.send_email.call_args[1] - self.assertEqual(call_kwargs['FromEmailAddress'], settings.DEFAULT_FROM_EMAIL) - self.assertIn('DOMAIN: Igorville.gov', call_kwargs['Content']['Simple']['Subject']['Data']) - self.assertIn('INFORMATION UPDATED: Org Name/Address', call_kwargs['Content']['Simple']['Body']['Text']['Data']) \ No newline at end of file + # Check that an email was sent + self.assertTrue(self.mock_client.send_email.called) + # Check email content + # check the call sequence for the email + args, kwargs = self.mock_client.send_email.call_args + self.assertIn("Content", kwargs) + self.assertIn("Simple", kwargs["Content"]) + self.assertIn("Subject", kwargs["Content"]["Simple"]) + self.assertIn("Body", kwargs["Content"]["Simple"]) + + # check for things in the email content (not an exhaustive list) + body = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"] + + self.assertIn("DOMAIN:", body) From ab4024bab509b54670f413b3945d18c0cd36cfab Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 25 Sep 2024 15:24:21 -0500 Subject: [PATCH 24/49] all tests passing --- src/registrar/templates/domain_users.html | 6 +- src/registrar/tests/test_emails.py | 2 +- src/registrar/tests/test_views_domain.py | 231 +++++++++++++++++++++- src/registrar/views/domain.py | 18 +- 4 files changed, 237 insertions(+), 20 deletions(-) diff --git a/src/registrar/templates/domain_users.html b/src/registrar/templates/domain_users.html index 412f4ee73..1b789e590 100644 --- a/src/registrar/templates/domain_users.html +++ b/src/registrar/templates/domain_users.html @@ -8,8 +8,7 @@

Domain managers can update all information related to a domain within the - .gov registrar, including contact details, senior official, security - email, and DNS name servers. + .gov registrar, including, security email and DNS name servers.

    @@ -17,7 +16,8 @@
  • After adding a domain manager, an email invitation will be sent to that user with instructions on how to set up an account.
  • All domain managers must keep their contact information updated and be responsive if contacted by the .gov team.
    • -
  • Domains must have at least one domain manager. You can’t remove yourself as a domain manager if you’re the only one assigned to this domain. Add another domain manager before you remove yourself from this domain.
    • +
  • All domain managers will be notified when updates are made to this domain.
    • +
  • Domains must have at least one domain manager. You can’t remove yourself as a domain manager if you’re the only one assigned to this domain.
{% if domain.permissions %} diff --git a/src/registrar/tests/test_emails.py b/src/registrar/tests/test_emails.py index 87e2d551f..21ba06316 100644 --- a/src/registrar/tests/test_emails.py +++ b/src/registrar/tests/test_emails.py @@ -71,7 +71,7 @@ class TestEmails(TestCase): "doesnotexist@igorville.com", context={"domain_request": self}, bcc_address=None, - cc=["test_email1@example.com", "test_email2@example.com"] + cc_addresses=["test_email1@example.com", "test_email2@example.com"] ) # check that an email was sent diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 43ea9bdd4..d258dc472 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -8,6 +8,7 @@ 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 @@ -67,6 +68,10 @@ class TestWithDomainPermissions(TestWithUser): datetime.combine(date.today() + timedelta(days=1), datetime.min.time()) ), ) + self.domain_dns_needed, _ = Domain.objects.get_or_create( + name="dns-needed.gov", + state=Domain.State.DNS_NEEDED, + ) self.domain_deleted, _ = Domain.objects.get_or_create( name="deleted.gov", state=Domain.State.DELETED, @@ -85,6 +90,12 @@ class TestWithDomainPermissions(TestWithUser): self.domain_information, _ = DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain) + self.security_contact, _ = PublicContact.objects.get_or_create( + domain=self.domain, + contact_type=PublicContact.ContactTypeChoices.SECURITY, + email="security@igorville.gov", + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_dsdata) DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_multdsdata) DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_dnssec_none) @@ -93,6 +104,8 @@ class TestWithDomainPermissions(TestWithUser): DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_just_nameserver) DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_on_hold) 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 @@ -101,6 +114,9 @@ class TestWithDomainPermissions(TestWithUser): UserDomainRole.objects.get_or_create( user=self.user, domain=self.domain_dsdata, role=UserDomainRole.Roles.MANAGER ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=self.domain_dns_needed, role=UserDomainRole.Roles.MANAGER + ) UserDomainRole.objects.get_or_create( user=self.user, domain=self.domain_multdsdata, @@ -1976,6 +1992,7 @@ class TestDomainChangeNotifications(TestDomainOverview): super().setUpClass() allowed_emails = [ AllowedEmail(email="info@example.com"), + AllowedEmail(email="doesnotexist@igorville.com"), ] AllowedEmail.objects.bulk_create(allowed_emails) @@ -1990,10 +2007,15 @@ class TestDomainChangeNotifications(TestDomainOverview): AllowedEmail.objects.all().delete() @boto3_mocking.patching - def test_notification_email_sent_on_org_name_change(self): + @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})) @@ -2003,22 +2025,215 @@ class TestDomainChangeNotifications(TestDomainOverview): self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): - success_result_page = org_name_page.form.submit() - # Check that the page loads successfully - self.assertEqual(success_result_page.status_code, 200) - self.assertContains(success_result_page, "Not igorville") + org_name_page.form.submit() # Check that an email was sent self.assertTrue(self.mock_client.send_email.called) + # Check email content # check the call sequence for the email - args, kwargs = self.mock_client.send_email.call_args + _, kwargs = self.mock_client.send_email.call_args self.assertIn("Content", kwargs) self.assertIn("Simple", kwargs["Content"]) self.assertIn("Subject", kwargs["Content"]["Simple"]) self.assertIn("Body", kwargs["Content"]["Simple"]) - # check for things in the email content (not an exhaustive list) body = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"] - self.assertIn("DOMAIN:", body) + self.assertIn("DOMAIN: igorville.gov", body) + self.assertIn("UPDATED BY: First Last info@example.com", body) + self.assertIn("INFORMATION UPDATED: Org Name/Address", body) + + @boto3_mocking.patching + @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" + 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.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] + + org_name_page.form["organization_name"] = "Not igorville" + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): + org_name_page.form.submit() + + # Check that an email was not sent + 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] + + security_email_page.form["security_email"] = "new_security@example.com" + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): + security_email_page.form.submit() + + self.assertTrue(self.mock_client.send_email.called) + + _, kwargs = self.mock_client.send_email.call_args + body = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"] + + self.assertIn("DOMAIN: igorville.gov", body) + self.assertIn("UPDATED BY: First Last info@example.com", body) + self.assertIn("INFORMATION UPDATED: Security Email", body) + + @boto3_mocking.patching + @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") + + # Prepare the data for the POST request + post_data = { + "disable_dnssec": "Disable DNSSEC", + } + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): + updated_page = self.client.post( + reverse("domain-dns-dnssec", kwargs={"pk": self.domain.id}), + post_data, + follow=True, + ) + + self.assertEqual(updated_page.status_code, 200) + + self.assertContains(updated_page, "Enable DNSSEC") + + self.assertTrue(self.mock_client.send_email.called) + + _, kwargs = self.mock_client.send_email.call_args + body = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"] + + self.assertIn("DOMAIN: igorville.gov", body) + self.assertIn("UPDATED BY: First Last info@example.com", body) + self.assertIn("INFORMATION UPDATED: DNSSec", body) + + @boto3_mocking.patching + @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] + + # Add DS data + ds_data_page.forms[0]["form-0-key_tag"] = "12345" + 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) + + # check some stuff about the email + _, kwargs = self.mock_client.send_email.call_args + body = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"] + + self.assertIn("DOMAIN: igorville.gov", body) + self.assertIn("UPDATED BY: First Last info@example.com", body) + self.assertIn("INFORMATION UPDATED: DS Data", body) + + @boto3_mocking.patching + @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] + + senior_official_page.form["first_name"] = "New" + senior_official_page.form["last_name"] = "Official" + senior_official_page.form["title"] = "Director" + senior_official_page.form["email"] = "new_official@example.com" + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): + senior_official_page.form.submit() + + self.assertTrue(self.mock_client.send_email.called) + + _, kwargs = self.mock_client.send_email.call_args + body = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"] + + self.assertIn("DOMAIN: igorville.gov", body) + self.assertIn("UPDATED BY: First Last info@example.com", body) + self.assertIn("INFORMATION UPDATED: Senior Official", body) + + @boto3_mocking.patching + @less_console_noise_decorator + 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( + 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] + + senior_official_page.form["first_name"] = "New" + senior_official_page.form["last_name"] = "Official" + senior_official_page.form["title"] = "Director" + senior_official_page.form["email"] = "new_official@example.com" + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): + 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] + + # add nameservers + nameservers_page.form["form-0-server"] = "ns1-new.igorville.gov" + nameservers_page.form["form-0-ip"] = "192.168.1.1" + nameservers_page.form["form-1-server"] = "ns2-new.igorville.gov" + nameservers_page.form["form-1-ip"] = "192.168.1.2" + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): + nameservers_page.form.submit() + + # Check that an email was not sent + self.assertFalse(self.mock_client.send_email.called) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 04b740f6e..22ece989b 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -119,6 +119,7 @@ class DomainFormBaseView(DomainBaseView, FormMixin): if form.is_valid(): return self.form_valid(form) else: + logger.debug(f"Form errors: {form.errors}") return self.form_invalid(form) def form_valid(self, form): @@ -164,6 +165,7 @@ class DomainFormBaseView(DomainBaseView, FormMixin): DomainDsdataFormset: "DS Data", DomainOrgNameAddressForm: "Org Name/Address", SeniorOfficialContactForm: "Senior Official", + NameserverFormset: "Nameservers", } # forms of these types should not send notifications if they're part of a portfolio/Organization @@ -179,14 +181,13 @@ class DomainFormBaseView(DomainBaseView, FormMixin): # 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 else: # don't notify for any other types of forms should_notify=False - + logger.info(f"Not notifying for {form.__class__}") if (should_notify and form.has_changed()) or force_send: - logger.info("Sending email to domain managers") - context={ "domain": self.object.name, "user": self.request.user, @@ -194,6 +195,8 @@ class DomainFormBaseView(DomainBaseView, FormMixin): "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}") def email_domain_managers(self, domain_name, template: str, subject_template: str, context: any = {}): """Send a single email built from a template to all managers for a given domain. @@ -489,8 +492,7 @@ class DomainNameserversView(DomainFormBaseView): This post method harmonizes using DomainBaseView and FormMixin """ - - logger.info("Posted to Namservers View") + logger.info(f"POST request to DomainNameserversView") self._get_domain(request) formset = self.get_form() @@ -500,6 +502,7 @@ 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) @@ -507,8 +510,6 @@ class DomainNameserversView(DomainFormBaseView): def form_valid(self, formset): """The formset is valid, perform something with it.""" - logger.info("------ Nameserver Form is valid -------") - self.request.session["nameservers_form_domain"] = self.object # Set the nameservers from the formset @@ -550,7 +551,8 @@ class DomainNameserversView(DomainFormBaseView): messages.error(self.request, NameserverError(code=nsErrorCodes.BAD_DATA)) logger.error(f"Registry error: {Err}") else: - self.send_update_notification(formset) + if self.object.state == Domain.State.READY: + self.send_update_notification(formset) messages.success( self.request, "The name servers for this domain have been updated. " From 43b48edf3639c22bd3a29439c1a23636671a8296 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 25 Sep 2024 15:31:27 -0500 Subject: [PATCH 25/49] linter errors --- src/registrar/config/settings.py | 2 +- src/registrar/tests/test_emails.py | 18 +++---- src/registrar/tests/test_views_domain.py | 60 +++++++++++----------- src/registrar/utility/email.py | 17 ++++--- src/registrar/views/domain.py | 63 ++++++++++++------------ 5 files changed, 78 insertions(+), 82 deletions(-) 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) From 7a4ed7ab059e6ae6688fcdc84617b7852557b46e Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 25 Sep 2024 15:44:49 -0500 Subject: [PATCH 26/49] minor email refactor --- src/registrar/utility/email.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 655c432ac..fc7d4f956 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -76,7 +76,7 @@ def send_templated_email( # noqa aws_secret_access_key=settings.AWS_SECRET_ACCESS_KEY, config=settings.BOTO_CONFIG, ) - logger.info(f"An email was sent! Template name: {template_name} to {to_address}") + logger.info(f"Connected to SES client! Template name: {template_name} to {to_address}") except Exception as exc: logger.debug("E-mail unable to send! Could not access the SES client.") raise EmailSendingError("Could not access the SES client.") from exc @@ -153,14 +153,12 @@ def get_sendable_addresses(addresses: list[str]) -> tuple[list[str], list[str]]: Paramaters: addresses: a list of strings representing all addresses to be checked. - - raises: - EmailSendingError if email sending is disabled """ 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) + logger.warning(message) + return ([],[]) else: AllowedEmail = apps.get_model("registrar", "AllowedEmail") allowed_emails = [] From 0c9db26a575fcae2ec5f9b239e5bd69ebe66e33f Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 25 Sep 2024 16:05:12 -0500 Subject: [PATCH 27/49] fix email test --- src/registrar/tests/test_emails.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/tests/test_emails.py b/src/registrar/tests/test_emails.py index abbbb274f..c3a84d22f 100644 --- a/src/registrar/tests/test_emails.py +++ b/src/registrar/tests/test_emails.py @@ -66,10 +66,10 @@ class TestEmails(TestCase): """Test sending email with cc works""" with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): send_templated_email( - "test content", - "test subject", + "emails/update_to_approved_domain.txt", + "emails/update_to_approved_domain_subject.txt", "doesnotexist@igorville.com", - context={"domain_request": self}, + context={"domain": "test", "user": "test", "date": 1, "changes": "test"}, bcc_address=None, cc_addresses=["test_email1@example.com", "test_email2@example.com"], ) From 2e3ff9e547a11300b8719d8bf5bf442b7620ec87 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 26 Sep 2024 10:58:39 -0500 Subject: [PATCH 28/49] fix tests --- src/registrar/models/domain_request.py | 6 ++-- src/registrar/tests/test_emails.py | 46 +++++++++++++++++------- src/registrar/tests/test_models.py | 8 ++--- src/registrar/tests/test_views_domain.py | 2 ++ src/registrar/utility/email.py | 6 ++-- src/registrar/views/domain.py | 2 +- 6 files changed, 48 insertions(+), 22 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 161d85ae5..fdba309f3 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -781,7 +781,7 @@ class DomainRequest(TimeStampedModel): if custom_email_content: context["custom_email_content"] = custom_email_content - + logger.info(f"Sending email to: {recipient.email}") send_templated_email( email_template, email_template_subject, @@ -823,11 +823,12 @@ class DomainRequest(TimeStampedModel): # requested_domain could be None here if not hasattr(self, "requested_domain") or self.requested_domain is None: raise ValueError("Requested domain is missing.") + logger.info(f"Submitting domain request: {self.requested_domain.name}") DraftDomain = apps.get_model("registrar.DraftDomain") if not DraftDomain.string_could_be_domain(self.requested_domain.name): raise ValueError("Requested domain is not a valid domain name.") - + logger.info(f"Draft Domain") # if the domain has not been submitted before this must be the first time if not self.first_submitted_date: self.first_submitted_date = timezone.now().date() @@ -835,6 +836,7 @@ class DomainRequest(TimeStampedModel): # Update last_submitted_date to today self.last_submitted_date = timezone.now().date() self.save() + logger.info(f"updated submission date") # Limit email notifications to transitions from Started and Withdrawn limited_statuses = [self.DomainRequestStatus.STARTED, self.DomainRequestStatus.WITHDRAWN] diff --git a/src/registrar/tests/test_emails.py b/src/registrar/tests/test_emails.py index c3a84d22f..3b1b45e98 100644 --- a/src/registrar/tests/test_emails.py +++ b/src/registrar/tests/test_emails.py @@ -71,7 +71,7 @@ class TestEmails(TestCase): "doesnotexist@igorville.com", context={"domain": "test", "user": "test", "date": 1, "changes": "test"}, bcc_address=None, - cc_addresses=["test_email1@example.com", "test_email2@example.com"], + cc_addresses=["testy2@town.com", "mayor@igorville.gov"], ) # check that an email was sent @@ -81,7 +81,7 @@ class TestEmails(TestCase): @less_console_noise_decorator def test_submission_confirmation(self): """Submission confirmation email works.""" - domain_request = completed_domain_request() + domain_request = completed_domain_request(user=User.objects.create(username="test", email="testy@town.com")) with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): domain_request.submit() @@ -118,7 +118,9 @@ class TestEmails(TestCase): @less_console_noise_decorator def test_submission_confirmation_no_current_website_spacing(self): """Test line spacing without current_website.""" - domain_request = completed_domain_request(has_current_website=False) + domain_request = completed_domain_request( + has_current_website=False, user=User.objects.create(username="test", email="testy@town.com") + ) with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): domain_request.submit() _, kwargs = self.mock_client.send_email.call_args @@ -131,7 +133,9 @@ class TestEmails(TestCase): @less_console_noise_decorator def test_submission_confirmation_current_website_spacing(self): """Test line spacing with current_website.""" - domain_request = completed_domain_request(has_current_website=True) + domain_request = completed_domain_request( + has_current_website=True, user=User.objects.create(username="test", email="testy@town.com") + ) with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): domain_request.submit() _, kwargs = self.mock_client.send_email.call_args @@ -148,7 +152,11 @@ class TestEmails(TestCase): # Create fake creator _creator = User.objects.create( - username="MrMeoward", first_name="Meoward", last_name="Jones", phone="(888) 888 8888" + username="MrMeoward", + first_name="Meoward", + last_name="Jones", + phone="(888) 888 8888", + email="testy@town.com", ) # Create a fake domain request @@ -165,7 +173,9 @@ class TestEmails(TestCase): @less_console_noise_decorator def test_submission_confirmation_no_other_contacts_spacing(self): """Test line spacing without other contacts.""" - domain_request = completed_domain_request(has_other_contacts=False) + domain_request = completed_domain_request( + has_other_contacts=False, user=User.objects.create(username="test", email="testy@town.com") + ) with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): domain_request.submit() _, kwargs = self.mock_client.send_email.call_args @@ -177,7 +187,9 @@ class TestEmails(TestCase): @less_console_noise_decorator def test_submission_confirmation_alternative_govdomain_spacing(self): """Test line spacing with alternative .gov domain.""" - domain_request = completed_domain_request(has_alternative_gov_domain=True) + domain_request = completed_domain_request( + has_alternative_gov_domain=True, user=User.objects.create(username="test", email="testy@town.com") + ) with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): domain_request.submit() _, kwargs = self.mock_client.send_email.call_args @@ -190,7 +202,9 @@ class TestEmails(TestCase): @less_console_noise_decorator def test_submission_confirmation_no_alternative_govdomain_spacing(self): """Test line spacing without alternative .gov domain.""" - domain_request = completed_domain_request(has_alternative_gov_domain=False) + domain_request = completed_domain_request( + has_alternative_gov_domain=False, user=User.objects.create(username="test", email="testy@town.com") + ) with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): domain_request.submit() _, kwargs = self.mock_client.send_email.call_args @@ -203,7 +217,9 @@ class TestEmails(TestCase): @less_console_noise_decorator def test_submission_confirmation_about_your_organization_spacing(self): """Test line spacing with about your organization.""" - domain_request = completed_domain_request(has_about_your_organization=True) + domain_request = completed_domain_request( + has_about_your_organization=True, user=User.objects.create(username="test", email="testy@town.com") + ) with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): domain_request.submit() _, kwargs = self.mock_client.send_email.call_args @@ -216,7 +232,9 @@ class TestEmails(TestCase): @less_console_noise_decorator def test_submission_confirmation_no_about_your_organization_spacing(self): """Test line spacing without about your organization.""" - domain_request = completed_domain_request(has_about_your_organization=False) + domain_request = completed_domain_request( + has_about_your_organization=False, user=User.objects.create(username="test", email="testy@town.com") + ) with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): domain_request.submit() _, kwargs = self.mock_client.send_email.call_args @@ -229,7 +247,9 @@ class TestEmails(TestCase): @less_console_noise_decorator def test_submission_confirmation_anything_else_spacing(self): """Test line spacing with anything else.""" - domain_request = completed_domain_request(has_anything_else=True) + domain_request = completed_domain_request( + has_anything_else=True, user=User.objects.create(username="test", email="testy@town.com") + ) with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): domain_request.submit() _, kwargs = self.mock_client.send_email.call_args @@ -241,7 +261,9 @@ class TestEmails(TestCase): @less_console_noise_decorator def test_submission_confirmation_no_anything_else_spacing(self): """Test line spacing without anything else.""" - domain_request = completed_domain_request(has_anything_else=False) + domain_request = completed_domain_request( + has_anything_else=False, user=User.objects.create(username="test", email="testy@town.com") + ) with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): domain_request.submit() _, kwargs = self.mock_client.send_email.call_args diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index a6e889503..8c9a888c2 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -268,7 +268,7 @@ class TestDomainRequest(TestCase): @less_console_noise_decorator def test_submit_from_withdrawn_sends_email(self): msg = "Create a withdrawn domain request and submit it and see if email was sent." - user, _ = User.objects.get_or_create(username="testy") + user, _ = User.objects.get_or_create(username="testy", email="testy@town.com") domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.WITHDRAWN, user=user) self.check_email_sent(domain_request, msg, "submit", 1, expected_content="Hi", expected_email=user.email) @@ -287,14 +287,14 @@ class TestDomainRequest(TestCase): @less_console_noise_decorator def test_approve_sends_email(self): msg = "Create a domain request and approve it and see if email was sent." - user, _ = User.objects.get_or_create(username="testy") + user, _ = User.objects.get_or_create(username="testy", email="testy@town.com") domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=user) self.check_email_sent(domain_request, msg, "approve", 1, expected_content="approved", expected_email=user.email) @less_console_noise_decorator def test_withdraw_sends_email(self): msg = "Create a domain request and withdraw it and see if email was sent." - user, _ = User.objects.get_or_create(username="testy") + user, _ = User.objects.get_or_create(username="testy", email="testy@town.com") domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=user) self.check_email_sent( domain_request, msg, "withdraw", 1, expected_content="withdrawn", expected_email=user.email @@ -303,7 +303,7 @@ class TestDomainRequest(TestCase): @less_console_noise_decorator def test_reject_sends_email(self): msg = "Create a domain request and reject it and see if email was sent." - user, _ = User.objects.get_or_create(username="testy") + user, _ = User.objects.get_or_create(username="testy", email="testy@town.com") domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.APPROVED, user=user) self.check_email_sent(domain_request, msg, "reject", 1, expected_content="Hi", expected_email=user.email) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 6ca96f8c0..7e9f5b9b8 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -250,6 +250,7 @@ class TestDomainDetail(TestDomainOverview): # At the time of this test's writing, there are 6 UNKNOWN domains inherited # from constructors. Let's reset. with less_console_noise(): + PublicContact.objects.all().delete() Domain.objects.all().delete() UserDomainRole.objects.all().delete() @@ -2002,6 +2003,7 @@ class TestDomainChangeNotifications(TestDomainOverview): super().tearDownClass() AllowedEmail.objects.all().delete() + @boto3_mocking.patching @less_console_noise_decorator def test_notification_on_org_name_change(self): diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index fc7d4f956..412838d10 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -34,7 +34,7 @@ def send_templated_email( # noqa ): """Send an email built from a template. - to_address and bcc_address currently only supports a single address. + to_address and bcc_address currently only support single addresses. cc_address is a list and can contain many addresses. Emails not in the whitelist (if applicable) will be filtered out before sending. @@ -111,7 +111,7 @@ def send_templated_email( # noqa }, }, ) - logger.info("Email sent to %s, bcc %s, cc %s", to_address, bcc_address, cc_addresses) + logger.info("Email sent to %s, bcc %s, cc %s", to_address, bcc_address, sendable_cc_addresses) else: ses_client = boto3.client( "ses", @@ -158,7 +158,7 @@ def get_sendable_addresses(addresses: list[str]) -> tuple[list[str], list[str]]: 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'." logger.warning(message) - return ([],[]) + return ([], []) else: AllowedEmail = apps.get_model("registrar", "AllowedEmail") allowed_emails = [] diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index a3f9d153c..9f6662291 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -413,7 +413,7 @@ class DomainSeniorOfficialView(DomainFormBaseView): # Set the domain information in the form so that it can be accessible # to associate a new Contact, if a new Contact is needed - # in the save() methodS + # in the save() method form.set_domain_info(self.object.domain_info) form.save() From 7d9eabb7271be811b79874e8421859592bb4d03b Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 26 Sep 2024 11:20:18 -0500 Subject: [PATCH 29/49] linter errors --- src/registrar/models/domain_request.py | 2 -- src/registrar/tests/test_views_domain.py | 1 - 2 files changed, 3 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index fdba309f3..08a431967 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -828,7 +828,6 @@ class DomainRequest(TimeStampedModel): DraftDomain = apps.get_model("registrar.DraftDomain") if not DraftDomain.string_could_be_domain(self.requested_domain.name): raise ValueError("Requested domain is not a valid domain name.") - logger.info(f"Draft Domain") # if the domain has not been submitted before this must be the first time if not self.first_submitted_date: self.first_submitted_date = timezone.now().date() @@ -836,7 +835,6 @@ class DomainRequest(TimeStampedModel): # Update last_submitted_date to today self.last_submitted_date = timezone.now().date() self.save() - logger.info(f"updated submission date") # Limit email notifications to transitions from Started and Withdrawn limited_statuses = [self.DomainRequestStatus.STARTED, self.DomainRequestStatus.WITHDRAWN] diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 7e9f5b9b8..939cdaaf9 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -2003,7 +2003,6 @@ class TestDomainChangeNotifications(TestDomainOverview): super().tearDownClass() AllowedEmail.objects.all().delete() - @boto3_mocking.patching @less_console_noise_decorator def test_notification_on_org_name_change(self): From 65d89872f2b49a9ce796a5ec8295342cfce55bac Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Thu, 26 Sep 2024 16:43:03 -0500 Subject: [PATCH 30/49] remove extra logging statements --- src/registrar/models/domain_request.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 08a431967..2aecb49f7 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -781,7 +781,6 @@ class DomainRequest(TimeStampedModel): if custom_email_content: context["custom_email_content"] = custom_email_content - logger.info(f"Sending email to: {recipient.email}") send_templated_email( email_template, email_template_subject, @@ -823,7 +822,6 @@ class DomainRequest(TimeStampedModel): # requested_domain could be None here if not hasattr(self, "requested_domain") or self.requested_domain is None: raise ValueError("Requested domain is missing.") - logger.info(f"Submitting domain request: {self.requested_domain.name}") DraftDomain = apps.get_model("registrar.DraftDomain") if not DraftDomain.string_could_be_domain(self.requested_domain.name): From 9940b1657e983828f937255117537ffb9fb3dacb Mon Sep 17 00:00:00 2001 From: Matt-Spence Date: Wed, 2 Oct 2024 11:26:12 -0500 Subject: [PATCH 31/49] Update src/registrar/utility/email.py Co-authored-by: zandercymatics <141044360+zandercymatics@users.noreply.github.com> --- src/registrar/utility/email.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 412838d10..ecae7ed93 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -91,7 +91,7 @@ def send_templated_email( # noqa # make sure we don't try and send an email to nowhere if not destination: - message = "E-mail unable to send, no valid recipients provided." + message = "Email unable to send, no valid recipients provided." raise EmailSendingError(message) try: From f26db7185b3a1a01822ccaa5bce684b131ab2c43 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 2 Oct 2024 11:54:29 -0500 Subject: [PATCH 32/49] review changes --- src/registrar/tests/test_emails.py | 31 ++++++++++++++++++++++++ src/registrar/tests/test_views_domain.py | 8 ++---- src/registrar/utility/email.py | 13 ++++++++-- 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/src/registrar/tests/test_emails.py b/src/registrar/tests/test_emails.py index 3b1b45e98..e76a6124f 100644 --- a/src/registrar/tests/test_emails.py +++ b/src/registrar/tests/test_emails.py @@ -77,6 +77,37 @@ class TestEmails(TestCase): # check that an email was sent self.assertTrue(self.mock_client.send_email.called) + # check the call sequence for the email + args, kwargs = self.mock_client.send_email.call_args + self.assertIn("Destination", kwargs) + self.assertIn("CcAddresses", kwargs["Destination"]) + + self.assertEqual(["testy2@town.com", "mayor@igorville.gov"], kwargs["Destination"]["CcAddresses"]) + + @boto3_mocking.patching + @override_settings(IS_PRODUCTION=True) + def test_email_with_cc_in_prod(self): + """Test sending email with cc works in prod""" + with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): + send_templated_email( + "emails/update_to_approved_domain.txt", + "emails/update_to_approved_domain_subject.txt", + "doesnotexist@igorville.com", + context={"domain": "test", "user": "test", "date": 1, "changes": "test"}, + bcc_address=None, + cc_addresses=["testy2@town.com", "mayor@igorville.gov"], + ) + + # check that an email was sent + self.assertTrue(self.mock_client.send_email.called) + + # check the call sequence for the email + args, kwargs = self.mock_client.send_email.call_args + self.assertIn("Destination", kwargs) + self.assertIn("CcAddresses", kwargs["Destination"]) + + self.assertEqual(["testy2@town.com", "mayor@igorville.gov"], kwargs["Destination"]["CcAddresses"]) + @boto3_mocking.patching @less_console_noise_decorator def test_submission_confirmation(self): diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 939cdaaf9..15e21169e 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -87,12 +87,6 @@ class TestWithDomainPermissions(TestWithUser): self.domain_information, _ = DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain) - self.security_contact, _ = PublicContact.objects.get_or_create( - domain=self.domain, - contact_type=PublicContact.ContactTypeChoices.SECURITY, - email="security@igorville.gov", - ) - DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_dsdata) DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_multdsdata) DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_dnssec_none) @@ -2007,6 +2001,8 @@ 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.""" + # We may end up sending emails on org name changes later, but it will be addressed + # in the portfolio itself, rather than the individual domain. self.domain_information.organization_name = "Town of Igorville" self.domain_information.address_line1 = "123 Main St" diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 412838d10..6c5f5f172 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -48,14 +48,19 @@ def send_templated_email( # noqa No valid recipient addresses are provided """ + # by default assume we can send to all addresses (prod has no whitelist) + sendable_cc_addresses = cc_addresses + 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) + + # if we're not in prod, we need to check the whitelist for CC'ed addresses sendable_cc_addresses, blocked_cc_addresses = get_sendable_addresses(cc_addresses) - if len(sendable_cc_addresses) < len(cc_addresses): + if blocked_cc_addresses: logger.warning("Some CC'ed addresses were removed: %s.", blocked_cc_addresses) template = get_template(template_name) @@ -111,7 +116,7 @@ def send_templated_email( # noqa }, }, ) - logger.info("Email sent to %s, bcc %s, cc %s", to_address, bcc_address, sendable_cc_addresses) + logger.info("Email sent to [%s], bcc [%s], cc %s", to_address, bcc_address, sendable_cc_addresses) else: ses_client = boto3.client( "ses", @@ -123,6 +128,10 @@ def send_templated_email( # noqa send_email_with_attachment( settings.DEFAULT_FROM_EMAIL, to_address, subject, email_body, attachment_file, ses_client ) + logger.info( + "Email with attachment sent to [%s], bcc [%s], cc %s", to_address, bcc_address, sendable_cc_addresses + ) + except Exception as exc: raise EmailSendingError("Could not send SES email.") from exc From a42ea0a19133ed61af7cc146928d4b0d16a9ead9 Mon Sep 17 00:00:00 2001 From: Matt-Spence Date: Wed, 2 Oct 2024 15:13:31 -0500 Subject: [PATCH 33/49] Update src/registrar/views/domain.py Co-authored-by: zandercymatics <141044360+zandercymatics@users.noreply.github.com> --- src/registrar/views/domain.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 9f6662291..88da7320a 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -226,7 +226,6 @@ class DomainFormBaseView(DomainBaseView, FormMixin): "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: From 50ca20fe576001f924d3c617c100733642687772 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 2 Oct 2024 15:15:59 -0500 Subject: [PATCH 34/49] minor fixes --- src/registrar/views/domain.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 9f6662291..206509e07 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -204,7 +204,7 @@ class DomainFormBaseView(DomainBaseView, FormMixin): 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={}): + def email_domain_managers(self, domain: Domain, 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 @@ -216,12 +216,6 @@ class DomainFormBaseView(DomainBaseView, FormMixin): Will log a warning if the email fails to send for any reason, but will not raise an error. """ - try: - 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 - ) manager_pks = UserDomainRole.objects.filter(domain=domain.pk, role=UserDomainRole.Roles.MANAGER).values_list( "user", flat=True ) @@ -233,7 +227,7 @@ class DomainFormBaseView(DomainBaseView, FormMixin): logger.warning( "Could not sent notification email to %s for domain %s", emails, - domain_name, + domain.name, exc_info=True, ) From 96d9fcd25205beaeb7b727b3a97e70ea3d23a0d6 Mon Sep 17 00:00:00 2001 From: Matt-Spence Date: Fri, 4 Oct 2024 16:44:02 -0500 Subject: [PATCH 35/49] Update src/registrar/templates/domain_users.html Co-authored-by: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> --- src/registrar/templates/domain_users.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/domain_users.html b/src/registrar/templates/domain_users.html index 1b789e590..a2eb3e604 100644 --- a/src/registrar/templates/domain_users.html +++ b/src/registrar/templates/domain_users.html @@ -8,7 +8,7 @@

Domain managers can update all information related to a domain within the - .gov registrar, including, security email and DNS name servers. + .gov registrar, including security email and DNS name servers.

    From 4c3d29399d456879335db27644ffe4fdaa64d0de Mon Sep 17 00:00:00 2001 From: Matt-Spence Date: Fri, 4 Oct 2024 16:44:26 -0500 Subject: [PATCH 36/49] Update src/registrar/templates/emails/update_to_approved_domain.txt Co-authored-by: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> --- src/registrar/templates/emails/update_to_approved_domain.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/templates/emails/update_to_approved_domain.txt b/src/registrar/templates/emails/update_to_approved_domain.txt index bc0950808..338b7d811 100644 --- a/src/registrar/templates/emails/update_to_approved_domain.txt +++ b/src/registrar/templates/emails/update_to_approved_domain.txt @@ -20,6 +20,7 @@ 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 From aeebe8c997d6205da029e523ae4133edd6257925 Mon Sep 17 00:00:00 2001 From: Matt-Spence Date: Fri, 4 Oct 2024 16:44:42 -0500 Subject: [PATCH 37/49] Update src/registrar/templates/emails/update_to_approved_domain.txt Co-authored-by: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> --- src/registrar/templates/emails/update_to_approved_domain.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/templates/emails/update_to_approved_domain.txt b/src/registrar/templates/emails/update_to_approved_domain.txt index 338b7d811..3ff1e05f2 100644 --- a/src/registrar/templates/emails/update_to_approved_domain.txt +++ b/src/registrar/templates/emails/update_to_approved_domain.txt @@ -12,6 +12,7 @@ You can view this update in the .gov registrar . Get help with managing your .gov domain . ---------------------------------------------------------------- + WHY DID YOU RECEIVE THIS EMAIL? You’re listed as a domain manager for {{domain}}, so you’ll receive a notification whenever changes are made to that domain. If you have questions or concerns, reach out to the person who made the change or reply to this email. From 35ea584a8f3a3b78ff1d3d815181c2f34207acf7 Mon Sep 17 00:00:00 2001 From: Matt-Spence Date: Fri, 4 Oct 2024 16:44:54 -0500 Subject: [PATCH 38/49] Update src/registrar/templates/emails/update_to_approved_domain.txt Co-authored-by: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> --- src/registrar/templates/emails/update_to_approved_domain.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/templates/emails/update_to_approved_domain.txt b/src/registrar/templates/emails/update_to_approved_domain.txt index 3ff1e05f2..7af71875b 100644 --- a/src/registrar/templates/emails/update_to_approved_domain.txt +++ b/src/registrar/templates/emails/update_to_approved_domain.txt @@ -8,7 +8,6 @@ UPDATED ON: {{date}} INFORMATION UPDATED: {{changes}} You can view this update in the .gov registrar . - Get help with managing your .gov domain . ---------------------------------------------------------------- From 83051ae6237f0e9e7081806dff63bfeda3301e18 Mon Sep 17 00:00:00 2001 From: Matt-Spence Date: Fri, 4 Oct 2024 16:45:02 -0500 Subject: [PATCH 39/49] Update src/registrar/templates/emails/update_to_approved_domain.txt Co-authored-by: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> --- src/registrar/templates/emails/update_to_approved_domain.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/templates/emails/update_to_approved_domain.txt b/src/registrar/templates/emails/update_to_approved_domain.txt index 7af71875b..6e5e9c521 100644 --- a/src/registrar/templates/emails/update_to_approved_domain.txt +++ b/src/registrar/templates/emails/update_to_approved_domain.txt @@ -6,6 +6,7 @@ DOMAIN: {{domain}} UPDATED BY: {{user}} UPDATED ON: {{date}} INFORMATION UPDATED: {{changes}} + You can view this update in the .gov registrar . Get help with managing your .gov domain . From 07d640dddca44a8e80ec1eef2ced43561f55241d Mon Sep 17 00:00:00 2001 From: Matt-Spence Date: Fri, 4 Oct 2024 16:45:09 -0500 Subject: [PATCH 40/49] Update src/registrar/templates/emails/update_to_approved_domain.txt Co-authored-by: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> --- src/registrar/templates/emails/update_to_approved_domain.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/templates/emails/update_to_approved_domain.txt b/src/registrar/templates/emails/update_to_approved_domain.txt index 6e5e9c521..8e615c30c 100644 --- a/src/registrar/templates/emails/update_to_approved_domain.txt +++ b/src/registrar/templates/emails/update_to_approved_domain.txt @@ -2,6 +2,7 @@ Hi, An update was made to a domain you manage. + DOMAIN: {{domain}} UPDATED BY: {{user}} UPDATED ON: {{date}} From 7646c91691af58231804499bd16ffe087772e8c3 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Mon, 7 Oct 2024 12:16:21 -0500 Subject: [PATCH 41/49] design review changes --- src/registrar/tests/test_views_domain.py | 12 ++++----- src/registrar/views/domain.py | 33 +++++++++++++++++------- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 15e21169e..3aea6d9e1 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -2035,7 +2035,7 @@ class TestDomainChangeNotifications(TestDomainOverview): self.assertIn("DOMAIN: igorville.gov", body) self.assertIn("UPDATED BY: First Last info@example.com", body) - self.assertIn("INFORMATION UPDATED: Org Name/Address", body) + self.assertIn("INFORMATION UPDATED: Organization details", body) @boto3_mocking.patching @less_console_noise_decorator @@ -2085,7 +2085,7 @@ class TestDomainChangeNotifications(TestDomainOverview): self.assertIn("DOMAIN: igorville.gov", body) self.assertIn("UPDATED BY: First Last info@example.com", body) - self.assertIn("INFORMATION UPDATED: Security Email", body) + self.assertIn("INFORMATION UPDATED: Security email", body) @boto3_mocking.patching @less_console_noise_decorator @@ -2147,7 +2147,7 @@ class TestDomainChangeNotifications(TestDomainOverview): self.assertIn("DOMAIN: igorville.gov", body) self.assertIn("UPDATED BY: First Last info@example.com", body) - self.assertIn("INFORMATION UPDATED: DS Data", body) + self.assertIn("INFORMATION UPDATED: DNSSEC / DS Data", body) @boto3_mocking.patching @less_console_noise_decorator @@ -2178,7 +2178,7 @@ class TestDomainChangeNotifications(TestDomainOverview): self.assertIn("DOMAIN: igorville.gov", body) self.assertIn("UPDATED BY: First Last info@example.com", body) - self.assertIn("INFORMATION UPDATED: Senior Official", body) + self.assertIn("INFORMATION UPDATED: Senior official", body) @boto3_mocking.patching @less_console_noise_decorator @@ -2219,9 +2219,9 @@ class TestDomainChangeNotifications(TestDomainOverview): session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] # add nameservers - nameservers_page.form["form-0-server"] = "ns1-new.igorville.gov" + nameservers_page.form["form-0-server"] = "ns1-new.dns-needed.gov" nameservers_page.form["form-0-ip"] = "192.168.1.1" - nameservers_page.form["form-1-server"] = "ns2-new.igorville.gov" + nameservers_page.form["form-1-server"] = "ns2-new.dns-needed.gov" nameservers_page.form["form-1-ip"] = "192.168.1.2" self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 206509e07..7bd5834c0 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -119,7 +119,6 @@ class DomainFormBaseView(DomainBaseView, FormMixin): if form.is_valid(): return self.form_valid(form) else: - logger.debug(f"Form errors: {form.errors}") return self.form_invalid(form) def form_valid(self, form): @@ -160,12 +159,12 @@ class DomainFormBaseView(DomainBaseView, FormMixin): # 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", - SeniorOfficialContactForm: "Senior Official", - NameserverFormset: "Nameservers", + DomainDsdataFormset: "DNSSEC / DS Data", + DomainOrgNameAddressForm: "Organization details", + SeniorOfficialContactForm: "Senior official", + NameserverFormset: "Name servers", } # forms of these types should not send notifications if they're part of a portfolio/Organization @@ -182,6 +181,9 @@ class DomainFormBaseView(DomainBaseView, FormMixin): info = self.get_domain_info_from_domain() if not info or info.portfolio: should_notify = False + elif "analyst_action" in self.session and "analyst_action_location" in self.session: + # action is being made by an analyst + should_notify = False else: # don't notify for any other types of forms should_notify = False @@ -220,9 +222,16 @@ class DomainFormBaseView(DomainBaseView, FormMixin): "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) + # Remove the current user so they aren't CC'ed, since they will be the "to_address" + emails.remove(self.request.user.email) + except ValueError: + pass + + try: + send_templated_email( + template, subject_template, to_address=self.request.user.email, context=context, cc_addresses=emails + ) except EmailSendingError: logger.warning( "Could not sent notification email to %s for domain %s", @@ -491,19 +500,25 @@ class DomainNameserversView(DomainFormBaseView): self._get_domain(request) formset = self.get_form() + logger.debug("got formet") + if "btn-cancel-click" in request.POST: url = self.get_success_url() return HttpResponseRedirect(url) if formset.is_valid(): + logger.debug("formset is valid") return self.form_valid(formset) else: + logger.debug("formset is invalid") + logger.debug(formset.errors) 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 + initial_state = self.object.state # Set the nameservers from the formset nameservers = [] @@ -544,7 +559,7 @@ class DomainNameserversView(DomainFormBaseView): messages.error(self.request, NameserverError(code=nsErrorCodes.BAD_DATA)) logger.error(f"Registry error: {Err}") else: - if self.object.state == Domain.State.READY: + if initial_state == Domain.State.READY: self.send_update_notification(formset) messages.success( self.request, From 491ce93b14fea7cc8cbdd2e8a9aa56fa0273ccc6 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Mon, 7 Oct 2024 12:35:19 -0500 Subject: [PATCH 42/49] linter issues --- src/registrar/config/settings.py | 2 +- src/registrar/views/domain.py | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 523b802a6..da58eee86 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -479,7 +479,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/views/domain.py b/src/registrar/views/domain.py index 7bd5834c0..5180fe515 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -224,13 +224,17 @@ class DomainFormBaseView(DomainBaseView, FormMixin): emails = list(User.objects.filter(pk__in=manager_pks).values_list("email", flat=True)) try: # Remove the current user so they aren't CC'ed, since they will be the "to_address" - emails.remove(self.request.user.email) + emails.remove(self.request.user.email) # type: ignore except ValueError: pass try: send_templated_email( - template, subject_template, to_address=self.request.user.email, context=context, cc_addresses=emails + template, + subject_template, + to_address=self.request.user.email, # type: ignore + context=context, + cc_addresses=emails, ) except EmailSendingError: logger.warning( From bca91a59ef75a334266a0c18f0c215e80cb2b6a6 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 10 Oct 2024 11:22:15 -0500 Subject: [PATCH 43/49] Fix edge case and add test --- src/registrar/tests/test_views_domain.py | 33 ++++++++++++++++++++++++ src/registrar/views/domain.py | 11 ++++---- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 14d504784..559df5d60 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -2064,6 +2064,39 @@ class TestDomainChangeNotifications(TestDomainOverview): # Check that an email was not sent self.assertFalse(self.mock_client.send_email.called) + @boto3_mocking.patching + @less_console_noise_decorator + def test_no_notification_on_change_by_analyst(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" + 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.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] + + session = self.app.session + session["analyst_action"] = "foo" + session["analyst_action_location"] = self.domain.id + session.save() + + org_name_page.form["organization_name"] = "Not igorville" + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): + org_name_page.form.submit() + + # Check that an email was not sent + self.assertFalse(self.mock_client.send_email.called) + + @boto3_mocking.patching @less_console_noise_decorator def test_notification_on_security_email_change(self): diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 5180fe515..04eab1383 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -173,22 +173,21 @@ class DomainFormBaseView(DomainBaseView, FormMixin): SeniorOfficialContactForm, } - if form.__class__ in form_label_dict: + is_analyst_action = ("analyst_action" in self.session and "analyst_action_location" in self.session) + + if form.__class__ in form_label_dict and not is_analyst_action: # these types of forms can cause notifications 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.debug("No notification sent: Domain is part of a portfolio") should_notify = False - elif "analyst_action" in self.session and "analyst_action_location" in self.session: - # action is being made by an analyst - should_notify = False else: # don't notify for any other types of forms should_notify = False - logger.info(f"Not notifying for {form.__class__}") - if (should_notify and form.has_changed()) or force_send: + if should_notify and (form.has_changed() or force_send): context = { "domain": self.object.name, "user": self.request.user, From 84c30f69eee8ea63d567563bf591fc62bd8cdb34 Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Fri, 11 Oct 2024 13:36:15 -0500 Subject: [PATCH 44/49] minor update to email template --- src/registrar/templates/emails/update_to_approved_domain.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/registrar/templates/emails/update_to_approved_domain.txt b/src/registrar/templates/emails/update_to_approved_domain.txt index 8e615c30c..99f86ea54 100644 --- a/src/registrar/templates/emails/update_to_approved_domain.txt +++ b/src/registrar/templates/emails/update_to_approved_domain.txt @@ -26,4 +26,6 @@ THANK YOU The .gov team Contact us Learn about .gov + +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) {% endautoescape %} \ No newline at end of file From bc80c13bd32365dcc44f576510bf6c31fffdb7cb Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Fri, 11 Oct 2024 14:56:24 -0500 Subject: [PATCH 45/49] fix notifications on analyst action --- src/registrar/views/domain.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 04eab1383..e54ec9c8f 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -174,16 +174,20 @@ class DomainFormBaseView(DomainBaseView, FormMixin): } is_analyst_action = ("analyst_action" in self.session and "analyst_action_location" in self.session) + should_notify=False - if form.__class__ in form_label_dict and not is_analyst_action: - # these types of forms can cause notifications - 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.debug("No notification sent: Domain is part of a portfolio") - should_notify = False + if form.__class__ in form_label_dict: + if is_analyst_action: + logger.debug("No notification sent: Action was conducted by an analyst") + else: + # these types of forms can cause notifications + 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.debug("No notification sent: Domain is part of a portfolio") + should_notify = False else: # don't notify for any other types of forms should_notify = False @@ -202,7 +206,7 @@ class DomainFormBaseView(DomainBaseView, FormMixin): ) else: logger.info( - f"Not notifying for {form.__class__}, form changes: {form.has_changed()}, force_send: {force_send}" + f"No notification sent for {form.__class__}. form changes: {form.has_changed()}, force_send: {force_send}" ) def email_domain_managers(self, domain: Domain, template: str, subject_template: str, context={}): From 9d1cdbc41b523472697552ce9cfedfc175de7f3d Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Fri, 11 Oct 2024 16:53:22 -0500 Subject: [PATCH 46/49] add some extra logging to debug in sandbox --- src/registrar/views/domain.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index e54ec9c8f..1e90780ca 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -174,6 +174,17 @@ class DomainFormBaseView(DomainBaseView, FormMixin): } is_analyst_action = ("analyst_action" in self.session and "analyst_action_location" in self.session) + + if "analyst_action" in self.session: + logger.info(f"analyst action: %s", self.session["analyst_action"]) + else: + logger.info("Analyst_action not found in session") + + if "analyst_action_location" in self.session: + logger.info(f"analyst action location: %s", self.session["analyst_action_location"]) + else: + logger.info("Analyst_action_location not found in session") + should_notify=False if form.__class__ in form_label_dict: From dbf17e34ceb6dc2e4ae244399869e44e1ffd67aa Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Tue, 15 Oct 2024 09:45:04 -0500 Subject: [PATCH 47/49] remove extraneous logging statements --- src/registrar/views/domain.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 1e90780ca..b0fc52cf1 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -175,16 +175,6 @@ class DomainFormBaseView(DomainBaseView, FormMixin): is_analyst_action = ("analyst_action" in self.session and "analyst_action_location" in self.session) - if "analyst_action" in self.session: - logger.info(f"analyst action: %s", self.session["analyst_action"]) - else: - logger.info("Analyst_action not found in session") - - if "analyst_action_location" in self.session: - logger.info(f"analyst action location: %s", self.session["analyst_action_location"]) - else: - logger.info("Analyst_action_location not found in session") - should_notify=False if form.__class__ in form_label_dict: From 7e39e8a0aec0a5b3a391c1655cec00e882de24d9 Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Thu, 17 Oct 2024 09:34:00 -0500 Subject: [PATCH 48/49] dns sec form label change --- src/registrar/views/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index b0fc52cf1..665defc77 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -160,7 +160,7 @@ class DomainFormBaseView(DomainBaseView, FormMixin): # send notification email for changes to any of these forms form_label_dict = { DomainSecurityEmailForm: "Security email", - DomainDnssecForm: "DNSSec", + DomainDnssecForm: "DNSSEC / DS Data", DomainDsdataFormset: "DNSSEC / DS Data", DomainOrgNameAddressForm: "Organization details", SeniorOfficialContactForm: "Senior official", From 172c5d31c5fea0375b6b61f4563d6de29dec3626 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Fri, 18 Oct 2024 14:14:34 -0500 Subject: [PATCH 49/49] linter errors and test cleanup --- src/docker-compose.yml | 2 +- src/registrar/tests/test_models.py | 1020 ------------------- src/registrar/tests/test_models_requests.py | 8 +- src/registrar/tests/test_views_domain.py | 3 +- src/registrar/views/domain.py | 10 +- 5 files changed, 10 insertions(+), 1033 deletions(-) diff --git a/src/docker-compose.yml b/src/docker-compose.yml index d64ba80d5..8cb2bd60f 100644 --- a/src/docker-compose.yml +++ b/src/docker-compose.yml @@ -25,7 +25,7 @@ services: # Run Django in debug mode on local - DJANGO_DEBUG=True # Set DJANGO_LOG_LEVEL in env - - DJANGO_LOG_LEVEL + - DJANGO_LOG_LEVEL=DEBUG # Run Django without production flags - IS_PRODUCTION=False # Tell Django where it is being hosted diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 86809f44f..02902c1a9 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -35,1026 +35,6 @@ from waffle.testutils import override_flag from api.tests.common import less_console_noise_decorator -<<<<<<< HEAD -@boto3_mocking.patching -class TestDomainRequest(TestCase): - @less_console_noise_decorator - def setUp(self): - - self.dummy_user, _ = Contact.objects.get_or_create( - email="mayor@igorville.com", first_name="Hello", last_name="World" - ) - self.dummy_user_2, _ = User.objects.get_or_create( - username="intern@igorville.com", email="intern@igorville.com", first_name="Lava", last_name="World" - ) - self.started_domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.STARTED, - name="started.gov", - ) - self.submitted_domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.SUBMITTED, - name="submitted.gov", - ) - self.in_review_domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.IN_REVIEW, - name="in-review.gov", - ) - self.action_needed_domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.ACTION_NEEDED, - name="action-needed.gov", - ) - self.approved_domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.APPROVED, - name="approved.gov", - ) - self.withdrawn_domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.WITHDRAWN, - name="withdrawn.gov", - ) - self.rejected_domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.REJECTED, - name="rejected.gov", - ) - self.ineligible_domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.INELIGIBLE, - name="ineligible.gov", - ) - - # Store all domain request statuses in a variable for ease of use - self.all_domain_requests = [ - self.started_domain_request, - self.submitted_domain_request, - self.in_review_domain_request, - self.action_needed_domain_request, - self.approved_domain_request, - self.withdrawn_domain_request, - self.rejected_domain_request, - self.ineligible_domain_request, - ] - - self.mock_client = MockSESClient() - - def tearDown(self): - super().tearDown() - DomainInformation.objects.all().delete() - DomainRequest.objects.all().delete() - DraftDomain.objects.all().delete() - Domain.objects.all().delete() - User.objects.all().delete() - self.mock_client.EMAILS_SENT.clear() - - def assertNotRaises(self, exception_type): - """Helper method for testing allowed transitions.""" - with less_console_noise(): - return self.assertRaises(Exception, None, exception_type) - - @less_console_noise_decorator - def test_request_is_withdrawable(self): - """Tests the is_withdrawable function""" - domain_request_1 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.SUBMITTED, - name="city2.gov", - ) - domain_request_2 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.IN_REVIEW, - name="city3.gov", - ) - domain_request_3 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.ACTION_NEEDED, - name="city4.gov", - ) - domain_request_4 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.REJECTED, - name="city5.gov", - ) - self.assertTrue(domain_request_1.is_withdrawable()) - self.assertTrue(domain_request_2.is_withdrawable()) - self.assertTrue(domain_request_3.is_withdrawable()) - self.assertFalse(domain_request_4.is_withdrawable()) - - @less_console_noise_decorator - def test_request_is_awaiting_review(self): - """Tests the is_awaiting_review function""" - domain_request_1 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.SUBMITTED, - name="city2.gov", - ) - domain_request_2 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.IN_REVIEW, - name="city3.gov", - ) - domain_request_3 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.ACTION_NEEDED, - name="city4.gov", - ) - domain_request_4 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.REJECTED, - name="city5.gov", - ) - self.assertTrue(domain_request_1.is_awaiting_review()) - self.assertTrue(domain_request_2.is_awaiting_review()) - self.assertFalse(domain_request_3.is_awaiting_review()) - self.assertFalse(domain_request_4.is_awaiting_review()) - - @less_console_noise_decorator - def test_federal_agency_set_to_non_federal_on_approve(self): - """Ensures that when the federal_agency field is 'none' when .approve() is called, - the field is set to the 'Non-Federal Agency' record""" - domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.IN_REVIEW, - name="city2.gov", - federal_agency=None, - ) - - # Ensure that the federal agency is None - self.assertEqual(domain_request.federal_agency, None) - - # Approve the request - domain_request.approve() - self.assertEqual(domain_request.status, DomainRequest.DomainRequestStatus.APPROVED) - - # After approval, it should be "Non-Federal agency" - expected_federal_agency = FederalAgency.objects.filter(agency="Non-Federal Agency").first() - self.assertEqual(domain_request.federal_agency, expected_federal_agency) - - def test_empty_create_fails(self): - """Can't create a completely empty domain request.""" - with less_console_noise(): - with transaction.atomic(): - with self.assertRaisesRegex(IntegrityError, "creator"): - DomainRequest.objects.create() - - @less_console_noise_decorator - def test_minimal_create(self): - """Can create with just a creator.""" - user, _ = User.objects.get_or_create(username="testy") - domain_request = DomainRequest.objects.create(creator=user) - self.assertEqual(domain_request.status, DomainRequest.DomainRequestStatus.STARTED) - - @less_console_noise_decorator - def test_full_create(self): - """Can create with all fields.""" - user, _ = User.objects.get_or_create(username="testy") - contact = Contact.objects.create() - com_website, _ = Website.objects.get_or_create(website="igorville.com") - gov_website, _ = Website.objects.get_or_create(website="igorville.gov") - domain, _ = DraftDomain.objects.get_or_create(name="igorville.gov") - domain_request = DomainRequest.objects.create( - creator=user, - investigator=user, - generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, - federal_type=BranchChoices.EXECUTIVE, - is_election_board=False, - organization_name="Test", - address_line1="100 Main St.", - address_line2="APT 1A", - state_territory="CA", - zipcode="12345-6789", - senior_official=contact, - requested_domain=domain, - purpose="Igorville rules!", - anything_else="All of Igorville loves the dotgov program.", - is_policy_acknowledged=True, - ) - domain_request.current_websites.add(com_website) - domain_request.alternative_domains.add(gov_website) - domain_request.other_contacts.add(contact) - domain_request.save() - - @less_console_noise_decorator - def test_domain_info(self): - """Can create domain info with all fields.""" - user, _ = User.objects.get_or_create(username="testy") - contact = Contact.objects.create() - domain, _ = Domain.objects.get_or_create(name="igorville.gov") - information = DomainInformation.objects.create( - creator=user, - generic_org_type=DomainInformation.OrganizationChoices.FEDERAL, - federal_type=BranchChoices.EXECUTIVE, - is_election_board=False, - organization_name="Test", - address_line1="100 Main St.", - address_line2="APT 1A", - state_territory="CA", - zipcode="12345-6789", - senior_official=contact, - purpose="Igorville rules!", - anything_else="All of Igorville loves the dotgov program.", - is_policy_acknowledged=True, - domain=domain, - ) - information.other_contacts.add(contact) - information.save() - self.assertEqual(information.domain.id, domain.id) - self.assertEqual(information.id, domain.domain_info.id) - - @less_console_noise_decorator - def test_status_fsm_submit_fail(self): - user, _ = User.objects.get_or_create(username="testy") - domain_request = DomainRequest.objects.create(creator=user) - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - with less_console_noise(): - with self.assertRaises(ValueError): - # can't submit a domain request with a null domain name - domain_request.submit() - - @less_console_noise_decorator - def test_status_fsm_submit_succeed(self): - user, _ = User.objects.get_or_create(username="testy") - site = DraftDomain.objects.create(name="igorville.gov") - domain_request = DomainRequest.objects.create(creator=user, requested_domain=site) - - # no email sent to creator so this emits a log warning - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - with less_console_noise(): - domain_request.submit() - self.assertEqual(domain_request.status, domain_request.DomainRequestStatus.SUBMITTED) - - @less_console_noise_decorator - def check_email_sent( - self, domain_request, msg, action, expected_count, expected_content=None, expected_email="mayor@igorville.com" - ): - """Check if an email was sent after performing an action.""" - email_allowed, _ = AllowedEmail.objects.get_or_create(email=expected_email) - with self.subTest(msg=msg, action=action): - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - # Perform the specified action - action_method = getattr(domain_request, action) - action_method() - - # Check if an email was sent - sent_emails = [ - email - for email in MockSESClient.EMAILS_SENT - if expected_email in email["kwargs"]["Destination"]["ToAddresses"] - ] - self.assertEqual(len(sent_emails), expected_count) - - if expected_content: - email_content = sent_emails[0]["kwargs"]["Content"]["Simple"]["Body"]["Text"]["Data"] - self.assertIn(expected_content, email_content) - - email_allowed.delete() - - @less_console_noise_decorator - def test_submit_from_started_sends_email_to_creator(self): - """tests that we send an email to the creator""" - msg = "Create a domain request and submit it and see if email was sent when the feature flag is on." - domain_request = completed_domain_request(user=self.dummy_user_2) - self.check_email_sent( - domain_request, msg, "submit", 1, expected_content="Lava", expected_email="intern@igorville.com" - ) - - @less_console_noise_decorator - def test_submit_from_withdrawn_sends_email(self): - msg = "Create a withdrawn domain request and submit it and see if email was sent." - user, _ = User.objects.get_or_create(username="testy", email="testy@town.com") - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.WITHDRAWN, user=user) - self.check_email_sent(domain_request, msg, "submit", 1, expected_content="Hi", expected_email=user.email) - - @less_console_noise_decorator - def test_submit_from_action_needed_does_not_send_email(self): - msg = "Create a domain request with ACTION_NEEDED status and submit it, check if email was not sent." - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.ACTION_NEEDED) - self.check_email_sent(domain_request, msg, "submit", 0) - - @less_console_noise_decorator - def test_submit_from_in_review_does_not_send_email(self): - msg = "Create a withdrawn domain request and submit it and see if email was sent." - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW) - self.check_email_sent(domain_request, msg, "submit", 0) - - @less_console_noise_decorator - def test_approve_sends_email(self): - msg = "Create a domain request and approve it and see if email was sent." - user, _ = User.objects.get_or_create(username="testy", email="testy@town.com") - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=user) - self.check_email_sent(domain_request, msg, "approve", 1, expected_content="approved", expected_email=user.email) - - @less_console_noise_decorator - def test_withdraw_sends_email(self): - msg = "Create a domain request and withdraw it and see if email was sent." - user, _ = User.objects.get_or_create(username="testy", email="testy@town.com") - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=user) - self.check_email_sent( - domain_request, msg, "withdraw", 1, expected_content="withdrawn", expected_email=user.email - ) - - @less_console_noise_decorator - def test_reject_sends_email(self): - msg = "Create a domain request and reject it and see if email was sent." - user, _ = User.objects.get_or_create(username="testy", email="testy@town.com") - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.APPROVED, user=user) - self.check_email_sent(domain_request, msg, "reject", 1, expected_content="Hi", expected_email=user.email) - - @less_console_noise_decorator - def test_reject_with_prejudice_does_not_send_email(self): - msg = "Create a domain request and reject it with prejudice and see if email was sent." - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.APPROVED) - self.check_email_sent(domain_request, msg, "reject_with_prejudice", 0) - - @less_console_noise_decorator - def assert_fsm_transition_raises_error(self, test_cases, method_to_run): - """Given a list of test cases, check if each transition throws the intended error""" - with boto3_mocking.clients.handler_for("sesv2", self.mock_client), less_console_noise(): - for domain_request, exception_type in test_cases: - with self.subTest(domain_request=domain_request, exception_type=exception_type): - with self.assertRaises(exception_type): - # Retrieve the method by name from the domain_request object and call it - method = getattr(domain_request, method_to_run) - # Call the method - method() - - @less_console_noise_decorator - def assert_fsm_transition_does_not_raise_error(self, test_cases, method_to_run): - """Given a list of test cases, ensure that none of them throw transition errors""" - with boto3_mocking.clients.handler_for("sesv2", self.mock_client), less_console_noise(): - for domain_request, exception_type in test_cases: - with self.subTest(domain_request=domain_request, exception_type=exception_type): - try: - # Retrieve the method by name from the DomainRequest object and call it - method = getattr(domain_request, method_to_run) - # Call the method - method() - except exception_type: - self.fail(f"{exception_type} was raised, but it was not expected.") - - @less_console_noise_decorator - def test_submit_transition_allowed_with_no_investigator(self): - """ - Tests for attempting to transition without an investigator. - For submit, this should be valid in all cases. - """ - - test_cases = [ - (self.started_domain_request, TransitionNotAllowed), - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.withdrawn_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to none - set_domain_request_investigators(self.all_domain_requests, None) - - self.assert_fsm_transition_does_not_raise_error(test_cases, "submit") - - @less_console_noise_decorator - def test_submit_transition_allowed_with_investigator_not_staff(self): - """ - Tests for attempting to transition with an investigator user that is not staff. - For submit, this should be valid in all cases. - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to a user with no staff privs - user, _ = User.objects.get_or_create(username="pancakesyrup", is_staff=False) - set_domain_request_investigators(self.all_domain_requests, user) - - self.assert_fsm_transition_does_not_raise_error(test_cases, "submit") - - @less_console_noise_decorator - def test_submit_transition_allowed(self): - """ - Test that calling submit from allowable statuses does raises TransitionNotAllowed. - """ - test_cases = [ - (self.started_domain_request, TransitionNotAllowed), - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.withdrawn_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_does_not_raise_error(test_cases, "submit") - - @less_console_noise_decorator - def test_submit_transition_allowed_twice(self): - """ - Test that rotating between submit and in_review doesn't throw an error - """ - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - try: - # Make a submission - self.in_review_domain_request.submit() - - # Rerun the old method to get back to the original state - self.in_review_domain_request.in_review() - - # Make another submission - self.in_review_domain_request.submit() - except TransitionNotAllowed: - self.fail("TransitionNotAllowed was raised, but it was not expected.") - - self.assertEqual(self.in_review_domain_request.status, DomainRequest.DomainRequestStatus.SUBMITTED) - - @less_console_noise_decorator - def test_submit_transition_not_allowed(self): - """ - Test that calling submit against transition rules raises TransitionNotAllowed. - """ - test_cases = [ - (self.submitted_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_raises_error(test_cases, "submit") - - @less_console_noise_decorator - def test_in_review_transition_allowed(self): - """ - Test that calling in_review from allowable statuses does raises TransitionNotAllowed. - """ - test_cases = [ - (self.submitted_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_does_not_raise_error(test_cases, "in_review") - - @less_console_noise_decorator - def test_in_review_transition_not_allowed_with_no_investigator(self): - """ - Tests for attempting to transition without an investigator - """ - - test_cases = [ - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to none - set_domain_request_investigators(self.all_domain_requests, None) - - self.assert_fsm_transition_raises_error(test_cases, "in_review") - - @less_console_noise_decorator - def test_in_review_transition_not_allowed_with_investigator_not_staff(self): - """ - Tests for attempting to transition with an investigator that is not staff. - This should throw an exception. - """ - - test_cases = [ - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to a user with no staff privs - user, _ = User.objects.get_or_create(username="pancakesyrup", is_staff=False) - set_domain_request_investigators(self.all_domain_requests, user) - - self.assert_fsm_transition_raises_error(test_cases, "in_review") - - @less_console_noise_decorator - def test_in_review_transition_not_allowed(self): - """ - Test that calling in_review against transition rules raises TransitionNotAllowed. - """ - test_cases = [ - (self.started_domain_request, TransitionNotAllowed), - (self.in_review_domain_request, TransitionNotAllowed), - (self.withdrawn_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_raises_error(test_cases, "in_review") - - @less_console_noise_decorator - def test_action_needed_transition_allowed(self): - """ - Test that calling action_needed from allowable statuses does raises TransitionNotAllowed. - """ - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_does_not_raise_error(test_cases, "action_needed") - - @less_console_noise_decorator - def test_action_needed_transition_not_allowed_with_no_investigator(self): - """ - Tests for attempting to transition without an investigator - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to none - set_domain_request_investigators(self.all_domain_requests, None) - - self.assert_fsm_transition_raises_error(test_cases, "action_needed") - - @less_console_noise_decorator - def test_action_needed_transition_not_allowed_with_investigator_not_staff(self): - """ - Tests for attempting to transition with an investigator that is not staff - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to a user with no staff privs - user, _ = User.objects.get_or_create(username="pancakesyrup", is_staff=False) - set_domain_request_investigators(self.all_domain_requests, user) - - self.assert_fsm_transition_raises_error(test_cases, "action_needed") - - @less_console_noise_decorator - def test_action_needed_transition_not_allowed(self): - """ - Test that calling action_needed against transition rules raises TransitionNotAllowed. - """ - test_cases = [ - (self.started_domain_request, TransitionNotAllowed), - (self.submitted_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.withdrawn_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_raises_error(test_cases, "action_needed") - - @less_console_noise_decorator - def test_approved_transition_allowed(self): - """ - Test that calling action_needed from allowable statuses does raises TransitionNotAllowed. - """ - test_cases = [ - (self.submitted_domain_request, TransitionNotAllowed), - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_does_not_raise_error(test_cases, "approve") - - @less_console_noise_decorator - def test_approved_transition_not_allowed_with_no_investigator(self): - """ - Tests for attempting to transition without an investigator - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to none - set_domain_request_investigators(self.all_domain_requests, None) - - self.assert_fsm_transition_raises_error(test_cases, "approve") - - @less_console_noise_decorator - def test_approved_transition_not_allowed_with_investigator_not_staff(self): - """ - Tests for attempting to transition with an investigator that is not staff - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to a user with no staff privs - user, _ = User.objects.get_or_create(username="pancakesyrup", is_staff=False) - set_domain_request_investigators(self.all_domain_requests, user) - - self.assert_fsm_transition_raises_error(test_cases, "approve") - - @less_console_noise_decorator - 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): - self.submitted_domain_request.approve(send_email=False) - - # Assert that no emails were sent - self.assertEqual(len(self.mock_client.EMAILS_SENT), 0) - - @less_console_noise_decorator - def test_approved_transition_not_allowed(self): - """ - Test that calling action_needed against transition rules raises TransitionNotAllowed. - """ - test_cases = [ - (self.started_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.withdrawn_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - self.assert_fsm_transition_raises_error(test_cases, "approve") - - @less_console_noise_decorator - def test_withdraw_transition_allowed(self): - """ - Test that calling action_needed from allowable statuses does raises TransitionNotAllowed. - """ - test_cases = [ - (self.submitted_domain_request, TransitionNotAllowed), - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_does_not_raise_error(test_cases, "withdraw") - - @less_console_noise_decorator - def test_withdraw_transition_allowed_with_no_investigator(self): - """ - Tests for attempting to transition without an investigator. - For withdraw, this should be valid in all cases. - """ - - test_cases = [ - (self.submitted_domain_request, TransitionNotAllowed), - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to none - set_domain_request_investigators(self.all_domain_requests, None) - - self.assert_fsm_transition_does_not_raise_error(test_cases, "withdraw") - - @less_console_noise_decorator - def test_withdraw_transition_allowed_with_investigator_not_staff(self): - """ - Tests for attempting to transition when investigator is not staff. - For withdraw, this should be valid in all cases. - """ - - test_cases = [ - (self.submitted_domain_request, TransitionNotAllowed), - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to a user with no staff privs - user, _ = User.objects.get_or_create(username="pancakesyrup", is_staff=False) - set_domain_request_investigators(self.all_domain_requests, user) - - self.assert_fsm_transition_does_not_raise_error(test_cases, "withdraw") - - @less_console_noise_decorator - def test_withdraw_transition_not_allowed(self): - """ - Test that calling action_needed against transition rules raises TransitionNotAllowed. - """ - test_cases = [ - (self.started_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.withdrawn_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_raises_error(test_cases, "withdraw") - - @less_console_noise_decorator - def test_reject_transition_allowed(self): - """ - Test that calling action_needed from allowable statuses does raises TransitionNotAllowed. - """ - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_does_not_raise_error(test_cases, "reject") - - @less_console_noise_decorator - def test_reject_transition_not_allowed_with_no_investigator(self): - """ - Tests for attempting to transition without an investigator - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to none - set_domain_request_investigators(self.all_domain_requests, None) - - self.assert_fsm_transition_raises_error(test_cases, "reject") - - @less_console_noise_decorator - def test_reject_transition_not_allowed_with_investigator_not_staff(self): - """ - Tests for attempting to transition when investigator is not staff - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to a user with no staff privs - user, _ = User.objects.get_or_create(username="pancakesyrup", is_staff=False) - set_domain_request_investigators(self.all_domain_requests, user) - - self.assert_fsm_transition_raises_error(test_cases, "reject") - - @less_console_noise_decorator - def test_reject_transition_not_allowed(self): - """ - Test that calling action_needed against transition rules raises TransitionNotAllowed. - """ - test_cases = [ - (self.started_domain_request, TransitionNotAllowed), - (self.submitted_domain_request, TransitionNotAllowed), - (self.withdrawn_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_raises_error(test_cases, "reject") - - @less_console_noise_decorator - def test_reject_with_prejudice_transition_allowed(self): - """ - Test that calling action_needed from allowable statuses does raises TransitionNotAllowed. - """ - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_does_not_raise_error(test_cases, "reject_with_prejudice") - - @less_console_noise_decorator - def test_reject_with_prejudice_transition_not_allowed_with_no_investigator(self): - """ - Tests for attempting to transition without an investigator - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to none - set_domain_request_investigators(self.all_domain_requests, None) - - self.assert_fsm_transition_raises_error(test_cases, "reject_with_prejudice") - - @less_console_noise_decorator - def test_reject_with_prejudice_not_allowed_with_investigator_not_staff(self): - """ - Tests for attempting to transition when investigator is not staff - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to a user with no staff privs - user, _ = User.objects.get_or_create(username="pancakesyrup", is_staff=False) - set_domain_request_investigators(self.all_domain_requests, user) - - self.assert_fsm_transition_raises_error(test_cases, "reject_with_prejudice") - - @less_console_noise_decorator - def test_reject_with_prejudice_transition_not_allowed(self): - """ - Test that calling action_needed against transition rules raises TransitionNotAllowed. - """ - test_cases = [ - (self.started_domain_request, TransitionNotAllowed), - (self.submitted_domain_request, TransitionNotAllowed), - (self.withdrawn_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_raises_error(test_cases, "reject_with_prejudice") - - @less_console_noise_decorator - def test_transition_not_allowed_approved_in_review_when_domain_is_active(self): - """Create a domain request with status approved, create a matching domain that - is active, and call in_review against transition rules""" - - domain = Domain.objects.create(name=self.approved_domain_request.requested_domain.name) - self.approved_domain_request.approved_domain = domain - self.approved_domain_request.save() - - # Define a custom implementation for is_active - def custom_is_active(self): - return True # Override to return True - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - # Use patch to temporarily replace is_active with the custom implementation - with patch.object(Domain, "is_active", custom_is_active): - # Now, when you call is_active on Domain, it will return True - with self.assertRaises(TransitionNotAllowed): - self.approved_domain_request.in_review() - - @less_console_noise_decorator - def test_transition_not_allowed_approved_action_needed_when_domain_is_active(self): - """Create a domain request with status approved, create a matching domain that - is active, and call action_needed against transition rules""" - - domain = Domain.objects.create(name=self.approved_domain_request.requested_domain.name) - self.approved_domain_request.approved_domain = domain - self.approved_domain_request.save() - - # Define a custom implementation for is_active - def custom_is_active(self): - return True # Override to return True - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - # Use patch to temporarily replace is_active with the custom implementation - with patch.object(Domain, "is_active", custom_is_active): - # Now, when you call is_active on Domain, it will return True - with self.assertRaises(TransitionNotAllowed): - self.approved_domain_request.action_needed() - - @less_console_noise_decorator - def test_transition_not_allowed_approved_rejected_when_domain_is_active(self): - """Create a domain request with status approved, create a matching domain that - is active, and call reject against transition rules""" - - domain = Domain.objects.create(name=self.approved_domain_request.requested_domain.name) - self.approved_domain_request.approved_domain = domain - self.approved_domain_request.save() - - # Define a custom implementation for is_active - def custom_is_active(self): - return True # Override to return True - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - # Use patch to temporarily replace is_active with the custom implementation - with patch.object(Domain, "is_active", custom_is_active): - # Now, when you call is_active on Domain, it will return True - with self.assertRaises(TransitionNotAllowed): - self.approved_domain_request.reject() - - @less_console_noise_decorator - def test_transition_not_allowed_approved_ineligible_when_domain_is_active(self): - """Create a domain request with status approved, create a matching domain that - is active, and call reject_with_prejudice against transition rules""" - - domain = Domain.objects.create(name=self.approved_domain_request.requested_domain.name) - self.approved_domain_request.approved_domain = domain - self.approved_domain_request.save() - - # Define a custom implementation for is_active - def custom_is_active(self): - return True # Override to return True - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - # Use patch to temporarily replace is_active with the custom implementation - with patch.object(Domain, "is_active", custom_is_active): - # Now, when you call is_active on Domain, it will return True - with self.assertRaises(TransitionNotAllowed): - self.approved_domain_request.reject_with_prejudice() - - @less_console_noise_decorator - def test_approve_from_rejected_clears_rejection_reason(self): - """When transitioning from rejected to approved on a domain request, - the rejection_reason is cleared.""" - - # Create a sample domain request - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.REJECTED) - domain_request.rejection_reason = DomainRequest.RejectionReasons.DOMAIN_PURPOSE - - # Approve - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - domain_request.approve() - - self.assertEqual(domain_request.status, DomainRequest.DomainRequestStatus.APPROVED) - self.assertEqual(domain_request.rejection_reason, None) - - @less_console_noise_decorator - def test_in_review_from_rejected_clears_rejection_reason(self): - """When transitioning from rejected to in_review on a domain request, - the rejection_reason is cleared.""" - - # Create a sample domain request - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.REJECTED) - domain_request.domain_is_not_active = True - domain_request.rejection_reason = DomainRequest.RejectionReasons.DOMAIN_PURPOSE - - # Approve - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - domain_request.in_review() - - self.assertEqual(domain_request.status, DomainRequest.DomainRequestStatus.IN_REVIEW) - self.assertEqual(domain_request.rejection_reason, None) - - @less_console_noise_decorator - def test_action_needed_from_rejected_clears_rejection_reason(self): - """When transitioning from rejected to action_needed on a domain request, - the rejection_reason is cleared.""" - - # Create a sample domain request - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.REJECTED) - domain_request.domain_is_not_active = True - domain_request.rejection_reason = DomainRequest.RejectionReasons.DOMAIN_PURPOSE - - # Approve - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - domain_request.action_needed() - - self.assertEqual(domain_request.status, DomainRequest.DomainRequestStatus.ACTION_NEEDED) - self.assertEqual(domain_request.rejection_reason, None) - - @less_console_noise_decorator - def test_has_rationale_returns_true(self): - """has_rationale() returns true when a domain request has no_other_contacts_rationale""" - self.started_domain_request.no_other_contacts_rationale = "You talkin' to me?" - self.started_domain_request.save() - self.assertEquals(self.started_domain_request.has_rationale(), True) - - @less_console_noise_decorator - def test_has_rationale_returns_false(self): - """has_rationale() returns false when a domain request has no no_other_contacts_rationale""" - self.assertEquals(self.started_domain_request.has_rationale(), False) - - @less_console_noise_decorator - def test_has_other_contacts_returns_true(self): - """has_other_contacts() returns true when a domain request has other_contacts""" - # completed_domain_request has other contacts by default - self.assertEquals(self.started_domain_request.has_other_contacts(), True) - - @less_console_noise_decorator - def test_has_other_contacts_returns_false(self): - """has_other_contacts() returns false when a domain request has no other_contacts""" - domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.STARTED, name="no-others.gov", has_other_contacts=False - ) - self.assertEquals(domain_request.has_other_contacts(), False) - - -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 - @less_console_noise_decorator - def test_approval_creates_role(self): - draft_domain, _ = DraftDomain.objects.get_or_create(name="igorville.gov") - user, _ = User.objects.get_or_create() - investigator, _ = User.objects.get_or_create(username="frenchtoast", is_staff=True) - domain_request = DomainRequest.objects.create( - creator=user, requested_domain=draft_domain, investigator=investigator - ) - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - # skip using the submit method - domain_request.status = DomainRequest.DomainRequestStatus.SUBMITTED - domain_request.approve() - - # should be a role for this user - domain = Domain.objects.get(name="igorville.gov") - self.assertTrue(UserDomainRole.objects.get(user=user, domain=domain)) - - -======= ->>>>>>> origin/main class TestDomainInformation(TestCase): """Test the DomainInformation model, when approved or otherwise""" diff --git a/src/registrar/tests/test_models_requests.py b/src/registrar/tests/test_models_requests.py index 2d2c615d7..339841be0 100644 --- a/src/registrar/tests/test_models_requests.py +++ b/src/registrar/tests/test_models_requests.py @@ -305,7 +305,7 @@ class TestDomainRequest(TestCase): @less_console_noise_decorator def test_submit_from_withdrawn_sends_email(self): msg = "Create a withdrawn domain request and submit it and see if email was sent." - user, _ = User.objects.get_or_create(username="testy") + user, _ = User.objects.get_or_create(username="testy", email="testy@town.com") domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.WITHDRAWN, user=user) self.check_email_sent(domain_request, msg, "submit", 1, expected_content="Hi", expected_email=user.email) @@ -324,14 +324,14 @@ class TestDomainRequest(TestCase): @less_console_noise_decorator def test_approve_sends_email(self): msg = "Create a domain request and approve it and see if email was sent." - user, _ = User.objects.get_or_create(username="testy") + user, _ = User.objects.get_or_create(username="testy", email="testy@town.com") domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=user) self.check_email_sent(domain_request, msg, "approve", 1, expected_content="approved", expected_email=user.email) @less_console_noise_decorator def test_withdraw_sends_email(self): msg = "Create a domain request and withdraw it and see if email was sent." - user, _ = User.objects.get_or_create(username="testy") + user, _ = User.objects.get_or_create(username="testy", email="testy@town.com") domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=user) self.check_email_sent( domain_request, msg, "withdraw", 1, expected_content="withdrawn", expected_email=user.email @@ -339,7 +339,7 @@ class TestDomainRequest(TestCase): def test_reject_sends_email(self): "Create a domain request and reject it and see if email was sent." - user, _ = User.objects.get_or_create(username="testy") + user, _ = User.objects.get_or_create(username="testy", email="testy@town.com") domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.APPROVED, user=user) expected_email = user.email email_allowed, _ = AllowedEmail.objects.get_or_create(email=expected_email) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 61e07660b..a375493be 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -2099,7 +2099,6 @@ class TestDomainChangeNotifications(TestDomainOverview): # Check that an email was not sent self.assertFalse(self.mock_client.send_email.called) - @boto3_mocking.patching @less_console_noise_decorator def test_notification_on_security_email_change(self): @@ -2154,7 +2153,7 @@ class TestDomainChangeNotifications(TestDomainOverview): self.assertIn("DOMAIN: igorville.gov", body) self.assertIn("UPDATED BY: First Last info@example.com", body) - self.assertIn("INFORMATION UPDATED: DNSSec", body) + self.assertIn("INFORMATION UPDATED: DNSSEC / DS Data", body) @boto3_mocking.patching @less_console_noise_decorator diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 665defc77..100c85f66 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -173,9 +173,9 @@ class DomainFormBaseView(DomainBaseView, FormMixin): SeniorOfficialContactForm, } - is_analyst_action = ("analyst_action" in self.session and "analyst_action_location" in self.session) - - should_notify=False + is_analyst_action = "analyst_action" in self.session and "analyst_action_location" in self.session + + should_notify = False if form.__class__ in form_label_dict: if is_analyst_action: @@ -206,9 +206,7 @@ class DomainFormBaseView(DomainBaseView, FormMixin): context, ) else: - logger.info( - f"No notification sent for {form.__class__}. form changes: {form.has_changed()}, force_send: {force_send}" - ) + logger.info(f"No notification sent for {form.__class__}.") def email_domain_managers(self, domain: Domain, template: str, subject_template: str, context={}): """Send a single email built from a template to all managers for a given domain.