fix: wrong conversation type when resolving 1on1 after receiving new …

…conversation event [WPB-5551] (#2247)
wireapp · Nov 22, 2023 · 83763fb · 83763fb
1 parent 043e931
commit 83763fb
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 28 deletions.
diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationMapper.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationMapper.kt
@@ -103,7 +103,7 @@ internal class ConversationMapperImpl(
     ): ConversationEntity = ConversationEntity(
         id = idMapper.fromApiToDao(apiModel.id),
         name = apiModel.name,
-        type = apiModel.getConversationType(selfUserTeamId),
+        type = apiModel.toConversationType(selfUserTeamId),
         teamId = apiModel.teamId,
         protocolInfo = apiModel.getProtocolInfo(mlsGroupState),
         mutedStatus = conversationStatusMapper.fromMutedStatusApiToDaoModel(apiModel.members.self.otrMutedStatus),
@@ -430,27 +430,6 @@ internal class ConversationMapperImpl(
         }
     }
 
-    private fun ConversationResponse.getConversationType(selfUserTeamId: TeamId?): ConversationEntity.Type {
-        return when (type) {
-            ConversationResponse.Type.SELF -> ConversationEntity.Type.SELF
-            ConversationResponse.Type.GROUP -> {
-                // Fake team 1:1 conversations
-                val onlyOneOtherMember = members.otherMembers.size == 1
-                val noCustomName = name.isNullOrBlank()
-                val belongsToSelfTeam = selfUserTeamId != null && selfUserTeamId.value == teamId
-                val isTeamOneOne = onlyOneOtherMember && noCustomName && belongsToSelfTeam
-                if (isTeamOneOne) {
-                    ConversationEntity.Type.ONE_ON_ONE
-                } else {
-                    ConversationEntity.Type.GROUP
-                }
-            }
-
-            ConversationResponse.Type.ONE_TO_ONE -> ConversationEntity.Type.ONE_ON_ONE
-            ConversationResponse.Type.WAIT_FOR_CONNECTION -> ConversationEntity.Type.CONNECTION_PENDING
-        }
-    }
-
     override fun verificationStatusFromEntity(verificationStatus: ConversationEntity.VerificationStatus) =
         Conversation.VerificationStatus.valueOf(verificationStatus.name)
 
@@ -459,6 +438,26 @@ internal class ConversationMapperImpl(
 
 }
 
+internal fun ConversationResponse.toConversationType(selfUserTeamId: TeamId?): ConversationEntity.Type {
+    return when (type) {
+        ConversationResponse.Type.SELF -> ConversationEntity.Type.SELF
+        ConversationResponse.Type.GROUP -> {
+            // Fake team 1:1 conversations
+            val onlyOneOtherMember = members.otherMembers.size == 1
+            val noCustomName = name.isNullOrBlank()
+            val belongsToSelfTeam = selfUserTeamId != null && selfUserTeamId.value == teamId
+            val isTeamOneOne = onlyOneOtherMember && noCustomName && belongsToSelfTeam
+            if (isTeamOneOne) {
+                ConversationEntity.Type.ONE_ON_ONE
+            } else {
+                ConversationEntity.Type.GROUP
+            }
+        }
+        ConversationResponse.Type.ONE_TO_ONE -> ConversationEntity.Type.ONE_ON_ONE
+        ConversationResponse.Type.WAIT_FOR_CONNECTION -> ConversationEntity.Type.CONNECTION_PENDING
+    }
+}
+
 private fun ConversationEntity.Type.fromDaoModelToType(): Conversation.Type = when (this) {
     ConversationEntity.Type.SELF -> Conversation.Type.SELF
     ConversationEntity.Type.ONE_ON_ONE -> Conversation.Type.ONE_ON_ONE

diff --git a/...in/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt b/...in/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt
@@ -18,13 +18,14 @@
 
 package com.wire.kalium.logic.sync.receiver.conversation
 
-import com.wire.kalium.logic.CoreFailure
 import com.wire.kalium.logic.data.conversation.ConversationRepository
 import com.wire.kalium.logic.data.conversation.NewGroupConversationSystemMessagesCreator
+import com.wire.kalium.logic.data.conversation.toConversationType
 import com.wire.kalium.logic.data.event.Event
 import com.wire.kalium.logic.data.event.EventLoggingStatus
 import com.wire.kalium.logic.data.event.logEventProcessing
 import com.wire.kalium.logic.data.id.SelfTeamIdProvider
+import com.wire.kalium.logic.data.id.TeamId
 import com.wire.kalium.logic.data.id.toDao
 import com.wire.kalium.logic.data.id.toModel
 import com.wire.kalium.logic.data.user.UserRepository
@@ -36,7 +37,7 @@ import com.wire.kalium.logic.functional.map
 import com.wire.kalium.logic.functional.onFailure
 import com.wire.kalium.logic.functional.onSuccess
 import com.wire.kalium.logic.kaliumLogger
-import com.wire.kalium.network.api.base.authenticated.conversation.ConversationResponse
+import com.wire.kalium.persistence.dao.conversation.ConversationEntity
 import com.wire.kalium.util.DateTimeUtil
 
 interface NewConversationEventHandler {
@@ -52,10 +53,11 @@ internal class NewConversationEventHandlerImpl(
 ) : NewConversationEventHandler {
 
     override suspend fun handle(event: Event.Conversation.NewConversation) {
+        val selfUserTeamId = selfTeamIdProvider().getOrNull()
         conversationRepository
-            .persistConversation(event.conversation, selfTeamIdProvider().getOrNull()?.value, true)
+            .persistConversation(event.conversation, selfUserTeamId?.value, true)
             .flatMap { isNewUnhandledConversation ->
-                resolveConversationIfOneOnOne(event)
+                resolveConversationIfOneOnOne(selfUserTeamId, event)
                     .flatMap {
                         conversationRepository.updateConversationModifiedDate(event.conversationId, DateTimeUtil.currentInstant())
                     }
@@ -71,8 +73,8 @@ internal class NewConversationEventHandlerImpl(
             }
     }
 
-    private suspend fun resolveConversationIfOneOnOne(event: Event.Conversation.NewConversation): Either<CoreFailure, Unit> =
-        if (event.conversation.type == ConversationResponse.Type.ONE_TO_ONE) {
+    private suspend fun resolveConversationIfOneOnOne(selfUserTeamId: TeamId?, event: Event.Conversation.NewConversation) =
+        if (event.conversation.toConversationType(selfUserTeamId) == ConversationEntity.Type.ONE_ON_ONE) {
             val otherUserId = event.conversation.members.otherMembers.first().id.toModel()
             oneOnOneResolver.resolveOneOnOneConversationWithUserId(otherUserId).map { Unit }
         } else Either.Right(Unit)

diff --git a/...c/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationMapperTest.kt b/...c/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationMapperTest.kt
@@ -196,6 +196,33 @@ class ConversationMapperTest {
         assertEquals(ConversationEntity.Type.GROUP, result.type)
     }
 
+    @Test
+    fun givenAFakeTeamOneOnOneConversationResponse_whenMappingFromConversationResponseToDaoType_thenShouldMapToOneOnOneType() {
+        val response = CONVERSATION_RESPONSE.copy(
+            // Looks like a Group
+            type = ConversationResponse.Type.GROUP,
+            // No Name
+            name = null,
+            // Only one other participant
+            members = CONVERSATION_RESPONSE.members.copy(otherMembers = listOf(OTHER_MEMBERS.first())),
+            // Same team as user
+            teamId = SELF_USER_TEAM_ID.value
+        )
+        val result = response.toConversationType(SELF_USER_TEAM_ID)
+        assertEquals(ConversationEntity.Type.ONE_ON_ONE, result)
+    }
+
+    @Test
+    fun givenAGroupConversationResponseWithoutName_whenMappingFromConversationResponseToDaoType_thenShouldMapToGroupType() {
+        val response = CONVERSATION_RESPONSE.copy(
+            type = ConversationResponse.Type.GROUP,
+            name = null,
+            teamId = null
+        )
+        val result = response.toConversationType(SELF_USER_TEAM_ID)
+        assertEquals(ConversationEntity.Type.GROUP, result)
+    }
+
     private companion object {
         val ORIGINAL_CONVERSATION_ID = ConversationId("original", "oDomain")
         val SELF_USER_TEAM_ID = TeamId("teamID")

diff --git a/...otlin/com/wire/kalium/network/api/base/authenticated/conversation/ConversationResponse.kt b/...otlin/com/wire/kalium/network/api/base/authenticated/conversation/ConversationResponse.kt
@@ -54,6 +54,13 @@ data class ConversationResponse(
     @SerialName("epoch")
     val epoch: ULong?,
 
+    @Deprecated(
+        "For team 1on1 it can be a false group type",
+        ReplaceWith(
+            "this.toConversationType(selfUserTeamId)",
+            "com.wire.kalium.logic.data.conversation.toConversationType",
+        )
+    )
     @Serializable(with = ConversationTypeSerializer::class)
     val type: Type,