Merge pull request #1899 from cisagov/za/1864-error-message-rejection-placement

(on getgov-za) Ticket #1864: Move error message for rejection reason required
This commit is contained in:
zandercymatics 2024-03-25 08:07:51 -06:00 committed by GitHub
commit 35ffdc7bd0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 60 additions and 10 deletions

View file

@ -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

View file

@ -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;
}

View file

@ -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)

View file

@ -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

View file

@ -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):