wip #887: invalid posts target should raise 400 instead of 403 for consistency

misago/threads/api/postendpoints/merge.py

@@ -1,8 +1,8 @@
 from rest_framework.response import Response
 
 from django.core.exceptions import PermissionDenied
-from django.utils.translation import ugettext as _
-from django.utils.translation import ungettext
+from django.utils import six
+from django.utils.translation import ugettext as _, ungettext
 
 from misago.acl import add_acl
 from misago.conf import settings
@@ -13,19 +13,14 @@ from misago.threads.serializers import PostSerializer
 MERGE_LIMIT = settings.MISAGO_POSTS_PER_PAGE + settings.MISAGO_POSTS_TAIL
 
 
-class MergeError(Exception):
-    def __init__(self, msg):
-        self.msg = msg
-
-
 def posts_merge_endpoint(request, thread):
     if not thread.acl['can_merge_posts']:
         raise PermissionDenied(_("You can't merge posts in this thread."))
 
     try:
         posts = clean_posts_for_merge(request, thread)
-    except MergeError as e:
-        return Response({'detail': e.msg}, status=400)
+    except PermissionDenied as e:
+        return Response({'detail': six.text_type(e)}, status=400)
 
     first_post, merged_posts = posts[0], posts[1:]
     for post in merged_posts:
@@ -60,17 +55,17 @@ def clean_posts_for_merge(request, thread):
     try:
         posts_ids = list(map(int, request.data.get('posts', [])))
     except (ValueError, TypeError):
-        raise MergeError(_("One or more post ids received were invalid."))
+        raise PermissionDenied(_("One or more post ids received were invalid."))
 
     if len(posts_ids) < 2:
-        raise MergeError(_("You have to select at least two posts to merge."))
+        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 MergeError(message % {'limit': 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')
@@ -88,19 +83,19 @@ def clean_posts_for_merge(request, thread):
             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 MergeError(authorship_error)
+                    raise PermissionDenied(authorship_error)
             else:
                 if post.poster_id or post.poster_name != posts[0].poster_name:
-                    raise MergeError(authorship_error)
+                    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 MergeError(_("Posts with different visibility can't be merged."))
+                    raise PermissionDenied(_("Posts with different visibility can't be merged."))
 
             posts.append(post)
 
     if len(posts) != len(posts_ids):
-        raise MergeError(_("One or more posts to merge could not be found."))
+        raise PermissionDenied(_("One or more posts to merge could not be found."))
 
     return posts

misago/threads/api/postendpoints/move.py

@@ -3,8 +3,7 @@ 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 _
-from django.utils.translation import ungettext
+from django.utils.translation import ugettext as _, ungettext
 
 from misago.conf import settings
 from misago.threads.permissions import allow_move_post, exclude_invisible_posts

misago/threads/api/postendpoints/split.py

@@ -1,8 +1,8 @@
 from rest_framework.response import Response
 
 from django.core.exceptions import PermissionDenied
-from django.utils.translation import ugettext as _
-from django.utils.translation import ungettext
+from django.utils import six
+from django.utils.translation import ugettext as _, ungettext
 
 from misago.conf import settings
 from misago.threads.models import Thread
@@ -14,19 +14,14 @@ from misago.threads.serializers import NewThreadSerializer
 SPLIT_LIMIT = settings.MISAGO_POSTS_PER_PAGE + settings.MISAGO_POSTS_TAIL
 
 
-class SplitError(Exception):
-    def __init__(self, msg):
-        self.msg = msg
-
-
 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 SplitError as e:
-        return Response({'detail': e.msg}, status=400)
+    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():
@@ -40,17 +35,17 @@ def clean_posts_for_split(request, thread):
     try:
         posts_ids = list(map(int, request.data.get('posts', [])))
     except (ValueError, TypeError):
-        raise SplitError(_("One or more post ids received were invalid."))
+        raise PermissionDenied(_("One or more post ids received were invalid."))
 
     if not posts_ids:
-        raise SplitError(_("You have to specify at least one post to split."))
+        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 SplitError(message % {'limit': 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')
@@ -65,7 +60,7 @@ def clean_posts_for_split(request, thread):
         posts.append(post)
 
     if len(posts) != len(posts_ids):
-        raise SplitError(_("One or more posts to split could not be found."))
+        raise PermissionDenied(_("One or more posts to split could not be found."))
 
     return posts
 

misago/threads/tests/test_thread_postmerge_api.py

@@ -158,7 +158,7 @@ class ThreadPostMergeApiTestCase(AuthenticatedUserTestCase):
             }),
             content_type="application/json",
         )
-        self.assertContains(response, "Events can't be merged.", status_code=403)
+        self.assertContains(response, "Events can't be merged.", status_code=400)
 
     def test_merge_notfound_pk(self):
         """api recjects nonexistant pk's"""
@@ -289,7 +289,7 @@ class ThreadPostMergeApiTestCase(AuthenticatedUserTestCase):
         self.assertContains(
             response,
             "This thread is closed. You can't merge posts in it.",
-            status_code=403,
+            status_code=400,
         )
 
         # allow closing threads
@@ -320,7 +320,7 @@ class ThreadPostMergeApiTestCase(AuthenticatedUserTestCase):
         self.assertContains(
             response,
             "This category is closed. You can't merge posts in it.",
-            status_code=403,
+            status_code=400,
         )
 
         # allow closing threads

misago/threads/tests/test_thread_postsplit_api.py

@@ -186,7 +186,7 @@ class ThreadPostSplitApiTestCase(AuthenticatedUserTestCase):
             }),
             content_type="application/json",
         )
-        self.assertContains(response, "Events can't be split.", status_code=403)
+        self.assertContains(response, "Events can't be split.", status_code=400)
 
     def test_split_first_post(self):
         """api rejects first post split"""
@@ -197,7 +197,7 @@ class ThreadPostSplitApiTestCase(AuthenticatedUserTestCase):
             }),
             content_type="application/json",
         )
-        self.assertContains(response, "You can't split thread's first post.", status_code=403)
+        self.assertContains(response, "You can't split thread's first post.", status_code=400)
 
     def test_split_hidden_posts(self):
         """api recjects attempt to split urneadable hidden post"""
@@ -209,7 +209,7 @@ class ThreadPostSplitApiTestCase(AuthenticatedUserTestCase):
             content_type="application/json",
         )
         self.assertContains(
-            response, "You can't split posts the content you can't see.", status_code=403
+            response, "You can't split posts the content you can't see.", status_code=400
         )
 
     def test_split_posts_closed_thread_no_permission(self):
@@ -227,7 +227,7 @@ class ThreadPostSplitApiTestCase(AuthenticatedUserTestCase):
             content_type="application/json",
         )
         self.assertContains(
-            response, "This thread is closed. You can't split posts in it.", status_code=403
+            response, "This thread is closed. You can't split posts in it.", status_code=400
         )
 
     def test_split_posts_closed_category_no_permission(self):
@@ -245,7 +245,7 @@ class ThreadPostSplitApiTestCase(AuthenticatedUserTestCase):
             content_type="application/json",
         )
         self.assertContains(
-            response, "This category is closed. You can't split posts in it.", status_code=403
+            response, "This category is closed. You can't split posts in it.", status_code=400
         )
 
     def test_split_other_thread_posts(self):