From f26374aae7a8fd09fb969cbf0db8cea0889d7ddb Mon Sep 17 00:00:00 2001 From: Dave Severns <149429124+dseverns-livefront@users.noreply.github.com> Date: Fri, 20 Sep 2024 15:53:45 -0400 Subject: [PATCH] [PM-10118] update selected generator type when returning to main tab. (#3942) --- .../feature/generator/GeneratorScreen.kt | 12 ++ .../feature/generator/GeneratorViewModel.kt | 48 ++++++-- .../feature/generator/GeneratorScreenTest.kt | 25 ++-- .../generator/GeneratorViewModelTest.kt | 108 ++++++++++++++++++ 4 files changed, 177 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreen.kt index 2efbfcfa8f..b3b2128331 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreen.kt @@ -54,9 +54,11 @@ import androidx.compose.ui.unit.DpSize import androidx.compose.ui.unit.dp import androidx.core.net.toUri import androidx.hilt.navigation.compose.hiltViewModel +import androidx.lifecycle.Lifecycle import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.x8bit.bitwarden.R import com.x8bit.bitwarden.ui.platform.base.util.EventsEffect +import com.x8bit.bitwarden.ui.platform.base.util.LivecycleEventEffect import com.x8bit.bitwarden.ui.platform.base.util.toDp import com.x8bit.bitwarden.ui.platform.components.appbar.BitwardenMediumTopAppBar import com.x8bit.bitwarden.ui.platform.components.appbar.BitwardenTopAppBar @@ -107,6 +109,16 @@ fun GeneratorScreen( val resources = context.resources val snackbarHostState = remember { SnackbarHostState() } + LivecycleEventEffect { _, event -> + when (event) { + Lifecycle.Event.ON_RESUME -> { + viewModel.trySendAction(GeneratorAction.LifecycleResume) + } + + else -> Unit + } + } + EventsEffect(viewModel = viewModel) { event -> when (event) { GeneratorEvent.NavigateToPasswordHistory -> onNavigateToPasswordHistory() diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt index 5a80e451d1..ac2b100a7f 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt @@ -134,9 +134,16 @@ class GeneratorViewModel @Inject constructor( is GeneratorAction.MainTypeOptionSelect -> handleMainTypeOptionSelect(action) is GeneratorAction.MainType -> handleMainTypeAction(action) is GeneratorAction.Internal -> handleInternalAction(action) + GeneratorAction.LifecycleResume -> handleOnResumed() } } + private fun handleOnResumed() { + // when the screen resumes we need to refresh the options for the current option from + // disk in the event they were changed while the screen was in the foreground. + loadOptions(shouldUseStorageOptions = true) + } + @Suppress("MaxLineLength") private fun handleMainTypeAction(action: GeneratorAction.MainType) { when (action) { @@ -267,16 +274,36 @@ class GeneratorViewModel @Inject constructor( //region Generation Handlers - private fun loadOptions() { + private fun loadOptions(shouldUseStorageOptions: Boolean = false) { when (val selectedType = state.selectedType) { - is Passcode -> loadPasscodeOptions( - selectedType = selectedType, - ) + is Passcode -> { + val mainType = if (shouldUseStorageOptions) { + generatorRepository + .getPasscodeGenerationOptions() + ?.passcodeType + ?.let { Passcode(it) } + ?: selectedType + } else { + selectedType + } + loadPasscodeOptions(selectedType = mainType) + } - is Username -> loadUsernameOptions( - selectedType = selectedType, - forceRegeneration = selectedType.selectedType !is ForwardedEmailAlias, - ) + is Username -> { + val mainType = if (shouldUseStorageOptions) { + generatorRepository + .getUsernameGenerationOptions() + ?.usernameType + ?.let { Username(it) } + ?: selectedType + } else { + selectedType + } + loadUsernameOptions( + selectedType = mainType, + forceRegeneration = mainType.selectedType !is ForwardedEmailAlias, + ) + } } } @@ -2103,6 +2130,11 @@ data class GeneratorState( */ sealed class GeneratorAction { + /** + * Indicates the UI has been entered a resumed lifecycle state. + */ + data object LifecycleResume : GeneratorAction() + /** * Indicates that the overflow option for password history has been clicked. */ diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreenTest.kt index 3976aa894d..3c21fa9ef0 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreenTest.kt @@ -545,7 +545,8 @@ class GeneratorScreenTest : BaseComposeTest() { .performScrollTo() .performClick() - verify(exactly = 0) { viewModel.trySendAction(any()) } + verify(exactly = 1) { viewModel.trySendAction(GeneratorAction.LifecycleResume) } + verify(exactly = 1) { viewModel.trySendAction(any()) } } @Suppress("MaxLineLength") @@ -573,7 +574,8 @@ class GeneratorScreenTest : BaseComposeTest() { .performScrollTo() .performClick() - verify(exactly = 0) { viewModel.trySendAction(any()) } + verify(exactly = 1) { viewModel.trySendAction(GeneratorAction.LifecycleResume) } + verify(exactly = 1) { viewModel.trySendAction(any()) } } @Suppress("MaxLineLength") @@ -642,8 +644,8 @@ class GeneratorScreenTest : BaseComposeTest() { .filterToOne(hasContentDescription("\u2212")) .performScrollTo() .performClick() - - verify(exactly = 0) { viewModel.trySendAction(any()) } + verify(exactly = 1) { viewModel.trySendAction(GeneratorAction.LifecycleResume) } + verify(exactly = 1) { viewModel.trySendAction(any()) } } @Suppress("MaxLineLength") @@ -670,8 +672,8 @@ class GeneratorScreenTest : BaseComposeTest() { .filterToOne(hasContentDescription("+")) .performScrollTo() .performClick() - - verify(exactly = 0) { viewModel.trySendAction(any()) } + verify(exactly = 1) { viewModel.trySendAction(GeneratorAction.LifecycleResume) } + verify(exactly = 1) { viewModel.trySendAction(any()) } } @Suppress("MaxLineLength") @@ -1002,7 +1004,8 @@ class GeneratorScreenTest : BaseComposeTest() { .filterToOne(hasContentDescription("\u2212")) .performScrollTo() .performClick() - verify(exactly = 0) { viewModel.trySendAction(any()) } + verify(exactly = 1) { viewModel.trySendAction(GeneratorAction.LifecycleResume) } + verify(exactly = 1) { viewModel.trySendAction(any()) } } @Test @@ -1030,7 +1033,8 @@ class GeneratorScreenTest : BaseComposeTest() { .filterToOne(hasContentDescription("+")) .performScrollTo() .performClick() - verify(exactly = 0) { viewModel.trySendAction(any()) } + verify(exactly = 1) { viewModel.trySendAction(GeneratorAction.LifecycleResume) } + verify(exactly = 1) { viewModel.trySendAction(any()) } } @Suppress("MaxLineLength") @@ -1690,6 +1694,11 @@ class GeneratorScreenTest : BaseComposeTest() { } } + @Test + fun `send LifecycleResumed action on screen resume`() { + verify { viewModel.trySendAction(GeneratorAction.LifecycleResume) } + } + //endregion Random Word Tests private fun updateState(state: GeneratorState) { diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt index aa930a4c9e..701760cd96 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt @@ -856,6 +856,114 @@ class GeneratorViewModelTest : BaseViewModelTest() { } } + @Test + fun `LifecycleResumedAction should use storage options derived state over VM state`() { + val initialState = initialUsernameState.copy( + selectedType = GeneratorState.MainType.Username( + selectedType = GeneratorState.MainType.Username.UsernameType.PlusAddressedEmail( + email = "currentEmail", + ), + ), + ) + val viewModel = createViewModel(initialState) + fakeGeneratorRepository.saveUsernameGenerationOptions( + UsernameGenerationOptions( + type = UsernameGenerationOptions.UsernameType.RANDOM_WORD, + ), + ) + val expectedState = initialState.copy( + selectedType = GeneratorState.MainType.Username( + selectedType = GeneratorState.MainType.Username.UsernameType.RandomWord(), + ), + generatedText = "randomWord", + ) + viewModel.trySendAction(GeneratorAction.LifecycleResume) + assertEquals( + expectedState, + viewModel.stateFlow.value, + ) + } + + @Test + fun `LifecycleResumedAction should use passcode storage options derived state over VM state`() { + val initialState = initialPasscodeState + val viewModel = createViewModel(initialState) + fakeGeneratorRepository.savePasscodeGenerationOptions( + PasscodeGenerationOptions( + type = PasscodeGenerationOptions.PasscodeType.PASSPHRASE, + length = 14, + allowAmbiguousChar = false, + hasNumbers = false, + minNumber = 3, + hasUppercase = false, + minUppercase = null, + hasLowercase = false, + minLowercase = null, + allowSpecial = false, + minSpecial = 0, + numWords = 3, + wordSeparator = "-", + allowCapitalize = false, + allowIncludeNumber = false, + ), + ) + val expectedState = initialState.copy( + selectedType = GeneratorState.MainType.Passcode( + selectedType = GeneratorState.MainType.Passcode.PasscodeType.Passphrase( + numWords = 3, + minNumWords = 3, + maxNumWords = 20, + wordSeparator = '-', + capitalize = false, + capitalizeEnabled = true, + includeNumber = false, + includeNumberEnabled = true, + + ), + ), + generatedText = "updatedPassphrase", + ) + viewModel.trySendAction(GeneratorAction.LifecycleResume) + assertEquals( + expectedState, + viewModel.stateFlow.value, + ) + } + + @Suppress("MaxLineLength") + @Test + fun `No loadOptions with default arguments should use VM state options derived state over VM state`() = + runTest { + val initialState = initialUsernameState.copy( + selectedType = GeneratorState.MainType.Username( + selectedType = GeneratorState.MainType.Username.UsernameType.PlusAddressedEmail( + email = "currentEmail", + ), + ), + ) + val viewModel = createViewModel(initialState) + // the state is updated via the call to `loadOptions()` in the init block + viewModel.stateFlow.test { + assertEquals( + initialState.copy(generatedText = "email+abcd1234@address.com"), + awaitItem(), + ) + // Setting the repository options to RANDOM_WORD to show this does NOT get used. + fakeGeneratorRepository.saveUsernameGenerationOptions( + UsernameGenerationOptions( + type = UsernameGenerationOptions.UsernameType.RANDOM_WORD, + ), + ) + // When this action is handled there will be another call to `loadOptions()` + // since we are using the default arguments with `shouldUseStorageOptions` set to + // false we should not expect a state update. + viewModel.trySendAction( + GeneratorAction.Internal.PasswordGeneratorPolicyReceive(policies = emptyList()), + ) + expectNoEvents() + } + } + @Nested inner class PasswordActions { private val defaultPasswordState = createPasswordState()