Skip to content

Commit

Permalink
feat: add flag to filter out archived conversations when fetching lis…
Browse files Browse the repository at this point in the history 
…t [WPB-4432] (#2082)

* feat: Add flag to decide whether to fetch archived conversations

* feat: added logs

* fix: fixed tests

* fix: added test with archived conversations

* fix: renamed base flag

* fix: renamed more vars

* fix: added tests

* fix: moved filter of archived conversations to DAO layer

* simplified logic
  • Loading branch information
@gongracr
gongracr authored Sep 26, 2023
1 parent 00d151f commit 5628a66
Show file tree
Hide file tree
Showing 11 changed files with 181 additions and 44 deletions.
2 changes: 1 addition & 1 deletion cli/src/appleMain/kotlin/com/wire/kalium/cli/commands/Command.kt
View file
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
6 changes: 3 additions & 3 deletions ...c/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationRepository.kt
View file
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
6 changes: 3 additions & 3 deletions ...n/com/wire/kalium/logic/feature/conversation/ObserveConversationListDetailsUseCaseImpl.kt
View file
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)
}
32 changes: 19 additions & 13 deletions ...lin/com/wire/kalium/logic/feature/conversation/UpdateConversationArchivedStatusUseCase.kt
View file
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
8 changes: 5 additions & 3 deletions ...c/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationRepositoryTest.kt
View file
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
71 changes: 64 additions & 7 deletions ...n/com/wire/kalium/logic/feature/conversation/ObserveConversationListDetailsUseCaseTest.kt
View file
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
38 changes: 34 additions & 4 deletions ...com/wire/kalium/logic/feature/conversation/UpdateConversationArchivedStatusUseCaseTest.kt
View file
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

0 comments on commit 5628a66

Please sign in to comment.