Skip to content

Commit

Permalink
fix: reusing PagingData crash [WPB-15055][WPB-15079][WPB-15064] 🍒 (#3778
Browse files Browse the repository at this point in the history 
)

Co-authored-by: Yamil Medina <[email protected]>
  • Loading branch information
@saleniuk @yamilmedina
saleniuk and yamilmedina authored Jan 9, 2025
1 parent b711329 commit f21c93e
Show file tree
Hide file tree
Showing 7 changed files with 200 additions and 91 deletions.
22 changes: 16 additions & 6 deletions app/src/main/kotlin/com/wire/android/ui/common/topappbar/search/SearchBarState.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,12 @@ import androidx.compose.runtime.setValue

@Composable
fun rememberSearchbarState(
initialIsSearchActive: Boolean = false,
searchQueryTextState: TextFieldState = rememberTextFieldState()
): SearchBarState = rememberSaveable(
saver = SearchBarState.saver(searchQueryTextState)
saver = SearchBarState.saver()
) {
SearchBarState(searchQueryTextState = searchQueryTextState)
SearchBarState(isSearchActive = initialIsSearchActive, searchQueryTextState = searchQueryTextState)
}

class SearchBarState(
Expand All @@ -57,14 +58,23 @@ class SearchBarState(
}

companion object {
fun saver(searchQueryTextState: TextFieldState): Saver<SearchBarState, *> = Saver(
fun saver(): Saver<SearchBarState, *> = Saver(
save = {
listOf(it.isSearchActive)
listOf(
it.isSearchActive,
with(TextFieldState.Saver) {
save(it.searchQueryTextState)
}
)
},
restore = {
SearchBarState(
isSearchActive = it[0],
searchQueryTextState = searchQueryTextState
isSearchActive = (it.getOrNull(0) as? Boolean) ?: false,
searchQueryTextState = it.getOrNull(1)?.let {
with(TextFieldState.Saver) {
restore(it)
}
} ?: TextFieldState()
)
}
)
Expand Down
9 changes: 4 additions & 5 deletions app/src/main/kotlin/com/wire/android/ui/home/archive/ArchiveScreen.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import com.wire.android.ui.home.conversationslist.common.previewConversationFold
import com.wire.android.ui.home.conversationslist.model.ConversationsSource
import com.wire.android.ui.theme.WireTheme
import com.wire.android.util.ui.PreviewMultipleThemes
import kotlinx.coroutines.flow.flowOf

@HomeNavGraph
@WireDestination
Expand All @@ -57,7 +56,7 @@ fun PreviewArchiveEmptyScreen() = WireTheme {
searchBarState = rememberSearchbarState(),
conversationsSource = ConversationsSource.ARCHIVE,
emptyListContent = { ArchiveEmptyContent() },
conversationListViewModel = ConversationListViewModelPreview(flowOf()),
conversationListViewModel = ConversationListViewModelPreview(previewConversationFoldersFlow(list = listOf())),
)
}

Expand All @@ -66,10 +65,10 @@ fun PreviewArchiveEmptyScreen() = WireTheme {
fun PreviewArchiveEmptySearchScreen() = WireTheme {
ConversationsScreenContent(
navigator = rememberNavigator {},
searchBarState = rememberSearchbarState(searchQueryTextState = TextFieldState(initialText = "er")),
searchBarState = rememberSearchbarState(initialIsSearchActive = true, searchQueryTextState = TextFieldState(initialText = "er")),
conversationsSource = ConversationsSource.ARCHIVE,
emptyListContent = { ArchiveEmptyContent() },
conversationListViewModel = ConversationListViewModelPreview(flowOf()),
conversationListViewModel = ConversationListViewModelPreview(previewConversationFoldersFlow(searchQuery = "er", list = listOf())),
)
}

Expand All @@ -78,7 +77,7 @@ fun PreviewArchiveEmptySearchScreen() = WireTheme {
fun PreviewArchiveScreen() = WireTheme {
ConversationsScreenContent(
navigator = rememberNavigator {},
searchBarState = rememberSearchbarState(searchQueryTextState = TextFieldState(initialText = "er")),
searchBarState = rememberSearchbarState(initialIsSearchActive = true, searchQueryTextState = TextFieldState(initialText = "er")),
conversationsSource = ConversationsSource.ARCHIVE,
emptyListContent = { ArchiveEmptyContent() },
conversationListViewModel = ConversationListViewModelPreview(previewConversationFoldersFlow(searchQuery = "er")),
Expand Down
150 changes: 83 additions & 67 deletions app/src/main/kotlin/com/wire/android/ui/home/conversationslist/ConversationListViewModel.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ class ConversationListViewModelPreview(
class ConversationListViewModelImpl @AssistedInject constructor(
@Assisted val conversationsSource: ConversationsSource,
@Assisted private val usePagination: Boolean = BuildConfig.PAGINATED_CONVERSATION_LIST_ENABLED,
dispatcher: DispatcherProvider,
private val dispatcher: DispatcherProvider,
private val updateConversationMutedStatus: UpdateConversationMutedStatusUseCase,
private val getConversationsPaginated: GetConversationsFromSearchUseCase,
private val observeConversationListDetailsWithEvents: ObserveConversationListDetailsWithEventsUseCase,
Expand Down Expand Up @@ -171,6 +171,7 @@ class ConversationListViewModelImpl @AssistedInject constructor(
override val closeBottomSheet = MutableSharedFlow<Unit>()

private val searchQueryFlow: MutableStateFlow<String> = MutableStateFlow("")
private val isSelfUserUnderLegalHoldFlow = MutableSharedFlow<Boolean>(replay = 1)

private val containsNewActivitiesSection = when (conversationsSource) {
ConversationsSource.MAIN,
Expand All @@ -185,39 +186,38 @@ class ConversationListViewModelImpl @AssistedInject constructor(
private val conversationsPaginatedFlow: Flow<PagingData<ConversationFolderItem>> = searchQueryFlow
.debounce { if (it.isEmpty()) 0L else DEFAULT_SEARCH_QUERY_DEBOUNCE }
.onStart { emit("") }
.combine(isSelfUserUnderLegalHoldFlow, ::Pair)
.distinctUntilChanged()
.flatMapLatest { searchQuery ->
.flatMapLatest { (searchQuery, isSelfUserUnderLegalHold) ->
getConversationsPaginated(
searchQuery = searchQuery,
fromArchive = conversationsSource == ConversationsSource.ARCHIVE,
conversationFilter = conversationsSource.toFilter(),
onlyInteractionEnabled = false,
newActivitiesOnTop = containsNewActivitiesSection,
).combine(observeLegalHoldStateForSelfUser()) { conversations, selfUserLegalHoldStatus ->
conversations.map {
it.hideIndicatorForSelfUserUnderLegalHold(selfUserLegalHoldStatus)
}
}.map {
it.insertSeparators { before, after ->
when {
// do not add separators if the list shouldn't show conversations grouped into different folders
!containsNewActivitiesSection -> null

before == null && after != null && after.hasNewActivitiesToShow ->
// list starts with items with "new activities"
ConversationFolder.Predefined.NewActivities

before == null && after != null && !after.hasNewActivitiesToShow ->
// list doesn't contain any items with "new activities"
ConversationFolder.Predefined.Conversations

before != null && before.hasNewActivitiesToShow && after != null && !after.hasNewActivitiesToShow ->
// end of "new activities" section and beginning of "conversations" section
ConversationFolder.Predefined.Conversations

else -> null
).map { pagingData ->
pagingData
.map { it.hideIndicatorForSelfUserUnderLegalHold(isSelfUserUnderLegalHold) }
.insertSeparators { before, after ->
when {
// do not add separators if the list shouldn't show conversations grouped into different folders
!containsNewActivitiesSection -> null

before == null && after != null && after.hasNewActivitiesToShow ->
// list starts with items with "new activities"
ConversationFolder.Predefined.NewActivities

before == null && after != null && !after.hasNewActivitiesToShow ->
// list doesn't contain any items with "new activities"
ConversationFolder.Predefined.Conversations

before != null && before.hasNewActivitiesToShow && after != null && !after.hasNewActivitiesToShow ->
// end of "new activities" section and beginning of "conversations" section
ConversationFolder.Predefined.Conversations

else -> null
}
}
}
}
}
.flowOn(dispatcher.io())
Expand All @@ -232,45 +232,59 @@ class ConversationListViewModelImpl @AssistedInject constructor(
private set

init {
observeSelfUserLegalHoldState()
if (!usePagination) {
viewModelScope.launch {
searchQueryFlow
.debounce { if (it.isEmpty()) 0L else DEFAULT_SEARCH_QUERY_DEBOUNCE }
.onStart { emit("") }
.distinctUntilChanged()
.flatMapLatest { searchQuery: String ->
observeConversationListDetailsWithEvents(
fromArchive = conversationsSource == ConversationsSource.ARCHIVE,
conversationFilter = conversationsSource.toFilter()
).combine(observeLegalHoldStateForSelfUser()) { conversations, selfUserLegalHoldStatus ->
conversations.map { conversationDetails ->
conversationDetails.toConversationItem(
userTypeMapper = userTypeMapper,
searchQuery = searchQuery,
selfUserTeamId = observeSelfUser().firstOrNull()?.teamId
).hideIndicatorForSelfUserUnderLegalHold(selfUserLegalHoldStatus)
} to searchQuery
}
}
.map { (conversationItems, searchQuery) ->
if (searchQuery.isEmpty()) {
conversationItems.withFolders(source = conversationsSource).toImmutableMap()
} else {
searchConversation(
conversationDetails = conversationItems,
searchQuery = searchQuery
).withFolders(source = conversationsSource).toImmutableMap()
}
observeNonPaginatedSearchConversationList()
}
}

private fun observeSelfUserLegalHoldState() {
viewModelScope.launch {
observeLegalHoldStateForSelfUser()
.map { it is LegalHoldStateForSelfUser.Enabled }
.flowOn(dispatcher.io())
.collect { isSelfUserUnderLegalHoldFlow.emit(it) }
}
}

private fun observeNonPaginatedSearchConversationList() {
viewModelScope.launch {
searchQueryFlow
.debounce { if (it.isEmpty()) 0L else DEFAULT_SEARCH_QUERY_DEBOUNCE }
.onStart { emit("") }
.distinctUntilChanged()
.flatMapLatest { searchQuery: String ->
observeConversationListDetailsWithEvents(
fromArchive = conversationsSource == ConversationsSource.ARCHIVE,
conversationFilter = conversationsSource.toFilter()
).combine(isSelfUserUnderLegalHoldFlow) { conversations, isSelfUserUnderLegalHold ->
conversations.map { conversationDetails ->
conversationDetails.toConversationItem(
userTypeMapper = userTypeMapper,
searchQuery = searchQuery,
selfUserTeamId = observeSelfUser().firstOrNull()?.teamId
).hideIndicatorForSelfUserUnderLegalHold(isSelfUserUnderLegalHold)
} to searchQuery
}
.flowOn(dispatcher.io())
.collect {
conversationListState = ConversationListState.NotPaginated(
isLoading = false,
conversations = it,
domain = currentAccount.domain
)
}
.map { (conversationItems, searchQuery) ->
if (searchQuery.isEmpty()) {
conversationItems.withFolders(source = conversationsSource).toImmutableMap()
} else {
searchConversation(
conversationDetails = conversationItems,
searchQuery = searchQuery
).withFolders(source = conversationsSource).toImmutableMap()
}
}
}
.flowOn(dispatcher.io())
.collect {
conversationListState = ConversationListState.NotPaginated(
isLoading = false,
conversations = it,
domain = currentAccount.domain
)
}
}
}

Expand Down Expand Up @@ -485,11 +499,13 @@ private fun ConversationsSource.toFilter(): ConversationFilter = when (this) {
is ConversationsSource.FOLDER -> ConversationFilter.Folder(folderId = folderId, folderName = folderName)
}

private fun ConversationItem.hideIndicatorForSelfUserUnderLegalHold(selfUserLegalHoldStatus: LegalHoldStateForSelfUser) =
// if self user is under legal hold then we shouldn't show legal hold indicator next to every conversation
// the indication is shown in the header of the conversation list for self user in that case and it's enough
when (selfUserLegalHoldStatus) {
is LegalHoldStateForSelfUser.Enabled -> when (this) {
/**
* If self user is under legal hold then we shouldn't show legal hold indicator next to every conversation as in that case
* the legal hold indication is shown in the header of the conversation list for self user in that case and it's enough.
*/
private fun ConversationItem.hideIndicatorForSelfUserUnderLegalHold(isSelfUserUnderLegalHold: Boolean) =
when (isSelfUserUnderLegalHold) {
true -> when (this) {
is ConversationItem.ConnectionConversation -> this.copy(showLegalHoldIndicator = false)
is ConversationItem.GroupConversation -> this.copy(showLegalHoldIndicator = false)
is ConversationItem.PrivateConversation -> this.copy(showLegalHoldIndicator = false)
Expand Down
6 changes: 1 addition & 5 deletions app/src/main/kotlin/com/wire/android/ui/home/conversationslist/ConversationsScreenContent.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ fun ConversationsScreenContent(
lazyListState: LazyListState = rememberLazyListState(),
loadingListContent: @Composable (LazyListState) -> Unit = { ConversationListLoadingContent(it) },
conversationsSource: ConversationsSource = ConversationsSource.MAIN,
initiallyLoaded: Boolean = LocalInspectionMode.current,
conversationListViewModel: ConversationListViewModel = when {
LocalInspectionMode.current -> ConversationListViewModelPreview()
else -> hiltViewModel<ConversationListViewModelImpl, ConversationListViewModelImpl.Factory>(
Expand Down Expand Up @@ -191,10 +190,7 @@ fun ConversationsScreenContent(
when (val state = conversationListViewModel.conversationListState) {
is ConversationListState.Paginated -> {
val lazyPagingItems = state.conversations.collectAsLazyPagingItems()
var showLoading by remember(conversationsSource) { mutableStateOf(!initiallyLoaded) }
if (lazyPagingItems.loadState.refresh != LoadState.Loading && showLoading) {
showLoading = false
}
val showLoading = lazyPagingItems.loadState.refresh == LoadState.Loading && lazyPagingItems.itemCount == 0

when {
// when conversation list is not yet fetched, show loading indicator
Expand Down
9 changes: 4 additions & 5 deletions app/src/main/kotlin/com/wire/android/ui/home/conversationslist/all/AllConversationsScreen.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import com.wire.android.ui.home.conversationslist.model.ConversationsSource
import com.wire.android.ui.theme.WireTheme
import com.wire.android.util.ui.PreviewMultipleThemes
import com.wire.kalium.logic.data.conversation.ConversationFilter
import kotlinx.coroutines.flow.flowOf

@HomeNavGraph(start = true)
@WireDestination
Expand Down Expand Up @@ -70,7 +69,7 @@ fun PreviewAllConversationsEmptyScreen() = WireTheme {
searchBarState = rememberSearchbarState(),
conversationsSource = ConversationsSource.MAIN,
emptyListContent = { ConversationsEmptyContent() },
conversationListViewModel = ConversationListViewModelPreview(flowOf()),
conversationListViewModel = ConversationListViewModelPreview(previewConversationFoldersFlow(list = listOf())),
)
}

Expand All @@ -79,10 +78,10 @@ fun PreviewAllConversationsEmptyScreen() = WireTheme {
fun PreviewAllConversationsEmptySearchScreen() = WireTheme {
ConversationsScreenContent(
navigator = rememberNavigator {},
searchBarState = rememberSearchbarState(searchQueryTextState = TextFieldState(initialText = "er")),
searchBarState = rememberSearchbarState(initialIsSearchActive = true, searchQueryTextState = TextFieldState(initialText = "er")),
conversationsSource = ConversationsSource.MAIN,
emptyListContent = { ConversationsEmptyContent() },
conversationListViewModel = ConversationListViewModelPreview(flowOf()),
conversationListViewModel = ConversationListViewModelPreview(previewConversationFoldersFlow(searchQuery = "er", list = listOf())),
)
}

Expand All @@ -91,7 +90,7 @@ fun PreviewAllConversationsEmptySearchScreen() = WireTheme {
fun PreviewAllConversationsSearchScreen() = WireTheme {
ConversationsScreenContent(
navigator = rememberNavigator {},
searchBarState = rememberSearchbarState(searchQueryTextState = TextFieldState(initialText = "er")),
searchBarState = rememberSearchbarState(initialIsSearchActive = true, searchQueryTextState = TextFieldState(initialText = "er")),
conversationsSource = ConversationsSource.MAIN,
emptyListContent = { ConversationsEmptyContent() },
conversationListViewModel = ConversationListViewModelPreview(previewConversationFoldersFlow("er")),
Expand Down
13 changes: 12 additions & 1 deletion app/src/main/kotlin/com/wire/android/ui/home/conversationslist/common/ConversationList.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import androidx.compose.runtime.snapshots.Snapshot
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.platform.LocalInspectionMode
import androidx.paging.LoadState
import androidx.paging.LoadStates
import androidx.paging.PagingData
import androidx.paging.compose.LazyPagingItems
import androidx.paging.compose.collectAsLazyPagingItems
Expand Down Expand Up @@ -235,7 +237,16 @@ fun previewConversationList(count: Int, startIndex: Int = 0, unread: Boolean = f
fun previewConversationFoldersFlow(
searchQuery: String = "",
list: List<ConversationFolderItem> = previewConversationFolders(searchQuery = searchQuery)
) = flowOf(PagingData.from(list))
) = flowOf(
PagingData.from(
data = list,
sourceLoadStates = LoadStates(
prepend = LoadState.NotLoading(true),
append = LoadState.NotLoading(true),
refresh = LoadState.NotLoading(true),
)
)
)

fun previewConversationFolders(withFolders: Boolean = true, searchQuery: String = "", unreadCount: Int = 3, readCount: Int = 6) =
buildList {
Expand Down
Loading

0 comments on commit f21c93e

Please sign in to comment.