my own review notes

doc/api/resources/checkin.rst

@@ -46,12 +46,15 @@ Checking a ticket in
                         this request twice with the same nonce, the second request will also succeed but will always
                         create only one check-in object even when the previous request was successful as well. This
                         allows for a certain level of idempotency and enables you to re-try after a connection failure.
+   :<json string exchange_medium_type: To perform an exchange to a reusable medium, pass the type of the new reusable medium
+   :<json string exchange_medium_identifier: To perform an exchange to a reusable media, pass the identifier of the new medium
+   :<json string exchange_link_action: To perform an exchange to a reusable media, pass `"append"` or `"replace"` depending on whether any previous ticket links of the medium should be kept
    :<json boolean use_order_locale: Specifies that pretix should use the customer's language (``locale`` field from the
                                     order) when building texts (currently only the ``reason_explanation`` response field).
                                     Defaults to ``false`` in which case the server will determine the language (currently
                                     the event default language, might change in the future with support for the
                                     ``Accept-Language`` header).
-   :>json string status: ``"ok"``, ``"incomplete"``, or ``"error"``
+   :>json string status: ``"ok"``, ``"incomplete"``, ``"exchange"``, or ``"error"``
    :>json string reason: Reason code, only set on status ``"error"``, see below for possible values.
    :>json string reason_explanation: Human-readable explanation, only set on status ``"error"`` and reason ``"rules"``, can be null.
    :>json object position: Copy of the matching order position (if any was found). The contents are the same as the
@@ -67,6 +70,8 @@ Checking a ticket in
    :>json object list: Excerpt of information about the matching :ref:`check-in list <rest-checkinlists>` (if any was found),
                        including the attributes ``id``, ``name``, ``event``, ``subevent``, and ``include_pending``.
    :>json object questions: List of questions to be answered for check-in, only set on status ``"incomplete"``.
+   :>json object media_policy: Reusable media policy (see documentation on items), only set on status ``"exchange"``.
+   :>json object media_type: Reusable media type (see documentation on items), only set on status ``"exchange"``.
 
    **Example request**:
 
@@ -224,6 +229,9 @@ Checking a ticket in
    * ``ambiguous`` - Multiple tickets match scan, rejected.
    * ``revoked`` - Ticket code has been revoked.
    * ``unapproved`` - Order has not yet been approved.
+   * ``already_exchanged`` - Ticket already has been exchanged for a reusable medium that must now be used for check-in.
+   * ``medium_invalid`` - Reusable medium identifier given was not found and could not be automatically created.
+   * ``medium_exists`` - Reusable medium identifier already exists, but expected to be new.
    * ``error`` - Internal error.
 
    In case of reason ``rules`` and ``invalid_time``, there might be an additional response field ``reason_explanation``

doc/api/resources/checkinlists.rst

@@ -602,7 +602,8 @@ Order position endpoints
 
       We no longer recommend using this API if you're building a ticket scanning application, as it has a few design
       flaws that can lead to `security issues`_ or compatibility issues due to barcode content characters that are not
-      URL-safe. We recommend to use our new :ref:`check-in API <rest-checkin>` instead.
+      URL-safe. We recommend to use our new :ref:`check-in API <rest-checkin>` instead. Advanced features like medium
+      exchange are only supported on the new API.
 
    :query boolean untrusted_input: If set to true, the lookup parameter is **always** interpreted as a ``secret``, never
                                    as an ``id``. This should be always set if you are passing through untrusted, scanned
@@ -741,6 +742,9 @@ Order position endpoints
    * ``ambiguous`` - Multiple tickets match scan, rejected.
    * ``revoked`` - Ticket code has been revoked.
    * ``unapproved`` - Order has not yet been approved.
+   * ``already_exchanged`` - Ticket already has been exchanged for a reusable medium that must now be used for check-in.
+   * ``medium_invalid`` - Reusable medium identifier given was not found and could not be automatically created.
+   * ``medium_exists`` - Reusable medium identifier already exists, but expected to be new.
 
    In case of reason ``rules`` or ``invalid_time``, there might be an additional response field ``reason_explanation``
    with a human-readable description of the violated rules. However, that field can also be missing or be ``null``.

src/pretix/api/serializers/checkin.py

@@ -26,7 +26,7 @@ from rest_framework.exceptions import ValidationError
 from pretix.api.serializers.event import SubEventSerializer
 from pretix.api.serializers.i18n import I18nAwareModelSerializer
 from pretix.base.media import MEDIA_TYPES
-from pretix.base.models import Checkin, CheckinList, Item
+from pretix.base.models import Checkin, CheckinList
 
 
 class CheckinListSerializer(I18nAwareModelSerializer):
