diff --git a/atst/forms/ccpo_review.py b/atst/forms/ccpo_review.py index 8b6ea64d..2636d529 100644 --- a/atst/forms/ccpo_review.py +++ b/atst/forms/ccpo_review.py @@ -7,7 +7,10 @@ from .validators import Alphabet, PhoneNumber class CCPOReviewForm(ValidatedForm): - comment = TextAreaField("Comments (optional)") + comment = TextAreaField( + "Instructions or comments", + description="Provide instructions or notes for additional information that is necessary to approve the request here. The requestor may then re-submit the updated request or initiate contact outside of AT-AT if further discussion is required. This message will be shared with the person making the JEDI request..", + ) fname_mao = StringField( "First Name (optional)", validators=[Optional(), Alphabet()] ) diff --git a/atst/forms/internal_comment.py b/atst/forms/internal_comment.py index a64833c8..71b88f7e 100644 --- a/atst/forms/internal_comment.py +++ b/atst/forms/internal_comment.py @@ -5,4 +5,8 @@ from .forms import ValidatedForm class InternalCommentForm(ValidatedForm): - text = TextAreaField(validators=[Optional()]) + text = TextAreaField( + "CCPO Internal Notes", + description="You may add additional comments and notes for internal CCPO reference and follow-up here.", + validators=[Optional()], + ) 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/atst/models/request_review.py b/atst/models/request_review.py index c46f1d11..20fa1ed4 100644 --- a/atst/models/request_review.py +++ b/atst/models/request_review.py @@ -8,7 +8,7 @@ class RequestReview(Base): __tablename__ = "request_reviews" id = Column(BigInteger, primary_key=True) - status = relationship("RequestStatusEvent", back_populates="review") + status = relationship("RequestStatusEvent", uselist=False, back_populates="review") user_id = Column(ForeignKey("users.id"), nullable=False) reviewer = relationship("User") diff --git a/atst/models/request_status_event.py b/atst/models/request_status_event.py index 283633b1..4680d162 100644 --- a/atst/models/request_status_event.py +++ b/atst/models/request_status_event.py @@ -48,6 +48,8 @@ class RequestStatusEvent(Base): def log_name(self): if self.new_status == RequestStatus.CHANGES_REQUESTED: return "Denied" + if self.new_status == RequestStatus.CHANGES_REQUESTED_TO_FINVER: + return "Denied" elif self.new_status == RequestStatus.PENDING_FINANCIAL_VERIFICATION: return "Accepted" else: diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index fb53802a..cee24c23 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -49,10 +49,9 @@ def login_redirect(): return redirect(url_for(".home")) -def _is_valid_certificate(request): - cert = request.environ.get("HTTP_X_SSL_CLIENT_CERT") - if cert: - result = app.crl_validator.validate(cert.encode()) - return result - else: - return False +@bp.route("/logout") +def logout(): + if session.get("user_id"): + del (session["user_id"]) + + return redirect(url_for(".home")) diff --git a/atst/routes/requests/approval.py b/atst/routes/requests/approval.py index 1843e127..28470b53 100644 --- a/atst/routes/requests/approval.py +++ b/atst/routes/requests/approval.py @@ -12,7 +12,6 @@ from . import requests_bp from atst.domain.requests import Requests from atst.domain.exceptions import NotFoundError from atst.forms.ccpo_review import CCPOReviewForm -from atst.forms.internal_comment import InternalCommentForm def map_ccpo_authorizing(user): @@ -26,8 +25,6 @@ def render_approval(request, form=None): if pending_final_approval and request.task_order: data["task_order"] = request.task_order.to_dictionary() - internal_comment_form = InternalCommentForm(text=request.internal_comments_text) - if not form: mo_data = map_ccpo_authorizing(g.current_user) form = CCPOReviewForm(data=mo_data) @@ -35,13 +32,12 @@ def render_approval(request, form=None): return render_template( "requests/approval.html", data=data, - status_events=reversed(request.status_events), + reviews=list(reversed(request.reviews)), request=request, current_status=request.status.value, pending_review=pending_review, financial_review=pending_final_approval, f=form or CCPOReviewForm(), - internal_comment_form=internal_comment_form, ) @@ -58,7 +54,7 @@ def submit_approval(request_id): form = CCPOReviewForm(http_request.form) if form.validate(): - if http_request.form.get("approved"): + if http_request.form.get("review") == "approving": Requests.advance(g.current_user, request, form.data) else: Requests.request_changes(g.current_user, request, form.data) @@ -84,13 +80,3 @@ def task_order_pdf_download(request_id): else: raise NotFoundError("task_order pdf") - - -@requests_bp.route("/requests/internal_comments/", methods=["POST"]) -def create_internal_comment(request_id): - form = InternalCommentForm(http_request.form) - if form.validate(): - request = Requests.get(g.current_user, request_id) - Requests.update_internal_comments(g.current_user, request, form.data["text"]) - - return redirect(url_for("requests.approval", request_id=request_id)) diff --git a/atst/routes/requests/financial_verification.py b/atst/routes/requests/financial_verification.py index 728f7f39..91baaa8c 100644 --- a/atst/routes/requests/financial_verification.py +++ b/atst/routes/requests/financial_verification.py @@ -38,7 +38,8 @@ def financial_verification(request_id=None): return render_template( "requests/financial_verification.html", f=form, - request_id=request_id, + request=request, + review_comment=request.review_comment, extended=is_extended(request), ) @@ -49,7 +50,7 @@ def update_financial_verification(request_id): existing_request = Requests.get(g.current_user, request_id) form = financial_form(existing_request, post_data) rerender_args = dict( - request_id=request_id, f=form, extended=is_extended(existing_request) + request=existing_request, f=form, extended=is_extended(existing_request) ) if form.validate(): diff --git a/atst/routes/requests/requests_form.py b/atst/routes/requests/requests_form.py index 80f76931..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, ) @@ -132,6 +133,10 @@ def view_request_details(request_id=None): or request.is_approved or request.is_pending_financial_verification_changes ) + requires_fv_action = ( + request.is_pending_financial_verification + or request.is_pending_financial_verification_changes + ) data = request.body if financial_review and request.task_order: @@ -140,10 +145,6 @@ def view_request_details(request_id=None): return render_template( "requests/details.html", data=data, - request_id=request.id, - status=request.status_displayname, - pending_review=request.is_pending_ccpo_action, - financial_verification=request.is_pending_financial_verification - or request.is_pending_financial_verification_changes, - financial_review=financial_review, + request=request, + requires_fv_action=requires_fv_action, ) diff --git a/js/components/forms/ccpo_approval.js b/js/components/forms/ccpo_approval.js new file mode 100644 index 00000000..7fe245ed --- /dev/null +++ b/js/components/forms/ccpo_approval.js @@ -0,0 +1,28 @@ +import textinput from '../text_input' + +export default { + name: 'ccpo-approval', + + components: { + textinput + }, + + data: function () { + return { + approving: false, + denying: false + } + }, + + methods: { + setReview: function (e) { + if (e.target.value === 'approving') { + this.approving = true + this.denying = false + } else { + this.approving = false + this.denying = true + } + }, + } +} diff --git a/js/components/local_datetime.js b/js/components/local_datetime.js new file mode 100644 index 00000000..94a338dc --- /dev/null +++ b/js/components/local_datetime.js @@ -0,0 +1,21 @@ +import { format } from 'date-fns' + +export default { + name: 'local-datetime', + + props: { + timestamp: String, + format: { + type: String, + default: 'MMM D YYYY H:mm' + } + }, + + computed: { + displayTime: function () { + return format(this.timestamp, this.format) + } + }, + + template: '' +} diff --git a/js/components/text_input.js b/js/components/text_input.js index 8e4f8048..7d0bb78d 100644 --- a/js/components/text_input.js +++ b/js/components/text_input.js @@ -19,7 +19,8 @@ export default { default: () => '' }, initialErrors: Array, - paragraph: String + paragraph: String, + noMaxWidth: String }, data: function () { diff --git a/js/index.js b/js/index.js index 375a90f2..2d195ce8 100644 --- a/js/index.js +++ b/js/index.js @@ -17,6 +17,8 @@ import Modal from './mixins/modal' import selector from './components/selector' import BudgetChart from './components/charts/budget_chart' import SpendTable from './components/tables/spend_table' +import CcpoApproval from './components/forms/ccpo_approval' +import LocalDatetime from './components/local_datetime' Vue.use(VTooltip) @@ -35,7 +37,9 @@ const app = new Vue({ NewProject, selector, BudgetChart, - SpendTable + SpendTable, + CcpoApproval, + LocalDatetime }, mounted: function() { const modalOpen = document.querySelector("#modalOpen") diff --git a/styles/components/_alerts.scss b/styles/components/_alerts.scss index e2da79fd..0aa6aee4 100644 --- a/styles/components/_alerts.scss +++ b/styles/components/_alerts.scss @@ -9,6 +9,10 @@ border-left-width: $gap / 2; border-left-style: solid; @include panel-margin; + + @include media($medium-screen) { + padding: $gap * 4; + } } @mixin alert-level($level) { @@ -53,6 +57,10 @@ .alert__title { @include h3; margin-top: 0; + + &:last-child { + margin-bottom: 0; + } } .alert__content { diff --git a/styles/elements/_action_group.scss b/styles/elements/_action_group.scss index fc38eb04..2e1fb191 100644 --- a/styles/elements/_action_group.scss +++ b/styles/elements/_action_group.scss @@ -5,6 +5,10 @@ margin-top: $gap * 4; margin-bottom: $gap * 4; + &--tight { + margin-top: $gap * 2; + } + .usa-button, a { margin: 0 0 0 ($gap * 2); diff --git a/styles/elements/_inputs.scss b/styles/elements/_inputs.scss index 84167345..babd6ce5 100644 --- a/styles/elements/_inputs.scss +++ b/styles/elements/_inputs.scss @@ -246,6 +246,19 @@ } } + &.no-max-width { + padding-right: $gap * 3; + + input, textarea, select, label { + max-width: none; + } + + .icon-validation { + left: auto; + right: - $gap * 4; + } + } + &.usa-input--error { @include input-state('error'); } diff --git a/styles/elements/_labels.scss b/styles/elements/_labels.scss index 104894e4..1e994013 100644 --- a/styles/elements/_labels.scss +++ b/styles/elements/_labels.scss @@ -15,6 +15,7 @@ margin: 0 $gap; padding: 0 $gap; border-radius: $gap / 2; + white-space: nowrap; &.label--info { background-color: $color-primary; diff --git a/styles/elements/_panels.scss b/styles/elements/_panels.scss index fd10671d..7d1b31d4 100644 --- a/styles/elements/_panels.scss +++ b/styles/elements/_panels.scss @@ -63,6 +63,7 @@ .panel__heading { padding: $gap * 2; + @include media($medium-screen) { padding: $gap * 4; } @@ -71,6 +72,10 @@ padding: $gap*2; } + &--divider { + border-bottom: 1px solid $color-gray-light; + } + h1, h2, h3, h4, h5, h6 { margin: 0; display: inline-block; @@ -90,15 +95,16 @@ justify-content: space-between; } } + + hr { + border: 0; + border-bottom: 1px dashed $color-gray-light; + margin: ($gap * 4) ($site-margins*-4); + } } .panel__actions { @include panel-actions; } -hr { - border: 0; - border-bottom: 1px dashed $color-gray-light; - margin: 0px $site-margins*-4; -} diff --git a/styles/sections/_request_approval.scss b/styles/sections/_request_approval.scss index 5769d5d9..f4b36140 100644 --- a/styles/sections/_request_approval.scss +++ b/styles/sections/_request_approval.scss @@ -32,6 +32,12 @@ } } + .request-approval__review { + .action-group { + margin-bottom: $gap * 6; + } + } + .approval-log { ol { list-style: none; @@ -43,7 +49,7 @@ border-top: 1px dashed $color-gray-light; &:first-child { - border-top-style: solid; + border-top: none; } @include media($medium-screen) { @@ -94,4 +100,10 @@ } } } + + .internal-notes { + textarea { + resize: vertical; + } + } } diff --git a/templates/components/text_input.html b/templates/components/text_input.html index ecd29f5c..f1cfb567 100644 --- a/templates/components/text_input.html +++ b/templates/components/text_input.html @@ -1,24 +1,35 @@ {% from "components/icon.html" import Icon %} {% from "components/tooltip.html" import Tooltip %} -{% macro TextInput(field, tooltip='', placeholder='', validation='anything', paragraph=False, initial_value='') -%} +{% macro TextInput( + field, + label=field.label | striptags, + description=field.description, + tooltip='', + placeholder='', + validation='anything', + paragraph=False, + initial_value='', + noMaxWidth=False) -%} +
+ v-bind:class="['usa-input usa-input--validation--' + validation, { 'usa-input--error': showError, 'usa-input--success': showValid, 'usa-input--validation--paragraph': paragraph, 'no-max-width': noMaxWidth }]">
diff --git a/templates/requests/_new.html b/templates/requests/_new.html index da27e795..1da2f66f 100644 --- a/templates/requests/_new.html +++ b/templates/requests/_new.html @@ -6,6 +6,10 @@ {% include 'requests/menu.html' %} + {% if review_comment %} + {% include 'requests/comment.html' %} + {% endif %} + {% block form_action %} {% if request_id %}
diff --git a/templates/requests/_review.html b/templates/requests/_review.html index d93a7ed2..a01272b7 100644 --- a/templates/requests/_review.html +++ b/templates/requests/_review.html @@ -6,8 +6,9 @@
{{ title | safe }}
- {% if data[section] and data[section][item_name] %} - {{ data[section][item_name] | findFilter(filter, filter_args) }} + {% set value = data.get(section, {}).get(item_name) %} + {% if value is not none %} + {{ value | findFilter(filter, filter_args) }} {% else %} {{ RequiredLabel() }} {% endif %} @@ -23,8 +24,6 @@ {% endif %} {% endmacro %} - -

