Orders API: Improve validation errors

src/pretix/api/serializers/cart.py

@@ -50,6 +50,12 @@ class CartPositionCreateSerializer(I18nAwareModelSerializer):
             new_quotas = (validated_data.get('variation').quotas.filter(subevent=validated_data.get('subevent'))
                           if validated_data.get('variation')
                           else validated_data.get('item').quotas.filter(subevent=validated_data.get('subevent')))
+            if len(new_quotas) == 0:
+                raise ValidationError(
+                    ugettext_lazy('The product "{}" is not assigned to a quota.').format(
+                        str(validated_data.get('item'))
+                    )
+                )
             for quota in new_quotas:
                 avail = quota.availability()
                 if avail[0] != Quota.AVAILABILITY_OK or (avail[1] is not None and avail[1] < 1):

src/pretix/api/serializers/order.py

@@ -3,6 +3,7 @@ from collections import Counter
 from decimal import Decimal
 
 from django.utils.timezone import now
+from django.utils.translation import ugettext_lazy
 from django_countries.fields import Country
 from rest_framework import serializers
 from rest_framework.exceptions import ValidationError
@@ -11,7 +12,7 @@ from rest_framework.reverse import reverse
 from pretix.api.serializers.i18n import I18nAwareModelSerializer
 from pretix.base.models import (
     Checkin, Invoice, InvoiceAddress, InvoiceLine, Order, OrderPosition,
-    Question, QuestionAnswer, Quota,
+    Question, QuestionAnswer,
 )
 from pretix.base.models.orders import CartPosition, OrderFee
 from pretix.base.pdf import get_variables
@@ -298,15 +299,15 @@ class OrderPositionCreateSerializer(I18nAwareModelSerializer):
         if data.get('item'):
             if data.get('item').has_variations:
                 if not data.get('variation'):
-                    raise ValidationError('You should specify a variation for this item.')
+                    raise ValidationError({'variation': ['You should specify a variation for this item.']})
                 else:
                     if data.get('variation').item != data.get('item'):
                         raise ValidationError(
-                            'The specified variation does not belong to the specified item.'
+                            {'variation': ['The specified variation does not belong to the specified item.']}
                         )
             elif data.get('variation'):
                 raise ValidationError(
-                    'You cannot specify a variation for this item.'
+                    {'variation': ['You cannot specify a variation for this item.']}
                 )
         return data
 
@@ -368,28 +369,42 @@ class OrderCreateSerializer(I18nAwareModelSerializer):
             raise ValidationError(
                 'An order cannot be empty.'
             )
+        errs = [{} for p in data]
         if any([p.get('positionid') for p in data]):
             if not all([p.get('positionid') for p in data]):
-                raise ValidationError(
+                for i, p in enumerate(data):
-                    'If you set position IDs manually, you need to do so for all positions.'
+                    if not p.get('positionid'):
-                )
+                        errs[i]['positionid'] = [
+                            'If you set position IDs manually, you need to do so for all positions.'
+                        ]
+                raise ValidationError(errs)
 
             last_non_add_on = None
             last_posid = 0
 
-            for p in data:
+            for i, p in enumerate(data):
                 if p['positionid'] != last_posid + 1:
-                    raise ValidationError("Position IDs need to be consecutive.")
+                    errs[i]['positionid'] = [
+                        'Position IDs need to be consecutive.'
+                    ]
                 if p.get('addon_to') and p['addon_to'] != last_non_add_on:
-                    raise ValidationError("If you set addon_to, you need to make sure that the referenced "
+                    errs[i]['addon_to'] = [
-                                          "position ID exists and is transmitted directly before its add-ons.")
+                        "If you set addon_to, you need to make sure that the referenced "
+                        "position ID exists and is transmitted directly before its add-ons."
+                    ]
 
                 if not p.get('addon_to'):
                     last_non_add_on = p['positionid']
                 last_posid = p['positionid']
 
         elif any([p.get('addon_to') for p in data]):
