From 6e6fd8623a82a8a77a711405a9f79db1d80e7dd9 Mon Sep 17 00:00:00 2001 From: cketti Date: Tue, 11 Jan 2022 01:19:24 +0100 Subject: [PATCH] Only remove notifications for messages currently displayed This only works for the Unified Inbox, the "new messages" view, and when viewing a single folder. Search views are currently not supported. --- .../k9/controller/MessagingController.java | 8 ++- .../k9/controller/NotificationOperations.kt | 63 +++++++++++++++++++ .../NewMailNotificationController.kt | 12 +++- .../NewMailNotificationManager.kt | 3 +- .../k9/notification/NotificationController.kt | 8 ++- .../k9/notification/NotificationRepository.kt | 5 +- .../fsck/k9/search/LocalSearchExtensions.kt | 12 ++++ .../NewMailNotificationManagerTest.kt | 20 ++++-- .../java/com/fsck/k9/activity/MessageList.kt | 23 ++++++- .../fsck/k9/fragment/MessageListFragment.kt | 5 -- 10 files changed, 138 insertions(+), 21 deletions(-) create mode 100644 app/core/src/main/java/com/fsck/k9/controller/NotificationOperations.kt diff --git a/app/core/src/main/java/com/fsck/k9/controller/MessagingController.java b/app/core/src/main/java/com/fsck/k9/controller/MessagingController.java index ba4ca011e..341a4bf03 100644 --- a/app/core/src/main/java/com/fsck/k9/controller/MessagingController.java +++ b/app/core/src/main/java/com/fsck/k9/controller/MessagingController.java @@ -127,6 +127,7 @@ public class MessagingController { private final MemorizingMessagingListener memorizingMessagingListener = new MemorizingMessagingListener(); private final MessageCountsProvider messageCountsProvider; private final DraftOperations draftOperations; + private final NotificationOperations notificationOperations; private MessagingListener checkMailListener = null; @@ -166,6 +167,7 @@ public class MessagingController { initializeControllerExtensions(controllerExtensions); draftOperations = new DraftOperations(this, messageStoreManager, saveMessageDataCreator); + notificationOperations = new NotificationOperations(notificationController, preferences, messageStoreManager); } private void initializeControllerExtensions(List controllerExtensions) { @@ -2553,9 +2555,9 @@ public class MessagingController { } } - public void removeNotificationsForAccount(Account account) { - put("removeNotificationsForAccount", null, () -> { - notificationController.clearNewMailNotifications(account, false); + public void clearNotifications(LocalSearch search) { + put("clearNotifications", null, () -> { + notificationOperations.clearNotifications(search); }); } diff --git a/app/core/src/main/java/com/fsck/k9/controller/NotificationOperations.kt b/app/core/src/main/java/com/fsck/k9/controller/NotificationOperations.kt new file mode 100644 index 000000000..8ab0a7f44 --- /dev/null +++ b/app/core/src/main/java/com/fsck/k9/controller/NotificationOperations.kt @@ -0,0 +1,63 @@ +package com.fsck.k9.controller + +import com.fsck.k9.Account +import com.fsck.k9.Preferences +import com.fsck.k9.mailstore.MessageStoreManager +import com.fsck.k9.notification.NotificationController +import com.fsck.k9.search.LocalSearch +import com.fsck.k9.search.isNewMessages +import com.fsck.k9.search.isSingleFolder +import com.fsck.k9.search.isUnifiedInbox + +internal class NotificationOperations( + private val notificationController: NotificationController, + private val preferences: Preferences, + private val messageStoreManager: MessageStoreManager +) { + fun clearNotifications(search: LocalSearch) { + if (search.isUnifiedInbox) { + clearUnifiedInboxNotifications() + } else if (search.isNewMessages) { + clearAllNotifications() + } else if (search.isSingleFolder) { + val account = search.firstAccount() ?: return + val folderId = search.folderIds.first() + clearNotifications(account, folderId) + } else { + // TODO: Remove notifications when updating the message list. That way we can easily remove only + // notifications for messages that are currently displayed in the list. + } + } + + private fun clearUnifiedInboxNotifications() { + for (account in preferences.accounts) { + val messageStore = messageStoreManager.getMessageStore(account) + + val folderIds = messageStore.getFolders(excludeLocalOnly = true) { folderDetails -> + if (folderDetails.isIntegrate) folderDetails.id else null + }.filterNotNull().toSet() + + if (folderIds.isNotEmpty()) { + notificationController.clearNewMailNotifications(account) { messageReferences -> + messageReferences.filter { messageReference -> messageReference.folderId in folderIds } + } + } + } + } + + private fun clearAllNotifications() { + for (account in preferences.accounts) { + notificationController.clearNewMailNotifications(account, clearNewMessageState = false) + } + } + + private fun clearNotifications(account: Account, folderId: Long) { + notificationController.clearNewMailNotifications(account) { messageReferences -> + messageReferences.filter { messageReference -> messageReference.folderId == folderId } + } + } + + private fun LocalSearch.firstAccount(): Account? { + return preferences.getAccount(accountUuids.first()) + } +} diff --git a/app/core/src/main/java/com/fsck/k9/notification/NewMailNotificationController.kt b/app/core/src/main/java/com/fsck/k9/notification/NewMailNotificationController.kt index 486692233..190144862 100644 --- a/app/core/src/main/java/com/fsck/k9/notification/NewMailNotificationController.kt +++ b/app/core/src/main/java/com/fsck/k9/notification/NewMailNotificationController.kt @@ -30,8 +30,16 @@ internal class NewMailNotificationController( processNewMailNotificationData(notificationData) } - fun removeNewMailNotifications(account: Account, selector: (List) -> List) { - val notificationData = newMailNotificationManager.removeNewMailNotifications(account, selector) + fun removeNewMailNotifications( + account: Account, + clearNewMessageState: Boolean, + selector: (List) -> List + ) { + val notificationData = newMailNotificationManager.removeNewMailNotifications( + account, + clearNewMessageState, + selector + ) if (notificationData != null) { processNewMailNotificationData(notificationData) diff --git a/app/core/src/main/java/com/fsck/k9/notification/NewMailNotificationManager.kt b/app/core/src/main/java/com/fsck/k9/notification/NewMailNotificationManager.kt index b3f48675f..8e1a8c3fe 100644 --- a/app/core/src/main/java/com/fsck/k9/notification/NewMailNotificationManager.kt +++ b/app/core/src/main/java/com/fsck/k9/notification/NewMailNotificationManager.kt @@ -65,9 +65,10 @@ internal class NewMailNotificationManager( fun removeNewMailNotifications( account: Account, + clearNewMessageState: Boolean, selector: (List) -> List ): NewMailNotificationData? { - val result = notificationRepository.removeNotifications(account, selector) ?: return null + val result = notificationRepository.removeNotifications(account, clearNewMessageState, selector) ?: return null val cancelNotificationIds = when { result.notificationData.isEmpty() -> { diff --git a/app/core/src/main/java/com/fsck/k9/notification/NotificationController.kt b/app/core/src/main/java/com/fsck/k9/notification/NotificationController.kt index 3033cb8f6..fe0144108 100644 --- a/app/core/src/main/java/com/fsck/k9/notification/NotificationController.kt +++ b/app/core/src/main/java/com/fsck/k9/notification/NotificationController.kt @@ -65,7 +65,13 @@ class NotificationController internal constructor( } fun removeNewMailNotification(account: Account, messageReference: MessageReference) { - newMailNotificationController.removeNewMailNotifications(account) { listOf(messageReference) } + newMailNotificationController.removeNewMailNotifications(account, clearNewMessageState = true) { + listOf(messageReference) + } + } + + fun clearNewMailNotifications(account: Account, selector: (List) -> List) { + newMailNotificationController.removeNewMailNotifications(account, clearNewMessageState = false, selector) } fun clearNewMailNotifications(account: Account, clearNewMessageState: Boolean) { diff --git a/app/core/src/main/java/com/fsck/k9/notification/NotificationRepository.kt b/app/core/src/main/java/com/fsck/k9/notification/NotificationRepository.kt index 0daea526e..b18f3686a 100644 --- a/app/core/src/main/java/com/fsck/k9/notification/NotificationRepository.kt +++ b/app/core/src/main/java/com/fsck/k9/notification/NotificationRepository.kt @@ -46,10 +46,13 @@ internal class NotificationRepository( @Synchronized fun removeNotifications( account: Account, + clearNewMessageState: Boolean = true, selector: (List) -> List ): RemoveNotificationsResult? { return notificationDataStore.removeNotifications(account, selector)?.also { result -> - persistNotificationDataStoreChanges(account, result.notificationStoreOperations) + if (clearNewMessageState) { + persistNotificationDataStoreChanges(account, result.notificationStoreOperations) + } } } diff --git a/app/core/src/main/java/com/fsck/k9/search/LocalSearchExtensions.kt b/app/core/src/main/java/com/fsck/k9/search/LocalSearchExtensions.kt index 8e49538da..d52844698 100644 --- a/app/core/src/main/java/com/fsck/k9/search/LocalSearchExtensions.kt +++ b/app/core/src/main/java/com/fsck/k9/search/LocalSearchExtensions.kt @@ -5,6 +5,18 @@ package com.fsck.k9.search import com.fsck.k9.Account import com.fsck.k9.Preferences +val LocalSearch.isUnifiedInbox: Boolean + get() = id == SearchAccount.UNIFIED_INBOX + +val LocalSearch.isNewMessages: Boolean + get() = id == SearchAccount.NEW_MESSAGES + +val LocalSearch.isSingleAccount: Boolean + get() = accountUuids.size == 1 + +val LocalSearch.isSingleFolder: Boolean + get() = isSingleAccount && folderIds.size == 1 + @JvmName("getAccountsFromLocalSearch") fun LocalSearch.getAccounts(preferences: Preferences): List { val accounts = preferences.accounts diff --git a/app/core/src/test/java/com/fsck/k9/notification/NewMailNotificationManagerTest.kt b/app/core/src/test/java/com/fsck/k9/notification/NewMailNotificationManagerTest.kt index 573b760d3..873dd7cb5 100644 --- a/app/core/src/test/java/com/fsck/k9/notification/NewMailNotificationManagerTest.kt +++ b/app/core/src/test/java/com/fsck/k9/notification/NewMailNotificationManagerTest.kt @@ -128,7 +128,9 @@ class NewMailNotificationManagerTest { @Test fun `remove notification when none was added before should return null`() { - val result = manager.removeNewMailNotifications(account) { listOf(createMessageReference("any")) } + val result = manager.removeNewMailNotifications(account, clearNewMessageState = true) { + listOf(createMessageReference("any")) + } assertThat(result).isNull() } @@ -144,7 +146,9 @@ class NewMailNotificationManagerTest { ) manager.addNewMailNotification(account, message, silent = false) - val result = manager.removeNewMailNotifications(account) { listOf(createMessageReference("untracked")) } + val result = manager.removeNewMailNotifications(account, clearNewMessageState = true) { + listOf(createMessageReference("untracked")) + } assertThat(result).isNull() } @@ -160,7 +164,9 @@ class NewMailNotificationManagerTest { ) manager.addNewMailNotification(account, message, silent = false) - val result = manager.removeNewMailNotifications(account) { listOf(createMessageReference("msg-1")) } + val result = manager.removeNewMailNotifications(account, clearNewMessageState = true) { + listOf(createMessageReference("msg-1")) + } assertNotNull(result) { data -> assertThat(data.cancelNotificationIds).containsExactly( @@ -200,7 +206,9 @@ class NewMailNotificationManagerTest { ) manager.addNewMailNotification(account, messageThree, silent = true) - val result = manager.removeNewMailNotifications(account) { listOf(createMessageReference("msg-2")) } + val result = manager.removeNewMailNotifications(account, clearNewMessageState = true) { + listOf(createMessageReference("msg-2")) + } assertNotNull(result) { data -> assertThat(data.cancelNotificationIds).isEqualTo(listOf(notificationIdTwo)) @@ -230,7 +238,9 @@ class NewMailNotificationManagerTest { manager.addNewMailNotification(account, message, silent = false) addMaximumNumberOfNotifications() - val result = manager.removeNewMailNotifications(account) { listOf(createMessageReference("msg-1")) } + val result = manager.removeNewMailNotifications(account, clearNewMessageState = true) { + listOf(createMessageReference("msg-1")) + } assertNotNull(result) { data -> assertThat(data.cancelNotificationIds).hasSize(1) diff --git a/app/ui/legacy/src/main/java/com/fsck/k9/activity/MessageList.kt b/app/ui/legacy/src/main/java/com/fsck/k9/activity/MessageList.kt index a027d0482..510982e81 100644 --- a/app/ui/legacy/src/main/java/com/fsck/k9/activity/MessageList.kt +++ b/app/ui/legacy/src/main/java/com/fsck/k9/activity/MessageList.kt @@ -32,6 +32,7 @@ import com.fsck.k9.Preferences import com.fsck.k9.account.BackgroundAccountRemover import com.fsck.k9.activity.compose.MessageActions import com.fsck.k9.controller.MessageReference +import com.fsck.k9.controller.MessagingController import com.fsck.k9.fragment.MessageListFragment import com.fsck.k9.fragment.MessageListFragment.MessageListFragmentListener import com.fsck.k9.helper.Contacts @@ -44,6 +45,7 @@ import com.fsck.k9.search.SearchAccount import com.fsck.k9.search.SearchSpecification import com.fsck.k9.search.SearchSpecification.SearchCondition import com.fsck.k9.search.SearchSpecification.SearchField +import com.fsck.k9.search.isUnifiedInbox import com.fsck.k9.ui.BuildConfig import com.fsck.k9.ui.K9Drawer import com.fsck.k9.ui.R @@ -92,6 +94,7 @@ open class MessageList : private val defaultFolderProvider: DefaultFolderProvider by inject() private val accountRemover: BackgroundAccountRemover by inject() private val generalSettingsManager: GeneralSettingsManager by inject() + private val messagingController: MessagingController by inject() private val permissionUiHelper: PermissionUiHelper = K9PermissionUiHelper(this) @@ -347,6 +350,7 @@ open class MessageList : } } setDrawerLockState() + onMessageListDisplayed() } } } @@ -541,6 +545,10 @@ open class MessageList : recreate() } + if (displayMode != DisplayMode.MESSAGE_VIEW) { + onMessageListDisplayed() + } + if (this !is Search) { // necessary b/c no guarantee Search.onStop will be called before MessageList.onResume // when returning from search results @@ -613,6 +621,8 @@ open class MessageList : search.addAllowedFolder(folderId) performSearch(search) + + onMessageListDisplayed() } private fun openFolderImmediately(folderId: Long) { @@ -1462,6 +1472,8 @@ open class MessageList : showDefaultTitleView() configureMenu(menu) + + onMessageListDisplayed() } private fun setDrawerLockState() { @@ -1519,6 +1531,14 @@ open class MessageList : } } + private fun onMessageListDisplayed() { + clearNotifications() + } + + private fun clearNotifications() { + messagingController.clearNotifications(search) + } + override fun startIntentSenderForResult( intent: IntentSender, requestCode: Int, @@ -1591,9 +1611,6 @@ open class MessageList : } } - private val LocalSearch.isUnifiedInbox: Boolean - get() = id == SearchAccount.UNIFIED_INBOX - private fun MessageReference.toLocalSearch(): LocalSearch { return LocalSearch().apply { addAccountUuid(accountUuid) diff --git a/app/ui/legacy/src/main/java/com/fsck/k9/fragment/MessageListFragment.kt b/app/ui/legacy/src/main/java/com/fsck/k9/fragment/MessageListFragment.kt index 9d6ead010..ecc400c3e 100644 --- a/app/ui/legacy/src/main/java/com/fsck/k9/fragment/MessageListFragment.kt +++ b/app/ui/legacy/src/main/java/com/fsck/k9/fragment/MessageListFragment.kt @@ -475,11 +475,6 @@ class MessageListFragment : messagingController.addListener(activityListener) - for (account in localSearch.getAccounts(preferences)) { - // TODO: Only remove notifications for messages in the currently displayed message list - messagingController.removeNotificationsForAccount(account) - } - updateTitle() }