Details of Use {% if editable %} diff --git a/templates/requests/approval.html b/templates/requests/approval.html index b9559712..5fff0b8a 100644 --- a/templates/requests/approval.html +++ b/templates/requests/approval.html @@ -16,8 +16,9 @@ {% endif %}
-
-

Request #{{ request.id }}

+
+

Request #{{ request.id }} +

{{ current_status }}
@@ -32,153 +33,162 @@
{% if pending_review %} - - {{ f.csrf_token }} -
-
-

Approval Notes

-
+
+ + {{ f.csrf_token }} -
+ +
+
+
+

Review this Request

-
+
+
+ + -

Instructions for the Requestor

+ + +
+
- Provide instructions or notes for additional information that is necessary to approve the request here. The requestor may then re-submit the updated request or initiate contact outside of AT-AT if further discussion is required. These notes will be visible to the person making the JEDI Cloud request. +
+

Message to Requestor (optional)

+
+ {{ TextInput( + f.comment, + label='Approval comments or notes', + description='Provide any comments or notes regarding the approval of this request. This message will be shared with the person making the JEDI request..', + paragraph=True, + noMaxWidth=True + ) }} +
- {{ TextInput(f.comment, paragraph=True, placeholder="Add notes or comments explaining what changes are being requested or why further discussion is needed about this request.") }} +
+ {{ TextInput( + f.comment, + label='Revision instructions or notes', + paragraph=True, + noMaxWidth=True + ) }} +
+
-
+
-
-
-

