From 78a39b5259fedad938971270c2e26ff4684251ea Mon Sep 17 00:00:00 2001 From: Tobias Kunze Date: Wed, 3 Apr 2024 12:31:39 +0200 Subject: [PATCH] Fix review assingment form --- doc/changelog.rst | 1 + src/pretalx/orga/forms/review.py | 85 +++++++++---------- .../templates/orga/review/assignment.html | 25 +----- src/pretalx/orga/views/review.py | 71 ++++------------ 4 files changed, 61 insertions(+), 121 deletions(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index e58d9e96f..6c27208bf 100644 --- a/doc/changelog.rst +++ b/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. diff --git a/src/pretalx/orga/forms/review.py b/src/pretalx/orga/forms/review.py index 6f34dea92..a31e6b308 100644 --- a/src/pretalx/orga/forms/review.py +++ b/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): diff --git a/src/pretalx/orga/templates/orga/review/assignment.html b/src/pretalx/orga/templates/orga/review/assignment.html index 8f9ebd554..56ffda572 100644 --- a/src/pretalx/orga/templates/orga/review/assignment.html +++ b/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 %} - - {% endcompress %}

{% translate "Assign reviewers" %}

@@ -140,31 +137,13 @@

{% translate "Assign reviewers" %}

- {% bootstrap_form form layout='inline' %} + {% bootstrap_form direction_form layout='inline' %}
{% csrf_token %} -
- {{ formset.management_form }} - {% bootstrap_formset_errors formset %} -
- {% for form in formset %} -
-
- {{ form.id }} -
-
- {% bootstrap_form_errors form %} - {% for field in form %}{% if field.label %} - {% bootstrap_field field layout='event' %} - {% endif %}{% endfor %} -
-
- {% endfor %} -
-
+ {% bootstrap_form form layout="event" %}
diff --git a/src/pretalx/orga/views/review.py b/src/pretalx/orga/views/review.py index 18c606ff7..4dc491dfb 100644 --- a/src/pretalx/orga/views/review.py +++ b/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,7 +705,9 @@ 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 @@ -716,60 +715,22 @@ def get_form(self): 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):