From 30a1bba79639c8d11b93d421fbe30564a5f9cee9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Bispo?= Date: Tue, 25 Feb 2025 19:06:45 +0000 Subject: [PATCH] [PM-15873] Fix PTR in sends listing page (#4784) --- .../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 bae917f92e..92d2519689 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 787622f997..003e291e33 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, ) }