From c59a28a9df5ce70dcd3a0e68d9003d1c8f9023a2 Mon Sep 17 00:00:00 2001 From: Phil Cappelli <150719757+phil-livefront@users.noreply.github.com> Date: Wed, 19 Feb 2025 14:35:46 -0500 Subject: [PATCH] PM 18033 - Only show `Setup Unlock` and `Autofill Setup` onboarding steps after new account creation (#4748) --- .../data/auth/repository/AuthRepository.kt | 2 +- .../auth/repository/AuthRepositoryImpl.kt | 18 ++++------- .../accountsetup/SetupAutoFillViewModel.kt | 14 ++++---- .../accountsetup/SetupCompleteViewModel.kt | 1 - .../accountsetup/SetupUnlockViewModel.kt | 2 +- .../CompleteRegistrationViewModel.kt | 6 ++++ .../auth/repository/AuthRepositoryTest.kt | 32 +++++++++++++++---- .../SetupAutoFillViewModelTest.kt | 5 +-- .../SetupCompleteViewModelTest.kt | 3 +- .../accountsetup/SetupUnlockViewModelTest.kt | 7 +--- .../CompleteRegistrationViewModelTest.kt | 14 ++++++++ 11 files changed, 63 insertions(+), 41 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepository.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepository.kt index 839b874579..ed0c784706 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepository.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepository.kt @@ -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. diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt index 674313d6ca..0f3cbca948 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt @@ -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 diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupAutoFillViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupAutoFillViewModel.kt index 742faa262d..17dba1285f 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupAutoFillViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupAutoFillViewModel.kt @@ -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, + ) } /** diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupCompleteViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupCompleteViewModel.kt index 24f4ab1b47..1e5dd61fb5 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupCompleteViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupCompleteViewModel.kt @@ -26,7 +26,6 @@ class SetupCompleteViewModel @Inject constructor( private fun handleCompleteSetup() { authRepository.setOnboardingStatus( - userId = state.userId, status = OnboardingStatus.COMPLETE, ) } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModel.kt index 5e1c7068f0..a0a39e29ea 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModel.kt @@ -208,7 +208,7 @@ class SetupUnlockViewModel @Inject constructor( } else { OnboardingStatus.AUTOFILL_SETUP } - authRepository.setOnboardingStatus(state.userId, nextStep) + authRepository.setOnboardingStatus(nextStep) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModel.kt index 16e46e5d74..daf6f53f2d 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModel.kt @@ -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) { diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt index 27a06169cf..cf28de9ed2 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt @@ -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") diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupAutoFillViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupAutoFillViewModelTest.kt index 0098a4c6bd..89509d9b03 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupAutoFillViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupAutoFillViewModelTest.kt @@ -42,7 +42,7 @@ class SetupAutoFillViewModelTest : BaseViewModelTest() { private val mutableUserStateFlow = MutableStateFlow(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, ) } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupCompleteViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupCompleteViewModelTest.kt index c9612460bd..bc0584437b 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupCompleteViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupCompleteViewModelTest.kt @@ -22,7 +22,7 @@ class SetupCompleteViewModelTest : BaseViewModelTest() { private val mutableUserStateFlow = MutableStateFlow(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, ) } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModelTest.kt index 915b8a92aa..aa6e65689c 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModelTest.kt @@ -34,7 +34,7 @@ class SetupUnlockViewModelTest : BaseViewModelTest() { private val mutableUserStateFlow = MutableStateFlow(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, ) } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModelTest.kt index c55dc652cf..d411a8685d 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModelTest.kt @@ -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