Add unit test

src/pretix/base/invoice.py

@@ -62,6 +62,63 @@ from pretix.presale.style import get_fonts
 logger = logging.getLogger(__name__)
 
 
+def addon_aware_groupby(iterable, key, is_addon):
+    """
+    We use groupby() to visually group identical lines on an invoice. For example, instead of
+
+    Product 1       5.00 EUR
+    Product 1       5.00 EUR
+    Product 1       5.00 EUR
+    Product 2       7.00 EUR
+
+    We want to print
+
+    3x Product 1    5.00 EUR = 15.00 EUR
+    Product 2       7.00 EUR
+
+    However, this fails for setups with addon-products since groupby() only groups consecutive
+    lines with the same identity. So in
+
+    Product 1       5.00 EUR
+    + Addon 1       2.00 EUR
+    Product 1       5.00 EUR
+    + Addon 1       2.00 EUR
+    Product 1       5.00 EUR
+    + Addon 2       3.00 EUR
+
+    There is no consecutive repetition of the same entity. This function provides a specialised groupby which
+    understands the product/addon relationship and packs groups of these addons together if they are, in fact,
+    identical groups:
+
+    2x Product 1    5.00 EUR = 10.00 EUR
+    + 2x Addon 1    2.00 EUR =  4.00 EUR
+    Product 1       5.00 EUR
+    + Addon 2       3.00 EUR
+    """
+    packed_groups = []
+
+    for i in iterable:
+        if is_addon(i):
+            packed_groups[-1][1].append(i)
+        else:
+            packed_groups.append((i, []))
+    # Each packed_groups element contains (parent product, list of addon products)
+
+    def _reorder(packed_groups):
+        # Emit the products as individual products again, reordered by "all parent products, then all addon products"
+        # within each group.
+        for group_grouper, grouped in groupby(packed_groups, key=lambda g: key(g[0]) + tuple(key(a) for a in g[1])):
+            grouped = list(grouped)
+            for grp in grouped:
+                yield grp[0]
+            for i in range(max(len(grp[1]) for grp in grouped)):
+                for grp in grouped:
+                    if len(grp[1]) > i:
+                        yield grp[1][i]
+
+    return groupby(_reorder(packed_groups), key)
+
+
 class NumberedCanvas(Canvas):
     def __init__(self, *args, **kwargs):
         self.font_regular = kwargs.pop('font_regular')
@@ -635,7 +692,11 @@ class ClassicInvoiceRenderer(BaseReportlabInvoiceRenderer):
                     line.event_date_from, line.event_date_to)
 
         total = Decimal('0.00')
-        for (description, tax_rate, tax_name, net_value, gross_value, *ignored), lines in groupby(self.invoice.lines.all(), key=_group_key):
+        for (description, tax_rate, tax_name, net_value, gross_value, *ignored), lines in addon_aware_groupby(
+            self.invoice.lines.all(),
+            key=_group_key,
+            is_addon=lambda l: l.description.startswith("  +"),
+        ):
             lines = list(lines)
             if has_taxes:
                 if len(lines) > 1:

src/pretix/control/forms/item.py

@@ -539,8 +539,6 @@ class ItemCreateForm(I18nModelForm):
                     v.pk = None
                     v.item = instance
                     v.save()
-                    if not variation.all_sales_channels:
-                        v.limit_sales_channels.set(variation.limit_sales_channels.all())
                     for mv in variation.meta_values.all():
                         mv.pk = None
                         mv.variation = v

src/tests/base/test_invoices.py

@@ -38,10 +38,12 @@ from decimal import Decimal
 
 import pytest
 from django.db import DatabaseError, transaction
+from django.utils.itercompat import is_iterable
 from django.utils.timezone import now
 from django_countries.fields import Country
 from django_scopes import scope, scopes_disabled
 
