Minor cleanup of the MigrateToMyItemsScreen (#6421)

This commit is contained in:
David Perez 2026-01-28 08:59:22 -06:00 committed by GitHub
parent 19a3697605
commit 009136ce1e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 141 additions and 179 deletions

View File

@ -268,7 +268,6 @@ fun NavGraphBuilder.vaultUnlockedGraph(
)
migrateToMyItemsDestination(
onNavigateToVault = { navController.popBackStack() },
onNavigateToLeaveOrganization = { organizationId, organizationName ->
navController.navigateToLeaveOrganization(
organizationId = organizationId,

View File

@ -70,12 +70,10 @@ fun NavController.navigateToMigrateToMyItems(
* Add the migrate to my items screen to the nav graph.
*/
fun NavGraphBuilder.migrateToMyItemsDestination(
onNavigateToVault: () -> Unit,
onNavigateToLeaveOrganization: (organizationId: String, organizationName: String) -> Unit,
) {
composableWithSlideTransitions<MigrateToMyItemsRoute> {
MigrateToMyItemsScreen(
onNavigateToVault = onNavigateToVault,
onNavigateToLeaveOrganization = onNavigateToLeaveOrganization,
)
}

View File

@ -3,13 +3,19 @@ package com.x8bit.bitwarden.ui.vault.feature.migratetomyitems
import androidx.compose.foundation.Image
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.WindowInsets
import androidx.compose.foundation.layout.WindowInsetsSides
import androidx.compose.foundation.layout.displayCutout
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.navigationBarsPadding
import androidx.compose.foundation.layout.only
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.layout.union
import androidx.compose.foundation.rememberScrollState
import androidx.compose.foundation.verticalScroll
import androidx.compose.material3.ScaffoldDefaults
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
@ -44,7 +50,6 @@ import com.x8bit.bitwarden.ui.vault.feature.migratetomyitems.handler.rememberMig
*/
@Composable
fun MigrateToMyItemsScreen(
onNavigateToVault: () -> Unit,
onNavigateToLeaveOrganization: (organizationId: String, organizationName: String) -> Unit,
viewModel: MigrateToMyItemsViewModel = hiltViewModel(),
intentManager: IntentManager = LocalIntentManager.current,
@ -54,10 +59,10 @@ fun MigrateToMyItemsScreen(
EventsEffect(viewModel = viewModel) { event ->
when (event) {
MigrateToMyItemsEvent.NavigateToVault -> onNavigateToVault()
is MigrateToMyItemsEvent.NavigateToLeaveOrganization -> {
onNavigateToLeaveOrganization(event.organizationId, event.organizationName)
}
is MigrateToMyItemsEvent.LaunchUri -> intentManager.launchUri(event.uri.toUri())
}
}
@ -68,7 +73,12 @@ fun MigrateToMyItemsScreen(
onDismissNoNetworkRequest = handlers.onDismissNoNetworkDialog,
)
BitwardenScaffold {
BitwardenScaffold(
contentWindowInsets = ScaffoldDefaults
.contentWindowInsets
.union(WindowInsets.displayCutout)
.only(WindowInsetsSides.Horizontal + WindowInsetsSides.Top),
) {
MigrateToMyItemsContent(
state = state,
onAcceptClick = handlers.onAcceptClick,

View File

@ -3,7 +3,6 @@ package com.x8bit.bitwarden.ui.vault.feature.migratetomyitems
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
@ -13,6 +12,7 @@ import com.bitwarden.ui.platform.resource.BitwardenString
import com.bitwarden.ui.util.Text
import com.bitwarden.ui.util.asText
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.platform.error.NoActiveUserException
import com.x8bit.bitwarden.data.platform.manager.event.OrganizationEventManager
import com.x8bit.bitwarden.data.platform.manager.model.OrganizationEvent
import com.x8bit.bitwarden.data.vault.manager.VaultMigrationManager
@ -91,9 +91,7 @@ class MigrateToMyItemsViewModel @Inject constructor(
trySendAction(
MigrateToMyItemsAction.Internal.MigrateToMyItemsResultReceived(
result = MigratePersonalVaultResult.Failure(
error = MissingPropertyException(
propertyName = "UserId",
),
error = NoActiveUserException(),
),
),
)
@ -164,7 +162,7 @@ class MigrateToMyItemsViewModel @Inject constructor(
),
)
clearDialog()
sendEvent(MigrateToMyItemsEvent.NavigateToVault)
// Navigation to vault is handled by state-based navigation.
}
is MigratePersonalVaultResult.Failure -> {
@ -174,26 +172,23 @@ class MigrateToMyItemsViewModel @Inject constructor(
mutableStateFlow.update {
it.copy(
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,
)
},
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,
)
},
)
}
}
@ -256,11 +251,6 @@ data class MigrateToMyItemsState(
* Models the events that can be sent from the [MigrateToMyItemsViewModel].
*/
sealed class MigrateToMyItemsEvent {
/**
* Navigate to the vault screen after accepting migration.
*/
data object NavigateToVault : MigrateToMyItemsEvent()
/**
* Navigate to the leave organization flow after declining.
*/

View File

@ -20,7 +20,6 @@ import org.junit.Test
import org.junit.jupiter.api.Assertions.assertTrue
class MigrateToMyItemsScreenTest : BitwardenComposeTest() {
private var onNavigateToVaultCalled = false
private var onNavigateToLeaveOrganizationCalled = false
private val intentManager: IntentManager = mockk {
@ -47,7 +46,6 @@ class MigrateToMyItemsScreenTest : BitwardenComposeTest() {
setContent(intentManager = intentManager) {
MigrateToMyItemsScreen(
viewModel = viewModel,
onNavigateToVault = { onNavigateToVaultCalled = true },
onNavigateToLeaveOrganization = { _, _ ->
onNavigateToLeaveOrganizationCalled = true
},
@ -101,12 +99,6 @@ class MigrateToMyItemsScreenTest : BitwardenComposeTest() {
}
}
@Test
fun `NavigateToVault event should trigger navigation callback`() {
mutableEventFlow.tryEmit(MigrateToMyItemsEvent.NavigateToVault)
assertTrue(onNavigateToVaultCalled)
}
@Test
fun `NavigateToLeaveOrganization event should trigger navigation callback`() {
mutableEventFlow.tryEmit(

View File

@ -2,7 +2,6 @@ package com.x8bit.bitwarden.ui.vault.feature.migratetomyitems
import androidx.lifecycle.SavedStateHandle
import app.cash.turbine.test
import com.bitwarden.core.data.repository.error.MissingPropertyException
import com.bitwarden.ui.platform.base.BaseViewModelTest
import com.bitwarden.ui.platform.components.snackbar.model.BitwardenSnackbarData
import com.bitwarden.ui.platform.manager.snackbar.SnackbarRelayManager
@ -10,6 +9,7 @@ import com.bitwarden.ui.platform.resource.BitwardenString
import com.bitwarden.ui.util.asText
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.auth.repository.model.UserState
import com.x8bit.bitwarden.data.platform.error.NoActiveUserException
import com.x8bit.bitwarden.data.platform.manager.event.OrganizationEventManager
import com.x8bit.bitwarden.data.platform.manager.model.OrganizationEvent
import com.x8bit.bitwarden.data.vault.manager.VaultMigrationManager
@ -21,15 +21,16 @@ import io.mockk.coVerify
import io.mockk.every
import io.mockk.just
import io.mockk.mockk
import io.mockk.mockkConstructor
import io.mockk.mockkStatic
import io.mockk.runs
import io.mockk.unmockkConstructor
import io.mockk.unmockkStatic
import io.mockk.verify
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.test.runTest
import org.junit.jupiter.api.AfterEach
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
@ -62,19 +63,25 @@ class MigrateToMyItemsViewModelTest : BaseViewModelTest() {
@BeforeEach
fun setup() {
mockkStatic(SavedStateHandle::toMigrateToMyItemsArgs)
mockkConstructor(NoActiveUserException::class)
every {
anyConstructed<NoActiveUserException>() == any<NoActiveUserException>()
} returns true
}
@AfterEach
fun tearDown() {
unmockkStatic(SavedStateHandle::toMigrateToMyItemsArgs)
unmockkConstructor(NoActiveUserException::class)
}
@Test
fun `initial state should be set from organization data`() {
val viewModel = createViewModel()
assertEquals(ORGANIZATION_NAME, viewModel.stateFlow.value.organizationName)
assertEquals(ORGANIZATION_ID, viewModel.stateFlow.value.organizationId)
assertNull(viewModel.stateFlow.value.dialog)
assertEquals(
DEFAULT_STATE,
viewModel.stateFlow.value,
)
}
@Test
@ -102,77 +109,23 @@ class MigrateToMyItemsViewModelTest : BaseViewModelTest() {
}
}
@Test
fun `AcceptClicked should navigate to vault on success`() = runTest {
val viewModel = createViewModel()
viewModel.eventFlow.test {
viewModel.trySendAction(MigrateToMyItemsAction.AcceptClicked)
assertEquals(MigrateToMyItemsEvent.NavigateToVault, awaitItem())
}
}
@Test
fun `AcceptClicked should show error dialog when userId is null`() = runTest {
mutableUserStateFlow.value = null
val viewModel = createViewModel()
viewModel.trySendAction(MigrateToMyItemsAction.AcceptClicked)
val dialog = viewModel.stateFlow.value.dialog as MigrateToMyItemsState.DialogState.Error
val throwableReference = dialog.throwable
assert(throwableReference is MissingPropertyException)
assertEquals(
"Missing the required UserId property",
throwableReference?.message,
)
assertEquals(
MigrateToMyItemsState.DialogState.Error(
title = BitwardenString.an_error_has_occurred.asText(),
message = BitwardenString.failed_to_migrate_items_to_x.asText(
ORGANIZATION_NAME,
),
throwable = throwableReference,
),
viewModel.stateFlow.value.dialog,
)
coVerify(exactly = 0) {
mockVaultMigrationManager.migratePersonalVault(any(), any())
}
}
@Suppress("MaxLineLength")
@Test
fun `MigrateToMyItemsResultReceived with success should track ItemOrganizationAccepted event, clear dialog, and navigate to vault`() =
fun `AcceptClicked with valid user and migration should show success snackbar and track the event`() =
runTest {
val viewModel = createViewModel()
viewModel.eventFlow.test {
// First show the loading dialog
viewModel.trySendAction(MigrateToMyItemsAction.AcceptClicked)
viewModel.trySendAction(MigrateToMyItemsAction.AcceptClicked)
// Migration completes and sends NavigateToVault event
assertEquals(MigrateToMyItemsEvent.NavigateToVault, awaitItem())
viewModel.trySendAction(
MigrateToMyItemsAction.Internal.MigrateToMyItemsResultReceived(
result = MigratePersonalVaultResult.Success,
),
coVerify(exactly = 1) {
mockVaultMigrationManager.migratePersonalVault(
userId = "test-user-id",
organizationId = ORGANIZATION_ID,
)
mockSnackbarRelayManager.sendSnackbarData(
relay = SnackbarRelay.VAULT_MIGRATED_TO_MY_ITEMS,
data = BitwardenSnackbarData(message = BitwardenString.items_transferred.asText()),
)
assertEquals(MigrateToMyItemsEvent.NavigateToVault, awaitItem())
}
assertNull(viewModel.stateFlow.value.dialog)
mockSnackbarRelayManager.sendSnackbarData(
relay = SnackbarRelay.LEFT_ORGANIZATION,
data = BitwardenSnackbarData(
message = BitwardenString.you_left_the_organization.asText(),
),
)
verify {
mockOrganizationEventManager.trackEvent(
event = OrganizationEvent.ItemOrganizationAccepted(
organizationId = ORGANIZATION_ID,
@ -182,29 +135,70 @@ class MigrateToMyItemsViewModelTest : BaseViewModelTest() {
}
@Test
fun `MigrateToMyItemsResultReceived with failure should show error dialog`() = runTest {
fun `AcceptClicked should show error dialog when userId is null`() = runTest {
mutableUserStateFlow.value = null
val viewModel = createViewModel()
viewModel.stateFlow.test {
awaitItem() // Initial state
viewModel.trySendAction(MigrateToMyItemsAction.AcceptClicked)
viewModel.trySendAction(
MigrateToMyItemsAction.Internal.MigrateToMyItemsResultReceived(
result = MigratePersonalVaultResult.Failure(null),
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 = NoActiveUserException(),
),
)
),
viewModel.stateFlow.value,
)
val errorState = awaitItem()
assert(errorState.dialog is MigrateToMyItemsState.DialogState.Error)
val errorDialog = errorState.dialog as MigrateToMyItemsState.DialogState.Error
assertEquals(BitwardenString.an_error_has_occurred.asText(), errorDialog.title)
assertEquals(
BitwardenString.failed_to_migrate_items_to_x.asText(ORGANIZATION_NAME),
errorDialog.message,
)
coVerify(exactly = 0) {
mockVaultMigrationManager.migratePersonalVault(any(), any())
}
}
@Suppress("MaxLineLength")
@Test
fun `MigrateToMyItemsResultReceived with valid user ID and failed migration should show error dialog`() =
runTest {
val viewModel = createViewModel()
val error = Throwable("Fail")
coEvery {
mockVaultMigrationManager.migratePersonalVault(any(), any())
} returns MigratePersonalVaultResult.Failure(error)
viewModel.stateFlow.test {
awaitItem() // Initial state
viewModel.trySendAction(MigrateToMyItemsAction.AcceptClicked)
assertEquals(
DEFAULT_STATE.copy(
dialog = MigrateToMyItemsState.DialogState.Loading(
message = BitwardenString.migrating_items_to_x.asText(ORGANIZATION_NAME),
),
),
awaitItem(),
)
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 = error,
),
),
awaitItem(),
)
}
}
@Suppress("MaxLineLength")
@Test
fun `MigrateToMyItemsResultReceived with timeout error should show DialogState NoNetwork`() =
@ -228,7 +222,8 @@ class MigrateToMyItemsViewModelTest : BaseViewModelTest() {
message = BitwardenString.internet_connection_required_message.asText(),
throwable = error,
),
), awaitItem(),
),
awaitItem(),
)
}
}
@ -253,45 +248,33 @@ class MigrateToMyItemsViewModelTest : BaseViewModelTest() {
val viewModel = createViewModel()
viewModel.eventFlow.test {
viewModel.trySendAction(MigrateToMyItemsAction.HelpLinkClicked)
val event = awaitItem()
assert(event is MigrateToMyItemsEvent.LaunchUri)
assertEquals(
"https://bitwarden.com/help/transfer-ownership/",
(event as MigrateToMyItemsEvent.LaunchUri).uri,
MigrateToMyItemsEvent.LaunchUri("https://bitwarden.com/help/transfer-ownership/"),
awaitItem(),
)
}
}
@Test
fun `DismissDialogClicked should clear dialog`() = runTest {
val viewModel = createViewModel()
val initialState = 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,
),
)
val viewModel = createViewModel(state = initialState)
viewModel.stateFlow.test {
awaitItem() // Initial state
// First show an error dialog
viewModel.trySendAction(
MigrateToMyItemsAction.Internal.MigrateToMyItemsResultReceived(
result = MigratePersonalVaultResult.Failure(null),
),
)
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(),
)
assertEquals(initialState, awaitItem())
// Dismiss the dialog
viewModel.trySendAction(MigrateToMyItemsAction.DismissDialogClicked)
assertEquals(
DEFAULT_STATE, awaitItem(),
DEFAULT_STATE,
awaitItem(),
)
}
}
@ -299,35 +282,25 @@ class MigrateToMyItemsViewModelTest : BaseViewModelTest() {
@Suppress("MaxLineLength")
@Test
fun `NoNetworkDismissDialogClicked should clear dialog and clear migration state`() = runTest {
val viewModel = createViewModel()
val error = SocketTimeoutException("Timeout")
val initialState = DEFAULT_STATE.copy(
dialog = MigrateToMyItemsState.DialogState.NoNetwork(
title = BitwardenString.internet_connection_required_title.asText(),
message = BitwardenString.internet_connection_required_message.asText(),
throwable = error,
),
)
val viewModel = createViewModel(state = initialState)
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(),
)
assertEquals(initialState, awaitItem())
// Dismiss the dialog
viewModel.trySendAction(MigrateToMyItemsAction.NoNetworkDismissDialogClicked)
verify { mockVaultMigrationManager.clearMigrationState() }
assertEquals(
DEFAULT_STATE, awaitItem(),
)
assertEquals(DEFAULT_STATE, awaitItem())
}
verify(exactly = 1) {
mockVaultMigrationManager.clearMigrationState()
}
}