clean up

src/pretix/base/models/cancellation.py

@@ -31,11 +31,7 @@ class RelativeFee:
         return self.reference_price * (self.percentage/100)
 
 
-@dataclass(frozen=True)
+Fee=Union[AbsoluteFee, RelativeFee]
-class CheckRes:
-    cancellation_possible: bool
-    reason: str
-CheckResult=Dict[str, CheckRes]
 
 @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
+    items = models.ManyToManyField(
-    item_variation=models.ForeignKey("ItemVariation", on_delete=models.CASCADE, null=True, blank=True) # probably m2m field to avoid duplicating rules
+        "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])

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,

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,
+            organizer=event.organizer, event=event,
-            order_status=Order.STATUS_PENDING
+            allowed_if_in_order_status=Order.STATUS_PENDING
         )
+        cancellation_rule.items.set([item1])
 
-        assert cancellation_rule._rule_order_status(order_position=op) == {
+        diff = OrderDiff.cancel_all(order)
-            'ORDER_STATUS': Ruling(
+
+        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,
+            organizer=event.organizer, event=event,
-            order_status=Order.STATUS_PAID
+            allowed_if_in_order_status=Order.STATUS_PAID
         )
+        cancellation_rule.items.set([item1])
 
-        assert cancellation_rule._rule_order_status(order_position=op) == {
+        assert cancellation_rule._check_order_status(diff=diff, order_position=op) == {
-            'ORDER_STATUS': Ruling(
+            '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(
+        cr1 = CancellationRule.objects.create(
-            organizer=event.organizer, event=event, item=item1,
+            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(
+        cr1=CancellationRule.objects.create(
-            organizer=event.organizer, event=event, item=item1,
+            organizer=event.organizer, event=event,
             allowed_until=NOW + timedelta(hours=1),
         )
+        cr1.items.set([item1])
 
         # free until 30 days before event
-        CancellationRule.objects.create(
+        cr2 = CancellationRule.objects.create(
-            organizer=event.organizer, event=event, item=item1,
+            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(
+        cr3 = CancellationRule.objects.create(
-            organizer=event.organizer, event=event, item=item1,
+            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(
+        cr4 = CancellationRule.objects.create(
-            organizer=event.organizer, event=event, item=item1,
+            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(
+        cr5 = CancellationRule.objects.create(
-            organizer=event.organizer, event=event, item=item1,
+            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(
+        cr1 = CancellationRule.objects.create(
-            organizer=event.organizer, event=event, item=item1,
+            organizer=event.organizer, event=event,
-            order_status=Order.STATUS_PENDING, fee_absolute_per_order=Decimal('10.00'),
+            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,
+        cr2 = CancellationRule.objects.create(
-            order_status=Order.STATUS_PAID
+            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