Add upper limit on positions in an order (#3806)

* Add upper limit on positions in an order

* Fix form validation

src/pretix/_base_settings.py

@@ -279,3 +279,5 @@
     ".bmp", ".tif", ".tiff"
 )
 FILE_UPLOAD_EXTENSIONS_OTHER = FILE_UPLOAD_EXTENSIONS_EMAIL_ATTACHMENT
+
+PRETIX_MAX_ORDER_SIZE = 500

src/pretix/api/serializers/order.py

@@ -1077,6 +1077,10 @@ def validate_positions(self, data):
             raise ValidationError(
                 'An order cannot be empty.'
             )
+        if len(data) > settings.PRETIX_MAX_ORDER_SIZE:
+            raise ValidationError(
+                'Orders cannot have more than %(max)s positions.' % {'max': settings.PRETIX_MAX_ORDER_SIZE}
+            )
         errs = [{} for p in data]
         if any([p.get('positionid') for p in data]):
             if not all([p.get('positionid') for p in data]):

src/pretix/base/services/cart.py

@@ -41,6 +41,7 @@
 
 from celery.exceptions import MaxRetriesExceededError
 from django import forms
+from django.conf import settings
 from django.core.exceptions import ValidationError
 from django.db import DatabaseError, transaction
 from django.db.models import Count, Exists, IntegerField, OuterRef, Q, Value
@@ -378,8 +379,9 @@ def _check_max_cart_size(self):
             cartsize += sum([op.count for op in self._operations if isinstance(op, self.AddOperation) and not op.addon_to])
             cartsize -= len([1 for op in self._operations if isinstance(op, self.RemoveOperation) if
                              not op.position.addon_to_id])
-            if cartsize > int(self.event.settings.max_items_per_order):
-                raise CartError(error_messages['max_items'] % self.event.settings.max_items_per_order)
+            limit = min(int(self.event.settings.max_items_per_order), settings.PRETIX_MAX_ORDER_SIZE)
+            if cartsize > limit:
+                raise CartError(error_messages['max_items'] % limit)
 
     def _check_item_constraints(self, op, current_ops=[]):
         if isinstance(op, (self.AddOperation, self.ExtendOperation)):

src/pretix/base/services/orderimport.py

@@ -23,6 +23,7 @@
 import io
 from decimal import Decimal
 
+from django.conf import settings as django_settings
 from django.core.exceptions import ValidationError
 from django.db import transaction
 from django.utils.timezone import now
@@ -124,6 +125,11 @@ def import_orders(event: Event, fileid: str, settings: dict, locale: str, user,
                     )
             data.append(values)
 
+        if settings['orders'] == 'one' and len(data) > django_settings.PRETIX_MAX_ORDER_SIZE:
+            raise DataImportError(
+                _('Orders cannot have more than %(max)s positions.') % {'max': django_settings.PRETIX_MAX_ORDER_SIZE}
+            )
+
         # Prepare model objects. Yes, this might consume lots of RAM, but allows us to make the actual SQL transaction
         # shorter. We'll see what works better in reality…
         lock_seats = []

src/pretix/base/services/orders.py

@@ -1512,6 +1512,7 @@ class OrderChangeManager:
             "You need to select at least %(min)s items of the product %(product)s.",
             "min"
         ),
+        'max_order_size': gettext_lazy('Orders cannot have more than %(max)s positions.'),
     }
     ItemOperation = namedtuple('ItemOperation', ('position', 'item', 'variation'))
     SubeventOperation = namedtuple('SubeventOperation', ('position', 'subevent'))
@@ -2599,6 +2600,14 @@ def _recalculate_total_and_payment_fee(self):
         self.order.total = total + payment_fee
         self.order.save()
 
+    def _check_order_size(self):
+        if (len(self.order.positions.all()) + len([op for op in self._operations if isinstance(op, self.AddOperation)])) > settings.PRETIX_MAX_ORDER_SIZE:
+            raise OrderError(
+                self.error_messages['max_order_size'] % {
+                    'max': settings.PRETIX_MAX_ORDER_SIZE,
+                }
+            )
+
     def _payment_fee_diff(self):
         total = self.order.total + self._totaldiff
         if self.open_payment:
@@ -2739,6 +2748,7 @@ def commit(self, check_quotas=True):
 
         # finally, incorporate difference in payment fees
         self._payment_fee_diff()
+        self._check_order_size()
 
         with transaction.atomic():
             locked_instance = Order.objects.select_for_update(of=OF_SELF).get(pk=self.order.pk)

src/pretix/base/settings.py

