From 0439525f0f8d9bfa540facca8dae854e6a496c66 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 22 Oct 2018 22:16:39 -0400 Subject: [PATCH] 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' ) }}