#893: wip posts move/merge API's

misago/threads/api/postendpoints/merge.py

@@ -1,19 +1,12 @@
 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.acl import add_acl
-from misago.conf import settings
-from misago.core.utils import clean_ids_list
-from misago.threads.permissions import allow_merge_post, exclude_invisible_posts
 from misago.threads.serializers import MergePostsSerializer, PostSerializer
 
 
-MERGE_LIMIT = settings.MISAGO_POSTS_PER_PAGE + settings.MISAGO_POSTS_TAIL
-
-
 def posts_merge_endpoint(request, thread):
     if not thread.acl['can_merge_posts']:
         raise PermissionDenied(_("You can't merge posts in this thread."))
@@ -63,53 +56,3 @@ def posts_merge_endpoint(request, thread):
     add_acl(request.user, first_post)
 
     return Response(PostSerializer(first_post, context={'user': request.user}).data)
-
-
-def clean_posts_for_merge(request, thread):
-    posts_ids = clean_ids_list(
-        request.data.get('posts', []),
-        _("One or more post ids received were invalid."),
-    )
-
-    if len(posts_ids) < 2:
-        raise PermissionDenied(_("You have to select at least two posts to merge."))
-    elif len(posts_ids) > MERGE_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.",
-            MERGE_LIMIT,
-        )
-        raise PermissionDenied(message % {'limit': MERGE_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_merge_post(request.user, post)
-
-        if not posts:
-            posts.append(post)
-        else:
-            authorship_error = _("Posts made by different users can't be merged.")
-            if posts[0].poster_id:
-                if post.poster_id != posts[0].poster_id:
-                    raise PermissionDenied(authorship_error)
-            else:
-                if post.poster_id or post.poster_name != posts[0].poster_name:
-                    raise PermissionDenied(authorship_error)
-
-            if posts[0].pk != thread.first_post_id:
-                if (posts[0].is_hidden != post.is_hidden or
-                        posts[0].is_unapproved != post.is_unapproved):
-                    raise PermissionDenied(_("Posts with different visibility can't be merged."))
-
-            posts.append(post)
-
-    if len(posts) != len(posts_ids):
-        raise PermissionDenied(_("One or more posts to merge could not be found."))
-
-    return posts

misago/threads/api/postendpoints/move.py

@@ -1,30 +1,40 @@
 from rest_framework.response import Response
 
 from django.core.exceptions import PermissionDenied
-from django.http import Http404
-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.permissions import allow_move_post, exclude_invisible_posts
-from misago.threads.utils import get_thread_id_from_url
-
-
-MOVE_LIMIT = settings.MISAGO_POSTS_PER_PAGE + settings.MISAGO_POSTS_TAIL
+from misago.threads.serializers import MovePostsSerializer
 
 
 def posts_move_endpoint(request, thread, viewmodel):
     if not thread.acl['can_move_posts']:
         raise PermissionDenied(_("You can't move posts in this thread."))
 
-    try:
-        new_thread = clean_thread_for_move(request, thread, viewmodel)
-        posts = clean_posts_for_move(request, thread)
-    except PermissionDenied as e:
-        return Response({'detail': six.text_type(e)}, status=400)
+    serializer = MovePostsSerializer(
+        data=request.data,
+        context={
+            'request': request,
+            'thread': thread,
+            'viewmodel': viewmodel,
+        }
+    )
+
+    if not serializer.is_valid():
+        if 'new_thread' in serializer.errors:
+            errors = serializer.errors['new_thread']
+        else:
+            errors = list(serializer.errors.values())[0]
+
+        return Response(
+            {
+                'detail': errors[0],
+            },
+            status=400,
+        )
+
+    new_thread = serializer.new_thread
 
-    for post in posts:
+    for post in serializer.posts_cache:
         post.move(new_thread)
         post.save()
 
@@ -42,59 +52,3 @@ def posts_move_endpoint(request, thread, viewmodel):
         new_thread.category.save()
 
     return Response({})
-
-
-def clean_thread_for_move(request, thread, viewmodel):
-    new_thread_id = get_thread_id_from_url(request, request.data.get('thread_url', None))
-    if not new_thread_id:
-        raise PermissionDenied(_("This is not a valid thread link."))
-    if new_thread_id == thread.pk:
-        raise PermissionDenied(_("Thread to move posts to is same as current one."))
-
-    try:
-        new_thread = viewmodel(request, new_thread_id).unwrap()
-    except Http404:
-        raise PermissionDenied(
-            _(
-                "The thread you have entered link to doesn't "
-                "exist or you don't have permission to see it."
-            )
-        )
-
-    if not new_thread.acl['can_reply']:
-        raise PermissionDenied(_("You can't move posts to threads you can't reply."))
-
-    return new_thread
-
-
-def clean_posts_for_move(request, thread):
-    posts_ids = clean_ids_list(
-        request.data.get('posts', []),
-        _("One or more post ids received were invalid."),
-    )
-
-    if not posts_ids:
-        raise PermissionDenied(_("You have to specify at least one post to move."))
-    elif len(posts_ids) > MOVE_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.",
-            MOVE_LIMIT,
-        )
-        raise PermissionDenied(message % {'limit': MOVE_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_move_post(request.user, post)
-        posts.append(post)
-
-    if len(posts) != len(posts_ids):
-        raise PermissionDenied(_("One or more posts to move could not be found."))
-
-    return posts

misago/threads/serializers/moderation.py

@@ -1,21 +1,27 @@
 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, can_start_thread, exclude_invisible_posts
+from misago.threads.permissions import (
+    allow_merge_post, allow_move_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
+
 
 
 __all__ = [
     'MergePostsSerializer',
+    'MovePostsSerializer',
     'NewThreadSerializer',
 ]
 
@@ -75,7 +81,9 @@ class MergePostsSerializer(serializers.Serializer):
                 if posts[0].pk != thread.first_post_id:
                     if (posts[0].is_hidden != post.is_hidden or
                             posts[0].is_unapproved != post.is_unapproved):
-                        raise serializers.ValidationError(_("Posts with different visibility can't be merged."))
+                        raise serializers.ValidationError(
+                            _("Posts with different visibility can't be merged.")
+                        )
 
                 posts.append(post)
 
@@ -83,6 +91,88 @@ class MergePostsSerializer(serializers.Serializer):
             raise serializers.ValidationError(_("One or more posts to merge could not be found."))
 
         self.posts_cache = posts
+
+        return data
+
+
+class MovePostsSerializer(serializers.Serializer):
+    thread_url = serializers.CharField(
+        error_messages={
+            'required': ugettext_lazy("Enter link to new thread."),
+        },
+    )
+    posts = serializers.ListField(
+        allow_empty=False,
+        child=serializers.IntegerField(
+            error_messages={
+                'invalid': ugettext_lazy("One or more post ids received were invalid."),
+            },
+        ),
+        error_messages={
+            'empty': ugettext_lazy("You have to specify at least one post to move."),
+        },
+    )
+
+    def validate_thread_url(self, data):
+        request = self.context['request']
+        thread = self.context['thread']
+        viewmodel = self.context['viewmodel']
+
+        new_thread_id = get_thread_id_from_url(request, data)
+        if not new_thread_id:
+            raise serializers.ValidationError(_("This is not a valid thread link."))
+        if new_thread_id == thread.pk:
+            raise serializers.ValidationError(_("Thread to move posts to is same as current one."))
+
+        try:
+            new_thread = viewmodel(request, new_thread_id).unwrap()
+        except Http404:
+            raise serializers.ValidationError(
+                _(
+                    "The thread you have entered link to doesn't "
+                    "exist or you don't have permission to see it."
+                )
+            )
+
+        if not new_thread.acl['can_reply']:
+            raise serializers.ValidationError(_("You can't move posts to threads you can't reply."))
+
+        self.new_thread = new_thread
+
+        return data
+
+    def validate_posts(self, data):
+        data = list(set(data))
+        if len(data) > POSTS_MOVE_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,
+            )
+            raise serializers.ValidationError(message % {'limit': POSTS_MOVE_LIMIT})
+
+        request = self.context['request']
+        thread = self.context['thread']
+
+        posts_queryset = exclude_invisible_posts(request.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_move_post(request.user, post)
+                posts.append(post)
+            except PermissionDenied as e:
+                raise serializers.ValidationError(six.text_type(e))
+
+        if len(posts) != len(data):
+            raise serializers.ValidationError(_("One or more posts to move could not be found."))
+
+        self.posts_cache = posts
+
         return data
 
 

misago/threads/tests/test_thread_pollvotes_api.py

@@ -187,13 +187,18 @@ class ThreadPostVotesTests(ThreadPollApiTestCase):
         """api validates if vote that user has made was empty"""
         self.delete_user_votes()
 
-        response = self.post(self.api_link)
-        self.assertContains(response, "Expected a list of items", status_code=400)
+        response = self.client.post(
+            self.api_link, '[]', content_type='application/json'
+        )
+        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()
 
+        response = self.post(self.api_link)
+        self.assertContains(response, "Expected a list of items", status_code=400)
+
         response = self.post(self.api_link, data={})
         self.assertContains(response, "Expected a list of items", status_code=400)
 

misago/threads/tests/test_thread_postmerge_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.merge import MERGE_LIMIT
 from misago.threads.models import Post, Thread
+from misago.threads.serializers.moderation import POSTS_MERGE_LIMIT
 from misago.users.testutils import AuthenticatedUserTestCase
 
 
@@ -155,12 +155,12 @@ class ThreadPostMergeApiTestCase(AuthenticatedUserTestCase):
         response = self.client.post(
             self.api_link,
             json.dumps({
-                'posts': list(range(MERGE_LIMIT + 1))
+                'posts': list(range(POSTS_MERGE_LIMIT + 1))
             }),
             content_type="application/json",
         )
         self.assertContains(
-            response, "No more than {} posts can be merged".format(MERGE_LIMIT), status_code=400
+            response, "No more than {} posts can be merged".format(POSTS_MERGE_LIMIT), status_code=400
         )
 
     def test_merge_event(self):

misago/threads/tests/test_thread_postmove_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.move import MOVE_LIMIT
 from misago.threads.models import Thread
+from misago.threads.serializers.moderation import POSTS_MOVE_LIMIT
 from misago.users.testutils import AuthenticatedUserTestCase
 
 
@@ -95,6 +95,24 @@ class ThreadPostMoveApiTestCase(AuthenticatedUserTestCase):
         response = self.client.post(self.api_link, json.dumps({}), content_type="application/json")
         self.assertEqual(response.status_code, 403)
 
+    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_permission(self):
         """api validates permission to move"""
         self.override_acl({'can_move_posts': 0})
@@ -105,7 +123,7 @@ class ThreadPostMoveApiTestCase(AuthenticatedUserTestCase):
     def test_move_no_url(self):
         """api validates if thread url was given"""
         response = self.client.post(self.api_link)
-        self.assertContains(response, "This is not a valid thread link.", status_code=400)
+        self.assertContains(response, "Enter link to new thread.", status_code=400)
 
     def test_invalid_url(self):
         """api validates thread url"""
@@ -121,6 +139,7 @@ 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
         )
@@ -174,6 +193,15 @@ class ThreadPostMoveApiTestCase(AuthenticatedUserTestCase):
         """api handles empty data"""
         other_thread = testutils.post_thread(self.category)
 
+        response = self.client.post(self.api_link)
+        self.assertContains(
+            response, "Enter link to new thread.", status_code=400
+        )
+
+    def test_empty_posts_data(self):
+        """api handles empty data"""
+        other_thread = testutils.post_thread(self.category)
+
         response = self.client.post(
             self.api_link, {
                 'thread_url': other_thread.get_absolute_url(),
@@ -212,7 +240,7 @@ class ThreadPostMoveApiTestCase(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", status_code=400
         )
 
     def test_invalid_posts_ids(self):
@@ -239,12 +267,12 @@ class ThreadPostMoveApiTestCase(AuthenticatedUserTestCase):
             self.api_link,
             json.dumps({
                 'thread_url': other_thread.get_absolute_url(),
-                'posts': list(range(MOVE_LIMIT + 1)),
+                'posts': list(range(POSTS_MOVE_LIMIT + 1)),
             }),
             content_type="application/json",
         )
         self.assertContains(
-            response, "No more than {} posts can be moved".format(MOVE_LIMIT), status_code=400
+            response, "No more than {} posts can be moved".format(POSTS_MOVE_LIMIT), status_code=400
         )
 
     def test_move_invisible(self):