#893 validate posts with serializer before deletion

misago/threads/api/postendpoints/delete.py

@@ -9,6 +9,7 @@ from misago.core.utils import clean_ids_list
 from misago.threads.moderation import posts as moderation
 from misago.threads.permissions import allow_delete_event, allow_delete_post
 from misago.threads.permissions import exclude_invisible_posts
+from misago.threads.serializers import DeletePostsSerializer
 
 
 DELETE_LIMIT = settings.MISAGO_POSTS_PER_PAGE + settings.MISAGO_POSTS_TAIL
@@ -27,12 +28,26 @@ def delete_post(request, thread, post):
 
 
 def delete_bulk(request, thread):
-    posts = clean_posts_for_delete(request, thread)
+    serializer = DeletePostsSerializer(
+        data={'posts': request.data},
+        context={
+            'thread': thread,
+            'user': request.user,
+        },
+    )
+
+    if not serializer.is_valid():
+        if 'posts' in serializer.errors:
+            errors = serializer.errors['posts']
+        else:
+            errors = list(serializer.errors.values())[0]
+        return Response({'detail': errors[0]}, status=400)
 
-    for post in posts:
+    for post in serializer.validated_data['posts']:
         post.delete()
 
     sync_related(thread)
+
     return Response({})
 
 
@@ -42,38 +57,3 @@ def sync_related(thread):
 
     thread.category.synchronize()
     thread.category.save()
-
-
-def clean_posts_for_delete(request, thread):
-    posts_ids = clean_ids_list(
-        request.data or [],
-        _("One or more post ids received were invalid."),
-    )
-
-    if not posts_ids:
-        raise PermissionDenied(_("You have to specify at least one post to delete."))
-    elif len(posts_ids) > DELETE_LIMIT:
-        message = ungettext(
-            "No more than %(limit)s post can be deleted at single time.",
-            "No more than %(limit)s posts can be deleted at single time.",
-            DELETE_LIMIT,
-        )
-        raise PermissionDenied(message % {'limit': DELETE_LIMIT})
-
-    posts_queryset = exclude_invisible_posts(request.user, thread.category, thread.post_set)
-    posts_queryset = posts_queryset.filter(id__in=posts_ids).order_by('id')
-
-    posts = []
-    for post in posts_queryset:
-        post.category = thread.category
-        post.thread = thread
-        if post.is_event:
-            allow_delete_event(request.user, post)
-        else:
-            allow_delete_post(request.user, post)
-        posts.append(post)
-
-    if len(posts) != len(posts_ids):
-        raise PermissionDenied(_("One or more posts to delete could not be found."))
-
-    return posts

misago/threads/api/postendpoints/move.py

@@ -24,13 +24,7 @@ def posts_move_endpoint(request, thread, viewmodel):
             errors = serializer.errors['thread_url']
         else:
             errors = list(serializer.errors.values())[0]
-
-        return Response(
-            {
-                'detail': errors[0],
-            },
-            status=400,
-        )
+        return Response({'detail': errors[0]}, status=400)
 
     new_thread = serializer.new_thread
 

misago/threads/serializers/moderation.py

@@ -8,8 +8,8 @@ from misago.acl import add_acl
 from misago.conf import settings
 from misago.threads.models import Thread
 from misago.threads.permissions import (
-    allow_merge_post, allow_move_post, allow_split_post,
-    can_start_thread, exclude_invisible_posts)
+    allow_delete_event, allow_delete_post, allow_merge_post, allow_move_post,
+    allow_split_post, can_start_thread, exclude_invisible_posts)
 from misago.threads.utils import get_thread_id_from_url
 from misago.threads.validators import validate_category, validate_title
 