-            raise ValidationError("If you set addon_to, you need to specify position IDs manually.")
+            errs = [
+                {'positionid': ["If you set addon_to on any position, you need to specify position IDs manually."]}
+                for p in data
+            ]
+
+        if any(errs):
+            raise ValidationError(errs)
         return data
 
     def create(self, validated_data):
@@ -405,29 +420,48 @@ class OrderCreateSerializer(I18nAwareModelSerializer):
 
             consume_carts = validated_data.pop('consume_carts', [])
             delete_cps = []
+            quota_avail_cache = {}
             if consume_carts:
                 for cp in CartPosition.objects.filter(event=self.context['event'], cart_id__in=consume_carts):
                     quotas = (cp.variation.quotas.filter(subevent=cp.subevent)
                               if cp.variation else cp.item.quotas.filter(subevent=cp.subevent))
+                    for quota in quotas:
+                        if quota not in quota_avail_cache:
+                            quota_avail_cache[quota] = list(quota.availability())
+                        if quota_avail_cache[quota][1] is not None:
+                            quota_avail_cache[quota][1] += 1
                     if cp.expires > now_dt:
                         quotadiff.subtract(quotas)
                     delete_cps.append(cp)
 
-            for pos_data in positions_data:
+            errs = [{} for p in positions_data]
+
+            for i, pos_data in enumerate(positions_data):
                 new_quotas = (pos_data.get('variation').quotas.filter(subevent=pos_data.get('subevent'))
                               if pos_data.get('variation')
                               else pos_data.get('item').quotas.filter(subevent=pos_data.get('subevent')))
+                if len(new_quotas) == 0:
+                    errs[i]['item'] = [ugettext_lazy('The product "{}" is not assigned to a quota.').format(
+                        str(pos_data.get('item'))
+                    )]
+                else:
+                    for quota in new_quotas:
+                        if quota not in quota_avail_cache:
+                            quota_avail_cache[quota] = list(quota.availability())
+
+                        if quota_avail_cache[quota][1] is not None:
+                            quota_avail_cache[quota][1] -= 1
+                            if quota_avail_cache[quota][1] < 0:
+                                errs[i]['item'] = [
+                                    ugettext_lazy('There is not enough quota available on quota "{}" to perform the operation.').format(
+                                        quota.name
+                                    )
+                                ]
+
                 quotadiff.update(new_quotas)
 
-            for quota, diff in quotadiff.items():
+            if any(errs):
-                if diff > 0:
+                raise ValidationError({'positions': errs})
-                    avail = quota.availability()
-                    if avail[0] != Quota.AVAILABILITY_OK or (avail[1] is not None and avail[1] < diff):
-                        raise ValidationError(
-                            'There is not enough quota available on quota "{}" to perform the operation.'.format(
-                                quota.name
-                            )
-                        )
 
             order = Order(event=self.context['event'], **validated_data)
             order.set_expires(subevents=[p['subevent'] for p in positions_data])

src/tests/api/test_cart.py

@@ -276,6 +276,15 @@ def test_cartpos_create_item_validation(token_client, organizer, event, item, it
             organizer.slug, event.slug
         ), format='json', data=res
     )
+    assert resp.status_code == 400
+    assert resp.data == ['The product "Budget Ticket" is not assigned to a quota.']
+
+    quota.variations.add(var1)
+    resp = token_client.post(
+        '/api/v1/organizers/{}/events/{}/cartpositions/'.format(
+            organizer.slug, event.slug
+        ), format='json', data=res
+    )
     assert resp.status_code == 201
 
     res['variation'] = var2.pk

src/tests/api/test_orders.py

