From 7ddf1d87bc7f2c4da8033e767fc79865ec8bf191 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 28 Jun 2024 14:55:40 -0600 Subject: [PATCH 01/51] changes --- src/docker-compose.yml | 2 +- src/registrar/admin.py | 10 +++--- src/registrar/models/domain_request.py | 31 ++++++++++++++----- .../action_needed_reasons/custom_email.txt | 2 ++ 4 files changed, 32 insertions(+), 13 deletions(-) create mode 100644 src/registrar/templates/emails/action_needed_reasons/custom_email.txt diff --git a/src/docker-compose.yml b/src/docker-compose.yml index 1a9064ac8..39282ff96 100644 --- a/src/docker-compose.yml +++ b/src/docker-compose.yml @@ -67,8 +67,8 @@ services: # command: "python" command: > bash -c " python manage.py migrate && - python manage.py load && python manage.py createcachetable && + python manage.py load && python manage.py runserver 0.0.0.0:8080" db: diff --git a/src/registrar/admin.py b/src/registrar/admin.py index eee2eda2f..44dedd25d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1690,8 +1690,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): return super().save_model(request, obj, form, change) # == Handle non-status changes == # - # Change this in #1901. Add a check on "not self.action_needed_reason_email" - if obj.action_needed_reason: + if obj.action_needed_reason and not self.action_needed_reason_email: self._handle_action_needed_reason_email(obj) should_save = True @@ -1911,14 +1910,17 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # Call the superclass method with updated extra_context return super().change_view(request, object_id, form_url, extra_context) + # TODO - scrap this approach and just centralize everything def get_all_action_needed_reason_emails_as_json(self, domain_request): """Returns a json dictionary of every action needed reason and its associated email for this particular domain request.""" emails = {} for action_needed_reason in domain_request.ActionNeededReasons: enum_value = action_needed_reason.value - # Change this in #1901. Just add a check for the current value. - emails[enum_value] = self._get_action_needed_reason_default_email_text(domain_request, enum_value) + if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: + emails[enum_value] = domain_request.action_needed_reason_email + else: + emails[enum_value] = self._get_action_needed_reason_default_email_text(domain_request, enum_value) return json.dumps(emails) def _get_action_needed_reason_default_email_text(self, domain_request, action_needed_reason: str): diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index d26bb3284..febf0a5e6 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -672,7 +672,7 @@ class DomainRequest(TimeStampedModel): logger.error(f"Can't query an approved domain while attempting {called_from}") def _send_status_update_email( - self, new_status, email_template, email_template_subject, send_email=True, bcc_address="", wrap_email=False + self, new_status, email_template, email_template_subject, bcc_address="", context=None, **kwargs ): """Send a status update email to the creator. @@ -683,13 +683,21 @@ class DomainRequest(TimeStampedModel): If the waffle flag "profile_feature" is active, then this email will be sent to the domain request creator rather than the submitter + kwargs: send_email: bool -> Used to bypass the send_templated_email function, in the event we just want to log that an email would have been sent, rather than actually sending one. wrap_email: bool -> Wraps emails using `wrap_text_and_preserve_paragraphs` if any given paragraph exceeds our desired max length (for prettier display). + + custom_email_content: str -> Renders an email with the content of this string as its body text. """ + # Email config options + wrap_email = kwargs.get("wrap_email", False) + send_email = kwargs.get("send_email", True) + custom_email_content = kwargs.get("custom_email_content", None) + recipient = self.creator if flag_is_active(None, "profile_feature") else self.submitter if recipient is None or recipient.email is None: logger.warning( @@ -705,15 +713,21 @@ class DomainRequest(TimeStampedModel): return None try: + if not context: + context = { + "domain_request": self, + # This is the user that we refer to in the email + "recipient": recipient, + } + + if custom_email_content: + context["custom_email_content"] = custom_email_content + send_templated_email( email_template, email_template_subject, recipient.email, - context={ - "domain_request": self, - # This is the user that we refer to in the email - "recipient": recipient, - }, + context=context, bcc_address=bcc_address, wrap_email=wrap_email, ) @@ -844,12 +858,12 @@ class DomainRequest(TimeStampedModel): if self.action_needed_reason and self.action_needed_reason != self.ActionNeededReasons.OTHER: self._send_action_needed_reason_email(send_email) - def _send_action_needed_reason_email(self, send_email=True): + def _send_action_needed_reason_email(self, send_email=True, custom_email_content=None): """Sends out an automatic email for each valid action needed reason provided""" # Store the filenames of the template and template subject email_template_name: str = "" - email_template_subject_name: str = "" + email_template_subject_name: str = "" if not custom_email_content else "custom_email" # Check for the "type" of action needed reason. can_send_email = True @@ -877,6 +891,7 @@ class DomainRequest(TimeStampedModel): email_template_subject=f"emails/action_needed_reasons/{email_template_subject_name}", send_email=send_email, bcc_address=bcc_address, + custom_email_content=custom_email_content, wrap_email=True, ) diff --git a/src/registrar/templates/emails/action_needed_reasons/custom_email.txt b/src/registrar/templates/emails/action_needed_reasons/custom_email.txt new file mode 100644 index 000000000..8e58c8f2e --- /dev/null +++ b/src/registrar/templates/emails/action_needed_reasons/custom_email.txt @@ -0,0 +1,2 @@ +{% comment %} {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} {% endcomment %} +{{ custom_email_content }} \ No newline at end of file From 1cbd9234bee5a49655330fec6ee0316c744f8b10 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 28 Jun 2024 15:24:22 -0600 Subject: [PATCH 02/51] basic logic --- src/registrar/admin.py | 14 +++++++------- src/registrar/assets/js/get-gov-admin.js | 3 ++- src/registrar/models/domain_request.py | 9 +++++---- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index b2db130d9..1fed780e1 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1606,7 +1606,6 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): "is_election_board", "federal_agency", "status_history", - "action_needed_reason_email", ) # Read only that we'll leverage for CISA Analysts @@ -1705,7 +1704,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): return super().save_model(request, obj, form, change) # == Handle non-status changes == # - if obj.action_needed_reason and not self.action_needed_reason_email: + if obj.action_needed_reason and not obj.action_needed_reason_email: self._handle_action_needed_reason_email(obj) should_save = True @@ -1933,13 +1932,14 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): emails = {} for action_needed_reason in domain_request.ActionNeededReasons: enum_value = action_needed_reason.value + custom_text = None if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: - emails[enum_value] = domain_request.action_needed_reason_email - else: - emails[enum_value] = self._get_action_needed_reason_default_email_text(domain_request, enum_value) + custom_text = domain_request.action_needed_reason_email + + emails[enum_value] = self._get_action_needed_reason_default_email_text(domain_request, enum_value, custom_text) return json.dumps(emails) - def _get_action_needed_reason_default_email_text(self, domain_request, action_needed_reason: str): + def _get_action_needed_reason_default_email_text(self, domain_request, action_needed_reason: str, custom_text=None): """Returns the default email associated with the given action needed reason""" if action_needed_reason is None or action_needed_reason == domain_request.ActionNeededReasons.OTHER: return { @@ -1961,7 +1961,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): return { "subject_text": subject_template.render(context=context), - "email_body_text": template.render(context=context), + "email_body_text": template.render(context=context) if not custom_text else custom_text, } def process_log_entry(self, log_entry): diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 898c41c4b..6d753744f 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -525,7 +525,7 @@ function initializeWidgetOnList(list, parentId) { */ (function () { let actionNeededReasonDropdown = document.querySelector("#id_action_needed_reason"); - let actionNeededEmail = document.querySelector("#action_needed_reason_email_view_more"); + let actionNeededEmail = document.querySelector("#id_action_needed_reason_email"); if(actionNeededReasonDropdown && actionNeededEmail) { // Add a change listener to the action needed reason dropdown handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail); @@ -546,6 +546,7 @@ function initializeWidgetOnList(list, parentId) { let actionNeededEmails = JSON.parse(document.getElementById('action-needed-emails-data').textContent) let emailData = actionNeededEmails[reason]; if (emailData) { + // TODO: do we need a revert to default button? let emailBody = emailData.email_body_text if (emailBody) { actionNeededEmail.value = emailBody diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index febf0a5e6..830ad9480 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -785,8 +785,8 @@ class DomainRequest(TimeStampedModel): "submission confirmation", "emails/submission_confirmation.txt", "emails/submission_confirmation_subject.txt", - True, - bcc_address, + send_email=True, + bcc_address=bcc_address, ) @transition( @@ -856,7 +856,8 @@ class DomainRequest(TimeStampedModel): # Send out an email if an action needed reason exists if self.action_needed_reason and self.action_needed_reason != self.ActionNeededReasons.OTHER: - self._send_action_needed_reason_email(send_email) + custom_email_content = self.action_needed_reason_email + self._send_action_needed_reason_email(send_email, custom_email_content) def _send_action_needed_reason_email(self, send_email=True, custom_email_content=None): """Sends out an automatic email for each valid action needed reason provided""" @@ -951,7 +952,7 @@ class DomainRequest(TimeStampedModel): "domain request approved", "emails/status_change_approved.txt", "emails/status_change_approved_subject.txt", - send_email, + send_email=send_email, ) @transition( From b6a70e6d3c585968d3b3c4291fcfbc75ac03e1c9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 1 Jul 2024 09:56:09 -0600 Subject: [PATCH 03/51] custom content logic --- src/registrar/admin.py | 6 +++++- src/registrar/models/domain_request.py | 11 +++++++---- .../emails/action_needed_reasons/custom_email.txt | 5 +++-- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 1fed780e1..e2cd6e17d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1948,7 +1948,11 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): } # Get the email body - template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" + if not custom_text: + template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" + else: + template_path = f"emails/action_needed_reasons/custom_email.txt" + template = get_template(template_path) # Get the email subject diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 830ad9480..62eb3f4f8 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -618,7 +618,8 @@ class DomainRequest(TimeStampedModel): if was_already_action_needed and (reason_exists and reason_changed): # We don't send emails out in state "other" if self.action_needed_reason != self.ActionNeededReasons.OTHER: - self._send_action_needed_reason_email() + _email_content = self.action_needed_reason_email + self._send_action_needed_reason_email(custom_email_content=_email_content) def sync_yes_no_form_fields(self): """Some yes/no forms use a db field to track whether it was checked or not. @@ -863,8 +864,8 @@ class DomainRequest(TimeStampedModel): """Sends out an automatic email for each valid action needed reason provided""" # Store the filenames of the template and template subject - email_template_name: str = "" - email_template_subject_name: str = "" if not custom_email_content else "custom_email" + email_template_name: str = "" if not custom_email_content else "custom_email.txt" + email_template_subject_name: str = "" # Check for the "type" of action needed reason. can_send_email = True @@ -876,8 +877,10 @@ class DomainRequest(TimeStampedModel): # Assumes that the template name matches the action needed reason if nothing is specified. # This is so you can override if you need, or have this taken care of for you. - if not email_template_name and not email_template_subject_name: + if not email_template_name: email_template_name = f"{self.action_needed_reason}.txt" + + if not email_template_subject_name: email_template_subject_name = f"{self.action_needed_reason}_subject.txt" bcc_address = "" diff --git a/src/registrar/templates/emails/action_needed_reasons/custom_email.txt b/src/registrar/templates/emails/action_needed_reasons/custom_email.txt index 8e58c8f2e..58e539816 100644 --- a/src/registrar/templates/emails/action_needed_reasons/custom_email.txt +++ b/src/registrar/templates/emails/action_needed_reasons/custom_email.txt @@ -1,2 +1,3 @@ -{% comment %} {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} {% endcomment %} -{{ custom_email_content }} \ No newline at end of file +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} +{{ custom_email_content }} +{% endautoescape %} From 5815d6733c9db3f258879178ace83cb069e3860b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 2 Jul 2024 11:28:58 -0600 Subject: [PATCH 04/51] hash --- src/registrar/models/domain_request.py | 29 +++++++++++++++++-- .../models/utility/generic_helper.py | 5 ++++ src/registrar/tests/test_admin.py | 11 +++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 62eb3f4f8..66c70f9a7 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1,7 +1,7 @@ from __future__ import annotations from typing import Union import logging - +from django.template.loader import get_template from django.apps import apps from django.conf import settings from django.db import models @@ -10,7 +10,7 @@ from django.utils import timezone from waffle import flag_is_active from registrar.models.domain import Domain from registrar.models.federal_agency import FederalAgency -from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper +from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper, convert_string_to_sha256_hash from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes from registrar.utility.constants import BranchChoices @@ -864,9 +864,16 @@ class DomainRequest(TimeStampedModel): """Sends out an automatic email for each valid action needed reason provided""" # Store the filenames of the template and template subject - email_template_name: str = "" if not custom_email_content else "custom_email.txt" + email_template_name: str = "" email_template_subject_name: str = "" + # Check if the current email that we sent out is the same as our defaults. + # If these hashes differ, then that means that we're sending custom content. + default_email_hash = self._get_action_needed_reason_email_hash() + current_email_hash = convert_string_to_sha256_hash(self.action_needed_reason_email) + if default_email_hash != current_email_hash: + email_template_name = "custom_email.txt" + # Check for the "type" of action needed reason. can_send_email = True match self.action_needed_reason: @@ -899,6 +906,22 @@ class DomainRequest(TimeStampedModel): wrap_email=True, ) + # TODO - rework this + def _get_action_needed_reason_email_hash(self): + """Returns the default email associated with the given action needed reason""" + if self.action_needed_reason is None or self.action_needed_reason == self.ActionNeededReasons.OTHER: + return None + + # Get the email body + template_path = f"emails/action_needed_reasons/{self.action_needed_reason}.txt" + template = get_template(template_path) + + recipient = self.creator if flag_is_active(None, "profile_feature") else self.submitter + # Return the content of the rendered views + context = {"domain_request": self, "recipient": recipient} + body_text = template.render(context=context) + return convert_string_to_sha256_hash(body_text) + @transition( field="status", source=[ diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index f9d4303c4..f35e2f619 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -2,6 +2,7 @@ import time import logging +import hashlib from urllib.parse import urlparse, urlunparse, urlencode @@ -321,3 +322,7 @@ def convert_queryset_to_dict(queryset, is_model=True, key="id"): request_dict = {value[key]: value for value in queryset} return request_dict + + +def convert_string_to_sha256_hash(string_to_convert): + return hashlib.sha256(string_to_convert.encode('utf-8')).hexdigest() \ No newline at end of file diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 44a131d55..3dd0cdb6c 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1540,6 +1540,17 @@ class TestDomainRequestAdmin(MockEppLib): # Should be unchanged from before self.assertEqual(len(self.mock_client.EMAILS_SENT), 4) + # Tests if an analyst can override existing email content + questionable_ao = DomainRequest.ActionNeededReasons.QUESTIONABLE_AUTHORIZING_OFFICIAL + domain_request.action_needed_reason_email = "custom email content" + domain_request.save() + self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=questionable_ao) + + self.assert_email_is_accurate( + "custom email content", 4, EMAIL, bcc_email_address=BCC_EMAIL + ) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 5) + def test_save_model_sends_submitted_email(self): """When transitioning to submitted from started or withdrawn on a domain request, an email is sent out. From 34f6b03078a09499da281fbcce7b1d13414ae60a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 2 Jul 2024 11:29:57 -0600 Subject: [PATCH 05/51] Update test_admin.py --- src/registrar/tests/test_admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 11f9bf5fe..ca5e4a93c 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1480,10 +1480,10 @@ class TestDomainRequestAdmin(MockEppLib): self.assertEqual(len(self.mock_client.EMAILS_SENT), 4) # Tests if an analyst can override existing email content - questionable_ao = DomainRequest.ActionNeededReasons.QUESTIONABLE_AUTHORIZING_OFFICIAL + questionable_so = DomainRequest.ActionNeededReasons.QUESTIONABLE_SENIOR_OFFICIAL domain_request.action_needed_reason_email = "custom email content" domain_request.save() - self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=questionable_ao) + self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=questionable_so) self.assert_email_is_accurate( "custom email content", 4, EMAIL, bcc_email_address=BCC_EMAIL From a8fa24411e9238f43626b32a1d644e8bdea1f2c8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 2 Jul 2024 12:56:26 -0600 Subject: [PATCH 06/51] Email logic (needs cleanup) --- src/registrar/admin.py | 21 ++++++++++++------- src/registrar/models/domain_request.py | 16 +++++++------- .../models/utility/generic_helper.py | 4 +++- src/registrar/utility/email.py | 2 ++ 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index a7e078f46..9efbf1dae 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1703,12 +1703,21 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): return super().save_model(request, obj, form, change) # == Handle non-status changes == # - if obj.action_needed_reason and not obj.action_needed_reason_email: - self._handle_action_needed_reason_email(obj) - should_save = True - # Get the original domain request from the database. original_obj = models.DomainRequest.objects.get(pk=obj.pk) + + if obj.action_needed_reason: + text = self._get_action_needed_reason_default_email_text(obj, obj.action_needed_reason) + body_text = text.get("email_body_text") + if body_text: + body_text.strip().lstrip("\n") + is_default_email = body_text == obj.action_needed_reason_email + reason_changed = obj.action_needed_reason != original_obj.action_needed_reason + if is_default_email and reason_changed: + obj.action_needed_reason_email = body_text + should_save = True + + if obj.status == original_obj.status: # If the status hasn't changed, let the base function take care of it return super().save_model(request, obj, form, change) @@ -1721,10 +1730,6 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): if should_save: return super().save_model(request, obj, form, change) - def _handle_action_needed_reason_email(self, obj): - text = self._get_action_needed_reason_default_email_text(obj, obj.action_needed_reason) - obj.action_needed_reason_email = text.get("email_body_text") - def _handle_status_change(self, request, obj, original_obj): """ Checks for various conditions when a status change is triggered. diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 2c155a6d9..53c5765e9 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -619,7 +619,7 @@ class DomainRequest(TimeStampedModel): # We don't send emails out in state "other" if self.action_needed_reason != self.ActionNeededReasons.OTHER: _email_content = self.action_needed_reason_email - self._send_action_needed_reason_email(custom_email_content=_email_content) + self._send_action_needed_reason_email(email_content=_email_content) def sync_yes_no_form_fields(self): """Some yes/no forms use a db field to track whether it was checked or not. @@ -857,10 +857,10 @@ class DomainRequest(TimeStampedModel): # Send out an email if an action needed reason exists if self.action_needed_reason and self.action_needed_reason != self.ActionNeededReasons.OTHER: - custom_email_content = self.action_needed_reason_email - self._send_action_needed_reason_email(send_email, custom_email_content) + email_content = self.action_needed_reason_email + self._send_action_needed_reason_email(send_email, email_content) - def _send_action_needed_reason_email(self, send_email=True, custom_email_content=None): + def _send_action_needed_reason_email(self, send_email=True, email_content=None): """Sends out an automatic email for each valid action needed reason provided""" # Store the filenames of the template and template subject @@ -871,7 +871,8 @@ class DomainRequest(TimeStampedModel): # If these hashes differ, then that means that we're sending custom content. default_email_hash = self._get_action_needed_reason_email_hash() current_email_hash = convert_string_to_sha256_hash(self.action_needed_reason_email) - if default_email_hash != current_email_hash: + if self.action_needed_reason_email and default_email_hash != current_email_hash: + print(f"sending custom email for: {current_email_hash}") email_template_name = "custom_email.txt" # Check for the "type" of action needed reason. @@ -902,7 +903,7 @@ class DomainRequest(TimeStampedModel): email_template_subject=f"emails/action_needed_reasons/{email_template_subject_name}", send_email=send_email, bcc_address=bcc_address, - custom_email_content=custom_email_content, + custom_email_content=email_content, wrap_email=True, ) @@ -919,7 +920,8 @@ class DomainRequest(TimeStampedModel): recipient = self.creator if flag_is_active(None, "profile_feature") else self.submitter # Return the content of the rendered views context = {"domain_request": self, "recipient": recipient} - body_text = template.render(context=context) + body_text = template.render(context=context).strip().lstrip("\n") + print(f"body: {body_text}") return convert_string_to_sha256_hash(body_text) @transition( diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index f35e2f619..75c2492c4 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -324,5 +324,7 @@ def convert_queryset_to_dict(queryset, is_model=True, key="id"): return request_dict -def convert_string_to_sha256_hash(string_to_convert): +def convert_string_to_sha256_hash(string_to_convert: str): + if not string_to_convert: + return None return hashlib.sha256(string_to_convert.encode('utf-8')).hexdigest() \ No newline at end of file diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 6fdaa3e3d..3d49dab02 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -45,6 +45,8 @@ def send_templated_email( template = get_template(template_name) email_body = template.render(context=context) + if email_body: + email_body.strip().lstrip("\n") subject_template = get_template(subject_template_name) subject = subject_template.render(context=context) From 55a74f63dd09478eb027c525669ef61c150cf0ef Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jul 2024 09:11:09 -0600 Subject: [PATCH 07/51] Cleanup --- src/registrar/models/domain_request.py | 18 +++++++----------- src/registrar/models/utility/generic_helper.py | 6 ------ 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 53c5765e9..5ec72aad5 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -868,11 +868,9 @@ class DomainRequest(TimeStampedModel): email_template_subject_name: str = "" # Check if the current email that we sent out is the same as our defaults. - # If these hashes differ, then that means that we're sending custom content. - default_email_hash = self._get_action_needed_reason_email_hash() - current_email_hash = convert_string_to_sha256_hash(self.action_needed_reason_email) - if self.action_needed_reason_email and default_email_hash != current_email_hash: - print(f"sending custom email for: {current_email_hash}") + # If these differ, then that means that we're sending custom content. + default_email = self.get_default_action_needed_reason_email(self.action_needed_reason) + if self.action_needed_reason_email and self.action_needed_reason_email != default_email: email_template_name = "custom_email.txt" # Check for the "type" of action needed reason. @@ -907,22 +905,20 @@ class DomainRequest(TimeStampedModel): wrap_email=True, ) - # TODO - rework this - def _get_action_needed_reason_email_hash(self): + def get_default_action_needed_reason_email(self, action_needed_reason): """Returns the default email associated with the given action needed reason""" - if self.action_needed_reason is None or self.action_needed_reason == self.ActionNeededReasons.OTHER: + if action_needed_reason is None or action_needed_reason == self.ActionNeededReasons.OTHER: return None # Get the email body - template_path = f"emails/action_needed_reasons/{self.action_needed_reason}.txt" + template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" template = get_template(template_path) recipient = self.creator if flag_is_active(None, "profile_feature") else self.submitter # Return the content of the rendered views context = {"domain_request": self, "recipient": recipient} body_text = template.render(context=context).strip().lstrip("\n") - print(f"body: {body_text}") - return convert_string_to_sha256_hash(body_text) + return body_text @transition( field="status", diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 75c2492c4..72dee8204 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -322,9 +322,3 @@ def convert_queryset_to_dict(queryset, is_model=True, key="id"): request_dict = {value[key]: value for value in queryset} return request_dict - - -def convert_string_to_sha256_hash(string_to_convert: str): - if not string_to_convert: - return None - return hashlib.sha256(string_to_convert.encode('utf-8')).hexdigest() \ No newline at end of file From 87c7a4dba282d46ea7d3ae7241f06a53d53be6b2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jul 2024 09:19:33 -0600 Subject: [PATCH 08/51] Update admin.py --- src/registrar/admin.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ae3a95740..a1ba6559d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1958,7 +1958,6 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # Call the superclass method with updated extra_context return super().change_view(request, object_id, form_url, extra_context) - # TODO - scrap this approach and just centralize everything def get_all_action_needed_reason_emails_as_json(self, domain_request): """Returns a json dictionary of every action needed reason and its associated email for this particular domain request.""" From 4bdf2cbe8f27c0e3712b44e9577b937b5c58607c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jul 2024 09:27:55 -0600 Subject: [PATCH 09/51] Fix imports --- src/registrar/assets/js/get-gov-admin.js | 1 - src/registrar/models/domain_request.py | 5 ++--- src/registrar/utility/email.py | 2 ++ 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 83c958566..2a01eb304 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -548,7 +548,6 @@ function initializeWidgetOnList(list, parentId) { let actionNeededEmails = JSON.parse(document.getElementById('action-needed-emails-data').textContent) let emailData = actionNeededEmails[reason]; if (emailData) { - // TODO: do we need a revert to default button? let emailBody = emailData.email_body_text if (emailBody) { actionNeededEmail.value = emailBody diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 7bb3a5a96..387840161 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -10,7 +10,7 @@ from django.utils import timezone from waffle import flag_is_active from registrar.models.domain import Domain from registrar.models.federal_agency import FederalAgency -from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper, convert_string_to_sha256_hash +from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes from registrar.utility.constants import BranchChoices @@ -627,8 +627,7 @@ class DomainRequest(TimeStampedModel): if was_already_action_needed and (reason_exists and reason_changed): # We don't send emails out in state "other" if self.action_needed_reason != self.ActionNeededReasons.OTHER: - _email_content = self.action_needed_reason_email - self._send_action_needed_reason_email(email_content=_email_content) + self._send_action_needed_reason_email(email_content=self.action_needed_reason_email) def sync_yes_no_form_fields(self): """Some yes/no forms use a db field to track whether it was checked or not. diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 3d49dab02..7f0b997fa 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -45,6 +45,8 @@ def send_templated_email( template = get_template(template_name) email_body = template.render(context=context) + + # Do cleanup on the email body. Mostly for emails with custom content. if email_body: email_body.strip().lstrip("\n") From 0641e9ad7255e068a73e3079cfaf6b891b9b1146 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Jul 2024 08:26:32 -0600 Subject: [PATCH 10/51] linting --- src/registrar/admin.py | 5 +++-- src/registrar/models/domain_request.py | 2 +- src/registrar/models/utility/generic_helper.py | 1 - src/registrar/tests/test_admin.py | 8 ++------ 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index a1ba6559d..850c385f0 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1746,7 +1746,6 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): obj.action_needed_reason_email = body_text should_save = True - if obj.status == original_obj.status: # If the status hasn't changed, let the base function take care of it return super().save_model(request, obj, form, change) @@ -1968,7 +1967,9 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: custom_text = domain_request.action_needed_reason_email - emails[enum_value] = self._get_action_needed_reason_default_email_text(domain_request, enum_value, custom_text) + emails[enum_value] = self._get_action_needed_reason_default_email_text( + domain_request, enum_value, custom_text + ) return json.dumps(emails) def _get_action_needed_reason_default_email_text(self, domain_request, action_needed_reason: str, custom_text=None): diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 387840161..632a16474 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -893,7 +893,7 @@ class DomainRequest(TimeStampedModel): # This is so you can override if you need, or have this taken care of for you. if not email_template_name: email_template_name = f"{self.action_needed_reason}.txt" - + if not email_template_subject_name: email_template_subject_name = f"{self.action_needed_reason}_subject.txt" diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 72dee8204..f9d4303c4 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -2,7 +2,6 @@ import time import logging -import hashlib from urllib.parse import urlparse, urlunparse, urlencode diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index f9a107dc3..fecc733e3 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1485,9 +1485,7 @@ class TestDomainRequestAdmin(MockEppLib): domain_request.save() self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=questionable_so) - self.assert_email_is_accurate( - "custom email content", 4, EMAIL, bcc_email_address=BCC_EMAIL - ) + self.assert_email_is_accurate("custom email content", 4, EMAIL, bcc_email_address=BCC_EMAIL) self.assertEqual(len(self.mock_client.EMAILS_SENT), 5) def test_save_model_sends_submitted_email(self): @@ -2234,6 +2232,7 @@ class TestDomainRequestAdmin(MockEppLib): self.assertContains(response, "Yes, select ineligible status") def test_readonly_when_restricted_creator(self): + self.maxDiff = None with less_console_noise(): domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW) with boto3_mocking.clients.handler_for("sesv2", self.mock_client): @@ -2252,7 +2251,6 @@ class TestDomainRequestAdmin(MockEppLib): "is_election_board", "federal_agency", "status_history", - "action_needed_reason_email", "id", "created_at", "updated_at", @@ -2314,7 +2312,6 @@ class TestDomainRequestAdmin(MockEppLib): "is_election_board", "federal_agency", "status_history", - "action_needed_reason_email", "creator", "about_your_organization", "requested_domain", @@ -2346,7 +2343,6 @@ class TestDomainRequestAdmin(MockEppLib): "is_election_board", "federal_agency", "status_history", - "action_needed_reason_email", ] self.assertEqual(readonly_fields, expected_fields) From 204b907a540586bcdab7e867546e228e70d58294 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Jul 2024 08:30:47 -0600 Subject: [PATCH 11/51] lint --- src/registrar/admin.py | 2 +- src/registrar/tests/test_admin.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 850c385f0..e50b262fb 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1984,7 +1984,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): if not custom_text: template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" else: - template_path = f"emails/action_needed_reasons/custom_email.txt" + template_path = "emails/action_needed_reasons/custom_email.txt" template = get_template(template_path) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index fecc733e3..c100477a6 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1464,7 +1464,7 @@ class TestDomainRequestAdmin(MockEppLib): ) self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) - # Test the email sent out for questionable_so + # Test that a custom email is sent out for questionable_so questionable_so = DomainRequest.ActionNeededReasons.QUESTIONABLE_SENIOR_OFFICIAL self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=questionable_so) self.assert_email_is_accurate( From f18b10d669a8c2b3d9db94bd798dfb20aa9191a8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Jul 2024 08:47:44 -0600 Subject: [PATCH 12/51] Add logic for if just the email is changed --- src/registrar/models/domain_request.py | 7 +++++-- src/registrar/tests/test_admin.py | 27 ++++++++++++++++++++++---- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 632a16474..5740d9504 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -598,6 +598,7 @@ class DomainRequest(TimeStampedModel): def _cache_status_and_action_needed_reason(self): """Maintains a cache of properties so we can avoid a DB call""" self._cached_action_needed_reason = self.action_needed_reason + self._cached_action_needed_reason_email = self.action_needed_reason_email self._cached_status = self.status def __init__(self, *args, **kwargs): @@ -614,7 +615,8 @@ class DomainRequest(TimeStampedModel): # Handle the action needed email. We send one when moving to action_needed, # but we don't send one when we are _already_ in the state and change the reason. - self.sync_action_needed_reason() + if self.status == self.DomainRequestStatus.ACTION_NEEDED and self.action_needed_reason: + self.sync_action_needed_reason() # Update the cached values after saving self._cache_status_and_action_needed_reason() @@ -624,7 +626,8 @@ class DomainRequest(TimeStampedModel): was_already_action_needed = self._cached_status == self.DomainRequestStatus.ACTION_NEEDED reason_exists = self._cached_action_needed_reason is not None and self.action_needed_reason is not None reason_changed = self._cached_action_needed_reason != self.action_needed_reason - if was_already_action_needed and (reason_exists and reason_changed): + reason_email_changed = self._cached_action_needed_reason_email != self.action_needed_reason_email + if was_already_action_needed and (reason_exists and reason_changed or reason_email_changed): # We don't send emails out in state "other" if self.action_needed_reason != self.ActionNeededReasons.OTHER: self._send_action_needed_reason_email(email_content=self.action_needed_reason_email) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index c100477a6..22bdc8558 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1376,7 +1376,9 @@ class TestDomainRequestAdmin(MockEppLib): self.assertContains(response, "status in [submitted,in review,action needed]", count=1) @less_console_noise_decorator - def transition_state_and_send_email(self, domain_request, status, rejection_reason=None, action_needed_reason=None): + def transition_state_and_send_email( + self, domain_request, status, rejection_reason=None, action_needed_reason=None, action_needed_reason_email=None + ): """Helper method for the email test cases.""" with boto3_mocking.clients.handler_for("sesv2", self.mock_client): @@ -1392,6 +1394,9 @@ class TestDomainRequestAdmin(MockEppLib): if action_needed_reason: domain_request.action_needed_reason = action_needed_reason + if action_needed_reason_email: + domain_request.action_needed_reason_email = action_needed_reason_email + # Use the model admin's save_model method self.admin.save_model(request, domain_request, form=None, change=True) @@ -1481,13 +1486,27 @@ class TestDomainRequestAdmin(MockEppLib): # Tests if an analyst can override existing email content questionable_so = DomainRequest.ActionNeededReasons.QUESTIONABLE_SENIOR_OFFICIAL - domain_request.action_needed_reason_email = "custom email content" - domain_request.save() - self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=questionable_so) + self.transition_state_and_send_email( + domain_request, + action_needed, + action_needed_reason=questionable_so, + action_needed_reason_email="custom email content", + ) self.assert_email_is_accurate("custom email content", 4, EMAIL, bcc_email_address=BCC_EMAIL) self.assertEqual(len(self.mock_client.EMAILS_SENT), 5) + # Tests if a new email gets sent when just the email is changed + self.transition_state_and_send_email( + domain_request, + action_needed, + action_needed_reason=questionable_so, + action_needed_reason_email="dummy email content", + ) + + self.assert_email_is_accurate("dummy email content", 5, EMAIL, bcc_email_address=BCC_EMAIL) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 6) + def test_save_model_sends_submitted_email(self): """When transitioning to submitted from started or withdrawn on a domain request, an email is sent out. From 425cfbcb4eae8b5b1e2a4ac2df5691a399ce9886 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Jul 2024 14:13:29 -0600 Subject: [PATCH 13/51] Logic refactor --- src/registrar/admin.py | 57 +++++++++++-------- src/registrar/assets/js/get-gov-admin.js | 6 ++ src/registrar/models/domain_request.py | 21 +------ .../admin/includes/detail_table_fieldset.html | 9 +++ 4 files changed, 48 insertions(+), 45 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e50b262fb..1561b7e1f 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1735,15 +1735,16 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # Get the original domain request from the database. original_obj = models.DomainRequest.objects.get(pk=obj.pk) - if obj.action_needed_reason: - text = self._get_action_needed_reason_default_email_text(obj, obj.action_needed_reason) + if obj.action_needed_reason and obj.action_needed_reason != obj.ActionNeededReasons.OTHER: + text = self._get_action_needed_reason_default_email(obj, obj.action_needed_reason) body_text = text.get("email_body_text") - if body_text: - body_text.strip().lstrip("\n") - is_default_email = body_text == obj.action_needed_reason_email - reason_changed = obj.action_needed_reason != original_obj.action_needed_reason - if is_default_email and reason_changed: - obj.action_needed_reason_email = body_text + reason_changed = obj.action_needed_reason != original_obj.action_needed_reason + is_default_email = body_text == obj.action_needed_reason_email + if body_text and is_default_email and reason_changed: + obj.action_needed_reason_email = body_text + should_save = True + elif not obj.action_needed_reason and obj.action_needed_reason_email: + obj.action_needed_reason_email = None should_save = True if obj.status == original_obj.status: @@ -1967,12 +1968,12 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: custom_text = domain_request.action_needed_reason_email - emails[enum_value] = self._get_action_needed_reason_default_email_text( + emails[enum_value] = self._get_action_needed_reason_default_email( domain_request, enum_value, custom_text ) return json.dumps(emails) - def _get_action_needed_reason_default_email_text(self, domain_request, action_needed_reason: str, custom_text=None): + def _get_action_needed_reason_default_email(self, domain_request, action_needed_reason: str, custom_text=None): """Returns the default email associated with the given action needed reason""" if action_needed_reason is None or action_needed_reason == domain_request.ActionNeededReasons.OTHER: return { @@ -1980,28 +1981,34 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): "email_body_text": None, } - # Get the email body - if not custom_text: - template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" - else: - template_path = "emails/action_needed_reasons/custom_email.txt" - - template = get_template(template_path) - - # Get the email subject - template_subject_path = f"emails/action_needed_reasons/{action_needed_reason}_subject.txt" - subject_template = get_template(template_subject_path) - if flag_is_active(None, "profile_feature"): # type: ignore recipient = domain_request.creator else: recipient = domain_request.submitter - # Return the content of the rendered views + # Return the context of the rendered views context = {"domain_request": domain_request, "recipient": recipient} + + # Get the email subject + template_subject_path = f"emails/action_needed_reasons/{action_needed_reason}_subject.txt" + subject_text = get_template(template_subject_path).render(context=context) + + # Get the email body + if not custom_text: + template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" + else: + template_path = "emails/action_needed_reasons/custom_email.txt" + context["custom_email_content"] = custom_text + + email_body_text = get_template(template_path).render(context=context) + + email_body_text_cleaned = None + if email_body_text: + email_body_text_cleaned = email_body_text.strip().lstrip("\n") + return { - "subject_text": subject_template.render(context=context), - "email_body_text": template.render(context=context) if not custom_text else custom_text, + "subject_text": subject_text, + "email_body_text": email_body_text_cleaned, } def process_log_entry(self, log_entry): diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 2a01eb304..3cf20311d 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -531,6 +531,12 @@ function initializeWidgetOnList(list, parentId) { if(actionNeededReasonDropdown && actionNeededEmail) { // Add a change listener to the action needed reason dropdown handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail); + + document.addEventListener('DOMContentLoaded', function() { + if (!actionNeededReasonDropdown.value || actionNeededReasonDropdown.value == "other") { + showNoEmailMessage(actionNeededEmail); + } + }); } function handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail) { diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 5740d9504..25138c54a 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -874,14 +874,10 @@ class DomainRequest(TimeStampedModel): def _send_action_needed_reason_email(self, send_email=True, email_content=None): """Sends out an automatic email for each valid action needed reason provided""" - # Store the filenames of the template and template subject email_template_name: str = "" email_template_subject_name: str = "" - # Check if the current email that we sent out is the same as our defaults. - # If these differ, then that means that we're sending custom content. - default_email = self.get_default_action_needed_reason_email(self.action_needed_reason) - if self.action_needed_reason_email and self.action_needed_reason_email != default_email: + if email_content is not None: email_template_name = "custom_email.txt" # Check for the "type" of action needed reason. @@ -916,21 +912,6 @@ class DomainRequest(TimeStampedModel): wrap_email=True, ) - def get_default_action_needed_reason_email(self, action_needed_reason): - """Returns the default email associated with the given action needed reason""" - if action_needed_reason is None or action_needed_reason == self.ActionNeededReasons.OTHER: - return None - - # Get the email body - template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" - template = get_template(template_path) - - recipient = self.creator if flag_is_active(None, "profile_feature") else self.submitter - # Return the content of the rendered views - context = {"domain_request": self, "recipient": recipient} - body_text = template.render(context=context).strip().lstrip("\n") - return body_text - @transition( field="status", source=[ diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 8b8748f80..2aa5c0a60 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -143,6 +143,15 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endwith %} {% endblock field_readonly %} +{% block field_other %} + {% if field.field.name == "action_needed_reason_email" %} + {{ field.field }} +
No email will be sent.
+ {% else %} + {{ field.field }} + {% endif %} +{% endblock field_other %} + {% block after_help_text %} {% if field.field.name == "action_needed_reason_email" %} {% comment %} From 91b9cb43795a61a6083b05a7f9a2ab220ee70466 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Jul 2024 14:29:36 -0600 Subject: [PATCH 14/51] Cleanup --- src/registrar/admin.py | 35 +++++++++++++++++--------- src/registrar/models/domain_request.py | 1 - 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 1561b7e1f..7d5708e62 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1735,17 +1735,12 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # Get the original domain request from the database. original_obj = models.DomainRequest.objects.get(pk=obj.pk) + # If the reason is in a state where we can send out an email, + # set the email to a default one if a custom email isn't provided. if obj.action_needed_reason and obj.action_needed_reason != obj.ActionNeededReasons.OTHER: - text = self._get_action_needed_reason_default_email(obj, obj.action_needed_reason) - body_text = text.get("email_body_text") - reason_changed = obj.action_needed_reason != original_obj.action_needed_reason - is_default_email = body_text == obj.action_needed_reason_email - if body_text and is_default_email and reason_changed: - obj.action_needed_reason_email = body_text - should_save = True - elif not obj.action_needed_reason and obj.action_needed_reason_email: + obj = self._handle_existing_action_needed_reason_email(obj, original_obj) + else: obj.action_needed_reason_email = None - should_save = True if obj.status == original_obj.status: # If the status hasn't changed, let the base function take care of it @@ -1759,6 +1754,24 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): if should_save: return super().save_model(request, obj, form, change) + def _handle_existing_action_needed_reason_email(self, obj, original_obj): + """ + Changes the action_needed_reason to the default email. + This occurs if the email changes and if it is empty. + """ + + # Get the default email + text = self._get_action_needed_reason_default_email(obj, obj.action_needed_reason) + body_text = text.get("email_body_text") + + # Set the action_needed_reason_email to the default + reason_changed = obj.action_needed_reason != original_obj.action_needed_reason + is_default_email = body_text == obj.action_needed_reason_email + if body_text and is_default_email and reason_changed: + obj.action_needed_reason_email = body_text + + return obj + def _handle_status_change(self, request, obj, original_obj): """ Checks for various conditions when a status change is triggered. @@ -1968,9 +1981,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: custom_text = domain_request.action_needed_reason_email - emails[enum_value] = self._get_action_needed_reason_default_email( - domain_request, enum_value, custom_text - ) + emails[enum_value] = self._get_action_needed_reason_default_email(domain_request, enum_value, custom_text) return json.dumps(emails) def _get_action_needed_reason_default_email(self, domain_request, action_needed_reason: str, custom_text=None): diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 25138c54a..1d26e0d2f 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1,7 +1,6 @@ from __future__ import annotations from typing import Union import logging -from django.template.loader import get_template from django.apps import apps from django.conf import settings from django.db import models From 72000b4a9b9b51e60d0156b66392b23bac905c06 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Jul 2024 14:44:52 -0600 Subject: [PATCH 15/51] Update email.py --- 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 7f0b997fa..e274893a2 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -46,7 +46,7 @@ def send_templated_email( template = get_template(template_name) email_body = template.render(context=context) - # Do cleanup on the email body. Mostly for emails with custom content. + # Do cleanup on the email body. For emails with custom content. if email_body: email_body.strip().lstrip("\n") From ee517768156c30254b7523f4a013c8ce0759e248 Mon Sep 17 00:00:00 2001 From: Rebecca HsiehNo email will be sent.
+ {% else %} + {{ field.field }} + {% endif %} +{% endblock field_other %} {% if field.field.name == "action_needed_reason_email" %} {{ field.field }}No email will be sent.
From 58730501a039ed3d59f4274a90ca9367ddc188f8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 09:49:39 -0600 Subject: [PATCH 30/51] Update detail_table_fieldset.html --- .../django/admin/includes/detail_table_fieldset.html | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index fffe15ff3..2aa5c0a60 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -144,13 +144,6 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endblock field_readonly %} {% block field_other %} - {% if field.field.name == "action_needed_reason_email" %} - {{ field.field }} -No email will be sent.
- {% else %} - {{ field.field }} - {% endif %} -{% endblock field_other %} {% if field.field.name == "action_needed_reason_email" %} {{ field.field }}No email will be sent.
From ed653e4562a099d805f9b1e35c37aa631983d782 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 11:11:23 -0600 Subject: [PATCH 31/51] add pr suggestion for spacing --- .../django/admin/includes/detail_table_fieldset.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 2aa5c0a60..f0ff7be3f 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -145,10 +145,10 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% block field_other %} {% if field.field.name == "action_needed_reason_email" %} - {{ field.field }} -No email will be sent.
+ {{ field.field }} +No email will be sent.
{% else %} - {{ field.field }} + {{ field.field }} {% endif %} {% endblock field_other %} From 025a128baef4c7a3fc08113df5a529d3ddd491c8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 11:54:27 -0600 Subject: [PATCH 32/51] Refactor --- src/registrar/admin.py | 68 +++++++++++------------- src/registrar/assets/js/get-gov-admin.js | 67 ++++++++++------------- 2 files changed, 58 insertions(+), 77 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7d5708e62..8f00ab519 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1731,46 +1731,37 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): if not change: return super().save_model(request, obj, form, change) - # == Handle non-status changes == # # Get the original domain request from the database. original_obj = models.DomainRequest.objects.get(pk=obj.pk) - # If the reason is in a state where we can send out an email, - # set the email to a default one if a custom email isn't provided. - if obj.action_needed_reason and obj.action_needed_reason != obj.ActionNeededReasons.OTHER: - obj = self._handle_existing_action_needed_reason_email(obj, original_obj) - else: + # == Handle action_needed_reason == # + # Store the email that was sent out if one was sent and it isn't saved to a variable yet + if not obj.action_needed_reason or obj.action_needed_reason == obj.ActionNeededReasons.OTHER: + # Reset the action needed email to none if we don't send out an email. obj.action_needed_reason_email = None + else: + # We send out an email -- store which one we send out. + # Set the email to a default one if a custom email isn't provided. + default_email = self._get_action_needed_reason_default_email(obj, obj.action_needed_reason) + body_text = default_email.get("email_body_text") + # Set the action_needed_reason_email to the default + reason_changed = obj.action_needed_reason != original_obj.action_needed_reason + is_default_email = body_text == obj.action_needed_reason_email + if body_text and is_default_email and reason_changed: + obj.action_needed_reason_email = body_text + + # == Handle status == # if obj.status == original_obj.status: # If the status hasn't changed, let the base function take care of it return super().save_model(request, obj, form, change) + else: + # Run some checks on the current object for invalid status changes + obj, should_save = self._handle_status_change(request, obj, original_obj) - # == Handle status changes == # - # Run some checks on the current object for invalid status changes - obj, should_save = self._handle_status_change(request, obj, original_obj) - - # We should only save if we don't display any errors in the steps above. - if should_save: - return super().save_model(request, obj, form, change) - - def _handle_existing_action_needed_reason_email(self, obj, original_obj): - """ - Changes the action_needed_reason to the default email. - This occurs if the email changes and if it is empty. - """ - - # Get the default email - text = self._get_action_needed_reason_default_email(obj, obj.action_needed_reason) - body_text = text.get("email_body_text") - - # Set the action_needed_reason_email to the default - reason_changed = obj.action_needed_reason != original_obj.action_needed_reason - is_default_email = body_text == obj.action_needed_reason_email - if body_text and is_default_email and reason_changed: - obj.action_needed_reason_email = body_text - - return obj + # We should only save if we don't display any errors in the steps above. + if should_save: + return super().save_model(request, obj, form, change) def _handle_status_change(self, request, obj, original_obj): """ @@ -1976,17 +1967,18 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): for this particular domain request.""" emails = {} for action_needed_reason in domain_request.ActionNeededReasons: - enum_value = action_needed_reason.value - custom_text = None - if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: - custom_text = domain_request.action_needed_reason_email + if action_needed_reason != DomainRequest.ActionNeededReasons.OTHER: + enum_value = action_needed_reason.value + custom_text = None + if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: + custom_text = domain_request.action_needed_reason_email - emails[enum_value] = self._get_action_needed_reason_default_email(domain_request, enum_value, custom_text) + emails[enum_value] = self._get_action_needed_reason_default_email(domain_request, enum_value, custom_text) return json.dumps(emails) - def _get_action_needed_reason_default_email(self, domain_request, action_needed_reason: str, custom_text=None): + def _get_action_needed_reason_default_email(self, domain_request, action_needed_reason, custom_text=None): """Returns the default email associated with the given action needed reason""" - if action_needed_reason is None or action_needed_reason == domain_request.ActionNeededReasons.OTHER: + if not action_needed_reason: return { "subject_text": None, "email_body_text": None, diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index f6aee5dbe..e41431ba3 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -528,65 +528,54 @@ function initializeWidgetOnList(list, parentId) { (function () { let actionNeededReasonDropdown = document.querySelector("#id_action_needed_reason"); let actionNeededEmail = document.querySelector("#id_action_needed_reason_email"); + let actionNeededEmailData = document.getElementById('action-needed-emails-data').textContent); let noEmailMessage = document.getElementById("no-email-message"); const emptyReasonText = "---------" const noEmailText = "No email will be sent." - if(actionNeededReasonDropdown && actionNeededEmail) { + if(actionNeededReasonDropdown && actionNeededEmail && actionNeededEmailData) { // Add a change listener to the action needed reason dropdown - handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail); + handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail, actionNeededEmailData); document.addEventListener('DOMContentLoaded', function() { - if (!actionNeededReasonDropdown.value) { - noEmailMessage.innerHTML = emptyReasonText - showNoEmailMessage(actionNeededEmail, noEmailMessage); - } else if (actionNeededReasonDropdown.value == "other") { - noEmailMessage.innerHTML = noEmailText + let reason = actionNeededReasonDropdown.value; + noEmailMessage.innerHTML = reason ? noEmailText : emptyReasonText; + if (reason && reason != "other") { + // Show the email + showElement(actionNeededEmail); + hideElement(noEmailMessage); + } else { + // Show the no email message + hideElement(actionNeededEmail); + showElement(noEmailMessage); } }); } - function handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail) { + function handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail, actionNeededEmailData) { actionNeededReasonDropdown.addEventListener("change", function() { let reason = actionNeededReasonDropdown.value; + let actionNeededEmailsJson = JSON.parse(actionNeededEmailData) - // If a reason isn't specified, no email will be sent. - // You also cannot save the model in this state. - // This flow occurs if you switch back to the empty picker state. - if(!reason) { - noEmailMessage.innerHTML = emptyReasonText - showNoEmailMessage(actionNeededEmail, noEmailMessage); - return; - }else if (reason === "other") { - noEmailMessage.innerHTML = noEmailText - } - - let actionNeededEmails = JSON.parse(document.getElementById('action-needed-emails-data').textContent) - let emailData = actionNeededEmails[reason]; - if (emailData) { + // Show the "no email will be sent" text only if a reason is actually selected. + noEmailMessage.innerHTML = reason ? noEmailText : emptyReasonText; + if (reason && reason in actionNeededEmailsJson) { + let emailData = actionNeededEmailsJson[reason]; let emailBody = emailData.email_body_text if (emailBody) { + // Show the email actionNeededEmail.value = emailBody - showActionNeededEmail(actionNeededEmail, noEmailMessage); + showElement(actionNeededEmail); + hideElement(noEmailMessage); }else { - showNoEmailMessage(actionNeededEmail, noEmailMessage); + // Show the no email message + hideElement(actionNeededEmail); + showElement(noEmailMessage); } }else { - showNoEmailMessage(actionNeededEmail, noEmailMessage); + // Show the no email message + hideElement(actionNeededEmail); + showElement(noEmailMessage); } - }); } - - // Show the text field. Hide the "no email" message. - function showActionNeededEmail(actionNeededEmail, noEmailMessage){ - showElement(actionNeededEmail); - hideElement(noEmailMessage); - } - - // Hide the text field. Show the "no email" message. - function showNoEmailMessage(actionNeededEmail, noEmailMessage) { - hideElement(actionNeededEmail); - showElement(noEmailMessage); - } - })(); From 6d63c9f868dc4881f0055115f62a5d73b27715e9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 11:59:36 -0600 Subject: [PATCH 33/51] Minor edits to not use kwargs and lint --- src/registrar/admin.py | 4 +++- src/registrar/models/domain_request.py | 25 ++++++++++++++++--------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8f00ab519..870fb900e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1973,7 +1973,9 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: custom_text = domain_request.action_needed_reason_email - emails[enum_value] = self._get_action_needed_reason_default_email(domain_request, enum_value, custom_text) + emails[enum_value] = self._get_action_needed_reason_default_email( + domain_request, enum_value, custom_text + ) return json.dumps(emails) def _get_action_needed_reason_default_email(self, domain_request, action_needed_reason, custom_text=None): diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 1d26e0d2f..f2f43be33 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -612,8 +612,8 @@ class DomainRequest(TimeStampedModel): super().save(*args, **kwargs) - # Handle the action needed email. We send one when moving to action_needed, - # but we don't send one when we are _already_ in the state and change the reason. + # Handle the action needed email. + # An email is sent out when action_needed_reason is changed or added. if self.status == self.DomainRequestStatus.ACTION_NEEDED and self.action_needed_reason: self.sync_action_needed_reason() @@ -683,7 +683,15 @@ class DomainRequest(TimeStampedModel): logger.error(f"Can't query an approved domain while attempting {called_from}") def _send_status_update_email( - self, new_status, email_template, email_template_subject, bcc_address="", context=None, **kwargs + self, + new_status, + email_template, + email_template_subject, + bcc_address="", + context=None, + send_email=True, + wrap_email=False, + custom_email_content=None, ): """Send a status update email to the creator. @@ -694,7 +702,11 @@ class DomainRequest(TimeStampedModel): If the waffle flag "profile_feature" is active, then this email will be sent to the domain request creator rather than the submitter - kwargs: + Optional args: + bcc_address: str -> the address to bcc to + + context: dict -> The context sent to the template + send_email: bool -> Used to bypass the send_templated_email function, in the event we just want to log that an email would have been sent, rather than actually sending one. @@ -704,11 +716,6 @@ class DomainRequest(TimeStampedModel): custom_email_content: str -> Renders an email with the content of this string as its body text. """ - # Email config options - wrap_email = kwargs.get("wrap_email", False) - send_email = kwargs.get("send_email", True) - custom_email_content = kwargs.get("custom_email_content", None) - recipient = self.creator if flag_is_active(None, "profile_feature") else self.submitter if recipient is None or recipient.email is None: logger.warning( From 8ff6d526525d20bfb9873c8b364e21afb7acd0f4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 12:12:04 -0600 Subject: [PATCH 34/51] Simplify logic further --- src/registrar/models/domain_request.py | 48 +++++++------------------- 1 file changed, 13 insertions(+), 35 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index f2f43be33..c8ec643e6 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -614,7 +614,7 @@ class DomainRequest(TimeStampedModel): # Handle the action needed email. # An email is sent out when action_needed_reason is changed or added. - if self.status == self.DomainRequestStatus.ACTION_NEEDED and self.action_needed_reason: + if self.action_needed_reason and self.status == self.DomainRequestStatus.ACTION_NEEDED: self.sync_action_needed_reason() # Update the cached values after saving @@ -625,8 +625,7 @@ class DomainRequest(TimeStampedModel): was_already_action_needed = self._cached_status == self.DomainRequestStatus.ACTION_NEEDED reason_exists = self._cached_action_needed_reason is not None and self.action_needed_reason is not None reason_changed = self._cached_action_needed_reason != self.action_needed_reason - reason_email_changed = self._cached_action_needed_reason_email != self.action_needed_reason_email - if was_already_action_needed and (reason_exists and reason_changed or reason_email_changed): + if was_already_action_needed and reason_exists and reason_changed: # We don't send emails out in state "other" if self.action_needed_reason != self.ActionNeededReasons.OTHER: self._send_action_needed_reason_email(email_content=self.action_needed_reason_email) @@ -880,43 +879,22 @@ class DomainRequest(TimeStampedModel): def _send_action_needed_reason_email(self, send_email=True, email_content=None): """Sends out an automatic email for each valid action needed reason provided""" - email_template_name: str = "" - email_template_subject_name: str = "" - - if email_content is not None: - email_template_name = "custom_email.txt" - - # Check for the "type" of action needed reason. - can_send_email = True - match self.action_needed_reason: - # Add to this match if you need to pass in a custom filename for these templates. - case self.ActionNeededReasons.OTHER, _: - # Unknown and other are default cases - do nothing - can_send_email = False - - # Assumes that the template name matches the action needed reason if nothing is specified. - # This is so you can override if you need, or have this taken care of for you. - if not email_template_name: - email_template_name = f"{self.action_needed_reason}.txt" - - if not email_template_subject_name: - email_template_subject_name = f"{self.action_needed_reason}_subject.txt" + email_template_name = "custom_email.txt" + email_template_subject_name = f"{self.action_needed_reason}_subject.txt" bcc_address = "" if settings.IS_PRODUCTION: bcc_address = settings.DEFAULT_FROM_EMAIL - # If we can, try to send out an email as long as send_email=True - if can_send_email: - self._send_status_update_email( - new_status="action needed", - email_template=f"emails/action_needed_reasons/{email_template_name}", - email_template_subject=f"emails/action_needed_reasons/{email_template_subject_name}", - send_email=send_email, - bcc_address=bcc_address, - custom_email_content=email_content, - wrap_email=True, - ) + self._send_status_update_email( + new_status="action needed", + email_template=f"emails/action_needed_reasons/{email_template_name}", + email_template_subject=f"emails/action_needed_reasons/{email_template_subject_name}", + send_email=send_email, + bcc_address=bcc_address, + custom_email_content=email_content, + wrap_email=True, + ) @transition( field="status", From 41835d275fd7bb701f215d08128660189b71b09d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 12:44:14 -0600 Subject: [PATCH 35/51] Further simplification --- src/registrar/admin.py | 58 +++++++----------------- src/registrar/assets/js/get-gov-admin.js | 2 +- 2 files changed, 17 insertions(+), 43 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 870fb900e..fd529b652 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1736,20 +1736,14 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # == Handle action_needed_reason == # # Store the email that was sent out if one was sent and it isn't saved to a variable yet - if not obj.action_needed_reason or obj.action_needed_reason == obj.ActionNeededReasons.OTHER: - # Reset the action needed email to none if we don't send out an email. - obj.action_needed_reason_email = None - else: - # We send out an email -- store which one we send out. - # Set the email to a default one if a custom email isn't provided. - default_email = self._get_action_needed_reason_default_email(obj, obj.action_needed_reason) - body_text = default_email.get("email_body_text") - - # Set the action_needed_reason_email to the default + default_email = self._get_action_needed_reason_default_email(obj, obj.action_needed_reason) + if default_email: + # Set the action_needed_reason_email to the default. + # Since this check occurs after save, if the user enters a value then + # we won't update. reason_changed = obj.action_needed_reason != original_obj.action_needed_reason - is_default_email = body_text == obj.action_needed_reason_email - if body_text and is_default_email and reason_changed: - obj.action_needed_reason_email = body_text + if reason_changed and default_email == obj.action_needed_reason_email: + obj.action_needed_reason_email = default_email # == Handle status == # if obj.status == original_obj.status: @@ -1967,24 +1961,16 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): for this particular domain request.""" emails = {} for action_needed_reason in domain_request.ActionNeededReasons: - if action_needed_reason != DomainRequest.ActionNeededReasons.OTHER: - enum_value = action_needed_reason.value - custom_text = None - if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: - custom_text = domain_request.action_needed_reason_email - - emails[enum_value] = self._get_action_needed_reason_default_email( - domain_request, enum_value, custom_text - ) + # Map the action_needed_reason to its default email + emails[action_needed_reason.value] = self._get_action_needed_reason_default_email( + domain_request, action_needed_reason.value + ) return json.dumps(emails) - def _get_action_needed_reason_default_email(self, domain_request, action_needed_reason, custom_text=None): + def _get_action_needed_reason_default_email(self, domain_request, action_needed_reason): """Returns the default email associated with the given action needed reason""" - if not action_needed_reason: - return { - "subject_text": None, - "email_body_text": None, - } + if not action_needed_reason or action_needed_reason == DomainRequest.ActionNeededReasons.OTHER: + return None if flag_is_active(None, "profile_feature"): # type: ignore recipient = domain_request.creator @@ -1994,27 +1980,15 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # Return the context of the rendered views context = {"domain_request": domain_request, "recipient": recipient} - # Get the email subject - template_subject_path = f"emails/action_needed_reasons/{action_needed_reason}_subject.txt" - subject_text = get_template(template_subject_path).render(context=context) - # Get the email body - if not custom_text: - template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" - else: - template_path = "emails/action_needed_reasons/custom_email.txt" - context["custom_email_content"] = custom_text + template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" email_body_text = get_template(template_path).render(context=context) - email_body_text_cleaned = None if email_body_text: email_body_text_cleaned = email_body_text.strip().lstrip("\n") - return { - "subject_text": subject_text, - "email_body_text": email_body_text_cleaned, - } + return email_body_text_cleaned def process_log_entry(self, log_entry): """Process a log entry and return filtered entry dictionary if applicable.""" diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index e41431ba3..b5df21538 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -528,7 +528,7 @@ function initializeWidgetOnList(list, parentId) { (function () { let actionNeededReasonDropdown = document.querySelector("#id_action_needed_reason"); let actionNeededEmail = document.querySelector("#id_action_needed_reason_email"); - let actionNeededEmailData = document.getElementById('action-needed-emails-data').textContent); + let actionNeededEmailData = document.getElementById('action-needed-emails-data').textContent; let noEmailMessage = document.getElementById("no-email-message"); const emptyReasonText = "---------" const noEmailText = "No email will be sent." From f6e82c7a989d2b27e5fab6a16ad01ed16d228fc2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 12:53:26 -0600 Subject: [PATCH 36/51] tests --- src/registrar/tests/test_admin.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 22bdc8558..34d64190f 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1496,7 +1496,8 @@ class TestDomainRequestAdmin(MockEppLib): self.assert_email_is_accurate("custom email content", 4, EMAIL, bcc_email_address=BCC_EMAIL) self.assertEqual(len(self.mock_client.EMAILS_SENT), 5) - # Tests if a new email gets sent when just the email is changed + # Tests if a new email gets sent when just the email is changed. + # An email should NOT be sent out if we just modify the email content. self.transition_state_and_send_email( domain_request, action_needed, @@ -1504,7 +1505,22 @@ class TestDomainRequestAdmin(MockEppLib): action_needed_reason_email="dummy email content", ) - self.assert_email_is_accurate("dummy email content", 5, EMAIL, bcc_email_address=BCC_EMAIL) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 5) + + # Set the request back to in review + domain_request.in_review() + + # no email was sent, so no email should be stored + self.assertEqual(domain_request.action_needed_reason_email, None) + + # Try sending another email when changing states AND including content + self.transition_state_and_send_email( + domain_request, + action_needed, + action_needed_reason=questionable_so, + action_needed_reason_email="custom content when starting anew", + ) + self.assert_email_is_accurate("custom content when starting anew", 5, EMAIL, bcc_email_address=BCC_EMAIL) self.assertEqual(len(self.mock_client.EMAILS_SENT), 6) def test_save_model_sends_submitted_email(self): From bac9dc94aa52aa1439b7658d45917b329e1a71f8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 12:59:28 -0600 Subject: [PATCH 37/51] Cleanup --- src/registrar/assets/js/get-gov-admin.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index b5df21538..e5aa495f5 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -530,7 +530,7 @@ function initializeWidgetOnList(list, parentId) { let actionNeededEmail = document.querySelector("#id_action_needed_reason_email"); let actionNeededEmailData = document.getElementById('action-needed-emails-data').textContent; let noEmailMessage = document.getElementById("no-email-message"); - const emptyReasonText = "---------" + const emptyReasonText = "-" const noEmailText = "No email will be sent." if(actionNeededReasonDropdown && actionNeededEmail && actionNeededEmailData) { // Add a change listener to the action needed reason dropdown @@ -558,9 +558,10 @@ function initializeWidgetOnList(list, parentId) { // Show the "no email will be sent" text only if a reason is actually selected. noEmailMessage.innerHTML = reason ? noEmailText : emptyReasonText; + console.log(`reaso: ${reason} vs in ${reason in actionNeededEmailsJson}`) + console.log(noEmailMessage) if (reason && reason in actionNeededEmailsJson) { - let emailData = actionNeededEmailsJson[reason]; - let emailBody = emailData.email_body_text + let emailBody = actionNeededEmailsJson[reason]; if (emailBody) { // Show the email actionNeededEmail.value = emailBody From 58605f11e5d7debd2ec700e7e7339b3bde9b3894 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 15:11:42 -0600 Subject: [PATCH 38/51] Readonly on send --- src/registrar/admin.py | 13 ++++- src/registrar/assets/js/get-gov-admin.js | 50 +++++++++++++------ .../admin/includes/detail_table_fieldset.html | 1 + 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index fd529b652..42d2872a2 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1742,8 +1742,11 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # Since this check occurs after save, if the user enters a value then # we won't update. reason_changed = obj.action_needed_reason != original_obj.action_needed_reason - if reason_changed and default_email == obj.action_needed_reason_email: - obj.action_needed_reason_email = default_email + if reason_changed: + request.session["action_needed_email_sent"] = True + logger.info("added session object") + if default_email == obj.action_needed_reason_email: + obj.action_needed_reason_email = default_email # == Handle status == # if obj.status == original_obj.status: @@ -1953,6 +1956,12 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): extra_context["action_needed_reason_emails"] = self.get_all_action_needed_reason_emails_as_json(obj) extra_context["has_profile_feature_flag"] = flag_is_active(request, "profile_feature") + # Denote if an action needed email was sent or not + email_sent = request.session.get("action_needed_email_sent", False) + extra_context["action_needed_email_sent"] = email_sent + if email_sent: + email_sent = request.session["action_needed_email_sent"] = False + # Call the superclass method with updated extra_context return super().change_view(request, object_id, form_url, extra_context) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index e5aa495f5..07722fdfc 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -36,6 +36,15 @@ function openInNewTab(el, removeAttribute = false){ } }; +// Adds or removes a boolean from our session +function addOrRemoveSessionBoolean(name, add){ + if (add) { + sessionStorage.setItem(name, "true"); + }else { + sessionStorage.removeItem(name); + } +} + // <<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>> // Event handlers. @@ -418,15 +427,6 @@ function initializeWidgetOnList(list, parentId) { object.classList.add("display-none"); } } - - // Adds or removes a boolean from our session - function addOrRemoveSessionBoolean(name, add){ - if (add) { - sessionStorage.setItem(name, "true"); - }else { - sessionStorage.removeItem(name); - } - } })(); /** An IIFE for toggling the submit bar on domain request forms @@ -529,9 +529,12 @@ function initializeWidgetOnList(list, parentId) { let actionNeededReasonDropdown = document.querySelector("#id_action_needed_reason"); let actionNeededEmail = document.querySelector("#id_action_needed_reason_email"); let actionNeededEmailData = document.getElementById('action-needed-emails-data').textContent; - let noEmailMessage = document.getElementById("no-email-message"); - const emptyReasonText = "-" - const noEmailText = "No email will be sent." + let noEmailMessage = document.querySelector("#no-email-message"); + const oldDropdownValue = actionNeededReasonDropdown ? actionNeededReasonDropdown.value : null; + const oldEmailValue = actionNeededEmailData ? actionNeededEmailData.value : null; + const emptyReasonText = "-"; + const noEmailText = "No email will be sent."; + const domainRequestId = actionNeededReasonDropdown ? document.querySelector("#domain_request_id").value : null if(actionNeededReasonDropdown && actionNeededEmail && actionNeededEmailData) { // Add a change listener to the action needed reason dropdown handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail, actionNeededEmailData); @@ -548,6 +551,16 @@ function initializeWidgetOnList(list, parentId) { hideElement(actionNeededEmail); showElement(noEmailMessage); } + + let emailWasSent = document.getElementById("action-needed-email-sent") + console.log(`email ${emailWasSent.value} vs session ${sessionStorage.getItem("actionNeededEmailSent")} vs id ${domainRequestId}`) + if (emailWasSent && emailWasSent.value) { + // add the session object + if (sessionStorage.getItem(`actionNeededEmailSent-${domainRequestId}`) === null) { + sessionStorage.setItem(`actionNeededEmailSent-${domainRequestId}`, domainRequestId); + } + actionNeededEmail.readOnly = true + } }); } @@ -558,8 +571,6 @@ function initializeWidgetOnList(list, parentId) { // Show the "no email will be sent" text only if a reason is actually selected. noEmailMessage.innerHTML = reason ? noEmailText : emptyReasonText; - console.log(`reaso: ${reason} vs in ${reason in actionNeededEmailsJson}`) - console.log(noEmailMessage) if (reason && reason in actionNeededEmailsJson) { let emailBody = actionNeededEmailsJson[reason]; if (emailBody) { @@ -567,6 +578,17 @@ function initializeWidgetOnList(list, parentId) { actionNeededEmail.value = emailBody showElement(actionNeededEmail); hideElement(noEmailMessage); + + // Reset the session object on change since change refreshes the email content. + // Only do this if we change the action needed reason, or if we: + // change the reason => modify email content => change back to old reason. + if (oldDropdownValue != actionNeededReasonDropdown.value || oldEmailValue != actionNeededEmail.value) { + let emailSent = sessionStorage.getItem(`actionNeededEmailSent-${domainRequestId}`) + if (emailSent !== null){ + sessionStorage.removeItem(`actionNeededEmailSent-${domainRequestId}`); + } + actionNeededEmail.readOnly = false; + } }else { // Show the no email message hideElement(actionNeededEmail); diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index f0ff7be3f..d477b55a7 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -147,6 +147,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% if field.field.name == "action_needed_reason_email" %} {{ field.field }}No email will be sent.
+ {% else %} {{ field.field }} {% endif %} From 2bb2af5089ee2c19484400239866626e405eaff0 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 15 Jul 2024 08:07:28 -0600 Subject: [PATCH 39/51] Readonly view --- src/registrar/assets/js/get-gov-admin.js | 21 +++++++++++++++++-- .../admin/includes/detail_table_fieldset.html | 20 ++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 07722fdfc..fbbea7003 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -559,7 +559,7 @@ function initializeWidgetOnList(list, parentId) { if (sessionStorage.getItem(`actionNeededEmailSent-${domainRequestId}`) === null) { sessionStorage.setItem(`actionNeededEmailSent-${domainRequestId}`, domainRequestId); } - actionNeededEmail.readOnly = true + hideReadonly(actionNeededEmail.parentElement) } }); } @@ -587,7 +587,7 @@ function initializeWidgetOnList(list, parentId) { if (emailSent !== null){ sessionStorage.removeItem(`actionNeededEmailSent-${domainRequestId}`); } - actionNeededEmail.readOnly = false; + showReadonly(actionNeededEmail.parentElement) } }else { // Show the no email message @@ -601,4 +601,21 @@ function initializeWidgetOnList(list, parentId) { } }); } + + function showReadonly(actionNeededEmailParent) { + let readonlyView = document.querySelector("#action-needed-reason-email-readonly") + if (readonlyView) { + hideElement(readonlyView) + showElement(actionNeededEmailParent) + } + } + + function hideReadonly(actionNeededEmailParent) { + let readonlyView = document.querySelector("#action-needed-reason-email-readonly") + if (readonlyView) { + showElement(readonlyView) + hideElement(actionNeededEmailParent) + } + } + })(); diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index d477b55a7..a23cd8f55 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -145,9 +145,29 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% block field_other %} {% if field.field.name == "action_needed_reason_email" %} +No email will be sent.
+No email will be sent.
+No email will be sent.