Browse Source

set_users_unread_private_threads_sync api change

Rafał Pitoń 8 years ago
parent
commit
d97a409942

+ 19 - 2
misago/threads/api/threadendpoints/patch.py

@@ -12,7 +12,7 @@ from misago.core.shortcuts import get_int_or_404, get_object_or_404
 
 
 from ...models import ThreadParticipant
 from ...models import ThreadParticipant
 from ...moderation import threads as moderation
 from ...moderation import threads as moderation
-from ...participants import add_participant, remove_participant
+from ...participants import add_participant, change_owner, remove_participant
 from ...permissions import (
 from ...permissions import (
     allow_add_participants, allow_add_participant,
     allow_add_participants, allow_add_participant,
     allow_change_owner, allow_remove_participant, allow_start_thread)
     allow_change_owner, allow_remove_participant, allow_start_thread)
@@ -244,10 +244,27 @@ def patch_remove_participant(request, thread, value):
     return {
     return {
         'deleted': len(thread.participants_list) == 1
         'deleted': len(thread.participants_list) == 1
     }
     }
-
 thread_patch_dispatcher.remove('participants', patch_remove_participant)
 thread_patch_dispatcher.remove('participants', patch_remove_participant)
 
 
 
 
+def patch_replace_owner(request, thread, value):
+    try:
+        user_id = int(value)
+    except (ValueError, TypeError):
+        raise PermissionDenied(_("Participant to remove is invalid."))
+
+    for participant in thread.participants_list:
+        if participant.user_id == user_id:
+            break
+    else:
+        raise PermissionDenied(_("Participant doesn't exist."))
+
+    allow_change_owner(request.user, thread)
+
+
+thread_patch_dispatcher.replace('owner', patch_replace_owner)
+
+
 def thread_patch_endpoint(request, thread):
 def thread_patch_endpoint(request, thread):
     old_title = thread.title
     old_title = thread.title
     old_is_hidden = thread.is_hidden
     old_is_hidden = thread.is_hidden

+ 40 - 8
misago/threads/participants.py

@@ -1,6 +1,7 @@
 from django.contrib.auth import get_user_model
 from django.contrib.auth import get_user_model
 from django.utils.translation import ugettext as _
 from django.utils.translation import ugettext as _
 
 
+from misago.core import deprecations
 from misago.core.mail import build_mail, send_messages
 from misago.core.mail import build_mail, send_messages
 
 
 from .events import record_event
 from .events import record_event
@@ -25,18 +26,42 @@ def make_participants_aware(user, thread):
     return thread.participants_list
     return thread.participants_list
 
 
 
 
+def set_users_unread_private_threads_sync(users=None, participants=None):
+    users_ids = []
+    if users:
+        users_ids += [u.pk for u in users]
+    if participants:
+        users_ids += [p.user_id for p in participants]
+
+    User = get_user_model()
+    User.objects.filter(id__in=set(users_ids)).update(
+        sync_unread_private_threads=True
+    )
+
+
 def set_owner(thread, user):
 def set_owner(thread, user):
     """
     """
-    Remove user's ownership over thread
+    Set user as thread's owner
     """
     """
     ThreadParticipant.objects.set_owner(thread, user)
     ThreadParticipant.objects.set_owner(thread, user)
 
 
 
 
-def set_users_unread_private_threads_sync(users):
-    User = get_user_model()
-    User.objects.filter(id__in=[u.pk for u in users]).update(
-        sync_unread_private_threads=True
-    )
+def change_owner(thread, user):
+    """
+    Replace thread's owner with other
+    """
+    ThreadParticipant.objects.set_owner(thread, user)
+    set_users_unread_private_threads_sync(participants=thread.participants_list)
+
+    if thread.participant and thread.participant.is_owner:
+        record_event(request, thread, 'changed_owner', {
+            'user': {
+                'username': user.username,
+                'url': user.get_absolute_url(),
+            }
+        })
+    else:
+        record_event(request, thread, 'tookover')
 
 
 
 
 def add_participant(request, thread, user):
 def add_participant(request, thread, user):
@@ -44,6 +69,7 @@ def add_participant(request, thread, user):
     Adds single participant to thread, registers this on the event
     Adds single participant to thread, registers this on the event
     """
     """
     add_participants(request, thread, [user])
     add_participants(request, thread, [user])
