wip delete post backend

misago/threads/api/threadendpoints/merge.py

@@ -1,5 +1,4 @@
 from django.core.exceptions import PermissionDenied
-from django.db import transaction
 from django.http import Http404
 from django.utils.translation import gettext as _
 from django.utils.translation import ungettext
@@ -27,7 +26,6 @@ class MergeError(Exception):
         self.msg = msg
 
 
-@transaction.atomic
 def thread_merge_endpoint(request, thread, viewmodel):
     if not thread.acl['can_merge']:
         raise PermissionDenied(_("You don't have permission to merge this thread with others."))
@@ -39,7 +37,7 @@ def thread_merge_endpoint(request, thread, viewmodel):
         return Response({'detail': _("You can't merge thread with itself.")}, status=400)
 
     try:
-        other_thread = viewmodel(request, other_thread_id).thread
+        other_thread = viewmodel(request, other_thread_id, select_for_update=True).thread
     except PermissionDenied as e:
         return Response({
             'detail': e.args[0]
@@ -121,7 +119,7 @@ def clean_threads_for_merge(request):
     threads_queryset = Thread.objects.filter(
         id__in=threads_ids,
         category__tree_id=threads_tree_id,
-    ).select_related('category').order_by('-id')
+    ).select_for_update().select_related('category').order_by('-id')
 
     threads = []
     for thread in threads_queryset:
@@ -135,7 +133,6 @@ def clean_threads_for_merge(request):
     return threads
 
 
-@transaction.atomic
 def merge_threads(request, validated_data, threads):
     new_thread = Thread(
         category=validated_data['category'],

misago/threads/api/threadposts.py

@@ -1,4 +1,5 @@
 from django.core.exceptions import PermissionDenied
+from django.db import transaction
 from django.utils.translation import ugettext as _
 
 from rest_framework import viewsets
@@ -9,7 +10,8 @@ from misago.core.shortcuts import get_int_or_404
 from misago.users.online.utils import make_users_status_aware
 
 from ..models import Post
-from ..permissions.threads import allow_edit_post, allow_reply_thread
+from ..moderation import posts as moderation
+from ..permissions.threads import allow_delete_post, allow_edit_post, allow_reply_thread
 from ..serializers import PostSerializer
 from ..viewmodels.post import ThreadPost
 from ..viewmodels.posts import ThreadPosts
@@ -20,24 +22,60 @@ from .postingendpoint import PostingEndpoint
 class ViewSet(viewsets.ViewSet):
     thread = None
     posts = None
+    post = None
 
-    def get_thread(self, request, pk):
-        return self.thread(request, get_int_or_404(pk), read_aware=True, subscription_aware=True)
+    def get_thread(self, request, pk, read_aware=True, subscription_aware=True, select_for_update=False):
+        return self.thread(
+            request,
+            get_int_or_404(pk),
+            None,
+            read_aware,
+            subscription_aware,
+            select_for_update
+        )
+
+    def get_thread_for_update(self, request, pk):
+        return self.get_thread(
+            request, pk,
+            read_aware=False,
+            subscription_aware=False,
+            select_for_update=True
+        )
 
     def get_posts(self, request, thread, page):
         return self.posts(request, thread, page)
 
+    def get_post(self, request, thread, pk, select_for_update=False):
+        return self.post(request, thread, get_int_or_404(pk), select_for_update)
+
+    def get_post_for_update(self, request, thread, pk):
+        return self.get_post(request, thread, pk, select_for_update=True)
+
+    def list(self, request, thread_pk):
+        page = get_int_or_404(request.query_params.get('page', 0))
+        if page == 1:
+            page = 0 # api allows explicit first page
+
+        thread = self.get_thread(request, thread_pk)
+        posts = self.get_posts(request, thread, page)
+
+        data = thread.get_frontend_context()
+        data['post_set'] = posts.get_frontend_context()
+
+        return Response(data)
+
+    @transaction.atomic
     def create(self, request, thread_pk):
-        thread = self.thread(request, get_int_or_404(thread_pk))
-        allow_reply_thread(request.user, thread.thread)
+        thread = self.get_thread_for_update(request, thread_pk).thread
+        allow_reply_thread(request.user, thread)
 
-        post = Post(thread=thread.thread, category=thread.category)
+        post = Post(thread=thread, category=thread.category)
 
         # Put them through posting pipeline
         posting = PostingEndpoint(
             request,
             PostingEndpoint.REPLY,
-            thread=thread.thread,
+            thread=thread,
             post=post
         )
 
@@ -57,9 +95,10 @@ class ViewSet(viewsets.ViewSet):
         else:
             return Response(posting.errors, status=400)
 
+    @transaction.atomic
     def update(self, request, thread_pk, pk):
-        thread = self.thread(request, get_int_or_404(thread_pk))
-        post = ThreadPost(request, thread, get_int_or_404(pk)).post
+        thread = self.get_thread_for_update(request, thread_pk)
+        post = self.get_post_for_update(request, thread, pk).post
 
         allow_edit_post(request.user, post)
 
@@ -88,23 +127,26 @@ class ViewSet(viewsets.ViewSet):
 
         return Response({})
 
-    def list(self, request, thread_pk):
-        page = get_int_or_404(request.query_params.get('page', 0))
-        if page == 1:
-            page = 0 # api allows explicit first page
+    @transaction.atomic
+    def delete(self, request, thread_pk, pk):
+        thread = self.get_thread_for_update(request, thread_pk)
+        post = self.get_post_for_update(request, thread, pk).post
 
-        thread = self.get_thread(request, thread_pk)
-        posts = self.get_posts(request, thread, page)
+        allow_delete_post(request.user, post)
+        moderation.delete_post(request.user, post)
 
-        data = thread.get_frontend_context()
-        data['post_set'] = posts.get_frontend_context()
+        thread.thread.synchronize()
+        thread.thread.save()
 
-        return Response(data)
+        thread.category.synchronize()
+        thread.category.save()
+
+        return Response({})
 
     @detail_route(methods=['get'], url_path='editor')
     def post_editor(self, request, thread_pk, pk):
         thread = self.thread(request, get_int_or_404(thread_pk))
-        post = ThreadPost(request, thread, get_int_or_404(pk)).post
+        post = self.post(request, thread, get_int_or_404(pk)).post
 
         allow_edit_post(request.user, post)
 
@@ -123,7 +165,7 @@ class ViewSet(viewsets.ViewSet):
         allow_reply_thread(request.user, thread.thread)
 
         if 'reply' in request.query_params:
-            reply_to = ThreadPost(request, thread, get_int_or_404(request.query_params['reply'])).post
+            reply_to = self.post(request, thread, get_int_or_404(request.query_params['reply'])).post
 
             if reply_to.is_hidden and not reply_to.acl['can_see_hidden']:
                 raise PermissionDenied(_("You can't reply to hidden posts"))
@@ -140,3 +182,4 @@ class ViewSet(viewsets.ViewSet):
 class ThreadPostsViewSet(ViewSet):
     thread = ForumThread
     posts = ThreadPosts
+    post = ThreadPost

misago/threads/api/threads.py

@@ -29,8 +29,23 @@ class ViewSet(viewsets.ViewSet):
     thread = None
     TREE_ID = None
 
-    def get_thread(self, request, pk):
-        return self.thread(request, get_int_or_404(pk), read_aware=True, subscription_aware=True)
+    def get_thread(self, request, pk, read_aware=True, subscription_aware=True, select_for_update=False):
+        return self.thread(
+            request,
+            get_int_or_404(pk),
+            None,
+            read_aware,
+            subscription_aware,
+            select_for_update
+        )
+
+    def get_thread_for_update(self, request, pk):
+        return self.get_thread(
+            request, pk,
+            read_aware=False,
+            subscription_aware=False,
+            select_for_update=True
+        )
 
     def list(self, request):
         return threads_list_endpoint(request)
@@ -39,12 +54,14 @@ class ViewSet(viewsets.ViewSet):
         thread = self.get_thread(request, pk)
         return Response(thread.get_frontend_context())
 
+    @transaction.atomic
     def partial_update(self, request, pk):
-        thread = self.get_thread(request, pk).thread
+        thread = self.get_thread_for_update(request, pk).thread
         return thread_patch_endpoint(request, thread)
 
+    @transaction.atomic
     def destroy(self, request, pk):
-        thread = self.get_thread(request, pk).thread
+        thread = self.get_thread_for_update(request, pk).thread
 
         if thread.acl.get('can_hide') == 2:
             moderation.delete_thread(request.user, thread)
@@ -82,11 +99,13 @@ class ThreadViewSet(ViewSet):
             return Response(posting.errors, status=400)
 
     @detail_route(methods=['post'], url_path='merge')
+    @transaction.atomic
     def thread_merge(self, request, pk):
-        thread = self.get_thread(request, pk).thread
+        thread = self.get_thread_for_update(request, pk).thread
         return thread_merge_endpoint(request, thread, self.thread)
 
     @list_route(methods=['post'], url_path='merge')
+    @transaction.atomic
     def threads_merge(self, request):
         return threads_merge_endpoint(request)
 

misago/threads/moderation/posts.py

@@ -33,9 +33,8 @@ def unprotect_post(user, post):
 
 
 def unhide_post(user, post):
-    if post.pk == post.thread.first_post_id:
-        raise ModerationError(_("You can't make original post "
-                                " visible without revealing thread."))
+    if post.is_first_post:
+        raise ModerationError(_("You can't make original post visible without revealing thread."))
 
     if post.is_hidden:
         post.is_hidden = False
@@ -46,9 +45,8 @@ def unhide_post(user, post):
 
 
 def hide_post(user, post):
-    if post.pk == post.thread.first_post_id:
-        raise ModerationError(_("You can't hide original "
-                                "post without hiding thread."))
+    if post.is_first_post:
+        raise ModerationError(_("You can't hide original post without hiding thread."))
 
     if not post.is_hidden:
         post.is_hidden = True
@@ -70,9 +68,8 @@ def hide_post(user, post):
 
 @atomic
 def delete_post(user, post):
-    if post.pk == post.thread.first_post_id:
-        raise ModerationError(_("You can't delete original "
-                                "post without deleting thread."))
+    if post.is_first_post:
+        raise ModerationError(_("You can't delete original post without deleting thread."))
 
     post.delete()
     return True

misago/threads/permissions/threads.py

@@ -435,7 +435,7 @@ def add_acl_to_event(user, event):
     can_hide_events = category_acl.get('can_hide_events', 0)
 
     event.acl.update({
-        'can_see_hidden': can_hide_events,
+        'can_see_hidden': can_hide_events > 0,
         'can_hide': can_hide_events > 0,
         'can_delete': can_hide_events == 2,
     })
@@ -544,8 +544,7 @@ def allow_edit_thread(user, target):
                 "You can't edit threads that are older than %(minutes)s minute.",
                 "You can't edit threads that are older than %(minutes)s minutes.",
                 category_acl['thread_edit_time'])
-            raise PermissionDenied(
-                message % {'minutes': category_acl['thread_edit_time']})
+            raise PermissionDenied(message % {'minutes': category_acl['thread_edit_time']})
 can_edit_thread = return_boolean(allow_edit_thread)
 
 
@@ -588,8 +587,7 @@ def allow_edit_post(user, target):
                 "You can't edit posts that are older than %(minutes)s minute.",
                 "You can't edit posts that are older than %(minutes)s minutes.",
                 category_acl['post_edit_time'])
