WIP merge conflict utility

misago/threads/api/threadendpoints/merge.py

@@ -6,10 +6,10 @@ from django.utils.translation import ugettext as _
 
 from misago.acl import add_acl
 from misago.threads.events import record_event
+from misago.threads.mergeconflict import MergeConflict
 from misago.threads.models import Thread
 from misago.threads.moderation import threads as moderation
 from misago.threads.permissions import allow_merge_thread
-from misago.threads.pollmergehandler import PollMergeHandler
 from misago.threads.serializers import (
     MergeThreadSerializer, MergeThreadsSerializer, ThreadsListSerializer)
 
@@ -31,6 +31,8 @@ def thread_merge_endpoint(request, thread, viewmodel):
             errors = serializer.errors['other_thread']
         elif 'poll' in serializer.errors:
             errors = serializer.errors['poll']
+        elif 'polls' in serializer.errors:
+            return Response({'polls': serializer.errors['polls']}, status=400)
         else:
             errors = list(serializer.errors.values())[0]
         return Response({'detail': errors[0]}, status=400)
@@ -39,18 +41,18 @@ def thread_merge_endpoint(request, thread, viewmodel):
     if serializer.validated_data.get('polls'):
         return Response({'polls': serializer.validated_data['polls']}, status=400)
 
-    # merge polls
+    # merge conflict
     other_thread = serializer.validated_data['other_thread']
-    poll = serializer.validated_data.get('poll')
 
-    if len(serializer.polls_handler.polls) == 1:
-        poll.move(other_thread)
-    elif serializer.polls_handler.is_merge_conflict():
+    poll = serializer.validated_data.get('poll')
+    if 'poll' in serializer.merge_conflict:
         if poll and poll.thread_id != other_thread.id:
             other_thread.poll.delete()
             poll.move(other_thread)
         elif not poll:
             other_thread.poll.delete()
+    elif poll:
+        poll.move(other_thread)
 
     # merge thread contents
     moderation.merge_thread(request, other_thread, thread)

misago/threads/mergeconflict.py

@@ -0,0 +1,127 @@
+from rest_framework.exceptions import ValidationError
+
+from django.utils.translation import ugettext as _
+
+from misago.threads.models import Poll
+
+
+class MergeConflictHandler(object):
+    def __init__(self, threads):
+        self.items = []
+        self.choices = {0: None}
+
+        self._is_valid = False
+        self._resolution = None
+
+        self.threads = threads
+        self.populate_from_threads(threads)
+
+        if len(self.items) == 1:
+            self._is_valid = True
+            self._resolution = self.items[0]
+
+    def populate_from_threads(self, threads):
+        raise NotImplementedError('merge handler must define populate_from_threads')
+
+    def is_merge_conflict(self):
+        return len(self.items) > 1
+
+    def set_resolution(self, resolution):
+        try:
+            resolution_clean = int(resolution)
+        except (TypeError, ValueError):
+            return
+
+        if resolution_clean in self.choices:
+            self._resolution = self.choices[resolution_clean]
+            self._is_valid = True
+
+    def is_valid(self):
+        return self._is_valid
+
+    def get_resolution(self):
+        return self._resolution or None
+
+
+class BestAnswerMergeHandler(MergeConflictHandler):
+    data_name = 'best_answer'
+    
+    def populate_from_threads(self, threads):
+        for thread in threads:
+            if thread.best_answer_id:
+                self.items.append(thread)
+                self.choices[thread.pk] = thread
+        self.items.sort(key=lambda choice: choice.id, reverse=True)
+
+    def get_available_resolutions(self):
+        resolutions = [[0, _("Unmark all best answers")]]
+        for thread in self.items:
+            resolutions.append([thread.pk, thread.title])
+        return resolutions
+
+
+class PollMergeHandler(MergeConflictHandler):
+    data_name = 'poll'
+
+    def populate_from_threads(self, threads):
+        for thread in threads:
+            try:
+                self.items.append(thread.poll)
+                self.choices[thread.poll.id] = thread.poll
+            except Poll.DoesNotExist:
+                pass
+        self.items.sort(key=lambda choice: choice.thread_id, reverse=True)
+
+    def get_available_resolutions(self):
+        resolutions = [[0, _("Delete all polls")]]
+        for poll in self.items:
+            resolutions.append([poll.id, poll.question])
+        return resolutions
+
+
+class MergeConflict(object):
+    """
+    Utility class single point of entry for detecting merge conflicts on different properties
+    and validating user resolutions.
+    """
+    HANDLERS = (
+        BestAnswerMergeHandler,
+        PollMergeHandler,
+    )
+
+    def __init__(self, data=None, threads=None):
+        self.data = data or {}
+        self._handlers = [Handler(threads) for Handler in self.HANDLERS]
+        self._conflicts = [i for i in self._handlers if i.is_merge_conflict()]
+        self.set_resolution(data)
+
+    def is_merge_conflict(self):
+        return bool(self._conflicts)
+
+    def get_merge_conflict(self):
+        return [i.data_name for i in self._conflicts]
+
+    def set_resolution(self, data):
+        for handler in self._conflicts:
+            data = self.data.get(handler.data_name)
+            handler.set_resolution(data)
+
+    def is_valid(self, raise_exception=False):
+        if raise_exception:
+            self.raise_exception()
+        return all([i.is_valid() for i in self._handlers])
+
+    def raise_exception(self):
+        errors = {}
+        for handler in self._conflicts:
+            if self.data.get(handler.data_name) is None:
+                key = '{}s'.format(handler.data_name)
+                errors[key] = handler.get_available_resolutions()
+            elif not handler.is_valid():
+                errors[handler.data_name] = [_("Invalid choice.")]
+        if errors:
+            raise ValidationError(errors)
+
+    def get_resolution(self):
+        resolved_handlers = [i for i in self._handlers if i.is_valid()]
+        return {i.data_name: i.get_resolution() for i in resolved_handlers}

