Review notes

Co-authored-by: Richard Schreiber <schreiber@rami.io>

src/pretix/base/models/log.py

@@ -37,7 +37,7 @@ import logging
 
 from django.contrib.contenttypes.fields import GenericForeignKey
 from django.contrib.contenttypes.models import ContentType
-from django.db import models
+from django.db import connections, models
 from django.utils.functional import cached_property
 
 from pretix.base.logentrytype_registry import log_entry_types, make_link
@@ -165,6 +165,15 @@ class LogEntry(models.Model):
     def delete(self, using=None, keep_parents=False):
         raise TypeError("Logs cannot be deleted.")
 
+    @classmethod
+    def bulk_create_and_postprocess(cls, objects):
+        if connections['default'].features.can_return_rows_from_bulk_insert:
+            cls.objects.bulk_create(objects)
+        else:
+            for le in objects:
+                le.save()
+        cls.bulk_postprocess(objects)
+
     @classmethod
     def bulk_postprocess(cls, objects):
         from pretix.api.webhooks import notify_webhooks

src/pretix/control/views/organizer.py

@@ -46,7 +46,7 @@ from django.conf import settings
 from django.contrib import messages
 from django.core.exceptions import PermissionDenied, ValidationError
 from django.core.files import File
-from django.db import connections, transaction
+from django.db import transaction
 from django.db.models import (
     Count, Exists, F, IntegerField, Max, Min, OuterRef, Prefetch,
     ProtectedError, Q, Subquery, Sum,
@@ -1159,13 +1159,7 @@ class DeviceBulkUpdateView(DeviceQueryMixin, OrganizerDetailViewMixin, Organizer
                 obj.log_action('pretix.device.changed', data=data, user=self.request.user, save=False)
             )
 
-        if connections['default'].features.can_return_rows_from_bulk_insert:
+        LogEntry.bulk_create_and_postprocess(log_entries)
-            LogEntry.objects.bulk_create(log_entries, batch_size=200)
-            LogEntry.bulk_postprocess(log_entries)
-        else:
-            for le in log_entries:
-                le.save()
-            LogEntry.bulk_postprocess(log_entries)
 
         messages.success(self.request, _('Your changes have been saved.'))
         return super().form_valid(form)

src/pretix/control/views/subevents.py

@@ -40,7 +40,7 @@ from dateutil.rrule import rruleset
 from django.contrib import messages
 from django.core.exceptions import ValidationError
 from django.core.files import File
-from django.db import connections, transaction
+from django.db import transaction
 from django.db.models import Count, F, Prefetch, ProtectedError
 from django.db.models.functions import Coalesce, TruncDate, TruncTime
 from django.forms import inlineformset_factory
@@ -657,24 +657,30 @@ class SubEventBulkAction(SubEventQueryMixin, EventPermissionRequiredMixin, View)
     @transaction.atomic
     def post(self, request, *args, **kwargs):
         if request.POST.get('action') == 'disable':
+            log_entries = []
             for obj in self.get_queryset():
-                obj.log_action(
+                log_entries.append(obj.log_action(
                     'pretix.subevent.changed', user=self.request.user, data={
                         'active': False
-                    }
+                    }, save=False
-                )
+                ))
                 obj.active = False
                 obj.save(update_fields=['active'])
+
+            LogEntry.bulk_create_and_postprocess(log_entries)
             messages.success(request, pgettext_lazy('subevent', 'The selected dates have been disabled.'))
         elif request.POST.get('action') == 'enable':
+            log_entries = []
             for obj in self.get_queryset():
-                obj.log_action(
+                log_entries.append(obj.log_action(
                     'pretix.subevent.changed', user=self.request.user, data={
                         'active': True
-                    }
+                    }, save=False
-                )
+                ))
                 obj.active = True
                 obj.save(update_fields=['active'])
+
+            LogEntry.bulk_create_and_postprocess(log_entries)
             messages.success(request, pgettext_lazy('subevent', 'The selected dates have been enabled.'))
         elif request.POST.get('action') == 'delete':
             return render(request, 'pretixcontrol/subevents/delete_bulk.html', {
@@ -682,22 +688,28 @@ class SubEventBulkAction(SubEventQueryMixin, EventPermissionRequiredMixin, View)
                 'forbidden': self.get_queryset().filter(orderposition__isnull=False).distinct(),
             })
         elif request.POST.get('action') == 'delete_confirm':
