Browse Source

Simplify patch error reporting

Rafał Pitoń 7 years ago
parent
commit
2efa34b338

+ 8 - 17
misago/api/patch.py

@@ -64,7 +64,7 @@ class ApiPatch(object):
             try:
             try:
                 self.dispatch_action(response, request, target, action)
                 self.dispatch_action(response, request, target, action)
             except HANDLED_EXCEPTIONS as exception:
             except HANDLED_EXCEPTIONS as exception:
-                return self.handle_exception(exception, path=action['path'])
+                return self.handle_exception(exception)
 
 
         return Response(response)
         return Response(response)
 
 
@@ -81,12 +81,8 @@ class ApiPatch(object):
                 try:
                 try:
                     self.dispatch_action(data['patch'], request, target, action)
                     self.dispatch_action(data['patch'], request, target, action)
                 except HANDLED_EXCEPTIONS as exception:
                 except HANDLED_EXCEPTIONS as exception:
-                    errors, status = self.get_error_data_status(exception, path=action['path'])
-                    data = {'id': target.pk, 'status': status, }
-                    if status == 400:
-                        data['invalid'] = errors
-                    else:
-                        data.update(errors)
+                    data, status = self.get_error_data_status(exception)
+                    data.update({'id': target.pk, 'status': status, })
                     break
                     break
             result.append(data)
             result.append(data)
 
 
@@ -138,24 +134,19 @@ class ApiPatch(object):
                 with transaction.atomic():
                 with transaction.atomic():
                     data.update(handler['handler'](request, target, action['value']))
                     data.update(handler['handler'](request, target, action['value']))
 
 
-    def handle_exception(self, exception, path=None):
-        data, status = self.get_error_data_status(exception, path)
+    def handle_exception(self, exception):
+        data, status = self.get_error_data_status(exception)
         return Response(data, status=status)
         return Response(data, status=status)
 
 
-    def get_error_data_status(self, exception, path=None):
-        if path:
-            # if path is set, we swap comma for underscore
-            # this makes it easier to process errors in frontend
-            path = path.replace('-', '_')
-
+    def get_error_data_status(self, exception):
         if isinstance(exception, InvalidAction):
         if isinstance(exception, InvalidAction):
             return {api_settings.NON_FIELD_ERRORS_KEY: [six.text_type(exception)]}, 400
             return {api_settings.NON_FIELD_ERRORS_KEY: [six.text_type(exception)]}, 400
 
 
         if isinstance(exception, serializers.ValidationError):
         if isinstance(exception, serializers.ValidationError):
-            return {path: exception.detail}, 400
+            return {'value': exception.detail}, 400
 
 
         if isinstance(exception, ValidationError):
         if isinstance(exception, ValidationError):
-            return {path: exception.messages}, 400
+            return {'value': exception.messages}, 400
 
 
         if isinstance(exception, PermissionDenied):
         if isinstance(exception, PermissionDenied):
             return {'detail': six.text_type(exception)}, 403
             return {'detail': six.text_type(exception)}, 403

+ 2 - 48
misago/api/tests/test_patch_dispatch.py

@@ -39,14 +39,6 @@ class ApiPatchDispatchTests(TestCase):
 
 
         patch.replace('error', action_error)
         patch.replace('error', action_error)
 
 
-        def action_custom_path_error(request, target, value):
-            if value == 'invalid':
-                raise ValidationError("invalid data here!")
-            if value == 'api_invalid':
-                raise ApiValidationError("invalid api data here!")
-
-        patch.replace('path-error', action_custom_path_error)
-
         def action_mutate(request, target, value):
         def action_mutate(request, target, value):
             return {'value': value * 2}
             return {'value': value * 2}
 
 
@@ -130,26 +122,7 @@ class ApiPatchDispatchTests(TestCase):
         )
         )
 
 
         self.assertEqual(response.status_code, 400)
         self.assertEqual(response.status_code, 400)
-        self.assertEqual(response.data, {'error': ["invalid data here!"]})
-
-        # op raised validation error in custom path
-        response = patch.dispatch(
-            MockRequest([
-                {
-                    'op': 'replace',
-                    'path': 'mutate',
-                    'value': 6,
-                },
-                {
-                    'op': 'replace',
-                    'path': 'path-error',
-                    'value': 'invalid',
-                },
-            ]), MockObject(13)
-        )
-
-        self.assertEqual(response.status_code, 400)
-        self.assertEqual(response.data, {'path_error': ["invalid data here!"]})
+        self.assertEqual(response.data, {'value': ["invalid data here!"]})
 
 
         # op raised api validation error
         # op raised api validation error
         response = patch.dispatch(
         response = patch.dispatch(
@@ -168,26 +141,7 @@ class ApiPatchDispatchTests(TestCase):
         )
         )
 
 
         self.assertEqual(response.status_code, 400)
         self.assertEqual(response.status_code, 400)
