From 7404cc4a59ceddfa01bcd07bc787ddbfd52a476d Mon Sep 17 00:00:00 2001 From: richard-dds Date: Thu, 25 Oct 2018 15:25:05 -0400 Subject: [PATCH 01/11] Commit after updating status --- atst/domain/requests/requests.py | 2 +- atst/models/request.py | 4 ++-- tests/routes/test_request_new.py | 8 ++------ 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/atst/domain/requests/requests.py b/atst/domain/requests/requests.py index 4e92c46f..0c966ed8 100644 --- a/atst/domain/requests/requests.py +++ b/atst/domain/requests/requests.py @@ -99,7 +99,7 @@ class Requests(object): new_status=status, revision=request.latest_revision ) request.status_events.append(status_event) - return request + return RequestsQuery.add_and_commit(request) @classmethod def should_auto_approve(cls, request): diff --git a/atst/models/request.py b/atst/models/request.py index 443f3c9b..4d518274 100644 --- a/atst/models/request.py +++ b/atst/models/request.py @@ -119,11 +119,11 @@ class Request(Base, mixins.TimestampsMixin, mixins.AuditableMixin): @property def latest_status(self): - return self.status_events[-1] + return self.status_events[-1] if self.status_events else None @property def status(self): - return self.latest_status.new_status + return self.latest_status.new_status if self.latest_status else None @property def status_displayname(self): diff --git a/tests/routes/test_request_new.py b/tests/routes/test_request_new.py index 87842c94..d64d21e0 100644 --- a/tests/routes/test_request_new.py +++ b/tests/routes/test_request_new.py @@ -237,13 +237,9 @@ def test_displays_ccpo_review_comment(user_session, client): ccpo = UserFactory.from_atat_role("ccpo") user_session(creator) request = RequestFactory.create(creator=creator) - status = RequestStatusEventFactory.create( - request=request, - revision=request.latest_revision, - new_status=RequestStatus.CHANGES_REQUESTED, - ) + request = Requests.set_status(request, RequestStatus.CHANGES_REQUESTED) review_comment = "add all of the correct info, instead of the incorrect info" - RequestReviewFactory.create(reviewer=ccpo, comment=review_comment, status=status) + RequestReviewFactory.create(reviewer=ccpo, comment=review_comment, status=request.status_events[-1]) response = client.get("/requests/new/1/{}".format(request.id)) body = response.data.decode() assert review_comment in body From f3be2d76eac2b33621e35db1c98f24824f57a9e6 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 29 Oct 2018 12:01:45 -0400 Subject: [PATCH 02/11] Create queue fixture --- tests/conftest.py | 10 ++++++++-- tests/test_queue.py | 16 ++-------------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 50a555e7..c1821b96 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,8 +9,7 @@ from tempfile import TemporaryDirectory from atst.app import make_app, make_config from atst.database import db as _db -from atst.domain.auth import logout -from atst.queue import queue +from atst.queue import queue as atst_queue import tests.factories as factories from tests.mocks import PDF_FILENAME @@ -136,3 +135,10 @@ def extended_financial_verification_data(pdf_upload): "clin_2003": "7000", "task_order": pdf_upload, } + + +@pytest.fixture(scope="function") +def queue(): + _queue = atst_queue + yield _queue + _queue.get_queue().empty() diff --git a/tests/test_queue.py b/tests/test_queue.py index da6069d4..eb377bc9 100644 --- a/tests/test_queue.py +++ b/tests/test_queue.py @@ -1,17 +1,5 @@ -import pytest -from atst.queue import queue - -# ensure queue is always empty for unit testing -@pytest.fixture(scope="function", autouse=True) -def reset_queue(): - queue.get_queue().empty() - yield - queue.get_queue().empty() - - -def test_send_mail(): - initial = len(queue.get_queue()) +def test_send_mail(queue): queue.send_mail( ["lordvader@geocities.net"], "death start", "how is it coming along?" ) - assert len(queue.get_queue()) == initial + 1 + assert len(queue.get_queue()) == 1 From 85034185bc8a3ea6cded28fb3eabcdace4ea3648 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 29 Oct 2018 14:57:25 -0400 Subject: [PATCH 03/11] Handle request status event transitions --- atst/domain/requests/requests.py | 10 ++++- atst/domain/requests/status_event_handler.py | 41 ++++++++++++++++++++ atst/queue.py | 8 ++-- templates/emails/request_status_change.txt | 5 +++ tests/conftest.py | 7 ++-- tests/domain/test_requests.py | 36 ++++++++++++++++- tests/routes/test_request_new.py | 4 +- 7 files changed, 99 insertions(+), 12 deletions(-) create mode 100644 atst/domain/requests/status_event_handler.py create mode 100644 templates/emails/request_status_change.txt diff --git a/atst/domain/requests/requests.py b/atst/domain/requests/requests.py index 0c966ed8..86e2908d 100644 --- a/atst/domain/requests/requests.py +++ b/atst/domain/requests/requests.py @@ -6,9 +6,11 @@ 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.queue import queue from .query import RequestsQuery from .authorization import RequestsAuthorization +from .status_event_handler import RequestStatusEventHandler def create_revision_from_request_body(body): @@ -95,11 +97,17 @@ class Requests(object): @classmethod def set_status(cls, request, status: RequestStatus): + old_status = request.status status_event = RequestStatusEvent( new_status=status, revision=request.latest_revision ) request.status_events.append(status_event) - return RequestsQuery.add_and_commit(request) + updated_request = RequestsQuery.add_and_commit(request) + RequestStatusEventHandler(queue).handle_status_change( + updated_request, old_status, status + ) + + return updated_request @classmethod def should_auto_approve(cls, request): diff --git a/atst/domain/requests/status_event_handler.py b/atst/domain/requests/status_event_handler.py new file mode 100644 index 00000000..a840642c --- /dev/null +++ b/atst/domain/requests/status_event_handler.py @@ -0,0 +1,41 @@ +from flask import render_template + +from atst.models.request_status_event import RequestStatus + + +class RequestStatusEventHandler(object): + def __init__(self, queue): + self.queue = queue + + def handle_status_change(self, request, old_status, new_status): + handler = self._get_handler(old_status, new_status) + if handler: + handler(request) + + def _get_handler(self, old_status, new_status): + return { + ( + RequestStatus.PENDING_CCPO_ACCEPTANCE, + RequestStatus.PENDING_FINANCIAL_VERIFICATION, + ): self._send_email, + ( + RequestStatus.PENDING_CCPO_ACCEPTANCE, + RequestStatus.CHANGES_REQUESTED, + ): self._send_email, + ( + RequestStatus.PENDING_CCPO_APPROVAL, + RequestStatus.CHANGES_REQUESTED_TO_FINVER, + ): self._send_email, + ( + RequestStatus.PENDING_CCPO_APPROVAL, + RequestStatus.APPROVED, + ): self._send_email, + }.get((old_status, new_status)) + + def _send_email(self, request): + email_body = render_template( + "emails/request_status_change.txt", request=request + ) + self.queue.send_mail( + [request.creator.email], "Your JEDI request status has changed", email_body + ) diff --git a/atst/queue.py b/atst/queue.py index e036a642..befe45b0 100644 --- a/atst/queue.py +++ b/atst/queue.py @@ -27,8 +27,8 @@ class ATSTQueue(RQ): # pylint: disable=pointless-string-statement """Instance methods to queue up application-specific jobs.""" - def send_mail(self, to, subject, body): - self._queue_job(ATSTQueue._send_mail, to, subject, body) + def send_mail(self, recipients, subject, body): + self._queue_job(ATSTQueue._send_mail, recipients, subject, body) # pylint: disable=pointless-string-statement """Class methods to actually perform the work. @@ -38,8 +38,8 @@ class ATSTQueue(RQ): """ @classmethod - def _send_mail(self, to, subject, body): - app.mailer.send(to, subject, body) + def _send_mail(self, recipients, subject, body): + app.mailer.send(recipients, subject, body) queue = ATSTQueue() diff --git a/templates/emails/request_status_change.txt b/templates/emails/request_status_change.txt new file mode 100644 index 00000000..75bb7a0b --- /dev/null +++ b/templates/emails/request_status_change.txt @@ -0,0 +1,5 @@ +

