Skip to content

Commit

Permalink
fix: infinite profile loading screen when user cannot be added (WPB-1…
Browse files Browse the repository at this point in the history
…4842) (#3187)

Co-authored-by: sergei.bakhtiarov <[email protected]>
  • Loading branch information
sergeibakhtiarov and sbakhtiarov authored Dec 18, 2024
1 parent 91b8319 commit bb9bd8f
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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?>>
Expand Down Expand Up @@ -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) =
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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> =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand All @@ -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))
}
Expand All @@ -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))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
)

Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit bb9bd8f

Please sign in to comment.