#893: added serializer for split thread endpoint

misago/threads/api/postendpoints/move.py

@@ -20,8 +20,8 @@ def posts_move_endpoint(request, thread, viewmodel):
     )
 
     if not serializer.is_valid():
-        if 'new_thread' in serializer.errors:
-            errors = serializer.errors['new_thread']
+        if 'thread_url' in serializer.errors:
+            errors = serializer.errors['thread_url']
         else:
             errors = list(serializer.errors.values())[0]
 

misago/threads/api/postendpoints/split.py

@@ -2,68 +2,38 @@ from rest_framework.response import Response
 
 from django.core.exceptions import PermissionDenied
 from django.utils import six
-from django.utils.translation import ugettext as _, ungettext
+from django.utils.translation import ugettext as _
 
-from misago.conf import settings
-from misago.core.utils import clean_ids_list
 from misago.threads.models import Thread
 from misago.threads.moderation import threads as moderation
-from misago.threads.permissions import allow_split_post, exclude_invisible_posts
-from misago.threads.serializers import NewThreadSerializer
-
-
-SPLIT_LIMIT = settings.MISAGO_POSTS_PER_PAGE + settings.MISAGO_POSTS_TAIL
+from misago.threads.serializers import SplitPostsSerializer
 
 
 def posts_split_endpoint(request, thread):
     if not thread.acl['can_move_posts']:
         raise PermissionDenied(_("You can't split posts from this thread."))
 