@@ -189,7 +189,8 @@ def test_order_list(token_client, organizer, event, order, item, taxrule, questi
     assert [] == resp.data['results']
 
     resp = token_client.get('/api/v1/organizers/{}/events/{}/orders/?modified_since={}'.format(
-        organizer.slug, event.slug, (order.last_modified - datetime.timedelta(hours=1)).isoformat().replace('+00:00', 'Z')
+        organizer.slug, event.slug,
+        (order.last_modified - datetime.timedelta(hours=1)).isoformat().replace('+00:00', 'Z')
     ))
     assert [res] == resp.data['results']
     resp = token_client.get('/api/v1/organizers/{}/events/{}/orders/?modified_since={}'.format(
@@ -197,7 +198,8 @@ def test_order_list(token_client, organizer, event, order, item, taxrule, questi
     ))
     assert [res] == resp.data['results']
     resp = token_client.get('/api/v1/organizers/{}/events/{}/orders/?modified_since={}'.format(
-        organizer.slug, event.slug, (order.last_modified + datetime.timedelta(hours=1)).isoformat().replace('+00:00', 'Z')
+        organizer.slug, event.slug,
+        (order.last_modified + datetime.timedelta(hours=1)).isoformat().replace('+00:00', 'Z')
     ))
     assert [] == resp.data['results']
 
@@ -1141,6 +1143,7 @@ def test_order_create_item_validation(token_client, organizer, event, item, item
     assert resp.data == {'positions': [{'item': ['The specified item does not belong to this event.']}]}
 
     var2 = item2.variations.create(value="A")
+    quota.variations.add(var2)
 
     res['positions'][0]['item'] = item.pk
     res['positions'][0]['variation'] = var2.pk
@@ -1150,7 +1153,7 @@ def test_order_create_item_validation(token_client, organizer, event, item, item
         ), format='json', data=res
     )
     assert resp.status_code == 400
-    assert resp.data == {'positions': [{'non_field_errors': ['You cannot specify a variation for this item.']}]}
+    assert resp.data == {'positions': [{'variation': ['You cannot specify a variation for this item.']}]}
 
     var1 = item.variations.create(value="A")
     res['positions'][0]['item'] = item.pk
@@ -1160,6 +1163,15 @@ def test_order_create_item_validation(token_client, organizer, event, item, item
             organizer.slug, event.slug
         ), format='json', data=res
     )
+    assert resp.status_code == 400
+    assert resp.data == {'positions': [{'item': ['The product "Budget Ticket" is not assigned to a quota.']}]}
+
+    quota.variations.add(var1)
+    resp = token_client.post(
+        '/api/v1/organizers/{}/events/{}/orders/'.format(
+            organizer.slug, event.slug
+        ), format='json', data=res
+    )
     assert resp.status_code == 201
 
     res['positions'][0]['variation'] = var2.pk
@@ -1169,7 +1181,8 @@ def test_order_create_item_validation(token_client, organizer, event, item, item
         ), format='json', data=res
     )
     assert resp.status_code == 400
-    assert resp.data == {'positions': [{'non_field_errors': ['The specified variation does not belong to the specified item.']}]}
+    assert resp.data == {
+        'positions': [{'variation': ['The specified variation does not belong to the specified item.']}]}
 
     res['positions'][0]['variation'] = None
     resp = token_client.post(
@@ -1178,7 +1191,7 @@ def test_order_create_item_validation(token_client, organizer, event, item, item
         ), format='json', data=res
     )
     assert resp.status_code == 400
-    assert resp.data == {'positions': [{'non_field_errors': ['You should specify a variation for this item.']}]}
+    assert resp.data == {'positions': [{'variation': ['You should specify a variation for this item.']}]}
 
 
 @pytest.mark.django_db
@@ -1253,9 +1266,18 @@ def test_order_create_positionid_validation(token_client, organizer, event, item
         ), format='json', data=res
     )
     assert resp.status_code == 400
