BIT-1524, BIT-898: Update generated text (#964)

This commit is contained in:
David Perez 2024-02-07 14:31:52 -06:00 committed by Álison Fernandes
parent 6294e656ce
commit dc2e07c130
5 changed files with 213 additions and 57 deletions

View file

@ -18,6 +18,7 @@ import androidx.compose.foundation.verticalScroll
import androidx.compose.material3.ExperimentalMaterial3Api
import androidx.compose.material3.OutlinedTextField
import androidx.compose.material3.Slider
import androidx.compose.material3.SliderDefaults
import androidx.compose.material3.SnackbarDuration
import androidx.compose.material3.SnackbarHost
import androidx.compose.material3.SnackbarHostState
@ -33,6 +34,7 @@ import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.input.nestedscroll.nestedScroll
import androidx.compose.ui.layout.onGloballyPositioned
import androidx.compose.ui.platform.LocalContext
@ -470,7 +472,7 @@ private fun PasscodeOptionsItem(
currentSubState: GeneratorState.MainType.Passcode,
onSubStateOptionClicked: (GeneratorState.MainType.Passcode.PasscodeTypeOption) -> Unit,
) {
val possibleSubStates = GeneratorState.MainType.Passcode.PasscodeTypeOption.values().toList()
val possibleSubStates = GeneratorState.MainType.Passcode.PasscodeTypeOption.entries
val optionsWithStrings = possibleSubStates.associateWith { stringResource(id = it.labelRes) }
BitwardenMultiSelectButton(
@ -622,6 +624,12 @@ private fun PasswordLengthSliderItem(
},
valueRange = sliderRange,
steps = maxValue - 1,
colors = SliderDefaults.colors(
activeTickColor = Color.Transparent,
inactiveTickColor = Color.Transparent,
disabledActiveTickColor = Color.Transparent,
disabledInactiveTickColor = Color.Transparent,
),
modifier = Modifier
.semantics { testTag = "PasswordLengthSlider" }
.weight(1f),
@ -782,8 +790,7 @@ private fun ColumnScope.PassphraseTypeContent(
PassphraseWordSeparatorInputItem(
wordSeparator = passphraseTypeState.wordSeparator,
onPassphraseWordSeparatorChange =
passphraseHandlers.onPassphraseWordSeparatorChange,
onPassphraseWordSeparatorChange = passphraseHandlers.onPassphraseWordSeparatorChange,
)
Spacer(modifier = Modifier.height(16.dp))
@ -835,13 +842,19 @@ private fun PassphraseWordSeparatorInputItem(
) {
BitwardenTextField(
label = stringResource(id = R.string.word_separator),
value = wordSeparator?.toString() ?: "",
value = wordSeparator?.toString().orEmpty(),
onValueChange = {
onPassphraseWordSeparatorChange(it.toCharArray().firstOrNull())
// When onValueChange triggers and we don't update the value for whatever reason,
// onValueChange triggers again with the previous value.
// To avoid passphrase regeneration, we filter out those re-emissions.
val char = it.firstOrNull()
if (char != wordSeparator) {
onPassphraseWordSeparatorChange(char)
}
},
modifier = Modifier
.semantics { testTag = "WordSeparatorEntry" }
.width(267.dp)
.fillMaxWidth()
.padding(horizontal = 16.dp),
)
}

View file

@ -57,6 +57,7 @@ import javax.inject.Inject
import kotlin.math.max
private const val KEY_STATE = "state"
private const val NO_GENERATED_TEXT: String = "-"
/**
* ViewModel responsible for handling user interactions in the generator screen.
@ -77,7 +78,7 @@ class GeneratorViewModel @Inject constructor(
private val policyManager: PolicyManager,
) : BaseViewModel<GeneratorState, GeneratorEvent, GeneratorAction>(
initialState = savedStateHandle[KEY_STATE] ?: GeneratorState(
generatedText = "",
generatedText = NO_GENERATED_TEXT,
selectedType = when (GeneratorArgs(savedStateHandle).type) {
GeneratorMode.Modal.Username -> Username()
GeneratorMode.Modal.Password -> Passcode()
@ -309,7 +310,7 @@ class GeneratorViewModel @Inject constructor(
}
}
private fun loadUsernameOptions(selectedType: Username) {
private fun loadUsernameOptions(selectedType: Username, forceRegeneration: Boolean = false) {
val options = generatorRepository.getUsernameGenerationOptions()
val updatedSelectedType = when (val type = selectedType.selectedType) {
is PlusAddressedEmail -> {
@ -351,7 +352,7 @@ class GeneratorViewModel @Inject constructor(
}
}
mutableStateFlow.update { it.copy(selectedType = updatedSelectedType) }
updateGeneratorMainType(forceRegeneration = forceRegeneration) { updatedSelectedType }
}
private fun savePasswordOptionsToDisk(password: Password) {
@ -520,7 +521,7 @@ class GeneratorViewModel @Inject constructor(
private suspend fun generatePassphrase(passphrase: Passphrase) {
val request = PassphraseGeneratorRequest(
numWords = passphrase.numWords.toUByte(),
wordSeparator = passphrase.wordSeparator.toString(),
wordSeparator = passphrase.wordSeparator?.toString() ?: " ",
capitalize = passphrase.capitalize,
includeNumber = passphrase.includeNumber,
)
@ -536,7 +537,7 @@ class GeneratorViewModel @Inject constructor(
private fun handleRegenerationClick() {
// Go through the update process with the current state to trigger a
// regeneration of the generated text for the same state.
updateGeneratorMainType(isManualRegeneration = true) { mutableStateFlow.value.selectedType }
updateGeneratorMainType(forceRegeneration = true) { mutableStateFlow.value.selectedType }
}
private fun handleCopyClick() {
@ -650,10 +651,12 @@ class GeneratorViewModel @Inject constructor(
private fun handleMainTypeOptionSelect(action: GeneratorAction.MainTypeOptionSelect) {
when (action.mainTypeOption) {
GeneratorState.MainTypeOption.PASSWORD -> {
loadPasscodeOptions(Passcode(), usePolicyDefault = true)
loadPasscodeOptions(selectedType = Passcode(), usePolicyDefault = true)
}
GeneratorState.MainTypeOption.USERNAME -> loadUsernameOptions(Username())
GeneratorState.MainTypeOption.USERNAME -> {
loadUsernameOptions(selectedType = Username(), forceRegeneration = true)
}
}
}
@ -883,18 +886,24 @@ class GeneratorViewModel @Inject constructor(
when (action.usernameTypeOption) {
Username.UsernameTypeOption.PLUS_ADDRESSED_EMAIL -> loadUsernameOptions(
selectedType = Username(selectedType = PlusAddressedEmail()),
forceRegeneration = true,
)
Username.UsernameTypeOption.CATCH_ALL_EMAIL -> loadUsernameOptions(
selectedType = Username(selectedType = CatchAllEmail()),
forceRegeneration = true,
)
// We do not force regeneration here since the API can fail if the data is entered
// incorrectly. This will only be generated when the user clicks the regenerate button.
Username.UsernameTypeOption.FORWARDED_EMAIL_ALIAS -> loadUsernameOptions(
selectedType = Username(selectedType = ForwardedEmailAlias()),
forceRegeneration = false,
)
Username.UsernameTypeOption.RANDOM_WORD -> loadUsernameOptions(
selectedType = Username(selectedType = RandomWord()),
forceRegeneration = true,
)
}
}
@ -1170,7 +1179,7 @@ class GeneratorViewModel @Inject constructor(
//region Utility Functions
private inline fun updateGeneratorMainType(
isManualRegeneration: Boolean = false,
forceRegeneration: Boolean = false,
crossinline block: (GeneratorState.MainType) -> GeneratorState.MainType?,
) {
val currentSelectedType = mutableStateFlow.value.selectedType
@ -1195,30 +1204,34 @@ class GeneratorViewModel @Inject constructor(
is Username -> when (val selectedType = updatedMainType.selectedType) {
is ForwardedEmailAlias -> {
saveForwardedEmailAliasServiceTypeToDisk(selectedType)
if (isManualRegeneration) {
if (forceRegeneration) {
generateForwardedEmailAlias(selectedType)
} else {
mutableStateFlow.update { it.copy(generatedText = NO_GENERATED_TEXT) }
}
}
is CatchAllEmail -> {
saveCatchAllEmailOptionsToDisk(selectedType)
if (isManualRegeneration) {
if (forceRegeneration) {
generateCatchAllEmail(selectedType)
} else {
mutableStateFlow.update { it.copy(generatedText = NO_GENERATED_TEXT) }
}
}
is PlusAddressedEmail -> {
savePlusAddressedEmailOptionsToDisk(selectedType)
if (isManualRegeneration) {
if (forceRegeneration) {
generatePlusAddressedEmail(selectedType)
} else {
mutableStateFlow.update { it.copy(generatedText = NO_GENERATED_TEXT) }
}
}
is RandomWord -> {
saveRandomWordOptionsToDisk(selectedType)
if (isManualRegeneration) {
generateRandomWordUsername(selectedType)
}
generateRandomWordUsername(selectedType)
}
}
}
@ -1226,7 +1239,10 @@ class GeneratorViewModel @Inject constructor(
}
private suspend fun generateForwardedEmailAlias(alias: ForwardedEmailAlias) {
val request = alias.selectedServiceType?.toUsernameGeneratorRequest() ?: return
val request = alias.selectedServiceType?.toUsernameGeneratorRequest() ?: run {
mutableStateFlow.update { it.copy(generatedText = NO_GENERATED_TEXT) }
return
}
val result = generatorRepository.generateForwardedServiceUsername(request)
sendAction(GeneratorAction.Internal.UpdateGeneratedForwardedServiceUsernameResult(result))
}
@ -1242,10 +1258,14 @@ class GeneratorViewModel @Inject constructor(
}
private suspend fun generateCatchAllEmail(catchAllEmail: CatchAllEmail) {
val domainName = catchAllEmail.domainName.orNullIfBlank() ?: run {
mutableStateFlow.update { it.copy(generatedText = NO_GENERATED_TEXT) }
return
}
val result = generatorRepository.generateCatchAllEmail(
UsernameGeneratorRequest.Catchall(
type = AppendType.Random,
domain = catchAllEmail.domainName,
domain = domainName,
),
)
sendAction(GeneratorAction.Internal.UpdateGeneratedCatchAllUsernameResult(result))
@ -1498,7 +1518,7 @@ data class GeneratorState(
* Provides a list of available main types for the generator.
*/
val typeOptions: List<MainTypeOption>
get() = MainTypeOption.values().toList()
get() = MainTypeOption.entries.toList()
/**
* Enum representing the main type options for the generator, such as PASSWORD and USERNAME.

View file

@ -2,18 +2,22 @@ package com.x8bit.bitwarden.ui.tools.feature.generator.util
import com.bitwarden.generators.ForwarderServiceType
import com.bitwarden.generators.UsernameGeneratorRequest
import com.x8bit.bitwarden.ui.platform.base.util.orNullIfBlank
import com.x8bit.bitwarden.ui.tools.feature.generator.GeneratorState.MainType.Username.UsernameType.ForwardedEmailAlias.ServiceType
/**
* Converts a [ServiceType] to a [UsernameGeneratorRequest.Forwarded].
*/
fun ServiceType.toUsernameGeneratorRequest(): UsernameGeneratorRequest.Forwarded {
@Suppress("ReturnCount", "LongMethod")
fun ServiceType.toUsernameGeneratorRequest(): UsernameGeneratorRequest.Forwarded? {
return when (this) {
is ServiceType.AddyIo -> {
val accessToken = this.apiAccessToken.orNullIfBlank() ?: return null
val domain = this.domainName.orNullIfBlank() ?: return null
UsernameGeneratorRequest.Forwarded(
service = ForwarderServiceType.AddyIo(
apiToken = this.apiAccessToken,
domain = this.domainName,
apiToken = accessToken,
domain = domain,
baseUrl = this.baseUrl,
),
website = null,
@ -21,31 +25,51 @@ fun ServiceType.toUsernameGeneratorRequest(): UsernameGeneratorRequest.Forwarded
}
is ServiceType.DuckDuckGo -> {
UsernameGeneratorRequest.Forwarded(
service = ForwarderServiceType.DuckDuckGo(token = this.apiKey),
website = null,
)
this
.apiKey
.orNullIfBlank()
?.let {
UsernameGeneratorRequest.Forwarded(
service = ForwarderServiceType.DuckDuckGo(token = it),
website = null,
)
}
}
is ServiceType.FirefoxRelay -> {
UsernameGeneratorRequest.Forwarded(
service = ForwarderServiceType.Firefox(apiToken = this.apiAccessToken),
website = null,
)
this
.apiAccessToken
.orNullIfBlank()
?.let {
UsernameGeneratorRequest.Forwarded(
service = ForwarderServiceType.Firefox(apiToken = it),
website = null,
)
}
}
is ServiceType.FastMail -> {
UsernameGeneratorRequest.Forwarded(
service = ForwarderServiceType.Fastmail(apiToken = this.apiKey),
website = null,
)
this
.apiKey
.orNullIfBlank()
?.let {
UsernameGeneratorRequest.Forwarded(
service = ForwarderServiceType.Fastmail(apiToken = it),
website = null,
)
}
}
is ServiceType.SimpleLogin -> {
UsernameGeneratorRequest.Forwarded(
service = ForwarderServiceType.SimpleLogin(apiKey = this.apiKey),
website = null,
)
this
.apiKey
.orNullIfBlank()
?.let {
UsernameGeneratorRequest.Forwarded(
service = ForwarderServiceType.SimpleLogin(apiKey = it),
website = null,
)
}
}
}
}

View file

@ -132,7 +132,10 @@ class GeneratorViewModelTest : BaseViewModelTest() {
viewModel.actionChannel.trySend(GeneratorAction.SelectClick)
assertEquals(GeneratorEvent.NavigateBack, eventTurbine.awaitItem())
assertEquals(GeneratorResult.Username("username"), generatorResultTurbine.awaitItem())
assertEquals(
GeneratorResult.Username(username = "-"),
generatorResultTurbine.awaitItem(),
)
}
}
@ -484,6 +487,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
val expectedState =
initialPasscodeState.copy(
generatedText = "email+abcd1234@address.com",
selectedType = GeneratorState.MainType.Username(
GeneratorState.MainType.Username.UsernameType.PlusAddressedEmail(
email = "currentEmail",
@ -558,6 +562,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
)
val expectedState = initialUsernameState.copy(
generatedText = "email+abcd1234@address.com",
selectedType = GeneratorState.MainType.Username(
selectedType = GeneratorState
.MainType
@ -587,6 +592,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
)
val expectedState = initialUsernameState.copy(
generatedText = "-",
selectedType = GeneratorState.MainType.Username(
selectedType = GeneratorState.MainType.Username.UsernameType.CatchAllEmail(),
),
@ -612,6 +618,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
)
val expectedState = initialUsernameState.copy(
generatedText = "-",
selectedType = GeneratorState.MainType.Username(
selectedType = GeneratorState
.MainType
@ -639,6 +646,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
)
val expectedState = initialUsernameState.copy(
generatedText = "randomWord",
selectedType = GeneratorState.MainType.Username(
selectedType = GeneratorState.MainType.Username.UsernameType.RandomWord(),
),
@ -1209,6 +1217,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
viewModel.actionChannel.trySend(action)
val expectedState = defaultForwardedEmailAliasState.copy(
generatedText = "-",
selectedType = GeneratorState.MainType.Username(
selectedType = GeneratorState
.MainType
@ -1262,6 +1271,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
viewModel.actionChannel.trySend(action)
val expectedState = defaultAddyIoState.copy(
generatedText = "-",
selectedType = GeneratorState.MainType.Username(
GeneratorState.MainType.Username.UsernameType.ForwardedEmailAlias(
selectedServiceType = GeneratorState
@ -1301,6 +1311,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
viewModel.actionChannel.trySend(action)
val expectedState = defaultAddyIoState.copy(
generatedText = "-",
selectedType = GeneratorState.MainType.Username(
GeneratorState.MainType.Username.UsernameType.ForwardedEmailAlias(
selectedServiceType = GeneratorState
@ -1351,6 +1362,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
viewModel.actionChannel.trySend(action)
val expectedState = defaultDuckDuckGoState.copy(
generatedText = "-",
selectedType = GeneratorState.MainType.Username(
GeneratorState.MainType.Username.UsernameType.ForwardedEmailAlias(
selectedServiceType = GeneratorState
@ -1401,6 +1413,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
viewModel.actionChannel.trySend(action)
val expectedState = defaultFastMailState.copy(
generatedText = "-",
selectedType = GeneratorState.MainType.Username(
GeneratorState.MainType.Username.UsernameType.ForwardedEmailAlias(
selectedServiceType = GeneratorState
@ -1452,6 +1465,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
viewModel.actionChannel.trySend(action)
val expectedState = defaultFirefoxRelayState.copy(
generatedText = "-",
selectedType = GeneratorState.MainType.Username(
GeneratorState.MainType.Username.UsernameType.ForwardedEmailAlias(
selectedServiceType = GeneratorState
@ -1503,6 +1517,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
viewModel.actionChannel.trySend(action)
val expectedState = defaultSimpleLoginState.copy(
generatedText = "-",
selectedType = GeneratorState.MainType.Username(
GeneratorState.MainType.Username.UsernameType.ForwardedEmailAlias(
selectedServiceType = GeneratorState
@ -1547,6 +1562,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
)
val expectedState = defaultPlusAddressedEmailState.copy(
generatedText = "-",
selectedType = GeneratorState.MainType.Username(
selectedType = GeneratorState
.MainType
@ -1589,6 +1605,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
)
val expectedState = defaultCatchAllEmailState.copy(
generatedText = "-",
selectedType = GeneratorState.MainType.Username(
selectedType = GeneratorState
.MainType
@ -1629,6 +1646,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
)
val expectedState = defaultRandomWordState.copy(
generatedText = "randomWord",
selectedType = GeneratorState.MainType.Username(
GeneratorState.MainType.Username.UsernameType.RandomWord(
capitalize = true,
@ -1654,6 +1672,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
)
val expectedState = defaultRandomWordState.copy(
generatedText = "randomWord",
selectedType = GeneratorState.MainType.Username(
GeneratorState.MainType.Username.UsernameType.RandomWord(
includeNumber = true,

View file

@ -1,12 +1,38 @@
package com.x8bit.bitwarden.ui.tools.feature.generator.util
import com.bitwarden.generators.ForwarderServiceType
import com.bitwarden.generators.UsernameGeneratorRequest
import com.x8bit.bitwarden.ui.tools.feature.generator.GeneratorState.MainType.Username.UsernameType.ForwardedEmailAlias.ServiceType
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertNull
import org.junit.jupiter.api.Test
internal class ServiceTypeExtensionsTest {
@Test
fun `toUsernameGeneratorRequest for AddyIo returns null when apiAccessToken is blank`() {
val addyIoServiceType = ServiceType.AddyIo(
apiAccessToken = "",
domainName = "test.com",
baseUrl = "http://test.com",
)
val request = addyIoServiceType.toUsernameGeneratorRequest()
assertNull(request)
}
@Test
fun `toUsernameGeneratorRequest for AddyIo returns null when domainName is blank`() {
val addyIoServiceType = ServiceType.AddyIo(
apiAccessToken = "testToken",
domainName = "",
baseUrl = "http://test.com",
)
val request = addyIoServiceType.toUsernameGeneratorRequest()
assertNull(request)
}
@Test
fun `toUsernameGeneratorRequest for AddyIo returns correct request`() {
val addyIoServiceType = ServiceType.AddyIo(
@ -17,14 +43,24 @@ internal class ServiceTypeExtensionsTest {
val request = addyIoServiceType.toUsernameGeneratorRequest()
assertEquals(
ForwarderServiceType.AddyIo(
apiToken = "testToken",
domain = "test.com",
baseUrl = "http://test.com",
UsernameGeneratorRequest.Forwarded(
service = ForwarderServiceType.AddyIo(
apiToken = "testToken",
domain = "test.com",
baseUrl = "http://test.com",
),
website = null,
),
request.service,
request,
)
assertEquals(null, request.website)
}
@Test
fun `toUsernameGeneratorRequest for DuckDuckGo returns null when apiKey is blank`() {
val duckDuckGoServiceType = ServiceType.DuckDuckGo(apiKey = "")
val request = duckDuckGoServiceType.toUsernameGeneratorRequest()
assertNull(request)
}
@Test
@ -32,8 +68,21 @@ internal class ServiceTypeExtensionsTest {
val duckDuckGoServiceType = ServiceType.DuckDuckGo(apiKey = "testKey")
val request = duckDuckGoServiceType.toUsernameGeneratorRequest()
assertEquals(ForwarderServiceType.DuckDuckGo("testKey"), request.service)
assertEquals(null, request.website)
assertEquals(
UsernameGeneratorRequest.Forwarded(
service = ForwarderServiceType.DuckDuckGo(token = "testKey"),
website = null,
),
request,
)
}
@Test
fun `toUsernameGeneratorRequest for FirefoxRelay returns null when apiAccessToken is blank`() {
val firefoxRelayServiceType = ServiceType.FirefoxRelay(apiAccessToken = "")
val request = firefoxRelayServiceType.toUsernameGeneratorRequest()
assertNull(request)
}
@Test
@ -41,8 +90,21 @@ internal class ServiceTypeExtensionsTest {
val firefoxRelayServiceType = ServiceType.FirefoxRelay(apiAccessToken = "testToken")
val request = firefoxRelayServiceType.toUsernameGeneratorRequest()
assertEquals(ForwarderServiceType.Firefox(apiToken = "testToken"), request.service)
assertEquals(null, request.website)
assertEquals(
UsernameGeneratorRequest.Forwarded(
service = ForwarderServiceType.Firefox(apiToken = "testToken"),
website = null,
),
request,
)
}
@Test
fun `toUsernameGeneratorRequest for FastMail returns null when apiKey is blank`() {
val fastMailServiceType = ServiceType.FastMail(apiKey = "")
val request = fastMailServiceType.toUsernameGeneratorRequest()
assertNull(request)
}
@Test
@ -50,8 +112,21 @@ internal class ServiceTypeExtensionsTest {
val fastMailServiceType = ServiceType.FastMail(apiKey = "testKey")
val request = fastMailServiceType.toUsernameGeneratorRequest()
assertEquals(ForwarderServiceType.Fastmail(apiToken = "testKey"), request.service)
assertEquals(null, request.website)
assertEquals(
UsernameGeneratorRequest.Forwarded(
service = ForwarderServiceType.Fastmail(apiToken = "testKey"),
website = null,
),
request,
)
}
@Test
fun `toUsernameGeneratorRequest for SimpleLogin returns null when apiKey is blank`() {
val simpleLoginServiceType = ServiceType.SimpleLogin(apiKey = "")
val request = simpleLoginServiceType.toUsernameGeneratorRequest()
assertNull(request)
}
@Test
@ -59,7 +134,12 @@ internal class ServiceTypeExtensionsTest {
val simpleLoginServiceType = ServiceType.SimpleLogin(apiKey = "testKey")
val request = simpleLoginServiceType.toUsernameGeneratorRequest()
assertEquals(ForwarderServiceType.SimpleLogin(apiKey = "testKey"), request.service)
assertEquals(null, request.website)
assertEquals(
UsernameGeneratorRequest.Forwarded(
service = ForwarderServiceType.SimpleLogin(apiKey = "testKey"),
website = null,
),
request,
)
}
}