-    try:
-        posts = clean_posts_for_split(request, thread)
-    except PermissionDenied as e:
-        return Response({'detail': six.text_type(e)}, status=400)
-
-    serializer = NewThreadSerializer(context=request.user, data=request.data)
-    if serializer.is_valid():
-        split_posts_to_new_thread(request, thread, serializer.validated_data, posts)
-        return Response({})
-    else:
-        return Response(serializer.errors, status=400)
-
-
-def clean_posts_for_split(request, thread):
-    posts_ids = clean_ids_list(
-        request.data.get('posts', []),
-        _("One or more post ids received were invalid."),
+    serializer = SplitPostsSerializer(
+        data=request.data,
+        context={
+            'thread': thread,
+            'user': request.user,
+        },
     )
 
-    if not posts_ids:
-        raise PermissionDenied(_("You have to specify at least one post to split."))
-    elif len(posts_ids) > SPLIT_LIMIT:
-        message = ungettext(
-            "No more than %(limit)s post can be split at single time.",
-            "No more than %(limit)s posts can be split at single time.",
-            SPLIT_LIMIT,
-        )
-        raise PermissionDenied(message % {'limit': SPLIT_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
-
-        allow_split_post(request.user, post)
+    if not serializer.is_valid():
+        if 'posts' in serializer.errors:
+            errors = {
+                'detail': serializer.errors['posts'][0]
+            }
+        else :
+            errors = serializer.errors
 
-        posts.append(post)
+        return Response(errors, status=400)
 
-    if len(posts) != len(posts_ids):
-        raise PermissionDenied(_("One or more posts to split could not be found."))
+    split_posts_to_new_thread(request, thread, serializer.validated_data, serializer.posts_cache)
 
-    return posts
+    return Response({})
 
 
 def split_posts_to_new_thread(request, thread, validated_data, posts):

misago/threads/api/threadendpoints/merge.py

@@ -113,7 +113,11 @@ def threads_merge_endpoint(request):
     if invalid_threads:
         return Response(invalid_threads, status=403)
 
-    serializer = NewThreadSerializer(context=request.user, data=request.data)
+    serializer = NewThreadSerializer(
+        data=request.data,
+        context={'user': request.user},
+    )
+
     if serializer.is_valid():
         polls_handler = PollMergeHandler(threads)
         if len(polls_handler.polls) == 1:

misago/threads/serializers/moderation.py

@@ -2,31 +2,32 @@ from rest_framework import serializers
 
 from django.core.exceptions import PermissionDenied, ValidationError
 from django.http import Http404
-from django.utils import six
 from django.utils.translation import ugettext as _, ugettext_lazy, ungettext
 
 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, can_start_thread, exclude_invisible_posts)
+    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
 
 
-POSTS_MERGE_LIMIT = settings.MISAGO_POSTS_PER_PAGE + settings.MISAGO_POSTS_TAIL
-POSTS_MOVE_LIMIT = settings.MISAGO_POSTS_PER_PAGE + settings.MISAGO_POSTS_TAIL
-
+POSTS_LIMIT = settings.MISAGO_POSTS_PER_PAGE + settings.MISAGO_POSTS_TAIL
 
 
 __all__ = [
     'MergePostsSerializer',
     'MovePostsSerializer',
     'NewThreadSerializer',
+    'SplitPostsSerializer',
 ]
 
 
 class MergePostsSerializer(serializers.Serializer):
+    error_empty_or_required = ugettext_lazy("You have to select at least two posts to merge.")
+
     posts = serializers.ListField(
         child=serializers.IntegerField(
             error_messages={
@@ -34,7 +35,7 @@ class MergePostsSerializer(serializers.Serializer):
             },
         ),
         error_messages={
-            'required': ugettext_lazy("You have to select at least two posts to merge."),
+            'required': error_empty_or_required,
         },
     )
 
@@ -42,14 +43,14 @@ class MergePostsSerializer(serializers.Serializer):
         data = list(set(data))
 
         if len(data) < 2:
-            raise serializers.ValidationError(_("You have to select at least two posts to merge."))
-        if len(data) > POSTS_MERGE_LIMIT:
+            raise serializers.ValidationError(self.error_empty_or_required)
+        if len(data) > POSTS_LIMIT:
             message = ungettext(
                 "No more than %(limit)s post can be merged at single time.",
                 "No more than %(limit)s posts can be merged at single time.",
-                POSTS_MERGE_LIMIT,
+                POSTS_LIMIT,
             )
-            raise serializers.ValidationError(message % {'limit': POSTS_MERGE_LIMIT})
+            raise serializers.ValidationError(message % {'limit': POSTS_LIMIT})
 
         user = self.context['user']
         thread = self.context['thread']
@@ -65,7 +66,7 @@ class MergePostsSerializer(serializers.Serializer):
             try:
                 allow_merge_post(user, post)
             except PermissionDenied as e:
-                raise serializers.ValidationError(six.text_type(e))
+                raise serializers.ValidationError(e)
 
             if not posts:
                 posts.append(post)
@@ -96,6 +97,8 @@ class MergePostsSerializer(serializers.Serializer):
 
 
 class MovePostsSerializer(serializers.Serializer):
+    error_empty_or_required = ugettext_lazy("You have to specify at least one post to move.")
+
     thread_url = serializers.CharField(
         error_messages={
             'required': ugettext_lazy("Enter link to new thread."),
@@ -109,7 +112,8 @@ class MovePostsSerializer(serializers.Serializer):
             },
         ),
         error_messages={
-            'empty': ugettext_lazy("You have to specify at least one post to move."),
+            'required': error_empty_or_required,
+            'empty': error_empty_or_required,
         },
     )
 
@@ -143,13 +147,13 @@ class MovePostsSerializer(serializers.Serializer):
 
     def validate_posts(self, data):
         data = list(set(data))
-        if len(data) > POSTS_MOVE_LIMIT:
+        if len(data) > POSTS_LIMIT:
             message = ungettext(
                 "No more than %(limit)s post can be moved at single time.",
                 "No more than %(limit)s posts can be moved at single time.",
-                POSTS_MOVE_LIMIT,
+                POSTS_LIMIT,
             )
-            raise serializers.ValidationError(message % {'limit': POSTS_MOVE_LIMIT})
+            raise serializers.ValidationError(message % {'limit': POSTS_LIMIT})
 
         request = self.context['request']
         thread = self.context['thread']
@@ -166,7 +170,7 @@ class MovePostsSerializer(serializers.Serializer):
                 allow_move_post(request.user, post)
                 posts.append(post)
             except PermissionDenied as e:
-                raise serializers.ValidationError(six.text_type(e))
+                raise serializers.ValidationError(e)
 
         if len(posts) != len(data):
             raise serializers.ValidationError(_("One or more posts to move could not be found."))
@@ -192,14 +196,14 @@ class NewThreadSerializer(serializers.Serializer):
         return validate_title(title)
 
     def validate_category(self, category_id):
-        self.category = validate_category(self.context, category_id)
-        if not can_start_thread(self.context, self.category):
+        self.category = validate_category(self.context['user'], category_id)
+        if not can_start_thread(self.context['user'], self.category):
             raise ValidationError(_("You can't create new threads in selected category."))
         return self.category
 
     def validate_weight(self, weight):
         try:
-            add_acl(self.context, self.category)
+            add_acl(self.context['user'], self.category)
         except AttributeError:
             return weight  # don't validate weight further if category failed
 
@@ -216,7 +220,7 @@ class NewThreadSerializer(serializers.Serializer):
 
     def validate_is_hidden(self, is_hidden):
         try:
-            add_acl(self.context, self.category)
+            add_acl(self.context['user'], self.category)
         except AttributeError:
             return is_hidden  # don't validate hidden further if category failed
 
@@ -226,7 +230,7 @@ class NewThreadSerializer(serializers.Serializer):
 
     def validate_is_closed(self, is_closed):
         try:
-            add_acl(self.context, self.category)
+            add_acl(self.context['user'], self.category)
         except AttributeError:
             return is_closed  # don't validate closed further if category failed
 
@@ -235,3 +239,54 @@ class NewThreadSerializer(serializers.Serializer):
                 _("You don't have permission to close threads in this category.")
             )
         return is_closed
