diff --git a/atst/domain/workspace_roles.py b/atst/domain/workspace_roles.py index c99b7390..131cbac9 100644 --- a/atst/domain/workspace_roles.py +++ b/atst/domain/workspace_roles.py @@ -2,7 +2,9 @@ from sqlalchemy.orm.exc import NoResultFound from atst.database import db from atst.models.workspace_role import ( - WorkspaceRole, Status as WorkspaceRoleStatus, MEMBER_STATUSES + WorkspaceRole, + Status as WorkspaceRoleStatus, + MEMBER_STATUSES, ) from atst.models.user import User @@ -12,8 +14,7 @@ from .exceptions import NotFoundError MEMBER_STATUS_CHOICES = [ - dict(name=key, display_name=value) - for key, value in MEMBER_STATUSES.items() + dict(name=key, display_name=value) for key, value in MEMBER_STATUSES.items() ] diff --git a/atst/forms/financial.py b/atst/forms/financial.py index 7ee17cfa..56197262 100644 --- a/atst/forms/financial.py +++ b/atst/forms/financial.py @@ -219,7 +219,7 @@ class FinancialVerificationForm(CacheableForm): return self.task_order.number @property - def has_task_order_pdf(self): + def has_pdf_upload(self): return isinstance(self.task_order.pdf.data, FileStorage) @property diff --git a/atst/models/workspace_role.py b/atst/models/workspace_role.py index 1da0bce8..4bd241df 100644 --- a/atst/models/workspace_role.py +++ b/atst/models/workspace_role.py @@ -82,21 +82,21 @@ class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): @property def display_status(self): if self.status == Status.ACTIVE: - return MEMBER_STATUSES['active'] + return MEMBER_STATUSES["active"] elif self.latest_invitation: if self.latest_invitation.is_revoked: - return MEMBER_STATUSES['revoked'] + return MEMBER_STATUSES["revoked"] elif self.latest_invitation.is_rejected_wrong_user: - return MEMBER_STATUSES['error'] + return MEMBER_STATUSES["error"] elif ( self.latest_invitation.is_rejected_expired or self.latest_invitation.is_expired ): - return MEMBER_STATUSES['expired'] + return MEMBER_STATUSES["expired"] else: - return MEMBER_STATUSES['pending'] + return MEMBER_STATUSES["pending"] else: - return MEMBER_STATUSES['unknown'] + return MEMBER_STATUSES["unknown"] @property def has_dod_id_error(self): diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 8f2369ac..4970fb24 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -31,12 +31,17 @@ class FinancialVerificationBase(object): fv = FinancialVerification(request) form = FinancialVerificationForm(obj=fv, formdata=_formdata) - if not form.has_task_order_pdf: - try: - attachment = Attachment.get_for_resource("task_order", self.request.id) - form.task_order.pdf.data = attachment.filename - except NotFoundError: - pass + if not form.has_pdf_upload: + if isinstance(form.task_order.pdf.data, Attachment): + form.task_order.pdf.data = form.task_order.pdf.data.filename + else: + try: + attachment = Attachment.get_for_resource( + "task_order", self.request.id + ) + form.task_order.pdf.data = attachment.filename + except NotFoundError: + pass return form diff --git a/tests/routes/requests/test_financial_verification.py b/tests/routes/requests/test_financial_verification.py index 1578fe4a..9a5917f6 100644 --- a/tests/routes/requests/test_financial_verification.py +++ b/tests/routes/requests/test_financial_verification.py @@ -11,13 +11,14 @@ from atst.routes.requests.financial_verification import ( ) from tests.mocks import MOCK_VALID_PE_ID -from tests.factories import RequestFactory, UserFactory +from tests.factories import RequestFactory, UserFactory, TaskOrderFactory from atst.forms.exceptions import FormValidationError from atst.domain.requests.financial_verification import ( PENumberValidator, TaskOrderNumberValidator, ) from atst.models.request_status_event import RequestStatus +from atst.models.attachment import Attachment from atst.domain.requests.query import RequestsQuery @@ -509,3 +510,30 @@ def test_pdf_clearing(fv_data, e_fv_data, pdf_upload, pdf_upload2): form = GetFinancialVerificationForm(user, request, is_extended=True).execute() assert form.task_order.pdf.data == pdf_upload2.filename + + +# TODO: This test manages an edge case for our current non-unique handling of +# task orders. Because two requests can reference the same task order but we +# only record one request ID on the PDF attachment, multiple task +# orders/requests reference the same task order but only one of them is noted +# in the related attachment entity. I have changed the handling in +# FinancialVerificationBase#_get_form to be more generous in how it finds the +# PDF filename and prepopulates the form data with that name. +def test_always_derives_pdf_filename(fv_data, e_fv_data, pdf_upload): + user = UserFactory.create() + request_one = RequestFactory.create(creator=user) + attachment = Attachment.attach( + pdf_upload, resource="task_order", resource_id=request_one.id + ) + task_order = TaskOrderFactory.create(pdf=attachment) + request_two = RequestFactory.create(creator=user, task_order=task_order) + + form_one = GetFinancialVerificationForm( + user, request_one, is_extended=True + ).execute() + form_two = GetFinancialVerificationForm( + user, request_two, is_extended=True + ).execute() + + assert form_one.task_order.pdf.data == attachment.filename + assert form_two.task_order.pdf.data == attachment.filename