From b803602e34688138c783692eb7c8c405583bef6a Mon Sep 17 00:00:00 2001
From: Todd Dembrey <todd.dembrey@torchbox.com>
Date: Tue, 28 Aug 2018 16:36:59 +0100
Subject: [PATCH] Make the display of NA scores more user friendly

---
 .../funds/includes/review_table.html          |  2 +-
 opentech/apply/review/models.py               |  6 +-
 opentech/apply/review/tests/test_views.py     | 78 ++++++++++++-------
 3 files changed, 58 insertions(+), 28 deletions(-)

diff --git a/opentech/apply/funds/templates/funds/includes/review_table.html b/opentech/apply/funds/templates/funds/includes/review_table.html
index 795f47c1a..cacd82ba3 100644
--- a/opentech/apply/funds/templates/funds/includes/review_table.html
+++ b/opentech/apply/funds/templates/funds/includes/review_table.html
@@ -5,7 +5,7 @@
             <tr class="tr tr--subchild light-grey-bg">
                 <th colspan="2"></th>
                 <th>{{ object.reviews.submitted.recommendation|traffic_light }}</th>
-                <th>{{ object.reviews.submitted.score|floatformat|default_if_none:'-' }}</th>
+                <th>{{ object.reviews.submitted.get_score_display|default_if_none:'-' }}</th>
             </tr>
             {% include 'funds/includes/review_table_row.html' with reviews=staff_reviews %}
         {% endif %}
diff --git a/opentech/apply/review/models.py b/opentech/apply/review/models.py
index 10bace3f1..c47a65c0d 100644
--- a/opentech/apply/review/models.py
+++ b/opentech/apply/review/models.py
@@ -19,6 +19,7 @@ from .blocks import (
     RecommendationCommentsBlock,
     ScoreFieldBlock,
 )
+from .options import NA
 
 
 class ReviewFormFieldsMixin(models.Model):
@@ -89,7 +90,7 @@ class ReviewQuerySet(models.QuerySet):
         return self.by_reviewers().recommendation()
 
     def score(self):
-        return self.aggregate(models.Avg('score'))['score__avg']
+        return self.exclude(score=NA).aggregate(models.Avg('score'))['score__avg']
 
     def recommendation(self):
         recommendations = self.values_list('recommendation', flat=True)
@@ -131,6 +132,9 @@ class Review(ReviewFormFieldsMixin, BaseStreamForm, AccessFormData, models.Model
     def get_comments_display(self, include_question=True):
         return self.render_answer(self.comment_field.id, include_question=include_question)
 
+    def get_score_display(self):
+        return '{:.1f}'.format(self.score) if self.score != NA else 'NA'
+
     def get_absolute_url(self):
         return reverse('apply:reviews:review', args=(self.id,))
 
diff --git a/opentech/apply/review/tests/test_views.py b/opentech/apply/review/tests/test_views.py
index 9f551d517..682500b50 100644
--- a/opentech/apply/review/tests/test_views.py
+++ b/opentech/apply/review/tests/test_views.py
@@ -5,6 +5,7 @@ from opentech.apply.users.tests.factories import StaffFactory, UserFactory
 from opentech.apply.utils.testing.tests import BaseViewTestCase
 
 from .factories import ReviewFactory, ReviewFormFieldsFactory, ReviewFormFactory
+from ..models import Review
 from ..options import NA
 
 
@@ -110,47 +111,72 @@ class StaffReviewFormTestCase(BaseViewTestCase):
         self.assertTrue(review.is_draft)
         self.assertIsNone(review.revision)
 
-    def test_score_calculated(self):
-        form = self.submission.round.review_forms.first()
-        score = 5
 
-        data = ReviewFormFieldsFactory.form_response(form.fields, {
-            field.id: {'score': score}
-            for field in form.form.score_fields
-        })
+class TestReviewScore(BaseViewTestCase):
+    user_factory = StaffFactory
+    url_name = 'funds:submissions:reviews:{}'
+    base_view_name = 'review'
 
-        self.post_page(self.submission, data, 'form')
-        review = self.submission.reviews.first()
-        self.assertEqual(review.score, score)
+    @classmethod
+    def setUpTestData(cls):
+        super().setUpTestData()
+        cls.submission = ApplicationSubmissionFactory(status='internal_review')
 
-    def test_average_score_calculated(self):
-        form = ReviewFormFactory(form_fields__multiple__score=2)
+    def get_kwargs(self, instance):
+        return {'submission_pk': instance.id}
+
+    def submit_review_scores(self, *scores):
+        if scores:
+            form = ReviewFormFactory(form_fields__multiple__score=len(scores))
+        else:
+            form = ReviewFormFactory(form_fields__exclude__score=True)
         review_form = self.submission.round.review_forms.first()
         review_form.form = form
         review_form.save()
 
-        score_1, score_2 = 1, 5
-
         data = ReviewFormFieldsFactory.form_response(form.form_fields, {
             field.id: {'score': score}
-            for field, score in zip(form.score_fields, [score_1, score_2])
+            for field, score in zip(form.score_fields, scores)
         })
 
-        self.post_page(self.submission, data, 'form')
-        review = self.submission.reviews.first()
-        self.assertEqual(review.score, (score_1 + score_2) / 2)
+        # Make a new person for every review
+        self.client.force_login(self.user_factory())
+        response = self.post_page(self.submission, data, 'form')
+        self.assertIn(
+            'funds/applicationsubmission_admin_detail.html',
+            response.template_name,
+            msg='Failed to post the form correctly'
+        )
+        self.client.force_login(self.user)
+        return self.submission.reviews.first()
 
-    def test_no_score_is_NA(self):
-        form = ReviewFormFactory(form_fields__exclude__score=True)
-        review_form = self.submission.round.review_forms.first()
-        review_form.form = form
-        review_form.save()
+    def test_score_calculated(self):
+        review = self.submit_review_scores(5)
+        self.assertEqual(review.score, 5)
 
-        data = ReviewFormFieldsFactory.form_response(form.form_fields)
-        self.post_page(self.submission, data, 'form')
-        review = self.submission.reviews.first()
+    def test_average_score_calculated(self):
+        review = self.submit_review_scores(1, 5)
+        self.assertEqual(review.score, (1 + 5) / 2)
+
+    def test_no_score_is_NA(self):
+        review = self.submit_review_scores()
         self.assertEqual(review.score, NA)
 
+    def test_na_not_included_in_review_average(self):
+        review = self.submit_review_scores(NA, 5)
+        self.assertEqual(review.score, 5)
+
+    def test_na_not_included_reviews_average(self):
+        self.submit_review_scores(NA)
+        self.assertIsNone(Review.objects.score())
+
+    def test_na_not_included_multiple_reviews_average(self):
+        self.submit_review_scores(NA)
+        self.submit_review_scores(5)
+
+        self.assertEqual(Review.objects.count(), 2)
+        self.assertEqual(Review.objects.score(), 5)
+
 
 class UserReviewFormTestCase(BaseViewTestCase):
     user_factory = UserFactory
-- 
GitLab