Merge tweaks and handling for best answers

misago/threads/models/post.py

@@ -118,7 +118,10 @@ class Post(models.Model):
         super(Post, self).delete(*args, **kwargs)
 
     def merge(self, other_post):
-        if not self.poster_id or self.poster_id != other_post.poster_id:
+        if self.poster_id != other_post.poster_id:
+            raise ValueError("post can't be merged with other user's post")
+        elif (self.poster_id is None and other_post.poster_id is None and
+                self.poster_name != other_post.poster_name):
             raise ValueError("post can't be merged with other user's post")
 
         if self.thread_id != other_post.thread_id:

misago/threads/models/thread.py

@@ -278,6 +278,15 @@ class Thread(models.Model):
             self.last_poster_slug = slugify(post.poster_name)
 
     def set_best_answer(self, user, post):
+        if post.thread_id != self.id:
+            raise ValueError("post to set as best answer must be in same thread")
+        if post.is_first_post:
+            raise ValueError("post to set as best answer can't be first post")
+        if post.is_hidden:
+            raise ValueError("post to set as best answer can't be hidden")
+        if post.is_unapproved:
+            raise ValueError("post to set as best answer can't be unapproved")
+
         self.best_answer = post
         self.best_answer_is_protected = post.is_protected
         self.best_answer_marked_on = timezone.now()
@@ -287,7 +296,7 @@ class Thread(models.Model):
 
     def clear_best_answer(self):
         self.best_answer = None
-        self.best_answer_is_protected = None
+        self.best_answer_is_protected = False
         self.best_answer_marked_on = None
         self.best_answer_marked_by = None
         self.best_answer_marked_by_name = None

misago/threads/serializers/moderation.py

@@ -133,23 +133,28 @@ class MergePostsSerializer(serializers.Serializer):
 
             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 serializers.ValidationError(authorship_error)
-                else:
-                    if post.poster_id or post.poster_name != posts[0].poster_name:
-                        raise serializers.ValidationError(authorship_error)
+                continue
+            
+            authorship_error = _("Posts made by different users can't be merged.")
+            if post.poster_id != posts[0].poster_id:
+                raise serializers.ValidationError(authorship_error)
+            elif (post.poster_id is None and posts[0].poster_id is None and 
+                    post.poster_name != posts[0].poster_name):
+                raise serializers.ValidationError(authorship_error)
+
+            if posts[0].is_first_post and post.is_best_answer:
+                raise serializers.ValidationError(
+                    _("Post marked as best answer can't be merged with thread's first post.")
+                )
 
-                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.")
-                        )
+            if not posts[0].is_first_post:
+                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.")
+                    )
 
-                posts.append(post)
+            posts.append(post)
 
         if len(posts) != len(data):
             raise serializers.ValidationError(_("One or more posts to merge could not be found."))

misago/threads/tests/test_post_model.py

@@ -143,6 +143,61 @@ class PostModelTests(TestCase):
         self.assertIn(other_post.parsed, self.post.parsed)
         self.assertTrue(self.post.is_valid)
 
