From 001a31d617a3b4db43e6c054c772fda31856d9e3 Mon Sep 17 00:00:00 2001
From: Pete Andrew <peter.andrew@torchbox.com>
Date: Tue, 28 Apr 2020 14:28:22 +0100
Subject: [PATCH] Ref 1905: Improve display of Add Review action

- Added review button as primary action. Uses existing review button but with altered text for complete draft review
- Button is only displayed when the user hasn't completed a review and when the submission is in internal or external review phases
---
 hypha/apply/funds/models/submissions.py       |   8 ++
 .../applicationsubmission_admin_detail.html   |   4 +-
 .../funds/applicationsubmission_detail.html   |   1 +
 ...ctions.html => admin_primary_actions.html} |   4 +
 .../includes/generic_primary_actions.html     |  11 ++
 .../funds/templatetags/primaryactions_tags.py |  13 ++
 hypha/apply/funds/tests/factories/models.py   |  11 +-
 hypha/apply/funds/tests/test_views.py         | 136 ++++++++++++++++++
 .../review/includes/review_button.html        |   2 +-
 9 files changed, 185 insertions(+), 5 deletions(-)
 rename hypha/apply/funds/templates/funds/includes/{actions.html => admin_primary_actions.html} (88%)
 create mode 100644 hypha/apply/funds/templates/funds/includes/generic_primary_actions.html
 create mode 100644 hypha/apply/funds/templatetags/primaryactions_tags.py

