Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add flag to filter out archived conversations when fetching list [WPB-4432] #2082

Merged
merged 12 commits into from
Sep 26, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ sealed class Command(
}

suspend fun prepare() {
conversations = userSession.conversations.observeConversationListDetails().first()
conversations = userSession.conversations.observeConversationListDetails(includeArchived = true).first()
updateFilter()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ interface ConversationRepository {

suspend fun getConversationList(): Either<StorageFailure, Flow<List<Conversation>>>
suspend fun observeConversationList(): Flow<List<Conversation>>
suspend fun observeConversationListDetails(): Flow<List<ConversationDetails>>
suspend fun observeConversationListDetails(includeArchived: Boolean): Flow<List<ConversationDetails>>
suspend fun observeConversationDetailsById(conversationID: ConversationId): Flow<Either<StorageFailure, ConversationDetails>>
suspend fun fetchConversation(conversationID: ConversationId): Either<CoreFailure, Unit>
suspend fun fetchSentConnectionConversation(conversationID: ConversationId): Either<CoreFailure, Unit>
Expand Down Expand Up @@ -400,9 +400,9 @@ internal class ConversationDataSource internal constructor(
return conversationDAO.getAllConversations().map { it.map(conversationMapper::fromDaoModel) }
}

override suspend fun observeConversationListDetails(): Flow<List<ConversationDetails>> =
override suspend fun observeConversationListDetails(includeArchived: Boolean): Flow<List<ConversationDetails>> =
combine(
conversationDAO.getAllConversationDetails(),
conversationDAO.getAllConversationDetails(includeArchived),
messageDAO.observeLastMessages(),
messageDAO.observeConversationsUnreadEvents(),
) { conversationList, lastMessageList, unreadEvents ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ import kotlinx.coroutines.flow.Flow
* @see ConversationDetails
*/
fun interface ObserveConversationListDetailsUseCase {
suspend operator fun invoke(): Flow<List<ConversationDetails>>
suspend operator fun invoke(includeArchived: Boolean): Flow<List<ConversationDetails>>
}

internal class ObserveConversationListDetailsUseCaseImpl(
private val conversationRepository: ConversationRepository,
) : ObserveConversationListDetailsUseCase {

override suspend operator fun invoke(): Flow<List<ConversationDetails>> =
conversationRepository.observeConversationListDetails()
override suspend operator fun invoke(includeArchived: Boolean): Flow<List<ConversationDetails>> =
conversationRepository.observeConversationListDetails(includeArchived)
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.wire.kalium.logic.data.conversation.ConversationRepository
import com.wire.kalium.logic.data.id.ConversationId
import com.wire.kalium.logic.functional.flatMap
import com.wire.kalium.logic.functional.fold
import com.wire.kalium.logic.functional.onFailure
import com.wire.kalium.logic.kaliumLogger
import com.wire.kalium.util.DateTimeUtil

Expand All @@ -30,12 +31,13 @@ interface UpdateConversationArchivedStatusUseCase {
* Use case that allows a conversation to mark a conversation as archived or not.
*
* @param conversationId the id of the conversation where status wants to be changed
* @param isConversationArchived new archived status to be updated on the given conversation
* @param shouldArchiveConversation new archived status to be updated on the given conversation
* @param archivedStatusTimestamp the timestamp when the archiving event occurred
* @return an [ConversationUpdateStatusResult] containing Success or Failure cases
*/
suspend operator fun invoke(
conversationId: ConversationId,
isConversationArchived: Boolean,
shouldArchiveConversation: Boolean,
archivedStatusTimestamp: Long = DateTimeUtil.currentInstant().toEpochMilliseconds()
): ArchiveStatusUpdateResult
}
Expand All @@ -46,19 +48,23 @@ internal class UpdateConversationArchivedStatusUseCaseImpl(

override suspend operator fun invoke(
conversationId: ConversationId,
isConversationArchived: Boolean,
shouldArchiveConversation: Boolean,
archivedStatusTimestamp: Long
): ArchiveStatusUpdateResult =
conversationRepository.updateArchivedStatusRemotely(conversationId, isConversationArchived, archivedStatusTimestamp)
.flatMap {
conversationRepository.updateArchivedStatusLocally(conversationId, isConversationArchived, archivedStatusTimestamp)
}.fold({
kaliumLogger.e("Something went wrong when updating convId (${conversationId.toLogString()}) to ($isConversationArchived")
ArchiveStatusUpdateResult.Failure
}, {
ArchiveStatusUpdateResult.Success
})

conversationRepository.updateArchivedStatusRemotely(conversationId, shouldArchiveConversation, archivedStatusTimestamp).onFailure {
kaliumLogger.e("Something went wrong when updating remotely convId (${conversationId.toLogString()}) archiving " +
"status to archived = ($shouldArchiveConversation)")
}.flatMap {
conversationRepository.updateArchivedStatusLocally(conversationId, shouldArchiveConversation, archivedStatusTimestamp)
}.fold({
kaliumLogger.e("Something went wrong when updating locally convId (${conversationId.toLogString()}) archiving " +
"status to archived = ($shouldArchiveConversation)")
ArchiveStatusUpdateResult.Failure
}, {
kaliumLogger.d("Successfully updated remotely and locally convId (${conversationId.toLogString()}) archiving " +
"status to archived = ($shouldArchiveConversation)")
ArchiveStatusUpdateResult.Success
})
}

sealed class ArchiveStatusUpdateResult {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,7 @@ class ConversationRepositoryTest {
// given
val conversationIdEntity = ConversationIDEntity("some_value", "some_domain")
val conversationId = QualifiedID("some_value", "some_domain")
val shouldFetchArchivedConversations = false

val conversationEntity = TestConversation.VIEW_ENTITY.copy(
id = conversationIdEntity,
Expand All @@ -686,7 +687,7 @@ class ConversationRepositoryTest {
.arrange()

// when
conversationRepository.observeConversationListDetails().test {
conversationRepository.observeConversationListDetails(shouldFetchArchivedConversations).test {
val result = awaitItem()

assertContains(result.map { it.conversation.id }, conversationId)
Expand Down Expand Up @@ -751,6 +752,7 @@ class ConversationRepositoryTest {
// given
val conversationIdEntity = ConversationIDEntity("some_value", "some_domain")
val conversationId = QualifiedID("some_value", "some_domain")
val shouldFetchArchivedConversations = false

val conversationEntity = TestConversation.VIEW_ENTITY.copy(
id = conversationIdEntity, type = ConversationEntity.Type.ONE_ON_ONE,
Expand All @@ -770,7 +772,7 @@ class ConversationRepositoryTest {
.arrange()

// when
conversationRepository.observeConversationListDetails().test {
conversationRepository.observeConversationListDetails(shouldFetchArchivedConversations).test {
val result = awaitItem()

assertContains(result.map { it.conversation.id }, conversationId)
Expand Down Expand Up @@ -1230,7 +1232,7 @@ class ConversationRepositoryTest {
fun withConversations(conversations: List<ConversationViewEntity>) = apply {
given(conversationDAO)
.suspendFunction(conversationDAO::getAllConversationDetails)
.whenInvoked()
.whenInvokedWith(any())
.thenReturn(flowOf(conversations))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ import kotlin.test.Ignore
import kotlin.test.Test
import kotlin.test.assertContentEquals
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertTrue

@Suppress("LongMethod")
@Ignore // TODO
Expand All @@ -59,6 +61,7 @@ class ObserveConversationListDetailsUseCaseTest {
val selfConversation = TestConversation.SELF()
val conversations = listOf(selfConversation, groupConversation)
val selfConversationDetails = ConversationDetails.Self(selfConversation)
val fetchArchivedConversations = false
val groupConversationDetails =
ConversationDetails.Group(
groupConversation,
Expand All @@ -77,7 +80,7 @@ class ObserveConversationListDetailsUseCaseTest {
.arrange()

// When
observeConversationsUseCase().collect()
observeConversationsUseCase(fetchArchivedConversations).collect()

// Then
with(arrangement) {
Expand All @@ -86,13 +89,62 @@ class ObserveConversationListDetailsUseCaseTest {
.wasInvoked(exactly = once)
}
}
@Test
fun givenSomeConversationsWithArchivedValues_whenFetchingOnlyNonArchived_thenTheseConversationsShouldNotBeReturned() = runTest {
// Given
val groupConversation1 = TestConversation.GROUP().copy(archived = true)
val group2Id = ConversationId("OtherId","someDomain")
val groupConversation2 = TestConversation.GROUP().copy(id = group2Id, archived = false)
val selfConversation = TestConversation.SELF()
val conversations = listOf(selfConversation, groupConversation1, groupConversation2)
val selfConversationDetails = ConversationDetails.Self(selfConversation)
val fetchArchivedConversations = false
val groupConversationDetails1 =
ConversationDetails.Group(
groupConversation1,
LegalHoldStatus.DISABLED,
lastMessage = null,
isSelfUserMember = true,
isSelfUserCreator = true,
unreadEventCount = emptyMap(),
selfRole = Conversation.Member.Role.Member
)
val groupConversationDetails2 =
ConversationDetails.Group(
groupConversation2,
LegalHoldStatus.DISABLED,
lastMessage = null,
isSelfUserMember = true,
isSelfUserCreator = true,
unreadEventCount = emptyMap(),
selfRole = Conversation.Member.Role.Member
)

val (arrangement, observeConversationsUseCase) = Arrangement()
.withConversationsList(conversations)
.withSuccessfulConversationsDetailsListUpdates(groupConversation1, listOf(groupConversationDetails1))
.withSuccessfulConversationsDetailsListUpdates(groupConversation2, listOf(groupConversationDetails2))
.withSuccessfulConversationsDetailsListUpdates(selfConversation, listOf(selfConversationDetails))
.arrange()

// When
observeConversationsUseCase(fetchArchivedConversations).test {
val conversationDetailsList = awaitItem()

// Then
assertFalse(conversationDetailsList.contains(groupConversationDetails2))
assertTrue(conversationDetailsList.contains(selfConversationDetails))
assertTrue(conversationDetailsList.contains(groupConversationDetails1))
}
}

@Test
fun givenSomeConversations_whenObservingDetailsList_thenObserveConversationDetailsShouldBeCalledForEachID() = runTest {
// Given
val selfConversation = TestConversation.SELF()
val groupConversation = TestConversation.GROUP()
val conversations = listOf(selfConversation, groupConversation)
val fetchArchivedConversations = false

val selfConversationDetails = ConversationDetails.Self(selfConversation)
val groupConversationDetails = ConversationDetails.Group(
Expand All @@ -112,7 +164,7 @@ class ObserveConversationListDetailsUseCaseTest {
.arrange()

// When
observeConversationsUseCase().collect()
observeConversationsUseCase(fetchArchivedConversations).collect()

with(arrangement) {
conversations.forEach { conversation ->
Expand All @@ -130,6 +182,7 @@ class ObserveConversationListDetailsUseCaseTest {
val oneOnOneConversation = TestConversation.ONE_ON_ONE
val groupConversation = TestConversation.GROUP()
val conversations = listOf(groupConversation, oneOnOneConversation)
val fetchArchivedConversations = false

val groupConversationUpdates = listOf(
ConversationDetails.Group(
Expand Down Expand Up @@ -169,7 +222,7 @@ class ObserveConversationListDetailsUseCaseTest {
.arrange()

// When, Then
observeConversationsUseCase().test {
observeConversationsUseCase(fetchArchivedConversations).test {
oneOnOneDetailsChannel.send(firstOneOnOneDetails)

val conversationList = awaitItem()
Expand All @@ -188,6 +241,7 @@ class ObserveConversationListDetailsUseCaseTest {
fun givenAConversationIsAddedToTheList_whenObservingDetailsList_thenTheUpdateIsPropagatedThroughTheFlow() = runTest {
// Given
val groupConversation = TestConversation.GROUP()
val fetchArchivedConversations = false
val groupConversationDetails = ConversationDetails.Group(
groupConversation,
LegalHoldStatus.DISABLED,
Expand All @@ -212,7 +266,7 @@ class ObserveConversationListDetailsUseCaseTest {
.arrange()

// When, Then
observeConversationsUseCase().test {
observeConversationsUseCase(fetchArchivedConversations).test {
assertContentEquals(listOf(groupConversationDetails), awaitItem())

conversationListUpdates.close()
Expand All @@ -225,6 +279,7 @@ class ObserveConversationListDetailsUseCaseTest {
fun givenAnOngoingCall_whenFetchingConversationDetails_thenTheConversationShouldHaveAnOngoingCall() = runTest {
// Given
val groupConversation = TestConversation.GROUP()
val fetchArchivedConversations = false
val groupConversationDetails = ConversationDetails.Group(
groupConversation,
LegalHoldStatus.DISABLED,
Expand All @@ -246,7 +301,7 @@ class ObserveConversationListDetailsUseCaseTest {
.arrange()

// When, Then
observeConversationsUseCase().test {
observeConversationsUseCase(fetchArchivedConversations).test {
assertEquals(true, (awaitItem()[0] as ConversationDetails.Group).hasOngoingCall)
}
}
Expand All @@ -255,6 +310,7 @@ class ObserveConversationListDetailsUseCaseTest {
fun givenAConversationWithoutAnOngoingCall_whenFetchingConversationDetails_thenTheConversationShouldNotHaveAnOngoingCall() = runTest {
// Given
val groupConversation = TestConversation.GROUP()
val fetchArchivedConversations = false

val groupConversationDetails = ConversationDetails.Group(
groupConversation,
Expand All @@ -277,7 +333,7 @@ class ObserveConversationListDetailsUseCaseTest {
.arrange()

// When, Then
observeConversationsUseCase().test {
observeConversationsUseCase(fetchArchivedConversations).test {
assertEquals(false, (awaitItem()[0] as ConversationDetails.Group).hasOngoingCall)
}
}
Expand All @@ -289,6 +345,7 @@ class ObserveConversationListDetailsUseCaseTest {
val successConversation = TestConversation.ONE_ON_ONE.copy(id = ConversationId("successId", "domain"))
val successConversationDetails = TestConversationDetails.CONVERSATION_ONE_ONE.copy(conversation = successConversation)
val failureConversation = TestConversation.ONE_ON_ONE.copy(id = ConversationId("failedId", "domain"))
val fetchArchivedConversations = false

val (_, observeConversationsUseCase) = Arrangement()
.withConversationsList(listOf(successConversation, failureConversation))
Expand All @@ -297,7 +354,7 @@ class ObserveConversationListDetailsUseCaseTest {
.arrange()

// When, Then
observeConversationsUseCase().test {
observeConversationsUseCase(fetchArchivedConversations).test {
assertEquals(awaitItem(), listOf(successConversationDetails))
awaitComplete()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package com.wire.kalium.logic.feature.conversation

import com.wire.kalium.logic.NetworkFailure
import com.wire.kalium.logic.StorageFailure
import com.wire.kalium.logic.data.conversation.ConversationRepository
import com.wire.kalium.logic.framework.TestConversation
import com.wire.kalium.logic.functional.Either
Expand Down Expand Up @@ -66,12 +67,12 @@ class UpdateConversationArchivedStatusUseCaseTest {

verify(conversationRepository)
.suspendFunction(conversationRepository::updateArchivedStatusRemotely)
.with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp})
.with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp })
.wasInvoked(exactly = once)

verify(conversationRepository)
.suspendFunction(conversationRepository::updateArchivedStatusLocally)
.with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp})
.with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp })
.wasInvoked(exactly = once)
}

Expand All @@ -91,13 +92,42 @@ class UpdateConversationArchivedStatusUseCaseTest {

verify(conversationRepository)
.suspendFunction(conversationRepository::updateArchivedStatusRemotely)
.with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp})
.with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp })
.wasInvoked(exactly = once)

verify(conversationRepository)
.suspendFunction(conversationRepository::updateArchivedStatusLocally)
.with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp})
.with(any(), eq(isConversationArchived), matching { it == archivedStatusTimestamp })
.wasNotInvoked()

}

@Test
fun givenAConversationId_whenInvokingAnArchivedStatusChangeAndFailsOnlyLocally_thenShouldReturnAFailureResult() = runTest {
val conversationId = TestConversation.ID
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 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 })
.wasInvoked(exactly = once)
}
}
Loading
Loading