From 01fca617e70e2ff689008e9f1cd568e1cb3b40aa Mon Sep 17 00:00:00 2001
From: dandds <dan-ctr@friends.dds.mil>
Date: Tue, 18 Sep 2018 11:25:33 -0400
Subject: [PATCH] refactor request details rendering logic

---
 atst/models/request.py                |  8 ++++++
 atst/routes/requests/approval.py      |  6 +----
 atst/routes/requests/requests_form.py |  8 +-----
 templates/requests/_review.html       |  2 +-
 templates/requests/approval.html      |  2 +-
 tests/routes/test_request_details.py  | 37 +++++++++++++++++++++++++++
 6 files changed, 49 insertions(+), 14 deletions(-)
 create mode 100644 tests/routes/test_request_details.py

diff --git a/atst/models/request.py b/atst/models/request.py
index 7a497222..d7072d35 100644
--- a/atst/models/request.py
+++ b/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
diff --git a/atst/routes/requests/approval.py b/atst/routes/requests/approval.py
index 28470b53..808bc7eb 100644
--- a/atst/routes/requests/approval.py
+++ b/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
-    pending_review = request.is_pending_ccpo_acceptance or pending_final_approval
-    if pending_final_approval and request.task_order:
+    if request.has_financial_data:
         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(),
     )
 
diff --git a/atst/routes/requests/requests_form.py b/atst/routes/requests/requests_form.py
index 925a561c..ae348ee4 100644
--- a/atst/routes/requests/requests_form.py
+++ b/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,
     )
diff --git a/templates/requests/_review.html b/templates/requests/_review.html
index f070edd4..e48f27d0 100644
--- a/templates/requests/_review.html
+++ b/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
diff --git a/templates/requests/approval.html b/templates/requests/approval.html
index 5fff0b8a..316ea8d7 100644
--- a/templates/requests/approval.html
+++ b/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 }}
diff --git a/tests/routes/test_request_details.py b/tests/routes/test_request_details.py
new file mode 100644
index 00000000..bd644182
--- /dev/null
+++ b/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)