Allow to create blocking vouchers for items with unspecified variation (#3932)

src/pretix/base/models/vouchers.py

@@ -351,9 +351,6 @@ class Voucher(LoggedModel):
                                         'variations.'))
             if variation and not item.variations.filter(pk=variation.pk).exists():
                 raise ValidationError(_('This variation does not belong to this product.'))
-            if item.has_variations and not variation and data.get('block_quota'):
-                raise ValidationError(_('You can only block quota if you specify a specific product variation. '
-                                        'Otherwise it might be unclear which quotas to block.'))
             if item.category and item.category.is_addon:
                 raise ValidationError(_('It is currently not possible to create vouchers for add-on products.'))
         elif block_quota:
@@ -431,7 +428,15 @@ class Voucher(LoggedModel):
             elif old_instance.variation:
                 quotas |= set(old_instance.variation.quotas.filter(subevent=old_instance.subevent))
             elif old_instance.item:
-                quotas |= set(old_instance.item.quotas.filter(subevent=old_instance.subevent))
+                if old_instance.item.has_variations:
+                    quotas |= set(
+                        Quota.objects.filter(pk__in=Quota.variations.through.objects.filter(
+                            itemvariation__item=old_instance.item,
+                            quota__subevent=old_instance.subevent,
+                        ).values('quota_id'))
+                    )
+                else:
+                    quotas |= set(old_instance.item.quotas.filter(subevent=old_instance.subevent))
         return quotas
 
     @staticmethod
@@ -446,13 +451,19 @@ class Voucher(LoggedModel):
 
         if quota:
             new_quotas = {quota}
-        elif item and item.has_variations and not variation:
-            raise ValidationError(_('You can only block quota if you specify a specific product variation. '
-                                    'Otherwise it might be unclear which quotas to block.'))
         elif item and variation:
             new_quotas = set(variation.quotas.filter(subevent=data.get('subevent')))
         elif item and not item.has_variations:
             new_quotas = set(item.quotas.filter(subevent=data.get('subevent')))
+        elif item and item.has_variations:
+            new_quotas = set(
+                Quota.objects.filter(
+                    pk__in=Quota.variations.through.objects.filter(
+                        itemvariation__item=old_instance.item,
+                        quota__subevent=data.get('subevent'),
+                    ).values('quota_id')
+                )
+            )
         else:
             raise ValidationError(_('You need to select a specific product or quota if this voucher should reserve '
                                     'tickets.'))

src/pretix/base/services/quotas.py

@@ -90,8 +90,8 @@ class QuotaAvailability:
         self._count_waitinglist = count_waitinglist
         self._ignore_closed = ignore_closed
         self._full_results = full_results
-        self._item_to_quotas = defaultdict(list)
+        self._item_to_quotas = defaultdict(set)
-        self._var_to_quotas = defaultdict(list)
+        self._var_to_quotas = defaultdict(set)
         self._early_out = early_out
         self._quota_objects = {}
         self.results = {}
@@ -243,13 +243,16 @@ class QuotaAvailability:
             quota_id__in=[q.pk for q in quotas]
         ).values('quota_id', 'item_id')
         for m in q_items:
-            self._item_to_quotas[m['item_id']].append(self._quota_objects[m['quota_id']])
+            self._item_to_quotas[m['item_id']].add(self._quota_objects[m['quota_id']])
 
         q_vars = Quota.variations.through.objects.filter(
             quota_id__in=[q.pk for q in quotas]
-        ).values('quota_id', 'itemvariation_id')
+        ).values('quota_id', 'itemvariation_id', 'itemvariation__item_id')
         for m in q_vars:
-            self._var_to_quotas[m['itemvariation_id']].append(self._quota_objects[m['quota_id']])
+            self._var_to_quotas[m['itemvariation_id']].add(self._quota_objects[m['quota_id']])
+            # We can't be 100% certain that a quota, when it is connected to a variation, is also always connected to
+            # the parent item, so we double-check here just to be sure.
+            self._item_to_quotas[m['itemvariation__item_id']].add(self._quota_objects[m['quota_id']])
 
         self._compute_orders(quotas, q_items, q_vars, size_left)
 
@@ -378,7 +381,10 @@ class QuotaAvailability:
             Q(
                 Q(
                     Q(variation_id__isnull=True) &
-                    Q(item_id__in={i['item_id'] for i in q_items if i['quota_id'] in quota_ids})
+                    Q(item_id__in=(
+                        {i['item_id'] for i in q_items if i['quota_id'] in quota_ids} |
+                        {i['itemvariation__item_id'] for i in q_vars if i['quota_id'] in quota_ids}
+                    ))
                 ) | Q(
                     variation_id__in={i['itemvariation_id'] for i in q_vars if i['quota_id'] in quota_ids}
                 ) | Q(

src/tests/base/test_models.py

@@ -283,6 +283,29 @@ class QuotaTestCase(BaseQuotaTestCase):
         v.save()
         self.assertEqual(self.var1.check_quotas(), (Quota.AVAILABILITY_ORDERED, 0))
 
+    @classscope(attr='o')
+    def test_voucher_all_variations(self):
+        self.quota.variations.add(self.var1)
+        self.quota.size = 1
+        self.quota.save()
+
+        self.quota2 = Quota.objects.create(name="Test", size=2, event=self.event)
+        self.quota2.variations.add(self.var2)
+
+        self.quota3 = Quota.objects.create(name="Test", size=2, event=self.event)
+        self.quota3.variations.add(self.var3)
+
+        v = Voucher.objects.create(item=self.item2, event=self.event)
+        self.assertEqual(self.var1.check_quotas(), (Quota.AVAILABILITY_OK, 1))
+        self.assertEqual(self.var2.check_quotas(), (Quota.AVAILABILITY_OK, 2))
+        self.assertEqual(self.var3.check_quotas(), (Quota.AVAILABILITY_OK, 2))
+
+        v.block_quota = True
+        v.save()
+        self.assertEqual(self.var1.check_quotas(), (Quota.AVAILABILITY_ORDERED, 0))
+        self.assertEqual(self.var2.check_quotas(), (Quota.AVAILABILITY_OK, 1))
+        self.assertEqual(self.var3.check_quotas(), (Quota.AVAILABILITY_OK, 2))
+
     @classscope(attr='o')
     def test_voucher_quota(self):
         self.quota.variations.add(self.var1)
@@ -979,9 +1002,8 @@ class VoucherTestCase(BaseQuotaTestCase):
 
     @classscope(attr='o')
     def test_voucher_specify_variation_for_block_quota(self):
-        with self.assertRaises(ValidationError):
+        v = Voucher(item=self.item2, block_quota=True, event=self.event)
-            v = Voucher(item=self.item2, block_quota=True, event=self.event)
+        v.clean()
-            v.clean()
 
     @classscope(attr='o')
     def test_voucher_no_item_but_variation(self):

src/tests/control/test_vouchers.py

@@ -254,7 +254,7 @@ class VoucherFormTest(SoupTestMixin, TransactionTestCase):
         self._create_voucher({
             'itemvar': '%d' % self.shirt.pk,
             'block_quota': 'on'
-        }, expected_failure=True)
+        })
 
     def test_create_blocking_item_voucher_quota_full_invalid(self):
         self.quota_shirts.size = 0