diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 565bdb011..283281ad2 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -270,14 +270,9 @@ class DomainRequestAdminForm(forms.ModelForm): Checks if the action_needed_reason field is not none. Adds form errors on failure. """ - is_valid = False - error_message = None - if action_needed_reason is None or action_needed_reason == "": + is_valid = action_needed_reason is not None and action_needed_reason != "" + if not is_valid: error_message = FSMDomainRequestError.get_error_message(FSMErrorCodes.NO_ACTION_NEEDED_REASON) - else: - is_valid = True - - if error_message is not None: self.add_error("action_needed_reason", error_message) return is_valid diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index e5a3bfb62..9f9e0f06a 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -328,6 +328,7 @@ function initializeWidgetOnList(list, parentId) { } } + // Adds or removes the display-none class to object depending on the value of boolean hideObject function hideOrShowDomObject(object, hideObject){ if (object){ if (hideObject){ @@ -346,10 +347,13 @@ function initializeWidgetOnList(list, parentId) { function handleBackButtonObserver(fieldsToObserve) { const observer = new PerformanceObserver((list) => { list.getEntries().forEach((entry) => { + // This currently only handles the navigation buttons if (entry.type === "back_forward") { + // For each field we specify... fieldsToObserve.forEach((fieldName) => { fieldClass = `.field-${fieldName}` field = document.querySelector(fieldClass) + // ...Grab its related session object to determine if it should be visible or not if (field) { shouldHideField = sessionStorage.getItem(`hide_${fieldName}`) hideOrShowDomObject(field, hideObject=shouldHideField) @@ -363,17 +367,30 @@ function initializeWidgetOnList(list, parentId) { observer.observe({ type: "navigation" }); } + // Links the given field we want to show/hide with a given value of the status selector, + // and maintains this state with a given session object. + // For now, we assume that the session object follows this pattern: `hide_${field_name}` function handleStatusChanges() { // Show/hide the rejection reason let rejectionReasonFormGroup = document.querySelector('.field-rejection_reason') + + // element to hide, statusToShowOn, sessionObjectName showHideFieldsOnStatusChange(rejectionReasonFormGroup, "rejected", "hide_rejection_reason"); - // Show/hude the action needed reason + // Show/hide the action needed reason let actionNeededReasonFormGroup = document.querySelector('.field-action_needed_reason'); + + // element to hide, statusToShowOn, sessionObjectName showHideFieldsOnStatusChange(actionNeededReasonFormGroup, "action needed", "hide_action_needed_reason"); } + + // Hookup the fields that we want to programatically show/hide depending on the current value of the status field. + // Add your field name to this function if you are adding another dynamic field. handleStatusChanges(); + // Add an observer to each field to track when the back button is pressed. This is so + // our current state doesn't get wiped by browser events. + // Add a field name to this array if you are adding another dynamic field. let fieldsToObserve = ["rejection_reason", "action_needed_reason"] handleBackButtonObserver(fieldsToObserve); })(); diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 26527ba98..8a1d6a93e 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1,6 +1,5 @@ from __future__ import annotations from typing import Union -import os import logging from django.apps import apps @@ -244,11 +243,12 @@ class DomainRequest(TimeStampedModel): ORGANIZATION_ELIGIBILITY = "org_not_eligible", "Org not eligible for a .gov domain" NAMING_REQUIREMENTS = "naming_not_met", "Naming requirements not met" OTHER = "other", "Other/Unspecified" - + class ActionNeededReasons(models.TextChoices): - """Defines common""" + """Defines common action needed reasons for domain requests""" + ELIGIBILITY_UNCLEAR = ("eligibility_unclear", "Unclear organization eligibility") - QUESTIONABLE_AUTHORIZING_OFFICIAL = ("questionable_authorizing_official" , "Questionable authorizing official") + QUESTIONABLE_AUTHORIZING_OFFICIAL = ("questionable_authorizing_official", "Questionable authorizing official") ALREADY_HAS_DOMAINS = ("already_has_domains", "Already has domains") BAD_NAME = ("bad_name", "Doesn’t meet naming requirements") OTHER = ("other", "Other (no auto-email sent)") @@ -752,23 +752,26 @@ class DomainRequest(TimeStampedModel): def _send_action_needed_reason_email(self, send_email=True): """Sends out an automatic email for each valid action needed reason provided""" + # Store the filenames of the template and template subject email_template_name: str = "" email_template_subject_name: str = "" + + # Check for the "type" of action needed reason. can_send_email = True match self.action_needed_reason: # Add to this match if you need to pass in a custom filename for these templates. case self.ActionNeededReasons.OTHER, _: # Unknown and other are default cases - do nothing can_send_email = False - - if can_send_email: - # Assumes that the template name matches the action needed reason if nothing is specified. - # This is so you can override if you need, or have this taken care of for you. - if not email_template_name and not email_template_subject_name: - reason = self.action_needed_reason - email_template_name = f"{reason}.txt" - email_template_subject_name = f"{reason}_subject.txt" + # Assumes that the template name matches the action needed reason if nothing is specified. + # This is so you can override if you need, or have this taken care of for you. + if not email_template_name and not email_template_subject_name: + email_template_name = f"{self.action_needed_reason}.txt" + email_template_subject_name = f"{self.action_needed_reason}_subject.txt" + + # If we can, try to send out an email as long as send_email=True + if can_send_email: self._send_status_update_email( new_status="action needed", email_template=f"emails/action_needed_reasons/{email_template_name}", diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index e2d390471..fa571331c 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1444,20 +1444,25 @@ class TestDomainRequestAdmin(MockEppLib): # The results are filtered by "status in [submitted,in review,action needed]" self.assertContains(response, "status in [submitted,in review,action needed]", count=1) - def transition_state_and_send_email(self, domain_request, status, rejection_reason=None): + @less_console_noise_decorator + def transition_state_and_send_email(self, domain_request, status, rejection_reason=None, action_needed_reason=None): """Helper method for the email test cases.""" with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - with less_console_noise(): - # Create a mock request - request = self.factory.post("/admin/registrar/domainrequest/{}/change/".format(domain_request.pk)) + # Create a mock request + request = self.factory.post("/admin/registrar/domainrequest/{}/change/".format(domain_request.pk)) - # Modify the domain request's properties - domain_request.status = status + # Modify the domain request's properties + domain_request.status = status + + if rejection_reason: domain_request.rejection_reason = rejection_reason - # Use the model admin's save_model method - self.admin.save_model(request, domain_request, form=None, change=True) + if action_needed_reason: + domain_request.action_needed_reason = action_needed_reason + + # Use the model admin's save_model method + self.admin.save_model(request, domain_request, form=None, change=True) def assert_email_is_accurate( self, expected_string, email_index, email_address, test_that_no_bcc=False, bcc_email_address="" @@ -1492,6 +1497,64 @@ class TestDomainRequestAdmin(MockEppLib): bcc_email = kwargs["Destination"]["BccAddresses"][0] self.assertEqual(bcc_email, bcc_email_address) + def test_action_needed_sends_reason_email(self): + """When an action needed reason is set, an email is sent out.""" + # Ensure there is no user with this email + EMAIL = "mayor@igorville.gov" + 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, True) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 1) + + # Revert back to in review to reset for the next assert + domain_request.status = DomainRequest.DomainRequestStatus.IN_REVIEW + domain_request.save() + + # 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, True) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 2) + + # Revert back to in review to reset for the next assert + domain_request.status = DomainRequest.DomainRequestStatus.IN_REVIEW + domain_request.save() + + # 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, True) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) + + # Revert back to in review to reset for the next assert + domain_request.status = DomainRequest.DomainRequestStatus.IN_REVIEW + domain_request.save() + + # Test the email sent out for questionable_ao + questionable_ao = DomainRequest.ActionNeededReasons.QUESTIONABLE_AUTHORIZING_OFFICIAL + self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=questionable_ao) + self.assert_email_is_accurate("AUTHORIZING OFFICIAL DOES NOT MEET ELIGIBILITY REQUIREMENTS", 3, EMAIL, True) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 4) + + # Revert back to in review to reset for the next assert + domain_request.status = DomainRequest.DomainRequestStatus.IN_REVIEW + domain_request.save() + + # 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) + def test_save_model_sends_submitted_email(self): """When transitioning to submitted from started or withdrawn on a domain request, an email is sent out.