Redo patch.dispatch_bulk to follow new bulk actions convention

misago/api/patch.py

@@ -79,19 +79,26 @@ class ApiPatch(object):
                 return Response({'detail': detail}, status=status)
 
         for target in targets:
-            patch = {'id': target.pk, 'status': 200}
+            data = {'id': target.pk, 'status': 200, 'patch': {}}
             for action in request.data['ops']:
                 try:
-                    self.dispatch_action(patch, request, target, action)
+                    self.dispatch_action(data['patch'], request, target, action)
                 except HANDLED_EXCEPTIONS as exception:
                     detail, status = self.get_error_detail_code(exception)
-                    patch = {
+                    data = {
                         'id': target.pk,
-                        'detail': detail,
                         'status': status,
+                        'detail': detail,
                     }
                     break
-            result.append(patch)
+            result.append(data)
+
+        # sort result items by id then cast id and status to string
+        # so we are compliant with our bulk actions spec
+        result.sort(key=lambda item: item['id'])
+        for data in result:
+            data['id'] = str(data['id'])
+            data['status'] = str(data['status'])
 
         # always returning 200 on op error saves us logic duplication
         # in the frontend, were we need to do success handling in both
@@ -111,11 +118,11 @@ class ApiPatch(object):
         if 'value' not in action:
             raise InvalidAction(_('"%s" op has to specify value.') % action.get('op'))
 
-    def dispatch_action(self, patch, request, target, action):
+    def dispatch_action(self, data, request, target, action):
         for handler in self._actions:
             if action['op'] == handler['op'] and action['path'] == handler['path']:
                 with transaction.atomic():
-                    patch.update(handler['handler'](request, target, action['value']))
+                    data.update(handler['handler'](request, target, action['value']))
 
     def get_error_detail_code(self, exception):
         if isinstance(exception, InvalidAction):

misago/api/tests/test_patch_dispatch_bulk.py

@@ -68,8 +68,8 @@ class ApiPatchDispatchBulkTests(TestCase):
 
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data, [
-            {'id': 5, 'value': 14, 'status': 200},
-            {'id': 7, 'value': 14, 'status': 200},
+            {'id': '5', 'status': '200', 'patch': {'value': 14}},
+            {'id': '7', 'status': '200', 'patch': {'value': 14}},
         ])
 
         # invalid action in bulk dispatch
@@ -98,7 +98,9 @@ class ApiPatchDispatchBulkTests(TestCase):
         )
 
         self.assertEqual(response.status_code, 400)
-        self.assertEqual(response.data['detail'], '"replace" op has to specify path.')
+        self.assertEqual(response.data, {
+            'detail': '"replace" op has to specify path.',
+        })
 
         # op raised validation error
         response = patch.dispatch_bulk(
@@ -129,8 +131,8 @@ class ApiPatchDispatchBulkTests(TestCase):
 
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data, [
-            {'id': 5, 'detail': ["invalid data here!"], 'status': 400},
-            {'id': 7, 'detail': ["invalid data here!"], 'status': 400},
+            {'id': '5', 'status': '400', 'detail': ["invalid data here!"]},
+            {'id': '7', 'status': '400', 'detail': ["invalid data here!"]},
         ])
 
         # op raised api validation error
@@ -162,8 +164,8 @@ class ApiPatchDispatchBulkTests(TestCase):
 
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data, [
-            {'id': 5, 'detail': ["invalid api data here!"], 'status': 400},
-            {'id': 7, 'detail': ["invalid api data here!"], 'status': 400},
+            {'id': '5', 'status': '400', 'detail': ["invalid api data here!"]},
+            {'id': '7', 'status': '400', 'detail': ["invalid api data here!"]},
         ])
 
         # action in bulk dispatch raised perm denied
@@ -195,8 +197,8 @@ class ApiPatchDispatchBulkTests(TestCase):
 
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data, [
-            {'id': 5, 'detail': "yo ain't doing that!", 'status': 403},
-            {'id': 7, 'detail': "yo ain't doing that!", 'status': 403},
+            {'id': '5', 'status': '403', 'detail': "yo ain't doing that!"},
+            {'id': '7', 'status': '403', 'detail': "yo ain't doing that!"},
         ])
 
         # action in bulk dispatch raised 404