misago/threads/pollmergehandler.py

@@ -1,52 +0,0 @@
-from django.utils.translation import ugettext as _
-
-from misago.threads.models import Poll
-
-
-class PollMergeHandler(object):
-    def __init__(self, threads):
-        self._list = []
-        self._choices = {0: None}
-
-        self._is_valid = False
-        self._resolution = None
-
-        self.threads = threads
-
-        for thread in threads:
-            try:
-                self._list.append(thread.poll)
-                self._choices[thread.poll.pk] = thread.poll
-            except Poll.DoesNotExist:
-                pass
-
-        self._list.sort(key=lambda choice: choice.thread_id)
-
-    @property
-    def polls(self):
-        return self._list
-
-    def is_merge_conflict(self):
-        return len(self._list) > 1
-
-    def get_available_resolutions(self):
-        resolutions = [(0, _("Delete all polls"))]
-        for poll in self._list:
-            resolutions.append((poll.pk, poll.question))
-        return resolutions
-
-    def set_resolution(self, resolution):
-        try:
-            resolution_clean = int(resolution)
-        except (TypeError, ValueError):
-            return
-
-        if resolution_clean in self._choices:
-            self._resolution = self._choices[resolution_clean]
-            self._is_valid = True
-
-    def is_valid(self):
-        return self._is_valid
-
-    def get_resolution(self):
-        return self._resolution or None

misago/threads/serializers/moderation.py

@@ -8,6 +8,7 @@ from django.utils.translation import ugettext as _, ugettext_lazy, ungettext
 from misago.acl import add_acl
 from misago.categories import THREADS_ROOT_NAME
 from misago.conf import settings