+from pretix.base.invoice import addon_aware_groupby
 from pretix.base.models import (
     Event, ExchangeRate, Invoice, InvoiceAddress, Item, ItemVariation, Order,
     OrderPosition, Organizer,
@@ -606,3 +608,55 @@ def test_sales_channels_qualify(env):
 
     event.settings.set('invoice_generate_sales_channels', [])
     assert invoice_qualified(order) is False
+
+
+def test_addon_aware_groupby():
+    def is_addon(item):
+        is_addon, id, price = item
+        return is_addon
+
+    def key(item):
+        return item
+
+    def listify(it):
+        return [listify(i) if is_iterable(i) else i for i in it]
+
+    assert listify(addon_aware_groupby([
+        (False, 1, 5.00),
+        (False, 1, 5.00),
+        (False, 1, 5.00),
+        (False, 2, 7.00),
+    ], key, is_addon)) == [
+        [[False, 1, 5.00], [
+            [False, 1, 5.00],
+            [False, 1, 5.00],
+            [False, 1, 5.00],
+        ]],
+        [[False, 2, 7.00], [
+            [False, 2, 7.00],
+        ]],
+    ]
+
+    assert listify(addon_aware_groupby([
+        (False, 1, 5.00),
+        (True, 101, 2.00),
+        (False, 1, 5.00),
+        (True, 101, 2.00),
+        (False, 1, 5.00),
+        (True, 102, 3.00),
+    ], key, is_addon)) == [
+        [[False, 1, 5.00], [
+            [False, 1, 5.00],
+            [False, 1, 5.00],
+        ]],
+        [[True, 101, 2.00], [
+            [True, 101, 2.00],
+            [True, 101, 2.00],
+        ]],
+        [[False, 1, 5.00], [
+            [False, 1, 5.00],
+        ]],
+        [[True, 102, 3.00], [
+            [True, 102, 3.00],
+        ]],
+    ]

src/tests/control/test_items.py

@@ -673,12 +673,7 @@ class ItemsTest(ItemFormTest):
         with scopes_disabled():
             q = Question.objects.create(event=self.event1, question="Size", type="N")
             q.items.add(self.item2)
-            self.item2.limit_sales_channels.set(self.orga1.sales_channels.filter(identifier__in=["web", "bar"]))
-            self.item2.all_sales_channels = False
-            self.item2.save()
-            self.var2.limit_sales_channels.set(self.orga1.sales_channels.filter(identifier__in=["web"]))
-            self.var2.all_sales_channels = False
-            self.var2.save()
+        self.item2.sales_channels = ["web", "bar"]
         prop = self.event1.item_meta_properties.create(name="Foo")
         self.item2.meta_values.create(property=prop, value="Bar")
 
@@ -695,7 +690,6 @@ class ItemsTest(ItemFormTest):
         with scopes_disabled():
             i_old = Item.objects.get(name__icontains='Business')
             i_new = Item.objects.get(name__icontains='Intermediate')
-            v2_new = i_new.variations.get(value__icontains='Gold')
             assert i_new.category == i_old.category
             assert i_new.description == i_old.description
             assert i_new.active == i_old.active
@@ -704,8 +698,7 @@ class ItemsTest(ItemFormTest):
             assert i_new.require_voucher == i_old.require_voucher
             assert i_new.hide_without_voucher == i_old.hide_without_voucher
             assert i_new.allow_cancel == i_old.allow_cancel
-            assert set(i_new.limit_sales_channels.values_list("identifier", flat=True)) == {"web", "bar"}
-            assert set(v2_new.limit_sales_channels.values_list("identifier", flat=True)) == {"web"}
+            assert set(i_new.limit_sales_channels.all()) == set(i_old.limit_sales_channels.all())
             assert i_new.meta_data == i_old.meta_data == {"Foo": "Bar"}
             assert set(i_new.questions.all()) == set(i_old.questions.all())
             assert set([str(v.value) for v in i_new.variations.all()]) == set([str(v.value) for v in i_old.variations.all()])