From 6c67166e86b212b4b938dfcfa5b198589e3e4f9b Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 21 Aug 2018 15:26:28 -0400 Subject: [PATCH 01/13] persist partial financial form data if task order number not found --- atst/forms/financial.py | 21 +++++++++++++++------ tests/forms/test_financial.py | 5 +++-- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/atst/forms/financial.py b/atst/forms/financial.py index e989d822..93d08258 100644 --- a/atst/forms/financial.py +++ b/atst/forms/financial.py @@ -1,7 +1,7 @@ import re from wtforms.fields.html5 import EmailField from wtforms.fields import StringField -from wtforms.validators import Required, Email, Regexp, ValidationError +from wtforms.validators import Required, Email, Regexp from atst.domain.exceptions import NotFoundError from atst.domain.pe_numbers import PENumbers @@ -58,6 +58,16 @@ def validate_pe_id(field, existing_request): return True +def validate_task_order_number(field): + try: + TaskOrders.get(field.data) + except NotFoundError: + field.errors += ("Task Order number not found",) + return False + + return True + + class BaseFinancialForm(ValidatedForm): def reset(self): """ @@ -119,11 +129,10 @@ class BaseFinancialForm(ValidatedForm): class FinancialForm(BaseFinancialForm): - def validate_task_order_number(form, field): - try: - TaskOrders.get(field.data) - except NotFoundError: - raise ValidationError("Task Order number not found") + def perform_extra_validation(self, existing_request): + previous_valid = super().perform_extra_validation(existing_request) + task_order_valid = validate_task_order_number(self.task_order_number) + return previous_valid and task_order_valid @property def is_missing_task_order_number(self): diff --git a/tests/forms/test_financial.py b/tests/forms/test_financial.py index 871c7c19..3f0d5e91 100644 --- a/tests/forms/test_financial.py +++ b/tests/forms/test_financial.py @@ -71,13 +71,14 @@ def test_ba_code_validation(input_, expected): def test_task_order_number_validation(monkeypatch): monkeypatch.setattr("atst.domain.task_orders.TaskOrders._client", lambda: MockEDAClient()) + monkeypatch.setattr("atst.forms.financial.validate_pe_id", lambda *args: True) form_invalid = FinancialForm(data={"task_order_number": "1234"}) - form_invalid.validate() + form_invalid.perform_extra_validation({}) assert "task_order_number" in form_invalid.errors form_valid = FinancialForm(data={"task_order_number": MockEDAClient.MOCK_CONTRACT_NUMBER}, eda_client=MockEDAClient()) - form_valid.validate() + form_valid.perform_extra_validation({}) assert "task_order_number" not in form_valid.errors From f2dbed1ef1a2492573a569586cca4d2f94b79606 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 21 Aug 2018 16:07:54 -0400 Subject: [PATCH 02/13] add new task order fields --- ...8a05fc_add_additional_task_order_fields.py | 44 +++++++++++++++++++ atst/models/task_order.py | 24 +++++++++- 2 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 alembic/versions/875e4b8a05fc_add_additional_task_order_fields.py diff --git a/alembic/versions/875e4b8a05fc_add_additional_task_order_fields.py b/alembic/versions/875e4b8a05fc_add_additional_task_order_fields.py new file mode 100644 index 00000000..a28792c9 --- /dev/null +++ b/alembic/versions/875e4b8a05fc_add_additional_task_order_fields.py @@ -0,0 +1,44 @@ +"""add additional task order fields + +Revision ID: 875e4b8a05fc +Revises: 05d6272bdb43 +Create Date: 2018-08-21 15:52:46.636928 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '875e4b8a05fc' +down_revision = '05d6272bdb43' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('task_order', sa.Column('clin_0001', sa.Integer(), nullable=True)) + op.add_column('task_order', sa.Column('clin_0003', sa.Integer(), nullable=True)) + op.add_column('task_order', sa.Column('clin_1001', sa.Integer(), nullable=True)) + op.add_column('task_order', sa.Column('clin_1003', sa.Integer(), nullable=True)) + op.add_column('task_order', sa.Column('clin_2001', sa.Integer(), nullable=True)) + op.add_column('task_order', sa.Column('clin_2003', sa.Integer(), nullable=True)) + op.add_column('task_order', sa.Column('funding_type', sa.String(), nullable=True)) + op.add_column('task_order', sa.Column('funding_type_other', sa.String(), nullable=True)) + op.add_column('task_order', sa.Column('source', sa.String(), nullable=True)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('task_order', 'source') + op.drop_column('task_order', 'funding_type_other') + op.drop_column('task_order', 'funding_type') + op.drop_column('task_order', 'clin_2003') + op.drop_column('task_order', 'clin_2001') + op.drop_column('task_order', 'clin_1003') + op.drop_column('task_order', 'clin_1001') + op.drop_column('task_order', 'clin_0003') + op.drop_column('task_order', 'clin_0001') + # ### end Alembic commands ### diff --git a/atst/models/task_order.py b/atst/models/task_order.py index 6bacb913..7494ac35 100644 --- a/atst/models/task_order.py +++ b/atst/models/task_order.py @@ -1,10 +1,32 @@ -from sqlalchemy import Column, Integer, String +from enum import Enum + +from sqlalchemy import Column, Integer, String, Enum as SQLAEnum from atst.models import Base +class Source(Enum): + MANUAL = "Manual" + EDA = "eda" + + +class FundingType(Enum): + RDTE = "RDTE" + OM = "OM" + PROC = "PROC" + OTHER = "OTHER" + class TaskOrder(Base): __tablename__ = "task_order" id = Column(Integer, primary_key=True) number = Column(String) + source = Column(SQLAEnum(Source)) + funding_type = Column(SQLAEnum(FundingType)) + funding_type_other = Column(String) + clin_0001 = Column(Integer) + clin_0003 = Column(Integer) + clin_1001 = Column(Integer) + clin_1003 = Column(Integer) + clin_2001 = Column(Integer) + clin_2003 = Column(Integer) From e3631da8ea822c7689ce4f8b7be0a4f01f001fd6 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 21 Aug 2018 16:24:30 -0400 Subject: [PATCH 03/13] request can update financial verification data --- ...401_add_request_task_order_relationship.py | 30 ++++++++++++ atst/domain/requests.py | 34 ++++++++++++- atst/domain/task_orders.py | 9 ++-- atst/models/request.py | 3 ++ atst/models/task_order.py | 2 +- tests/domain/test_requests.py | 48 ++++++++++++++++++- tests/domain/test_task_orders.py | 2 + 7 files changed, 120 insertions(+), 8 deletions(-) create mode 100644 alembic/versions/0845b2f0f401_add_request_task_order_relationship.py diff --git a/alembic/versions/0845b2f0f401_add_request_task_order_relationship.py b/alembic/versions/0845b2f0f401_add_request_task_order_relationship.py new file mode 100644 index 00000000..4b00c41f --- /dev/null +++ b/alembic/versions/0845b2f0f401_add_request_task_order_relationship.py @@ -0,0 +1,30 @@ +"""add request -> task order relationship + +Revision ID: 0845b2f0f401 +Revises: 875e4b8a05fc +Create Date: 2018-08-22 09:58:43.770718 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '0845b2f0f401' +down_revision = '875e4b8a05fc' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('requests', sa.Column('task_order_id', sa.Integer(), nullable=True)) + op.create_foreign_key(None, 'requests', 'task_order', ['task_order_id'], ['id']) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint(None, 'requests', type_='foreignkey') + op.drop_column('requests', 'task_order_id') + # ### end Alembic commands ### diff --git a/atst/domain/requests.py b/atst/domain/requests.py index 4f956b6c..45de8c53 100644 --- a/atst/domain/requests.py +++ b/atst/domain/requests.py @@ -5,9 +5,11 @@ from sqlalchemy.orm.exc import NoResultFound from sqlalchemy.orm.attributes import flag_modified from atst.models.request import Request +from atst.models.task_order import TaskOrder, Source as TaskOrderSource from atst.models.request_status_event import RequestStatusEvent, RequestStatus from atst.domain.workspaces import Workspaces from atst.database import db +from atst.domain.task_orders import TaskOrders from .exceptions import NotFoundError @@ -107,14 +109,20 @@ 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) # Without this, sqlalchemy won't notice the change to request.body, # since it doesn't track dictionary mutations by default. flag_modified(request, "body") - db.session.add(request) - db.session.commit() + return request return request @@ -215,3 +223,25 @@ WHERE requests_with_status.status = :status def completed_count(cls): return Requests.status_count(RequestStatus.APPROVED) + _TASK_ORDER_DATA = [col.name for col in TaskOrder.__table__.c if col.name != "id"] + + @classmethod + def update_financial_verification(cls, request_id, financial_data): + request = Requests.get(request_id) + + 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} + + task_order = None + if task_order_data: + 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"]) + + 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/domain/task_orders.py b/atst/domain/task_orders.py index fbc587a1..3a07195d 100644 --- a/atst/domain/task_orders.py +++ b/atst/domain/task_orders.py @@ -2,7 +2,7 @@ from sqlalchemy.orm.exc import NoResultFound from flask import current_app as app from atst.database import db -from atst.models.task_order import TaskOrder +from atst.models.task_order import TaskOrder, Source from .exceptions import NotFoundError @@ -26,13 +26,14 @@ class TaskOrders(object): def _get_from_eda(cls, order_number): to_data = TaskOrders._client().get_contract(order_number, status="y") if to_data: - return TaskOrders.create(to_data["contract_no"]) + # TODO: we need to determine exactly what we're getting and storing from the EDA client + return TaskOrders.create(number=to_data["contract_no"], source=Source.EDA) else: raise NotFoundError("task_order") @classmethod - def create(cls, order_number): - task_order = TaskOrder(number=order_number) + def create(cls, **kwargs): + task_order = TaskOrder(**kwargs) db.session.add(task_order) db.session.commit() diff --git a/atst/models/request.py b/atst/models/request.py index 44474ca3..3d79a706 100644 --- a/atst/models/request.py +++ b/atst/models/request.py @@ -20,6 +20,9 @@ class Request(Base): user_id = Column(ForeignKey("users.id"), nullable=False) creator = relationship("User") + task_order_id = Column(ForeignKey("task_order.id")) + task_order = relationship("TaskOrder") + @property def status(self): return self.status_events[-1].new_status diff --git a/atst/models/task_order.py b/atst/models/task_order.py index 7494ac35..5c34935e 100644 --- a/atst/models/task_order.py +++ b/atst/models/task_order.py @@ -20,7 +20,7 @@ class TaskOrder(Base): __tablename__ = "task_order" id = Column(Integer, primary_key=True) - number = Column(String) + number = Column(String, unique=True) source = Column(SQLAEnum(Source)) funding_type = Column(SQLAEnum(FundingType)) funding_type_other = Column(String) diff --git a/tests/domain/test_requests.py b/tests/domain/test_requests.py index 575697ba..a2992ae8 100644 --- a/tests/domain/test_requests.py +++ b/tests/domain/test_requests.py @@ -5,8 +5,9 @@ from atst.domain.exceptions import NotFoundError from atst.domain.requests import Requests from atst.models.request import Request from atst.models.request_status_event import RequestStatus +from atst.models.task_order import Source as TaskOrderSource -from tests.factories import RequestFactory, UserFactory, RequestStatusEventFactory +from tests.factories import RequestFactory, UserFactory, RequestStatusEventFactory, TaskOrderFactory @pytest.fixture(scope="function") @@ -91,3 +92,48 @@ def test_status_count_scoped_to_creator(session): assert Requests.status_count(RequestStatus.STARTED) == 2 assert Requests.status_count(RequestStatus.STARTED, creator=user) == 1 + + +request_financial_data = { + "pe_id": "123", + "task_order_number": "021345", + "fname_co": "Contracting", + "lname_co": "Officer", + "email_co": "jane@mail.mil", + "office_co": "WHS", + "fname_cor": "Officer", + "lname_cor": "Representative", + "email_cor": "jane@mail.mil", + "office_cor": "WHS", + "uii_ids": "1234", + "treasury_code": "00123456", + "ba_code": "024A", +} +task_order_financial_data = { + "funding_type": "RDTE", + "funding_type_other": "other", + "clin_0001": 50000, + "clin_0003": 13000, + "clin_1001": 30000, + "clin_1003": 7000, + "clin_2001": 30000, + "clin_2003": 7000, +} + + +# 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() + 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 + + 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 + diff --git a/tests/domain/test_task_orders.py b/tests/domain/test_task_orders.py index 27b5d2de..9f37e30a 100644 --- a/tests/domain/test_task_orders.py +++ b/tests/domain/test_task_orders.py @@ -1,5 +1,6 @@ import pytest +from atst.models.task_order import Source as TaskOrderSource from atst.domain.exceptions import NotFoundError from atst.domain.task_orders import TaskOrders from atst.eda_client import MockEDAClient @@ -19,6 +20,7 @@ def test_can_get_task_order_from_eda(monkeypatch): to = TaskOrders.get(MockEDAClient.MOCK_CONTRACT_NUMBER) assert to.number == MockEDAClient.MOCK_CONTRACT_NUMBER + assert to.source == TaskOrderSource.EDA def test_nonexistent_task_order_raises_without_client(): From d66a496fbc18b6febd4aeaa39ef095114832bc06 Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 22 Aug 2018 13:00:23 -0400 Subject: [PATCH 04/13] financial verification should create or find a Task Order for the request --- atst/domain/requests.py | 33 +++++++++++++------ atst/forms/financial.py | 25 ++++++++++---- .../routes/requests/financial_verification.py | 3 +- tests/domain/test_requests.py | 30 ++++++++++------- 4 files changed, 61 insertions(+), 30 deletions(-) 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 From f1ec71fe4207c9769493e32308782543445a9ba4 Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 22 Aug 2018 13:35:06 -0400 Subject: [PATCH 05/13] set pending CCPO review status on request with complete financial information --- atst/domain/requests.py | 26 ++++++++++++------- .../routes/requests/financial_verification.py | 2 +- tests/domain/test_requests.py | 6 +++++ 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/atst/domain/requests.py b/atst/domain/requests.py index a1159d48..c0dc596e 100644 --- a/atst/domain/requests.py +++ b/atst/domain/requests.py @@ -232,23 +232,16 @@ WHERE requests_with_status.status = :status _TASK_ORDER_DATA = [col.name for col in TaskOrder.__table__.c if col.name != "id"] @classmethod - def update_financial_verification(cls, request_id, financial_data): + def update_financial_verification(cls, request_id, financial_data, completed=False): 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} + task_order_number = request_data.pop("task_order_number") - task_order = None - if task_order_data: - 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 + task_order = Requests._get_or_create_task_order(task_order_number, task_order_data) if task_order: request.task_order = task_order @@ -256,5 +249,18 @@ WHERE requests_with_status.status = :status Requests._merge_body(request, {"financial_verification": request_data}) + if completed: + Requests.set_status(request, RequestStatus.PENDING_CCPO_APPROVAL) + db.session.add(request) db.session.commit() + + @classmethod + def _get_or_create_task_order(cls, number, task_order_data={}): + if task_order_data: + return TaskOrders.create(**task_order_data, number=number, source=TaskOrderSource.MANUAL) + else: + try: + return TaskOrders.get(number) + except NotFoundError: + return diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 4283451b..554f9179 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -39,7 +39,7 @@ def update_financial_verification(request_id): valid = form.perform_extra_validation( existing_request.body.get("financial_verification") ) - updated_request = Requests.update_financial_verification(request_id, form.data) + updated_request = Requests.update_financial_verification(request_id, form.data, completed=valid) 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 8ef88c34..8cae80b1 100644 --- a/tests/domain/test_requests.py +++ b/tests/domain/test_requests.py @@ -143,3 +143,9 @@ def test_update_financial_verification_with_invalid_task_order(): Requests.update_financial_verification(request.id, request_financial_data) assert not request.task_order + +def test_update_financial_verification_completed(): + request = RequestFactory.create() + Requests.update_financial_verification(request.id, request_financial_data, completed=True) + assert request.status == RequestStatus.PENDING_CCPO_APPROVAL + From 217183c1423fc01a5cc2207d296f51fbd0449f88 Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 22 Aug 2018 14:13:37 -0400 Subject: [PATCH 06/13] submitting final financial verification is a top-level Requests method --- atst/domain/requests.py | 17 +++++++++++++---- atst/routes/requests/financial_verification.py | 4 +++- tests/domain/test_requests.py | 6 ------ 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/atst/domain/requests.py b/atst/domain/requests.py index c0dc596e..6bcdad8f 100644 --- a/atst/domain/requests.py +++ b/atst/domain/requests.py @@ -232,7 +232,7 @@ WHERE requests_with_status.status = :status _TASK_ORDER_DATA = [col.name for col in TaskOrder.__table__.c if col.name != "id"] @classmethod - def update_financial_verification(cls, request_id, financial_data, completed=False): + def update_financial_verification(cls, request_id, financial_data): request = Requests._get_with_lock(request_id) if not request: return @@ -249,9 +249,6 @@ WHERE requests_with_status.status = :status Requests._merge_body(request, {"financial_verification": request_data}) - if completed: - Requests.set_status(request, RequestStatus.PENDING_CCPO_APPROVAL) - db.session.add(request) db.session.commit() @@ -264,3 +261,15 @@ WHERE requests_with_status.status = :status return TaskOrders.get(number) except NotFoundError: return + + @classmethod + def submit_financial_verification(cls, request_id): + request = Requests._get_with_lock(request_id) + if not request: + return + + Requests.set_status(request, RequestStatus.PENDING_CCPO_APPROVAL) + + db.session.add(request) + db.session.commit() + diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 554f9179..34e3b006 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -39,10 +39,12 @@ def update_financial_verification(request_id): valid = form.perform_extra_validation( existing_request.body.get("financial_verification") ) - updated_request = Requests.update_financial_verification(request_id, form.data, completed=valid) + updated_request = Requests.update_financial_verification(request_id, form.data) if valid: + Requests.submit_financial_verification(request_id) new_workspace = Requests.approve_and_create_workspace(updated_request) return redirect(url_for("workspaces.workspace_projects", workspace_id=new_workspace.id, newWorkspace=True)) + else: form.reset() return render_template( diff --git a/tests/domain/test_requests.py b/tests/domain/test_requests.py index 8cae80b1..8ef88c34 100644 --- a/tests/domain/test_requests.py +++ b/tests/domain/test_requests.py @@ -143,9 +143,3 @@ def test_update_financial_verification_with_invalid_task_order(): Requests.update_financial_verification(request.id, request_financial_data) assert not request.task_order - -def test_update_financial_verification_completed(): - request = RequestFactory.create() - Requests.update_financial_verification(request.id, request_financial_data, completed=True) - assert request.status == RequestStatus.PENDING_CCPO_APPROVAL - From db2abac340f9355acd16a09c6f7da9e6ca0c13b4 Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 22 Aug 2018 14:32:59 -0400 Subject: [PATCH 07/13] add TODO --- atst/eda_client.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/atst/eda_client.py b/atst/eda_client.py index 82c01217..3251499e 100644 --- a/atst/eda_client.py +++ b/atst/eda_client.py @@ -73,6 +73,9 @@ class MockEDAClient(EDAClientBase): MOCK_CONTRACT_NUMBER = "DCA10096D0052" + # TODO: It seems likely that this will have to supply CLIN data form the + # EDA returnclinXML API call, in addition to the basic task order data + # below. See the EDA docs. def get_contract(self, contract_number, status): if contract_number == self.MOCK_CONTRACT_NUMBER and status == "y": return { From 5850002d2ad5a650854c98f9536ac53d90d1c01e Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 23 Aug 2018 09:06:46 -0400 Subject: [PATCH 08/13] remove mutable default argument --- atst/domain/requests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atst/domain/requests.py b/atst/domain/requests.py index 6bcdad8f..67b6ae36 100644 --- a/atst/domain/requests.py +++ b/atst/domain/requests.py @@ -253,7 +253,7 @@ WHERE requests_with_status.status = :status db.session.commit() @classmethod - def _get_or_create_task_order(cls, number, task_order_data={}): + def _get_or_create_task_order(cls, number, task_order_data=None): if task_order_data: return TaskOrders.create(**task_order_data, number=number, source=TaskOrderSource.MANUAL) else: From b2ec0764a745025748156cb06afd9f72d8c69c20 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 23 Aug 2018 09:27:52 -0400 Subject: [PATCH 09/13] fix alembix migration chain after rebase --- .../versions/875e4b8a05fc_add_additional_task_order_fields.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/alembic/versions/875e4b8a05fc_add_additional_task_order_fields.py b/alembic/versions/875e4b8a05fc_add_additional_task_order_fields.py index a28792c9..c9ee8ee0 100644 --- a/alembic/versions/875e4b8a05fc_add_additional_task_order_fields.py +++ b/alembic/versions/875e4b8a05fc_add_additional_task_order_fields.py @@ -1,7 +1,7 @@ """add additional task order fields Revision ID: 875e4b8a05fc -Revises: 05d6272bdb43 +Revises: f36f130622b9 Create Date: 2018-08-21 15:52:46.636928 """ @@ -11,7 +11,7 @@ import sqlalchemy as sa # revision identifiers, used by Alembic. revision = '875e4b8a05fc' -down_revision = '05d6272bdb43' +down_revision = 'f36f130622b9' branch_labels = None depends_on = None From 40320baf10f1539ef296e1a0e7252974e08963f9 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 23 Aug 2018 09:28:08 -0400 Subject: [PATCH 10/13] fix requests methods after rebase --- atst/domain/requests.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/atst/domain/requests.py b/atst/domain/requests.py index 67b6ae36..beda987d 100644 --- a/atst/domain/requests.py +++ b/atst/domain/requests.py @@ -106,6 +106,8 @@ class Requests(object): db.session.add(request) db.session.commit() + return request + @classmethod def _get_with_lock(cls, request_id): try: @@ -130,8 +132,6 @@ class Requests(object): return request - return request - @classmethod def approve_and_create_workspace(cls, request): approved_request = Requests.set_status(request, RequestStatus.APPROVED) @@ -247,11 +247,13 @@ WHERE requests_with_status.status = :status request.task_order = task_order db.session.add(task_order) - Requests._merge_body(request, {"financial_verification": request_data}) + request = Requests._merge_body(request, {"financial_verification": request_data}) db.session.add(request) db.session.commit() + return request + @classmethod def _get_or_create_task_order(cls, number, task_order_data=None): if task_order_data: From e30d4e238c315a8cdc979b7827bcf3b73df5e507 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 23 Aug 2018 10:11:00 -0400 Subject: [PATCH 11/13] expect CLIN data with no commas from the financial verification form --- atst/forms/financial.py | 20 +++++++++----------- tests/routes/test_financial_verification.py | 12 ++++++------ 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/atst/forms/financial.py b/atst/forms/financial.py index 1402daa7..83ee7e37 100644 --- a/atst/forms/financial.py +++ b/atst/forms/financial.py @@ -68,11 +68,9 @@ def validate_task_order_number(field): return True -def formatted_number_to_int(num): - if not num: - return - else: - return int(num.replace(",","")) +def number_to_int(num): + if num: + return int(num) class BaseFinancialForm(ValidatedForm): @@ -171,40 +169,40 @@ class ExtendedFinancialForm(BaseFinancialForm): "
CLIN 0001
-
Unclassified IaaS and PaaS Amount
", validators=[Required()], description="Review your task order document, the amounts for each CLIN must match exactly here", - filters=[formatted_number_to_int] + filters=[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", - filters=[formatted_number_to_int] + filters=[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", - filters=[formatted_number_to_int] + filters=[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", - filters=[formatted_number_to_int] + filters=[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", - filters=[formatted_number_to_int] + filters=[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", - filters=[formatted_number_to_int] + filters=[number_to_int] ) diff --git a/tests/routes/test_financial_verification.py b/tests/routes/test_financial_verification.py index d6552846..ccb8c119 100644 --- a/tests/routes/test_financial_verification.py +++ b/tests/routes/test_financial_verification.py @@ -27,12 +27,12 @@ class TestPENumberInForm: extended_data = { "funding_type": "RDTE", "funding_type_other": "other", - "clin_0001": "50,000", - "clin_0003": "13,000", - "clin_1001": "30,000", - "clin_1003": "7,000", - "clin_2001": "30,000", - "clin_2003": "7,000", + "clin_0001": "50000", + "clin_0003": "13000", + "clin_1001": "30000", + "clin_1003": "7000", + "clin_2001": "30000", + "clin_2003": "7000", } def _set_monkeypatches(self, monkeypatch): From 8aa6185c91938e0b790f2a8df5b7132d488fa8b7 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 23 Aug 2018 10:15:19 -0400 Subject: [PATCH 12/13] don't need to explicitly add the task order when saving the related request --- atst/domain/requests.py | 1 - 1 file changed, 1 deletion(-) diff --git a/atst/domain/requests.py b/atst/domain/requests.py index beda987d..0c98e72b 100644 --- a/atst/domain/requests.py +++ b/atst/domain/requests.py @@ -245,7 +245,6 @@ WHERE requests_with_status.status = :status if task_order: request.task_order = task_order - db.session.add(task_order) request = Requests._merge_body(request, {"financial_verification": request_data}) From f32a3eb90dbb19fefe60b479d6e7c70fde3b1a0b Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 23 Aug 2018 10:26:43 -0400 Subject: [PATCH 13/13] move task order concerns into that domain class --- atst/domain/requests.py | 69 ++++++++++++++++++-------------------- atst/domain/task_orders.py | 13 +++++++ 2 files changed, 45 insertions(+), 37 deletions(-) diff --git a/atst/domain/requests.py b/atst/domain/requests.py index 0c98e72b..e569bc4b 100644 --- a/atst/domain/requests.py +++ b/atst/domain/requests.py @@ -5,7 +5,6 @@ from sqlalchemy.orm.exc import NoResultFound from sqlalchemy.orm.attributes import flag_modified from atst.models.request import Request -from atst.models.task_order import TaskOrder, Source as TaskOrderSource from atst.models.request_status_event import RequestStatusEvent, RequestStatus from atst.domain.workspaces import Workspaces from atst.database import db @@ -54,6 +53,7 @@ class Requests(object): and_(Request.id == request_id, Request.creator == creator) ) ).scalar() + except exc.DataError: return False @@ -73,10 +73,9 @@ class Requests(object): filters.append(Request.creator == creator) requests = ( - db.session.query(Request) - .filter(*filters) - .order_by(Request.time_created.desc()) - .all() + db.session.query(Request).filter(*filters).order_by( + Request.time_created.desc() + ).all() ) return requests @@ -114,11 +113,11 @@ class Requests(object): # 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 return ( - db.session.query(Request) - .filter_by(id=request_id) - .with_for_update(of=Request) - .one() + db.session.query(Request).filter_by(id=request_id).with_for_update( + of=Request + ).one() ) + except NoResultFound: return @@ -153,8 +152,10 @@ class Requests(object): return { RequestStatus.STARTED: "mission_owner", RequestStatus.PENDING_FINANCIAL_VERIFICATION: "mission_owner", - RequestStatus.PENDING_CCPO_APPROVAL: "ccpo" - }.get(request.status) + RequestStatus.PENDING_CCPO_APPROVAL: "ccpo", + }.get( + request.status + ) @classmethod def should_auto_approve(cls, request): @@ -166,16 +167,13 @@ class Requests(object): return dollar_value < cls.AUTO_APPROVE_THRESHOLD _VALID_SUBMISSION_STATUSES = [ - RequestStatus.STARTED, - RequestStatus.CHANGES_REQUESTED, + RequestStatus.STARTED, RequestStatus.CHANGES_REQUESTED ] @classmethod def should_allow_submission(cls, request): all_request_sections = [ - "details_of_use", - "information_about_you", - "primary_poc", + "details_of_use", "information_about_you", "primary_poc" ] existing_request_sections = request.body.keys() return request.status in Requests._VALID_SUBMISSION_STATUSES and all( @@ -215,11 +213,13 @@ WHERE requests_with_status.status = :status @classmethod def in_progress_count(cls): - return sum([ - Requests.status_count(RequestStatus.STARTED), - Requests.status_count(RequestStatus.PENDING_FINANCIAL_VERIFICATION), - Requests.status_count(RequestStatus.CHANGES_REQUESTED), - ]) + return sum( + [ + Requests.status_count(RequestStatus.STARTED), + Requests.status_count(RequestStatus.PENDING_FINANCIAL_VERIFICATION), + Requests.status_count(RequestStatus.CHANGES_REQUESTED), + ] + ) @classmethod def pending_ccpo_count(cls): @@ -229,8 +229,6 @@ WHERE requests_with_status.status = :status def completed_count(cls): return Requests.status_count(RequestStatus.APPROVED) - _TASK_ORDER_DATA = [col.name for col in TaskOrder.__table__.c if col.name != "id"] - @classmethod def update_financial_verification(cls, request_id, financial_data): request = Requests._get_with_lock(request_id) @@ -238,31 +236,29 @@ WHERE requests_with_status.status = :status 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} + task_order_data = { + k: request_data.pop(k) + for (k, v) in financial_data.items() + if k in TaskOrders.TASK_ORDER_DATA + } task_order_number = request_data.pop("task_order_number") - task_order = Requests._get_or_create_task_order(task_order_number, task_order_data) + task_order = TaskOrders.get_or_create_task_order( + task_order_number, task_order_data + ) if task_order: request.task_order = task_order - request = Requests._merge_body(request, {"financial_verification": request_data}) + request = Requests._merge_body( + request, {"financial_verification": request_data} + ) db.session.add(request) db.session.commit() return request - @classmethod - def _get_or_create_task_order(cls, number, task_order_data=None): - if task_order_data: - return TaskOrders.create(**task_order_data, number=number, source=TaskOrderSource.MANUAL) - else: - try: - return TaskOrders.get(number) - except NotFoundError: - return - @classmethod def submit_financial_verification(cls, request_id): request = Requests._get_with_lock(request_id) @@ -273,4 +269,3 @@ WHERE requests_with_status.status = :status db.session.add(request) db.session.commit() - diff --git a/atst/domain/task_orders.py b/atst/domain/task_orders.py index 3a07195d..d03910d4 100644 --- a/atst/domain/task_orders.py +++ b/atst/domain/task_orders.py @@ -7,6 +7,7 @@ from .exceptions import NotFoundError class TaskOrders(object): + TASK_ORDER_DATA = [col.name for col in TaskOrder.__table__.c if col.name != "id"] @classmethod def get(cls, order_number): @@ -28,6 +29,7 @@ class TaskOrders(object): if to_data: # TODO: we need to determine exactly what we're getting and storing from the EDA client return TaskOrders.create(number=to_data["contract_no"], source=Source.EDA) + else: raise NotFoundError("task_order") @@ -43,3 +45,14 @@ class TaskOrders(object): @classmethod def _client(cls): return app.eda_client + + @classmethod + def get_or_create_task_order(cls, number, task_order_data=None): + try: + return TaskOrders.get(number) + + except NotFoundError: + if task_order_data: + return TaskOrders.create( + **task_order_data, number=number, source=Source.MANUAL + )