From cfc97ee17268c10aa9ccec44557b82a231d6d608 Mon Sep 17 00:00:00 2001
From: Todd Dembrey <todd.dembrey@torchbox.com>
Date: Thu, 14 Feb 2019 08:40:08 +0000
Subject: [PATCH] GH-957: Tidy up tests to seperate out the post-save tests
---
opentech/apply/funds/forms.py | 7 +--
opentech/apply/funds/tests/test_views.py | 73 +++++++++++++++++++-----
opentech/apply/funds/views.py | 5 --
3 files changed, 62 insertions(+), 23 deletions(-)
diff --git a/opentech/apply/funds/forms.py b/opentech/apply/funds/forms.py
index 0a85d2865..9e302e9e3 100644
--- a/opentech/apply/funds/forms.py
+++ b/opentech/apply/funds/forms.py
@@ -65,12 +65,11 @@ class UpdateReviewersForm(forms.ModelForm):
def __init__(self, *args, **kwargs):
self.user = kwargs.pop('user')
- self.request = kwargs.pop('request', None)
super().__init__(*args, **kwargs)
- self.submitted_reviewers = User.objects.filter(id__in=self.instance.reviews.values('author'))
- self.submitted_reviewers = self.submitted_reviewers.exclude(
- id__in=self.instance.assigned.with_roles().values('reviewer'))
+ self.submitted_reviewers = User.objects.filter(
+ id__in=self.instance.reviews.values('author'),
+ )
if self.can_alter_external_reviewers(self.instance, self.user):
reviewers = self.instance.reviewers.all()
diff --git a/opentech/apply/funds/tests/test_views.py b/opentech/apply/funds/tests/test_views.py
index 761f73d46..ebbd9c029 100644
--- a/opentech/apply/funds/tests/test_views.py
+++ b/opentech/apply/funds/tests/test_views.py
@@ -1,6 +1,8 @@
from datetime import datetime, timedelta
import json
+from django.test import TestCase
+
from opentech.apply.activity.models import Activity, INTERNAL
from opentech.apply.determinations.tests.factories import DeterminationFactory
from opentech.apply.funds.tests.factories import (
@@ -282,7 +284,7 @@ class TestReviewersUpdateView(BaseSubmissionViewTestCase):
self.assertCountEqual(submission.reviewers.all(), self.reviewers)
def test_lead_can_change_role_reviewer_and_review_remains(self):
- submission = ApplicationSubmissionFactory(lead=self.user)
+ submission = ApplicationSubmissionFactory()
AssignedWithRoleReviewersFactory(role=self.roles[0], submission=submission, reviewer=self.staff[0])
# Add a review from that staff reviewer
@@ -291,23 +293,66 @@ class TestReviewersUpdateView(BaseSubmissionViewTestCase):
# Assign a different reviewer to the same role
self.post_form(submission, reviewer_roles=[self.staff[1]])
- # Make sure that the ex-role-reviewer (self.staff[0]) still has a assignedreviewer record
- self.assertEqual(submission.reviewers.all().count(), 2)
- self.assertIn(self.staff[0], submission.reviewers.all())
+ # Make sure that the ex-role-reviewer is still assigned record
+ self.assertCountEqual(submission.reviewers.all(), self.staff[0:2])
- def test_reviewer_exists_after_submitting_review(self):
- submission = ApplicationSubmissionFactory(lead=self.user, status='external_review', workflow_stages=2)
- AssignedWithRoleReviewersFactory(role=self.roles[0], submission=submission, reviewer=self.staff[0])
- AssignedWithRoleReviewersFactory(role=self.roles[1], submission=submission, reviewer=self.staff[1])
- AssignedReviewersFactory(submission=submission, reviewer=self.reviewers[0])
- # Now, let's check that we have 3 reviewers to begin with
- self.assertEqual(submission.reviewers.all().count(), 3)
+ def test_can_remove_external_reviewer_and_review_remains(self):
+ submission = ApplicationSubmissionFactory(lead=self.user)
+ reviewer = self.reviewers[0]
+ AssignedReviewersFactory(submission=submission, reviewer=reviewer)
+ ReviewFactory(submission=submission, author=reviewer)
+
+ # Assign a different reviewer to the same role
+ self.post_form(submission, reviewers=[])
+
+ self.assertCountEqual(submission.reviewers.all(), [reviewer])
+
+
+class TestPostSaveOnReview(TestCase):
+ def test_external_reviewer_exists_after_review(self):
+ reviewer = ReviewerFactory()
+ submission = ApplicationSubmissionFactory()
# Add a review from a new reviewer who isn't assigned
- ReviewFactory(submission=submission, author=self.reviewers[1])
+ ReviewFactory(submission=submission, author=reviewer)
+
+ self.assertCountEqual(submission.reviewers.all(), [reviewer])
+
+ def test_assigned_external_reviewer_exists_after_review(self):
+ reviewer = ReviewerFactory()
+ submission = ApplicationSubmissionFactory()
+
+ # Add a review from a new reviewer who is assigned
+ AssignedReviewersFactory(submission=submission, reviewer=reviewer)
+ ReviewFactory(submission=submission, author=reviewer)
+
+ self.assertCountEqual(submission.reviewers.all(), [reviewer])
+
+ def test_staff_reviewer_exists_after_submitting_review(self):
+ staff = StaffFactory()
+ submission = ApplicationSubmissionFactory()
+
+ ReviewFactory(submission=submission, author=staff)
+
+ self.assertCountEqual(submission.reviewers.all(), [staff])
+
+ def test_assigned_staff_reviewer_exists_after_review(self):
+ staff = StaffFactory()
+ submission = ApplicationSubmissionFactory()
+
+ AssignedReviewersFactory(submission=submission, reviewer=staff)
+ ReviewFactory(submission=submission, author=staff)
+
+ self.assertCountEqual(submission.reviewers.all(), [staff])
+
+ def test_role_reviewer_exists_after_review(self):
+ staff = StaffFactory()
+ submission = ApplicationSubmissionFactory()
+
+ AssignedWithRoleReviewersFactory(submission=submission, reviewer=staff)
+ ReviewFactory(submission=submission, author=staff)
- # Now there should be 4 reviewers assigned, because an outsider reviewed
- self.assertEqual(submission.reviewers.all().count(), 4)
+ self.assertCountEqual(submission.reviewers.all(), [staff])
class TestApplicantSubmissionView(BaseSubmissionViewTestCase):
diff --git a/opentech/apply/funds/views.py b/opentech/apply/funds/views.py
index ffb43a23b..4e5167f33 100644
--- a/opentech/apply/funds/views.py
+++ b/opentech/apply/funds/views.py
@@ -296,11 +296,6 @@ class UpdateReviewersView(DelegatedViewMixin, UpdateView):
form_class = UpdateReviewersForm
context_name = 'reviewer_form'
- def get_form_kwargs(self):
- kwargs = super().get_form_kwargs()
- kwargs['request'] = self.request
- return kwargs
-
def form_invalid(self, form):
messages.error(
self.request,
--
GitLab