Your JEDI request status has changed

+ +The status of your JEDI Cloud request - {{ request.name }} - was recently updated. Log in to see whether this change requires an action or response from you. + +{{ url_for('requests.view_request_details', request_id=request.id, _external=True) }} diff --git a/tests/conftest.py b/tests/conftest.py index c1821b96..67912c1d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -137,8 +137,7 @@ def extended_financial_verification_data(pdf_upload): } -@pytest.fixture(scope="function") +@pytest.fixture(scope="function", autouse=True) def queue(): - _queue = atst_queue - yield _queue - _queue.get_queue().empty() + yield atst_queue + atst_queue.get_queue().empty() diff --git a/tests/domain/test_requests.py b/tests/domain/test_requests.py index 90e3655d..72971568 100644 --- a/tests/domain/test_requests.py +++ b/tests/domain/test_requests.py @@ -6,13 +6,11 @@ from atst.domain.requests import Requests from atst.domain.requests.authorization import RequestsAuthorization from atst.models.request import Request from atst.models.request_status_event import RequestStatus -from atst.models.task_order import Source as TaskOrderSource from tests.factories import ( RequestFactory, UserFactory, RequestStatusEventFactory, - TaskOrderFactory, RequestRevisionFactory, RequestReviewFactory, ) @@ -222,3 +220,37 @@ def test_random_user_cannot_view_request(): request = RequestFactory.create() assert not RequestsAuthorization(user, request).can_view + + +def test_pending_finver_triggers_notification(queue): + request = RequestFactory.create() + request = Requests.set_status(request, RequestStatus.PENDING_CCPO_ACCEPTANCE) + request = Requests.set_status(request, RequestStatus.PENDING_FINANCIAL_VERIFICATION) + assert len(queue.get_queue()) == 1 + + +def test_changes_requested_triggers_notification(queue): + request = RequestFactory.create() + request = Requests.set_status(request, RequestStatus.PENDING_CCPO_ACCEPTANCE) + request = Requests.set_status(request, RequestStatus.CHANGES_REQUESTED) + assert len(queue.get_queue()) == 1 + + +def test_changes_requested_to_finver_triggers_notification(queue): + request = RequestFactory.create() + request = Requests.set_status(request, RequestStatus.PENDING_CCPO_APPROVAL) + request = Requests.set_status(request, RequestStatus.CHANGES_REQUESTED_TO_FINVER) + assert len(queue.get_queue()) == 1 + + +def test_approval_triggers_notification(queue): + request = RequestFactory.create() + request = Requests.set_status(request, RequestStatus.PENDING_CCPO_APPROVAL) + request = Requests.set_status(request, RequestStatus.APPROVED) + assert len(queue.get_queue()) == 1 + + +def test_submitted_does_not_trigger_notification(queue): + request = RequestFactory.create() + request = Requests.set_status(request, RequestStatus.SUBMITTED) + assert len(queue.get_queue()) == 0 diff --git a/tests/routes/test_request_new.py b/tests/routes/test_request_new.py index d64d21e0..81d195a8 100644 --- a/tests/routes/test_request_new.py +++ b/tests/routes/test_request_new.py @@ -239,7 +239,9 @@ def test_displays_ccpo_review_comment(user_session, client): request = RequestFactory.create(creator=creator) request = Requests.set_status(request, RequestStatus.CHANGES_REQUESTED) review_comment = "add all of the correct info, instead of the incorrect info" - RequestReviewFactory.create(reviewer=ccpo, comment=review_comment, status=request.status_events[-1]) + RequestReviewFactory.create( + reviewer=ccpo, comment=review_comment, status=request.status_events[-1] + ) response = client.get("/requests/new/1/{}".format(request.id)) body = response.data.decode() assert review_comment in body From 0ba3fbe4dc465773ac3b9f92cab7bdf2df1f3b5b Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 29 Oct 2018 15:37:57 -0400 Subject: [PATCH 04/11] Use request.displayname --- templates/emails/request_status_change.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/emails/request_status_change.txt b/templates/emails/request_status_change.txt index 75bb7a0b..8ae8b18a 100644 --- a/templates/emails/request_status_change.txt +++ b/templates/emails/request_status_change.txt @@ -1,5 +1,5 @@

