From 93d101bec1dfc966defad9eca3842e3e695d2a5c Mon Sep 17 00:00:00 2001
From: Todd Dembrey <todd.dembrey@torchbox.com>
Date: Wed, 22 Aug 2018 18:08:18 +0100
Subject: [PATCH] make sure staff cant mess with external reviewers by accident
---
opentech/apply/funds/forms.py | 14 ++-
opentech/apply/funds/tests/test_views.py | 121 ++++++++++++++++++++++-
2 files changed, 130 insertions(+), 5 deletions(-)
diff --git a/opentech/apply/funds/forms.py b/opentech/apply/funds/forms.py
index c5efce9d4..dd435d559 100644
--- a/opentech/apply/funds/forms.py
+++ b/opentech/apply/funds/forms.py
@@ -52,8 +52,11 @@ class UpdateReviewersForm(forms.ModelForm):
model = ApplicationSubmission
fields: list = []
+ def can_alter_reviewers(self, user):
+ return self.instance.stage.has_external_review and user == self.instance.lead
+
def __init__(self, *args, **kwargs):
- user = kwargs.pop('user')
+ self.user = kwargs.pop('user')
super().__init__(*args, **kwargs)
reviewers = self.instance.reviewers.all()
self.submitted_reviewers = User.objects.filter(id__in=self.instance.reviews.values('author'))
@@ -62,7 +65,7 @@ class UpdateReviewersForm(forms.ModelForm):
staff_field.queryset = staff_field.queryset.exclude(id__in=self.submitted_reviewers)
staff_field.initial = reviewers
- if self.instance.stage.has_external_review and user == self.instance.lead:
+ if self.can_alter_reviewers(self.user):
review_field = self.fields['reviewer_reviewers']
review_field.queryset = review_field.queryset.exclude(id__in=self.submitted_reviewers)
review_field.initial = reviewers
@@ -71,9 +74,14 @@ class UpdateReviewersForm(forms.ModelForm):
def save(self, *args, **kwargs):
instance = super().save(*args, **kwargs)
+ if self.can_alter_reviewers(self.user):
+ reviewers = self.cleaned_data.get('reviewer_reviewers')
+ else:
+ reviewers = instance.reviewers_not_reviewed
+
instance.reviewers.set(
self.cleaned_data['staff_reviewers'] |
- self.cleaned_data.get('reviewer_reviewers', User.objects.none()) |
+ reviewers |
self.submitted_reviewers
)
return instance
diff --git a/opentech/apply/funds/tests/test_views.py b/opentech/apply/funds/tests/test_views.py
index 16e33d86a..0e68c7d06 100644
--- a/opentech/apply/funds/tests/test_views.py
+++ b/opentech/apply/funds/tests/test_views.py
@@ -11,7 +11,12 @@ from opentech.apply.funds.tests.factories import (
SealedSubmissionFactory,
)
from opentech.apply.stream_forms.testing.factories import flatten_for_form
-from opentech.apply.users.tests.factories import UserFactory, StaffFactory, SuperUserFactory
+from opentech.apply.users.tests.factories import (
+ ReviewerFactory,
+ StaffFactory,
+ SuperUserFactory,
+ UserFactory,
+)
from opentech.apply.utils.testing.tests import BaseViewTestCase
from ..models import ApplicationRevision
@@ -122,7 +127,7 @@ class TestStaffSubmissionView(BaseSubmissionViewTestCase):
data = prepare_form_data(self.submission, title=new_title)
response = self.post_page(self.submission, {'submit': True, **data}, 'edit')
- url = self.url_from_pattern('funds:submissions:detail', kwargs={'pk': self.submission.id})
+ url = self.url(self.submission)
self.assertRedirects(response, url)
submission = self.refresh(self.submission)
@@ -132,6 +137,118 @@ class TestStaffSubmissionView(BaseSubmissionViewTestCase):
self.assertEqual(new_title, submission.title)
+class TestReviewersUpdateView(BaseSubmissionViewTestCase):
+ user_factory = StaffFactory
+
+ def post_form(self, submission, staff=list(), reviewers=list()):
+ return self.post_page(submission, {
+ 'form-submitted-reviewer_form': '',
+ 'staff_reviewers': [s.id for s in staff],
+ 'reviewer_reviewers': [r.id for r in reviewers]
+ })
+
+ def test_lead_can_add_staff_single(self):
+ submission = ApplicationSubmissionFactory(lead=self.user)
+ staff = StaffFactory.create_batch(4)
+
+ self.post_form(submission, staff)
+
+ self.assertCountEqual(submission.reviewers.all(), staff)
+
+ def test_lead_can_remove_staff_single(self):
+ staff = StaffFactory.create_batch(4)
+ submission = ApplicationSubmissionFactory(lead=self.user, reviewers=staff)
+ self.assertCountEqual(submission.reviewers.all(), staff)
+
+ self.post_form(submission, [])
+
+ self.assertCountEqual(submission.reviewers.all(), [])
+
+ def test_lead_can_remove_some_staff(self):
+ staff = StaffFactory.create_batch(4)
+ submission = ApplicationSubmissionFactory(lead=self.user, reviewers=staff)
+ self.assertCountEqual(submission.reviewers.all(), staff)
+
+ self.post_form(submission, staff[0:2])
+
+ self.assertCountEqual(submission.reviewers.all(), staff[0:2])
+
+ def test_lead_cant_add_reviewers_single(self):
+ submission = ApplicationSubmissionFactory(lead=self.user)
+ reviewers = ReviewerFactory.create_batch(4)
+
+ self.post_form(submission, reviewers=reviewers)
+
+ self.assertCountEqual(submission.reviewers.all(), [])
+
+ def test_lead_can_add_staff_and_reviewers_for_proposal(self):
+ submission = InvitedToProposalFactory(lead=self.user)
+
+ staff = StaffFactory.create_batch(4)
+ reviewers = ReviewerFactory.create_batch(4)
+
+ self.post_form(submission, staff, reviewers)
+
+ self.assertCountEqual(submission.reviewers.all(), staff + reviewers)
+
+ def test_lead_can_remove_staff_and_reviewers_for_proposal(self):
+ staff = StaffFactory.create_batch(4)
+ reviewers = ReviewerFactory.create_batch(4)
+
+ submission = InvitedToProposalFactory(lead=self.user, reviewers=staff + reviewers)
+ self.assertCountEqual(submission.reviewers.all(), staff + reviewers)
+
+ self.post_form(submission)
+
+ self.assertCountEqual(submission.reviewers.all(), [])
+
+ def test_lead_can_remove_some_staff_and_reviewers_for_proposal(self):
+ staff = StaffFactory.create_batch(4)
+ reviewers = ReviewerFactory.create_batch(4)
+
+ submission = InvitedToProposalFactory(lead=self.user, reviewers=staff + reviewers)
+ self.assertCountEqual(submission.reviewers.all(), staff + reviewers)
+
+ self.post_form(submission, staff[0:2], reviewers[0:2])
+
+ self.assertCountEqual(submission.reviewers.all(), staff[0:2] + reviewers[0:2])
+
+ def test_staff_can_add_staff_single(self):
+ submission = ApplicationSubmissionFactory()
+ staff = StaffFactory.create_batch(4)
+
+ self.post_form(submission, staff)
+
+ self.assertCountEqual(submission.reviewers.all(), staff)
+
+ def test_staff_can_remove_staff_single(self):
+ staff = StaffFactory.create_batch(4)
+ submission = ApplicationSubmissionFactory(reviewers=staff)
+ self.assertCountEqual(submission.reviewers.all(), staff)
+
+ self.post_form(submission, [])
+
+ self.assertCountEqual(submission.reviewers.all(), [])
+
+ def test_staff_cant_add_reviewers_proposal(self):
+ submission = ApplicationSubmissionFactory()
+ reviewers = ReviewerFactory.create_batch(4)
+
+ self.post_form(submission, reviewers=reviewers)
+
+ self.assertCountEqual(submission.reviewers.all(), [])
+
+ def test_staff_cant_remove_reviewers_proposal(self):
+ reviewers = ReviewerFactory.create_batch(4)
+
+ submission = ApplicationSubmissionFactory(reviewers=reviewers)
+ self.assertCountEqual(submission.reviewers.all(), reviewers)
+
+ self.post_form(submission, reviewers=[])
+
+ self.assertCountEqual(submission.reviewers.all(), reviewers)
+
+
class TestApplicantSubmissionView(BaseSubmissionViewTestCase):
user_factory = UserFactory
--
GitLab