diff --git a/opentech/apply/funds/forms.py b/opentech/apply/funds/forms.py index 936e48e18546332c667731f02732dc0a42646582..84f615c1f165b5835159bc7768a4717d56e4edad 100644 --- a/opentech/apply/funds/forms.py +++ b/opentech/apply/funds/forms.py @@ -92,7 +92,7 @@ class UpdateReviewersForm(forms.ModelForm): } for role in ReviewerRole.objects.all().order_by('order'): - field_name = 'reviewer_' + str(role) + field_name = 'reviewer_role_' + str(role) self.roles[field_name] = role self.fields[field_name] = forms.ModelChoiceField( @@ -123,7 +123,7 @@ class UpdateReviewersForm(forms.ModelForm): ] # If any of the users match and are set to multiple roles, throw an error - if len(role_reviewers) != len(set(role_reviewers)): + if len(role_reviewers) != len(set(role_reviewers)) and any(role_reviewers): self.add_error(None, _('Users cannot be assigned to multiple roles.')) return cleaned_data @@ -140,7 +140,7 @@ class UpdateReviewersForm(forms.ModelForm): assignedreviewers__submission=instance, assignedreviewers__role__isnull=True ).exclude( - id__in=[user.id for user in assigned_roles.values()] + id__in=[user.id for user in assigned_roles.values() if user] ) if self.can_alter_external_reviewers(self.instance, self.user): diff --git a/opentech/apply/funds/tests/factories/models.py b/opentech/apply/funds/tests/factories/models.py index 817e88e32ce3a4f560d22ccdff43a0611b7e5527..713b3dc1b046fa2e1f0d93c80f7d5e9244aee248 100644 --- a/opentech/apply/funds/tests/factories/models.py +++ b/opentech/apply/funds/tests/factories/models.py @@ -51,6 +51,7 @@ __all__ = [ 'ScreeningStatusFactory', 'SealedRoundFactory', 'SealedSubmissionFactory', + 'ReviewerRoleFactory', 'TodayRoundFactory', 'workflow_for_stages', ] diff --git a/opentech/apply/funds/tests/test_views.py b/opentech/apply/funds/tests/test_views.py index 24f2b7172c58544f06a8f8828c5bc37730793298..d71b69897439f30a873485cd0d719c7fe02e6451 100644 --- a/opentech/apply/funds/tests/test_views.py +++ b/opentech/apply/funds/tests/test_views.py @@ -6,8 +6,10 @@ from opentech.apply.determinations.tests.factories import DeterminationFactory from opentech.apply.funds.tests.factories import ( ApplicationSubmissionFactory, ApplicationRevisionFactory, + AssignedWithRoleReviewersFactory, InvitedToProposalFactory, LabSubmissionFactory, + ReviewerRoleFactory, ScreeningStatusFactory, SealedRoundFactory, SealedSubmissionFactory, @@ -207,13 +209,38 @@ class TestReviewersUpdateView(BaseSubmissionViewTestCase): @classmethod def setUpTestData(cls): super().setUpTestData() + cls.staff = StaffFactory.create_batch(4) cls.reviewers = ReviewerFactory.create_batch(4) + cls.roles = ReviewerRoleFactory.create_batch(2) - def post_form(self, submission, reviewers=list()): - return self.post_page(submission, { + def post_form(self, submission, reviewer_roles=list(), reviewers=list()): + data = { 'form-submitted-reviewer_form': '', 'reviewer_reviewers': [r.id for r in reviewers] - }) + } + data.update( + **{ + f'reviewer_role_{str(role)}': reviewer.id + for role, reviewer in zip(self.roles, reviewer_roles) + } + ) + return self.post_page(submission, data) + + def test_lead_can_add_staff_single(self): + submission = ApplicationSubmissionFactory(lead=self.user) + + self.post_form(submission, [self.staff[0]]) + + self.assertCountEqual(submission.reviewers.all(), [self.staff[0]]) + + def test_lead_can_change_staff_single(self): + submission = ApplicationSubmissionFactory(lead=self.user) + AssignedWithRoleReviewersFactory(role=self.roles[0], submission=submission, reviewer=self.staff[0]) + self.assertCountEqual(submission.reviewers.all(), [self.staff[0]]) + + self.post_form(submission, [self.staff[1]]) + + self.assertCountEqual(submission.reviewers.all(), [self.staff[1]]) def test_lead_cant_add_reviewers_single(self): submission = ApplicationSubmissionFactory(lead=self.user) @@ -225,7 +252,7 @@ class TestReviewersUpdateView(BaseSubmissionViewTestCase): def test_lead_can_add_reviewers_for_proposal(self): submission = InvitedToProposalFactory(lead=self.user) - self.post_form(submission, self.reviewers) + self.post_form(submission, reviewers=self.reviewers) self.assertCountEqual(submission.reviewers.all(), self.reviewers) @@ -241,7 +268,7 @@ class TestReviewersUpdateView(BaseSubmissionViewTestCase): submission = InvitedToProposalFactory(lead=self.user, reviewers=self.reviewers) self.assertCountEqual(submission.reviewers.all(), self.reviewers) - self.post_form(submission, self.reviewers[0:2]) + self.post_form(submission, reviewers=self.reviewers[0:2]) self.assertCountEqual(submission.reviewers.all(), self.reviewers[0:2])