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 <schreiber@pretix.eu>

---------

Co-authored-by: Richard Schreiber <schreiber@pretix.eu>

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

src/pretix/presale/templates/pretixpresale/event/fragment_addon_choice.html

@@ -372,6 +372,19 @@
                 </article>
             {% endif %}
         {% endfor %}
+
+        {% if c.items_missing %}
+            <div class="product-appendix-row">
+                <p class="text-muted">
+                    {% trans "There are products selected in this add-on category that currently cannot be changed because they are not on sale:" %}
+                </p>
+                <ul class="text-muted">
+                    {% for itemvar, count in c.items_missing.items %}
+                        <li>{{ count }}x {{ itemvar.0 }}{% if itemvar.1 %} – {{ itemvar.1 }}{% endif %}</li>
+                    {% endfor %}
+                </ul>
+            </div>
+        {% endif %}
     </fieldset>
     {% endwith %}
 {% empty %}

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

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;

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 '<li>1x Workshop 1</li>' 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 '<li>1x Workshop 1</li>' 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