From e72b980d942d0223bce7d4d5fefbb79f48c37bf3 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 15 Oct 2018 12:28:18 -0400 Subject: [PATCH 01/47] Refactor FinVer to make room for draft feature --- .../domain/requests/financial_verification.py | 57 ++++ atst/forms/exceptions.py | 6 + atst/forms/financial.py | 114 ++----- atst/models/request.py | 4 + .../routes/requests/financial_verification.py | 183 +++++------ tests/forms/test_financial.py | 45 +-- tests/mocks.py | 2 +- tests/routes/test_financial_verification.py | 305 +++++++----------- 8 files changed, 304 insertions(+), 412 deletions(-) create mode 100644 atst/domain/requests/financial_verification.py create mode 100644 atst/forms/exceptions.py diff --git a/atst/domain/requests/financial_verification.py b/atst/domain/requests/financial_verification.py new file mode 100644 index 00000000..cf3d5896 --- /dev/null +++ b/atst/domain/requests/financial_verification.py @@ -0,0 +1,57 @@ +import re + +from atst.domain.task_orders import TaskOrders +from atst.domain.pe_numbers import PENumbers +from atst.domain.exceptions import NotFoundError + + +class PENumberValidator(object): + PE_REGEX = re.compile( + r""" + (0?\d) # program identifier + (0?\d) # category + (\d) # activity + (\d+) # sponsor element + (.+) # service + """, + re.X, + ) + + def validate(self, request, field): + if self._same_as_previous(request, field): + return True + + try: + PENumbers.get(field.data) + except NotFoundError: + return False + + return True + + def suggest_pe_id(self, pe_id): + suggestion = pe_id + match = self.PE_REGEX.match(pe_id) + if match: + (program, category, activity, sponsor, service) = match.groups() + if len(program) < 2: + program = "0" + program + if len(category) < 2: + category = "0" + category + suggestion = "".join((program, category, activity, sponsor, service)) + + if suggestion != pe_id: + return suggestion + return None + + def _same_as_previous(self, request, field): + return request.pe_number == field.data + + +class TaskOrderNumberValidator(object): + def validate(self, field): + try: + TaskOrders.get(field.data) + except NotFoundError: + return False + + return True diff --git a/atst/forms/exceptions.py b/atst/forms/exceptions.py new file mode 100644 index 00000000..43fb99cf --- /dev/null +++ b/atst/forms/exceptions.py @@ -0,0 +1,6 @@ +class FormValidationError(Exception): + + message = "Form validation failed." + + def __init__(self, form): + self.form = form diff --git a/atst/forms/financial.py b/atst/forms/financial.py index fa7f0bb1..69671dca 100644 --- a/atst/forms/financial.py +++ b/atst/forms/financial.py @@ -2,77 +2,20 @@ import re import pendulum from wtforms.fields.html5 import DateField, EmailField from wtforms.fields import StringField, FileField -from wtforms.validators import InputRequired, Email, Regexp +from wtforms.validators import DataRequired, Email, Regexp from flask_wtf.file import FileAllowed -from atst.domain.exceptions import NotFoundError -from atst.domain.pe_numbers import PENumbers -from atst.domain.task_orders import TaskOrders - from .fields import NewlineListField, SelectField, NumberStringField from .forms import ValidatedForm from .data import FUNDING_TYPES from .validators import DateRange -PE_REGEX = re.compile( - r""" - (0?\d) # program identifier - (0?\d) # category - (\d) # activity - (\d+) # sponsor element - (.+) # service -""", - re.X, -) - TREASURY_CODE_REGEX = re.compile(r"^0*([1-9]{4}|[1-9]{6})$") BA_CODE_REGEX = re.compile(r"[0-9]{2}\w?$") -def suggest_pe_id(pe_id): - suggestion = pe_id - match = PE_REGEX.match(pe_id) - if match: - (program, category, activity, sponsor, service) = match.groups() - if len(program) < 2: - program = "0" + program - if len(category) < 2: - category = "0" + category - suggestion = "".join((program, category, activity, sponsor, service)) - - if suggestion != pe_id: - return suggestion - return None - - -def validate_pe_id(field, existing_request): - try: - PENumbers.get(field.data) - except NotFoundError: - suggestion = 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,) - return False - - 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 - - def number_to_int(num): if num: return int(num) @@ -86,12 +29,6 @@ class BaseFinancialForm(ValidatedForm): """ self.uii_ids.process_data(self.uii_ids.data) - def perform_extra_validation(self, existing_request): - valid = True - if not existing_request or existing_request.get("pe_id") != self.pe_id.data: - valid = validate_pe_id(self.pe_id, existing_request) - return valid - @property def is_missing_task_order_number(self): return False @@ -99,7 +36,7 @@ class BaseFinancialForm(ValidatedForm): task_order_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.", - validators=[InputRequired()], + validators=[DataRequired()], ) uii_ids = NewlineListField( @@ -110,43 +47,38 @@ class BaseFinancialForm(ValidatedForm): pe_id = StringField( "Program Element Number", description="PE numbers help the Department of Defense identify which offices' budgets are contributing towards this resource use.
It should be 7 digits followed by 1-3 letters, and should have a zero as the first and third digits.", - validators=[InputRequired()], + validators=[DataRequired()], ) treasury_code = StringField( "Program Treasury Code", description="Program Treasury Code (or Appropriations Code) identifies resource types.
It should be a four digit or six digit number, optionally prefixed by one or more zeros.", - validators=[InputRequired(), Regexp(TREASURY_CODE_REGEX)], + validators=[DataRequired(), Regexp(TREASURY_CODE_REGEX)], ) ba_code = StringField( "Program Budget Activity (BA) Code", description="BA Code is used to identify the purposes, projects, or types of activities financed by the appropriation fund.
It should be two digits, followed by an optional letter.", - validators=[InputRequired(), Regexp(BA_CODE_REGEX)], + validators=[DataRequired(), Regexp(BA_CODE_REGEX)], ) - fname_co = StringField("KO First Name", validators=[InputRequired()]) - lname_co = StringField("KO Last Name", validators=[InputRequired()]) + fname_co = StringField("KO First Name", validators=[DataRequired()]) + lname_co = StringField("KO Last Name", validators=[DataRequired()]) - email_co = EmailField("KO Email", validators=[InputRequired(), Email()]) + email_co = EmailField("KO Email", validators=[DataRequired(), Email()]) - office_co = StringField("KO Office", validators=[InputRequired()]) + office_co = StringField("KO Office", validators=[DataRequired()]) - fname_cor = StringField("COR First Name", validators=[InputRequired()]) + fname_cor = StringField("COR First Name", validators=[DataRequired()]) - lname_cor = StringField("COR Last Name", validators=[InputRequired()]) + lname_cor = StringField("COR Last Name", validators=[DataRequired()]) - email_cor = EmailField("COR Email", validators=[InputRequired(), Email()]) + email_cor = EmailField("COR Email", validators=[DataRequired(), Email()]) - office_cor = StringField("COR Office", validators=[InputRequired()]) + office_cor = StringField("COR Office", validators=[DataRequired()]) class FinancialForm(BaseFinancialForm): - 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): return "task_order_number" in self.errors @@ -159,13 +91,13 @@ class FinancialForm(BaseFinancialForm): class ExtendedFinancialForm(BaseFinancialForm): def validate(self, *args, **kwargs): if self.funding_type.data == "OTHER": - self.funding_type_other.validators.append(InputRequired()) + self.funding_type_other.validators.append(DataRequired()) return super().validate(*args, **kwargs) funding_type = SelectField( description="What is the source of funding?", choices=FUNDING_TYPES, - validators=[InputRequired()], + validators=[DataRequired()], render_kw={"required": False}, ) @@ -175,7 +107,7 @@ class ExtendedFinancialForm(BaseFinancialForm): "Task Order Expiration Date", description="Please enter the expiration date for the Task Order", validators=[ - InputRequired(), + DataRequired(), DateRange( lower_bound=pendulum.duration(days=0), upper_bound=pendulum.duration(years=100), @@ -187,42 +119,42 @@ class ExtendedFinancialForm(BaseFinancialForm): clin_0001 = NumberStringField( "
CLIN 0001
-
Unclassified IaaS and PaaS Amount
", - validators=[InputRequired()], + validators=[DataRequired()], description="Review your task order document, the amounts for each CLIN must match exactly here", filters=[number_to_int], ) clin_0003 = NumberStringField( "
CLIN 0003
-
Unclassified Cloud Support Package
", - validators=[InputRequired()], + validators=[DataRequired()], description="Review your task order document, the amounts for each CLIN must match exactly here", filters=[number_to_int], ) clin_1001 = NumberStringField( "
CLIN 1001
-
Unclassified IaaS and PaaS Amount
OPTION PERIOD 1
", - validators=[InputRequired()], + validators=[DataRequired()], description="Review your task order document, the amounts for each CLIN must match exactly here", filters=[number_to_int], ) clin_1003 = NumberStringField( "
CLIN 1003
-
Unclassified Cloud Support Package
OPTION PERIOD 1
", - validators=[InputRequired()], + validators=[DataRequired()], description="Review your task order document, the amounts for each CLIN must match exactly here", filters=[number_to_int], ) clin_2001 = NumberStringField( "
CLIN 2001
-
Unclassified IaaS and PaaS Amount
OPTION PERIOD 2
", - validators=[InputRequired()], + validators=[DataRequired()], description="Review your task order document, the amounts for each CLIN must match exactly here", filters=[number_to_int], ) clin_2003 = NumberStringField( "
CLIN 2003
-
Unclassified Cloud Support Package
OPTION PERIOD 2
", - validators=[InputRequired()], + validators=[DataRequired()], description="Review your task order document, the amounts for each CLIN must match exactly here", filters=[number_to_int], ) @@ -231,6 +163,6 @@ class ExtendedFinancialForm(BaseFinancialForm): "Upload a copy of your Task Order", validators=[ FileAllowed(["pdf"], "Only PDF documents can be uploaded."), - InputRequired(), + DataRequired(), ], ) diff --git a/atst/models/request.py b/atst/models/request.py index 36a6b78e..1e1abcac 100644 --- a/atst/models/request.py +++ b/atst/models/request.py @@ -224,6 +224,10 @@ class Request(Base, mixins.TimestampsMixin, mixins.AuditableMixin): def contracting_officer_email(self): return self.latest_revision.email_co + @property + def pe_number(self): + return self.body.get("financial_verification", {}).get("pe_id") + def __repr__(self): return "".format( self.status_displayname, diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 5b84392e..c01bbc6d 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -4,89 +4,87 @@ from flask import request as http_request from . import requests_bp from atst.domain.requests import Requests from atst.forms.financial import FinancialForm, ExtendedFinancialForm +from atst.forms.exceptions import FormValidationError +from atst.domain.requests.financial_verification import ( + PENumberValidator, + TaskOrderNumberValidator, +) -class FinancialVerification: - def __init__(self, request, extended=False, post_data=None): +class UpdateFinancialVerification(object): + def __init__( + self, + pe_validator, + task_order_validator, + user, + request, + fv_data, + is_extended=False, + ): + self.pe_validator = pe_validator + self.task_order_validator = task_order_validator + self.user = user self.request = request - self._extended = extended - self._post_data = post_data - self._form = None - self.reset() + self.fv_data = fv_data + self.is_extended = is_extended - def reset(self): - self._updateable = False - self._valid = False - self.workspace = None - if self._form: - self._form.reset() - - @property - def is_extended(self): - return self._extended or self.is_pending_changes - - @property - def is_pending_changes(self): - return self.request.is_pending_financial_verification_changes - - @property - def _task_order_data(self): - if self.request.task_order: - task_order = self.request.task_order - data = task_order.to_dictionary() - data["task_order_number"] = task_order.number - data["funding_type"] = task_order.funding_type.value - return data + def _get_form(self): + if self.is_extended: + return ExtendedFinancialForm(data=self.fv_data) else: - return {} + return FinancialForm(data=self.fv_data) - @property - def _form_data(self): - if self._post_data: - return self._post_data - else: - form_data = self.request.body.get("financial_verification", {}) - form_data.update(self._task_order_data) + def execute(self): + form = self._get_form() - return form_data + should_update = True + should_submit = True + updated_request = None + submitted = False + workspace = None - @property - def form(self): - if not self._form: - if self.is_extended: - self._form = ExtendedFinancialForm(data=self._form_data) - else: - self._form = FinancialForm(data=self._form_data) + if not form.validate(): + should_update = False - return self._form + if not self.pe_validator.validate(self.request, form.pe_id): + suggestion = self.pe_validator.suggest_pe_id(form.pe_id.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 "") + form.pe_id.errors += (error_str,) + should_submit = False - def validate(self): - if self.form.validate(): - self._updateable = True - self._valid = self.form.perform_extra_validation( - self.request.body.get("financial_verification") + if not self.task_order_validator.validate(form.task_order_number): + form.task_order_number.errors += ("Task Order number not found",) + should_submit = False + + if should_update: + updated_request = Requests.update_financial_verification( + self.request.id, form.data ) else: - self._updateable = False - self._valid = False + form.reset() + raise FormValidationError(form) - return self._valid + if should_submit: + updated_request = Requests.submit_financial_verification(updated_request) + if updated_request.is_financially_verified: + workspace = Requests.approve_and_create_workspace(updated_request) + submitted = True + else: + form.reset() + raise FormValidationError(form) - @property - def pending(self): - return self.request.is_pending_ccpo_approval - - def finalize(self): - if self._updateable: - self.request = Requests.update_financial_verification( - self.request.id, self.form.data - ) - - if self._valid: - self.request = Requests.submit_financial_verification(self.request) - - if self.request.is_financially_verified: - self.workspace = Requests.approve_and_create_workspace(self.request) + if submitted: + return { + "state": "submitted", + "workspace": workspace, + "request": updated_request, + } + else: + return {"state": "pending", "request": updated_request} @requests_bp.route("/requests/verify/", methods=["GET"]) @@ -106,29 +104,32 @@ def financial_verification(request_id): @requests_bp.route("/requests/verify/", methods=["POST"]) def update_financial_verification(request_id): request = Requests.get(g.current_user, request_id) - finver = FinancialVerification( - request, extended=http_request.args.get("extended"), post_data=http_request.form - ) + fv_data = http_request.form + is_extended = http_request.args.get("extended") - finver.validate() - - finver.finalize() - - if finver.workspace: - return redirect( - url_for( - "workspaces.new_project", - workspace_id=finver.workspace.id, - newWorkspace=True, - ) - ) - elif finver.pending: - return redirect(url_for("requests.requests_index", modal="pendingCCPOApproval")) - else: - finver.reset() + try: + response_context = UpdateFinancialVerification( + PENumberValidator(), + TaskOrderNumberValidator(), + g.current_user, + request, + fv_data, + is_extended=is_extended, + ).execute() + except FormValidationError as e: return render_template( "requests/financial_verification.html", - jedi_request=finver.request, - f=finver.form, - extended=finver.is_extended, + jedi_request=request, + f=e.form, + extended=is_extended, ) + + if response_context["state"] == "submitted": + workspace = response_context["workspace"] + return redirect( + url_for( + "workspaces.new_project", workspace_id=workspace.id, newWorkspace=True + ) + ) + elif response_context["state"] == "pending": + return redirect(url_for("requests.requests_index", modal="pendingCCPOApproval")) diff --git a/tests/forms/test_financial.py b/tests/forms/test_financial.py index 5a4c80dd..925aec2a 100644 --- a/tests/forms/test_financial.py +++ b/tests/forms/test_financial.py @@ -1,22 +1,22 @@ import pytest from werkzeug.datastructures import ImmutableMultiDict -from atst.forms.financial import suggest_pe_id, FinancialForm, ExtendedFinancialForm +from atst.forms.financial import FinancialForm, ExtendedFinancialForm from atst.eda_client import MockEDAClient -@pytest.mark.parametrize( - "input_,expected", - [ - ("0603502N", None), - ("0603502NZ", None), - ("603502N", "0603502N"), - ("063502N", "0603502N"), - ("63502N", "0603502N"), - ], -) -def test_suggest_pe_id(input_, expected): - assert suggest_pe_id(input_) == expected +# @pytest.mark.parametrize( +# "input_,expected", +# [ +# ("0603502N", None), +# ("0603502NZ", None), +# ("603502N", "0603502N"), +# ("063502N", "0603502N"), +# ("63502N", "0603502N"), +# ], +# ) +# def test_suggest_pe_id(input_, expected): +# assert suggest_pe_id(input_) == expected def test_funding_type_other_not_required_if_funding_type_is_not_other(): @@ -82,25 +82,6 @@ def test_ba_code_validation(input_, expected): assert is_valid == 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.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.perform_extra_validation({}) - - assert "task_order_number" not in form_valid.errors - - def test_can_submit_zero_for_clin(): form_first = ExtendedFinancialForm() form_first.validate() diff --git a/tests/mocks.py b/tests/mocks.py index e61d80c1..b42521dc 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -6,7 +6,7 @@ MOCK_REQUEST = RequestFactory.build(creator=MOCK_USER) DOD_SDN_INFO = {"first_name": "ART", "last_name": "GARFUNKEL", "dod_id": "5892460358"} DOD_SDN = f"CN={DOD_SDN_INFO['last_name']}.{DOD_SDN_INFO['first_name']}.G.{DOD_SDN_INFO['dod_id']},OU=OTHER,OU=PKI,OU=DoD,O=U.S. Government,C=US" -MOCK_VALID_PE_ID = "8675309U" +MOCK_VALID_PE_ID = "080675309U" FIXTURE_EMAIL_ADDRESS = "artgarfunkel@uso.mil" diff --git a/tests/routes/test_financial_verification.py b/tests/routes/test_financial_verification.py index 91630abe..482b4eb8 100644 --- a/tests/routes/test_financial_verification.py +++ b/tests/routes/test_financial_verification.py @@ -1,12 +1,9 @@ -import urllib import pytest -from flask import url_for from atst.eda_client import MockEDAClient -from atst.models.request_status_event import RequestStatus -from atst.routes.requests.financial_verification import FinancialVerification +from atst.routes.requests.financial_verification import UpdateFinancialVerification -from tests.mocks import MOCK_REQUEST, MOCK_USER +from tests.mocks import MOCK_REQUEST, MOCK_USER, MOCK_VALID_PE_ID from tests.factories import ( PENumberFactory, RequestFactory, @@ -14,206 +11,120 @@ from tests.factories import ( RequestStatusEventFactory, RequestReviewFactory, ) +from atst.forms.exceptions import FormValidationError +from atst.domain.requests.financial_verification import ( + PENumberValidator, + TaskOrderNumberValidator, +) + +required_data = { + "pe_id": "123", + "task_order_number": MockEDAClient.MOCK_CONTRACT_NUMBER, + "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": "02A", +} -class TestPENumberInForm: - - required_data = { - "pe_id": "123", - "task_order_number": MockEDAClient.MOCK_CONTRACT_NUMBER, - "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": "02A", - } - - def _set_monkeypatches(self, monkeypatch): - monkeypatch.setattr( - "atst.forms.financial.FinancialForm.validate", lambda s: True - ) - user = UserFactory.create() - monkeypatch.setattr("atst.domain.auth.get_current_user", lambda *args: user) - return user - - def submit_data(self, client, user, data, extended=False): - request = RequestFactory.create(creator=user) - url_kwargs = {"request_id": request.id} - if extended: - url_kwargs["extended"] = True - response = client.post( - url_for("requests.financial_verification", **url_kwargs), - data=data, - follow_redirects=False, - ) - return response - - def test_submit_request_form_with_invalid_pe_id(self, monkeypatch, client): - user = self._set_monkeypatches(monkeypatch) - - response = self.submit_data(client, user, self.required_data) - - assert "We couldn't find that PE number" in response.data.decode() - assert response.status_code == 200 - - def test_submit_request_form_with_unchanged_pe_id(self, monkeypatch, client): - user = self._set_monkeypatches(monkeypatch) - - data = dict(self.required_data) - data["pe_id"] = "0101110F" - - response = self.submit_data(client, user, data) - - assert response.status_code == 302 - assert "/workspaces" in response.headers.get("Location") - - def test_submit_request_form_with_new_valid_pe_id(self, monkeypatch, client): - user = self._set_monkeypatches(monkeypatch) - pe = PENumberFactory.create(number="8675309U", description="sample PE number") - - data = dict(self.required_data) - data["pe_id"] = pe.number - - response = self.submit_data(client, user, data) - - assert response.status_code == 302 - assert "/workspaces" in response.headers.get("Location") - - def test_submit_request_form_with_missing_pe_id(self, monkeypatch, client): - user = self._set_monkeypatches(monkeypatch) - - data = dict(self.required_data) - data["pe_id"] = "" - - response = self.submit_data(client, user, data) - - assert "There were some errors" in response.data.decode() - assert response.status_code == 200 - - def test_submit_financial_form_with_invalid_task_order( - self, monkeypatch, user_session, client - ): - user = UserFactory.create() - user_session(user) - - data = dict(self.required_data) - data["pe_id"] = "0101110F" - data["task_order_number"] = "1234" - - response = self.submit_data(client, user, data) - - assert "extended=True" in response.data.decode() - - def test_submit_financial_form_with_valid_task_order( - self, monkeypatch, user_session, client - ): - user = UserFactory.create() - monkeypatch.setattr( - "atst.domain.requests.Requests.get", lambda *args: MOCK_REQUEST - ) - user_session(user) - - data = dict(self.required_data) - data["pe_id"] = "0101110F" - data["task_order_number"] = MockEDAClient.MOCK_CONTRACT_NUMBER - - response = self.submit_data(client, user, data) - - assert "enter TO information manually" not in response.data.decode() - - def test_submit_extended_financial_form( - self, monkeypatch, user_session, client, extended_financial_verification_data - ): - user = UserFactory.create() - request = RequestFactory.create(creator=user) - monkeypatch.setattr("atst.domain.requests.Requests.get", lambda *args: request) - monkeypatch.setattr("atst.forms.financial.validate_pe_id", lambda *args: True) - user_session(user) - data = {**self.required_data, **extended_financial_verification_data} - data["task_order_number"] = "1234567" - - response = self.submit_data(client, user, data, extended=True) - - assert response.status_code == 302 - assert "/requests" in response.headers.get("Location") - - def test_submit_invalid_extended_financial_form( - self, monkeypatch, user_session, client, extended_financial_verification_data - ): - monkeypatch.setattr("atst.forms.financial.validate_pe_id", lambda *args: True) - user = UserFactory.create() - user_session(user) - data = {**self.required_data, **extended_financial_verification_data} - data["task_order_number"] = "1234567" - del (data["clin_0001"]) - - response = self.submit_data(client, user, data, extended=True) - - assert response.status_code == 200 +class MockPEValidator(object): + def validate(self, request, field): + return True -def test_displays_ccpo_review_comment(user_session, client): - creator = UserFactory.create() - ccpo = UserFactory.from_atat_role("ccpo") - user_session(creator) - request = RequestFactory.create(creator=creator) - status = RequestStatusEventFactory.create( - revision=request.latest_revision, - new_status=RequestStatus.CHANGES_REQUESTED_TO_FINVER, - request=request, +class MockTaskOrderValidator(object): + def validate(self, field): + return True + + +def test_update(): + request = RequestFactory.create() + user = UserFactory.create() + data = {**required_data, "pe_id": MOCK_VALID_PE_ID} + + response_context = UpdateFinancialVerification( + MockPEValidator(), + MockTaskOrderValidator(), + user, + request, + data, + is_extended=False, + ).execute() + + assert response_context.get("workspace") + + +def test_re_enter_pe_number(): + request = RequestFactory.create() + user = UserFactory.create() + data = {**required_data, "pe_id": "0101228M"} + update_fv = UpdateFinancialVerification( + PENumberValidator(), + MockTaskOrderValidator(), + user, + request, + data, + is_extended=False, ) - review_comment = "add all of the correct info, instead of the incorrect info" - RequestReviewFactory.create(reviewer=ccpo, comment=review_comment, status=status) - response = client.get("/requests/verify/{}".format(request.id)) - body = response.data.decode() - assert review_comment in body + + with pytest.raises(FormValidationError): + update_fv.execute() + response_context = update_fv.execute() + + assert response_context.get("status", "submitted") -class TestFinancialVerification: - def _service_object(self, request=None, extended=False, post_data={}): - if not request: - self.request = RequestFactory.create() - else: - self.request = request +def test_invalid_task_order_number(): + request = RequestFactory.create() + user = UserFactory.create() + data = {**required_data, "task_order_number": "DCA10096D0051"} + update_fv = UpdateFinancialVerification( + MockPEValidator(), + TaskOrderNumberValidator(), + user, + request, + data, + is_extended=False, + ) - return FinancialVerification( - self.request, extended=extended, post_data=post_data - ) + with pytest.raises(FormValidationError): + update_fv.execute() - def test_is_extended(self): - finver_one = self._service_object() - assert not finver_one.is_extended - finver_two = self._service_object( - request=RequestFactory.create_with_status( - RequestStatus.CHANGES_REQUESTED_TO_FINVER - ) - ) - assert finver_two.is_extended - finver_three = self._service_object(extended=True) - assert finver_three.is_extended - def test_is_pending_changes(self): - finver_one = self._service_object() - assert not finver_one.is_pending_changes - finver_two = self._service_object( - request=RequestFactory.create_with_status( - RequestStatus.CHANGES_REQUESTED_TO_FINVER - ) - ) - assert finver_two.is_pending_changes +def test_extended_fv_data(extended_financial_verification_data): + request = RequestFactory.create() + user = UserFactory.create() + data = {**required_data, **extended_financial_verification_data} + update_fv = UpdateFinancialVerification( + MockPEValidator(), + TaskOrderNumberValidator(), + user, + request, + data, + is_extended=True, + ) - def test_pending(self): - finver_one = self._service_object() - assert not finver_one.pending - finver_two = self._service_object( - request=RequestFactory.create_with_status( - RequestStatus.PENDING_CCPO_APPROVAL - ) - ) - assert finver_two.pending + assert update_fv.execute() + + +def test_missing_extended_fv_data(): + request = RequestFactory.create() + user = UserFactory.create() + update_fv = UpdateFinancialVerification( + MockPEValidator(), + TaskOrderNumberValidator(), + user, + request, + required_data, + is_extended=True, + ) + + with pytest.raises(FormValidationError): + update_fv.execute() From e80ae4c092520a15556204d2308a132c86c25cb7 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 15 Oct 2018 14:49:21 -0400 Subject: [PATCH 02/47] Add GetFinancialVerificationForm --- .../routes/requests/financial_verification.py | 51 ++++++++++++++++--- 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index c01bbc6d..f4486e7a 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -11,6 +11,32 @@ from atst.domain.requests.financial_verification import ( ) +class GetFinancialVerificationForm(object): + def __init__(self, user, request, is_extended=False): + self.user = user + self.request = request + self.is_extended = is_extended + + def _get_form(self): + data = {} + if self.request.task_order: + task_order_dict = self.request.task_order.to_dictionary() + task_order_dict.update({ + "task_order_number": self.request.task_order.number, + "funding_type": self.request.task_order.funding_type.value + }) + data = {**data, **task_order_dict} + + if self.is_extended: + return ExtendedFinancialForm(data=data) + else: + return FinancialForm(data=data) + + def execute(self): + form = self._get_form() + return {"form": form} + + class UpdateFinancialVerification(object): def __init__( self, @@ -29,10 +55,19 @@ class UpdateFinancialVerification(object): self.is_extended = is_extended def _get_form(self): + data = self.fv_data + if self.request.task_order: + task_order_dict = self.request.task_order.to_dictionary() + task_order_dict.update({ + "task_order_number": self.request.task_order.number, + "funding_type": self.request.task_order.funding_type.value + }) + data = {**data, **task_order_dict} + if self.is_extended: - return ExtendedFinancialForm(data=self.fv_data) + return ExtendedFinancialForm(data=data) else: - return FinancialForm(data=self.fv_data) + return FinancialForm(data=data) def execute(self): form = self._get_form() @@ -90,14 +125,16 @@ class UpdateFinancialVerification(object): @requests_bp.route("/requests/verify/", methods=["GET"]) def financial_verification(request_id): request = Requests.get(g.current_user, request_id) - finver = FinancialVerification(request, extended=http_request.args.get("extended")) + is_extended = http_request.args.get("extended") + + response_context = GetFinancialVerificationForm(g.current_user, request, is_extended=is_extended).execute() return render_template( "requests/financial_verification.html", - f=finver.form, - jedi_request=finver.request, - review_comment=finver.request.review_comment, - extended=finver.is_extended, + f=response_context["form"], + jedi_request=request, + review_comment=request.review_comment, + extended=is_extended, ) From ed7d508e305af30af08aed5f0170e0fa3d0c96d4 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 15 Oct 2018 15:16:20 -0400 Subject: [PATCH 03/47] Add funding type for task order --- atst/domain/task_orders.py | 4 ++-- atst/routes/requests/financial_verification.py | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/atst/domain/task_orders.py b/atst/domain/task_orders.py index b8653af5..b214293b 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, Source +from atst.models.task_order import TaskOrder, Source, FundingType from atst.models.attachment import Attachment from .exceptions import NotFoundError @@ -29,7 +29,7 @@ class TaskOrders(object): to_data = TaskOrders._client().get_contract(order_number, status="y") if to_data: # TODO: we need to determine exactly what we're getting and storing from the EDA client - return TaskOrders.create(source=Source.EDA, **to_data) + return TaskOrders.create(number=to_data["contract_no"], source=Source.EDA, funding_type=FundingType.PROC) else: raise NotFoundError("task_order") diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index f4486e7a..2ec1b4da 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -19,6 +19,11 @@ class GetFinancialVerificationForm(object): def _get_form(self): data = {} + + fv_data = self.request.body.get("financial_verification") + if fv_data: + data = {**data, **fv_data} + if self.request.task_order: task_order_dict = self.request.task_order.to_dictionary() task_order_dict.update({ From 4b8585234ff8ffc4df8202b654e8ecc74a7d0799 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 15 Oct 2018 16:22:46 -0400 Subject: [PATCH 04/47] Use formdata and InputRequired --- atst/forms/financial.py | 46 +++++++++---------- .../routes/requests/financial_verification.py | 12 ++++- 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/atst/forms/financial.py b/atst/forms/financial.py index 69671dca..d9d953de 100644 --- a/atst/forms/financial.py +++ b/atst/forms/financial.py @@ -2,7 +2,7 @@ import re import pendulum from wtforms.fields.html5 import DateField, EmailField from wtforms.fields import StringField, FileField -from wtforms.validators import DataRequired, Email, Regexp +from wtforms.validators import InputRequired, Email, Regexp from flask_wtf.file import FileAllowed from .fields import NewlineListField, SelectField, NumberStringField @@ -36,7 +36,7 @@ class BaseFinancialForm(ValidatedForm): task_order_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.", - validators=[DataRequired()], + validators=[InputRequired()], ) uii_ids = NewlineListField( @@ -47,35 +47,35 @@ class BaseFinancialForm(ValidatedForm): pe_id = StringField( "Program Element Number", description="PE numbers help the Department of Defense identify which offices' budgets are contributing towards this resource use.
It should be 7 digits followed by 1-3 letters, and should have a zero as the first and third digits.", - validators=[DataRequired()], + validators=[InputRequired()], ) treasury_code = StringField( "Program Treasury Code", description="Program Treasury Code (or Appropriations Code) identifies resource types.
It should be a four digit or six digit number, optionally prefixed by one or more zeros.", - validators=[DataRequired(), Regexp(TREASURY_CODE_REGEX)], + validators=[InputRequired(), Regexp(TREASURY_CODE_REGEX)], ) ba_code = StringField( "Program Budget Activity (BA) Code", description="BA Code is used to identify the purposes, projects, or types of activities financed by the appropriation fund.
It should be two digits, followed by an optional letter.", - validators=[DataRequired(), Regexp(BA_CODE_REGEX)], + validators=[InputRequired(), Regexp(BA_CODE_REGEX)], ) - fname_co = StringField("KO First Name", validators=[DataRequired()]) - lname_co = StringField("KO Last Name", validators=[DataRequired()]) + fname_co = StringField("KO First Name", validators=[InputRequired()]) + lname_co = StringField("KO Last Name", validators=[InputRequired()]) - email_co = EmailField("KO Email", validators=[DataRequired(), Email()]) + email_co = EmailField("KO Email", validators=[InputRequired(), Email()]) - office_co = StringField("KO Office", validators=[DataRequired()]) + office_co = StringField("KO Office", validators=[InputRequired()]) - fname_cor = StringField("COR First Name", validators=[DataRequired()]) + fname_cor = StringField("COR First Name", validators=[InputRequired()]) - lname_cor = StringField("COR Last Name", validators=[DataRequired()]) + lname_cor = StringField("COR Last Name", validators=[InputRequired()]) - email_cor = EmailField("COR Email", validators=[DataRequired(), Email()]) + email_cor = EmailField("COR Email", validators=[InputRequired(), Email()]) - office_cor = StringField("COR Office", validators=[DataRequired()]) + office_cor = StringField("COR Office", validators=[InputRequired()]) class FinancialForm(BaseFinancialForm): @@ -91,13 +91,13 @@ class FinancialForm(BaseFinancialForm): class ExtendedFinancialForm(BaseFinancialForm): def validate(self, *args, **kwargs): if self.funding_type.data == "OTHER": - self.funding_type_other.validators.append(DataRequired()) + self.funding_type_other.validators.append(InputRequired()) return super().validate(*args, **kwargs) funding_type = SelectField( description="What is the source of funding?", choices=FUNDING_TYPES, - validators=[DataRequired()], + validators=[InputRequired()], render_kw={"required": False}, ) @@ -107,7 +107,7 @@ class ExtendedFinancialForm(BaseFinancialForm): "Task Order Expiration Date", description="Please enter the expiration date for the Task Order", validators=[ - DataRequired(), + InputRequired(), DateRange( lower_bound=pendulum.duration(days=0), upper_bound=pendulum.duration(years=100), @@ -119,42 +119,42 @@ class ExtendedFinancialForm(BaseFinancialForm): clin_0001 = NumberStringField( "
CLIN 0001
-
Unclassified IaaS and PaaS Amount
", - validators=[DataRequired()], + validators=[InputRequired()], description="Review your task order document, the amounts for each CLIN must match exactly here", filters=[number_to_int], ) clin_0003 = NumberStringField( "
CLIN 0003
-
Unclassified Cloud Support Package
", - validators=[DataRequired()], + validators=[InputRequired()], description="Review your task order document, the amounts for each CLIN must match exactly here", filters=[number_to_int], ) clin_1001 = NumberStringField( "
CLIN 1001
-
Unclassified IaaS and PaaS Amount
OPTION PERIOD 1
", - validators=[DataRequired()], + validators=[InputRequired()], description="Review your task order document, the amounts for each CLIN must match exactly here", filters=[number_to_int], ) clin_1003 = NumberStringField( "
CLIN 1003
-
Unclassified Cloud Support Package
OPTION PERIOD 1
", - validators=[DataRequired()], + validators=[InputRequired()], description="Review your task order document, the amounts for each CLIN must match exactly here", filters=[number_to_int], ) clin_2001 = NumberStringField( "
CLIN 2001
-
Unclassified IaaS and PaaS Amount
OPTION PERIOD 2
", - validators=[DataRequired()], + validators=[InputRequired()], description="Review your task order document, the amounts for each CLIN must match exactly here", filters=[number_to_int], ) clin_2003 = NumberStringField( "
CLIN 2003
-
Unclassified Cloud Support Package
OPTION PERIOD 2
", - validators=[DataRequired()], + validators=[InputRequired()], description="Review your task order document, the amounts for each CLIN must match exactly here", filters=[number_to_int], ) @@ -163,6 +163,6 @@ class ExtendedFinancialForm(BaseFinancialForm): "Upload a copy of your Task Order", validators=[ FileAllowed(["pdf"], "Only PDF documents can be uploaded."), - DataRequired(), + InputRequired(), ], ) diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 2ec1b4da..c79dcecc 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -1,5 +1,6 @@ from flask import g, render_template, redirect, url_for from flask import request as http_request +from werkzeug.datastructures import ImmutableMultiDict from . import requests_bp from atst.domain.requests import Requests @@ -61,6 +62,10 @@ class UpdateFinancialVerification(object): def _get_form(self): data = self.fv_data + + existing_fv_data = self.request.body.get("financial_verification", {}) + data = {**data, **existing_fv_data} + if self.request.task_order: task_order_dict = self.request.task_order.to_dictionary() task_order_dict.update({ @@ -69,10 +74,11 @@ class UpdateFinancialVerification(object): }) data = {**data, **task_order_dict} + mdict = ImmutableMultiDict(data) if self.is_extended: - return ExtendedFinancialForm(data=data) + return ExtendedFinancialForm(formdata=mdict) else: - return FinancialForm(data=data) + return FinancialForm(formdata=mdict) def execute(self): form = self._get_form() @@ -149,6 +155,8 @@ def update_financial_verification(request_id): fv_data = http_request.form is_extended = http_request.args.get("extended") + import ipdb; ipdb.set_trace() + try: response_context = UpdateFinancialVerification( PENumberValidator(), From 965c5f3e0010866a2dbf9c2620f63d20270c5473 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 15 Oct 2018 16:39:49 -0400 Subject: [PATCH 05/47] Pass field data, not field --- atst/domain/requests/financial_verification.py | 14 +++++++------- atst/routes/requests/financial_verification.py | 6 ++---- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/atst/domain/requests/financial_verification.py b/atst/domain/requests/financial_verification.py index cf3d5896..3197c53e 100644 --- a/atst/domain/requests/financial_verification.py +++ b/atst/domain/requests/financial_verification.py @@ -17,12 +17,12 @@ class PENumberValidator(object): re.X, ) - def validate(self, request, field): - if self._same_as_previous(request, field): + def validate(self, request, pe_id): + if self._same_as_previous(request, pe_id): return True try: - PENumbers.get(field.data) + PENumbers.get(pe_id) except NotFoundError: return False @@ -43,14 +43,14 @@ class PENumberValidator(object): return suggestion return None - def _same_as_previous(self, request, field): - return request.pe_number == field.data + def _same_as_previous(self, request, pe_id): + return request.pe_number == pe_id class TaskOrderNumberValidator(object): - def validate(self, field): + def validate(self, task_order_number): try: - TaskOrders.get(field.data) + TaskOrders.get(task_order_number) except NotFoundError: return False diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index c79dcecc..d2c6242e 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -92,7 +92,7 @@ class UpdateFinancialVerification(object): if not form.validate(): should_update = False - if not self.pe_validator.validate(self.request, form.pe_id): + if not self.pe_validator.validate(self.request, form.pe_id.data): suggestion = self.pe_validator.suggest_pe_id(form.pe_id.data) error_str = ( "We couldn't find that PE number. {}" @@ -102,7 +102,7 @@ class UpdateFinancialVerification(object): form.pe_id.errors += (error_str,) should_submit = False - if not self.task_order_validator.validate(form.task_order_number): + if not self.task_order_validator.validate(form.task_order_number.data): form.task_order_number.errors += ("Task Order number not found",) should_submit = False @@ -155,8 +155,6 @@ def update_financial_verification(request_id): fv_data = http_request.form is_extended = http_request.args.get("extended") - import ipdb; ipdb.set_trace() - try: response_context = UpdateFinancialVerification( PENumberValidator(), From 54192c6c5d9428bbf6b0ab07e70a19c0c3da616c Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 15 Oct 2018 16:57:43 -0400 Subject: [PATCH 06/47] Implement SaveFinancialVerificationDraft --- atst/forms/financial.py | 7 +- .../routes/requests/financial_verification.py | 93 ++++++++++++++ tests/routes/test_financial_verification.py | 120 +++++++++++++----- 3 files changed, 187 insertions(+), 33 deletions(-) diff --git a/atst/forms/financial.py b/atst/forms/financial.py index d9d953de..61d334cd 100644 --- a/atst/forms/financial.py +++ b/atst/forms/financial.py @@ -2,7 +2,7 @@ import re import pendulum from wtforms.fields.html5 import DateField, EmailField from wtforms.fields import StringField, FileField -from wtforms.validators import InputRequired, Email, Regexp +from wtforms.validators import InputRequired, Email, Regexp, Optional from flask_wtf.file import FileAllowed from .fields import NewlineListField, SelectField, NumberStringField @@ -29,6 +29,11 @@ class BaseFinancialForm(ValidatedForm): """ self.uii_ids.process_data(self.uii_ids.data) + def validate_draft(self): + for field in self: + field.validators.insert(0, Optional()) + return self.validate() + @property def is_missing_task_order_number(self): return False diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index d2c6242e..47b7be2a 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -133,6 +133,73 @@ class UpdateFinancialVerification(object): return {"state": "pending", "request": updated_request} +class SaveFinancialVerificationDraft(object): + def __init__( + self, + pe_validator, + task_order_validator, + user, + request, + fv_data, + is_extended=False, + ): + self.pe_validator = pe_validator + self.task_order_validator = task_order_validator + self.user = user + self.request = request + self.fv_data = fv_data + self.is_extended = is_extended + + def _get_form(self): + data = self.fv_data + + existing_fv_data = self.request.body.get("financial_verification", {}) + data = {**data, **existing_fv_data} + + if self.request.task_order: + task_order_dict = self.request.task_order.to_dictionary() + task_order_dict.update({ + "task_order_number": self.request.task_order.number, + "funding_type": self.request.task_order.funding_type.value + }) + data = {**data, **task_order_dict} + + mdict = ImmutableMultiDict(data) + if self.is_extended: + return ExtendedFinancialForm(formdata=mdict) + else: + return FinancialForm(formdata=mdict) + + def execute(self): + form = self._get_form() + valid = True + + if not form.validate_draft(): + valid = False + + if valid and form.pe_id.data and not self.pe_validator.validate(self.request, form.pe_id.data): + suggestion = self.pe_validator.suggest_pe_id(form.pe_id.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 "") + form.pe_id.errors += (error_str,) + valid = False + + if valid and form.task_order_number.data and not self.task_order_validator.validate(form.task_order_number.data): + form.task_order_number.errors += ("Task Order number not found",) + valid = False + + if not valid: + form.reset() + raise FormValidationError(form) + else: + updated_request = Requests.update_financial_verification(self.request.id, form.data) + return {"request": updated_request} + + + @requests_bp.route("/requests/verify/", methods=["GET"]) def financial_verification(request_id): request = Requests.get(g.current_user, request_id) @@ -181,3 +248,29 @@ def update_financial_verification(request_id): ) elif response_context["state"] == "pending": return redirect(url_for("requests.requests_index", modal="pendingCCPOApproval")) + + +@requests_bp.route("/requests/verify//draft", methods=["POST"]) +def save_financial_verification_draft(request_id): + request = Requests.get(g.current_user, request_id) + fv_data = http_request.form + is_extended = http_request.args.get("extended") + + try: + response_context = SaveFinancialVerificationDraft( + PENumberValidator(), + TaskOrderNumberValidator(), + g.current_user, + request, + fv_data, + is_extended=is_extended, + ).execute() + except FormValidationError as e: + return render_template( + "requests/financial_verification.html", + jedi_request=request, + f=e.form, + extended=is_extended, + ) + + return redirect(url_for("requests.requests_index")) diff --git a/tests/routes/test_financial_verification.py b/tests/routes/test_financial_verification.py index 482b4eb8..a800869c 100644 --- a/tests/routes/test_financial_verification.py +++ b/tests/routes/test_financial_verification.py @@ -1,7 +1,8 @@ import pytest +from unittest.mock import MagicMock from atst.eda_client import MockEDAClient -from atst.routes.requests.financial_verification import UpdateFinancialVerification +from atst.routes.requests.financial_verification import UpdateFinancialVerification, SaveFinancialVerificationDraft from tests.mocks import MOCK_REQUEST, MOCK_USER, MOCK_VALID_PE_ID from tests.factories import ( @@ -17,23 +18,31 @@ from atst.domain.requests.financial_verification import ( TaskOrderNumberValidator, ) -required_data = { - "pe_id": "123", - "task_order_number": MockEDAClient.MOCK_CONTRACT_NUMBER, - "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": "02A", -} +@pytest.fixture +def fv_data(): + return { + "pe_id": "123", + "task_order_number": MockEDAClient.MOCK_CONTRACT_NUMBER, + "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": "02A", + } +TrueValidator = MagicMock() +TrueValidator.validate = MagicMock(return_value=True) + +FalseValidator = MagicMock() +FalseValidator.validate = MagicMock(return_value=False) + class MockPEValidator(object): def validate(self, request, field): return True @@ -44,14 +53,14 @@ class MockTaskOrderValidator(object): return True -def test_update(): +def test_update(fv_data): request = RequestFactory.create() user = UserFactory.create() - data = {**required_data, "pe_id": MOCK_VALID_PE_ID} + data = {**fv_data, "pe_id": MOCK_VALID_PE_ID} response_context = UpdateFinancialVerification( - MockPEValidator(), - MockTaskOrderValidator(), + TrueValidator, + TrueValidator, user, request, data, @@ -61,13 +70,13 @@ def test_update(): assert response_context.get("workspace") -def test_re_enter_pe_number(): +def test_re_enter_pe_number(fv_data): request = RequestFactory.create() user = UserFactory.create() - data = {**required_data, "pe_id": "0101228M"} + data = {**fv_data, "pe_id": "0101228M"} update_fv = UpdateFinancialVerification( PENumberValidator(), - MockTaskOrderValidator(), + TrueValidator, user, request, data, @@ -81,12 +90,12 @@ def test_re_enter_pe_number(): assert response_context.get("status", "submitted") -def test_invalid_task_order_number(): +def test_invalid_task_order_number(fv_data): request = RequestFactory.create() user = UserFactory.create() - data = {**required_data, "task_order_number": "DCA10096D0051"} + data = {**fv_data, "task_order_number": "DCA10096D0051"} update_fv = UpdateFinancialVerification( - MockPEValidator(), + TrueValidator, TaskOrderNumberValidator(), user, request, @@ -98,12 +107,12 @@ def test_invalid_task_order_number(): update_fv.execute() -def test_extended_fv_data(extended_financial_verification_data): +def test_extended_fv_data(fv_data, extended_financial_verification_data): request = RequestFactory.create() user = UserFactory.create() - data = {**required_data, **extended_financial_verification_data} + data = {**fv_data, **extended_financial_verification_data} update_fv = UpdateFinancialVerification( - MockPEValidator(), + TrueValidator, TaskOrderNumberValidator(), user, request, @@ -114,17 +123,64 @@ def test_extended_fv_data(extended_financial_verification_data): assert update_fv.execute() -def test_missing_extended_fv_data(): +def test_missing_extended_fv_data(fv_data): request = RequestFactory.create() user = UserFactory.create() update_fv = UpdateFinancialVerification( - MockPEValidator(), + TrueValidator, TaskOrderNumberValidator(), user, request, - required_data, + fv_data, is_extended=True, ) with pytest.raises(FormValidationError): update_fv.execute() + + +def test_save_empty_draft(): + request = RequestFactory.create() + user = UserFactory.create() + save_draft = SaveFinancialVerificationDraft( + TrueValidator, + TrueValidator, + user, + request, + {}, + is_extended=False, + ) + + assert save_draft.execute() + + +def test_save_draft_with_invalid_task_order(fv_data): + request = RequestFactory.create() + user = UserFactory.create() + save_draft = SaveFinancialVerificationDraft( + TrueValidator, + FalseValidator, + user, + request, + fv_data, + is_extended=False, + ) + + with pytest.raises(FormValidationError): + assert save_draft.execute() + + +def test_save_draft_with_invalid_pe_number(fv_data): + request = RequestFactory.create() + user = UserFactory.create() + save_draft = SaveFinancialVerificationDraft( + FalseValidator, + TrueValidator, + user, + request, + fv_data, + is_extended=False, + ) + + with pytest.raises(FormValidationError): + assert save_draft.execute() From 789e5c09065063ce506e6a05a7143d926c618fdc Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 15 Oct 2018 17:08:39 -0400 Subject: [PATCH 07/47] DRY out _get_form --- .../routes/requests/financial_verification.py | 95 ++++++------------- 1 file changed, 29 insertions(+), 66 deletions(-) diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 47b7be2a..cf5a6d95 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -12,38 +12,41 @@ from atst.domain.requests.financial_verification import ( ) -class GetFinancialVerificationForm(object): +class FinancialVerificationBase(object): + + def _get_form(self, request, is_extended, input_data): + data = input_data + + fv_data = request.body.get("financial_verification", {}) + data = {**data, **fv_data} + + if request.task_order: + task_order_dict = request.task_order.to_dictionary() + task_order_dict.update({ + "task_order_number": request.task_order.number, + "funding_type": request.task_order.funding_type.value + }) + data = {**data, **task_order_dict} + + mdict = ImmutableMultiDict(data) + if is_extended: + return ExtendedFinancialForm(formdata=mdict) + else: + return FinancialForm(formdata=mdict) + + +class GetFinancialVerificationForm(FinancialVerificationBase): def __init__(self, user, request, is_extended=False): self.user = user self.request = request self.is_extended = is_extended - def _get_form(self): - data = {} - - fv_data = self.request.body.get("financial_verification") - if fv_data: - data = {**data, **fv_data} - - if self.request.task_order: - task_order_dict = self.request.task_order.to_dictionary() - task_order_dict.update({ - "task_order_number": self.request.task_order.number, - "funding_type": self.request.task_order.funding_type.value - }) - data = {**data, **task_order_dict} - - if self.is_extended: - return ExtendedFinancialForm(data=data) - else: - return FinancialForm(data=data) - def execute(self): - form = self._get_form() + form = self._get_form(self.request, self.is_extended, {}) return {"form": form} -class UpdateFinancialVerification(object): +class UpdateFinancialVerification(FinancialVerificationBase): def __init__( self, pe_validator, @@ -60,28 +63,8 @@ class UpdateFinancialVerification(object): self.fv_data = fv_data self.is_extended = is_extended - def _get_form(self): - data = self.fv_data - - existing_fv_data = self.request.body.get("financial_verification", {}) - data = {**data, **existing_fv_data} - - if self.request.task_order: - task_order_dict = self.request.task_order.to_dictionary() - task_order_dict.update({ - "task_order_number": self.request.task_order.number, - "funding_type": self.request.task_order.funding_type.value - }) - data = {**data, **task_order_dict} - - mdict = ImmutableMultiDict(data) - if self.is_extended: - return ExtendedFinancialForm(formdata=mdict) - else: - return FinancialForm(formdata=mdict) - def execute(self): - form = self._get_form() + form = self._get_form(self.request, self.is_extended, self.fv_data) should_update = True should_submit = True @@ -133,7 +116,7 @@ class UpdateFinancialVerification(object): return {"state": "pending", "request": updated_request} -class SaveFinancialVerificationDraft(object): +class SaveFinancialVerificationDraft(FinancialVerificationBase): def __init__( self, pe_validator, @@ -150,28 +133,8 @@ class SaveFinancialVerificationDraft(object): self.fv_data = fv_data self.is_extended = is_extended - def _get_form(self): - data = self.fv_data - - existing_fv_data = self.request.body.get("financial_verification", {}) - data = {**data, **existing_fv_data} - - if self.request.task_order: - task_order_dict = self.request.task_order.to_dictionary() - task_order_dict.update({ - "task_order_number": self.request.task_order.number, - "funding_type": self.request.task_order.funding_type.value - }) - data = {**data, **task_order_dict} - - mdict = ImmutableMultiDict(data) - if self.is_extended: - return ExtendedFinancialForm(formdata=mdict) - else: - return FinancialForm(formdata=mdict) - def execute(self): - form = self._get_form() + form = self._get_form(self.request, self.is_extended, self.fv_data) valid = True if not form.validate_draft(): From aef4878a3916360424ca802c62170ad62eaad156 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 15 Oct 2018 17:12:43 -0400 Subject: [PATCH 08/47] DRY out pe_number and task_order_number errors --- .../routes/requests/financial_verification.py | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index cf5a6d95..03518fe1 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -34,6 +34,17 @@ class FinancialVerificationBase(object): else: return FinancialForm(formdata=mdict) + def _apply_pe_number_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,) + + def _apply_task_order_number_error(self, field): + field.errors += ("Task Order number not found",) class GetFinancialVerificationForm(FinancialVerificationBase): def __init__(self, user, request, is_extended=False): @@ -76,17 +87,11 @@ class UpdateFinancialVerification(FinancialVerificationBase): should_update = False if not self.pe_validator.validate(self.request, form.pe_id.data): - suggestion = self.pe_validator.suggest_pe_id(form.pe_id.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 "") - form.pe_id.errors += (error_str,) + self._apply_pe_number_error(form.pe_id) should_submit = False if not self.task_order_validator.validate(form.task_order_number.data): - form.task_order_number.errors += ("Task Order number not found",) + self._apply_task_order_number_error(form.task_order_number) should_submit = False if should_update: @@ -141,17 +146,11 @@ class SaveFinancialVerificationDraft(FinancialVerificationBase): valid = False if valid and form.pe_id.data and not self.pe_validator.validate(self.request, form.pe_id.data): - suggestion = self.pe_validator.suggest_pe_id(form.pe_id.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 "") - form.pe_id.errors += (error_str,) + self._apply_pe_number_error(form.pe_id) valid = False if valid and form.task_order_number.data and not self.task_order_validator.validate(form.task_order_number.data): - form.task_order_number.errors += ("Task Order number not found",) + self._apply_task_order_number_error(form.task_order_number) valid = False if not valid: From 8994b3086c7507df7786b0f9d62eedc88927ee7a Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 15 Oct 2018 17:13:22 -0400 Subject: [PATCH 09/47] Formatting --- atst/domain/task_orders.py | 6 ++- .../routes/requests/financial_verification.py | 33 +++++++++---- tests/routes/test_financial_verification.py | 49 +++++-------------- 3 files changed, 40 insertions(+), 48 deletions(-) diff --git a/atst/domain/task_orders.py b/atst/domain/task_orders.py index b214293b..943447c2 100644 --- a/atst/domain/task_orders.py +++ b/atst/domain/task_orders.py @@ -29,7 +29,11 @@ class TaskOrders(object): to_data = TaskOrders._client().get_contract(order_number, status="y") 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, funding_type=FundingType.PROC) + return TaskOrders.create( + number=to_data["contract_no"], + source=Source.EDA, + funding_type=FundingType.PROC, + ) else: raise NotFoundError("task_order") diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 03518fe1..e9edcb78 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -13,7 +13,6 @@ from atst.domain.requests.financial_verification import ( class FinancialVerificationBase(object): - def _get_form(self, request, is_extended, input_data): data = input_data @@ -22,10 +21,12 @@ class FinancialVerificationBase(object): if request.task_order: task_order_dict = request.task_order.to_dictionary() - task_order_dict.update({ - "task_order_number": request.task_order.number, - "funding_type": request.task_order.funding_type.value - }) + task_order_dict.update( + { + "task_order_number": request.task_order.number, + "funding_type": request.task_order.funding_type.value, + } + ) data = {**data, **task_order_dict} mdict = ImmutableMultiDict(data) @@ -46,6 +47,7 @@ class FinancialVerificationBase(object): def _apply_task_order_number_error(self, field): field.errors += ("Task Order number not found",) + class GetFinancialVerificationForm(FinancialVerificationBase): def __init__(self, user, request, is_extended=False): self.user = user @@ -145,11 +147,19 @@ class SaveFinancialVerificationDraft(FinancialVerificationBase): if not form.validate_draft(): valid = False - if valid and form.pe_id.data and not self.pe_validator.validate(self.request, form.pe_id.data): + if ( + valid + and form.pe_id.data + and not self.pe_validator.validate(self.request, form.pe_id.data) + ): self._apply_pe_number_error(form.pe_id) valid = False - if valid and form.task_order_number.data and not self.task_order_validator.validate(form.task_order_number.data): + if ( + valid + and form.task_order_number.data + and not self.task_order_validator.validate(form.task_order_number.data) + ): self._apply_task_order_number_error(form.task_order_number) valid = False @@ -157,17 +167,20 @@ class SaveFinancialVerificationDraft(FinancialVerificationBase): form.reset() raise FormValidationError(form) else: - updated_request = Requests.update_financial_verification(self.request.id, form.data) + updated_request = Requests.update_financial_verification( + self.request.id, form.data + ) return {"request": updated_request} - @requests_bp.route("/requests/verify/", methods=["GET"]) def financial_verification(request_id): request = Requests.get(g.current_user, request_id) is_extended = http_request.args.get("extended") - response_context = GetFinancialVerificationForm(g.current_user, request, is_extended=is_extended).execute() + response_context = GetFinancialVerificationForm( + g.current_user, request, is_extended=is_extended + ).execute() return render_template( "requests/financial_verification.html", diff --git a/tests/routes/test_financial_verification.py b/tests/routes/test_financial_verification.py index a800869c..3b290205 100644 --- a/tests/routes/test_financial_verification.py +++ b/tests/routes/test_financial_verification.py @@ -2,7 +2,10 @@ import pytest from unittest.mock import MagicMock from atst.eda_client import MockEDAClient -from atst.routes.requests.financial_verification import UpdateFinancialVerification, SaveFinancialVerificationDraft +from atst.routes.requests.financial_verification import ( + UpdateFinancialVerification, + SaveFinancialVerificationDraft, +) from tests.mocks import MOCK_REQUEST, MOCK_USER, MOCK_VALID_PE_ID from tests.factories import ( @@ -18,6 +21,7 @@ from atst.domain.requests.financial_verification import ( TaskOrderNumberValidator, ) + @pytest.fixture def fv_data(): return { @@ -43,6 +47,7 @@ TrueValidator.validate = MagicMock(return_value=True) FalseValidator = MagicMock() FalseValidator.validate = MagicMock(return_value=False) + class MockPEValidator(object): def validate(self, request, field): return True @@ -59,12 +64,7 @@ def test_update(fv_data): data = {**fv_data, "pe_id": MOCK_VALID_PE_ID} response_context = UpdateFinancialVerification( - TrueValidator, - TrueValidator, - user, - request, - data, - is_extended=False, + TrueValidator, TrueValidator, user, request, data, is_extended=False ).execute() assert response_context.get("workspace") @@ -75,12 +75,7 @@ def test_re_enter_pe_number(fv_data): user = UserFactory.create() data = {**fv_data, "pe_id": "0101228M"} update_fv = UpdateFinancialVerification( - PENumberValidator(), - TrueValidator, - user, - request, - data, - is_extended=False, + PENumberValidator(), TrueValidator, user, request, data, is_extended=False ) with pytest.raises(FormValidationError): @@ -112,12 +107,7 @@ def test_extended_fv_data(fv_data, extended_financial_verification_data): user = UserFactory.create() data = {**fv_data, **extended_financial_verification_data} update_fv = UpdateFinancialVerification( - TrueValidator, - TaskOrderNumberValidator(), - user, - request, - data, - is_extended=True, + TrueValidator, TaskOrderNumberValidator(), user, request, data, is_extended=True ) assert update_fv.execute() @@ -143,12 +133,7 @@ def test_save_empty_draft(): request = RequestFactory.create() user = UserFactory.create() save_draft = SaveFinancialVerificationDraft( - TrueValidator, - TrueValidator, - user, - request, - {}, - is_extended=False, + TrueValidator, TrueValidator, user, request, {}, is_extended=False ) assert save_draft.execute() @@ -158,12 +143,7 @@ def test_save_draft_with_invalid_task_order(fv_data): request = RequestFactory.create() user = UserFactory.create() save_draft = SaveFinancialVerificationDraft( - TrueValidator, - FalseValidator, - user, - request, - fv_data, - is_extended=False, + TrueValidator, FalseValidator, user, request, fv_data, is_extended=False ) with pytest.raises(FormValidationError): @@ -174,12 +154,7 @@ def test_save_draft_with_invalid_pe_number(fv_data): request = RequestFactory.create() user = UserFactory.create() save_draft = SaveFinancialVerificationDraft( - FalseValidator, - TrueValidator, - user, - request, - fv_data, - is_extended=False, + FalseValidator, TrueValidator, user, request, fv_data, is_extended=False ) with pytest.raises(FormValidationError): From 8ec796861e2ee5ad196e5d859c86c6b451c8db37 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 16 Oct 2018 10:27:24 -0400 Subject: [PATCH 10/47] Fix SaveFinancialVerificationDraft --- atst/routes/requests/financial_verification.py | 13 +++++-------- tests/routes/test_financial_verification.py | 12 ++++++++++++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index e9edcb78..37c1ad24 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -14,10 +14,7 @@ from atst.domain.requests.financial_verification import ( class FinancialVerificationBase(object): def _get_form(self, request, is_extended, input_data): - data = input_data - - fv_data = request.body.get("financial_verification", {}) - data = {**data, **fv_data} + existing_fv_data = request.financial_verification if request.task_order: task_order_dict = request.task_order.to_dictionary() @@ -27,13 +24,13 @@ class FinancialVerificationBase(object): "funding_type": request.task_order.funding_type.value, } ) - data = {**data, **task_order_dict} + existing_fv_data = {**existing_fv_data, **task_order_dict} - mdict = ImmutableMultiDict(data) + mdict = ImmutableMultiDict(input_data) if is_extended: - return ExtendedFinancialForm(formdata=mdict) + return ExtendedFinancialForm(formdata=mdict, data=existing_fv_data) else: - return FinancialForm(formdata=mdict) + return FinancialForm(formdata=mdict, data=existing_fv_data) def _apply_pe_number_error(self, field): suggestion = self.pe_validator.suggest_pe_id(field.data) diff --git a/tests/routes/test_financial_verification.py b/tests/routes/test_financial_verification.py index 3b290205..7863b834 100644 --- a/tests/routes/test_financial_verification.py +++ b/tests/routes/test_financial_verification.py @@ -139,6 +139,18 @@ def test_save_empty_draft(): assert save_draft.execute() +def test_save_draft_with_ba_code(): + request = RequestFactory.create() + user = UserFactory.create() + data = {"ba_code": "02A"} + save_draft = SaveFinancialVerificationDraft( + TrueValidator, TrueValidator, user, request, data, is_extended=False + ) + + response_context = save_draft.execute() + request = response_context["request"] + + def test_save_draft_with_invalid_task_order(fv_data): request = RequestFactory.create() user = UserFactory.create() From 421ab655119dc11e9a673c283259289550aef01a Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 16 Oct 2018 10:27:49 -0400 Subject: [PATCH 11/47] Hook up Save Draft button --- templates/requests/financial_verification.html | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/templates/requests/financial_verification.html b/templates/requests/financial_verification.html index 5d998905..5e8cd666 100644 --- a/templates/requests/financial_verification.html +++ b/templates/requests/financial_verification.html @@ -42,13 +42,7 @@ {% endcall %} {% endif %} - {% block form_action %} - {% if extended %} -
- {% else %} - - {% endif %} - {% endblock %} + {{ f.csrf_token }} {% block form %} @@ -180,7 +174,8 @@ {% endblock form %} {% block next %}
- + +
{% endblock %}
From 5075778bb1bee9214975cfd269de6c9a28bc1e19 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 16 Oct 2018 10:42:41 -0400 Subject: [PATCH 12/47] Remove unused variable --- atst/routes/requests/financial_verification.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 37c1ad24..3a3dde2f 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -229,7 +229,7 @@ def save_financial_verification_draft(request_id): is_extended = http_request.args.get("extended") try: - response_context = SaveFinancialVerificationDraft( + SaveFinancialVerificationDraft( PENumberValidator(), TaskOrderNumberValidator(), g.current_user, From 87ff0dfee01c5ff3c86efd76dd8829b48ba368b4 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 16 Oct 2018 11:00:37 -0400 Subject: [PATCH 13/47] formdata should be None for GetFinancialVerificationForm --- atst/routes/requests/financial_verification.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 3a3dde2f..23304e14 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -13,7 +13,7 @@ from atst.domain.requests.financial_verification import ( class FinancialVerificationBase(object): - def _get_form(self, request, is_extended, input_data): + def _get_form(self, request, is_extended, formdata=None): existing_fv_data = request.financial_verification if request.task_order: @@ -26,7 +26,7 @@ class FinancialVerificationBase(object): ) existing_fv_data = {**existing_fv_data, **task_order_dict} - mdict = ImmutableMultiDict(input_data) + mdict = ImmutableMultiDict(formdata) if formdata is not None else None if is_extended: return ExtendedFinancialForm(formdata=mdict, data=existing_fv_data) else: @@ -52,7 +52,7 @@ class GetFinancialVerificationForm(FinancialVerificationBase): self.is_extended = is_extended def execute(self): - form = self._get_form(self.request, self.is_extended, {}) + form = self._get_form(self.request, self.is_extended) return {"form": form} From 0d9c03b54f8ba593b66fff453a5bbc59cb8f88e5 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 16 Oct 2018 11:15:06 -0400 Subject: [PATCH 14/47] Refactor, add test --- .../routes/requests/financial_verification.py | 44 +++++++++---------- tests/routes/test_financial_verification.py | 23 +++++----- 2 files changed, 34 insertions(+), 33 deletions(-) diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 23304e14..c1fe43bc 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -142,32 +142,30 @@ class SaveFinancialVerificationDraft(FinancialVerificationBase): valid = True if not form.validate_draft(): - valid = False - - if ( - valid - and form.pe_id.data - and not self.pe_validator.validate(self.request, form.pe_id.data) - ): - self._apply_pe_number_error(form.pe_id) - valid = False - - if ( - valid - and form.task_order_number.data - and not self.task_order_validator.validate(form.task_order_number.data) - ): - self._apply_task_order_number_error(form.task_order_number) - valid = False - - if not valid: form.reset() raise FormValidationError(form) - else: - updated_request = Requests.update_financial_verification( - self.request.id, form.data - ) + + if form.pe_id.data and not self.pe_validator.validate( + 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( + form.task_order_number.data + ): + valid = False + self._apply_task_order_number_error(form.task_order_number) + + updated_request = Requests.update_financial_verification( + self.request.id, form.data + ) + + if valid: return {"request": updated_request} + else: + form.reset() + raise FormValidationError(form) @requests_bp.route("/requests/verify/", methods=["GET"]) diff --git a/tests/routes/test_financial_verification.py b/tests/routes/test_financial_verification.py index 7863b834..c2e22da2 100644 --- a/tests/routes/test_financial_verification.py +++ b/tests/routes/test_financial_verification.py @@ -48,16 +48,6 @@ FalseValidator = MagicMock() FalseValidator.validate = MagicMock(return_value=False) -class MockPEValidator(object): - def validate(self, request, field): - return True - - -class MockTaskOrderValidator(object): - def validate(self, field): - return True - - def test_update(fv_data): request = RequestFactory.create() user = UserFactory.create() @@ -171,3 +161,16 @@ def test_save_draft_with_invalid_pe_number(fv_data): with pytest.raises(FormValidationError): assert save_draft.execute() + + +def test_save_draft_re_enter_pe_number(fv_data): + request = RequestFactory.create() + user = UserFactory.create() + data = {**fv_data, "pe_id": "0101228M"} + save_fv = SaveFinancialVerificationDraft( + PENumberValidator(), TrueValidator, user, request, data, is_extended=False + ) + + with pytest.raises(FormValidationError): + save_fv.execute() + response_context = save_fv.execute() From 8c9488229cf33a457381bbb849cebce3509b6991 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 16 Oct 2018 11:16:53 -0400 Subject: [PATCH 15/47] Factor out _raise --- atst/routes/requests/financial_verification.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index c1fe43bc..3fbacf6f 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -44,6 +44,10 @@ class FinancialVerificationBase(object): def _apply_task_order_number_error(self, field): field.errors += ("Task Order number not found",) + def _raise(self, form): + form.reset() + raise FormValidationError(form) + class GetFinancialVerificationForm(FinancialVerificationBase): def __init__(self, user, request, is_extended=False): @@ -98,8 +102,7 @@ class UpdateFinancialVerification(FinancialVerificationBase): self.request.id, form.data ) else: - form.reset() - raise FormValidationError(form) + self._raise(form) if should_submit: updated_request = Requests.submit_financial_verification(updated_request) @@ -107,8 +110,7 @@ class UpdateFinancialVerification(FinancialVerificationBase): workspace = Requests.approve_and_create_workspace(updated_request) submitted = True else: - form.reset() - raise FormValidationError(form) + self._raise(form) if submitted: return { @@ -142,8 +144,7 @@ class SaveFinancialVerificationDraft(FinancialVerificationBase): valid = True if not form.validate_draft(): - form.reset() - raise FormValidationError(form) + self._raise(form) if form.pe_id.data and not self.pe_validator.validate( self.request, form.pe_id.data @@ -164,8 +165,7 @@ class SaveFinancialVerificationDraft(FinancialVerificationBase): if valid: return {"request": updated_request} else: - form.reset() - raise FormValidationError(form) + self._raise(form) @requests_bp.route("/requests/verify/", methods=["GET"]) From f56dd5621eb4912c8b5ffe0964286f1bcee87fef Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 16 Oct 2018 14:15:27 -0400 Subject: [PATCH 16/47] Revive suggest_pe_id tests --- tests/forms/test_financial.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/forms/test_financial.py b/tests/forms/test_financial.py index 925aec2a..b1f08b64 100644 --- a/tests/forms/test_financial.py +++ b/tests/forms/test_financial.py @@ -2,21 +2,21 @@ import pytest from werkzeug.datastructures import ImmutableMultiDict from atst.forms.financial import FinancialForm, ExtendedFinancialForm -from atst.eda_client import MockEDAClient +from atst.domain.requests.financial_verification import PENumberValidator -# @pytest.mark.parametrize( -# "input_,expected", -# [ -# ("0603502N", None), -# ("0603502NZ", None), -# ("603502N", "0603502N"), -# ("063502N", "0603502N"), -# ("63502N", "0603502N"), -# ], -# ) -# def test_suggest_pe_id(input_, expected): -# assert suggest_pe_id(input_) == expected +@pytest.mark.parametrize( + "input_,expected", + [ + ("0603502N", None), + ("0603502NZ", None), + ("603502N", "0603502N"), + ("063502N", "0603502N"), + ("63502N", "0603502N"), + ], +) +def test_suggest_pe_id(input_, expected): + assert PENumberValidator().suggest_pe_id(input_) == expected def test_funding_type_other_not_required_if_funding_type_is_not_other(): From 96c03782773002e47a4938e6f0d73cf1bea9c679 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 16 Oct 2018 15:44:47 -0400 Subject: [PATCH 17/47] Add simple tests for finver routes --- tests/routes/test_financial_verification.py | 59 +++++++++++++++++++-- 1 file changed, 54 insertions(+), 5 deletions(-) diff --git a/tests/routes/test_financial_verification.py b/tests/routes/test_financial_verification.py index c2e22da2..7d3ad76f 100644 --- a/tests/routes/test_financial_verification.py +++ b/tests/routes/test_financial_verification.py @@ -1,5 +1,6 @@ import pytest from unittest.mock import MagicMock +from flask import url_for from atst.eda_client import MockEDAClient from atst.routes.requests.financial_verification import ( @@ -48,7 +49,7 @@ FalseValidator = MagicMock() FalseValidator.validate = MagicMock(return_value=False) -def test_update(fv_data): +def test_update_fv(fv_data): request = RequestFactory.create() user = UserFactory.create() data = {**fv_data, "pe_id": MOCK_VALID_PE_ID} @@ -60,7 +61,7 @@ def test_update(fv_data): assert response_context.get("workspace") -def test_re_enter_pe_number(fv_data): +def test_update_fv_re_enter_pe_number(fv_data): request = RequestFactory.create() user = UserFactory.create() data = {**fv_data, "pe_id": "0101228M"} @@ -75,7 +76,7 @@ def test_re_enter_pe_number(fv_data): assert response_context.get("status", "submitted") -def test_invalid_task_order_number(fv_data): +def test_update_fv_invalid_task_order_number(fv_data): request = RequestFactory.create() user = UserFactory.create() data = {**fv_data, "task_order_number": "DCA10096D0051"} @@ -92,7 +93,7 @@ def test_invalid_task_order_number(fv_data): update_fv.execute() -def test_extended_fv_data(fv_data, extended_financial_verification_data): +def test_update_fv_extended(fv_data, extended_financial_verification_data): request = RequestFactory.create() user = UserFactory.create() data = {**fv_data, **extended_financial_verification_data} @@ -103,7 +104,7 @@ def test_extended_fv_data(fv_data, extended_financial_verification_data): assert update_fv.execute() -def test_missing_extended_fv_data(fv_data): +def test_update_fv_missing_extended_data(fv_data): request = RequestFactory.create() user = UserFactory.create() update_fv = UpdateFinancialVerification( @@ -119,6 +120,15 @@ def test_missing_extended_fv_data(fv_data): update_fv.execute() +def test_update_fv_submission(fv_data): + request = RequestFactory.create() + user = UserFactory.create() + response_context = UpdateFinancialVerification( + TrueValidator, TrueValidator, user, request, fv_data + ).execute() + assert response_context + + def test_save_empty_draft(): request = RequestFactory.create() user = UserFactory.create() @@ -174,3 +184,42 @@ def test_save_draft_re_enter_pe_number(fv_data): with pytest.raises(FormValidationError): save_fv.execute() response_context = save_fv.execute() + + +def test_update_fv_route(client, user_session, fv_data): + user = UserFactory.create() + request = RequestFactory.create(creator=user) + user_session(user) + response = client.post( + url_for("requests.financial_verification", request_id=request.id), + data=fv_data, + follow_redirects=False, + ) + + assert response.status_code == 200 + + +def test_save_fv_draft_route(client, user_session, fv_data): + user = UserFactory.create() + request = RequestFactory.create(creator=user) + user_session(user) + response = client.post( + url_for("requests.save_financial_verification_draft", request_id=request.id), + data=fv_data, + follow_redirects=False, + ) + + assert response.status_code == 200 + + +def test_get_fv_form_route(client, user_session, fv_data): + user = UserFactory.create() + request = RequestFactory.create(creator=user) + user_session(user) + response = client.get( + url_for("requests.financial_verification", request_id=request.id), + data=fv_data, + follow_redirects=False, + ) + + assert response.status_code == 200 From 4f1a3e8d0f75ca714ced68c94d696d2862198c93 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 16 Oct 2018 16:34:36 -0400 Subject: [PATCH 18/47] Simplify return values --- .../routes/requests/financial_verification.py | 40 +++++-------------- tests/routes/test_financial_verification.py | 18 ++++----- 2 files changed, 20 insertions(+), 38 deletions(-) diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 3fbacf6f..74a3d748 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -56,8 +56,7 @@ class GetFinancialVerificationForm(FinancialVerificationBase): self.is_extended = is_extended def execute(self): - form = self._get_form(self.request, self.is_extended) - return {"form": form} + return self._get_form(self.request, self.is_extended) class UpdateFinancialVerification(FinancialVerificationBase): @@ -83,8 +82,6 @@ class UpdateFinancialVerification(FinancialVerificationBase): should_update = True should_submit = True updated_request = None - submitted = False - workspace = None if not form.validate(): should_update = False @@ -101,25 +98,10 @@ class UpdateFinancialVerification(FinancialVerificationBase): updated_request = Requests.update_financial_verification( self.request.id, form.data ) - else: - self._raise(form) + if should_submit: + return Requests.submit_financial_verification(updated_request) - if should_submit: - updated_request = Requests.submit_financial_verification(updated_request) - if updated_request.is_financially_verified: - workspace = Requests.approve_and_create_workspace(updated_request) - submitted = True - else: - self._raise(form) - - if submitted: - return { - "state": "submitted", - "workspace": workspace, - "request": updated_request, - } - else: - return {"state": "pending", "request": updated_request} + self._raise(form) class SaveFinancialVerificationDraft(FinancialVerificationBase): @@ -163,7 +145,7 @@ class SaveFinancialVerificationDraft(FinancialVerificationBase): ) if valid: - return {"request": updated_request} + return updated_request else: self._raise(form) @@ -173,13 +155,13 @@ def financial_verification(request_id): request = Requests.get(g.current_user, request_id) is_extended = http_request.args.get("extended") - response_context = GetFinancialVerificationForm( + form = GetFinancialVerificationForm( g.current_user, request, is_extended=is_extended ).execute() return render_template( "requests/financial_verification.html", - f=response_context["form"], + f=form, jedi_request=request, review_comment=request.review_comment, extended=is_extended, @@ -193,7 +175,7 @@ def update_financial_verification(request_id): is_extended = http_request.args.get("extended") try: - response_context = UpdateFinancialVerification( + updated_request = UpdateFinancialVerification( PENumberValidator(), TaskOrderNumberValidator(), g.current_user, @@ -209,14 +191,14 @@ def update_financial_verification(request_id): extended=is_extended, ) - if response_context["state"] == "submitted": - workspace = response_context["workspace"] + if updated_request.is_pending_ccpo_approval: + workspace = Requests.approve_and_create_workspace(updated_request) return redirect( url_for( "workspaces.new_project", workspace_id=workspace.id, newWorkspace=True ) ) - elif response_context["state"] == "pending": + else: return redirect(url_for("requests.requests_index", modal="pendingCCPOApproval")) diff --git a/tests/routes/test_financial_verification.py b/tests/routes/test_financial_verification.py index 7d3ad76f..551f16ad 100644 --- a/tests/routes/test_financial_verification.py +++ b/tests/routes/test_financial_verification.py @@ -54,11 +54,12 @@ def test_update_fv(fv_data): user = UserFactory.create() data = {**fv_data, "pe_id": MOCK_VALID_PE_ID} - response_context = UpdateFinancialVerification( + updated_request = UpdateFinancialVerification( TrueValidator, TrueValidator, user, request, data, is_extended=False ).execute() - assert response_context.get("workspace") + assert updated_request.is_pending_ccpo_approval + def test_update_fv_re_enter_pe_number(fv_data): @@ -71,9 +72,9 @@ def test_update_fv_re_enter_pe_number(fv_data): with pytest.raises(FormValidationError): update_fv.execute() - response_context = update_fv.execute() + updated_request = update_fv.execute() - assert response_context.get("status", "submitted") + assert updated_request.is_pending_ccpo_approval def test_update_fv_invalid_task_order_number(fv_data): @@ -123,10 +124,10 @@ def test_update_fv_missing_extended_data(fv_data): def test_update_fv_submission(fv_data): request = RequestFactory.create() user = UserFactory.create() - response_context = UpdateFinancialVerification( + updated_request = UpdateFinancialVerification( TrueValidator, TrueValidator, user, request, fv_data ).execute() - assert response_context + assert updated_request def test_save_empty_draft(): @@ -147,8 +148,7 @@ def test_save_draft_with_ba_code(): TrueValidator, TrueValidator, user, request, data, is_extended=False ) - response_context = save_draft.execute() - request = response_context["request"] + assert save_draft.execute() def test_save_draft_with_invalid_task_order(fv_data): @@ -183,7 +183,7 @@ def test_save_draft_re_enter_pe_number(fv_data): with pytest.raises(FormValidationError): save_fv.execute() - response_context = save_fv.execute() + save_fv.execute() def test_update_fv_route(client, user_session, fv_data): From 5bd41a101654e5816da37ab69eb20a0b7d7a3455 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 16 Oct 2018 20:33:27 -0400 Subject: [PATCH 19/47] Fix bug where saving draft caused submit to always succeed --- atst/forms/financial.py | 16 ++++++++++++++-- tests/routes/test_financial_verification.py | 15 ++++++++++++++- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/atst/forms/financial.py b/atst/forms/financial.py index 61d334cd..55abaf78 100644 --- a/atst/forms/financial.py +++ b/atst/forms/financial.py @@ -6,7 +6,7 @@ from wtforms.validators import InputRequired, Email, Regexp, Optional from flask_wtf.file import FileAllowed from .fields import NewlineListField, SelectField, NumberStringField -from .forms import ValidatedForm +from atst.forms.forms import ValidatedForm from .data import FUNDING_TYPES from .validators import DateRange @@ -30,9 +30,21 @@ class BaseFinancialForm(ValidatedForm): self.uii_ids.process_data(self.uii_ids.data) 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. + """ for field in self: field.validators.insert(0, Optional()) - return self.validate() + + valid = self.validate() + + for field in self: + field.validators.pop(0) + + return valid @property def is_missing_task_order_number(self): diff --git a/tests/routes/test_financial_verification.py b/tests/routes/test_financial_verification.py index 551f16ad..5fa4e978 100644 --- a/tests/routes/test_financial_verification.py +++ b/tests/routes/test_financial_verification.py @@ -61,7 +61,6 @@ def test_update_fv(fv_data): assert updated_request.is_pending_ccpo_approval - def test_update_fv_re_enter_pe_number(fv_data): request = RequestFactory.create() user = UserFactory.create() @@ -186,6 +185,20 @@ def test_save_draft_re_enter_pe_number(fv_data): save_fv.execute() +def test_save_draft_and_then_submit(): + request = RequestFactory.create() + user = UserFactory.create() + data = {"ba_code": "02A"} + updated_request = SaveFinancialVerificationDraft( + TrueValidator, TrueValidator, user, request, data, is_extended=False + ).execute() + + with pytest.raises(FormValidationError): + UpdateFinancialVerification( + TrueValidator, TrueValidator, user, updated_request, data + ).execute() + + def test_update_fv_route(client, user_session, fv_data): user = UserFactory.create() request = RequestFactory.create(creator=user) From 348687cd03ad19b826aef489f87390a38f9a7448 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Thu, 18 Oct 2018 10:18:24 -0400 Subject: [PATCH 20/47] Ensure that task order PDF makes it into the FV form --- atst/routes/requests/financial_verification.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 74a3d748..0c887393 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -171,7 +171,7 @@ def financial_verification(request_id): @requests_bp.route("/requests/verify/", methods=["POST"]) def update_financial_verification(request_id): request = Requests.get(g.current_user, request_id) - fv_data = http_request.form + fv_data = {**http_request.form, **http_request.files} is_extended = http_request.args.get("extended") try: From 650cc02678b5bd7e26f2ba637f90d0f2e1d813cb Mon Sep 17 00:00:00 2001 From: richard-dds Date: Thu, 18 Oct 2018 10:53:20 -0400 Subject: [PATCH 21/47] Always pop the task_order --- atst/domain/requests/requests.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/atst/domain/requests/requests.py b/atst/domain/requests/requests.py index 753149ca..db2c44c9 100644 --- a/atst/domain/requests/requests.py +++ b/atst/domain/requests/requests.py @@ -171,10 +171,9 @@ class Requests(object): else: task_order_number = request_data.get("task_order_number") - if "task_order" in request_data and isinstance( - request_data["task_order"], FileStorage - ): - task_order_data["pdf"] = request_data.pop("task_order") + task_order_file = request_data.pop("task_order", None) + if isinstance(task_order_file, FileStorage): + task_order_data["pdf"] = task_order_file task_order = TaskOrders.get_or_create_task_order( task_order_number, task_order_data From 3cf0907d0d478279e8d0f8dac20a32b0fbbb1f97 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Thu, 18 Oct 2018 13:48:47 -0400 Subject: [PATCH 22/47] Add test to ensure that pdf is attached to TO --- tests/routes/test_financial_verification.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/routes/test_financial_verification.py b/tests/routes/test_financial_verification.py index 5fa4e978..554beec3 100644 --- a/tests/routes/test_financial_verification.py +++ b/tests/routes/test_financial_verification.py @@ -199,6 +199,20 @@ def test_save_draft_and_then_submit(): ).execute() +def test_updated_request_has_pdf(fv_data, extended_financial_verification_data): + request = RequestFactory.create() + user = UserFactory.create() + data = { + **fv_data, + **extended_financial_verification_data, + "task_order_number": "DCA10096D0051", + } + updated_request = UpdateFinancialVerification( + TrueValidator, TrueValidator, user, request, data, is_extended=True + ).execute() + assert updated_request.task_order.pdf + + def test_update_fv_route(client, user_session, fv_data): user = UserFactory.create() request = RequestFactory.create(creator=user) From 5d2b976e5f9da3e9f0fd2ffa257bb76db27449d3 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Fri, 19 Oct 2018 10:57:18 -0400 Subject: [PATCH 23/47] Preemptively upload task order PDF --- .../9c24c609878a_resource_attachments.py | 32 +++++++++++ atst/domain/requests/requests.py | 57 +++++++++++++------ atst/domain/task_orders.py | 10 ++++ atst/models/attachment.py | 43 +++++++++++++- .../routes/requests/financial_verification.py | 55 ++++++++++++++++-- js/components/forms/financial.js | 17 +++++- .../requests/financial_verification.html | 22 ++++--- tests/routes/test_financial_verification.py | 13 +++++ 8 files changed, 217 insertions(+), 32 deletions(-) create mode 100644 alembic/versions/9c24c609878a_resource_attachments.py diff --git a/alembic/versions/9c24c609878a_resource_attachments.py b/alembic/versions/9c24c609878a_resource_attachments.py new file mode 100644 index 00000000..ecb19a91 --- /dev/null +++ b/alembic/versions/9c24c609878a_resource_attachments.py @@ -0,0 +1,32 @@ +"""resource attachments + +Revision ID: 9c24c609878a +Revises: 903d7c66ff1d +Create Date: 2018-10-18 17:14:25.566215 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '9c24c609878a' +down_revision = '903d7c66ff1d' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('attachments', sa.Column('resource', sa.String(), nullable=True)) + op.add_column('attachments', sa.Column('resource_id', postgresql.UUID(as_uuid=True), nullable=True)) + op.create_index(op.f('ix_attachments_resource_id'), 'attachments', ['resource_id'], unique=False) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index(op.f('ix_attachments_resource_id'), table_name='attachments') + op.drop_column('attachments', 'resource_id') + op.drop_column('attachments', 'resource') + # ### end Alembic commands ### diff --git a/atst/domain/requests/requests.py b/atst/domain/requests/requests.py index db2c44c9..be38880e 100644 --- a/atst/domain/requests/requests.py +++ b/atst/domain/requests/requests.py @@ -13,6 +13,11 @@ from .query import RequestsQuery from .authorization import RequestsAuthorization +def pick(keys, d): + _keys = set(keys) + return {k: v for (k, v) in d.items() if k in _keys} + + def create_revision_from_request_body(body): body = {k: v for p in body.values() for k, v in p.items()} DATES = ["start_date", "date_latest_training"] @@ -156,33 +161,51 @@ class Requests(object): return Requests.status_count(RequestStatus.APPROVED) @classmethod - def update_financial_verification(cls, request_id, financial_data): + def update_financial_verification(cls, request_id, financial_data, task_order=None): request = RequestsQuery.get_with_lock(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 TaskOrders.TASK_ORDER_DATA - } + # request_data = financial_data.copy() + # task_order_data = { + # k: request_data.pop(k) + # for (k, v) in financial_data.items() + # if k in TaskOrders.TASK_ORDER_DATA + # } - if task_order_data: - task_order_number = request_data.pop("task_order_number") - else: - task_order_number = request_data.get("task_order_number") + # if task_order_data: + # task_order_number = request_data.pop("task_order_number") + # else: + # task_order_number = request_data.get("task_order_number") - task_order_file = request_data.pop("task_order", None) - if isinstance(task_order_file, FileStorage): - task_order_data["pdf"] = task_order_file + # task_order_file = request_data.pop("task_order", None) + # if isinstance(task_order_file, FileStorage): + # task_order_data["pdf"] = task_order_file - task_order = TaskOrders.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 + # ) + + delta = pick( + [ + "uii_ids", + "pe_id", + "treasury_code", + "ba_code", + "fname_co", + "lname_co", + "email_co", + "office_co", + "fname_cor", + "lname_cor", + "email_cor", + "office_cor", + ], + financial_data, ) if task_order: request.task_order = task_order - request = Requests.update(request.id, {"financial_verification": request_data}) + request = Requests.update(request.id, {"financial_verification": delta}) return request diff --git a/atst/domain/task_orders.py b/atst/domain/task_orders.py index 943447c2..afe4b6a0 100644 --- a/atst/domain/task_orders.py +++ b/atst/domain/task_orders.py @@ -67,3 +67,13 @@ class TaskOrders(object): source=Source.MANUAL, pdf=attachment, ) + + @classmethod + def get_or_create(cls, number, attachment=None, data=None): + try: + return TaskOrders.get(number) + except NotFoundError: + data = data or {} + return TaskOrders.create( + **data, number=number, pdf=attachment, source=Source.MANUAL + ) diff --git a/atst/models/attachment.py b/atst/models/attachment.py index 9838ea05..9a8dfcbf 100644 --- a/atst/models/attachment.py +++ b/atst/models/attachment.py @@ -1,9 +1,12 @@ from sqlalchemy import Column, String +from sqlalchemy.dialects.postgresql import UUID +from sqlalchemy.orm.exc import NoResultFound from flask import current_app as app from atst.models import Base, types, mixins from atst.database import db from atst.uploader import UploadError +from atst.domain.exceptions import NotFoundError class AttachmentError(Exception): @@ -16,20 +19,56 @@ class Attachment(Base, mixins.TimestampsMixin): id = types.Id() filename = Column(String, nullable=False) object_name = Column(String, unique=True, nullable=False) + resource = Column(String) + resource_id = Column(UUID(as_uuid=True), index=True) @classmethod - def attach(cls, fyle): + def attach(cls, fyle, resource=None, resource_id=None): try: filename, object_name = app.uploader.upload(fyle) except UploadError as e: raise AttachmentError("Could not add attachment. " + str(e)) - attachment = Attachment(filename=filename, object_name=object_name) + attachment = Attachment( + filename=filename, + object_name=object_name, + resource=resource, + resource_id=resource_id, + ) db.session.add(attachment) db.session.commit() return attachment + @classmethod + def get(cls, id_): + try: + return db.session.query(Attachment).filter_by(id=id_).one() + except NoResultFound: + raise NotFoundError("attachment") + + @classmethod + def get_for_resource(cls, resource, resource_id): + try: + return ( + db.session.query(Attachment) + .filter_by(resource=resource, resource_id=resource_id) + .one() + ) + except NoResultFound: + raise NotFoundError("attachment") + + @classmethod + def delete_for_resource(cls, resource, resource_id): + try: + return ( + db.session.query(Attachment) + .filter_by(resource=resource, resource_id=resource_id) + .delete() + ) + except NoResultFound: + raise NotFoundError("attachment") + def __repr__(self): return "".format(self.filename, self.id) diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 0c887393..dbafce03 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -1,15 +1,18 @@ from flask import g, render_template, redirect, url_for from flask import request as http_request -from werkzeug.datastructures import ImmutableMultiDict +from werkzeug.datastructures import ImmutableMultiDict, FileStorage from . import requests_bp from atst.domain.requests import Requests from atst.forms.financial import FinancialForm, ExtendedFinancialForm from atst.forms.exceptions import FormValidationError +from atst.domain.exceptions import NotFoundError from atst.domain.requests.financial_verification import ( PENumberValidator, TaskOrderNumberValidator, ) +from atst.models.attachment import Attachment +from atst.domain.task_orders import TaskOrders class FinancialVerificationBase(object): @@ -28,10 +31,46 @@ class FinancialVerificationBase(object): mdict = ImmutableMultiDict(formdata) if formdata is not None else None if is_extended: + try: + attachment = Attachment.get_for_resource("task_order", self.request.id) + existing_fv_data["task_order"] = attachment.filename + except NotFoundError: + pass + return ExtendedFinancialForm(formdata=mdict, data=existing_fv_data) else: return FinancialForm(formdata=mdict, data=existing_fv_data) + def _process_attachment(self, is_extended, form): + attachment = None + if self.is_extended: + attachment = None + if isinstance(form.task_order.data, FileStorage): + Attachment.delete_for_resource("task_order", self.request.id) + attachment = Attachment.attach( + form.task_order.data, "task_order", self.request.id + ) + elif isinstance(form.task_order.data, str): + attachment = Attachment.get_for_resource("task_order", self.request.id) + + if attachment: + form.task_order.data = attachment.id + + return attachment + + def _try_create_task_order(self, form, attachment): + form_data = form.data + + task_order_number = form_data.get("task_order_number") + if task_order_number: + task_order_data = { + k: v for (k, v) in form_data.items() if k in TaskOrders.TASK_ORDER_DATA + } + return TaskOrders.get_or_create( + task_order_number, attachment=attachment, data=task_order_data + ) + return None + def _apply_pe_number_error(self, field): suggestion = self.pe_validator.suggest_pe_id(field.data) error_str = ( @@ -56,7 +95,8 @@ class GetFinancialVerificationForm(FinancialVerificationBase): self.is_extended = is_extended def execute(self): - return self._get_form(self.request, self.is_extended) + form = self._get_form(self.request, self.is_extended) + return form class UpdateFinancialVerification(FinancialVerificationBase): @@ -94,9 +134,12 @@ class UpdateFinancialVerification(FinancialVerificationBase): self._apply_task_order_number_error(form.task_order_number) should_submit = False + attachment = self._process_attachment(self.is_extended, form) + if should_update: + task_order = self._try_create_task_order(form, attachment) updated_request = Requests.update_financial_verification( - self.request.id, form.data + self.request.id, form.data, task_order=task_order ) if should_submit: return Requests.submit_financial_verification(updated_request) @@ -140,8 +183,10 @@ class SaveFinancialVerificationDraft(FinancialVerificationBase): 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) updated_request = Requests.update_financial_verification( - self.request.id, form.data + self.request.id, form.data, task_order=task_order ) if valid: @@ -205,7 +250,7 @@ def update_financial_verification(request_id): @requests_bp.route("/requests/verify//draft", methods=["POST"]) def save_financial_verification_draft(request_id): request = Requests.get(g.current_user, request_id) - fv_data = http_request.form + fv_data = {**http_request.form, **http_request.files} is_extended = http_request.args.get("extended") try: diff --git a/js/components/forms/financial.js b/js/components/forms/financial.js index c4c3b02d..febe8d47 100644 --- a/js/components/forms/financial.js +++ b/js/components/forms/financial.js @@ -25,7 +25,22 @@ export default { } = this.initialData return { - funding_type + funding_type, + shouldForceShowTaskOrder: false + } + }, + + computed: { + showTaskOrder: function() { + return !this.initialData.task_order || this.shouldForceShowTaskOrder + } + }, + + methods: { + forceShowTaskOrder: function(e) { + console.log("forceShowTaskOrder", e) + e.preventDefault() + this.shouldForceShowTaskOrder = true } } } diff --git a/templates/requests/financial_verification.html b/templates/requests/financial_verification.html index 5e8cd666..59a05c5d 100644 --- a/templates/requests/financial_verification.html +++ b/templates/requests/financial_verification.html @@ -106,13 +106,21 @@ validation='dollars' ) }} -
- {{ f.task_order.label }} - {{ f.task_order }} - {% for error in f.task_order.errors %} - {{error}} - {% endfor %} -
+ + {% endif %} diff --git a/tests/routes/test_financial_verification.py b/tests/routes/test_financial_verification.py index 554beec3..59f8f95c 100644 --- a/tests/routes/test_financial_verification.py +++ b/tests/routes/test_financial_verification.py @@ -4,6 +4,7 @@ from flask import url_for from atst.eda_client import MockEDAClient from atst.routes.requests.financial_verification import ( + GetFinancialVerificationForm, UpdateFinancialVerification, SaveFinancialVerificationDraft, ) @@ -213,6 +214,18 @@ def test_updated_request_has_pdf(fv_data, extended_financial_verification_data): assert updated_request.task_order.pdf +def test_can_save_draft_with_just_pdf(extended_financial_verification_data): + request = RequestFactory.create() + user = UserFactory.create() + data = {"task_order": extended_financial_verification_data["task_order"]} + SaveFinancialVerificationDraft( + TrueValidator, TrueValidator, user, request, data, is_extended=True + ).execute() + + form = GetFinancialVerificationForm(user, request, is_extended=True).execute() + assert form.task_order + + def test_update_fv_route(client, user_session, fv_data): user = UserFactory.create() request = RequestFactory.create(creator=user) From 22cbca5951722600bd9ea2a4573a538d30b02404 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Fri, 19 Oct 2018 14:49:50 -0400 Subject: [PATCH 24/47] Remember if finver form is extended --- alembic/versions/edf509c974f6_extended_fv.py | 28 ++++++++++++++++ atst/domain/requests/requests.py | 3 +- atst/forms/financial.py | 1 + atst/models/mixins/auditable.py | 14 +------- atst/models/request.py | 3 +- atst/models/request_revision.py | 1 + .../routes/requests/financial_verification.py | 22 +++++++++---- atst/utils/__init__.py | 14 ++++++++ tests/domain/test_requests.py | 33 ------------------- 9 files changed, 64 insertions(+), 55 deletions(-) create mode 100644 alembic/versions/edf509c974f6_extended_fv.py diff --git a/alembic/versions/edf509c974f6_extended_fv.py b/alembic/versions/edf509c974f6_extended_fv.py new file mode 100644 index 00000000..8877f659 --- /dev/null +++ b/alembic/versions/edf509c974f6_extended_fv.py @@ -0,0 +1,28 @@ +"""extended fv + +Revision ID: edf509c974f6 +Revises: 9c24c609878a +Create Date: 2018-10-19 14:06:45.396974 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'edf509c974f6' +down_revision = '9c24c609878a' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('request_revisions', sa.Column('extended', sa.Boolean(), nullable=True)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('request_revisions', 'extended') + # ### end Alembic commands ### diff --git a/atst/domain/requests/requests.py b/atst/domain/requests/requests.py index be38880e..c5fc799c 100644 --- a/atst/domain/requests/requests.py +++ b/atst/domain/requests/requests.py @@ -161,7 +161,7 @@ class Requests(object): return Requests.status_count(RequestStatus.APPROVED) @classmethod - def update_financial_verification(cls, request_id, financial_data, task_order=None): + def update_financial_verification(cls, request_id, financial_data, extended=False, task_order=None): request = RequestsQuery.get_with_lock(request_id) # request_data = financial_data.copy() @@ -201,6 +201,7 @@ class Requests(object): ], financial_data, ) + delta = {**delta, "extended": extended} if task_order: request.task_order = task_order diff --git a/atst/forms/financial.py b/atst/forms/financial.py index 55abaf78..b1c4c6b9 100644 --- a/atst/forms/financial.py +++ b/atst/forms/financial.py @@ -182,4 +182,5 @@ class ExtendedFinancialForm(BaseFinancialForm): FileAllowed(["pdf"], "Only PDF documents can be uploaded."), InputRequired(), ], + render_kw={"required": False} ) diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py index 2a47d1c9..1b6ae3d2 100644 --- a/atst/models/mixins/auditable.py +++ b/atst/models/mixins/auditable.py @@ -1,26 +1,14 @@ from sqlalchemy import event from flask import g -import re from atst.models.audit_event import AuditEvent +from atst.utils import camel_to_snake, getattr_path ACTION_CREATE = "create" ACTION_UPDATE = "update" ACTION_DELETE = "delete" -def getattr_path(obj, path, default=None): - _obj = obj - for item in path.split("."): - _obj = getattr(_obj, item, default) - return _obj - - -def camel_to_snake(camel_cased): - s1 = re.sub("(.)([A-Z][a-z]+)", r"\1_\2", camel_cased) - return re.sub("([a-z0-9])([A-Z])", r"\1_\2", s1).lower() - - class AuditableMixin(object): @staticmethod def create_audit_event(connection, resource, action): diff --git a/atst/models/request.py b/atst/models/request.py index 1e1abcac..056fbbfe 100644 --- a/atst/models/request.py +++ b/atst/models/request.py @@ -100,6 +100,7 @@ class Request(Base, mixins.TimestampsMixin, mixins.AuditableMixin): "uii_ids", "treasury_code", "ba_code", + "extended" ] @property @@ -135,7 +136,7 @@ class Request(Base, mixins.TimestampsMixin, mixins.AuditableMixin): @property def financial_verification(self): - return self.body.get("financial_verification") + return self.body.get("financial_verification", {}) @property def is_financially_verified(self): diff --git a/atst/models/request_revision.py b/atst/models/request_revision.py index 1d2da465..8f39d108 100644 --- a/atst/models/request_revision.py +++ b/atst/models/request_revision.py @@ -77,6 +77,7 @@ class RequestRevision(Base, mixins.TimestampsMixin, mixins.AuditableMixin): uii_ids = Column(ARRAY(String)) treasury_code = Column(String) ba_code = Column(String) + extended = Column(Boolean, default=False) def __repr__(self): return "".format( diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index dbafce03..889daf35 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -13,6 +13,11 @@ from atst.domain.requests.financial_verification import ( ) from atst.models.attachment import Attachment from atst.domain.task_orders import TaskOrders +from atst.utils import getattr_path + + +def fv_extended(_http_request): + return bool(_http_request.args.get("extended")) class FinancialVerificationBase(object): @@ -24,7 +29,7 @@ class FinancialVerificationBase(object): task_order_dict.update( { "task_order_number": request.task_order.number, - "funding_type": request.task_order.funding_type.value, + "funding_type": getattr_path(request, "task_order.funding_type.value") } ) existing_fv_data = {**existing_fv_data, **task_order_dict} @@ -51,7 +56,10 @@ class FinancialVerificationBase(object): form.task_order.data, "task_order", self.request.id ) elif isinstance(form.task_order.data, str): - attachment = Attachment.get_for_resource("task_order", self.request.id) + try: + attachment = Attachment.get_for_resource("task_order", self.request.id) + except NotFoundError: + pass if attachment: form.task_order.data = attachment.id @@ -139,7 +147,7 @@ class UpdateFinancialVerification(FinancialVerificationBase): if should_update: task_order = self._try_create_task_order(form, attachment) updated_request = Requests.update_financial_verification( - self.request.id, form.data, task_order=task_order + self.request.id, form.data, extended=self.is_extended, task_order=task_order ) if should_submit: return Requests.submit_financial_verification(updated_request) @@ -186,7 +194,7 @@ class SaveFinancialVerificationDraft(FinancialVerificationBase): attachment = self._process_attachment(self.is_extended, form) task_order = self._try_create_task_order(form, attachment) updated_request = Requests.update_financial_verification( - self.request.id, form.data, task_order=task_order + self.request.id, form.data, extended=self.is_extended, task_order=task_order ) if valid: @@ -198,7 +206,7 @@ class SaveFinancialVerificationDraft(FinancialVerificationBase): @requests_bp.route("/requests/verify/", methods=["GET"]) def financial_verification(request_id): request = Requests.get(g.current_user, request_id) - is_extended = http_request.args.get("extended") + is_extended = fv_extended(http_request) or request.financial_verification.get("extended", False) form = GetFinancialVerificationForm( g.current_user, request, is_extended=is_extended @@ -217,7 +225,7 @@ def financial_verification(request_id): def update_financial_verification(request_id): request = Requests.get(g.current_user, request_id) fv_data = {**http_request.form, **http_request.files} - is_extended = http_request.args.get("extended") + is_extended = fv_extended(http_request) try: updated_request = UpdateFinancialVerification( @@ -251,7 +259,7 @@ def update_financial_verification(request_id): def save_financial_verification_draft(request_id): request = Requests.get(g.current_user, request_id) fv_data = {**http_request.form, **http_request.files} - is_extended = http_request.args.get("extended") + is_extended = fv_extended(http_request) try: SaveFinancialVerificationDraft( diff --git a/atst/utils/__init__.py b/atst/utils/__init__.py index 3923a341..ff48d12b 100644 --- a/atst/utils/__init__.py +++ b/atst/utils/__init__.py @@ -1,3 +1,5 @@ +import re + def first_or_none(predicate, lst): return next((x for x in lst if predicate(x)), None) @@ -18,3 +20,15 @@ def deep_merge(source, destination: dict): return b return _deep_merge(source, dict(destination)) + + +def getattr_path(obj, path, default=None): + _obj = obj + for item in path.split("."): + _obj = getattr(_obj, item, default) + return _obj + + +def camel_to_snake(camel_cased): + s1 = re.sub("(.)([A-Z][a-z]+)", r"\1_\2", camel_cased) + return re.sub("([a-z0-9])([A-Z])", r"\1_\2", s1).lower() diff --git a/tests/domain/test_requests.py b/tests/domain/test_requests.py index fad94973..90e3655d 100644 --- a/tests/domain/test_requests.py +++ b/tests/domain/test_requests.py @@ -143,39 +143,6 @@ request_financial_data = { } -def test_update_financial_verification_without_task_order( - extended_financial_verification_data -): - request = RequestFactory.create() - financial_data = {**request_financial_data, **extended_financial_verification_data} - Requests.update_financial_verification(request.id, financial_data) - assert request.task_order - assert request.task_order.clin_0001 == int( - extended_financial_verification_data["clin_0001"] - ) - assert request.task_order.source == TaskOrderSource.MANUAL - assert request.task_order.pdf - - -def test_update_financial_verification_with_task_order(): - task_order = TaskOrderFactory.create(source=TaskOrderSource.EDA) - 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 - assert "task_order_number" in request.body.get("financial_verification") - assert ( - request.body["financial_verification"]["task_order_number"] - == request_financial_data["task_order_number"] - ) - - def test_set_status_sets_revision(): request = RequestFactory.create() Requests.set_status(request, RequestStatus.APPROVED) From c81ce71eb9226e1fad41049edd8149fd8a965701 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Fri, 19 Oct 2018 14:57:12 -0400 Subject: [PATCH 25/47] Display attachment filename, not id --- atst/routes/requests/financial_verification.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 889daf35..4b799458 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -62,7 +62,7 @@ class FinancialVerificationBase(object): pass if attachment: - form.task_order.data = attachment.id + form.task_order.data = attachment.filename return attachment From 99a61aa9a6400dc6ecc289b2cf871e3dd61e90b3 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Fri, 19 Oct 2018 15:03:53 -0400 Subject: [PATCH 26/47] Formatting --- atst/domain/requests/requests.py | 4 +++- atst/forms/financial.py | 2 +- atst/models/request.py | 2 +- atst/routes/requests/financial_verification.py | 17 +++++++++++++---- atst/utils/__init__.py | 1 + 5 files changed, 19 insertions(+), 7 deletions(-) diff --git a/atst/domain/requests/requests.py b/atst/domain/requests/requests.py index c5fc799c..3bb75d99 100644 --- a/atst/domain/requests/requests.py +++ b/atst/domain/requests/requests.py @@ -161,7 +161,9 @@ class Requests(object): return Requests.status_count(RequestStatus.APPROVED) @classmethod - def update_financial_verification(cls, request_id, financial_data, extended=False, task_order=None): + def update_financial_verification( + cls, request_id, financial_data, extended=False, task_order=None + ): request = RequestsQuery.get_with_lock(request_id) # request_data = financial_data.copy() diff --git a/atst/forms/financial.py b/atst/forms/financial.py index b1c4c6b9..bf0ccf72 100644 --- a/atst/forms/financial.py +++ b/atst/forms/financial.py @@ -182,5 +182,5 @@ class ExtendedFinancialForm(BaseFinancialForm): FileAllowed(["pdf"], "Only PDF documents can be uploaded."), InputRequired(), ], - render_kw={"required": False} + render_kw={"required": False}, ) diff --git a/atst/models/request.py b/atst/models/request.py index 056fbbfe..bb2a62b4 100644 --- a/atst/models/request.py +++ b/atst/models/request.py @@ -100,7 +100,7 @@ class Request(Base, mixins.TimestampsMixin, mixins.AuditableMixin): "uii_ids", "treasury_code", "ba_code", - "extended" + "extended", ] @property diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 4b799458..62d3648b 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -29,7 +29,9 @@ class FinancialVerificationBase(object): task_order_dict.update( { "task_order_number": request.task_order.number, - "funding_type": getattr_path(request, "task_order.funding_type.value") + "funding_type": getattr_path( + request, "task_order.funding_type.value" + ), } ) existing_fv_data = {**existing_fv_data, **task_order_dict} @@ -57,7 +59,9 @@ class FinancialVerificationBase(object): ) elif isinstance(form.task_order.data, str): try: - attachment = Attachment.get_for_resource("task_order", self.request.id) + attachment = Attachment.get_for_resource( + "task_order", self.request.id + ) except NotFoundError: pass @@ -147,7 +151,10 @@ class UpdateFinancialVerification(FinancialVerificationBase): if should_update: task_order = self._try_create_task_order(form, attachment) updated_request = Requests.update_financial_verification( - self.request.id, form.data, extended=self.is_extended, task_order=task_order + self.request.id, + form.data, + extended=self.is_extended, + task_order=task_order, ) if should_submit: return Requests.submit_financial_verification(updated_request) @@ -206,7 +213,9 @@ class SaveFinancialVerificationDraft(FinancialVerificationBase): @requests_bp.route("/requests/verify/", methods=["GET"]) def financial_verification(request_id): request = Requests.get(g.current_user, request_id) - is_extended = fv_extended(http_request) or request.financial_verification.get("extended", False) + is_extended = fv_extended(http_request) or request.financial_verification.get( + "extended", False + ) form = GetFinancialVerificationForm( g.current_user, request, is_extended=is_extended diff --git a/atst/utils/__init__.py b/atst/utils/__init__.py index ff48d12b..51eb9b01 100644 --- a/atst/utils/__init__.py +++ b/atst/utils/__init__.py @@ -1,5 +1,6 @@ import re + def first_or_none(predicate, lst): return next((x for x in lst if predicate(x)), None) From da7b02d4f0647651337d75e352d97d89b4aab9bd Mon Sep 17 00:00:00 2001 From: richard-dds Date: Fri, 19 Oct 2018 15:33:59 -0400 Subject: [PATCH 27/47] Rebase fixes --- .../9c24c609878a_resource_attachments.py | 2 +- atst/domain/requests/requests.py | 24 +------------------ atst/domain/task_orders.py | 4 +--- 3 files changed, 3 insertions(+), 27 deletions(-) diff --git a/alembic/versions/9c24c609878a_resource_attachments.py b/alembic/versions/9c24c609878a_resource_attachments.py index ecb19a91..6a14a228 100644 --- a/alembic/versions/9c24c609878a_resource_attachments.py +++ b/alembic/versions/9c24c609878a_resource_attachments.py @@ -11,7 +11,7 @@ from sqlalchemy.dialects import postgresql # revision identifiers, used by Alembic. revision = '9c24c609878a' -down_revision = '903d7c66ff1d' +down_revision = 'c99026ab9918' branch_labels = None depends_on = None diff --git a/atst/domain/requests/requests.py b/atst/domain/requests/requests.py index 3bb75d99..af838d6d 100644 --- a/atst/domain/requests/requests.py +++ b/atst/domain/requests/requests.py @@ -1,7 +1,5 @@ -from werkzeug.datastructures import FileStorage import dateutil -from atst.domain.task_orders import TaskOrders from atst.domain.workspaces import Workspaces from atst.models.request_revision import RequestRevision from atst.models.request_status_event import RequestStatusEvent, RequestStatus @@ -166,26 +164,6 @@ class Requests(object): ): request = RequestsQuery.get_with_lock(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 TaskOrders.TASK_ORDER_DATA - # } - - # if task_order_data: - # task_order_number = request_data.pop("task_order_number") - # else: - # task_order_number = request_data.get("task_order_number") - - # task_order_file = request_data.pop("task_order", None) - # if isinstance(task_order_file, FileStorage): - # task_order_data["pdf"] = task_order_file - - # task_order = TaskOrders.get_or_create_task_order( - # task_order_number, task_order_data - # ) - delta = pick( [ "uii_ids", @@ -203,7 +181,7 @@ class Requests(object): ], financial_data, ) - delta = {**delta, "extended": extended} + delta["extended"] = extended if task_order: request.task_order = task_order diff --git a/atst/domain/task_orders.py b/atst/domain/task_orders.py index afe4b6a0..c01a38f9 100644 --- a/atst/domain/task_orders.py +++ b/atst/domain/task_orders.py @@ -30,9 +30,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, - funding_type=FundingType.PROC, + source=Source.EDA, funding_type=FundingType.PROC, **to_data ) else: From 68a18b834ec61ad6513977e5ae2cb7a3c1913e04 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 22 Oct 2018 09:52:35 -0400 Subject: [PATCH 28/47] Use param, not attribute --- atst/routes/requests/financial_verification.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 62d3648b..21b7b987 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -50,7 +50,7 @@ class FinancialVerificationBase(object): def _process_attachment(self, is_extended, form): attachment = None - if self.is_extended: + if is_extended: attachment = None if isinstance(form.task_order.data, FileStorage): Attachment.delete_for_resource("task_order", self.request.id) From a1735874e63c5086106dbb85af3791aaa7715d29 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 22 Oct 2018 10:17:04 -0400 Subject: [PATCH 29/47] Determine if finver form should be extended by checking TO --- alembic/versions/edf509c974f6_extended_fv.py | 28 ------------------- atst/domain/requests/requests.py | 5 +--- atst/models/request.py | 10 ++++++- atst/models/request_revision.py | 1 - .../routes/requests/financial_verification.py | 19 +++++++------ tests/routes/test_financial_verification.py | 19 +++++++++++++ 6 files changed, 39 insertions(+), 43 deletions(-) delete mode 100644 alembic/versions/edf509c974f6_extended_fv.py diff --git a/alembic/versions/edf509c974f6_extended_fv.py b/alembic/versions/edf509c974f6_extended_fv.py deleted file mode 100644 index 8877f659..00000000 --- a/alembic/versions/edf509c974f6_extended_fv.py +++ /dev/null @@ -1,28 +0,0 @@ -"""extended fv - -Revision ID: edf509c974f6 -Revises: 9c24c609878a -Create Date: 2018-10-19 14:06:45.396974 - -""" -from alembic import op -import sqlalchemy as sa - - -# revision identifiers, used by Alembic. -revision = 'edf509c974f6' -down_revision = '9c24c609878a' -branch_labels = None -depends_on = None - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.add_column('request_revisions', sa.Column('extended', sa.Boolean(), nullable=True)) - # ### end Alembic commands ### - - -def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.drop_column('request_revisions', 'extended') - # ### end Alembic commands ### diff --git a/atst/domain/requests/requests.py b/atst/domain/requests/requests.py index af838d6d..028739c9 100644 --- a/atst/domain/requests/requests.py +++ b/atst/domain/requests/requests.py @@ -159,9 +159,7 @@ class Requests(object): return Requests.status_count(RequestStatus.APPROVED) @classmethod - def update_financial_verification( - cls, request_id, financial_data, extended=False, task_order=None - ): + def update_financial_verification(cls, request_id, financial_data, task_order=None): request = RequestsQuery.get_with_lock(request_id) delta = pick( @@ -181,7 +179,6 @@ class Requests(object): ], financial_data, ) - delta["extended"] = extended if task_order: request.task_order = task_order diff --git a/atst/models/request.py b/atst/models/request.py index bb2a62b4..443f3c9b 100644 --- a/atst/models/request.py +++ b/atst/models/request.py @@ -6,6 +6,7 @@ from atst.models import Base, types, mixins from atst.models.request_status_event import RequestStatus from atst.utils import first_or_none from atst.models.request_revision import RequestRevision +from atst.models.task_order import Source as TaskOrderSource def map_properties_to_dict(properties, instance): @@ -100,7 +101,6 @@ class Request(Base, mixins.TimestampsMixin, mixins.AuditableMixin): "uii_ids", "treasury_code", "ba_code", - "extended", ] @property @@ -229,6 +229,14 @@ class Request(Base, mixins.TimestampsMixin, mixins.AuditableMixin): def pe_number(self): return self.body.get("financial_verification", {}).get("pe_id") + @property + def has_manual_task_order(self): + return ( + self.task_order.source == TaskOrderSource.MANUAL + if self.task_order is not None + else None + ) + def __repr__(self): return "".format( self.status_displayname, diff --git a/atst/models/request_revision.py b/atst/models/request_revision.py index 8f39d108..1d2da465 100644 --- a/atst/models/request_revision.py +++ b/atst/models/request_revision.py @@ -77,7 +77,6 @@ class RequestRevision(Base, mixins.TimestampsMixin, mixins.AuditableMixin): uii_ids = Column(ARRAY(String)) treasury_code = Column(String) ba_code = Column(String) - extended = Column(Boolean, default=False) def __repr__(self): return "".format( diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 21b7b987..bc052c4d 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -17,7 +17,7 @@ from atst.utils import getattr_path def fv_extended(_http_request): - return bool(_http_request.args.get("extended")) + return _http_request.args.get("extended", "false").lower() in ["true", "t"] class FinancialVerificationBase(object): @@ -151,10 +151,7 @@ class UpdateFinancialVerification(FinancialVerificationBase): if should_update: task_order = self._try_create_task_order(form, attachment) updated_request = Requests.update_financial_verification( - self.request.id, - form.data, - extended=self.is_extended, - task_order=task_order, + self.request.id, form.data, task_order=task_order ) if should_submit: return Requests.submit_financial_verification(updated_request) @@ -201,7 +198,7 @@ class SaveFinancialVerificationDraft(FinancialVerificationBase): attachment = self._process_attachment(self.is_extended, form) task_order = self._try_create_task_order(form, attachment) updated_request = Requests.update_financial_verification( - self.request.id, form.data, extended=self.is_extended, task_order=task_order + self.request.id, form.data, task_order=task_order ) if valid: @@ -213,9 +210,13 @@ class SaveFinancialVerificationDraft(FinancialVerificationBase): @requests_bp.route("/requests/verify/", methods=["GET"]) def financial_verification(request_id): request = Requests.get(g.current_user, request_id) - is_extended = fv_extended(http_request) or request.financial_verification.get( - "extended", False - ) + is_extended = fv_extended(http_request) + + should_be_extended = not is_extended and request.has_manual_task_order + if should_be_extended: + return redirect( + url_for(".financial_verification", request_id=request_id, extended=True) + ) form = GetFinancialVerificationForm( g.current_user, request, is_extended=is_extended diff --git a/tests/routes/test_financial_verification.py b/tests/routes/test_financial_verification.py index 59f8f95c..5a492eed 100644 --- a/tests/routes/test_financial_verification.py +++ b/tests/routes/test_financial_verification.py @@ -263,3 +263,22 @@ 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): + user = UserFactory.create() + request = RequestFactory.create(creator=user) + + data = {**fv_data, "task_order_number": "DCA10096D0053"} + + UpdateFinancialVerification( + TrueValidator, TrueValidator, user, request, data, is_extended=False + ).execute() + + user_session(user) + response = client.get( + url_for("requests.financial_verification", request_id=request.id), + data=fv_data, + follow_redirects=False, + ) + assert "extended" in response.headers["Location"] From 39bbfb745ad0567db8cee25fbb831bb4adb42266 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 22 Oct 2018 16:16:58 -0400 Subject: [PATCH 30/47] Fix task order creation --- atst/domain/requests/requests.py | 12 +++--- atst/domain/task_orders.py | 11 +++++- .../routes/requests/financial_verification.py | 38 ++++++++++++++----- atst/utils/__init__.py | 22 ++++++++++- tests/domain/test_task_orders.py | 10 ----- tests/routes/test_financial_verification.py | 17 +++++++++ 6 files changed, 81 insertions(+), 29 deletions(-) diff --git a/atst/domain/requests/requests.py b/atst/domain/requests/requests.py index 028739c9..1f8adf12 100644 --- a/atst/domain/requests/requests.py +++ b/atst/domain/requests/requests.py @@ -5,17 +5,12 @@ from atst.models.request_revision import RequestRevision from atst.models.request_status_event import RequestStatusEvent, RequestStatus from atst.models.request_review import RequestReview from atst.models.request_internal_comment import RequestInternalComment -from atst.utils import deep_merge +from atst.utils import deep_merge, pick from .query import RequestsQuery from .authorization import RequestsAuthorization -def pick(keys, d): - _keys = set(keys) - return {k: v for (k, v) in d.items() if k in _keys} - - def create_revision_from_request_body(body): body = {k: v for p in body.values() for k, v in p.items()} DATES = ["start_date", "date_latest_training"] @@ -79,7 +74,10 @@ class Requests(object): @classmethod def update(cls, request_id, request_delta): request = RequestsQuery.get_with_lock(request_id) + return Requests._update(request, request_delta) + @classmethod + def _update(cls, request, request_delta): new_body = deep_merge(request_delta, request.body) revision = create_revision_from_request_body(new_body) request.revisions.append(revision) @@ -183,7 +181,7 @@ class Requests(object): if task_order: request.task_order = task_order - request = Requests.update(request.id, {"financial_verification": delta}) + request = Requests._update(request, {"financial_verification": delta}) return request diff --git a/atst/domain/task_orders.py b/atst/domain/task_orders.py index c01a38f9..539c4682 100644 --- a/atst/domain/task_orders.py +++ b/atst/domain/task_orders.py @@ -5,6 +5,7 @@ from atst.database import db from atst.models.task_order import TaskOrder, Source, FundingType from atst.models.attachment import Attachment from .exceptions import NotFoundError +from atst.utils import drop, update_obj class TaskOrders(object): @@ -38,7 +39,8 @@ class TaskOrders(object): @classmethod def create(cls, **kwargs): - task_order = TaskOrder(**kwargs) + to_data = drop(["source"], kwargs) + task_order = TaskOrder(source=Source.MANUAL, **to_data) db.session.add(task_order) db.session.commit() @@ -75,3 +77,10 @@ class TaskOrders(object): return TaskOrders.create( **data, number=number, pdf=attachment, source=Source.MANUAL ) + + @classmethod + def update(cls, task_order, dct): + updated = update_obj(task_order, dct) + db.session.add(updated) + db.session.commit() + return updated diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index bc052c4d..63064a53 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -13,7 +13,7 @@ from atst.domain.requests.financial_verification import ( ) from atst.models.attachment import Attachment from atst.domain.task_orders import TaskOrders -from atst.utils import getattr_path +from atst.utils import getattr_path, update_obj def fv_extended(_http_request): @@ -73,15 +73,33 @@ class FinancialVerificationBase(object): def _try_create_task_order(self, form, attachment): form_data = form.data - task_order_number = form_data.get("task_order_number") - if task_order_number: - task_order_data = { - k: v for (k, v) in form_data.items() if k in TaskOrders.TASK_ORDER_DATA - } - return TaskOrders.get_or_create( - task_order_number, attachment=attachment, data=task_order_data - ) - return None + task_order_number = form_data.pop("task_order_number") + if task_order_number is None: + return None + + task_order_data = { + k: v for (k, v) in form_data.items() if k in TaskOrders.TASK_ORDER_DATA + } + task_order_data["number"] = task_order_number + funding_type = getattr_path(form_data, "funding_type.data") + task_order_data["funding_type"] = funding_type if funding_type != "" else None + + if attachment: + task_order_data["pdf"] = attachment + + try: + task_order = TaskOrders.get(task_order_number) + task_order = TaskOrders.update(task_order, task_order_data) + return task_order + except NotFoundError: + pass + + try: + return TaskOrders._get_from_eda(task_order_number) + except NotFoundError: + pass + + return TaskOrders.create(**task_order_data) def _apply_pe_number_error(self, field): suggestion = self.pe_validator.suggest_pe_id(field.data) diff --git a/atst/utils/__init__.py b/atst/utils/__init__.py index 51eb9b01..d5c846e3 100644 --- a/atst/utils/__init__.py +++ b/atst/utils/__init__.py @@ -26,10 +26,30 @@ def deep_merge(source, destination: dict): def getattr_path(obj, path, default=None): _obj = obj for item in path.split("."): - _obj = getattr(_obj, item, default) + if isinstance(_obj, dict): + _obj = _obj.get(item) + else: + _obj = getattr(_obj, item, default) return _obj +def update_obj(obj, dct): + for k, v in dct.items(): + if hasattr(obj, k) and v is not None: + setattr(obj, k, v) + return obj + + def camel_to_snake(camel_cased): s1 = re.sub("(.)([A-Z][a-z]+)", r"\1_\2", camel_cased) return re.sub("([a-z0-9])([A-Z])", r"\1_\2", s1).lower() + + +def drop(keys, dct): + _keys = set(keys) + return {k: v for k, v in dct.items() if k not in _keys} + + +def pick(keys, dct): + _keys = set(keys) + return {k: v for (k, v) in dct.items() if k in _keys} diff --git a/tests/domain/test_task_orders.py b/tests/domain/test_task_orders.py index cc83d284..ebdbf4d3 100644 --- a/tests/domain/test_task_orders.py +++ b/tests/domain/test_task_orders.py @@ -15,16 +15,6 @@ def test_can_get_task_order(): assert to.id == to.id -def test_can_get_task_order_from_eda(monkeypatch): - monkeypatch.setattr( - "atst.domain.task_orders.TaskOrders._client", lambda: MockEDAClient() - ) - 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(): with pytest.raises(NotFoundError): TaskOrders.get("some fake number") diff --git a/tests/routes/test_financial_verification.py b/tests/routes/test_financial_verification.py index 5a492eed..0b2c10c4 100644 --- a/tests/routes/test_financial_verification.py +++ b/tests/routes/test_financial_verification.py @@ -226,6 +226,23 @@ def test_can_save_draft_with_just_pdf(extended_financial_verification_data): assert form.task_order +def test_task_order_info_present_in_extended_form( + fv_data, extended_financial_verification_data +): + request = RequestFactory.create() + user = UserFactory.create() + data = { + "clin_0001": extended_financial_verification_data["clin_0001"], + "task_order_number": fv_data["task_order_number"], + } + SaveFinancialVerificationDraft( + TrueValidator, TrueValidator, user, request, data, is_extended=True + ).execute() + + form = GetFinancialVerificationForm(user, request, is_extended=True).execute() + assert form.clin_0001.data + + def test_update_fv_route(client, user_session, fv_data): user = UserFactory.create() request = RequestFactory.create(creator=user) From a84954799d16a60debd86015dd164da509505da2 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 22 Oct 2018 17:07:38 -0400 Subject: [PATCH 31/47] Fixes --- .../routes/requests/financial_verification.py | 6 ++-- tests/routes/test_financial_verification.py | 31 +++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 63064a53..47d0622d 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -13,7 +13,7 @@ from atst.domain.requests.financial_verification import ( ) from atst.models.attachment import Attachment from atst.domain.task_orders import TaskOrders -from atst.utils import getattr_path, update_obj +from atst.utils import getattr_path def fv_extended(_http_request): @@ -74,14 +74,14 @@ class FinancialVerificationBase(object): form_data = form.data task_order_number = form_data.pop("task_order_number") - if task_order_number is None: + if not task_order_number: return None task_order_data = { k: v for (k, v) in form_data.items() if k in TaskOrders.TASK_ORDER_DATA } task_order_data["number"] = task_order_number - funding_type = getattr_path(form_data, "funding_type.data") + funding_type = form_data.get("funding_type") task_order_data["funding_type"] = funding_type if funding_type != "" else None if attachment: diff --git a/tests/routes/test_financial_verification.py b/tests/routes/test_financial_verification.py index 0b2c10c4..aea6f95b 100644 --- a/tests/routes/test_financial_verification.py +++ b/tests/routes/test_financial_verification.py @@ -22,6 +22,7 @@ from atst.domain.requests.financial_verification import ( PENumberValidator, TaskOrderNumberValidator, ) +from atst.utils import pick @pytest.fixture @@ -243,6 +244,36 @@ def test_task_order_info_present_in_extended_form( assert form.clin_0001.data +def test_simple_form_does_not_generate_task_order(fv_data): + request = RequestFactory.create() + user = UserFactory.create() + data = pick(["uii_ids"], fv_data) + updated_request = SaveFinancialVerificationDraft( + TrueValidator, TrueValidator, user, request, data, is_extended=False + ).execute() + + assert updated_request.task_order is None + + +def test_can_save_draft_with_funding_type( + fv_data, extended_financial_verification_data +): + request = RequestFactory.create() + user = UserFactory.create() + data = { + "task_order_number": fv_data["task_order_number"], + "funding_type": extended_financial_verification_data["funding_type"], + } + updated_request = SaveFinancialVerificationDraft( + TrueValidator, TrueValidator, user, request, data, is_extended=False + ).execute() + + import ipdb + + ipdb.set_trace() + assert updated_request.task_order.funding_type + + def test_update_fv_route(client, user_session, fv_data): user = UserFactory.create() request = RequestFactory.create(creator=user) From 1606bad0169c5d3f9100501ed0bbd78d8735b494 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 22 Oct 2018 17:34:57 -0400 Subject: [PATCH 32/47] Allow FV to be submitted without newly uploaded pdf --- atst/forms/financial.py | 16 +++++++++++++++- atst/routes/requests/financial_verification.py | 7 ++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/atst/forms/financial.py b/atst/forms/financial.py index bf0ccf72..8a8ecc31 100644 --- a/atst/forms/financial.py +++ b/atst/forms/financial.py @@ -29,6 +29,9 @@ class BaseFinancialForm(ValidatedForm): """ self.uii_ids.process_data(self.uii_ids.data) + def validate(self, **kwargs): + return super().validate() + def validate_draft(self): """ Another stupid workaround. Maybe there isn't a better way. @@ -109,7 +112,18 @@ class ExtendedFinancialForm(BaseFinancialForm): def validate(self, *args, **kwargs): if self.funding_type.data == "OTHER": self.funding_type_other.validators.append(InputRequired()) - return super().validate(*args, **kwargs) + + to_validator = None + if kwargs.get("has_attachment"): + to_validators = list(self.task_order.validators) + self.task_order.validators = [] + + valid = super().validate(*args, **kwargs) + + if to_validator: + self.task_order.validators = to_validators + + return valid funding_type = SelectField( description="What is the source of funding?", diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 47d0622d..bbd01a04 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -21,6 +21,7 @@ def fv_extended(_http_request): class FinancialVerificationBase(object): + def _get_form(self, request, is_extended, formdata=None): existing_fv_data = request.financial_verification @@ -153,7 +154,9 @@ class UpdateFinancialVerification(FinancialVerificationBase): should_submit = True updated_request = None - if not form.validate(): + attachment = self._process_attachment(self.is_extended, form) + + if not form.validate(has_attachment=attachment): should_update = False if not self.pe_validator.validate(self.request, form.pe_id.data): @@ -164,8 +167,6 @@ class UpdateFinancialVerification(FinancialVerificationBase): self._apply_task_order_number_error(form.task_order_number) should_submit = False - attachment = self._process_attachment(self.is_extended, form) - if should_update: task_order = self._try_create_task_order(form, attachment) updated_request = Requests.update_financial_verification( From 0439525f0f8d9bfa540facca8dae854e6a496c66 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 22 Oct 2018 22:16:39 -0400 Subject: [PATCH 33/47] Refactor fv form to separate TO from request --- atst/domain/requests/requests.py | 25 +-- atst/domain/task_orders.py | 37 +--- atst/forms/financial.py | 192 ++++++++++-------- .../routes/requests/financial_verification.py | 66 +++--- js/components/forms/financial.js | 2 +- .../requests/financial_verification.html | 54 ++--- tests/domain/test_task_orders.py | 8 - tests/forms/test_financial.py | 154 +++++++------- tests/routes/test_financial_verification.py | 86 ++++---- 9 files changed, 295 insertions(+), 329 deletions(-) diff --git a/atst/domain/requests/requests.py b/atst/domain/requests/requests.py index 1f8adf12..223d9d5e 100644 --- a/atst/domain/requests/requests.py +++ b/atst/domain/requests/requests.py @@ -82,9 +82,7 @@ class Requests(object): revision = create_revision_from_request_body(new_body) request.revisions.append(revision) - request = RequestsQuery.add_and_commit(request) - - return request + return RequestsQuery.add_and_commit(request) @classmethod def approve_and_create_workspace(cls, request): @@ -160,29 +158,10 @@ class Requests(object): def update_financial_verification(cls, request_id, financial_data, task_order=None): request = RequestsQuery.get_with_lock(request_id) - delta = pick( - [ - "uii_ids", - "pe_id", - "treasury_code", - "ba_code", - "fname_co", - "lname_co", - "email_co", - "office_co", - "fname_cor", - "lname_cor", - "email_cor", - "office_cor", - ], - financial_data, - ) - if task_order: request.task_order = task_order - request = Requests._update(request, {"financial_verification": delta}) - + request = Requests._update(request, {"financial_verification": financial_data}) return request @classmethod diff --git a/atst/domain/task_orders.py b/atst/domain/task_orders.py index 539c4682..a0ba06e8 100644 --- a/atst/domain/task_orders.py +++ b/atst/domain/task_orders.py @@ -19,14 +19,14 @@ class TaskOrders(object): ) except NoResultFound: if TaskOrders._client(): - task_order = TaskOrders._get_from_eda(order_number) + task_order = TaskOrders.get_from_eda(order_number) else: raise NotFoundError("task_order") return task_order @classmethod - def _get_from_eda(cls, order_number): + def get_from_eda(cls, order_number): to_data = TaskOrders._client().get_contract(order_number, status="y") if to_data: # TODO: we need to determine exactly what we're getting and storing from the EDA client @@ -38,9 +38,9 @@ class TaskOrders(object): raise NotFoundError("task_order") @classmethod - def create(cls, **kwargs): - to_data = drop(["source"], kwargs) - task_order = TaskOrder(source=Source.MANUAL, **to_data) + def create(cls, source=Source.MANUAL, **kwargs): + to_data = {k: v for k, v in kwargs.items() if v not in ["", None]} + task_order = TaskOrder(source=source, **to_data) db.session.add(task_order) db.session.commit() @@ -51,33 +51,6 @@ class TaskOrders(object): 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: - pdf_file = task_order_data.pop("pdf") - # should catch the error here - attachment = Attachment.attach(pdf_file) - return TaskOrders.create( - **task_order_data, - number=number, - source=Source.MANUAL, - pdf=attachment, - ) - - @classmethod - def get_or_create(cls, number, attachment=None, data=None): - try: - return TaskOrders.get(number) - except NotFoundError: - data = data or {} - return TaskOrders.create( - **data, number=number, pdf=attachment, source=Source.MANUAL - ) - @classmethod def update(cls, task_order, dct): updated = update_obj(task_order, dct) diff --git a/atst/forms/financial.py b/atst/forms/financial.py index 8a8ecc31..b626b895 100644 --- a/atst/forms/financial.py +++ b/atst/forms/financial.py @@ -1,7 +1,7 @@ import re import pendulum from wtforms.fields.html5 import DateField, EmailField -from wtforms.fields import StringField, FileField +from wtforms.fields import StringField, FileField, FormField from wtforms.validators import InputRequired, Email, Regexp, Optional from flask_wtf.file import FileAllowed @@ -21,17 +21,16 @@ def number_to_int(num): return int(num) -class BaseFinancialForm(ValidatedForm): - def reset(self): - """ - Reset UII info so that it can be de-parsed rendered properly. - This is a stupid workaround, and there's probably a better way. - """ - self.uii_ids.process_data(self.uii_ids.data) +def coerce_choice(val): + if val is None: + return None + elif isinstance(val, str): + return val + else: + return val.value - def validate(self, **kwargs): - return super().validate() +class DraftValidateMixin(object): def validate_draft(self): """ Another stupid workaround. Maybe there isn't a better way. @@ -49,86 +48,19 @@ class BaseFinancialForm(ValidatedForm): return valid - @property - def is_missing_task_order_number(self): - return False - task_order_number = StringField( +class TaskOrderForm(ValidatedForm, DraftValidateMixin): + 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.", validators=[InputRequired()], ) - uii_ids = NewlineListField( - "Unique Item Identifier (UII)s related to your application(s) if you already have them.", - description="If you have more than one UII, place each one on a new line.", - ) - - pe_id = StringField( - "Program Element Number", - description="PE numbers help the Department of Defense identify which offices' budgets are contributing towards this resource use.
It should be 7 digits followed by 1-3 letters, and should have a zero as the first and third digits.", - validators=[InputRequired()], - ) - - treasury_code = StringField( - "Program Treasury Code", - description="Program Treasury Code (or Appropriations Code) identifies resource types.
It should be a four digit or six digit number, optionally prefixed by one or more zeros.", - validators=[InputRequired(), Regexp(TREASURY_CODE_REGEX)], - ) - - ba_code = StringField( - "Program Budget Activity (BA) Code", - description="BA Code is used to identify the purposes, projects, or types of activities financed by the appropriation fund.
It should be two digits, followed by an optional letter.", - validators=[InputRequired(), Regexp(BA_CODE_REGEX)], - ) - - fname_co = StringField("KO First Name", validators=[InputRequired()]) - lname_co = StringField("KO Last Name", validators=[InputRequired()]) - - email_co = EmailField("KO Email", validators=[InputRequired(), Email()]) - - office_co = StringField("KO Office", validators=[InputRequired()]) - - fname_cor = StringField("COR First Name", validators=[InputRequired()]) - - lname_cor = StringField("COR Last Name", validators=[InputRequired()]) - - email_cor = EmailField("COR Email", validators=[InputRequired(), Email()]) - - office_cor = StringField("COR Office", validators=[InputRequired()]) - - -class FinancialForm(BaseFinancialForm): - @property - def is_missing_task_order_number(self): - return "task_order_number" in self.errors - - @property - def is_only_missing_task_order_number(self): - return "task_order_number" in self.errors and len(self.errors) == 1 - - -class ExtendedFinancialForm(BaseFinancialForm): - def validate(self, *args, **kwargs): - if self.funding_type.data == "OTHER": - self.funding_type_other.validators.append(InputRequired()) - - to_validator = None - if kwargs.get("has_attachment"): - to_validators = list(self.task_order.validators) - self.task_order.validators = [] - - valid = super().validate(*args, **kwargs) - - if to_validator: - self.task_order.validators = to_validators - - return valid - funding_type = SelectField( description="What is the source of funding?", choices=FUNDING_TYPES, validators=[InputRequired()], + coerce=coerce_choice, render_kw={"required": False}, ) @@ -190,7 +122,7 @@ class ExtendedFinancialForm(BaseFinancialForm): filters=[number_to_int], ) - task_order = FileField( + pdf = FileField( "Upload a copy of your Task Order", validators=[ FileAllowed(["pdf"], "Only PDF documents can be uploaded."), @@ -198,3 +130,101 @@ class ExtendedFinancialForm(BaseFinancialForm): ], render_kw={"required": False}, ) + + +class RequestFinancialVerificationForm(ValidatedForm, DraftValidateMixin): + uii_ids = NewlineListField( + "Unique Item Identifier (UII)s related to your application(s) if you already have them.", + description="If you have more than one UII, place each one on a new line.", + ) + + pe_id = StringField( + "Program Element Number", + description="PE numbers help the Department of Defense identify which offices' budgets are contributing towards this resource use.
It should be 7 digits followed by 1-3 letters, and should have a zero as the first and third digits.", + validators=[InputRequired()], + ) + + treasury_code = StringField( + "Program Treasury Code", + description="Program Treasury Code (or Appropriations Code) identifies resource types.
It should be a four digit or six digit number, optionally prefixed by one or more zeros.", + validators=[InputRequired(), Regexp(TREASURY_CODE_REGEX)], + ) + + ba_code = StringField( + "Program Budget Activity (BA) Code", + description="BA Code is used to identify the purposes, projects, or types of activities financed by the appropriation fund.
It should be two digits, followed by an optional letter.", + validators=[InputRequired(), Regexp(BA_CODE_REGEX)], + ) + + fname_co = StringField("KO First Name", validators=[InputRequired()]) + lname_co = StringField("KO Last Name", validators=[InputRequired()]) + + email_co = EmailField("KO Email", validators=[InputRequired(), Email()]) + + office_co = StringField("KO Office", validators=[InputRequired()]) + + fname_cor = StringField("COR First Name", validators=[InputRequired()]) + + lname_cor = StringField("COR Last Name", validators=[InputRequired()]) + + email_cor = EmailField("COR Email", validators=[InputRequired(), Email()]) + + office_cor = StringField("COR Office", validators=[InputRequired()]) + + def reset(self): + """ + Reset UII info so that it can be de-parsed rendered properly. + This is a stupid workaround, and there's probably a better way. + """ + self.uii_ids.process_data(self.uii_ids.data) + + +class FinancialVerificationForm(ValidatedForm): + + task_order = FormField(TaskOrderForm) + request = FormField(RequestFinancialVerificationForm) + + def validate(self, *args, **kwargs): + if self.task_order.funding_type.data == "OTHER": + self.task_order.funding_type_other.validators.append(InputRequired()) + + to_number_validators = None + if kwargs.get("has_attachment"): + to_number_validators = list(self.task_order.number.validators) + self.task_order.number.validators = [] + + valid = super().validate() + + if to_number_validators: + self.task_order.number.validators = to_number_validators + + return valid + + def do_validate_request(self): + """ + Called do_validate_request to avoid being considered an inline + validator by wtforms. + """ + return self.request.validate(self) + + def validate_draft(self): + return self.task_order.validate_draft() and self.request.validate_draft() + + def reset(self): + self.request.reset() + + @property + def pe_id(self): + return self.request.pe_id + + @property + def task_order_number(self): + return self.task_order.number + + @property + def is_missing_task_order_number(self): + return "number" in self.errors.get("task_order", {}) + + @property + def is_only_missing_task_order_number(self): + return "task_order_number" in self.errors and len(self.errors) == 1 diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index bbd01a04..2e4b2d90 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -4,7 +4,7 @@ from werkzeug.datastructures import ImmutableMultiDict, FileStorage from . import requests_bp from atst.domain.requests import Requests -from atst.forms.financial import FinancialForm, ExtendedFinancialForm +from atst.forms.financial import FinancialVerificationForm from atst.forms.exceptions import FormValidationError from atst.domain.exceptions import NotFoundError from atst.domain.requests.financial_verification import ( @@ -13,52 +13,42 @@ from atst.domain.requests.financial_verification import ( ) from atst.models.attachment import Attachment from atst.domain.task_orders import TaskOrders -from atst.utils import getattr_path def fv_extended(_http_request): return _http_request.args.get("extended", "false").lower() in ["true", "t"] +class FinancialVerification(object): + def __init__(self, request): + self.request = request.latest_revision + self.task_order = request.task_order + + class FinancialVerificationBase(object): - def _get_form(self, request, is_extended, formdata=None): - existing_fv_data = request.financial_verification - - if request.task_order: - task_order_dict = request.task_order.to_dictionary() - task_order_dict.update( - { - "task_order_number": request.task_order.number, - "funding_type": getattr_path( - request, "task_order.funding_type.value" - ), - } - ) - existing_fv_data = {**existing_fv_data, **task_order_dict} - - mdict = ImmutableMultiDict(formdata) if formdata is not None else None + _formdata = ImmutableMultiDict(formdata) if formdata is not None else None + fv = FinancialVerification(request) + form = FinancialVerificationForm(obj=fv, formdata=_formdata) if is_extended: try: attachment = Attachment.get_for_resource("task_order", self.request.id) - existing_fv_data["task_order"] = attachment.filename + form.task_order.pdf.data = attachment.filename except NotFoundError: pass - return ExtendedFinancialForm(formdata=mdict, data=existing_fv_data) - else: - return FinancialForm(formdata=mdict, data=existing_fv_data) + return form def _process_attachment(self, is_extended, form): attachment = None if is_extended: attachment = None - if isinstance(form.task_order.data, FileStorage): + if isinstance(form.task_order.pdf.data, FileStorage): Attachment.delete_for_resource("task_order", self.request.id) attachment = Attachment.attach( - form.task_order.data, "task_order", self.request.id + form.task_order.pdf.data, "task_order", self.request.id ) - elif isinstance(form.task_order.data, str): + elif isinstance(form.task_order.pdf.data, str): try: attachment = Attachment.get_for_resource( "task_order", self.request.id @@ -67,23 +57,16 @@ class FinancialVerificationBase(object): pass if attachment: - form.task_order.data = attachment.filename + form.task_order.pdf.data = attachment.filename return attachment def _try_create_task_order(self, form, attachment): - form_data = form.data - - task_order_number = form_data.pop("task_order_number") + task_order_number = form.task_order.number.data if not task_order_number: return None - task_order_data = { - k: v for (k, v) in form_data.items() if k in TaskOrders.TASK_ORDER_DATA - } - task_order_data["number"] = task_order_number - funding_type = form_data.get("funding_type") - task_order_data["funding_type"] = funding_type if funding_type != "" else None + task_order_data = form.task_order.data if attachment: task_order_data["pdf"] = attachment @@ -96,7 +79,7 @@ class FinancialVerificationBase(object): pass try: - return TaskOrders._get_from_eda(task_order_number) + return TaskOrders.get_from_eda(task_order_number) except NotFoundError: pass @@ -156,8 +139,11 @@ class UpdateFinancialVerification(FinancialVerificationBase): attachment = self._process_attachment(self.is_extended, form) - if not form.validate(has_attachment=attachment): - should_update = False + if self.is_extended: + 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): self._apply_pe_number_error(form.pe_id) @@ -170,7 +156,7 @@ class UpdateFinancialVerification(FinancialVerificationBase): if should_update: task_order = self._try_create_task_order(form, attachment) updated_request = Requests.update_financial_verification( - self.request.id, form.data, task_order=task_order + self.request.id, form.request.data, task_order=task_order ) if should_submit: return Requests.submit_financial_verification(updated_request) @@ -217,7 +203,7 @@ class SaveFinancialVerificationDraft(FinancialVerificationBase): attachment = self._process_attachment(self.is_extended, form) task_order = self._try_create_task_order(form, attachment) updated_request = Requests.update_financial_verification( - self.request.id, form.data, task_order=task_order + self.request.id, form.request.data, task_order=task_order ) if valid: diff --git a/js/components/forms/financial.js b/js/components/forms/financial.js index febe8d47..d2c41f17 100644 --- a/js/components/forms/financial.js +++ b/js/components/forms/financial.js @@ -32,7 +32,7 @@ export default { computed: { showTaskOrder: function() { - return !this.initialData.task_order || this.shouldForceShowTaskOrder + return this.initialData.task_order.number || this.shouldForceShowTaskOrder } }, diff --git a/templates/requests/financial_verification.html b/templates/requests/financial_verification.html index 59a05c5d..f81c803e 100644 --- a/templates/requests/financial_verification.html +++ b/templates/requests/financial_verification.html @@ -68,55 +68,55 @@ {% if extended %}
- {{ OptionsInput(f.funding_type) }} + {{ OptionsInput(f.task_order.funding_type) }} - {{ DateInput(f.expiration_date, placeholder='MM / DD / YYYY', validation='date', tooltip='Please enter the expiration date for the task order only and do not include options that you may choose to exercise in the future.') }} + {{ DateInput(f.task_order.expiration_date, placeholder='MM / DD / YYYY', validation='date', tooltip='Please enter the expiration date for the task order only and do not include options that you may choose to exercise in the future.') }} {{ TextInput( - f.clin_0001, + f.task_order.clin_0001, validation='dollars' ) }} {{ TextInput( - f.clin_0003, + f.task_order.clin_0003, validation='dollars' ) }} {{ TextInput( - f.clin_1001, + f.task_order.clin_1001, validation='dollars' ) }} {{ TextInput( - f.clin_1003, + f.task_order.clin_1003, validation='dollars' ) }} {{ TextInput( - f.clin_2001, + f.task_order.clin_2001, validation='dollars' ) }} {{ TextInput( - f.clin_2003, + f.task_order.clin_2003, validation='dollars' ) }}