@@ -18,6 +18,7 @@ POSTS_LIMIT = settings.MISAGO_POSTS_PER_PAGE + settings.MISAGO_POSTS_TAIL
 
 
 __all__ = [
+    'DeletePostsSerializer',
     'MergePostsSerializer',
     'MovePostsSerializer',
     'NewThreadSerializer',
@@ -25,6 +26,56 @@ __all__ = [
 ]
 
 
+class DeletePostsSerializer(serializers.Serializer):
+    error_empty_or_required = ugettext_lazy("You have to specify at least one post to delete.")
+
+    posts = serializers.ListField(
+        allow_empty=False,
+        child=serializers.IntegerField(
+            error_messages={
+                'invalid': ugettext_lazy("One or more post ids received were invalid."),
+            },
+        ),
+        error_messages={
+            'required': error_empty_or_required,
+            'null': error_empty_or_required,
+            'empty': error_empty_or_required,
+        },
+    )
+
+    def validate_posts(self, data):
+        if len(data) > POSTS_LIMIT:
+            message = ungettext(
+                "No more than %(limit)s post can be deleted at single time.",
+                "No more than %(limit)s posts can be deleted at single time.",
+                POSTS_LIMIT,
+            )
+            raise ValidationError(message % {'limit': POSTS_LIMIT})
+
+        user = self.context['user']
+        thread = self.context['thread']
+
+        posts_queryset = exclude_invisible_posts(user, thread.category, thread.post_set)
+        posts_queryset = posts_queryset.filter(id__in=data).order_by('id')
+
+        posts = []
+        for post in posts_queryset:
+            post.category = thread.category
+            post.thread = thread
+
+            if post.is_event:
+                allow_delete_event(user, post)
+            else:
+                allow_delete_post(user, post)
+
+            posts.append(post)
+
+        if len(posts) != len(data):
+            raise PermissionDenied(_("One or more posts to delete could not be found."))
+
+        return posts
+
+
 class MergePostsSerializer(serializers.Serializer):
     error_empty_or_required = ugettext_lazy("You have to select at least two posts to merge.")
 
@@ -35,6 +86,7 @@ class MergePostsSerializer(serializers.Serializer):
             },
         ),
         error_messages={
+            'null': error_empty_or_required,
             'required': error_empty_or_required,
         },
     )
@@ -112,8 +164,9 @@ class MovePostsSerializer(serializers.Serializer):
             },
         ),
         error_messages={
-            'required': error_empty_or_required,
             'empty': error_empty_or_required,
+            'null': error_empty_or_required,
+            'required': error_empty_or_required,
         },
     )
 
@@ -252,8 +305,9 @@ class SplitPostsSerializer(NewThreadSerializer):
             },
         ),
         error_messages={
-            'required': error_empty_or_required,
             'empty': error_empty_or_required,
+            'null': error_empty_or_required,
+            'required': error_empty_or_required,
         },
     )
 

misago/threads/tests/test_thread_postbulkdelete_api.py

@@ -40,7 +40,7 @@ class PostBulkDeleteApiTests(ThreadsApiTestCase):
     def test_delete_no_ids(self):
         """api requires ids to delete"""
         response = self.delete(self.api_link)
-        self.assertContains(response, "You have to specify at least one post to delete.", status_code=403)
+        self.assertContains(response, "You have to specify at least one post to delete.", status_code=400)
 
     def test_validate_ids(self):
         """api validates that ids are list of ints"""
@@ -50,13 +50,13 @@ class PostBulkDeleteApiTests(ThreadsApiTestCase):
         })
 
         response = self.delete(self.api_link, True)
-        self.assertContains(response, "One or more post ids received were invalid.", status_code=403)
+        self.assertContains(response, "Expected a list of items", status_code=400)
 
         response = self.delete(self.api_link, 'abbss')
-        self.assertContains(response, "One or more post ids received were invalid.", status_code=403)
+        self.assertContains(response, "Expected a list of items", status_code=400)
 
         response = self.delete(self.api_link, [1, 2, 3, 'a', 'b', 'x'])
-        self.assertContains(response, "One or more post ids received were invalid.", status_code=403)
+        self.assertContains(response, "One or more post ids received were invalid.", status_code=400)
 
     def test_validate_ids_length(self):
         """api validates that ids are list of ints"""
@@ -66,7 +66,7 @@ class PostBulkDeleteApiTests(ThreadsApiTestCase):
         })
 
         response = self.delete(self.api_link, list(range(100)))
-        self.assertContains(response, "No more than 24 posts can be deleted at single time.", status_code=403)
+        self.assertContains(response, "No more than 24 posts can be deleted at single time.", status_code=400)
 
     def test_validate_posts_exist(self):
         """api validates that ids are visible posts"""