From a7ad61c1079cfca98c1a5079a58491ce5ad3787a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Wed, 4 Dec 2024 16:53:43 +0100 Subject: [PATCH 1/5] fix: reload WireActivity when account is switched in CallActivity [WPB-11325] --- .../com/wire/android/ui/WireActivity.kt | 17 +++ .../wire/android/ui/WireActivityViewModel.kt | 143 ++++++------------ .../wire/android/ui/calling/CallActivity.kt | 10 +- .../ui/calling/CallActivityViewModel.kt | 9 +- .../android/util/SwitchAccountObserver.kt | 57 +++++++ .../android/ui/CallActivityViewModelTest.kt | 54 ++++++- .../android/ui/WireActivityViewModelTest.kt | 63 ++++++++ 7 files changed, 252 insertions(+), 101 deletions(-) create mode 100644 app/src/main/kotlin/com/wire/android/util/SwitchAccountObserver.kt diff --git a/app/src/main/kotlin/com/wire/android/ui/WireActivity.kt b/app/src/main/kotlin/com/wire/android/ui/WireActivity.kt index 53ee314367c..9ef9e539c98 100644 --- a/app/src/main/kotlin/com/wire/android/ui/WireActivity.kt +++ b/app/src/main/kotlin/com/wire/android/ui/WireActivity.kt @@ -107,6 +107,7 @@ import com.wire.android.ui.theme.ThemeOption import com.wire.android.ui.theme.WireTheme import com.wire.android.ui.userprofile.self.dialog.LogoutOptionsDialog import com.wire.android.ui.userprofile.self.dialog.LogoutOptionsDialogState +import com.wire.android.util.SwitchAccountObserver import com.wire.android.util.CurrentScreenManager import com.wire.android.util.LocalSyncStateObserver import com.wire.android.util.SyncStateObserver @@ -135,6 +136,9 @@ class WireActivity : AppCompatActivity() { @Inject lateinit var lockCodeTimeManager: Lazy + @Inject + lateinit var switchAccountObserver: SwitchAccountObserver + private val viewModel: WireActivityViewModel by viewModels() private val featureFlagNotificationViewModel: FeatureFlagNotificationViewModel by viewModels() @@ -345,6 +349,19 @@ class WireActivity : AppCompatActivity() { navigator.navController.removeOnDestinationChangedListener(currentScreenManager) } } + + DisposableEffect(switchAccountObserver, navigator) { + NavigationSwitchAccountActions { + lifecycleScope.launch(Dispatchers.Main) { + navigator.navigate(it) + } + }.let { + switchAccountObserver.register(it) + onDispose { + switchAccountObserver.unregister(it) + } + } + } } @Composable diff --git a/app/src/main/kotlin/com/wire/android/ui/WireActivityViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/WireActivityViewModel.kt index 624b6aa74a5..c3ed534727e 100644 --- a/app/src/main/kotlin/com/wire/android/ui/WireActivityViewModel.kt +++ b/app/src/main/kotlin/com/wire/android/ui/WireActivityViewModel.kt @@ -80,10 +80,9 @@ import com.wire.kalium.logic.feature.user.webSocketStatus.ObservePersistentWebSo import com.wire.kalium.util.DateTimeUtil.toIsoDateTimeString import dagger.Lazy import dagger.hilt.android.lifecycle.HiltViewModel -import kotlinx.coroutines.Deferred import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.async import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.distinctUntilChanged @@ -131,24 +130,19 @@ class WireActivityViewModel @Inject constructor( private val _observeSyncFlowState: MutableStateFlow = MutableStateFlow(null) val observeSyncFlowState: StateFlow = _observeSyncFlowState - private val userIdDeferred: Deferred = viewModelScope.async(dispatchers.io()) { - currentSessionFlow.get().invoke() - .distinctUntilChanged() - .map { result -> - if (result is CurrentSessionResult.Success) { - if (result.accountInfo.isValid()) { - result.accountInfo.userId - } else { - null - } - } else { - null - } - } - .distinctUntilChanged() - .flowOn(dispatchers.io()) - .shareIn(viewModelScope, SharingStarted.WhileSubscribed(), 1).first() - } + private val observeCurrentAccountInfo: SharedFlow = currentSessionFlow.get().invoke() + .map { (it as? CurrentSessionResult.Success)?.accountInfo } + .distinctUntilChanged() + .flowOn(dispatchers.io()) + .shareIn(viewModelScope, SharingStarted.WhileSubscribed(), 1) + + private val observeCurrentValidUserId: SharedFlow = observeCurrentAccountInfo + .map { + if (it?.isValid() == true) it.userId else null + } + .distinctUntilChanged() + .flowOn(dispatchers.io()) + .shareIn(viewModelScope, SharingStarted.WhileSubscribed(), 1) init { observeSyncState() @@ -159,21 +153,10 @@ class WireActivityViewModel @Inject constructor( observeLogoutState() } - @Suppress("TooGenericExceptionCaught") - private fun shouldEnrollToE2ei() = viewModelScope.async(dispatchers.io()) { - try { - val userId = userIdDeferred.await() - if (userId != null) { - observeIfE2EIRequiredDuringLoginUseCaseProviderFactory.create(userId) - .observeIfE2EIIsRequiredDuringLogin().first() ?: false - } else { - false - } - } catch (e: NullPointerException) { - appLogger.e("Error while observing E2EI state: $e") - false - } - } + private suspend fun shouldEnrollToE2ei(): Boolean = observeCurrentValidUserId.first()?.let { + observeIfE2EIRequiredDuringLoginUseCaseProviderFactory.create(it) + .observeIfE2EIIsRequiredDuringLogin().first() ?: false + } ?: false private fun observeAppThemeState() { viewModelScope.launch(dispatchers.io()) { @@ -185,33 +168,28 @@ class WireActivityViewModel @Inject constructor( } } - @Suppress("TooGenericExceptionCaught") private fun observeSyncState() { viewModelScope.launch(dispatchers.io()) { - try { - val userId = userIdDeferred.await() - if (userId != null) { - observeSyncStateUseCaseProviderFactory.create(userId).observeSyncState() - } else { - flowOf(null) - .distinctUntilChanged() - .collect { _observeSyncFlowState.emit(it) } + observeCurrentValidUserId + .flatMapLatest { userId -> + userId?.let { + observeSyncStateUseCaseProviderFactory.create(userId).observeSyncState() + } ?: flowOf(null) + } + .distinctUntilChanged() + .flowOn(dispatchers.io()) + .collect { + _observeSyncFlowState.emit(it) } - } catch (e: NullPointerException) { - appLogger.e("Error while observing sync state: $e") - } } } private fun observeLogoutState() { viewModelScope.launch(dispatchers.io()) { - currentSessionFlow.get().invoke() - .distinctUntilChanged() + observeCurrentAccountInfo .collect { - if (it is CurrentSessionResult.Success) { - if (it.accountInfo.isValid().not()) { - handleInvalidSession((it.accountInfo as AccountInfo.Invalid).logoutReason) - } + if (it is AccountInfo.Invalid) { + handleInvalidSession(it.logoutReason) } } } @@ -247,40 +225,27 @@ class WireActivityViewModel @Inject constructor( @Suppress("TooGenericExceptionCaught") private fun observeScreenshotCensoringConfigState() { viewModelScope.launch(dispatchers.io()) { - try { - val userId = userIdDeferred.await() - if (userId != null) { - observeScreenshotCensoringConfigUseCaseProviderFactory.create(userId) - .observeScreenshotCensoringConfig().collect { result -> - globalAppState = globalAppState.copy( - screenshotCensoringEnabled = result is ObserveScreenshotCensoringConfigResult.Enabled - ) - } - } else { - globalAppState = globalAppState.copy( - screenshotCensoringEnabled = false - ) + observeCurrentValidUserId + .flatMapLatest { currentValidUserId -> + currentValidUserId?.let { + observeScreenshotCensoringConfigUseCaseProviderFactory.create(it) + .observeScreenshotCensoringConfig() + .map { result -> + result is ObserveScreenshotCensoringConfigResult.Enabled + } + } ?: flowOf(false) + } + .collect { + globalAppState = globalAppState.copy(screenshotCensoringEnabled = it) } - } catch (exception: NullPointerException) { - globalAppState = globalAppState.copy( - screenshotCensoringEnabled = false - ) - } } } - suspend fun initialAppState(): InitialAppState { - val shouldMigrate = viewModelScope.async(dispatchers.io()) { - shouldMigrate() - } - val shouldLogin = viewModelScope.async(dispatchers.io()) { - shouldLogIn() - } - val shouldEnrollToE2ei = shouldEnrollToE2ei() - return when { - shouldMigrate.await() -> InitialAppState.NOT_MIGRATED - shouldLogin.await() -> InitialAppState.NOT_LOGGED_IN - shouldEnrollToE2ei.await() -> InitialAppState.ENROLL_E2EI + suspend fun initialAppState(): InitialAppState = withContext(dispatchers.io()) { + when { + shouldMigrate() -> InitialAppState.NOT_MIGRATED + shouldLogIn() -> InitialAppState.NOT_LOGGED_IN + shouldEnrollToE2ei() -> InitialAppState.ENROLL_E2EI else -> InitialAppState.LOGGED_IN } } @@ -517,17 +482,7 @@ class WireActivityViewModel @Inject constructor( globalAppState = globalAppState.copy(conversationJoinedDialog = null) } - private suspend fun shouldLogIn(): Boolean = !hasValidCurrentSession() - - private suspend fun hasValidCurrentSession(): Boolean = - // TODO: the usage of currentSessionFlow is a temporary solution, it should be replaced with a proper solution - currentSessionFlow.get().invoke().first().let { - when (it) { - is CurrentSessionResult.Failure.Generic -> false - CurrentSessionResult.Failure.SessionNotFound -> false - is CurrentSessionResult.Success -> true - } - } + private suspend fun shouldLogIn(): Boolean = observeCurrentValidUserId.first() == null private suspend fun shouldMigrate(): Boolean = migrationManager.get().shouldMigrate() diff --git a/app/src/main/kotlin/com/wire/android/ui/calling/CallActivity.kt b/app/src/main/kotlin/com/wire/android/ui/calling/CallActivity.kt index 0ef6ae777a9..b9320fb4c18 100644 --- a/app/src/main/kotlin/com/wire/android/ui/calling/CallActivity.kt +++ b/app/src/main/kotlin/com/wire/android/ui/calling/CallActivity.kt @@ -24,10 +24,18 @@ import androidx.activity.viewModels import androidx.appcompat.app.AppCompatActivity import androidx.lifecycle.lifecycleScope import com.wire.android.ui.AppLockActivity +import com.wire.android.util.SwitchAccountObserver import com.wire.kalium.logic.data.id.QualifiedIdMapperImpl +import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.launch +import javax.inject.Inject +@AndroidEntryPoint abstract class CallActivity : AppCompatActivity() { + + @Inject + lateinit var switchAccountObserver: SwitchAccountObserver + companion object { const val EXTRA_CONVERSATION_ID = "conversation_id" const val EXTRA_USER_ID = "user_id" @@ -40,7 +48,7 @@ abstract class CallActivity : AppCompatActivity() { fun switchAccountIfNeeded(userId: String?) { userId?.let { qualifiedIdMapper.fromStringToQualifiedID(it).run { - callActivityViewModel.switchAccountIfNeeded(this) + callActivityViewModel.switchAccountIfNeeded(userId = this, actions = switchAccountObserver) } } } diff --git a/app/src/main/kotlin/com/wire/android/ui/calling/CallActivityViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/calling/CallActivityViewModel.kt index b504798e1e9..8b8db966c63 100644 --- a/app/src/main/kotlin/com/wire/android/ui/calling/CallActivityViewModel.kt +++ b/app/src/main/kotlin/com/wire/android/ui/calling/CallActivityViewModel.kt @@ -21,6 +21,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.wire.android.di.ObserveScreenshotCensoringConfigUseCaseProvider import com.wire.android.feature.AccountSwitchUseCase +import com.wire.android.feature.SwitchAccountActions import com.wire.android.feature.SwitchAccountParam import com.wire.android.util.dispatchers.DispatcherProvider import com.wire.kalium.logic.data.user.UserId @@ -59,15 +60,17 @@ class CallActivityViewModel @Inject constructor( } } - fun switchAccountIfNeeded(userId: UserId) { - viewModelScope.launch(Dispatchers.IO) { + fun switchAccountIfNeeded(userId: UserId, actions: SwitchAccountActions) { + viewModelScope.launch(Dispatchers.IO) { val shouldSwitchAccount = when (val result = currentSession()) { is CurrentSessionResult.Failure.Generic -> true CurrentSessionResult.Failure.SessionNotFound -> true is CurrentSessionResult.Success -> result.accountInfo.userId != userId } if (shouldSwitchAccount) { - accountSwitch(SwitchAccountParam.SwitchToAccount(userId)) + accountSwitch(SwitchAccountParam.SwitchToAccount(userId)).also { + it.callAction(actions) + } } } } diff --git a/app/src/main/kotlin/com/wire/android/util/SwitchAccountObserver.kt b/app/src/main/kotlin/com/wire/android/util/SwitchAccountObserver.kt new file mode 100644 index 00000000000..b0b9cd85893 --- /dev/null +++ b/app/src/main/kotlin/com/wire/android/util/SwitchAccountObserver.kt @@ -0,0 +1,57 @@ +/* + * Wire + * Copyright (C) 2024 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.util + +import com.wire.android.feature.SwitchAccountActions +import javax.inject.Inject +import javax.inject.Singleton + + +@Singleton +class SwitchAccountObserver @Inject constructor() : SwitchAccountActions { + private val lock = Object() + private val items = mutableListOf() + + fun register(actions: SwitchAccountActions) { + synchronized(lock) { + items.add(actions) + } + } + + fun unregister(actions: SwitchAccountActions) { + synchronized(lock) { + items.remove(actions) + } + } + + override fun switchedToAnotherAccount() { + synchronized(lock) { + items.forEach { + it.switchedToAnotherAccount() + } + } + } + + override fun noOtherAccountToSwitch() { + synchronized(lock) { + items.forEach { + it.noOtherAccountToSwitch() + } + } + } +} diff --git a/app/src/test/kotlin/com/wire/android/ui/CallActivityViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/CallActivityViewModelTest.kt index 6f18b93c1ad..7dada2e35d0 100644 --- a/app/src/test/kotlin/com/wire/android/ui/CallActivityViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/CallActivityViewModelTest.kt @@ -20,6 +20,8 @@ package com.wire.android.ui import com.wire.android.config.TestDispatcherProvider import com.wire.android.di.ObserveScreenshotCensoringConfigUseCaseProvider import com.wire.android.feature.AccountSwitchUseCase +import com.wire.android.feature.NavigationSwitchAccountActions +import com.wire.android.feature.SwitchAccountActions import com.wire.android.feature.SwitchAccountResult import com.wire.android.ui.calling.CallActivityViewModel import com.wire.kalium.logic.data.auth.AccountInfo @@ -33,6 +35,7 @@ import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every import io.mockk.impl.annotations.MockK +import io.mockk.mockk import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest @@ -87,7 +90,7 @@ class CallActivityViewModelTest { .withAccountSwitch(SwitchAccountResult.Failure) .arrange() - viewModel.switchAccountIfNeeded(userId) + viewModel.switchAccountIfNeeded(userId, NavigationSwitchAccountActions {}) advanceUntilIdle() coVerify(exactly = 1) { arrangement.accountSwitch(any()) } @@ -101,7 +104,7 @@ class CallActivityViewModelTest { .withAccountSwitch(SwitchAccountResult.SwitchedToAnotherAccount) .arrange() - viewModel.switchAccountIfNeeded(UserId("anotherUserId", "domain")) + viewModel.switchAccountIfNeeded(UserId("anotherUserId", "domain"), NavigationSwitchAccountActions {}) advanceUntilIdle() coVerify(exactly = 1) { arrangement.accountSwitch(any()) } @@ -115,11 +118,56 @@ class CallActivityViewModelTest { .withAccountSwitch(SwitchAccountResult.SwitchedToAnotherAccount) .arrange() - viewModel.switchAccountIfNeeded(userId) + viewModel.switchAccountIfNeeded(userId, NavigationSwitchAccountActions {}) coVerify(inverse = true) { arrangement.accountSwitch(any()) } } + private fun testCallingSwitchAccountActions( + switchAccountResult: SwitchAccountResult, + switchedToAnotherAccountCalled: Boolean = false, + noOtherAccountToSwitchCalled: Boolean = false, + ) = runTest { + val (_, viewModel) = Arrangement() + .withCurrentSessionReturning(CurrentSessionResult.Success(AccountInfo.Valid(UserId("user", "domain")))) + .withAccountSwitch(switchAccountResult) + .arrange() + val switchAccountActions = mockk() + + viewModel.switchAccountIfNeeded(UserId("anotherUser", "domain"), switchAccountActions) + + coVerify(exactly = if (switchedToAnotherAccountCalled) 1 else 0) { switchAccountActions.switchedToAnotherAccount() } + coVerify(exactly = if (noOtherAccountToSwitchCalled) 1 else 0) { switchAccountActions.noOtherAccountToSwitch() } + } + + @Test + fun `given no other account to switch, when switching, then call proper action`() = testCallingSwitchAccountActions( + switchAccountResult = SwitchAccountResult.NoOtherAccountToSwitch, + switchedToAnotherAccountCalled = false, + noOtherAccountToSwitchCalled = true, + ) + + @Test + fun `given account switched, when switching, then call proper action`() = testCallingSwitchAccountActions( + switchAccountResult = SwitchAccountResult.SwitchedToAnotherAccount, + switchedToAnotherAccountCalled = true, + noOtherAccountToSwitchCalled = false, + ) + + @Test + fun `given invalid account, when switching, then do not call any action`() = testCallingSwitchAccountActions( + switchAccountResult = SwitchAccountResult.GivenAccountIsInvalid, + switchedToAnotherAccountCalled = false, + noOtherAccountToSwitchCalled = false, + ) + + @Test + fun `given failure, when switching, then do not call any action`() = testCallingSwitchAccountActions( + switchAccountResult = SwitchAccountResult.Failure, + switchedToAnotherAccountCalled = false, + noOtherAccountToSwitchCalled = false, + ) + private class Arrangement { @MockK diff --git a/app/src/test/kotlin/com/wire/android/ui/WireActivityViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/WireActivityViewModelTest.kt index 4b492a219b3..f76d1e4fd3a 100644 --- a/app/src/test/kotlin/com/wire/android/ui/WireActivityViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/WireActivityViewModelTest.kt @@ -23,6 +23,7 @@ package com.wire.android.ui import android.content.Intent import androidx.work.WorkManager import androidx.work.impl.OperationImpl +import app.cash.turbine.test import com.wire.android.config.CoroutineTestExtension import com.wire.android.config.TestDispatcherProvider import com.wire.android.config.mockUri @@ -56,6 +57,7 @@ import com.wire.kalium.logic.data.conversation.Conversation import com.wire.kalium.logic.data.id.ConversationId import com.wire.kalium.logic.data.id.QualifiedID import com.wire.kalium.logic.data.logout.LogoutReason +import com.wire.kalium.logic.data.sync.SyncState import com.wire.kalium.logic.data.user.UserId import com.wire.kalium.logic.feature.appVersioning.ObserveIfAppUpdateRequiredUseCase import com.wire.kalium.logic.feature.call.usecase.ObserveEstablishedCallsUseCase @@ -88,6 +90,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.map import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import org.amshove.kluent.internal.assertEquals @@ -635,6 +638,50 @@ class WireActivityViewModelTest { assertEquals(false, viewModel.globalAppState.screenshotCensoringEnabled) } + @Test + fun `given session changes, when observing screenshot censoring, then update screenshot censoring state`() = runTest { + val firstSession = AccountInfo.Valid(UserId("user1", "domain1")) + val secondSession = AccountInfo.Valid(UserId("user2", "domain2")) + val firstSessionScreenshotCensoringConfig = ObserveScreenshotCensoringConfigResult.Disabled + val secondSessionScreenshotCensoringConfig = ObserveScreenshotCensoringConfigResult.Enabled.ChosenByUser + val currentSessionFlow = MutableStateFlow(firstSession) + val (_, viewModel) = Arrangement() + .withCurrentSessionFlow(currentSessionFlow.map { CurrentSessionResult.Success(it) }) + .withScreenshotCensoringConfigForUser(firstSession.userId, firstSessionScreenshotCensoringConfig) + .withScreenshotCensoringConfigForUser(secondSession.userId, secondSessionScreenshotCensoringConfig) + .arrange() + advanceUntilIdle() + assertEquals(false, viewModel.globalAppState.screenshotCensoringEnabled) + + currentSessionFlow.emit(secondSession) + advanceUntilIdle() + assertEquals(true, viewModel.globalAppState.screenshotCensoringEnabled) + } + + @Test + fun `given session changes, when observing sync state, then update sync state`() = runTest { + val firstSession = AccountInfo.Valid(UserId("user1", "domain1")) + val secondSession = AccountInfo.Valid(UserId("user2", "domain2")) + val firstSessionSyncState = SyncState.Live + val secondSessionSyncState = SyncState.SlowSync + val currentSessionFlow = MutableStateFlow(firstSession) + val (_, viewModel) = Arrangement() + .withCurrentSessionFlow(currentSessionFlow.map { CurrentSessionResult.Success(it) }) + .withSyncStateForUser(firstSession.userId, firstSessionSyncState) + .withSyncStateForUser(secondSession.userId, secondSessionSyncState) + .arrange() + advanceUntilIdle() + viewModel.observeSyncFlowState.test { + assertEquals(firstSessionSyncState, awaitItem()) + + currentSessionFlow.emit(secondSession) + advanceUntilIdle() + assertEquals(secondSessionSyncState, awaitItem()) + + expectNoEvents() + } + } + @Test fun `given app theme change, when observing it, then update state with theme option`() = runTest { val (_, viewModel) = Arrangement() @@ -806,6 +853,10 @@ class WireActivityViewModelTest { return this } + fun withCurrentSessionFlow(result: Flow): Arrangement = apply { + coEvery { currentSessionFlow() } returns result + } + fun withDeepLinkResult(result: DeepLinkResult, isSharingIntent: Boolean = false): Arrangement { coEvery { deepLinkProcessor(any(), isSharingIntent) } returns result return this @@ -877,6 +928,18 @@ class WireActivityViewModelTest { coEvery { observeScreenshotCensoringConfigUseCase() } returns flowOf(result) } + suspend fun withScreenshotCensoringConfigForUser(id: UserId, result: ObserveScreenshotCensoringConfigResult) = apply { + val useCase = mockk() + coEvery { observeScreenshotCensoringConfigUseCaseProviderFactory.create(id).observeScreenshotCensoringConfig } returns useCase + coEvery { useCase() } returns flowOf(result) + } + + suspend fun withSyncStateForUser(id: UserId, result: SyncState) = apply { + val useCase = mockk() + coEvery { observeSyncStateUseCaseProviderFactory.create(id).observeSyncState } returns useCase + coEvery { useCase() } returns flowOf(result) + } + suspend fun withThemeOption(themeOption: ThemeOption) = apply { coEvery { globalDataStore.selectedThemeOptionFlow() } returns flowOf(themeOption) } From 3e08e784fc7ba8daf0fa5dea777766f72d97ca8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Wed, 4 Dec 2024 17:20:49 +0100 Subject: [PATCH 2/5] fix detekt --- .../kotlin/com/wire/android/util/SwitchAccountObserver.kt | 1 - .../kotlin/com/wire/android/ui/WireActivityViewModelTest.kt | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/main/kotlin/com/wire/android/util/SwitchAccountObserver.kt b/app/src/main/kotlin/com/wire/android/util/SwitchAccountObserver.kt index b0b9cd85893..ad75b3a7eb4 100644 --- a/app/src/main/kotlin/com/wire/android/util/SwitchAccountObserver.kt +++ b/app/src/main/kotlin/com/wire/android/util/SwitchAccountObserver.kt @@ -21,7 +21,6 @@ import com.wire.android.feature.SwitchAccountActions import javax.inject.Inject import javax.inject.Singleton - @Singleton class SwitchAccountObserver @Inject constructor() : SwitchAccountActions { private val lock = Object() diff --git a/app/src/test/kotlin/com/wire/android/ui/WireActivityViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/WireActivityViewModelTest.kt index f76d1e4fd3a..af4c3b5944b 100644 --- a/app/src/test/kotlin/com/wire/android/ui/WireActivityViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/WireActivityViewModelTest.kt @@ -930,7 +930,9 @@ class WireActivityViewModelTest { suspend fun withScreenshotCensoringConfigForUser(id: UserId, result: ObserveScreenshotCensoringConfigResult) = apply { val useCase = mockk() - coEvery { observeScreenshotCensoringConfigUseCaseProviderFactory.create(id).observeScreenshotCensoringConfig } returns useCase + coEvery { + observeScreenshotCensoringConfigUseCaseProviderFactory.create(id).observeScreenshotCensoringConfig + } returns useCase coEvery { useCase() } returns flowOf(result) } From 57fdb196f74e71aee94b6483948bf36bea962935 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Wed, 4 Dec 2024 18:08:29 +0100 Subject: [PATCH 3/5] show full screen intent only for current session --- .../notification/CallNotificationManager.kt | 34 +++++++++--- .../CallNotificationManagerTest.kt | 53 ++++++++++++++++++- 2 files changed, 77 insertions(+), 10 deletions(-) diff --git a/app/src/main/kotlin/com/wire/android/notification/CallNotificationManager.kt b/app/src/main/kotlin/com/wire/android/notification/CallNotificationManager.kt index a01b02beea0..6b6ba3108c8 100644 --- a/app/src/main/kotlin/com/wire/android/notification/CallNotificationManager.kt +++ b/app/src/main/kotlin/com/wire/android/notification/CallNotificationManager.kt @@ -27,14 +27,17 @@ import androidx.core.app.NotificationCompat.FOREGROUND_SERVICE_IMMEDIATE import androidx.core.app.NotificationManagerCompat import com.wire.android.R import com.wire.android.appLogger +import com.wire.android.di.KaliumCoreLogic import com.wire.android.notification.NotificationConstants.INCOMING_CALL_ID_PREFIX import com.wire.android.util.dispatchers.DispatcherProvider +import com.wire.kalium.logic.CoreLogic import com.wire.kalium.logic.data.call.Call import com.wire.kalium.logic.data.call.CallStatus import com.wire.kalium.logic.data.conversation.Conversation import com.wire.kalium.logic.data.id.ConversationId import com.wire.kalium.logic.data.id.QualifiedID import com.wire.kalium.logic.data.user.UserId +import com.wire.kalium.logic.feature.session.CurrentSessionResult import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.flow.MutableSharedFlow @@ -57,9 +60,10 @@ import javax.inject.Singleton @Singleton @Suppress("TooManyFunctions") class CallNotificationManager @Inject constructor( - private val context: Context, + context: Context, dispatcherProvider: DispatcherProvider, val builder: CallNotificationBuilder, + @KaliumCoreLogic private val coreLogic: CoreLogic, ) { private val notificationManager = NotificationManagerCompat.from(context) @@ -97,8 +101,19 @@ class CallNotificationManager @Inject constructor( hideOutdatedIncomingCallNotifications(allCurrentCalls) // show current incoming call notifications appLogger.i("$TAG: showing ${newCalls.size} new incoming calls (all incoming calls: ${allCurrentCalls.size})") + + val currentSessionId = (coreLogic.getGlobalScope().session.currentSession() as? CurrentSessionResult.Success)?.let { + if (it.accountInfo.isValid()) it.accountInfo.userId else null + } newCalls.forEach { data -> - showIncomingCallNotification(data) + /** + * For now only show full screen intent for current session, as if shown for another session it will switch to that + * session and the user won't know that he/she is receiving a call as a different account. + * For calls that are not for the current session it will show the notification as a heads up notification. + * In the future we can implement showing on the incoming call screen as what account the user will answer + * or even give them the option to change it themselves on that screen. + */ + showIncomingCallNotification(data = data, asFullScreenIntent = currentSessionId == data.userId) } } } @@ -157,14 +172,14 @@ class CallNotificationManager @Inject constructor( @SuppressLint("MissingPermission") @VisibleForTesting - internal fun showIncomingCallNotification(data: CallNotificationData) { + internal fun showIncomingCallNotification(data: CallNotificationData, asFullScreenIntent: Boolean) { appLogger.i( "$TAG: showing incoming call notification for user ${data.userId.toLogString()}" + " and conversation ${data.conversationId.toLogString()}" ) val tag = NotificationConstants.getIncomingCallTag(data.userId.toString()) val id = NotificationConstants.getIncomingCallId(data.userId.toString(), data.conversationId.toString()) - val notification = builder.getIncomingCallNotification(data) + val notification = builder.getIncomingCallNotification(data, asFullScreenIntent) notificationManager.notify(tag, id, notification) } @@ -202,13 +217,12 @@ class CallNotificationBuilder @Inject constructor( .setVisibility(NotificationCompat.VISIBILITY_PUBLIC) .setForegroundServiceBehavior(FOREGROUND_SERVICE_IMMEDIATE) .addAction(getHangUpCallAction(context, conversationIdString, userIdString)) - .setFullScreenIntent(outgoingCallPendingIntent(context, conversationIdString), true) .setContentIntent(outgoingCallPendingIntent(context, conversationIdString)) .setDeleteIntent(callNotificationDismissedPendingIntent(context, userIdString, conversationIdString)) .build() } - fun getIncomingCallNotification(data: CallNotificationData): Notification { + fun getIncomingCallNotification(data: CallNotificationData, asFullScreenIntent: Boolean): Notification { val conversationIdString = data.conversationId.toString() val userIdString = data.userId.toString() val title = getNotificationTitle(data) @@ -228,9 +242,14 @@ class CallNotificationBuilder @Inject constructor( .setVisibility(NotificationCompat.VISIBILITY_PUBLIC) .addAction(getDeclineCallAction(context, conversationIdString, userIdString)) .addAction(getOpenIncomingCallAction(context, conversationIdString, userIdString)) - .setFullScreenIntent(fullScreenIncomingCallPendingIntent(context, conversationIdString, userIdString), true) + .setContentIntent(fullScreenIncomingCallPendingIntent(context, conversationIdString, userIdString)) .setDeleteIntent(callNotificationDismissedPendingIntent(context, userIdString, conversationIdString)) + .let { + if (asFullScreenIntent) { + it.setFullScreenIntent(fullScreenIncomingCallPendingIntent(context, conversationIdString, userIdString), true) + } else it + } .build() // Added FLAG_INSISTENT so the ringing sound repeats itself until an action is done. @@ -259,7 +278,6 @@ class CallNotificationBuilder @Inject constructor( .setForegroundServiceBehavior(FOREGROUND_SERVICE_IMMEDIATE) .addAction(getHangUpCallAction(context, conversationIdString, userIdString)) .addAction(getOpenOngoingCallAction(context, conversationIdString)) - .setFullScreenIntent(openOngoingCallPendingIntent(context, conversationIdString), true) .setContentIntent(openOngoingCallPendingIntent(context, conversationIdString)) .setDeleteIntent(callNotificationDismissedPendingIntent(context, userIdString, conversationIdString)) .build() diff --git a/app/src/test/kotlin/com/wire/android/notification/CallNotificationManagerTest.kt b/app/src/test/kotlin/com/wire/android/notification/CallNotificationManagerTest.kt index c98e8ca99ad..95ec0fcf680 100644 --- a/app/src/test/kotlin/com/wire/android/notification/CallNotificationManagerTest.kt +++ b/app/src/test/kotlin/com/wire/android/notification/CallNotificationManagerTest.kt @@ -23,13 +23,17 @@ import android.service.notification.StatusBarNotification import androidx.core.app.NotificationManagerCompat import com.wire.android.config.TestDispatcherProvider import com.wire.android.notification.CallNotificationManager.Companion.DEBOUNCE_TIME +import com.wire.kalium.logic.CoreLogic +import com.wire.kalium.logic.data.auth.AccountInfo import com.wire.kalium.logic.data.call.Call import com.wire.kalium.logic.data.call.CallStatus import com.wire.kalium.logic.data.conversation.Conversation import com.wire.kalium.logic.data.id.ConversationId import com.wire.kalium.logic.data.user.UserId +import com.wire.kalium.logic.feature.session.CurrentSessionResult import io.mockk.MockKAnnotations import io.mockk.clearMocks +import io.mockk.coEvery import io.mockk.every import io.mockk.impl.annotations.MockK import io.mockk.mockk @@ -351,6 +355,42 @@ class CallNotificationManagerTest { verify(exactly = 1) { arrangement.notificationManager.cancel(tag, id) } } + @Test + fun `given incoming call for current session, when handling incoming call, then show it as full screen intent`() = + runTest(dispatcherProvider.main()) { + // given + val currentSession = AccountInfo.Valid(UserId("currentUserId", "domain")) + val userName = "user name" + val (arrangement, callNotificationManager) = Arrangement() + .withCurrentSession(currentSession) + .arrange() + // when + callNotificationManager.handleIncomingCalls(listOf(TEST_CALL1), currentSession.userId, userName) + advanceUntilIdle() + // then + verify(exactly = 1) { + arrangement.callNotificationBuilder.getIncomingCallNotification(data = any(), asFullScreenIntent = eq(true)) + } + } + + @Test + fun `given incoming call for another session, when handling incoming call, then do not show it as full screen intent`() = + runTest(dispatcherProvider.main()) { + // given + val currentSession = AccountInfo.Valid(UserId("currentUserId", "domain")) + val userName = "user name" + val (arrangement, callNotificationManager) = Arrangement() + .withCurrentSession(currentSession) + .arrange() + // when + callNotificationManager.handleIncomingCalls(listOf(TEST_CALL1), TEST_USER_ID1, userName) + advanceUntilIdle() + // then + verify(exactly = 1) { + arrangement.callNotificationBuilder.getIncomingCallNotification(data = any(), asFullScreenIntent = eq(false)) + } + } + private inner class Arrangement { @MockK @@ -362,11 +402,16 @@ class CallNotificationManagerTest { @MockK lateinit var callNotificationBuilder: CallNotificationBuilder + @MockK + lateinit var coreLogic: CoreLogic + init { MockKAnnotations.init(this, relaxUnitFun = true) mockkStatic(NotificationManagerCompat::from) every { NotificationManagerCompat.from(any()) } returns notificationManager withActiveNotifications(emptyList()) + every { callNotificationBuilder.getIncomingCallNotification(any(), any()) } returns mockk() + withCurrentSession(AccountInfo.Valid(UserId("userId", "domain"))) } fun clearRecordedCallsForNotificationManager() { @@ -381,14 +426,18 @@ class CallNotificationManagerTest { } fun withIncomingNotificationForUserAndCall(notification: Notification, forCallNotificationData: CallNotificationData) = apply { - every { callNotificationBuilder.getIncomingCallNotification(eq(forCallNotificationData)) } returns notification + every { callNotificationBuilder.getIncomingCallNotification(eq(forCallNotificationData), any()) } returns notification } fun withActiveNotifications(list: List) = apply { every { notificationManager.activeNotifications } returns list } - fun arrange() = this to CallNotificationManager(context, dispatcherProvider, callNotificationBuilder) + fun withCurrentSession(accountInfo: AccountInfo) = apply { + coEvery { coreLogic.getGlobalScope().session.currentSession() } returns CurrentSessionResult.Success(accountInfo) + } + + fun arrange() = this to CallNotificationManager(context, dispatcherProvider, callNotificationBuilder, coreLogic) } companion object { From fa902fd8421e9adce0840ed8d28dd10dbaf8de7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Wed, 4 Dec 2024 18:23:16 +0100 Subject: [PATCH 4/5] fix tests --- .../android/ui/CallActivityViewModelTest.kt | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/app/src/test/kotlin/com/wire/android/ui/CallActivityViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/CallActivityViewModelTest.kt index 7dada2e35d0..fc78d92c815 100644 --- a/app/src/test/kotlin/com/wire/android/ui/CallActivityViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/CallActivityViewModelTest.kt @@ -20,7 +20,6 @@ package com.wire.android.ui import com.wire.android.config.TestDispatcherProvider import com.wire.android.di.ObserveScreenshotCensoringConfigUseCaseProvider import com.wire.android.feature.AccountSwitchUseCase -import com.wire.android.feature.NavigationSwitchAccountActions import com.wire.android.feature.SwitchAccountActions import com.wire.android.feature.SwitchAccountResult import com.wire.android.ui.calling.CallActivityViewModel @@ -35,7 +34,6 @@ import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every import io.mockk.impl.annotations.MockK -import io.mockk.mockk import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest @@ -90,7 +88,7 @@ class CallActivityViewModelTest { .withAccountSwitch(SwitchAccountResult.Failure) .arrange() - viewModel.switchAccountIfNeeded(userId, NavigationSwitchAccountActions {}) + viewModel.switchAccountIfNeeded(userId, arrangement.switchAccountActions) advanceUntilIdle() coVerify(exactly = 1) { arrangement.accountSwitch(any()) } @@ -104,7 +102,7 @@ class CallActivityViewModelTest { .withAccountSwitch(SwitchAccountResult.SwitchedToAnotherAccount) .arrange() - viewModel.switchAccountIfNeeded(UserId("anotherUserId", "domain"), NavigationSwitchAccountActions {}) + viewModel.switchAccountIfNeeded(UserId("anotherUserId", "domain"), arrangement.switchAccountActions) advanceUntilIdle() coVerify(exactly = 1) { arrangement.accountSwitch(any()) } @@ -118,7 +116,7 @@ class CallActivityViewModelTest { .withAccountSwitch(SwitchAccountResult.SwitchedToAnotherAccount) .arrange() - viewModel.switchAccountIfNeeded(userId, NavigationSwitchAccountActions {}) + viewModel.switchAccountIfNeeded(userId, arrangement.switchAccountActions) coVerify(inverse = true) { arrangement.accountSwitch(any()) } } @@ -128,16 +126,15 @@ class CallActivityViewModelTest { switchedToAnotherAccountCalled: Boolean = false, noOtherAccountToSwitchCalled: Boolean = false, ) = runTest { - val (_, viewModel) = Arrangement() + val (arrangement, viewModel) = Arrangement() .withCurrentSessionReturning(CurrentSessionResult.Success(AccountInfo.Valid(UserId("user", "domain")))) .withAccountSwitch(switchAccountResult) .arrange() - val switchAccountActions = mockk() - viewModel.switchAccountIfNeeded(UserId("anotherUser", "domain"), switchAccountActions) + viewModel.switchAccountIfNeeded(UserId("anotherUser", "domain"), arrangement.switchAccountActions) - coVerify(exactly = if (switchedToAnotherAccountCalled) 1 else 0) { switchAccountActions.switchedToAnotherAccount() } - coVerify(exactly = if (noOtherAccountToSwitchCalled) 1 else 0) { switchAccountActions.noOtherAccountToSwitch() } + coVerify(exactly = if (switchedToAnotherAccountCalled) 1 else 0) { arrangement.switchAccountActions.switchedToAnotherAccount() } + coVerify(exactly = if (noOtherAccountToSwitchCalled) 1 else 0) { arrangement.switchAccountActions.noOtherAccountToSwitch() } } @Test @@ -183,6 +180,9 @@ class CallActivityViewModelTest { @MockK private lateinit var observeScreenshotCensoringConfig: ObserveScreenshotCensoringConfigUseCase + @MockK + lateinit var switchAccountActions: SwitchAccountActions + init { MockKAnnotations.init(this, relaxUnitFun = true) From 711b25879294fde325140a8cc21a4e9599e7788e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Wed, 4 Dec 2024 18:28:25 +0100 Subject: [PATCH 5/5] fix detekt --- .../wire/android/notification/CallNotificationManager.kt | 5 +++-- .../com/wire/android/ui/CallActivityViewModelTest.kt | 8 ++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/app/src/main/kotlin/com/wire/android/notification/CallNotificationManager.kt b/app/src/main/kotlin/com/wire/android/notification/CallNotificationManager.kt index 6b6ba3108c8..822e75f4205 100644 --- a/app/src/main/kotlin/com/wire/android/notification/CallNotificationManager.kt +++ b/app/src/main/kotlin/com/wire/android/notification/CallNotificationManager.kt @@ -242,13 +242,14 @@ class CallNotificationBuilder @Inject constructor( .setVisibility(NotificationCompat.VISIBILITY_PUBLIC) .addAction(getDeclineCallAction(context, conversationIdString, userIdString)) .addAction(getOpenIncomingCallAction(context, conversationIdString, userIdString)) - .setContentIntent(fullScreenIncomingCallPendingIntent(context, conversationIdString, userIdString)) .setDeleteIntent(callNotificationDismissedPendingIntent(context, userIdString, conversationIdString)) .let { if (asFullScreenIntent) { it.setFullScreenIntent(fullScreenIncomingCallPendingIntent(context, conversationIdString, userIdString), true) - } else it + } else { + it + } } .build() diff --git a/app/src/test/kotlin/com/wire/android/ui/CallActivityViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/CallActivityViewModelTest.kt index fc78d92c815..1c7345672a2 100644 --- a/app/src/test/kotlin/com/wire/android/ui/CallActivityViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/CallActivityViewModelTest.kt @@ -133,8 +133,12 @@ class CallActivityViewModelTest { viewModel.switchAccountIfNeeded(UserId("anotherUser", "domain"), arrangement.switchAccountActions) - coVerify(exactly = if (switchedToAnotherAccountCalled) 1 else 0) { arrangement.switchAccountActions.switchedToAnotherAccount() } - coVerify(exactly = if (noOtherAccountToSwitchCalled) 1 else 0) { arrangement.switchAccountActions.noOtherAccountToSwitch() } + coVerify(exactly = if (switchedToAnotherAccountCalled) 1 else 0) { + arrangement.switchAccountActions.switchedToAnotherAccount() + } + coVerify(exactly = if (noOtherAccountToSwitchCalled) 1 else 0) { + arrangement.switchAccountActions.noOtherAccountToSwitch() + } } @Test