diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManagerImpl.kt index afd729a79..7d5ebf8f8 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManagerImpl.kt @@ -159,6 +159,13 @@ class FirstTimeActionManagerImpl @Inject constructor( .getHasSeenAddLoginCoachMarkFlow() // default value of false. .map { it ?: false } + .combine( + featureFlagManager.getFeatureFlagFlow(FlagKey.OnboardingFlow), + ) { hasSeenTour, featureIsEnabled -> + // If the feature flag is off always return true so observers know + // the card has not been shown. + hasSeenTour || !featureIsEnabled + } .distinctUntilChanged() /** diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditItemContent.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditItemContent.kt index 3f378c436..e0cf75c42 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditItemContent.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditItemContent.kt @@ -92,7 +92,9 @@ fun CoachMarkScope<AddEditItemCoachMark>.VaultAddEditContent( actionText = stringResource(R.string.get_started), onActionClick = loginItemTypeHandlers.onStartLoginCoachMarkTour, onDismissClick = loginItemTypeHandlers.onDismissLearnAboutLoginsCard, - modifier = Modifier.standardHorizontalMargin(), + modifier = Modifier + .fillMaxWidth() + .standardHorizontalMargin(), ) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt index 1ada08969..5b517262d 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt @@ -15,13 +15,11 @@ import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CreateCredentialReques import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2RegisterCredentialResult import com.x8bit.bitwarden.data.autofill.fido2.model.UserVerificationRequirement import com.x8bit.bitwarden.data.autofill.util.isActiveWithFido2Credentials -import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManager 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.FlagKey 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.toAutofillSaveItemOrNull @@ -66,7 +64,6 @@ import com.x8bit.bitwarden.ui.vault.model.VaultCollection import com.x8bit.bitwarden.ui.vault.model.VaultIdentityTitle import com.x8bit.bitwarden.ui.vault.model.VaultLinkedFieldType import dagger.hilt.android.lifecycle.HiltViewModel -import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map @@ -108,7 +105,6 @@ class VaultAddEditViewModel @Inject constructor( private val organizationEventManager: OrganizationEventManager, private val networkConnectionManager: NetworkConnectionManager, private val firstTimeActionManager: FirstTimeActionManager, - private val featureFlagManager: FeatureFlagManager, ) : BaseViewModel<VaultAddEditState, VaultAddEditEvent, VaultAddEditAction>( // We load the state from the savedStateHandle for testing purposes. initialState = savedStateHandle[KEY_STATE] @@ -221,13 +217,9 @@ class VaultAddEditViewModel @Inject constructor( firstTimeActionManager .hasSeenAddLoginCoachMarkFlow - .combine( - featureFlagManager.getFeatureFlagFlow(key = FlagKey.OnboardingFlow), - ) { hasSeenTour, onboardFeatureFlag -> - // The result of this will hide the card if the tour has been shown or if the - // onboarding flow feature is off. + .map { hasSeenTour -> VaultAddEditAction.Internal.HasSeenAddLoginCoachMarkValueChangeReceive( - newValue = hasSeenTour || !onboardFeatureFlag, + hasSeenCoachMark = hasSeenTour, ) } .onEach(::sendAction) @@ -1495,7 +1487,7 @@ class VaultAddEditViewModel @Inject constructor( ) { mutableStateFlow.update { it.copy( - hasSeenLearnAboutLoginsCard = action.newValue, + hasSeenLearnAboutLoginsCard = action.hasSeenCoachMark, ) } } @@ -3211,7 +3203,7 @@ sealed class VaultAddEditAction { * The value for the hasSeenAddLoginCoachMark has changed. */ data class HasSeenAddLoginCoachMarkValueChangeReceive( - val newValue: Boolean, + val hasSeenCoachMark: Boolean, ) : Internal() } } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManagerTest.kt index ca30c70b8..44bf8b665 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManagerTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManagerTest.kt @@ -36,8 +36,10 @@ class FirstTimeActionManagerTest { } private val mutableImportLoginsFlow = MutableStateFlow(false) + private val mutableOnboardingFeatureFlow = MutableStateFlow(false) private val featureFlagManager = mockk<FeatureFlagManager> { every { getFeatureFlagFlow(FlagKey.ImportLoginsFlow) } returns mutableImportLoginsFlow + every { getFeatureFlagFlow(FlagKey.OnboardingFlow) } returns mutableOnboardingFeatureFlow } private val mutableAutofillEnabledFlow = MutableStateFlow(false) @@ -300,14 +302,28 @@ class FirstTimeActionManagerTest { @Test fun `hasSeenAddLoginCoachMarkFlow updates when disk source updates`() = runTest { + // Enable the feature for this test. + mutableOnboardingFeatureFlow.update { true } firstTimeActionManager.hasSeenAddLoginCoachMarkFlow.test { - // null will be mapped to false + // Null will be mapped to false. assertFalse(awaitItem()) fakeSettingsDiskSource.storeHasSeenAddLoginCoachMark(hasSeen = true) assertTrue(awaitItem()) } } + @Test + fun `hasSeenAddLoginCoachMarkFlow updates when feature flag for onboarding updates`() = + runTest { + firstTimeActionManager.hasSeenAddLoginCoachMarkFlow.test { + // Null will be mapped to false but feature being "off" will override to true. + assertTrue(awaitItem()) + mutableOnboardingFeatureFlow.update { true } + // Will use the value from disk source (null ?: false). + assertFalse(awaitItem()) + } + } + @Test fun `hasSeenAddLoginCoachMarkTour sets the value to true in the disk source`() { assertNull(fakeSettingsDiskSource.getHasSeenAddLoginCoachMark()) diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt index efb441b6f..3cd043f05 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt @@ -25,7 +25,6 @@ import com.x8bit.bitwarden.data.autofill.fido2.model.createMockFido2CreateCreden import com.x8bit.bitwarden.data.autofill.model.AutofillSaveItem import com.x8bit.bitwarden.data.autofill.model.AutofillSelectionData import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager -import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManager @@ -33,7 +32,6 @@ import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManagerImpl 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.FirstTimeState -import com.x8bit.bitwarden.data.platform.manager.model.FlagKey import com.x8bit.bitwarden.data.platform.manager.model.OrganizationEvent import com.x8bit.bitwarden.data.platform.manager.model.SpecialCircumstance import com.x8bit.bitwarden.data.platform.manager.network.NetworkConnectionManager @@ -175,14 +173,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { every { hasSeenAddLoginCoachMarkFlow } returns mutableHasSeenAddLoginCoachMarkFlow } - // Feature flag default to be enabled. - private val mutableOnboardingFeatureFlagFlow = MutableStateFlow(true) - private val featureFlagManager = mockk<FeatureFlagManager> { - every { - getFeatureFlagFlow(FlagKey.OnboardingFlow) - } returns mutableOnboardingFeatureFlagFlow - } - @BeforeEach fun setup() { mockkStatic(CipherView::toViewState) @@ -2623,25 +2613,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { } } - @Test - fun `when OnboardFlow feature flag is off, shouldShowLearnAboutNewLogins should be false`() { - mutableOnboardingFeatureFlagFlow.update { true } - val viewModel = createAddVaultItemViewModel( - savedStateHandle = createSavedStateHandleWithState( - state = createVaultAddItemState( - typeContentViewState = createLoginTypeContentViewState(), - ), - vaultAddEditType = VaultAddEditType.AddItem( - vaultItemCipherType = VaultItemCipherType.LOGIN, - ), - ), - ) - assertTrue(viewModel.stateFlow.value.shouldShowLearnAboutNewLogins) - - mutableOnboardingFeatureFlagFlow.update { false } - assertFalse(viewModel.stateFlow.value.shouldShowLearnAboutNewLogins) - } - @Suppress("MaxLineLength") @Test fun `when first time action manager has seen logins tour value updates to true shouldShowLearnAboutNewLogins should update to false`() { @@ -2710,7 +2681,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { @Suppress("MaxLineLength") @Test - fun `LearnAboutLoginsDismissed action calls first time action manager hasSeenAddLoginCoachMarkTour called and show coach mark event sent`() = + fun `StartLearnAboutLogins action calls first time action manager hasSeenAddLoginCoachMarkTour called and show coach mark event sent`() = runTest { val viewModel = createAddVaultItemViewModel() viewModel.eventFlow.test { @@ -3246,7 +3217,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { organizationEventManager = organizationEventManager, networkConnectionManager = networkConnectionManager, firstTimeActionManager = firstTimeActionManager, - featureFlagManager = featureFlagManager, ) } @@ -4508,7 +4478,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { organizationEventManager = organizationEventManager, networkConnectionManager = networkConnectionManager, firstTimeActionManager = firstTimeActionManager, - featureFlagManager = featureFlagManager, ) private fun createVaultData(