Authorizing Officials

-

This section is not visible to the person making the request. It is only viewable by CCPO staff.

-

Provide the name of the key officials for both parties that have authorized this request to move forward.

+

Authorizing Officials (optional)

+

Provide the name of the key officials for both parties that have authorized this request to move forward. This section is not visible to the person making the request. It is only viewable by CCPO staff.

+
+ +

Mission Authorizing Official

+ +
+
+ {{ TextInput(f.fname_mao, placeholder="First name of mission authorizing official") }} +
+ +
+ {{ TextInput(f.lname_mao, placeholder="Last name of mission authorizing official") }} +
+
+ +
+
+ {{ TextInput(f.email_mao, placeholder="name@mail.mil", validation='email') }} +
+ +
+ {{ TextInput(f.phone_mao, placeholder="(123) 456-7890", validation='usPhone') }} +
+
+ +
+ +

CCPO Authorizing Official

+ +
+
+ {{ TextInput(f.fname_ccpo, placeholder="First name of CCPO authorizing official") }} +
+ +
+ {{ TextInput(f.lname_ccpo, placeholder="Last name of CCPO authorizing official") }} +
+
+
+
+
+ +
+ + + + {{ Icon('x') }} + Cancel +
-

Mission Authorizing Official

- - -
- -
- {{ TextInput(f.fname_mao, placeholder="First name of mission authorizing official") }} -
- -
- {{ TextInput(f.lname_mao, placeholder="Last name of mission authorizing official") }} -
- -
- -
- -
- {{ TextInput(f.email_mao, placeholder="name@mail.mil") }} -
- - -
- {{ TextInput(f.phone_mao, placeholder="(123) 456-7890", validation='usPhone') }} -
- -
- - -

