From 4c587864ef8564ae10b762650657a613f4943a47 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 14 Aug 2018 16:19:43 -0400 Subject: [PATCH 1/4] add Requests method for counting all requests with a given status --- atst/domain/requests.py | 20 ++++++++++++++++++++ tests/conftest.py | 1 + tests/domain/test_requests.py | 15 ++++++++++++++- tests/factories.py | 1 + 4 files changed, 36 insertions(+), 1 deletion(-) diff --git a/atst/domain/requests.py b/atst/domain/requests.py index 23290085..3d50d887 100644 --- a/atst/domain/requests.py +++ b/atst/domain/requests.py @@ -1,4 +1,6 @@ +from enum import Enum from sqlalchemy import exists, and_, exc +from sqlalchemy.sql import text from sqlalchemy.orm.exc import NoResultFound from sqlalchemy.orm.attributes import flag_modified @@ -154,3 +156,21 @@ class Requests(object): @classmethod def is_pending_ccpo_approval(cls, request): return request.status == RequestStatus.PENDING_CCPO_APPROVAL + + @classmethod + def count_status(self, status): + raw = text(""" +SELECT count(requests_with_status.id) +FROM ( + SELECT DISTINCT ON (rse.request_id) r.*, rse.new_status as status + FROM request_status_events rse JOIN requests r ON r.id = rse.request_id + ORDER BY rse.request_id, rse.sequence DESC +) as requests_with_status +WHERE requests_with_status.status = :status; + """) + if isinstance(status, Enum): + status = status.name + results = db.session.execute(raw, {"status": status}).fetchone() + (count,) = results + return count + diff --git a/tests/conftest.py b/tests/conftest.py index 12e7c320..e5094c1b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -69,6 +69,7 @@ def session(db, request): ] for factory in factory_list: factory._meta.sqlalchemy_session = session + factory._meta.sqlalchemy_session_persistence = "commit" yield session diff --git a/tests/domain/test_requests.py b/tests/domain/test_requests.py index 2044d52b..420294e2 100644 --- a/tests/domain/test_requests.py +++ b/tests/domain/test_requests.py @@ -3,9 +3,10 @@ from uuid import uuid4 from atst.domain.exceptions import NotFoundError from atst.domain.requests import Requests +from atst.models.request import Request from atst.models.request_status_event import RequestStatus -from tests.factories import RequestFactory, UserFactory +from tests.factories import RequestFactory, UserFactory, RequestStatusEventFactory @pytest.fixture(scope="function") @@ -63,3 +64,15 @@ def test_exists(session): request = RequestFactory.create(creator=user_allowed) assert Requests.exists(request.id, user_allowed) assert not Requests.exists(request.id, user_denied) + + +def test_count_status(session): + # make sure table is empty + session.query(Request).delete() + + request1 = RequestFactory.create() + request2 = RequestFactory.create() + RequestStatusEventFactory.create(sequence=2, request_id=request2.id, new_status=RequestStatus.PENDING_FINANCIAL_VERIFICATION) + + assert Requests.count_status(RequestStatus.PENDING_FINANCIAL_VERIFICATION) == 1 + assert Requests.count_status(RequestStatus.STARTED) == 1 diff --git a/tests/factories.py b/tests/factories.py index 3e133c45..ad12ef56 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -39,6 +39,7 @@ class RequestStatusEventFactory(factory.alchemy.SQLAlchemyModelFactory): model = RequestStatusEvent id = factory.Sequence(lambda x: uuid4()) + sequence = 1 class RequestFactory(factory.alchemy.SQLAlchemyModelFactory): From e97fdbf140dc22ee8c90fcd9e1f7383f74c43161 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 14 Aug 2018 16:57:27 -0400 Subject: [PATCH 2/4] implement real KPI counts --- atst/domain/requests.py | 18 ++++++++++++++- atst/models/request_status_event.py | 1 + atst/routes/requests/index.py | 36 +++++++++++++++++++++-------- templates/requests.html | 12 +++++----- tests/domain/test_requests.py | 7 +++--- 5 files changed, 55 insertions(+), 19 deletions(-) diff --git a/atst/domain/requests.py b/atst/domain/requests.py index 3d50d887..f36551d3 100644 --- a/atst/domain/requests.py +++ b/atst/domain/requests.py @@ -158,7 +158,7 @@ class Requests(object): return request.status == RequestStatus.PENDING_CCPO_APPROVAL @classmethod - def count_status(self, status): + def status_count(cls, status): raw = text(""" SELECT count(requests_with_status.id) FROM ( @@ -174,3 +174,19 @@ WHERE requests_with_status.status = :status; (count,) = results return count + @classmethod + def in_progress_count(cls): + return sum([ + Requests.status_count(RequestStatus.STARTED), + Requests.status_count(RequestStatus.PENDING_FINANCIAL_VERIFICATION), + Requests.status_count(RequestStatus.CHANGES_REQUESTED), + ]) + + @classmethod + def pending_ccpo_count(cls): + return Requests.status_count(RequestStatus.PENDING_CCPO_APPROVAL) + + @classmethod + def completed_count(cls): + return Requests.status_count(RequestStatus.APPROVED) + diff --git a/atst/models/request_status_event.py b/atst/models/request_status_event.py index 81505f2a..cb092403 100644 --- a/atst/models/request_status_event.py +++ b/atst/models/request_status_event.py @@ -12,6 +12,7 @@ class RequestStatus(Enum): STARTED = "Started" PENDING_FINANCIAL_VERIFICATION = "Pending Financial Verification" PENDING_CCPO_APPROVAL = "Pending CCPO Approval" + CHANGES_REQUESTED = "Changes Requested" APPROVED = "Approved" EXPIRED = "Expired" DELETED = "Deleted" diff --git a/atst/routes/requests/index.py b/atst/routes/requests/index.py index 7130c722..69da2058 100644 --- a/atst/routes/requests/index.py +++ b/atst/routes/requests/index.py @@ -32,22 +32,40 @@ def map_request(request): @requests_bp.route("/requests", methods=["GET"]) def requests_index(): - requests = [] - is_ccpo = Permissions.REVIEW_AND_APPROVE_JEDI_WORKSPACE_REQUEST in g.current_user.atat_permissions - if is_ccpo: - requests = Requests.get_many() - else: - requests = Requests.get_many(creator=g.current_user) + if Permissions.REVIEW_AND_APPROVE_JEDI_WORKSPACE_REQUEST in g.current_user.atat_permissions: + return _ccpo_view() + else: + return _non_ccpo_view() + + +def _ccpo_view(): + requests = Requests.get_many() mapped_requests = [map_request(r) for r in requests] - pending_fv = not is_ccpo and any(Requests.is_pending_financial_verification(r) for r in requests) - pending_ccpo = not is_ccpo and any(Requests.is_pending_ccpo_approval(r) for r in requests) + return render_template( + "requests.html", + requests=mapped_requests, + pending_financial_verification=False, + pending_ccpo_approval=False, + extended_view=True, + kpi_inprogress=Requests.in_progress_count(), + kpi_pending=Requests.pending_ccpo_count(), + kpi_completed=Requests.completed_count(), + ) + + +def _non_ccpo_view(): + requests = Requests.get_many(creator=g.current_user) + mapped_requests = [map_request(r) for r in requests] + + 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) return render_template( "requests.html", requests=mapped_requests, pending_financial_verification=pending_fv, pending_ccpo_approval=pending_ccpo, - extended_view=is_ccpo + extended_view=False, ) diff --git a/templates/requests.html b/templates/requests.html index 42d6ac1b..53950e10 100644 --- a/templates/requests.html +++ b/templates/requests.html @@ -51,16 +51,16 @@ {% if extended_view %}
-
3
-
Pending Requests
+
{{ kpi_inprogress }}
+
In Progress
-
2,456
-
Completed Requests This Year
+
{{ kpi_pending }}
+
Pending CCPO Action
-
234
-
Denied Requests
+
{{ kpi_completed }}
+
Completed (Overall)
{% endif %} diff --git a/tests/domain/test_requests.py b/tests/domain/test_requests.py index 420294e2..3e4e66e4 100644 --- a/tests/domain/test_requests.py +++ b/tests/domain/test_requests.py @@ -66,7 +66,7 @@ def test_exists(session): assert not Requests.exists(request.id, user_denied) -def test_count_status(session): +def test_status_count(session): # make sure table is empty session.query(Request).delete() @@ -74,5 +74,6 @@ def test_count_status(session): request2 = RequestFactory.create() RequestStatusEventFactory.create(sequence=2, request_id=request2.id, new_status=RequestStatus.PENDING_FINANCIAL_VERIFICATION) - assert Requests.count_status(RequestStatus.PENDING_FINANCIAL_VERIFICATION) == 1 - assert Requests.count_status(RequestStatus.STARTED) == 1 + assert Requests.status_count(RequestStatus.PENDING_FINANCIAL_VERIFICATION) == 1 + assert Requests.status_count(RequestStatus.STARTED) == 1 + assert Requests.in_progress_count() == 2 From 3a77e07c3f7fc39be50ce72c27fc159d30209985 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 14 Aug 2018 17:02:53 -0400 Subject: [PATCH 3/4] update request statuses; deleted -> canceled --- atst/models/request_status_event.py | 2 +- tests/models/test_requests.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/atst/models/request_status_event.py b/atst/models/request_status_event.py index cb092403..810a4427 100644 --- a/atst/models/request_status_event.py +++ b/atst/models/request_status_event.py @@ -15,7 +15,7 @@ class RequestStatus(Enum): CHANGES_REQUESTED = "Changes Requested" APPROVED = "Approved" EXPIRED = "Expired" - DELETED = "Deleted" + CANCELED = "Canceled" class RequestStatusEvent(Base): diff --git a/tests/models/test_requests.py b/tests/models/test_requests.py index d6592a25..987723af 100644 --- a/tests/models/test_requests.py +++ b/tests/models/test_requests.py @@ -65,6 +65,6 @@ def test_request_status_pending_expired_displayname(): def test_request_status_pending_deleted_displayname(): request = RequestFactory.create() - request = Requests.set_status(request, RequestStatus.DELETED) + request = Requests.set_status(request, RequestStatus.CANCELED) - assert request.status_displayname == "Deleted" + assert request.status_displayname == "Canceled" From 920ad1c5544aff8226428be5dcaab4f0eab1bd56 Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 15 Aug 2018 11:43:12 -0400 Subject: [PATCH 4/4] Requests.status_count can be scoped to creator --- atst/domain/requests.py | 20 +++++++++++++------- tests/domain/test_requests.py | 11 +++++++++++ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/atst/domain/requests.py b/atst/domain/requests.py index f36551d3..fa122d71 100644 --- a/atst/domain/requests.py +++ b/atst/domain/requests.py @@ -158,19 +158,25 @@ class Requests(object): return request.status == RequestStatus.PENDING_CCPO_APPROVAL @classmethod - def status_count(cls, status): - raw = text(""" + def status_count(cls, status, creator=None): + if isinstance(status, Enum): + status = status.name + bindings = {"status": status} + raw = """ SELECT count(requests_with_status.id) FROM ( SELECT DISTINCT ON (rse.request_id) r.*, rse.new_status as status FROM request_status_events rse JOIN requests r ON r.id = rse.request_id ORDER BY rse.request_id, rse.sequence DESC ) as requests_with_status -WHERE requests_with_status.status = :status; - """) - if isinstance(status, Enum): - status = status.name - results = db.session.execute(raw, {"status": status}).fetchone() +WHERE requests_with_status.status = :status + """ + + if creator: + raw += " AND requests_with_status.user_id = :user_id" + bindings["user_id"] = creator.id + + results = db.session.execute(text(raw), bindings).fetchone() (count,) = results return count diff --git a/tests/domain/test_requests.py b/tests/domain/test_requests.py index 3e4e66e4..332e3c6c 100644 --- a/tests/domain/test_requests.py +++ b/tests/domain/test_requests.py @@ -77,3 +77,14 @@ def test_status_count(session): assert Requests.status_count(RequestStatus.PENDING_FINANCIAL_VERIFICATION) == 1 assert Requests.status_count(RequestStatus.STARTED) == 1 assert Requests.in_progress_count() == 2 + +def test_status_count_scoped_to_creator(session): + # make sure table is empty + session.query(Request).delete() + + user = UserFactory.create() + request1 = RequestFactory.create() + request2 = RequestFactory.create(creator=user) + + assert Requests.status_count(RequestStatus.STARTED) == 2 + assert Requests.status_count(RequestStatus.STARTED, creator=user) == 1