@@ -88,9 +88,9 @@ class CheckinRPCRedeemInputSerializer(serializers.Serializer):
     nonce = serializers.CharField(required=False, allow_null=True)
     datetime = serializers.DateTimeField(required=False, allow_null=True)
     answers = serializers.JSONField(required=False, allow_null=True)
-    media_type = serializers.ChoiceField(required=False, choices=MEDIA_TYPES)
+    exchange_medium_type = serializers.ChoiceField(required=False, choices=MEDIA_TYPES)
-    media_identifier = serializers.CharField(required=False)
+    exchange_medium_identifier = serializers.CharField(required=False)
-    media_action = serializers.ChoiceField(required=False, choices=[
+    exchange_link_action = serializers.ChoiceField(required=False, choices=[
         ('append', 'append'),
         ('replace', 'replace'),
     ])

src/pretix/api/views/checkin.py

@@ -457,7 +457,7 @@ def _checkin_list_position_queryset(checkinlists, ignore_status=False, ignore_pr
 def _redeem_process(*, checkinlists, raw_barcode, answers_data, datetime, force, checkin_type, ignore_unpaid, nonce,
                     untrusted_input, user, auth, expand, pdf_data, request, questions_supported, canceled_supported,
                     source_type='barcode', legacy_url_support=False, simulate=False, gate=None, use_order_locale=False,
-                    media_type=None, media_identifier=None, media_action=None):
+                    exchange_medium_type=None, exchange_medium_identifier=None, exchange_link_action=None):
     if not checkinlists:
         raise ValidationError('No check-in list passed.')
 
@@ -526,7 +526,7 @@ def _redeem_process(*, checkinlists, raw_barcode, answers_data, datetime, force,
     #    with respecting the force option), or it's a reusable medium (-> proceed with that)
     if not op_candidates:
         try:
-            media = ReusableMedium.objects.active().filter(
+            medium = ReusableMedium.objects.active().filter(
                 Exists(ReusableMedium.linked_orderpositions.through.objects.filter(reusablemedium_id=OuterRef('pk')))
             ).get(
                 organizer_id=checkinlists[0].event.organizer_id,
@@ -634,7 +634,7 @@ def _redeem_process(*, checkinlists, raw_barcode, answers_data, datetime, force,
                     'list': MiniCheckinListSerializer(list_by_event[revoked_matches[0].event_id]).data,
                 }, status=400)
         else:
-            linked_ops = media.linked_orderpositions.all().select_related("order").prefetch_related("addons")
+            linked_ops = medium.linked_orderpositions.all().select_related("order").prefetch_related("addons")
             linked_event_ids = {op.order.event_id for op in linked_ops}
             if not any(event_id in list_by_event for event_id in linked_event_ids):
                 # Medium exists but connected ticket is for the wrong event
@@ -665,7 +665,7 @@ def _redeem_process(*, checkinlists, raw_barcode, answers_data, datetime, force,
             op_candidates = []
             for op in linked_ops:
                 if op.order.event_id in list_by_event:
-                    reusable_medium_used = media
+                    reusable_medium_used = medium
                     op_candidates.append(op)
                     if list_by_event[op.order.event_id].addon_match:
                         op_candidates += list(op.addons.all())
@@ -805,58 +805,56 @@ def _redeem_process(*, checkinlists, raw_barcode, answers_data, datetime, force,
         locale = op.order.event.settings.locale
     with language(locale):
         try:
-            if all(k is not None for k in [media_type, media_identifier, media_action]) and not media:
+            exchange_requested = any(k is not None for k in [exchange_medium_type, exchange_medium_identifier, exchange_link_action])
-                with transaction.atomic():
-                    media = perform_media_exchange(
-                        organizer=request.organizer,
-                        media_type=media_type,
-                        media_identifier=media_identifier,
-                        media_action=media_action,
-                        op=op,
-                    )
-                    source_type = media.media_type.identifier
 
-                    perform_checkin(
+            if exchange_requested:
-                        op=op,
+                if any(k is None for k in [exchange_medium_type, exchange_medium_identifier, exchange_link_action]):
-                        clist=list_by_event[op.order.event_id],
+                    raise ValidationError("If you set any of exchange_medium_type, exchange_medium_identifier, or "
-                        given_answers=given_answers,
+                                          "èxchange_link_action, you need to set all of them.")
-                        force=force,
+                if medium:
-                        ignore_unpaid=ignore_unpaid,
+                    # Cannot scan a medium and then request to exchange it
-                        nonce=nonce,
+                    raise ReusableMedium.DuplicateEntry()
-                        datetime=datetime,
+
-                        questions_supported=questions_supported,
+            checkin_args = dict(
-                        canceled_supported=canceled_supported,
+                op=op,
+                clist=list_by_event[op.order.event_id],
+                given_answers=given_answers,
+                force=force,
+                ignore_unpaid=ignore_unpaid,
+                nonce=nonce,
+                datetime=datetime,
+                questions_supported=questions_supported,
+                canceled_supported=canceled_supported,
+                user=user,
+                auth=auth,
+                type=checkin_type,
+                raw_barcode=raw_barcode_for_checkin,
+                raw_source_type=source_type,
+                from_revoked_secret=from_revoked_secret,
+                simulate=simulate,
+                gate=gate,
+                reusable_medium=medium,
+            )
+
+            if exchange_requested:
+                with transaction.atomic():
+                    # Do exchange and check-in atomically, i.e. both succeed or both fail
+                    medium = perform_media_exchange(
+                        organizer=request.organizer,
+                        media_type=exchange_medium_type,
+                        identifier=exchange_medium_identifier,
+                        link_action=exchange_link_action,
+                        link_orderposition=op,
                         user=user,
                         auth=auth,
-                        type=checkin_type,
+                    )
-                        raw_barcode=raw_barcode_for_checkin,
+                    source_type = media.media_type.identifier
-                        raw_source_type=source_type,
+                    checkin_args['medium'] = medium
-                        from_revoked_secret=from_revoked_secret,
+                    perform_checkin(
-                        simulate=simulate,
-                        gate=gate,
                         reusable_media=media,
                     )
             else:
-                perform_checkin(
+                perform_checkin(**checkin_args)
-                    op=op,
-                    clist=list_by_event[op.order.event_id],
-                    given_answers=given_answers,
-                    force=force,
-                    ignore_unpaid=ignore_unpaid,
-                    nonce=nonce,
-                    datetime=datetime,
-                    questions_supported=questions_supported,
-                    canceled_supported=canceled_supported,
-                    user=user,
-                    auth=auth,
-                    type=checkin_type,
-                    raw_barcode=raw_barcode_for_checkin,
-                    raw_source_type=source_type,
-                    from_revoked_secret=from_revoked_secret,
-                    simulate=simulate,
-                    gate=gate,
-                    reusable_media=media,
-                )
         except RequiredQuestionsError as e:
             return Response({
                 'status': 'incomplete',
@@ -879,11 +877,21 @@ def _redeem_process(*, checkinlists, raw_barcode, answers_data, datetime, force,
                 'list': MiniCheckinListSerializer(list_by_event[op.order.event_id]).data,
                 'reason_explanation': e.msg,
             }, status=400)
+        except ReusableMedium.DoesNotExist:
+            return Response({
+                'status': 'error',
+                'reason': Checkin.REASON_MEDIUM_INVALID,
+                'reason_explanation': 'Reusable medium identifier not found',
+                'require_attention': op.require_checkin_attention,
+                'checkin_texts': op.checkin_texts,
+                'position': CheckinListOrderPositionSerializer(op, context=_make_context(context, op.order.event)).data,
+                'list': MiniCheckinListSerializer(list_by_event[op.order.event_id]).data,
+            }, status=400)
         except ReusableMedium.DuplicateEntry:
             return Response({
                 'status': 'error',
-                'reason': Checkin.REASON_AMBIGUOUS,
+                'reason': Checkin.REASON_MEDIUM_EXISTS,
-                'reason_explanation': 'Reusable medium identifier is ambigous',
+                'reason_explanation': 'Reusable medium identifier already exists',
                 'require_attention': op.require_checkin_attention,
                 'checkin_texts': op.checkin_texts,
                 'position': CheckinListOrderPositionSerializer(op, context=_make_context(context, op.order.event)).data,
@@ -1076,9 +1084,9 @@ class CheckinRPCRedeemView(views.APIView):
             canceled_supported=True,
             request=self.request,  # this is not clean, but we need it in the serializers for URL generation
             legacy_url_support=False,
-            media_type=s.validated_data.get('media_type'),
+            exchange_medium_type=s.validated_data.get('exchange_medium_type'),
-            media_identifier=s.validated_data.get('media_identifier'),
+            exchange_medium_identifier=s.validated_data.get('exchange_medium_identifier'),
-            media_action=s.validated_data.get('media_action'),
+            exchange_link_action=s.validated_data.get('exchange_link_action'),
         )
 
 

src/pretix/base/models/checkin.py

@@ -346,6 +346,8 @@ class Checkin(models.Model):
     REASON_INCOMPLETE = 'incomplete'
     REASON_ALREADY_REDEEMED = 'already_redeemed'
     REASON_AMBIGUOUS = 'ambiguous'
+    REASON_MEDIUM_INVALID = 'medium_invalid'
+    REASON_MEDIUM_EXISTS = 'medium_exists'
     REASON_ERROR = 'error'
     REASON_BLOCKED = 'blocked'
     REASON_UNAPPROVED = 'unapproved'
@@ -368,6 +370,8 @@ class Checkin(models.Model):
         (REASON_INVALID_TIME, _('Ticket not valid at this time')),
         (REASON_ANNULLED, _('Check-in annulled')),
         (REASON_ALREADY_EXCHANGED, _('Ticket already exchanged')),
+        (REASON_MEDIUM_INVALID, _('Reusable medium invalid')),
+        (REASON_MEDIUM_EXISTS, _('Reusable medium already exists')),
     )
 
     successful = models.BooleanField(

src/pretix/base/services/checkin.py

@@ -948,7 +948,7 @@ def perform_checkin(op: OrderPosition, clist: CheckinList, given_answers: dict,
                     ignore_unpaid=False, nonce=None, datetime=None, questions_supported=True,
                     user=None, auth=None, canceled_supported=False, type=Checkin.TYPE_ENTRY,
                     raw_barcode=None, raw_source_type=None, from_revoked_secret=False, simulate=False,
-                    gate=None, reusable_media=None):
+                    gate=None, reusable_medium=None):
     """
     Create a checkin for this particular order position and check-in list. Fails with CheckInError if the check in is
     not valid at this time.
@@ -964,6 +964,7 @@ def perform_checkin(op: OrderPosition, clist: CheckinList, given_answers: dict,
     :param datetime: The datetime of the checkin, defaults to now.
     :param simulate: If true, the check-in is not saved.
     :param gate: The gate the check-in was performed at.
+    :param reusable_medium: The medium that is available for an exchange
     """
 
     # !!!!!!!!!
@@ -1112,8 +1113,9 @@ def perform_checkin(op: OrderPosition, clist: CheckinList, given_answers: dict,
 
         required_media_policy = op.item.media_policy
         required_media_type = op.item.media_type
+        require_a_medium = required_media_policy and required_media_type
         linked_media = op.linked_media
-        if not reusable_media and required_media_policy and required_media_type and not force:
+        if require_a_medium and not reusable_medium and not force:
             if not linked_media.exists():
                 raise RequiredMediaExchangeError(
                     _('Ticket needs to be exchanged to a suitable medium.'),

src/pretix/base/services/media.py

@@ -72,35 +72,62 @@ def get_keysets_for_organizer(organizer):
     return sets
 
 
-def perform_media_exchange(organizer, media_type, media_identifier, media_action, op):
+def perform_media_exchange(organizer, media_type, identifier, link_action, link_orderposition, user, auth):
+    """
+    Create or retrieve a medium
+
+    :param organizer: Organizer to operate in
+    :param media_type: Type of medium to operate with
+    :param identifier: Identifier of the medium
+    :param link_action: one of `"append"` or `"replace"`
+    :param link_orderposition: Position to link to the medium
+    :return: ReusableMedium
+    """
     medium = None
-    media_policy = op.item.media_policy
+    media_policy = link_orderposition.item.media_policy
+    if link_action not in ('append', 'replace'):
+        raise ValueError("Invalid link_action")
 
-    if media_policy in [Item.MEDIA_POLICY_REUSE, Item.MEDIA_POLICY_REUSE_OR_NEW]:
+    if media_policy == Item.MEDIA_POLICY_REUSE:
-        try:
+        # Will and should raise ReusableMedium.DoesNotExist if not found
-            medium = ReusableMedium.objects.get(
+        medium = ReusableMedium.objects.get(
-                type=media_type,
+            type=media_type,
-                identifier=media_identifier,
+            identifier=identifier,
-                organizer=organizer,
+            organizer=organizer,
+        )
+
+    elif media_policy == Item.MEDIA_POLICY_REUSE_OR_NEW:
+        medium, created = ReusableMedium.objects.get_or_create(
+            type=media_type,
+            identifier=identifier,
+            organizer=organizer,
+        )
+        if created:
+            medium.log_action(
+                'pretix.reusable_medium.created.auto',
+                user=user,
+                auth=auth,
             )
-        except ReusableMedium.DoesNotExist:
-            pass
 
-    if not medium and media_policy in [Item.MEDIA_POLICY_NEW, Item.MEDIA_POLICY_REUSE_OR_NEW]:
+    elif media_policy == Item.MEDIA_POLICY_NEW:
         try:
             medium = ReusableMedium.objects.create(
                 type=media_type,
-                identifier=media_identifier,
+                identifier=identifier,
                 organizer=organizer,
             )
         except IntegrityError:
             raise ReusableMedium.DuplicateEntry()
+        else:
+            medium.log_action(
+                'pretix.reusable_medium.created.auto',
+                user=user,
+                auth=auth,
+            )
 
-    if medium:
+    if link_action == 'append':
-        if media_action == 'append':
+        medium.linked_orderpositions.add(link_orderposition)
-            medium.linked_orderpositions.add(*[op])
+    elif link_action == 'replace':
-        elif media_action == 'replace':
+        medium.linked_orderpositions.set([link_orderposition])
-            medium.linked_orderpositions.set([op])
-        medium.save()
 
     return medium