From f0a9c8be5371c7fa9a950196db474fd3e72439a9 Mon Sep 17 00:00:00 2001 From: Konrad Pozniak Date: Wed, 22 May 2024 20:10:32 +0200 Subject: [PATCH] fix notification filters behaving weirdly (#4437) This does four things - set `enablePlaceholders = false` on `PagingConfig`s to avoid Paging Data that contains null placeholders, we don't want them (everywhere, not just in notifications) - make sure NotificationsPagingAdapter does not crash when it encounters a null placeholder - makes sure the notifications refresh correctly when the filters change - the filters are now also respected when loading a gap closes #4433 --- .../account/media/AccountMediaViewModel.kt | 3 ++- .../conversation/ConversationsViewModel.kt | 5 +++- .../domainblocks/DomainBlocksRepository.kt | 6 ++++- .../components/drafts/DraftsViewModel.kt | 5 +++- .../followedtags/FollowedTagsViewModel.kt | 5 +++- .../notifications/NotificationsFragment.kt | 2 +- .../NotificationsPagingAdapter.kt | 3 +-- .../notifications/NotificationsViewModel.kt | 23 +++++++++++-------- .../components/report/ReportViewModel.kt | 6 ++++- .../scheduled/ScheduledStatusViewModel.kt | 6 ++++- .../components/search/SearchViewModel.kt | 18 ++++++++++++--- .../viewmodel/CachedTimelineViewModel.kt | 5 +++- .../viewmodel/NetworkTimelineViewModel.kt | 5 +++- 13 files changed, 68 insertions(+), 24 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/account/media/AccountMediaViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/account/media/AccountMediaViewModel.kt index 0f9077286..38ddcc26d 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/account/media/AccountMediaViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/account/media/AccountMediaViewModel.kt @@ -45,7 +45,8 @@ class AccountMediaViewModel @Inject constructor( val media = Pager( config = PagingConfig( pageSize = LOAD_AT_ONCE, - prefetchDistance = LOAD_AT_ONCE * 2 + prefetchDistance = LOAD_AT_ONCE * 2, + enablePlaceholders = false ), pagingSourceFactory = { AccountMediaPagingSource( diff --git a/app/src/main/java/com/keylesspalace/tusky/components/conversation/ConversationsViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/conversation/ConversationsViewModel.kt index f26936451..584eb39fe 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/conversation/ConversationsViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/conversation/ConversationsViewModel.kt @@ -44,7 +44,10 @@ class ConversationsViewModel @Inject constructor( @OptIn(ExperimentalPagingApi::class) val conversationFlow = Pager( - config = PagingConfig(pageSize = 30), + config = PagingConfig( + pageSize = 30, + enablePlaceholders = false + ), remoteMediator = ConversationsRemoteMediator(api, database, accountManager), pagingSourceFactory = { val activeAccount = accountManager.activeAccount diff --git a/app/src/main/java/com/keylesspalace/tusky/components/domainblocks/DomainBlocksRepository.kt b/app/src/main/java/com/keylesspalace/tusky/components/domainblocks/DomainBlocksRepository.kt index bdc9b9367..7d0917b06 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/domainblocks/DomainBlocksRepository.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/domainblocks/DomainBlocksRepository.kt @@ -39,7 +39,11 @@ class DomainBlocksRepository @Inject constructor( @OptIn(ExperimentalPagingApi::class) val domainPager = Pager( - config = PagingConfig(pageSize = PAGE_SIZE, initialLoadSize = PAGE_SIZE), + config = PagingConfig( + pageSize = PAGE_SIZE, + initialLoadSize = PAGE_SIZE, + enablePlaceholders = false + ), remoteMediator = DomainBlocksRemoteMediator(api, this), pagingSourceFactory = factory ).flow diff --git a/app/src/main/java/com/keylesspalace/tusky/components/drafts/DraftsViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/drafts/DraftsViewModel.kt index 32b36bbed..e5f3cd992 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/drafts/DraftsViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/drafts/DraftsViewModel.kt @@ -39,7 +39,10 @@ class DraftsViewModel @Inject constructor( ) : ViewModel() { val drafts = Pager( - config = PagingConfig(pageSize = 20), + config = PagingConfig( + pageSize = 20, + enablePlaceholders = false + ), pagingSourceFactory = { database.draftDao().draftsPagingSource( accountManager.activeAccount?.id!! diff --git a/app/src/main/java/com/keylesspalace/tusky/components/followedtags/FollowedTagsViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/followedtags/FollowedTagsViewModel.kt index 742967243..1c130fa1e 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/followedtags/FollowedTagsViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/followedtags/FollowedTagsViewModel.kt @@ -26,7 +26,10 @@ class FollowedTagsViewModel @Inject constructor( @OptIn(ExperimentalPagingApi::class) val pager = Pager( - config = PagingConfig(pageSize = 100), + config = PagingConfig( + pageSize = 100, + enablePlaceholders = false + ), remoteMediator = FollowedTagsRemoteMediator(api, this), pagingSourceFactory = { FollowedTagsPagingSource( diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt index ed8eb30e5..6bd076805 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt @@ -468,7 +468,7 @@ class NotificationsFragment : menuBinding.listView.setChoiceMode(ListView.CHOICE_MODE_MULTIPLE) Notification.Type.visibleTypes.forEachIndexed { index, type -> - menuBinding.listView.setItemChecked(index, !viewModel.filters.value.contains(type)) + menuBinding.listView.setItemChecked(index, !viewModel.excludes.value.contains(type)) } window.setContentView(menuBinding.root) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsPagingAdapter.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsPagingAdapter.kt index 8f131020e..c3d00c142 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsPagingAdapter.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsPagingAdapter.kt @@ -89,8 +89,7 @@ class NotificationsPagingAdapter( else -> VIEW_TYPE_UNKNOWN } } - is NotificationViewData.Placeholder -> VIEW_TYPE_PLACEHOLDER - null -> throw IllegalStateException("no item at position $position") + else -> VIEW_TYPE_PLACEHOLDER } } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt index 361b102d5..35e3094e2 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt @@ -78,15 +78,15 @@ class NotificationsViewModel @Inject constructor( private val refreshTrigger = MutableStateFlow(0L) - private val _filters = MutableStateFlow( + private val _excludes = MutableStateFlow( accountManager.activeAccount?.let { account -> deserialize(account.notificationsFilter) } ?: emptySet() ) - val filters: StateFlow> = _filters.asStateFlow() + val excludes: StateFlow> = _excludes.asStateFlow() /** Map from notification id to translation. */ private val translations = MutableStateFlow(mapOf()) - private var remoteMediator = NotificationsRemoteMediator(accountManager, api, db, filters.value) + private var remoteMediator = NotificationsRemoteMediator(accountManager, api, db, excludes.value) private var readingOrder: ReadingOrder = ReadingOrder.from(preferences.getString(PrefKeys.READING_ORDER, null)) @@ -94,7 +94,10 @@ class NotificationsViewModel @Inject constructor( @OptIn(ExperimentalPagingApi::class, ExperimentalCoroutinesApi::class) val notifications = refreshTrigger.flatMapLatest { Pager( - config = PagingConfig(pageSize = LOAD_AT_ONCE), + config = PagingConfig( + pageSize = LOAD_AT_ONCE, + enablePlaceholders = false + ), remoteMediator = remoteMediator, pagingSourceFactory = { val activeAccount = accountManager.activeAccount @@ -128,16 +131,16 @@ class NotificationsViewModel @Inject constructor( } fun updateNotificationFilters(newFilters: Set) { - if (newFilters != _filters.value) { + if (newFilters != _excludes.value) { val account = accountManager.activeAccount if (account != null) { viewModelScope.launch { account.notificationsFilter = serialize(newFilters) accountManager.saveAccount(account) remoteMediator.excludes = newFilters - // clear the cache to trigger a reload db.notificationsDao().cleanupNotifications(account.id, 0) - _filters.value = newFilters + refreshTrigger.value++ + _excludes.value = newFilters } } } @@ -297,14 +300,16 @@ class NotificationsViewModel @Inject constructor( ReadingOrder.OLDEST_FIRST -> api.notifications( maxId = idAbovePlaceholder, minId = idBelowPlaceholder, - limit = TimelineViewModel.LOAD_AT_ONCE + limit = TimelineViewModel.LOAD_AT_ONCE, + excludes = excludes.value ) // Using sinceId, loads up to LOAD_AT_ONCE statuses immediately before // maxId, and no smaller than minId. ReadingOrder.NEWEST_FIRST -> api.notifications( maxId = idAbovePlaceholder, sinceId = idBelowPlaceholder, - limit = TimelineViewModel.LOAD_AT_ONCE + limit = TimelineViewModel.LOAD_AT_ONCE, + excludes = excludes.value ) } } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/report/ReportViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/report/ReportViewModel.kt index a8960756c..5bb629f11 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/report/ReportViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/report/ReportViewModel.kt @@ -77,7 +77,11 @@ class ReportViewModel @Inject constructor( val statusesFlow = accountIdFlow.flatMapLatest { accountId -> Pager( initialKey = statusId, - config = PagingConfig(pageSize = 20, initialLoadSize = 20), + config = PagingConfig( + pageSize = 20, + initialLoadSize = 20, + enablePlaceholders = false + ), pagingSourceFactory = { StatusesPagingSource(accountId, mastodonApi) } ).flow } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/scheduled/ScheduledStatusViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/scheduled/ScheduledStatusViewModel.kt index 6b5b84f15..92a31d157 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/scheduled/ScheduledStatusViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/scheduled/ScheduledStatusViewModel.kt @@ -38,7 +38,11 @@ class ScheduledStatusViewModel @Inject constructor( private val pagingSourceFactory = ScheduledStatusPagingSourceFactory(mastodonApi) val data = Pager( - config = PagingConfig(pageSize = 20, initialLoadSize = 20), + config = PagingConfig( + pageSize = 20, + initialLoadSize = 20, + enablePlaceholders = false + ), pagingSourceFactory = pagingSourceFactory ).flow .cachedIn(viewModelScope) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/search/SearchViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/search/SearchViewModel.kt index 5e9b57a82..6ef6db46b 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/search/SearchViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/search/SearchViewModel.kt @@ -88,19 +88,31 @@ class SearchViewModel @Inject constructor( } val statusesFlow = Pager( - config = PagingConfig(pageSize = DEFAULT_LOAD_SIZE, initialLoadSize = DEFAULT_LOAD_SIZE), + config = PagingConfig( + pageSize = DEFAULT_LOAD_SIZE, + initialLoadSize = DEFAULT_LOAD_SIZE, + enablePlaceholders = false + ), pagingSourceFactory = statusesPagingSourceFactory ).flow .cachedIn(viewModelScope) val accountsFlow = Pager( - config = PagingConfig(pageSize = DEFAULT_LOAD_SIZE, initialLoadSize = DEFAULT_LOAD_SIZE), + config = PagingConfig( + pageSize = DEFAULT_LOAD_SIZE, + initialLoadSize = DEFAULT_LOAD_SIZE, + enablePlaceholders = false + ), pagingSourceFactory = accountsPagingSourceFactory ).flow .cachedIn(viewModelScope) val hashtagsFlow = Pager( - config = PagingConfig(pageSize = DEFAULT_LOAD_SIZE, initialLoadSize = DEFAULT_LOAD_SIZE), + config = PagingConfig( + pageSize = DEFAULT_LOAD_SIZE, + initialLoadSize = DEFAULT_LOAD_SIZE, + enablePlaceholders = false + ), pagingSourceFactory = hashtagsPagingSourceFactory ).flow .cachedIn(viewModelScope) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/timeline/viewmodel/CachedTimelineViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/timeline/viewmodel/CachedTimelineViewModel.kt index c3a9d7ec4..deb9c6b1c 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/timeline/viewmodel/CachedTimelineViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/timeline/viewmodel/CachedTimelineViewModel.kt @@ -85,7 +85,10 @@ class CachedTimelineViewModel @Inject constructor( @OptIn(ExperimentalPagingApi::class) override val statuses = Pager( - config = PagingConfig(pageSize = LOAD_AT_ONCE), + config = PagingConfig( + pageSize = LOAD_AT_ONCE, + enablePlaceholders = false + ), remoteMediator = CachedTimelineRemoteMediator(accountManager, api, db), pagingSourceFactory = { val activeAccount = accountManager.activeAccount diff --git a/app/src/main/java/com/keylesspalace/tusky/components/timeline/viewmodel/NetworkTimelineViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/timeline/viewmodel/NetworkTimelineViewModel.kt index 1d6ea27ac..5baed2093 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/timeline/viewmodel/NetworkTimelineViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/timeline/viewmodel/NetworkTimelineViewModel.kt @@ -87,7 +87,10 @@ class NetworkTimelineViewModel @Inject constructor( @OptIn(ExperimentalPagingApi::class) override val statuses = Pager( - config = PagingConfig(pageSize = LOAD_AT_ONCE), + config = PagingConfig( + pageSize = LOAD_AT_ONCE, + enablePlaceholders = false + ), pagingSourceFactory = { NetworkTimelinePagingSource( viewModel = this