From ce722796310e3f805d00186ed64794bd714a9ecc Mon Sep 17 00:00:00 2001 From: Mohamad Jaara Date: Mon, 27 Nov 2023 12:12:49 +0100 Subject: [PATCH 1/2] refactor: update the list of conversation members when adding a new members fails with 403 --- .../kalium/logic/data/user/UserRepository.kt | 1 - .../kalium/logic/feature/UserSessionScope.kt | 1 - .../AddMemberToConversationUseCase.kt | 35 ++++++++++++--- .../feature/conversation/ConversationScope.kt | 6 +-- .../AddMemberToConversationUseCaseTest.kt | 44 +++++++++++++++++-- .../arrangement/UserRepositoryArrangement.kt | 18 ++++++++ 6 files changed, 90 insertions(+), 15 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserRepository.kt index faee56c88b8..c804b7a349b 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserRepository.kt @@ -248,7 +248,6 @@ internal class UserDataSource internal constructor( .flatMap { persistUsers(listOf(userProfileDTO), it) } } - @Suppress("MagicNumber") override suspend fun fetchUsersByIds(qualifiedUserIdList: Set): Either = if (qualifiedUserIdList.isEmpty()) { Either.Right(Unit) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt index 3260839f07f..2b0ef03c0dd 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt @@ -1523,7 +1523,6 @@ class UserSessionScope internal constructor( messages.sendConfirmation, renamedConversationHandler, qualifiedIdMapper, - team.isSelfATeamMember, globalScope.serverConfigRepository, userStorage, userPropertyRepository, diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/AddMemberToConversationUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/AddMemberToConversationUseCase.kt index e23531f56f4..45431a2361c 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/AddMemberToConversationUseCase.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/AddMemberToConversationUseCase.kt @@ -19,10 +19,15 @@ package com.wire.kalium.logic.feature.conversation import com.wire.kalium.logic.CoreFailure +import com.wire.kalium.logic.NetworkFailure import com.wire.kalium.logic.data.conversation.ConversationGroupRepository import com.wire.kalium.logic.data.id.ConversationId import com.wire.kalium.logic.data.user.UserId +import com.wire.kalium.logic.data.user.UserRepository import com.wire.kalium.logic.functional.fold +import com.wire.kalium.logic.functional.onFailure +import com.wire.kalium.network.exceptions.KaliumException +import io.ktor.http.HttpStatusCode /** * This use case will add a member(s) to a given conversation. @@ -42,13 +47,31 @@ interface AddMemberToConversationUseCase { } internal class AddMemberToConversationUseCaseImpl( - private val conversationGroupRepository: ConversationGroupRepository + private val conversationGroupRepository: ConversationGroupRepository, + private val userRepository: UserRepository ) : AddMemberToConversationUseCase { override suspend fun invoke(conversationId: ConversationId, userIdList: List): AddMemberToConversationUseCase.Result { - return conversationGroupRepository.addMembers(userIdList, conversationId).fold({ - AddMemberToConversationUseCase.Result.Failure(it) - }, { - AddMemberToConversationUseCase.Result.Success - }) + return conversationGroupRepository.addMembers(userIdList, conversationId) + .onFailure { + when (it) { + is NetworkFailure.ServerMiscommunication -> { + if (it.kaliumException is KaliumException.InvalidRequestError && + it.kaliumException.errorResponse.code == HttpStatusCode.Forbidden.value) { + handle403Error(userIdList) + } + } + + else -> { } + } + } + .fold({ + AddMemberToConversationUseCase.Result.Failure(it) + }, { + AddMemberToConversationUseCase.Result.Success + }) + } + + private suspend fun handle403Error(userIdList: List) { + userRepository.fetchUsersByIds(userIdList.toSet()) } } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/ConversationScope.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/ConversationScope.kt index 08d1cac07e0..df520ecfb56 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/ConversationScope.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/ConversationScope.kt @@ -64,7 +64,6 @@ import com.wire.kalium.logic.feature.team.DeleteTeamConversationUseCase import com.wire.kalium.logic.feature.team.DeleteTeamConversationUseCaseImpl import com.wire.kalium.logic.feature.team.GetSelfTeamUseCase import com.wire.kalium.logic.feature.team.GetSelfTeamUseCaseImpl -import com.wire.kalium.logic.feature.user.IsSelfATeamMemberUseCase import com.wire.kalium.logic.sync.SyncManager import com.wire.kalium.logic.sync.receiver.conversation.RenamedConversationEventHandler import com.wire.kalium.logic.sync.receiver.handler.CodeUpdateHandlerImpl @@ -89,10 +88,9 @@ class ConversationScope internal constructor( private val sendConfirmation: SendConfirmationUseCase, private val renamedConversationHandler: RenamedConversationEventHandler, private val qualifiedIdMapper: QualifiedIdMapper, - private val isSelfATeamMember: IsSelfATeamMemberUseCase, private val serverConfigRepository: ServerConfigRepository, private val userStorage: UserStorage, - private val userPropertyRepository: UserPropertyRepository, + userPropertyRepository: UserPropertyRepository, private val oneOnOneResolver: OneOnOneResolver, private val scope: CoroutineScope ) { @@ -154,7 +152,7 @@ class ConversationScope internal constructor( ) val addMemberToConversationUseCase: AddMemberToConversationUseCase - get() = AddMemberToConversationUseCaseImpl(conversationGroupRepository) + get() = AddMemberToConversationUseCaseImpl(conversationGroupRepository, userRepository) val addServiceToConversationUseCase: AddServiceToConversationUseCase get() = AddServiceToConversationUseCase(groupRepository = conversationGroupRepository) diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/conversation/AddMemberToConversationUseCaseTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/conversation/AddMemberToConversationUseCaseTest.kt index 168bd211f60..90969d8011b 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/conversation/AddMemberToConversationUseCaseTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/conversation/AddMemberToConversationUseCaseTest.kt @@ -19,10 +19,15 @@ package com.wire.kalium.logic.feature.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.ConversationGroupRepository import com.wire.kalium.logic.framework.TestConversation import com.wire.kalium.logic.functional.Either +import com.wire.kalium.logic.util.arrangement.UserRepositoryArrangement +import com.wire.kalium.logic.util.arrangement.UserRepositoryArrangementImpl +import com.wire.kalium.network.api.base.model.ErrorResponse +import com.wire.kalium.network.exceptions.KaliumException import io.mockative.Mock import io.mockative.any import io.mockative.classOf @@ -70,12 +75,45 @@ class AddMemberToConversationUseCaseTest { .wasInvoked(exactly = once) } - private class Arrangement { + @Test + fun givenServerResponseWith403_whenAddingToGroupConversionFail_thenUpdateUsersInfo() = runTest { + + val error = NetworkFailure.ServerMiscommunication( + KaliumException.InvalidRequestError( + errorResponse = ErrorResponse( + code = 403, + message = "Forbidden", + label = "Forbidden" + ) + ) + ) + val (arrangement, addMemberUseCase) = Arrangement() + .arrange { + withAddMembers(Either.Left(error)) + withFetchUsersByIdReturning(Either.Right(Unit)) + } + + val result = addMemberUseCase(TestConversation.ID, listOf(TestConversation.USER_1)) + assertIs(result) + + verify(arrangement.conversationGroupRepository) + .suspendFunction(arrangement.conversationGroupRepository::addMembers) + .with(eq(listOf(TestConversation.USER_1)), eq(TestConversation.ID)) + .wasInvoked(exactly = once) + + verify(arrangement.userRepository) + .suspendFunction(arrangement.userRepository::fetchUsersByIds) + .with(eq(setOf(TestConversation.USER_1))) + .wasInvoked(exactly = once) + } + + private class Arrangement : UserRepositoryArrangement by UserRepositoryArrangementImpl() { @Mock val conversationGroupRepository = mock(classOf()) private val addMemberUseCase = AddMemberToConversationUseCaseImpl( - conversationGroupRepository + conversationGroupRepository, + userRepository ) fun withAddMembers(either: Either) = apply { @@ -85,6 +123,6 @@ class AddMemberToConversationUseCaseTest { .thenReturn(either) } - fun arrange() = this to addMemberUseCase + fun arrange(block: Arrangement.() -> Unit = { }) = apply(block).let { this to addMemberUseCase } } } diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/util/arrangement/UserRepositoryArrangement.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/util/arrangement/UserRepositoryArrangement.kt index 0c113d09cf8..2765bd50995 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/util/arrangement/UserRepositoryArrangement.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/util/arrangement/UserRepositoryArrangement.kt @@ -20,11 +20,13 @@ package com.wire.kalium.logic.util.arrangement import com.wire.kalium.logic.CoreFailure import com.wire.kalium.logic.data.user.OtherUser import com.wire.kalium.logic.data.user.SelfUser +import com.wire.kalium.logic.data.user.UserId import com.wire.kalium.logic.data.user.UserRepository import com.wire.kalium.logic.functional.Either import io.mockative.Mock import io.mockative.any import io.mockative.given +import io.mockative.matchers.Matcher import io.mockative.mock import kotlinx.coroutines.flow.Flow @@ -50,6 +52,11 @@ internal interface UserRepositoryArrangement { fun withFetchAllOtherUsersReturning(result: Either) fun withFetchUserInfoReturning(result: Either) + + fun withFetchUsersByIdReturning( + result: Either, + userIdList: Matcher> = any() + ) } internal class UserRepositoryArrangementImpl: UserRepositoryArrangement { @@ -121,4 +128,15 @@ internal class UserRepositoryArrangementImpl: UserRepositoryArrangement { .thenReturn(result) } + override fun withFetchUsersByIdReturning( + result: Either, + userIdList: Matcher> + ) { + given(userRepository) + .suspendFunction(userRepository::fetchUsersByIds) + .whenInvokedWith(userIdList) + .thenReturn(result) + } + + } From 0fbd651956ce600d20a4ed5d54b3e3305794b2fd Mon Sep 17 00:00:00 2001 From: Mohamad Jaara Date: Wed, 29 Nov 2023 15:21:38 +0100 Subject: [PATCH 2/2] Update logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/AddMemberToConversationUseCase.kt Co-authored-by: Yamil Medina --- .../feature/conversation/AddMemberToConversationUseCase.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/AddMemberToConversationUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/AddMemberToConversationUseCase.kt index 45431a2361c..d89fa263465 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/AddMemberToConversationUseCase.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/AddMemberToConversationUseCase.kt @@ -61,7 +61,7 @@ internal class AddMemberToConversationUseCaseImpl( } } - else -> { } + else -> { /* do nothing */ } } } .fold({