From f1760bdfa640431a42a8fe8136305b196411cb44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=AD=C3=B0ir=20Valberg=20Gu=C3=B0mundsson?= Date: Sun, 30 Jul 2023 22:45:09 +0200 Subject: [PATCH] Fix overeager idempotency of create_tickets. --- src/shop/models.py | 18 ++++++++++--- src/shop/tests.py | 65 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/src/shop/models.py b/src/shop/models.py index 7cb743310..4d6f3f050 100644 --- a/src/shop/models.py +++ b/src/shop/models.py @@ -822,12 +822,22 @@ def create_tickets(self, request: Optional[HttpRequest] = None): return tickets if sub_product_relations: - # For each bought product we create a ticket for each sub product - for _i in range(self.quantity): - # Iterate over all ticket types that are related to this product + # If there are sub products, we need to create a ticket group to match + # the quantity of the OPR. + + # Get the pre-existing ticket groups - cast to list, so we can append to it + ticket_groups = list(self.ticketgroups.all()) - ticket_group, _ = TicketGroup.objects.get_or_create(opr=self) + # If we have less ticket groups than the quantity of the OPR, we need to create more + if len(ticket_groups) < self.quantity: + # We want to create the difference between the quantity of the OPR and the number of ticket groups + difference = self.quantity - len(ticket_groups) + for _i in range(difference): + ticket_group = TicketGroup.objects.create(opr=self) + ticket_groups.append(ticket_group) + # For each bought product we create a ticket for each sub product + for ticket_group in ticket_groups: for sub_product_relation in sub_product_relations: new_tickets = self._create_tickets_helper( product=sub_product_relation.sub_product, diff --git a/src/shop/tests.py b/src/shop/tests.py index b11abdebb..05cfbb325 100644 --- a/src/shop/tests.py +++ b/src/shop/tests.py @@ -458,6 +458,71 @@ def test_sub_products_created_sub_product_single_ticket_per_product_true(self): 1, ) + def test_ticket_generation_is_idempotent(self): + """Test that calling create_tickets multiple times does not create more tickets.""" + bundle_product = ProductFactory() + sub_product = ProductFactory( + ticket_type=TicketTypeFactory(), + ) + bundle_product.sub_products.add( + sub_product, + through_defaults={"number_of_tickets": 5}, + ) + order = OrderFactory(user=self.user) + OrderProductRelationFactory(order=order, product=bundle_product, quantity=2) + + order.mark_as_paid() + self.assertEqual( + ShopTicket.objects.filter(opr__order=order).count(), + 10, + ) + self.assertEqual( + TicketGroup.objects.filter(opr=order.oprs.first()).count(), + 2, + ) + + # Calling create_tickets again should not create more tickets + order.create_tickets() + self.assertEqual( + ShopTicket.objects.filter(opr__order=order).count(), + 10, + ) + self.assertEqual( + TicketGroup.objects.filter(opr=order.oprs.first()).count(), + 2, + ) + + # Add a new sub product to the bundle + sub_product_2 = ProductFactory( + ticket_type=TicketTypeFactory(), + ) + bundle_product.sub_products.add( + sub_product_2, + through_defaults={"number_of_tickets": 5}, + ) + + # Calling create_tickets again should create 10 new tickets + order.create_tickets() + self.assertEqual( + ShopTicket.objects.filter(opr__order=order).count(), + 20, + ) + self.assertEqual( + TicketGroup.objects.filter(opr=order.oprs.first()).count(), + 2, + ) + + # Calling create_tickets again should not create more tickets + order.create_tickets() + self.assertEqual( + ShopTicket.objects.filter(opr__order=order).count(), + 20, + ) + self.assertEqual( + TicketGroup.objects.filter(opr=order.oprs.first()).count(), + 2, + ) + class TestOrderProductRelationModel(TestCase): def test_refunded_cannot_be_larger_than_quantity(self):