From 73d385181348acb63c55c07910557f1edc993e8d Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 14 Sep 2018 10:40:59 -0400 Subject: [PATCH 1/4] request model knows its most recent review comment --- atst/models/request.py | 10 ++++++++++ tests/models/test_requests.py | 23 +++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/atst/models/request.py b/atst/models/request.py index de374907..d7436355 100644 --- a/atst/models/request.py +++ b/atst/models/request.py @@ -194,3 +194,13 @@ class Request(Base): @property def is_approved(self): return self.status == RequestStatus.APPROVED + + @property + def review_comment(self): + if ( + self.status == RequestStatus.CHANGES_REQUESTED + or self.status == RequestStatus.CHANGES_REQUESTED_TO_FINVER + ): + review = self.latest_status.review + if review: + return review.comment diff --git a/tests/models/test_requests.py b/tests/models/test_requests.py index 7f5a3650..528d0b06 100644 --- a/tests/models/test_requests.py +++ b/tests/models/test_requests.py @@ -92,3 +92,26 @@ def test_reviews(): RequestStatusEventFactory.create(revision=request.latest_revision), ] assert len(request.reviews) == 2 + + +def test_review_comment(): + request = RequestFactory.create() + ccpo = UserFactory.from_atat_role("ccpo") + request.status_events = [ + RequestStatusEventFactory.create( + revision=request.latest_revision, + new_status=RequestStatus.CHANGES_REQUESTED, + review=RequestReviewFactory.create(reviewer=ccpo, comment="do better"), + ) + ] + assert request.review_comment == "do better" + + request.status_events = [ + RequestStatusEventFactory.create( + revision=request.latest_revision, + new_status=RequestStatus.APPROVED, + review=RequestReviewFactory.create(reviewer=ccpo, comment="much better"), + ) + ] + + assert not request.review_comment From 7f9c7dcaece45021dc2492f791dc5af827290dd7 Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 14 Sep 2018 11:03:53 -0400 Subject: [PATCH 2/4] display review comment for request that needs changes --- atst/routes/requests/requests_form.py | 1 + templates/requests/_new.html | 8 ++++++++ tests/routes/test_request_new.py | 27 ++++++++++++++++++++++++++- 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/atst/routes/requests/requests_form.py b/atst/routes/requests/requests_form.py index 8e14b0e2..b0252b86 100644 --- a/atst/routes/requests/requests_form.py +++ b/atst/routes/requests/requests_form.py @@ -62,6 +62,7 @@ def requests_form_update(screen=1, request_id=None): next_screen=screen + 1, request_id=request_id, jedi_request=jedi_flow.request, + review_comment=request.review_comment, can_submit=jedi_flow.can_submit, ) diff --git a/templates/requests/_new.html b/templates/requests/_new.html index da27e795..fcdf1c95 100644 --- a/templates/requests/_new.html +++ b/templates/requests/_new.html @@ -1,11 +1,19 @@ {% extends "base.html" %} +{% from "components/alert.html" import Alert %} + {% block content %}
{% include 'requests/menu.html' %} + {% if review_comment %} + {{ Alert('Changes Requested', + message="

CCPO has requested changes to your submission with the following notes:
" + review_comment + "
Please contact info@jedi.cloud or 123-123-4567 for further discussion.

", + level='warning') }} + {% endif %} + {% block form_action %} {% if request_id %}
diff --git a/tests/routes/test_request_new.py b/tests/routes/test_request_new.py index b27f4758..9bc8a668 100644 --- a/tests/routes/test_request_new.py +++ b/tests/routes/test_request_new.py @@ -1,5 +1,12 @@ import re -from tests.factories import RequestFactory, UserFactory, RequestRevisionFactory +from tests.factories import ( + RequestFactory, + UserFactory, + RequestRevisionFactory, + RequestStatusEventFactory, + RequestReviewFactory, +) +from atst.models.request_status_event import RequestStatus from atst.domain.roles import Roles from atst.domain.requests import Requests from urllib.parse import urlencode @@ -213,3 +220,21 @@ def test_can_review_data(user_session, client): # assert a sampling of the request data is on the review page assert request.body["primary_poc"]["fname_poc"] in body assert request.body["information_about_you"]["email_request"] in body + + +def test_displays_ccpo_review_comment(user_session, client): + creator = UserFactory.create() + ccpo = UserFactory.from_atat_role("ccpo") + user_session(creator) + request = RequestFactory.create(creator=creator) + review_comment = "add all of the correct info, instead of the incorrect info" + request.status_events = [ + RequestStatusEventFactory.create( + revision=request.latest_revision, + new_status=RequestStatus.CHANGES_REQUESTED, + review=RequestReviewFactory.create(reviewer=ccpo, comment=review_comment), + ) + ] + response = client.get("/requests/new/1/{}".format(request.id)) + body = response.data.decode() + assert review_comment in body From 8359a2a079aabcf2619cacf83a441cfbbfda0201 Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 14 Sep 2018 11:33:02 -0400 Subject: [PATCH 3/4] display review comment for financial verification that needs changes --- .../routes/requests/financial_verification.py | 1 + templates/requests/_new.html | 6 +---- templates/requests/comment.html | 5 ++++ .../requests/financial_verification.html | 4 +++ tests/routes/test_financial_verification.py | 27 ++++++++++++++++++- 5 files changed, 37 insertions(+), 6 deletions(-) create mode 100644 templates/requests/comment.html diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 0be12e7b..91baaa8c 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -39,6 +39,7 @@ def financial_verification(request_id=None): "requests/financial_verification.html", f=form, request=request, + review_comment=request.review_comment, extended=is_extended(request), ) diff --git a/templates/requests/_new.html b/templates/requests/_new.html index fcdf1c95..1da2f66f 100644 --- a/templates/requests/_new.html +++ b/templates/requests/_new.html @@ -1,7 +1,5 @@ {% extends "base.html" %} -{% from "components/alert.html" import Alert %} - {% block content %}
@@ -9,9 +7,7 @@ {% include 'requests/menu.html' %} {% if review_comment %} - {{ Alert('Changes Requested', - message="

CCPO has requested changes to your submission with the following notes:
" + review_comment + "
Please contact info@jedi.cloud or 123-123-4567 for further discussion.

", - level='warning') }} + {% include 'requests/comment.html' %} {% endif %} {% block form_action %} diff --git a/templates/requests/comment.html b/templates/requests/comment.html new file mode 100644 index 00000000..9d76e3dd --- /dev/null +++ b/templates/requests/comment.html @@ -0,0 +1,5 @@ +{% from "components/alert.html" import Alert %} + +{{ Alert('Changes Requested', + message="

CCPO has requested changes to your submission with the following notes:
" + review_comment + "
Please contact info@jedi.cloud or 123-123-4567 for further discussion.

", + level='warning') }} diff --git a/templates/requests/financial_verification.html b/templates/requests/financial_verification.html index 25a1c0b9..4ae287af 100644 --- a/templates/requests/financial_verification.html +++ b/templates/requests/financial_verification.html @@ -12,6 +12,10 @@ {{ Alert('Pending Financial Verification', fragment="fragments/pending_financial_verification.html") }} {% endif %} +{% if review_comment %} + {% include 'requests/comment.html' %} +{% endif %} +
diff --git a/tests/routes/test_financial_verification.py b/tests/routes/test_financial_verification.py index 71c38f00..1a2c4606 100644 --- a/tests/routes/test_financial_verification.py +++ b/tests/routes/test_financial_verification.py @@ -3,9 +3,16 @@ import pytest from flask import url_for from atst.eda_client import MockEDAClient +from atst.models.request_status_event import RequestStatus from tests.mocks import MOCK_REQUEST, MOCK_USER -from tests.factories import PENumberFactory, RequestFactory, UserFactory +from tests.factories import ( + PENumberFactory, + RequestFactory, + UserFactory, + RequestStatusEventFactory, + RequestReviewFactory, +) class TestPENumberInForm: @@ -148,3 +155,21 @@ class TestPENumberInForm: response = self.submit_data(client, user, data, extended=True) assert response.status_code == 200 + + +def test_displays_ccpo_review_comment(user_session, client): + creator = UserFactory.create() + ccpo = UserFactory.from_atat_role("ccpo") + user_session(creator) + request = RequestFactory.create(creator=creator) + review_comment = "add all of the correct info, instead of the incorrect info" + request.status_events = [ + RequestStatusEventFactory.create( + revision=request.latest_revision, + new_status=RequestStatus.CHANGES_REQUESTED_TO_FINVER, + review=RequestReviewFactory.create(reviewer=ccpo, comment=review_comment), + ) + ] + response = client.get("/requests/verify/{}".format(request.id)) + body = response.data.decode() + assert review_comment in body From 66ab56e518c9a75f16da5dc15a33de60b94ff70b Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 17 Sep 2018 09:15:58 -0400 Subject: [PATCH 4/4] use callable form of alert macro for request changes alert --- templates/requests/comment.html | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/templates/requests/comment.html b/templates/requests/comment.html index 9d76e3dd..2558ff90 100644 --- a/templates/requests/comment.html +++ b/templates/requests/comment.html @@ -1,5 +1,9 @@ {% from "components/alert.html" import Alert %} -{{ Alert('Changes Requested', - message="

CCPO has requested changes to your submission with the following notes:
" + review_comment + "
Please contact info@jedi.cloud or 123-123-4567 for further discussion.

", - level='warning') }} +{% call Alert('Changes Requested', level='warning') %} +

CCPO has requested changes to your submission with the following notes: +
+ {{ review_comment }} +
+ Please contact info@jedi.cloud or 123-123-4567 for further discussion.

+{% endcall %}