+from misago.threads.mergeconflict import MergeConflict
 from misago.threads.models import Thread
 from misago.threads.permissions import (
     allow_delete_best_answer, allow_delete_event, allow_delete_post, allow_delete_thread,
@@ -15,7 +16,6 @@ from misago.threads.permissions import (
     allow_move_post, allow_split_post,
     can_reply_thread, can_see_thread,
     can_start_thread, exclude_invisible_posts)
-from misago.threads.pollmergehandler import PollMergeHandler
 from misago.threads.threadtypes import trees_map
 from misago.threads.utils import get_thread_id_from_url
 from misago.threads.validators import validate_category, validate_title
@@ -458,21 +458,10 @@ class MergeThreadSerializer(serializers.Serializer):
         thread = self.context['thread']
         other_thread = data['other_thread']
 
-        polls_handler = PollMergeHandler([thread, other_thread])
-
-        if len(polls_handler.polls) == 1:
-            data['poll'] = polls_handler.polls[0]
-        elif polls_handler.is_merge_conflict():
-            if 'poll' in data:
-                polls_handler.set_resolution(data['poll'])
-                if polls_handler.is_valid():
-                    data['poll'] = polls_handler.get_resolution()
-                else:
-                    raise serializers.ValidationError({'poll': _("Invalid choice.")})
-            else:
-                data['polls'] = polls_handler.get_available_resolutions()
-
-        self.polls_handler = polls_handler
+        merge_conflict = MergeConflict(data, [thread, other_thread])
+        merge_conflict.is_valid(raise_exception=True)
+        data.update(merge_conflict.get_resolution())
+        self.merge_conflict = merge_conflict.get_merge_conflict()
 
         return data
 

misago/threads/tests/test_mergeconflict.py

@@ -0,0 +1,209 @@
+from rest_framework.exceptions import ValidationError
+
+from django.contrib.auth import get_user_model
+from django.test import TestCase
+
+from misago.categories.models import Category
+
+from misago.threads import testutils
+from misago.threads.mergeconflict import MergeConflict
+
+
+UserModel = get_user_model()
+
+
+class MergeConflictTests(TestCase):
+    def setUp(self):
+        self.category = Category.objects.get(slug='first-category')
+        self.user = UserModel.objects.create_user('bob', 'bob@test.com', 'Pass.123')
+
+    def create_plain_thread(self):
+        return testutils.post_thread(self.category)
+
+    def create_poll_thread(self):
+        thread = testutils.post_thread(self.category)
+        testutils.post_poll(thread, self.user)
+        return thread
+
+    def create_best_answer_thread(self):
+        thread = testutils.post_thread(self.category)
+        best_answer = testutils.reply_thread(thread)
+        thread.set_best_answer(self.user, best_answer)
+        thread.synchronize()
+        thread.save()
+        return thread
+
+    def test_plain_threads_no_conflicts(self):
+        """threads without items of interest don't conflict"""
+        threads = [self.create_plain_thread() for i in range(10)]
+        merge_conflict = MergeConflict(threads=threads)
+        self.assertFalse(merge_conflict.is_merge_conflict())
+        self.assertEqual(merge_conflict.get_merge_conflict(), [])
+
+        merge_conflict.is_valid(raise_exception=True)
+        self.assertEqual(merge_conflict.get_resolution(), {})
+
+    def test_one_best_answer_one_plain(self):
+        """thread with best answer and plain thread don't conflict"""
+        threads = [
+            self.create_best_answer_thread(),
+            self.create_plain_thread(),
+        ]
+        merge_conflict = MergeConflict(threads=threads)
+        self.assertFalse(merge_conflict.is_merge_conflict())
+        self.assertEqual(merge_conflict.get_merge_conflict(), [])
+
+        merge_conflict.is_valid(raise_exception=True)
+        self.assertEqual(merge_conflict.get_resolution(), {
+            'best_answer': threads[0],
+        })
+
+    def test_one_poll_one_plain(self):
+        """thread with poll and plain thread don't conflict"""
+        threads = [
+            self.create_poll_thread(),
+            self.create_plain_thread(),
+        ]
+        merge_conflict = MergeConflict(threads=threads)
+        self.assertFalse(merge_conflict.is_merge_conflict())
+        self.assertEqual(merge_conflict.get_merge_conflict(), [])
+
+        merge_conflict.is_valid(raise_exception=True)
+        self.assertEqual(merge_conflict.get_resolution(), {
+            'poll': threads[0].poll,
+        })
+
+    def test_one_best_answer_one_poll(self):
+        """thread with best answer and thread with poll don't conflict"""
+        threads = [
+            self.create_poll_thread(),
+            self.create_best_answer_thread(),
+        ]
+        merge_conflict = MergeConflict(threads=threads)
+        self.assertFalse(merge_conflict.is_merge_conflict())
+
+    def test_one_best_answer_one_poll_one_plain(self):
+        """thread with best answer, thread with poll and plain thread don't conflict"""
+        threads = [
+            self.create_plain_thread(),
+            self.create_poll_thread(),
+            self.create_best_answer_thread(),
+        ]
+        merge_conflict = MergeConflict(threads=threads)
+        self.assertFalse(merge_conflict.is_merge_conflict())
+
+    def test_three_best_answers_one_poll_two_plain_conflict(self):
+        """three threads with best answer, thread with poll and two plain threads conflict"""
+        best_answers = [self.create_best_answer_thread() for i in range(3)]
+        polls = [self.create_poll_thread()]
+        threads = [
+            self.create_plain_thread(),
+            self.create_plain_thread(),
+        ] + best_answers + polls
+
+        merge_conflict = MergeConflict(threads=threads)
+        self.assertTrue(merge_conflict.is_merge_conflict())
+        self.assertEqual(merge_conflict.get_merge_conflict(), ['best_answer'])
+
+        # without choice, conflict lists resolutions
+        try:
+            merge_conflict.is_valid(raise_exception=True)
+            self.fail("merge_conflict.is_valid() should raise ValidationError")
+        except ValidationError as e:
+            self.assertTrue(merge_conflict.is_merge_conflict())
+            self.assertEqual(merge_conflict.get_merge_conflict(), ['best_answer'])
+            self.assertEqual(e.detail, {
+                'best_answers': [['0', 'Unmark all best answers']] + [
+                    [
+                        str(thread.id),
+                        thread.title,
+                    ] for thread in reversed(best_answers)
+                ]
+            })
+
+        # conflict validates choice
+        try:
+            merge_conflict = MergeConflict({'best_answer': threads[0].id}, threads)
+            merge_conflict.is_valid(raise_exception=True)
+            self.fail("merge_conflict.is_valid() should raise ValidationError")
+        except ValidationError as e:
+            self.assertTrue(merge_conflict.is_merge_conflict())
+            self.assertEqual(e.detail, {'best_answer': ['Invalid choice.']})
+
+        # conflict returns selected resolution
+        merge_conflict = MergeConflict({'best_answer': best_answers[0].id}, threads)
+        self.assertTrue(merge_conflict.is_merge_conflict())
+        self.assertEqual(merge_conflict.get_merge_conflict(), ['best_answer'])
+        merge_conflict.is_valid(raise_exception=True)
+        self.assertEqual(merge_conflict.get_resolution(), {
+            'best_answer': best_answers[0],
+            'poll': polls[0].poll,
+        })
+
+        # conflict returns no-choice resolution
+        merge_conflict = MergeConflict({'best_answer': 0}, threads)
+        self.assertTrue(merge_conflict.is_merge_conflict())
+        self.assertEqual(merge_conflict.get_merge_conflict(), ['best_answer'])
+        merge_conflict.is_valid(raise_exception=True)
+        self.assertEqual(merge_conflict.get_resolution(), {
+            'best_answer': None,
+            'poll': polls[0].poll,
+        })
+
+    def test_one_best_answer_three_poll_two_plain_conflict(self):
+        """three threads with best answer, thread with poll and two plain threads conflict"""
+        best_answers = [self.create_best_answer_thread()]
+        polls = [self.create_poll_thread() for i in range(3)]
+        threads = [
+            self.create_plain_thread(),
+            self.create_plain_thread(),
+        ] + best_answers + polls
+
+        merge_conflict = MergeConflict(threads=threads)
+        self.assertTrue(merge_conflict.is_merge_conflict())
+        self.assertEqual(merge_conflict.get_merge_conflict(), ['poll'])
+
+        # without choice, conflict lists resolutions
+        try:
+            merge_conflict.is_valid(raise_exception=True)
+            self.fail("merge_conflict.is_valid() should raise ValidationError")
+        except ValidationError as e:
+            self.assertTrue(merge_conflict.is_merge_conflict())
+            self.assertEqual(merge_conflict.get_merge_conflict(), ['poll'])
+            self.assertEqual(e.detail, {
+                'polls': [['0', 'Delete all polls']] + [
+                    [
+                        str(thread.poll.id),
+                        thread.poll.question,
+                    ] for thread in reversed(polls)
+                ]
+            })
+
+        # conflict validates choice
+        try:
+            merge_conflict = MergeConflict({'poll': threads[0].id}, threads)
+            merge_conflict.is_valid(raise_exception=True)
+            self.fail("merge_conflict.is_valid() should raise ValidationError")
+        except ValidationError as e:
+            self.assertTrue(merge_conflict.is_merge_conflict())
+            self.assertEqual(e.detail, {'poll': ['Invalid choice.']})
+
+        # conflict returns selected resolution
+        merge_conflict = MergeConflict({'poll': polls[0].poll.id}, threads)
+        self.assertTrue(merge_conflict.is_merge_conflict())
+        self.assertEqual(merge_conflict.get_merge_conflict(), ['poll'])
+        merge_conflict.is_valid(raise_exception=True)
+        self.assertEqual(merge_conflict.get_resolution(), {
+            'best_answer': best_answers[0],
+            'poll': polls[0].poll,
+        })
+
+        # conflict returns no-choice resolution
+        merge_conflict = MergeConflict({'poll': 0}, threads)
+        self.assertTrue(merge_conflict.is_merge_conflict())
+        self.assertEqual(merge_conflict.get_merge_conflict(), ['poll'])
+        merge_conflict.is_valid(raise_exception=True)
+        self.assertEqual(merge_conflict.get_resolution(), {
+            'best_answer': best_answers[0],
+            'poll': None,
+        })

misago/threads/tests/test_thread_merge_api.py

@@ -497,9 +497,9 @@ class ThreadMergeApiTests(ThreadsApiTestCase):
         self.assertEqual(
             response.json(), {
                 'polls': [
-                    [0, "Delete all polls"],
-                    [poll.pk, poll.question],
-                    [other_poll.pk, other_poll.question],
+                    ['0', "Delete all polls"],
+                    [str(poll.pk), poll.question],
+                    [str(other_poll.pk), other_poll.question],
                 ]
             }
         )