+    def test_merge_best_answer(self):
+        """merge method merges best answer into post"""
+        best_answer = Post.objects.create(
+            category=self.category,
+            thread=self.thread,
+            poster=self.user,
+            poster_name=self.user.username,
+            poster_ip='127.0.0.1',
+            original="I am other message!",
+            parsed="<p>I am other message!</p>",
+            checksum="nope",
+            posted_on=timezone.now() + timedelta(minutes=5),
+            updated_on=timezone.now() + timedelta(minutes=5),
+        )
+
+        self.thread.set_best_answer(self.user, best_answer)
+        self.thread.save()
+
+        best_answer.merge(self.post)
+        self.assertEqual(self.thread.best_answer, self.post)
+
+    def test_merge_in_best_answer(self):
+        """merge method merges post into best answert"""
+        best_answer = Post.objects.create(
+            category=self.category,
+            thread=self.thread,
+            poster=self.user,
+            poster_name=self.user.username,
+            poster_ip='127.0.0.1',
+            original="I am other message!",
+            parsed="<p>I am other message!</p>",
+            checksum="nope",
+            posted_on=timezone.now() + timedelta(minutes=5),
+            updated_on=timezone.now() + timedelta(minutes=5),
+        )
+
+        other_post = Post.objects.create(
+            category=self.category,
+            thread=self.thread,
+            poster=self.user,
+            poster_name=self.user.username,
+            poster_ip='127.0.0.1',
+            original="I am other message!",
+            parsed="<p>I am other message!</p>",
+            checksum="nope",
+            posted_on=timezone.now() + timedelta(minutes=5),
+            updated_on=timezone.now() + timedelta(minutes=5),
+        )
+
+        self.thread.set_best_answer(self.user, best_answer)
+        self.thread.save()
+
+        other_post.merge(best_answer)
+        self.assertEqual(self.thread.best_answer, best_answer)
+
     def test_move(self):
         """move method moves post to other thread"""
         new_thread = Thread.objects.create(

misago/threads/tests/test_thread_model.py

@@ -5,6 +5,7 @@ from django.test import TestCase
 from django.utils import timezone
 
 from misago.categories.models import Category
+from misago.threads import testutils
 from misago.threads.models import Poll, Post, Thread, ThreadParticipant
 
 
@@ -29,7 +30,7 @@ class ThreadModelTests(TestCase):
         self.thread.set_title("Test thread")
         self.thread.save()
 
-        post = Post.objects.create(
+        Post.objects.create(
             category=self.category,
             thread=self.thread,
             poster_name='Tester',
@@ -41,8 +42,7 @@ class ThreadModelTests(TestCase):
             updated_on=datetime,
         )
 
-        self.thread.first_post = post
-        self.thread.last_post = post
+        self.thread.synchronize()
         self.thread.save()
 
     def test_synchronize(self):
@@ -283,9 +283,68 @@ class ThreadModelTests(TestCase):
         self.assertEqual(self.thread.last_poster_name, user.username)
         self.assertEqual(self.thread.last_poster_slug, user.slug)
 
+    def test_set_best_answer(self):
+        """set_best_answer sets best answer and setter data on thread"""
+        user = UserModel.objects.create_user("Bob", "bob@boberson.com", "Pass.123")
+
+        best_answer = Post.objects.create(
+            category=self.category,
+            thread=self.thread,
+            poster=user,
+            poster_name=user.username,
+            poster_ip='127.0.0.1',
+            original="Hello! I am test message!",
+            parsed="<p>Hello! I am test message!</p>",
+            checksum="nope",
+            posted_on=timezone.now(),
+            updated_on=timezone.now(),
+            is_protected=True,
+        )
+
+        self.thread.synchronize()
+        self.thread.save()
+
+        self.thread.set_best_answer(user, best_answer)
+        self.thread.save()
+
+        self.assertEqual(self.thread.best_answer, best_answer)
+        self.assertTrue(self.thread.best_answer_is_protected)
+        self.assertTrue(self.thread.best_answer_marked_on)
+        self.assertEqual(self.thread.best_answer_marked_by, user)
+        self.assertEqual(self.thread.best_answer_marked_by_name, user.username)
+        self.assertEqual(self.thread.best_answer_marked_by_slug, user.slug)
+
+        # clear best answer
+        self.thread.clear_best_answer()
+
+        self.assertIsNone(self.thread.best_answer, best_answer)
+        self.assertFalse(self.thread.best_answer_is_protected)
+        self.assertIsNone(self.thread.best_answer_marked_on)
+        self.assertIsNone(self.thread.best_answer_marked_by)
+        self.assertIsNone(self.thread.best_answer_marked_by_name)
+        self.assertIsNone(self.thread.best_answer_marked_by_slug)
+
+    def test_set_invalid_best_answer(self):
+        """set_best_answer implements some assertions for data integrity"""
+        user = UserModel.objects.create_user("Bob", "bob@boberson.com", "Pass.123")
+
+        other_thread = testutils.post_thread(self.category)
+        with self.assertRaises(ValueError):
+            self.thread.set_best_answer(user, other_thread.first_post)
+
+        with self.assertRaises(ValueError):
+            self.thread.set_best_answer(user, self.thread.first_post)
+
+        with self.assertRaises(ValueError):
+            reply = testutils.reply_thread(self.thread, is_hidden=True)
+            self.thread.set_best_answer(user, reply)
+
+        with self.assertRaises(ValueError):
+            reply = testutils.reply_thread(self.thread, is_unapproved=True)
+            self.thread.set_best_answer(user, reply)
+
     def test_move(self):
         """move(new_category) moves thread to other category"""
-        # pick category instead of category (so we don't have to create one)
         root_category = Category.objects.root_category()
         Category(
             name='New Category',

misago/threads/tests/test_thread_postmerge_api.py

@@ -268,8 +268,8 @@ class ThreadPostMergeApiTestCase(AuthenticatedUserTestCase):
             self.api_link,
             json.dumps({
                 'posts': [
-                    testutils.reply_thread(self.thread, poster="Bob", is_hidden=True).pk,
-                    testutils.reply_thread(self.thread, poster="Bob", is_hidden=False).pk,
+                    testutils.reply_thread(self.thread, poster=self.user, is_hidden=True).pk,
+                    testutils.reply_thread(self.thread, poster=self.user, is_hidden=False).pk,
                 ]
             }),
             content_type="application/json",
@@ -286,8 +286,8 @@ class ThreadPostMergeApiTestCase(AuthenticatedUserTestCase):
             self.api_link,
             json.dumps({
                 'posts': [
-                    testutils.reply_thread(self.thread, poster="Bob", is_unapproved=True).pk,
-                    testutils.reply_thread(self.thread, poster="Bob", is_unapproved=False).pk,
+                    testutils.reply_thread(self.thread, poster=self.user, is_unapproved=True).pk,
+                    testutils.reply_thread(self.thread, poster=self.user, is_unapproved=False).pk,
                 ]
             }),
             content_type="application/json",
@@ -358,6 +358,32 @@ class ThreadPostMergeApiTestCase(AuthenticatedUserTestCase):
         )
         self.assertEqual(response.status_code, 200)
 
+    def test_merge_best_answer_first_post(self):
+        """api recjects attempt to merge best_answer with first post"""
+        self.thread.first_post.poster = self.user
+        self.thread.first_post.save()
+
+        self.post.poster = self.user
+        self.post.save()
+
+        self.thread.set_best_answer(self.user, self.post)
+        self.thread.save()
+         
+        response = self.client.post(
+            self.api_link,
+            json.dumps({
+                'posts': [
+                    self.thread.first_post.pk,
+                    self.post.pk,
+                ]
+            }),
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.json(), {
+            'detail': "Post marked as best answer can't be merged with thread's first post."
+        })
+
     def test_merge_posts(self):
         """api merges two posts"""
         post_a = testutils.reply_thread(self.thread, poster=self.user, message="Battęry")
@@ -383,6 +409,20 @@ class ThreadPostMergeApiTestCase(AuthenticatedUserTestCase):
         merged_post = Post.objects.get(pk=post_a.pk)
         self.assertEqual(merged_post.parsed, '{}\n{}'.format(post_a.parsed, post_b.parsed))
 
+    def test_merge_guest_posts(self):
+        """api recjects attempt to merge posts made by same guest"""
+        response = self.client.post(
+            self.api_link,
+            json.dumps({
+                'posts': [
+                    testutils.reply_thread(self.thread, poster="Bob").pk,
+                    testutils.reply_thread(self.thread, poster="Bob").pk,
+                ]
+            }),
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, 200)
+
     def test_merge_hidden_posts(self):
         """api merges two hidden posts"""
         self.override_acl({'can_hide_posts': 1})
@@ -434,6 +474,51 @@ class ThreadPostMergeApiTestCase(AuthenticatedUserTestCase):
         )
         self.assertEqual(response.status_code, 200)
 
+    def test_merge_best_answer(self):
+        """api merges best answer with other post"""
+        best_answer = testutils.reply_thread(self.thread, poster="Bob")
+
+        self.thread.set_best_answer(self.user, best_answer)
+        self.thread.save()
+         
+        response = self.client.post(
+            self.api_link,
+            json.dumps({
+                'posts': [
+                    best_answer.pk,
+                    testutils.reply_thread(self.thread, poster="Bob").pk,
+                ]
+            }),
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, 200)
+
+        self.refresh_thread()
+        self.assertEqual(self.thread.best_answer, best_answer)
+
+    def test_merge_best_answer_in(self):
+        """api merges best answer into other post"""
+        other_post = testutils.reply_thread(self.thread, poster="Bob")
+        best_answer = testutils.reply_thread(self.thread, poster="Bob")
+
+        self.thread.set_best_answer(self.user, best_answer)
+        self.thread.save()
+         
+        response = self.client.post(
+            self.api_link,
+            json.dumps({
+                'posts': [
+                    best_answer.pk,
+                    other_post.pk,
+                ]
+            }),
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, 200)
+
+        self.refresh_thread()
+        self.assertEqual(self.thread.best_answer, other_post)
+
     def test_merge_remove_reads(self):
         """two posts merge removes read tracker from post"""
         post_a = testutils.reply_thread(self.thread, poster=self.user, message="Battęry")