Skip to content

Commit

Permalink
fix: 1:1s end up at the bottom of the list after migration [WPB-15452]
Browse files Browse the repository at this point in the history
  • Loading branch information
@saleniuk
saleniuk committed Jan 16, 2025
1 parent 76697f1 commit 81d146f
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 23 deletions.
28 changes: 21 additions & 7 deletions .../src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneMigrator.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,11 @@ import com.wire.kalium.logic.functional.Either
import com.wire.kalium.logic.functional.flatMap
import com.wire.kalium.logic.functional.fold
import com.wire.kalium.logic.functional.foldToEitherWhileRight
import com.wire.kalium.logic.functional.getOrNull
import com.wire.kalium.logic.functional.map
import com.wire.kalium.logic.kaliumLogger
import com.wire.kalium.util.DateTimeUtil
import kotlinx.datetime.Instant

interface OneOnOneMigrator {
/**
Expand All @@ -58,7 +61,8 @@ internal class OneOnOneMigratorImpl(
private val conversationRepository: ConversationRepository,
private val messageRepository: MessageRepository,
private val userRepository: UserRepository,
private val systemMessageInserter: SystemMessageInserter
private val systemMessageInserter: SystemMessageInserter,
private val currentInstant: CurrentInstantProvider = CurrentInstantProvider { DateTimeUtil.currentInstant() },
) : OneOnOneMigrator {

override suspend fun migrateToProteus(user: OtherUser): Either<CoreFailure, ConversationId> =
Expand Down Expand Up @@ -140,16 +144,26 @@ internal class OneOnOneMigratorImpl(
).flatMap { proteusOneOnOneConversations ->
// We can theoretically have more than one proteus 1-1 conversation with
// team members since there was no backend safeguards against this
proteusOneOnOneConversations.foldToEitherWhileRight(Unit) { proteusOneOnOneConversation, _ ->
kaliumLogger.d(
"migrating proteus ${proteusOneOnOneConversation.toLogString()} " +
"to MLS conv ${targetConversation.toLogString()}"
)
proteusOneOnOneConversations.foldToEitherWhileRight(null as Instant?) { proteusOneOnOneConversation, lastModifiedDate ->
messageRepository.moveMessagesToAnotherConversation(
originalConversation = proteusOneOnOneConversation,
targetConversation = targetConversation
)
).map {
// Emit most recent last modified date of the proteus conversations to pass it to the target conversation
conversationRepository.getConversationById(proteusOneOnOneConversation).getOrNull()?.lastModifiedDate.let {
listOfNotNull(lastModifiedDate, it).maxOrNull()
}
}
}.map { lastModifiedDate ->
// Fallback to current time if not found as it means that it's completely new conversation without any history
lastModifiedDate ?: currentInstant()
}
}.flatMap { lastModifiedDate ->
conversationRepository.updateConversationModifiedDate(targetConversation, lastModifiedDate)
}
}
}

fun interface CurrentInstantProvider {
operator fun invoke(): Instant
}
98 changes: 82 additions & 16 deletions .../commonTest/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneMigratorTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,17 @@ import com.wire.kalium.logic.util.arrangement.repository.UserRepositoryArrangeme
import com.wire.kalium.logic.util.arrangement.repository.UserRepositoryArrangementImpl
import com.wire.kalium.logic.util.shouldFail
import com.wire.kalium.logic.util.shouldSucceed
import com.wire.kalium.util.DateTimeUtil
import io.mockative.Mock
import io.mockative.any
import io.mockative.coVerify
import io.mockative.eq
import io.mockative.every
import io.mockative.mock
import io.mockative.once
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.test.runTest
import kotlinx.datetime.Instant
import kotlin.test.Test
import kotlin.test.assertEquals

Expand All @@ -65,6 +70,25 @@ class OneOnOneMigratorTest {
}.wasNotInvoked()
}

@Test
fun givenUnassignedOneOnOne_whenMigratingToProteus_thenShouldAssignOneOnOneConversation() = runTest {
val user = TestUser.OTHER.copy(
activeOneOnOneConversationId = null
)

val (arrangement, oneOneMigrator) = arrange {
withGetOneOnOneConversationsWithOtherUserReturning(Either.Right(listOf(TestConversation.ID)))
withUpdateOneOnOneConversationReturning(Either.Right(Unit))
}

oneOneMigrator.migrateToProteus(user)
.shouldSucceed()

coVerify {
arrangement.userRepository.updateActiveOneOnOneConversation(eq(user.id), eq(TestConversation.ID))
}.wasInvoked()
}

@Test
fun givenNoExistingTeamOneOnOne_whenMigratingToProteus_thenShouldCreateGroupConversation() = runTest {
val user = TestUser.OTHER.copy(
Expand Down Expand Up @@ -171,8 +195,10 @@ class OneOnOneMigratorTest {
val (_, oneOnOneMigrator) = arrange {
withResolveConversationReturning(Either.Right(TestConversation.ID))
withGetOneOnOneConversationsWithOtherUserReturning(Either.Right(listOf(TestConversation.ID)))
withGetConversationByIdReturning(TestConversation.CONVERSATION.copy(id = TestConversation.ID))
withMoveMessagesToAnotherConversation(Either.Right(Unit))
withUpdateOneOnOneConversationReturning(Either.Left(failure))
withUpdateConversationModifiedDate(Either.Right(Unit))
}

oneOnOneMigrator.migrateToMLS(user)
Expand All @@ -191,8 +217,10 @@ class OneOnOneMigratorTest {
val (arrangement, oneOnOneMigrator) = arrange {
withResolveConversationReturning(Either.Right(resolvedConversationId))
withGetOneOnOneConversationsWithOtherUserReturning(Either.Right(listOf(originalConversationId)))
withGetConversationByIdReturning(TestConversation.CONVERSATION.copy(id = originalConversationId))
withMoveMessagesToAnotherConversation(Either.Right(Unit))
withUpdateOneOnOneConversationReturning(Either.Right(Unit))
withUpdateConversationModifiedDate(Either.Right(Unit))
}

oneOnOneMigrator.migrateToMLS(user)
Expand All @@ -217,8 +245,10 @@ class OneOnOneMigratorTest {
val (arrangement, oneOnOneMigrator) = arrange {
withResolveConversationReturning(Either.Right(resolvedConversationId))
withGetOneOnOneConversationsWithOtherUserReturning(Either.Right(listOf(originalConversationId)))
withGetConversationByIdReturning(TestConversation.CONVERSATION.copy(id = originalConversationId))
withMoveMessagesToAnotherConversation(Either.Right(Unit))
withUpdateOneOnOneConversationReturning(Either.Right(Unit))
withUpdateConversationModifiedDate(Either.Right(Unit))
}

oneOnOneMigrator.migrateToMLS(user)
Expand All @@ -234,30 +264,51 @@ class OneOnOneMigratorTest {
}

@Test
fun givenExistingTeamOneOnOne_whenMigratingToProteus_thenShouldNOTCreateGroupConversation() = runTest {
val user = TestUser.OTHER.copy(
activeOneOnOneConversationId = null
fun givenProteusConversation_whenMigratingToMLS_thenUpdateLastModifiedDateFromOriginalConversation() = runTest {
val lastModified = DateTimeUtil.currentInstant()
val originalConversation = TestConversation.CONVERSATION.copy(
id = ConversationId("someRandomConversationId", "testDomain"),
lastModifiedDate = lastModified
)

val (arrangement, oneOneMigrator) = arrange {
withGetOneOnOneConversationsWithOtherUserReturning(Either.Right(listOf(TestConversation.ONE_ON_ONE().id)))
val resolvedConversationId = ConversationId("resolvedMLSConversationId", "anotherDomain")
val user = TestUser.OTHER.copy(activeOneOnOneConversationId = originalConversation.id)
val (arrangement, oneOnOneMigrator) = arrange {
withResolveConversationReturning(Either.Right(resolvedConversationId))
withGetOneOnOneConversationsWithOtherUserReturning(Either.Right(listOf(originalConversation.id)))
withGetConversationByIdReturning(originalConversation)
withMoveMessagesToAnotherConversation(Either.Right(Unit))
withUpdateOneOnOneConversationReturning(Either.Right(Unit))
withUpdateConversationModifiedDate(Either.Right(Unit))
}

oneOneMigrator.migrateExistingProteus(user)
oneOnOneMigrator.migrateToMLS(user)
.shouldSucceed()

coVerify {
arrangement.conversationGroupRepository.createGroupConversation(
name = eq<String?>(null),
usersList = eq(listOf(TestUser.OTHER.id)),
options = eq(ConversationOptions())
)
}.wasNotInvoked()
arrangement.conversationRepository.updateConversationModifiedDate(eq(resolvedConversationId), eq(lastModified))
}.wasInvoked(exactly = once)
}

@Test
fun givenNoProteusConversation_whenMigratingToMLS_thenUpdateLastModifiedDateToBeCurrentDate() = runTest {
val resolvedConversationId = ConversationId("resolvedMLSConversationId", "anotherDomain")
val lastModified = DateTimeUtil.currentInstant()
val user = TestUser.OTHER.copy(activeOneOnOneConversationId = null)
val (arrangement, oneOnOneMigrator) = arrange {
withResolveConversationReturning(Either.Right(resolvedConversationId))
withGetOneOnOneConversationsWithOtherUserReturning(Either.Right(listOf()))
withMoveMessagesToAnotherConversation(Either.Right(Unit))
withUpdateConversationModifiedDate(Either.Right(Unit))
withUpdateOneOnOneConversationReturning(Either.Right(Unit))
withCurrentInstant(lastModified)
}

oneOnOneMigrator.migrateToMLS(user)
.shouldSucceed()

coVerify {
arrangement.userRepository.updateActiveOneOnOneConversation(eq(TestUser.OTHER.id), eq(TestConversation.ONE_ON_ONE().id))
}.wasInvoked()
arrangement.conversationRepository.updateConversationModifiedDate(eq(resolvedConversationId), eq(lastModified))
}.wasInvoked(exactly = once)
}

private class Arrangement(private val block: suspend Arrangement.() -> Unit) :
Expand All @@ -266,6 +317,20 @@ class OneOnOneMigratorTest {
ConversationRepositoryArrangement by ConversationRepositoryArrangementImpl(),
ConversationGroupRepositoryArrangement by ConversationGroupRepositoryArrangementImpl(),
UserRepositoryArrangement by UserRepositoryArrangementImpl() {

@Mock
val currentInstantProvider: CurrentInstantProvider = mock(CurrentInstantProvider::class)

fun withCurrentInstant(currentInstant: Instant) {
every {
currentInstantProvider()
}.returns(currentInstant)
}

init {
withCurrentInstant(DateTimeUtil.currentInstant())
}

fun arrange() = run {
runBlocking { block() }
this@Arrangement to OneOnOneMigratorImpl(
Expand All @@ -274,7 +339,8 @@ class OneOnOneMigratorTest {
conversationRepository = conversationRepository,
messageRepository = messageRepository,
userRepository = userRepository,
systemMessageInserter = systemMessageInserter
systemMessageInserter = systemMessageInserter,
currentInstant = currentInstantProvider,
)
}
}
Expand Down

0 comments on commit 81d146f

Please sign in to comment.