From 635f975b6cb5bbb8170a982e4b713e432861e9d4 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Fri, 2 Dec 2022 15:13:10 +0100 Subject: [PATCH] Fix missing unregister of pusher when notifications are disabled --- .../features/home/HomeActivityViewModel.kt | 10 +++++++ ...leNotificationsForCurrentSessionUseCase.kt | 11 +------- ...tificationsForCurrentSessionUseCaseTest.kt | 28 +------------------ 3 files changed, 12 insertions(+), 37 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/HomeActivityViewModel.kt b/vector/src/main/java/im/vector/app/features/home/HomeActivityViewModel.kt index 26034fc09c..a54ce2cff3 100644 --- a/vector/src/main/java/im/vector/app/features/home/HomeActivityViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/HomeActivityViewModel.kt @@ -29,6 +29,7 @@ import im.vector.app.core.platform.VectorViewModel import im.vector.app.core.pushers.EnsureFcmTokenIsRetrievedUseCase import im.vector.app.core.pushers.PushersManager import im.vector.app.core.pushers.RegisterUnifiedPushUseCase +import im.vector.app.core.pushers.UnregisterUnifiedPushUseCase import im.vector.app.features.analytics.AnalyticsConfig import im.vector.app.features.analytics.AnalyticsTracker import im.vector.app.features.analytics.extensions.toAnalyticsType @@ -92,6 +93,7 @@ class HomeActivityViewModel @AssistedInject constructor( private val stopOngoingVoiceBroadcastUseCase: StopOngoingVoiceBroadcastUseCase, private val pushersManager: PushersManager, private val registerUnifiedPushUseCase: RegisterUnifiedPushUseCase, + private val unregisterUnifiedPushUseCase: UnregisterUnifiedPushUseCase, private val ensureFcmTokenIsRetrievedUseCase: EnsureFcmTokenIsRetrievedUseCase, ) : VectorViewModel(initialState) { @@ -130,6 +132,8 @@ class HomeActivityViewModel @AssistedInject constructor( private fun registerUnifiedPushIfNeeded() { if (vectorPreferences.areNotificationEnabledForDevice()) { registerUnifiedPush(distributor = "") + } else { + unregisterUnifiedPush() } } @@ -146,6 +150,12 @@ class HomeActivityViewModel @AssistedInject constructor( } } + private fun unregisterUnifiedPush() { + viewModelScope.launch { + unregisterUnifiedPushUseCase.execute(pushersManager) + } + } + private fun observeReleaseNotes() = withState { state -> if (vectorPreferences.isNewAppLayoutEnabled()) { // we don't want to show release notes for new users or after relogin diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCase.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCase.kt index daa58578d6..0c50a296f3 100644 --- a/vector/src/main/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCase.kt @@ -16,27 +16,18 @@ package im.vector.app.features.settings.notifications -import im.vector.app.core.di.ActiveSessionHolder import im.vector.app.core.pushers.PushersManager import im.vector.app.core.pushers.UnregisterUnifiedPushUseCase -import im.vector.app.features.settings.devices.v2.notification.CheckIfCanToggleNotificationsViaPusherUseCase import javax.inject.Inject class DisableNotificationsForCurrentSessionUseCase @Inject constructor( - private val activeSessionHolder: ActiveSessionHolder, private val pushersManager: PushersManager, - private val checkIfCanToggleNotificationsViaPusherUseCase: CheckIfCanToggleNotificationsViaPusherUseCase, private val toggleNotificationsForCurrentSessionUseCase: ToggleNotificationsForCurrentSessionUseCase, private val unregisterUnifiedPushUseCase: UnregisterUnifiedPushUseCase, ) { suspend fun execute() { - val session = activeSessionHolder.getSafeActiveSession() ?: return toggleNotificationsForCurrentSessionUseCase.execute(enabled = false) - - // handle case when server does not support toggle of pusher - if (!checkIfCanToggleNotificationsViaPusherUseCase.execute(session)) { - unregisterUnifiedPushUseCase.execute(pushersManager) - } + unregisterUnifiedPushUseCase.execute(pushersManager) } } diff --git a/vector/src/test/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCaseTest.kt index b7749d0252..669b20fc1a 100644 --- a/vector/src/test/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCaseTest.kt +++ b/vector/src/test/java/im/vector/app/features/settings/notifications/DisableNotificationsForCurrentSessionUseCaseTest.kt @@ -17,54 +17,28 @@ package im.vector.app.features.settings.notifications import im.vector.app.core.pushers.UnregisterUnifiedPushUseCase -import im.vector.app.features.settings.devices.v2.notification.CheckIfCanToggleNotificationsViaPusherUseCase -import im.vector.app.test.fakes.FakeActiveSessionHolder import im.vector.app.test.fakes.FakePushersManager import io.mockk.coJustRun import io.mockk.coVerify -import io.mockk.every import io.mockk.mockk import kotlinx.coroutines.test.runTest import org.junit.Test class DisableNotificationsForCurrentSessionUseCaseTest { - private val fakeActiveSessionHolder = FakeActiveSessionHolder() private val fakePushersManager = FakePushersManager() - private val fakeCheckIfCanToggleNotificationsViaPusherUseCase = mockk() private val fakeToggleNotificationsForCurrentSessionUseCase = mockk() private val fakeUnregisterUnifiedPushUseCase = mockk() private val disableNotificationsForCurrentSessionUseCase = DisableNotificationsForCurrentSessionUseCase( - activeSessionHolder = fakeActiveSessionHolder.instance, pushersManager = fakePushersManager.instance, - checkIfCanToggleNotificationsViaPusherUseCase = fakeCheckIfCanToggleNotificationsViaPusherUseCase, toggleNotificationsForCurrentSessionUseCase = fakeToggleNotificationsForCurrentSessionUseCase, unregisterUnifiedPushUseCase = fakeUnregisterUnifiedPushUseCase, ) @Test - fun `given toggle via pusher is possible when execute then disable notification via toggle of existing pusher`() = runTest { + fun `when execute then disable notifications and unregister the pusher`() = runTest { // Given - val fakeSession = fakeActiveSessionHolder.fakeSession - every { fakeCheckIfCanToggleNotificationsViaPusherUseCase.execute(fakeSession) } returns true - coJustRun { fakeToggleNotificationsForCurrentSessionUseCase.execute(any()) } - - // When - disableNotificationsForCurrentSessionUseCase.execute() - - // Then - coVerify { fakeToggleNotificationsForCurrentSessionUseCase.execute(false) } - coVerify(inverse = true) { - fakeUnregisterUnifiedPushUseCase.execute(any()) - } - } - - @Test - fun `given toggle via pusher is NOT possible when execute then disable notification by unregistering the pusher`() = runTest { - // Given - val fakeSession = fakeActiveSessionHolder.fakeSession - every { fakeCheckIfCanToggleNotificationsViaPusherUseCase.execute(fakeSession) } returns false coJustRun { fakeToggleNotificationsForCurrentSessionUseCase.execute(any()) } coJustRun { fakeUnregisterUnifiedPushUseCase.execute(any()) }