+
+
+class SplitPostsSerializer(NewThreadSerializer):
+    error_empty_or_required = ugettext_lazy("You have to specify at least one post to split.")
+
+    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,
+            '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 split at single time.",
+                "No more than %(limit)s posts can be split at single time.",
+                POSTS_LIMIT,
+            )
+            raise ValidationError(message % {'limit': POSTS_LIMIT})
+
+        thread = self.context['thread']
+        user = self.context['user']
+
+        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
+
+            try:
+                allow_split_post(user, post)
+            except PermissionDenied as e:
+                raise ValidationError(e)
+
+            posts.append(post)
+
+        if len(posts) != len(data):
+            raise ValidationError(_("One or more posts to split could not be found."))
+
+        self.posts_cache = posts
+
+        return data

misago/threads/tests/test_thread_pollvotes_api.py

@@ -183,7 +183,7 @@ class ThreadPostVotesTests(ThreadPollApiTestCase):
         response = self.post(self.api_link)
         self.assertEqual(response.status_code, 403)
 
-    def test_empty_vote(self):
+    def test_empty_vote_json(self):
         """api validates if vote that user has made was empty"""
         self.delete_user_votes()
 
@@ -192,6 +192,13 @@ class ThreadPostVotesTests(ThreadPollApiTestCase):
         )
         self.assertContains(response, "You have to make a choice.", status_code=400)
 
+    def test_empty_vote_form(self):
+        """api validates if vote that user has made was empty"""
+        self.delete_user_votes()
+
+        response = self.client.post(self.api_link)
+        self.assertContains(response, "You have to make a choice.", status_code=400)
+
     def test_malformed_vote(self):
         """api validates if vote that user has made was correctly structured"""
         self.delete_user_votes()

misago/threads/tests/test_thread_postmerge_api.py

@@ -9,7 +9,7 @@ from misago.acl.testutils import override_acl
 from misago.categories.models import Category
 from misago.threads import testutils
 from misago.threads.models import Post, Thread
-from misago.threads.serializers.moderation import POSTS_MERGE_LIMIT
+from misago.threads.serializers.moderation import POSTS_LIMIT
 from misago.users.testutils import AuthenticatedUserTestCase
 
 
@@ -71,8 +71,8 @@ class ThreadPostMergeApiTestCase(AuthenticatedUserTestCase):
         )
         self.assertContains(response, "You can't merge posts in this thread.", status_code=403)
 
-    def test_empty_data(self):
-        """api handles empty data"""
+    def test_empty_data_json(self):
+        """api handles empty json data"""
         response = self.client.post(
             self.api_link, json.dumps({}), content_type="application/json"
         )
@@ -80,6 +80,14 @@ class ThreadPostMergeApiTestCase(AuthenticatedUserTestCase):
             response, "You have to select at least two posts to merge.", status_code=400
         )
 
+    def test_empty_data_form(self):
+        """api handles empty form data"""
+        response = self.client.post(self.api_link, {})
+
+        self.assertContains(
+            response, "You have to select at least two posts to merge.", status_code=400
+        )
+
     def test_invalid_data(self):
         """api handles post that is invalid type"""
         self.override_acl()
@@ -155,12 +163,12 @@ class ThreadPostMergeApiTestCase(AuthenticatedUserTestCase):
         response = self.client.post(
             self.api_link,
             json.dumps({
-                'posts': list(range(POSTS_MERGE_LIMIT + 1))
+                'posts': list(range(POSTS_LIMIT + 1))
             }),
             content_type="application/json",
         )
         self.assertContains(
-            response, "No more than {} posts can be merged".format(POSTS_MERGE_LIMIT), status_code=400
+            response, "No more than {} posts can be merged".format(POSTS_LIMIT), status_code=400
         )
 
     def test_merge_event(self):

misago/threads/tests/test_thread_postmove_api.py

@@ -9,7 +9,7 @@ from misago.acl.testutils import override_acl
 from misago.categories.models import Category
 from misago.threads import testutils
 from misago.threads.models import Thread
-from misago.threads.serializers.moderation import POSTS_MOVE_LIMIT
+from misago.threads.serializers.moderation import POSTS_LIMIT
 from misago.users.testutils import AuthenticatedUserTestCase
 
 
@@ -139,7 +139,6 @@ class ThreadPostMoveApiTestCase(AuthenticatedUserTestCase):
                 'thread_url': self.thread.get_absolute_url(),
             }
         )
