diff --git a/doc/api/resources/giftcards.rst b/doc/api/resources/giftcards.rst index e785bd621d..99b0bed1df 100644 --- a/doc/api/resources/giftcards.rst +++ b/doc/api/resources/giftcards.rst @@ -142,7 +142,7 @@ Endpoints want to change. You can change all fields of the resource except the ``id``, ``secret``, and ``currency`` fields. Be careful when - modifying the ``value`` field to avoid race conditions. + modifying the ``value`` field to avoid race conditions. We recommend to use the ``transact`` method described below. **Example request**: @@ -170,7 +170,48 @@ Endpoints "id": 1, "secret": "HLBYVELFRC77NCQY", "currency": "EUR", - "value": "13.37" + "value": "14.00" + } + + :param organizer: The ``slug`` field of the organizer to modify + :param id: The ``id`` field of the gift card to modify + :statuscode 200: no error + :statuscode 400: The gift card could not be modified due to invalid submitted data + :statuscode 401: Authentication failure + :statuscode 403: The requested organizer does not exist **or** you have no permission to change this resource. + +.. http:post:: /api/v1/organizers/(organizer)/giftcards/(id)/transact/ + + Atomically change the value of a gift card. A positive amount will increase the value of the gift card, + a negative amount will decrease it. + + **Example request**: + + .. sourcecode:: http + + PATCH /api/v1/organizers/bigevents/giftcards/1/transact/ HTTP/1.1 + Host: pretix.eu + Accept: application/json, text/javascript + Content-Type: application/json + Content-Length: 94 + + { + "value": "2.00" + } + + **Example response**: + + .. sourcecode:: http + + HTTP/1.1 200 OK + Vary: Accept + Content-Type: application/json + + { + "id": 1, + "secret": "HLBYVELFRC77NCQY", + "currency": "EUR", + "value": "15.37" } :param organizer: The ``slug`` field of the organizer to modify diff --git a/src/pretix/api/views/organizer.py b/src/pretix/api/views/organizer.py index b0b9de0e0a..f3a5508bf6 100644 --- a/src/pretix/api/views/organizer.py +++ b/src/pretix/api/views/organizer.py @@ -1,6 +1,8 @@ from django.db import transaction -from rest_framework import filters, viewsets +from rest_framework import filters, serializers, status, viewsets +from rest_framework.decorators import action from rest_framework.exceptions import MethodNotAllowed, PermissionDenied +from rest_framework.response import Response from pretix.api.models import OAuthAccessToken from pretix.api.serializers.organizer import ( @@ -103,7 +105,7 @@ class GiftCardViewSet(viewsets.ModelViewSet): value = serializer.validated_data.pop('value') inst = serializer.save(issuer=self.request.organizer) inst.transactions.create(value=value) - self.request.organizer.log_action( + inst.log_action( 'pretix.giftcards.transaction.manual', user=self.request.user, auth=self.request.auth, @@ -112,12 +114,13 @@ class GiftCardViewSet(viewsets.ModelViewSet): @transaction.atomic() def perform_update(self, serializer): + GiftCard.objects.select_for_update().get(pk=self.get_object().pk) old_value = serializer.instance.value value = serializer.validated_data.pop('value') inst = serializer.save(secret=serializer.instance.secret, currency=serializer.instance.currency) diff = value - old_value inst.transactions.create(value=diff) - self.request.organizer.log_action( + inst.log_action( 'pretix.giftcards.transaction.manual', user=self.request.user, auth=self.request.auth, @@ -125,5 +128,21 @@ class GiftCardViewSet(viewsets.ModelViewSet): ) return inst + @action(detail=True, methods=["POST"]) + @transaction.atomic() + def transact(self, request, **kwargs): + gc = GiftCard.objects.select_for_update().get(pk=self.get_object().pk) + value = serializers.DecimalField(max_digits=10, decimal_places=2).to_internal_value( + request.data.get('value') + ) + gc.transactions.create(value=value) + gc.log_action( + 'pretix.giftcards.transaction.manual', + user=self.request.user, + auth=self.request.auth, + data={'value': value} + ) + return Response(GiftCardSerializer(gc).data, status=status.HTTP_200_OK) + def perform_destroy(self, instance): raise MethodNotAllowed("Gift cards cannot be deleted.") diff --git a/src/pretix/base/models/giftcards.py b/src/pretix/base/models/giftcards.py index be255904a6..d5b6702da8 100644 --- a/src/pretix/base/models/giftcards.py +++ b/src/pretix/base/models/giftcards.py @@ -6,6 +6,7 @@ from django.db.models import Sum from django.utils.crypto import get_random_string from django.utils.translation import ugettext_lazy as _ +from pretix.base.banlist import banned from pretix.base.models import LoggedModel @@ -13,7 +14,7 @@ def gen_giftcard_secret(): charset = list('ABCDEFGHJKLMNPQRSTUVWXYZ3789') while True: code = get_random_string(length=settings.ENTROPY['giftcard_secret'], allowed_chars=charset) - if not GiftCard.objects.filter(secret=code).exists(): + if not banned(code) and not GiftCard.objects.filter(secret=code).exists(): return code diff --git a/src/pretix/base/services/orders.py b/src/pretix/base/services/orders.py index 63b94f9f89..6417749037 100644 --- a/src/pretix/base/services/orders.py +++ b/src/pretix/base/services/orders.py @@ -590,7 +590,8 @@ def _get_fees(positions: List[CartPosition], payment_provider: BasePaymentProvid def _create_order(event: Event, email: str, positions: List[CartPosition], now_dt: datetime, payment_provider: BasePaymentProvider, locale: str=None, address: InvoiceAddress=None, - meta_info: dict=None, sales_channel: str='web', gift_cards: list=None): + meta_info: dict=None, sales_channel: str='web', gift_cards: list=None, + shown_total=None): p = None with transaction.atomic(): checked_gift_cards = [] @@ -655,6 +656,17 @@ def _create_order(event: Event, email: str, positions: List[CartPosition], now_d p.save() pending_sum -= val + # Safety check: Is the amount we're now going to charge the same amount the user has been shown when they + # pressed "Confirm purchase"? If not, we should better warn the user and show the confirmation page again. + # The only *known* case where this happens is if a gift card is used in two concurrent sessions. + if shown_total is not None: + if Decimal(shown_total) != pending_sum: + raise OrderError( + _('While trying to place your order, we noticed that the order total has changed. Either one of ' + 'the prices changed just now, or a gift card you used has been used in the meantime. Please ' + 'check the prices below and try again.') + ) + if payment_provider and not order.require_approval: p = order.payments.create( state=OrderPayment.PAYMENT_STATE_CREATED, @@ -708,7 +720,7 @@ def _order_placed_email_attendee(event: Event, order: Order, position: OrderPosi def _perform_order(event: Event, payment_provider: str, position_ids: List[str], email: str, locale: str, address: int, meta_info: dict=None, sales_channel: str='web', - gift_cards: list=None): + gift_cards: list=None, shown_total=None): if payment_provider: pprov = event.get_payment_providers().get(payment_provider) if not pprov: @@ -758,7 +770,7 @@ def _perform_order(event: Event, payment_provider: str, position_ids: List[str], _check_positions(event, now_dt, positions, address=addr) order, payment = _create_order(event, email, positions, now_dt, pprov, locale=locale, address=addr, meta_info=meta_info, sales_channel=sales_channel, - gift_cards=gift_cards) + gift_cards=gift_cards, shown_total=shown_total) free_order_flow = payment and payment_provider == 'free' and order.pending_sum == Decimal('0.00') and not order.require_approval if free_order_flow: @@ -1517,12 +1529,12 @@ class OrderChangeManager: @app.task(base=ProfiledEventTask, bind=True, max_retries=5, default_retry_delay=1, throws=(OrderError,)) def perform_order(self, event: Event, payment_provider: str, positions: List[str], email: str=None, locale: str=None, address: int=None, meta_info: dict=None, - sales_channel: str='web', gift_cards: list=None): + sales_channel: str='web', gift_cards: list=None, shown_total=None): with language(locale): try: try: return _perform_order(event, payment_provider, positions, email, locale, address, meta_info, - sales_channel, gift_cards) + sales_channel, gift_cards, shown_total) except LockTimeoutException: self.retry() except (MaxRetriesExceededError, LockTimeoutException): diff --git a/src/pretix/control/views/organizer.py b/src/pretix/control/views/organizer.py index ee36624d35..3d9a5a0b07 100644 --- a/src/pretix/control/views/organizer.py +++ b/src/pretix/control/views/organizer.py @@ -977,7 +977,7 @@ class GiftCardDetailView(OrganizerDetailViewMixin, OrganizerPermissionRequiredMi @transaction.atomic() def post(self, request, *args, **kwargs): - self.object = self.get_object() + self.object = GiftCard.objects.select_for_update().get(pk=self.get_object().pk) if 'value' in request.POST: try: value = DecimalField().to_python(request.POST.get('value')) diff --git a/src/pretix/presale/checkoutflow.py b/src/pretix/presale/checkoutflow.py index a0f9c0c8d0..e35295ed71 100644 --- a/src/pretix/presale/checkoutflow.py +++ b/src/pretix/presale/checkoutflow.py @@ -632,6 +632,8 @@ class ConfirmStep(CartMixin, AsyncAction, TemplateFlowStep): ctx['cart_session'] = self.cart_session ctx['invoice_address_asked'] = self.address_asked + self.cart_session['shown_total'] = str(ctx['cart']['total']) + email = self.cart_session.get('contact_form_data', {}).get('email') if email != settings.PRETIX_EMAIL_NONE_VALUE: ctx['contact_info'] = [ @@ -709,7 +711,8 @@ class ConfirmStep(CartMixin, AsyncAction, TemplateFlowStep): return self.do(self.request.event.id, self.payment_provider.identifier if self.payment_provider else None, [p.id for p in self.positions], self.cart_session.get('email'), translation.get_language(), self.invoice_address.pk, meta_info, - request.sales_channel, self.cart_session.get('gift_cards')) + request.sales_channel, self.cart_session.get('gift_cards'), + self.cart_session['shown_total']) def get_success_message(self, value): create_empty_cart_id(self.request) diff --git a/src/pretix/settings.py b/src/pretix/settings.py index dbc6ccbdcb..3eb6ee1479 100644 --- a/src/pretix/settings.py +++ b/src/pretix/settings.py @@ -241,7 +241,7 @@ ENTROPY = { 'order_code': config.getint('entropy', 'order_code', fallback=5), 'ticket_secret': config.getint('entropy', 'ticket_secret', fallback=32), 'voucher_code': config.getint('entropy', 'voucher_code', fallback=16), - 'giftcard_secret': config.getint('entropy', 'giftcard_secret', fallback=16), + 'giftcard_secret': config.getint('entropy', 'giftcard_secret', fallback=12), } # Internal settings diff --git a/src/tests/api/test_giftcards.py b/src/tests/api/test_giftcards.py index 97cedb7119..012bc8aeb5 100644 --- a/src/tests/api/test_giftcards.py +++ b/src/tests/api/test_giftcards.py @@ -95,6 +95,20 @@ def test_giftcard_patch(token_client, organizer, event, giftcard): assert giftcard.currency == "EUR" +@pytest.mark.django_db +def test_giftcard_transact(token_client, organizer, event, giftcard): + resp = token_client.post( + '/api/v1/organizers/{}/giftcards/{}/transact/'.format(organizer.slug, giftcard.pk), + { + 'value': '10.00', + }, + format='json' + ) + assert resp.status_code == 200 + giftcard.refresh_from_db() + assert giftcard.value == Decimal('33.00') + + @pytest.mark.django_db def test_giftcard_no_deletion(token_client, organizer, event, giftcard): resp = token_client.delete(