From 51848cba5ee58cef91a6857b3e2a8155075b8079 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 24 Nov 2023 15:22:06 +0100 Subject: [PATCH] fix: wrong conversation type when resolving 1on1 after receiving new conversation event [WPB-5551] (#2248) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: wrong conversation type when resolving 1on1 after receiving new conversation event [WPB-5551] (#2247) * trigger build --------- Co-authored-by: Michał Saleniuk <30429749+saleniuk@users.noreply.github.com> Co-authored-by: Michał Saleniuk --- .../data/conversation/ConversationMapper.kt | 43 +++++++++---------- .../NewConversationEventHandler.kt | 14 +++--- .../conversation/ConversationMapperTest.kt | 27 ++++++++++++ .../conversation/ConversationResponse.kt | 7 +++ 4 files changed, 63 insertions(+), 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 index fd9254f00bd..036fec4680b 100644 --- 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/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt index 8d5e3b1ea8b..46d857d301b 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt +++ b/logic/src/commonMain/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 = - 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/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationMapperTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationMapperTest.kt index 72d0c5a8cba..cac367ef6c3 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationMapperTest.kt +++ b/logic/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/network/src/commonMain/kotlin/com/wire/kalium/network/api/base/authenticated/conversation/ConversationResponse.kt b/network/src/commonMain/kotlin/com/wire/kalium/network/api/base/authenticated/conversation/ConversationResponse.kt index 6b759524718..745ec27ffad 100644 --- a/network/src/commonMain/kotlin/com/wire/kalium/network/api/base/authenticated/conversation/ConversationResponse.kt +++ b/network/src/commonMain/kotlin/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,