Add unit tests

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
+        is_valid = action_needed_reason is not None and action_needed_reason != ""
-        error_message = None
+        if not is_valid:
-        if action_needed_reason is None or action_needed_reason == "":
             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

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);
 })();

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}",

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
-                # Create a mock request
+            request = self.factory.post("/admin/registrar/domainrequest/{}/change/".format(domain_request.pk))
-                request = self.factory.post("/admin/registrar/domainrequest/{}/change/".format(domain_request.pk))
 
-                # Modify the domain request's properties
+            # Modify the domain request's properties
-                domain_request.status = status
+            domain_request.status = status
+
+            if rejection_reason:
                 domain_request.rejection_reason = rejection_reason
 
-                # Use the model admin's save_model method
+            if action_needed_reason:
-                self.admin.save_model(request, domain_request, form=None, change=True)
+                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.