Refactor: separate forms from view, improve code style

src/pretix/presale/forms/auth.py

@@ -0,0 +1,220 @@
+from django.contrib.auth import authenticate
+from django.core.validators import RegexValidator
+from django import forms
+from django.forms import Form
+from django.contrib.auth.forms import AuthenticationForm as BaseAuthenticationForm
+from django.utils.translation import ugettext_lazy as _
+from django.conf import settings
+from pretix.base.models import User
+
+
+class LoginForm(BaseAuthenticationForm):
+    username = forms.CharField(
+        label=_('Username'),
+        help_text=(
+            _('If you registered for multiple events, your username is your email address.')
+            if settings.PRETIX_GLOBAL_REGISTRATION
+            else None
+        )
+    )
+    password = forms.CharField(
+        label=_('Password'),
+        widget=forms.PasswordInput
+    )
+
+    error_messages = {
+        'invalid_login': _("Please enter a correct username and password."),
+        'inactive': _("This account is inactive."),
+    }
+
+    def __init__(self, request=None, *args, **kwargs):
+        self.request = request
+        self.user_cache = None
+        super(forms.Form, self).__init__(*args, **kwargs)
+
+    def clean(self):
+        username = self.cleaned_data.get('username')
+        password = self.cleaned_data.get('password')
+
+        if username and password:
+            if '@' in username:
+                identifier = username.lower()
+            else:
+                identifier = "%s@%s.event.pretix" % (username, self.request.event.identity)
+            self.user_cache = authenticate(identifier=identifier,
+                                           password=password)
+            if self.user_cache is None:
+                raise forms.ValidationError(
+                    self.error_messages['invalid_login'],
+                    code='invalid_login',
+                )
+            else:
+                self.confirm_login_allowed(self.user_cache)
+
+        return self.cleaned_data
+
+
+class GlobalRegistrationForm(forms.Form):
+    error_messages = {
+        'duplicate_email': _("You already registered with that e-mail address, please use the login form."),
+        'pw_mismatch': _("Please enter the same password twice"),
+    }
+    email = forms.EmailField(
+        label=_('Email address'),
+        required=True
+    )
+    password = forms.CharField(
+        label=_('Password'),
+        widget=forms.PasswordInput,
+        required=True
+    )
+    password_repeat = forms.CharField(
+        label=_('Repeat password'),
+        widget=forms.PasswordInput
+    )
+
+    def clean(self):
+        password1 = self.cleaned_data.get('password')
+        password2 = self.cleaned_data.get('password_repeat')
+
+        if password1 and password1 != password2:
+            raise forms.ValidationError(
+                self.error_messages['pw_mismatch'],
+                code='pw_mismatch',
+            )
+
+        return self.cleaned_data
+
+    def clean_email(self):
+        email = self.cleaned_data['email']
+        if User.objects.filter(identifier=email).exists():
+            raise forms.ValidationError(
+                self.error_messages['duplicate_email'],
+                code='duplicate_email',
+            )
+        return email
+
+
+class LocalRegistrationForm(forms.Form):
+    error_messages = {
+        'invalid_username': _("Please only use characters, numbers or ./+/-/_ in your username."),
+        'duplicate_username': _("This username is already taken. Please choose a different one."),
+        'pw_mismatch': _("Please enter the same password twice"),
+    }
+    username = forms.CharField(
+        label=_('Username'),
+        validators=[
+            RegexValidator(
+                regex='^[a-zA-Z0-9\.+\-_]*$',
+                code='invalid_username',
+                message=error_messages['invalid_username']
+            ),
+        ],
+        required=True
+    )
+    email = forms.EmailField(
+        label=_('E-mail address'),
+        required=False
+    )
+    password = forms.CharField(
+        label=_('Password'),
+        widget=forms.PasswordInput,
+        required=True
+    )
+    password_repeat = forms.CharField(
+        label=_('Repeat password'),
+        widget=forms.PasswordInput
+    )
+
+    def __init__(self, request, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.request = request
+        self.fields['email'].required = request.event.settings.user_mail_required
+
+    def clean(self):
+        password1 = self.cleaned_data.get('password')
+        password2 = self.cleaned_data.get('password_repeat')
+
+        if password1 and password1 != password2:
+            raise forms.ValidationError(
+                self.error_messages['pw_mismatch'],
+                code='pw_mismatch',
+            )
+
+        return self.cleaned_data
+
+    def clean_username(self):
+        username = self.cleaned_data['username']
+        if User.objects.filter(event=self.request.event, username=username).exists():
+            raise forms.ValidationError(
+                self.error_messages['duplicate_username'],
+                code='duplicate_username',
+            )
+        return username
+
+
+class PasswordRecoverForm(Form):
+    error_messages = {
+        'pw_mismatch': _("Please enter the same password twice"),
+    }
+    password = forms.CharField(
+        label=_('Password'),
+        widget=forms.PasswordInput,
+        required=True
+    )
+    password_repeat = forms.CharField(
+        label=_('Repeat password'),
+        widget=forms.PasswordInput
+    )
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
+    def clean(self):
+        password1 = self.cleaned_data.get('password')
+        password2 = self.cleaned_data.get('password_repeat')
+
+        if password1 and password1 != password2:
+            raise forms.ValidationError(
+                self.error_messages['pw_mismatch'],
+                code='pw_mismatch',
+            )
+
+        return self.cleaned_data
+
+
+class PasswordForgotForm(Form):
+    username = forms.CharField(
+        label=_('Username or E-mail'),
+    )
+
+    def __init__(self, event, *args, **kwargs):
+        self.event = event
+        super().__init__(*args, **kwargs)
+
+    def clean_username(self):
+        username = self.cleaned_data['username']
+        try:
+            self.cleaned_data['user'] = User.objects.get(
+                identifier=username, event__isnull=True
+            )
+            return username
+        except User.DoesNotExist:
+            pass
+        try:
+            self.cleaned_data['user'] = User.objects.get(
+                username=username, event=self.event
+            )
+            return username
+        except User.DoesNotExist:
+            pass
+        try:
+            self.cleaned_data['user'] = User.objects.get(
+                email=username, event=self.event
+            )
+            return username
+        except:
+            raise forms.ValidationError(
+                _("We are unable to find a user matching the data you provided."),
+                code='unknown_user',
+            )

src/pretix/presale/forms/checkout.py

@@ -0,0 +1,71 @@
+from django import forms
+from django.utils.translation import ugettext_lazy as _
+from pretix.base.models import Question
+
+
+class QuestionsForm(forms.Form):
+    """
+    This form class is responsible for asking order-related questions. This includes
+    the attendee name for admission tickets, if the corresponding setting is enabled,
+    as well as additional questions defined by the organizer.
+    """
+
+    def __init__(self, *args, **kwargs):
+        """
+        Takes two additional keyword arguments:
+
+        :param cartpos: The cart position the form should be for
+        :param event: The event this belongs to
+        """
+        cartpos = kwargs.pop('cartpos', None)
+        orderpos = kwargs.pop('orderpos', None)
+        item = cartpos.item if cartpos else orderpos.item
+        questions = list(item.questions.all())
+        event = kwargs.pop('event')
+
+        super().__init__(*args, **kwargs)
+
+        if item.admission and event.settings.attendee_names_asked:
+            self.fields['attendee_name'] = forms.CharField(
+                max_length=255, required=event.settings.attendee_names_required,
+                label=_('Attendee name'),
+                initial=(cartpos.attendee_name if cartpos else orderpos.attendee_name)
+            )
+
+        for q in questions:
+            # Do we already have an answer? Provide it as the initial value
+            answers = [
+                a for a
+                in (cartpos.answers.all() if cartpos else orderpos.answers.all())
+                if a.question_id == q.identity
+            ]
+            if answers:
+                initial = answers[0].answer
+            else:
+                initial = None
+            if q.type == Question.TYPE_BOOLEAN:
+                field = forms.BooleanField(
+                    label=q.question, required=q.required,
+                    initial=initial
+                )
+            elif q.type == Question.TYPE_NUMBER:
+                field = forms.DecimalField(
+                    label=q.question, required=q.required,
+                    initial=initial
+                )
+            elif q.type == Question.TYPE_STRING:
+                field = forms.CharField(
+                    label=q.question, required=q.required,
+                    initial=initial
+                )
+            elif q.type == Question.TYPE_TEXT:
+                field = forms.CharField(
+                    label=q.question, required=q.required,
+                    widget=forms.Textarea,
+                    initial=initial
+                )
+            field.question = q
+            if answers:
+                # Cache the answer object for later use
+                field.answer = answers[0]
+            self.fields['question_%s' % q.identity] = field

src/pretix/presale/views/checkout.py

@@ -1,9 +1,9 @@
 from datetime import timedelta, datetime
+
 from django.contrib import messages
 from django.core.urlresolvers import reverse
 from django.db import transaction
 from django.db.models import Q, Sum
-from django import forms
 from django.http import HttpRequest
 from django.shortcuts import redirect
 from django.utils.functional import cached_property
@@ -11,82 +11,13 @@ from django.utils.timezone import now
 from django.views.generic import TemplateView
 from django.utils.translation import ugettext_lazy as _
 from pretix.base.mail import mail
-from pretix.base.models import CartPosition, Question, QuestionAnswer, Quota, Order, OrderPosition
+from pretix.base.models import CartPosition, QuestionAnswer, Quota, Order, OrderPosition
 from pretix.base.signals import register_payment_providers
-
+from pretix.presale.forms.checkout import QuestionsForm
 from pretix.presale.views import EventViewMixin, CartDisplayMixin, EventLoginRequiredMixin
 
 
-class QuestionsForm(forms.Form):
-    """
-    This form class is responsible for asking order-related questions. This includes
-    the attendee name for admission tickets, if the corresponding setting is enabled,
-    as well as additional questions defined by the organizer.
-    """
-
-    def __init__(self, *args, **kwargs):
-        """
-        Takes two additional keyword arguments:
-
-        :param cartpos: The cart position the form should be for
-        :param event: The event this belongs to
-        """
-        cartpos = kwargs.pop('cartpos', None)
-        orderpos = kwargs.pop('orderpos', None)
-        item = cartpos.item if cartpos else orderpos.item
-        questions = list(item.questions.all())
-        event = kwargs.pop('event')
-
-        super().__init__(*args, **kwargs)
-
-        if item.admission and event.settings.attendee_names_asked:
-            self.fields['attendee_name'] = forms.CharField(
-                max_length=255, required=event.settings.attendee_names_required,
-                label=_('Attendee name'),
-                initial=(cartpos.attendee_name if cartpos else orderpos.attendee_name)
-            )
-
-        for q in questions:
-            # Do we already have an answer? Provide it as the initial value
-            answers = [
-                a for a
-                in (cartpos.answers.all() if cartpos else orderpos.answers.all())
-                if a.question_id == q.identity
-            ]
-            if answers:
-                initial = answers[0].answer
-            else:
-                initial = None
-            if q.type == Question.TYPE_BOOLEAN:
-                field = forms.BooleanField(
-                    label=q.question, required=q.required,
-                    initial=initial
-                )
-            elif q.type == Question.TYPE_NUMBER:
-                field = forms.DecimalField(
-                    label=q.question, required=q.required,
-                    initial=initial
-                )
-            elif q.type == Question.TYPE_STRING:
-                field = forms.CharField(
-                    label=q.question, required=q.required,
-                    initial=initial
-                )
-            elif q.type == Question.TYPE_TEXT:
-                field = forms.CharField(
-                    label=q.question, required=q.required,
-                    widget=forms.Textarea,
-                    initial=initial
-                )
-            field.question = q
-            if answers:
-                # Cache the answer object for later use
-                field.answer = answers[0]
-            self.fields['question_%s' % q.identity] = field
-
-
 class CheckoutView(TemplateView):
-
     def get_payment_url(self):
         return reverse('presale:event.checkout.payment', kwargs={
             'event': self.request.event.slug,
@@ -120,7 +51,6 @@ class CheckoutView(TemplateView):
 
 
 class QuestionsViewMixin:
-
     @cached_property
     def forms(self):
         """
@@ -358,7 +288,8 @@ class OrderConfirm(EventViewMixin, CartDisplayMixin, EventLoginRequiredMixin, Ch
                         if not self.request.event.presale_end or now() < self.request.event.presale_end:
                             cp = cp.clone()
                             cartpos[i] = cp
-                            cp.expires = now() + timedelta(minutes=self.request.event.settings.get('reservation_time', as_type=int))
+                            cp.expires = now() + timedelta(
+                                minutes=self.request.event.settings.get('reservation_time', as_type=int))
                             cp.save()
                     else:
                         cp.delete()  # Sorry!

src/pretix/presale/views/event.py

@@ -1,23 +1,22 @@
 import json
+
 from django.contrib import messages
 from django.contrib.auth import authenticate, logout
 from django.core import signing
 from django.core.signing import SignatureExpired, BadSignature
 from django.core.urlresolvers import reverse
-from django.core.validators import RegexValidator
 from django.db.models import Count
-from django import forms
-from django.forms import Form
 from django.shortcuts import redirect
 from django.utils.functional import cached_property
-from django.contrib.auth.forms import AuthenticationForm as BaseAuthenticationForm
 from django.contrib.auth import login
 from django.views.generic import TemplateView, View
 from django.utils.translation import ugettext_lazy as _
 from django.conf import settings
 from pretix.base.mail import mail
 from pretix.base.models import User
-
+from pretix.presale.forms.auth import GlobalRegistrationForm, LocalRegistrationForm, PasswordForgotForm, \
+    PasswordRecoverForm
+from pretix.presale.forms.auth import LoginForm
 from pretix.presale.views import EventViewMixin, CartDisplayMixin, EventLoginRequiredMixin
 from pretix.presale.views.cart import CartAdd
 
@@ -59,162 +58,21 @@ class EventIndex(EventViewMixin, CartDisplayMixin, TemplateView):
         items = [item for item in items if len(item.available_variations) > 0]
 
         # Regroup those by category
-        context['items_by_category'] = sorted([
-            # a group is a tuple of a category and a list of items
-            (cat, [i for i in items if i.category == cat])
-            for cat in set([i.category for i in items])  # insert categories into a set for uniqueness
-            # a set is unsorted, so sort again by category
-        ], key=lambda group: (group[0].position, group[0].identity) if group[0] is not None else (0, ""))
+        context['items_by_category'] = sorted(
+            [
+                # a group is a tuple of a category and a list of items
+                (cat, [i for i in items if i.category == cat])
+                for cat in set([i.category for i in items])
+                # insert categories into a set for uniqueness
+                # a set is unsorted, so sort again by category
+            ],
+            key=lambda group: (group[0].position, group[0].identity) if group[0] is not None else (0, "")
+        )
 
         context['cart'] = self.get_cart() if self.request.user.is_authenticated() else None
         return context
 
 
-class LoginForm(BaseAuthenticationForm):
-    username = forms.CharField(
-        label=_('Username'),
-        help_text=(
-            _('If you registered for multiple events, your username is your email address.')
-            if settings.PRETIX_GLOBAL_REGISTRATION
-            else None
-        )
-    )
-    password = forms.CharField(
-        label=_('Password'),
-        widget=forms.PasswordInput
-    )
-
-    error_messages = {
-        'invalid_login': _("Please enter a correct username and password."),
-        'inactive': _("This account is inactive."),
-    }
-
-    def __init__(self, request=None, *args, **kwargs):
-        self.request = request
-        self.user_cache = None
-        super(forms.Form, self).__init__(*args, **kwargs)
-
-    def clean(self):
-        username = self.cleaned_data.get('username')
-        password = self.cleaned_data.get('password')
-
-        if username and password:
-            if '@' in username:
-                identifier = username.lower()
-            else:
-                identifier = "%s@%s.event.pretix" % (username, self.request.event.identity)
-            self.user_cache = authenticate(identifier=identifier,
-                                           password=password)
-            if self.user_cache is None:
-                raise forms.ValidationError(
-                    self.error_messages['invalid_login'],
-                    code='invalid_login',
-                )
-            else:
-                self.confirm_login_allowed(self.user_cache)
-
-        return self.cleaned_data
-
-
-class GlobalRegistrationForm(forms.Form):
-    error_messages = {
-        'duplicate_email': _("You already registered with that e-mail address, please use the login form."),
-        'pw_mismatch': _("Please enter the same password twice"),
-    }
-    email = forms.EmailField(
-        label=_('Email address'),
-        required=True
-    )
-    password = forms.CharField(
-        label=_('Password'),
-        widget=forms.PasswordInput,
-        required=True
-    )
-    password_repeat = forms.CharField(
-        label=_('Repeat password'),
-        widget=forms.PasswordInput
-    )
-
-    def clean(self):
-        password1 = self.cleaned_data.get('password')
-        password2 = self.cleaned_data.get('password_repeat')
-
-        if password1 and password1 != password2:
-            raise forms.ValidationError(
-                self.error_messages['pw_mismatch'],
-                code='pw_mismatch',
-            )
-
-        return self.cleaned_data
-
-    def clean_email(self):
-        email = self.cleaned_data['email']
-        if User.objects.filter(identifier=email).exists():
-            raise forms.ValidationError(
-                self.error_messages['duplicate_email'],
-                code='duplicate_email',
-            )
-        return email
-
-
-class LocalRegistrationForm(forms.Form):
-    error_messages = {
-        'invalid_username': _("Please only use characters, numbers or ./+/-/_ in your username."),
-        'duplicate_username': _("This username is already taken. Please choose a different one."),
-        'pw_mismatch': _("Please enter the same password twice"),
-    }
-    username = forms.CharField(
-        label=_('Username'),
-        validators=[
-            RegexValidator(
-                regex='^[a-zA-Z0-9\.+\-_]*$',
-                code='invalid_username',
-                message=error_messages['invalid_username']
-            ),
-        ],
-        required=True
-    )
-    email = forms.EmailField(
-        label=_('E-mail address'),
-        required=False
-    )
-    password = forms.CharField(
-        label=_('Password'),
-        widget=forms.PasswordInput,
-        required=True
-    )
-    password_repeat = forms.CharField(
-        label=_('Repeat password'),
-        widget=forms.PasswordInput
-    )
-
-    def __init__(self, request, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-        self.request = request
-        self.fields['email'].required = request.event.settings.user_mail_required
-
-    def clean(self):
-        password1 = self.cleaned_data.get('password')
-        password2 = self.cleaned_data.get('password_repeat')
-
-        if password1 and password1 != password2:
-            raise forms.ValidationError(
-                self.error_messages['pw_mismatch'],
-                code='pw_mismatch',
-            )
-
-        return self.cleaned_data
-
-    def clean_username(self):
-        username = self.cleaned_data['username']
-        if User.objects.filter(event=self.request.event, username=username).exists():
-            raise forms.ValidationError(
-                self.error_messages['duplicate_username'],
-                code='duplicate_username',
-            )
-        return username
-
-
 class EventLogin(EventViewMixin, TemplateView):
     template_name = 'pretixpresale/event/login.html'
 
@@ -301,73 +159,6 @@ class EventLogin(EventViewMixin, TemplateView):
         return context
 
 
-class PasswordRecoverForm(Form):
-    error_messages = {
-        'pw_mismatch': _("Please enter the same password twice"),
-    }
-    password = forms.CharField(
-        label=_('Password'),
-        widget=forms.PasswordInput,
-        required=True
-    )
-    password_repeat = forms.CharField(
-        label=_('Repeat password'),
-        widget=forms.PasswordInput
-    )
-
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-
-    def clean(self):
-        password1 = self.cleaned_data.get('password')
-        password2 = self.cleaned_data.get('password_repeat')
-
-        if password1 and password1 != password2:
-            raise forms.ValidationError(
-                self.error_messages['pw_mismatch'],
-                code='pw_mismatch',
-            )
-
-        return self.cleaned_data
-
-
-class PasswordForgotForm(Form):
-    username = forms.CharField(
-        label=_('Username or E-mail'),
-    )
-
-    def __init__(self, event, *args, **kwargs):
-        self.event = event
-        super().__init__(*args, **kwargs)
-
-    def clean_username(self):
-        username = self.cleaned_data['username']
-        try:
-            self.cleaned_data['user'] = User.objects.get(
-                identifier=username, event__isnull=True
-            )
-            return username
-        except User.DoesNotExist:
-            pass
-        try:
-            self.cleaned_data['user'] = User.objects.get(
-                username=username, event=self.event
-            )
-            return username
-        except User.DoesNotExist:
-            pass
-        try:
-            self.cleaned_data['user'] = User.objects.get(
-                email=username, event=self.event
-            )
-            return username
-        except:
-            raise forms.ValidationError(
-                _("We are unable to find a user matching the data you provided."),
-                code='unknown_user',
-            )
-
-
 class EventForgot(EventViewMixin, TemplateView):
     template_name = 'pretixpresale/event/forgot.html'
 
@@ -398,8 +189,8 @@ class EventForgot(EventViewMixin, TemplateView):
                         'url': settings.SITE_URL + reverse('presale:event.forgot.recover', kwargs={
                             'event': self.request.event.slug,
                             'organizer': self.request.event.organizer.slug,
-                            }) + '?token=' + self.generate_token(user),
-                        },
+                        }) + '?token=' + self.generate_token(user),
+                    },
                     self.request.event
                 )
                 messages.success(request, _('We sent you an e-mail containing further instructions.'))

src/pretix/presale/views/order.py

@@ -1,4 +1,3 @@
-from io import BytesIO
 from django.contrib import messages
 from django.core.urlresolvers import reverse
 from django.shortcuts import redirect
@@ -6,16 +5,14 @@ from django.utils.timezone import now
 from django.utils.translation import ugettext_lazy as _
 from django.utils.functional import cached_property
 from django.views.generic import TemplateView, View
-from django.http import HttpResponseNotFound, HttpResponseForbidden, HttpResponse
+from django.http import HttpResponseNotFound, HttpResponseForbidden
 from pretix.base.models import Order, OrderPosition
 from pretix.base.signals import register_payment_providers, register_ticket_outputs
 from pretix.presale.views import EventViewMixin, EventLoginRequiredMixin, CartDisplayMixin
 from pretix.presale.views.checkout import QuestionsViewMixin
-from django.contrib.staticfiles import finders
 
 
 class OrderDetailMixin:
-
     @cached_property
     def order(self):
         try:
@@ -163,7 +160,6 @@ class OrderCancel(EventViewMixin, EventLoginRequiredMixin, OrderDetailMixin,
 
 class OrderDownload(EventViewMixin, EventLoginRequiredMixin, OrderDetailMixin,
                     View):
-
     def get_order_url(self):
         return reverse('presale:event.order', kwargs={
             'event': self.request.event.slug,