diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 5b88a9ce2..0096f59b5 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -16,7 +16,7 @@ from django.urls import reverse from dateutil.relativedelta import relativedelta # type: ignore from epplibwrapper.errors import ErrorCode, RegistryError from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website -from registrar.utility.errors import FSMApplicationError, FSMErrorCodes +from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes from registrar.views.utility.mixins import OrderableFieldsMixin from django.contrib.admin.views.main import ORDER_VAR from registrar.widgets import NoAutocompleteFilteredSelectMultiple @@ -133,6 +133,7 @@ class DomainRequestAdminForm(forms.ModelForm): cleaned_data = super().clean() status = cleaned_data.get("status") investigator = cleaned_data.get("investigator") + rejection_reason = cleaned_data.get("rejection_reason") # Get the old status initial_status = self.initial.get("status", None) @@ -153,8 +154,32 @@ class DomainRequestAdminForm(forms.ModelForm): # Will call "add_error" if any issues are found. self._check_for_valid_investigator(investigator) + # If the status is rejected, a rejection reason must exist + if status == DomainRequest.DomainRequestStatus.REJECTED: + self._check_for_valid_rejection_reason(rejection_reason) + return cleaned_data + def _check_for_valid_rejection_reason(self, rejection_reason) -> bool: + """ + Checks if the rejection_reason field is not none. + Adds form errors on failure. + """ + is_valid = False + + # Check if a rejection reason exists. Rejection is not possible without one. + error_message = None + if rejection_reason is None or rejection_reason == "": + # Lets grab the error message from a common location + error_message = FSMDomainRequestError.get_error_message(FSMErrorCodes.NO_REJECTION_REASON) + else: + is_valid = True + + if error_message is not None: + self.add_error("rejection_reason", error_message) + + return is_valid + def _check_for_valid_investigator(self, investigator) -> bool: """ Checks if the investigator field is not none, and is staff. @@ -167,9 +192,9 @@ class DomainRequestAdminForm(forms.ModelForm): error_message = None if investigator is None: # Lets grab the error message from a common location - error_message = FSMApplicationError.get_error_message(FSMErrorCodes.NO_INVESTIGATOR) + error_message = FSMDomainRequestError.get_error_message(FSMErrorCodes.NO_INVESTIGATOR) elif not investigator.is_staff: - error_message = FSMApplicationError.get_error_message(FSMErrorCodes.INVESTIGATOR_NOT_STAFF) + error_message = FSMDomainRequestError.get_error_message(FSMErrorCodes.INVESTIGATOR_NOT_STAFF) else: is_valid = True @@ -994,7 +1019,7 @@ class DomainRequestAdmin(ListHeaderAdmin): if self.value() == "0": return queryset.filter(Q(is_election_board=False) | Q(is_election_board=None)) - change_form_template = "django/admin/domain_application_change_form.html" + change_form_template = "django/admin/domain_request_change_form.html" # Columns list_display = [ @@ -1209,7 +1234,7 @@ class DomainRequestAdmin(ListHeaderAdmin): # This condition should never be triggered. # The opposite of this condition is acceptable (rejected -> other status and rejection_reason) # because we clean up the rejection reason in the transition in the model. - error_message = "A rejection reason is required." + error_message = FSMDomainRequestError.get_error_message(FSMErrorCodes.NO_REJECTION_REASON) else: # This is an fsm in model which will throw an error if the # transition condition is violated, so we roll back the @@ -1218,11 +1243,11 @@ class DomainRequestAdmin(ListHeaderAdmin): obj.status = original_obj.status # Try to perform the status change. - # Catch FSMApplicationError's and return the message, + # Catch FSMDomainRequestError's and return the message, # as these are typically user errors. try: selected_method() - except FSMApplicationError as err: + except FSMDomainRequestError as err: logger.warning(f"An error encountered when trying to change status: {err}") error_message = err.message diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 18025a9cb..b7a494aef 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -346,3 +346,7 @@ input.admin-confirm-button { .usa-summary-box__dhs-color { color: $dhs-blue-70; } + +.errors span.select2-selection { + border: 1px solid var(--error-fg) !important; +} \ No newline at end of file diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 7527529a1..f4581de93 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -9,7 +9,7 @@ from django.db import models from django_fsm import FSMField, transition # type: ignore from django.utils import timezone from registrar.models.domain import Domain -from registrar.utility.errors import FSMApplicationError, FSMErrorCodes +from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes from .utility.time_stamped_model import TimeStampedModel from ..utility.email import send_templated_email, EmailSendingError @@ -791,7 +791,7 @@ class DomainRequest(TimeStampedModel): # == Check that the domain_request is valid == # if Domain.objects.filter(name=self.requested_domain.name).exists(): - raise FSMApplicationError(code=FSMErrorCodes.APPROVE_DOMAIN_IN_USE) + raise FSMDomainRequestError(code=FSMErrorCodes.APPROVE_DOMAIN_IN_USE) # == Create the domain and related components == # created_domain = Domain.objects.create(name=self.requested_domain.name) diff --git a/src/registrar/templates/django/admin/domain_application_change_form.html b/src/registrar/templates/django/admin/domain_request_change_form.html similarity index 100% rename from src/registrar/templates/django/admin/domain_application_change_form.html rename to src/registrar/templates/django/admin/domain_request_change_form.html diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 28a500ae2..8ed61d76c 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -555,6 +555,24 @@ class TestDomainRequestAdminForm(TestCase): expected_choices = [("started", "Started"), ("submitted", "Submitted")] self.assertEqual(form.fields["status"].widget.choices, expected_choices) + def test_form_no_rejection_reason(self): + with less_console_noise(): + # Create a form instance with the test domain request + form = DomainRequestAdminForm(instance=self.domain_request) + + form = DomainRequestAdminForm( + instance=self.domain_request, + data={ + "status": DomainRequest.DomainRequestStatus.REJECTED, + "rejection_reason": None, + }, + ) + self.assertFalse(form.is_valid()) + self.assertIn("rejection_reason", form.errors) + + rejection_reason = form.errors.get("rejection_reason") + self.assertEqual(rejection_reason, ["A rejection reason is required."]) + def test_form_choices_when_no_instance(self): with less_console_noise(): # Create a form instance without an instance diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 00c65ce57..f00c59bd0 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -78,15 +78,17 @@ class FSMErrorCodes(IntEnum): - 2 NO_INVESTIGATOR No investigator is assigned - 3 INVESTIGATOR_NOT_STAFF Investigator is a non-staff user - 4 INVESTIGATOR_NOT_SUBMITTER The form submitter is not the investigator + - 5 NO_REJECTION_REASON No rejection reason is specified """ APPROVE_DOMAIN_IN_USE = 1 NO_INVESTIGATOR = 2 INVESTIGATOR_NOT_STAFF = 3 INVESTIGATOR_NOT_SUBMITTER = 4 + NO_REJECTION_REASON = 5 -class FSMApplicationError(Exception): +class FSMDomainRequestError(Exception): """ Used to raise exceptions when doing FSM Transitions. Uses `FSMErrorCodes` as an enum. @@ -97,6 +99,7 @@ class FSMApplicationError(Exception): FSMErrorCodes.NO_INVESTIGATOR: ("Investigator is required for this status."), FSMErrorCodes.INVESTIGATOR_NOT_STAFF: ("Investigator is not a staff user."), FSMErrorCodes.INVESTIGATOR_NOT_SUBMITTER: ("Only the assigned investigator can make this change."), + FSMErrorCodes.NO_REJECTION_REASON: ("A rejection reason is required."), } def __init__(self, *args, code=None, **kwargs):