Review notes

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

doc/api/resources/orders.rst

@@ -75,9 +75,8 @@ positions                             list of objects            List of order p
 fees                                  list of objects            List of fees included in the order total. By default, only
                                                                  non-canceled fees are included.
 ├ id                                  integer                    Internal ID of the fee record
-├ fee_type                            string                     Type of fee (currently ``payment``, ``shipping``,
+├ fee_type                            string                     Type of fee (currently ``payment``, ``passbook``,
-                                                                 ``service``, ``cancellation``, ``insurance``, ``late``,
+                                                                 ``other``)
-                                                                 ``other``, ``giftcard``)
 ├ value                               money (string)             Fee amount
 ├ description                         string                     Human-readable string with more details (can be empty)
 ├ internal_type                       string                     Internal string (i.e. ID of the payment provider),
@@ -2245,9 +2244,6 @@ otherwise, such as splitting an order or changing fees.
 
    * ``cancel_fees``: A list of objects with the single key ``fee`` specifying an order fee ID.
 
-   * ``create_fees``: A list of objects describing new order fees with the fields ``fee_type``, ``value``, ``description``,
-     ``internal_type``, ``tax_rule``
-
    * ``recalculate_taxes``: If set to ``"keep_net"``, all taxes will be recalculated based on the tax rule and invoice
      address, the net price will be kept. If set to ``"keep_gross"``, the gross price will be kept. If set to ``null``
      (the default) the taxes are not recalculated.
