Remove from all conversations when leaving group

karrot/activities/receivers.py

@@ -17,11 +17,11 @@
 def leave_group_handler(sender, instance, **kwargs):
     group = instance.group
     user = instance.user
-    for _ in Activity.objects. \
+    for activity in Activity.objects. \
             filter(date__startswith__gte=timezone.now()). \
             filter(participants__in=[user, ]). \
             filter(place__group=group):
-        _.remove_participant(user)
+        activity.remove_participant(user)
 
 
 @receiver(post_save, sender=Feedback)

karrot/conversations/migrations/0043__cleanup_conversation_participants.py

@@ -0,0 +1,23 @@
+from datetime import timedelta
+
+from django.db import migrations
+from django.db.models import Func, F
+
+from karrot.conversations.models import ConversationParticipant
+
+
+def cleanup_conversation_participants(apps, schema_editor):
+    ConversationParticipant = apps.get_model('conversations', 'ConversationParticipant')
+    ConversationThreadParticipant = apps.get_model('conversations', 'ConversationThreadParticipant')
+    ConversationParticipant.objects.filter(conversation__group__isnull=False).exclude(conversation__group__members=F('user')).delete()
+    ConversationThreadParticipant.objects.filter(thread__conversation__group__isnull=False).exclude(thread__conversation__group__members=F('user')).delete()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ('conversations', '0042_conversationmessageattachment'),
+    ]
+
+    operations = [
+        migrations.RunPython(cleanup_conversation_participants, reverse_code=migrations.RunPython.noop, elidable=True),
+    ]

karrot/conversations/models.py

@@ -95,6 +95,8 @@ def join(self, user, **kwargs):
         return participant
 
     def leave(self, user):
+        if self.target and self.target.conversation_supports_threads:
+            ConversationThreadParticipant.objects.filter(user=user, thread__conversation=self).delete()
         self.conversationparticipant_set.filter(user=user).delete()
 
     def sync_users(self, desired_users):

karrot/conversations/tests/test_api.py

@@ -3,6 +3,7 @@
 from email.utils import parseaddr
 from os.path import isfile, basename
 from urllib.parse import quote
+from freezegun import freeze_time
 
 from dateutil.parser import parse
 from django.core import mail
@@ -14,9 +15,9 @@
 from karrot.applications.factories import ApplicationFactory
 from karrot.conversations.factories import ConversationFactory
 from karrot.conversations.models import ConversationParticipant, Conversation, ConversationMessage, \
-    ConversationMessageReaction, ConversationNotificationStatus
+    ConversationMessageReaction, ConversationNotificationStatus, ConversationThreadParticipant
 from karrot.groups.factories import GroupFactory
-from karrot.groups.models import GroupStatus
+from karrot.groups.models import GroupStatus, GroupMembership
 from karrot.issues.factories import IssueFactory
 from karrot.offers.factories import OfferFactory
 from karrot.activities.factories import ActivityFactory
@@ -1176,6 +1177,81 @@ def test_write_message_in_closed_conversation_fails(self):
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN, response.data)
 
 
+class TestLeavingGroupLeavesConversations(APITestCase):
+    def setUp(self):
+        self.user = VerifiedUserFactory()
+        self.other_user = VerifiedUserFactory()
+        self.group = GroupFactory(members=[self.user, self.other_user])
+        self.other_group = GroupFactory(members=[self.user, self.other_user])
+        self.place = PlaceFactory(group=self.group, subscribers=[self.user])
+        self.activity = ActivityFactory(place=self.place)
+        self.membership = GroupMembership.objects.get(group=self.group, user=self.user)
+
+    def test_leaves_group_wall_conversation(self):
+        self.client.force_login(user=self.user)
+        query = self.group.conversation.participants.filter(pk=self.user.id)
+        self.assertTrue(query.exists())
+        self.membership.delete()
+        self.assertFalse(query.exists())
+
+    def test_leaves_place_wall_conversation(self):
+        self.client.force_login(user=self.user)
+        query = self.place.conversation.participants.filter(pk=self.user.id)
+        self.assertTrue(query.exists())
+        self.membership.delete()
+        self.assertFalse(query.exists())
+
+    def test_leaves_activity_conversation(self):
+        self.client.force_login(user=self.user)
+        self.activity.add_participant(self.user)
+        query = self.activity.conversation.participants.filter(pk=self.user.id)
+        self.assertTrue(query.exists())
+        self.membership.delete()
+        self.assertFalse(query.exists())
+
+    def test_leaves_past_activity_conversations(self):
+        self.client.force_login(user=self.user)
+        self.activity.add_participant(self.user)
+        self.assertTrue(self.activity.conversation.participants.filter(pk=self.user.id).exists())
+
+        after_the_activity_is_over = self.activity.date.end + timedelta(hours=2)
+
+        # wind forward time past the activity
+        with freeze_time(after_the_activity_is_over, tick=True):
+            self.membership.delete()
+            self.assertFalse(self.group.members.filter(pk=self.user.id).exists())
+            self.assertFalse(self.activity.conversation.participants.filter(pk=self.user.id).exists())
+
+    def test_leaves_non_subscribed_place_conversations(self):
+        place = PlaceFactory(group=self.group)  # not a subscriber
+        place.conversation.join(self.user)
+
+        other_group_place = PlaceFactory(group=self.other_group)
+        other_group_place.conversation.join(self.user)
+
+        self.membership.delete()
+        self.assertFalse(place.conversation.participants.filter(pk=self.user.id).exists())
+
+        # still in convo for other group, just to make sure :)
+        self.assertTrue(other_group_place.conversation.participants.filter(pk=self.user.id).exists())
+
+    def test_leaves_offer_conversations(self):
+        offer = OfferFactory(group=self.group)
+        offer.conversation.join(self.user)
+        self.membership.delete()
+        self.assertFalse(offer.conversation.participants.filter(pk=self.user.id).exists())
+
+    def test_leaves_threads(self):
+        conversation = self.group.conversation
+        message = conversation.messages.create(author=self.user, content='bla')
+        reply = ConversationMessage.objects.create(
+            author=self.user, conversation=conversation, thread=message, content='reply'
+        )
+        self.assertTrue(ConversationThreadParticipant.objects.filter(user=self.user, thread=reply.thread).exists())
+        self.membership.delete()
+        self.assertFalse(ConversationThreadParticipant.objects.filter(user=self.user, thread=reply.thread).exists())
+
+
 class TestPrivateConversationAPI(APITestCase):
     def setUp(self):
         self.user = VerifiedUserFactory()

