From c1db94dec3b1d71ef3a7f158ebcc7b136f9b469f Mon Sep 17 00:00:00 2001 From: Lukas Bockstaller Date: Mon, 23 Mar 2026 14:22:14 +0100 Subject: [PATCH] clean up --- src/pretix/base/models/cancellation.py | 68 ++++++++------ src/pretix/base/models/orders.py | 1 + .../base/test_self_service_cancellation.py | 90 ++++++++++++------- 3 files changed, 100 insertions(+), 59 deletions(-) diff --git a/src/pretix/base/models/cancellation.py b/src/pretix/base/models/cancellation.py index 8f506d90c2..c56a074089 100644 --- a/src/pretix/base/models/cancellation.py +++ b/src/pretix/base/models/cancellation.py @@ -31,11 +31,7 @@ class RelativeFee: return self.reference_price * (self.percentage/100) -@dataclass(frozen=True) -class CheckRes: - cancellation_possible: bool - reason: str -CheckResult=Dict[str, CheckRes] +Fee=Union[AbsoluteFee, RelativeFee] @dataclass(frozen=True) class OrderDiff: @@ -46,8 +42,18 @@ class OrderDiff: def cancellations(self): return self.prev.difference(self.next) + @staticmethod + def cancel_all(order: Order) -> "OrderDiff": + return OrderDiff(order=order, prev=set(order.positions.all()), next=set()) + +@dataclass(frozen=True) +class CheckRes: + cancellation_possible: bool + reason: str + +CheckResult=Dict[str, CheckRes] + -Fee=Union[AbsoluteFee, RelativeFee] CheckFn=Callable[[OrderDiff, OrderPosition], CheckResult] @@ -60,6 +66,7 @@ def merge_check_results(a: CheckResult, b: CheckResult) -> CheckResult: result[key] = b_inner return result + @dataclass(frozen=True) class Ruling: rule_id: int @@ -101,24 +108,24 @@ class CancellationRuleQuerySet(models.QuerySet): return [self._evaluate_op(diff, position) for position in diff.order.positions.all()] def _evaluate_op(self, diff: OrderDiff, order_position: OrderPosition) -> List[Ruling]: - consequences=[rule.apply(diff, order_position) for rule in self] + consequences = [] + for rule in self: + if order_position.item in rule.items.all() or order_position.variation in rule.variations.all(): + consequences.append(rule.apply(diff, order_position)) consequences.sort() return consequences -ALLOWED_STATUS_CHARS={char for char, _ in Order.STATUS_CHOICE} # {'n', 'p', 'e', 'c'} - def validate_status_chars(value): - invalid=set(value) - ALLOWED_STATUS_CHARS + invalid=set(value) - Order.ALLOWED_STATUS_CHARS if invalid: raise ValidationError( - f"Invalid characters: {invalid}. Allowed: {ALLOWED_STATUS_CHARS}" + f"Invalid characters: {invalid}. Allowed: {Order.ALLOWED_STATUS_CHARS}" ) if len(value) != len(set(value)): raise ValidationError("Duplicate characters are not allowed.") - class CancellationRule(models.Model): """ @@ -137,17 +144,22 @@ class CancellationRule(models.Model): related_name="orders", on_delete=models.CASCADE ) - item=models.ForeignKey("Item", on_delete=models.CASCADE, null=True, blank=True) # probably m2m field to avoid duplicating rules - item_variation=models.ForeignKey("ItemVariation", on_delete=models.CASCADE, null=True, blank=True) # probably m2m field to avoid duplicating rules - + items = models.ManyToManyField( + "Item", + verbose_name=_("Items"), + ) + variations=models.ManyToManyField( + "ItemVariation", + verbose_name=_("Item variations"), + ) allowed_if_in_order_status=models.CharField( max_length=4, choices=Order.STATUS_CHOICE, verbose_name=_("Cancellation possible if order is in status"), - validators=[validate_status_chars] + validators=[validate_status_chars], + default="".join(Order.ALLOWED_STATUS_CHARS), ) - allowed_until=ModelRelativeDateTimeField(null=True, blank=True) except_after=ModelRelativeDateTimeField(null=True, blank=True) @@ -171,7 +183,6 @@ class CancellationRule(models.Model): verbose_name=_("Absolute Fee per Item"), default=Decimal("0.00"), ) # wird als sum() kombiniert - fee_absolute_per_order=models.DecimalField( max_digits=13, decimal_places=2, @@ -191,6 +202,11 @@ class CancellationRule(models.Model): self._system_check_not_checked_in, self._system_check_not_discounted] + # TODO weitere System Checks + # OrderPositions mit Item.min_per_order dürfen nur storniert werden, wenn genug übrig bleiben oder alle des gleichen Items storniert werden + # OrderPositions mit addon_to != None dürfen nur über den bestehenden Add-On-Flow storniert werden + # OrderPositions mit is_bundled dürfen nur mit der Parent-Position zusammen storniert werden + @staticmethod def _system_check_not_checked_in(diff: OrderDiff, order_position: OrderPosition) -> CheckResult: check_id = "SYSTEM_TICKET_NOT_USED" @@ -218,7 +234,7 @@ class CancellationRule(models.Model): :param order_position: :return CheckResults: """ - check_id = "SYSTEM_TICKET_NOT_USED" + check_id = "SYSTEM_TICKET_NOT_DISCOUNTED" if order_position in diff.cancellations(): if order_position.discount is None: @@ -273,29 +289,23 @@ class CancellationRule(models.Model): def _check_order_status(self, diff: OrderDiff, order_position: OrderPosition) -> CheckResult: check_id = "ORDER_STATUS" - if not self.allowed_until and not self.allowed_until: + if diff.order.status == "".join(Order.ALLOWED_STATUS_CHARS): return {check_id: CheckRes( cancellation_possible=True, reason=f"Orders in every status can be cancelled", )} - elif order_position.order.status in self.allowed_if_in_order_status: + elif diff.order.status in self.allowed_if_in_order_status: return {check_id: CheckRes( cancellation_possible=True, - reason=f"Order in required status: '{order_position.order.status}'", + reason=f"Order in required status: '{diff.order.status}'", )} else: return {check_id: CheckRes( cancellation_possible=False, - reason=f"Order in status '{order_position.order.status}' cannot be canceled", + reason=f"Order in status '{diff.order.status}' cannot be canceled", )} - # OrderPositions mit Item.min_per_order dürfen nur storniert werden, wenn genug übrig bleiben oder alle des gleichen Items storniert werden - # OrderPositions mit addon_to != None dürfen nur über den bestehenden Add-On-Flow storniert werden - # OrderPositions mit is_bundled dürfen nur mit der Parent-Position zusammen storniert werden - - - def apply(self, diff: OrderDiff, order_position: OrderPosition) -> Ruling: check_results=reduce(merge_check_results, [rule(diff, order_position) for rule in self.checks]) diff --git a/src/pretix/base/models/orders.py b/src/pretix/base/models/orders.py index e74e450e8c..bef8d8a936 100644 --- a/src/pretix/base/models/orders.py +++ b/src/pretix/base/models/orders.py @@ -203,6 +203,7 @@ class Order(LockModel, LoggedModel): (STATUS_EXPIRED, _("expired")), (STATUS_CANCELED, _("canceled")), ) + ALLOWED_STATUS_CHARS={char for char, _ in STATUS_CHOICE} # {'n', 'p', 'e', 'c'} code = models.CharField( max_length=16, diff --git a/src/tests/base/test_self_service_cancellation.py b/src/tests/base/test_self_service_cancellation.py index f3c6d30880..a0a2c64692 100644 --- a/src/tests/base/test_self_service_cancellation.py +++ b/src/tests/base/test_self_service_cancellation.py @@ -9,6 +9,8 @@ from freezegun import freeze_time from pretix.base.models import Event, Item, Order, OrderPosition, Organizer from pretix.base.models.cancellation import CancellationRule, Ruling +from pretix.base.models.cancellation import CheckRes +from pretix.base.models.cancellation import OrderDiff NOW = now() DAYS_UNTIL_EVENT=60 @@ -51,24 +53,28 @@ def test_status_rule(event, item1, order): ) cancellation_rule = CancellationRule.objects.create( - organizer=event.organizer, event=event, item=item1, - order_status=Order.STATUS_PENDING + organizer=event.organizer, event=event, + allowed_if_in_order_status=Order.STATUS_PENDING ) + cancellation_rule.items.set([item1]) - assert cancellation_rule._rule_order_status(order_position=op) == { - 'ORDER_STATUS': Ruling( + diff = OrderDiff.cancel_all(order) + + assert cancellation_rule._check_order_status(diff=diff, order_position=op) == { + 'ORDER_STATUS': CheckRes( cancellation_possible=True, reason="Order in required status: 'n'", ), } cancellation_rule = CancellationRule.objects.create( - organizer=event.organizer, event=event, item=item1, - order_status=Order.STATUS_PAID + organizer=event.organizer, event=event, + allowed_if_in_order_status=Order.STATUS_PAID ) + cancellation_rule.items.set([item1]) - assert cancellation_rule._rule_order_status(order_position=op) == { - 'ORDER_STATUS': Ruling( + assert cancellation_rule._check_order_status(diff=diff, order_position=op) == { + 'ORDER_STATUS': CheckRes( cancellation_possible=False, reason="Order in status 'n' cannot be canceled", ), @@ -86,17 +92,21 @@ def test_timing(event, item1, order): price=Decimal("0.00"), attendee_name_parts={'full_name': "Peter"}, positionid=1 ) - CancellationRule.objects.create( - organizer=event.organizer, event=event, item=item1, + cr1 = CancellationRule.objects.create( + organizer=event.organizer, event=event, allowed_until=now() + timedelta(hours=1), ) + cr1.items.set([item1]) + + + diff = OrderDiff.cancel_all(order) with freeze_time(now()): - possible, verdicts = CancellationRule.objects.all().cancellation_possible(order) + possible, verdicts = CancellationRule.objects.all().cancellation_possible(diff) assert possible == True with freeze_time(now()+timedelta(hours=2)): - possible, verdicts=CancellationRule.objects.all().cancellation_possible(order) + possible, verdicts=CancellationRule.objects.all().cancellation_possible(diff) assert possible == False @@ -112,37 +122,52 @@ def test_multiple_limits(event, item1, order): ) # free in the first hour after booking - CancellationRule.objects.create( - organizer=event.organizer, event=event, item=item1, + cr1=CancellationRule.objects.create( + organizer=event.organizer, event=event, allowed_until=NOW + timedelta(hours=1), ) + cr1.items.set([item1]) # free until 30 days before event - CancellationRule.objects.create( - organizer=event.organizer, event=event, item=item1, + cr2 = CancellationRule.objects.create( + organizer=event.organizer, event=event, allowed_until=EVENT_START - timedelta(days=30), ) + cr2.items.set([item1]) # 50% until 14 days before event - CancellationRule.objects.create( - organizer=event.organizer, event=event, item=item1, + cr3 = CancellationRule.objects.create( + organizer=event.organizer, event=event, allowed_until=EVENT_START - timedelta(days=14), fee_percentage_per_item=Decimal(50.0) ) + cr3.items.set([item1]) # 80% until 7 days before event - CancellationRule.objects.create( - organizer=event.organizer, event=event, item=item1, + cr4 = CancellationRule.objects.create( + organizer=event.organizer, event=event, allowed_until=EVENT_START - timedelta(days=7), fee_percentage_per_item=Decimal(80.0) ) + cr4.items.set([item1]) # 100% until 1 day before event - CancellationRule.objects.create( - organizer=event.organizer, event=event, item=item1, + cr5 = CancellationRule.objects.create( + organizer=event.organizer, event=event, allowed_until=EVENT_START - timedelta(days=1), fee_percentage_per_item=Decimal(100) ) + cr5.items.set([item1]) + + # Cancellation is not allowed at all, but rule doesn't match the item + CancellationRule.objects.create( + organizer=event.organizer, event=event, + allowed_until=NOW, + fee_percentage_per_item=Decimal(100) + ) + + + diff = OrderDiff.cancel_all(order) possible_trace = [] cost_trace = [] @@ -151,7 +176,7 @@ def test_multiple_limits(event, item1, order): today = NOW + timedelta(days=days) with freeze_time(today): possible, verdicts=CancellationRule.objects.all().cancellation_possible( - order) + diff) possible_trace.append(possible) cost_trace.append(verdicts[0].total_fee) @@ -173,16 +198,21 @@ def test_cancellation_rule_query_set(event, item1, order): price=Decimal("0.00"), attendee_name_parts={'full_name': "Peter"}, positionid=1 ) - CancellationRule.objects.create( - organizer=event.organizer, event=event, item=item1, - order_status=Order.STATUS_PENDING, fee_absolute_per_order=Decimal('10.00'), + cr1 = CancellationRule.objects.create( + organizer=event.organizer, event=event, + allowed_if_in_order_status=Order.STATUS_PENDING, fee_absolute_per_order=Decimal('10.00'), ) + cr1.items.set([item1]) - CancellationRule.objects.create( - organizer=event.organizer, event=event, item=item1, - order_status=Order.STATUS_PAID + + cr2 = CancellationRule.objects.create( + organizer=event.organizer, event=event, + allowed_if_in_order_status=Order.STATUS_PAID ) + cr2.items.set([item1]) - possible, verdicts = CancellationRule.objects.all().cancellation_possible(order) + diff = OrderDiff.cancel_all(order) + + possible, verdicts = CancellationRule.objects.all().cancellation_possible(diff) assert possible == True