From 61b1ceeb9ce6c68d38de4e3ee7f31dccb1e33755 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 24 Sep 2024 14:22:25 -0600 Subject: [PATCH 01/34] Refactor --- src/registrar/admin.py | 67 +++-- src/registrar/assets/js/get-gov-admin.js | 280 ++++++++++++------ src/registrar/models/domain_request.py | 49 +-- .../custom_email.txt | 0 .../emails/includes/email_footer.txt | 10 + .../status_change_rejected_header.txt | 8 + .../contacts_not_verified.txt | 8 + .../rejection_reasons/naming_not_met.txt | 15 + .../rejection_reasons/org_has_domain.txt | 15 + .../rejection_reasons/org_not_eligible.txt | 14 + .../emails/rejection_reasons/other.txt | 15 + .../rejection_reasons/purpose_not_met.txt | 15 + .../requestor_not_eligible.txt | 14 + ..._subject.txt => status_change_subject.txt} | 0 src/registrar/utility/admin_helpers.py | 65 +++- src/registrar/views/utility/api_views.py | 2 +- 16 files changed, 437 insertions(+), 140 deletions(-) rename src/registrar/templates/emails/{action_needed_reasons => includes}/custom_email.txt (100%) create mode 100644 src/registrar/templates/emails/includes/email_footer.txt create mode 100644 src/registrar/templates/emails/includes/status_change_rejected_header.txt create mode 100644 src/registrar/templates/emails/rejection_reasons/contacts_not_verified.txt create mode 100644 src/registrar/templates/emails/rejection_reasons/naming_not_met.txt create mode 100644 src/registrar/templates/emails/rejection_reasons/org_has_domain.txt create mode 100644 src/registrar/templates/emails/rejection_reasons/org_not_eligible.txt create mode 100644 src/registrar/templates/emails/rejection_reasons/other.txt create mode 100644 src/registrar/templates/emails/rejection_reasons/purpose_not_met.txt create mode 100644 src/registrar/templates/emails/rejection_reasons/requestor_not_eligible.txt rename src/registrar/templates/emails/{status_change_rejected_subject.txt => status_change_subject.txt} (100%) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 15f1ccb79..b524f9d0a 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -21,7 +21,12 @@ from registrar.models.user_domain_role import UserDomainRole from waffle.admin import FlagAdmin from waffle.models import Sample, Switch -from registrar.utility.admin_helpers import get_all_action_needed_reason_emails, get_action_needed_reason_default_email +from registrar.utility.admin_helpers import ( + get_all_action_needed_reason_emails, + get_action_needed_reason_default_email, + get_all_rejection_reason_emails, + get_rejection_reason_default_email, +) from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website, SeniorOfficial from registrar.utility.constants import BranchChoices from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes @@ -234,6 +239,7 @@ class Meta: } labels = { "action_needed_reason_email": "Email", + "rejection_reason_email": "Email", } def __init__(self, *args, **kwargs): @@ -1755,6 +1761,7 @@ def status_history(self, obj): "status_history", "status", "rejection_reason", + "rejection_reason_email", "action_needed_reason", "action_needed_reason_email", "investigator", @@ -1938,24 +1945,16 @@ def save_model(self, request, obj, form, change): original_obj = models.DomainRequest.objects.get(pk=obj.pk) # == Handle action_needed_reason == # + action_needed_reason_changed = obj.action_needed_reason != original_obj.action_needed_reason + if action_needed_reason_changed: + obj = self._handle_action_needed_reason(request, obj, original_obj) - reason_changed = obj.action_needed_reason != original_obj.action_needed_reason - if reason_changed: - # Track the fact that we sent out an email - request.session["action_needed_email_sent"] = True - - # Set the action_needed_reason_email to the default if nothing exists. - # Since this check occurs after save, if the user enters a value then we won't update. - - default_email = get_action_needed_reason_default_email(request, obj, obj.action_needed_reason) - if obj.action_needed_reason_email: - emails = get_all_action_needed_reason_emails(request, obj) - is_custom_email = obj.action_needed_reason_email not in emails.values() - if not is_custom_email: - obj.action_needed_reason_email = default_email - else: - obj.action_needed_reason_email = default_email + # == Handle rejection_reason == # + rejection_reason_changed = obj.rejection_reason != original_obj.rejection_reason + if rejection_reason_changed: + obj = self._handle_rejection_reason(request, obj, original_obj) + # == Handle allowed emails == # if obj.status in DomainRequest.get_statuses_that_send_emails() and not settings.IS_PRODUCTION: self._check_for_valid_email(request, obj) @@ -1971,6 +1970,40 @@ def save_model(self, request, obj, form, change): if should_save: return super().save_model(request, obj, form, change) + def _handle_action_needed_reason(self, request, obj, original_obj): + # Track the fact that we sent out an email + request.session["action_needed_email_sent"] = True + + # Set the action_needed_reason_email to the default if nothing exists. + # Since this check occurs after save, if the user enters a value then we won't update. + + default_email = get_action_needed_reason_default_email(obj, obj.action_needed_reason) + if obj.action_needed_reason_email: + emails = get_all_action_needed_reason_emails(obj) + is_custom_email = obj.action_needed_reason_email not in emails.values() + if not is_custom_email: + obj.action_needed_reason_email = default_email + else: + obj.action_needed_reason_email = default_email + return obj + + def _handle_rejection_reason(self, request, obj, original_obj): + # Track the fact that we sent out an email + request.session["rejection_reason_email_sent"] = True + + # Set the rejection_reason_email to the default if nothing exists. + # Since this check occurs after save, if the user enters a value then we won't update. + + default_email = get_rejection_reason_default_email(obj, obj.action_needed_reason) + if obj.rejection_reason_email: + emails = get_all_rejection_reason_emails(obj) + is_custom_email = obj.rejection_reason_email not in emails.values() + if not is_custom_email: + obj.rejection_reason_email = default_email + else: + obj.rejection_reason_email = default_email + return obj + def _check_for_valid_email(self, request, obj): """Certain emails are whitelisted in non-production environments, so we should display that information using this function. diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 7e3c086c4..7a7b63ba4 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -500,83 +500,41 @@ function initializeWidgetOnList(list, parentId) { } })(); - -/** An IIFE that hooks to the show/hide button underneath action needed reason. - * This shows the auto generated email on action needed reason. -*/ -document.addEventListener('DOMContentLoaded', function() { - const dropdown = document.getElementById("id_action_needed_reason"); - const textarea = document.getElementById("id_action_needed_reason_email") - const domainRequestId = dropdown ? document.getElementById("domain_request_id").value : null - const textareaPlaceholder = document.querySelector(".field-action_needed_reason_email__placeholder"); - const directEditButton = document.querySelector('.field-action_needed_reason_email__edit'); - const modalTrigger = document.querySelector('.field-action_needed_reason_email__modal-trigger'); - const modalConfirm = document.getElementById('confirm-edit-email'); - const formLabel = document.querySelector('label[for="id_action_needed_reason_email"]'); - let lastSentEmailContent = document.getElementById("last-sent-email-content"); - const initialDropdownValue = dropdown ? dropdown.value : null; - const initialEmailValue = textarea.value; - - // We will use the const to control the modal - let isEmailAlreadySentConst = lastSentEmailContent.value.replace(/\s+/g, '') === textarea.value.replace(/\s+/g, ''); - // We will use the function to control the label and help - function isEmailAlreadySent() { - return lastSentEmailContent.value.replace(/\s+/g, '') === textarea.value.replace(/\s+/g, ''); - } - - if (!dropdown || !textarea || !domainRequestId || !formLabel || !modalConfirm) return; - const apiUrl = document.getElementById("get-action-needed-email-for-user-json").value; - - function updateUserInterface(reason) { - if (!reason) { - // No reason selected, we will set the label to "Email", show the "Make a selection" placeholder, hide the trigger, textarea, hide the help text - formLabel.innerHTML = "Email:"; - textareaPlaceholder.innerHTML = "Select an action needed reason to see email"; - showElement(textareaPlaceholder); - hideElement(directEditButton); - hideElement(modalTrigger); - hideElement(textarea); - } else if (reason === 'other') { - // 'Other' selected, we will set the label to "Email", show the "No email will be sent" placeholder, hide the trigger, textarea, hide the help text - formLabel.innerHTML = "Email:"; - textareaPlaceholder.innerHTML = "No email will be sent"; - showElement(textareaPlaceholder); - hideElement(directEditButton); - hideElement(modalTrigger); - hideElement(textarea); - } else { - // A triggering selection is selected, all hands on board: - textarea.setAttribute('readonly', true); - showElement(textarea); - hideElement(textareaPlaceholder); - - if (isEmailAlreadySentConst) { - hideElement(directEditButton); - showElement(modalTrigger); - } else { - showElement(directEditButton); - hideElement(modalTrigger); - } - if (isEmailAlreadySent()) { - formLabel.innerHTML = "Email sent to creator:"; - } else { - formLabel.innerHTML = "Email:"; - } +class CustomizableEmailBase { + constructor(dropdown, textarea, textareaPlaceholder, directEditButton, modalTrigger, modalConfirm, formLabel, lastSentEmailContent, apiUrl) { + this.dropdown = dropdown; + this.textarea = textarea; + this.textareaPlaceholder = textareaPlaceholder; + this.directEditButton = directEditButton; + this.modalTrigger = modalTrigger; + this.modalConfirm = modalConfirm; + this.formLabel = formLabel; + this.lastSentEmailContent = lastSentEmailContent; + this.apiUrl = apiUrl; + + this.domainRequestId = this.dropdown ? document.getElementById("domain_request_id").value : null + this.initialDropdownValue = this.dropdown ? this.dropdown.value : null; + this.initialEmailValue = this.textarea ? this.textarea.value : null; + + this.isEmailAlreadySentConst; + if (lastSentEmailContent && textarea) { + this.isEmailAlreadySentConst = lastSentEmailContent.value.replace(/\s+/g, '') === textarea.value.replace(/\s+/g, ''); } } - // Initialize UI - updateUserInterface(dropdown.value); - - dropdown.addEventListener("change", function() { - const reason = dropdown.value; - // Update the UI - updateUserInterface(reason); - if (reason && reason !== "other") { - // If it's not the initial value - if (initialDropdownValue !== dropdown.value || initialEmailValue !== textarea.value) { + initializeDropdown(errorMessage) { + this.dropdown.addEventListener("change", () => { + console.log(this.dropdown) + let reason = this.dropdown.value; + if (this.initialDropdownValue !== this.dropdown.value || this.initialEmailValue !== this.textarea.value) { + let searchParams = new URLSearchParams( + { + "reason": reason, + "domain_request_id": this.domainRequestId, + } + ); // Replace the email content - fetch(`${apiUrl}?reason=${reason}&domain_request_id=${domainRequestId}`) + fetch(`${this.apiUrl}?${searchParams.toString()}`) .then(response => { return response.json().then(data => data); }) @@ -584,30 +542,174 @@ document.addEventListener('DOMContentLoaded', function() { if (data.error) { console.error("Error in AJAX call: " + data.error); }else { - textarea.value = data.action_needed_email; + this.textarea.value = data.action_needed_email; } - updateUserInterface(reason); + this.updateUserInterface(reason); }) .catch(error => { - console.error("Error action needed email: ", error) + console.error(errorMessage, error) }); } + }); + } + + initializeModalConfirm() { + this.modalConfirm.addEventListener("click", () => { + this.textarea.removeAttribute('readonly'); + this.textarea.focus(); + hideElement(this.directEditButton); + hideElement(this.modalTrigger); + }); + } + + initializeDirectEditButton() { + this.directEditButton.addEventListener("click", () => { + this.textarea.removeAttribute('readonly'); + this.textarea.focus(); + hideElement(this.directEditButton); + hideElement(this.modalTrigger); + }); + } + + isEmailAlreadySent() { + return this.lastSentEmailContent.value.replace(/\s+/g, '') === this.textarea.value.replace(/\s+/g, ''); + } + + updateUserInterface(reason) { + if (!reason) { + // No reason selected, we will set the label to "Email", show the "Make a selection" placeholder, hide the trigger, textarea, hide the help text + this.showPlaceholder("Email:", "Select an action needed reason to see email"); + } else if (reason === 'other') { + // 'Other' selected, we will set the label to "Email", show the "No email will be sent" placeholder, hide the trigger, textarea, hide the help text + this.showPlaceholder("Email:", "No email will be sent"); + } else { + // A triggering selection is selected, all hands on board: + this.textarea.setAttribute('readonly', true); + showElement(this.textarea); + hideElement(this.textareaPlaceholder); + + if (this.isEmailAlreadySentConst) { + hideElement(this.directEditButton); + showElement(this.modalTrigger); + } else { + showElement(this.directEditButton); + hideElement(this.modalTrigger); + } + + if (this.isEmailAlreadySent()) { + this.formLabel.innerHTML = "Email sent to creator:"; + } else { + this.formLabel.innerHTML = "Email:"; + } } + } - }); + showPlaceholder(formLabelText, placeholderText) { + this.formLabel.innerHTML = formLabelText; + this.textareaPlaceholder.innerHTML = placeholderText; + showElement(this.textareaPlaceholder); + hideElement(this.directEditButton); + hideElement(this.modalTrigger); + hideElement(this.textarea); + } +} - modalConfirm.addEventListener("click", () => { - textarea.removeAttribute('readonly'); - textarea.focus(); - hideElement(directEditButton); - hideElement(modalTrigger); - }); - directEditButton.addEventListener("click", () => { - textarea.removeAttribute('readonly'); - textarea.focus(); - hideElement(directEditButton); - hideElement(modalTrigger); - }); + + +class customActionNeededEmail extends CustomizableEmailBase { + constructor() { + const dropdown = document.getElementById("id_action_needed_reason"); + const textarea = document.getElementById("id_action_needed_reason_email") + const textareaPlaceholder = document.querySelector(".field-action_needed_reason_email__placeholder"); + const directEditButton = document.querySelector('.field-action_needed_reason_email__edit'); + const modalTrigger = document.querySelector('.field-action_needed_reason_email__modal-trigger'); + const modalConfirm = document.getElementById('confirm-edit-email'); + const formLabel = document.querySelector('label[for="id_action_needed_reason_email"]'); + const lastSentEmailContent = document.getElementById("last-sent-email-content"); + + let apiContainer = document.getElementById("get-action-needed-email-for-user-json") + const apiUrl = apiContainer ? apiContainer.value : null; + super( + dropdown, + textarea, + textareaPlaceholder, + directEditButton, + modalTrigger, + modalConfirm, + formLabel, + lastSentEmailContent, + apiUrl + ); + } + + loadActionNeededEmail() { + this.updateUserInterface(this.dropdown.value); + this.initializeDropdown("Error when attempting to grab action needed email: ") + this.initializeModalConfirm() + this.initializeDirectEditButton() + } +} + +/** An IIFE that hooks to the show/hide button underneath action needed reason. + * This shows the auto generated email on action needed reason. +*/ +document.addEventListener('DOMContentLoaded', function() { + const customEmail = new customActionNeededEmail(); + if (!customEmail.dropdown || !customEmail.textarea || !customEmail.domainRequestId || !customEmail.formLabel || !customEmail.modalConfirm){ + return; + } + + // Initialize UI + customEmail.loadActionNeededEmail() +}); + + +class customRejectedEmail extends CustomizableEmailBase { + constructor() { + const dropdown = document.getElementById("id_rejection_reason"); + const textarea = document.getElementById("id_rejection_reason_email") + const textareaPlaceholder = document.querySelector(".field-rejection_reason_email__placeholder"); + const directEditButton = document.querySelector('.field-rejection_reason_email__edit'); + const modalTrigger = document.querySelector('.field-rejection_reason_email__modal-trigger'); + const modalConfirm = document.getElementById('confirm-edit-email'); + const formLabel = document.querySelector('label[for="id_rejection_reason_email"]'); + const lastSentEmailContent = document.getElementById("last-sent-email-content"); + + let apiContainer = document.getElementById("get-rejection-reason-email-for-user-json") + const apiUrl = apiContainer ? apiContainer.value : null; + super( + dropdown, + textarea, + textareaPlaceholder, + directEditButton, + modalTrigger, + modalConfirm, + formLabel, + lastSentEmailContent, + apiUrl + ); + } + + loadRejectedEmail() { + this.updateUserInterface(this.dropdown.value); + this.initializeDropdown("Error when attempting to grab rejected email: ") + this.initializeModalConfirm() + this.initializeDirectEditButton() + } +} + + +/** An IIFE that hooks to the show/hide button underneath rejected reason. + * This shows the auto generated email on action needed reason. +*/ +document.addEventListener('DOMContentLoaded', function() { + const customEmail = new customRejectedEmail(); + if (!customEmail.dropdown || !customEmail.textarea || !customEmail.domainRequestId || !customEmail.formLabel || !customEmail.modalConfirm){ + return; + } + + // Initialize UI + customEmail.loadRejectedEmail() }); diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index bb8693ac1..b1cc00bde 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -300,6 +300,11 @@ def get_action_needed_reason_label(cls, action_needed_reason: str): blank=True, ) + rejection_reason_email = models.TextField( + null=True, + blank=True, + ) + action_needed_reason = models.TextField( choices=ActionNeededReasons.choices, null=True, @@ -798,6 +803,21 @@ def _send_status_update_email( except EmailSendingError: logger.warning("Failed to send confirmation email", exc_info=True) + def _send_custom_status_update_email(self, email_content): + """Wrapper for `_send_status_update_email` that bcc's help@get.gov + and sends an email equivalent to the 'email_content' variable.""" + if settings.IS_PRODUCTION: + bcc_address = settings.DEFAULT_FROM_EMAIL + + self._send_status_update_email( + new_status="action needed", + email_template=f"emails/includes/custom_email.txt", + email_template_subject=f"emails/status_change_subject.txt", + bcc_address=bcc_address, + custom_email_content=email_content, + wrap_email=True, + ) + def investigator_exists_and_is_staff(self): """Checks if the current investigator is in a valid state for a state transition""" is_valid = True @@ -901,7 +921,7 @@ def in_review(self): target=DomainRequestStatus.ACTION_NEEDED, conditions=[domain_is_not_active, investigator_exists_and_is_staff], ) - def action_needed(self, send_email=True): + def action_needed(self): """Send back an domain request that is under investigation or rejected. This action is logged. @@ -924,27 +944,7 @@ def action_needed(self, send_email=True): # Send out an email if an action needed reason exists if self.action_needed_reason and self.action_needed_reason != self.ActionNeededReasons.OTHER: 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, email_content=None): - """Sends out an automatic email for each valid action needed reason provided""" - - 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 - - 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_custom_status_update_email(email_content) @transition( field="status", @@ -1051,6 +1051,11 @@ def reject(self): "emails/status_change_rejected_subject.txt", ) + # Send out an email if a rejection reason exists + if self.rejection_reason: + email_content = self.rejection_reason_email + self._send_custom_status_update_email(email_content) + @transition( field="status", source=[ diff --git a/src/registrar/templates/emails/action_needed_reasons/custom_email.txt b/src/registrar/templates/emails/includes/custom_email.txt similarity index 100% rename from src/registrar/templates/emails/action_needed_reasons/custom_email.txt rename to src/registrar/templates/emails/includes/custom_email.txt diff --git a/src/registrar/templates/emails/includes/email_footer.txt b/src/registrar/templates/emails/includes/email_footer.txt new file mode 100644 index 000000000..f10d82a91 --- /dev/null +++ b/src/registrar/templates/emails/includes/email_footer.txt @@ -0,0 +1,10 @@ +THANK YOU +.Gov helps the public identify official, trusted information. Thank you for requesting a .gov domain. + +---------------------------------------------------------------- + +The .gov team +Contact us: +Learn about .gov + +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) \ No newline at end of file diff --git a/src/registrar/templates/emails/includes/status_change_rejected_header.txt b/src/registrar/templates/emails/includes/status_change_rejected_header.txt new file mode 100644 index 000000000..16b7c73a9 --- /dev/null +++ b/src/registrar/templates/emails/includes/status_change_rejected_header.txt @@ -0,0 +1,8 @@ +Hi, {{ recipient.first_name }}. + +Your .gov domain request has been rejected. + +DOMAIN REQUESTED: {{ domain_request.requested_domain.name }} +REQUEST RECEIVED ON: {{ domain_request.last_submitted_date|date }} +STATUS: Rejected +---------------------------------------------------------------- \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/contacts_not_verified.txt b/src/registrar/templates/emails/rejection_reasons/contacts_not_verified.txt new file mode 100644 index 000000000..c35c82c2b --- /dev/null +++ b/src/registrar/templates/emails/rejection_reasons/contacts_not_verified.txt @@ -0,0 +1,8 @@ +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} +{% include "emails/includes/status_change_rejected_header.html" %} +REJECTION REASON +Your domain request was rejected because we could not verify the organizational +contacts you provided. If you have questions or comments, reply to this email. + +{% include "emails/includes/email_footer.html" %} +{% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/naming_not_met.txt b/src/registrar/templates/emails/rejection_reasons/naming_not_met.txt new file mode 100644 index 000000000..3e57d579d --- /dev/null +++ b/src/registrar/templates/emails/rejection_reasons/naming_not_met.txt @@ -0,0 +1,15 @@ +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} +{% include "emails/includes/status_change_rejected_header.html" %} +REJECTION REASON +Your domain request was rejected because it does not meet our naming requirements. +Domains should uniquely identify a government organization and be clear to the +general public. Learn more about naming requirements for your type of organization +. + + +YOU CAN SUBMIT A NEW REQUEST +We encourage you to request a domain that meets our requirements. If you have +questions or want to discuss potential domain names, reply to this email. + +{% include "emails/includes/email_footer.html" %} +{% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/org_has_domain.txt b/src/registrar/templates/emails/rejection_reasons/org_has_domain.txt new file mode 100644 index 000000000..26757efd6 --- /dev/null +++ b/src/registrar/templates/emails/rejection_reasons/org_has_domain.txt @@ -0,0 +1,15 @@ +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} +{% include "emails/includes/status_change_rejected_header.html" %} +REJECTION REASON +Your domain request was rejected because {{ domain_request.organization_name }} has a .gov domain. Our +practice is to approve one domain per online service per government organization. We +evaluate additional requests on a case-by-case basis. You did not provide sufficient +justification for an additional domain. + +Read more about our practice of approving one domain per online service +. + +If you have questions or comments, reply to this email. + +{% include "emails/includes/email_footer.html" %} +{% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/org_not_eligible.txt b/src/registrar/templates/emails/rejection_reasons/org_not_eligible.txt new file mode 100644 index 000000000..3c7de3f42 --- /dev/null +++ b/src/registrar/templates/emails/rejection_reasons/org_not_eligible.txt @@ -0,0 +1,14 @@ +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} +{% include "emails/includes/status_change_rejected_header.html" %} +REJECTION REASON +Your domain request was rejected because we determined that {{ domain_request.organization_name }} is not +eligible for a .gov domain. .Gov domains are only available to official U.S.-based +government organizations. + +Learn more about eligibility for .gov domains +. + +If you have questions or comments, reply to this email. + +{% include "emails/includes/email_footer.html" %} +{% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/other.txt b/src/registrar/templates/emails/rejection_reasons/other.txt new file mode 100644 index 000000000..6835a45e0 --- /dev/null +++ b/src/registrar/templates/emails/rejection_reasons/other.txt @@ -0,0 +1,15 @@ +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} +{% include "emails/includes/status_change_rejected_header.html" %} +YOU CAN SUBMIT A NEW REQUEST +If your organization is eligible for a .gov domain and you meet our other requirements, you can submit a new request. + +Learn more about: +- Eligibility for a .gov domain +- Choosing a .gov domain name + + +NEED ASSISTANCE? +If you have questions about this domain request or need help choosing a new domain name, reply to this email. + +{% include "emails/includes/email_footer.html" %} +{% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/purpose_not_met.txt b/src/registrar/templates/emails/rejection_reasons/purpose_not_met.txt new file mode 100644 index 000000000..57bce78f0 --- /dev/null +++ b/src/registrar/templates/emails/rejection_reasons/purpose_not_met.txt @@ -0,0 +1,15 @@ +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} +{% include "emails/includes/status_change_rejected_header.html" %} +REJECTION REASON +Your domain request was rejected because the purpose you provided did not meet our +requirements. You didn’t provide enough information about how you intend to use the +domain. + +Learn more about: +- Eligibility for a .gov domain +- What you can and can’t do with .gov domains + +If you have questions or comments, reply to this email. + +{% include "emails/includes/email_footer.html" %} +{% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/requestor_not_eligible.txt b/src/registrar/templates/emails/rejection_reasons/requestor_not_eligible.txt new file mode 100644 index 000000000..7974c1690 --- /dev/null +++ b/src/registrar/templates/emails/rejection_reasons/requestor_not_eligible.txt @@ -0,0 +1,14 @@ +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} +{% include "emails/includes/status_change_rejected_header.html" %} +REJECTION REASON +Your domain request was rejected because we don’t believe you’re eligible to request a +.gov domain on behalf of {{ domain_request.organization_name }}. You must be a government employee, or be +working on behalf of a government organization, to request a .gov domain. + + +DEMONSTRATE ELIGIBILITY +If you can provide more information that demonstrates your eligibility, or you want to +discuss further, reply to this email. + +{% include "emails/includes/email_footer.html" %} +{% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/status_change_rejected_subject.txt b/src/registrar/templates/emails/status_change_subject.txt similarity index 100% rename from src/registrar/templates/emails/status_change_rejected_subject.txt rename to src/registrar/templates/emails/status_change_subject.txt diff --git a/src/registrar/utility/admin_helpers.py b/src/registrar/utility/admin_helpers.py index 0b99bba13..87edc2106 100644 --- a/src/registrar/utility/admin_helpers.py +++ b/src/registrar/utility/admin_helpers.py @@ -2,23 +2,66 @@ from django.template.loader import get_template -def get_all_action_needed_reason_emails(request, domain_request): +def get_all_action_needed_reason_emails(domain_request): """Returns a dictionary of every action needed reason and its associated email for this particular domain request.""" + return _get_all_default_emails( + reasons=DomainRequest.ActionNeededReasons, + # Where the emails are stored. This assumes that it contains a list of .txt files with the reason. + path_root="emails/action_needed_reasons", + # What reasons don't send out emails (none is handled automagically) + excluded_reasons=[DomainRequest.ActionNeededReasons.OTHER], + # Who to send it to, and from what domain request to reference + domain_request=domain_request + ) + +def get_action_needed_reason_default_email(domain_request, action_needed_reason): + """Returns the default email associated with the given action needed reason""" + return _get_default_email( + domain_request, + path_root="emails/rejection_reasons", + reason=action_needed_reason, + excluded_reasons=[DomainRequest.ActionNeededReasons.OTHER] + ) + + +def get_all_rejection_reason_emails(domain_request): + """Returns a dictionary of every rejection reason and its associated email + for this particular domain request.""" + return _get_all_default_emails( + reasons=DomainRequest.RejectionReasons, + # Where the emails are stored. This assumes that it contains a list of .txt files with the reason. + path_root="emails/rejection_reasons", + # What reasons don't send out emails (none is handled automagically) + excluded_reasons=[DomainRequest.RejectionReasons.OTHER], + # Who to send it to, and from what domain request to reference + domain_request=domain_request + ) + + +def get_rejection_reason_default_email(domain_request, action_needed_reason): + """Returns the default email associated with the given rejection reason""" + return _get_default_email( + domain_request, + path_root="emails/rejection_reasons", + reason=action_needed_reason, + excluded_reasons=[DomainRequest.RejectionReasons.OTHER] + ) + +def _get_all_default_emails(reasons, path_root, excluded_reasons, domain_request): emails = {} - for action_needed_reason in domain_request.ActionNeededReasons: - # Map the action_needed_reason to its default email - emails[action_needed_reason.value] = get_action_needed_reason_default_email( - request, domain_request, action_needed_reason.value + for reason in reasons: + # Map the reason to its default email + emails[reason.value] = _get_default_email( + domain_request, path_root, reason, excluded_reasons ) - return emails - +def _get_default_email(domain_request, path_root, reason, excluded_reasons=None): + if not reason: + return None -def get_action_needed_reason_default_email(request, domain_request, action_needed_reason): - """Returns the default email associated with the given action needed reason""" - if not action_needed_reason or action_needed_reason == DomainRequest.ActionNeededReasons.OTHER: + if excluded_reasons and reason in excluded_reasons: return None recipient = domain_request.creator @@ -26,7 +69,7 @@ def get_action_needed_reason_default_email(request, domain_request, action_neede context = {"domain_request": domain_request, "recipient": recipient} # Get the email body - template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" + template_path = f"{path_root}/{reason}.txt" email_body_text = get_template(template_path).render(context=context) email_body_text_cleaned = None diff --git a/src/registrar/views/utility/api_views.py b/src/registrar/views/utility/api_views.py index 6a6269baa..973f85855 100644 --- a/src/registrar/views/utility/api_views.py +++ b/src/registrar/views/utility/api_views.py @@ -90,5 +90,5 @@ def get_action_needed_email_for_user_json(request): return JsonResponse({"error": "No domain_request_id specified"}, status=404) domain_request = DomainRequest.objects.filter(id=domain_request_id).first() - emails = get_all_action_needed_reason_emails(request, domain_request) + emails = get_all_action_needed_reason_emails(domain_request) return JsonResponse({"action_needed_email": emails.get(reason)}, status=200) From 48b9206ffc2ff992361c49f9a549defd37471057 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 27 Sep 2024 09:07:15 -0600 Subject: [PATCH 02/34] migration --- ...129_domainrequest_rejection_reason_email.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 src/registrar/migrations/0129_domainrequest_rejection_reason_email.py diff --git a/src/registrar/migrations/0129_domainrequest_rejection_reason_email.py b/src/registrar/migrations/0129_domainrequest_rejection_reason_email.py new file mode 100644 index 000000000..6aaef7f87 --- /dev/null +++ b/src/registrar/migrations/0129_domainrequest_rejection_reason_email.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.10 on 2024-09-26 21:18 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0128_alter_domaininformation_state_territory_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="domainrequest", + name="rejection_reason_email", + field=models.TextField(blank=True, null=True), + ), + ] From 9b23262d61c094e307e2106e41e7f664f9fc0732 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 27 Sep 2024 11:34:45 -0600 Subject: [PATCH 03/34] Initial architecture for rejection reason --- src/registrar/admin.py | 2 +- src/registrar/assets/js/get-gov-admin.js | 149 +++++++++++++++--- src/registrar/config/urls.py | 6 + src/registrar/models/domain_request.py | 56 ++++--- .../admin/domain_request_change_form.html | 2 + .../admin/includes/detail_table_fieldset.html | 88 +++++++++++ .../contacts_not_verified.txt | 4 +- .../rejection_reasons/naming_not_met.txt | 4 +- .../rejection_reasons/org_has_domain.txt | 4 +- .../rejection_reasons/org_not_eligible.txt | 4 +- .../emails/rejection_reasons/other.txt | 15 -- .../rejection_reasons/purpose_not_met.txt | 4 +- .../requestor_not_eligible.txt | 4 +- src/registrar/utility/admin_helpers.py | 15 +- src/registrar/views/utility/api_views.py | 31 +++- 15 files changed, 306 insertions(+), 82 deletions(-) delete mode 100644 src/registrar/templates/emails/rejection_reasons/other.txt diff --git a/src/registrar/admin.py b/src/registrar/admin.py index b524f9d0a..a5066d3b0 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1994,7 +1994,7 @@ def _handle_rejection_reason(self, request, obj, original_obj): # Set the rejection_reason_email to the default if nothing exists. # Since this check occurs after save, if the user enters a value then we won't update. - default_email = get_rejection_reason_default_email(obj, obj.action_needed_reason) + default_email = get_rejection_reason_default_email(obj, obj.rejection_reason) if obj.rejection_reason_email: emails = get_all_rejection_reason_emails(obj) is_custom_email = obj.rejection_reason_email not in emails.values() diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 7a7b63ba4..95b28660f 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -348,7 +348,7 @@ function initializeWidgetOnList(list, parentId) { * status select and to show/hide the rejection reason */ (function (){ - let rejectionReasonFormGroup = document.querySelector('.field-rejection_reason') + let rejectionReasonFormGroup = document.querySelector('.field-rejection_reason'); // This is the "action needed reason" field let actionNeededReasonFormGroup = document.querySelector('.field-action_needed_reason'); // This is the "Email" field @@ -501,7 +501,7 @@ function initializeWidgetOnList(list, parentId) { })(); class CustomizableEmailBase { - constructor(dropdown, textarea, textareaPlaceholder, directEditButton, modalTrigger, modalConfirm, formLabel, lastSentEmailContent, apiUrl) { + constructor(dropdown, textarea, textareaPlaceholder, directEditButton, modalTrigger, modalConfirm, formLabel, lastSentEmailContent, apiUrl, textAreaFormGroup, dropdownFormGroup) { this.dropdown = dropdown; this.textarea = textarea; this.textareaPlaceholder = textareaPlaceholder; @@ -512,6 +512,11 @@ class CustomizableEmailBase { this.lastSentEmailContent = lastSentEmailContent; this.apiUrl = apiUrl; + // These fields are hidden on pageload + this.textAreaFormGroup = textAreaFormGroup; + this.dropdownFormGroup = dropdownFormGroup; + this.statusSelect = document.getElementById("id_status"); + this.domainRequestId = this.dropdown ? document.getElementById("domain_request_id").value : null this.initialDropdownValue = this.dropdown ? this.dropdown.value : null; this.initialEmailValue = this.textarea ? this.textarea.value : null; @@ -520,11 +525,47 @@ class CustomizableEmailBase { if (lastSentEmailContent && textarea) { this.isEmailAlreadySentConst = lastSentEmailContent.value.replace(/\s+/g, '') === textarea.value.replace(/\s+/g, ''); } + + } + + // Handle showing/hiding the related fields on page load. + initializeFormGroups(statusToCheck, sessionVariableName) { + let isStatus = statusSelect.value == statusToCheck; + + // Initial handling of these groups. + updateFormGroupVisibility(isStatus, isStatus); + + // Listen to change events and handle rejectionReasonFormGroup display, then save status to session storage + this.statusSelect.addEventListener('change', () => { + // Show the action needed field if the status is what we expect. + // Then track if its shown or hidden in our session cache. + isStatus = statusSelect.value == statusToCheck; + updateFormGroupVisibility(isStatus, isStatus); + addOrRemoveSessionBoolean(sessionVariableName, add=isStatus); + }); + + // Listen to Back/Forward button navigation and handle rejectionReasonFormGroup display based on session storage + // When you navigate using forward/back after changing status but not saving, when you land back on the DA page the + // status select will say (for example) Rejected but the selected option can be something else. To manage the show/hide + // accurately for this edge case, we use cache and test for the back/forward navigation. + const observer = new PerformanceObserver((list) => { + list.getEntries().forEach((entry) => { + if (entry.type === "back_forward") { + let showTextAreaFormGroup = sessionStorage.getItem(sessionVariableName) !== null; + updateFormGroupVisibility(showTextAreaFormGroup, isStatus); + } + }); + }); + observer.observe({ type: "navigation" }); + } + + updateFormGroupVisibility(showTextAreaFormGroup, showdropDownFormGroup) { + showTextAreaFormGroup ? showElement(this.textAreaFormGroup) : hideElement(this.textAreaFormGroup); + showdropDownFormGroup ? showElement(this.dropdownFormGroup) : hideElement(this.dropdownFormGroup); } initializeDropdown(errorMessage) { this.dropdown.addEventListener("change", () => { - console.log(this.dropdown) let reason = this.dropdown.value; if (this.initialDropdownValue !== this.dropdown.value || this.initialEmailValue !== this.textarea.value) { let searchParams = new URLSearchParams( @@ -542,7 +583,7 @@ class CustomizableEmailBase { if (data.error) { console.error("Error in AJAX call: " + data.error); }else { - this.textarea.value = data.action_needed_email; + this.textarea.value = data.email; } this.updateUserInterface(reason); }) @@ -578,32 +619,47 @@ class CustomizableEmailBase { updateUserInterface(reason) { if (!reason) { // No reason selected, we will set the label to "Email", show the "Make a selection" placeholder, hide the trigger, textarea, hide the help text - this.showPlaceholder("Email:", "Select an action needed reason to see email"); + this.showPlaceholderNoReason(); } else if (reason === 'other') { // 'Other' selected, we will set the label to "Email", show the "No email will be sent" placeholder, hide the trigger, textarea, hide the help text - this.showPlaceholder("Email:", "No email will be sent"); + this.showPlaceholderOtherReason(); } else { - // A triggering selection is selected, all hands on board: - this.textarea.setAttribute('readonly', true); - showElement(this.textarea); - hideElement(this.textareaPlaceholder); - - if (this.isEmailAlreadySentConst) { - hideElement(this.directEditButton); - showElement(this.modalTrigger); - } else { - showElement(this.directEditButton); - hideElement(this.modalTrigger); - } + this.showReadonlyTextarea(); + } + } - if (this.isEmailAlreadySent()) { - this.formLabel.innerHTML = "Email sent to creator:"; - } else { - this.formLabel.innerHTML = "Email:"; - } + // Helper function that makes overriding the readonly textarea easy + showReadonlyTextarea() { + // A triggering selection is selected, all hands on board: + this.textarea.setAttribute('readonly', true); + showElement(this.textarea); + hideElement(this.textareaPlaceholder); + + if (this.isEmailAlreadySentConst) { + hideElement(this.directEditButton); + showElement(this.modalTrigger); + } else { + showElement(this.directEditButton); + hideElement(this.modalTrigger); + } + + if (this.isEmailAlreadySent()) { + this.formLabel.innerHTML = "Email sent to creator:"; + } else { + this.formLabel.innerHTML = "Email:"; } } + // Helper function that makes overriding the placeholder reason easy + showPlaceholderNoReason() { + this.showPlaceholder("Email:", "Select a reason to see email"); + } + + // Helper function that makes overriding the placeholder reason easy + showPlaceholderOtherReason() { + this.showPlaceholder("Email:", "No email will be sent"); + } + showPlaceholder(formLabelText, placeholderText) { this.formLabel.innerHTML = formLabelText; this.textareaPlaceholder.innerHTML = placeholderText; @@ -629,6 +685,11 @@ class customActionNeededEmail extends CustomizableEmailBase { let apiContainer = document.getElementById("get-action-needed-email-for-user-json") const apiUrl = apiContainer ? apiContainer.value : null; + + // These fields are hidden on pageload + const textAreaFormGroup = document.querySelector('.field-action_needed_reason'); + const dropdownFormGroup = document.querySelector('.field-action_needed_reason_email'); + super( dropdown, textarea, @@ -638,16 +699,32 @@ class customActionNeededEmail extends CustomizableEmailBase { modalConfirm, formLabel, lastSentEmailContent, - apiUrl + apiUrl, + textAreaFormGroup, + dropdownFormGroup, ); + } loadActionNeededEmail() { + if (this.textAreaFormGroup && this.dropdownFormGroup) { + this.initializeFormGroups("action needed", "showActionNeededReason"); + } this.updateUserInterface(this.dropdown.value); this.initializeDropdown("Error when attempting to grab action needed email: ") this.initializeModalConfirm() this.initializeDirectEditButton() } + + // Overrides the placeholder text when no reason is selected + showPlaceholderNoReason() { + this.showPlaceholder("Email:", "Select an action needed reason to see email"); + } + + // Overrides the placeholder text when the reason other is selected + showPlaceholderOtherReason() { + this.showPlaceholder("Email:", "No email will be sent"); + } } /** An IIFE that hooks to the show/hide button underneath action needed reason. @@ -675,8 +752,13 @@ class customRejectedEmail extends CustomizableEmailBase { const formLabel = document.querySelector('label[for="id_rejection_reason_email"]'); const lastSentEmailContent = document.getElementById("last-sent-email-content"); - let apiContainer = document.getElementById("get-rejection-reason-email-for-user-json") + let apiContainer = document.getElementById("get-rejection-email-for-user-json"); const apiUrl = apiContainer ? apiContainer.value : null; + + // These fields are hidden on pageload + const textAreaFormGroup = document.querySelector('.field-rejection_reason'); + const dropdownFormGroup = document.querySelector('.field-rejection_reason_email'); + super( dropdown, textarea, @@ -686,16 +768,31 @@ class customRejectedEmail extends CustomizableEmailBase { modalConfirm, formLabel, lastSentEmailContent, - apiUrl + apiUrl, + textAreaFormGroup, + dropdownFormGroup, ); } loadRejectedEmail() { + if (this.textAreaFormGroup && this.dropdownFormGroup) { + this.initializeFormGroups("rejected", "showRejectionReason"); + } this.updateUserInterface(this.dropdown.value); this.initializeDropdown("Error when attempting to grab rejected email: ") this.initializeModalConfirm() this.initializeDirectEditButton() } + + // Overrides the placeholder text when no reason is selected + showPlaceholderNoReason() { + this.showPlaceholder("Email:", "Select a rejection reason to see email"); + } + + // Overrides the placeholder text when the reason other is selected + showPlaceholderOtherReason() { + this.showPlaceholder("Email:", "No email will be sent"); + } } diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 76c77955f..eace14d4a 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -29,6 +29,7 @@ get_senior_official_from_federal_agency_json, get_federal_and_portfolio_types_from_federal_agency_json, get_action_needed_email_for_user_json, + get_rejection_email_for_user_json, ) from registrar.views.domains_json import get_domains_json from registrar.views.utility import always_404 @@ -159,6 +160,11 @@ get_action_needed_email_for_user_json, name="get-action-needed-email-for-user-json", ), + path( + "admin/api/get-rejection-email-for-user-json/", + get_rejection_email_for_user_json, + name="get-rejection-email-for-user-json", + ), path("admin/", admin.site.urls), path( "reports/export_data_type_user/", diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index b1cc00bde..c00953736 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -640,15 +640,16 @@ def sync_organization_type(self): # Actually updates the organization_type field org_type_helper.create_or_update_organization_type() - def _cache_status_and_action_needed_reason(self): + def _cache_status_and_status_reasons(self): """Maintains a cache of properties so we can avoid a DB call""" self._cached_action_needed_reason = self.action_needed_reason + self._cached_rejection_reason = self.rejection_reason self._cached_status = self.status def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) # Store original values for caching purposes. Used to compare them on save. - self._cache_status_and_action_needed_reason() + self._cache_status_and_status_reasons() def save(self, *args, **kwargs): """Save override for custom properties""" @@ -662,21 +663,42 @@ def save(self, *args, **kwargs): # Handle the action needed email. # An email is sent out when action_needed_reason is changed or added. - if self.action_needed_reason and self.status == self.DomainRequestStatus.ACTION_NEEDED: - self.sync_action_needed_reason() + if self.status == self.DomainRequestStatus.ACTION_NEEDED: + self.send_another_status_reason_email( + checked_status=self.DomainRequestStatus.ACTION_NEEDED, + old_reason=self._cached_action_needed_reason, + new_reason=self.action_needed_reason, + excluded_reasons=[DomainRequest.ActionNeededReasons.OTHER], + email_to_send=self.action_needed_reason_email + ) + elif self.status == self.DomainRequestStatus.REJECTED: + self.send_another_status_reason_email( + checked_status=self.DomainRequestStatus.REJECTED, + old_reason=self._cached_rejection_reason, + new_reason=self.rejection_reason, + excluded_reasons=[DomainRequest.RejectionReasons.OTHER], + email_to_send=self.rejection_reason_email, + ) # Update the cached values after saving - self._cache_status_and_action_needed_reason() - - def sync_action_needed_reason(self): - """Checks if we need to send another action needed email""" - 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: - # 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._cache_status_and_status_reasons() + + def send_another_status_reason_email(self, checked_status, old_reason, new_reason, excluded_reasons, email_to_send): + """Helper function to send out a second status email when the status remains the same, + but the reason has changed.""" + + # If the status itself changed, then we already sent out an email + if self._cached_status != checked_status or old_reason is None: + return + + # We should never send an email if no reason was specified + # Additionally, Don't send out emails for reasons that shouldn't send them + if new_reason is None or self.action_needed_reason in excluded_reasons: + return + + # Only send out an email if the underlying email itself changed + if old_reason != new_reason: + self._send_custom_status_update_email(email_content=email_to_send) def sync_yes_no_form_fields(self): """Some yes/no forms use a db field to track whether it was checked or not. @@ -806,9 +828,7 @@ def _send_status_update_email( def _send_custom_status_update_email(self, email_content): """Wrapper for `_send_status_update_email` that bcc's help@get.gov and sends an email equivalent to the 'email_content' variable.""" - if settings.IS_PRODUCTION: - bcc_address = settings.DEFAULT_FROM_EMAIL - + bcc_address = settings.DEFAULT_FROM_EMAIL if settings.IS_PRODUCTION else "" self._send_status_update_email( new_status="action needed", email_template=f"emails/includes/custom_email.txt", diff --git a/src/registrar/templates/django/admin/domain_request_change_form.html b/src/registrar/templates/django/admin/domain_request_change_form.html index afdd9e6c2..8d58bc696 100644 --- a/src/registrar/templates/django/admin/domain_request_change_form.html +++ b/src/registrar/templates/django/admin/domain_request_change_form.html @@ -10,6 +10,8 @@ {% url 'get-action-needed-email-for-user-json' as url %} + {% url 'get-rejection-email-for-user-json' as url_2 %} + {% for fieldset in adminform %} {% comment %} TODO: this will eventually need to be changed to something like this 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 0540a7b60..8f4e65ddc 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -226,6 +226,94 @@