@@ -306,9 +306,11 @@ def unserialize(cls, s):
         'serializer_class': serializers.IntegerField,
         'serializer_kwargs': dict(
             min_value=1,
+            max_value=settings.PRETIX_MAX_ORDER_SIZE,
         ),
         'form_kwargs': dict(
             min_value=1,
+            max_value=settings.PRETIX_MAX_ORDER_SIZE,
             required=True,
             label=_("Maximum number of items per order"),
             help_text=_("Add-on products will not be counted.")

src/tests/api/test_order_create.py

@@ -26,6 +26,7 @@
 from unittest import mock
 
 import pytest
+from django.conf import settings
 from django.core import mail as djmail
 from django.core.files.base import ContentFile
 from django.utils.timezone import now
@@ -278,6 +279,31 @@ def test_order_create(token_client, organizer, event, item, quota, question):
         assert o.transactions.count() == 2
 
 
+@pytest.mark.django_db
+def test_order_create_max_size(token_client, organizer, event, item, quota, question):
+    quota.size = settings.PRETIX_MAX_ORDER_SIZE * 2
+    quota.save()
+    res = copy.deepcopy(ORDER_CREATE_PAYLOAD)
+    res['positions'] = [
+        {
+            "item": item.pk,
+            "variation": None,
+            "price": "23.00",
+            "attendee_name_parts": {"full_name": "Peter"},
+            "attendee_email": None,
+            "addon_to": None,
+            "subevent": None
+        }
+    ] * (settings.PRETIX_MAX_ORDER_SIZE + 1)
+    resp = token_client.post(
+        '/api/v1/organizers/{}/events/{}/orders/'.format(
+            organizer.slug, event.slug
+        ), format='json', data=res
+    )
+    assert resp.status_code == 400
+    assert resp.data == {"positions": [f"Orders cannot have more than {settings.PRETIX_MAX_ORDER_SIZE} positions."]}
+
+
 @pytest.mark.django_db
 def test_order_create_expires(token_client, organizer, event, item, quota, question):
     res = copy.deepcopy(ORDER_CREATE_PAYLOAD)

src/tests/base/test_orderimport.py

@@ -25,6 +25,7 @@
 from io import StringIO
 
 import pytest
+from django.conf import settings as django_settings
 from django.core.files.base import ContentFile
 from django.utils.timezone import now
 from django_scopes import scopes_disabled
@@ -57,7 +58,7 @@ def user():
     return User.objects.create_user('test@localhost', 'test')
 
 
-def inputfile_factory():
+def inputfile_factory(multiplier=1):
     d = [
         {
             'A': 'Dieter',
@@ -103,6 +104,8 @@ def inputfile_factory():
             'L': '',
         },
     ]
+    if multiplier > 1:
+        d = d * multiplier
     f = StringIO()
     w = csv.DictWriter(f, ['A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L'], dialect=csv.excel)
     w.writeheader()
@@ -166,6 +169,19 @@ def test_import_as_one_order(user, event, item):
     assert set(pos.positionid for pos in o.positions.all()) == {1, 2, 3}
 
 
+@pytest.mark.django_db
+@scopes_disabled()
+def test_import_as_one_order_max_size(user, event, item):
+    settings = dict(DEFAULT_SETTINGS)
+    settings['item'] = 'static:{}'.format(item.pk)
+    settings['orders'] = 'one'
+
+    import_orders.apply(
+        args=(event.pk, inputfile_factory(multiplier=django_settings.PRETIX_MAX_ORDER_SIZE).id, settings, 'en', user.pk)
+    )
+    assert event.orders.count() == 0
+
+
 @pytest.mark.django_db
 @scopes_disabled()
 def test_import_in_test_mode(user, event, item):

src/tests/base/test_orders.py

@@ -26,6 +26,7 @@
 from zoneinfo import ZoneInfo
 
 import pytest
+from django.conf import settings
 from django.core import mail as djmail
 from django.db.models import F, Sum
 from django.test import TestCase
@@ -1829,6 +1830,13 @@ def test_add_item_quota_required(self):
         with self.assertRaises(OrderError):
             self.ocm.add_position(self.shirt, None, None, None)
 
+    @classscope(attr='o')
+    def test_add_item_limit(self):
+        for i in range(settings.PRETIX_MAX_ORDER_SIZE):
+            self.ocm.add_position(self.shirt, None, None, None)
+        with self.assertRaises(OrderError):
+            self.ocm.commit()
+
     @classscope(attr='o')
     def test_add_item_success(self):
         self.ocm.add_position(self.shirt, None, None, None)

src/tests/presale/test_cart.py

@@ -38,6 +38,7 @@
 from decimal import Decimal
 
 from bs4 import BeautifulSoup
+from django.conf import settings
 from django.test import TestCase
 from django.utils.timezone import now
 from django_countries.fields import Country
@@ -991,6 +992,23 @@ def test_max_items(self):
         with scopes_disabled():
             self.assertEqual(CartPosition.objects.filter(cart_id=self.session_key, event=self.event).count(), 1)
 
+    def test_max_items_global(self):
+        with scopes_disabled():
+            CartPosition.objects.create(
+                event=self.event, cart_id=self.session_key, item=self.ticket,
+                price=23, expires=now() + timedelta(minutes=10)
+            )
+        self.event.settings.max_items_per_order = settings.PRETIX_MAX_ORDER_SIZE + 100
+        response = self.client.post('/%s/%s/cart/add' % (self.orga.slug, self.event.slug), {
+            'item_%d' % self.ticket.id: str(settings.PRETIX_MAX_ORDER_SIZE + 1),
+        }, follow=True)
+        self.assertRedirects(response, '/%s/%s/?require_cookie=true' % (self.orga.slug, self.event.slug),
+                             target_status_code=200)
+        doc = BeautifulSoup(response.rendered_content, "lxml")
+        self.assertIn('more than', doc.select('.alert-danger')[0].text)
+        with scopes_disabled():
+            self.assertEqual(CartPosition.objects.filter(cart_id=self.session_key, event=self.event).count(), 1)
+
     def test_max_items_unlimited_sales_channel(self):
         with scopes_disabled():
             CartPosition.objects.create(