-    assert resp.data == {'positions': ['If you set addon_to, you need to make sure that the '
+    assert resp.data == {
-                                       'referenced position ID exists and is transmitted directly '
+        'positions': [
-                                       'before its add-ons.']}
+            {},
+            {
+                'addon_to': [
+                    'If you set addon_to, you need to make sure that the '
+                    'referenced position ID exists and is transmitted directly '
+                    'before its add-ons.'
+                ]
+            }
+        ]
+    }
 
     res['positions'] = [
         {
@@ -1285,7 +1307,10 @@ def test_order_create_positionid_validation(token_client, organizer, event, item
         ), format='json', data=res
     )
     assert resp.status_code == 400
-    assert resp.data == {'positions': ['If you set addon_to, you need to specify position IDs manually.']}
+    assert resp.data == {'positions': [
+        {'positionid': ["If you set addon_to on any position, you need to specify position IDs manually."]},
+        {'positionid': ["If you set addon_to on any position, you need to specify position IDs manually."]}
+    ]}
 
     res['positions'] = [
         {
@@ -1314,7 +1339,14 @@ def test_order_create_positionid_validation(token_client, organizer, event, item
         ), format='json', data=res
     )
     assert resp.status_code == 400
-    assert resp.data == {'positions': ['If you set position IDs manually, you need to do so for all positions.']}
+    assert resp.data == {
+        'positions': [
+            {},
+            {
+                'positionid': ['If you set position IDs manually, you need to do so for all positions.']
+            }
+        ]
+    }
 
     res['positions'] = [
         {
@@ -1344,7 +1376,14 @@ def test_order_create_positionid_validation(token_client, organizer, event, item
         ), format='json', data=res
     )
     assert resp.status_code == 400
-    assert resp.data == {'positions': ['Position IDs need to be consecutive.']}
+    assert resp.data == {
+        'positions': [
+            {},
+            {
+                'positionid': ['Position IDs need to be consecutive.']
+            }
+        ]
+    }
 
 
 @pytest.mark.django_db
@@ -1358,7 +1397,8 @@ def test_order_create_answer_validation(token_client, organizer, event, item, qu
         ), format='json', data=res
     )
     assert resp.status_code == 400
-    assert resp.data == {'positions': [{'answers': [{'question': ['The specified question does not belong to this event.']}]}]}
+    assert resp.data == {
+        'positions': [{'answers': [{'question': ['The specified question does not belong to this event.']}]}]}
 
     res['positions'][0]['answers'][0]['question'] = question.pk
     res['positions'][0]['answers'][0]['options'] = [question.options.first().pk]
@@ -1368,7 +1408,8 @@ def test_order_create_answer_validation(token_client, organizer, event, item, qu
         ), format='json', data=res
     )
     assert resp.status_code == 400
-    assert resp.data == {'positions': [{'answers': [{'non_field_errors': ['You should not specify options if the question is not of a choice type.']}]}]}
+    assert resp.data == {'positions': [{'answers': [
+        {'non_field_errors': ['You should not specify options if the question is not of a choice type.']}]}]}
 
     question.type = Question.TYPE_CHOICE
     question.save()
@@ -1379,7 +1420,8 @@ def test_order_create_answer_validation(token_client, organizer, event, item, qu
         ), format='json', data=res
     )
     assert resp.status_code == 400
-    assert resp.data == {'positions': [{'answers': [{'non_field_errors': ['You need to specify options if the question is of a choice type.']}]}]}
+    assert resp.data == {'positions': [
+        {'answers': [{'non_field_errors': ['You need to specify options if the question is of a choice type.']}]}]}
 
     question.options.create(answer="L")
     res['positions'][0]['answers'][0]['options'] = [
@@ -1392,7 +1434,8 @@ def test_order_create_answer_validation(token_client, organizer, event, item, qu
         ), format='json', data=res
     )
     assert resp.status_code == 400
-    assert resp.data == {'positions': [{'answers': [{'non_field_errors': ['You can specify at most one option for this question.']}]}]}
+    assert resp.data == {
+        'positions': [{'answers': [{'non_field_errors': ['You can specify at most one option for this question.']}]}]}
 
     question.type = Question.TYPE_FILE
     question.save()
@@ -1402,7 +1445,8 @@ def test_order_create_answer_validation(token_client, organizer, event, item, qu
         ), format='json', data=res
     )
     assert resp.status_code == 400
-    assert resp.data == {'positions': [{'answers': [{'non_field_errors': ['File uploads are currently not supported via the API.']}]}]}
+    assert resp.data == {
+        'positions': [{'answers': [{'non_field_errors': ['File uploads are currently not supported via the API.']}]}]}
 
     question.type = Question.TYPE_CHOICE_MULTIPLE
     question.save()
@@ -1487,7 +1531,8 @@ def test_order_create_answer_validation(token_client, organizer, event, item, qu
         ), format='json', data=res
     )
     assert resp.status_code == 400