CCPO Authorizing Official

- -
- -
- {{ TextInput(f.fname_ccpo, placeholder="First name of CCPO authorizing official") }} -
- -
- {{ TextInput(f.lname_ccpo, placeholder="Last name of CCPO authorizing official") }} -
- -
- - - -
- -
- - - - {{ Icon('x') }} - Cancel - -
- + + +
{% endif %} -
-

CCPO Internal Notes

-

You may add additional comments and notes for internal CCPO reference and follow-up here.

-
-
-
- {{ internal_comment_form.csrf_token }} - {{ TextInput(internal_comment_form.text, paragraph=True) }} - -
-
-
-

- +
+
+
+

CCPO Activity Log

+
-
-
-

Approval Log

-
-
+ {% if reviews %}
    + {% for review in reviews %} +
  1. +
    +
    + {{ review.log_name }} +

    {{ review.status.log_name }} by {{ review.full_name_reviewer }}

    + {% if review.comment %} +

    {{ review.comment }}

    + {% endif %} - {% for status_event in status_events %} - {% if status_event.review %} -
  2. -
    -
    -

    {{ status_event.log_name }} by {{ status_event.review.full_name_reviewer }}

    - {% if status_event.review.comment %} -

    {{ status_event.review.comment }}

    +
    + {% if review.lname_mao %} +
    +

    Mission Owner approval on behalf of:

    + {{ review.full_name_mao }} + {{ review.email_mao }} + {{ review.phone_mao }} +
    {% endif %} -
    - {% if status_event.review.lname_mao %} -
    -

    Mission Owner approval on behalf of:

    - {{ status_event.review.full_name_mao }} - {{ status_event.review.email_mao }} - {{ status_event.review.phone_mao }} -
    - {% endif %} - - {% if status_event.review.lname_ccpo %} -
    -

    CCPO approval on behalf of:

    - {{ status_event.review.full_name_ccpo }} -
    - {% endif %} -
    + {% if review.lname_ccpo %} +
    +

    CCPO approval on behalf of:

    + {{ review.full_name_ccpo }} +
    + {% endif %}
    - {% set timestamp=status_event.time_created | formattedDate("%Y-%m-%d %H:%M:%S %Z") %} -
    -
    -
    3. - {% endif %} +
    + {% set timestamp=review.status.time_created | formattedDate("%Y-%m-%d %H:%M:%S %Z") %} +
    + +
    +
    +
    3. {% endfor %} -
