Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PM-15873] Cherrypick: PTR remaining changes #4785

Merged
merged 2 commits into from
Feb 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -45,6 +48,7 @@ class SendViewModel @Inject constructor(
settingsRepo: SettingsRepository,
private val vaultRepo: VaultRepository,
policyManager: PolicyManager,
private val networkConnectionManager: NetworkConnectionManager,
) : BaseViewModel<SendState, SendEvent, SendAction>(
// We load the state from the savedStateHandle for testing purposes.
initialState = savedStateHandle[KEY_STATE]
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
}
}
}

Expand Down Expand Up @@ -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()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<VaultItemListingState, VaultItemListingEvent, VaultItemListingsAction>(
initialState = run {
val userState = requireNotNull(authRepository.userStateFlow.value)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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(),
),
)
}
}

Expand Down Expand Up @@ -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()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -457,6 +511,7 @@ class SendViewModelTest : BaseViewModelTest() {
settingsRepo = settingsRepository,
vaultRepo = vaultRepository,
policyManager = policyManager,
networkConnectionManager = networkConnectionManager,
)
}

Expand Down
Loading