fix: No SystemMessage on new 1o1 conversation [WPB-8608] 🍒

logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/conversation/MemberJoinEventHandler.kt

@@ -21,6 +21,7 @@ package com.wire.kalium.logic.sync.receiver.conversation
 import com.benasher44.uuid.uuid4
 import com.wire.kalium.logger.KaliumLogger
 import com.wire.kalium.logic.CoreFailure
+import com.wire.kalium.logic.data.conversation.Conversation
 import com.wire.kalium.logic.data.conversation.ConversationRepository
 import com.wire.kalium.logic.data.event.Event
 import com.wire.kalium.logic.data.event.EventLoggingStatus
@@ -72,17 +73,9 @@ internal class MemberJoinEventHandlerImpl(
                 userRepository.fetchUsersIfUnknownByIds(event.members.map { it.id }.toSet())
                 conversationRepository.persistMembers(event.members, event.conversationId)
             }.onSuccess {
-                val message = Message.System(
-                    id = event.id.ifEmpty { uuid4().toString() },
-                    content = MessageContent.MemberChange.Added(members = event.members.map { it.id }),
-                    conversationId = event.conversationId,
-                    date = event.timestampIso,
-                    senderUserId = event.addedBy,
-                    status = Message.Status.Sent,
-                    visibility = Message.Visibility.VISIBLE,
-                    expirationData = null
-                )
-                persistMessage(message)
+                conversationRepository.detailsById(event.conversationId).onSuccess { conversation ->
+                    if (conversation.type == Conversation.Type.GROUP) addSystemMessage(event)
+                }
                 legalHoldHandler.handleConversationMembersChanged(event.conversationId)
                 kaliumLogger
                     .logEventProcessing(
@@ -97,4 +90,18 @@ internal class MemberJoinEventHandlerImpl(
                         Pair("errorInfo", "$it")
                     )
             }
+
+    private suspend fun addSystemMessage(event: Event.Conversation.MemberJoin) {
+        val message = Message.System(
+            id = event.id.ifEmpty { uuid4().toString() },
+            content = MessageContent.MemberChange.Added(members = event.members.map { it.id }),
+            conversationId = event.conversationId,
+            date = event.timestampIso,
+            senderUserId = event.addedBy,
+            status = Message.Status.Sent,
+            visibility = Message.Visibility.VISIBLE,
+            expirationData = null
+        )
+        persistMessage(message)
+    }
 }

logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/conversation/MemberJoinEventHandlerTest.kt

@@ -18,15 +18,11 @@
 
 package com.wire.kalium.logic.sync.receiver.conversation
 
-import com.wire.kalium.logic.CoreFailure
 import com.wire.kalium.logic.NetworkFailure
-import com.wire.kalium.logic.StorageFailure
 import com.wire.kalium.logic.data.conversation.Conversation.Member
-import com.wire.kalium.logic.data.conversation.ConversationRepository
 import com.wire.kalium.logic.data.message.Message
 import com.wire.kalium.logic.data.message.MessageContent
-import com.wire.kalium.logic.data.message.PersistMessageUseCase
-import com.wire.kalium.logic.data.user.UserRepository
+import com.wire.kalium.logic.framework.TestConversation
 import com.wire.kalium.logic.framework.TestEvent
 import com.wire.kalium.logic.framework.TestUser
 import com.wire.kalium.logic.functional.Either
@@ -38,6 +34,16 @@ import io.mockative.coVerify
 import io.mockative.eq
 import io.mockative.matches
 import io.mockative.mock
+import com.wire.kalium.logic.functional.left
+import com.wire.kalium.logic.functional.right
+import com.wire.kalium.logic.util.arrangement.eventHandler.LegalHoldHandlerArrangement
+import com.wire.kalium.logic.util.arrangement.eventHandler.LegalHoldHandlerArrangementImpl
+import com.wire.kalium.logic.util.arrangement.repository.ConversationRepositoryArrangement
+import com.wire.kalium.logic.util.arrangement.repository.ConversationRepositoryArrangementImpl
+import com.wire.kalium.logic.util.arrangement.repository.UserRepositoryArrangement
+import com.wire.kalium.logic.util.arrangement.repository.UserRepositoryArrangementImpl
+import com.wire.kalium.logic.util.arrangement.usecase.PersistMessageUseCaseArrangement
+import com.wire.kalium.logic.util.arrangement.usecase.PersistMessageUseCaseArrangementImpl
 import io.mockative.once
 import kotlinx.coroutines.test.runTest
 import kotlin.test.Test
