From 86c886eeb5c08b49baa3cd66beb43c7de69860dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= <30429749+saleniuk@users.noreply.github.com> Date: Mon, 20 Nov 2023 16:13:01 +0100 Subject: [PATCH 1/2] fix: new 1on1 conversation not visible on list [WPB-5551] (#2237) * fix: new 1on1 conversation not visible on list [WPB-5551] * remove unneeded observation and add tests --- .../kalium/logic/feature/UserSessionScope.kt | 3 +- .../NewConversationEventHandler.kt | 26 +++- .../NewConversationEventHandlerTest.kt | 144 +++++++++++++----- 3 files changed, 128 insertions(+), 45 deletions(-) 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 index fc9949dac81..a4c702d9ad9 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt @@ -1230,7 +1230,8 @@ class UserSessionScope internal constructor( conversationRepository, userRepository, selfTeamId, - conversations.newGroupConversationSystemMessagesCreator + conversations.newGroupConversationSystemMessagesCreator, + oneOnOneResolver, ) private val deletedConversationHandler: DeletedConversationEventHandler get() = DeletedConversationEventHandlerImpl( diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt index 035a3a6965e..8d5e3b1ea8b 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt @@ -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 { @@ -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) @@ -63,6 +71,12 @@ internal class NewConversationEventHandlerImpl( } } + private suspend fun resolveConversationIfOneOnOne(event: Event.Conversation.NewConversation): Either = + 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. diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandlerTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandlerTest.kt index e4d4c3151a7..dcd48ca4eba 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandlerTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandlerTest.kt @@ -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 @@ -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) @@ -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( @@ -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( @@ -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( @@ -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()) @@ -282,12 +325,16 @@ class NewConversationEventHandlerTest { @Mock private val qualifiedIdMapper = mock(classOf()) + @Mock + val oneOnOneResolver = mock(classOf()) + private val newConversationEventHandler: NewConversationEventHandler = NewConversationEventHandlerImpl( conversationRepository, userRepository, selfTeamIdProvider, - newGroupConversationSystemMessagesCreator + newGroupConversationSystemMessagesCreator, + oneOnOneResolver ) fun withUpdateConversationModifiedDateReturning(result: Either) = apply { @@ -330,7 +377,7 @@ class NewConversationEventHandlerTest { .thenReturn(Either.Right(Unit)) } - suspend fun withFetchUsersIfUnknownIds(members: Set) = apply { + fun withFetchUsersIfUnknownIds(members: Set) = apply { given(userRepository) .suspendFunction(userRepository::fetchUsersIfUnknownByIds) .whenInvokedWith(eq(members)) @@ -358,7 +405,28 @@ class NewConversationEventHandlerTest { .thenReturn(qualifiedId) } + fun withResolveOneOnOneConversationWithUserId(result: Either) = 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 + ) + } + } From a768a24831c962c00cc2b7b28d773803f2d7f8fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Mon, 20 Nov 2023 16:18:29 +0100 Subject: [PATCH 2/2] trigger build