Browse Source

wip remove private thread participant api

Rafał Pitoń 8 years ago
parent
commit
0ef5b955d8

+ 28 - 10
misago/threads/api/threadendpoints/patch.py

@@ -14,8 +14,8 @@ 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, remove_participant
 from ...permissions import (
 from ...permissions import (
-    allow_start_thread, allow_takeover, allow_add_participants,
-    allow_add_participant, allow_remove_participants)
+    allow_add_participants, allow_add_participant,
+    allow_change_owner, allow_remove_participant, allow_start_thread)
 from ...serializers import ThreadParticipantSerializer
 from ...serializers import ThreadParticipantSerializer
 from ...utils import add_categories_to_items
 from ...utils import add_categories_to_items
 from ...validators import validate_title
 from ...validators import validate_title
@@ -198,17 +198,21 @@ def patch_subscribtion(request, thread, value):
 thread_patch_dispatcher.replace('subscription', patch_subscribtion)
 thread_patch_dispatcher.replace('subscription', patch_subscribtion)
 
 
 
 
-def patch_add_participants(request, thread, value):
+def patch_add_participant(request, thread, value):
     allow_add_participants(request.user, thread)
     allow_add_participants(request.user, thread)
 
 
     User = get_user_model()
     User = get_user_model()
     try:
     try:
-        participant = User.objects.get(slug=six.text_type(value).strip().lower())
+        username = six.text_type(value).strip().lower()
+        if not username:
+            raise PermissionDenied(
+                _("You have to enter new participant's username."))
+        participant = User.objects.get(slug=username)
     except User.DoesNotExist:
     except User.DoesNotExist:
-        raise PermissionDenied("No user with such name exists.")
+        raise PermissionDenied(_("No user with such name exists."))
 
 
     if participant in [p.user for p in thread.participants_list]:
     if participant in [p.user for p in thread.participants_list]:
-        raise PermissionDenied("This user is already thread participant.")
+        raise PermissionDenied(_("This user is already thread participant."))
 
 
     allow_add_participant(request.user, participant)
     allow_add_participant(request.user, participant)
     add_participant(request, thread, participant)
     add_participant(request, thread, participant)
@@ -218,12 +222,26 @@ def patch_add_participants(request, thread, value):
             ThreadParticipant(user=participant, is_owner=False)
             ThreadParticipant(user=participant, is_owner=False)
         ).data
         ).data
     }
     }
