From c4e99712062766439832a6ea05a2a26fcad533f4 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 10 Sep 2018 10:28:44 -0400 Subject: [PATCH] distinguish final CCPO approval from acceptance of initial request --- atst/domain/requests.py | 39 +++++++++++++++------------ atst/models/request.py | 1 + atst/models/request_status_event.py | 1 + atst/routes/requests/index.py | 8 +++--- tests/domain/test_requests.py | 4 +-- tests/routes/test_request_approval.py | 2 +- tests/test_integration.py | 2 +- 7 files changed, 32 insertions(+), 25 deletions(-) diff --git a/atst/domain/requests.py b/atst/domain/requests.py index 7c2897cf..c34009e5 100644 --- a/atst/domain/requests.py +++ b/atst/domain/requests.py @@ -76,10 +76,9 @@ class Requests(object): filters.append(Request.creator == creator) requests = ( - db.session.query(Request) - .filter(*filters) - .order_by(Request.time_created.desc()) - .all() + db.session.query(Request).filter(*filters).order_by( + Request.time_created.desc() + ).all() ) return requests @@ -91,7 +90,7 @@ class Requests(object): if Requests.should_auto_approve(request): new_status = RequestStatus.PENDING_FINANCIAL_VERIFICATION else: - new_status = RequestStatus.PENDING_CCPO_APPROVAL + new_status = RequestStatus.PENDING_CCPO_ACCEPTANCE request = Requests.set_status(request, new_status) @@ -119,10 +118,9 @@ class Requests(object): # Query for request matching id, acquiring a row-level write lock. # https://www.postgresql.org/docs/10/static/sql-select.html#SQL-FOR-UPDATE-SHARE return ( - db.session.query(Request) - .filter_by(id=request_id) - .with_for_update(of=Request) - .one() + db.session.query(Request).filter_by(id=request_id).with_for_update( + of=Request + ).one() ) except NoResultFound: @@ -156,16 +154,13 @@ class Requests(object): return dollar_value < cls.AUTO_APPROVE_THRESHOLD _VALID_SUBMISSION_STATUSES = [ - RequestStatus.STARTED, - RequestStatus.CHANGES_REQUESTED, + RequestStatus.STARTED, RequestStatus.CHANGES_REQUESTED ] @classmethod def should_allow_submission(cls, request): all_request_sections = [ - "details_of_use", - "information_about_you", - "primary_poc", + "details_of_use", "information_about_you", "primary_poc" ] existing_request_sections = request.body.keys() return request.status in Requests._VALID_SUBMISSION_STATUSES and all( @@ -176,6 +171,10 @@ class Requests(object): def is_pending_financial_verification(cls, request): return request.status == RequestStatus.PENDING_FINANCIAL_VERIFICATION + @classmethod + def is_pending_ccpo_acceptance(cls, request): + return request.status == RequestStatus.PENDING_CCPO_ACCEPTANCE + @classmethod def is_pending_ccpo_approval(cls, request): return request.status == RequestStatus.PENDING_CCPO_APPROVAL @@ -215,7 +214,12 @@ WHERE requests_with_status.status = :status @classmethod def pending_ccpo_count(cls): - return Requests.status_count(RequestStatus.PENDING_CCPO_APPROVAL) + return sum( + [ + Requests.status_count(RequestStatus.PENDING_CCPO_ACCEPTANCE), + Requests.status_count(RequestStatus.PENDING_CCPO_APPROVAL), + ] + ) @classmethod def completed_count(cls): @@ -237,8 +241,9 @@ WHERE requests_with_status.status = :status else: task_order_number = request_data.get("task_order_number") - if "task_order" in request_data and isinstance( - request_data["task_order"], FileStorage + if ( + "task_order" in request_data + and isinstance(request_data["task_order"], FileStorage) ): task_order_data["pdf"] = request_data.pop("task_order") diff --git a/atst/models/request.py b/atst/models/request.py index 71609702..c991a708 100644 --- a/atst/models/request.py +++ b/atst/models/request.py @@ -157,6 +157,7 @@ class Request(Base): return { RequestStatus.PENDING_FINANCIAL_VERIFICATION: "mission_owner", RequestStatus.PENDING_CCPO_APPROVAL: "ccpo", + RequestStatus.PENDING_CCPO_ACCEPTANCE: "ccpo", }.get(self.status) @property diff --git a/atst/models/request_status_event.py b/atst/models/request_status_event.py index 11ae7dad..a5373bdd 100644 --- a/atst/models/request_status_event.py +++ b/atst/models/request_status_event.py @@ -13,6 +13,7 @@ class RequestStatus(Enum): STARTED = "Started" SUBMITTED = "Submitted" PENDING_FINANCIAL_VERIFICATION = "Pending Financial Verification" + PENDING_CCPO_ACCEPTANCE = "Pending CCPO Acceptance" PENDING_CCPO_APPROVAL = "Pending CCPO Approval" CHANGES_REQUESTED = "Changes Requested" APPROVED = "Approved" diff --git a/atst/routes/requests/index.py b/atst/routes/requests/index.py index 75be2c87..7032c12f 100644 --- a/atst/routes/requests/index.py +++ b/atst/routes/requests/index.py @@ -30,7 +30,7 @@ class RequestsIndex(object): return { "requests": mapped_requests, "pending_financial_verification": False, - "pending_ccpo_approval": False, + "pending_ccpo_acceptance": False, "extended_view": True, "kpi_inprogress": Requests.in_progress_count(), "kpi_pending": Requests.pending_ccpo_count(), @@ -47,12 +47,12 @@ class RequestsIndex(object): pending_fv = any( Requests.is_pending_financial_verification(r) for r in requests ) - pending_ccpo = any(Requests.is_pending_ccpo_approval(r) for r in requests) + pending_ccpo = any(Requests.is_pending_ccpo_acceptance(r) for r in requests) return { "requests": mapped_requests, "pending_financial_verification": pending_fv, - "pending_ccpo_approval": pending_ccpo, + "pending_ccpo_acceptance": pending_ccpo, "num_action_required": num_action_required, "extended_view": False, } @@ -76,7 +76,7 @@ class RequestsIndex(object): edit_link = url_for( "requests.financial_verification", request_id=request.id ) - elif Requests.is_pending_ccpo_approval(request): + elif Requests.is_pending_ccpo_acceptance(request) or Requests.is_pending_ccpo_approval(request): edit_link = url_for("requests.view_pending_request", request_id=request.id) else: edit_link = url_for( diff --git a/tests/domain/test_requests.py b/tests/domain/test_requests.py index a0fd97ed..3d587cfa 100644 --- a/tests/domain/test_requests.py +++ b/tests/domain/test_requests.py @@ -51,14 +51,14 @@ def test_dont_auto_approve_if_dollar_value_is_1m_or_above(): new_request = RequestFactory.create(initial_revision={"dollar_value": 1000000}) request = Requests.submit(new_request) - assert request.status == RequestStatus.PENDING_CCPO_APPROVAL + assert request.status == RequestStatus.PENDING_CCPO_ACCEPTANCE def test_dont_auto_approve_if_no_dollar_value_specified(): new_request = RequestFactory.create(initial_revision={}) request = Requests.submit(new_request) - assert request.status == RequestStatus.PENDING_CCPO_APPROVAL + assert request.status == RequestStatus.PENDING_CCPO_ACCEPTANCE def test_should_allow_submission(): diff --git a/tests/routes/test_request_approval.py b/tests/routes/test_request_approval.py index 0198f816..36881805 100644 --- a/tests/routes/test_request_approval.py +++ b/tests/routes/test_request_approval.py @@ -71,4 +71,4 @@ def test_can_submit_request_approval(client, user_session): response = client.post( url_for("requests.submit_approval", request_id=request.id), data=review_data ) - assert response.status_code == 301 + assert response.status_code == 302 diff --git a/tests/test_integration.py b/tests/test_integration.py index 51a75789..152e960b 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -81,4 +81,4 @@ def test_stepthrough_request_form(user_session, screens, client): ) finished_request = Requests.get(user, req_id) - assert finished_request.status == RequestStatus.PENDING_CCPO_APPROVAL + assert finished_request.status == RequestStatus.PENDING_CCPO_ACCEPTANCE