diff --git a/atst/domain/requests/financial_verification.py b/atst/domain/requests/financial_verification.py index 3197c53e..380b53e4 100644 --- a/atst/domain/requests/financial_verification.py +++ b/atst/domain/requests/financial_verification.py @@ -17,12 +17,15 @@ class PENumberValidator(object): re.X, ) - def validate(self, request, pe_id): - if self._same_as_previous(request, pe_id): + def validate(self, request, field): + if field.errors: + return False + + if self._same_as_previous(request, field.data): return True try: - PENumbers.get(pe_id) + PENumbers.get(field.data) except NotFoundError: return False @@ -46,12 +49,27 @@ class PENumberValidator(object): def _same_as_previous(self, request, pe_id): return request.pe_number == pe_id + def _apply_error(self, field): + suggestion = self.pe_validator.suggest_pe_id(field.data) + error_str = ( + "We couldn't find that PE number. {}" + "If you have double checked it you can submit anyway. " + "Your request will need to go through a manual review." + ).format('Did you mean "{}"? '.format(suggestion) if suggestion else "") + field.errors += (error_str,) + field.errors = list(field.errors) + class TaskOrderNumberValidator(object): - def validate(self, task_order_number): + def validate(self, field): try: - TaskOrders.get(task_order_number) + TaskOrders.get(field.data) except NotFoundError: + self._apply_error(field) return False return True + + def _apply_error(self, field): + field.errors += ("Task Order number not found",) + field.errors = list(field.errors) diff --git a/atst/forms/financial.py b/atst/forms/financial.py index 504c5a49..15ddb955 100644 --- a/atst/forms/financial.py +++ b/atst/forms/financial.py @@ -33,8 +33,6 @@ def coerce_choice(val): class DraftValidateMixin(object): def validate_draft(self): """ - Another stupid workaround. Maybe there isn't a better way. - Make all fields optional before validation, and then return them to their previous state. """ @@ -50,6 +48,19 @@ class DraftValidateMixin(object): class TaskOrderForm(ValidatedForm, DraftValidateMixin): + def do_validate_number(self): + for field in self: + if field.name != "task_order-number": + field.validators.insert(0, Optional()) + + valid = super().validate() + + for field in self: + if field.name != "task_order-number": + field.validators.pop(0) + + return valid + number = StringField( "Task Order Number associated with this request", description="Include the original Task Order number (including the 000X at the end). Do not include any modification numbers. Note that there may be a lag between approving a task order and when it becomes available in our system.", @@ -185,6 +196,9 @@ class FinancialVerificationForm(ValidatedForm): request = FormField(RequestFinancialVerificationForm) def validate(self, *args, **kwargs): + if not kwargs.get("is_extended", True): + return self.do_validate_request() + if self.task_order.funding_type.data == "OTHER": self.task_order.funding_type_other.validators.append(InputRequired()) @@ -205,7 +219,9 @@ class FinancialVerificationForm(ValidatedForm): Called do_validate_request to avoid being considered an inline validator by wtforms. """ - return self.request.validate(self) + request_valid = self.request.validate(self) + task_order_valid = self.task_order.do_validate_number() + return request_valid and task_order_valid def validate_draft(self): return self.task_order.validate_draft() and self.request.validate_draft() diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 2e4b2d90..cd2caf0f 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -61,7 +61,7 @@ class FinancialVerificationBase(object): return attachment - def _try_create_task_order(self, form, attachment): + def _try_create_task_order(self, form, attachment, is_extended): task_order_number = form.task_order.number.data if not task_order_number: return None @@ -83,7 +83,10 @@ class FinancialVerificationBase(object): except NotFoundError: pass - return TaskOrders.create(**task_order_data) + if is_extended: + return TaskOrders.create(**task_order_data) + else: + return None def _apply_pe_number_error(self, field): suggestion = self.pe_validator.suggest_pe_id(field.data) @@ -93,9 +96,11 @@ class FinancialVerificationBase(object): "Your request will need to go through a manual review." ).format('Did you mean "{}"? '.format(suggestion) if suggestion else "") field.errors += (error_str,) + field.errors = list(field.errors) def _apply_task_order_number_error(self, field): field.errors += ("Task Order number not found",) + field.errors = list(field.errors) def _raise(self, form): form.reset() @@ -139,22 +144,17 @@ class UpdateFinancialVerification(FinancialVerificationBase): attachment = self._process_attachment(self.is_extended, form) - if self.is_extended: - if not form.validate(has_attachment=attachment): - should_update = False - else: - should_update = form.do_validate_request() + if not form.validate(is_extended=self.is_extended, has_attachment=attachment): + should_update = False - if not self.pe_validator.validate(self.request, form.pe_id.data): - self._apply_pe_number_error(form.pe_id) + if not self.pe_validator.validate(self.request, form.pe_id): should_submit = False - if not self.task_order_validator.validate(form.task_order_number.data): - self._apply_task_order_number_error(form.task_order_number) + if not self.task_order_validator.validate(form.task_order.number): should_submit = False if should_update: - task_order = self._try_create_task_order(form, attachment) + task_order = self._try_create_task_order(form, attachment, self.is_extended) updated_request = Requests.update_financial_verification( self.request.id, form.request.data, task_order=task_order ) @@ -188,20 +188,14 @@ class SaveFinancialVerificationDraft(FinancialVerificationBase): if not form.validate_draft(): self._raise(form) - if form.pe_id.data and not self.pe_validator.validate( - self.request, form.pe_id.data - ): + if not self.pe_validator.validate(self.request, form.pe_id): valid = False - self._apply_pe_number_error(form.pe_id) - if form.task_order_number.data and not self.task_order_validator.validate( - form.task_order_number.data - ): + if form.task_order.number.data and not self.task_order_validator.validate(form.task_order.number): valid = False - self._apply_task_order_number_error(form.task_order_number) attachment = self._process_attachment(self.is_extended, form) - task_order = self._try_create_task_order(form, attachment) + task_order = self._try_create_task_order(form, attachment, self.is_extended) updated_request = Requests.update_financial_verification( self.request.id, form.request.data, task_order=task_order ) diff --git a/templates/components/text_input.html b/templates/components/text_input.html index bbff201b..b48f3bad 100644 --- a/templates/components/text_input.html +++ b/templates/components/text_input.html @@ -18,7 +18,7 @@ {% if paragraph %}paragraph='true'{% endif %} {% if noMaxWidth %}no-max-width='true'{% endif %} {% if initial_value or field.data is not none %}initial-value='{{ initial_value or field.data }}'{% endif %} - {% if field.errors %}v-bind:initial-errors='{{ field.errors }}'{% endif %} + {% if field.errors %}v-bind:initial-errors='{{ field.errors | list }}'{% endif %} key='{{ field.name }}' inline-template> diff --git a/tests/routes/test_financial_verification.py b/tests/routes/test_financial_verification.py index 23d6196c..1a167ba3 100644 --- a/tests/routes/test_financial_verification.py +++ b/tests/routes/test_financial_verification.py @@ -328,14 +328,14 @@ def test_get_fv_form_route(client, user_session, fv_data): assert response.status_code == 200 -def test_manual_task_order_triggers_extended_form(client, user_session, fv_data): +def test_manual_task_order_triggers_extended_form(client, user_session, fv_data, e_fv_data): user = UserFactory.create() request = RequestFactory.create(creator=user) - data = {**fv_data, "task_order-number": "DCA10096D0053"} + data = {**fv_data, **e_fv_data, "task_order-number": "DCA10096D0053"} UpdateFinancialVerification( - TrueValidator, TrueValidator, user, request, data, is_extended=False + TrueValidator, TrueValidator, user, request, data, is_extended=True ).execute() user_session(user)