refactor: update the list of conversation members when adding a new m…

…embers fails with 403
wireapp · Nov 27, 2023 · ce72279 · ce72279
1 parent cec7bbd
commit ce72279
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 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
@@ -248,7 +248,6 @@ internal class UserDataSource internal constructor(
                     .flatMap { persistUsers(listOf(userProfileDTO), it) }
             }
 
-    @Suppress("MagicNumber")
     override suspend fun fetchUsersByIds(qualifiedUserIdList: Set<UserId>): Either<CoreFailure, Unit> =
         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
@@ -1523,7 +1523,6 @@ class UserSessionScope internal constructor(
             messages.sendConfirmation,
             renamedConversationHandler,
             qualifiedIdMapper,
-            team.isSelfATeamMember,
             globalScope.serverConfigRepository,
             userStorage,
             userPropertyRepository,

diff --git a/...nMain/kotlin/com/wire/kalium/logic/feature/conversation/AddMemberToConversationUseCase.kt b/...nMain/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<UserId>): 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<UserId>) {
+        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
@@ -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/...t/kotlin/com/wire/kalium/logic/feature/conversation/AddMemberToConversationUseCaseTest.kt b/...t/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<AddMemberToConversationUseCase.Result.Failure>(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<ConversationGroupRepository>())
 
         private val addMemberUseCase = AddMemberToConversationUseCaseImpl(
-            conversationGroupRepository
+            conversationGroupRepository,
+            userRepository
         )
 
         fun withAddMembers(either: Either<CoreFailure, Unit>) = 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/...src/commonTest/kotlin/com/wire/kalium/logic/util/arrangement/UserRepositoryArrangement.kt b/...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<CoreFailure, Unit>)
 
     fun withFetchUserInfoReturning(result: Either<CoreFailure, Unit>)
+
+    fun withFetchUsersByIdReturning(
+        result: Either<CoreFailure, Unit>,
+        userIdList: Matcher<Set<UserId>> = any()
+    )
 }
 
 internal class UserRepositoryArrangementImpl: UserRepositoryArrangement {
@@ -121,4 +128,15 @@ internal class UserRepositoryArrangementImpl: UserRepositoryArrangement {
             .thenReturn(result)
     }
 
+    override fun withFetchUsersByIdReturning(
+        result: Either<CoreFailure, Unit>,
+        userIdList: Matcher<Set<UserId>>
+        ) {
+        given(userRepository)
+            .suspendFunction(userRepository::fetchUsersByIds)
+            .whenInvokedWith(userIdList)
+            .thenReturn(result)
+    }
+
+
 }