Browse Source

wip #884: bulk posts update

Rafał Pitoń 7 years ago
parent
commit
e3e46edfef

+ 2 - 8
misago/core/apipatch.py

@@ -68,11 +68,6 @@ class ApiPatch(object):
             return Response(patch)
             return Response(patch)
 
 
     def dispatch_bulk(self, request, targets):
     def dispatch_bulk(self, request, targets):
-        if not isinstance(request.data.get('ops'), list):
-            return Response({
-                'detail': "bulk PATCH request's ops value should be list of operations",
-            }, status=400)
-
         is_errored = False
         is_errored = False
         result = []
         result = []
 
 
@@ -84,7 +79,6 @@ class ApiPatch(object):
                 try:
                 try:
                     self.validate_action(action)
                     self.validate_action(action)
                     self.dispatch_action(patch, request, target, action)
                     self.dispatch_action(patch, request, target, action)
-                    detail.append('ok')
                 except Http404:
                 except Http404:
                     is_errored = True
                     is_errored = True
                     detail.append('NOT FOUND')
                     detail.append('NOT FOUND')
@@ -93,8 +87,8 @@ class ApiPatch(object):
                     is_errored = True
                     is_errored = True
                     detail.append(e.args[0])
                     detail.append(e.args[0])
                     break
                     break
-
-            patch['detail'] = detail
+            if detail:
+                patch['detail'] = detail
             result.append(patch)
             result.append(patch)
 
 
         if is_errored:
         if is_errored:

+ 37 - 40
misago/threads/api/postendpoints/patch_post.py

@@ -1,3 +1,6 @@
+from rest_framework import serializers
+from rest_framework.response import Response
+
 from django.core.exceptions import PermissionDenied
 from django.core.exceptions import PermissionDenied
 from django.utils.translation import ugettext as _
 from django.utils.translation import ugettext as _
 
 
@@ -126,69 +129,50 @@ post_patch_dispatcher.replace('is-hidden', patch_is_hidden)
 
 
 
 
 def post_patch_endpoint(request, post):
 def post_patch_endpoint(request, post):
-    old_is_hidden = post.is_hidden
     old_is_unapproved = post.is_unapproved
     old_is_unapproved = post.is_unapproved
-    old_thread = post.thread
-    old_category = post.category
 
 
     response = post_patch_dispatcher.dispatch(request, post)
     response = post_patch_dispatcher.dispatch(request, post)
 
 
-    # diff posts's state against pre-patch and resync category if necessary
-    hidden_changed = old_is_hidden != post.is_hidden
-    unapproved_changed = old_is_unapproved != post.is_unapproved
-    thread_changed = old_thread != post.thread
-    category_changed = old_category != post.category
-
-    if hidden_changed or unapproved_changed or thread_changed or category_changed:
+    # diff posts's state against pre-patch and resync thread/category if necessarys
+    if old_is_unapproved != post.is_unapproved:
         post.thread.synchronize()
         post.thread.synchronize()
         post.thread.save()
         post.thread.save()
 
 
         post.category.synchronize()
         post.category.synchronize()
         post.category.save()
         post.category.save()
 
 
-        if thread_changed:
-            old_thread.synchronize()
-            old_thread.save()
-
-        if category_changed:
-            old_category.synchronize()
-            old_category.save()
     return response
     return response
 
 
 
 
 def bulk_patch_endpoint(request, thread):
 def bulk_patch_endpoint(request, thread):
-    posts = clean_posts_for_patch(request, thread)
+    serializer = BulkPatchSerializer(data=request.data)
+    if not serializer.is_valid():
+        return Response(serializer.errors, status=400)
 
 
-    hidden_posts = 0
-    revealed_posts = 0
-    moved_posts = 0
+    posts = clean_posts_for_patch(request, thread, serializer.data['ids'])
+
+    old_unapproved_posts = [p.is_unapproved for p in posts].count(True)
 
 
     response = post_patch_dispatcher.dispatch_bulk(request, posts)
     response = post_patch_dispatcher.dispatch_bulk(request, posts)
 
 
