diff --git a/atst/domain/requests.py b/atst/domain/requests.py index 45de8c53..a1159d48 100644 --- a/atst/domain/requests.py +++ b/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: - task_order = TaskOrders.get(financial_data["task_order_number"]) + 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) - request.task_order = task_order Requests._merge_body(request, {"financial_verification": request_data}) - db.session.add(task_order) db.session.add(request) db.session.commit() diff --git a/atst/forms/financial.py b/atst/forms/financial.py index 93d08258..1402daa7 100644 --- a/atst/forms/financial.py +++ b/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( "
CLIN 0001
-
Unclassified IaaS and PaaS Amount
", 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( "
CLIN 0003
-
Unclassified Cloud Support Package
", 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( "
CLIN 1001
-
Unclassified IaaS and PaaS Amount
OPTION PERIOD 1
", 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( "
CLIN 1003
-
Unclassified Cloud Support Package
OPTION PERIOD 1
", 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( "
CLIN 2001
-
Unclassified IaaS and PaaS Amount
OPTION PERIOD 2
", 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( "
CLIN 2003
-
Unclassified Cloud Support Package
OPTION PERIOD 2
", 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] ) diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index c16c6e10..4283451b 100644 --- a/atst/routes/requests/financial_verification.py +++ b/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)) diff --git a/tests/domain/test_requests.py b/tests/domain/test_requests.py index a2992ae8..8ef88c34 100644 --- a/tests/domain/test_requests.py +++ b/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