fix: infinite profile loading screen when user cannot be added (WPB-1…

…4842)
wireapp · Dec 17, 2024 · 48bb02d · 48bb02d
1 parent 91b8319
commit 48bb02d
Show file tree

Hide file tree

Showing 7 changed files with 58 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
@@ -100,7 +100,7 @@ interface UserRepository {
      * Fetches user information for all of users id stored in the DB
      */
     suspend fun fetchAllOtherUsers(): Either<CoreFailure, Unit>
-    suspend fun fetchUsersByIds(qualifiedUserIdList: Set<UserId>): Either<CoreFailure, Unit>
+    suspend fun fetchUsersByIds(qualifiedUserIdList: Set<UserId>): Either<CoreFailure, Boolean>
     suspend fun fetchUsersIfUnknownByIds(ids: Set<UserId>): Either<CoreFailure, Unit>
     suspend fun observeSelfUser(): Flow<SelfUser>
     suspend fun observeSelfUserWithTeam(): Flow<Pair<SelfUser, Team?>>
@@ -266,7 +266,7 @@ internal class UserDataSource internal constructor(
 
     override suspend fun fetchAllOtherUsers(): Either<CoreFailure, Unit> {
         val ids = userDAO.allOtherUsersId().map(UserIDEntity::toModel).toSet()
-        return fetchUsersByIds(ids)
+        return fetchUsersByIds(ids).map { }
     }
 
     override suspend fun fetchUserInfo(userId: UserId) =
@@ -328,8 +328,8 @@ internal class UserDataSource internal constructor(
                 }
         }
 
-    override suspend fun fetchUsersByIds(qualifiedUserIdList: Set<UserId>): Either<CoreFailure, Unit> =
-        fetchUsersByIdsReturningListUsersDTO(qualifiedUserIdList).map { }
+    override suspend fun fetchUsersByIds(qualifiedUserIdList: Set<UserId>): Either<CoreFailure, Boolean> =
+        fetchUsersByIdsReturningListUsersDTO(qualifiedUserIdList).map { it.usersFound.isNotEmpty() }
 
     private suspend fun fetchTeamMembersByIds(userProfileList: List<UserProfileDTO>): Either<CoreFailure, List<TeamMemberDTO>> {
         val selfUserDomain = selfUserId.domain
@@ -411,7 +411,7 @@ internal class UserDataSource internal constructor(
         qualifiedIDList.filterNot { knownUsers.any { userEntity -> userEntity.id == it && !userEntity.name.isNullOrBlank() } }
     }.flatMap { missingIds ->
         if (missingIds.isEmpty()) Either.Right(Unit)
-        else fetchUsersByIds(missingIds.map { it.toModel() }.toSet())
+        else fetchUsersByIds(missingIds.map { it.toModel() }.toSet()).map { }
     }
 
     @OptIn(ExperimentalCoroutinesApi::class)
@@ -629,7 +629,7 @@ internal class UserDataSource internal constructor(
     }.flatMap { usersWithoutMetadata ->
         kaliumLogger.d("Numbers of users to refresh: ${usersWithoutMetadata.size}")
         val userIds = usersWithoutMetadata.map { it.id.toModel() }.toSet()
-        fetchUsersByIds(userIds)
+        fetchUsersByIds(userIds).map { }
     }
 
     override suspend fun removeUserBrokenAsset(qualifiedID: QualifiedID) = wrapStorageRequest {

diff --git a/.../src/commonMain/kotlin/com/wire/kalium/logic/feature/message/MessageSendFailureHandler.kt b/.../src/commonMain/kotlin/com/wire/kalium/logic/feature/message/MessageSendFailureHandler.kt
@@ -111,7 +111,7 @@ class MessageSendFailureHandlerImpl internal constructor(
 
     private suspend fun syncUserIds(userId: Set<UserId>): Either<CoreFailure, Unit> {
         return if (userId.isEmpty()) Either.Right(Unit)
-        else userRepository.fetchUsersByIds(userId)
+        else userRepository.fetchUsersByIds(userId).map { }
     }
 
     private suspend fun addMissingClients(missingClients: Map<UserId, List<ClientId>>): Either<CoreFailure, Unit> =

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/user/ObserveUserInfoUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/user/ObserveUserInfoUseCase.kt
@@ -77,7 +77,15 @@ internal class ObserveUserInfoUseCaseImpl(
                 either.fold({ storageFailure ->
                     if (storageFailure is StorageFailure.DataNotFound) {
                         userRepository.fetchUsersByIds(setOf(userId))
-                            .fold({ ObserveOtherUserResult(fetchUserError = it) }) { ObserveOtherUserResult() }
+                            .fold({ ObserveOtherUserResult(fetchUserError = it) }) { usersFound ->
+                                if (usersFound) {
+                                    // Fetched users are persisted
+                                    ObserveOtherUserResult()
+                                } else {
+                                    // Users cannot be found
+                                    ObserveOtherUserResult(fetchUserError = StorageFailure.DataNotFound)
+                                }
+                            }
                     } else {
                         ObserveOtherUserResult(getKnownUserError = storageFailure)
                     }

diff --git a/.../src/commonTest/kotlin/com/wire/kalium/logic/data/prekey/MessageSendFailureHandlerTest.kt b/.../src/commonTest/kotlin/com/wire/kalium/logic/data/prekey/MessageSendFailureHandlerTest.kt
@@ -400,7 +400,7 @@ class MessageSendFailureHandlerTest {
         suspend fun withFetchUsersByIdSuccess() = apply {
             coEvery {
                 userRepository.fetchUsersByIds(any())
-            }.returns(Either.Right(Unit))
+            }.returns(Either.Right(true))
         }
 
         suspend fun withFetchUsersByIdFailure(failure: CoreFailure) = apply {

diff --git a/.../commonTest/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneResolverTest.kt b/.../commonTest/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneResolverTest.kt
@@ -101,7 +101,7 @@ class OneOnOneResolverTest {
         // given
         val oneOnOneUser = TestUser.OTHER.copy(id = TestUser.OTHER_USER_ID)
         val (arrangement, resolver) = arrange {
-            withFetchUsersByIdReturning(Either.Right(Unit))
+            withFetchUsersByIdReturning(Either.Right(true))
             withGetProtocolForUser(Either.Right(SupportedProtocol.MLS))
             withMigrateToMLSReturns(Either.Right(TestConversation.ID))
         }
@@ -128,7 +128,7 @@ class OneOnOneResolverTest {
         // given
         val oneOnOneUser = TestUser.OTHER.copy(id = TestUser.OTHER_USER_ID)
         val (arrangement, resolver) = arrange {
-            withFetchUsersByIdReturning(Either.Right(Unit))
+            withFetchUsersByIdReturning(Either.Right(true))
             withGetProtocolForUser(Either.Right(SupportedProtocol.MLS))
             withMigrateToMLSReturns(Either.Right(TestConversation.ID))
         }
@@ -149,7 +149,7 @@ class OneOnOneResolverTest {
         val oneOnOneUser = TestUser.OTHER.copy(id = TestUser.OTHER_USER_ID)
         val (arrangement, resolver) = arrange {
             withGetKnownUserReturning(flowOf(oneOnOneUser))
-            withFetchUsersByIdReturning(Either.Right(Unit))
+            withFetchUsersByIdReturning(Either.Right(true))
             withGetProtocolForUser(Either.Right(SupportedProtocol.MLS))
             withMigrateToMLSReturns(Either.Right(TestConversation.ID))
         }

diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/user/ObserveUserInfoUseCaseTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/user/ObserveUserInfoUseCaseTest.kt
@@ -20,6 +20,7 @@ package com.wire.kalium.logic.feature.user
 
 import app.cash.turbine.test
 import com.wire.kalium.logic.CoreFailure
+import com.wire.kalium.logic.StorageFailure
 import com.wire.kalium.logic.data.team.TeamRepository
 import com.wire.kalium.logic.data.user.UserId
 import com.wire.kalium.logic.data.user.UserRepository
@@ -231,6 +232,32 @@ class ObserveUserInfoUseCaseTest {
         }
     }
 
+    @Test
+    fun givenAUserIdWhichIsNotInDBAndNotOnServer_whenInvokingObserveUserInfo_thenErrorIsReturned() = runTest {
+        // given
+        val (arrangement, useCase) = arrangement
+            .withFailingUserRetrieveFromDB()
+            .withSuccessfulTeamRetrieve(localTeamPresent = true)
+            .withSuccessfulUserFetchingNoUsersFound()
+            .arrange()
+
+        // when
+        useCase(userId).test {
+            val result = awaitItem()
+
+            // then
+            assertIs<GetUserInfoResult.Failure>(result)
+
+            with(arrangement) {
+                coVerify {
+                    userRepository.fetchUsersByIds(any())
+                }.wasInvoked(once)
+            }
+
+            awaitComplete()
+        }
+    }
+
     private class ObserveUserInfoUseCaseTestArrangement {
 
         @Mock
@@ -276,7 +303,15 @@ class ObserveUserInfoUseCaseTest {
         suspend fun withSuccessfulUserFetching(): ObserveUserInfoUseCaseTestArrangement {
             coEvery {
                 userRepository.fetchUsersByIds(any())
-            }.returns(Either.Right(Unit))
+            }.returns(Either.Right(true))
+
+            return this
+        }
+
+        suspend fun withSuccessfulUserFetchingNoUsersFound(): ObserveUserInfoUseCaseTestArrangement {
+            coEvery {
+                userRepository.fetchUsersByIds(any())
+            }.returns(Either.Right(false))
 
             return this
         }

diff --git a/...est/kotlin/com/wire/kalium/logic/util/arrangement/repository/UserRepositoryArrangement.kt b/...est/kotlin/com/wire/kalium/logic/util/arrangement/repository/UserRepositoryArrangement.kt
@@ -72,7 +72,7 @@ internal interface UserRepositoryArrangement {
     suspend fun withFetchUserInfoReturning(result: Either<CoreFailure, Unit>)
 
     suspend fun withFetchUsersByIdReturning(
-        result: Either<CoreFailure, Unit>,
+        result: Either<CoreFailure, Boolean>,
         userIdList: Matcher<Set<UserId>> = AnyMatcher(valueOf())
     )
 
@@ -186,7 +186,7 @@ internal open class UserRepositoryArrangementImpl : UserRepositoryArrangement {
     }
 
     override suspend fun withFetchUsersByIdReturning(
-        result: Either<CoreFailure, Unit>,
+        result: Either<CoreFailure, Boolean>,
         userIdList: Matcher<Set<UserId>>
     ) {
         coEvery {