+    new_unapproved_posts = [p.is_unapproved for p in posts].count(True)
 
 
-def clean_posts_for_patch(request, thread):
-    if not isinstance(request.data, dict):
-        raise PermissionDenied(_("Bulk PATCH request should be a dict with ids and ops keys."))
+    if old_unapproved_posts != new_unapproved_posts:
+        thread.synchronize()
+        thread.save()
 
 
-    # todo: move this ids list cleanup step to utility
+        thread.category.synchronize()
+        thread.category.save()
+
+    return response
 
 
-    try:
-        posts_ids = list(map(int, request.data.get('ids', [])))
-    except (ValueError, TypeError):
-        raise PermissionDenied(_("One or more post ids received were invalid."))
-
-    if not posts_ids:
-        raise PermissionDenied(_("You have to specify at least one post to update."))
-    elif len(posts_ids) > PATCH_LIMIT:
-        message = ungettext(
-            "No more than %(limit)s post can be updated at single time.",
-            "No more than %(limit)s posts can be updated at single time.",
-            PATCH_LIMIT,
-        )
-        raise PermissionDenied(message % {'limit': PATCH_LIMIT})
 
 
+def clean_posts_for_patch(request, thread, posts_ids):
     posts_queryset = exclude_invisible_posts(request.user, thread.category, thread.post_set)
     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_queryset = posts_queryset.filter(
+        id__in=posts_ids,
+        is_event=False,
+    ).order_by('id')
 
 
     posts = []
     posts = []
     for post in posts_queryset:
     for post in posts_queryset:
@@ -200,3 +184,16 @@ def clean_posts_for_patch(request, thread):
         raise PermissionDenied(_("One or more posts to update could not be found."))
         raise PermissionDenied(_("One or more posts to update could not be found."))
 
 
     return posts
     return posts
+
+
+class BulkPatchSerializer(serializers.Serializer):
+    ids = serializers.ListField(
+        child=serializers.IntegerField(min_value=1),
+        max_length=PATCH_LIMIT,
+        min_length=1,
+    )
+    ops = serializers.ListField(
+        child=serializers.DictField(),
+        min_length=1,
+        max_length=10,
+    )

+ 6 - 2
misago/threads/api/threadposts.py

@@ -20,7 +20,7 @@ from .postendpoints.likes import likes_list_endpoint
 from .postendpoints.merge import posts_merge_endpoint
 from .postendpoints.merge import posts_merge_endpoint
 from .postendpoints.move import posts_move_endpoint
 from .postendpoints.move import posts_move_endpoint
 from .postendpoints.patch_event import event_patch_endpoint
 from .postendpoints.patch_event import event_patch_endpoint
-from .postendpoints.patch_post import post_patch_endpoint
+from .postendpoints.patch_post import post_patch_endpoint, bulk_patch_endpoint
 from .postendpoints.read import post_read_endpoint
 from .postendpoints.read import post_read_endpoint
 from .postendpoints.split import posts_split_endpoint
 from .postendpoints.split import posts_split_endpoint
 from .postingendpoint import PostingEndpoint
 from .postingendpoint import PostingEndpoint
@@ -147,8 +147,12 @@ class ViewSet(viewsets.ViewSet):
         else:
         else:
             return Response(posting.errors, status=400)
             return Response(posting.errors, status=400)
 
 
+    def patch(self, request, thread_pk):
+        thread = self.get_thread(request, thread_pk)
+        return bulk_patch_endpoint(request, thread.unwrap())
+
     @transaction.atomic
     @transaction.atomic
-    def partial_update(self, request, thread_pk, pk=None):
+    def partial_update(self, request, thread_pk, pk):
         thread = self.get_thread(request, thread_pk)
         thread = self.get_thread(request, thread_pk)
         post = self.get_post(request, thread, pk).unwrap()
         post = self.get_post(request, thread, pk).unwrap()
 
 

+ 697 - 0
misago/threads/tests/test_thread_postbulkpatch_api.py