+
     record_event(request, thread, 'added_participant', {
     record_event(request, thread, 'added_participant', {
         'user': {
         'user': {
             'username': user.username,
             'username': user.username,
@@ -58,7 +84,13 @@ def add_participants(request, thread, users):
     notify them about being added to thread
     notify them about being added to thread
     """
     """
     ThreadParticipant.objects.add_participants(thread, users)
     ThreadParticipant.objects.add_participants(thread, users)
-    set_users_unread_private_threads_sync(users)
+
+    try:
+        thread_participants = thread.participants_list
+    except AttributeError:
+        thread_participants = []
+
+    set_users_unread_private_threads_sync(users=users, participants=thread_participants)
 
 
     emails = []
     emails = []
     for user in users:
     for user in users:
@@ -97,7 +129,7 @@ def remove_participant(request, thread, user):
         else:
         else:
             remaining_participants.append(participant.user)
             remaining_participants.append(participant.user)
 
 
-    set_users_unread_private_threads_sync(remaining_participants + [user])
+    set_users_unread_private_threads_sync(participants=thread.participants_list)
 
 
     if not remaining_participants:
     if not remaining_participants:
         thread.delete()
         thread.delete()

+ 21 - 3
misago/threads/permissions/privatethreads.py

@@ -183,7 +183,7 @@ can_use_private_threads = return_boolean(allow_use_private_threads)
 
 
 
 
 def allow_see_private_thread(user, target):
 def allow_see_private_thread(user, target):
-    if user.acl.get('can_moderate_private_threads'):
+    if user.acl['can_moderate_private_threads']:
         can_see_reported = target.has_reported_posts
         can_see_reported = target.has_reported_posts
     else:
     else:
         can_see_reported = False
         can_see_reported = False
@@ -211,28 +211,46 @@ def allow_change_owner(user, target):
 
 
     if not (is_owner and is_moderator):
     if not (is_owner and is_moderator):
         raise PermissionDenied(
         raise PermissionDenied(
-            _("Only threaf owner and moderators can change threads owners."))
+            _("Only thread owner and moderators can change threads owners."))
+
+    if not is_moderator and target.is_closed:
+        raise PermissionDenied(
+            _("Only moderators can change closed threads owners."))
 can_change_owner = return_boolean(allow_change_owner)
 can_change_owner = return_boolean(allow_change_owner)
 
 
 
 
 def allow_add_participants(user, target):
 def allow_add_participants(user, target):
+    if user.acl['can_moderate_private_threads']:
+        return
+
     if not target.participant or not target.participant.is_owner:
     if not target.participant or not target.participant.is_owner:
         raise PermissionDenied(
         raise PermissionDenied(
             _("You have to be thread owner to add new participants to it."))
             _("You have to be thread owner to add new participants to it."))
 
 
+    if target.is_closed:
+        raise PermissionDenied(
+            _("Only moderators can add participants to closed threads."))
+
     max_participants = user.acl['max_private_thread_participants']
     max_participants = user.acl['max_private_thread_participants']
     current_participants = len(target.participants_list) - 1
     current_participants = len(target.participants_list) - 1
 
 
-    if current_participants >= max_participants :
+    if current_participants >= max_participants:
         raise PermissionDenied(
         raise PermissionDenied(
             _("You can't add any more new users to this thread."))
             _("You can't add any more new users to this thread."))
 can_add_participants = return_boolean(allow_add_participants)
 can_add_participants = return_boolean(allow_add_participants)
 
 
 
 
 def allow_remove_participant(user, thread, target):
 def allow_remove_participant(user, thread, target):
+    if user.acl['can_moderate_private_threads']:
+        return
+
     if user == target:
     if user == target:
         return # we can always remove ourselves
         return # we can always remove ourselves
 
 
+    if thread.is_closed:
+        raise PermissionDenied(
+            _("Only moderators can remove participants from closed threads."))
+
     if not thread.participant or not thread.participant.is_owner:
     if not thread.participant or not thread.participant.is_owner:
         raise PermissionDenied(
         raise PermissionDenied(
             _("You have to be thread owner to remove participants from it."))
             _("You have to be thread owner to remove participants from it."))

+ 37 - 2
misago/threads/tests/test_participants.py

@@ -108,7 +108,7 @@ class ParticipantsTests(TestCase):
     def test_set_users_unread_private_threads_sync(self):
     def test_set_users_unread_private_threads_sync(self):
         """
         """
         set_users_unread_private_threads_sync sets sync_unread_private_threads
         set_users_unread_private_threads_sync sets sync_unread_private_threads
-        flag on user model to true
+        flag on users provided to true
         """
         """
         User = get_user_model()
         User = get_user_model()
         users = [
         users = [
@@ -116,6 +116,41 @@ class ParticipantsTests(TestCase):
             User.objects.create_user("Bob2", "bob2@boberson.com", "Pass.123"),
             User.objects.create_user("Bob2", "bob2@boberson.com", "Pass.123"),
         ]
         ]
 
 
-        set_users_unread_private_threads_sync(users)
+        set_users_unread_private_threads_sync(users=users)
+        for user in users:
+            User.objects.get(pk=user.pk, sync_unread_private_threads=True)
+
+    def test_set_participants_unread_private_threads_sync(self):
+        """
+        set_users_unread_private_threads_sync sets sync_unread_private_threads
+        flag on participants provided to true
+        """
+        User = get_user_model()
+        users = [
+            User.objects.create_user("Bob1", "bob1@boberson.com", "Pass.123"),
+            User.objects.create_user("Bob2", "bob2@boberson.com", "Pass.123"),
+        ]
+
+        participants = [ThreadParticipant(user=u) for u in users]
+
+        set_users_unread_private_threads_sync(participants=participants)
+        for user in users:
+            User.objects.get(pk=user.pk, sync_unread_private_threads=True)
+
+    def test_set_participants_isers_unread_private_threads_sync(self):
+        """
+        set_users_unread_private_threads_sync sets sync_unread_private_threads
+        flag on users and participants provided to true
+        """
+        User = get_user_model()
+        users = [
+            User.objects.create_user("Bob1", "bob1@boberson.com", "Pass.123"),
+        ]
+
+        participants = [ThreadParticipant(user=u) for u in users]
+
+        users.append(User.objects.create_user("Bob2", "bob2@boberson.com", "Pass.123"))
+
+        set_users_unread_private_threads_sync(users=users, participants=participants)
         for user in users:
         for user in users:
             User.objects.get(pk=user.pk, sync_unread_private_threads=True)
             User.objects.get(pk=user.pk, sync_unread_private_threads=True)

+ 147 - 15
misago/threads/tests/test_privatethread_patch_api.py

@@ -17,19 +17,16 @@ class PrivateThreadPatchApiTestCase(PrivateThreadsTestCase):
         self.thread = testutils.post_thread(self.category, poster=self.user)
         self.thread = testutils.post_thread(self.category, poster=self.user)
         self.api_link = self.thread.get_api_url()
         self.api_link = self.thread.get_api_url()
 
 
+        User = get_user_model()
+        self.other_user = get_user_model().objects.create_user(
+            'BobBoberson', 'bob@boberson.com', 'pass123')
+
     def patch(self, api_link, ops):
     def patch(self, api_link, ops):
         return self.client.patch(
         return self.client.patch(
             api_link, json.dumps(ops), content_type="application/json")
             api_link, json.dumps(ops), content_type="application/json")
 
 
 
 
 class PrivateThreadAddParticipantApiTests(PrivateThreadPatchApiTestCase):
 class PrivateThreadAddParticipantApiTests(PrivateThreadPatchApiTestCase):
-    def setUp(self):
-        super(PrivateThreadAddParticipantApiTests, self).setUp()
-
-        User = get_user_model()
-        self.other_user = get_user_model().objects.create_user(
-            'BobBoberson', 'bob@boberson.com', 'pass123')
-
     def test_add_participant_not_owner(self):
     def test_add_participant_not_owner(self):
         """non-owner can't add participant"""
         """non-owner can't add participant"""
         ThreadParticipant.objects.add_participants(self.thread, [self.user])
         ThreadParticipant.objects.add_participants(self.thread, [self.user])
@@ -98,13 +95,23 @@ class PrivateThreadAddParticipantApiTests(PrivateThreadPatchApiTestCase):
         self.assertContains(
         self.assertContains(
             response, "You can't add any more new users to this thread.", status_code=400)
             response, "You can't add any more new users to this thread.", status_code=400)
 
 
+    def test_add_user_closed_thread(self):
+        """adding user to closed thread fails for non-moderator"""
+        ThreadParticipant.objects.set_owner(self.thread, self.user)
+
+        self.thread.is_closed = True
+        self.thread.save()
+
+        response = self.patch(self.api_link, [
+            {'op': 'add', 'path': 'participants', 'value': self.other_user.username}
+        ])
+        self.assertContains(
+            response, "Only moderators can add participants to closed threads.", status_code=400)
+
     def test_add_user(self):
     def test_add_user(self):
         """adding user to thread add user to thread as participant, sets event and emails him"""
         """adding user to thread add user to thread as participant, sets event and emails him"""
         ThreadParticipant.objects.set_owner(self.thread, self.user)
         ThreadParticipant.objects.set_owner(self.thread, self.user)
 
 
-        self.other_user.email = 'rafio.xudb@gmail.com'
-        self.other_user.save()
-
         response = self.patch(self.api_link, [
         response = self.patch(self.api_link, [
             {'op': 'add', 'path': 'participants', 'value': self.other_user.username}
             {'op': 'add', 'path': 'participants', 'value': self.other_user.username}
         ])
         ])
@@ -129,15 +136,81 @@ class PrivateThreadAddParticipantApiTests(PrivateThreadPatchApiTestCase):
         self.assertIn(self.user.username, email.subject)
         self.assertIn(self.user.username, email.subject)
         self.assertIn(self.thread.title, email.subject)
         self.assertIn(self.thread.title, email.subject)
 
 
+    def test_add_user_too_many_users_moderator(self):
+        """moderators bypass users limit"""
+        ThreadParticipant.objects.set_owner(self.thread, self.user)
 
 
-class PrivateThreadRemoveParticipantApiTests(PrivateThreadPatchApiTestCase):
-    def setUp(self):
-        super(PrivateThreadRemoveParticipantApiTests, self).setUp()
+        override_acl(self.user, {
+            'can_moderate_private_threads': 1
+        })
 
 
         User = get_user_model()
         User = get_user_model()
-        self.other_user = get_user_model().objects.create_user(
-            'BobBoberson', 'bob@boberson.com', 'pass123')
+        for i in range(self.user.acl['max_private_thread_participants']):
+            user = User.objects.create_user(
+                'User{}'.format(i), 'user{}@example.com'.format(i), 'Pass.123')
+            ThreadParticipant.objects.add_participants(self.thread, [user])
 
 
+        response = self.patch(self.api_link, [
+            {'op': 'add', 'path': 'participants', 'value': self.other_user.username}
+        ])
+
+        self.assertEqual(response.json()['participant'], {
+            'id': self.other_user.id,
+            'username': self.other_user.username,
+            'avatar_hash': self.other_user.avatar_hash,
+            'url': self.other_user.get_absolute_url(),
+            'is_owner': False,
+        })
+
+        # event was set on thread
+        event = self.thread.post_set.order_by('id').last()
+        self.assertTrue(event.is_event)
+        self.assertTrue(event.event_type, 'added_participant')
+
+        # notification about new private thread was sent to other user
+        self.assertEqual(len(mail.outbox), 1)
+        email = mail.outbox[-1]
+
+        self.assertIn(self.user.username, email.subject)
+        self.assertIn(self.thread.title, email.subject)
+
+    def test_add_user_to_closed_moderator(self):
+        """moderators can add users to closed threads"""
+        ThreadParticipant.objects.set_owner(self.thread, self.user)
+
+        self.thread.is_closed = True
+        self.thread.save()
+
+        override_acl(self.user, {
+            'can_moderate_private_threads': 1
+        })
+
+        response = self.patch(self.api_link, [
+            {'op': 'add', 'path': 'participants', 'value': self.other_user.username}
+        ])
+
+        self.assertEqual(response.json()['participant'], {
+            'id': self.other_user.id,
+            'username': self.other_user.username,
+            'avatar_hash': self.other_user.avatar_hash,
+            'url': self.other_user.get_absolute_url(),
+            'is_owner': False,
+        })
+
+        # event was set on thread
+        event = self.thread.post_set.order_by('id').last()
+        self.assertTrue(event.is_event)
+        self.assertTrue(event.event_type, 'added_participant')
+
+        # notification about new private thread was sent to other user
+        self.assertEqual(len(mail.outbox), 1)
+        email = mail.outbox[-1]
+
+        self.assertIn(self.user.username, email.subject)
+        self.assertIn(self.thread.title, email.subject)
+
+
+class PrivateThreadRemoveParticipantApiTests(PrivateThreadPatchApiTestCase):
     def test_remove_invalid(self):
     def test_remove_invalid(self):
         """removed user has to be participant"""
         """removed user has to be participant"""
         ThreadParticipant.objects.set_owner(self.thread, self.user)
         ThreadParticipant.objects.set_owner(self.thread, self.user)
@@ -172,6 +245,20 @@ class PrivateThreadRemoveParticipantApiTests(PrivateThreadPatchApiTestCase):
         self.assertContains(
         self.assertContains(
             response, "be thread owner to remove participants from it", status_code=400)
             response, "be thread owner to remove participants from it", status_code=400)
 
 
+    def test_owner_remove_user_closed_thread(self):
+        """api disallows owner to remove other user from closed thread"""
+        ThreadParticipant.objects.set_owner(self.thread, self.user)
+        ThreadParticipant.objects.add_participants(self.thread, [self.other_user])
+
+        self.thread.is_closed = True
+        self.thread.save()
+
+        response = self.patch(self.api_link, [
+            {'op': 'remove', 'path': 'participants', 'value': self.other_user.pk}
+        ])
+        self.assertContains(
+            response, "moderators can remove participants from closed threads", status_code=400)
+
     def test_user_leave_thread(self):
     def test_user_leave_thread(self):
         """api allows user to remove himself from thread"""
         """api allows user to remove himself from thread"""
         ThreadParticipant.objects.set_owner(self.thread, self.other_user)
         ThreadParticipant.objects.set_owner(self.thread, self.other_user)
@@ -201,6 +288,38 @@ class PrivateThreadRemoveParticipantApiTests(PrivateThreadPatchApiTestCase):
         self.assertEqual(self.thread.participants.count(), 1)
         self.assertEqual(self.thread.participants.count(), 1)
         self.assertEqual(self.thread.participants.filter(pk=self.user.pk).count(), 0)
         self.assertEqual(self.thread.participants.filter(pk=self.user.pk).count(), 0)
 
 
+    def test_user_leave_closed_thread(self):
+        """api allows user to remove himself from closed thread"""
+        ThreadParticipant.objects.set_owner(self.thread, self.other_user)
+        ThreadParticipant.objects.add_participants(self.thread, [self.user])
+
+        self.thread.is_closed = True
+        self.thread.save()
+
+        response = self.patch(self.api_link, [
+            {'op': 'remove', 'path': 'participants', 'value': self.user.pk}
+        ])
+
+        self.assertEqual(response.status_code, 200)
+        self.assertFalse(response.json()['deleted'])
+
+        # thread still exists
+        self.assertTrue(Thread.objects.get(pk=self.thread.pk))
+
+        # leave event has valid type
+        event = self.thread.post_set.order_by('id').last()
+        self.assertTrue(event.is_event)
+        self.assertTrue(event.event_type, 'participant_left')
+
+        # users were flagged for sync
+        User = get_user_model()
+        self.assertTrue(User.objects.get(pk=self.other_user.pk).sync_unread_private_threads)
+        self.assertTrue(User.objects.get(pk=self.user.pk).sync_unread_private_threads)
+
+        # user was removed from participation
+        self.assertEqual(self.thread.participants.count(), 1)
+        self.assertEqual(self.thread.participants.filter(pk=self.user.pk).count(), 0)
+
     def test_owner_remove_user(self):
     def test_owner_remove_user(self):
         """api allows owner to remove other user"""
         """api allows owner to remove other user"""
         ThreadParticipant.objects.set_owner(self.thread, self.user)
         ThreadParticipant.objects.set_owner(self.thread, self.user)
@@ -280,4 +399,17 @@ class PrivateThreadRemoveParticipantApiTests(PrivateThreadPatchApiTestCase):
 
 
 
 
 class PrivateThreadTakeOverApiTests(PrivateThreadPatchApiTestCase):
 class PrivateThreadTakeOverApiTests(PrivateThreadPatchApiTestCase):
+    """
+    Todo:
+
+    - handle no permission to change owner
+    - handle non-int new owner ID
+    - handle invalid int of new owner ID
+    - handle new owner id being same as current owner
+    - handle owner fail to pass ownership in closed thread
+    - handle owner pass ownership
+    - handle owner being changed by moderator
+    - handle owner being changed by moderator in closed thread
+
+    """
     pass
     pass