Simpler API for cart removal

src/pretix/base/services/cart.py

@@ -27,6 +27,7 @@ error_messages = {
     'busy': _('We were not able to process your request completely as the '
               'server was too busy. Please try again.'),
     'empty': _('You did not select any products.'),
+    'unknown_position': _('Unknown cart position.'),
     'not_for_sale': _('You selected a product which is not available for sale.'),
     'unavailable': _('Some of the products you selected are no longer available. '
                      'Please see below for details.'),
@@ -269,29 +270,26 @@ class CartManager:
         self._voucher_use_diff += voucher_use_diff
         self._operations += operations
 
-    def remove_items(self, items: List[dict]):
+    def remove_item(self, pos_id: int):
         # TODO: We could calculate quotadiffs and voucherdiffs here, which would lead to more
         # flexible usages (e.g. a RemoveOperation and an AddOperation in the same transaction
         # could cancel each other out quota-wise). However, we are not taking this performance
         # penalty for now as there is currently no outside interface that would allow building
         # such a transaction.
-        for i in items:
+        try:
-            cw = Q(cart_id=self.cart_id) & Q(item_id=i['item']) & Q(event=self.event)
+            cp = self.positions.get(pk=pos_id)
-            if i['variation']:
+        except CartPosition.DoesNotExist:
-                cw &= Q(variation_id=i['variation'])
+            raise CartError(error_messages['unknown_position'])
-            else:
+        self._operations.append(self.RemoveOperation(position=cp))
-                cw &= Q(variation__isnull=True)
+
-            # Prefer to delete positions that have the same price as the one the user clicked on, after thet
+    def clear(self):
-            # prefer the most expensive ones.
+        # TODO: We could calculate quotadiffs and voucherdiffs here, which would lead to more
-            cnt = i['count']
+        # flexible usages (e.g. a RemoveOperation and an AddOperation in the same transaction
-            if i['price']:
+        # could cancel each other out quota-wise). However, we are not taking this performance
-                correctprice = CartPosition.objects.filter(cw).filter(price=Decimal(i['price'].replace(",", ".")))[:cnt]
+        # penalty for now as there is currently no outside interface that would allow building
-                for cp in correctprice:
+        # such a transaction.
-                    self._operations.append(self.RemoveOperation(position=cp))
+        for cp in self.positions.all():
-                cnt -= len(correctprice)
+            self._operations.append(self.RemoveOperation(position=cp))
-            if cnt > 0:
-                for cp in CartPosition.objects.filter(cw).order_by("-price")[:cnt]:
-                    self._operations.append(self.RemoveOperation(position=cp))
 
     def set_addons(self, addons):
         self._update_items_cache(
@@ -566,11 +564,11 @@ def add_items_to_cart(self, event: int, items: List[dict], cart_id: str=None, lo
 
 
 @app.task(base=ProfiledTask, bind=True, max_retries=5, default_retry_delay=1, throws=(CartError,))
-def remove_items_from_cart(self, event: int, items: List[dict], cart_id: str=None, locale='en') -> None:
+def remove_cart_position(self, event: int, position: int, cart_id: str=None, locale='en') -> None:
     """
     Removes a list of items from a user's cart.
     :param event: The event ID in question
-    :param items: A list of dicts with the keys item, variation, number, custom_price, voucher
+    :param position: A cart position ID
     :param session: Session ID of a guest
     """
     with language(locale):
@@ -578,7 +576,7 @@ def remove_items_from_cart(self, event: int, items: List[dict], cart_id: str=Non
         try:
             try:
                 cm = CartManager(event=event, cart_id=cart_id)
-                cm.remove_items(items)
+                cm.remove_item(position)
                 cm.commit()
             except LockTimeoutException:
                 self.retry()
@@ -587,20 +585,41 @@ def remove_items_from_cart(self, event: int, items: List[dict], cart_id: str=Non
 
 
 @app.task(base=ProfiledTask, bind=True, max_retries=5, default_retry_delay=1, throws=(CartError,))
-def set_cart_addons(self, event: int, addons: List[dict], cart_id: str=None) -> None:
+def clear_cart(self, event: int, cart_id: str=None, locale='en') -> None:
+    """
+    Removes a list of items from a user's cart.
+    :param event: The event ID in question
+    :param session: Session ID of a guest
+    """
+    with language(locale):
+        event = Event.objects.get(id=event)
+        try:
+            try:
+                cm = CartManager(event=event, cart_id=cart_id)
+                cm.clear()
+                cm.commit()
+            except LockTimeoutException:
+                self.retry()
+        except (MaxRetriesExceededError, LockTimeoutException):
+            raise CartError(error_messages['busy'])
+
+
+@app.task(base=ProfiledTask, bind=True, max_retries=5, default_retry_delay=1, throws=(CartError,))
+def set_cart_addons(self, event: int, addons: List[dict], cart_id: str=None, locale='en') -> None:
     """
     Removes a list of items from a user's cart.
     :param event: The event ID in question
     :param addons: A list of dicts with the keys addon_to, item, variation
     :param session: Session ID of a guest
     """
-    event = Event.objects.get(id=event)
+    with language(locale):
-    try:
+        event = Event.objects.get(id=event)
         try:
-            cm = CartManager(event=event, cart_id=cart_id)
+            try:
-            cm.set_addons(addons)
+                cm = CartManager(event=event, cart_id=cart_id)
-            cm.commit()
+                cm.set_addons(addons)
-        except LockTimeoutException:
+                cm.commit()
-            self.retry()
+            except LockTimeoutException:
-    except (MaxRetriesExceededError, LockTimeoutException):
+                self.retry()
-        raise CartError(error_messages['busy'])
+        except (MaxRetriesExceededError, LockTimeoutException):
+            raise CartError(error_messages['busy'])

src/pretix/presale/templates/pretixpresale/event/fragment_cart.html

@@ -45,19 +45,10 @@
                 <form action="{% eventurl event "presale:event.cart.remove" %}"
                         method="post" data-asynctask>
                     {% csrf_token %}
-
+                    <input type="hidden" name="id" value="{{ line.id }}" />
-                    {% if line.variation %}
+                    <button class="btn btn-mini btn-link">
-                        <input type="hidden" name="variation_{{ line.item.id }}_{{ line.variation.id }}"
+                        <i class="fa fa-minus"></i>
-                                value="1" />
+                    </button>
-                        <input type="hidden" name="price_{{ line.item.id }}_{{ line.variation.id }}"
-                                value="{{ line.price }}" />
-                    {% else %}
-                        <input type="hidden" name="item_{{ line.item.id }}"
-                                value="1" />
-                        <input type="hidden" name="price_{{ line.item.id }}"
-                                value="{{ line.price }}" />
-                    {% endif %}
-                    <button class="btn btn-mini btn-link"><i class="fa fa-minus"></i></button>
                 </form>
                 {% endif %}
                 {{ line.count }}

src/pretix/presale/templates/pretixpresale/event/index.html

@@ -26,17 +26,8 @@
                 </em>
                 <div class="row checkout-button-row">
                     <div class="col-md-4 col-xs-12">
-                        <form method="post" data-asynctask action="{% eventurl request.event "presale:event.cart.remove" %}" >
+                        <form method="post" data-asynctask action="{% eventurl request.event "presale:event.cart.clear" %}">
                             {% csrf_token %}
-                            {% for line in cart.positions %}
-                                {% if line.variation %}
-                                    <input type="hidden" name="variation_{{ line.item.id }}_{{ line.variation.id }}" value="{{ line.count }}" />
-                                    <input type="hidden" name="price_{{ line.item.id }}_{{ line.variation.id }}" value="{{ line.price }}" />
-                                {% else %}
-                                    <input type="hidden" name="item_{{ line.item.id }}" value="{{ line.count }}" />
-                                    <input type="hidden" name="price_{{ line.item.id }}" value="{{ line.price }}" />
-                                {% endif %}
-                            {% endfor %}
                             <button class="btn btn-block btn-default btn-lg" type="submit"><i class="fa fa-close"></i> {% trans "Empty cart" %}</button>
                         </form>
                     </div>

src/pretix/presale/urls.py

@@ -15,6 +15,7 @@ import pretix.presale.views.waiting
 event_patterns = [
     url(r'^cart/add$', pretix.presale.views.cart.CartAdd.as_view(), name='event.cart.add'),
     url(r'^cart/remove$', pretix.presale.views.cart.CartRemove.as_view(), name='event.cart.remove'),
+    url(r'^cart/clear$', pretix.presale.views.cart.CartClear.as_view(), name='event.cart.clear'),
     url(r'^waitinglist', pretix.presale.views.waiting.WaitingView.as_view(), name='event.waitinglist'),
     url(r'^checkout/start$', pretix.presale.views.checkout.CheckoutView.as_view(), name='event.checkout.start'),
     url(r'^redeem$', pretix.presale.views.cart.RedeemView.as_view(),

src/pretix/presale/views/cart.py

@@ -10,7 +10,7 @@ from django.views.generic import TemplateView, View
 from pretix.base.decimal import round_decimal
 from pretix.base.models import CartPosition, Quota, Voucher
 from pretix.base.services.cart import (
-    CartError, add_items_to_cart, remove_items_from_cart,
+    CartError, add_items_to_cart, clear_cart, remove_cart_position,
 )
 from pretix.multidomain.urlreverse import eventreverse
 from pretix.presale.views import EventViewMixin
@@ -105,19 +105,18 @@ class CartActionMixin:
 
 
 class CartRemove(EventViewMixin, CartActionMixin, AsyncAction, View):
-    task = remove_items_from_cart
+    task = remove_cart_position
     known_errortypes = ['CartError']
 
     def get_success_message(self, value):
         if CartPosition.objects.filter(cart_id=self.request.session.session_key).exists():
             return _('Your cart has been updated.')
         else:
-            return _('Your cart is empty.')
+            return _('Your cart is now empty.')
 
     def post(self, request, *args, **kwargs):
-        items = self._items_from_post_data()
+        if 'id' in request.POST:
-        if items:
+            return self.do(self.request.event.id, request.POST.get('id'), self.request.session.session_key, translation.get_language())
-            return self.do(self.request.event.id, items, self.request.session.session_key, translation.get_language())
         else:
             if 'ajax' in self.request.GET or 'ajax' in self.request.POST:
                 return JsonResponse({
@@ -127,6 +126,17 @@ class CartRemove(EventViewMixin, CartActionMixin, AsyncAction, View):
                 return redirect(self.get_error_url())
 
 
+class CartClear(EventViewMixin, CartActionMixin, AsyncAction, View):
+    task = clear_cart
+    known_errortypes = ['CartError']
+
+    def get_success_message(self, value):
+        return _('Your cart is now empty.')
+
+    def post(self, request, *args, **kwargs):
+        return self.do(self.request.event.id, self.request.session.session_key, translation.get_language())
+
+
 class CartAdd(EventViewMixin, CartActionMixin, AsyncAction, View):
     task = add_items_to_cart
     known_errortypes = ['CartError']

src/tests/presale/test_cart.py

@@ -495,12 +495,12 @@ class CartTest(CartTestMixin, TestCase):
         self.assertFalse(CartPosition.objects.filter(id=cp1.id).exists())
 
     def test_remove_simple(self):
-        CartPosition.objects.create(
+        cp = CartPosition.objects.create(
             event=self.event, cart_id=self.session_key, item=self.ticket,
             price=23, expires=now() + timedelta(minutes=10)
         )
         response = self.client.post('/%s/%s/cart/remove' % (self.orga.slug, self.event.slug), {
-            'item_%d' % self.ticket.id: '1',
+            'id': cp.pk
         }, follow=True)
         doc = BeautifulSoup(response.rendered_content, "lxml")
         self.assertIn('empty', doc.select('.alert-success')[0].text)
@@ -525,19 +525,19 @@ class CartTest(CartTestMixin, TestCase):
         self.assertFalse(CartPosition.objects.filter(cart_id=self.session_key, event=self.event).exists())
 
     def test_remove_variation(self):
-        CartPosition.objects.create(
+        cp = CartPosition.objects.create(
             event=self.event, cart_id=self.session_key, item=self.shirt, variation=self.shirt_red,
             price=14, expires=now() + timedelta(minutes=10)
         )
         response = self.client.post('/%s/%s/cart/remove' % (self.orga.slug, self.event.slug), {
-            'variation_%d_%d' % (self.shirt.id, self.shirt_red.id): '1',
+            'id': cp.pk
         }, follow=True)
         doc = BeautifulSoup(response.rendered_content, "lxml")
         self.assertIn('empty', doc.select('.alert-success')[0].text)
         self.assertFalse(CartPosition.objects.filter(cart_id=self.session_key, event=self.event).exists())
 
     def test_remove_one_of_multiple(self):
-        CartPosition.objects.create(
+        cp = CartPosition.objects.create(
             event=self.event, cart_id=self.session_key, item=self.ticket,
             price=23, expires=now() + timedelta(minutes=10)
         )
@@ -546,28 +546,12 @@ class CartTest(CartTestMixin, TestCase):
             price=23, expires=now() + timedelta(minutes=10)
         )
         response = self.client.post('/%s/%s/cart/remove' % (self.orga.slug, self.event.slug), {
-            'item_%d' % self.ticket.id: '1',
+            'id': cp.pk
         }, follow=True)
         doc = BeautifulSoup(response.rendered_content, "lxml")
         self.assertIn('updated', doc.select('.alert-success')[0].text)
         self.assertEqual(CartPosition.objects.filter(cart_id=self.session_key, event=self.event).count(), 1)
 
-    def test_remove_multiple(self):
-        CartPosition.objects.create(
-            event=self.event, cart_id=self.session_key, item=self.ticket,
-            price=23, expires=now() + timedelta(minutes=10)
-        )
-        CartPosition.objects.create(
-            event=self.event, cart_id=self.session_key, item=self.ticket,
-            price=23, expires=now() + timedelta(minutes=10)
-        )
-        response = self.client.post('/%s/%s/cart/remove' % (self.orga.slug, self.event.slug), {
-            'item_%d' % self.ticket.id: '2',
-        }, follow=True)
-        doc = BeautifulSoup(response.rendered_content, "lxml")
-        self.assertIn('empty', doc.select('.alert-success')[0].text)
-        self.assertFalse(CartPosition.objects.filter(cart_id=self.session_key, event=self.event).exists())
-
     def test_remove_all(self):
         CartPosition.objects.create(
             event=self.event, cart_id=self.session_key, item=self.ticket,
@@ -581,50 +565,11 @@ class CartTest(CartTestMixin, TestCase):
             event=self.event, cart_id=self.session_key, item=self.shirt, variation=self.shirt_red,
             price=14, expires=now() + timedelta(minutes=10)
         )
-        response = self.client.post('/%s/%s/cart/remove' % (self.orga.slug, self.event.slug), {
+        response = self.client.post('/%s/%s/cart/clear' % (self.orga.slug, self.event.slug), {}, follow=True)
-            'item_%d' % self.ticket.id: '2',
-            'variation_%d_%d' % (self.shirt.id, self.shirt_red.id): '1',
-        }, follow=True)
         doc = BeautifulSoup(response.rendered_content, "lxml")
         self.assertIn('empty', doc.select('.alert-success')[0].text)
         self.assertFalse(CartPosition.objects.filter(cart_id=self.session_key, event=self.event).exists())
 
-    def test_remove_all_same_variation_different_price(self):
-        CartPosition.objects.create(
-            event=self.event, cart_id=self.session_key, item=self.shirt, variation=self.shirt_red,
-            price=14, expires=now() + timedelta(minutes=10)
-        )
-        v = Voucher.objects.create(item=self.shirt, variation=self.shirt_red, value=Decimal('10.00'), event=self.event)
-        self.client.post('/%s/%s/cart/add' % (self.orga.slug, self.event.slug), {
-            'variation_%d_%d_voucher' % (self.shirt.id, self.shirt_red.id): v.code,
-        }, follow=True)
-        response = self.client.post('/%s/%s/cart/remove' % (self.orga.slug, self.event.slug), {
-            'variation_%d_%d' % (self.shirt.id, self.shirt_red.id): ('1', '1'),
-        }, follow=True)
-        doc = BeautifulSoup(response.rendered_content, "lxml")
-        self.assertIn('empty', doc.select('.alert-success')[0].text)
-        self.assertFalse(CartPosition.objects.filter(cart_id=self.session_key, event=self.event).exists())
-
-    def test_remove_most_expensive(self):
-        CartPosition.objects.create(
-            event=self.event, cart_id=self.session_key, item=self.ticket,
-            price=23, expires=now() + timedelta(minutes=10)
-        )
-        CartPosition.objects.create(
-            event=self.event, cart_id=self.session_key, item=self.ticket,
-            price=20, expires=now() + timedelta(minutes=10)
-        )
-        response = self.client.post('/%s/%s/cart/remove' % (self.orga.slug, self.event.slug), {
-            'item_%d' % self.ticket.id: '1',
-        }, follow=True)
-        doc = BeautifulSoup(response.rendered_content, "lxml")
-        self.assertIn('updated', doc.select('.alert-success')[0].text)
-        objs = list(CartPosition.objects.filter(cart_id=self.session_key, event=self.event))
-        self.assertEqual(len(objs), 1)
-        self.assertEqual(objs[0].item, self.ticket)
-        self.assertIsNone(objs[0].variation)
-        self.assertEqual(objs[0].price, 20)
-
     def test_voucher(self):
         v = Voucher.objects.create(item=self.ticket, event=self.event)
         self.client.post('/%s/%s/cart/add' % (self.orga.slug, self.event.slug), {