diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 9c3b8075e..a33b8eaa9 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -786,8 +786,8 @@ class DomainApplicationAdminForm(forms.ModelForm): fields = "__all__" def __init__(self, *args, **kwargs): + self.request = kwargs.pop("request", None) super().__init__(*args, **kwargs) - application = kwargs.get("instance") if application and application.pk: current_state = application.status @@ -813,27 +813,42 @@ class DomainApplicationAdminForm(forms.ModelForm): # after clean_fields. it is used to determine form level errors. # is_valid is typically called from view during a post cleaned_data = super().clean() - investigator = cleaned_data.get("investigator") status = cleaned_data.get("status") - requested_domain = cleaned_data.get("requested_domain") + investigator = cleaned_data.get("investigator") - # Check if an investigator is assigned. No approval is possible without one. - # TODO - add form level error - # TODO - maybe add modal if approver is not investigator as superuser - if investigator is None: - error_message = ApplicationStatusError.get_error_message(FSMErrorCodes.APPROVE_NO_INVESTIGATOR) - self.add_error("investigator", error_message) - else: - # Investigators must be staff users. - # This is handled elsewhere, but we should check here as a precaution. - if not investigator.is_staff: - error_message = ApplicationStatusError.get_error_message(FSMErrorCodes.APPROVE_INVESTIGATOR_NOT_STAFF) - self.add_error("investigator", error_message) - #if status == DomainApplication.ApplicationStatus.APPROVED and investigator != request.user: - #raise ValidationError("Only the assigned investigator can approve this application.") + if status == DomainApplication.ApplicationStatus.APPROVED: + # Checks the "investigators" field for validity. + # That field must obey certain conditions when an application is approved. + # Will call "add_error" if any issues are found. + self._check_investigators_on_approval(investigator) return cleaned_data + def _check_investigators_on_approval(self, investigator): + """Checks the investigator field when an approval occurs""" + + error_message = None + # Check if an investigator is assigned. No approval is possible without one. + if investigator is not None: + + if not investigator.is_staff: + # Investigators must be staff users. + # This is handled elsewhere, but we should check here as a precaution. + error_message = ApplicationStatusError.get_error_message(FSMErrorCodes.APPROVE_INVESTIGATOR_NOT_STAFF) + elif investigator != self.request.user: + # If the submitting user is not the investigator, block this action. + # This is to enforce accountability. Superusers do not have this restriction. + error_message = ApplicationStatusError.get_error_message( + FSMErrorCodes.APPROVE_INVESTIGATOR_NOT_SUBMITTER + ) + else: + error_message = ApplicationStatusError.get_error_message(FSMErrorCodes.APPROVE_NO_INVESTIGATOR) + + # Add the error + if error_message is not None: + self.add_error("investigator", error_message) + + class DomainApplicationAdmin(ListHeaderAdmin): """Custom domain applications admin class.""" @@ -977,6 +992,23 @@ class DomainApplicationAdmin(ListHeaderAdmin): # Table ordering ordering = ["requested_domain__name"] + def get_form(self, request, obj=None, **kwargs): + """ + Workaround to pass the request context to the underlying DomainApplicationAdminForm form object. + This is so we can do things like check the current user against a form value at submission. + """ + # Call the superclass's get_form method to get the form class + da_form = super().get_form(request, obj, **kwargs) + + # Define a wrapper class for the form that includes the request in its initialization. + # This is needed such that we can inject request without otherwise altering it. + class DomainApplicationFormWrapper(da_form): + def __new__(cls, *args, **form_kwargs): + form_kwargs["request"] = request + return da_form(*args, **form_kwargs) + + return DomainApplicationFormWrapper + # Trigger action when a fieldset is changed def save_model(self, request, obj, form, change): # TODO - there is an existing bug in these in that they redirect diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index 5de6dc482..5f4ce8a01 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -740,8 +740,6 @@ class DomainApplication(TimeStampedModel): raise ApplicationStatusError(code=FSMErrorCodes.APPROVE_DOMAIN_IN_USE) # Check if an investigator is assigned. No approval is possible without one. - # TODO - add form level error - # TODO - maybe add modal if approver is not investigator as superuser if self.investigator is None: raise ApplicationStatusError(code=FSMErrorCodes.APPROVE_NO_INVESTIGATOR) diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 8d77ecd2e..a4359b0c7 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -78,10 +78,12 @@ class FSMErrorCodes(IntEnum): - 1 APPROVE_DOMAIN_IN_USE The domain is already in use - 2 APPROVE_NO_INVESTIGATOR No investigator is assigned when approving - 3 APPROVE_INVESTIGATOR_NOT_STAFF Investigator is a non-staff user + - 4 APPROVE_INVESTIGATOR_NOT_SUBMITTER The form submitter is not the investigator """ APPROVE_DOMAIN_IN_USE = 1 APPROVE_NO_INVESTIGATOR = 2 APPROVE_INVESTIGATOR_NOT_STAFF = 3 + APPROVE_INVESTIGATOR_NOT_SUBMITTER = 4 # (Q for reviewers) What should this be called? @@ -102,6 +104,9 @@ class ApplicationStatusError(Exception): FSMErrorCodes.APPROVE_INVESTIGATOR_NOT_STAFF: ( "Cannot approve. Investigator is not a staff user." ), + FSMErrorCodes.APPROVE_INVESTIGATOR_NOT_SUBMITTER: ( + "Cannot approve. Only the assigned investigator can approve this application." + ) } def __init__(self, *args, code=None, **kwargs):