From 2a5c24482e965959c1f7580bfd685da14f7e4500 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Tue, 23 Jun 2020 13:05:54 +0200 Subject: [PATCH] Question list: Drop pagination, allow to mix ordering with system fields --- src/pretix/base/forms/questions.py | 34 +++- src/pretix/base/settings.py | 4 + .../pretixcontrol/items/questions.html | 146 +++++++------- src/pretix/control/urls.py | 3 - src/pretix/control/views/item.py | 184 ++++++++++-------- .../pretixcontrol/js/ui/dragndroplist.js | 17 +- src/tests/control/test_items.py | 29 --- src/tests/control/test_permissions.py | 2 - 8 files changed, 215 insertions(+), 204 deletions(-) diff --git a/src/pretix/base/forms/questions.py b/src/pretix/base/forms/questions.py index 54c41279f..2e6fa4bc0 100644 --- a/src/pretix/base/forms/questions.py +++ b/src/pretix/base/forms/questions.py @@ -244,8 +244,10 @@ class BaseQuestionsForm(forms.Form): super().__init__(*args, **kwargs) + add_fields = {} + if item.admission and event.settings.attendee_names_asked: - self.fields['attendee_name_parts'] = NamePartsFormField( + add_fields['attendee_name_parts'] = NamePartsFormField( max_length=255, required=event.settings.attendee_names_required and not self.all_optional, scheme=event.settings.name_scheme, @@ -254,7 +256,7 @@ class BaseQuestionsForm(forms.Form): initial=(cartpos.attendee_name_parts if cartpos else orderpos.attendee_name_parts), ) if item.admission and event.settings.attendee_emails_asked: - self.fields['attendee_email'] = forms.EmailField( + add_fields['attendee_email'] = forms.EmailField( required=event.settings.attendee_emails_required and not self.all_optional, label=_('Attendee email'), initial=(cartpos.attendee_email if cartpos else orderpos.attendee_email), @@ -265,14 +267,14 @@ class BaseQuestionsForm(forms.Form): ) ) if item.admission and event.settings.attendee_company_asked: - self.fields['company'] = forms.CharField( + add_fields['company'] = forms.CharField( required=event.settings.attendee_company_required and not self.all_optional, label=_('Company'), initial=(cartpos.company if cartpos else orderpos.company), ) if item.admission and event.settings.attendee_addresses_asked: - self.fields['street'] = forms.CharField( + add_fields['street'] = forms.CharField( required=event.settings.attendee_addresses_required and not self.all_optional, label=_('Address'), widget=forms.Textarea(attrs={ @@ -282,7 +284,7 @@ class BaseQuestionsForm(forms.Form): }), initial=(cartpos.street if cartpos else orderpos.street), ) - self.fields['zipcode'] = forms.CharField( + add_fields['zipcode'] = forms.CharField( required=event.settings.attendee_addresses_required and not self.all_optional, label=_('ZIP code'), initial=(cartpos.zipcode if cartpos else orderpos.zipcode), @@ -290,7 +292,7 @@ class BaseQuestionsForm(forms.Form): 'autocomplete': 'postal-code', }), ) - self.fields['city'] = forms.CharField( + add_fields['city'] = forms.CharField( required=event.settings.attendee_addresses_required and not self.all_optional, label=_('City'), initial=(cartpos.city if cartpos else orderpos.city), @@ -299,7 +301,7 @@ class BaseQuestionsForm(forms.Form): }), ) country = (cartpos.country if cartpos else orderpos.country) or guess_country(event) - self.fields['country'] = CountryField( + add_fields['country'] = CountryField( countries=CachedCountries ).formfield( required=event.settings.attendee_addresses_required and not self.all_optional, @@ -324,7 +326,7 @@ class BaseQuestionsForm(forms.Form): self.data = self.data.copy() del self.data[fprefix + 'state'] - self.fields['state'] = forms.ChoiceField( + add_fields['state'] = forms.ChoiceField( label=pgettext_lazy('address', 'State'), required=False, choices=c, @@ -332,7 +334,14 @@ class BaseQuestionsForm(forms.Form): 'autocomplete': 'address-level1', }), ) - self.fields['state'].widget.is_required = True + add_fields['state'].widget.is_required = True + + field_positions = list( + [ + (n, event.settings.system_question_order.get(n if n != 'state' else 'country', 0)) + for n in add_fields.keys() + ] + ) for q in questions: # Do we already have an answer? Provide it as the initial value @@ -485,7 +494,12 @@ class BaseQuestionsForm(forms.Form): field._required = q.required and not self.all_optional field.required = False - self.fields['question_%s' % q.id] = field + add_fields['question_%s' % q.id] = field + field_positions.append(('question_%s' % q.id, q.position)) + + field_positions.sort(key=lambda e: e[1]) + for fname, p in field_positions: + self.fields[fname] = add_fields[fname] responses = question_form_fields.send(sender=event, position=pos) data = pos.meta_info_data diff --git a/src/pretix/base/settings.py b/src/pretix/base/settings.py index b250cfeac..835f8cda3 100644 --- a/src/pretix/base/settings.py +++ b/src/pretix/base/settings.py @@ -56,6 +56,10 @@ DEFAULTS = { ) }, + 'system_question_order': { + 'default': {}, + 'type': dict, + }, 'attendee_names_asked': { 'default': 'True', 'type': bool, diff --git a/src/pretix/control/templates/pretixcontrol/items/questions.html b/src/pretix/control/templates/pretixcontrol/items/questions.html index b9792a1bb..a067cdcbd 100644 --- a/src/pretix/control/templates/pretixcontrol/items/questions.html +++ b/src/pretix/control/templates/pretixcontrol/items/questions.html @@ -10,81 +10,89 @@ {% endblocktrans %}

{% csrf_token %} - {% if questions|length == 0 %} -
-

- {% blocktrans trimmed %} - You haven't created any questions yet. - {% endblocktrans %} -

- - {% trans "Create a new question" %} -
- {% else %} -

- {% trans "Create a new question" %} - -

-
- - - - - - - - - - - - - - - {% for q in questions %} - - - + + {% endfor %} + +
{% trans "Question" %}{% trans "Type" %}{% trans "Products" %}
{{ q.question }} - +

+ {% trans "Create a new question" %} + +

+
+ + + + + + + + + + + + + + + {% for q in questions %} + + + - - + + - - + + - - + + - - {% endfor %} - -
{% trans "Question" %}{% trans "Type" %}{% trans "Products" %}
+ + {% if q.pk %} + + {% endif %} + {{ q.question }} + {% if q.pk %} + + {% endif %} + + + {% if q.pk %} {{ q.get_type_display }} - - {% if q.required %} - - {% endif %} - - {% if q.ask_during_checkin %} - - {% endif %} + {% else %} + {% trans "System question" %} + {% endif %} + + {% if q.required %} + + {% endif %} + + {% if q.pk and q.ask_during_checkin %} + + {% endif %} - - {% if q.hidden %} - - {% endif %} - + + {% if q.pk and q.hidden %} + + {% endif %} + + {% if q.pk %} - - - - + {% else %} + {% trans "All admission products" %} + {% endif %} + + + {% if q.pk %} + -
-
- {% include "pretixcontrol/pagination.html" %} - {% endif %} + {% else %} + + {% endif %} +
+
{% endblock %} diff --git a/src/pretix/control/urls.py b/src/pretix/control/urls.py index fd6b03356..617ce0df4 100644 --- a/src/pretix/control/urls.py +++ b/src/pretix/control/urls.py @@ -180,9 +180,6 @@ urlpatterns = [ url(r'^questions/reorder$', item.reorder_questions, name='event.items.questions.reorder'), url(r'^questions/(?P\d+)/delete$', item.QuestionDelete.as_view(), name='event.items.questions.delete'), - url(r'^questions/(?P\d+)/up$', item.question_move_up, name='event.items.questions.up'), - url(r'^questions/(?P\d+)/down$', item.question_move_down, - name='event.items.questions.down'), url(r'^questions/(?P\d+)/$', item.QuestionView.as_view(), name='event.items.questions.show'), url(r'^questions/(?P\d+)/change$', item.QuestionUpdate.as_view(), diff --git a/src/pretix/control/views/item.py b/src/pretix/control/views/item.py index e831ad288..60cba92ca 100644 --- a/src/pretix/control/views/item.py +++ b/src/pretix/control/views/item.py @@ -1,12 +1,12 @@ import json -from collections import OrderedDict +from collections import OrderedDict, namedtuple from json.decoder import JSONDecodeError from django.contrib import messages from django.core.exceptions import PermissionDenied from django.core.files import File from django.db import transaction -from django.db.models import Count, Exists, F, Max, OuterRef, Prefetch, Q +from django.db.models import Count, Exists, F, OuterRef, Prefetch, Q from django.forms.models import inlineformset_factory from django.http import ( Http404, HttpResponse, HttpResponseBadRequest, HttpResponseRedirect, @@ -279,7 +279,12 @@ def category_move_down(request, organizer, event, category): event=request.event.slug) -class QuestionList(PaginationMixin, ListView): +FakeQuestion = namedtuple( + 'FakeQuestion', 'id question position required' +) + + +class QuestionList(ListView): model = Question context_object_name = 'questions' template_name = 'pretixcontrol/items/questions.html' @@ -287,98 +292,125 @@ class QuestionList(PaginationMixin, ListView): def get_queryset(self): return self.request.event.questions.prefetch_related('items') + def get_context_data(self, **kwargs): + ctx = super().get_context_data(**kwargs) + ctx['questions'] = list(ctx['questions']) + + if self.request.event.settings.attendee_names_asked: + ctx['questions'].append( + FakeQuestion( + id='attendee_name_parts', + question=_('Attendee name'), + position=self.request.event.settings.system_question_order.get( + 'attendee_name_parts', 0 + ), + required=self.request.event.settings.attendee_names_required, + ) + ) + + if self.request.event.settings.attendee_emails_asked: + ctx['questions'].append( + FakeQuestion( + id='attendee_email', + question=_('Attendee email'), + position=self.request.event.settings.system_question_order.get( + 'attendee_email', 0 + ), + required=self.request.event.settings.attendee_emails_required, + ) + ) + + if self.request.event.settings.attendee_emails_asked: + ctx['questions'].append( + FakeQuestion( + id='company', + question=_('Company'), + position=self.request.event.settings.system_question_order.get( + 'company', 0 + ), + required=self.request.event.settings.attendee_company_required, + ) + ) + + if self.request.event.settings.attendee_addresses_asked: + ctx['questions'].append( + FakeQuestion( + id='street', + question=_('Street'), + position=self.request.event.settings.system_question_order.get( + 'street', 0 + ), + required=self.request.event.settings.attendee_addresses_required, + ) + ) + ctx['questions'].append( + FakeQuestion( + id='zipcode', + question=_('ZIP code'), + position=self.request.event.settings.system_question_order.get( + 'zipcode', 0 + ), + required=self.request.event.settings.attendee_addresses_required, + ) + ) + ctx['questions'].append( + FakeQuestion( + id='city', + question=_('City'), + position=self.request.event.settings.system_question_order.get( + 'city', 0 + ), + required=self.request.event.settings.attendee_addresses_required, + ) + ) + ctx['questions'].append( + FakeQuestion( + id='country', + question=_('Country'), + position=self.request.event.settings.system_question_order.get( + 'country', 0 + ), + required=self.request.event.settings.attendee_addresses_required, + ) + ) + + ctx['questions'].sort(key=lambda q: q.position) + return ctx + @transaction.atomic @event_permission_required("can_change_items") def reorder_questions(request, organizer, event): try: - ids = [int(id) for id in json.loads(request.body.decode('utf-8'))['ids']] + ids = json.loads(request.body.decode('utf-8'))['ids'] except (JSONDecodeError, KeyError, ValueError): return HttpResponseBadRequest("expected JSON: {ids:[]}") - input_questions = request.event.questions.filter(id__in=ids) + input_questions = request.event.questions.filter(id__in=[i for i in ids if i.isdigit()]) - if input_questions.count() != len(ids): + if input_questions.count() != len([i for i in ids if i.isdigit()]): raise Http404(_("Some of the provided question ids are invalid.")) - first = input_questions.first() - last = input_questions.last() - original_lowest_score = (first.position, first.id) - original_highest_score = (last.position, last.id) + if input_questions.count() != request.event.questions.count(): + raise Http404(_("Not all questions have been selected.")) - if request.event.questions.filter( - Q(Q(position__gt=original_lowest_score[0]) - | Q(Q(position=original_lowest_score[0]) & Q(pk__gt=original_lowest_score[1]))) - & - Q(Q(position__lt=original_highest_score[0]) - | Q(Q(position=original_highest_score[0]) & Q(pk__lt=original_highest_score[1]))) - ).exclude(id__in=ids).exists(): - return HttpResponseBadRequest("ids need to be from a consecutive range of questions") - - highest_position_on_previous_page = request.event.questions.filter( - Q(position__lt=original_lowest_score[0]) - | Q(Q(position=original_lowest_score[0]) & Q(pk__lt=original_lowest_score[1])) - ).aggregate(m=Max('position'))['m'] or 0 - - questions_on_later_pages = request.event.questions.filter( - Q(position__gt=original_highest_score[0]) - | Q(Q(position=original_highest_score[0]) & Q(pk__gt=original_highest_score[1])) - ) - - ordered_questions = sorted(input_questions, key=lambda k: ids.index(k.pk)) - - for i, q in enumerate(ordered_questions + list(questions_on_later_pages)): - pos = highest_position_on_previous_page + 1 + i + for q in input_questions: + pos = ids.index(str(q.pk)) if pos != q.position: # Save unneccessary UPDATE queries q.position = pos q.save(update_fields=['position']) + system_question_order = {} + for s in ('attendee_name_parts', 'attendee_email', 'company', 'street', 'zipcode', 'city', 'country'): + if s in ids: + system_question_order[s] = ids.index(s) + else: + system_question_order[s] = -1 + request.event.settings.system_question_order = system_question_order + return HttpResponse() -def question_move(request, question, up=True): - """ - This is a helper function to avoid duplicating code in question_move_up and - question_move_down. It takes a question and a direction and then tries to bring - all items for this question in a new order. - """ - try: - question = request.event.questions.get( - id=question - ) - except Question.DoesNotExist: - raise Http404(_("The selected question does not exist.")) - questions = list(request.event.questions.order_by("position")) - - index = questions.index(question) - if index != 0 and up: - questions[index - 1], questions[index] = questions[index], questions[index - 1] - elif index != len(questions) - 1 and not up: - questions[index + 1], questions[index] = questions[index], questions[index + 1] - - for i, qt in enumerate(questions): - if qt.position != i: - qt.position = i - qt.save() - messages.success(request, _('The order of questions has been updated.')) - - -@event_permission_required("can_change_items") -def question_move_up(request, organizer, event, question): - question_move(request, question, up=True) - return redirect('control:event.items.questions', - organizer=request.event.organizer.slug, - event=request.event.slug) - - -@event_permission_required("can_change_items") -def question_move_down(request, organizer, event, question): - question_move(request, question, up=False) - return redirect('control:event.items.questions', - organizer=request.event.organizer.slug, - event=request.event.slug) - - class QuestionDelete(EventPermissionRequiredMixin, DeleteView): model = Question template_name = 'pretixcontrol/items/question_delete.html' diff --git a/src/pretix/static/pretixcontrol/js/ui/dragndroplist.js b/src/pretix/static/pretixcontrol/js/ui/dragndroplist.js index fd1fba2c8..a9b1e9ac7 100644 --- a/src/pretix/static/pretixcontrol/js/ui/dragndroplist.js +++ b/src/pretix/static/pretixcontrol/js/ui/dragndroplist.js @@ -3,21 +3,10 @@ $(function () { $("[data-dnd-url]").each(function(){ var container = $(this), url = container.data("dnd-url"), - up = container.find("a:has(.fa-arrow-up)"), handle = $(''); - function hideArrows(container){ - var up = container.find("a:has(.fa-arrow-up)"), - firstUp = up.first(), - down = container.find("a:has(.fa-arrow-down)"), - lastDown = down.last(); - up.not(firstUp).css("display","none"); - down.not(lastDown).css("display","none"); - firstUp.css("display","inline-block"); - lastDown.css("display","inline-block"); - } - up.after(handle); - hideArrows(container); + console.log(container, container.find(".dnd-container")); + container.find(".dnd-container").append(handle); Sortable.create(container.get(0), { handle: ".dnd-sort-handle", @@ -25,8 +14,6 @@ $(function () { var container = $(evt.to), ids = container.find("[data-dnd-id]").toArray().map(function (e) { return e.dataset.dndId; }); - hideArrows(container); - $.ajax( { 'type': 'POST', diff --git a/src/tests/control/test_items.py b/src/tests/control/test_items.py index 7b645c503..fd5cc3cf9 100644 --- a/src/tests/control/test_items.py +++ b/src/tests/control/test_items.py @@ -171,35 +171,6 @@ class QuestionsTest(ItemFormTest): self.assertTrue(c.required) assert str(Question.objects.get(id=c.id).question) == 'How old are you?' - def test_sort(self): - with scopes_disabled(): - q1 = Question.objects.create(event=self.event1, question="Vegetarian?", type="N", required=True, position=0) - q2 = Question.objects.create(event=self.event1, question="Food allergies?", position=1) - doc = self.get_doc('/control/event/%s/%s/questions/' % (self.orga1.slug, self.event1.slug)) - self.assertIn("Vegetarian?", doc.select("table > tbody > tr")[0].text) - self.assertIn("Food allergies?", doc.select("table > tbody > tr")[1].text) - - self.client.get('/control/event/%s/%s/questions/%s/down' % (self.orga1.slug, self.event1.slug, q1.id)) - doc = self.get_doc('/control/event/%s/%s/questions/' % (self.orga1.slug, self.event1.slug)) - self.assertIn("Vegetarian?", doc.select("table > tbody > tr")[1].text) - self.assertIn("Food allergies?", doc.select("table > tbody > tr")[0].text) - - self.client.get('/control/event/%s/%s/questions/%s/up' % (self.orga1.slug, self.event1.slug, q1.id)) - doc = self.get_doc('/control/event/%s/%s/questions/' % (self.orga1.slug, self.event1.slug)) - self.assertIn("Vegetarian?", doc.select("table > tbody > tr")[0].text) - self.assertIn("Food allergies?", doc.select("table > tbody > tr")[1].text) - - self.client.post( - '/control/event/%s/%s/questions/reorder' % (self.orga1.slug, self.event1.slug), - { - "ids": [q2.id, q1.id] - }, - content_type='application/json' - ) - doc = self.get_doc('/control/event/%s/%s/questions/' % (self.orga1.slug, self.event1.slug)) - self.assertIn("Vegetarian?", doc.select("table > tbody > tr")[1].text) - self.assertIn("Food allergies?", doc.select("table > tbody > tr")[0].text) - def test_delete(self): with scopes_disabled(): c = Question.objects.create(event=self.event1, question="What is your shoe size?", type="N", required=True) diff --git a/src/tests/control/test_permissions.py b/src/tests/control/test_permissions.py index ab93485cf..aa0246c8d 100644 --- a/src/tests/control/test_permissions.py +++ b/src/tests/control/test_permissions.py @@ -260,8 +260,6 @@ event_permission_urls = [ # ("can_change_items", "questions/", 200), ("can_change_items", "questions/2/", 404), ("can_change_items", "questions/2/delete", 404), - ("can_change_items", "questions/2/up", 404), - ("can_change_items", "questions/2/down", 404), ("can_change_items", "questions/reorder", 400), ("can_change_items", "questions/add", 200), # ("can_change_items", "quotas/", 200),