fix: duplicated system messages for adding/removing participants mls …

…(WPB-6249) (#2426)

* fix: handle local creation id of event with localid to avoid duplication

* fix: adding participants failure message handle only in case there are, plus docs

* fix: more  docs

---------

Co-authored-by: Mojtaba Chenani <[email protected]>

logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt

@@ -207,7 +207,7 @@ internal class ConversationGroupRepositoryImpl(
         remainingAttempts: Int = 2
     ): Either<CoreFailure, Unit> {
         return when (val addingMemberResult = mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList)) {
-            is Either.Right -> handleMLSMembersAdded(conversationId, userIdList, failedUsersList)
+            is Either.Right -> handleMLSMembersNotAdded(conversationId, failedUsersList)
             is Either.Left -> {
                 addingMemberResult.value.handleMLSMembersFailed(
                     conversationId = conversationId,
@@ -262,13 +262,12 @@ internal class ConversationGroupRepositoryImpl(
         }
     }
 
-    private suspend fun handleMLSMembersAdded(
+    private suspend fun handleMLSMembersNotAdded(
         conversationId: ConversationId,
-        userIdList: List<UserId>,
         failedUsersList: Set<UserId>
     ): Either<CoreFailure, Unit> {
-        return newGroupConversationSystemMessagesCreator.value.conversationResolvedMembersAddedAndFailed(
-            conversationId.toDao(), userIdList, failedUsersList.toList()
+        return newGroupConversationSystemMessagesCreator.value.conversationFailedToAddMembers(
+            conversationId, failedUsersList
         ).flatMap {
             Either.Right(Unit)
         }

logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt

@@ -68,6 +68,7 @@ import com.wire.kalium.network.exceptions.isMlsCommitMissingReferences
 import com.wire.kalium.network.exceptions.isMlsStaleMessage
 import com.wire.kalium.persistence.dao.conversation.ConversationDAO
 import com.wire.kalium.persistence.dao.conversation.ConversationEntity
+import com.wire.kalium.persistence.dao.message.LocalId
 import com.wire.kalium.util.DateTimeUtil
 import com.wire.kalium.util.KaliumDispatcher
 import com.wire.kalium.util.KaliumDispatcherImpl
@@ -367,7 +368,7 @@ internal class MLSConversationDataSource(
 
     private suspend fun processCommitBundleEvents(events: List<EventContentDTO>) {
         events.forEach { eventContentDTO ->
-            val event = MapperProvider.eventMapper(selfUserId).fromEventContentDTO("", eventContentDTO, true, false)
+            val event = MapperProvider.eventMapper(selfUserId).fromEventContentDTO(LocalId.generate(), eventContentDTO, true, false)
             if (event is Event.Conversation) {
                 commitBundleEventReceiver.onEvent(event)
             }

logic/src/commonMain/kotlin/com/wire/kalium/logic/data/message/MessageContent.kt

@@ -170,6 +170,7 @@ sealed class MessageContent {
             @SerialName("data")
             val targets: Targets? = null,
         )
+
         @Serializable
         data class Targets(
             @SerialName("targets")
@@ -249,11 +250,36 @@ sealed class MessageContent {
     // server message content types
     // TODO: rename members to userList
     sealed class MemberChange(open val members: List<UserId>) : System() {
+        /**
+         * A member(s) was added to the conversation.
+         */
         data class Added(override val members: List<UserId>) : MemberChange(members)
+
+        /**
+         * A member(s) was removed from the conversation.
+         */
         data class Removed(override val members: List<UserId>) : MemberChange(members)
+
+        /**
+         * A member(s) was removed from the team.
+         */
         data class RemovedFromTeam(override val members: List<UserId>) : MemberChange(members)
+
+        /**
+         * A member(s) was not added to the conversation.
+         * Note: This is only valid for the creator of the conversation, local-only.
+         */
         data class FailedToAdd(override val members: List<UserId>) : MemberChange(members)
+
+        /**
+         * A member(s) was added to the conversation while the conversation was being created.
+         * Note: This is only valid for the creator of the conversation, local-only.
+         */
         data class CreationAdded(override val members: List<UserId>) : MemberChange(members)
+
+        /**
+         * Member(s) removed from the conversation, due to some backend stopped to federate between them, or us.
+         */
         data class FederationRemoved(override val members: List<UserId>) : MemberChange(members)
     }
 
@@ -339,11 +365,13 @@ sealed class MessageContent {
         data class Removed(val domain: String) : FederationStopped()
         data class ConnectionRemoved(val domainList: List<String>) : FederationStopped()
     }
+
     sealed class LegalHold : System() {
         sealed class ForMembers(open val members: List<UserId>) : LegalHold() {
             data class Enabled(override val members: List<UserId>) : ForMembers(members)
             data class Disabled(override val members: List<UserId>) : ForMembers(members)
         }
+
         sealed class ForConversation : LegalHold() {
             data object Enabled : ForConversation()
             data object Disabled : ForConversation()

logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt

@@ -452,7 +452,7 @@ class ConversationGroupRepositoryTest {
             .withProtocolInfoById(MLS_PROTOCOL_INFO)
             .withAddMemberAPISucceedChanged()
             .withSuccessfulAddMemberToMLSGroup()
-            .withInsertAddedAndFailedSystemMessageSuccess()
+            .withInsertFailedToAddSystemMessageSuccess()
             .arrange()
 
         conversationGroupRepository.addMembers(listOf(TestConversation.USER_1), TestConversation.ID)
@@ -1201,7 +1201,7 @@ class ConversationGroupRepositoryTest {
                 KEY_PACKAGES_NOT_AVAILABLE_FAILURE,
                 Either.Right(Unit)
             )
-            .withInsertAddedAndFailedSystemMessageSuccess()
+            .withInsertFailedToAddSystemMessageSuccess()
             .arrange()
 
         // when
@@ -1223,8 +1223,8 @@ class ConversationGroupRepositoryTest {
             }).wasInvoked(exactly = once)
 
         verify(arrangement.newGroupConversationSystemMessagesCreator)
-            .suspendFunction(arrangement.newGroupConversationSystemMessagesCreator::conversationResolvedMembersAddedAndFailed)
-            .with(anything(), matching { it.size == 1 }, matching { it.size == 1 })
+            .suspendFunction(arrangement.newGroupConversationSystemMessagesCreator::conversationFailedToAddMembers)
+            .with(anything(), matching { it.size == 1 })
             .wasInvoked(exactly = once)
     }
 
@@ -1241,7 +1241,7 @@ class ConversationGroupRepositoryTest {
                 buildCommitBundleFederatedFailure("otherDomain"),
                 Either.Right(Unit)
             )
-            .withInsertAddedAndFailedSystemMessageSuccess()
+            .withInsertFailedToAddSystemMessageSuccess()
             .arrange()
 
         // when
@@ -1263,8 +1263,8 @@ class ConversationGroupRepositoryTest {
             }).wasInvoked(exactly = once)
 
         verify(arrangement.newGroupConversationSystemMessagesCreator)
-            .suspendFunction(arrangement.newGroupConversationSystemMessagesCreator::conversationResolvedMembersAddedAndFailed)
-            .with(anything(), matching { it.size == 1 }, matching { it.size == 1 })
+            .suspendFunction(arrangement.newGroupConversationSystemMessagesCreator::conversationFailedToAddMembers)
+            .with(anything(), matching { it.size == 1 })
             .wasInvoked(exactly = once)
     }
 

logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepositoryTest.kt

@@ -76,6 +76,7 @@ import com.wire.kalium.persistence.dao.UserIDEntity
 import com.wire.kalium.persistence.dao.conversation.ConversationDAO
 import com.wire.kalium.persistence.dao.conversation.ConversationEntity
 import com.wire.kalium.persistence.dao.conversation.E2EIConversationClientInfoEntity
+import com.wire.kalium.persistence.dao.message.LocalId
 import com.wire.kalium.util.DateTimeUtil
 import io.ktor.util.decodeBase64Bytes
 import io.ktor.util.encodeBase64
@@ -392,7 +393,7 @@ class MLSConversationRepositoryTest {
     }
 
     @Test
-    fun givenSuccessfulResponses_whenCallingAddMemberToMLSGroup_thenMemberJoinEventIsProcessed() = runTest {
+    fun givenSuccessfulResponses_whenCallingAddMemberToMLSGroup_thenMemberJoinEventIsProcessedWithLocalId() = runTest {
         val (arrangement, mlsConversationRepository) = Arrangement()
             .withCommitPendingProposalsReturningNothing()
             .withClaimKeyPackagesSuccessful()
@@ -406,7 +407,7 @@ class MLSConversationRepositoryTest {
 
         verify(arrangement.commitBundleEventReceiver)
             .suspendFunction(arrangement.commitBundleEventReceiver::onEvent)
-            .with(anyInstanceOf(Event.Conversation.MemberJoin::class))
+            .with(matching { it is Event.Conversation.MemberJoin && LocalId.check(it.id) })
             .wasInvoked(once)
     }
 
@@ -752,7 +753,7 @@ class MLSConversationRepositoryTest {
     }
 
     @Test
-    fun givenSuccessfulResponses_whenCallingRemoveMemberFromGroup_thenMemberLeaveEventIsProcessed() = runTest {
+    fun givenSuccessfulResponses_whenCallingRemoveMemberFromGroup_thenMemberLeaveEventIsProcessedWithLocalId() = runTest {
         val (arrangement, mlsConversationRepository) = Arrangement()
             .withCommitPendingProposalsReturningNothing()
             .withGetMLSClientSuccessful()
@@ -767,7 +768,7 @@ class MLSConversationRepositoryTest {
 
         verify(arrangement.commitBundleEventReceiver)
             .suspendFunction(arrangement.commitBundleEventReceiver::onEvent)
-            .with(anyInstanceOf(Event.Conversation.MemberLeave::class))
+            .with(matching { it is Event.Conversation.MemberLeave && LocalId.check(it.id) })
             .wasInvoked(once)
     }
 

persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/message/MessageEntity.kt

@@ -203,7 +203,37 @@ sealed interface MessageEntity {
     }
 
     enum class MemberChangeType {
-        ADDED, REMOVED, CREATION_ADDED, FAILED_TO_ADD, FEDERATION_REMOVED, REMOVED_FROM_TEAM;
+        /**
+         * A member(s) was added to the conversation.
+         */
+        ADDED,
+
+        /**
+         * A member(s) was removed from the conversation.
+         */
+        REMOVED,
+
+        /**
+         * A member(s) was added to the conversation while the conversation was being created.
+         * Note: This is only valid for the creator of the conversation, local-only.
+         */
+        CREATION_ADDED,
+
+        /**
+         * A member(s) was not added to the conversation.
+         * Note: This is only valid for the creator of the conversation, local-only.
+         */
+        FAILED_TO_ADD,
+
+        /**
+         * Member(s) removed from the conversation, due to some backend stopped to federate between them, or us.
+         */
+        FEDERATION_REMOVED,
+
+        /**
+         * A member(s) was removed from the team.
+         */
+        REMOVED_FROM_TEAM;
     }
 
     enum class FederationType {
@@ -339,6 +369,7 @@ sealed class MessageEntityContent {
     data object MissedCall : System()
     data object CryptoSessionReset : System()
     data class ConversationRenamed(val conversationName: String) : System()
+
     @Deprecated("not maintained and will be deleted")
     data class TeamMemberRemoved(val userName: String) : System()
     data class NewConversationReceiptMode(val receiptMode: Boolean) : System()
@@ -419,6 +450,7 @@ sealed class MessagePreviewEntityContent {
         val otherUserIdList: List<UserIDEntity>,
         val isContainSelfUserId: Boolean,
     ) : MessagePreviewEntityContent()
+
     data class TeamMembersRemoved(
         val senderName: String?,
         val otherUserIdList: List<UserIDEntity>,