More validation fixes

atst/domain/requests/financial_verification.py

@@ -17,12 +17,15 @@ class PENumberValidator(object):
         re.X,
     )
 
-    def validate(self, request, pe_id):
+    def validate(self, request, field):
-        if self._same_as_previous(request, pe_id):
+        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)

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

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
 
+        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(is_extended=self.is_extended, has_attachment=attachment):
-            if not form.validate(has_attachment=attachment):
             should_update = False
-        else:
-            should_update = form.do_validate_request()
 
-        if not self.pe_validator.validate(self.request, form.pe_id.data):
+        if not self.pe_validator.validate(self.request, form.pe_id):
-            self._apply_pe_number_error(form.pe_id)
             should_submit = False
 
-        if not self.task_order_validator.validate(form.task_order_number.data):
+        if not self.task_order_validator.validate(form.task_order.number):
-            self._apply_task_order_number_error(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(
+        if not self.pe_validator.validate(self.request, form.pe_id):
-            self.request, form.pe_id.data
-        ):
             valid = False
-            self._apply_pe_number_error(form.pe_id)
 
-        if form.task_order_number.data and not self.task_order_validator.validate(
+        if form.task_order.number.data and not self.task_order_validator.validate(form.task_order.number):
-            form.task_order_number.data
-        ):
             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
         )

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>
 

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)