Your JEDI request status has changed

-The status of your JEDI Cloud request - {{ request.name }} - was recently updated. Log in to see whether this change requires an action or response from you. +The status of your JEDI Cloud request - {{ request.displayname }} - was recently updated. Log in to see whether this change requires an action or response from you. {{ url_for('requests.view_request_details', request_id=request.id, _external=True) }} From b60a8ff57d1ca2c9a5c3ef784367c773bebeb7dc Mon Sep 17 00:00:00 2001 From: richard-dds Date: Mon, 29 Oct 2018 16:15:41 -0400 Subject: [PATCH 05/11] Remove header tags since email is plaintext --- templates/emails/request_status_change.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/emails/request_status_change.txt b/templates/emails/request_status_change.txt index 8ae8b18a..5ad799b2 100644 --- a/templates/emails/request_status_change.txt +++ b/templates/emails/request_status_change.txt @@ -1,4 +1,4 @@ -

Your JEDI request status has changed

+Your JEDI request status has changed The status of your JEDI Cloud request - {{ request.displayname }} - was recently updated. Log in to see whether this change requires an action or response from you. From c731af3d7cd8f6430605fc452ef20d950bbcb9b9 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 30 Oct 2018 15:02:40 -0400 Subject: [PATCH 06/11] Simpler event handler dispatch --- atst/domain/requests/status_event_handler.py | 44 ++++++++++---------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/atst/domain/requests/status_event_handler.py b/atst/domain/requests/status_event_handler.py index a840642c..4fa44b95 100644 --- a/atst/domain/requests/status_event_handler.py +++ b/atst/domain/requests/status_event_handler.py @@ -4,33 +4,31 @@ from atst.models.request_status_event import RequestStatus class RequestStatusEventHandler(object): + STATUS_TRANSITIONS = set([ + ( + RequestStatus.PENDING_CCPO_ACCEPTANCE, + RequestStatus.PENDING_FINANCIAL_VERIFICATION, + ), + ( + RequestStatus.PENDING_CCPO_ACCEPTANCE, + RequestStatus.CHANGES_REQUESTED, + ), + ( + RequestStatus.PENDING_CCPO_APPROVAL, + RequestStatus.CHANGES_REQUESTED_TO_FINVER, + ), + ( + RequestStatus.PENDING_CCPO_APPROVAL, + RequestStatus.APPROVED, + ), + ]) + def __init__(self, queue): self.queue = queue def handle_status_change(self, request, old_status, new_status): - handler = self._get_handler(old_status, new_status) - if handler: - handler(request) - - def _get_handler(self, old_status, new_status): - return { - ( - RequestStatus.PENDING_CCPO_ACCEPTANCE, - RequestStatus.PENDING_FINANCIAL_VERIFICATION, - ): self._send_email, - ( - RequestStatus.PENDING_CCPO_ACCEPTANCE, - RequestStatus.CHANGES_REQUESTED, - ): self._send_email, - ( - RequestStatus.PENDING_CCPO_APPROVAL, - RequestStatus.CHANGES_REQUESTED_TO_FINVER, - ): self._send_email, - ( - RequestStatus.PENDING_CCPO_APPROVAL, - RequestStatus.APPROVED, - ): self._send_email, - }.get((old_status, new_status)) + if (old_status, new_status) in self.STATUS_TRANSITIONS: + self._send_email(request) def _send_email(self, request): email_body = render_template( From b4e3cb6f050509d5108c03992e946e00b66866d6 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 30 Oct 2018 15:15:02 -0400 Subject: [PATCH 07/11] Formatting --- atst/domain/requests/status_event_handler.py | 32 +++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/atst/domain/requests/status_event_handler.py b/atst/domain/requests/status_event_handler.py index 4fa44b95..3c4c9f87 100644 --- a/atst/domain/requests/status_event_handler.py +++ b/atst/domain/requests/status_event_handler.py @@ -4,24 +4,20 @@ from atst.models.request_status_event import RequestStatus class RequestStatusEventHandler(object): - STATUS_TRANSITIONS = set([ - ( - RequestStatus.PENDING_CCPO_ACCEPTANCE, - RequestStatus.PENDING_FINANCIAL_VERIFICATION, - ), - ( - RequestStatus.PENDING_CCPO_ACCEPTANCE, - RequestStatus.CHANGES_REQUESTED, - ), - ( - RequestStatus.PENDING_CCPO_APPROVAL, - RequestStatus.CHANGES_REQUESTED_TO_FINVER, - ), - ( - RequestStatus.PENDING_CCPO_APPROVAL, - RequestStatus.APPROVED, - ), - ]) + STATUS_TRANSITIONS = set( + [ + ( + RequestStatus.PENDING_CCPO_ACCEPTANCE, + RequestStatus.PENDING_FINANCIAL_VERIFICATION, + ), + (RequestStatus.PENDING_CCPO_ACCEPTANCE, RequestStatus.CHANGES_REQUESTED), + ( + RequestStatus.PENDING_CCPO_APPROVAL, + RequestStatus.CHANGES_REQUESTED_TO_FINVER, + ), + (RequestStatus.PENDING_CCPO_APPROVAL, RequestStatus.APPROVED), + ] + ) def __init__(self, queue): self.queue = queue From 13363ee9256a876fa37a7ff08f44a85ccae6243e Mon Sep 17 00:00:00 2001 From: richard-dds Date: Tue, 30 Oct 2018 17:05:49 -0400 Subject: [PATCH 08/11] Assert more specific details about the job --- tests/domain/test_requests.py | 61 +++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/tests/domain/test_requests.py b/tests/domain/test_requests.py index 72971568..23e2122e 100644 --- a/tests/domain/test_requests.py +++ b/tests/domain/test_requests.py @@ -222,35 +222,42 @@ def test_random_user_cannot_view_request(): assert not RequestsAuthorization(user, request).can_view -def test_pending_finver_triggers_notification(queue): - request = RequestFactory.create() - request = Requests.set_status(request, RequestStatus.PENDING_CCPO_ACCEPTANCE) - request = Requests.set_status(request, RequestStatus.PENDING_FINANCIAL_VERIFICATION) - assert len(queue.get_queue()) == 1 +class TestStatusNotifications(object): + def _assert_job(self, queue, request): + assert len(queue.get_queue()) == 1 + job = queue.get_queue().jobs[0] + assert job.func == queue._send_mail + assert job.args[0] == [request.creator.email] + def test_pending_finver_triggers_notification(self, queue): + request = RequestFactory.create() + request = Requests.set_status(request, RequestStatus.PENDING_CCPO_ACCEPTANCE) + request = Requests.set_status( + request, RequestStatus.PENDING_FINANCIAL_VERIFICATION + ) + self._assert_job(queue, request) -def test_changes_requested_triggers_notification(queue): - request = RequestFactory.create() - request = Requests.set_status(request, RequestStatus.PENDING_CCPO_ACCEPTANCE) - request = Requests.set_status(request, RequestStatus.CHANGES_REQUESTED) - assert len(queue.get_queue()) == 1 + def test_changes_requested_triggers_notification(self, queue): + request = RequestFactory.create() + request = Requests.set_status(request, RequestStatus.PENDING_CCPO_ACCEPTANCE) + request = Requests.set_status(request, RequestStatus.CHANGES_REQUESTED) + self._assert_job(queue, request) + def test_changes_requested_to_finver_triggers_notification(self, queue): + request = RequestFactory.create() + request = Requests.set_status(request, RequestStatus.PENDING_CCPO_APPROVAL) + request = Requests.set_status( + request, RequestStatus.CHANGES_REQUESTED_TO_FINVER + ) + self._assert_job(queue, request) -def test_changes_requested_to_finver_triggers_notification(queue): - request = RequestFactory.create() - request = Requests.set_status(request, RequestStatus.PENDING_CCPO_APPROVAL) - request = Requests.set_status(request, RequestStatus.CHANGES_REQUESTED_TO_FINVER) - assert len(queue.get_queue()) == 1 + def test_approval_triggers_notification(self, queue): + request = RequestFactory.create() + request = Requests.set_status(request, RequestStatus.PENDING_CCPO_APPROVAL) + request = Requests.set_status(request, RequestStatus.APPROVED) + self._assert_job(queue, request) - -def test_approval_triggers_notification(queue): - request = RequestFactory.create() - request = Requests.set_status(request, RequestStatus.PENDING_CCPO_APPROVAL) - request = Requests.set_status(request, RequestStatus.APPROVED) - assert len(queue.get_queue()) == 1 - - -def test_submitted_does_not_trigger_notification(queue): - request = RequestFactory.create() - request = Requests.set_status(request, RequestStatus.SUBMITTED) - assert len(queue.get_queue()) == 0 + def test_submitted_does_not_trigger_notification(self, queue): + request = RequestFactory.create() + request = Requests.set_status(request, RequestStatus.SUBMITTED) + assert len(queue.get_queue()) == 0 From 06e03489b680030c4e9e9553ea39ba6f31db4eb9 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 31 Oct 2018 11:06:57 -0400 Subject: [PATCH 09/11] Move request edit link logic into its own route --- atst/domain/authz.py | 4 ++++ atst/routes/requests/index.py | 21 +----------------- atst/routes/requests/requests_form.py | 32 +++++++++++++++++++++++++++ tests/routes/test_requests_index.py | 15 ------------- 4 files changed, 37 insertions(+), 35 deletions(-) diff --git a/atst/domain/authz.py b/atst/domain/authz.py index ce422736..8dff0258 100644 --- a/atst/domain/authz.py +++ b/atst/domain/authz.py @@ -30,3 +30,7 @@ class Authorization(object): @classmethod def can_view_audit_log(cls, user): return Authorization.has_atat_permission(user, Permissions.VIEW_AUDIT_LOG) + + @classmethod + def is_ccpo(cls, user): + return user.atat_role.name == "ccpo" diff --git a/atst/routes/requests/index.py b/atst/routes/requests/index.py index b479c848..99e3ad5c 100644 --- a/atst/routes/requests/index.py +++ b/atst/routes/requests/index.py @@ -55,25 +55,6 @@ class RequestsIndex(object): "extended_view": False, } - def _edit_link_for_request(self, viewing_role, request): - if request.creator == g.current_user: - if request.is_pending_financial_verification: - return url_for("requests.financial_verification", request_id=request.id) - elif request.is_pending_financial_verification_changes: - return url_for( - "requests.financial_verification", - request_id=request.id, - extended=True, - ) - elif request.is_approved: - return url_for("requests.view_request_details", request_id=request.id) - else: - return url_for( - "requests.requests_form_update", screen=1, request_id=request.id - ) - elif viewing_role == "ccpo": - return url_for("requests.approval", request_id=request.id) - def _map_request(self, request, viewing_role): time_created = pendulum.instance(request.time_created) is_new = time_created.add(days=1) > pendulum.now() @@ -92,7 +73,7 @@ class RequestsIndex(object): "last_edited_timestamp": request.latest_revision.time_updated, "full_name": request.creator.full_name, "annual_usage": annual_usage, - "edit_link": self._edit_link_for_request(viewing_role, request), + "edit_link": url_for("requests.edit", request_id=request.id), "action_required": request.action_required_by == viewing_role, "dod_component": request.latest_revision.dod_component, } diff --git a/atst/routes/requests/requests_form.py b/atst/routes/requests/requests_form.py index 1d7a4019..c88c1c38 100644 --- a/atst/routes/requests/requests_form.py +++ b/atst/routes/requests/requests_form.py @@ -2,6 +2,7 @@ from flask import g, redirect, render_template, url_for, request as http_request from . import requests_bp from atst.domain.requests import Requests +from atst.domain.authz import Authorization from atst.routes.requests.jedi_request_flow import JEDIRequestFlow from atst.models.request_status_event import RequestStatus from atst.forms.data import ( @@ -144,3 +145,34 @@ def view_request_details(request_id=None): jedi_request=request, requires_fv_action=requires_fv_action, ) + + +@requests_bp.route("/requests/edit/") +def edit(request_id): + user = g.current_user + request = Requests.get(user, request_id) + is_ccpo = Authorization.is_ccpo(user) + + redirect_url = "" + + if request.creator == user: + if request.is_pending_financial_verification: + redirect_url = url_for( + "requests.financial_verification", request_id=request.id + ) + elif request.is_pending_financial_verification_changes: + redirect_url = url_for( + "requests.financial_verification", request_id=request.id, extended=True + ) + elif request.is_approved: + redirect_url = url_for( + "requests.view_request_details", request_id=request.id + ) + else: + redirect_url = url_for( + "requests.requests_form_update", screen=1, request_id=request.id + ) + elif is_ccpo: + redirect_url = url_for("requests.approval", request_id=request.id) + + return redirect(redirect_url) diff --git a/tests/routes/test_requests_index.py b/tests/routes/test_requests_index.py index 29300b65..6de9bf5e 100644 --- a/tests/routes/test_requests_index.py +++ b/tests/routes/test_requests_index.py @@ -23,18 +23,3 @@ def test_action_required_ccpo(): context = RequestsIndex(ccpo).execute() assert context["num_action_required"] == 1 - - -def test_ccpo_sees_approval_screen(): - ccpo = UserFactory.from_atat_role("ccpo") - request = RequestFactory.create() - Requests.submit(request) - ccpo_context = RequestsIndex(ccpo).execute() - assert ccpo_context["requests"][0]["edit_link"] == url_for( - "requests.approval", request_id=request.id - ) - - mo_context = RequestsIndex(request.creator).execute() - assert mo_context["requests"][0]["edit_link"] != url_for( - "requests.approval", request_id=request.id - ) From b4b96f2e7e2d1692c87300a6e2db87858f60de3e Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 31 Oct 2018 11:10:38 -0400 Subject: [PATCH 10/11] Use requests/edit link in status change email --- templates/emails/request_status_change.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/emails/request_status_change.txt b/templates/emails/request_status_change.txt index 5ad799b2..5b48c739 100644 --- a/templates/emails/request_status_change.txt +++ b/templates/emails/request_status_change.txt @@ -2,4 +2,4 @@ Your JEDI request status has changed The status of your JEDI Cloud request - {{ request.displayname }} - was recently updated. Log in to see whether this change requires an action or response from you. -{{ url_for('requests.view_request_details', request_id=request.id, _external=True) }} +{{ url_for('requests.edit', request_id=request.id, _external=True) }} From 39b70b7057c03ab802893bb6d8c4ebf61196ef91 Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 31 Oct 2018 15:32:26 -0400 Subject: [PATCH 11/11] Add tests for /requests/edit --- tests/routes/test_request_edit.py | 52 +++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 tests/routes/test_request_edit.py diff --git a/tests/routes/test_request_edit.py b/tests/routes/test_request_edit.py new file mode 100644 index 00000000..37e7b243 --- /dev/null +++ b/tests/routes/test_request_edit.py @@ -0,0 +1,52 @@ +from tests.factories import UserFactory, RequestFactory +from atst.models.request_status_event import RequestStatus + + +def test_creator_pending_finver(client, user_session): + request = RequestFactory.create_with_status( + RequestStatus.PENDING_FINANCIAL_VERIFICATION + ) + user_session(request.creator) + response = client.get( + "/requests/edit/{}".format(request.id), follow_redirects=False + ) + assert "verify" in response.location + + +def test_creator_pending_finver_changes(client, user_session): + request = RequestFactory.create_with_status( + RequestStatus.CHANGES_REQUESTED_TO_FINVER + ) + user_session(request.creator) + response = client.get( + "/requests/edit/{}".format(request.id), follow_redirects=False + ) + assert "verify" in response.location + + +def test_creator_approved(client, user_session): + request = RequestFactory.create_with_status(RequestStatus.APPROVED) + user_session(request.creator) + response = client.get( + "/requests/edit/{}".format(request.id), follow_redirects=False + ) + assert "details" in response.location + + +def test_creator_approved(client, user_session): + request = RequestFactory.create_with_status(RequestStatus.STARTED) + user_session(request.creator) + response = client.get( + "/requests/edit/{}".format(request.id), follow_redirects=False + ) + assert "new" in response.location + + +def test_ccpo(client, user_session): + ccpo = UserFactory.from_atat_role("ccpo") + request = RequestFactory.create_with_status(RequestStatus.STARTED) + user_session(ccpo) + response = client.get( + "/requests/edit/{}".format(request.id), follow_redirects=False + ) + assert "approval" in response.location