+ {% else %} +
+

No CCPO approvals or request changes have been recorded yet.

+
+ {% endif %}
+ {% endblock %} diff --git a/templates/requests/comment.html b/templates/requests/comment.html new file mode 100644 index 00000000..2558ff90 --- /dev/null +++ b/templates/requests/comment.html @@ -0,0 +1,9 @@ +{% from "components/alert.html" import Alert %} + +{% 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 %} diff --git a/templates/requests/details.html b/templates/requests/details.html index 07bd49a7..89bdd105 100644 --- a/templates/requests/details.html +++ b/templates/requests/details.html @@ -5,21 +5,28 @@ {% block content %}
- {% if financial_verification %} - {% include 'requests/review_menu.html' %} - {% endif %} + {% if request.is_pending_ccpo_acceptance %} + + {{ Alert('Request submitted. Approval pending.', fragment="fragments/pending_ccpo_acceptance_alert.html") }} + + {% elif request.is_pending_ccpo_approval %} + + {% call Alert('Pending CCPO Approval') %} +

The CCPO will review and respond to your Financial Verification submission in 3 business days. You will be notified via email or phone.

+

Once the financial verification is approved you will be invited to create your JEDI Workspace and set-up your projects. Click here for more details.

+ {% endcall %} + + {% elif requires_fv_action %} + + {{ Alert('Pending Financial Verification', fragment="fragments/pending_financial_verification.html") }} + {% include 'requests/review_menu.html' %} - {% if pending_review %} - {{ Alert('Your request is being reviewed', - message="

You cannot edit your submitted request while it is under review. Your request will be reviewed within 3 business days.

", - level='warning' - ) }} {% endif %}

Request Details


-

#{{ request_id }} {{ status }}

+

#{{ request.id }} {{ request.status_displayname }}

diff --git a/templates/requests/financial_verification.html b/templates/requests/financial_verification.html index f3950f00..4ae287af 100644 --- a/templates/requests/financial_verification.html +++ b/templates/requests/financial_verification.html @@ -8,6 +8,14 @@ {% include 'requests/review_menu.html' %} +{% if request.is_pending_financial_verification %} + {{ Alert('Pending Financial Verification', fragment="fragments/pending_financial_verification.html") }} +{% endif %} + +{% if review_comment %} + {% include 'requests/comment.html' %} +{% endif %} +
@@ -19,21 +27,18 @@ {% endif %} {% if f.is_missing_task_order_number %} - {% set extended_url = url_for('requests.financial_verification', request_id=request_id, extended=True) %} - {{ Alert('Task Order not found in EDA', - message="We could not find your Task Order in our system of record, EDA. - Please confirm that you have entered it correctly.
- Enter Task Order information manually - "|format(extended_url), - level='warning' - ) }} + {% set extended_url = url_for('requests.financial_verification', request_id=request.id, extended=True) %} + {% call Alert('Task Order not found in EDA', level='warning') %} + We could not find your Task Order in our system of record, EDA. Please confirm that you have entered it correctly.
+ Enter Task Order information manually + {% endcall %} {% endif %} {% block form_action %} {% if extended %} -
+ {% else %} - + {% endif %} {% endblock %} @@ -52,7 +57,7 @@

Financial Verification

-

Order #{{ request_id }}

+

Order #{{ request.id }}

diff --git a/templates/requests/index.html b/templates/requests/index.html index dfee7184..9411776d 100644 --- a/templates/requests/index.html +++ b/templates/requests/index.html @@ -45,18 +45,6 @@ {% else %} - {% if pending_financial_verification %} - - {{ Alert('Pending Financial Verification', fragment="fragments/pending_financial_verification.html") }} - - {% endif %} - - {% if pending_ccpo_approval %} - - {{ Alert('Request submitted. Approval pending.', fragment="fragments/pending_ccpo_approval_alert.html") }} - - {% endif %} - {% if extended_view %}
diff --git a/templates/requests/review_menu.html b/templates/requests/review_menu.html index b8546233..f6f36aa0 100644 --- a/templates/requests/review_menu.html +++ b/templates/requests/review_menu.html @@ -1,5 +1,5 @@ -{% set pending_url=url_for('requests.view_request_details', request_id=request_id) %} -{% set financial_url=url_for('requests.financial_verification', request_id=request_id) %} +{% set pending_url=url_for('requests.view_request_details', request_id=request.id) %} +{% set financial_url=url_for('requests.financial_verification', request_id=request.id) %}
  • 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 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 diff --git a/tests/routes/test_request_approval.py b/tests/routes/test_request_approval.py index 1955a04f..205bb28e 100644 --- a/tests/routes/test_request_approval.py +++ b/tests/routes/test_request_approval.py @@ -87,7 +87,7 @@ def test_can_submit_request_approval(client, user_session): status=RequestStatus.PENDING_CCPO_ACCEPTANCE ) review_data = RequestReviewFactory.dictionary() - review_data["approved"] = True + review_data["review"] = "approving" response = client.post( url_for("requests.submit_approval", request_id=request.id), data=review_data ) @@ -102,7 +102,7 @@ def test_can_submit_request_denial(client, user_session): status=RequestStatus.PENDING_CCPO_ACCEPTANCE ) review_data = RequestReviewFactory.dictionary() - review_data["denied"] = True + review_data["review"] = "denying" response = client.post( url_for("requests.submit_approval", request_id=request.id), data=review_data ) 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 diff --git a/tests/test_auth.py b/tests/test_auth.py index 2bf24b69..c6af8448 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -1,3 +1,5 @@ +from urllib.parse import urlparse + import pytest from flask import session, url_for from .mocks import DOD_SDN_INFO, DOD_SDN, FIXTURE_EMAIL_ADDRESS @@ -14,6 +16,17 @@ def _fetch_user_info(c, t): return MOCK_USER +def _login(client, verify="SUCCESS", sdn=DOD_SDN, cert=""): + return client.get( + url_for("atst.login_redirect"), + environ_base={ + "HTTP_X_SSL_CLIENT_VERIFY": verify, + "HTTP_X_SSL_CLIENT_S_DN": sdn, + "HTTP_X_SSL_CLIENT_CERT": cert, + }, + ) + + def test_successful_login_redirect_non_ccpo(client, monkeypatch): monkeypatch.setattr( "atst.domain.authnid.AuthenticationContext.authenticate", lambda *args: True @@ -23,14 +36,7 @@ def test_successful_login_redirect_non_ccpo(client, monkeypatch): lambda *args: UserFactory.create(), ) - resp = client.get( - "/login-redirect", - environ_base={ - "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", - "HTTP_X_SSL_CLIENT_S_DN": "", - "HTTP_X_SSL_CLIENT_CERT": "", - }, - ) + resp = _login(client) assert resp.status_code == 302 assert "home" in resp.headers["Location"] @@ -47,14 +53,7 @@ def test_successful_login_redirect_ccpo(client, monkeypatch): lambda *args: UserFactory.create(atat_role=role), ) - resp = client.get( - "/login-redirect", - environ_base={ - "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", - "HTTP_X_SSL_CLIENT_S_DN": "", - "HTTP_X_SSL_CLIENT_CERT": "", - }, - ) + resp = _login(client) assert resp.status_code == 302 assert "home" in resp.headers["Location"] @@ -62,7 +61,7 @@ def test_successful_login_redirect_ccpo(client, monkeypatch): def test_unsuccessful_login_redirect(client, monkeypatch): - resp = client.get("/login-redirect") + resp = client.get(url_for("atst.login_redirect")) assert resp.status_code == 401 assert "user_id" not in session @@ -99,26 +98,12 @@ def test_crl_validation_on_login(client): bad_cert = open("ssl/client-certs/bad-atat.mil.crt").read() # bad cert is on the test CRL - resp = client.get( - "/login-redirect", - environ_base={ - "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", - "HTTP_X_SSL_CLIENT_S_DN": DOD_SDN, - "HTTP_X_SSL_CLIENT_CERT": bad_cert, - }, - ) + resp = _login(client, cert=bad_cert) assert resp.status_code == 401 assert "user_id" not in session # good cert is not on the test CRL, passes - resp = client.get( - "/login-redirect", - environ_base={ - "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", - "HTTP_X_SSL_CLIENT_S_DN": DOD_SDN, - "HTTP_X_SSL_CLIENT_CERT": good_cert, - }, - ) + resp = _login(client, cert=good_cert) assert session["user_id"] @@ -132,14 +117,7 @@ def test_creates_new_user_on_login(monkeypatch, client): with pytest.raises(NotFoundError): Users.get_by_dod_id(DOD_SDN_INFO["dod_id"]) - resp = client.get( - "/login-redirect", - environ_base={ - "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", - "HTTP_X_SSL_CLIENT_S_DN": DOD_SDN, - "HTTP_X_SSL_CLIENT_CERT": cert_file, - }, - ) + resp = _login(client, cert=cert_file) user = Users.get_by_dod_id(DOD_SDN_INFO["dod_id"]) assert user.first_name == DOD_SDN_INFO["first_name"] @@ -148,23 +126,36 @@ def test_creates_new_user_on_login(monkeypatch, client): def test_creates_new_user_without_email_on_login(monkeypatch, client): - monkeypatch.setattr("atst.routes._is_valid_certificate", lambda *args: True) cert_file = open("ssl/client-certs/atat.mil.crt").read() # ensure user does not exist with pytest.raises(NotFoundError): Users.get_by_dod_id(DOD_SDN_INFO["dod_id"]) - resp = client.get( - "/login-redirect", - environ_base={ - "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", - "HTTP_X_SSL_CLIENT_S_DN": DOD_SDN, - "HTTP_X_SSL_CLIENT_CERT": cert_file, - }, - ) + resp = _login(client, cert=cert_file) user = Users.get_by_dod_id(DOD_SDN_INFO["dod_id"]) assert user.first_name == DOD_SDN_INFO["first_name"] assert user.last_name == DOD_SDN_INFO["last_name"] assert user.email == None + + +def test_logout(app, client, monkeypatch): + monkeypatch.setattr( + "atst.domain.authnid.AuthenticationContext.authenticate", lambda s: True + ) + monkeypatch.setattr( + "atst.domain.authnid.AuthenticationContext.get_user", + lambda s: UserFactory.create(), + ) + # create a real session + resp = _login(client) + resp_success = client.get(url_for("requests.requests_index")) + # verify session is valid + assert resp_success.status_code == 200 + client.get(url_for("atst.logout")) + resp_failure = client.get(url_for("requests.requests_index")) + # verify that logging out has cleared the session + assert resp_failure.status_code == 302 + destination = urlparse(resp_failure.headers["Location"]).path + assert destination == url_for("atst.root")