-
         self.assertContains(
             response, "Thread to move posts to is same as current one.", status_code=400
         )
@@ -198,15 +197,33 @@ class ThreadPostMoveApiTestCase(AuthenticatedUserTestCase):
             response, "Enter link to new thread.", status_code=400
         )
 
-    def test_empty_posts_data(self):
-        """api handles empty data"""
+    def test_empty_posts_data_json(self):
+        """api handles empty json data"""
         other_thread = testutils.post_thread(self.category)
 
         response = self.client.post(
-            self.api_link, {
+            self.api_link,
+            json.dumps({
                 'thread_url': other_thread.get_absolute_url(),
-            }
+            }),
+            content_type="application/json",
+        )
+
+        self.assertContains(
+            response, "You have to specify at least one post to move.", status_code=400
+        )
+
+    def test_empty_posts_data_form(self):
+        """api handles empty form data"""
+        other_thread = testutils.post_thread(self.category)
+
+        response = self.client.post(
+            self.api_link,
+            {
+                'thread_url': other_thread.get_absolute_url(),
+            },
         )
+
         self.assertContains(
             response, "You have to specify at least one post to move.", status_code=400
         )
@@ -267,12 +284,12 @@ class ThreadPostMoveApiTestCase(AuthenticatedUserTestCase):
             self.api_link,
             json.dumps({
                 'thread_url': other_thread.get_absolute_url(),
-                'posts': list(range(POSTS_MOVE_LIMIT + 1)),
+                'posts': list(range(POSTS_LIMIT + 1)),
             }),
             content_type="application/json",
         )
         self.assertContains(
-            response, "No more than {} posts can be moved".format(POSTS_MOVE_LIMIT), status_code=400
+            response, "No more than {} posts can be moved".format(POSTS_LIMIT), status_code=400
         )
 
     def test_move_invisible(self):

misago/threads/tests/test_thread_postsplit_api.py

@@ -8,8 +8,8 @@ from django.urls import reverse
 from misago.acl.testutils import override_acl
 from misago.categories.models import Category
 from misago.threads import testutils
-from misago.threads.api.postendpoints.split import SPLIT_LIMIT
 from misago.threads.models import Thread
+from misago.threads.serializers.moderation import POSTS_LIMIT
 from misago.users.testutils import AuthenticatedUserTestCase
 
 
@@ -112,10 +112,39 @@ class ThreadPostSplitApiTestCase(AuthenticatedUserTestCase):
             response, "You have to specify at least one post to split.", status_code=400
         )
 
+    def test_invalid_data(self):
+        """api handles post that is invalid type"""
+        self.override_acl()
+        response = self.client.post(self.api_link, '[]', content_type="application/json")
+        self.assertContains(response, "Invalid data. Expected a dictionary", status_code=400)
+
+        self.override_acl()
+        response = self.client.post(self.api_link, '123', content_type="application/json")
+        self.assertContains(response, "Invalid data. Expected a dictionary", status_code=400)
+
+        self.override_acl()
+        response = self.client.post(self.api_link, '"string"', content_type="application/json")
+        self.assertContains(response, "Invalid data. Expected a dictionary", status_code=400)
+
+        self.override_acl()
+        response = self.client.post(self.api_link, 'malformed', content_type="application/json")
+        self.assertContains(response, "JSON parse error", status_code=400)
+
     def test_no_posts_ids(self):
         """api rejects no posts ids"""
         response = self.client.post(
             self.api_link,
+            json.dumps({}),
+            content_type="application/json",
+        )
+        self.assertContains(
+            response, "You have to specify at least one post to split.", status_code=400
+        )
+
+    def test_empty_posts_ids(self):
+        """api rejects empty posts ids list"""
+        response = self.client.post(
+            self.api_link,
             json.dumps({
                 'posts': [],
             }),
@@ -135,7 +164,7 @@ class ThreadPostSplitApiTestCase(AuthenticatedUserTestCase):
             content_type="application/json",
         )
         self.assertContains(
-            response, "One or more post ids received were invalid.", status_code=400
+            response, "Expected a list of items but got type", status_code=400
         )
 
     def test_invalid_posts_ids(self):
@@ -156,12 +185,12 @@ class ThreadPostSplitApiTestCase(AuthenticatedUserTestCase):
         response = self.client.post(
             self.api_link,
             json.dumps({
-                'posts': list(range(SPLIT_LIMIT + 1)),
+                'posts': list(range(POSTS_LIMIT + 1)),
             }),
             content_type="application/json",
         )
         self.assertContains(
-            response, "No more than {} posts can be split".format(SPLIT_LIMIT), status_code=400
+            response, "No more than {} posts can be split".format(POSTS_LIMIT), status_code=400
         )
 
     def test_split_invisible(self):