From c310afa59a27a68ea7e8f00812aba60dc44e4c59 Mon Sep 17 00:00:00 2001
From: Erin Mullaney <erin.mullaney@torchbox.com>
Date: Fri, 22 Feb 2019 11:34:53 -0500
Subject: [PATCH] GH-958: Update the batch reviewer form to handle role
reviewers
---
opentech/apply/funds/forms.py | 105 +++++++++++++++++++++-------------
opentech/apply/funds/utils.py | 68 ++++++++++++++++++++++
opentech/apply/funds/views.py | 27 +++++----
3 files changed, 146 insertions(+), 54 deletions(-)
create mode 100644 opentech/apply/funds/utils.py
diff --git a/opentech/apply/funds/forms.py b/opentech/apply/funds/forms.py
index 99f11d75b..6a24fa2c4 100644
--- a/opentech/apply/funds/forms.py
+++ b/opentech/apply/funds/forms.py
@@ -6,7 +6,8 @@ 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 ApplicationSubmission, ReviewerRole
+from .utils import render_icon, save_reviewers_with_roles
from .widgets import Select2MultiCheckboxesWidget
from .workflow import get_action_mapping
@@ -100,23 +101,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 +128,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 +141,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
@@ -166,6 +152,7 @@ class UpdateReviewersForm(forms.ModelForm):
def save(self, *args, **kwargs):
instance = super().save(*args, **kwargs)
+<<<<<<< variant A
"""
1. Update role reviewers
2. Update non-role reviewers
@@ -204,34 +191,72 @@ class UpdateReviewersForm(forms.ModelForm):
) for reviewer in reviewers
if reviewer.id not in remaining_reviewers
)
-
- # 3. Add in anyone who has already reviewed but who is not selected as a reviewer on the form
- orphaned_reviews = instance.reviews.exclude(
- author__in=instance.assigned.values('reviewer')
- ).select_related('author')
-
- AssignedReviewers.objects.bulk_create(
- AssignedReviewers(
- submission=instance,
- role=None,
- reviewer=review.author
- ) for review in orphaned_reviews
+>>>>>>> variant B
+======= end
+
+ instance = save_reviewers_with_roles(
+ submission=instance,
+ role_fields=self.role_fields,
+ cleaned_data=self.cleaned_data,
+ alter_external_reviewers=self.can_alter_external_reviewers(self.instance, self.user),
+ submitted_reviewers=self.submitted_reviewers,
)
return instance
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 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/utils.py b/opentech/apply/funds/utils.py
new file mode 100644
index 000000000..8ce4f7170
--- /dev/null
+++ b/opentech/apply/funds/utils.py
@@ -0,0 +1,68 @@
+from opentech.apply.utils.image import generate_image_tag
+
+from .models import AssignedReviewers
+
+
+def save_reviewers_with_roles(submission, role_fields, cleaned_data, alter_external_reviewers=False, submitted_reviewers=None):
+ """
+ 1. Update role reviewers
+ 2. Update non-role reviewers
+ 2a. Remove those not on form
+ 2b. Add in any new non-role reviewers selected
+ 3. Add in anyone who has already reviewed but who is not selected as a reviewer on the form
+ """
+ # 1. Update role reviewers
+ assigned_roles = {
+ role: cleaned_data[field]
+ for field, role in role_fields.items()
+ }
+ for role, reviewer in assigned_roles.items():
+ if reviewer:
+ AssignedReviewers.objects.update_or_create(
+ submission=submission,
+ role=role,
+ defaults={'reviewer': reviewer},
+ )
+
+ # 2. Update non-role reviewers
+ # 2a. Remove those not on form
+ if alter_external_reviewers:
+ reviewers = cleaned_data['reviewer_reviewers']
+ assigned_reviewers = submission.assigned.without_roles()
+ assigned_reviewers.exclude(
+ reviewer__in=reviewers | submitted_reviewers
+ ).delete()
+
+ remaining_reviewers = assigned_reviewers.values_list('reviewer_id', flat=True)
+
+ # 2b. Add in any new non-role reviewers selected
+ AssignedReviewers.objects.bulk_create(
+ AssignedReviewers(
+ submission=submission,
+ role=None,
+ reviewer=reviewer
+ ) for reviewer in reviewers
+ if reviewer.id not in remaining_reviewers
+ )
+
+ # 3. Add in anyone who has already reviewed but who is not selected as a reviewer on the form
+ orphaned_reviews = submission.reviews.exclude(
+ author__in=submission.assigned.values('reviewer')
+ ).select_related('author')
+
+ AssignedReviewers.objects.bulk_create(
+ AssignedReviewers(
+ submission=submission,
+ role=None,
+ reviewer=review.author
+ ) for review in orphaned_reviews
+ )
+
+ return submission
+
+
+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 a2e4499a7..0849db79b 100644
--- a/opentech/apply/funds/views.py
+++ b/opentech/apply/funds/views.py
@@ -54,6 +54,7 @@ from .tables import (
SubmissionReviewerFilterAndSearch,
SummarySubmissionsTable,
)
+from .utils import save_reviewers_with_roles
from .workflow import STAGE_CHANGE_ACTIONS, PHASES_MAPPING, review_statuses
@@ -97,23 +98,18 @@ 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)
+ save_reviewers_with_roles(
+ submission=submission,
+ role_fields=form.role_fields,
+ cleaned_data=form.cleaned_data,
+ alter_external_reviewers=False,
+ submitted_reviewers=None,
+ )
+ reviewers = submissions.first().assigned.with_roles().all()
messenger(
MESSAGES.BATCH_REVIEWERS_UPDATED,
@@ -122,9 +118,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):
--
GitLab