From fa037c32f28d70df7f8576e272e3eaa2fb509d88 Mon Sep 17 00:00:00 2001 From: Gonzo Date: Tue, 10 Oct 2023 10:40:08 +0200 Subject: [PATCH] fix: muting conversations when archiving (#2123) --- ...UpdateConversationArchivedStatusUseCase.kt | 22 ++ ...teConversationArchivedStatusUseCaseTest.kt | 227 +++++++++++++----- 2 files changed, 191 insertions(+), 58 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/UpdateConversationArchivedStatusUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/UpdateConversationArchivedStatusUseCase.kt index 87aab1af348..2ccd979b3dc 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/UpdateConversationArchivedStatusUseCase.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/UpdateConversationArchivedStatusUseCase.kt @@ -19,6 +19,7 @@ package com.wire.kalium.logic.feature.conversation import com.wire.kalium.logic.data.conversation.ConversationRepository +import com.wire.kalium.logic.data.conversation.MutedConversationStatus import com.wire.kalium.logic.data.id.ConversationId import com.wire.kalium.logic.functional.flatMap import com.wire.kalium.logic.functional.fold @@ -63,6 +64,27 @@ internal class UpdateConversationArchivedStatusUseCaseImpl( }, { kaliumLogger.d("Successfully updated remotely and locally convId (${conversationId.toLogString()}) archiving " + "status to archived = ($shouldArchiveConversation)") + + // Now we should make sure the conversation gets muted if it's archived or un-muted if it's unarchived + val updatedMutedStatus = if (shouldArchiveConversation) { + MutedConversationStatus.AllMuted + } else { + MutedConversationStatus.AllAllowed + } + conversationRepository.updateMutedStatusRemotely(conversationId, updatedMutedStatus, archivedStatusTimestamp) + .flatMap { + conversationRepository.updateMutedStatusLocally(conversationId, updatedMutedStatus, archivedStatusTimestamp) + }.fold({ + kaliumLogger.e( + "Something went wrong when updating the muting status of the convId: " + + "(${conversationId.toLogString()}) to (${updatedMutedStatus.status}" + ) + }, { + kaliumLogger.d( + "Successfully updated remotely and locally the muting status of the convId: " + + "(${conversationId.toLogString()}) to (${updatedMutedStatus.status}") + }) + // Even if the muting status update fails, we should still return success as the archived status update was successful ArchiveStatusUpdateResult.Success }) } diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/conversation/UpdateConversationArchivedStatusUseCaseTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/conversation/UpdateConversationArchivedStatusUseCaseTest.kt index cffbb95f80f..c910116e34b 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/conversation/UpdateConversationArchivedStatusUseCaseTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/conversation/UpdateConversationArchivedStatusUseCaseTest.kt @@ -31,20 +31,11 @@ import io.mockative.mock import io.mockative.once import io.mockative.verify import kotlinx.coroutines.test.runTest -import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertTrue class UpdateConversationArchivedStatusUseCaseTest { - @Mock - private val conversationRepository: ConversationRepository = mock(ConversationRepository::class) - - private lateinit var updateConversationArchivedStatus: UpdateConversationArchivedStatusUseCase - - @BeforeTest - fun setup() { - updateConversationArchivedStatus = UpdateConversationArchivedStatusUseCaseImpl(conversationRepository) - } @Test fun givenAConversationId_whenInvokingAnArchivedStatusChange_thenShouldDelegateTheCallAndReturnASuccessResult() = runTest { @@ -52,28 +43,25 @@ class UpdateConversationArchivedStatusUseCaseTest { val isConversationArchived = true val archivedStatusTimestamp = 123456789L - given(conversationRepository) - .suspendFunction(conversationRepository::updateArchivedStatusRemotely) - .whenInvokedWith(any(), any(), any()) - .thenReturn(Either.Right(Unit)) - - given(conversationRepository) - .suspendFunction(conversationRepository::updateArchivedStatusLocally) - .whenInvokedWith(any(), any(), any()) - .thenReturn(Either.Right(Unit)) + val (arrangement, updateConversationArchivedStatus) = Arrangement() + .withSuccessfulMutingUpdates() + .withUpdateArchivedStatusFullSuccess() + .arrange() val result = updateConversationArchivedStatus(conversationId, isConversationArchived, archivedStatusTimestamp) assertEquals(ArchiveStatusUpdateResult.Success::class, result::class) - verify(conversationRepository) - .suspendFunction(conversationRepository::updateArchivedStatusRemotely) - .with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp }) - .wasInvoked(exactly = once) - - verify(conversationRepository) - .suspendFunction(conversationRepository::updateArchivedStatusLocally) - .with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp }) - .wasInvoked(exactly = once) + with(arrangement) { + verify(conversationRepository) + .suspendFunction(conversationRepository::updateArchivedStatusRemotely) + .with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp }) + .wasInvoked(exactly = once) + + verify(conversationRepository) + .suspendFunction(conversationRepository::updateArchivedStatusLocally) + .with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp }) + .wasInvoked(exactly = once) + } } @Test @@ -82,24 +70,25 @@ class UpdateConversationArchivedStatusUseCaseTest { val isConversationArchived = true val archivedStatusTimestamp = 123456789L - given(conversationRepository) - .suspendFunction(conversationRepository::updateArchivedStatusRemotely) - .whenInvokedWith(any(), any(), any()) - .thenReturn(Either.Left(NetworkFailure.ServerMiscommunication(RuntimeException("some error")))) + val (arrangement, updateConversationArchivedStatus) = Arrangement() + .withSuccessfulMutingUpdates() + .withRemoteUpdateArchivedStatusFailure() + .arrange() val result = updateConversationArchivedStatus(conversationId, isConversationArchived, archivedStatusTimestamp) assertEquals(ArchiveStatusUpdateResult.Failure::class, result::class) - verify(conversationRepository) - .suspendFunction(conversationRepository::updateArchivedStatusRemotely) - .with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp }) - .wasInvoked(exactly = once) - - verify(conversationRepository) - .suspendFunction(conversationRepository::updateArchivedStatusLocally) - .with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp }) - .wasNotInvoked() - + with(arrangement) { + verify(conversationRepository) + .suspendFunction(conversationRepository::updateArchivedStatusRemotely) + .with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp }) + .wasInvoked(exactly = once) + + verify(conversationRepository) + .suspendFunction(conversationRepository::updateArchivedStatusLocally) + .with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp }) + .wasNotInvoked() + } } @Test @@ -108,26 +97,148 @@ class UpdateConversationArchivedStatusUseCaseTest { val isConversationArchived = true val archivedStatusTimestamp = 123456789L - given(conversationRepository) - .suspendFunction(conversationRepository::updateArchivedStatusRemotely) - .whenInvokedWith(any(), any(), any()) - .thenReturn(Either.Right(Unit)) - given(conversationRepository) - .suspendFunction(conversationRepository::updateArchivedStatusLocally) - .whenInvokedWith(any(), any(), any()) - .thenReturn(Either.Left(StorageFailure.DataNotFound)) + val (arrangement, updateConversationArchivedStatus) = Arrangement() + .withLocalUpdateArchivedStatusFailure() + .withSuccessfulMutingUpdates() + .arrange() val result = updateConversationArchivedStatus(conversationId, isConversationArchived, archivedStatusTimestamp) assertEquals(ArchiveStatusUpdateResult.Failure::class, result::class) - verify(conversationRepository) - .suspendFunction(conversationRepository::updateArchivedStatusRemotely) - .with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp }) - .wasInvoked(exactly = once) + with(arrangement) { + verify(conversationRepository) + .suspendFunction(conversationRepository::updateArchivedStatusRemotely) + .with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp }) + .wasInvoked(exactly = once) + + verify(conversationRepository) + .suspendFunction(conversationRepository::updateArchivedStatusLocally) + .with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp }) + .wasInvoked(exactly = once) + } + } - verify(conversationRepository) - .suspendFunction(conversationRepository::updateArchivedStatusLocally) - .with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp }) - .wasInvoked(exactly = once) + @Test + fun givenAConversationId_whenInvokingAnArchivedStatusChangeAndFailsUpdatingRemoteMutingStatus_thenItShouldStillReturnASuccessResult() = + runTest { + val conversationId = TestConversation.ID + val isConversationArchived = true + val archivedStatusTimestamp = 123456789L + + val (arrangement, updateConversationArchivedStatus) = Arrangement() + .withUpdateArchivedStatusFullSuccess() + .withRemoteErrorMutingUpdates() + .arrange() + + val result = updateConversationArchivedStatus(conversationId, isConversationArchived, archivedStatusTimestamp) + assertTrue(result is ArchiveStatusUpdateResult.Success) + + with(arrangement) { + verify(conversationRepository) + .suspendFunction(conversationRepository::updateArchivedStatusRemotely) + .with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp }) + .wasInvoked(exactly = once) + + verify(conversationRepository) + .suspendFunction(conversationRepository::updateArchivedStatusLocally) + .with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp }) + .wasInvoked(exactly = once) + } + } + + @Test + fun givenAConversationId_whenInvokingAnArchivedStatusChangeAndFailsUpdatingLocalMutingStatus_thenItShouldStillReturnASuccessResult() = + runTest { + val conversationId = TestConversation.ID + val isConversationArchived = true + val archivedStatusTimestamp = 123456789L + + val (arrangement, updateConversationArchivedStatus) = Arrangement() + .withUpdateArchivedStatusFullSuccess() + .withLocalErrorMutingUpdates() + .arrange() + + val result = updateConversationArchivedStatus(conversationId, isConversationArchived, archivedStatusTimestamp) + assertTrue(result is ArchiveStatusUpdateResult.Success) + + with(arrangement) { + verify(conversationRepository) + .suspendFunction(conversationRepository::updateArchivedStatusRemotely) + .with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp }) + .wasInvoked(exactly = once) + + verify(conversationRepository) + .suspendFunction(conversationRepository::updateArchivedStatusLocally) + .with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp }) + .wasInvoked(exactly = once) + } + } + + private class Arrangement { + @Mock + val conversationRepository: ConversationRepository = mock(ConversationRepository::class) + + private val updateArchivedStatus = UpdateConversationArchivedStatusUseCaseImpl(conversationRepository) + + fun withUpdateArchivedStatusFullSuccess() = apply { + given(conversationRepository) + .suspendFunction(conversationRepository::updateArchivedStatusRemotely) + .whenInvokedWith(any(), any(), any()) + .thenReturn(Either.Right(Unit)) + + given(conversationRepository) + .suspendFunction(conversationRepository::updateArchivedStatusLocally) + .whenInvokedWith(any(), any(), any()) + .thenReturn(Either.Right(Unit)) + } + + fun withRemoteUpdateArchivedStatusFailure() = apply { + given(conversationRepository) + .suspendFunction(conversationRepository::updateArchivedStatusRemotely) + .whenInvokedWith(any(), any(), any()) + .thenReturn(Either.Left(NetworkFailure.ServerMiscommunication(RuntimeException("some error")))) + } + + fun withLocalUpdateArchivedStatusFailure() = apply { + given(conversationRepository) + .suspendFunction(conversationRepository::updateArchivedStatusRemotely) + .whenInvokedWith(any(), any(), any()) + .thenReturn(Either.Right(Unit)) + given(conversationRepository) + .suspendFunction(conversationRepository::updateArchivedStatusLocally) + .whenInvokedWith(any(), any(), any()) + .thenReturn(Either.Left(StorageFailure.DataNotFound)) + } + + fun withSuccessfulMutingUpdates() = apply { + given(conversationRepository) + .suspendFunction(conversationRepository::updateMutedStatusRemotely) + .whenInvokedWith(any(), any(), any()) + .thenReturn(Either.Right(Unit)) + given(conversationRepository) + .suspendFunction(conversationRepository::updateMutedStatusLocally) + .whenInvokedWith(any(), any(), any()) + .thenReturn(Either.Right(Unit)) + } + + fun withRemoteErrorMutingUpdates() = apply { + given(conversationRepository) + .suspendFunction(conversationRepository::updateMutedStatusRemotely) + .whenInvokedWith(any(), any(), any()) + .thenReturn(Either.Left(NetworkFailure.NoNetworkConnection(RuntimeException("some error")))) + } + + fun withLocalErrorMutingUpdates() = apply { + given(conversationRepository) + .suspendFunction(conversationRepository::updateMutedStatusRemotely) + .whenInvokedWith(any(), any(), any()) + .thenReturn(Either.Right(Unit)) + given(conversationRepository) + .suspendFunction(conversationRepository::updateMutedStatusLocally) + .whenInvokedWith(any(), any(), any()) + .thenReturn(Either.Left(StorageFailure.DataNotFound)) + } + + fun arrange() = this to updateArchivedStatus } }