PM 18033 - Only show Setup Unlock and Autofill Setup onboarding steps after new account creation (#4748)

This commit is contained in:
Phil Cappelli
2025-02-19 14:35:46 -05:00
committed by GitHub
parent 1349165156
commit c59a28a9df
11 changed files with 63 additions and 41 deletions

View File

@@ -421,7 +421,7 @@ interface AuthRepository : AuthenticatorProvider, AuthRequestManager {
/**
* Update the value of the onboarding status for the user.
*/
fun setOnboardingStatus(userId: String, status: OnboardingStatus?)
fun setOnboardingStatus(status: OnboardingStatus)
/**
* Checks if a new device notice should be displayed.

View File

@@ -1375,8 +1375,13 @@ class AuthRepositoryImpl(
)
}
override fun setOnboardingStatus(userId: String, status: OnboardingStatus?) {
authDiskSource.storeOnboardingStatus(userId = userId, onboardingStatus = status)
override fun setOnboardingStatus(status: OnboardingStatus) {
activeUserId?.let { userId ->
authDiskSource.storeOnboardingStatus(
userId = userId,
onboardingStatus = status,
)
}
}
override fun getNewDeviceNoticeState(): NewDeviceNoticeState? {
@@ -1765,15 +1770,6 @@ class AuthRepositoryImpl(
)
settingsRepository.hasUserLoggedInOrCreatedAccount = true
val shouldSetOnboardingStatus = featureFlagManager.getFeatureFlag(FlagKey.OnboardingFlow) &&
!settingsRepository.getUserHasLoggedInValue(userId = userId)
if (shouldSetOnboardingStatus) {
setOnboardingStatus(
userId = userId,
status = OnboardingStatus.NOT_STARTED,
)
}
authDiskSource.userState = userStateJson
loginResponse.key?.let {
// Only set the value if it's present, since we may have set it already

View File

@@ -100,13 +100,13 @@ class SetupAutoFillViewModel @Inject constructor(
private fun handleTurnOnLaterConfirmClick() {
firstTimeActionManager.storeShowAutoFillSettingBadge(showBadge = true)
updateOnboardingStatusToNextStep()
updateOnboardingStatusToFinalStep()
}
private fun handleContinueClick() {
firstTimeActionManager.storeShowAutoFillSettingBadge(showBadge = false)
if (state.isInitialSetup) {
updateOnboardingStatusToNextStep()
updateOnboardingStatusToFinalStep()
} else {
sendEvent(SetupAutoFillEvent.NavigateBack)
}
@@ -120,12 +120,10 @@ class SetupAutoFillViewModel @Inject constructor(
}
}
private fun updateOnboardingStatusToNextStep() =
authRepository
.setOnboardingStatus(
userId = state.userId,
status = OnboardingStatus.FINAL_STEP,
)
private fun updateOnboardingStatusToFinalStep() =
authRepository.setOnboardingStatus(
status = OnboardingStatus.FINAL_STEP,
)
}
/**

View File

@@ -26,7 +26,6 @@ class SetupCompleteViewModel @Inject constructor(
private fun handleCompleteSetup() {
authRepository.setOnboardingStatus(
userId = state.userId,
status = OnboardingStatus.COMPLETE,
)
}

View File

@@ -208,7 +208,7 @@ class SetupUnlockViewModel @Inject constructor(
} else {
OnboardingStatus.AUTOFILL_SETUP
}
authRepository.setOnboardingStatus(state.userId, nextStep)
authRepository.setOnboardingStatus(nextStep)
}
}

View File

@@ -4,6 +4,7 @@ import android.os.Parcelable
import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.viewModelScope
import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.data.auth.datasource.disk.model.OnboardingStatus
import com.x8bit.bitwarden.data.auth.datasource.sdk.model.PasswordStrength
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.auth.repository.model.LoginResult
@@ -272,6 +273,11 @@ class CompleteRegistrationViewModel @Inject constructor(
message = R.string.account_created_success.asText(),
),
)
authRepository.setOnboardingStatus(
status = OnboardingStatus.NOT_STARTED,
)
// If the login result is Success the state based navigation will take care of it.
// otherwise we need to navigate to the login screen.
if (action.loginResult !is LoginResult.Success) {

View File

@@ -6316,9 +6316,29 @@ class AuthRepositoryTest {
@Test
fun `setOnboardingStatus should save the onboarding status to disk`() {
val userId = "userId"
repository.setOnboardingStatus(userId = userId, status = OnboardingStatus.NOT_STARTED)
assertEquals(OnboardingStatus.NOT_STARTED, fakeAuthDiskSource.getOnboardingStatus(userId))
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
repository.setOnboardingStatus(status = OnboardingStatus.NOT_STARTED)
assertEquals(
OnboardingStatus.NOT_STARTED,
fakeAuthDiskSource.getOnboardingStatus(USER_ID_1),
)
}
@Test
fun `setOnboardingStatus with no userId does not change the onboarding state`() {
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
repository.setOnboardingStatus(status = OnboardingStatus.NOT_STARTED)
assertEquals(
OnboardingStatus.NOT_STARTED,
fakeAuthDiskSource.getOnboardingStatus(USER_ID_1),
)
fakeAuthDiskSource.userState = null
repository.setOnboardingStatus(status = OnboardingStatus.COMPLETE)
val activeUserId = SINGLE_USER_STATE_1.activeUserId
assertFalse(
fakeAuthDiskSource.getOnboardingStatus(activeUserId) == OnboardingStatus.COMPLETE,
)
}
@Test
@@ -6378,10 +6398,8 @@ class AuthRepositoryTest {
userId = USER_ID_1,
passwordHash = PASSWORD_HASH,
)
assertEquals(
OnboardingStatus.NOT_STARTED,
fakeAuthDiskSource.getOnboardingStatus(USER_ID_1),
)
// This should only be set after they complete a registration and not based on login.
assertNull(fakeAuthDiskSource.getOnboardingStatus(USER_ID_1))
}
@Suppress("MaxLineLength")

View File

@@ -42,7 +42,7 @@ class SetupAutoFillViewModelTest : BaseViewModelTest() {
private val mutableUserStateFlow = MutableStateFlow<UserState?>(mockUserState)
private val authRepository: AuthRepository = mockk {
every { userStateFlow } returns mutableUserStateFlow
every { setOnboardingStatus(any(), any()) } just runs
every { setOnboardingStatus(any()) } just runs
}
@Test
@@ -119,7 +119,6 @@ class SetupAutoFillViewModelTest : BaseViewModelTest() {
viewModel.trySendAction(SetupAutoFillAction.TurnOnLaterConfirmClick)
verify {
authRepository.setOnboardingStatus(
DEFAULT_USER_ID,
OnboardingStatus.FINAL_STEP,
)
}
@@ -132,7 +131,6 @@ class SetupAutoFillViewModelTest : BaseViewModelTest() {
viewModel.trySendAction(SetupAutoFillAction.ContinueClick)
verify(exactly = 1) {
authRepository.setOnboardingStatus(
DEFAULT_USER_ID,
OnboardingStatus.FINAL_STEP,
)
firstTimeActionManager.storeShowAutoFillSettingBadge(showBadge = false)
@@ -157,7 +155,6 @@ class SetupAutoFillViewModelTest : BaseViewModelTest() {
}
verify(exactly = 0) {
authRepository.setOnboardingStatus(
DEFAULT_USER_ID,
OnboardingStatus.FINAL_STEP,
)
}

View File

@@ -22,7 +22,7 @@ class SetupCompleteViewModelTest : BaseViewModelTest() {
private val mutableUserStateFlow = MutableStateFlow<UserState?>(mockUserState)
private val authRepository: AuthRepository = mockk {
every { userStateFlow } returns mutableUserStateFlow
every { setOnboardingStatus(any(), any()) } just runs
every { setOnboardingStatus(any()) } just runs
}
@Test
@@ -52,7 +52,6 @@ class SetupCompleteViewModelTest : BaseViewModelTest() {
viewModel.trySendAction(SetupCompleteAction.CompleteSetup)
verify {
authRepository.setOnboardingStatus(
DEFAULT_USER_ID,
OnboardingStatus.COMPLETE,
)
}

View File

@@ -34,7 +34,7 @@ class SetupUnlockViewModelTest : BaseViewModelTest() {
private val mutableUserStateFlow = MutableStateFlow<UserState?>(DEFAULT_USER_STATE)
private val authRepository: AuthRepository = mockk {
every { userStateFlow } returns mutableUserStateFlow
every { setOnboardingStatus(userId = any(), status = any()) } just runs
every { setOnboardingStatus(status = any()) } just runs
}
private val mutableAutofillEnabledStateFlow = MutableStateFlow(false)
@@ -78,7 +78,6 @@ class SetupUnlockViewModelTest : BaseViewModelTest() {
viewModel.trySendAction(SetupUnlockAction.ContinueClick)
verify {
authRepository.setOnboardingStatus(
userId = DEFAULT_USER_ID,
status = OnboardingStatus.AUTOFILL_SETUP,
)
}
@@ -98,7 +97,6 @@ class SetupUnlockViewModelTest : BaseViewModelTest() {
}
verify(exactly = 0) {
authRepository.setOnboardingStatus(
userId = DEFAULT_USER_ID,
status = OnboardingStatus.AUTOFILL_SETUP,
)
}
@@ -111,7 +109,6 @@ class SetupUnlockViewModelTest : BaseViewModelTest() {
viewModel.trySendAction(SetupUnlockAction.SetUpLaterClick)
verify {
authRepository.setOnboardingStatus(
userId = DEFAULT_USER_ID,
status = OnboardingStatus.AUTOFILL_SETUP,
)
firstTimeActionManager.storeShowUnlockSettingBadge(showBadge = true)
@@ -126,7 +123,6 @@ class SetupUnlockViewModelTest : BaseViewModelTest() {
viewModel.trySendAction(SetupUnlockAction.ContinueClick)
verify(exactly = 1) {
authRepository.setOnboardingStatus(
userId = DEFAULT_USER_ID,
status = OnboardingStatus.FINAL_STEP,
)
firstTimeActionManager.storeShowUnlockSettingBadge(showBadge = false)
@@ -142,7 +138,6 @@ class SetupUnlockViewModelTest : BaseViewModelTest() {
viewModel.trySendAction(SetupUnlockAction.SetUpLaterClick)
verify {
authRepository.setOnboardingStatus(
userId = DEFAULT_USER_ID,
status = OnboardingStatus.FINAL_STEP,
)
}

View File

@@ -3,6 +3,7 @@ package com.x8bit.bitwarden.ui.auth.feature.completeregistration
import androidx.lifecycle.SavedStateHandle
import app.cash.turbine.test
import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.data.auth.datasource.disk.model.OnboardingStatus
import com.x8bit.bitwarden.data.auth.datasource.sdk.model.PasswordStrength.LEVEL_0
import com.x8bit.bitwarden.data.auth.datasource.sdk.model.PasswordStrength.LEVEL_1
import com.x8bit.bitwarden.data.auth.datasource.sdk.model.PasswordStrength.LEVEL_2
@@ -31,12 +32,15 @@ import com.x8bit.bitwarden.ui.auth.feature.completeregistration.CompleteRegistra
import com.x8bit.bitwarden.ui.auth.feature.completeregistration.CompleteRegistrationAction.PasswordInputChange
import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest
import com.x8bit.bitwarden.ui.platform.base.util.asText
import io.mockk.Runs
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.every
import io.mockk.just
import io.mockk.mockk
import io.mockk.mockkStatic
import io.mockk.unmockkStatic
import io.mockk.verify
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.test.runTest
import org.junit.jupiter.api.AfterEach
@@ -76,6 +80,10 @@ class CompleteRegistrationViewModelTest : BaseViewModelTest() {
isMasterPasswordStrong = any(),
)
} returns RegisterResult.Success(captchaToken = CAPTCHA_BYPASS_TOKEN)
coEvery {
setOnboardingStatus(OnboardingStatus.NOT_STARTED)
} just Runs
}
private val fakeEnvironmentRepository = FakeEnvironmentRepository()
@@ -222,6 +230,9 @@ class CompleteRegistrationViewModelTest : BaseViewModelTest() {
assertTrue(awaitItem() is CompleteRegistrationEvent.ShowToast)
expectNoEvents()
}
verify(exactly = 1) {
mockAuthRepository.setOnboardingStatus(OnboardingStatus.NOT_STARTED)
}
}
@Suppress("MaxLineLength")
@@ -245,6 +256,9 @@ class CompleteRegistrationViewModelTest : BaseViewModelTest() {
awaitItem(),
)
}
verify(exactly = 1) {
mockAuthRepository.setOnboardingStatus(OnboardingStatus.NOT_STARTED)
}
}
@Test