Skip to content

Commit

Permalink
fix: reusing PagingData crash [WPB-15055][WPB-15079][WPB-15064] (#3758)
Browse files Browse the repository at this point in the history
  • Loading branch information
@saleniuk
saleniuk authored Dec 30, 2024
1 parent 10efa7e commit 58d3802
Show file tree
Hide file tree
Showing 8 changed files with 209 additions and 101 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
167 changes: 91 additions & 76 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 @@ -24,6 +24,7 @@ import androidx.compose.runtime.setValue
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import androidx.paging.PagingData
import androidx.paging.cachedIn
import androidx.paging.insertSeparators
import androidx.paging.map
import com.wire.android.BuildConfig
Expand Down Expand Up @@ -126,7 +127,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 @@ -161,6 +162,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 @@ -174,94 +176,105 @@ 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())
.cachedIn(viewModelScope)

private var notPaginatedConversationListState by mutableStateOf(ConversationListState.NotPaginated())
override val conversationListState: ConversationListState
get() = if (usePagination) {
ConversationListState.Paginated(
conversations = conversationsPaginatedFlow,
domain = currentAccount.domain
)
} else {
notPaginatedConversationListState
override var conversationListState by mutableStateOf(
when (usePagination) {
true -> ConversationListState.Paginated(conversations = conversationsPaginatedFlow, domain = currentAccount.domain)
false -> ConversationListState.NotPaginated()
}
)
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 {
notPaginatedConversationListState = notPaginatedConversationListState.copy(
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 @@ -446,11 +459,13 @@ private fun ConversationsSource.toFilter(): ConversationFilter = when (this) {
ConversationsSource.ONE_ON_ONE -> ConversationFilter.ONE_ON_ONE
}

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 @@ -88,7 +88,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 @@ -189,10 +188,7 @@ fun ConversationsScreenContent(
when (val state = conversationListViewModel.conversationListState) {
is ConversationListState.Paginated -> {
val lazyPagingItems = state.conversations.collectAsLazyPagingItems()
var showLoading by remember { 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 @@ -33,7 +33,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 @@ -103,7 +102,7 @@ fun PreviewAllConversationsEmptyScreen() = WireTheme {
searchBarState = rememberSearchbarState(),
conversationsSource = ConversationsSource.MAIN,
emptyListContent = { ConversationsEmptyContent() },
conversationListViewModel = ConversationListViewModelPreview(flowOf()),
conversationListViewModel = ConversationListViewModelPreview(previewConversationFoldersFlow(list = listOf())),
)
}

Expand All @@ -112,10 +111,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 @@ -124,7 +123,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
Loading

0 comments on commit 58d3802

Please sign in to comment.