@@ -228,8 +230,8 @@ class ApiPatchDispatchBulkTests(TestCase):
 
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data, [
-            {'id': 5, 'detail': "NOT FOUND", 'status': 404},
-            {'id': 7, 'detail': "NOT FOUND", 'status': 404},
+            {'id': '5', 'status': '404', 'detail': "NOT FOUND"},
+            {'id': '7', 'status': '404', 'detail': "NOT FOUND"},
         ])
 
         # action in bulk dispatch raised 404 with message
@@ -261,6 +263,6 @@ class ApiPatchDispatchBulkTests(TestCase):
 
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data, [
-            {'id': 5, 'detail': "something was removed", 'status': 404},
-            {'id': 7, 'detail': "something was removed", 'status': 404},
+            {'id': '5', 'status': '404', 'detail': "something was removed"},
+            {'id': '7', 'status': '404', 'detail': "something was removed"},
         ])

misago/threads/serializers/moderation.py

@@ -516,7 +516,7 @@ class MergeThreadsSerializer(NewThreadSerializer):
             })
 
         if invalid_threads:
-            invalid_threads.sort(key=lambda e: e['id'])
+            invalid_threads.sort(key=lambda item: item['id'])
             raise ValidationError({'merge': invalid_threads})
 
         return valid_threads

misago/threads/tests/test_thread_postbulkpatch_api.py

@@ -215,9 +215,11 @@ class PostsAddAclApiTests(ThreadPostBulkPatchApiTestCase):
         self.assertEqual(response.status_code, 200)
 
         response_json = response.json()
-        for i, post in enumerate(self.posts):
-            self.assertEqual(response_json[i]['id'], post.id)
-            self.assertTrue(response_json[i]['acl'])
+        for i, post_id in enumerate(self.ids):
+            data = response_json[i]
+            self.assertEqual(data['id'], str(post_id))
+            self.assertEqual(data['status'], '200')
+            self.assertTrue(data['patch']['acl'])
 
 
 class BulkPostProtectApiTests(ThreadPostBulkPatchApiTestCase):
@@ -241,11 +243,15 @@ class BulkPostProtectApiTests(ThreadPostBulkPatchApiTestCase):
             }
         )
         self.assertEqual(response.status_code, 200)
-
-        response_json = response.json()
-        for i, post in enumerate(self.posts):
-            self.assertEqual(response_json[i]['id'], post.id)
-            self.assertTrue(response_json[i]['is_protected'])
+        self.assertEqual(response.json(), [
+            {
+                'id': str(post_id),
+                'status': '200',
+                'patch': {
+                    'is_protected': True,
+                },
+            } for post_id in self.ids
+        ])
 
         for post in Post.objects.filter(id__in=self.ids):
             self.assertTrue(post.is_protected)
@@ -269,10 +275,10 @@ class BulkPostProtectApiTests(ThreadPostBulkPatchApiTestCase):
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.json(), [
             {
-                'id': post.id,
-                'status': 403,
+                'id': str(post_id),
+                'status': '403',
                 'detail': "You can't protect posts in this category.",
-            } for post in self.posts
+            } for post_id in self.ids
         ])
 
         for post in Post.objects.filter(id__in=self.ids):
@@ -306,11 +312,15 @@ class BulkPostsApproveApiTests(ThreadPostBulkPatchApiTestCase):
             }
         )
         self.assertEqual(response.status_code, 200)
-
-        response_json = response.json()
-        for i, post in enumerate(self.posts):
-            self.assertEqual(response_json[i]['id'], post.id)
-            self.assertFalse(response_json[i]['is_unapproved'])
+        self.assertEqual(response.json(), [
+            {
+                'id': str(post_id),
+                'status': '200',
+                'patch': {
+                    'is_unapproved': False,
+                },
+            } for post_id in self.ids
+        ])
 
         for post in Post.objects.filter(id__in=self.ids):
             self.assertFalse(post.is_unapproved)

misago/threads/tests/test_threads_bulkpatch_api.py

@@ -14,13 +14,13 @@ class ThreadsBulkPatchApiTestCase(ThreadsApiTestCase):
     def setUp(self):
         super(ThreadsBulkPatchApiTestCase, self).setUp()
 
-        self.threads = list(reversed([
+        self.threads = [
             testutils.post_thread(category=self.category),
             testutils.post_thread(category=self.category),
             testutils.post_thread(category=self.category),
-        ]))
+        ]
 
-        self.ids = list(reversed([t.id for t in self.threads]))
+        self.ids = [t.id for t in self.threads]
 
         self.api_link = reverse('misago:api:thread-list')
 
