Fix review assingment form

doc/changelog.rst

@@ -3,6 +3,7 @@
 Release Notes
 =============
 
+- :bug:`orga:review` Assigning reviewers could lead to incorrect assignments when browsers cached the form, but new reviewers were added to the team, shifting the overall order of input fields.
 - :feature:`cfp` Choice and multiple choice questions now use a drop-down with typeahead (search for options) when they have a lot of options.
 - :feature:`orga,1079` All images in forms in the organiser area now include a preview of the saved image, and open a lightbox instead of the image file when clicked.
 - :announcement:`admin` We now recommend that you use a virtualenv instead of the ``pip --user`` installation method, and have updated our install and upgrade documentation accordingly.

src/pretalx/orga/forms/review.py

@@ -183,57 +183,56 @@ class DirectionForm(forms.Form):
     )
 
 
-class ReviewerForProposalForm(forms.ModelForm):
-    def __init__(self, *args, reviewers=None, **kwargs):
+class ReviewAssignmentForm(forms.Form):
+    def __init__(self, *args, event=None, **kwargs):
+        self.event = event
+        self.reviewers = (
+            User.objects.filter(teams__in=self.event.teams.filter(is_reviewer=True))
+            .order_by("name")
+            .distinct()
+        ).prefetch_related("assigned_reviews")
+        self.submissions = self.event.submissions.order_by("title").prefetch_related(
+            "assigned_reviewers"
+        )
         super().__init__(*args, **kwargs)
-        self.fields["assigned_reviewers"].queryset = reviewers
-        self.fields["assigned_reviewers"].label = self.instance.title
 
-    def save(self, *args, **kwargs):
-        # No calling 'super().save()' here – it would potentially update a submission's code!
-        instance = self.instance
-        if "assigned_reviewers" in self.changed_data:
-            new_code = self.cleaned_data.get("code")
-            if instance.code != new_code:
-                instance = instance.event.submissions.all().get(code=new_code)
-            instance.assigned_reviewers.set(self.cleaned_data["assigned_reviewers"])
 
-    class Meta:
-        model = Submission
-        fields = ["assigned_reviewers", "code"]
-        widgets = {
-            "assigned_reviewers": forms.SelectMultiple(attrs={"class": "select2"}),
-            "code": forms.HiddenInput(),
-        }
+class ReviewerForProposalForm(ReviewAssignmentForm):
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        review_choices = [(r.id, r.name) for r in self.reviewers]
+        for submission in self.submissions:
+            self.fields[submission.code] = forms.MultipleChoiceField(
+                choices=review_choices,
+                widget=forms.SelectMultiple(attrs={"class": "select2"}),
+                initial=list(
+                    submission.assigned_reviewers.values_list("id", flat=True)
+                ),
+                label=submission.title,
+                required=False,
+            )
+
+    def save(self, *args, **kwargs):
+        for submission in self.submissions:
+            submission.assigned_reviewers.set(self.cleaned_data[submission.code])
 
 
-class ProposalForReviewerForm(forms.ModelForm):
-    def __init__(self, *args, proposals=None, **kwargs):
+class ProposalForReviewerForm(ReviewAssignmentForm):
+    def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        initial = proposals.filter(assigned_reviewers__in=[self.instance]).values_list(
-            "id", flat=True
-        )
-        self.fields["assigned_reviews"] = forms.MultipleChoiceField(
-            choices=((p.id, p.title) for p in proposals),
-            widget=forms.SelectMultiple(attrs={"class": "select2"}),
-            label=self.instance.name,
-            initial=list(initial),
-            required=False,
-        )
+        submission_choices = [(s.id, s.title) for s in self.submissions]
+        for reviewer in self.reviewers:
+            self.fields[reviewer.code] = forms.MultipleChoiceField(
+                choices=submission_choices,
+                widget=forms.SelectMultiple(attrs={"class": "select2"}),
+                initial=list(reviewer.assigned_reviews.values_list("id", flat=True)),
+                label=reviewer.name,
+                required=False,
+            )
 
     def save(self, *args, **kwargs):
-        # No calling 'super().save()' here – it would potentially update a user's code!
-        instance = self.instance
-        if "assigned_reviews" in self.changed_data:
-            new_code = self.cleaned_data.get("code")
-            if instance.code != new_code:
-                instance = User.objects.get(code=new_code)
-            instance.assigned_reviews.set(self.cleaned_data["assigned_reviews"])
-
-    class Meta:
-        model = User
-        fields = ["code"]
-        widgets = {"code": forms.HiddenInput()}
+        for reviewer in self.reviewers:
+            reviewer.assigned_reviews.set(self.cleaned_data[reviewer.code])
 
 
 class ReviewExportForm(ExportForm):

src/pretalx/orga/templates/orga/review/assignment.html

@@ -2,13 +2,10 @@
 {% load bootstrap4 %}
 {% load compress %}
 {% load i18n %}
-{% load formset_tags %}
 {% load static %}
 
 {% block content %}
     {% compress js %}
