From d6e4c42c4ce63a271b4a448b43a21f3c85bc491d Mon Sep 17 00:00:00 2001
From: Todd Dembrey <todd.dembrey@torchbox.com>
Date: Mon, 1 Apr 2019 09:19:11 +0100
Subject: [PATCH] GH-985: De-duplicate authors from opinions and reviews
---
opentech/apply/funds/models/submissions.py | 36 ++++++++++++--------
opentech/apply/funds/tests/test_models.py | 39 ++++++++++++++++++++--
2 files changed, 59 insertions(+), 16 deletions(-)
diff --git a/opentech/apply/funds/models/submissions.py b/opentech/apply/funds/models/submissions.py
index 26b709591..6e233fd4f 100644
--- a/opentech/apply/funds/models/submissions.py
+++ b/opentech/apply/funds/models/submissions.py
@@ -53,6 +53,11 @@ from ..workflow import (
)
+class SubqueryCount(Subquery):
+ template = "(SELECT count(*) FROM (%(subquery)s) _count)"
+ output_field = models.IntegerField()
+
+
class JSONOrderable(models.QuerySet):
json_field = ''
@@ -126,6 +131,7 @@ class ApplicationSubmissionQueryset(JSONOrderable):
return self.exclude(next__isnull=False)
def for_table(self, user):
+ User = get_user_model()
activities = self.model.activities.field.model
latest_activity = activities.objects.filter(submission=OuterRef('id')).select_related('user')
comments = activities.comments.filter(submission=OuterRef('id')).visible_to(user)
@@ -146,24 +152,26 @@ class ApplicationSubmissionQueryset(JSONOrderable):
),
0,
),
- review_count=Coalesce(
- Subquery(
- reviews.values('submission').annotate(count=Count('pk')).values('count'),
- output_field=IntegerField(),
- ),
- 0,
- ),
opinion_disagree=Subquery(
- opinions.filter(opinion=DISAGREE).values('review').annotate(count=Count('pk')).values('count'),
+ opinions.filter(opinion=DISAGREE).values(
+ 'review__submission'
+ ).annotate(count=Count('*')).values('count')[:1],
output_field=IntegerField(),
),
- review_staff_count=Subquery(
- reviews.by_staff().values('submission').annotate(count=Count('pk')).values('count'),
- output_field=IntegerField(),
+ review_staff_count=SubqueryCount(
+ User.objects.staff().filter(
+ Q(submissions_reviewer=OuterRef('id')) | Q(reviewopinion__review__submission=OuterRef('id'))
+ ).distinct().values('pk')
),
- review_submitted_count=Subquery(
- reviews.submitted().values('submission').annotate(count=Count('pk')).values('count'),
- output_field=IntegerField(),
+ review_count=SubqueryCount(
+ User.objects.filter(
+ Q(submissions_reviewer=OuterRef('id')) | Q(reviewopinion__review__submission=OuterRef('id'))
+ ).distinct().values('pk')
+ ),
+ review_submitted_count=SubqueryCount(
+ User.objects.filter(
+ Q(review__submission=OuterRef('id')) | Q(reviewopinion__review__submission=OuterRef('id'))
+ ).distinct().values('pk')
),
review_recommendation=Case(
When(opinion_disagree__gt=0, then=MAYBE),
diff --git a/opentech/apply/funds/tests/test_models.py b/opentech/apply/funds/tests/test_models.py
index 37e1ce9f9..2022f4b73 100644
--- a/opentech/apply/funds/tests/test_models.py
+++ b/opentech/apply/funds/tests/test_models.py
@@ -19,6 +19,7 @@ from opentech.apply.users.tests.factories import StaffFactory
from .factories import (
ApplicationSubmissionFactory,
+ AssignedReviewersFactory,
CustomFormFieldsFactory,
FundTypeFactory,
LabFactory,
@@ -502,6 +503,18 @@ class TestRequestForPartners(TestCase):
class TestForTableQueryset(TestCase):
+ def test_assigned_but_not_reviewed(self):
+ staff = StaffFactory()
+ submission = ApplicationSubmissionFactory()
+ AssignedReviewersFactory(submission=submission, reviewer=staff)
+
+ qs = ApplicationSubmission.objects.for_table(user=staff)
+ submission = qs[0]
+ self.assertEqual(submission.opinion_disagree, None)
+ self.assertEqual(submission.review_count, 1)
+ self.assertEqual(submission.review_submitted_count, 0)
+ self.assertEqual(submission.review_recommendation, None)
+
def test_review_outcome(self):
staff = StaffFactory()
submission = ApplicationSubmissionFactory()
@@ -510,6 +523,7 @@ class TestForTableQueryset(TestCase):
submission = qs[0]
self.assertEqual(submission.opinion_disagree, None)
self.assertEqual(submission.review_count, 1)
+ self.assertEqual(submission.review_submitted_count, 1)
self.assertEqual(submission.review_recommendation, NO)
def test_disagree_review_is_maybe(self):
@@ -520,7 +534,26 @@ class TestForTableQueryset(TestCase):
qs = ApplicationSubmission.objects.for_table(user=staff)
submission = qs[0]
self.assertEqual(submission.opinion_disagree, 1)
- self.assertEqual(submission.review_count, 1)
+ self.assertEqual(submission.review_count, 2)
+ self.assertEqual(submission.review_submitted_count, 2)
+ self.assertEqual(submission.review_recommendation, MAYBE)
+
+ def test_dont_double_count_review_and_opinion(self):
+ staff = StaffFactory()
+ submission = ApplicationSubmissionFactory()
+
+ review = ReviewFactory(submission=submission, author=staff)
+ opinion = ReviewOpinionFactory(opinion_disagree=True, review=review)
+
+ # Another pair of review/opinion
+ review_two = ReviewFactory(author=opinion.author, submission=submission)
+ ReviewOpinionFactory(opinion_disagree=True, author=staff, review=review_two)
+
+ qs = ApplicationSubmission.objects.for_table(user=staff)
+ submission = qs[0]
+ self.assertEqual(submission.opinion_disagree, 2)
+ self.assertEqual(submission.review_count, 2)
+ self.assertEqual(submission.review_submitted_count, 2)
self.assertEqual(submission.review_recommendation, MAYBE)
def test_submissions_dont_conflict(self):
@@ -535,10 +568,12 @@ class TestForTableQueryset(TestCase):
qs = ApplicationSubmission.objects.for_table(user=staff)
submission = qs[0]
self.assertEqual(submission.opinion_disagree, 1)
- self.assertEqual(submission.review_count, 1)
+ self.assertEqual(submission.review_count, 2)
+ self.assertEqual(submission.review_submitted_count, 2)
self.assertEqual(submission.review_recommendation, MAYBE)
submission = qs[1]
self.assertEqual(submission.opinion_disagree, None)
self.assertEqual(submission.review_count, 1)
+ self.assertEqual(submission.review_submitted_count, 1)
self.assertEqual(submission.review_recommendation, NO)
--
GitLab