@@ -175,9 +175,11 @@ class ThreadAddAclApiTests(ThreadsBulkPatchApiTestCase):
         self.assertEqual(response.status_code, 200)
 
         response_json = response.json()
-        for i, thread in enumerate(self.threads):
-            self.assertEqual(response_json[i]['id'], thread.id)
-            self.assertTrue(response_json[i]['acl'])
+        for i, thread_id in enumerate(self.ids):
+            data = response_json[i]
+            self.assertEqual(data['id'], str(thread_id))
+            self.assertEqual(data['status'], '200')
+            self.assertTrue(data['patch']['acl'])
 
 
 class ThreadsBulkChangeTitleApiTests(ThreadsBulkPatchApiTestCase):
@@ -199,11 +201,15 @@ class ThreadsBulkChangeTitleApiTests(ThreadsBulkPatchApiTestCase):
             }
         )
         self.assertEqual(response.status_code, 200)
-
-        response_json = response.json()
-        for i, thread in enumerate(self.threads):
-            self.assertEqual(response_json[i]['id'], thread.id)
-            self.assertEqual(response_json[i]['title'], 'Changed the title!')
+        self.assertEqual(response.json(), [
+            {
+                'id': str(thread_id),
+                'status': '200',
+                'patch': {
+                    'title': 'Changed the title!',
+                }
+            } for thread_id in self.ids
+        ])
 
         for thread in Thread.objects.filter(id__in=self.ids):
             self.assertEqual(thread.title, 'Changed the title!')
@@ -231,10 +237,10 @@ class ThreadsBulkChangeTitleApiTests(ThreadsBulkPatchApiTestCase):
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.json(), [
             {
-                'id': thread_id,
-                'status': 403,
+                'id': str(thread_id),
+                'status': '403',
                 'detail': "You can't edit threads in this category.",
-            } for thread_id in sorted(self.ids, reverse=True)
+            } for thread_id in self.ids
         ])
 
 
@@ -302,11 +308,15 @@ class ThreadsBulkMoveApiTests(ThreadsBulkPatchApiTestCase):
             }
         )
         self.assertEqual(response.status_code, 200)
-
-        response_json = response.json()
-        for i, thread in enumerate(self.threads):
-            self.assertEqual(response_json[i]['id'], thread.id)
-            self.assertEqual(response_json[i]['category'], self.category_b.pk)
+        self.assertEqual(response.json(), [
+            {
+                'id': str(thread_id),
+                'status': '200',
+                'patch': {
+                    'category': self.category_b.pk,
+                },
+            } for thread_id in self.ids
+        ])
 
         for thread in Thread.objects.filter(id__in=self.ids):
             self.assertEqual(thread.category_id, self.category_b.pk)
@@ -337,11 +347,15 @@ class ThreadsBulksHideApiTests(ThreadsBulkPatchApiTestCase):
             }
         )
         self.assertEqual(response.status_code, 200)
-
-        response_json = response.json()
-        for i, thread in enumerate(self.threads):
-            self.assertEqual(response_json[i]['id'], thread.id)
-            self.assertTrue(response_json[i]['is_hidden'])
+        self.assertEqual(response.json(), [
+            {
+                'id': str(thread_id),
+                'status': '200',
+                'patch': {
+                    'is_hidden': True,
+                },
+            } for thread_id in self.ids
+        ])
 
         for thread in Thread.objects.filter(id__in=self.ids):
             self.assertTrue(thread.is_hidden)
@@ -382,12 +396,16 @@ class ThreadsBulksApproveApiTests(ThreadsBulkPatchApiTestCase):
             }
         )
         self.assertEqual(response.status_code, 200)
-
-        response_json = response.json()
-        for i, thread in enumerate(self.threads):
-            self.assertEqual(response_json[i]['id'], thread.id)
-            self.assertFalse(response_json[i]['is_unapproved'])
-            self.assertFalse(response_json[i]['has_unapproved_posts'])
+        self.assertEqual(response.json(), [
+            {
+                'id': str(thread_id),
+                'status': '200',
+                'patch': {
+                    'is_unapproved': False,
+                    'has_unapproved_posts': False,
+                },
+            } for thread_id in self.ids
+        ])
 
         for thread in Thread.objects.filter(id__in=self.ids):
             self.assertFalse(thread.is_unapproved)