@@ -0,0 +1,697 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+import json
+from datetime import timedelta
+
+from django.urls import reverse
+from django.utils import timezone
+
+from misago.acl.testutils import override_acl
+from misago.categories.models import Category
+from misago.threads import testutils
+from misago.threads.models import Post
+from misago.users.testutils import AuthenticatedUserTestCase
+
+
+class ThreadPostBulkPatchApiTestCase(AuthenticatedUserTestCase):
+    def setUp(self):
+        super(ThreadPostBulkPatchApiTestCase, self).setUp()
+
+        self.category = Category.objects.get(slug='first-category')
+        self.thread = testutils.post_thread(category=self.category)
+        self.posts = [
+            testutils.reply_thread(self.thread, poster=self.user),
+            testutils.reply_thread(self.thread),
+            testutils.reply_thread(self.thread, poster=self.user),
+        ]
+
+        self.ids = [p.id for p in self.posts]
+
+        self.api_link = reverse(
+            'misago:api:thread-post-list',
+            kwargs={
+                'thread_pk': self.thread.pk,
+            }
+        )
+
+    def patch(self, api_link, ops):
+        return self.client.patch(api_link, json.dumps(ops), content_type="application/json")
+
+    def override_acl(self, extra_acl=None):
+        new_acl = self.user.acl_cache
+        new_acl['categories'][self.category.pk].update({
+            'can_see': 1,
+            'can_browse': 1,
+            'can_start_threads': 0,
+            'can_reply_threads': 0,
+            'can_edit_posts': 1,
+        })
+
+        if extra_acl:
+            new_acl['categories'][self.category.pk].update(extra_acl)
+
+        override_acl(self.user, new_acl)
+
+
+class BulkPatchSerializerTests(ThreadPostBulkPatchApiTestCase):
+    def test_invalid_input_type(self):
+        """api rejects invalid input type"""
+        response = self.patch(self.api_link, [1, 2, 3])
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.json(), {
+            'non_field_errors': [
+                "Invalid data. Expected a dictionary, but got list.",
+            ],
+        })
+
+    def test_missing_input_keys(self):
+        """api rejects input with missing keys"""
+        response = self.patch(self.api_link, {})
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.json(), {
+            'ids': [
+                "This field is required.",
+            ],
+            'ops': [
+                "This field is required.",
+            ],
+        })
+
+    def test_empty_input_keys(self):
+        """api rejects input with empty keys"""
+        response = self.patch(self.api_link, {
+            'ids': [],
+            'ops': [],
+        })
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.json(), {
+            'ids': [
+                "Ensure this field has at least 1 elements.",
+            ],
+            'ops': [
+                "Ensure this field has at least 1 elements.",
+            ],
+        })
+
+    def test_invalid_input_keys(self):
+        """api rejects input with invalid keys"""
+        response = self.patch(self.api_link, {
+            'ids': ['a'],
+            'ops': [1],
+        })
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.json(), {
+            'ids': [
+                "A valid integer is required.",
+            ],
+            'ops': [
+                'Expected a dictionary of items but got type "int".',
+            ],
+        })
+
+    def test_too_small_id(self):
+        """api rejects input with implausiple id"""
+        response = self.patch(self.api_link, {
+            'ids': [0],
+            'ops': [{}],
+        })
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.json(), {
+            'ids': [
+                "Ensure this value is greater than or equal to 1.",
+            ],
+        })
+
+    def test_too_large_input(self):
+        """api rejects too large input"""
+        response = self.patch(self.api_link, {
+            'ids': [i + 1 for i in range(200)],
+            'ops': [{} for i in range(200)],
+        })
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.json(), {
+            'ids': [
+                "Ensure this field has no more than 24 elements.",
+            ],
+            'ops': [
+                "Ensure this field has no more than 10 elements.",
+            ],
+        })
+
+    def test_invalid_id(self):
+        """api rejects too large input"""
+        response = self.patch(self.api_link, {
+            'ids': [i + 1 for i in range(200)],
+            'ops': [{} for i in range(200)],
+        })
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.json(), {
+            'ids': [
+                "Ensure this field has no more than 24 elements.",
+            ],
+            'ops': [
+                "Ensure this field has no more than 10 elements.",
+            ],
+        })
+
+    def test_posts_not_found(self):
+        """api fails to find posts"""
+        posts = [
+            testutils.reply_thread(self.thread, is_hidden=True),
+            testutils.reply_thread(self.thread, is_unapproved=True),
+        ]
+
+        response = self.patch(self.api_link, {
+            'ids': [p.id for p in posts],
+            'ops': [{}],
+        })
+
+        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.json(), {
+            'detail': "One or more posts to update could not be found.",
+        })
+
+    def test_ops_invalid(self):
+        """api validates descriptions"""
+        response = self.patch(self.api_link, {
+            'ids': self.ids[:1],
+            'ops': [{}],
+        })
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.json(), [
+            {'id': self.ids[0], 'detail': ['undefined op']},
+        ])
+
+    def test_anonymous_user(self):
+        """anonymous users can't use bulk actions"""
+        self.logout_user()
+
+        response = self.patch(self.api_link, {
+            'ids': self.ids[:1],
+            'ops': [{}],
+        })
+        self.assertEqual(response.status_code, 403)
+
+    def test_events(self):
+        """cant use bulk actions for events"""
+        for post in self.posts:
+            post.is_event = True
+            post.save()
+
+        response = self.patch(self.api_link, {
+            'ids': self.ids,
+            'ops': [{}],
+        })
+
+        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.json(), {
+            'detail': "One or more posts to update could not be found.",
+        })
+
+
+class PostsAddAclApiTests(ThreadPostBulkPatchApiTestCase):
+    def test_add_acl_true(self):
+        """api adds current event's acl to response"""
+        response = self.patch(self.api_link, {
+            'ids': self.ids,
+            'ops': [
+                {
+                    'op': 'add',
+                    'path': 'acl',
+                    'value': True,
+                },
+            ]
+        })
+        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'])
+
+    def test_add_acl_false(self):
+        """if value is false, api won't add acl to the response, but will set empty key"""
+        response = self.patch(self.api_link, {
+            'ids': self.ids,
+            'ops': [
+                {
+                    'op': 'add',
+                    'path': 'acl',
+                    'value': False,
+                },
+            ]
+        })
+        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.assertIsNone(response_json[i]['acl'])
+
+
+class BulkPostProtectApiTests(ThreadPostBulkPatchApiTestCase):
+    def test_protect_post(self):
+        """api makes it possible to protect post"""
+        self.override_acl({
+            'can_protect_posts': 1,
+            'can_edit_posts': 2,
+        })
+
+        response = self.patch(
+            self.api_link, {
+                'ids': self.ids,
+                'ops': [
+                    {
+                        'op': 'replace',
+                        'path': 'is-protected',
+                        'value': True,
+                    },
+                ]
+            }
+        )
+        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'])
+
+        for post in Post.objects.filter(id__in=self.ids):
+            self.assertTrue(post.is_protected)
+
+    def test_unprotect_post(self):
+        """api makes it possible to unprotect protected post"""
+        self.override_acl({
+            'can_protect_posts': 1,
+            'can_edit_posts': 2,
+        })
+
+        for post in self.posts:
+            post.is_protected = True
+            post.save()
+
+        response = self.patch(
+            self.api_link, {
+                'ids': self.ids,
+                'ops': [
+                    {
+                        'op': 'replace',
+                        'path': 'is-protected',
+                        'value': False,
+                    },
+                ]
+            }
+        )
+        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_protected'])
+
+        for post in Post.objects.filter(id__in=self.ids):
+            self.assertFalse(post.is_protected)
+
+    def test_protect_post_no_permission(self):
+        """api validates permission to protect post"""
+        self.override_acl({'can_protect_posts': 0})
+
+        response = self.patch(
+            self.api_link, {
+                'ids': self.ids,
+                'ops': [
+                    {
+                        'op': 'replace',
+                        'path': 'is-protected',
+                        'value': True,
+                    },
+                ]
+            }
+        )
+        self.assertEqual(response.status_code, 400)
+
+        response_json = response.json()
+        for i, post in enumerate(self.posts):
+            self.assertEqual(response_json[i]['id'], post.id)
+            self.assertEqual(
+                response_json[i]['detail'],
+                ["You can't protect posts in this category."],
+            )
+
+        for post in Post.objects.filter(id__in=self.ids):
+            self.assertFalse(post.is_protected)
+
+    def test_unprotect_post_no_permission(self):
+        """api validates permission to unprotect post"""
+        for post in self.posts:
+            post.is_protected = True
+            post.save()
+
+        self.override_acl({'can_protect_posts': 0})
+
+        response = self.patch(
+            self.api_link, {
+                'ids': self.ids,
+                'ops': [
+                    {
+                        'op': 'replace',
+                        'path': 'is-protected',
+                        'value': False,
+                    },
+                ]
+            }
+        )
+        self.assertEqual(response.status_code, 400)
+
+        response_json = response.json()
+        for i, post in enumerate(self.posts):
+            self.assertEqual(response_json[i]['id'], post.id)
+            self.assertEqual(
+                response_json[i]['detail'],
+                ["You can't protect posts in this category."],
+            )
+
+        for post in Post.objects.filter(id__in=self.ids):
+            self.assertTrue(post.is_protected)
+
+    def test_protect_post_not_editable(self):
+        """api validates if we can edit post we want to protect"""
+        self.override_acl({
+            'can_protect_posts': 1,
+            'can_edit_posts': 0,
+        })
+
+        response = self.patch(
+            self.api_link, {
+                'ids': self.ids,
+                'ops': [
+                    {
+                        'op': 'replace',
+                        'path': 'is-protected',
+                        'value': True,
+                    },
+                ]
+            }
+        )
+        self.assertEqual(response.status_code, 400)
+
+        response_json = response.json()
+        for i, post in enumerate(self.posts):
+            self.assertEqual(response_json[i]['id'], post.id)
+            self.assertEqual(
+                response_json[i]['detail'],
+                ["You can't protect posts you can't edit."],
+            )
+
+        for post in Post.objects.filter(id__in=self.ids):
+            self.assertFalse(post.is_protected)
+
+    def test_unprotect_post_not_editable(self):
+        """api validates if we can edit post we want to protect"""
+        for post in self.posts:
+            post.is_protected = True
+            post.save()
+
+
+        self.override_acl({
+            'can_protect_posts': 1,
+            'can_edit_posts': 0,
+        })
+
+        response = self.patch(
+            self.api_link, {
+                'ids': self.ids,
+                'ops': [
+                    {
+                        'op': 'replace',
+                        'path': 'is-protected',
+                        'value': False,
+                    },
+                ]
+            }
+        )
+        self.assertEqual(response.status_code, 400)
+
+        response_json = response.json()
+        for i, post in enumerate(self.posts):
+            self.assertEqual(response_json[i]['id'], post.id)
+            self.assertEqual(
+                response_json[i]['detail'],
+                ["You can't protect posts you can't edit."],
+            )
+
+        for post in Post.objects.filter(id__in=self.ids):
+            self.assertTrue(post.is_protected)
+
+
+class PostsApproveApiTests(ThreadPostBulkPatchApiTestCase):
+    def test_approve_post(self):
+        """api makes it possible to approve post"""
+        for post in self.posts:
+            post.is_unapproved = True
+            post.save()
+
+        self.override_acl({'can_approve_content': 1})
+
+        response = self.patch(
+            self.api_link, {
+                'ids': self.ids,
+                'ops': [
+                    {
+                        'op': 'replace',
+                        'path': 'is-unapproved',
+                        'value': False,
+                    },
+                ]
+            }
+        )
+        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'])
+
+        for post in Post.objects.filter(id__in=self.ids):
+            self.assertFalse(post.is_unapproved)
+
+    def test_unapprove_post(self):
+        """unapproving posts is not supported by api"""
+        self.override_acl({'can_approve_content': 1})
+
+        response = self.patch(
+            self.api_link, {
+                'ids': self.ids,
+                'ops': [
+                    {
+                        'op': 'replace',
+                        'path': 'is-unapproved',
+                        'value': True,
+                    },
+                ]
+            }
+        )
+        self.assertEqual(response.status_code, 400)
+
+        response_json = response.json()
+        for i, post in enumerate(self.posts):
+            self.assertEqual(response_json[i]['id'], post.id)
+            self.assertEqual(
+                response_json[i]['detail'],
+                ["Content approval can't be reversed."],
+            )
+
+        for post in Post.objects.filter(id__in=self.ids):
+            self.assertFalse(post.is_unapproved)
+
+    def test_approve_post_no_permission(self):
+        """api validates approval permission"""
+        for post in self.posts:
+            post.poster = self.user
+            post.is_unapproved = True
+            post.save()
+
+        self.override_acl({'can_approve_content': 0})
+
+        response = self.patch(
+            self.api_link, {
+                'ids': self.ids,
+                'ops': [
+                    {
+                        'op': 'replace',
+                        'path': 'is-unapproved',
+                        'value': False,
+                    },
+                ]
+            }
+        )
+        self.assertEqual(response.status_code, 400)
+
+        response_json = response.json()
+        for i, post in enumerate(self.posts):
+            self.assertEqual(response_json[i]['id'], post.id)
+            self.assertEqual(
+                response_json[i]['detail'],
+                ["You can't approve posts in this category."],
+            )
+
+        for post in Post.objects.filter(id__in=self.ids):
+            self.assertTrue(post.is_unapproved)
+
+    def test_approve_post_closed_thread_no_permission(self):
+        """api validates approval permission in closed threads"""
+        for post in self.posts:
+            post.is_unapproved = True
+            post.save()
+
+        self.thread.is_closed = True
+        self.thread.save()
+
+        self.override_acl({
+            'can_approve_content': 1,
+            'can_close_threads': 0,
+        })
+
+        response = self.patch(
+            self.api_link, {
+                'ids': self.ids,
+                'ops': [
+                    {
+                        'op': 'replace',
+                        'path': 'is-unapproved',
+                        'value': False,
+                    },
+                ]
+            }
+        )
+        self.assertEqual(response.status_code, 400)
+
+        response_json = response.json()
+        for i, post in enumerate(self.posts):
+            self.assertEqual(response_json[i]['id'], post.id)
+            self.assertEqual(
+                response_json[i]['detail'],
+                ["This thread is closed. You can't approve posts in it."],
+            )
+
+        for post in Post.objects.filter(id__in=self.ids):
+            self.assertTrue(post.is_unapproved)
+
+    def test_approve_post_closed_category_no_permission(self):
+        """api validates approval permission in closed categories"""
+        for post in self.posts:
+            post.is_unapproved = True
+            post.save()
+
+        self.category.is_closed = True
+        self.category.save()
+
+        self.override_acl({
+            'can_approve_content': 1,
+            'can_close_threads': 0,
+        })
+
+        response = self.patch(
+            self.api_link, {
+                'ids': self.ids,
+                'ops': [
+                    {
+                        'op': 'replace',
+                        'path': 'is-unapproved',
+                        'value': False,
+                    },
+                ]
+            }
+        )
+        self.assertEqual(response.status_code, 400)
+
+        response_json = response.json()
+        for i, post in enumerate(self.posts):
+            self.assertEqual(response_json[i]['id'], post.id)
+            self.assertEqual(
+                response_json[i]['detail'],
+                ["This category is closed. You can't approve posts in it."],
+            )
+
+        for post in Post.objects.filter(id__in=self.ids):
+            self.assertTrue(post.is_unapproved)
+
+    def test_approve_first_post(self):
+        """api approve first post fails"""
+        for post in self.posts:
+            post.is_unapproved = True
+            post.save()
+
+        self.thread.set_first_post(self.posts[0])
+        self.thread.save()
+
+        self.override_acl({'can_approve_content': 1})
+
+        response = self.patch(
+            self.api_link, {
+                'ids': self.ids,
+                'ops': [
+                    {
+                        'op': 'replace',
+                        'path': 'is-unapproved',
+                        'value': False,
+                    },
+                ]
+            }
+        )
+        self.assertEqual(response.status_code, 400)
+
+        response_json = response.json()
+        self.assertEqual(response_json[0], {
+            'id': self.posts[0].id,
+            'detail': ["You can't approve thread's first post."],
+        })
+
+        for post in Post.objects.filter(id__in=self.ids):
+            if post.id == self.ids[0]:
+                self.assertTrue(post.is_unapproved)
+            else:
+                self.assertFalse(post.is_unapproved)
+
+    def test_approve_hidden_post(self):
+        """api approve hidden post fails"""
+        for post in self.posts:
+            post.is_unapproved = True
+            post.is_hidden = True
+            post.save()
+
+        self.override_acl({'can_approve_content': 1})
+
+        response = self.patch(
+            self.api_link, {
+                'ids': self.ids,
+                'ops': [
+                    {
+                        'op': 'replace',
+                        'path': 'is-unapproved',
+                        'value': False,
+                    },
+                ]
+            }
+        )
+        self.assertEqual(response.status_code, 400)
+
+        response_json = response.json()
+        for i, post in enumerate(self.posts):
+            self.assertEqual(response_json[i]['id'], post.id)
+            self.assertEqual(
+                response_json[i]['detail'],
+                ["You can't approve posts the content you can't see."],
+            )
+
+        for post in Post.objects.filter(id__in=self.ids):
+            self.assertTrue(post.is_unapproved)

+ 0 - 9
misago/threads/tests/test_thread_postpatch_api.py

@@ -81,15 +81,6 @@ class PostAddAclApiTests(ThreadPostPatchApiTestCase):
         response_json = response.json()
         response_json = response.json()
         self.assertIsNone(response_json['acl'])
         self.assertIsNone(response_json['acl'])
 
 
-        response = self.patch(self.api_link, [
-            {
-                'op': 'add',
-                'path': 'acl',
-                'value': True,
-            },
-        ])
-        self.assertEqual(response.status_code, 200)
-
 
 
 class PostProtectApiTests(ThreadPostPatchApiTestCase):
 class PostProtectApiTests(ThreadPostPatchApiTestCase):
     def test_protect_post(self):
     def test_protect_post(self):