-    assert resp.data == {'positions': [{'answers': [{'non_field_errors': ['Please specify "true" or "false" for boolean questions.']}]}]}
+    assert resp.data == {
+        'positions': [{'answers': [{'non_field_errors': ['Please specify "true" or "false" for boolean questions.']}]}]}
 
     question.type = Question.TYPE_DATE
     question.save()
@@ -1512,7 +1557,8 @@ def test_order_create_answer_validation(token_client, organizer, event, item, qu
         ), format='json', data=res
     )
     assert resp.status_code == 400
-    assert resp.data == {'positions': [{'answers': [{'non_field_errors': ['Date has wrong format. Use one of these formats instead: YYYY[-MM[-DD]].']}]}]}
+    assert resp.data == {'positions': [{'answers': [
+        {'non_field_errors': ['Date has wrong format. Use one of these formats instead: YYYY[-MM[-DD]].']}]}]}
 
     question.type = Question.TYPE_DATETIME
     question.save()
@@ -1564,27 +1610,13 @@ def test_order_create_answer_validation(token_client, organizer, event, item, qu
         ), format='json', data=res
     )
     assert resp.status_code == 400
-    assert resp.data == {'positions': [{'answers': [{'non_field_errors': ['Time has wrong format. Use one of these formats instead: hh:mm[:ss[.uuuuuu]].']}]}]}
+    assert resp.data == {'positions': [{'answers': [
+        {'non_field_errors': ['Time has wrong format. Use one of these formats instead: hh:mm[:ss[.uuuuuu]].']}]}]}
 
 
 @pytest.mark.django_db
 def test_order_create_quota_validation(token_client, organizer, event, item, quota, question):
     res = copy.deepcopy(ORDER_CREATE_PAYLOAD)
-    res['positions'][0]['item'] = item.pk
-    res['positions'][0]['answers'][0]['question'] = question.pk
-
-    quota.size = 0
-    quota.save()
-    resp = token_client.post(
-        '/api/v1/organizers/{}/events/{}/orders/'.format(
-            organizer.slug, event.slug
-        ), format='json', data=res
-    )
-    assert resp.status_code == 400
-    assert resp.data == ['There is not enough quota available on quota "Budget Quota" to perform the operation.']
-
-    quota.size = 1
-    quota.save()
     res['positions'] = [
         {
             "positionid": 1,
@@ -1609,13 +1641,36 @@ def test_order_create_quota_validation(token_client, organizer, event, item, quo
             "subevent": None
         }
     ]
+
+    quota.size = 0
+    quota.save()
     resp = token_client.post(
         '/api/v1/organizers/{}/events/{}/orders/'.format(
             organizer.slug, event.slug
         ), format='json', data=res
     )
     assert resp.status_code == 400
-    assert resp.data == ['There is not enough quota available on quota "Budget Quota" to perform the operation.']
+    assert resp.data == {
+        'positions': [
+            {'item': ['There is not enough quota available on quota "Budget Quota" to perform the operation.']},
+            {'item': ['There is not enough quota available on quota "Budget Quota" to perform the operation.']},
+        ]
+    }
+
+    quota.size = 1
+    quota.save()
+    resp = token_client.post(
+        '/api/v1/organizers/{}/events/{}/orders/'.format(
+            organizer.slug, event.slug
+        ), format='json', data=res
+    )
+    assert resp.status_code == 400
+    assert resp.data == {
+        'positions': [
+            {},
+            {'item': ['There is not enough quota available on quota "Budget Quota" to perform the operation.']},
+        ]
+    }
 
 
 @pytest.mark.django_db
@@ -1638,7 +1693,11 @@ def test_order_create_quota_consume_cart(token_client, organizer, event, item, q
         ), format='json', data=res
     )
     assert resp.status_code == 400
-    assert resp.data == ['There is not enough quota available on quota "Budget Quota" to perform the operation.']
+    assert resp.data == {
+        'positions': [
+            {'item': ['There is not enough quota available on quota "Budget Quota" to perform the operation.']},
+        ]
+    }
 
     res['consume_carts'] = [cr.cart_id]
     resp = token_client.post(