Merge pull request #299 from dod-ccpo/details-view-tests

refactor request details rendering logic

atst/models/request.py

@@ -204,3 +204,11 @@ class Request(Base):
             review = self.latest_status.review
             if review:
                 return review.comment
+
+    @property
+    def has_financial_data(self):
+        return (
+            self.is_pending_ccpo_approval
+            or self.is_pending_financial_verification_changes
+            or self.is_approved
+        ) and self.task_order

atst/routes/requests/approval.py

@@ -20,9 +20,7 @@ def map_ccpo_authorizing(user):
 
 def render_approval(request, form=None):
     data = request.body
-    pending_final_approval = request.is_pending_ccpo_approval
+    if request.has_financial_data:
-    pending_review = request.is_pending_ccpo_acceptance or pending_final_approval
-    if pending_final_approval and request.task_order:
         data["task_order"] = request.task_order.to_dictionary()
 
     if not form:
@@ -35,8 +33,6 @@ def render_approval(request, form=None):
         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(),
     )
 

atst/routes/requests/requests_form.py

@@ -128,24 +128,18 @@ def requests_submit(request_id=None):
 @requests_bp.route("/requests/details/<string:request_id>", methods=["GET"])
 def view_request_details(request_id=None):
     request = Requests.get(g.current_user, request_id)
-    financial_review = (
-        request.is_pending_ccpo_approval
-        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:
+    if request.has_financial_data:
         data["task_order"] = request.task_order.to_dictionary()
 
     return render_template(
         "requests/details.html",
         data=data,
         request=request,
-        financial_review=financial_review,
         requires_fv_action=requires_fv_action,
     )

templates/requests/_review.html

@@ -132,7 +132,7 @@
   {{ DefinitionReviewField("DoD ID", "primary_poc", "dodid_poc") }}
 </dl>
 
-{% if financial_review %}
+{% if request.has_financial_data %}
   <hr>
   <h2>
     Financial Verification

templates/requests/approval.html

@@ -32,7 +32,7 @@
 
     </section>
 
-    {% if pending_review %}
+    {% if request.is_pending_ccpo_action %}
     <section class='request-approval__review'>
       <form method="POST" action="{{ url_for("requests.submit_approval", request_id=request.id) }}" autocomplete="off">
         {{ f.csrf_token }}

tests/routes/test_request_details.py

@@ -0,0 +1,37 @@
+import re
+from flask import url_for
+
+from atst.models.request_status_event import RequestStatus
+
+from tests.factories import RequestFactory, TaskOrderFactory, UserFactory
+
+
+def test_can_show_financial_data(client, user_session):
+    user = UserFactory.create()
+    user_session(user)
+
+    task_order = TaskOrderFactory.create()
+    request = RequestFactory.create_with_status(
+        status=RequestStatus.PENDING_CCPO_APPROVAL, task_order=task_order, creator=user
+    )
+    response = client.get(
+        url_for("requests.view_request_details", request_id=request.id)
+    )
+
+    body = response.data.decode()
+    assert re.search(">\s+Financial Verification\s+<", body)
+
+
+def test_can_not_show_financial_data(client, user_session):
+    user = UserFactory.create()
+    user_session(user)
+
+    request = RequestFactory.create_with_status(
+        status=RequestStatus.PENDING_CCPO_ACCEPTANCE, creator=user
+    )
+    response = client.get(
+        url_for("requests.view_request_details", request_id=request.id)
+    )
+
+    body = response.data.decode()
+    assert not re.search(">\s+Financial Verification\s+<", body)