financial verification should create or find a Task Order for the request

atst/domain/requests.py

@@ -97,10 +97,21 @@ class Requests(object):
 
     @classmethod
     def update(cls, request_id, request_delta):
+        request = Requests._get_with_lock(request_id)
+        if not request:
+            return
+
+        request = Requests._merge_body(request, request_delta)
+
+        db.session.add(request)
+        db.session.commit()
+
+    @classmethod
+    def _get_with_lock(cls, request_id):
         try:
             # Query for request matching id, acquiring a row-level write lock.
             # https://www.postgresql.org/docs/10/static/sql-select.html#SQL-FOR-UPDATE-SHARE
-            request = (
+            return (
                 db.session.query(Request)
                 .filter_by(id=request_id)
                 .with_for_update(of=Request)
@@ -109,11 +120,6 @@ class Requests(object):
         except NoResultFound:
             return
 
-        request = Requests._merge_body(request, request_delta)
-
-        db.session.add(request)
-        db.session.commit()
-
     @classmethod
     def _merge_body(cls, request, request_delta):
         request.body = deep_merge(request_delta, request.body)
@@ -227,7 +233,9 @@ WHERE requests_with_status.status = :status
 
     @classmethod
     def update_financial_verification(cls, request_id, financial_data):
-        request = Requests.get(request_id)
+        request = Requests._get_with_lock(request_id)
+        if not request:
+            return
 
         request_data = financial_data.copy()
         task_order_data = {k: request_data.pop(k) for (k,v) in financial_data.items() if k in Requests._TASK_ORDER_DATA}
@@ -237,11 +245,16 @@ WHERE requests_with_status.status = :status
             task_order_data["number"] = request_data.pop("task_order_number")
             task_order = TaskOrders.create(**task_order_data, source=TaskOrderSource.MANUAL)
         else:
+            try:
                 task_order = TaskOrders.get(financial_data["task_order_number"])
+            except NotFoundError:
+                pass
 
+        if task_order:
             request.task_order = task_order
+            db.session.add(task_order)
+
         Requests._merge_body(request, {"financial_verification": request_data})
 
-        db.session.add(task_order)
         db.session.add(request)
         db.session.commit()

atst/forms/financial.py

@@ -68,6 +68,13 @@ def validate_task_order_number(field):
     return True
 
 
+def formatted_number_to_int(num):
+    if not num:
+        return
+    else:
+        return int(num.replace(",",""))
+
+
 class BaseFinancialForm(ValidatedForm):
     def reset(self):
         """
@@ -163,35 +170,41 @@ class ExtendedFinancialForm(BaseFinancialForm):
     clin_0001 = StringField(
         "<dl><dt>CLIN 0001</dt> - <dd>Unclassified IaaS and PaaS Amount</dd></dl>",
         validators=[Required()],
-        description="Review your task order document, the amounts for each CLIN must match exactly here"
+        description="Review your task order document, the amounts for each CLIN must match exactly here",
+        filters=[formatted_number_to_int]
     )
 
     clin_0003 = StringField(
         "<dl><dt>CLIN 0003</dt> - <dd>Unclassified Cloud Support Package</dd></dl>",
         validators=[Required()],
-        description="Review your task order document, the amounts for each CLIN must match exactly here"
+        description="Review your task order document, the amounts for each CLIN must match exactly here",
+        filters=[formatted_number_to_int]
     )
 
     clin_1001 = StringField(
         "<dl><dt>CLIN 1001</dt> - <dd>Unclassified IaaS and PaaS Amount <br> OPTION PERIOD 1</dd></dl>",
         validators=[Required()],
-        description="Review your task order document, the amounts for each CLIN must match exactly here"
+        description="Review your task order document, the amounts for each CLIN must match exactly here",
+        filters=[formatted_number_to_int]
     )
 
     clin_1003 = StringField(
         "<dl><dt>CLIN 1003</dt> - <dd>Unclassified Cloud Support Package <br> OPTION PERIOD 1</dd></dl>",
         validators=[Required()],
-        description="Review your task order document, the amounts for each CLIN must match exactly here"
+        description="Review your task order document, the amounts for each CLIN must match exactly here",
+        filters=[formatted_number_to_int]
     )
 
     clin_2001 = StringField(
         "<dl><dt>CLIN 2001</dt> - <dd>Unclassified IaaS and PaaS Amount <br> OPTION PERIOD 2</dd></dl>",
         validators=[Required()],
-        description="Review your task order document, the amounts for each CLIN must match exactly here"
+        description="Review your task order document, the amounts for each CLIN must match exactly here",
+        filters=[formatted_number_to_int]
     )
 
     clin_2003 = StringField(
         "<dl><dt>CLIN 2003</dt> - <dd>Unclassified Cloud Support Package <br> OPTION PERIOD 2</dd></dl>",
         validators=[Required()],
-        description="Review your task order document, the amounts for each CLIN must match exactly here"
+        description="Review your task order document, the amounts for each CLIN must match exactly here",
+        filters=[formatted_number_to_int]
     )

atst/routes/requests/financial_verification.py

@@ -36,11 +36,10 @@ def update_financial_verification(request_id):
     )
 
     if form.validate():
-        request_data = {"financial_verification": form.data}
         valid = form.perform_extra_validation(
             existing_request.body.get("financial_verification")
         )
-        updated_request = Requests.update(request_id, request_data)
+        updated_request = Requests.update_financial_verification(request_id, form.data)
         if valid:
             new_workspace = Requests.approve_and_create_workspace(updated_request)
             return redirect(url_for("workspaces.workspace_projects", workspace_id=new_workspace.id, newWorkspace=True))

tests/domain/test_requests.py

@@ -121,19 +121,25 @@ task_order_financial_data = {
 }
 
 
-# without a matching task order, should create one with status "MANUAL";
-# with a matching task order, should associate to an existing one
-def test_update_financial_verification():
-    request1 = RequestFactory.create()
+def test_update_financial_verification_without_task_order():
+    request = RequestFactory.create()
     financial_data = { **request_financial_data, **task_order_financial_data }
-    Requests.update_financial_verification(request1.id, financial_data)
-    assert request1.task_order
-    assert request1.task_order.clin_0001 == task_order_financial_data["clin_0001"]
-    assert request1.task_order.source == TaskOrderSource.MANUAL
+    Requests.update_financial_verification(request.id, financial_data)
+    assert request.task_order
+    assert request.task_order.clin_0001 == task_order_financial_data["clin_0001"]
+    assert request.task_order.source == TaskOrderSource.MANUAL
 
+
+def test_update_financial_verification_with_task_order():
     task_order = TaskOrderFactory.create(source=TaskOrderSource.EDA)
-    new_financial_verification_data = { **request_financial_data, "task_order_number": task_order.number }
-    request2 = RequestFactory.create()
-    Requests.update_financial_verification(request2.id, new_financial_verification_data)
-    assert request2.task_order == task_order
+    financial_data = { **request_financial_data, "task_order_number": task_order.number }
+    request = RequestFactory.create()
+    Requests.update_financial_verification(request.id, financial_data)
+    assert request.task_order == task_order
+
+
+def test_update_financial_verification_with_invalid_task_order():
+    request = RequestFactory.create()
+    Requests.update_financial_verification(request.id, request_financial_data)
+    assert not request.task_order