From 26fdcc287221090de9474ccb347ab61f5cc00f14 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Thu, 29 Jan 2026 20:42:43 +0100 Subject: [PATCH] Order changes: Do not allow to double-book add-ons (Z#23220592) (#5851) * Order changes: Do not allow to double-book add-ons * tests * Update src/pretix/presale/templates/pretixpresale/event/fragment_addon_choice.html Co-authored-by: Richard Schreiber --------- Co-authored-by: Richard Schreiber --- src/pretix/base/services/orders.py | 72 ++++++++++--------- .../event/fragment_addon_choice.html | 13 ++++ src/pretix/presale/views/order.py | 9 ++- .../static/pretixpresale/scss/_event.scss | 9 +++ src/tests/presale/test_order_change.py | 36 +++++++++- 5 files changed, 101 insertions(+), 38 deletions(-) diff --git a/src/pretix/base/services/orders.py b/src/pretix/base/services/orders.py index 365247483..5d1d61153 100644 --- a/src/pretix/base/services/orders.py +++ b/src/pretix/base/services/orders.py @@ -2094,6 +2094,43 @@ class OrderChangeManager: ) item_counts[item] += 1 + # Detect removed add-ons and create RemoveOperations + for cp, al in list(current_addons.items()): + for k, v in al.items(): + input_num = input_addons[cp.id].get(k, 0) + current_num = len(current_addons[cp].get(k, [])) + if input_num < current_num: + for a in current_addons[cp][k][:current_num - input_num]: + if a.canceled: + continue + is_unavailable = ( + # If an item is no longer available due to time, it should usually also be no longer + # user-removable, because e.g. the stock has already been ordered. + # We always pass has_voucher=True because if a product now requires a voucher, it usually does + # not mean it should be unremovable for others. + # This also prevents accidental removal through the UI because a hidden product will no longer + # be part of the input. + (a.variation and a.variation.unavailability_reason(has_voucher=True, subevent=a.subevent)) + or (a.variation and not a.variation.all_sales_channels and not a.variation.limit_sales_channels.contains(self.order.sales_channel)) + or a.item.unavailability_reason(has_voucher=True, subevent=a.subevent) + or ( + not a.item.all_sales_channels and + not a.item.limit_sales_channels.contains(self.order.sales_channel) + ) + ) + if is_unavailable: + # "Re-select" add-on + selected_addons[cp.id, a.item.category_id][a.item_id, a.variation_id] += 1 + continue + if a.checkins.filter(list__consider_tickets_used=True).exists(): + raise OrderError( + error_messages['addon_already_checked_in'] % { + 'addon': str(a.item.name), + } + ) + self.cancel(a) + item_counts[a.item] -= 1 + # Check constraints on the add-on combinations for op in toplevel_op: item = op.item @@ -2126,41 +2163,6 @@ class OrderChangeManager: } ) - # Detect removed add-ons and create RemoveOperations - for cp, al in list(current_addons.items()): - for k, v in al.items(): - input_num = input_addons[cp.id].get(k, 0) - current_num = len(current_addons[cp].get(k, [])) - if input_num < current_num: - for a in current_addons[cp][k][:current_num - input_num]: - if a.canceled: - continue - is_unavailable = ( - # If an item is no longer available due to time, it should usually also be no longer - # user-removable, because e.g. the stock has already been ordered. - # We always pass has_voucher=True because if a product now requires a voucher, it usually does - # not mean it should be unremovable for others. - # This also prevents accidental removal through the UI because a hidden product will no longer - # be part of the input. - (a.variation and a.variation.unavailability_reason(has_voucher=True, subevent=a.subevent)) - or (a.variation and not a.variation.all_sales_channels and not a.variation.limit_sales_channels.contains(self.order.sales_channel)) - or a.item.unavailability_reason(has_voucher=True, subevent=a.subevent) - or ( - not item.all_sales_channels and - not item.limit_sales_channels.contains(self.order.sales_channel) - ) - ) - if is_unavailable: - continue - if a.checkins.filter(list__consider_tickets_used=True).exists(): - raise OrderError( - error_messages['addon_already_checked_in'] % { - 'addon': str(a.item.name), - } - ) - self.cancel(a) - item_counts[a.item] -= 1 - for item, count in item_counts.items(): if count == 0: continue diff --git a/src/pretix/presale/templates/pretixpresale/event/fragment_addon_choice.html b/src/pretix/presale/templates/pretixpresale/event/fragment_addon_choice.html index db31fb5ca..f0ca9166d 100644 --- a/src/pretix/presale/templates/pretixpresale/event/fragment_addon_choice.html +++ b/src/pretix/presale/templates/pretixpresale/event/fragment_addon_choice.html @@ -372,6 +372,19 @@ {% endif %} {% endfor %} + + {% if c.items_missing %} +
+

+ {% trans "There are products selected in this add-on category that currently cannot be changed because they are not on sale:" %} +

+
    + {% for itemvar, count in c.items_missing.items %} +
  • {{ count }}x {{ itemvar.0 }}{% if itemvar.1 %} – {{ itemvar.1 }}{% endif %}
    • + {% endfor %} +
+
+ {% endif %} {% endwith %} {% empty %} diff --git a/src/pretix/presale/views/order.py b/src/pretix/presale/views/order.py index 685b78506..ef57ae47e 100644 --- a/src/pretix/presale/views/order.py +++ b/src/pretix/presale/views/order.py @@ -40,7 +40,7 @@ import logging import mimetypes import os import re -from collections import OrderedDict, defaultdict +from collections import Counter, OrderedDict, defaultdict from decimal import Decimal from urllib.parse import quote @@ -1431,11 +1431,13 @@ class OrderChangeMixin: 'categories': [] } current_addon_products = defaultdict(list) + current_addon_products_missing = Counter() for a in p.addons.all(): if a.canceled: continue if not a.is_bundled: current_addon_products[a.item_id, a.variation_id].append(a) + current_addon_products_missing[a.item, a.variation] += 1 for iao in p.item.addons.all(): ckey = '{}-{}'.format(p.subevent.pk if p.subevent else 0, iao.addon_category.pk) @@ -1473,6 +1475,7 @@ class OrderChangeMixin: if i.has_variations: for v in i.available_variations: v.initial = len(current_addon_products[i.pk, v.pk]) + current_addon_products_missing[i, v] = 0 if v.initial and i.free_price: a = current_addon_products[i.pk, v.pk][0] v.initial_price = TaxedPrice( @@ -1488,6 +1491,7 @@ class OrderChangeMixin: i.expand = any(v.initial for v in i.available_variations) else: i.initial = len(current_addon_products[i.pk, None]) + current_addon_products_missing[i, None] = 0 if i.initial and i.free_price: a = current_addon_products[i.pk, None][0] i.initial_price = TaxedPrice( @@ -1509,7 +1513,8 @@ class OrderChangeMixin: 'min_count': iao.min_count, 'max_count': iao.max_count, 'iao': iao, - 'items': [i for i in items if not i.require_voucher] + 'items': [i for i in items if not i.require_voucher], + 'items_missing': {k: v for k, v in current_addon_products_missing.items() if v}, }) return positions diff --git a/src/pretix/static/pretixpresale/scss/_event.scss b/src/pretix/static/pretixpresale/scss/_event.scss index 4fe7404b3..5e0279447 100644 --- a/src/pretix/static/pretixpresale/scss/_event.scss +++ b/src/pretix/static/pretixpresale/scss/_event.scss @@ -99,6 +99,15 @@ } } +.product-appendix-row { + border-top: 1px solid $table-border-color; + border-bottom: 1px solid $table-border-color; + padding: 1.25*$line-height-computed 0; + & > :last-child { + margin-bottom: 0; + } +} + article.item-with-variations { margin: 0 -15px; padding: 0 15px; diff --git a/src/tests/presale/test_order_change.py b/src/tests/presale/test_order_change.py index 0bcafeb4e..24c110c38 100644 --- a/src/tests/presale/test_order_change.py +++ b/src/tests/presale/test_order_change.py @@ -1015,7 +1015,8 @@ class OrderChangeAddonsTest(BaseOrdersTest): '/%s/%s/order/%s/%s/change' % (self.orga.slug, self.event.slug, self.order.code, self.order.secret) ) assert response.status_code == 200 - assert 'Workshop 1' not in response.content.decode() + assert '
  • 1x Workshop 1
    • ' in response.content.decode() + assert f'cp_{self.ticket_pos.pk}_item_{self.workshop1.pk}' not in response.content.decode() response = self.client.post( '/%s/%s/order/%s/%s/change' % (self.orga.slug, self.event.slug, self.order.code, self.order.secret), @@ -1035,6 +1036,39 @@ class OrderChangeAddonsTest(BaseOrdersTest): with scopes_disabled(): assert self.ticket_pos.addons.count() == 2 + def test_do_not_overbook_unavailable_on_adding(self): + self.iao.max_count = 1 + self.iao.save() + self.workshop1.available_until = now() - datetime.timedelta(days=1) + self.workshop1.save() + with scopes_disabled(): + OrderPosition.objects.create( + order=self.order, + item=self.workshop1, + variation=None, + price=Decimal("12"), + addon_to=self.ticket_pos, + attendee_name_parts={'full_name': "Peter"} + ) + self.order.total += Decimal("12") + self.order.save() + + response = self.client.get( + '/%s/%s/order/%s/%s/change' % (self.orga.slug, self.event.slug, self.order.code, self.order.secret) + ) + assert response.status_code == 200 + assert '
  • 1x Workshop 1
    • ' in response.content.decode() + assert f'cp_{self.ticket_pos.pk}_item_{self.workshop1.pk}' not in response.content.decode() + + response = self.client.post( + '/%s/%s/order/%s/%s/change' % (self.orga.slug, self.event.slug, self.order.code, self.order.secret), + { + f'cp_{self.ticket_pos.pk}_variation_{self.workshop2.pk}_{self.workshop2a.pk}': '1' + }, + follow=True + ) + assert 'alert-danger' in response.content.decode() + def test_remove_addon_checked_in(self): with scopes_disabled(): self.event.settings.change_allow_user_if_checked_in = True