Skip to content

Commit

Permalink
fix: new 1on1 conversation not visible on list [WPB-5551] (#2237)
Browse files Browse the repository at this point in the history 
* fix: new 1on1 conversation not visible on list [WPB-5551]

* remove unneeded observation and add tests
  • Loading branch information
@saleniuk @github-actions
saleniuk authored and github-actions[bot] committed Nov 20, 2023
1 parent b80ce0f commit 86c886e
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 45 deletions.
3 changes: 2 additions & 1 deletion logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1230,7 +1230,8 @@ class UserSessionScope internal constructor(
conversationRepository,
userRepository,
selfTeamId,
conversations.newGroupConversationSystemMessagesCreator
conversations.newGroupConversationSystemMessagesCreator,
oneOnOneResolver,
)
private val deletedConversationHandler: DeletedConversationEventHandler
get() = DeletedConversationEventHandlerImpl(
Expand Down
26 changes: 20 additions & 6 deletions ...in/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,25 @@

package com.wire.kalium.logic.sync.receiver.conversation

import com.wire.kalium.logic.CoreFailure
import com.wire.kalium.logic.data.conversation.ConversationRepository
import com.wire.kalium.logic.data.conversation.NewGroupConversationSystemMessagesCreator
import com.wire.kalium.logic.data.event.Event
import com.wire.kalium.logic.data.event.EventLoggingStatus
import com.wire.kalium.logic.data.event.logEventProcessing
import com.wire.kalium.logic.data.id.SelfTeamIdProvider
import com.wire.kalium.logic.data.id.toDao
import com.wire.kalium.logic.data.id.toModel
import com.wire.kalium.logic.data.user.UserRepository
import com.wire.kalium.logic.data.id.SelfTeamIdProvider
import com.wire.kalium.logic.feature.conversation.mls.OneOnOneResolver
import com.wire.kalium.logic.functional.Either
import com.wire.kalium.logic.functional.flatMap
import com.wire.kalium.logic.functional.getOrNull
import com.wire.kalium.logic.functional.map
import com.wire.kalium.logic.functional.onFailure
import com.wire.kalium.logic.functional.onSuccess
import com.wire.kalium.logic.kaliumLogger
import com.wire.kalium.network.api.base.authenticated.conversation.ConversationResponse
import com.wire.kalium.util.DateTimeUtil

interface NewConversationEventHandler {
Expand All @@ -44,17 +48,21 @@ internal class NewConversationEventHandlerImpl(
private val userRepository: UserRepository,
private val selfTeamIdProvider: SelfTeamIdProvider,
private val newGroupConversationSystemMessagesCreator: NewGroupConversationSystemMessagesCreator,
private val oneOnOneResolver: OneOnOneResolver,
) : NewConversationEventHandler {

override suspend fun handle(event: Event.Conversation.NewConversation) {
conversationRepository
.persistConversation(event.conversation, selfTeamIdProvider().getOrNull()?.value, true)
.flatMap { isNewUnhandledConversation ->
conversationRepository.updateConversationModifiedDate(event.conversationId, DateTimeUtil.currentInstant())
Either.Right(isNewUnhandledConversation)
}.flatMap { isNewUnhandledConversation ->
userRepository.fetchUsersIfUnknownByIds(event.conversation.members.otherMembers.map { it.id.toModel() }.toSet())
Either.Right(isNewUnhandledConversation)
resolveConversationIfOneOnOne(event)
.flatMap {
conversationRepository.updateConversationModifiedDate(event.conversationId, DateTimeUtil.currentInstant())
}
.flatMap {
userRepository.fetchUsersIfUnknownByIds(event.conversation.members.otherMembers.map { it.id.toModel() }.toSet())
}
.map { isNewUnhandledConversation }
}.onSuccess { isNewUnhandledConversation ->
createSystemMessagesForNewConversation(isNewUnhandledConversation, event)
kaliumLogger.logEventProcessing(EventLoggingStatus.SUCCESS, event)
Expand All @@ -63,6 +71,12 @@ internal class NewConversationEventHandlerImpl(
}
}

private suspend fun resolveConversationIfOneOnOne(event: Event.Conversation.NewConversation): Either<CoreFailure, Unit> =
if (event.conversation.type == ConversationResponse.Type.ONE_TO_ONE) {
val otherUserId = event.conversation.members.otherMembers.first().id.toModel()
oneOnOneResolver.resolveOneOnOneConversationWithUserId(otherUserId).map { Unit }
} else Either.Right(Unit)

/**
* Creates system messages for new conversation.
* Conversation started, members added and failed, read receipt status.
Expand Down
144 changes: 106 additions & 38 deletions ...otlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandlerTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,16 @@ import com.wire.kalium.logic.StorageFailure
import com.wire.kalium.logic.data.conversation.ConversationRepository
import com.wire.kalium.logic.data.conversation.NewGroupConversationSystemMessagesCreator
import com.wire.kalium.logic.data.event.Event
import com.wire.kalium.logic.data.id.ConversationId
import com.wire.kalium.logic.data.id.QualifiedID
import com.wire.kalium.logic.data.id.QualifiedIdMapper
import com.wire.kalium.logic.data.id.SelfTeamIdProvider
import com.wire.kalium.logic.data.id.TeamId
import com.wire.kalium.logic.data.id.toDao
import com.wire.kalium.logic.data.id.toModel
import com.wire.kalium.logic.data.user.UserId
import com.wire.kalium.logic.data.user.UserRepository
import com.wire.kalium.logic.data.id.SelfTeamIdProvider
import com.wire.kalium.logic.feature.conversation.mls.OneOnOneResolver
import com.wire.kalium.logic.framework.TestConversation
import com.wire.kalium.logic.framework.TestTeam
import com.wire.kalium.logic.framework.TestUser
Expand Down Expand Up @@ -58,15 +60,7 @@ class NewConversationEventHandlerTest {

@Test
fun givenNewConversationOriginatedFromEvent_whenHandlingIt_thenPersistConversationShouldBeCalled() = runTest {
val event = Event.Conversation.NewConversation(
id = "eventId",
conversationId = TestConversation.ID,
transient = false,
live = false,
timestampIso = "timestamp",
conversation = TestConversation.CONVERSATION_RESPONSE,
senderUserId = TestUser.SELF.id
)
val event = testNewConversationEvent()
val members = event.conversation.members.otherMembers.map { it.id.toModel() }.toSet()
val teamIdValue = "teamId"
val teamId = TeamId(teamIdValue)
Expand Down Expand Up @@ -102,16 +96,7 @@ class NewConversationEventHandlerTest {

@Test
fun givenNewConversationEvent_whenHandlingIt_thenConversationLastModifiedShouldBeUpdated() = runTest {
val event = Event.Conversation.NewConversation(
id = "eventId",
conversationId = TestConversation.ID,
transient = false,
live = false,
timestampIso = "timestamp",
conversation = TestConversation.CONVERSATION_RESPONSE,
senderUserId = TestUser.SELF.id
)

val event = testNewConversationEvent()
val members = event.conversation.members.otherMembers.map { it.id.toModel() }.toSet()
val teamId = TestTeam.TEAM_ID
val creatorQualifiedId = QualifiedID(
Expand Down Expand Up @@ -142,19 +127,12 @@ class NewConversationEventHandlerTest {
@Test
fun givenNewGroupConversationEvent_whenHandlingIt_thenPersistTheSystemMessagesForNewConversation() = runTest {
// given
val event = Event.Conversation.NewConversation(
id = "eventId",
conversationId = TestConversation.ID,
transient = false,
live = false,
timestampIso = "timestamp",
val event = testNewConversationEvent(
conversation = TestConversation.CONVERSATION_RESPONSE.copy(
creator = "creatorId@creatorDomain",
receiptMode = ReceiptMode.ENABLED
),
senderUserId = TestUser.SELF.id
)

val members = event.conversation.members.otherMembers.map { it.id.toModel() }.toSet()
val teamId = TestTeam.TEAM_ID
val creatorQualifiedId = QualifiedID(
Expand Down Expand Up @@ -209,19 +187,12 @@ class NewConversationEventHandlerTest {
fun givenNewGroupConversationEvent_whenHandlingItAndAlreadyPresent_thenShouldSkipPersistingTheSystemMessagesForNewConversation() =
runTest {
// given
val event = Event.Conversation.NewConversation(
id = "eventId",
conversationId = TestConversation.ID,
transient = false,
live = false,
timestampIso = "timestamp",
val event = testNewConversationEvent(
conversation = TestConversation.CONVERSATION_RESPONSE.copy(
creator = "creatorId@creatorDomain",
receiptMode = ReceiptMode.ENABLED
),
senderUserId = TestUser.SELF.id
)

val members = event.conversation.members.otherMembers.map { it.id.toModel() }.toSet()
val teamId = TestTeam.TEAM_ID
val creatorQualifiedId = QualifiedID(
Expand Down Expand Up @@ -266,6 +237,78 @@ class NewConversationEventHandlerTest {
.wasNotInvoked()
}

@Test
fun givenNewGroupConversationEvent_whenHandlingIt_thenShouldSkipExecutingOneOnOneResolver() =
runTest {
// given
val event = testNewConversationEvent(
conversation = TestConversation.CONVERSATION_RESPONSE.copy(type = ConversationResponse.Type.GROUP),
)
val members = event.conversation.members.otherMembers.map { it.id.toModel() }.toSet()
val teamId = TestTeam.TEAM_ID
val creatorQualifiedId = QualifiedID("creatorId", "creatorDomain")
val (arrangement, eventHandler) = Arrangement()
.withUpdateConversationModifiedDateReturning(Either.Right(Unit))
.withPersistingConversations(Either.Right(false))
.withFetchUsersIfUnknownIds(members)
.withSelfUserTeamId(Either.Right(teamId))
.withConversationStartedSystemMessage()
.withConversationResolvedMembersSystemMessage()
.withReadReceiptsSystemMessage()
.withQualifiedId(creatorQualifiedId)
.arrange()

// when
eventHandler.handle(event)

// then
verify(arrangement.oneOnOneResolver)
.suspendFunction(arrangement.oneOnOneResolver::resolveOneOnOneConversationWithUserId)
.with(any())
.wasNotInvoked()
verify(arrangement.oneOnOneResolver)
.suspendFunction(arrangement.oneOnOneResolver::resolveOneOnOneConversationWithUser)
.with(any())
.wasNotInvoked()
verify(arrangement.oneOnOneResolver)
.suspendFunction(arrangement.oneOnOneResolver::scheduleResolveOneOnOneConversationWithUserId)
.with(any())
.wasNotInvoked()
}

@Test
fun givenNewOneOnOneConversationEvent_whenHandlingIt_thenShouldExecuteOneOnOneResolver() =
runTest {
// given
val event = testNewConversationEvent(
conversation = TestConversation.CONVERSATION_RESPONSE.copy(type = ConversationResponse.Type.ONE_TO_ONE),
)
val members = event.conversation.members.otherMembers.map { it.id.toModel() }.toSet()
val otherUserId = members.first()
val teamId = TestTeam.TEAM_ID
val creatorQualifiedId = QualifiedID("creatorId", "creatorDomain")
val (arrangement, eventHandler) = Arrangement()
.withUpdateConversationModifiedDateReturning(Either.Right(Unit))
.withPersistingConversations(Either.Right(false))
.withFetchUsersIfUnknownIds(members)
.withSelfUserTeamId(Either.Right(teamId))
.withConversationStartedSystemMessage()
.withConversationResolvedMembersSystemMessage()
.withReadReceiptsSystemMessage()
.withQualifiedId(creatorQualifiedId)
.withResolveOneOnOneConversationWithUserId(Either.Right(event.conversationId))
.arrange()

// when
eventHandler.handle(event)

// then
verify(arrangement.oneOnOneResolver)
.suspendFunction(arrangement.oneOnOneResolver::resolveOneOnOneConversationWithUserId)
.with(eq(otherUserId))
.wasInvoked(exactly = once)
}

private class Arrangement {
@Mock
val conversationRepository = mock(classOf<ConversationRepository>())
Expand All @@ -282,12 +325,16 @@ class NewConversationEventHandlerTest {
@Mock
private val qualifiedIdMapper = mock(classOf<QualifiedIdMapper>())

@Mock
val oneOnOneResolver = mock(classOf<OneOnOneResolver>())


private val newConversationEventHandler: NewConversationEventHandler = NewConversationEventHandlerImpl(
conversationRepository,
userRepository,
selfTeamIdProvider,
newGroupConversationSystemMessagesCreator
newGroupConversationSystemMessagesCreator,
oneOnOneResolver
)

fun withUpdateConversationModifiedDateReturning(result: Either<StorageFailure, Unit>) = apply {
Expand Down Expand Up @@ -330,7 +377,7 @@ class NewConversationEventHandlerTest {
.thenReturn(Either.Right(Unit))
}

suspend fun withFetchUsersIfUnknownIds(members: Set<QualifiedID>) = apply {
fun withFetchUsersIfUnknownIds(members: Set<QualifiedID>) = apply {
given(userRepository)
.suspendFunction(userRepository::fetchUsersIfUnknownByIds)
.whenInvokedWith(eq(members))
Expand Down Expand Up @@ -358,7 +405,28 @@ class NewConversationEventHandlerTest {
.thenReturn(qualifiedId)
}

fun withResolveOneOnOneConversationWithUserId(result: Either<CoreFailure, ConversationId>) = apply {
given(oneOnOneResolver)
.suspendFunction(oneOnOneResolver::resolveOneOnOneConversationWithUserId)
.whenInvokedWith(any())
.thenReturn(result)
}

fun arrange() = this to newConversationEventHandler
}

companion object {
private fun testNewConversationEvent(
conversation: ConversationResponse = TestConversation.CONVERSATION_RESPONSE,
) = Event.Conversation.NewConversation(
id = "eventId",
conversationId = TestConversation.ID,
transient = false,
live = false,
timestampIso = "timestamp",
conversation = conversation,
senderUserId = TestUser.SELF.id
)
}

}

0 comments on commit 86c886e

Please sign in to comment.