-            raise PermissionDenied(
-                message % {'minutes': category_acl['post_edit_time']})
+            raise PermissionDenied(message % {'minutes': category_acl['post_edit_time']})
 can_edit_post = return_boolean(allow_edit_post)
 
 
@@ -615,7 +613,7 @@ def allow_unhide_post(user, target):
         if target.is_protected and not category_acl['can_protect_posts']:
             raise PermissionDenied(_("This post is protected. You can't reveal it."))
 
-        if has_time_to_edit_post(user, target):
+        if not has_time_to_edit_post(user, target):
             message = ungettext(
                 "You can't reveal posts that are older than %(minutes)s minute.",
                 "You can't reveal posts that are older than %(minutes)s minutes.",
@@ -651,7 +649,7 @@ def allow_hide_post(user, target):
         if target.is_protected and not category_acl['can_protect_posts']:
             raise PermissionDenied(_("This post is protected. You can't hide it."))
 
-        if has_time_to_edit_post(user, target):
+        if not has_time_to_edit_post(user, target):
             message = ungettext(
                 "You can't hide posts that are older than %(minutes)s minute.",
                 "You can't hide posts that are older than %(minutes)s minutes.",
@@ -672,7 +670,7 @@ def allow_delete_post(user, target):
     category_acl = user.acl['categories'].get(target.category_id, {})
 
     if category_acl['can_hide_posts'] != 2:
-        if not category_acl['can_hide_own_posts'] != 2:
+        if category_acl['can_hide_own_posts'] != 2:
             raise PermissionDenied(_("You can't delete posts in this category."))
 
         if user.id != target.poster_id:
@@ -685,10 +683,9 @@ def allow_delete_post(user, target):
                 raise PermissionDenied(_("This thread is closed. You can't delete posts from it."))
 
         if target.is_protected and not category_acl['can_protect_posts']:
-            raise PermissionDenied(
-                _("This post is protected. You can't delete it."))
+            raise PermissionDenied(_("This post is protected. You can't delete it."))
 
-        if has_time_to_edit_post(user, target):
+        if not has_time_to_edit_post(user, target):
             message = ungettext(
                 "You can't delete posts that are older than %(minutes)s minute.",
                 "You can't delete posts that are older than %(minutes)s minutes.",
@@ -717,23 +714,23 @@ def can_change_owned_thread(user, target):
 
 
 def has_time_to_edit_thread(user, target):
-    category_acl = user.acl['categories'].get(target.category_id, {})
-    if category_acl.get('thread_edit_time'):
+    edit_time = user.acl['categories'].get(target.category_id, {}).get('thread_edit_time', 0)
+    if edit_time:
         diff = timezone.now() - target.started_on
         diff_minutes = int(diff.total_seconds() / 60)
 
-        return diff_minutes < category_acl.get('thread_edit_time')
+        return diff_minutes < edit_time
     else:
         return True
 
 
 def has_time_to_edit_post(user, target):
-    category_acl = user.acl['categories'].get(target.category_id, {})
-    if category_acl.get('post_edit_time'):
+    edit_time = user.acl['categories'].get(target.category_id, {}).get('post_edit_time', 0)
+    if edit_time:
         diff = timezone.now() - target.posted_on
         diff_minutes = int(diff.total_seconds() / 60)
 
-        return diff_minutes < category_acl.get('post_edit_time')
+        return diff_minutes < edit_time
     else:
         return True
 

misago/threads/tests/test_threads_posts_api.py

@@ -0,0 +1,87 @@
+import time
+
+from django.core.urlresolvers import reverse
+
+from .. import testutils
+from ..models import Post, Thread
+from .test_threads_api import ThreadsApiTestCase
+
+
+class PostDeleteApiTests(ThreadsApiTestCase):
+    def setUp(self):
+        super(PostDeleteApiTests, self).setUp()
+
+        self.post = testutils.reply_thread(self.thread, poster=self.user)
+
+        self.api_link = reverse('misago:api:thread-post-detail', kwargs={
+            'thread_pk': self.thread.pk,
+            'pk': self.post.pk
+        })
+
+    def refresh_thread(self):
+        self.thread = Thread.objects.get(pk=self.thread.pk)
+
+    def test_delete_anonymous(self):
+        """api validates if deleting user is authenticated"""
+        self.logout_user()
+
+        response = self.client.delete(self.api_link)
+        self.assertContains(response, "This action is not available to guests.", status_code=403)
+
+    def test_no_permission(self):
+        """api validates permission to delete post"""
+        self.override_acl({
+            'can_hide_own_posts': 1,
+            'can_hide_posts': 1
+        })
+
+        response = self.client.delete(self.api_link)
+        self.assertContains(response, "You can't delete posts in this category.", status_code=403)
+
+    def test_delete_first_post(self):
+        """api disallows first post's deletion"""
+        self.override_acl({
+            'can_hide_own_posts': 2,
+            'can_hide_posts': 2
+        })
+
+        api_link = reverse('misago:api:thread-post-detail', kwargs={
+            'thread_pk': self.thread.pk,
+            'pk': self.thread.first_post_id
+        })
+
+        response = self.client.delete(api_link)
+        self.assertContains(response, "You can't delete thread's first post.", status_code=403)
+
+    def test_delete_owned_post(self):
+        """api deletes owned thread post"""
+        self.override_acl({
+            'post_edit_time': 0,
+            'can_hide_own_posts': 2,
+            'can_hide_posts': 0
+        })
+
+        response = self.client.delete(self.api_link)
+        self.assertEqual(response.status_code, 200)
+
+        self.refresh_thread()
+
+        self.assertNotEqual(self.thread.last_post_id, self.post.pk)
+        with self.assertRaises(Post.DoesNotExist):
+            self.thread.post_set.get(pk=self.post.pk)
+
+    def test_delete_post(self):
+        """api deletes thread post"""
+        self.override_acl({
+            'can_hide_own_posts': 0,
+            'can_hide_posts': 2
+        })
+
+        response = self.client.delete(self.api_link)
+        self.assertEqual(response.status_code, 200)
+
+        self.refresh_thread()
+
+        self.assertNotEqual(self.thread.last_post_id, self.post.pk)
+        with self.assertRaises(Post.DoesNotExist):
+            self.thread.post_set.get(pk=self.post.pk)

misago/threads/viewmodels/post.py

@@ -6,15 +6,24 @@ from ..permissions.threads import allow_see_post, exclude_invisible_posts
 
 
 class ViewModel(object):
-    def __init__(self, request, thread, pk):
-        post = self.get_post(request, thread, pk)
+    def __init__(self, request, thread, pk, select_for_update=False):
+        post = self.get_post(request, thread, pk, select_for_update)
 
         add_acl(request.user, post)
 
         self.post = post
 
-    def get_post(self, request, thread, pk):
+    def get_post(self, request, thread, pk, select_for_update=False):
         queryset = self.get_queryset(request, thread.thread)
+        if select_for_update:
+            queryset = queryset.select_for_update()
+        else:
+            queryset = queryset.select_related(
+                'poster',
+                'poster__rank',
+                'poster__ban_cache'
+            )
+
         post = get_object_or_404(queryset, pk=pk, is_event=False)
 
         post.category = thread.category
@@ -24,12 +33,7 @@ class ViewModel(object):
         return post
 
     def get_queryset(self, request, thread):
-        queryset = thread.post_set.select_related(
-            'poster',
-            'poster__rank',
-            'poster__ban_cache'
-        )
-        return exclude_invisible_posts(request.user, thread.category, queryset)
+        return exclude_invisible_posts(request.user, thread.category, thread.post_set)
 
 
 class ThreadPost(ViewModel):

misago/threads/viewmodels/thread.py

@@ -12,13 +12,12 @@ from ..serializers import ThreadSerializer
 from ..subscriptions import make_subscription_aware
 
 
-BASE_QUERYSET = Thread.objects.select_related(
-    'category', 'starter', 'starter__rank', 'starter__ban_cache', 'starter__online_tracker')
+BASE_RELATIONS = ('category', 'starter', 'starter__rank', 'starter__ban_cache', 'starter__online_tracker')
 
 
 class ViewModel(object):
-    def __init__(self, request, pk, slug=None, read_aware=False, subscription_aware=False):
-        thread = self.get_thread(request, pk, slug)
+    def __init__(self, request, pk, slug=None, read_aware=False, subscription_aware=False, select_for_update=False):
+        thread = self.get_thread(request, pk, slug, select_for_update)
 
         thread.path = self.get_thread_path(thread.category)
 
@@ -34,7 +33,7 @@ class ViewModel(object):
         self.category = thread.category
         self.path = thread.path
 
-    def get_thread(self, request, pk, slug=None):
+    def get_thread(self, request, pk, slug=None, select_for_update=False):
         raise NotImplementedError('Thread view model has to implement get_thread(request, pk, slug=None)')
 
     def get_thread_path(self, category):
@@ -68,11 +67,16 @@ class ViewModel(object):
 
 
 class ForumThread(ViewModel):
-    def get_thread(self, request, pk, slug=None):
+    def get_thread(self, request, pk, slug=None, select_for_update=False):
+        if select_for_update:
+            queryset = Thread.objects.select_for_update().select_related('category')
+        else:
+            queryset = Thread.objects.select_related(*BASE_RELATIONS)
+
         thread = get_object_or_404(
-            BASE_QUERYSET,
+            queryset,
             pk=pk,
-            category__tree_id=Category.objects.root_category().tree_id,
+            category__tree_id=Category.objects.root_category().tree_id
         )
 
         allow_see_thread(request.user, thread)