diff --git a/atst/domain/authz.py b/atst/domain/authz.py index fc4b4c32..8a304fb6 100644 --- a/atst/domain/authz.py +++ b/atst/domain/authz.py @@ -29,3 +29,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/domain/requests/requests.py b/atst/domain/requests/requests.py index 4e92c46f..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 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..3c4c9f87 --- /dev/null +++ b/atst/domain/requests/status_event_handler.py @@ -0,0 +1,35 @@ +from flask import render_template + +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): + if (old_status, new_status) in self.STATUS_TRANSITIONS: + self._send_email(request) + + 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/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/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/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/templates/emails/request_status_change.txt b/templates/emails/request_status_change.txt new file mode 100644 index 00000000..5b48c739 --- /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.displayname }} - was recently updated. Log in to see whether this change requires an action or response from you. + +{{ url_for('requests.edit', request_id=request.id, _external=True) }} diff --git a/tests/conftest.py b/tests/conftest.py index 50a555e7..67912c1d 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,9 @@ def extended_financial_verification_data(pdf_upload): "clin_2003": "7000", "task_order": pdf_upload, } + + +@pytest.fixture(scope="function", autouse=True) +def queue(): + yield atst_queue + atst_queue.get_queue().empty() diff --git a/tests/domain/test_requests.py b/tests/domain/test_requests.py index 90e3655d..23e2122e 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,44 @@ def test_random_user_cannot_view_request(): request = RequestFactory.create() assert not RequestsAuthorization(user, request).can_view + + +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(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_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_submitted_does_not_trigger_notification(self, queue): + request = RequestFactory.create() + request = Requests.set_status(request, RequestStatus.SUBMITTED) + assert len(queue.get_queue()) == 0 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 diff --git a/tests/routes/test_request_new.py b/tests/routes/test_request_new.py index 87842c94..81d195a8 100644 --- a/tests/routes/test_request_new.py +++ b/tests/routes/test_request_new.py @@ -237,13 +237,11 @@ 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 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 - ) 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