diff --git a/opentech/apply/activity/messaging.py b/opentech/apply/activity/messaging.py
index 137c2fb02fde86e36d287606a68d0ef0f249ce63..a380859104349d9153131bd0ef0d107d4a4ec1ed 100644
--- a/opentech/apply/activity/messaging.py
+++ b/opentech/apply/activity/messaging.py
@@ -242,7 +242,13 @@ class ActivityAdapter(AdapterBase):
return ' '.join(message)
def batch_reviewers_updated(self, added, **kwargs):
- return 'Batch ' + self.reviewers_updated(added, **kwargs)
+ base = ['Batch Reviewers Updated.']
+ base.extend([
+ f'{str(user)} as {role.name}.'
+ for role, user in added
+ if user
+ ])
+ return ' '.join(base)
def batch_determination(self, submissions, determinations, **kwargs):
submission = submissions[0]
@@ -395,7 +401,11 @@ class SlackAdapter(AdapterBase):
def handle_batch_reviewers(self, submissions, links, user, added, **kwargs):
submissions_text = self.slack_links(links, submissions)
- reviewers_text = ', '.join([str(user) for user in added])
+ reviewers_text = ' '.join([
+ f'{str(user)} as {role.name},'
+ for role, user in added
+ if user
+ ])
return (
'{user} has batch added {reviewers_text} as reviewers on: {submissions_text}'.format(
user=user,
@@ -624,9 +634,15 @@ class DjangoMessagesAdapter(AdapterBase):
}
def batch_reviewers_updated(self, added, submissions, **kwargs):
+ reviewers_text = ' '.join([
+ f'{str(user)} as {role.name},'
+ for role, user in added
+ if user
+ ])
+
return (
'Batch reviewers added: ' +
- ', '.join([str(user) for user in added]) +
+ reviewers_text +
' to ' +
', '.join(['"{}"'.format(submission.title) for submission in submissions])
)
diff --git a/opentech/apply/funds/forms.py b/opentech/apply/funds/forms.py
index 99f11d75b1c651e2342999a241347a381785a908..426a0d0a7246b07bd409b9d19b95a07cfc3d4f22 100644
--- a/opentech/apply/funds/forms.py
+++ b/opentech/apply/funds/forms.py
@@ -4,9 +4,9 @@ from django.utils.translation import ugettext_lazy as _
from django_select2.forms import Select2Widget
from opentech.apply.users.models import User
-from opentech.apply.utils.image import generate_image_tag
-from .models import ApplicationSubmission, AssignedReviewers, ReviewerRole
+from .models import AssignedReviewers, ApplicationSubmission, ReviewerRole
+from .utils import render_icon
from .widgets import Select2MultiCheckboxesWidget
from .workflow import get_action_mapping
@@ -100,23 +100,14 @@ class UpdateReviewersForm(forms.ModelForm):
)
}
- staff_reviewers = User.objects.staff().only('full_name', 'pk')
-
self.role_fields = {}
- for role in ReviewerRole.objects.all().order_by('order'):
- field_name = 'role_reviewer_' + slugify(str(role))
- self.role_fields[field_name] = role
-
- self.fields[field_name] = forms.ModelChoiceField(
- queryset=staff_reviewers,
- widget=Select2Widget(attrs={
- 'data-placeholder': 'Select a reviewer',
- }),
- required=False,
- label=mark_safe(self.render_icon(role.icon) + f'{role.name} Reviewer'),
- )
- # Pre-populate form field
- self.fields[field_name].initial = assigned_roles.get(role)
+ field_data = make_role_reviewer_fields()
+
+ for data in field_data:
+ field_name = data['field_name']
+ self.fields[field_name] = data['field']
+ self.role_fields[field_name] = data['role']
+ self.fields[field_name].initial = assigned_roles.get(data['role'])
self.submitted_reviewers = User.objects.filter(
id__in=self.instance.reviews.values('author'),
@@ -136,12 +127,6 @@ class UpdateReviewersForm(forms.ModelForm):
else:
self.fields.pop('reviewer_reviewers')
- def render_icon(self, image):
- if not image:
- return ''
- filter_spec = 'fill-20x20'
- return generate_image_tag(image, filter_spec)
-
def prepare_field(self, field_name, initial, excluded):
field = self.fields[field_name]
field.queryset = field.queryset.exclude(id__in=excluded)
@@ -155,7 +140,7 @@ class UpdateReviewersForm(forms.ModelForm):
role_reviewers = [
user
for field, user in self.cleaned_data.items()
- if field != 'reviewer_reviewers'
+ if field in self.role_fields
]
# If any of the users match and are set to multiple roles, throw an error
@@ -222,16 +207,105 @@ class UpdateReviewersForm(forms.ModelForm):
class BatchUpdateReviewersForm(forms.Form):
- staff_reviewers = forms.ModelMultipleChoiceField(
- queryset=User.objects.staff(),
- widget=Select2MultiCheckboxesWidget(attrs={'data-placeholder': 'Staff'}),
- )
submissions = forms.CharField(widget=forms.HiddenInput(attrs={'class': 'js-submissions-id'}))
def __init__(self, *args, user=None, round=None, **kwargs):
super().__init__(*args, **kwargs)
+ self.role_fields = {}
+ field_data = make_role_reviewer_fields()
+
+ for data in field_data:
+ field_name = data['field_name']
+ self.fields[field_name] = data['field']
+ self.role_fields[field_name] = data['role']
+
def clean_submissions(self):
value = self.cleaned_data['submissions']
submission_ids = [int(submission) for submission in value.split(',')]
return ApplicationSubmission.objects.filter(id__in=submission_ids)
+
+ def clean(self):
+ cleaned_data = super().clean()
+ role_reviewers = [
+ user
+ for field, user in self.cleaned_data.items()
+ if field in self.role_fields
+ ]
+
+ # If any of the users match and are set to multiple roles, throw an error
+ 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
+
+ def save(self):
+ submissions = self.cleaned_data['submissions']
+ assigned_roles = {
+ role: self.cleaned_data[field]
+ for field, role in self.role_fields.items()
+ }
+ for role, reviewer in assigned_roles.items():
+ if reviewer:
+ existing_assignments = AssignedReviewers.objects.filter(
+ submission__in=submissions,
+ role=role,
+ )
+
+ about_to_be_orphaned = [
+ [assigned.reviewer, assigned.submission]
+ for assigned in existing_assignments
+ if submissions.get(
+ pk=assigned.submission.pk
+ ).reviews.filter(author=assigned.reviewer).exists()
+ ]
+
+ # Being reassigned
+ AssignedReviewers.objects.filter(
+ submission__in=submissions,
+ role__isnull=False,
+ reviewer=reviewer,
+ ).delete()
+
+ existing_assignments.update(reviewer=reviewer)
+
+ AssignedReviewers.objects.bulk_create(
+ AssignedReviewers(
+ role=role,
+ reviewer=reviewer,
+ submission=submission,
+ ) for submission in submissions.exclude(pk__in=existing_assignments.values('submission'))
+ )
+
+ AssignedReviewers.objects.bulk_create(
+ AssignedReviewers(
+ role=None,
+ reviewer=reviewer,
+ submission=submission,
+ ) for reviewer, submission in about_to_be_orphaned
+ )
+
+ return None
+
+
+def make_role_reviewer_fields():
+ role_fields = []
+ staff_reviewers = User.objects.staff().only('full_name', 'pk')
+
+ for role in ReviewerRole.objects.all().order_by('order'):
+ field_name = 'role_reviewer_' + slugify(str(role))
+ field = forms.ModelChoiceField(
+ queryset=staff_reviewers,
+ widget=Select2Widget(attrs={
+ 'data-placeholder': 'Select a reviewer',
+ }),
+ required=False,
+ label=mark_safe(render_icon(role.icon) + f'{role.name} Reviewer'),
+ )
+ role_fields.append({
+ 'role': role,
+ 'field': field,
+ 'field_name': field_name,
+ })
+
+ return role_fields
diff --git a/opentech/apply/funds/tests/views/test_batch_reviewers.py b/opentech/apply/funds/tests/views/test_batch_reviewers.py
new file mode 100644
index 0000000000000000000000000000000000000000..5df52e496b2a7893e445e0765c2eb2f996590269
--- /dev/null
+++ b/opentech/apply/funds/tests/views/test_batch_reviewers.py
@@ -0,0 +1,88 @@
+from django.utils.text import slugify
+
+from opentech.apply.funds.tests.factories import (
+ ApplicationSubmissionFactory,
+ AssignedWithRoleReviewersFactory,
+ ReviewerRoleFactory,
+)
+from opentech.apply.users.tests.factories import (
+ ReviewerFactory,
+ StaffFactory,
+)
+from opentech.apply.review.tests.factories import ReviewFactory
+from opentech.apply.utils.testing.tests import BaseViewTestCase
+
+
+class BaseBatchReviewerTestCase(BaseViewTestCase):
+ url_name = 'funds:submissions:{}'
+ base_view_name = 'list'
+
+ @classmethod
+ def setUpTestData(cls):
+ super().setUpTestData()
+ cls.submissions = ApplicationSubmissionFactory.create_batch(4)
+ cls.staff = StaffFactory.create_batch(4)
+ cls.reviewers = ReviewerFactory.create_batch(4)
+ cls.roles = ReviewerRoleFactory.create_batch(2)
+
+ def data(self, reviewer_roles, submissions):
+ data = {
+ 'form-submitted-batch_reviewer_form': 'Update',
+ 'submissions': ','.join([str(submission.id) for submission in submissions]),
+ }
+
+ data.update(
+ **{
+ f'role_reviewer_{slugify(str(role))}': reviewer.id
+ for role, reviewer in zip(self.roles, reviewer_roles)
+ }
+ )
+ return data
+
+
+class StaffTestCase(BaseBatchReviewerTestCase):
+ user_factory = StaffFactory
+
+ def test_can_assign_role_reviewers(self):
+ reviewer_roles = [self.staff[0]]
+ submissions = self.submissions[0:2]
+ self.post_page(data=self.data(reviewer_roles, submissions))
+ for submission in submissions:
+ self.assertEqual(submission.assigned.count(), 1)
+ self.assertEqual(submission.assigned.first().reviewer, self.staff[0])
+ self.assertEqual(submission.assigned.first().role, self.roles[0])
+
+ def test_can_reassign_role_reviewers(self):
+ AssignedWithRoleReviewersFactory(reviewer=self.staff[1], submission=self.submissions[0], role=self.roles[0])
+ AssignedWithRoleReviewersFactory(reviewer=self.staff[1], submission=self.submissions[1], role=self.roles[0])
+ submissions = self.submissions[0:2]
+ reviewer_roles = [self.staff[0]]
+ self.post_page(data=self.data(reviewer_roles, submissions))
+ for submission in submissions:
+ self.assertEqual(submission.assigned.count(), 1)
+ self.assertEqual(submission.assigned.first().reviewer, self.staff[0])
+ self.assertEqual(submission.assigned.first().role, self.roles[0])
+
+ def test_can_reassign_from_other_role_reviewers(self):
+ AssignedWithRoleReviewersFactory(reviewer=self.staff[0], submission=self.submissions[0], role=self.roles[1])
+ AssignedWithRoleReviewersFactory(reviewer=self.staff[0], submission=self.submissions[1], role=self.roles[1])
+ submissions = self.submissions[0:2]
+ reviewer_roles = [self.staff[0]]
+ self.post_page(data=self.data(reviewer_roles, submissions))
+ for submission in submissions:
+ self.assertEqual(submission.assigned.count(), 1)
+ self.assertEqual(submission.assigned.first().reviewer, self.staff[0])
+ self.assertEqual(submission.assigned.first().role, self.roles[0])
+
+ def test_doesnt_remove_if_already_reviewed(self):
+ AssignedWithRoleReviewersFactory(reviewer=self.staff[1], submission=self.submissions[0], role=self.roles[0])
+ ReviewFactory(author=self.staff[1], submission=self.submissions[0], draft=False)
+ ReviewFactory(author=self.staff[1], submission=self.submissions[1], draft=False)
+ submissions = self.submissions[0:2]
+ reviewer_roles = [self.staff[0]]
+ self.post_page(data=self.data(reviewer_roles, submissions))
+ for submission in submissions:
+ self.assertEqual(submission.assigned.count(), 2)
+ reviewers = submission.assigned.values_list('reviewer', flat=True)
+ self.assertIn(self.staff[0].pk, reviewers)
+ self.assertIn(self.staff[1].pk, reviewers)
diff --git a/opentech/apply/funds/utils.py b/opentech/apply/funds/utils.py
new file mode 100644
index 0000000000000000000000000000000000000000..3a3578dbd3826efae067c78d8c17ca9e79dccc84
--- /dev/null
+++ b/opentech/apply/funds/utils.py
@@ -0,0 +1,8 @@
+from opentech.apply.utils.image import generate_image_tag
+
+
+def render_icon(image):
+ if not image:
+ return ''
+ filter_spec = 'fill-20x20'
+ return generate_image_tag(image, filter_spec)
diff --git a/opentech/apply/funds/views.py b/opentech/apply/funds/views.py
index a2e4499a799630dc41ed2063b8af0db8e560c110..f97c6092fd1ccfb149fd3b0b1f21ecda58f6aa10 100644
--- a/opentech/apply/funds/views.py
+++ b/opentech/apply/funds/views.py
@@ -97,23 +97,13 @@ class BatchUpdateReviewersView(DelegatedViewMixin, FormView):
form_class = BatchUpdateReviewersForm
context_name = 'batch_reviewer_form'
- def form_invalid(self, form):
- messages.error(self.request, mark_safe(_('Sorry something went wrong') + form.errors.as_ul()))
- return super().form_invalid(form)
-
def form_valid(self, form):
- """
- Loop through all submissions selected on the page,
- Add any reviewers that were selected, only if they are not
- currently saved to that submission.
- Send out a message of updates.
- """
- reviewers = form.cleaned_data['staff_reviewers']
-
submissions = form.cleaned_data['submissions']
-
- for submission in submissions:
- submission.reviewers.add(*reviewers)
+ form.save()
+ reviewers = [
+ [role, form.cleaned_data[field_name]]
+ for field_name, role in form.role_fields.items()
+ ]
messenger(
MESSAGES.BATCH_REVIEWERS_UPDATED,
@@ -122,9 +112,12 @@ class BatchUpdateReviewersView(DelegatedViewMixin, FormView):
submissions=submissions,
added=reviewers,
)
-
return super().form_valid(form)
+ def form_invalid(self, form):
+ messages.error(self.request, mark_safe(_('Sorry something went wrong') + form.errors.as_ul()))
+ return super().form_invalid(form)
+
@method_decorator(staff_required, name='dispatch')
class BatchProgressSubmissionView(DelegatedViewMixin, FormView):