diff --git a/hypha/apply/funds/models/submissions.py b/hypha/apply/funds/models/submissions.py
index 538274ec6..bd92c6b5b 100644
--- a/hypha/apply/funds/models/submissions.py
+++ b/hypha/apply/funds/models/submissions.py
@@ -722,6 +722,14 @@ class ApplicationSubmission(
 
         return adjusted_index == len(stages)
 
+    @property
+    def in_internal_review_phase(self):
+        return self.status in PHASES_MAPPING['internal-review']['statuses']
+
+    @property
+    def in_external_review_phase(self):
+        return self.status in PHASES_MAPPING['external-review']['statuses']
+
     # Methods for accessing data on the submission
 
     def get_data(self):
diff --git a/hypha/apply/funds/templates/funds/applicationsubmission_admin_detail.html b/hypha/apply/funds/templates/funds/applicationsubmission_admin_detail.html
index 4c601f75c..854d7c9af 100644
--- a/hypha/apply/funds/templates/funds/applicationsubmission_admin_detail.html
+++ b/hypha/apply/funds/templates/funds/applicationsubmission_admin_detail.html
@@ -9,13 +9,13 @@
 {% block mobile_actions %}
     <a class="js-actions-toggle button button--white button--full-width button--actions">Actions to take</a>
     <div class="js-actions-sidebar sidebar__inner sidebar__inner--light-blue sidebar__inner--actions sidebar__inner--mobile">
-        {% include "funds/includes/actions.html"  %}
+        {% include "funds/includes/admin_primary_actions.html"  %}
     </div>
 {% endblock %}
 
 {% block sidebar_top %}
     <div class="js-actions-sidebar sidebar__inner sidebar__inner--light-blue sidebar__inner--actions {% if mobile %}sidebar__inner--mobile{% endif %}">
-        {% include "funds/includes/actions.html" %}
+        {% include "funds/includes/admin_primary_actions.html" %}
     </div>
     {% include "funds/includes/screening_form.html" %}
     {% include "funds/includes/progress_form.html" %}
diff --git a/hypha/apply/funds/templates/funds/applicationsubmission_detail.html b/hypha/apply/funds/templates/funds/applicationsubmission_detail.html
index 491ad9c57..e0d294c2b 100644
--- a/hypha/apply/funds/templates/funds/applicationsubmission_detail.html
+++ b/hypha/apply/funds/templates/funds/applicationsubmission_detail.html
@@ -95,6 +95,7 @@
 
             <aside class="sidebar">
                 {% block sidebar_top %}
+                    {% include "funds/includes/generic_primary_actions.html" %}
                 {% endblock %}
 
                 {% block flags %}
diff --git a/hypha/apply/funds/templates/funds/includes/actions.html b/hypha/apply/funds/templates/funds/includes/admin_primary_actions.html
similarity index 88%
rename from hypha/apply/funds/templates/funds/includes/actions.html
rename to hypha/apply/funds/templates/funds/includes/admin_primary_actions.html
index 56c86171f..ce24879d7 100644
--- a/hypha/apply/funds/templates/funds/includes/actions.html
+++ b/hypha/apply/funds/templates/funds/includes/admin_primary_actions.html
@@ -20,6 +20,10 @@
     {% include 'determinations/includes/determination_button.html' with submission=object class="button--bottom-space" draft_text="Complete draft determination" %}
 {% endif %}
 
+{% if object.in_internal_review_phase or object.in_external_review_phase %}
+    {% include 'review/includes/review_button.html' with submission=object class="button--full-width button--bottom-space" draft_text="Complete draft review" %}
+{% endif %}
+
 <a data-fancybox data-src="#update-status" class="button button--primary button--full-width {% if progress_form.should_show %}is-not-disabled{% else %}is-disabled{% endif %}" href="#">Update status</a>
 
 
diff --git a/hypha/apply/funds/templates/funds/includes/generic_primary_actions.html b/hypha/apply/funds/templates/funds/includes/generic_primary_actions.html
new file mode 100644
index 000000000..f6c6d4478
--- /dev/null
+++ b/hypha/apply/funds/templates/funds/includes/generic_primary_actions.html
@@ -0,0 +1,11 @@
+{% load primaryactions_tags %}
+
+{% if request.user|should_display_primary_actions_block:object %}
+    <div class="js-actions-sidebar sidebar__inner sidebar__inner--light-blue sidebar__inner--actions {% if mobile %}sidebar__inner--mobile{% endif %}">
+        <h5>Actions to take</h5>
+
+        {% if object.in_internal_review_phase or object.in_external_review_phase %}
+            {% include 'review/includes/review_button.html' with submission=object class="button--full-width button--bottom-space" draft_text="Complete draft review" %}
+        {% endif %}
+    </div>
+{% endif %}
diff --git a/hypha/apply/funds/templatetags/primaryactions_tags.py b/hypha/apply/funds/templatetags/primaryactions_tags.py
new file mode 100644
index 000000000..f866f6555
--- /dev/null
+++ b/hypha/apply/funds/templatetags/primaryactions_tags.py
@@ -0,0 +1,13 @@
+from django import template
+
+register = template.Library()
+
+
+@register.filter
+def should_display_primary_actions_block(user, submission):
+    review_primary_action_displayed = submission.can_review(user) and (submission.in_internal_review_phase or submission.in_external_review_phase)
+
+    if review_primary_action_displayed:
+        return True
+    else:
+        return False
diff --git a/hypha/apply/funds/tests/factories/models.py b/hypha/apply/funds/tests/factories/models.py
index b0c94dbc6..8658936ed 100644
--- a/hypha/apply/funds/tests/factories/models.py
+++ b/hypha/apply/funds/tests/factories/models.py
@@ -26,7 +26,7 @@ from hypha.apply.funds.models.forms import (
     RoundBaseForm,
     RoundBaseReviewForm,
 )
-from hypha.apply.funds.workflow import ConceptProposal, Request
+from hypha.apply.funds.workflow import ConceptProposal, Request, RequestExternal
 from hypha.apply.home.factories import ApplyHomePageFactory
 from hypha.apply.stream_forms.testing.factories import FormDataFactory
 from hypha.apply.users.groups import REVIEWER_GROUP_NAME, STAFF_GROUP_NAME
@@ -223,6 +223,7 @@ class ApplicationSubmissionFactory(factory.DjangoModelFactory):
         rejected = factory.Trait(
             status='rejected'
         )
+        with_external_review = False
 
     form_fields = blocks.CustomFormFieldsFactory
     form_data = factory.SubFactory(
@@ -230,7 +231,6 @@ class ApplicationSubmissionFactory(factory.DjangoModelFactory):
         form_fields=factory.SelfAttribute('..form_fields'),
     )
     page = factory.SelfAttribute('.round.fund')
-    workflow_name = factory.LazyAttribute(lambda o: workflow_for_stages(o.workflow_stages))
     round = factory.SubFactory(
         RoundFactory,
         workflow_name=factory.SelfAttribute('..workflow_name'),
@@ -241,6 +241,13 @@ class ApplicationSubmissionFactory(factory.DjangoModelFactory):
     live_revision = None
     draft_revision = None
 
+    @factory.lazy_attribute
+    def workflow_name(self):
+        if self.with_external_review:
+            return RequestExternal.admin_name
+        else:
+            return workflow_for_stages(self.workflow_stages)
+
     @factory.post_generation
     def reviewers(self, create, reviewers, **kwargs):
         if create and reviewers:
diff --git a/hypha/apply/funds/tests/test_views.py b/hypha/apply/funds/tests/test_views.py
index e4015cb94..cd30eb9d7 100644
--- a/hypha/apply/funds/tests/test_views.py
+++ b/hypha/apply/funds/tests/test_views.py
@@ -265,6 +265,69 @@ class TestStaffSubmissionView(BaseSubmissionViewTestCase):
         buttons = BeautifulSoup(response.content, 'html5lib').find(class_='sidebar').find_all('a', text='Screen application')
         self.assertEqual(len(buttons), 0)
 
+    def test_can_see_create_review_primary_action(self):
+        def assert_create_review_displayed(submission, button_text):
+            response = self.get_page(submission)
+            # Ignore whitespace (including line breaks) in button text
+            pattern = re.compile(rf'\s*{button_text}\s*')
+            buttons = BeautifulSoup(response.content, 'html5lib').find(class_='js-actions-sidebar').find_all('a', class_='button--primary', text=pattern)
+            self.assertEqual(len(buttons), 1)
+
+        submission = ApplicationSubmissionFactory(with_external_review=True, status='ext_internal_review')
+
+        # Phase: internal_review, no review
+        # "Add a review" should be displayed
+        assert_create_review_displayed(submission, 'Add a review')
+
+        # Phase: internal_review, draft review created
+        # "Complete draft review" should be displayed
+        review = ReviewFactory(submission=submission, author__reviewer=self.user, is_draft=True)
+        assert_create_review_displayed(submission, 'Complete draft review')
+        review.delete()
+
+        # Phase: external_review, no review
+        # "Add a review" should be displayed
+        submission.perform_transition('ext_post_review_discussion', self.user)
+        submission.perform_transition('ext_external_review', self.user)
+        assert_create_review_displayed(submission, 'Add a review')
+
+        # Phase: external_review, draft review created
+        # "Complete draft review" should be displayed
+        ReviewFactory(submission=submission, author__reviewer=self.user, is_draft=True)
+        assert_create_review_displayed(submission, 'Complete draft review')
+
+    def test_cant_see_create_review_primary_action(self):
+        def assert_create_review_not_displayed(submission, button_text):
+            response = self.get_page(submission)
+            # Ignore whitespace (including line breaks) in button text
+            pattern = re.compile(rf'\s*{button_text}\s*')
+            buttons = BeautifulSoup(response.content, 'html5lib').find(class_='js-actions-sidebar').find_all('a', class_='button--primary', text=pattern)
+            self.assertEqual(len(buttons), 0)
+
+        submission = ApplicationSubmissionFactory(with_external_review=True)
+
+        # Phase: received / in_discussion
+        # "Add a review" should not be displayed
+        # "Complete draft review" should not be displayed
+        assert_create_review_not_displayed(submission, 'Add a review')
+        assert_create_review_not_displayed(submission, 'Complete draft review')
+
+        # Phase: internal_review, review completed
+        # "Add a review" should not be displayed
+        # "Update draft review" should not be displayed
+        submission.perform_transition('ext_internal_review', self.user)
+        ReviewFactory(submission=submission, author__reviewer=self.user, is_draft=False)
+        assert_create_review_not_displayed(submission, 'Add a review')
+        assert_create_review_not_displayed(submission, 'Complete draft review')
+
+        # Phase: external_review, review completed
+        # "Add a review" should not be displayed
+        # "Update draft review" should not be displayed
+        submission.perform_transition('ext_post_review_discussion', self.user)
+        submission.perform_transition('ext_external_review', self.user)
+        assert_create_review_not_displayed(submission, 'Add a review')
+        assert_create_review_not_displayed(submission, 'Complete draft review')
+
 
 class TestReviewersUpdateView(BaseSubmissionViewTestCase):
     user_factory = StaffFactory
@@ -424,6 +487,59 @@ class TestReviewerSubmissionView(BaseSubmissionViewTestCase):
         assert_add_determination_not_displayed(submission, 'Add determination')
         assert_add_determination_not_displayed(submission, 'Complete draft determination')
 
+    def test_can_see_create_review_primary_action(self):
+        def assert_create_review_displayed(submission, button_text):
+            response = self.get_page(submission)
+            # Ignore whitespace (including line breaks) in button text
+            pattern = re.compile(rf'\s*{button_text}\s*')
+            buttons = BeautifulSoup(response.content, 'html5lib').find(class_='js-actions-sidebar').find_all('a', class_='button--primary', text=pattern)
+            self.assertEqual(len(buttons), 1)
+
+        submission = ApplicationSubmissionFactory(with_external_review=True, status='ext_external_review', user=self.applicant, reviewers=[self.user])
+
+        # Phase: external_review, no review
+        # "Add a review" should be displayed
+        submission.perform_transition('ext_post_review_discussion', self.user)
+        submission.perform_transition('ext_external_review', self.user)
+        assert_create_review_displayed(submission, 'Add a review')
+
+        # Phase: external_review, draft review created
+        # "Complete draft review" should be displayed
+        ReviewFactory(submission=submission, author__reviewer=self.user, is_draft=True)
+        assert_create_review_displayed(submission, 'Complete draft review')
+
+    def test_cant_see_create_review_primary_action(self):
+        def assert_create_review_not_displayed(submission, button_text):
+            response = self.get_page(submission)
+            # Ignore whitespace (including line breaks) in button text
+            pattern = re.compile(rf'\s*{button_text}\s*')
+            buttons = BeautifulSoup(response.content, 'html5lib').find_all('a', class_='button--primary', text=pattern)
+            self.assertEqual(len(buttons), 0)
+
+        submission = ApplicationSubmissionFactory(with_external_review=True, user=self.applicant, reviewers=[self.user])
+
+        # Phase: received / in_discussion
+        # "Add a review" should not be displayed
+        # "Complete draft review" should not be displayed
+        assert_create_review_not_displayed(submission, 'Add a review')
+        assert_create_review_not_displayed(submission, 'Complete draft review')
+
+        # Phase: internal_review, only viewable by staff users
+        # "Add a review" should not be displayed
+        # "Update draft review" should not be displayed
+        submission.perform_transition('ext_internal_review', self.user)
+        assert_create_review_not_displayed(submission, 'Add a review')
+        assert_create_review_not_displayed(submission, 'Complete draft review')
+
+        # Phase: external_review, review completed
+        # "Add a review" should not be displayed
+        # "Update draft review" should not be displayed
+        submission.perform_transition('ext_post_review_discussion', self.user)
+        submission.perform_transition('ext_external_review', self.user)
+        ReviewFactory(submission=submission, author__reviewer=self.user, is_draft=False)
+        assert_create_review_not_displayed(submission, 'Add a review')
+        assert_create_review_not_displayed(submission, 'Complete draft review')
+
 
 class TestApplicantSubmissionView(BaseSubmissionViewTestCase):
     user_factory = ApplicantFactory
@@ -541,6 +657,26 @@ class TestApplicantSubmissionView(BaseSubmissionViewTestCase):
         assert_add_determination_not_displayed(submission, 'Add determination')
         assert_add_determination_not_displayed(submission, 'Complete draft determination')
 
+    def test_cant_see_create_review_primary_action(self):
+        def assert_create_review_not_displayed(submission):
+            response = self.get_page(submission)
+            # Ignore whitespace (including line breaks) in button text
+            pattern = re.compile(rf'\s*Add a review\s*')
+            buttons = BeautifulSoup(response.content, 'html5lib').find_all('a', class_='button--primary', text=pattern)
+            self.assertEqual(len(buttons), 0)
+
+        submission = ApplicationSubmissionFactory(user=self.user)
+
+        # Phase: received / in_discussion
+        # "Add a review" should not be displayed
+        assert_create_review_not_displayed(submission)
+
+        # Phase: internal_review
+        # "Add a review" should not be displayed
+        staff_user = StaffFactory()
+        submission.perform_transition('internal_review', staff_user)
+        assert_create_review_not_displayed(submission)
+
 
 class TestRevisionsView(BaseSubmissionViewTestCase):
     user_factory = ApplicantFactory
diff --git a/hypha/apply/review/templates/review/includes/review_button.html b/hypha/apply/review/templates/review/includes/review_button.html
index 4cd8a06ff..2f28a5e16 100644
--- a/hypha/apply/review/templates/review/includes/review_button.html
+++ b/hypha/apply/review/templates/review/includes/review_button.html
@@ -3,7 +3,7 @@
     {% if request.user|has_draft:submission or request.user|can_review:submission %}
         <a target="_blank" href="{% url 'apply:submissions:reviews:form' submission_pk=submission.id %}" class="button button--primary {{ class }}">
             {% if request.user|has_draft:submission %}
-                Update draft
+                {{ draft_text|default:"Update draft" }}
             {% elif request.user|can_review:submission %}
                 Create review
             {% endif %}
-- 
GitLab