From bd9cba78cc02c7af263bca1962901e6b043dac0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Tue, 25 Feb 2025 20:52:32 +0000 Subject: [PATCH 1/2] [PM-15873] Fix PTR on item listing page (#4778) cp: ac7fbfd129ff2befac8f8b7675b7abea7cc5fa3b [ac7fbfd12] --- .../itemlisting/VaultItemListingViewModel.kt | 61 ++++++++++++--- .../VaultItemListingViewModelTest.kt | 78 ++++++++++++++++++- 2 files changed, 127 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt index f5fb19244af..21f0ceda31b 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt @@ -32,6 +32,7 @@ import com.x8bit.bitwarden.data.platform.manager.ciphermatching.CipherMatchingMa import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager import com.x8bit.bitwarden.data.platform.manager.event.OrganizationEventManager import com.x8bit.bitwarden.data.platform.manager.model.OrganizationEvent +import com.x8bit.bitwarden.data.platform.manager.network.NetworkConnectionManager import com.x8bit.bitwarden.data.platform.manager.util.toAutofillSelectionDataOrNull import com.x8bit.bitwarden.data.platform.manager.util.toFido2AssertionRequestOrNull import com.x8bit.bitwarden.data.platform.manager.util.toFido2CreateRequestOrNull @@ -83,6 +84,7 @@ import com.x8bit.bitwarden.ui.vault.model.VaultItemCipherType import com.x8bit.bitwarden.ui.vault.util.toVaultItemCipherType import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.collections.immutable.ImmutableList +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach @@ -114,6 +116,7 @@ class VaultItemListingViewModel @Inject constructor( private val fido2OriginManager: Fido2OriginManager, private val fido2CredentialManager: Fido2CredentialManager, private val organizationEventManager: OrganizationEventManager, + private val networkConnectionManager: NetworkConnectionManager, ) : BaseViewModel( initialState = run { val userState = requireNotNull(authRepository.userStateFlow.value) @@ -306,11 +309,17 @@ class VaultItemListingViewModel @Inject constructor( vaultRepository.sync(forced = true) } + @Suppress("MagicNumber") private fun handleRefreshPull() { mutableStateFlow.update { it.copy(isRefreshing = true) } - // The Pull-To-Refresh composable is already in the refreshing state. - // We will reset that state when sendDataStateFlow emits later on. - vaultRepository.sync(forced = false) + viewModelScope.launch { + delay(250) + if (networkConnectionManager.isNetworkConnected) { + vaultRepository.sync(forced = false) + } else { + sendAction(VaultItemListingsAction.Internal.InternetConnectionErrorReceived) + } + } } private fun handleConfirmOverwriteExistingPasskeyClick( @@ -1018,14 +1027,25 @@ class VaultItemListingViewModel @Inject constructor( } private fun handleSyncClick() { - mutableStateFlow.update { - it.copy( - dialogState = VaultItemListingState.DialogState.Loading( - message = R.string.syncing.asText(), - ), - ) + if (networkConnectionManager.isNetworkConnected) { + mutableStateFlow.update { + it.copy( + dialogState = VaultItemListingState.DialogState.Loading( + message = R.string.syncing.asText(), + ), + ) + } + vaultRepository.sync(forced = true) + } else { + mutableStateFlow.update { + it.copy( + dialogState = VaultItemListingState.DialogState.Error( + R.string.internet_connection_required_title.asText(), + R.string.internet_connection_required_message.asText(), + ), + ) + } } - vaultRepository.sync(forced = true) } private fun handleSearchIconClick() { @@ -1150,6 +1170,22 @@ class VaultItemListingViewModel @Inject constructor( is VaultItemListingsAction.Internal.Fido2AssertionResultReceive -> { handleFido2AssertionResultReceive(action) } + + VaultItemListingsAction.Internal.InternetConnectionErrorReceived -> { + handleInternetConnectionErrorReceived() + } + } + } + + private fun handleInternetConnectionErrorReceived() { + mutableStateFlow.update { + it.copy( + isRefreshing = false, + dialogState = VaultItemListingState.DialogState.Error( + R.string.internet_connection_required_title.asText(), + R.string.internet_connection_required_message.asText(), + ), + ) } } @@ -2768,6 +2804,11 @@ sealed class VaultItemListingsAction { data class Fido2AssertionResultReceive( val result: Fido2CredentialAssertionResult, ) : Internal() + + /** + * Indicates that the there is not internet connection. + */ + data object InternetConnectionErrorReceived : Internal() } } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt index 8d9ae23524d..97189f1e32c 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt @@ -42,6 +42,7 @@ import com.x8bit.bitwarden.data.platform.manager.event.OrganizationEventManager import com.x8bit.bitwarden.data.platform.manager.model.FirstTimeState import com.x8bit.bitwarden.data.platform.manager.model.OrganizationEvent import com.x8bit.bitwarden.data.platform.manager.model.SpecialCircumstance +import com.x8bit.bitwarden.data.platform.manager.network.NetworkConnectionManager import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.platform.repository.model.DataState @@ -89,8 +90,10 @@ import io.mockk.runs import io.mockk.unmockkStatic import io.mockk.verify import kotlinx.collections.immutable.persistentListOf +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertFalse @@ -185,6 +188,10 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { every { trackEvent(event = any()) } just runs } + private val networkConnectionManager: NetworkConnectionManager = mockk { + every { isNetworkConnected } returns true + } + private val initialState = createVaultItemListingState() private val initialSavedStateHandle = createSavedStateHandleWithVaultItemListingType( vaultItemListingType = VaultItemListingType.Login, @@ -363,6 +370,27 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { } } + @Test + fun `on SyncClick should show the no network dialog if no connection is available`() { + val viewModel = createVaultItemListingViewModel() + every { + networkConnectionManager.isNetworkConnected + } returns false + viewModel.trySendAction(VaultItemListingsAction.SyncClick) + assertEquals( + initialState.copy( + dialogState = VaultItemListingState.DialogState.Error( + R.string.internet_connection_required_title.asText(), + R.string.internet_connection_required_message.asText(), + ), + ), + viewModel.stateFlow.value, + ) + verify(exactly = 0) { + vaultRepository.sync(forced = true) + } + } + @Suppress("MaxLineLength") @Test fun `ItemClick for vault item when accessibility autofill should post to the accessibilitySelectionManager`() = @@ -2451,17 +2479,43 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { assertTrue(viewModel.stateFlow.value.isIconLoadingDisabled) } + @OptIn(ExperimentalCoroutinesApi::class) @Test - fun `RefreshPull should call vault repository sync`() { + fun `RefreshPull should call vault repository sync`() = runTest { val viewModel = createVaultItemListingViewModel() viewModel.trySendAction(VaultItemListingsAction.RefreshPull) - + advanceTimeBy(300) verify(exactly = 1) { vaultRepository.sync(forced = false) } } + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun `RefreshPull should show network error if no internet connection`() = runTest { + val viewModel = createVaultItemListingViewModel() + every { + networkConnectionManager.isNetworkConnected + } returns false + + viewModel.trySendAction(VaultItemListingsAction.RefreshPull) + advanceTimeBy(300) + assertEquals( + initialState.copy( + isRefreshing = false, + dialogState = VaultItemListingState.DialogState.Error( + R.string.internet_connection_required_title.asText(), + R.string.internet_connection_required_message.asText(), + ), + ), + viewModel.stateFlow.value, + ) + verify(exactly = 0) { + vaultRepository.sync(forced = false) + } + } + @Test fun `PullToRefreshEnableReceive should update isPullToRefreshEnabled`() = runTest { val viewModel = createVaultItemListingViewModel() @@ -4461,6 +4515,25 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { ) } + @Test + fun `InternetConnectionErrorReceived should show network error if no internet connection`() = + runTest { + val viewModel = createVaultItemListingViewModel() + viewModel.trySendAction( + VaultItemListingsAction.Internal.InternetConnectionErrorReceived, + ) + assertEquals( + initialState.copy( + isRefreshing = false, + dialogState = VaultItemListingState.DialogState.Error( + R.string.internet_connection_required_title.asText(), + R.string.internet_connection_required_message.asText(), + ), + ), + viewModel.stateFlow.value, + ) + } + @Suppress("CyclomaticComplexMethod") private fun createSavedStateHandleWithVaultItemListingType( vaultItemListingType: VaultItemListingType, @@ -4523,6 +4596,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { fido2CredentialManager = fido2CredentialManager, organizationEventManager = organizationEventManager, fido2OriginManager = fido2OriginManager, + networkConnectionManager = networkConnectionManager, ) @Suppress("MaxLineLength") From 80dbbf97563b865a5723a71c97fb3166ebd28d95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Tue, 25 Feb 2025 20:54:06 +0000 Subject: [PATCH 2/2] [PM-15873] Fix PTR in sends listing page (#4784) commit: 30a1bba79639c8d11b93d421fbe30564a5f9cee9 [30a1bba79] --- .../ui/tools/feature/send/SendViewModel.kt | 60 ++++++++++++++++--- .../tools/feature/send/SendViewModelTest.kt | 59 +++++++++++++++++- 2 files changed, 110 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/SendViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/SendViewModel.kt index bae917f92ed..92d25196895 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/SendViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/SendViewModel.kt @@ -7,6 +7,7 @@ import androidx.lifecycle.viewModelScope import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager +import com.x8bit.bitwarden.data.platform.manager.network.NetworkConnectionManager import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.platform.repository.model.DataState @@ -22,7 +23,9 @@ import com.x8bit.bitwarden.ui.platform.base.util.asText import com.x8bit.bitwarden.ui.platform.components.model.IconRes import com.x8bit.bitwarden.ui.tools.feature.send.util.toViewState import com.x8bit.bitwarden.ui.vault.feature.item.VaultItemScreen +import com.x8bit.bitwarden.ui.vault.feature.itemlisting.VaultItemListingsAction.Internal import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach @@ -36,7 +39,7 @@ private const val KEY_STATE = "state" /** * View model for the send screen. */ -@Suppress("TooManyFunctions") +@Suppress("LongParameterList", "TooManyFunctions") @HiltViewModel class SendViewModel @Inject constructor( savedStateHandle: SavedStateHandle, @@ -45,6 +48,7 @@ class SendViewModel @Inject constructor( settingsRepo: SettingsRepository, private val vaultRepo: VaultRepository, policyManager: PolicyManager, + private val networkConnectionManager: NetworkConnectionManager, ) : BaseViewModel( // We load the state from the savedStateHandle for testing purposes. initialState = savedStateHandle[KEY_STATE] @@ -109,6 +113,22 @@ class SendViewModel @Inject constructor( is SendAction.Internal.SendDataReceive -> handleSendDataReceive(action) is SendAction.Internal.PolicyUpdateReceive -> handlePolicyUpdateReceive(action) + + SendAction.Internal.InternetConnectionErrorReceived -> { + handleInternetConnectionErrorReceived() + } + } + + private fun handleInternetConnectionErrorReceived() { + mutableStateFlow.update { + it.copy( + isRefreshing = false, + dialogState = SendState.DialogState.Error( + R.string.internet_connection_required_title.asText(), + R.string.internet_connection_required_message.asText(), + ), + ) + } } private fun handlePullToRefreshEnableReceive( @@ -250,10 +270,25 @@ class SendViewModel @Inject constructor( } private fun handleSyncClick() { - mutableStateFlow.update { - it.copy(dialogState = SendState.DialogState.Loading(R.string.syncing.asText())) + if (networkConnectionManager.isNetworkConnected) { + mutableStateFlow.update { + it.copy( + dialogState = SendState.DialogState.Loading( + message = R.string.syncing.asText(), + ), + ) + } + vaultRepo.sync(forced = true) + } else { + mutableStateFlow.update { + it.copy( + dialogState = SendState.DialogState.Error( + R.string.internet_connection_required_title.asText(), + R.string.internet_connection_required_message.asText(), + ), + ) + } } - vaultRepo.sync(forced = true) } private fun handleCopyClick(action: SendAction.CopyClick) { @@ -307,11 +342,17 @@ class SendViewModel @Inject constructor( mutableStateFlow.update { it.copy(dialogState = null) } } + @Suppress("MagicNumber") private fun handleRefreshPull() { mutableStateFlow.update { it.copy(isRefreshing = true) } - // The Pull-To-Refresh composable is already in the refreshing state. - // We will reset that state when sendDataStateFlow emits later on. - vaultRepo.sync(forced = false) + viewModelScope.launch { + delay(250) + if (networkConnectionManager.isNetworkConnected) { + vaultRepo.sync(forced = false) + } else { + sendAction(SendAction.Internal.InternetConnectionErrorReceived) + } + } } } @@ -559,6 +600,11 @@ sealed class SendAction { data class PolicyUpdateReceive( val policyDisablesSend: Boolean, ) : Internal() + + /** + * Indicates that the there is not internet connection. + */ + data object InternetConnectionErrorReceived : Internal() } } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/SendViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/SendViewModelTest.kt index 787622f9978..003e291e33e 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/SendViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/SendViewModelTest.kt @@ -5,6 +5,7 @@ import app.cash.turbine.test import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager +import com.x8bit.bitwarden.data.platform.manager.network.NetworkConnectionManager import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.platform.repository.model.DataState @@ -27,8 +28,10 @@ import io.mockk.mockkStatic import io.mockk.runs import io.mockk.unmockkStatic import io.mockk.verify +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.assertEquals @@ -58,6 +61,10 @@ class SendViewModelTest : BaseViewModelTest() { every { getActivePoliciesFlow(type = PolicyTypeJson.DISABLE_SEND) } returns emptyFlow() } + private val networkConnectionManager: NetworkConnectionManager = mockk { + every { isNetworkConnected } returns true + } + @BeforeEach fun setup() { mockkStatic(SendData::toViewState) @@ -240,6 +247,27 @@ class SendViewModelTest : BaseViewModelTest() { } } + @Test + fun `on SyncClick should show the no network dialog if no connection is available`() { + val viewModel = createViewModel() + every { + networkConnectionManager.isNetworkConnected + } returns false + viewModel.trySendAction(SendAction.SyncClick) + assertEquals( + DEFAULT_STATE.copy( + dialogState = SendState.DialogState.Error( + R.string.internet_connection_required_title.asText(), + R.string.internet_connection_required_message.asText(), + ), + ), + viewModel.stateFlow.value, + ) + verify(exactly = 0) { + vaultRepo.sync(forced = true) + } + } + @Test fun `CopyClick should call setText on the ClipboardManager`() = runTest { val viewModel = createViewModel() @@ -414,18 +442,44 @@ class SendViewModelTest : BaseViewModelTest() { ) } + @OptIn(ExperimentalCoroutinesApi::class) @Test - fun `RefreshPull should call vault repository sync`() { + fun `RefreshPull should call vault repository sync`() = runTest { every { vaultRepo.sync(forced = false) } just runs val viewModel = createViewModel() viewModel.trySendAction(SendAction.RefreshPull) - + advanceTimeBy(300) verify(exactly = 1) { vaultRepo.sync(forced = false) } } + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun `RefreshPull should show network error if no internet connection`() = runTest { + val viewModel = createViewModel() + every { + networkConnectionManager.isNetworkConnected + } returns false + + viewModel.trySendAction(SendAction.RefreshPull) + advanceTimeBy(300) + assertEquals( + DEFAULT_STATE.copy( + isRefreshing = false, + dialogState = SendState.DialogState.Error( + R.string.internet_connection_required_title.asText(), + R.string.internet_connection_required_message.asText(), + ), + ), + viewModel.stateFlow.value, + ) + verify(exactly = 0) { + vaultRepo.sync(forced = false) + } + } + @Test fun `PullToRefreshEnableReceive should update isPullToRefreshEnabled`() = runTest { val viewModel = createViewModel() @@ -457,6 +511,7 @@ class SendViewModelTest : BaseViewModelTest() { settingsRepo = settingsRepository, vaultRepo = vaultRepository, policyManager = policyManager, + networkConnectionManager = networkConnectionManager, ) }