+            log_entries = []
+            to_delete = []
             for obj in self.get_queryset():
                 try:
                     if not obj.allow_delete():
                         raise ProtectedError('only deactivate', [obj])
-                    CartPosition.objects.filter(addon_to__subevent=obj).delete()
+                    log_entries.append(obj.log_action('pretix.subevent.deleted', user=self.request.user, save=False))
-                    obj.cartposition_set.all().delete()
+                    to_delete.append(obj.pk)
-                    obj.log_action('pretix.subevent.deleted', user=self.request.user)
-                    obj.delete()
                 except ProtectedError:
-                    obj.log_action(
+                    log_entries.append(obj.log_action(
                         'pretix.subevent.changed', user=self.request.user, data={
                             'active': False
-                        }
+                        }, save=False,
-                    )
+                    ))
                     obj.active = False
                     obj.save(update_fields=['active'])
+
+            if to_delete:
+                CartPosition.objects.filter(addon_to__subevent_id__in=to_delete).delete()
+                CartPosition.objects.filter(subevent_id__in=to_delete).delete()
+                SubEvent.objects.filter(pk__in=to_delete).delete()
+            LogEntry.bulk_create_and_postprocess(log_entries)
             messages.success(request, pgettext_lazy('subevent', 'The selected dates have been deleted or disabled.'))
         return redirect(self.get_success_url())
 
@@ -1009,13 +1021,7 @@ class SubEventBulkCreate(SubEventEditorMixin, EventPermissionRequiredMixin, Asyn
                 f.save()
         set_progress(90)
 
-        if connections['default'].features.can_return_rows_from_bulk_insert:
+        LogEntry.bulk_create_and_postprocess(log_entries)
-            LogEntry.objects.bulk_create(log_entries)
-            LogEntry.bulk_postprocess(log_entries)
-        else:
-            for le in log_entries:
-                le.save()
-            LogEntry.bulk_postprocess(log_entries)
 
         self.request.event.cache.clear()
         return len(subevents)
@@ -1578,13 +1584,7 @@ class SubEventBulkEdit(SubEventQueryMixin, EventPermissionRequiredMixin, FormVie
         self.save_itemvars()
         self.save_meta()
 
-        if connections['default'].features.can_return_rows_from_bulk_insert:
+        LogEntry.bulk_create_and_postprocess(log_entries)
-            LogEntry.objects.bulk_create(log_entries, batch_size=200)
-            LogEntry.bulk_postprocess(log_entries)
-        else:
-            for le in log_entries:
-                le.save()
-            LogEntry.bulk_postprocess(log_entries)
 
         self.request.event.cache.clear()
         messages.success(self.request, _('Your changes have been saved.'))

src/pretix/control/views/vouchers.py

@@ -477,7 +477,7 @@ class VoucherBulkCreate(EventPermissionRequiredMixin, AsyncFormView):
                 log_entries.append(
                     v.log_action('pretix.voucher.added', data=data, user=self.request.user, save=False)
                 )
-            LogEntry.objects.bulk_create(log_entries)
+            LogEntry.bulk_create_and_postprocess(log_entries)
             form.post_bulk_save(batch_vouchers)
             batch_vouchers.clear()
             set_progress(len(voucherids) / total_num * (50. if form.cleaned_data['send'] else 100.))
@@ -619,19 +619,26 @@ class VoucherBulkAction(EventPermissionRequiredMixin, View):
                 'forbidden': self.objects.exclude(redeemed=0),
             })
         elif request.POST.get('action') == 'delete_confirm':
+            log_entries = []
+            to_delete = []
             for obj in self.objects:
                 if obj.allow_delete():
-                    obj.log_action('pretix.voucher.deleted', user=self.request.user)
+                    log_entries.append(obj.log_action('pretix.voucher.deleted', user=self.request.user, save=False))
-                    CartPosition.objects.filter(addon_to__voucher=obj).delete()
+                    to_delete.append(obj.pk)
-                    obj.cartposition_set.all().delete()
-                    obj.delete()
                 else:
-                    obj.log_action('pretix.voucher.changed', user=self.request.user, data={
+                    log_entries.append(obj.log_action('pretix.voucher.changed', user=self.request.user, data={
                         'max_usages': min(obj.redeemed, obj.max_usages),
                         'bulk': True
-                    })
+                    }), save=False)
                     obj.max_usages = min(obj.redeemed, obj.max_usages)
                     obj.save(update_fields=['max_usages'])