-        self.assertEqual(response.data, {'error': ["invalid api data here!"]})
-
-        # op raised api validation error in custom path
-        response = patch.dispatch(
-            MockRequest([
-                {
-                    'op': 'replace',
-                    'path': 'mutate',
-                    'value': 6,
-                },
-                {
-                    'op': 'replace',
-                    'path': 'path-error',
-                    'value': 'api_invalid',
-                },
-            ]), MockObject(13)
-        )
-
-        self.assertEqual(response.status_code, 400)
-        self.assertEqual(response.data, {'path_error': ["invalid api data here!"]})
+        self.assertEqual(response.data, {'value': ["invalid api data here!"]})
 
 
         # action in dispatch raised perm denied
         # action in dispatch raised perm denied
         response = patch.dispatch(
         response = patch.dispatch(

+ 4 - 58
misago/api/tests/test_patch_dispatch_bulk.py

@@ -39,14 +39,6 @@ class ApiPatchDispatchBulkTests(TestCase):
 
 
         patch.replace('error', action_error)
         patch.replace('error', action_error)
 
 
-        def action_custom_path_error(request, target, value):
-            if value == 'invalid':
-                raise ValidationError("invalid data here!")
-            if value == 'api_invalid':
-                raise ApiValidationError("invalid api data here!")
-
-        patch.replace('path-error', action_custom_path_error)
-
         def action_mutate(request, target, value):
         def action_mutate(request, target, value):
             return {'value': value * 2}
             return {'value': value * 2}
 
 
@@ -138,31 +130,8 @@ class ApiPatchDispatchBulkTests(TestCase):
 
 
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data, [
         self.assertEqual(response.data, [
-            {'id': '5', 'status': '400', 'invalid': {'error': ["invalid data here!"]}},
-            {'id': '7', 'status': '400', 'invalid': {'error': ["invalid data here!"]}},
-        ])
-
-        # op raised api validation error dict in custom path
-        response = patch.dispatch_bulk(
-            MockRequest([
-                {
-                    'op': 'replace',
-                    'path': 'mutate',
-                    'value': 6,
-                },
-                {
-                    'op': 'replace',
-                    'path': 'path-error',
-                    'value': 'api_invalid',
-                },
-            ]),
-            [MockObject(5), MockObject(7)],
-        )
-
-        self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.data, [
-            {'id': '5', 'status': '400', 'invalid': {'path_error': ["invalid api data here!"]}},
-            {'id': '7', 'status': '400', 'invalid': {'path_error': ["invalid api data here!"]}},
+            {'id': '5', 'status': '400', 'value': ["invalid data here!"]},
+            {'id': '7', 'status': '400', 'value': ["invalid data here!"]},
         ])
         ])
 
 
         # op raised api validation error
         # op raised api validation error
@@ -184,31 +153,8 @@ class ApiPatchDispatchBulkTests(TestCase):
 
 
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data, [
         self.assertEqual(response.data, [
-            {'id': '5', 'status': '400', 'invalid': {'error': ["invalid api data here!"]}},
-            {'id': '7', 'status': '400', 'invalid': {'error': ["invalid api data here!"]}},
-        ])
-
-        # op raised api validation error in custom path
-        response = patch.dispatch_bulk(
-            MockRequest([
-                {
-                    'op': 'replace',
-                    'path': 'mutate',
-                    'value': 6,
-                },
-                {
-                    'op': 'replace',
-                    'path': 'path-error',
-                    'value': 'api_invalid',
-                },
-            ]),
-            [MockObject(5), MockObject(7)],
-        )
-
-        self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.data, [
-            {'id': '5', 'status': '400', 'invalid': {'path_error': ["invalid api data here!"]}},
-            {'id': '7', 'status': '400', 'invalid': {'path_error': ["invalid api data here!"]}},
+            {'id': '5', 'status': '400', 'value': ["invalid api data here!"]},
+            {'id': '7', 'status': '400', 'value': ["invalid api data here!"]},
         ])
         ])
 
 
         # action in bulk dispatch raised perm denied
         # action in bulk dispatch raised perm denied