@@ -49,12 +55,10 @@ class MemberJoinEventHandlerTest {
         val newMembers = listOf(Member(TestUser.SELF.id, Member.Role.Member))
         val event = TestEvent.memberJoin(members = newMembers)
 
-        val (arrangement, eventHandler) = Arrangement()
-            .withPersistingMessageReturning(Either.Right(Unit))
-            .withFetchConversationIfUnknownSucceeding()
-            .withPersistMembersSucceeding()
-            .withFetchUsersIfUnknownByIdsReturning(Either.Right(Unit))
-            .arrange()
+        val (arrangement, eventHandler) = arrange {
+            withFetchConversationSucceeding()
+            withConversationDetailsByIdReturning(TestConversation.CONVERSATION.right())
+        }
 
         eventHandler.handle(event)
 
@@ -72,12 +76,10 @@ class MemberJoinEventHandlerTest {
         val newMembers = listOf(Member(TestUser.USER_ID, Member.Role.Member))
         val event = TestEvent.memberJoin(members = newMembers)
 
-        val (arrangement, eventHandler) = Arrangement()
-            .withPersistingMessageReturning(Either.Right(Unit))
-            .withFetchConversationIfUnknownSucceeding()
-            .withPersistMembersSucceeding()
-            .withFetchUsersIfUnknownByIdsReturning(Either.Right(Unit))
-            .arrange()
+        val (arrangement, eventHandler) = arrange {
+            withFetchConversationSucceeding()
+            withConversationDetailsByIdReturning(TestConversation.CONVERSATION.right())
+        }
 
         eventHandler.handle(event)
 
@@ -91,12 +93,11 @@ class MemberJoinEventHandlerTest {
         val newMembers = listOf(Member(TestUser.USER_ID, Member.Role.Member))
         val event = TestEvent.memberJoin(members = newMembers)
 
-        val (arrangement, eventHandler) = Arrangement()
-            .withPersistingMessageReturning(Either.Right(Unit))
-            .withFetchConversationIfUnknownFailing(NetworkFailure.NoNetworkConnection(null))
-            .withFetchUsersIfUnknownByIdsReturning(Either.Right(Unit))
-            .withPersistMembersSucceeding()
-            .arrange()
+        val (arrangement, eventHandler) = arrange {
+            withFetchConversationIfUnknownFailingWith(NetworkFailure.NoNetworkConnection(null))
+            withFetchConversation(NetworkFailure.NoNetworkConnection(null).left())
+            withConversationDetailsByIdReturning(TestConversation.CONVERSATION.right())
+        }
 
         eventHandler.handle(event)
 
@@ -110,40 +111,51 @@ class MemberJoinEventHandlerTest {
         val newMembers = listOf(Member(TestUser.USER_ID, Member.Role.Admin))
         val event = TestEvent.memberJoin(members = newMembers)
 
-        val (arrangement, eventHandler) = Arrangement()
-            .withPersistingMessageReturning(Either.Right(Unit))
-            .withFetchConversationIfUnknownSucceeding()
-            .withFetchUsersIfUnknownByIdsReturning(Either.Right(Unit))
-            .withPersistMembersSucceeding()
-            .arrange()
+        val (arrangement, eventHandler) = arrange {
+            withFetchConversationSucceeding()
+            withConversationDetailsByIdReturning(TestConversation.GROUP().right())
+        }
 
         eventHandler.handle(event)
 
         coVerify {
-            arrangement.persistMessage.invoke(
+            arrangement.persistMessageUseCase.invoke(
                 matches {
                     it is Message.System && it.content is MessageContent.MemberChange
                 }
             )
         }.wasInvoked(exactly = once)
     }
 
+    @Test
+    fun givenMemberJoinEventIn1o1Conversation_whenHandlingIt_thenShouldNotPersistSystemMessage() = runTest {
+        val newMembers = listOf(Member(TestUser.USER_ID, Member.Role.Admin))
+        val event = TestEvent.memberJoin(members = newMembers)
+
+        val (arrangement, eventHandler) = arrange {
+            withFetchConversationSucceeding()
+            withConversationDetailsByIdReturning(TestConversation.CONVERSATION.right())
+        }
+
+        eventHandler.handle(event)
+
+        coVerify { arrangement.persistMessageUseCase(any()) }.wasNotInvoked()
+    }
+
     @Test
     fun givenMemberJoinEventWithEmptyId_whenHandlingIt_thenShouldPersistSystemMessage() = runTest {
         val newMembers = listOf(Member(TestUser.USER_ID, Member.Role.Admin))
         val event = TestEvent.memberJoin(members = newMembers).copy(id = "")
 
-        val (arrangement, eventHandler) = Arrangement()
-            .withPersistingMessageReturning(Either.Right(Unit))
-            .withFetchConversationIfUnknownSucceeding()
-            .withFetchUsersIfUnknownByIdsReturning(Either.Right(Unit))
-            .withPersistMembersSucceeding()
-            .arrange()
+        val (arrangement, eventHandler) = arrange {
+            withFetchConversationSucceeding()
+            withConversationDetailsByIdReturning(TestConversation.GROUP().right())
+        }
 
         eventHandler.handle(event)
 
         coVerify {
-            arrangement.persistMessage.invoke(
+            arrangement.persistMessageUseCase.invoke(
                 matches {
                     it is Message.System && it.content is MessageContent.MemberChange && it.id.isNotEmpty()
                 }
@@ -156,12 +168,13 @@ class MemberJoinEventHandlerTest {
         // given
         val newMembers = listOf(Member(TestUser.USER_ID, Member.Role.Admin))
         val event = TestEvent.memberJoin(members = newMembers)
-        val (arrangement, eventHandler) = Arrangement()
-            .withPersistingMessageReturning(Either.Right(Unit))
-            .withFetchConversationIfUnknownSucceeding()
-            .withFetchUsersIfUnknownByIdsReturning(Either.Right(Unit))
-            .withPersistMembersSucceeding()
-            .arrange()
+        val (arrangement, eventHandler) = arrange {
+            withPersistingMessage(Either.Right(Unit))
+            withFetchConversationSucceeding()
+            withConversationDetailsByIdReturning(TestConversation.GROUP().right())
+            withFetchUsersIfUnknownByIdsReturning(Either.Right(Unit))
+            withPersistMembers(Unit.right())
+        }
         // when
         eventHandler.handle(event)
         // then
@@ -170,68 +183,31 @@ class MemberJoinEventHandlerTest {
         }.wasInvoked(exactly = once)
     }
 
-    private class Arrangement {
-        @Mock
-        val persistMessage = mock(PersistMessageUseCase::class)
-
-        @Mock
-        val conversationRepository = mock(ConversationRepository::class)
-
-        @Mock
-        private val userRepository = mock(UserRepository::class)
-
-        @Mock
-        val legalHoldHandler = mock(LegalHoldHandler::class)
-
-        private val memberJoinEventHandler: MemberJoinEventHandler = MemberJoinEventHandlerImpl(
-            conversationRepository = conversationRepository,
-            userRepository = userRepository,
-            persistMessage = persistMessage,
-            legalHoldHandler = legalHoldHandler
-        )
-
-        suspend fun withPersistingMessageReturning(result: Either<CoreFailure, Unit>) = apply {
-            coEvery {
-                persistMessage.invoke(any())
-            }.returns(result)
-        }
-
-        suspend fun withFetchConversationIfUnknownSucceeding() = apply {
-            coEvery {
-                conversationRepository.fetchConversationIfUnknown(any())
-            }.returns(Either.Right(Unit))
-            coEvery {
-                conversationRepository.fetchConversation(any())
-            }.returns(Either.Right(Unit))
-        }
+    private class Arrangement(
+        private val block: suspend Arrangement.() -> Unit
+    ) : ConversationRepositoryArrangement by ConversationRepositoryArrangementImpl(),
+        UserRepositoryArrangement by UserRepositoryArrangementImpl(),
+        PersistMessageUseCaseArrangement by PersistMessageUseCaseArrangementImpl(),
+        LegalHoldHandlerArrangement by LegalHoldHandlerArrangementImpl() {
 
-        suspend fun withFetchConversationIfUnknownFailing(coreFailure: CoreFailure) = apply {
-            coEvery {
-                conversationRepository.fetchConversationIfUnknown(any())
-            }.returns(Either.Left(coreFailure))
-            coEvery {
-                conversationRepository.fetchConversation(any())
-            }.returns(Either.Left(coreFailure))
+        suspend fun withFetchConversationSucceeding() = apply {
+            withFetchConversationIfUnknownSucceeding()
+            withFetchConversation(Unit.right())
         }
 
-        suspend fun withPersistMembersSucceeding() = apply {
-            coEvery {
-                conversationRepository.persistMembers(any(), any())
-            }.returns(Either.Right(Unit))
-        }
+        suspend fun arrange() = run {
+            block()
 
-        suspend fun withFetchUsersIfUnknownByIdsReturning(result: Either<StorageFailure, Unit>) = apply {
-            coEvery {
-                userRepository.fetchUsersIfUnknownByIds(any())
-            }.returns(result)
-        }
+            withPersistingMessage(Unit.right())
+            withFetchUsersIfUnknownByIdsReturning(Unit.right())
+            withPersistMembers(Unit.right())
+            withHandleConversationMembersChanged(Unit.right())
 
-        suspend fun arrange() = run {
-            coEvery {
-                legalHoldHandler.handleConversationMembersChanged(any())
-            }.returns(Either.Right(Unit))
-            this to memberJoinEventHandler
+            this to MemberJoinEventHandlerImpl(conversationRepository, userRepository, persistMessageUseCase, legalHoldHandler)
         }
     }
 
+    private companion object {
+        suspend fun arrange(block: suspend Arrangement.() -> Unit) = Arrangement(block).arrange()
+    }
 }

logic/src/commonTest/kotlin/com/wire/kalium/logic/util/arrangement/eventHandler/LegalHoldHandlerArrangement.kt

@@ -0,0 +1,42 @@
+/*
+ * Wire
+ * Copyright (C) 2024 Wire Swiss GmbH
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see http://www.gnu.org/licenses/.
+ */
+package com.wire.kalium.logic.util.arrangement.eventHandler
+
+import com.wire.kalium.logic.CoreFailure
+import com.wire.kalium.logic.functional.Either
+import com.wire.kalium.logic.sync.receiver.handler.legalhold.LegalHoldHandler
+import io.mockative.Mock
+import io.mockative.any
+import io.mockative.coEvery
+import io.mockative.mock
+
+internal interface LegalHoldHandlerArrangement {
+    val legalHoldHandler: LegalHoldHandler
+
+    suspend fun withHandleConversationMembersChanged(result: Either<CoreFailure, Unit>)
+}
+
+internal open class LegalHoldHandlerArrangementImpl : LegalHoldHandlerArrangement {
+    @Mock
+    override val legalHoldHandler: LegalHoldHandler = mock(LegalHoldHandler::class)
+
+    override suspend fun withHandleConversationMembersChanged(result: Either<CoreFailure, Unit>) {
+        coEvery { legalHoldHandler.handleConversationMembersChanged(any()) }.returns(result)
+    }
+
+}

logic/src/commonTest/kotlin/com/wire/kalium/logic/util/arrangement/repository/ConversationRepositoryArrangement.kt

@@ -103,6 +103,7 @@ internal interface ConversationRepositoryArrangement {
 
     suspend fun withSelectGroupStatusMembersNamesAndHandles(result: Either<StorageFailure, EpochChangesData>)
     suspend fun withConversationDetailsByIdReturning(result: Either<StorageFailure, Conversation>)
+    suspend fun withPersistMembers(result: Either<StorageFailure, Unit>)
 }
 
 internal open class ConversationRepositoryArrangementImpl : ConversationRepositoryArrangement {
@@ -259,4 +260,8 @@ internal open class ConversationRepositoryArrangementImpl : ConversationReposito
             conversationRepository.detailsById(any())
         }.returns(result)
     }
+
+    override suspend fun withPersistMembers(result: Either<StorageFailure, Unit>) {
+        coEvery { conversationRepository.persistMembers(any(), any()) }.returns(result)
+    }
 }