karrot/conversations/tests/test_data_migrations.py

@@ -0,0 +1,73 @@
+from karrot.tests.utils import TestMigrations
+from karrot.utils.tests.fake import faker
+
+
+class TestCleanupConversationParticipantsMigration(TestMigrations):
+    """ Testing our conversation participant cleanup migration
+
+    It's a bit of a basic test, as it doesn't run all the code where
+    the magic really happens. But it at least ensures it runs in a basic way :)
+    """
+
+    migrate_from = [
+        ('users', '0027_fix_usernames'),
+        ('groups', '0049_auto_20220930_1506'),
+        ('conversations', '0042_conversationmessageattachment'),
+    ]
+    migrate_to = [
+        ('groups', '0049_auto_20220930_1506'),
+        ('conversations', '0043__cleanup_conversation_participants'),
+    ]
+
+    def setUpBeforeMigration(self, apps):
+        User = apps.get_model('users', 'User')
+        Group = apps.get_model('groups', 'Group')
+        GroupMembership = apps.get_model('groups', 'GroupMembership')
+        Conversation = apps.get_model('conversations', 'Conversation')
+        ConversationMessage = apps.get_model('conversations', 'ConversationMessage')
+        ConversationParticipant = apps.get_model('conversations', 'ConversationParticipant')
+        ConversationThreadParticipant = apps.get_model('conversations', 'ConversationThreadParticipant')
+
+        user = User.objects.create()
+        group = Group.objects.create(name=faker.name())
+        other_group = Group.objects.create(name=faker.name())
+
+        for g in [group, other_group]:
+            GroupMembership.objects.create(group=g, user=user)
+            conversation = Conversation.objects.create(group=g)
+            ConversationParticipant.objects.create(conversation=conversation, user=user)
+            message = ConversationMessage.objects.create(conversation=conversation, content='hello', author=user)
+            ConversationMessage.objects.create(conversation=conversation, content='reply', author=user, thread=message)
+            ConversationThreadParticipant.objects.create(user=user, thread=message)
+
+        self.assertEqual(ConversationParticipant.objects.count(), 2)
+        self.assertEqual(ConversationThreadParticipant.objects.count(), 2)
+
+        GroupMembership.objects.filter(group=group, user=user).delete()
+
+        self.user_id = user.id
+        self.group_id = group.id
+        self.other_group_id = other_group.id
+
+    def test_removes_participants(self):
+        User = self.apps.get_model('users', 'User')
+        Group = self.apps.get_model('groups', 'Group')
+        ConversationParticipant = self.apps.get_model('conversations', 'ConversationParticipant')
+        ConversationThreadParticipant = self.apps.get_model('conversations', 'ConversationThreadParticipant')
+
+        user = User.objects.get(id=self.user_id)
+        group = Group.objects.get(id=self.group_id)
+        other_group = Group.objects.get(id=self.other_group_id)
+
+        self.assertEqual(ConversationParticipant.objects.count(), 1)
+        self.assertEqual(ConversationThreadParticipant.objects.count(), 1)
+
+        self.assertFalse(ConversationParticipant.objects.filter(conversation__group=group, user=user).exists())
+        self.assertFalse(
+            ConversationThreadParticipant.objects.filter(thread__conversation__group=group, user=user).exists()
+        )
+
+        self.assertTrue(ConversationParticipant.objects.filter(conversation__group=other_group, user=user).exists())
+        self.assertTrue(
+            ConversationThreadParticipant.objects.filter(thread__conversation__group=other_group, user=user).exists()
+        )

karrot/groups/receivers.py

@@ -51,8 +51,9 @@ def group_member_added(sender, instance, created, **kwargs):
 def group_member_removed(sender, instance, **kwargs):
     group = instance.group
     user = instance.user
-    conversation = Conversation.objects.get_for_target(group)
-    if conversation:
+
+    # leave all conversations related to this group
+    for conversation in Conversation.objects.filter(group=group, participants__in=[user]):
         conversation.leave(user)
 
     stats.group_left(group)