@@ -2267,12 +2263,17 @@ otherwise, such as splitting an order or changing fees.
       Content-Type: application/json
 
       {
+        "cancel_positions": [
+          {
+            "position": 12373
+          }
+        ],
         "patch_positions": [
           {
             "position": 12374,
             "body": {
               "item": 12,
-              "variation": null,
+              "variation": None,
               "subevent": 562,
               "seat": "seat-guid-2",
               "price": "99.99",
@@ -2280,11 +2281,6 @@ otherwise, such as splitting an order or changing fees.
             }
           }
         ],
-        "cancel_positions": [
-          {
-            "position": 12373
-          }
-        ],
         "split_positions": [
           {
             "position": 12375
@@ -2293,7 +2289,7 @@ otherwise, such as splitting an order or changing fees.
         "create_positions": [
           {
             "item": 12,
-            "variation": null,
+            "variation": None,
             "subevent": 562,
             "seat": "seat-guid-2",
             "price": "99.99",
@@ -2301,26 +2297,17 @@ otherwise, such as splitting an order or changing fees.
             "attendee_name": "Peter",
           }
         ],
-        "patch_fees": [
-          {
-            "fee": 51,
-            "body": {
-              "value": "12.00"
-            }
-          }
-        ],
         "cancel_fees": [
           {
             "fee": 49
           }
         ],
-        "create_fees": [
+        "change_fees": [
           {
-            "fee_type": "other",
+            "fee": 51,
-            "value": "1.50",
+            "body": {
-            "description": "Example Fee",
+              "value": "12.00"
-            "internal_type": "",
+            }
-            "tax_rule": 15
           }
         ],
         "reissue_invoice": true,

src/pretix/api/serializers/orderchange.py

@@ -30,7 +30,7 @@ from rest_framework.exceptions import ValidationError
 
 from pretix.api.serializers.order import (
     AnswerCreateSerializer, AnswerSerializer, CompatibleCountryField,
-    OrderFeeCreateSerializer, OrderPositionCreateSerializer,
+    OrderPositionCreateSerializer,
 )
 from pretix.base.models import ItemVariation, Order, OrderFee, OrderPosition
 from pretix.base.services.orders import OrderError
@@ -104,54 +104,6 @@ class OrderPositionCreateForExistingOrderSerializer(OrderPositionCreateSerialize
             raise ValidationError(str(e))
 
 
-class OrderFeeCreateForExistingOrderSerializer(OrderFeeCreateSerializer):
-    order = serializers.SlugRelatedField(slug_field='code', queryset=Order.objects.none(), required=True, allow_null=False)
-    value = serializers.DecimalField(required=True, allow_null=False, decimal_places=2,
-                                     max_digits=13)
-    internal_type = serializers.CharField(required=False, default="")
-
-    class Meta:
-        model = OrderFee
-        fields = ('order', 'fee_type', 'value', 'description', 'internal_type', 'tax_rule')
-
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-        if not self.context:
-            return
-        self.fields['order'].queryset = self.context['event'].orders.all()
-        self.fields['tax_rule'].queryset = self.context['event'].tax_rules.all()
-        if 'order' in self.context:
-            del self.fields['order']
-
-    def validate(self, data):
-        data = super().validate(data)
-        if 'order' in self.context:
-            data['order'] = self.context['order']
-        return data
-
-    def create(self, validated_data):
-        ocm = self.context['ocm']
-
-        try:
-            f = OrderFee(
-                order=validated_data['order'],
-                fee_type=validated_data['fee_type'],
-                value=validated_data.get('value'),
-                description=validated_data.get('description'),
-                internal_type=validated_data.get('internal_type'),
-                tax_rule=validated_data.get('tax_rule'),
-            )
-            f._calculate_tax()
-            ocm.add_fee(f)
-            if self.context.get('commit', True):
-                ocm.commit()
-                return validated_data['order'].fees.order_by('-pk').first()
-            else:
-                return OrderFee()  # fake to appease DRF
-        except OrderError as e:
-            raise ValidationError(str(e))
-
-
 class OrderPositionInfoPatchSerializer(serializers.ModelSerializer):
     answers = AnswerSerializer(many=True)
     country = CompatibleCountryField(source='*')
@@ -449,9 +401,6 @@ class OrderChangeOperationSerializer(serializers.Serializer):
         self.fields['split_positions'] = SelectPositionSerializer(
             many=True, required=False, context=self.context
         )
-        self.fields['create_fees'] = OrderFeeCreateForExistingOrderSerializer(
-            many=True, required=False, context=self.context
-        )
         self.fields['patch_fees'] = PatchFeeSerializer(
             many=True, required=False, context=self.context
         )

src/pretix/api/views/order.py

@@ -63,8 +63,7 @@ from pretix.api.serializers.order import (
 )
 from pretix.api.serializers.orderchange import (
     BlockNameSerializer, OrderChangeOperationSerializer,
-    OrderFeeChangeSerializer, OrderFeeCreateForExistingOrderSerializer,
+    OrderFeeChangeSerializer, OrderPositionChangeSerializer,
-    OrderPositionChangeSerializer,
     OrderPositionCreateForExistingOrderSerializer,
     OrderPositionInfoPatchSerializer,
 )
@@ -989,12 +988,6 @@ class EventOrderViewSet(OrderViewSetMixin, viewsets.ModelViewSet):
                 ocm.cancel_fee(r['fee'])
                 canceled_fees.add(r['fee'])
 
-            for r in serializer.validated_data.get('create_fees', []):
-                pos_serializer = OrderFeeCreateForExistingOrderSerializer(
-                    context={'ocm': ocm, 'commit': False, 'event': request.event, **self.get_serializer_context()},
-                )
-                pos_serializer.create(r)
-
             for r in serializer.validated_data.get('patch_fees', []):
                 if r['fee'] in canceled_fees:
                     continue

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:

src/tests/api/test_order_change.py

@@ -1797,13 +1797,6 @@ def test_order_change_cancel_and_create(token_client, organizer, event, order, q
                 'price': '99.99'
             },
         ],
-        'create_fees': [
-            {
-                'value': '5.99',
-                'fee_type': 'service',
-                'description': 'Service fee',
-            },
-        ],
         'cancel_fees': [
             {
                 'fee': f.pk,
@@ -1825,11 +1818,6 @@ def test_order_change_cancel_and_create(token_client, organizer, event, order, q
         assert p_new.price == Decimal('99.99')
         f.refresh_from_db()
         assert f.canceled
-        f_new = order.all_fees.get(fee_type=OrderFee.FEE_TYPE_SERVICE)
-        assert f_new.value == Decimal('5.99')
-        assert f_new.description == "Service fee"
-        assert f_new.internal_type == ""
-        assert f_new.tax_value == Decimal('0.00')
 
 
 @pytest.mark.django_db