[PM-28990] Skipping vault migration on Network or Timeout error (#6393)

This commit is contained in:
aj-rosado 2026-01-23 16:06:17 +00:00 committed by GitHub
parent 0395d489c2
commit 9f1fad8be0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 323 additions and 38 deletions

View File

@ -68,6 +68,7 @@ fun LeaveOrganizationScreen(
LeaveOrganizationDialogs(
dialogState = state.dialogState,
onDismissRequest = handlers.onDismissDialog,
onDismissNoNetworkRequest = handlers.onDismissNoNetworkDialog,
)
val scrollBehavior = TopAppBarDefaults.pinnedScrollBehavior(rememberTopAppBarState())
@ -98,6 +99,7 @@ fun LeaveOrganizationScreen(
private fun LeaveOrganizationDialogs(
dialogState: LeaveOrganizationState.DialogState?,
onDismissRequest: () -> Unit,
onDismissNoNetworkRequest: () -> Unit,
) {
when (dialogState) {
LeaveOrganizationState.DialogState.Loading -> {
@ -108,13 +110,22 @@ private fun LeaveOrganizationDialogs(
is LeaveOrganizationState.DialogState.Error -> {
BitwardenBasicDialog(
title = stringResource(id = BitwardenString.an_error_has_occurred),
title = dialogState.title(),
message = dialogState.message(),
throwable = dialogState.error,
onDismissRequest = onDismissRequest,
)
}
is LeaveOrganizationState.DialogState.NoNetwork -> {
BitwardenBasicDialog(
title = dialogState.title(),
message = dialogState.message(),
throwable = dialogState.error,
onDismissRequest = onDismissNoNetworkRequest,
)
}
null -> Unit
}
}

View File

@ -3,6 +3,8 @@ package com.x8bit.bitwarden.ui.vault.feature.leaveorganization
import android.os.Parcelable
import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.viewModelScope
import com.bitwarden.network.util.isNoConnectionError
import com.bitwarden.network.util.isTimeoutError
import com.bitwarden.ui.platform.base.BaseViewModel
import com.bitwarden.ui.platform.components.snackbar.model.BitwardenSnackbarData
import com.bitwarden.ui.platform.manager.snackbar.SnackbarRelayManager
@ -58,6 +60,7 @@ class LeaveOrganizationViewModel @Inject constructor(
LeaveOrganizationAction.LeaveOrganizationClick -> handleLeaveOrganizationClick()
LeaveOrganizationAction.HelpLinkClick -> handleHelpLinkClick()
LeaveOrganizationAction.DismissDialog -> handleDismissDialog()
LeaveOrganizationAction.DismissNoNetworkDialog -> handleDismissNoNetworkDialog()
is LeaveOrganizationAction.Internal.RevokeFromOrganizationResultReceived -> {
handleRevokeFromOrganizationResultReceived(action)
}
@ -89,9 +92,12 @@ class LeaveOrganizationViewModel @Inject constructor(
}
private fun handleDismissDialog() {
mutableStateFlow.update {
it.copy(dialogState = null)
}
clearDialog()
}
private fun handleDismissNoNetworkDialog() {
vaultMigrationManager.clearMigrationState()
clearDialog()
}
private fun handleRevokeFromOrganizationResultReceived(
@ -118,17 +124,39 @@ class LeaveOrganizationViewModel @Inject constructor(
}
is RevokeFromOrganizationResult.Error -> {
val isNetworkError = result.error.isNoConnectionError() ||
result.error.isTimeoutError()
mutableStateFlow.update {
it.copy(
dialogState = LeaveOrganizationState.DialogState.Error(
message = BitwardenString.generic_error_message.asText(),
error = result.error,
),
dialogState = if (isNetworkError) {
LeaveOrganizationState.DialogState.NoNetwork(
title = BitwardenString.internet_connection_required_title.asText(),
message = BitwardenString
.internet_connection_required_message
.asText(),
error = result.error,
)
} else {
LeaveOrganizationState.DialogState.Error(
title = BitwardenString.an_error_has_occurred.asText(),
message = BitwardenString.generic_error_message.asText(),
error = result.error,
)
},
)
}
}
}
}
private fun clearDialog() {
mutableStateFlow.update {
it.copy(
dialogState = null,
)
}
}
}
/**
@ -156,6 +184,17 @@ data class LeaveOrganizationState(
*/
@Parcelize
data class Error(
val title: Text,
val message: Text,
val error: Throwable? = null,
) : DialogState()
/**
* No network connection dialog when leave operation fails due to network issues.
*/
@Parcelize
data class NoNetwork(
val title: Text,
val message: Text,
val error: Throwable? = null,
) : DialogState()
@ -201,6 +240,11 @@ sealed class LeaveOrganizationAction {
*/
data object DismissDialog : LeaveOrganizationAction()
/**
* User dismissed the NoNetwork dialog.
*/
data object DismissNoNetworkDialog : LeaveOrganizationAction()
/**
* Internal actions for ViewModel processing.
*/

View File

@ -13,6 +13,7 @@ data class LeaveOrganizationHandler(
val onLeaveClick: () -> Unit,
val onHelpClick: () -> Unit,
val onDismissDialog: () -> Unit,
val onDismissNoNetworkDialog: () -> Unit,
) {
@Suppress("UndocumentedPublicClass")
companion object {
@ -34,6 +35,9 @@ data class LeaveOrganizationHandler(
onDismissDialog = {
viewModel.trySendAction(LeaveOrganizationAction.DismissDialog)
},
onDismissNoNetworkDialog = {
viewModel.trySendAction(LeaveOrganizationAction.DismissNoNetworkDialog)
},
)
}
}

View File

@ -65,6 +65,7 @@ fun MigrateToMyItemsScreen(
MigrateToMyItemsDialogs(
dialog = state.dialog,
onDismissRequest = handlers.onDismissDialog,
onDismissNoNetworkRequest = handlers.onDismissNoNetworkDialog,
)
BitwardenScaffold {
@ -84,6 +85,7 @@ fun MigrateToMyItemsScreen(
private fun MigrateToMyItemsDialogs(
dialog: MigrateToMyItemsState.DialogState?,
onDismissRequest: () -> Unit,
onDismissNoNetworkRequest: () -> Unit,
) {
when (dialog) {
is MigrateToMyItemsState.DialogState.Error -> {
@ -99,6 +101,15 @@ private fun MigrateToMyItemsDialogs(
BitwardenLoadingDialog(text = dialog.message())
}
is MigrateToMyItemsState.DialogState.NoNetwork -> {
BitwardenBasicDialog(
title = dialog.title(),
message = dialog.message(),
throwable = dialog.throwable,
onDismissRequest = onDismissNoNetworkRequest,
)
}
null -> Unit
}
}

View File

@ -4,6 +4,8 @@ import android.os.Parcelable
import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.viewModelScope
import com.bitwarden.core.data.repository.error.MissingPropertyException
import com.bitwarden.network.util.isNoConnectionError
import com.bitwarden.network.util.isTimeoutError
import com.bitwarden.ui.platform.base.BaseViewModel
import com.bitwarden.ui.platform.resource.BitwardenString
import com.bitwarden.ui.util.Text
@ -61,6 +63,9 @@ class MigrateToMyItemsViewModel @Inject constructor(
MigrateToMyItemsAction.DeclineAndLeaveClicked -> handleDeclineAndLeaveClicked()
MigrateToMyItemsAction.HelpLinkClicked -> handleHelpLinkClicked()
MigrateToMyItemsAction.DismissDialogClicked -> handleDismissDialogClicked()
MigrateToMyItemsAction.NoNetworkDismissDialogClicked ->
handleNoNetworkDismissDialogClicked()
is MigrateToMyItemsAction.Internal -> handleInternalAction(action)
}
}
@ -125,6 +130,11 @@ class MigrateToMyItemsViewModel @Inject constructor(
clearDialog()
}
private fun handleNoNetworkDismissDialogClicked() {
vaultMigrationManager.clearMigrationState()
clearDialog()
}
private fun handleInternalAction(action: MigrateToMyItemsAction.Internal) {
when (action) {
is MigrateToMyItemsAction.Internal.MigrateToMyItemsResultReceived -> {
@ -149,15 +159,31 @@ class MigrateToMyItemsViewModel @Inject constructor(
is MigratePersonalVaultResult.Failure -> {
Timber.e(result.error, "Failed to migrate personal vault")
val isNetworkOrTimeoutError = result.error.isNoConnectionError() ||
result.error.isTimeoutError()
mutableStateFlow.update {
it.copy(
dialog = MigrateToMyItemsState.DialogState.Error(
title = BitwardenString.an_error_has_occurred.asText(),
message = BitwardenString.failed_to_migrate_items_to_x.asText(
it.organizationName,
),
throwable = result.error,
),
dialog =
if (isNetworkOrTimeoutError) {
MigrateToMyItemsState.DialogState.NoNetwork(
title = BitwardenString
.internet_connection_required_title
.asText(),
message = BitwardenString
.internet_connection_required_message
.asText(),
throwable = result.error,
)
} else {
MigrateToMyItemsState.DialogState.Error(
title = BitwardenString.an_error_has_occurred.asText(),
message = BitwardenString.failed_to_migrate_items_to_x.asText(
it.organizationName,
),
throwable = result.error,
)
},
)
}
}
@ -165,7 +191,11 @@ class MigrateToMyItemsViewModel @Inject constructor(
}
private fun clearDialog() {
mutableStateFlow.update { it.copy(dialog = null) }
mutableStateFlow.update {
it.copy(
dialog = null,
)
}
}
}
@ -199,6 +229,16 @@ data class MigrateToMyItemsState(
val message: Text,
val throwable: Throwable?,
) : DialogState()
/**
* No network connection dialog when migration operation fails due to network issues.
*/
@Parcelize
data class NoNetwork(
val title: Text,
val message: Text,
val throwable: Throwable? = null,
) : DialogState()
}
}
@ -249,6 +289,11 @@ sealed class MigrateToMyItemsAction {
*/
data object DismissDialogClicked : MigrateToMyItemsAction()
/**
* User dismissed the NoNetwork dialog.
*/
data object NoNetworkDismissDialogClicked : MigrateToMyItemsAction()
/**
* Models internal actions that the [MigrateToMyItemsViewModel] itself may send.
*/

View File

@ -19,6 +19,7 @@ class MigrateToMyItemsHandler(
val onDeclineClick: () -> Unit,
val onHelpClick: () -> Unit,
val onDismissDialog: () -> Unit,
val onDismissNoNetworkDialog: () -> Unit,
) {
@Suppress("UndocumentedPublicClass")
companion object {
@ -40,6 +41,9 @@ class MigrateToMyItemsHandler(
onDismissDialog = {
viewModel.trySendAction(MigrateToMyItemsAction.DismissDialogClicked)
},
onDismissNoNetworkDialog = {
viewModel.trySendAction(MigrateToMyItemsAction.NoNetworkDismissDialogClicked)
},
)
}
}

View File

@ -10,6 +10,7 @@ import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.compose.ui.test.performScrollTo
import com.bitwarden.core.data.repository.util.bufferedMutableSharedFlow
import com.bitwarden.ui.platform.resource.BitwardenString
import com.bitwarden.ui.util.asText
import com.x8bit.bitwarden.ui.platform.base.BitwardenComposeTest
import io.mockk.every
@ -135,6 +136,7 @@ class LeaveOrganizationScreenTest : BitwardenComposeTest() {
mutableStateFlow.update {
it.copy(
dialogState = LeaveOrganizationState.DialogState.Error(
title = BitwardenString.an_error_has_occurred.asText(),
message = errorMessage.asText(),
error = Throwable("Test error"),
),
@ -157,6 +159,7 @@ class LeaveOrganizationScreenTest : BitwardenComposeTest() {
mutableStateFlow.update {
it.copy(
dialogState = LeaveOrganizationState.DialogState.Error(
title = BitwardenString.an_error_has_occurred.asText(),
message = "Error message".asText(),
),
)

View File

@ -35,6 +35,7 @@ import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertNull
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import java.net.UnknownHostException
class LeaveOrganizationViewModelTest : BaseViewModelTest() {
@ -149,20 +150,69 @@ class LeaveOrganizationViewModelTest : BaseViewModelTest() {
}
@Test
fun `LeaveOrganizationClick with Error should show error dialog`() = runTest {
val error = Throwable("Test error")
coEvery {
mockAuthRepository.revokeFromOrganization(ORGANIZATION_ID)
} returns RevokeFromOrganizationResult.Error(error)
fun `DismissDialogClicked should clear dialog and clear migration state`() = runTest {
val viewModel = createViewModel()
viewModel.trySendAction(LeaveOrganizationAction.LeaveOrganizationClick)
val state = viewModel.stateFlow.value
assert(state.dialogState is LeaveOrganizationState.DialogState.Error)
val dialogState = state.dialogState as LeaveOrganizationState.DialogState.Error
assertEquals(BitwardenString.generic_error_message.asText(), dialogState.message)
assertEquals(error, dialogState.error)
viewModel.stateFlow.test {
awaitItem()
viewModel.trySendAction(
LeaveOrganizationAction.Internal.RevokeFromOrganizationResultReceived(
result = RevokeFromOrganizationResult.Error(null),
),
)
assertEquals(
DEFAULT_STATE.copy(
dialogState = LeaveOrganizationState.DialogState.Error(
title = BitwardenString.an_error_has_occurred.asText(),
message = BitwardenString.generic_error_message.asText(),
error = null,
),
), awaitItem(),
)
// Dismiss the dialog
viewModel.trySendAction(LeaveOrganizationAction.DismissDialog)
verify(exactly = 0) { mockVaultMigrationManager.clearMigrationState() }
assertEquals(
DEFAULT_STATE, awaitItem(),
)
}
}
@Test
fun `DismissNoNetworkDialogClicked should clear dialog and clear migration state`() = runTest {
val viewModel = createViewModel()
val error = UnknownHostException("No network")
viewModel.stateFlow.test {
awaitItem()
viewModel.trySendAction(
LeaveOrganizationAction.Internal.RevokeFromOrganizationResultReceived(
result = RevokeFromOrganizationResult.Error(
error = error,
),
),
)
assertEquals(
DEFAULT_STATE.copy(
dialogState = LeaveOrganizationState.DialogState.NoNetwork(
title = BitwardenString.internet_connection_required_title.asText(),
message = BitwardenString.internet_connection_required_message.asText(),
error = error,
),
), awaitItem(),
)
// Dismiss the dialog and clear migration
viewModel.trySendAction(LeaveOrganizationAction.DismissNoNetworkDialog)
verify(exactly = 1) { mockVaultMigrationManager.clearMigrationState() }
assertEquals(
DEFAULT_STATE, awaitItem(),
)
}
}
@Test
@ -202,18 +252,14 @@ class LeaveOrganizationViewModelTest : BaseViewModelTest() {
}
private fun createViewModel(
savedStateHandle: SavedStateHandle = SavedStateHandle(),
state: LeaveOrganizationState = DEFAULT_STATE,
): LeaveOrganizationViewModel {
every { savedStateHandle.toLeaveOrganizationArgs() } returns LeaveOrganizationArgs(
organizationId = ORGANIZATION_ID,
organizationName = ORGANIZATION_NAME,
)
return LeaveOrganizationViewModel(
authRepository = mockAuthRepository,
snackbarRelayManager = mockSnackbarRelayManager,
organizationEventManager = mockOrganizationEventManager,
vaultMigrationManager = mockVaultMigrationManager,
savedStateHandle = savedStateHandle,
savedStateHandle = SavedStateHandle(mapOf("state" to state)),
)
}
}
@ -258,3 +304,9 @@ private val DEFAULT_USER_STATE = UserState(
),
),
)
private val DEFAULT_STATE = LeaveOrganizationState(
organizationId = ORGANIZATION_ID,
organizationName = ORGANIZATION_NAME,
dialogState = null,
)

View File

@ -29,6 +29,7 @@ import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertNull
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import java.net.SocketTimeoutException
class MigrateToMyItemsViewModelTest : BaseViewModelTest() {
@ -44,6 +45,7 @@ class MigrateToMyItemsViewModelTest : BaseViewModelTest() {
coEvery {
migratePersonalVault(any(), any())
} returns MigratePersonalVaultResult.Success
every { clearMigrationState() } just runs
}
private val mockVaultSyncManager: VaultSyncManager = mockk(relaxed = true)
private val mockAuthRepository: AuthRepository = mockk {
@ -189,6 +191,34 @@ class MigrateToMyItemsViewModelTest : BaseViewModelTest() {
}
}
@Suppress("MaxLineLength")
@Test
fun `MigrateToMyItemsResultReceived with timeout error should show DialogState NoNetwork`() =
runTest {
val error = SocketTimeoutException("Timeout")
val viewModel = createViewModel()
viewModel.stateFlow.test {
awaitItem() // Initial state
viewModel.trySendAction(
MigrateToMyItemsAction.Internal.MigrateToMyItemsResultReceived(
result = MigratePersonalVaultResult.Failure(error),
),
)
assertEquals(
DEFAULT_STATE.copy(
dialog = MigrateToMyItemsState.DialogState.NoNetwork(
title = BitwardenString.internet_connection_required_title.asText(),
message = BitwardenString.internet_connection_required_message.asText(),
throwable = error,
),
), awaitItem(),
)
}
}
@Test
fun `DeclineAndLeaveClicked sends NavigateToLeaveOrganization event`() = runTest {
val viewModel = createViewModel()
@ -231,13 +261,59 @@ class MigrateToMyItemsViewModelTest : BaseViewModelTest() {
result = MigratePersonalVaultResult.Failure(null),
),
)
val errorState = awaitItem()
assert(errorState.dialog is MigrateToMyItemsState.DialogState.Error)
assertEquals(
DEFAULT_STATE.copy(
dialog = MigrateToMyItemsState.DialogState.Error(
title = BitwardenString.an_error_has_occurred.asText(),
message = BitwardenString.failed_to_migrate_items_to_x.asText(
ORGANIZATION_NAME,
),
throwable = null,
),
), awaitItem(),
)
// Dismiss the dialog
viewModel.trySendAction(MigrateToMyItemsAction.DismissDialogClicked)
val clearedState = awaitItem()
assertNull(clearedState.dialog)
assertEquals(
DEFAULT_STATE, awaitItem(),
)
}
}
@Suppress("MaxLineLength")
@Test
fun `NoNetworkDismissDialogClicked should clear dialog and clear migration state`() = runTest {
val viewModel = createViewModel()
val error = SocketTimeoutException("Timeout")
viewModel.stateFlow.test {
awaitItem() // Initial state
// First show an error dialog
viewModel.trySendAction(
MigrateToMyItemsAction.Internal.MigrateToMyItemsResultReceived(
result = MigratePersonalVaultResult.Failure(error = error),
),
)
assertEquals(
DEFAULT_STATE.copy(
dialog = MigrateToMyItemsState.DialogState.NoNetwork(
title = BitwardenString.internet_connection_required_title.asText(),
message = BitwardenString.internet_connection_required_message.asText(),
throwable = error,
),
), awaitItem(),
)
// Dismiss the dialog
viewModel.trySendAction(MigrateToMyItemsAction.NoNetworkDismissDialogClicked)
verify { mockVaultMigrationManager.clearMigrationState() }
assertEquals(
DEFAULT_STATE, awaitItem(),
)
}
}

View File

@ -1,6 +1,7 @@
package com.bitwarden.network.util
import okio.ByteString.Companion.decodeBase64
import java.net.SocketTimeoutException
import java.net.UnknownHostException
import java.nio.charset.Charset
import java.security.cert.CertPathValidatorException
@ -44,6 +45,14 @@ fun Throwable?.isNoConnectionError(): Boolean {
this?.cause?.isNoConnectionError() ?: false
}
/**
* Returns true if the throwable represents a timeout error.
*/
fun Throwable?.isTimeoutError(): Boolean {
return this is SocketTimeoutException ||
this?.cause?.isTimeoutError() ?: false
}
/**
* Returns true if the throwable represents a SSL handshake error.
*/

View File

@ -3,6 +3,7 @@ package com.bitwarden.network.util
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertNull
import java.net.SocketTimeoutException
import java.net.UnknownHostException
import java.security.cert.CertPathValidatorException
import javax.net.ssl.SSLHandshakeException
@ -60,6 +61,31 @@ class NetworkUtilsTest {
)
}
@Test
fun `isTimeoutError should return return true for SocketTimeoutException`() {
assertEquals(
true,
SocketTimeoutException().isTimeoutError(),
)
}
@Test
fun `isTimeoutError should return return false for not SocketTimeoutException`() {
assertEquals(
false,
IllegalStateException().isTimeoutError(),
)
}
@Suppress("MaxLineLength")
@Test
fun `isTimeoutError should return return true if exceptions cause is SocketTimeoutException`() {
assertEquals(
true,
Exception(SocketTimeoutException()).isTimeoutError(),
)
}
@Test
fun `isSslHandshakeError should return return true for SSLHandshakeException`() {
assertEquals(