From 8d9bcafbbec9ba63c3a4636fd754016528338d42 Mon Sep 17 00:00:00 2001 From: Oussama Hassine Date: Sat, 14 Sep 2024 13:43:23 +0200 Subject: [PATCH] fix: Read receipts not generated when responding from notification (#2994) --- .../logic/data/message/MessageRepository.kt | 3 --- .../data/message/PersistMessageUseCase.kt | 2 -- .../data/message/MessageRepositoryTest.kt | 4 +-- .../data/message/PersistMessageUseCaseTest.kt | 26 ++++--------------- .../persistence/dao/message/MessageDAO.kt | 1 - .../persistence/dao/message/MessageDAOImpl.kt | 6 ----- .../dao/message/MessageTextEditTest.kt | 13 ++++------ 7 files changed, 12 insertions(+), 43 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/message/MessageRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/message/MessageRepository.kt index 66ce73703f3..dd0f47b5996 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/message/MessageRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/message/MessageRepository.kt @@ -80,7 +80,6 @@ internal interface MessageRepository { ) suspend fun persistMessage( message: Message.Standalone, - updateConversationReadDate: Boolean = false, updateConversationModifiedDate: Boolean = false, ): Either @@ -321,12 +320,10 @@ internal class MessageDataSource internal constructor( ) override suspend fun persistMessage( message: Message.Standalone, - updateConversationReadDate: Boolean, updateConversationModifiedDate: Boolean, ): Either = wrapStorageRequest { messageDAO.insertOrIgnoreMessage( messageMapper.fromMessageToEntity(message), - updateConversationReadDate, updateConversationModifiedDate ) } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/message/PersistMessageUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/message/PersistMessageUseCase.kt index cf443d8ac59..0d0288de34f 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/message/PersistMessageUseCase.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/message/PersistMessageUseCase.kt @@ -44,11 +44,9 @@ internal class PersistMessageUseCaseImpl( override suspend operator fun invoke(message: Message.Standalone): Either { val modifiedMessage = getExpectsReadConfirmationFromMessage(message) val isSelfSender = message.isSelfTheSender(selfUserId) - val shouldUpdateConversationReadDate = modifiedMessage.content !is MessageContent.MissedCall && isSelfSender return messageRepository.persistMessage( message = modifiedMessage, - updateConversationReadDate = shouldUpdateConversationReadDate, updateConversationModifiedDate = message.content.shouldUpdateConversationOrder() ).onSuccess { val isConversationMuted = it == InsertMessageResult.INSERTED_INTO_MUTED_CONVERSATION diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/message/MessageRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/message/MessageRepositoryTest.kt index 20daf4044a2..2462cc408d9 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/message/MessageRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/message/MessageRepositoryTest.kt @@ -163,7 +163,7 @@ class MessageRepositoryTest { }.wasInvoked(exactly = once) coVerify { - messageDAO.insertOrIgnoreMessage(eq(mappedEntity), any(), any()) + messageDAO.insertOrIgnoreMessage(eq(mappedEntity), any()) }.wasInvoked(exactly = once) } } @@ -764,7 +764,7 @@ class MessageRepositoryTest { suspend fun withInsertOrIgnoreMessage(result: InsertMessageResult) = apply { coEvery { - messageDAO.insertOrIgnoreMessage(any(), any(), any()) + messageDAO.insertOrIgnoreMessage(any(), any()) }.returns(result) } diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/message/PersistMessageUseCaseTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/message/PersistMessageUseCaseTest.kt index 14ebd92ac6f..5d57fd7140e 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/message/PersistMessageUseCaseTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/message/PersistMessageUseCaseTest.kt @@ -52,7 +52,7 @@ class PersistMessageUseCaseTest { result.shouldFail() coVerify { - arrangement.messageRepository.persistMessage(any(), any(), any()) + arrangement.messageRepository.persistMessage(any(), any()) }.wasInvoked(once) coVerify { @@ -76,7 +76,7 @@ class PersistMessageUseCaseTest { result.shouldSucceed() coVerify { - arrangement.messageRepository.persistMessage(any(), any(), any()) + arrangement.messageRepository.persistMessage(any(), any()) }.wasInvoked(once) coVerify { @@ -102,7 +102,7 @@ class PersistMessageUseCaseTest { result.shouldSucceed() coVerify { - arrangement.messageRepository.persistMessage(any(), any(), any()) + arrangement.messageRepository.persistMessage(any(), any()) }.wasInvoked(once) coVerify { @@ -114,22 +114,6 @@ class PersistMessageUseCaseTest { }.wasNotInvoked() } - @Test - fun givenMissedCallMessage_whenRunningUseCase_thenCallPersistMessageWithUpdateConversationReadDateIsFalse() = - runTest { - val (arrangement, persistMessage) = Arrangement() - .withPersistMessageSuccess() - .arrange() - val missedCallMessage = TestMessage.MISSED_CALL_MESSAGE - - val result = persistMessage.invoke(missedCallMessage) - - result.shouldSucceed() - coVerify { - arrangement.messageRepository.persistMessage(any(), eq(false), any()) - }.wasInvoked(once) - } - private class Arrangement { @Mock val messageRepository = mock(MessageRepository::class) @@ -145,13 +129,13 @@ class PersistMessageUseCaseTest { suspend fun withPersistMessageSuccess() = apply { coEvery { - messageRepository.persistMessage(any(), any(), any()) + messageRepository.persistMessage(any(), any()) }.returns(Either.Right(InsertMessageResult.INSERTED_NEED_TO_NOTIFY_USER)) } suspend fun withPersistMessageFailure() = apply { coEvery { - messageRepository.persistMessage(any(), any(), any()) + messageRepository.persistMessage(any(), any()) }.returns(Either.Left(CoreFailure.InvalidEventSenderID)) } diff --git a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/message/MessageDAO.kt b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/message/MessageDAO.kt index 870e2042528..e37edbceea0 100644 --- a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/message/MessageDAO.kt +++ b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/message/MessageDAO.kt @@ -50,7 +50,6 @@ interface MessageDAO { */ suspend fun insertOrIgnoreMessage( message: MessageEntity, - updateConversationReadDate: Boolean = false, updateConversationModifiedDate: Boolean = false ): InsertMessageResult diff --git a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/message/MessageDAOImpl.kt b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/message/MessageDAOImpl.kt index 46d8da0057e..7df0c58a1ae 100644 --- a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/message/MessageDAOImpl.kt +++ b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/message/MessageDAOImpl.kt @@ -89,17 +89,11 @@ internal class MessageDAOImpl internal constructor( override suspend fun insertOrIgnoreMessage( message: MessageEntity, - updateConversationReadDate: Boolean, updateConversationModifiedDate: Boolean ) = withContext(coroutineContext) { queries.transactionWithResult { val messageCreationInstant = message.date - if (updateConversationReadDate) { - conversationsQueries.updateConversationReadDate(messageCreationInstant, message.conversationId) - unreadEventsQueries.deleteUnreadEvents(message.date, message.conversationId) - } - insertInDB(message) val needsToBeNotified = nonSuspendNeedsToBeNotified(message.id, message.conversationId) diff --git a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/message/MessageTextEditTest.kt b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/message/MessageTextEditTest.kt index 9fa8b7fd8da..e2d5fb9a3e6 100644 --- a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/message/MessageTextEditTest.kt +++ b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/message/MessageTextEditTest.kt @@ -38,7 +38,6 @@ import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue -@OptIn(ExperimentalCoroutinesApi::class) class MessageTextEditTest : BaseMessageTest() { @Test @@ -247,7 +246,7 @@ class MessageTextEditTest : BaseMessageTest() { } @Test - fun givenTextWasInsertedAndIsRead_whenUpdatingContentWithSelfMention_thenUnreadEventShouldNotChange() = runTest { + fun givenTextWasInsertedAndIsNotRead_whenUpdatingContentWithSelfMention_thenUnreadEventShouldNotChange() = runTest { // Given val initMentions = listOf( MessageEntity.Mention(0, 1, SELF_USER_ID), @@ -255,8 +254,7 @@ class MessageTextEditTest : BaseMessageTest() { ) insertInitialDataWithMentions( - mentions = initMentions, - updateConversationReadDate = true + mentions = initMentions ) // When @@ -276,20 +274,19 @@ class MessageTextEditTest : BaseMessageTest() { val unreadEvents = messageDAO.observeUnreadEvents() .first()[CONVERSATION_ID] - assertTrue(unreadEvents.isNullOrEmpty()) + assertNotNull(unreadEvents) + assertEquals(1, unreadEvents.size) } private suspend fun insertInitialDataWithMentions( mentions: List, - updateConversationReadDate: Boolean = false ) { super.insertInitialData() messageDAO.insertOrIgnoreMessage( ORIGINAL_MESSAGE.copy( content = ORIGINAL_CONTENT.copy(mentions = mentions) - ), - updateConversationReadDate = updateConversationReadDate + ) ) }