{% endif %} + {% elif field.field.name == "rejection_reason_email" %} +
+ – +
+ + {{ field.field }} + + + Change Edit email +
+
+
+

+ Are you sure you want to edit this email? +

+
+

+ The creator of this request already received an email for this status/reason: +

+
    +
  • Status: Rejected
  • +
  • Reason: {{ original_object.get_rejection_reason_display }}
  • +
+

+ If you edit this email's text, the system will send another email to + the creator after you “save” your changes. If you do not want to send another email, click “cancel” below. +

+
+ + +
+ +
+
+ + {% if original_object.rejection_reason_reason_email %} + + {% else %} + + {% endif %} {% else %} {{ field.field }} {% endif %} diff --git a/src/registrar/templates/emails/rejection_reasons/contacts_not_verified.txt b/src/registrar/templates/emails/rejection_reasons/contacts_not_verified.txt index c35c82c2b..525a3a00a 100644 --- a/src/registrar/templates/emails/rejection_reasons/contacts_not_verified.txt +++ b/src/registrar/templates/emails/rejection_reasons/contacts_not_verified.txt @@ -1,8 +1,8 @@ {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} -{% include "emails/includes/status_change_rejected_header.html" %} +{% include "emails/includes/status_change_rejected_header.txt" %} REJECTION REASON Your domain request was rejected because we could not verify the organizational contacts you provided. If you have questions or comments, reply to this email. -{% include "emails/includes/email_footer.html" %} +{% include "emails/includes/email_footer.txt" %} {% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/naming_not_met.txt b/src/registrar/templates/emails/rejection_reasons/naming_not_met.txt index 3e57d579d..eb2e5e4c0 100644 --- a/src/registrar/templates/emails/rejection_reasons/naming_not_met.txt +++ b/src/registrar/templates/emails/rejection_reasons/naming_not_met.txt @@ -1,5 +1,5 @@ {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} -{% include "emails/includes/status_change_rejected_header.html" %} +{% include "emails/includes/status_change_rejected_header.txt" %} REJECTION REASON Your domain request was rejected because it does not meet our naming requirements. Domains should uniquely identify a government organization and be clear to the @@ -11,5 +11,5 @@ YOU CAN SUBMIT A NEW REQUEST We encourage you to request a domain that meets our requirements. If you have questions or want to discuss potential domain names, reply to this email. -{% include "emails/includes/email_footer.html" %} +{% include "emails/includes/email_footer.txt" %} {% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/org_has_domain.txt b/src/registrar/templates/emails/rejection_reasons/org_has_domain.txt index 26757efd6..175518ac3 100644 --- a/src/registrar/templates/emails/rejection_reasons/org_has_domain.txt +++ b/src/registrar/templates/emails/rejection_reasons/org_has_domain.txt @@ -1,5 +1,5 @@ {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} -{% include "emails/includes/status_change_rejected_header.html" %} +{% include "emails/includes/status_change_rejected_header.txt" %} REJECTION REASON Your domain request was rejected because {{ domain_request.organization_name }} has a .gov domain. Our practice is to approve one domain per online service per government organization. We @@ -11,5 +11,5 @@ Read more about our practice of approving one domain per online service If you have questions or comments, reply to this email. -{% include "emails/includes/email_footer.html" %} +{% include "emails/includes/email_footer.txt" %} {% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/org_not_eligible.txt b/src/registrar/templates/emails/rejection_reasons/org_not_eligible.txt index 3c7de3f42..606184706 100644 --- a/src/registrar/templates/emails/rejection_reasons/org_not_eligible.txt +++ b/src/registrar/templates/emails/rejection_reasons/org_not_eligible.txt @@ -1,5 +1,5 @@ {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} -{% include "emails/includes/status_change_rejected_header.html" %} +{% include "emails/includes/status_change_rejected_header.txt" %} REJECTION REASON Your domain request was rejected because we determined that {{ domain_request.organization_name }} is not eligible for a .gov domain. .Gov domains are only available to official U.S.-based @@ -10,5 +10,5 @@ Learn more about eligibility for .gov domains If you have questions or comments, reply to this email. -{% include "emails/includes/email_footer.html" %} +{% include "emails/includes/email_footer.txt" %} {% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/other.txt b/src/registrar/templates/emails/rejection_reasons/other.txt deleted file mode 100644 index 6835a45e0..000000000 --- a/src/registrar/templates/emails/rejection_reasons/other.txt +++ /dev/null @@ -1,15 +0,0 @@ -{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} -{% include "emails/includes/status_change_rejected_header.html" %} -YOU CAN SUBMIT A NEW REQUEST -If your organization is eligible for a .gov domain and you meet our other requirements, you can submit a new request. - -Learn more about: -- Eligibility for a .gov domain -- Choosing a .gov domain name - - -NEED ASSISTANCE? -If you have questions about this domain request or need help choosing a new domain name, reply to this email. - -{% include "emails/includes/email_footer.html" %} -{% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/purpose_not_met.txt b/src/registrar/templates/emails/rejection_reasons/purpose_not_met.txt index 57bce78f0..d95a9e0b3 100644 --- a/src/registrar/templates/emails/rejection_reasons/purpose_not_met.txt +++ b/src/registrar/templates/emails/rejection_reasons/purpose_not_met.txt @@ -1,5 +1,5 @@ {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} -{% include "emails/includes/status_change_rejected_header.html" %} +{% include "emails/includes/status_change_rejected_header.txt" %} REJECTION REASON Your domain request was rejected because the purpose you provided did not meet our requirements. You didn’t provide enough information about how you intend to use the @@ -11,5 +11,5 @@ Learn more about: If you have questions or comments, reply to this email. -{% include "emails/includes/email_footer.html" %} +{% include "emails/includes/email_footer.txt" %} {% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/requestor_not_eligible.txt b/src/registrar/templates/emails/rejection_reasons/requestor_not_eligible.txt index 7974c1690..deeb2d9da 100644 --- a/src/registrar/templates/emails/rejection_reasons/requestor_not_eligible.txt +++ b/src/registrar/templates/emails/rejection_reasons/requestor_not_eligible.txt @@ -1,5 +1,5 @@ {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} -{% include "emails/includes/status_change_rejected_header.html" %} +{% include "emails/includes/status_change_rejected_header.txt" %} REJECTION REASON Your domain request was rejected because we don’t believe you’re eligible to request a .gov domain on behalf of {{ domain_request.organization_name }}. You must be a government employee, or be @@ -10,5 +10,5 @@ DEMONSTRATE ELIGIBILITY If you can provide more information that demonstrates your eligibility, or you want to discuss further, reply to this email. -{% include "emails/includes/email_footer.html" %} +{% include "emails/includes/email_footer.txt" %} {% endautoescape %} \ No newline at end of file diff --git a/src/registrar/utility/admin_helpers.py b/src/registrar/utility/admin_helpers.py index 87edc2106..20760164e 100644 --- a/src/registrar/utility/admin_helpers.py +++ b/src/registrar/utility/admin_helpers.py @@ -19,9 +19,9 @@ def get_all_action_needed_reason_emails(domain_request): def get_action_needed_reason_default_email(domain_request, action_needed_reason): """Returns the default email associated with the given action needed reason""" return _get_default_email( - domain_request, - path_root="emails/rejection_reasons", - reason=action_needed_reason, + domain_request, + path_root="emails/action_needed_reasons", + reason=action_needed_reason, excluded_reasons=[DomainRequest.ActionNeededReasons.OTHER] ) @@ -40,12 +40,12 @@ def get_all_rejection_reason_emails(domain_request): ) -def get_rejection_reason_default_email(domain_request, action_needed_reason): +def get_rejection_reason_default_email(domain_request, rejection_reason): """Returns the default email associated with the given rejection reason""" return _get_default_email( - domain_request, - path_root="emails/rejection_reasons", - reason=action_needed_reason, + domain_request, + path_root="emails/rejection_reasons", + reason=rejection_reason, excluded_reasons=[DomainRequest.RejectionReasons.OTHER] ) @@ -56,6 +56,7 @@ def _get_all_default_emails(reasons, path_root, excluded_reasons, domain_request emails[reason.value] = _get_default_email( domain_request, path_root, reason, excluded_reasons ) + return emails def _get_default_email(domain_request, path_root, reason, excluded_reasons=None): if not reason: diff --git a/src/registrar/views/utility/api_views.py b/src/registrar/views/utility/api_views.py index 973f85855..bef6e5017 100644 --- a/src/registrar/views/utility/api_views.py +++ b/src/registrar/views/utility/api_views.py @@ -4,7 +4,7 @@ from registrar.models import FederalAgency, SeniorOfficial, DomainRequest from django.contrib.admin.views.decorators import staff_member_required from django.contrib.auth.decorators import login_required -from registrar.utility.admin_helpers import get_all_action_needed_reason_emails +from registrar.utility.admin_helpers import get_action_needed_reason_default_email, get_rejection_reason_default_email from registrar.models.portfolio import Portfolio from registrar.utility.constants import BranchChoices @@ -90,5 +90,30 @@ def get_action_needed_email_for_user_json(request): return JsonResponse({"error": "No domain_request_id specified"}, status=404) domain_request = DomainRequest.objects.filter(id=domain_request_id).first() - emails = get_all_action_needed_reason_emails(domain_request) - return JsonResponse({"action_needed_email": emails.get(reason)}, status=200) + + email = get_action_needed_reason_default_email(domain_request, reason) + return JsonResponse({"email": email}, status=200) + + +@login_required +@staff_member_required +def get_rejection_email_for_user_json(request): + """Returns a default rejection email for a given user""" + + # This API is only accessible to admins and analysts + superuser_perm = request.user.has_perm("registrar.full_access_permission") + analyst_perm = request.user.has_perm("registrar.analyst_access_permission") + if not request.user.is_authenticated or not any([analyst_perm, superuser_perm]): + return JsonResponse({"error": "You do not have access to this resource"}, status=403) + + reason = request.GET.get("reason") + domain_request_id = request.GET.get("domain_request_id") + if not reason: + return JsonResponse({"error": "No reason specified"}, status=404) + + if not domain_request_id: + return JsonResponse({"error": "No domain_request_id specified"}, status=404) + + domain_request = DomainRequest.objects.filter(id=domain_request_id).first() + email = get_rejection_reason_default_email(domain_request, reason) + return JsonResponse({"email": email}, status=200) From 1ce0724d3401358dbb94765c6be2d2d21e6d0171 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 27 Sep 2024 11:52:47 -0600 Subject: [PATCH 04/34] Cleanup + send email logic --- src/registrar/assets/js/get-gov-admin.js | 78 ++----------------- src/registrar/models/domain_request.py | 6 -- .../admin/includes/detail_table_fieldset.html | 8 +- src/registrar/utility/email.py | 2 +- 4 files changed, 13 insertions(+), 81 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 95b28660f..17304ce97 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -344,69 +344,6 @@ function initializeWidgetOnList(list, parentId) { } } -/** An IIFE for admin in DjangoAdmin to listen to changes on the domain request - * status select and to show/hide the rejection reason -*/ -(function (){ - let rejectionReasonFormGroup = document.querySelector('.field-rejection_reason'); - // This is the "action needed reason" field - let actionNeededReasonFormGroup = document.querySelector('.field-action_needed_reason'); - // This is the "Email" field - let actionNeededReasonEmailFormGroup = document.querySelector('.field-action_needed_reason_email') - - if (rejectionReasonFormGroup && actionNeededReasonFormGroup && actionNeededReasonEmailFormGroup) { - let statusSelect = document.getElementById('id_status') - let isRejected = statusSelect.value == "rejected" - let isActionNeeded = statusSelect.value == "action needed" - - // Initial handling of rejectionReasonFormGroup display - showOrHideObject(rejectionReasonFormGroup, show=isRejected) - showOrHideObject(actionNeededReasonFormGroup, show=isActionNeeded) - showOrHideObject(actionNeededReasonEmailFormGroup, show=isActionNeeded) - - // Listen to change events and handle rejectionReasonFormGroup display, then save status to session storage - statusSelect.addEventListener('change', function() { - // Show the rejection reason field if the status is rejected. - // Then track if its shown or hidden in our session cache. - isRejected = statusSelect.value == "rejected" - showOrHideObject(rejectionReasonFormGroup, show=isRejected) - addOrRemoveSessionBoolean("showRejectionReason", add=isRejected) - - isActionNeeded = statusSelect.value == "action needed" - showOrHideObject(actionNeededReasonFormGroup, show=isActionNeeded) - showOrHideObject(actionNeededReasonEmailFormGroup, show=isActionNeeded) - addOrRemoveSessionBoolean("showActionNeededReason", add=isActionNeeded) - }); - - // Listen to Back/Forward button navigation and handle rejectionReasonFormGroup display based on session storage - - // When you navigate using forward/back after changing status but not saving, when you land back on the DA page the - // status select will say (for example) Rejected but the selected option can be something else. To manage the show/hide - // accurately for this edge case, we use cache and test for the back/forward navigation. - const observer = new PerformanceObserver((list) => { - list.getEntries().forEach((entry) => { - if (entry.type === "back_forward") { - let showRejectionReason = sessionStorage.getItem("showRejectionReason") !== null - showOrHideObject(rejectionReasonFormGroup, show=showRejectionReason) - - let showActionNeededReason = sessionStorage.getItem("showActionNeededReason") !== null - showOrHideObject(actionNeededReasonFormGroup, show=showActionNeededReason) - showOrHideObject(actionNeededReasonEmailFormGroup, show=isActionNeeded) - } - }); - }); - observer.observe({ type: "navigation" }); - } - - // Adds or removes the display-none class to object depending on the value of boolean show - function showOrHideObject(object, show){ - if (show){ - object.classList.remove("display-none"); - }else { - object.classList.add("display-none"); - } - } -})(); /** An IIFE for toggling the submit bar on domain request forms */ @@ -530,17 +467,17 @@ class CustomizableEmailBase { // Handle showing/hiding the related fields on page load. initializeFormGroups(statusToCheck, sessionVariableName) { - let isStatus = statusSelect.value == statusToCheck; + let isStatus = this.statusSelect.value == statusToCheck; // Initial handling of these groups. - updateFormGroupVisibility(isStatus, isStatus); + this.updateFormGroupVisibility(isStatus, isStatus); // Listen to change events and handle rejectionReasonFormGroup display, then save status to session storage this.statusSelect.addEventListener('change', () => { // Show the action needed field if the status is what we expect. // Then track if its shown or hidden in our session cache. - isStatus = statusSelect.value == statusToCheck; - updateFormGroupVisibility(isStatus, isStatus); + isStatus = this.statusSelect.value == statusToCheck; + this.updateFormGroupVisibility(isStatus, isStatus); addOrRemoveSessionBoolean(sessionVariableName, add=isStatus); }); @@ -552,7 +489,7 @@ class CustomizableEmailBase { list.getEntries().forEach((entry) => { if (entry.type === "back_forward") { let showTextAreaFormGroup = sessionStorage.getItem(sessionVariableName) !== null; - updateFormGroupVisibility(showTextAreaFormGroup, isStatus); + this.updateFormGroupVisibility(showTextAreaFormGroup, isStatus); } }); }); @@ -681,7 +618,7 @@ class customActionNeededEmail extends CustomizableEmailBase { const modalTrigger = document.querySelector('.field-action_needed_reason_email__modal-trigger'); const modalConfirm = document.getElementById('confirm-edit-email'); const formLabel = document.querySelector('label[for="id_action_needed_reason_email"]'); - const lastSentEmailContent = document.getElementById("last-sent-email-content"); + const lastSentEmailContent = document.getElementById("last-sent-action-needed-email-content"); let apiContainer = document.getElementById("get-action-needed-email-for-user-json") const apiUrl = apiContainer ? apiContainer.value : null; @@ -750,7 +687,7 @@ class customRejectedEmail extends CustomizableEmailBase { const modalTrigger = document.querySelector('.field-rejection_reason_email__modal-trigger'); const modalConfirm = document.getElementById('confirm-edit-email'); const formLabel = document.querySelector('label[for="id_rejection_reason_email"]'); - const lastSentEmailContent = document.getElementById("last-sent-email-content"); + const lastSentEmailContent = document.getElementById("last-sent-rejection-email-content"); let apiContainer = document.getElementById("get-rejection-email-for-user-json"); const apiUrl = apiContainer ? apiContainer.value : null; @@ -776,6 +713,7 @@ class customRejectedEmail extends CustomizableEmailBase { loadRejectedEmail() { if (this.textAreaFormGroup && this.dropdownFormGroup) { + // TODO: fix this for rejected this.initializeFormGroups("rejected", "showRejectionReason"); } this.updateUserInterface(this.dropdown.value); diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index c00953736..72e0e4773 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1065,12 +1065,6 @@ def reject(self): if self.status == self.DomainRequestStatus.APPROVED: self.delete_and_clean_up_domain("reject") - self._send_status_update_email( - "action needed", - "emails/status_change_rejected.txt", - "emails/status_change_rejected_subject.txt", - ) - # Send out an email if a rejection reason exists if self.rejection_reason: email_content = self.rejection_reason_email 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 8f4e65ddc..0bef5d10d 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -221,9 +221,9 @@

{% if original_object.action_needed_reason_email %} - + {% else %} - + {% endif %} {% elif field.field.name == "rejection_reason_email" %} @@ -310,9 +310,9 @@

{% if original_object.rejection_reason_reason_email %} - + {% else %} - + {% endif %} {% else %} {{ field.field }} diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 5f8a93bd9..3f508fb96 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -78,7 +78,7 @@ def send_templated_email( # Wrap the email body to a maximum width of 80 characters per line. # Not all email clients support CSS to do this, and our .txt files require parsing. if wrap_email: - email_body = wrap_text_and_preserve_paragraphs(email_body, width=80) + email_body = wrap_text_and_preserve_paragraphs(email_body, width=110) ses_client.send_email( FromEmailAddress=settings.DEFAULT_FROM_EMAIL, From e37a95ee1ee2190d0316df989c76b80300fb978f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 27 Sep 2024 11:56:17 -0600 Subject: [PATCH 05/34] Typo --- .../templates/django/admin/includes/detail_table_fieldset.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 0bef5d10d..38a0b97c7 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -309,7 +309,7 @@

- {% if original_object.rejection_reason_reason_email %} + {% if original_object.rejection_reason_email %} {% else %} From c084ec62f09a20af9d728b273a37b337efe9ac30 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 27 Sep 2024 12:45:50 -0600 Subject: [PATCH 06/34] check for page --- src/registrar/assets/js/get-gov-admin.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 17304ce97..5d8328bf4 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -668,12 +668,13 @@ class customActionNeededEmail extends CustomizableEmailBase { * This shows the auto generated email on action needed reason. */ document.addEventListener('DOMContentLoaded', function() { - const customEmail = new customActionNeededEmail(); - if (!customEmail.dropdown || !customEmail.textarea || !customEmail.domainRequestId || !customEmail.formLabel || !customEmail.modalConfirm){ + const domainRequestForm = document.getElementById("domainrequest_form"); + if (!domainRequestForm) { return; } // Initialize UI + const customEmail = new customActionNeededEmail(); customEmail.loadActionNeededEmail() }); @@ -738,12 +739,13 @@ class customRejectedEmail extends CustomizableEmailBase { * This shows the auto generated email on action needed reason. */ document.addEventListener('DOMContentLoaded', function() { - const customEmail = new customRejectedEmail(); - if (!customEmail.dropdown || !customEmail.textarea || !customEmail.domainRequestId || !customEmail.formLabel || !customEmail.modalConfirm){ + const domainRequestForm = document.getElementById("domainrequest_form"); + if (!domainRequestForm) { return; } // Initialize UI + const customEmail = new customRejectedEmail(); customEmail.loadRejectedEmail() }); From ff5212fb272a0395545786f3e0746a7a17b8ce7c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 10:04:29 -0600 Subject: [PATCH 07/34] Remove unhelpful logic --- src/registrar/admin.py | 44 ------------------------------------------ 1 file changed, 44 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index a5066d3b0..b9204666b 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1944,16 +1944,6 @@ def save_model(self, request, obj, form, change): # Get the original domain request from the database. original_obj = models.DomainRequest.objects.get(pk=obj.pk) - # == Handle action_needed_reason == # - action_needed_reason_changed = obj.action_needed_reason != original_obj.action_needed_reason - if action_needed_reason_changed: - obj = self._handle_action_needed_reason(request, obj, original_obj) - - # == Handle rejection_reason == # - rejection_reason_changed = obj.rejection_reason != original_obj.rejection_reason - if rejection_reason_changed: - obj = self._handle_rejection_reason(request, obj, original_obj) - # == Handle allowed emails == # if obj.status in DomainRequest.get_statuses_that_send_emails() and not settings.IS_PRODUCTION: self._check_for_valid_email(request, obj) @@ -1970,40 +1960,6 @@ def save_model(self, request, obj, form, change): if should_save: return super().save_model(request, obj, form, change) - def _handle_action_needed_reason(self, request, obj, original_obj): - # Track the fact that we sent out an email - request.session["action_needed_email_sent"] = True - - # Set the action_needed_reason_email to the default if nothing exists. - # Since this check occurs after save, if the user enters a value then we won't update. - - default_email = get_action_needed_reason_default_email(obj, obj.action_needed_reason) - if obj.action_needed_reason_email: - emails = get_all_action_needed_reason_emails(obj) - is_custom_email = obj.action_needed_reason_email not in emails.values() - if not is_custom_email: - obj.action_needed_reason_email = default_email - else: - obj.action_needed_reason_email = default_email - return obj - - def _handle_rejection_reason(self, request, obj, original_obj): - # Track the fact that we sent out an email - request.session["rejection_reason_email_sent"] = True - - # Set the rejection_reason_email to the default if nothing exists. - # Since this check occurs after save, if the user enters a value then we won't update. - - default_email = get_rejection_reason_default_email(obj, obj.rejection_reason) - if obj.rejection_reason_email: - emails = get_all_rejection_reason_emails(obj) - is_custom_email = obj.rejection_reason_email not in emails.values() - if not is_custom_email: - obj.rejection_reason_email = default_email - else: - obj.rejection_reason_email = default_email - return obj - def _check_for_valid_email(self, request, obj): """Certain emails are whitelisted in non-production environments, so we should display that information using this function. From 7a5eda065bc17e225088d662b097a4658adc4f14 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 10:20:36 -0600 Subject: [PATCH 08/34] Fix back button bug --- src/registrar/assets/js/get-gov-admin.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 5d8328bf4..cbf2cf351 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -449,7 +449,7 @@ class CustomizableEmailBase { this.lastSentEmailContent = lastSentEmailContent; this.apiUrl = apiUrl; - // These fields are hidden on pageload + // These fields are hidden/shown on pageload depending on the current status this.textAreaFormGroup = textAreaFormGroup; this.dropdownFormGroup = dropdownFormGroup; this.statusSelect = document.getElementById("id_status"); @@ -459,8 +459,8 @@ class CustomizableEmailBase { this.initialEmailValue = this.textarea ? this.textarea.value : null; this.isEmailAlreadySentConst; - if (lastSentEmailContent && textarea) { - this.isEmailAlreadySentConst = lastSentEmailContent.value.replace(/\s+/g, '') === textarea.value.replace(/\s+/g, ''); + if (this.lastSentEmailContent && this.textarea) { + this.isEmailAlreadySentConst = this.lastSentEmailContent.value.replace(/\s+/g, '') === this.textarea.value.replace(/\s+/g, ''); } } @@ -478,7 +478,7 @@ class CustomizableEmailBase { // Then track if its shown or hidden in our session cache. isStatus = this.statusSelect.value == statusToCheck; this.updateFormGroupVisibility(isStatus, isStatus); - addOrRemoveSessionBoolean(sessionVariableName, add=isStatus); + addOrRemoveSessionBoolean(sessionVariableName, isStatus); }); // Listen to Back/Forward button navigation and handle rejectionReasonFormGroup display based on session storage @@ -489,16 +489,16 @@ class CustomizableEmailBase { list.getEntries().forEach((entry) => { if (entry.type === "back_forward") { let showTextAreaFormGroup = sessionStorage.getItem(sessionVariableName) !== null; - this.updateFormGroupVisibility(showTextAreaFormGroup, isStatus); + this.updateFormGroupVisibility(showTextAreaFormGroup, showTextAreaFormGroup); } }); }); observer.observe({ type: "navigation" }); } - updateFormGroupVisibility(showTextAreaFormGroup, showdropDownFormGroup) { + updateFormGroupVisibility(showTextAreaFormGroup, showDropDownFormGroup) { showTextAreaFormGroup ? showElement(this.textAreaFormGroup) : hideElement(this.textAreaFormGroup); - showdropDownFormGroup ? showElement(this.dropdownFormGroup) : hideElement(this.dropdownFormGroup); + showDropDownFormGroup ? showElement(this.dropdownFormGroup) : hideElement(this.dropdownFormGroup); } initializeDropdown(errorMessage) { From cfa1879909af49d502913d0ba4a94159949b1453 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 12:33:08 -0600 Subject: [PATCH 09/34] cleanup --- src/registrar/assets/js/get-gov-admin.js | 11 ++++------- src/registrar/models/domain_request.py | 4 ++-- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index cbf2cf351..f8d4d679d 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -644,9 +644,9 @@ class customActionNeededEmail extends CustomizableEmailBase { } loadActionNeededEmail() { - if (this.textAreaFormGroup && this.dropdownFormGroup) { - this.initializeFormGroups("action needed", "showActionNeededReason"); - } + // Hide/show the email fields depending on the current status + this.initializeFormGroups("action needed", "showActionNeededReason"); + // Setup the textarea, edit button, helper text this.updateUserInterface(this.dropdown.value); this.initializeDropdown("Error when attempting to grab action needed email: ") this.initializeModalConfirm() @@ -713,10 +713,7 @@ class customRejectedEmail extends CustomizableEmailBase { } loadRejectedEmail() { - if (this.textAreaFormGroup && this.dropdownFormGroup) { - // TODO: fix this for rejected - this.initializeFormGroups("rejected", "showRejectionReason"); - } + this.initializeFormGroups("rejected", "showRejectionReason"); this.updateUserInterface(this.dropdown.value); this.initializeDropdown("Error when attempting to grab rejected email: ") this.initializeModalConfirm() diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 72e0e4773..3ccb490a9 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -693,8 +693,8 @@ def send_another_status_reason_email(self, checked_status, old_reason, new_reaso # We should never send an email if no reason was specified # Additionally, Don't send out emails for reasons that shouldn't send them - if new_reason is None or self.action_needed_reason in excluded_reasons: - return + if new_reason is None or new_reason in excluded_reasons: + return # Only send out an email if the underlying email itself changed if old_reason != new_reason: From 7cc5231cc0a70646b7569147307e4956382ae675 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 14:35:22 -0600 Subject: [PATCH 10/34] cleanup --- src/registrar/assets/js/get-gov-admin.js | 191 ++++++++++++----------- src/registrar/models/domain_request.py | 52 +++--- 2 files changed, 128 insertions(+), 115 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index f8d4d679d..1886b9292 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -437,23 +437,46 @@ function initializeWidgetOnList(list, parentId) { } })(); + class CustomizableEmailBase { - constructor(dropdown, textarea, textareaPlaceholder, directEditButton, modalTrigger, modalConfirm, formLabel, lastSentEmailContent, apiUrl, textAreaFormGroup, dropdownFormGroup) { - this.dropdown = dropdown; - this.textarea = textarea; - this.textareaPlaceholder = textareaPlaceholder; - this.directEditButton = directEditButton; - this.modalTrigger = modalTrigger; - this.modalConfirm = modalConfirm; - this.formLabel = formLabel; - this.lastSentEmailContent = lastSentEmailContent; - this.apiUrl = apiUrl; + + /** + * @param {Object} config - must contain the following: + * @property {HTMLElement} dropdown - The dropdown element. + * @property {HTMLElement} textarea - The textarea element. + * @property {HTMLElement} textareaPlaceholder - The placeholder for the textarea. + * @property {HTMLElement} directEditButton - The button to directly edit the email. + * @property {HTMLElement} modalTrigger - The trigger for the modal. + * @property {HTMLElement} modalConfirm - The confirm button in the modal. + * @property {HTMLElement} formLabel - The label for the form. + * @property {HTMLElement} lastSentEmailContent - The last sent email content element. + * @property {HTMLElement} textAreaFormGroup - The form group for the textarea. + * @property {HTMLElement} dropdownFormGroup - The form group for the dropdown. + * @property {string} apiUrl - The API URL for fetching email content. + * @property {string} statusToCheck - The status to check against. Used for show/hide on textAreaFormGroup/dropdownFormGroup. + * @property {string} sessionVariableName - The session variable name. Used for show/hide on textAreaFormGroup/dropdownFormGroup. + * @property {string} apiErrorMessage - The error message that the ajax call returns. + */ + constructor(config) { + this.dropdown = config.dropdown; + this.textarea = config.textarea; + this.textareaPlaceholder = config.textareaPlaceholder; + this.directEditButton = config.directEditButton; + this.modalTrigger = config.modalTrigger; + this.modalConfirm = config.modalConfirm; + this.formLabel = config.formLabel; + this.lastSentEmailContent = config.lastSentEmailContent; + this.apiUrl = config.apiUrl; + this.apiErrorMessage = config.apiErrorMessage; // These fields are hidden/shown on pageload depending on the current status - this.textAreaFormGroup = textAreaFormGroup; - this.dropdownFormGroup = dropdownFormGroup; - this.statusSelect = document.getElementById("id_status"); + this.textAreaFormGroup = config.textAreaFormGroup; + this.dropdownFormGroup = config.dropdownFormGroup; + this.statusToCheck = config.statusToCheck; + this.sessionVariableName = config.sessionVariableName; + // Non-configurable variables + this.statusSelect = document.getElementById("id_status"); this.domainRequestId = this.dropdown ? document.getElementById("domain_request_id").value : null this.initialDropdownValue = this.dropdown ? this.dropdown.value : null; this.initialEmailValue = this.textarea ? this.textarea.value : null; @@ -466,19 +489,19 @@ class CustomizableEmailBase { } // Handle showing/hiding the related fields on page load. - initializeFormGroups(statusToCheck, sessionVariableName) { - let isStatus = this.statusSelect.value == statusToCheck; + initializeFormGroups() { + let isStatus = this.statusSelect.value == this.statusToCheck; // Initial handling of these groups. - this.updateFormGroupVisibility(isStatus, isStatus); + this.updateFormGroupVisibility(isStatus); // Listen to change events and handle rejectionReasonFormGroup display, then save status to session storage this.statusSelect.addEventListener('change', () => { // Show the action needed field if the status is what we expect. // Then track if its shown or hidden in our session cache. - isStatus = this.statusSelect.value == statusToCheck; - this.updateFormGroupVisibility(isStatus, isStatus); - addOrRemoveSessionBoolean(sessionVariableName, isStatus); + isStatus = this.statusSelect.value == this.statusToCheck; + this.updateFormGroupVisibility(isStatus); + addOrRemoveSessionBoolean(this.sessionVariableName, isStatus); }); // Listen to Back/Forward button navigation and handle rejectionReasonFormGroup display based on session storage @@ -487,21 +510,26 @@ class CustomizableEmailBase { // accurately for this edge case, we use cache and test for the back/forward navigation. const observer = new PerformanceObserver((list) => { list.getEntries().forEach((entry) => { - if (entry.type === "back_forward") { - let showTextAreaFormGroup = sessionStorage.getItem(sessionVariableName) !== null; - this.updateFormGroupVisibility(showTextAreaFormGroup, showTextAreaFormGroup); - } + if (entry.type === "back_forward") { + let showTextAreaFormGroup = sessionStorage.getItem(this.sessionVariableName) !== null; + this.updateFormGroupVisibility(showTextAreaFormGroup); + } }); }); observer.observe({ type: "navigation" }); } - updateFormGroupVisibility(showTextAreaFormGroup, showDropDownFormGroup) { - showTextAreaFormGroup ? showElement(this.textAreaFormGroup) : hideElement(this.textAreaFormGroup); - showDropDownFormGroup ? showElement(this.dropdownFormGroup) : hideElement(this.dropdownFormGroup); + updateFormGroupVisibility(showFormGroups) { + if (showFormGroups) { + showElement(this.textAreaFormGroup); + showElement(this.dropdownFormGroup); + }else { + hideElement(this.textAreaFormGroup); + hideElement(this.dropdownFormGroup); + } } - initializeDropdown(errorMessage) { + initializeDropdown() { this.dropdown.addEventListener("change", () => { let reason = this.dropdown.value; if (this.initialDropdownValue !== this.dropdown.value || this.initialEmailValue !== this.textarea.value) { @@ -525,7 +553,7 @@ class CustomizableEmailBase { this.updateUserInterface(reason); }) .catch(error => { - console.error(errorMessage, error) + console.error(this.apiErrorMessage, error) }); } }); @@ -553,7 +581,7 @@ class CustomizableEmailBase { return this.lastSentEmailContent.value.replace(/\s+/g, '') === this.textarea.value.replace(/\s+/g, ''); } - updateUserInterface(reason) { + updateUserInterface(reason=this.dropdown.value) { if (!reason) { // No reason selected, we will set the label to "Email", show the "Make a selection" placeholder, hide the trigger, textarea, hide the help text this.showPlaceholderNoReason(); @@ -611,46 +639,33 @@ class CustomizableEmailBase { class customActionNeededEmail extends CustomizableEmailBase { constructor() { - const dropdown = document.getElementById("id_action_needed_reason"); - const textarea = document.getElementById("id_action_needed_reason_email") - const textareaPlaceholder = document.querySelector(".field-action_needed_reason_email__placeholder"); - const directEditButton = document.querySelector('.field-action_needed_reason_email__edit'); - const modalTrigger = document.querySelector('.field-action_needed_reason_email__modal-trigger'); - const modalConfirm = document.getElementById('confirm-edit-email'); - const formLabel = document.querySelector('label[for="id_action_needed_reason_email"]'); - const lastSentEmailContent = document.getElementById("last-sent-action-needed-email-content"); - - let apiContainer = document.getElementById("get-action-needed-email-for-user-json") - const apiUrl = apiContainer ? apiContainer.value : null; - - // These fields are hidden on pageload - const textAreaFormGroup = document.querySelector('.field-action_needed_reason'); - const dropdownFormGroup = document.querySelector('.field-action_needed_reason_email'); - - super( - dropdown, - textarea, - textareaPlaceholder, - directEditButton, - modalTrigger, - modalConfirm, - formLabel, - lastSentEmailContent, - apiUrl, - textAreaFormGroup, - dropdownFormGroup, - ); - + const emailConfig = { + dropdown: document.getElementById("id_action_needed_reason"), + textarea: document.getElementById("id_action_needed_reason_email"), + textareaPlaceholder: document.querySelector(".field-action_needed_reason_email__placeholder"), + directEditButton: document.querySelector('.field-action_needed_reason_email__edit'), + modalTrigger: document.querySelector('.field-action_needed_reason_email__modal-trigger'), + modalConfirm: document.getElementById('confirm-edit-email'), + formLabel: document.querySelector('label[for="id_action_needed_reason_email"]'), + lastSentEmailContent: document.getElementById("last-sent-action-needed-email-content"), + apiUrl: document.getElementById("get-action-needed-email-for-user-json")?.value || null, + textAreaFormGroup: document.querySelector('.field-action_needed_reason'), + dropdownFormGroup: document.querySelector('.field-action_needed_reason_email'), + statusToCheck: "rejected", + sessionVariableName: "showRejectionReason", + apiErrorMessage: "Error when attempting to grab rejected email: " + } + super(emailConfig); } loadActionNeededEmail() { // Hide/show the email fields depending on the current status - this.initializeFormGroups("action needed", "showActionNeededReason"); + this.initializeFormGroups(); // Setup the textarea, edit button, helper text - this.updateUserInterface(this.dropdown.value); - this.initializeDropdown("Error when attempting to grab action needed email: ") - this.initializeModalConfirm() - this.initializeDirectEditButton() + this.updateUserInterface(); + this.initializeDropdown(); + this.initializeModalConfirm(); + this.initializeDirectEditButton(); } // Overrides the placeholder text when no reason is selected @@ -681,40 +696,28 @@ document.addEventListener('DOMContentLoaded', function() { class customRejectedEmail extends CustomizableEmailBase { constructor() { - const dropdown = document.getElementById("id_rejection_reason"); - const textarea = document.getElementById("id_rejection_reason_email") - const textareaPlaceholder = document.querySelector(".field-rejection_reason_email__placeholder"); - const directEditButton = document.querySelector('.field-rejection_reason_email__edit'); - const modalTrigger = document.querySelector('.field-rejection_reason_email__modal-trigger'); - const modalConfirm = document.getElementById('confirm-edit-email'); - const formLabel = document.querySelector('label[for="id_rejection_reason_email"]'); - const lastSentEmailContent = document.getElementById("last-sent-rejection-email-content"); - - let apiContainer = document.getElementById("get-rejection-email-for-user-json"); - const apiUrl = apiContainer ? apiContainer.value : null; - - // These fields are hidden on pageload - const textAreaFormGroup = document.querySelector('.field-rejection_reason'); - const dropdownFormGroup = document.querySelector('.field-rejection_reason_email'); - - super( - dropdown, - textarea, - textareaPlaceholder, - directEditButton, - modalTrigger, - modalConfirm, - formLabel, - lastSentEmailContent, - apiUrl, - textAreaFormGroup, - dropdownFormGroup, - ); + const emailConfig = { + dropdown: document.getElementById("id_rejection_reason"), + textarea: document.getElementById("id_rejection_reason_email"), + textareaPlaceholder: document.querySelector(".field-rejection_reason_email__placeholder"), + directEditButton: document.querySelector('.field-rejection_reason_email__edit'), + modalTrigger: document.querySelector('.field-rejection_reason_email__modal-trigger'), + modalConfirm: document.getElementById('confirm-edit-email'), + formLabel: document.querySelector('label[for="id_rejection_reason_email"]'), + lastSentEmailContent: document.getElementById("last-sent-rejection-email-content"), + apiUrl: document.getElementById("get-rejection-email-for-user-json")?.value || null, + textAreaFormGroup: document.querySelector('.field-rejection_reason'), + dropdownFormGroup: document.querySelector('.field-rejection_reason_email'), + statusToCheck: "rejected", + sessionVariableName: "showRejectionReason", + errorMessage: "Error when attempting to grab rejected email: " + }; + super(emailConfig); } loadRejectedEmail() { this.initializeFormGroups("rejected", "showRejectionReason"); - this.updateUserInterface(this.dropdown.value); + this.updateUserInterface(); this.initializeDropdown("Error when attempting to grab rejected email: ") this.initializeModalConfirm() this.initializeDirectEditButton() diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 3ccb490a9..ac353406d 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -661,39 +661,49 @@ def save(self, *args, **kwargs): super().save(*args, **kwargs) - # 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: - self.send_another_status_reason_email( - checked_status=self.DomainRequestStatus.ACTION_NEEDED, - old_reason=self._cached_action_needed_reason, - new_reason=self.action_needed_reason, - excluded_reasons=[DomainRequest.ActionNeededReasons.OTHER], - email_to_send=self.action_needed_reason_email - ) - elif self.status == self.DomainRequestStatus.REJECTED: - self.send_another_status_reason_email( - checked_status=self.DomainRequestStatus.REJECTED, - old_reason=self._cached_rejection_reason, - new_reason=self.rejection_reason, - excluded_reasons=[DomainRequest.RejectionReasons.OTHER], - email_to_send=self.rejection_reason_email, - ) + # Handle custom status emails. + # An email is sent out when a, for example, action_needed_reason is changed or added. + statuses_that_send_custom_emails = [self.DomainRequestStatus.ACTION_NEEDED, self.DomainRequestStatus.REJECTED] + if self.status in statuses_that_send_custom_emails: + self.send_another_status_reason_email(self.status) # Update the cached values after saving self._cache_status_and_status_reasons() - def send_another_status_reason_email(self, checked_status, old_reason, new_reason, excluded_reasons, email_to_send): + def send_another_status_reason_email(self, status): """Helper function to send out a second status email when the status remains the same, but the reason has changed.""" + # Currently, we store all this information in three variables. + # When adding new reasons, this can be a lot to manage so we store it here + # in a centralized location. However, this may need to change if this scales. + status_information = { + self.DomainRequestStatus.ACTION_NEEDED: { + "cached_reason": self._cached_action_needed_reason, + "reason": self.action_needed_reason, + "email": self.action_needed_reason_email, + "excluded_reasons": [DomainRequest.ActionNeededReasons.OTHER], + }, + self.DomainRequestStatus.REJECTED: { + "cached_reason": self._cached_rejection_reason, + "reason": self.rejection_reason, + "email": self.rejection_reason_email, + "excluded_reasons": [DomainRequest.RejectionReasons.OTHER], + } + } + + current_status = status_information.get(status) + old_reason = status_information.get("cached_reason") + new_reason = status_information.get("reason") + email_to_send = status_information.get("email") + # If the status itself changed, then we already sent out an email - if self._cached_status != checked_status or old_reason is None: + if self._cached_status != status or old_reason is None: return # We should never send an email if no reason was specified # Additionally, Don't send out emails for reasons that shouldn't send them - if new_reason is None or new_reason in excluded_reasons: + if new_reason is None or new_reason in current_status.get("excluded_reasons"): return # Only send out an email if the underlying email itself changed From 6bb907c6bc84cd37b92fc0201e85e25f8711f850 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 15:05:09 -0600 Subject: [PATCH 11/34] Simplify backend logic --- src/registrar/assets/js/get-gov-admin.js | 6 +- src/registrar/models/domain_request.py | 70 ++++++++++-------------- 2 files changed, 32 insertions(+), 44 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 1886b9292..dbe7b01d3 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -651,9 +651,9 @@ class customActionNeededEmail extends CustomizableEmailBase { apiUrl: document.getElementById("get-action-needed-email-for-user-json")?.value || null, textAreaFormGroup: document.querySelector('.field-action_needed_reason'), dropdownFormGroup: document.querySelector('.field-action_needed_reason_email'), - statusToCheck: "rejected", - sessionVariableName: "showRejectionReason", - apiErrorMessage: "Error when attempting to grab rejected email: " + statusToCheck: "action needed", + sessionVariableName: "showActionNeededReason", + apiErrorMessage: "Error when attempting to grab action needed email: " } super(emailConfig); } diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index ac353406d..2210ac9d9 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -665,12 +665,12 @@ def save(self, *args, **kwargs): # An email is sent out when a, for example, action_needed_reason is changed or added. statuses_that_send_custom_emails = [self.DomainRequestStatus.ACTION_NEEDED, self.DomainRequestStatus.REJECTED] if self.status in statuses_that_send_custom_emails: - self.send_another_status_reason_email(self.status) + self.send_custom_status_update_email(self.status) # Update the cached values after saving self._cache_status_and_status_reasons() - def send_another_status_reason_email(self, status): + def send_custom_status_update_email(self, status): """Helper function to send out a second status email when the status remains the same, but the reason has changed.""" @@ -691,24 +691,29 @@ def send_another_status_reason_email(self, status): "excluded_reasons": [DomainRequest.RejectionReasons.OTHER], } } + status_info = status_information.get(status) - current_status = status_information.get(status) - old_reason = status_information.get("cached_reason") - new_reason = status_information.get("reason") - email_to_send = status_information.get("email") - - # If the status itself changed, then we already sent out an email - if self._cached_status != status or old_reason is None: + # Don't send an email if there is nothing to send. + if status_info.get("email") is None: + logger.warning("send_custom_status_update_email() => Tried sending an empty email.") return - # We should never send an email if no reason was specified - # Additionally, Don't send out emails for reasons that shouldn't send them - if new_reason is None or new_reason in current_status.get("excluded_reasons"): + # We should never send an email if no reason was specified. + # Additionally, Don't send out emails for reasons that shouldn't send them. + if status_info.get("reason") is None or status_info.get("reason") in status_info.get("excluded_reasons"): return - # Only send out an email if the underlying email itself changed - if old_reason != new_reason: - self._send_custom_status_update_email(email_content=email_to_send) + # Only send out an email if the underlying reason itself changed or if no email was sent previously. + if status_info.get("cached_reason") != status_info.get("reason") or status_info.get("cached_reason") is None: + bcc_address = settings.DEFAULT_FROM_EMAIL if settings.IS_PRODUCTION else "" + self._send_status_update_email( + new_status="action needed", + email_template=f"emails/includes/custom_email.txt", + email_template_subject=f"emails/status_change_subject.txt", + bcc_address=bcc_address, + custom_email_content=status_info.get("email"), + wrap_email=True, + ) def sync_yes_no_form_fields(self): """Some yes/no forms use a db field to track whether it was checked or not. @@ -835,19 +840,6 @@ def _send_status_update_email( except EmailSendingError: logger.warning("Failed to send confirmation email", exc_info=True) - def _send_custom_status_update_email(self, email_content): - """Wrapper for `_send_status_update_email` that bcc's help@get.gov - and sends an email equivalent to the 'email_content' variable.""" - bcc_address = settings.DEFAULT_FROM_EMAIL if settings.IS_PRODUCTION else "" - self._send_status_update_email( - new_status="action needed", - email_template=f"emails/includes/custom_email.txt", - email_template_subject=f"emails/status_change_subject.txt", - bcc_address=bcc_address, - custom_email_content=email_content, - wrap_email=True, - ) - def investigator_exists_and_is_staff(self): """Checks if the current investigator is in a valid state for a state transition""" is_valid = True @@ -959,23 +951,23 @@ def action_needed(self): This action cleans up the rejection status if moving away from rejected. As side effects this will delete the domain and domain_information - (will cascade) when they exist.""" + (will cascade) when they exist. + + Afterwards, we send out an email for action_needed in def save(). + See the function send_custom_status_update_email. + """ if self.status == self.DomainRequestStatus.APPROVED: self.delete_and_clean_up_domain("reject_with_prejudice") elif self.status == self.DomainRequestStatus.REJECTED: self.rejection_reason = None + # Check if the tuple is setup correctly, then grab its value. + literal = DomainRequest.DomainRequestStatus.ACTION_NEEDED - # Check if the tuple is setup correctly, then grab its value action_needed = literal if literal is not None else "Action Needed" logger.info(f"A status change occurred. {self} was changed to '{action_needed}'") - # Send out an email if an action needed reason exists - if self.action_needed_reason and self.action_needed_reason != self.ActionNeededReasons.OTHER: - email_content = self.action_needed_reason_email - self._send_custom_status_update_email(email_content) - @transition( field="status", source=[ @@ -1070,16 +1062,12 @@ def reject(self): """Reject an domain request that has been submitted. As side effects this will delete the domain and domain_information - (will cascade), and send an email notification.""" + (will cascade), and send an email notification using send_custom_status_update_email. + """ if self.status == self.DomainRequestStatus.APPROVED: self.delete_and_clean_up_domain("reject") - # Send out an email if a rejection reason exists - if self.rejection_reason: - email_content = self.rejection_reason_email - self._send_custom_status_update_email(email_content) - @transition( field="status", source=[ From 5a96855ccc20abdef4d80280711c0a0aea4bd16a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 1 Oct 2024 09:24:48 -0600 Subject: [PATCH 12/34] Simplify PR Remove refactors + simplify logic --- src/registrar/admin.py | 6 - src/registrar/models/domain_request.py | 14 +- .../emails/includes/email_footer.txt | 10 -- .../status_change_rejected_header.txt | 8 - .../contacts_not_verified.txt | 8 - .../rejection_reasons/naming_not_met.txt | 15 -- .../rejection_reasons/org_has_domain.txt | 15 -- .../rejection_reasons/org_not_eligible.txt | 14 -- .../rejection_reasons/purpose_not_met.txt | 15 -- .../requestor_not_eligible.txt | 14 -- .../emails/status_change_rejected.txt | 16 +- src/registrar/tests/test_admin_request.py | 151 +++++++++++------- src/registrar/tests/test_models.py | 4 + src/registrar/utility/admin_helpers.py | 54 +------ 14 files changed, 120 insertions(+), 224 deletions(-) delete mode 100644 src/registrar/templates/emails/includes/email_footer.txt delete mode 100644 src/registrar/templates/emails/includes/status_change_rejected_header.txt delete mode 100644 src/registrar/templates/emails/rejection_reasons/contacts_not_verified.txt delete mode 100644 src/registrar/templates/emails/rejection_reasons/naming_not_met.txt delete mode 100644 src/registrar/templates/emails/rejection_reasons/org_has_domain.txt delete mode 100644 src/registrar/templates/emails/rejection_reasons/org_not_eligible.txt delete mode 100644 src/registrar/templates/emails/rejection_reasons/purpose_not_met.txt delete mode 100644 src/registrar/templates/emails/rejection_reasons/requestor_not_eligible.txt diff --git a/src/registrar/admin.py b/src/registrar/admin.py index b9204666b..84e7a22ff 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -21,12 +21,6 @@ from registrar.models.user_domain_role import UserDomainRole from waffle.admin import FlagAdmin from waffle.models import Sample, Switch -from registrar.utility.admin_helpers import ( - get_all_action_needed_reason_emails, - get_action_needed_reason_default_email, - get_all_rejection_reason_emails, - get_rejection_reason_default_email, -) from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website, SeniorOfficial from registrar.utility.constants import BranchChoices from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 2210ac9d9..a41550ffb 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -254,18 +254,18 @@ class OrganizationChoicesVerbose(models.TextChoices): ) class RejectionReasons(models.TextChoices): - DOMAIN_PURPOSE = "purpose_not_met", "Purpose requirements not met" - REQUESTOR = "requestor_not_eligible", "Requestor not eligible to make request" - SECOND_DOMAIN_REASONING = ( + DOMAIN_PURPOSE = "domain_purpose", "Purpose requirements not met" + REQUESTOR_NOT_ELIGIBLE = "requestor_not_eligible", "Requestor not eligible to make request" + ORG_HAS_DOMAIN = ( "org_has_domain", "Org already has a .gov domain", ) - CONTACTS_OR_ORGANIZATION_LEGITIMACY = ( + CONTACTS_NOT_VERIFIED = ( "contacts_not_verified", "Org contacts couldn't be verified", ) - ORGANIZATION_ELIGIBILITY = "org_not_eligible", "Org not eligible for a .gov domain" - NAMING_REQUIREMENTS = "naming_not_met", "Naming requirements not met" + ORG_NOT_ELIGIBLE = "org_not_eligible", "Org not eligible for a .gov domain" + NAMING_REQUIREMENTS = "naming_requirements", "Naming requirements not met" OTHER = "other", "Other/Unspecified" @classmethod @@ -958,7 +958,7 @@ def action_needed(self): """ if self.status == self.DomainRequestStatus.APPROVED: - self.delete_and_clean_up_domain("reject_with_prejudice") + self.delete_and_clean_up_domain("action_needed") elif self.status == self.DomainRequestStatus.REJECTED: self.rejection_reason = None diff --git a/src/registrar/templates/emails/includes/email_footer.txt b/src/registrar/templates/emails/includes/email_footer.txt deleted file mode 100644 index f10d82a91..000000000 --- a/src/registrar/templates/emails/includes/email_footer.txt +++ /dev/null @@ -1,10 +0,0 @@ -THANK YOU -.Gov helps the public identify official, trusted information. Thank you for requesting a .gov domain. - ----------------------------------------------------------------- - -The .gov team -Contact us: -Learn about .gov - -The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) \ No newline at end of file diff --git a/src/registrar/templates/emails/includes/status_change_rejected_header.txt b/src/registrar/templates/emails/includes/status_change_rejected_header.txt deleted file mode 100644 index 16b7c73a9..000000000 --- a/src/registrar/templates/emails/includes/status_change_rejected_header.txt +++ /dev/null @@ -1,8 +0,0 @@ -Hi, {{ recipient.first_name }}. - -Your .gov domain request has been rejected. - -DOMAIN REQUESTED: {{ domain_request.requested_domain.name }} -REQUEST RECEIVED ON: {{ domain_request.last_submitted_date|date }} -STATUS: Rejected ----------------------------------------------------------------- \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/contacts_not_verified.txt b/src/registrar/templates/emails/rejection_reasons/contacts_not_verified.txt deleted file mode 100644 index 525a3a00a..000000000 --- a/src/registrar/templates/emails/rejection_reasons/contacts_not_verified.txt +++ /dev/null @@ -1,8 +0,0 @@ -{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} -{% include "emails/includes/status_change_rejected_header.txt" %} -REJECTION REASON -Your domain request was rejected because we could not verify the organizational -contacts you provided. If you have questions or comments, reply to this email. - -{% include "emails/includes/email_footer.txt" %} -{% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/naming_not_met.txt b/src/registrar/templates/emails/rejection_reasons/naming_not_met.txt deleted file mode 100644 index eb2e5e4c0..000000000 --- a/src/registrar/templates/emails/rejection_reasons/naming_not_met.txt +++ /dev/null @@ -1,15 +0,0 @@ -{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} -{% include "emails/includes/status_change_rejected_header.txt" %} -REJECTION REASON -Your domain request was rejected because it does not meet our naming requirements. -Domains should uniquely identify a government organization and be clear to the -general public. Learn more about naming requirements for your type of organization -. - - -YOU CAN SUBMIT A NEW REQUEST -We encourage you to request a domain that meets our requirements. If you have -questions or want to discuss potential domain names, reply to this email. - -{% include "emails/includes/email_footer.txt" %} -{% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/org_has_domain.txt b/src/registrar/templates/emails/rejection_reasons/org_has_domain.txt deleted file mode 100644 index 175518ac3..000000000 --- a/src/registrar/templates/emails/rejection_reasons/org_has_domain.txt +++ /dev/null @@ -1,15 +0,0 @@ -{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} -{% include "emails/includes/status_change_rejected_header.txt" %} -REJECTION REASON -Your domain request was rejected because {{ domain_request.organization_name }} has a .gov domain. Our -practice is to approve one domain per online service per government organization. We -evaluate additional requests on a case-by-case basis. You did not provide sufficient -justification for an additional domain. - -Read more about our practice of approving one domain per online service -. - -If you have questions or comments, reply to this email. - -{% include "emails/includes/email_footer.txt" %} -{% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/org_not_eligible.txt b/src/registrar/templates/emails/rejection_reasons/org_not_eligible.txt deleted file mode 100644 index 606184706..000000000 --- a/src/registrar/templates/emails/rejection_reasons/org_not_eligible.txt +++ /dev/null @@ -1,14 +0,0 @@ -{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} -{% include "emails/includes/status_change_rejected_header.txt" %} -REJECTION REASON -Your domain request was rejected because we determined that {{ domain_request.organization_name }} is not -eligible for a .gov domain. .Gov domains are only available to official U.S.-based -government organizations. - -Learn more about eligibility for .gov domains -. - -If you have questions or comments, reply to this email. - -{% include "emails/includes/email_footer.txt" %} -{% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/purpose_not_met.txt b/src/registrar/templates/emails/rejection_reasons/purpose_not_met.txt deleted file mode 100644 index d95a9e0b3..000000000 --- a/src/registrar/templates/emails/rejection_reasons/purpose_not_met.txt +++ /dev/null @@ -1,15 +0,0 @@ -{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} -{% include "emails/includes/status_change_rejected_header.txt" %} -REJECTION REASON -Your domain request was rejected because the purpose you provided did not meet our -requirements. You didn’t provide enough information about how you intend to use the -domain. - -Learn more about: -- Eligibility for a .gov domain -- What you can and can’t do with .gov domains - -If you have questions or comments, reply to this email. - -{% include "emails/includes/email_footer.txt" %} -{% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/rejection_reasons/requestor_not_eligible.txt b/src/registrar/templates/emails/rejection_reasons/requestor_not_eligible.txt deleted file mode 100644 index deeb2d9da..000000000 --- a/src/registrar/templates/emails/rejection_reasons/requestor_not_eligible.txt +++ /dev/null @@ -1,14 +0,0 @@ -{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} -{% include "emails/includes/status_change_rejected_header.txt" %} -REJECTION REASON -Your domain request was rejected because we don’t believe you’re eligible to request a -.gov domain on behalf of {{ domain_request.organization_name }}. You must be a government employee, or be -working on behalf of a government organization, to request a .gov domain. - - -DEMONSTRATE ELIGIBILITY -If you can provide more information that demonstrates your eligibility, or you want to -discuss further, reply to this email. - -{% include "emails/includes/email_footer.txt" %} -{% endautoescape %} \ No newline at end of file diff --git a/src/registrar/templates/emails/status_change_rejected.txt b/src/registrar/templates/emails/status_change_rejected.txt index 4e5250162..62e8d6acb 100644 --- a/src/registrar/templates/emails/status_change_rejected.txt +++ b/src/registrar/templates/emails/status_change_rejected.txt @@ -8,8 +8,8 @@ REQUEST RECEIVED ON: {{ domain_request.last_submitted_date|date }} STATUS: Rejected ---------------------------------------------------------------- -{% if domain_request.rejection_reason != 'other' %} -REJECTION REASON{% endif %}{% if domain_request.rejection_reason == 'purpose_not_met' %} +{% if reason != domain_request.RejectionReasons.DOMAIN_PURPOSE.OTHER %} +REJECTION REASON{% endif %}{% if reason == domain_request.RejectionReasons.DOMAIN_PURPOSE %} Your domain request was rejected because the purpose you provided did not meet our requirements. You didn’t provide enough information about how you intend to use the domain. @@ -18,7 +18,7 @@ Learn more about: - Eligibility for a .gov domain - What you can and can’t do with .gov domains -If you have questions or comments, reply to this email.{% elif domain_request.rejection_reason == 'requestor_not_eligible' %} +If you have questions or comments, reply to this email.{% elif reason == domain_request.RejectionReasons.DOMAIN_PURPOSE.REQUESTOR_NOT_ELIGIBLE %} Your domain request was rejected because we don’t believe you’re eligible to request a .gov domain on behalf of {{ domain_request.organization_name }}. You must be a government employee, or be working on behalf of a government organization, to request a .gov domain. @@ -26,7 +26,7 @@ working on behalf of a government organization, to request a .gov domain. DEMONSTRATE ELIGIBILITY If you can provide more information that demonstrates your eligibility, or you want to -discuss further, reply to this email.{% elif domain_request.rejection_reason == 'org_has_domain' %} +discuss further, reply to this email.{% elif reason == domain_request.RejectionReasons.DOMAIN_PURPOSE.ORG_HAS_DOMAIN %} Your domain request was rejected because {{ domain_request.organization_name }} has a .gov domain. Our practice is to approve one domain per online service per government organization. We evaluate additional requests on a case-by-case basis. You did not provide sufficient @@ -35,9 +35,9 @@ justification for an additional domain. Read more about our practice of approving one domain per online service . -If you have questions or comments, reply to this email.{% elif domain_request.rejection_reason == 'contacts_not_verified' %} +If you have questions or comments, reply to this email.{% elif reason == 'contacts_not_verified' %} Your domain request was rejected because we could not verify the organizational -contacts you provided. If you have questions or comments, reply to this email.{% elif domain_request.rejection_reason == 'org_not_eligible' %} +contacts you provided. If you have questions or comments, reply to this email.{% elif reason == domain_request.RejectionReasons.DOMAIN_PURPOSE.ORG_NOT_ELIGIBLE %} Your domain request was rejected because we determined that {{ domain_request.organization_name }} is not eligible for a .gov domain. .Gov domains are only available to official U.S.-based government organizations. @@ -46,7 +46,7 @@ Learn more about eligibility for .gov domains . If you have questions or comments, reply to this email. -{% elif domain_request.rejection_reason == 'naming_not_met' %} +{% elif reason == domain_request.RejectionReasons.DOMAIN_PURPOSE.NAMING_NOT_MET %} Your domain request was rejected because it does not meet our naming requirements. Domains should uniquely identify a government organization and be clear to the general public. Learn more about naming requirements for your type of organization @@ -55,7 +55,7 @@ general public. Learn more about naming requirements for your type of organizati YOU CAN SUBMIT A NEW REQUEST We encourage you to request a domain that meets our requirements. If you have -questions or want to discuss potential domain names, reply to this email.{% elif domain_request.rejection_reason == 'other' %} +questions or want to discuss potential domain names, reply to this email.{% elif reason == domain_request.RejectionReasons.DOMAIN_PURPOSE.OTHER %} YOU CAN SUBMIT A NEW REQUEST If your organization is eligible for a .gov domain and you meet our other requirements, you can submit a new request. diff --git a/src/registrar/tests/test_admin_request.py b/src/registrar/tests/test_admin_request.py index a9b073472..382a1e973 100644 --- a/src/registrar/tests/test_admin_request.py +++ b/src/registrar/tests/test_admin_request.py @@ -595,7 +595,12 @@ def test_default_status_in_domain_requests_list(self): @less_console_noise_decorator def transition_state_and_send_email( - self, domain_request, status, rejection_reason=None, action_needed_reason=None, action_needed_reason_email=None + self, + domain_request, + status, + rejection_reason=None, + rejection_reason_email=None, + action_needed_reason=None, action_needed_reason_email=None ): """Helper method for the email test cases.""" @@ -612,6 +617,9 @@ def transition_state_and_send_email( if rejection_reason: domain_request.rejection_reason = rejection_reason + + if rejection_reason_email: + domain_request.rejection_reason_email = rejection_reason_email if action_needed_reason: domain_request.action_needed_reason = action_needed_reason @@ -632,6 +640,7 @@ def assert_email_is_accurate( with less_console_noise(): # Access the arguments passed to send_email call_args = self.mock_client.EMAILS_SENT + logger.info(f"what are the call args? {call_args}") kwargs = call_args[email_index]["kwargs"] # Retrieve the email details from the arguments @@ -757,55 +766,85 @@ def test_action_needed_sends_reason_email_prod_bcc(self): ) self.assertEqual(len(self.mock_client.EMAILS_SENT), 6) - # def test_action_needed_sends_reason_email_prod_bcc(self): - # """When an action needed reason is set, an email is sent out and help@get.gov - # is BCC'd in production""" - # # Ensure there is no user with this email - # EMAIL = "mayor@igorville.gov" - # BCC_EMAIL = settings.DEFAULT_FROM_EMAIL - # User.objects.filter(email=EMAIL).delete() - # in_review = DomainRequest.DomainRequestStatus.IN_REVIEW - # action_needed = DomainRequest.DomainRequestStatus.ACTION_NEEDED - - # # Create a sample domain request - # domain_request = completed_domain_request(status=in_review) - - # # Test the email sent out for already_has_domains - # already_has_domains = DomainRequest.ActionNeededReasons.ALREADY_HAS_DOMAINS - # self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=already_has_domains) - # self.assert_email_is_accurate("ORGANIZATION ALREADY HAS A .GOV DOMAIN", 0, EMAIL, bcc_email_address=BCC_EMAIL) - # self.assertEqual(len(self.mock_client.EMAILS_SENT), 1) - - # # Test the email sent out for bad_name - # bad_name = DomainRequest.ActionNeededReasons.BAD_NAME - # self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=bad_name) - # self.assert_email_is_accurate( - # "DOMAIN NAME DOES NOT MEET .GOV REQUIREMENTS", 1, EMAIL, bcc_email_address=BCC_EMAIL - # ) - # self.assertEqual(len(self.mock_client.EMAILS_SENT), 2) - - # # Test the email sent out for eligibility_unclear - # eligibility_unclear = DomainRequest.ActionNeededReasons.ELIGIBILITY_UNCLEAR - # self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=eligibility_unclear) - # self.assert_email_is_accurate( - # "ORGANIZATION MAY NOT MEET ELIGIBILITY REQUIREMENTS", 2, EMAIL, bcc_email_address=BCC_EMAIL - # ) - # self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) - - # # Test the email 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( - # "SENIOR OFFICIAL DOES NOT MEET ELIGIBILITY REQUIREMENTS", 3, EMAIL, bcc_email_address=BCC_EMAIL - # ) - # self.assertEqual(len(self.mock_client.EMAILS_SENT), 4) - - # # Assert that no other emails are sent on OTHER - # other = DomainRequest.ActionNeededReasons.OTHER - # self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=other) - - # # Should be unchanged from before - # self.assertEqual(len(self.mock_client.EMAILS_SENT), 4) + @override_settings(IS_PRODUCTION=True) + @less_console_noise_decorator + def test_rejected_sends_reason_email_prod_bcc(self): + """When a rejection reason is set, an email is sent out and help@get.gov + is BCC'd in production""" + # Create fake creator + EMAIL = "meoward.jones@igorville.gov" + + _creator = User.objects.create( + username="MrMeoward", + first_name="Meoward", + last_name="Jones", + email=EMAIL, + phone="(555) 123 12345", + title="Treat inspector", + ) + + BCC_EMAIL = settings.DEFAULT_FROM_EMAIL + in_review = DomainRequest.DomainRequestStatus.IN_REVIEW + rejected = DomainRequest.DomainRequestStatus.REJECTED + + # Create a sample domain request + domain_request = completed_domain_request(status=in_review, user=_creator) + + expected_emails = { + DomainRequest.RejectionReasons.DOMAIN_PURPOSE: "You didn’t provide enough information about how", + DomainRequest.RejectionReasons.REQUESTOR_NOT_ELIGIBLE: "You must be a government employee, or be", + DomainRequest.RejectionReasons.ORG_HAS_DOMAIN: "Our practice is to approve one domain", + DomainRequest.RejectionReasons.CONTACTS_NOT_VERIFIED: "we could not verify the organizational", + DomainRequest.RejectionReasons.ORG_NOT_ELIGIBLE: ".Gov domains are only available to official U.S.-based", + DomainRequest.RejectionReasons.NAMING_REQUIREMENTS: "does not meet our naming requirements", + # TODO - add back other? + #DomainRequest.RejectionReasons.OTHER: "", + } + for i, (reason, email_content) in enumerate(expected_emails.items()): + with self.subTest(reason=reason): + self.transition_state_and_send_email(domain_request, status=rejected, rejection_reason=reason) + self.assert_email_is_accurate(email_content, i, EMAIL, bcc_email_address=BCC_EMAIL) + self.assertEqual(len(self.mock_client.EMAILS_SENT), i+1) + + # Tests if an analyst can override existing email content + domain_purpose = DomainRequest.RejectionReasons.DOMAIN_PURPOSE + self.transition_state_and_send_email( + domain_request, + status=rejected, + rejection_reason=domain_purpose, + rejection_reason_email="custom email content", + ) + + logger.info(f"look: {len(self.mock_client.EMAILS_SENT)}") + domain_request.refresh_from_db() + self.assert_email_is_accurate("custom email content", 6, _creator.email, bcc_email_address=BCC_EMAIL) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 7) + + # 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, + status=rejected, + action_needed_reason=domain_purpose, + action_needed_reason_email="dummy email content", + ) + + self.assertEqual(len(self.mock_client.EMAILS_SENT), 7) + + # Set the request back to in review + domain_request.in_review() + + # Try sending another email when changing states AND including content + self.transition_state_and_send_email( + domain_request, + status=rejected, + rejection_reason=domain_purpose, + rejection_reason_email="custom content when starting anew", + ) + self.assert_email_is_accurate( + "custom content when starting anew", 7, _creator.email, bcc_email_address=BCC_EMAIL + ) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 8) @less_console_noise_decorator def test_save_model_sends_submitted_email(self): @@ -1034,7 +1073,7 @@ def test_save_model_sends_rejected_email_requestor(self): # Reject for reason REQUESTOR and test email including dynamic organization name self.transition_state_and_send_email( - domain_request, DomainRequest.DomainRequestStatus.REJECTED, DomainRequest.RejectionReasons.REQUESTOR + domain_request, DomainRequest.DomainRequestStatus.REJECTED, DomainRequest.RejectionReasons.REQUESTOR_NOT_ELIGIBLE ) self.assert_email_is_accurate( "Your domain request was rejected because we don’t believe you’re eligible to request a \n.gov " @@ -1072,7 +1111,7 @@ def test_save_model_sends_rejected_email_org_has_domain(self): self.transition_state_and_send_email( domain_request, DomainRequest.DomainRequestStatus.REJECTED, - DomainRequest.RejectionReasons.SECOND_DOMAIN_REASONING, + DomainRequest.RejectionReasons.ORG_HAS_DOMAIN, ) self.assert_email_is_accurate( "Your domain request was rejected because Testorg has a .gov domain.", 0, _creator.email @@ -1108,7 +1147,7 @@ def test_save_model_sends_rejected_email_contacts_or_org_legitimacy(self): self.transition_state_and_send_email( domain_request, DomainRequest.DomainRequestStatus.REJECTED, - DomainRequest.RejectionReasons.CONTACTS_OR_ORGANIZATION_LEGITIMACY, + DomainRequest.RejectionReasons.CONTACTS_NOT_VERIFIED, ) self.assert_email_is_accurate( "Your domain request was rejected because we could not verify the organizational \n" @@ -1146,7 +1185,7 @@ def test_save_model_sends_rejected_email_org_eligibility(self): self.transition_state_and_send_email( domain_request, DomainRequest.DomainRequestStatus.REJECTED, - DomainRequest.RejectionReasons.ORGANIZATION_ELIGIBILITY, + DomainRequest.RejectionReasons.ORG_NOT_ELIGIBLE, ) self.assert_email_is_accurate( "Your domain request was rejected because we determined that Testorg is not \neligible for " @@ -1275,7 +1314,7 @@ def test_transition_to_rejected_with_rejection_reason_does_not_trigger_error(sel stack.enter_context(patch.object(messages, "error")) stack.enter_context(patch.object(messages, "warning")) domain_request.status = DomainRequest.DomainRequestStatus.REJECTED - domain_request.rejection_reason = DomainRequest.RejectionReasons.CONTACTS_OR_ORGANIZATION_LEGITIMACY + domain_request.rejection_reason = DomainRequest.RejectionReasons.CONTACTS_NOT_VERIFIED self.admin.save_model(request, domain_request, None, True) @@ -1840,7 +1879,7 @@ def test_side_effects_when_saving_approved_to_rejected(self): self.trigger_saving_approved_to_another_state( False, DomainRequest.DomainRequestStatus.REJECTED, - DomainRequest.RejectionReasons.CONTACTS_OR_ORGANIZATION_LEGITIMACY, + DomainRequest.RejectionReasons.CONTACTS_NOT_VERIFIED, ) def test_side_effects_when_saving_approved_to_ineligible(self): diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index a6cac1389..dab8ff242 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -39,9 +39,13 @@ from django_fsm import TransitionNotAllowed from waffle.testutils import override_flag +import logging + from api.tests.common import less_console_noise_decorator +logger = logging.getLogger(__name__) + @boto3_mocking.patching class TestDomainRequest(TestCase): @less_console_noise_decorator diff --git a/src/registrar/utility/admin_helpers.py b/src/registrar/utility/admin_helpers.py index 20760164e..2f6d2ae8b 100644 --- a/src/registrar/utility/admin_helpers.py +++ b/src/registrar/utility/admin_helpers.py @@ -2,63 +2,26 @@ from django.template.loader import get_template -def get_all_action_needed_reason_emails(domain_request): - """Returns a dictionary of every action needed reason and its associated email - for this particular domain request.""" - return _get_all_default_emails( - reasons=DomainRequest.ActionNeededReasons, - # Where the emails are stored. This assumes that it contains a list of .txt files with the reason. - path_root="emails/action_needed_reasons", - # What reasons don't send out emails (none is handled automagically) - excluded_reasons=[DomainRequest.ActionNeededReasons.OTHER], - # Who to send it to, and from what domain request to reference - domain_request=domain_request - ) - - def get_action_needed_reason_default_email(domain_request, action_needed_reason): """Returns the default email associated with the given action needed reason""" return _get_default_email( domain_request, - path_root="emails/action_needed_reasons", + file_path=f"emails/action_needed_reasons/{action_needed_reason}.txt", reason=action_needed_reason, excluded_reasons=[DomainRequest.ActionNeededReasons.OTHER] ) -def get_all_rejection_reason_emails(domain_request): - """Returns a dictionary of every rejection reason and its associated email - for this particular domain request.""" - return _get_all_default_emails( - reasons=DomainRequest.RejectionReasons, - # Where the emails are stored. This assumes that it contains a list of .txt files with the reason. - path_root="emails/rejection_reasons", - # What reasons don't send out emails (none is handled automagically) - excluded_reasons=[DomainRequest.RejectionReasons.OTHER], - # Who to send it to, and from what domain request to reference - domain_request=domain_request - ) - - def get_rejection_reason_default_email(domain_request, rejection_reason): """Returns the default email associated with the given rejection reason""" return _get_default_email( domain_request, - path_root="emails/rejection_reasons", + file_path="emails/status_change_rejected.txt", reason=rejection_reason, excluded_reasons=[DomainRequest.RejectionReasons.OTHER] ) -def _get_all_default_emails(reasons, path_root, excluded_reasons, domain_request): - emails = {} - for reason in reasons: - # Map the reason to its default email - emails[reason.value] = _get_default_email( - domain_request, path_root, reason, excluded_reasons - ) - return emails - -def _get_default_email(domain_request, path_root, reason, excluded_reasons=None): +def _get_default_email(domain_request, file_path, reason, excluded_reasons=None): if not reason: return None @@ -67,14 +30,9 @@ def _get_default_email(domain_request, path_root, reason, excluded_reasons=None) recipient = domain_request.creator # Return the context of the rendered views - context = {"domain_request": domain_request, "recipient": recipient} - - # Get the email body - template_path = f"{path_root}/{reason}.txt" + context = {"domain_request": domain_request, "recipient": recipient, "reason": reason} - 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") + email_body_text = get_template(file_path).render(context=context) + email_body_text_cleaned = email_body_text.strip().lstrip("\n") if email_body_text else None return email_body_text_cleaned From 47d226ff5e32ed2f202aeb2fcacbc5f51d7f9db6 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 1 Oct 2024 09:28:07 -0600 Subject: [PATCH 13/34] Update domain_request.py --- src/registrar/models/domain_request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index a41550ffb..426d81ea3 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -707,7 +707,7 @@ def send_custom_status_update_email(self, status): if status_info.get("cached_reason") != status_info.get("reason") or status_info.get("cached_reason") is None: bcc_address = settings.DEFAULT_FROM_EMAIL if settings.IS_PRODUCTION else "" self._send_status_update_email( - new_status="action needed", + new_status=status.label, email_template=f"emails/includes/custom_email.txt", email_template_subject=f"emails/status_change_subject.txt", bcc_address=bcc_address, From 162369f5db4ddfda404d67aa50afa8091a7051f1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 1 Oct 2024 09:32:25 -0600 Subject: [PATCH 14/34] add migration --- ...30_alter_domainrequest_rejection_reason.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 src/registrar/migrations/0130_alter_domainrequest_rejection_reason.py diff --git a/src/registrar/migrations/0130_alter_domainrequest_rejection_reason.py b/src/registrar/migrations/0130_alter_domainrequest_rejection_reason.py new file mode 100644 index 000000000..58a09772b --- /dev/null +++ b/src/registrar/migrations/0130_alter_domainrequest_rejection_reason.py @@ -0,0 +1,30 @@ +# Generated by Django 4.2.10 on 2024-10-01 15:31 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0129_domainrequest_rejection_reason_email"), + ] + + operations = [ + migrations.AlterField( + model_name="domainrequest", + name="rejection_reason", + field=models.TextField( + blank=True, + choices=[ + ("domain_purpose", "Purpose requirements not met"), + ("requestor_not_eligible", "Requestor not eligible to make request"), + ("org_has_domain", "Org already has a .gov domain"), + ("contacts_not_verified", "Org contacts couldn't be verified"), + ("org_not_eligible", "Org not eligible for a .gov domain"), + ("naming_requirements", "Naming requirements not met"), + ("other", "Other/Unspecified"), + ], + null=True, + ), + ), + ] From 0a7d4e460d84dffd810c855c728af656e4f3dfd7 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 1 Oct 2024 09:40:39 -0600 Subject: [PATCH 15/34] Exclude other reason for now --- src/registrar/assets/js/get-gov-admin.js | 13 ++++++++----- src/registrar/utility/admin_helpers.py | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index dbe7b01d3..c8df161bb 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -581,11 +581,11 @@ class CustomizableEmailBase { return this.lastSentEmailContent.value.replace(/\s+/g, '') === this.textarea.value.replace(/\s+/g, ''); } - updateUserInterface(reason=this.dropdown.value) { + updateUserInterface(reason=this.dropdown.value, excluded_reasons=["other"]) { if (!reason) { // No reason selected, we will set the label to "Email", show the "Make a selection" placeholder, hide the trigger, textarea, hide the help text this.showPlaceholderNoReason(); - } else if (reason === 'other') { + } else if (excluded_reasons.includes(reason)) { // 'Other' selected, we will set the label to "Email", show the "No email will be sent" placeholder, hide the trigger, textarea, hide the help text this.showPlaceholderOtherReason(); } else { @@ -728,10 +728,13 @@ class customRejectedEmail extends CustomizableEmailBase { this.showPlaceholder("Email:", "Select a rejection reason to see email"); } - // Overrides the placeholder text when the reason other is selected - showPlaceholderOtherReason() { - this.showPlaceholder("Email:", "No email will be sent"); + updateUserInterface(reason=this.dropdown.value, excluded_reasons=[]) { + super.updateUserInterface(reason, excluded_reasons); } + // Overrides the placeholder text when the reason other is selected + // showPlaceholderOtherReason() { + // this.showPlaceholder("Email:", "No email will be sent"); + // } } diff --git a/src/registrar/utility/admin_helpers.py b/src/registrar/utility/admin_helpers.py index 2f6d2ae8b..19ea4c7b5 100644 --- a/src/registrar/utility/admin_helpers.py +++ b/src/registrar/utility/admin_helpers.py @@ -18,7 +18,7 @@ def get_rejection_reason_default_email(domain_request, rejection_reason): domain_request, file_path="emails/status_change_rejected.txt", reason=rejection_reason, - excluded_reasons=[DomainRequest.RejectionReasons.OTHER] + # excluded_reasons=[DomainRequest.RejectionReasons.OTHER] ) def _get_default_email(domain_request, file_path, reason, excluded_reasons=None): From 06e4daef6ac4395385c4ff8eb993af98ecaaf5c1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 1 Oct 2024 10:28:40 -0600 Subject: [PATCH 16/34] lint --- src/registrar/models/domain_request.py | 4 ++-- src/registrar/tests/test_admin_request.py | 23 +++++++++++++---------- src/registrar/tests/test_models.py | 3 +++ src/registrar/utility/admin_helpers.py | 3 ++- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 426d81ea3..617143ac7 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -689,7 +689,7 @@ def send_custom_status_update_email(self, status): "reason": self.rejection_reason, "email": self.rejection_reason_email, "excluded_reasons": [DomainRequest.RejectionReasons.OTHER], - } + }, } status_info = status_information.get(status) @@ -952,7 +952,7 @@ def action_needed(self): As side effects this will delete the domain and domain_information (will cascade) when they exist. - + Afterwards, we send out an email for action_needed in def save(). See the function send_custom_status_update_email. """ diff --git a/src/registrar/tests/test_admin_request.py b/src/registrar/tests/test_admin_request.py index 382a1e973..5104f23fb 100644 --- a/src/registrar/tests/test_admin_request.py +++ b/src/registrar/tests/test_admin_request.py @@ -595,12 +595,13 @@ def test_default_status_in_domain_requests_list(self): @less_console_noise_decorator def transition_state_and_send_email( - self, - domain_request, - status, - rejection_reason=None, - rejection_reason_email=None, - action_needed_reason=None, action_needed_reason_email=None + self, + domain_request, + status, + rejection_reason=None, + rejection_reason_email=None, + action_needed_reason=None, + action_needed_reason_email=None, ): """Helper method for the email test cases.""" @@ -617,7 +618,7 @@ def transition_state_and_send_email( if rejection_reason: domain_request.rejection_reason = rejection_reason - + if rejection_reason_email: domain_request.rejection_reason_email = rejection_reason_email @@ -798,13 +799,13 @@ def test_rejected_sends_reason_email_prod_bcc(self): DomainRequest.RejectionReasons.ORG_NOT_ELIGIBLE: ".Gov domains are only available to official U.S.-based", DomainRequest.RejectionReasons.NAMING_REQUIREMENTS: "does not meet our naming requirements", # TODO - add back other? - #DomainRequest.RejectionReasons.OTHER: "", + # DomainRequest.RejectionReasons.OTHER: "", } for i, (reason, email_content) in enumerate(expected_emails.items()): with self.subTest(reason=reason): self.transition_state_and_send_email(domain_request, status=rejected, rejection_reason=reason) self.assert_email_is_accurate(email_content, i, EMAIL, bcc_email_address=BCC_EMAIL) - self.assertEqual(len(self.mock_client.EMAILS_SENT), i+1) + self.assertEqual(len(self.mock_client.EMAILS_SENT), i + 1) # Tests if an analyst can override existing email content domain_purpose = DomainRequest.RejectionReasons.DOMAIN_PURPOSE @@ -1073,7 +1074,9 @@ def test_save_model_sends_rejected_email_requestor(self): # Reject for reason REQUESTOR and test email including dynamic organization name self.transition_state_and_send_email( - domain_request, DomainRequest.DomainRequestStatus.REJECTED, DomainRequest.RejectionReasons.REQUESTOR_NOT_ELIGIBLE + domain_request, + DomainRequest.DomainRequestStatus.REJECTED, + DomainRequest.RejectionReasons.REQUESTOR_NOT_ELIGIBLE, ) self.assert_email_is_accurate( "Your domain request was rejected because we don’t believe you’re eligible to request a \n.gov " diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index dab8ff242..0b01ee0a6 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -46,6 +46,7 @@ logger = logging.getLogger(__name__) + @boto3_mocking.patching class TestDomainRequest(TestCase): @less_console_noise_decorator @@ -293,6 +294,8 @@ def check_email_sent( # Perform the specified action action_method = getattr(domain_request, action) action_method() + domain_request.save() + domain_request.refresh_from_db() # Check if an email was sent sent_emails = [ diff --git a/src/registrar/utility/admin_helpers.py b/src/registrar/utility/admin_helpers.py index 19ea4c7b5..ad7ecae19 100644 --- a/src/registrar/utility/admin_helpers.py +++ b/src/registrar/utility/admin_helpers.py @@ -8,7 +8,7 @@ def get_action_needed_reason_default_email(domain_request, action_needed_reason) domain_request, file_path=f"emails/action_needed_reasons/{action_needed_reason}.txt", reason=action_needed_reason, - excluded_reasons=[DomainRequest.ActionNeededReasons.OTHER] + excluded_reasons=[DomainRequest.ActionNeededReasons.OTHER], ) @@ -21,6 +21,7 @@ def get_rejection_reason_default_email(domain_request, rejection_reason): # excluded_reasons=[DomainRequest.RejectionReasons.OTHER] ) + def _get_default_email(domain_request, file_path, reason, excluded_reasons=None): if not reason: return None From ad43fabbab0b5cd097b9c18215ed03ca387d92e9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 1 Oct 2024 14:22:56 -0600 Subject: [PATCH 17/34] fix unit tests --- src/registrar/admin.py | 10 +++ src/registrar/models/domain_request.py | 5 +- .../emails/status_change_rejected.txt | 2 +- src/registrar/tests/test_admin_request.py | 68 ++++++------------- src/registrar/tests/test_api.py | 68 +++++++++++++++++++ 5 files changed, 103 insertions(+), 50 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 9584e3942..4410565f7 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -5,6 +5,7 @@ from django.db.models import Value, CharField, Q from django.db.models.functions import Concat, Coalesce from django.http import HttpResponseRedirect +from registrar.utility.admin_helpers import get_action_needed_reason_default_email, get_rejection_reason_default_email from django.conf import settings from django.shortcuts import redirect from django_fsm import get_available_FIELD_transitions, FSMField @@ -1939,6 +1940,15 @@ def save_model(self, request, obj, form, change): # Get the original domain request from the database. original_obj = models.DomainRequest.objects.get(pk=obj.pk) + # == Handle action needed and rejected emails == # + # Edge case: this logic is handled by javascript, so contexts outside that must be handled + if obj.status == DomainRequest.DomainRequestStatus.ACTION_NEEDED: + if obj.action_needed_reason and not obj.action_needed_reason_email: + obj.action_needed_reason_email = get_action_needed_reason_default_email(obj, obj.action_needed_reason) + elif obj.status == DomainRequest.DomainRequestStatus.REJECTED: + if obj.rejection_reason and not obj.rejection_reason_email: + obj.rejection_reason_email = get_rejection_reason_default_email(obj, obj.rejection_reason) + # == Handle allowed emails == # if obj.status in DomainRequest.get_statuses_that_send_emails() and not settings.IS_PRODUCTION: self._check_for_valid_email(request, obj) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 617143ac7..4877b3756 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -688,7 +688,8 @@ def send_custom_status_update_email(self, status): "cached_reason": self._cached_rejection_reason, "reason": self.rejection_reason, "email": self.rejection_reason_email, - "excluded_reasons": [DomainRequest.RejectionReasons.OTHER], + "excluded_reasons": [], + # "excluded_reasons": [DomainRequest.RejectionReasons.OTHER], }, } status_info = status_information.get(status) @@ -707,7 +708,7 @@ def send_custom_status_update_email(self, status): if status_info.get("cached_reason") != status_info.get("reason") or status_info.get("cached_reason") is None: bcc_address = settings.DEFAULT_FROM_EMAIL if settings.IS_PRODUCTION else "" self._send_status_update_email( - new_status=status.label, + new_status=status, email_template=f"emails/includes/custom_email.txt", email_template_subject=f"emails/status_change_subject.txt", bcc_address=bcc_address, diff --git a/src/registrar/templates/emails/status_change_rejected.txt b/src/registrar/templates/emails/status_change_rejected.txt index 62e8d6acb..b1d989bf1 100644 --- a/src/registrar/templates/emails/status_change_rejected.txt +++ b/src/registrar/templates/emails/status_change_rejected.txt @@ -46,7 +46,7 @@ Learn more about eligibility for .gov domains . If you have questions or comments, reply to this email. -{% elif reason == domain_request.RejectionReasons.DOMAIN_PURPOSE.NAMING_NOT_MET %} +{% elif reason == domain_request.RejectionReasons.DOMAIN_PURPOSE.NAMING_REQUIREMENTS %} Your domain request was rejected because it does not meet our naming requirements. Domains should uniquely identify a government organization and be clear to the general public. Learn more about naming requirements for your type of organization diff --git a/src/registrar/tests/test_admin_request.py b/src/registrar/tests/test_admin_request.py index 5104f23fb..55aacd25d 100644 --- a/src/registrar/tests/test_admin_request.py +++ b/src/registrar/tests/test_admin_request.py @@ -599,7 +599,6 @@ def transition_state_and_send_email( domain_request, status, rejection_reason=None, - rejection_reason_email=None, action_needed_reason=None, action_needed_reason_email=None, ): @@ -619,9 +618,6 @@ def transition_state_and_send_email( if rejection_reason: domain_request.rejection_reason = rejection_reason - if rejection_reason_email: - domain_request.rejection_reason_email = rejection_reason_email - if action_needed_reason: domain_request.action_needed_reason = action_needed_reason @@ -697,6 +693,10 @@ def test_action_needed_sends_reason_email_prod_bcc(self): self.assert_email_is_accurate("ORGANIZATION ALREADY HAS A .GOV DOMAIN", 0, EMAIL, bcc_email_address=BCC_EMAIL) self.assertEqual(len(self.mock_client.EMAILS_SENT), 1) + # We use javascript to reset the content of this. It is only automatically set + # if the email itself is somehow None. + self._reset_action_needed_email(domain_request) + # Test the email sent out for bad_name bad_name = DomainRequest.ActionNeededReasons.BAD_NAME self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=bad_name) @@ -704,6 +704,7 @@ def test_action_needed_sends_reason_email_prod_bcc(self): "DOMAIN NAME DOES NOT MEET .GOV REQUIREMENTS", 1, EMAIL, bcc_email_address=BCC_EMAIL ) self.assertEqual(len(self.mock_client.EMAILS_SENT), 2) + self._reset_action_needed_email(domain_request) # Test the email sent out for eligibility_unclear eligibility_unclear = DomainRequest.ActionNeededReasons.ELIGIBILITY_UNCLEAR @@ -712,6 +713,7 @@ def test_action_needed_sends_reason_email_prod_bcc(self): "ORGANIZATION MAY NOT MEET ELIGIBILITY REQUIREMENTS", 2, EMAIL, bcc_email_address=BCC_EMAIL ) self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) + self._reset_action_needed_email(domain_request) # Test that a custom email is sent out for questionable_so questionable_so = DomainRequest.ActionNeededReasons.QUESTIONABLE_SENIOR_OFFICIAL @@ -720,6 +722,7 @@ def test_action_needed_sends_reason_email_prod_bcc(self): "SENIOR OFFICIAL DOES NOT MEET ELIGIBILITY REQUIREMENTS", 3, _creator.email, bcc_email_address=BCC_EMAIL ) self.assertEqual(len(self.mock_client.EMAILS_SENT), 4) + self._reset_action_needed_email(domain_request) # Assert that no other emails are sent on OTHER other = DomainRequest.ActionNeededReasons.OTHER @@ -727,6 +730,7 @@ def test_action_needed_sends_reason_email_prod_bcc(self): # Should be unchanged from before self.assertEqual(len(self.mock_client.EMAILS_SENT), 4) + self._reset_action_needed_email(domain_request) # Tests if an analyst can override existing email content questionable_so = DomainRequest.ActionNeededReasons.QUESTIONABLE_SENIOR_OFFICIAL @@ -740,6 +744,7 @@ def test_action_needed_sends_reason_email_prod_bcc(self): domain_request.refresh_from_db() self.assert_email_is_accurate("custom email content", 4, _creator.email, bcc_email_address=BCC_EMAIL) self.assertEqual(len(self.mock_client.EMAILS_SENT), 5) + self._reset_action_needed_email(domain_request) # 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. @@ -751,6 +756,7 @@ def test_action_needed_sends_reason_email_prod_bcc(self): ) self.assertEqual(len(self.mock_client.EMAILS_SENT), 5) + self._reset_action_needed_email(domain_request) # Set the request back to in review domain_request.in_review() @@ -767,6 +773,12 @@ def test_action_needed_sends_reason_email_prod_bcc(self): ) self.assertEqual(len(self.mock_client.EMAILS_SENT), 6) + def _reset_action_needed_email(self, domain_request): + """Sets the given action needed email back to none""" + domain_request.action_needed_reason_email = None + domain_request.save() + domain_request.refresh_from_db() + @override_settings(IS_PRODUCTION=True) @less_console_noise_decorator def test_rejected_sends_reason_email_prod_bcc(self): @@ -794,58 +806,20 @@ def test_rejected_sends_reason_email_prod_bcc(self): expected_emails = { DomainRequest.RejectionReasons.DOMAIN_PURPOSE: "You didn’t provide enough information about how", DomainRequest.RejectionReasons.REQUESTOR_NOT_ELIGIBLE: "You must be a government employee, or be", - DomainRequest.RejectionReasons.ORG_HAS_DOMAIN: "Our practice is to approve one domain", + DomainRequest.RejectionReasons.ORG_HAS_DOMAIN: "practice is to approve one domain", DomainRequest.RejectionReasons.CONTACTS_NOT_VERIFIED: "we could not verify the organizational", DomainRequest.RejectionReasons.ORG_NOT_ELIGIBLE: ".Gov domains are only available to official U.S.-based", DomainRequest.RejectionReasons.NAMING_REQUIREMENTS: "does not meet our naming requirements", - # TODO - add back other? - # DomainRequest.RejectionReasons.OTHER: "", + DomainRequest.RejectionReasons.OTHER: "YOU CAN SUBMIT A NEW REQUEST", } for i, (reason, email_content) in enumerate(expected_emails.items()): with self.subTest(reason=reason): self.transition_state_and_send_email(domain_request, status=rejected, rejection_reason=reason) self.assert_email_is_accurate(email_content, i, EMAIL, bcc_email_address=BCC_EMAIL) self.assertEqual(len(self.mock_client.EMAILS_SENT), i + 1) - - # Tests if an analyst can override existing email content - domain_purpose = DomainRequest.RejectionReasons.DOMAIN_PURPOSE - self.transition_state_and_send_email( - domain_request, - status=rejected, - rejection_reason=domain_purpose, - rejection_reason_email="custom email content", - ) - - logger.info(f"look: {len(self.mock_client.EMAILS_SENT)}") - domain_request.refresh_from_db() - self.assert_email_is_accurate("custom email content", 6, _creator.email, bcc_email_address=BCC_EMAIL) - self.assertEqual(len(self.mock_client.EMAILS_SENT), 7) - - # 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, - status=rejected, - action_needed_reason=domain_purpose, - action_needed_reason_email="dummy email content", - ) - - self.assertEqual(len(self.mock_client.EMAILS_SENT), 7) - - # Set the request back to in review - domain_request.in_review() - - # Try sending another email when changing states AND including content - self.transition_state_and_send_email( - domain_request, - status=rejected, - rejection_reason=domain_purpose, - rejection_reason_email="custom content when starting anew", - ) - self.assert_email_is_accurate( - "custom content when starting anew", 7, _creator.email, bcc_email_address=BCC_EMAIL - ) - self.assertEqual(len(self.mock_client.EMAILS_SENT), 8) + domain_request.rejection_reason_email = None + domain_request.save() + domain_request.refresh_from_db() @less_console_noise_decorator def test_save_model_sends_submitted_email(self): diff --git a/src/registrar/tests/test_api.py b/src/registrar/tests/test_api.py index ef5385d72..d60099570 100644 --- a/src/registrar/tests/test_api.py +++ b/src/registrar/tests/test_api.py @@ -177,3 +177,71 @@ def test_get_action_needed_email_for_user_json_regular(self): }, ) self.assertEqual(response.status_code, 302) + + +class GetRejectionEmailForUserJsonTest(TestCase): + def setUp(self): + self.client = Client() + self.superuser = create_superuser() + self.analyst_user = create_user() + self.agency = FederalAgency.objects.create(agency="Test Agency") + self.domain_request = completed_domain_request( + federal_agency=self.agency, + name="test.gov", + status=DomainRequest.DomainRequestStatus.ACTION_NEEDED, + ) + + self.api_url = reverse("get-action-needed-email-for-user-json") + + def tearDown(self): + DomainRequest.objects.all().delete() + User.objects.all().delete() + FederalAgency.objects.all().delete() + + @less_console_noise_decorator + def test_get_action_needed_email_for_user_json_superuser(self): + """Test that a superuser can fetch the action needed email.""" + self.client.force_login(self.superuser) + + response = self.client.get( + self.api_url, + { + "reason": DomainRequest.ActionNeededReasons.ELIGIBILITY_UNCLEAR, + "domain_request_id": self.domain_request.id, + }, + ) + self.assertEqual(response.status_code, 200) + data = response.json() + self.assertIn("action_needed_email", data) + self.assertIn("ORGANIZATION MAY NOT MEET ELIGIBILITY REQUIREMENTS", data["action_needed_email"]) + + @less_console_noise_decorator + def test_get_action_needed_email_for_user_json_analyst(self): + """Test that an analyst can fetch the action needed email.""" + self.client.force_login(self.analyst_user) + + response = self.client.get( + self.api_url, + { + "reason": DomainRequest.ActionNeededReasons.QUESTIONABLE_SENIOR_OFFICIAL, + "domain_request_id": self.domain_request.id, + }, + ) + self.assertEqual(response.status_code, 200) + data = response.json() + self.assertIn("action_needed_email", data) + self.assertIn("SENIOR OFFICIAL DOES NOT MEET ELIGIBILITY REQUIREMENTS", data["action_needed_email"]) + + @less_console_noise_decorator + def test_get_action_needed_email_for_user_json_regular(self): + """Test that a regular user receives a 403 with an error message.""" + p = "password" + self.client.login(username="testuser", password=p) + response = self.client.get( + self.api_url, + { + "reason": DomainRequest.ActionNeededReasons.QUESTIONABLE_SENIOR_OFFICIAL, + "domain_request_id": self.domain_request.id, + }, + ) + self.assertEqual(response.status_code, 302) From 997fc1bbed611d0776cc3ce3665dc26fee5fa8cd Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 2 Oct 2024 09:22:56 -0600 Subject: [PATCH 18/34] Remove no longer relevant test + linting --- src/registrar/admin.py | 16 ++++++++++------ src/registrar/models/domain_request.py | 4 ++-- src/registrar/tests/test_models.py | 7 ------- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 4410565f7..55b1e5fc4 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1942,12 +1942,7 @@ def save_model(self, request, obj, form, change): # == Handle action needed and rejected emails == # # Edge case: this logic is handled by javascript, so contexts outside that must be handled - if obj.status == DomainRequest.DomainRequestStatus.ACTION_NEEDED: - if obj.action_needed_reason and not obj.action_needed_reason_email: - obj.action_needed_reason_email = get_action_needed_reason_default_email(obj, obj.action_needed_reason) - elif obj.status == DomainRequest.DomainRequestStatus.REJECTED: - if obj.rejection_reason and not obj.rejection_reason_email: - obj.rejection_reason_email = get_rejection_reason_default_email(obj, obj.rejection_reason) + obj = self._handle_custom_emails() # == Handle allowed emails == # if obj.status in DomainRequest.get_statuses_that_send_emails() and not settings.IS_PRODUCTION: @@ -1965,6 +1960,15 @@ def save_model(self, request, obj, form, change): if should_save: return super().save_model(request, obj, form, change) + def _handle_custom_emails(self, obj): + if obj.status == DomainRequest.DomainRequestStatus.ACTION_NEEDED: + if obj.action_needed_reason and not obj.action_needed_reason_email: + obj.action_needed_reason_email = get_action_needed_reason_default_email(obj, obj.action_needed_reason) + elif obj.status == DomainRequest.DomainRequestStatus.REJECTED: + if obj.rejection_reason and not obj.rejection_reason_email: + obj.rejection_reason_email = get_rejection_reason_default_email(obj, obj.rejection_reason) + return obj + def _check_for_valid_email(self, request, obj): """Certain emails are whitelisted in non-production environments, so we should display that information using this function. diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 4877b3756..f43ba80d4 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -709,8 +709,8 @@ def send_custom_status_update_email(self, status): bcc_address = settings.DEFAULT_FROM_EMAIL if settings.IS_PRODUCTION else "" self._send_status_update_email( new_status=status, - email_template=f"emails/includes/custom_email.txt", - email_template_subject=f"emails/status_change_subject.txt", + email_template="emails/includes/custom_email.txt", + email_template_subject="emails/status_change_subject.txt", bcc_address=bcc_address, custom_email_content=status_info.get("email"), wrap_email=True, diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 0b01ee0a6..ba52ba21d 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -355,13 +355,6 @@ def test_withdraw_sends_email(self): domain_request, msg, "withdraw", 1, expected_content="withdrawn", expected_email=user.email ) - @less_console_noise_decorator - def test_reject_sends_email(self): - msg = "Create a domain request and reject it and see if email was sent." - user, _ = User.objects.get_or_create(username="testy") - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.APPROVED, user=user) - self.check_email_sent(domain_request, msg, "reject", 1, expected_content="Hi", expected_email=user.email) - @less_console_noise_decorator def test_reject_with_prejudice_does_not_send_email(self): msg = "Create a domain request and reject it with prejudice and see if email was sent." From db0d4a1eb233ffbec0b6cdd3ecc2b583e7076e67 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 2 Oct 2024 10:30:08 -0600 Subject: [PATCH 19/34] Update admin.py --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 55b1e5fc4..efb832f6d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1942,7 +1942,7 @@ def save_model(self, request, obj, form, change): # == Handle action needed and rejected emails == # # Edge case: this logic is handled by javascript, so contexts outside that must be handled - obj = self._handle_custom_emails() + obj = self._handle_custom_emails(obj) # == Handle allowed emails == # if obj.status in DomainRequest.get_statuses_that_send_emails() and not settings.IS_PRODUCTION: From 27662029520fd3677e3754b071267a40b7d9a4e4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 2 Oct 2024 11:26:56 -0600 Subject: [PATCH 20/34] toggle email wrap --- src/registrar/models/domain_request.py | 4 +++- src/registrar/utility/email.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index f43ba80d4..c939bfedb 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -683,6 +683,7 @@ def send_custom_status_update_email(self, status): "reason": self.action_needed_reason, "email": self.action_needed_reason_email, "excluded_reasons": [DomainRequest.ActionNeededReasons.OTHER], + "wrap_email": True, }, self.DomainRequestStatus.REJECTED: { "cached_reason": self._cached_rejection_reason, @@ -690,6 +691,7 @@ def send_custom_status_update_email(self, status): "email": self.rejection_reason_email, "excluded_reasons": [], # "excluded_reasons": [DomainRequest.RejectionReasons.OTHER], + "wrap_email": False, }, } status_info = status_information.get(status) @@ -713,7 +715,7 @@ def send_custom_status_update_email(self, status): email_template_subject="emails/status_change_subject.txt", bcc_address=bcc_address, custom_email_content=status_info.get("email"), - wrap_email=True, + wrap_email=status_information.get("wrap_email"), ) def sync_yes_no_form_fields(self): diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 3f508fb96..5f8a93bd9 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -78,7 +78,7 @@ def send_templated_email( # Wrap the email body to a maximum width of 80 characters per line. # Not all email clients support CSS to do this, and our .txt files require parsing. if wrap_email: - email_body = wrap_text_and_preserve_paragraphs(email_body, width=110) + email_body = wrap_text_and_preserve_paragraphs(email_body, width=80) ses_client.send_email( FromEmailAddress=settings.DEFAULT_FROM_EMAIL, From 14241976fc31e36cbdfdcc4ca01aa1869380dab7 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 2 Oct 2024 11:57:30 -0600 Subject: [PATCH 21/34] Fix remaining tests --- src/registrar/tests/test_admin_request.py | 1 + src/registrar/tests/test_api.py | 28 +++++++++++------------ 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/registrar/tests/test_admin_request.py b/src/registrar/tests/test_admin_request.py index 55aacd25d..d5ffa4bc7 100644 --- a/src/registrar/tests/test_admin_request.py +++ b/src/registrar/tests/test_admin_request.py @@ -1637,6 +1637,7 @@ def test_readonly_when_restricted_creator(self): "updated_at", "status", "rejection_reason", + "rejection_reason_email", "action_needed_reason", "action_needed_reason_email", "federal_agency", diff --git a/src/registrar/tests/test_api.py b/src/registrar/tests/test_api.py index d60099570..22ad2b376 100644 --- a/src/registrar/tests/test_api.py +++ b/src/registrar/tests/test_api.py @@ -144,8 +144,8 @@ def test_get_action_needed_email_for_user_json_superuser(self): ) self.assertEqual(response.status_code, 200) data = response.json() - self.assertIn("action_needed_email", data) - self.assertIn("ORGANIZATION MAY NOT MEET ELIGIBILITY REQUIREMENTS", data["action_needed_email"]) + self.assertIn("email", data) + self.assertIn("ORGANIZATION MAY NOT MEET ELIGIBILITY REQUIREMENTS", data["email"]) @less_console_noise_decorator def test_get_action_needed_email_for_user_json_analyst(self): @@ -161,8 +161,8 @@ def test_get_action_needed_email_for_user_json_analyst(self): ) self.assertEqual(response.status_code, 200) data = response.json() - self.assertIn("action_needed_email", data) - self.assertIn("SENIOR OFFICIAL DOES NOT MEET ELIGIBILITY REQUIREMENTS", data["action_needed_email"]) + self.assertIn("email", data) + self.assertIn("SENIOR OFFICIAL DOES NOT MEET ELIGIBILITY REQUIREMENTS", data["email"]) @less_console_noise_decorator def test_get_action_needed_email_for_user_json_regular(self): @@ -188,10 +188,10 @@ def setUp(self): self.domain_request = completed_domain_request( federal_agency=self.agency, name="test.gov", - status=DomainRequest.DomainRequestStatus.ACTION_NEEDED, + status=DomainRequest.DomainRequestStatus.REJECTED, ) - self.api_url = reverse("get-action-needed-email-for-user-json") + self.api_url = reverse("get-rejection-email-for-user-json") def tearDown(self): DomainRequest.objects.all().delete() @@ -199,21 +199,21 @@ def tearDown(self): FederalAgency.objects.all().delete() @less_console_noise_decorator - def test_get_action_needed_email_for_user_json_superuser(self): + def test_get_rejected_email_for_user_json_superuser(self): """Test that a superuser can fetch the action needed email.""" self.client.force_login(self.superuser) response = self.client.get( self.api_url, { - "reason": DomainRequest.ActionNeededReasons.ELIGIBILITY_UNCLEAR, + "reason": DomainRequest.RejectionReasons.CONTACTS_NOT_VERIFIED, "domain_request_id": self.domain_request.id, }, ) self.assertEqual(response.status_code, 200) data = response.json() - self.assertIn("action_needed_email", data) - self.assertIn("ORGANIZATION MAY NOT MEET ELIGIBILITY REQUIREMENTS", data["action_needed_email"]) + self.assertIn("email", data) + self.assertIn("we could not verify the organizational", data["email"]) @less_console_noise_decorator def test_get_action_needed_email_for_user_json_analyst(self): @@ -223,14 +223,14 @@ def test_get_action_needed_email_for_user_json_analyst(self): response = self.client.get( self.api_url, { - "reason": DomainRequest.ActionNeededReasons.QUESTIONABLE_SENIOR_OFFICIAL, + "reason": DomainRequest.RejectionReasons.CONTACTS_NOT_VERIFIED, "domain_request_id": self.domain_request.id, }, ) self.assertEqual(response.status_code, 200) data = response.json() - self.assertIn("action_needed_email", data) - self.assertIn("SENIOR OFFICIAL DOES NOT MEET ELIGIBILITY REQUIREMENTS", data["action_needed_email"]) + self.assertIn("email", data) + self.assertIn("we could not verify the organizational", data["email"]) @less_console_noise_decorator def test_get_action_needed_email_for_user_json_regular(self): @@ -240,7 +240,7 @@ def test_get_action_needed_email_for_user_json_regular(self): response = self.client.get( self.api_url, { - "reason": DomainRequest.ActionNeededReasons.QUESTIONABLE_SENIOR_OFFICIAL, + "reason": DomainRequest.RejectionReasons.CONTACTS_NOT_VERIFIED, "domain_request_id": self.domain_request.id, }, ) From b30e0173c4497527dc4f9cb1bb2348d971de7ee4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 2 Oct 2024 11:58:00 -0600 Subject: [PATCH 22/34] rename --- src/registrar/tests/test_api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_api.py b/src/registrar/tests/test_api.py index 22ad2b376..307a5553f 100644 --- a/src/registrar/tests/test_api.py +++ b/src/registrar/tests/test_api.py @@ -216,7 +216,7 @@ def test_get_rejected_email_for_user_json_superuser(self): self.assertIn("we could not verify the organizational", data["email"]) @less_console_noise_decorator - def test_get_action_needed_email_for_user_json_analyst(self): + def test_get_rejected_email_for_user_json_analyst(self): """Test that an analyst can fetch the action needed email.""" self.client.force_login(self.analyst_user) @@ -233,7 +233,7 @@ def test_get_action_needed_email_for_user_json_analyst(self): self.assertIn("we could not verify the organizational", data["email"]) @less_console_noise_decorator - def test_get_action_needed_email_for_user_json_regular(self): + def test_get_rejected_email_for_user_json_regular(self): """Test that a regular user receives a 403 with an error message.""" p = "password" self.client.login(username="testuser", password=p) From 02e18a372c81e7b82028eb96cb6412cabf3c0c73 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 2 Oct 2024 14:03:53 -0600 Subject: [PATCH 23/34] Update get-gov-admin.js --- src/registrar/assets/js/get-gov-admin.js | 8 ++++---- 1 file 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 c8df161bb..ee61811d6 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -716,11 +716,11 @@ class customRejectedEmail extends CustomizableEmailBase { } loadRejectedEmail() { - this.initializeFormGroups("rejected", "showRejectionReason"); + this.initializeFormGroups(); this.updateUserInterface(); - this.initializeDropdown("Error when attempting to grab rejected email: ") - this.initializeModalConfirm() - this.initializeDirectEditButton() + this.initializeDropdown(); + this.initializeModalConfirm(); + this.initializeDirectEditButton(); } // Overrides the placeholder text when no reason is selected From ade6733b0e99a7448dcbc9943d01567afcc9ade9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 8 Oct 2024 12:35:52 -0600 Subject: [PATCH 24/34] Remove bad import --- src/registrar/admin.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 0f9714113..c282b3495 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -5,7 +5,11 @@ from django.db.models import Value, CharField, Q from django.db.models.functions import Concat, Coalesce from django.http import HttpResponseRedirect -from registrar.utility.admin_helpers import get_action_needed_reason_default_email, get_rejection_reason_default_email +from registrar.utility.admin_helpers import ( + get_action_needed_reason_default_email, + get_rejection_reason_default_email, + get_field_links_as_list, +) from django.conf import settings from django.shortcuts import redirect from django_fsm import get_available_FIELD_transitions, FSMField @@ -21,11 +25,6 @@ from registrar.models.user_domain_role import UserDomainRole from waffle.admin import FlagAdmin from waffle.models import Sample, Switch -from registrar.utility.admin_helpers import ( - get_all_action_needed_reason_emails, - get_action_needed_reason_default_email, - get_field_links_as_list, -) from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website, SeniorOfficial from registrar.utility.constants import BranchChoices from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes From e3d2dc0cb8d24c7fd73011b9c689051e3c9c28e9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 8 Oct 2024 12:50:37 -0600 Subject: [PATCH 25/34] Remove old migrations (merging) --- .../migrations/0131_create_groups_v17.py | 37 ------------------- ...er_domaininformation_portfolio_and_more.py | 36 ------------------ 2 files changed, 73 deletions(-) delete mode 100644 src/registrar/migrations/0131_create_groups_v17.py delete mode 100644 src/registrar/migrations/0132_alter_domaininformation_portfolio_and_more.py diff --git a/src/registrar/migrations/0131_create_groups_v17.py b/src/registrar/migrations/0131_create_groups_v17.py deleted file mode 100644 index 04cd5163c..000000000 --- a/src/registrar/migrations/0131_create_groups_v17.py +++ /dev/null @@ -1,37 +0,0 @@ -# This migration creates the create_full_access_group and create_cisa_analyst_group groups -# It is dependent on 0079 (which populates federal agencies) -# If permissions on the groups need changing, edit CISA_ANALYST_GROUP_PERMISSIONS -# in the user_group model then: -# [NOT RECOMMENDED] -# step 1: docker-compose exec app ./manage.py migrate --fake registrar 0035_contenttypes_permissions -# step 2: docker-compose exec app ./manage.py migrate registrar 0036_create_groups -# step 3: fake run the latest migration in the migrations list -# [RECOMMENDED] -# Alternatively: -# step 1: duplicate the migration that loads data -# step 2: docker-compose exec app ./manage.py migrate - -from django.db import migrations -from registrar.models import UserGroup -from typing import Any - - -# For linting: RunPython expects a function reference, -# so let's give it one -def create_groups(apps, schema_editor) -> Any: - UserGroup.create_cisa_analyst_group(apps, schema_editor) - UserGroup.create_full_access_group(apps, schema_editor) - - -class Migration(migrations.Migration): - dependencies = [ - ("registrar", "0130_remove_federalagency_initials_federalagency_acronym_and_more"), - ] - - operations = [ - migrations.RunPython( - create_groups, - reverse_code=migrations.RunPython.noop, - atomic=True, - ), - ] diff --git a/src/registrar/migrations/0132_alter_domaininformation_portfolio_and_more.py b/src/registrar/migrations/0132_alter_domaininformation_portfolio_and_more.py deleted file mode 100644 index e0d2b35f9..000000000 --- a/src/registrar/migrations/0132_alter_domaininformation_portfolio_and_more.py +++ /dev/null @@ -1,36 +0,0 @@ -# Generated by Django 4.2.10 on 2024-10-02 14:17 - -from django.db import migrations, models -import django.db.models.deletion - - -class Migration(migrations.Migration): - - dependencies = [ - ("registrar", "0131_create_groups_v17"), - ] - - operations = [ - migrations.AlterField( - model_name="domaininformation", - name="portfolio", - field=models.ForeignKey( - blank=True, - null=True, - on_delete=django.db.models.deletion.PROTECT, - related_name="information_portfolio", - to="registrar.portfolio", - ), - ), - migrations.AlterField( - model_name="domainrequest", - name="portfolio", - field=models.ForeignKey( - blank=True, - null=True, - on_delete=django.db.models.deletion.PROTECT, - related_name="DomainRequest_portfolio", - to="registrar.portfolio", - ), - ), - ] From 081c7622463c9a0d79ae506ac7c67a948c89107e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 8 Oct 2024 12:52:17 -0600 Subject: [PATCH 26/34] Revert "Remove old migrations (merging)" This reverts commit e3d2dc0cb8d24c7fd73011b9c689051e3c9c28e9. --- .../migrations/0131_create_groups_v17.py | 37 +++++++++++++++++++ ...er_domaininformation_portfolio_and_more.py | 36 ++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 src/registrar/migrations/0131_create_groups_v17.py create mode 100644 src/registrar/migrations/0132_alter_domaininformation_portfolio_and_more.py diff --git a/src/registrar/migrations/0131_create_groups_v17.py b/src/registrar/migrations/0131_create_groups_v17.py new file mode 100644 index 000000000..04cd5163c --- /dev/null +++ b/src/registrar/migrations/0131_create_groups_v17.py @@ -0,0 +1,37 @@ +# This migration creates the create_full_access_group and create_cisa_analyst_group groups +# It is dependent on 0079 (which populates federal agencies) +# If permissions on the groups need changing, edit CISA_ANALYST_GROUP_PERMISSIONS +# in the user_group model then: +# [NOT RECOMMENDED] +# step 1: docker-compose exec app ./manage.py migrate --fake registrar 0035_contenttypes_permissions +# step 2: docker-compose exec app ./manage.py migrate registrar 0036_create_groups +# step 3: fake run the latest migration in the migrations list +# [RECOMMENDED] +# Alternatively: +# step 1: duplicate the migration that loads data +# step 2: docker-compose exec app ./manage.py migrate + +from django.db import migrations +from registrar.models import UserGroup +from typing import Any + + +# For linting: RunPython expects a function reference, +# so let's give it one +def create_groups(apps, schema_editor) -> Any: + UserGroup.create_cisa_analyst_group(apps, schema_editor) + UserGroup.create_full_access_group(apps, schema_editor) + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0130_remove_federalagency_initials_federalagency_acronym_and_more"), + ] + + operations = [ + migrations.RunPython( + create_groups, + reverse_code=migrations.RunPython.noop, + atomic=True, + ), + ] diff --git a/src/registrar/migrations/0132_alter_domaininformation_portfolio_and_more.py b/src/registrar/migrations/0132_alter_domaininformation_portfolio_and_more.py new file mode 100644 index 000000000..e0d2b35f9 --- /dev/null +++ b/src/registrar/migrations/0132_alter_domaininformation_portfolio_and_more.py @@ -0,0 +1,36 @@ +# Generated by Django 4.2.10 on 2024-10-02 14:17 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0131_create_groups_v17"), + ] + + operations = [ + migrations.AlterField( + model_name="domaininformation", + name="portfolio", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="information_portfolio", + to="registrar.portfolio", + ), + ), + migrations.AlterField( + model_name="domainrequest", + name="portfolio", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="DomainRequest_portfolio", + to="registrar.portfolio", + ), + ), + ] From a4ffc75efacfa1c7de83aa2eeafb91498eccbdc4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 8 Oct 2024 12:53:02 -0600 Subject: [PATCH 27/34] Fix migrations --- ...129_domainrequest_rejection_reason_email.py | 18 ------------------ ...request_rejection_reason_email_and_more.py} | 9 +++++++-- 2 files changed, 7 insertions(+), 20 deletions(-) delete mode 100644 src/registrar/migrations/0129_domainrequest_rejection_reason_email.py rename src/registrar/migrations/{0130_alter_domainrequest_rejection_reason.py => 0133_domainrequest_rejection_reason_email_and_more.py} (75%) diff --git a/src/registrar/migrations/0129_domainrequest_rejection_reason_email.py b/src/registrar/migrations/0129_domainrequest_rejection_reason_email.py deleted file mode 100644 index 6aaef7f87..000000000 --- a/src/registrar/migrations/0129_domainrequest_rejection_reason_email.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 4.2.10 on 2024-09-26 21:18 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ("registrar", "0128_alter_domaininformation_state_territory_and_more"), - ] - - operations = [ - migrations.AddField( - model_name="domainrequest", - name="rejection_reason_email", - field=models.TextField(blank=True, null=True), - ), - ] diff --git a/src/registrar/migrations/0130_alter_domainrequest_rejection_reason.py b/src/registrar/migrations/0133_domainrequest_rejection_reason_email_and_more.py similarity index 75% rename from src/registrar/migrations/0130_alter_domainrequest_rejection_reason.py rename to src/registrar/migrations/0133_domainrequest_rejection_reason_email_and_more.py index 58a09772b..383c3ebfa 100644 --- a/src/registrar/migrations/0130_alter_domainrequest_rejection_reason.py +++ b/src/registrar/migrations/0133_domainrequest_rejection_reason_email_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-10-01 15:31 +# Generated by Django 4.2.10 on 2024-10-08 18:52 from django.db import migrations, models @@ -6,10 +6,15 @@ class Migration(migrations.Migration): dependencies = [ - ("registrar", "0129_domainrequest_rejection_reason_email"), + ("registrar", "0132_alter_domaininformation_portfolio_and_more"), ] operations = [ + migrations.AddField( + model_name="domainrequest", + name="rejection_reason_email", + field=models.TextField(blank=True, null=True), + ), migrations.AlterField( model_name="domainrequest", name="rejection_reason", From 7c69205deb586b27a141aa94875c0b92700875f9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 9 Oct 2024 08:34:40 -0600 Subject: [PATCH 28/34] Update test_models.py --- src/registrar/tests/test_models.py | 981 ----------------------------- 1 file changed, 981 deletions(-) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 96ca3871f..7664264c3 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -40,987 +40,6 @@ logger = logging.getLogger(__name__) -@boto3_mocking.patching -class TestDomainRequest(TestCase): - @less_console_noise_decorator - def setUp(self): - - self.dummy_user, _ = Contact.objects.get_or_create( - email="mayor@igorville.com", first_name="Hello", last_name="World" - ) - self.dummy_user_2, _ = User.objects.get_or_create( - username="intern@igorville.com", email="intern@igorville.com", first_name="Lava", last_name="World" - ) - self.started_domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.STARTED, - name="started.gov", - ) - self.submitted_domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.SUBMITTED, - name="submitted.gov", - ) - self.in_review_domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.IN_REVIEW, - name="in-review.gov", - ) - self.action_needed_domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.ACTION_NEEDED, - name="action-needed.gov", - ) - self.approved_domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.APPROVED, - name="approved.gov", - ) - self.withdrawn_domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.WITHDRAWN, - name="withdrawn.gov", - ) - self.rejected_domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.REJECTED, - name="rejected.gov", - ) - self.ineligible_domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.INELIGIBLE, - name="ineligible.gov", - ) - - # Store all domain request statuses in a variable for ease of use - self.all_domain_requests = [ - self.started_domain_request, - self.submitted_domain_request, - self.in_review_domain_request, - self.action_needed_domain_request, - self.approved_domain_request, - self.withdrawn_domain_request, - self.rejected_domain_request, - self.ineligible_domain_request, - ] - - self.mock_client = MockSESClient() - - def tearDown(self): - super().tearDown() - DomainInformation.objects.all().delete() - DomainRequest.objects.all().delete() - DraftDomain.objects.all().delete() - Domain.objects.all().delete() - User.objects.all().delete() - self.mock_client.EMAILS_SENT.clear() - - def assertNotRaises(self, exception_type): - """Helper method for testing allowed transitions.""" - with less_console_noise(): - return self.assertRaises(Exception, None, exception_type) - - @less_console_noise_decorator - def test_request_is_withdrawable(self): - """Tests the is_withdrawable function""" - domain_request_1 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.SUBMITTED, - name="city2.gov", - ) - domain_request_2 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.IN_REVIEW, - name="city3.gov", - ) - domain_request_3 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.ACTION_NEEDED, - name="city4.gov", - ) - domain_request_4 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.REJECTED, - name="city5.gov", - ) - self.assertTrue(domain_request_1.is_withdrawable()) - self.assertTrue(domain_request_2.is_withdrawable()) - self.assertTrue(domain_request_3.is_withdrawable()) - self.assertFalse(domain_request_4.is_withdrawable()) - - @less_console_noise_decorator - def test_request_is_awaiting_review(self): - """Tests the is_awaiting_review function""" - domain_request_1 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.SUBMITTED, - name="city2.gov", - ) - domain_request_2 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.IN_REVIEW, - name="city3.gov", - ) - domain_request_3 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.ACTION_NEEDED, - name="city4.gov", - ) - domain_request_4 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.REJECTED, - name="city5.gov", - ) - self.assertTrue(domain_request_1.is_awaiting_review()) - self.assertTrue(domain_request_2.is_awaiting_review()) - self.assertFalse(domain_request_3.is_awaiting_review()) - self.assertFalse(domain_request_4.is_awaiting_review()) - - @less_console_noise_decorator - def test_federal_agency_set_to_non_federal_on_approve(self): - """Ensures that when the federal_agency field is 'none' when .approve() is called, - the field is set to the 'Non-Federal Agency' record""" - domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.IN_REVIEW, - name="city2.gov", - federal_agency=None, - ) - - # Ensure that the federal agency is None - self.assertEqual(domain_request.federal_agency, None) - - # Approve the request - domain_request.approve() - self.assertEqual(domain_request.status, DomainRequest.DomainRequestStatus.APPROVED) - - # After approval, it should be "Non-Federal agency" - expected_federal_agency = FederalAgency.objects.filter(agency="Non-Federal Agency").first() - self.assertEqual(domain_request.federal_agency, expected_federal_agency) - - def test_empty_create_fails(self): - """Can't create a completely empty domain request.""" - with less_console_noise(): - with transaction.atomic(): - with self.assertRaisesRegex(IntegrityError, "creator"): - DomainRequest.objects.create() - - @less_console_noise_decorator - def test_minimal_create(self): - """Can create with just a creator.""" - user, _ = User.objects.get_or_create(username="testy") - domain_request = DomainRequest.objects.create(creator=user) - self.assertEqual(domain_request.status, DomainRequest.DomainRequestStatus.STARTED) - - @less_console_noise_decorator - def test_full_create(self): - """Can create with all fields.""" - user, _ = User.objects.get_or_create(username="testy") - contact = Contact.objects.create() - com_website, _ = Website.objects.get_or_create(website="igorville.com") - gov_website, _ = Website.objects.get_or_create(website="igorville.gov") - domain, _ = DraftDomain.objects.get_or_create(name="igorville.gov") - domain_request = DomainRequest.objects.create( - creator=user, - investigator=user, - generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, - federal_type=BranchChoices.EXECUTIVE, - is_election_board=False, - organization_name="Test", - address_line1="100 Main St.", - address_line2="APT 1A", - state_territory="CA", - zipcode="12345-6789", - senior_official=contact, - requested_domain=domain, - purpose="Igorville rules!", - anything_else="All of Igorville loves the dotgov program.", - is_policy_acknowledged=True, - ) - domain_request.current_websites.add(com_website) - domain_request.alternative_domains.add(gov_website) - domain_request.other_contacts.add(contact) - domain_request.save() - - @less_console_noise_decorator - def test_domain_info(self): - """Can create domain info with all fields.""" - user, _ = User.objects.get_or_create(username="testy") - contact = Contact.objects.create() - domain, _ = Domain.objects.get_or_create(name="igorville.gov") - information = DomainInformation.objects.create( - creator=user, - generic_org_type=DomainInformation.OrganizationChoices.FEDERAL, - federal_type=BranchChoices.EXECUTIVE, - is_election_board=False, - organization_name="Test", - address_line1="100 Main St.", - address_line2="APT 1A", - state_territory="CA", - zipcode="12345-6789", - senior_official=contact, - purpose="Igorville rules!", - anything_else="All of Igorville loves the dotgov program.", - is_policy_acknowledged=True, - domain=domain, - ) - information.other_contacts.add(contact) - information.save() - self.assertEqual(information.domain.id, domain.id) - self.assertEqual(information.id, domain.domain_info.id) - - @less_console_noise_decorator - def test_status_fsm_submit_fail(self): - user, _ = User.objects.get_or_create(username="testy") - domain_request = DomainRequest.objects.create(creator=user) - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - with less_console_noise(): - with self.assertRaises(ValueError): - # can't submit a domain request with a null domain name - domain_request.submit() - - @less_console_noise_decorator - def test_status_fsm_submit_succeed(self): - user, _ = User.objects.get_or_create(username="testy") - site = DraftDomain.objects.create(name="igorville.gov") - domain_request = DomainRequest.objects.create(creator=user, requested_domain=site) - - # no email sent to creator so this emits a log warning - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - with less_console_noise(): - domain_request.submit() - self.assertEqual(domain_request.status, domain_request.DomainRequestStatus.SUBMITTED) - - @less_console_noise_decorator - def check_email_sent( - self, domain_request, msg, action, expected_count, expected_content=None, expected_email="mayor@igorville.com" - ): - """Check if an email was sent after performing an action.""" - email_allowed, _ = AllowedEmail.objects.get_or_create(email=expected_email) - with self.subTest(msg=msg, action=action): - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - # Perform the specified action - action_method = getattr(domain_request, action) - action_method() - domain_request.save() - domain_request.refresh_from_db() - - # Check if an email was sent - sent_emails = [ - email - for email in MockSESClient.EMAILS_SENT - if expected_email in email["kwargs"]["Destination"]["ToAddresses"] - ] - self.assertEqual(len(sent_emails), expected_count) - - if expected_content: - email_content = sent_emails[0]["kwargs"]["Content"]["Simple"]["Body"]["Text"]["Data"] - self.assertIn(expected_content, email_content) - - email_allowed.delete() - - @less_console_noise_decorator - def test_submit_from_started_sends_email_to_creator(self): - """tests that we send an email to the creator""" - msg = "Create a domain request and submit it and see if email was sent when the feature flag is on." - domain_request = completed_domain_request(user=self.dummy_user_2) - self.check_email_sent( - domain_request, msg, "submit", 1, expected_content="Lava", expected_email="intern@igorville.com" - ) - - @less_console_noise_decorator - def test_submit_from_withdrawn_sends_email(self): - msg = "Create a withdrawn domain request and submit it and see if email was sent." - user, _ = User.objects.get_or_create(username="testy") - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.WITHDRAWN, user=user) - self.check_email_sent(domain_request, msg, "submit", 1, expected_content="Hi", expected_email=user.email) - - @less_console_noise_decorator - def test_submit_from_action_needed_does_not_send_email(self): - msg = "Create a domain request with ACTION_NEEDED status and submit it, check if email was not sent." - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.ACTION_NEEDED) - self.check_email_sent(domain_request, msg, "submit", 0) - - @less_console_noise_decorator - def test_submit_from_in_review_does_not_send_email(self): - msg = "Create a withdrawn domain request and submit it and see if email was sent." - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW) - self.check_email_sent(domain_request, msg, "submit", 0) - - @less_console_noise_decorator - def test_approve_sends_email(self): - msg = "Create a domain request and approve it and see if email was sent." - user, _ = User.objects.get_or_create(username="testy") - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=user) - self.check_email_sent(domain_request, msg, "approve", 1, expected_content="approved", expected_email=user.email) - - @less_console_noise_decorator - def test_withdraw_sends_email(self): - msg = "Create a domain request and withdraw it and see if email was sent." - user, _ = User.objects.get_or_create(username="testy") - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=user) - self.check_email_sent( - domain_request, msg, "withdraw", 1, expected_content="withdrawn", expected_email=user.email - ) - - @less_console_noise_decorator - def test_reject_with_prejudice_does_not_send_email(self): - msg = "Create a domain request and reject it with prejudice and see if email was sent." - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.APPROVED) - self.check_email_sent(domain_request, msg, "reject_with_prejudice", 0) - - @less_console_noise_decorator - def assert_fsm_transition_raises_error(self, test_cases, method_to_run): - """Given a list of test cases, check if each transition throws the intended error""" - with boto3_mocking.clients.handler_for("sesv2", self.mock_client), less_console_noise(): - for domain_request, exception_type in test_cases: - with self.subTest(domain_request=domain_request, exception_type=exception_type): - with self.assertRaises(exception_type): - # Retrieve the method by name from the domain_request object and call it - method = getattr(domain_request, method_to_run) - # Call the method - method() - - @less_console_noise_decorator - def assert_fsm_transition_does_not_raise_error(self, test_cases, method_to_run): - """Given a list of test cases, ensure that none of them throw transition errors""" - with boto3_mocking.clients.handler_for("sesv2", self.mock_client), less_console_noise(): - for domain_request, exception_type in test_cases: - with self.subTest(domain_request=domain_request, exception_type=exception_type): - try: - # Retrieve the method by name from the DomainRequest object and call it - method = getattr(domain_request, method_to_run) - # Call the method - method() - except exception_type: - self.fail(f"{exception_type} was raised, but it was not expected.") - - @less_console_noise_decorator - def test_submit_transition_allowed_with_no_investigator(self): - """ - Tests for attempting to transition without an investigator. - For submit, this should be valid in all cases. - """ - - test_cases = [ - (self.started_domain_request, TransitionNotAllowed), - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.withdrawn_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to none - set_domain_request_investigators(self.all_domain_requests, None) - - self.assert_fsm_transition_does_not_raise_error(test_cases, "submit") - - @less_console_noise_decorator - def test_submit_transition_allowed_with_investigator_not_staff(self): - """ - Tests for attempting to transition with an investigator user that is not staff. - For submit, this should be valid in all cases. - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to a user with no staff privs - user, _ = User.objects.get_or_create(username="pancakesyrup", is_staff=False) - set_domain_request_investigators(self.all_domain_requests, user) - - self.assert_fsm_transition_does_not_raise_error(test_cases, "submit") - - @less_console_noise_decorator - def test_submit_transition_allowed(self): - """ - Test that calling submit from allowable statuses does raises TransitionNotAllowed. - """ - test_cases = [ - (self.started_domain_request, TransitionNotAllowed), - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.withdrawn_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_does_not_raise_error(test_cases, "submit") - - @less_console_noise_decorator - def test_submit_transition_allowed_twice(self): - """ - Test that rotating between submit and in_review doesn't throw an error - """ - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - try: - # Make a submission - self.in_review_domain_request.submit() - - # Rerun the old method to get back to the original state - self.in_review_domain_request.in_review() - - # Make another submission - self.in_review_domain_request.submit() - except TransitionNotAllowed: - self.fail("TransitionNotAllowed was raised, but it was not expected.") - - self.assertEqual(self.in_review_domain_request.status, DomainRequest.DomainRequestStatus.SUBMITTED) - - @less_console_noise_decorator - def test_submit_transition_not_allowed(self): - """ - Test that calling submit against transition rules raises TransitionNotAllowed. - """ - test_cases = [ - (self.submitted_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_raises_error(test_cases, "submit") - - @less_console_noise_decorator - def test_in_review_transition_allowed(self): - """ - Test that calling in_review from allowable statuses does raises TransitionNotAllowed. - """ - test_cases = [ - (self.submitted_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_does_not_raise_error(test_cases, "in_review") - - @less_console_noise_decorator - def test_in_review_transition_not_allowed_with_no_investigator(self): - """ - Tests for attempting to transition without an investigator - """ - - test_cases = [ - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to none - set_domain_request_investigators(self.all_domain_requests, None) - - self.assert_fsm_transition_raises_error(test_cases, "in_review") - - @less_console_noise_decorator - def test_in_review_transition_not_allowed_with_investigator_not_staff(self): - """ - Tests for attempting to transition with an investigator that is not staff. - This should throw an exception. - """ - - test_cases = [ - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to a user with no staff privs - user, _ = User.objects.get_or_create(username="pancakesyrup", is_staff=False) - set_domain_request_investigators(self.all_domain_requests, user) - - self.assert_fsm_transition_raises_error(test_cases, "in_review") - - @less_console_noise_decorator - def test_in_review_transition_not_allowed(self): - """ - Test that calling in_review against transition rules raises TransitionNotAllowed. - """ - test_cases = [ - (self.started_domain_request, TransitionNotAllowed), - (self.in_review_domain_request, TransitionNotAllowed), - (self.withdrawn_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_raises_error(test_cases, "in_review") - - @less_console_noise_decorator - def test_action_needed_transition_allowed(self): - """ - Test that calling action_needed from allowable statuses does raises TransitionNotAllowed. - """ - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_does_not_raise_error(test_cases, "action_needed") - - @less_console_noise_decorator - def test_action_needed_transition_not_allowed_with_no_investigator(self): - """ - Tests for attempting to transition without an investigator - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to none - set_domain_request_investigators(self.all_domain_requests, None) - - self.assert_fsm_transition_raises_error(test_cases, "action_needed") - - @less_console_noise_decorator - def test_action_needed_transition_not_allowed_with_investigator_not_staff(self): - """ - Tests for attempting to transition with an investigator that is not staff - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to a user with no staff privs - user, _ = User.objects.get_or_create(username="pancakesyrup", is_staff=False) - set_domain_request_investigators(self.all_domain_requests, user) - - self.assert_fsm_transition_raises_error(test_cases, "action_needed") - - @less_console_noise_decorator - def test_action_needed_transition_not_allowed(self): - """ - Test that calling action_needed against transition rules raises TransitionNotAllowed. - """ - test_cases = [ - (self.started_domain_request, TransitionNotAllowed), - (self.submitted_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.withdrawn_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_raises_error(test_cases, "action_needed") - - @less_console_noise_decorator - def test_approved_transition_allowed(self): - """ - Test that calling action_needed from allowable statuses does raises TransitionNotAllowed. - """ - test_cases = [ - (self.submitted_domain_request, TransitionNotAllowed), - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_does_not_raise_error(test_cases, "approve") - - @less_console_noise_decorator - def test_approved_transition_not_allowed_with_no_investigator(self): - """ - Tests for attempting to transition without an investigator - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to none - set_domain_request_investigators(self.all_domain_requests, None) - - self.assert_fsm_transition_raises_error(test_cases, "approve") - - @less_console_noise_decorator - def test_approved_transition_not_allowed_with_investigator_not_staff(self): - """ - Tests for attempting to transition with an investigator that is not staff - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to a user with no staff privs - user, _ = User.objects.get_or_create(username="pancakesyrup", is_staff=False) - set_domain_request_investigators(self.all_domain_requests, user) - - self.assert_fsm_transition_raises_error(test_cases, "approve") - - @less_console_noise_decorator - def test_approved_skips_sending_email(self): - """ - Test that calling .approve with send_email=False doesn't actually send - an email - """ - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - self.submitted_domain_request.approve(send_email=False) - - # Assert that no emails were sent - self.assertEqual(len(self.mock_client.EMAILS_SENT), 0) - - @less_console_noise_decorator - def test_approved_transition_not_allowed(self): - """ - Test that calling action_needed against transition rules raises TransitionNotAllowed. - """ - test_cases = [ - (self.started_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.withdrawn_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - self.assert_fsm_transition_raises_error(test_cases, "approve") - - @less_console_noise_decorator - def test_withdraw_transition_allowed(self): - """ - Test that calling action_needed from allowable statuses does raises TransitionNotAllowed. - """ - test_cases = [ - (self.submitted_domain_request, TransitionNotAllowed), - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_does_not_raise_error(test_cases, "withdraw") - - @less_console_noise_decorator - def test_withdraw_transition_allowed_with_no_investigator(self): - """ - Tests for attempting to transition without an investigator. - For withdraw, this should be valid in all cases. - """ - - test_cases = [ - (self.submitted_domain_request, TransitionNotAllowed), - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to none - set_domain_request_investigators(self.all_domain_requests, None) - - self.assert_fsm_transition_does_not_raise_error(test_cases, "withdraw") - - @less_console_noise_decorator - def test_withdraw_transition_allowed_with_investigator_not_staff(self): - """ - Tests for attempting to transition when investigator is not staff. - For withdraw, this should be valid in all cases. - """ - - test_cases = [ - (self.submitted_domain_request, TransitionNotAllowed), - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to a user with no staff privs - user, _ = User.objects.get_or_create(username="pancakesyrup", is_staff=False) - set_domain_request_investigators(self.all_domain_requests, user) - - self.assert_fsm_transition_does_not_raise_error(test_cases, "withdraw") - - @less_console_noise_decorator - def test_withdraw_transition_not_allowed(self): - """ - Test that calling action_needed against transition rules raises TransitionNotAllowed. - """ - test_cases = [ - (self.started_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.withdrawn_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_raises_error(test_cases, "withdraw") - - @less_console_noise_decorator - def test_reject_transition_allowed(self): - """ - Test that calling action_needed from allowable statuses does raises TransitionNotAllowed. - """ - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_does_not_raise_error(test_cases, "reject") - - @less_console_noise_decorator - def test_reject_transition_not_allowed_with_no_investigator(self): - """ - Tests for attempting to transition without an investigator - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to none - set_domain_request_investigators(self.all_domain_requests, None) - - self.assert_fsm_transition_raises_error(test_cases, "reject") - - @less_console_noise_decorator - def test_reject_transition_not_allowed_with_investigator_not_staff(self): - """ - Tests for attempting to transition when investigator is not staff - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to a user with no staff privs - user, _ = User.objects.get_or_create(username="pancakesyrup", is_staff=False) - set_domain_request_investigators(self.all_domain_requests, user) - - self.assert_fsm_transition_raises_error(test_cases, "reject") - - @less_console_noise_decorator - def test_reject_transition_not_allowed(self): - """ - Test that calling action_needed against transition rules raises TransitionNotAllowed. - """ - test_cases = [ - (self.started_domain_request, TransitionNotAllowed), - (self.submitted_domain_request, TransitionNotAllowed), - (self.withdrawn_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_raises_error(test_cases, "reject") - - @less_console_noise_decorator - def test_reject_with_prejudice_transition_allowed(self): - """ - Test that calling action_needed from allowable statuses does raises TransitionNotAllowed. - """ - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_does_not_raise_error(test_cases, "reject_with_prejudice") - - @less_console_noise_decorator - def test_reject_with_prejudice_transition_not_allowed_with_no_investigator(self): - """ - Tests for attempting to transition without an investigator - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to none - set_domain_request_investigators(self.all_domain_requests, None) - - self.assert_fsm_transition_raises_error(test_cases, "reject_with_prejudice") - - @less_console_noise_decorator - def test_reject_with_prejudice_not_allowed_with_investigator_not_staff(self): - """ - Tests for attempting to transition when investigator is not staff - """ - - test_cases = [ - (self.in_review_domain_request, TransitionNotAllowed), - (self.action_needed_domain_request, TransitionNotAllowed), - (self.approved_domain_request, TransitionNotAllowed), - (self.rejected_domain_request, TransitionNotAllowed), - ] - - # Set all investigators to a user with no staff privs - user, _ = User.objects.get_or_create(username="pancakesyrup", is_staff=False) - set_domain_request_investigators(self.all_domain_requests, user) - - self.assert_fsm_transition_raises_error(test_cases, "reject_with_prejudice") - - @less_console_noise_decorator - def test_reject_with_prejudice_transition_not_allowed(self): - """ - Test that calling action_needed against transition rules raises TransitionNotAllowed. - """ - test_cases = [ - (self.started_domain_request, TransitionNotAllowed), - (self.submitted_domain_request, TransitionNotAllowed), - (self.withdrawn_domain_request, TransitionNotAllowed), - (self.ineligible_domain_request, TransitionNotAllowed), - ] - - self.assert_fsm_transition_raises_error(test_cases, "reject_with_prejudice") - - @less_console_noise_decorator - def test_transition_not_allowed_approved_in_review_when_domain_is_active(self): - """Create a domain request with status approved, create a matching domain that - is active, and call in_review against transition rules""" - - domain = Domain.objects.create(name=self.approved_domain_request.requested_domain.name) - self.approved_domain_request.approved_domain = domain - self.approved_domain_request.save() - - # Define a custom implementation for is_active - def custom_is_active(self): - return True # Override to return True - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - # Use patch to temporarily replace is_active with the custom implementation - with patch.object(Domain, "is_active", custom_is_active): - # Now, when you call is_active on Domain, it will return True - with self.assertRaises(TransitionNotAllowed): - self.approved_domain_request.in_review() - - @less_console_noise_decorator - def test_transition_not_allowed_approved_action_needed_when_domain_is_active(self): - """Create a domain request with status approved, create a matching domain that - is active, and call action_needed against transition rules""" - - domain = Domain.objects.create(name=self.approved_domain_request.requested_domain.name) - self.approved_domain_request.approved_domain = domain - self.approved_domain_request.save() - - # Define a custom implementation for is_active - def custom_is_active(self): - return True # Override to return True - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - # Use patch to temporarily replace is_active with the custom implementation - with patch.object(Domain, "is_active", custom_is_active): - # Now, when you call is_active on Domain, it will return True - with self.assertRaises(TransitionNotAllowed): - self.approved_domain_request.action_needed() - - @less_console_noise_decorator - def test_transition_not_allowed_approved_rejected_when_domain_is_active(self): - """Create a domain request with status approved, create a matching domain that - is active, and call reject against transition rules""" - - domain = Domain.objects.create(name=self.approved_domain_request.requested_domain.name) - self.approved_domain_request.approved_domain = domain - self.approved_domain_request.save() - - # Define a custom implementation for is_active - def custom_is_active(self): - return True # Override to return True - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - # Use patch to temporarily replace is_active with the custom implementation - with patch.object(Domain, "is_active", custom_is_active): - # Now, when you call is_active on Domain, it will return True - with self.assertRaises(TransitionNotAllowed): - self.approved_domain_request.reject() - - @less_console_noise_decorator - def test_transition_not_allowed_approved_ineligible_when_domain_is_active(self): - """Create a domain request with status approved, create a matching domain that - is active, and call reject_with_prejudice against transition rules""" - - domain = Domain.objects.create(name=self.approved_domain_request.requested_domain.name) - self.approved_domain_request.approved_domain = domain - self.approved_domain_request.save() - - # Define a custom implementation for is_active - def custom_is_active(self): - return True # Override to return True - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - # Use patch to temporarily replace is_active with the custom implementation - with patch.object(Domain, "is_active", custom_is_active): - # Now, when you call is_active on Domain, it will return True - with self.assertRaises(TransitionNotAllowed): - self.approved_domain_request.reject_with_prejudice() - - @less_console_noise_decorator - def test_approve_from_rejected_clears_rejection_reason(self): - """When transitioning from rejected to approved on a domain request, - the rejection_reason is cleared.""" - - # Create a sample domain request - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.REJECTED) - domain_request.rejection_reason = DomainRequest.RejectionReasons.DOMAIN_PURPOSE - - # Approve - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - domain_request.approve() - - self.assertEqual(domain_request.status, DomainRequest.DomainRequestStatus.APPROVED) - self.assertEqual(domain_request.rejection_reason, None) - - @less_console_noise_decorator - def test_in_review_from_rejected_clears_rejection_reason(self): - """When transitioning from rejected to in_review on a domain request, - the rejection_reason is cleared.""" - - # Create a sample domain request - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.REJECTED) - domain_request.domain_is_not_active = True - domain_request.rejection_reason = DomainRequest.RejectionReasons.DOMAIN_PURPOSE - - # Approve - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - domain_request.in_review() - - self.assertEqual(domain_request.status, DomainRequest.DomainRequestStatus.IN_REVIEW) - self.assertEqual(domain_request.rejection_reason, None) - - @less_console_noise_decorator - def test_action_needed_from_rejected_clears_rejection_reason(self): - """When transitioning from rejected to action_needed on a domain request, - the rejection_reason is cleared.""" - - # Create a sample domain request - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.REJECTED) - domain_request.domain_is_not_active = True - domain_request.rejection_reason = DomainRequest.RejectionReasons.DOMAIN_PURPOSE - - # Approve - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - domain_request.action_needed() - - self.assertEqual(domain_request.status, DomainRequest.DomainRequestStatus.ACTION_NEEDED) - self.assertEqual(domain_request.rejection_reason, None) - - @less_console_noise_decorator - def test_has_rationale_returns_true(self): - """has_rationale() returns true when a domain request has no_other_contacts_rationale""" - self.started_domain_request.no_other_contacts_rationale = "You talkin' to me?" - self.started_domain_request.save() - self.assertEquals(self.started_domain_request.has_rationale(), True) - - @less_console_noise_decorator - def test_has_rationale_returns_false(self): - """has_rationale() returns false when a domain request has no no_other_contacts_rationale""" - self.assertEquals(self.started_domain_request.has_rationale(), False) - - @less_console_noise_decorator - def test_has_other_contacts_returns_true(self): - """has_other_contacts() returns true when a domain request has other_contacts""" - # completed_domain_request has other contacts by default - self.assertEquals(self.started_domain_request.has_other_contacts(), True) - - @less_console_noise_decorator - def test_has_other_contacts_returns_false(self): - """has_other_contacts() returns false when a domain request has no other_contacts""" - domain_request = completed_domain_request( - status=DomainRequest.DomainRequestStatus.STARTED, name="no-others.gov", has_other_contacts=False - ) - self.assertEquals(domain_request.has_other_contacts(), False) - - class TestPermissions(TestCase): """Test the User-Domain-Role connection.""" From 3ba429333415ad55740b93a8ec5a241e09e6491a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 9 Oct 2024 08:37:01 -0600 Subject: [PATCH 29/34] Update test_models.py --- src/registrar/tests/test_models.py | 37 ------------------------------ 1 file changed, 37 deletions(-) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 7664264c3..5bfdb2937 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -31,46 +31,9 @@ create_test_user, ) from waffle.testutils import override_flag - -import logging - from api.tests.common import less_console_noise_decorator -logger = logging.getLogger(__name__) - - -class TestPermissions(TestCase): - """Test the User-Domain-Role connection.""" - - def setUp(self): - super().setUp() - self.mock_client = MockSESClient() - - def tearDown(self): - super().tearDown() - self.mock_client.EMAILS_SENT.clear() - - @boto3_mocking.patching - @less_console_noise_decorator - def test_approval_creates_role(self): - draft_domain, _ = DraftDomain.objects.get_or_create(name="igorville.gov") - user, _ = User.objects.get_or_create() - investigator, _ = User.objects.get_or_create(username="frenchtoast", is_staff=True) - domain_request = DomainRequest.objects.create( - creator=user, requested_domain=draft_domain, investigator=investigator - ) - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - # skip using the submit method - domain_request.status = DomainRequest.DomainRequestStatus.SUBMITTED - domain_request.approve() - - # should be a role for this user - domain = Domain.objects.get(name="igorville.gov") - self.assertTrue(UserDomainRole.objects.get(user=user, domain=domain)) - - class TestDomainInformation(TestCase): """Test the DomainInformation model, when approved or otherwise""" From bd603f69fb8a854acdb63dd988601b352374078f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 9 Oct 2024 08:41:08 -0600 Subject: [PATCH 30/34] Remove space --- src/registrar/tests/test_models.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 5bfdb2937..33ae90da9 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -31,6 +31,7 @@ create_test_user, ) from waffle.testutils import override_flag + from api.tests.common import less_console_noise_decorator From ec1df251b892920466b3f545c3dbe9a8b16197d5 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 9 Oct 2024 09:04:37 -0600 Subject: [PATCH 31/34] remove bad test + lint --- src/registrar/admin.py | 4 ++-- src/registrar/tests/test_models_requests.py | 7 ------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index fda1db672..ac4a5c07f 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -6,8 +6,8 @@ from django.db.models.functions import Concat, Coalesce from django.http import HttpResponseRedirect from registrar.utility.admin_helpers import ( - get_action_needed_reason_default_email, - get_rejection_reason_default_email, + get_action_needed_reason_default_email, + get_rejection_reason_default_email, get_field_links_as_list, ) from django.conf import settings diff --git a/src/registrar/tests/test_models_requests.py b/src/registrar/tests/test_models_requests.py index 9e86f5f9c..3be2225ee 100644 --- a/src/registrar/tests/test_models_requests.py +++ b/src/registrar/tests/test_models_requests.py @@ -337,13 +337,6 @@ def test_withdraw_sends_email(self): domain_request, msg, "withdraw", 1, expected_content="withdrawn", expected_email=user.email ) - @less_console_noise_decorator - def test_reject_sends_email(self): - msg = "Create a domain request and reject it and see if email was sent." - user, _ = User.objects.get_or_create(username="testy") - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.APPROVED, user=user) - self.check_email_sent(domain_request, msg, "reject", 1, expected_content="Hi", expected_email=user.email) - @less_console_noise_decorator def test_reject_with_prejudice_does_not_send_email(self): msg = "Create a domain request and reject it with prejudice and see if email was sent." From dbfc54837ad41089132f7ed258d60b6579ee0bea Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 10 Oct 2024 09:06:10 -0600 Subject: [PATCH 32/34] Fix test + some minor cleanup stuff --- src/registrar/models/domain_request.py | 10 ++++++- .../admin/includes/detail_table_fieldset.html | 8 +++--- src/registrar/tests/test_admin_request.py | 1 - src/registrar/tests/test_models_requests.py | 27 ++++++++++++++++++- 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 8f6dc44e8..b9e3315d5 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -704,6 +704,7 @@ def send_custom_status_update_email(self, status): # We should never send an email if no reason was specified. # Additionally, Don't send out emails for reasons that shouldn't send them. if status_info.get("reason") is None or status_info.get("reason") in status_info.get("excluded_reasons"): + logger.warning("send_custom_status_update_email() => Tried sending a status email without a reason.") return # Only send out an email if the underlying reason itself changed or if no email was sent previously. @@ -1064,8 +1065,15 @@ def withdraw(self): def reject(self): """Reject an domain request that has been submitted. + This action is logged. + + This action cleans up the action needed status if moving away from action needed. + As side effects this will delete the domain and domain_information - (will cascade), and send an email notification using send_custom_status_update_email. + (will cascade) when they exist. + + Afterwards, we send out an email for reject in def save(). + See the function send_custom_status_update_email. """ if self.status == self.DomainRequestStatus.APPROVED: 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 6a46fd9f2..f9aa9ac19 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -221,9 +221,9 @@

{% if original_object.action_needed_reason_email %} - + {% else %} - + {% endif %} {% elif field.field.name == "rejection_reason_email" %} @@ -310,9 +310,9 @@

{% if original_object.rejection_reason_email %} - + {% else %} - + {% endif %} {% else %} {{ field.field }} diff --git a/src/registrar/tests/test_admin_request.py b/src/registrar/tests/test_admin_request.py index cdff63124..217756359 100644 --- a/src/registrar/tests/test_admin_request.py +++ b/src/registrar/tests/test_admin_request.py @@ -637,7 +637,6 @@ def assert_email_is_accurate( with less_console_noise(): # Access the arguments passed to send_email call_args = self.mock_client.EMAILS_SENT - logger.info(f"what are the call args? {call_args}") kwargs = call_args[email_index]["kwargs"] # Retrieve the email details from the arguments diff --git a/src/registrar/tests/test_models_requests.py b/src/registrar/tests/test_models_requests.py index 3be2225ee..ea0303230 100644 --- a/src/registrar/tests/test_models_requests.py +++ b/src/registrar/tests/test_models_requests.py @@ -267,7 +267,6 @@ def test_status_fsm_submit_succeed(self): domain_request.submit() self.assertEqual(domain_request.status, domain_request.DomainRequestStatus.SUBMITTED) - @less_console_noise_decorator def check_email_sent( self, domain_request, msg, action, expected_count, expected_content=None, expected_email="mayor@igorville.com" ): @@ -278,6 +277,7 @@ def check_email_sent( # Perform the specified action action_method = getattr(domain_request, action) action_method() + domain_request.save() # Check if an email was sent sent_emails = [ @@ -337,6 +337,31 @@ def test_withdraw_sends_email(self): domain_request, msg, "withdraw", 1, expected_content="withdrawn", expected_email=user.email ) + def test_reject_sends_email(self): + "Create a domain request and reject it and see if email was sent." + user, _ = User.objects.get_or_create(username="testy") + domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.APPROVED, user=user) + expected_email=user.email + email_allowed, _ = AllowedEmail.objects.get_or_create(email=expected_email) + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): + domain_request.reject() + domain_request.rejection_reason = domain_request.RejectionReasons.CONTACTS_NOT_VERIFIED + domain_request.rejection_reason_email = "test" + domain_request.save() + + # Check if an email was sent + sent_emails = [ + email + for email in MockSESClient.EMAILS_SENT + if expected_email in email["kwargs"]["Destination"]["ToAddresses"] + ] + self.assertEqual(len(sent_emails), 1) + + email_content = sent_emails[0]["kwargs"]["Content"]["Simple"]["Body"]["Text"]["Data"] + self.assertIn("test", email_content) + + email_allowed.delete() + @less_console_noise_decorator def test_reject_with_prejudice_does_not_send_email(self): msg = "Create a domain request and reject it with prejudice and see if email was sent." From 3e54bb1e593aeaebf59191857b3994daace0181c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 10 Oct 2024 09:52:56 -0600 Subject: [PATCH 33/34] simplify javascript --- src/registrar/assets/js/get-gov-admin.js | 48 +++++++++++-------- .../admin/includes/detail_table_fieldset.html | 35 +++++++------- 2 files changed, 44 insertions(+), 39 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 01b370366..fd50fbb0c 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -444,30 +444,23 @@ class CustomizableEmailBase { * @param {Object} config - must contain the following: * @property {HTMLElement} dropdown - The dropdown element. * @property {HTMLElement} textarea - The textarea element. - * @property {HTMLElement} textareaPlaceholder - The placeholder for the textarea. - * @property {HTMLElement} directEditButton - The button to directly edit the email. - * @property {HTMLElement} modalTrigger - The trigger for the modal. - * @property {HTMLElement} modalConfirm - The confirm button in the modal. - * @property {HTMLElement} formLabel - The label for the form. * @property {HTMLElement} lastSentEmailContent - The last sent email content element. * @property {HTMLElement} textAreaFormGroup - The form group for the textarea. * @property {HTMLElement} dropdownFormGroup - The form group for the dropdown. + * @property {HTMLElement} modalConfirm - The confirm button in the modal. * @property {string} apiUrl - The API URL for fetching email content. * @property {string} statusToCheck - The status to check against. Used for show/hide on textAreaFormGroup/dropdownFormGroup. * @property {string} sessionVariableName - The session variable name. Used for show/hide on textAreaFormGroup/dropdownFormGroup. * @property {string} apiErrorMessage - The error message that the ajax call returns. */ - constructor(config) { + constructor(config) { + this.config = config; this.dropdown = config.dropdown; this.textarea = config.textarea; - this.textareaPlaceholder = config.textareaPlaceholder; - this.directEditButton = config.directEditButton; - this.modalTrigger = config.modalTrigger; - this.modalConfirm = config.modalConfirm; - this.formLabel = config.formLabel; this.lastSentEmailContent = config.lastSentEmailContent; this.apiUrl = config.apiUrl; this.apiErrorMessage = config.apiErrorMessage; + this.modalConfirm = config.modalConfirm; // These fields are hidden/shown on pageload depending on the current status this.textAreaFormGroup = config.textAreaFormGroup; @@ -481,6 +474,14 @@ class CustomizableEmailBase { this.initialDropdownValue = this.dropdown ? this.dropdown.value : null; this.initialEmailValue = this.textarea ? this.textarea.value : null; + // Find other fields near the textarea + const parentDiv = this.textarea ? this.textarea.closest(".flex-container") : null; + this.directEditButton = parentDiv ? parentDiv.querySelector(".edit-email-button") : null; + this.modalTrigger = parentDiv ? parentDiv.querySelector(".edit-button-modal-trigger") : null; + + this.textareaPlaceholder = parentDiv ? parentDiv.querySelector(".custom-email-placeholder") : null; + this.formLabel = this.textarea ? document.querySelector(`label[for="${this.textarea.id}"]`) : null; + this.isEmailAlreadySentConst; if (this.lastSentEmailContent && this.textarea) { this.isEmailAlreadySentConst = this.lastSentEmailContent.value.replace(/\s+/g, '') === this.textarea.value.replace(/\s+/g, ''); @@ -642,12 +643,8 @@ class customActionNeededEmail extends CustomizableEmailBase { const emailConfig = { dropdown: document.getElementById("id_action_needed_reason"), textarea: document.getElementById("id_action_needed_reason_email"), - textareaPlaceholder: document.querySelector(".field-action_needed_reason_email__placeholder"), - directEditButton: document.querySelector('.field-action_needed_reason_email__edit'), - modalTrigger: document.querySelector('.field-action_needed_reason_email__modal-trigger'), - modalConfirm: document.getElementById('confirm-edit-email'), - formLabel: document.querySelector('label[for="id_action_needed_reason_email"]'), lastSentEmailContent: document.getElementById("last-sent-action-needed-email-content"), + modalConfirm: document.getElementById("action-needed-reason__confirm-edit-email"), apiUrl: document.getElementById("get-action-needed-email-for-user-json")?.value || null, textAreaFormGroup: document.querySelector('.field-action_needed_reason'), dropdownFormGroup: document.querySelector('.field-action_needed_reason_email'), @@ -690,6 +687,13 @@ document.addEventListener('DOMContentLoaded', function() { // Initialize UI const customEmail = new customActionNeededEmail(); + + // Check that every variable was setup correctly + const nullItems = Object.entries(customEmail.config).filter(([key, value]) => value === null).map(([key]) => key); + if (nullItems.length > 0) { + console.error(`Failed to load customActionNeededEmail(). Some variables were null: ${nullItems.join(", ")}`) + return; + } customEmail.loadActionNeededEmail() }); @@ -699,12 +703,8 @@ class customRejectedEmail extends CustomizableEmailBase { const emailConfig = { dropdown: document.getElementById("id_rejection_reason"), textarea: document.getElementById("id_rejection_reason_email"), - textareaPlaceholder: document.querySelector(".field-rejection_reason_email__placeholder"), - directEditButton: document.querySelector('.field-rejection_reason_email__edit'), - modalTrigger: document.querySelector('.field-rejection_reason_email__modal-trigger'), - modalConfirm: document.getElementById('confirm-edit-email'), - formLabel: document.querySelector('label[for="id_rejection_reason_email"]'), lastSentEmailContent: document.getElementById("last-sent-rejection-email-content"), + modalConfirm: document.getElementById("rejection-reason__confirm-edit-email"), apiUrl: document.getElementById("get-rejection-email-for-user-json")?.value || null, textAreaFormGroup: document.querySelector('.field-rejection_reason'), dropdownFormGroup: document.querySelector('.field-rejection_reason_email'), @@ -749,6 +749,12 @@ document.addEventListener('DOMContentLoaded', function() { // Initialize UI const customEmail = new customRejectedEmail(); + // Check that every variable was setup correctly + const nullItems = Object.entries(customEmail.config).filter(([key, value]) => value === null).map(([key]) => key); + if (nullItems.length > 0) { + console.error(`Failed to load customRejectedEmail(). Some variables were null: ${nullItems.join(", ")}`) + return; + } customEmail.loadRejectedEmail() }); 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 f9aa9ac19..317604c5e 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -137,29 +137,28 @@ {% block field_other %} {% if field.field.name == "action_needed_reason_email" %} + {{ field.field }} -
+ - {{ field.field }} - Change Edit email
@@ -187,8 +186,8 @@

  • Change Edit email
    @@ -276,8 +275,8 @@