+
+            if to_delete:
+                CartPosition.objects.filter(addon_to__voucher_id__in=to_delete).delete()
+                CartPosition.objects.filter(voucher_id__in=to_delete).delete()
+                Voucher.objects.filter(pk__in=to_delete).delete()
+
+            LogEntry.bulk_create_and_postprocess(log_entries)
             messages.success(request, _('The selected vouchers have been deleted or disabled.'))
         return redirect(self.get_success_url())
 

src/pretix/control/views/waitinglist.py

@@ -49,7 +49,7 @@ from django.utils.translation import gettext_lazy as _, pgettext
 from django.views import View
 from django.views.generic import ListView
 
-from pretix.base.models import Item, Quota, WaitingListEntry
+from pretix.base.models import Item, LogEntry, Quota, WaitingListEntry
 from pretix.base.models.waitinglist import WaitingListException
 from pretix.base.services.waitinglist import assign_automatically
 from pretix.base.views.tasks import AsyncAction
@@ -160,10 +160,15 @@ class WaitingListActionView(EventPermissionRequiredMixin, WaitingListQuerySetMix
                 'forbidden': self.get_queryset().filter(voucher__isnull=False),
             })
         elif request.POST.get('action') == 'delete_confirm':
-            for obj in self.get_queryset(force_filtered=True):
+            with transaction.atomic():
-                if not obj.voucher_id:
+                log_entries = []
-                    obj.log_action('pretix.event.orders.waitinglist.deleted', user=self.request.user)
+                to_delete = []
-                    obj.delete()
+                for obj in self.get_queryset(force_filtered=True):
+                    if not obj.voucher_id:
+                        log_entries.append(obj.log_action('pretix.event.orders.waitinglist.deleted', user=self.request.user, save=False))
+                        to_delete.append(obj.pk)
+                WaitingListEntry.objects.filter(id__in=to_delete).delete()
+                LogEntry.bulk_create_and_postprocess(log_entries)
             messages.success(request, _('The selected entries have been deleted.'))
             return self._redirect_back()
 
@@ -186,16 +191,17 @@ class WaitingListActionView(EventPermissionRequiredMixin, WaitingListQuerySetMix
 
         if 'move_top' in request.POST:
             try:
-                wle = WaitingListEntry.objects.get(
+                with transaction.atomic():
-                    pk=request.POST.get('move_top'), event=self.request.event,
+                    wle = WaitingListEntry.objects.get(
-                )
+                        pk=request.POST.get('move_top'), event=self.request.event,
-                wle.priority = self.request.event.waitinglistentries.aggregate(m=Max('priority'))['m'] + 1
+                    )
-                wle.save(update_fields=['priority'])
+                    wle.priority = self.request.event.waitinglistentries.aggregate(m=Max('priority'))['m'] + 1
-                wle.log_action(
+                    wle.save(update_fields=['priority'])
-                    'pretix.event.orders.waitinglist.changed',
+                    wle.log_action(
-                    data={'priority': wle.priority},
+                        'pretix.event.orders.waitinglist.changed',
-                    user=self.request.user,
+                        data={'priority': wle.priority},
-                )
+                        user=self.request.user,
+                    )
                 messages.success(request, _('The waiting list entry has been moved to the top.'))
                 return self._redirect_back()
             except WaitingListEntry.DoesNotExist:
@@ -204,16 +210,17 @@ class WaitingListActionView(EventPermissionRequiredMixin, WaitingListQuerySetMix
 
         if 'move_end' in request.POST:
             try:
-                wle = WaitingListEntry.objects.get(
+                with transaction.atomic():
-                    pk=request.POST.get('move_end'), event=self.request.event,
+                    wle = WaitingListEntry.objects.get(
-                )
+                        pk=request.POST.get('move_end'), event=self.request.event,
-                wle.priority = self.request.event.waitinglistentries.aggregate(m=Min('priority'))['m'] - 1
+                    )
-                wle.save(update_fields=['priority'])
+                    wle.priority = self.request.event.waitinglistentries.aggregate(m=Min('priority'))['m'] - 1
-                wle.log_action(
+                    wle.save(update_fields=['priority'])
-                    'pretix.event.orders.waitinglist.changed',
+                    wle.log_action(
-                    data={'priority': wle.priority},
+                        'pretix.event.orders.waitinglist.changed',
-                    user=self.request.user,
+                        data={'priority': wle.priority},
-                )
+                        user=self.request.user,
+                    )
                 messages.success(request, _('The waiting list entry has been moved to the end of the list.'))
                 return self._redirect_back()
             except WaitingListEntry.DoesNotExist: