From 6bf23b0fddbac3aa8df6f81db35c91fdcc058aba Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Thu, 29 Feb 2024 16:45:41 +0100 Subject: [PATCH] Allow to create blocking vouchers for items with unspecified variation (#3932) --- src/pretix/base/models/vouchers.py | 25 ++++++++++++++++++------- src/pretix/base/services/quotas.py | 18 ++++++++++++------ src/tests/base/test_models.py | 28 +++++++++++++++++++++++++--- src/tests/control/test_vouchers.py | 2 +- 4 files changed, 56 insertions(+), 17 deletions(-) diff --git a/src/pretix/base/models/vouchers.py b/src/pretix/base/models/vouchers.py index 0a60ad802fc..d57c7318ba9 100644 --- a/src/pretix/base/models/vouchers.py +++ b/src/pretix/base/models/vouchers.py @@ -351,9 +351,6 @@ def clean_item_properties(data, event, quota, item, variation, block_quota=False '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 @@ def clean_quota_get_ignored(old_instance): 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 @@ def clean_quota_check(data, cnt, old_instance, event, quota, item, variation): 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.')) diff --git a/src/pretix/base/services/quotas.py b/src/pretix/base/services/quotas.py index cc454a2357e..efefa894b5c 100644 --- a/src/pretix/base/services/quotas.py +++ b/src/pretix/base/services/quotas.py @@ -90,8 +90,8 @@ def __init__(self, count_waitinglist=True, ignore_closed=False, full_results=Fal self._count_waitinglist = count_waitinglist self._ignore_closed = ignore_closed self._full_results = full_results - self._item_to_quotas = defaultdict(list) - self._var_to_quotas = defaultdict(list) + self._item_to_quotas = defaultdict(set) + self._var_to_quotas = defaultdict(set) self._early_out = early_out self._quota_objects = {} self.results = {} @@ -243,13 +243,16 @@ def _compute(self, quotas, now_dt): 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 @@ def _compute_vouchers(self, quotas, q_items, q_vars, size_left, now_dt): 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( diff --git a/src/tests/base/test_models.py b/src/tests/base/test_models.py index be3b1de7b34..e839498f4da 100644 --- a/src/tests/base/test_models.py +++ b/src/tests/base/test_models.py @@ -283,6 +283,29 @@ def test_voucher_variation(self): 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 @@ def test_voucher_item_does_not_match_variation(self): @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.clean() + v = Voucher(item=self.item2, block_quota=True, event=self.event) + v.clean() @classscope(attr='o') def test_voucher_no_item_but_variation(self): diff --git a/src/tests/control/test_vouchers.py b/src/tests/control/test_vouchers.py index ce587e43f15..8e385d0e94e 100644 --- a/src/tests/control/test_vouchers.py +++ b/src/tests/control/test_vouchers.py @@ -254,7 +254,7 @@ def test_create_blocking_item_voucher_quota_full(self): 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