-thread_patch_dispatcher.add('participants', patch_add_participants)
+thread_patch_dispatcher.add('participants', patch_add_participant)
 
 
 
 
-def patch_remove_participants(request, thread, value):
-    pass
-thread_patch_dispatcher.remove('participants', patch_remove_participants)
+def patch_remove_participant(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_remove_participant(request.user, thread, participant.user)
+
+    raise NotImplementedError('this execution path is incomplete!')
+
+thread_patch_dispatcher.remove('participants', patch_remove_participant)
 
 
 
 
 def thread_patch_endpoint(request, thread):
 def thread_patch_endpoint(request, thread):

+ 15 - 16
misago/threads/permissions/privatethreads.py

@@ -20,12 +20,12 @@ __all__ = [
     'can_see_private_thread',
     'can_see_private_thread',
     'allow_see_private_post',
     'allow_see_private_post',
     'can_see_private_post',
     'can_see_private_post',
-    'allow_takeover',
-    'can_takeover',
+    'allow_change_owner',
+    'can_change_owner',
     'allow_add_participants',
     'allow_add_participants',
     'can_add_participants',
     'can_add_participants',
-    'allow_remove_participants',
-    'can_remove_participants',
+    'allow_remove_participant',
+    'can_remove_participant',
     'allow_add_participant',
     'allow_add_participant',
     'can_add_participant',
     'can_add_participant',
     'allow_message_user',
     'allow_message_user',
@@ -162,9 +162,8 @@ def add_acl_to_thread(user, thread):
         thread.participant = None
         thread.participant = None
 
 
     thread.acl.update({
     thread.acl.update({
-        'can_takeover': can_takeover(user, thread),
+        'can_change_owner': can_change_owner(user, thread),
         'can_add_participants': can_add_participants(user, thread),
         'can_add_participants': can_add_participants(user, thread),
-        'can_remove_participants': can_remove_participants(user, thread)
     })
     })
 
 
 
 
@@ -206,14 +205,14 @@ def allow_see_private_post(user, target):
 can_see_private_post = return_boolean(allow_see_private_post)
 can_see_private_post = return_boolean(allow_see_private_post)
 
 
 
 
-def allow_takeover(user, target):
-    if target.participant and target.participant.is_owner:
-        raise PermissionDenied(
-            _("You are already this thread's owner."))
-    if not user.acl['can_moderate_private_threads']:
+def allow_change_owner(user, target):
+    is_moderator = user.acl['can_moderate_private_threads']
+    is_owner = target.participant and target.participant.is_owner
+
+    if not (is_owner and is_moderator):
         raise PermissionDenied(
         raise PermissionDenied(
-            _("Only private threads moderators can take over private threads."))
-can_takeover = return_boolean(allow_takeover)
+            _("Only threaf owner and moderators can change threads owners."))
+can_change_owner = return_boolean(allow_change_owner)
 
 
 
 
 def allow_add_participants(user, target):
 def allow_add_participants(user, target):
@@ -230,14 +229,14 @@ def allow_add_participants(user, target):
 can_add_participants = return_boolean(allow_add_participants)
 can_add_participants = return_boolean(allow_add_participants)
 
 
 
 
-def allow_remove_participants(user, target):
+def allow_remove_participant(user, thread, target):
     if user == target:
     if user == target:
         return # we can always remove ourselves
         return # we can always remove ourselves
 
 
-    if not target.participant or not target.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."))
-can_remove_participants = return_boolean(allow_remove_participants)
+can_remove_participant = return_boolean(allow_remove_participant)
 
 
 
 
 def allow_add_participant(user, target):
 def allow_add_participant(user, target):

+ 95 - 1
misago/threads/tests/test_privatethread_patch_api.py

@@ -40,6 +40,17 @@ class PrivateThreadAddParticipantApiTests(PrivateThreadPatchApiTestCase):
         self.assertContains(
         self.assertContains(
             response, "be thread owner to add new participants to it", status_code=400)
             response, "be thread owner to add new participants to it", status_code=400)
 
 
+    def test_add_empty_username(self):
+        """path validates username"""
+        ThreadParticipant.objects.set_owner(self.thread, self.user)
+
+        response = self.patch(self.api_link, [
+            {'op': 'add', 'path': 'participants', 'value': ''}
+        ])
+
+        self.assertContains(
+            response, "You have to enter new participant's username.", status_code=400)
+
     def test_add_nonexistant_user(self):
     def test_add_nonexistant_user(self):
         """can't user two times"""
         """can't user two times"""
         ThreadParticipant.objects.set_owner(self.thread, self.user)
         ThreadParticipant.objects.set_owner(self.thread, self.user)
@@ -116,7 +127,90 @@ class PrivateThreadAddParticipantApiTests(PrivateThreadPatchApiTestCase):
 
 
 
 
 class PrivateThreadRemoveParticipantApiTests(PrivateThreadPatchApiTestCase):
 class PrivateThreadRemoveParticipantApiTests(PrivateThreadPatchApiTestCase):
-    pass
+    def setUp(self):
+        super(PrivateThreadRemoveParticipantApiTests, self).setUp()
+
+        User = get_user_model()
+        self.other_user = get_user_model().objects.create_user(
+            'BobBoberson', 'bob@boberson.com', 'pass123')
+
+    def test_remove_invalid(self):
+        """removed user has to be participant"""
+        ThreadParticipant.objects.set_owner(self.thread, self.user)
+
+        response = self.patch(self.api_link, [
+            {'op': 'remove', 'path': 'participants', 'value': 'string'}
+        ])
+
+        self.assertContains(
+            response, "Participant to remove is invalid.", status_code=400)
+
+    def test_remove_nonexistant(self):
+        """removed user has to be participant"""
+        ThreadParticipant.objects.set_owner(self.thread, self.user)
+
+        response = self.patch(self.api_link, [
+            {'op': 'remove', 'path': 'participants', 'value': self.other_user.pk}
+        ])
+
+        self.assertContains(
+            response, "Participant doesn't exist.", status_code=400)
+
+    def test_remove_not_owner(self):
+        """api validates if user trying to remove other user is an owner"""
+        ThreadParticipant.objects.set_owner(self.thread, self.other_user)
+        ThreadParticipant.objects.add_participants(self.thread, [self.user])
+
+        response = self.patch(self.api_link, [
+            {'op': 'remove', 'path': 'participants', 'value': self.other_user.pk}
+        ])
+
+        self.assertContains(
+            response, "be thread owner to remove participants from it", status_code=400)
+
+    def test_user_leave_thread(self):
+        """api allows user to remove himself from thread"""
+        ThreadParticipant.objects.set_owner(self.thread, self.other_user)
+        ThreadParticipant.objects.add_participants(self.thread, [self.user])
+
+        response = self.patch(self.api_link, [
+            {'op': 'remove', 'path': 'participants', 'value': self.user.pk}
+        ])
+
+        raise NotImplementedError('this test scenario is incomplete!')
+
+    def test_owner_remove_user(self):
+        """api allows owner to remove other user"""
+        ThreadParticipant.objects.set_owner(self.thread, self.user)
+        ThreadParticipant.objects.add_participants(self.thread, [self.other_user])
+
+        response = self.patch(self.api_link, [
+            {'op': 'remove', 'path': 'participants', 'value': self.other_user.pk}
+        ])
+
+        raise NotImplementedError('this test scenario is incomplete!')
+
+    def test_owner_leave_thread(self):
+        """api allows owner to remove hisemf from thread, causing thread to close"""
+        ThreadParticipant.objects.set_owner(self.thread, self.user)
+        ThreadParticipant.objects.add_participants(self.thread, [self.other_user])
+
+        response = self.patch(self.api_link, [
+            {'op': 'remove', 'path': 'participants', 'value': self.user.pk}
+        ])
+
+        raise NotImplementedError('this test scenario is incomplete!')
+
+    def test_last_user_leave_thread(self):
+        """api allows last user leave thread, causing thread to delete"""
+        ThreadParticipant.objects.set_owner(self.thread, self.user)
+        ThreadParticipant.objects.add_participants(self.thread, [self.other_user])
+
+        response = self.patch(self.api_link, [
+            {'op': 'remove', 'path': 'participants', 'value': self.user.pk}
+        ])
+
+        raise NotImplementedError('this test scenario is incomplete!')
 
 
 
 
 class PrivateThreadTakeOverApiTests(PrivateThreadPatchApiTestCase):
 class PrivateThreadTakeOverApiTests(PrivateThreadPatchApiTestCase):