-        <script defer src="{% static "js/jquery.formset.js" %}"></script>
-        <script defer src="{% static "cfp/js/animateFormset.js" %}"></script>
         <script defer src="{% static "orga/js/reviewAssignment.js" %}"></script>
     {% endcompress %}
     <h2>{% translate "Assign reviewers" %}</h2>
@@ -140,31 +137,13 @@ <h2>{% translate "Assign reviewers" %}</h2>
                 </div>
                 <div class="ml-auto mr-auto col-md-4">
                     <form id="direction">
-                        {% bootstrap_form form layout='inline' %}
+                        {% bootstrap_form direction_form layout='inline' %}
                     </form>
                 </div>
                 <form method="post" class="d-flex flex-column select2-textbox">
                     {% csrf_token %}
 
-                    <div class="formset" data-formset data-formset-prefix="{{ formset.prefix }}">
-                        {{ formset.management_form }}
-                        {% bootstrap_formset_errors formset %}
-                        <div data-formset-body>
-                            {% for form in formset %}
-                                <div data-formset-form>
-                                    <div class="sr-only">
-                                        {{ form.id }}
-                                    </div>
-                                    <div class="reviewer-formset">
-                                        {% bootstrap_form_errors form %}
-                                        {% for field in form %}{% if field.label %}
-                                            {% bootstrap_field field layout='event' %}
-                                        {% endif %}{% endfor %}
-                                    </div>
-                                </div>
-                            {% endfor %}
-                        </div>
-                    </div>
+                    {% bootstrap_form form layout="event" %}
                     <div class="submit-group panel">
                         <span></span>
                         <span>

src/pretalx/orga/views/review.py

@@ -5,7 +5,6 @@
 from django.contrib import messages
 from django.db import transaction
 from django.db.models import Count, Max, OuterRef, Q, Subquery
-from django.forms.models import BaseModelFormSet, modelformset_factory
 from django.shortcuts import get_object_or_404, redirect
 from django.utils.functional import cached_property
 from django.utils.translation import gettext_lazy as _
@@ -25,7 +24,6 @@
 )
 from pretalx.orga.forms.submission import SubmissionStateChangeForm
 from pretalx.orga.views.submission import BaseSubmissionList
-from pretalx.person.models import User
 from pretalx.submission.forms import QuestionsForm, SubmissionFilterForm
 from pretalx.submission.models import Review, Submission, SubmissionStates
 
@@ -699,7 +697,6 @@ def post(self, request, **kwargs):
 class ReviewAssignment(EventPermissionRequired, FormView):
     template_name = "orga/review/assignment.html"
     permission_required = "orga.change_settings"
-    form_class = DirectionForm
 
     @cached_property
     def form_type(self):
@@ -708,68 +705,32 @@ def form_type(self):
             return "reviewer"
         return direction
 
-    def get_form(self):
+    @context
+    @cached_property
+    def direction_form(self):
         return DirectionForm(self.request.GET)
 
     @context
     @cached_property
     def review_teams(self):
         return self.request.event.teams.filter(is_reviewer=True)
 
-    @context
-    @cached_property
-    def formset(self):
-        proposals = self.request.event.submissions.order_by("title")
-        reviewers = (
-            User.objects.filter(
-                teams__in=self.request.event.teams.filter(is_reviewer=True)
-            )
-            .order_by("name")
-            .distinct()
-        )
-
+    def get_form(self):
         if self.form_type == "submission":
-            formset_class = modelformset_factory(
-                model=Submission,
-                form=ReviewerForProposalForm,
-                formset=BaseModelFormSet,
-                can_delete=False,
-                extra=0,
-                max_num=0,
-            )
-            result = formset_class(
-                self.request.POST if self.request.method == "POST" else None,
-                files=self.request.FILES if self.request.method == "POST" else None,
-                queryset=proposals,
-                form_kwargs={"reviewers": reviewers},
-                prefix="formset",
-            )
-            return result
+            form_class = ReviewerForProposalForm
         else:
-            formset_class = modelformset_factory(
-                User,
-                form=ProposalForReviewerForm,
-                formset=BaseModelFormSet,
-                can_delete=False,
-                extra=0,
-                max_num=0,
-            )
-            return formset_class(
-                self.request.POST if self.request.method == "POST" else None,
-                files=self.request.FILES if self.request.method == "POST" else None,
-                queryset=reviewers,
-                form_kwargs={"proposals": proposals},
-                prefix="formset",
-            )
-
-    def post(self, request, *args, **kwargs):
-        if not self.formset.is_valid():
-            return self.get(self.request, *self.args, **self.kwargs)
+            form_class = ProposalForReviewerForm
+        return form_class(
+            self.request.POST if self.request.method == "POST" else None,
+            files=self.request.FILES if self.request.method == "POST" else None,
+            event=self.request.event,
+            prefix=self.form_type,
+        )
 
-        for form in self.formset:
-            form.save()
-        messages.success(request, _("Saved!"))
-        return self.get(self.request, *self.args, **self.kwargs)
+    def form_valid(self, form):
+        form.save()
+        messages.success(self.request, _("Saved!"))
+        return redirect(self.request.event.orga_urls.review_assignments)
 
 
 class ReviewAssignmentImport(EventPermissionRequired, FormView):