mirror of
https://github.com/signalapp/Signal-Android.git
synced 2026-08-04 20:34:14 +01:00
Improve error message when you can't get a voice call for verification code.
This commit is contained in:
committed by
Michelle Tang
parent
ad3d537d92
commit
71bd8f9ec3
+48
-2
@@ -53,7 +53,9 @@ import androidx.lifecycle.Lifecycle
|
||||
import androidx.lifecycle.compose.LifecycleEventEffect
|
||||
import kotlinx.coroutines.delay
|
||||
import org.signal.core.ui.compose.AllDevicePreviews
|
||||
import org.signal.core.ui.compose.Dialogs
|
||||
import org.signal.core.ui.compose.Previews
|
||||
import org.signal.network.api.RegistrationApiV2.VerificationCodeTransport
|
||||
import org.signal.registration.R
|
||||
import org.signal.registration.screens.OnePaneRegistrationScaffold
|
||||
import org.signal.registration.screens.RegistrationScaffold
|
||||
@@ -100,8 +102,6 @@ fun VerificationCodeScreen(
|
||||
state.snackbars.incorrectVerificationCode -> resources.getString(R.string.VerificationCodeScreen__incorrect_code) to VerificationCodeScreenEvents.IncorrectVerificationCodeSnackbarDismissed
|
||||
state.snackbars.networkError -> resources.getString(R.string.VerificationCodeScreen__network_error) to VerificationCodeScreenEvents.NetworkErrorSnackbarDismissed
|
||||
state.snackbars.rateLimitedRetryAfter != null -> resources.getString(R.string.VerificationCodeScreen__too_many_attempts_try_again_in_s, state.snackbars.rateLimitedRetryAfter.toString()) to VerificationCodeScreenEvents.RateLimitedSnackbarDismissed
|
||||
state.snackbars.unableToSendSms -> resources.getString(R.string.VerificationCodeScreen__unable_to_send_sms) to VerificationCodeScreenEvents.UnableToSendSmsSnackbarDismissed
|
||||
state.snackbars.couldNotRequestCodeWithSelectedTransport -> resources.getString(R.string.VerificationCodeScreen__could_not_send_code_via_selected_method) to VerificationCodeScreenEvents.CouldNotRequestCodeWithSelectedTransportSnackbarDismissed
|
||||
state.snackbars.unknownError -> resources.getString(R.string.VerificationCodeScreen__an_unexpected_error_occurred) to VerificationCodeScreenEvents.UnknownErrorSnackbarDismissed
|
||||
state.snackbars.registrationError -> resources.getString(R.string.VerificationCodeScreen__registration_error) to VerificationCodeScreenEvents.RegistrationErrorSnackbarDismissed
|
||||
else -> return@LaunchedEffect
|
||||
@@ -111,6 +111,8 @@ fun VerificationCodeScreen(
|
||||
onEvent(dismissedEvent)
|
||||
}
|
||||
|
||||
RequestCodeErrorDialogs(state.dialogs, onEvent)
|
||||
|
||||
LaunchedEffect(state.focusedDigitIndex) {
|
||||
focusRequesters[state.focusedDigitIndex].requestFocus()
|
||||
}
|
||||
@@ -149,6 +151,50 @@ fun VerificationCodeScreen(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Modal dialogs for failures that occur while requesting a verification code (resend SMS / call me). Unlike the
|
||||
* inline snackbars used for code submission, these block until acknowledged so the user can't miss them.
|
||||
*/
|
||||
@Composable
|
||||
private fun RequestCodeErrorDialogs(dialogs: VerificationCodeState.Dialogs, onEvent: (VerificationCodeScreenEvents) -> Unit) {
|
||||
dialogs.providerRejectedTransport?.let { transport ->
|
||||
val message = when (transport) {
|
||||
VerificationCodeTransport.VOICE -> stringResource(R.string.VerificationCodeScreen__could_not_call_provider_rejected)
|
||||
VerificationCodeTransport.SMS -> stringResource(R.string.VerificationCodeScreen__could_not_sms_provider_rejected)
|
||||
}
|
||||
Dialogs.SimpleMessageDialog(
|
||||
message = message,
|
||||
dismiss = stringResource(android.R.string.ok),
|
||||
onDismiss = { onEvent(VerificationCodeScreenEvents.ProviderRejectedDialogDismissed) }
|
||||
)
|
||||
return
|
||||
}
|
||||
|
||||
val simpleError: Pair<String, VerificationCodeScreenEvents>? = when {
|
||||
dialogs.networkError -> stringResource(R.string.VerificationCodeScreen__network_error) to VerificationCodeScreenEvents.NetworkErrorDialogDismissed
|
||||
dialogs.rateLimitedRetryAfter != null -> {
|
||||
val message = if (dialogs.rateLimitedRetryAfter.isPositive()) {
|
||||
stringResource(R.string.VerificationCodeScreen__too_many_attempts_try_again_in_s, dialogs.rateLimitedRetryAfter.toString())
|
||||
} else {
|
||||
stringResource(R.string.VerificationCodeScreen__too_many_attempts)
|
||||
}
|
||||
message to VerificationCodeScreenEvents.RateLimitedDialogDismissed
|
||||
}
|
||||
dialogs.couldNotRequestCodeWithSelectedTransport -> stringResource(R.string.VerificationCodeScreen__could_not_send_code_via_selected_method) to VerificationCodeScreenEvents.CouldNotRequestCodeWithSelectedTransportDialogDismissed
|
||||
dialogs.unableToSendSms -> stringResource(R.string.VerificationCodeScreen__unable_to_send_sms) to VerificationCodeScreenEvents.UnableToSendSmsDialogDismissed
|
||||
dialogs.unknownError -> stringResource(R.string.VerificationCodeScreen__an_unexpected_error_occurred) to VerificationCodeScreenEvents.UnknownErrorDialogDismissed
|
||||
else -> null
|
||||
}
|
||||
|
||||
simpleError?.let { (message, dismissedEvent) ->
|
||||
Dialogs.SimpleMessageDialog(
|
||||
message = message,
|
||||
dismiss = stringResource(android.R.string.ok),
|
||||
onDismiss = { onEvent(dismissedEvent) }
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@Composable
|
||||
private fun OnePaneLayout(
|
||||
params: RegistrationScaffold.Params.OnePane,
|
||||
|
||||
+16
-4
@@ -54,11 +54,23 @@ sealed class VerificationCodeScreenEvents {
|
||||
/** The rate limited snackbar was shown and dismissed. */
|
||||
data object RateLimitedSnackbarDismissed : VerificationCodeScreenEvents()
|
||||
|
||||
/** The unable-to-send-SMS snackbar was shown and dismissed. */
|
||||
data object UnableToSendSmsSnackbarDismissed : VerificationCodeScreenEvents()
|
||||
/** The network error dialog from requesting a code was dismissed. */
|
||||
data object NetworkErrorDialogDismissed : VerificationCodeScreenEvents()
|
||||
|
||||
/** The could-not-request-code-with-selected-transport snackbar was shown and dismissed. */
|
||||
data object CouldNotRequestCodeWithSelectedTransportSnackbarDismissed : VerificationCodeScreenEvents()
|
||||
/** The unknown error dialog from requesting a code was dismissed. */
|
||||
data object UnknownErrorDialogDismissed : VerificationCodeScreenEvents()
|
||||
|
||||
/** The rate limited dialog from requesting a code was dismissed. */
|
||||
data object RateLimitedDialogDismissed : VerificationCodeScreenEvents()
|
||||
|
||||
/** The unable-to-send-SMS dialog was dismissed. */
|
||||
data object UnableToSendSmsDialogDismissed : VerificationCodeScreenEvents()
|
||||
|
||||
/** The could-not-request-code-with-selected-transport dialog was dismissed. */
|
||||
data object CouldNotRequestCodeWithSelectedTransportDialogDismissed : VerificationCodeScreenEvents()
|
||||
|
||||
/** The delivery-provider-rejected dialog was dismissed. */
|
||||
data object ProviderRejectedDialogDismissed : VerificationCodeScreenEvents()
|
||||
|
||||
/** The incorrect verification code snackbar was shown and dismissed. */
|
||||
data object IncorrectVerificationCodeSnackbarDismissed : VerificationCodeScreenEvents()
|
||||
|
||||
+16
-5
@@ -6,6 +6,7 @@
|
||||
package org.signal.registration.screens.verificationcode
|
||||
|
||||
import org.signal.network.api.RegistrationApiV2.SessionMetadata
|
||||
import org.signal.network.api.RegistrationApiV2.VerificationCodeTransport
|
||||
import kotlin.time.Duration
|
||||
import kotlin.time.Duration.Companion.seconds
|
||||
|
||||
@@ -19,9 +20,10 @@ data class VerificationCodeState(
|
||||
val digits: List<String> = List(CODE_LENGTH) { "" },
|
||||
val focusedDigitIndex: Int = 0,
|
||||
val showContactSupportSheet: Boolean = false,
|
||||
val snackbars: Snackbars = Snackbars()
|
||||
val snackbars: Snackbars = Snackbars(),
|
||||
val dialogs: Dialogs = Dialogs()
|
||||
) {
|
||||
override fun toString(): String = "VerificationCodeState(sessionMetadata=$sessionMetadata, e164=$e164, isSubmittingCode=$isSubmittingCode, rateLimits=$rateLimits, incorrectCodeAttempts=$incorrectCodeAttempts, autoFillCode=${autoFillCode?.let { "present" }}, digitsEntered=${digits.count { it.isNotEmpty() }}, focusedDigitIndex=$focusedDigitIndex, showContactSupportSheet=$showContactSupportSheet, snackbars=$snackbars)"
|
||||
override fun toString(): String = "VerificationCodeState(sessionMetadata=$sessionMetadata, e164=$e164, isSubmittingCode=$isSubmittingCode, rateLimits=$rateLimits, incorrectCodeAttempts=$incorrectCodeAttempts, autoFillCode=${autoFillCode?.let { "present" }}, digitsEntered=${digits.count { it.isNotEmpty() }}, focusedDigitIndex=$focusedDigitIndex, showContactSupportSheet=$showContactSupportSheet, snackbars=$snackbars, dialogs=$dialogs)"
|
||||
|
||||
/**
|
||||
* The full code as currently entered. Only meaningful when [isComplete] is true.
|
||||
@@ -42,15 +44,24 @@ data class VerificationCodeState(
|
||||
fun emptyDigits(): List<String> = List(CODE_LENGTH) { "" }
|
||||
}
|
||||
|
||||
/** Transient error messages shown as snackbars. Cleared once the snackbar has been shown and dismissed. */
|
||||
/** Transient errors from submitting a code or registering, shown as snackbars. Cleared once shown and dismissed. */
|
||||
data class Snackbars(
|
||||
val networkError: Boolean = false,
|
||||
val unknownError: Boolean = false,
|
||||
val rateLimitedRetryAfter: Duration? = null,
|
||||
val incorrectVerificationCode: Boolean = false,
|
||||
val registrationError: Boolean = false
|
||||
)
|
||||
|
||||
/** Errors from requesting a verification code (resend SMS / call me), shown as modal dialogs so they aren't missed. */
|
||||
data class Dialogs(
|
||||
val networkError: Boolean = false,
|
||||
val unknownError: Boolean = false,
|
||||
val rateLimitedRetryAfter: Duration? = null,
|
||||
val unableToSendSms: Boolean = false,
|
||||
val couldNotRequestCodeWithSelectedTransport: Boolean = false,
|
||||
val incorrectVerificationCode: Boolean = false,
|
||||
val registrationError: Boolean = false
|
||||
/** Nonnull when the delivery provider rejected the request. Carries the failed transport for accurate wording. */
|
||||
val providerRejectedTransport: VerificationCodeTransport? = null
|
||||
)
|
||||
|
||||
/**
|
||||
|
||||
+13
-9
@@ -139,8 +139,12 @@ class VerificationCodeViewModel(
|
||||
is VerificationCodeScreenEvents.NetworkErrorSnackbarDismissed -> state.copy(snackbars = state.snackbars.copy(networkError = false))
|
||||
is VerificationCodeScreenEvents.UnknownErrorSnackbarDismissed -> state.copy(snackbars = state.snackbars.copy(unknownError = false))
|
||||
is VerificationCodeScreenEvents.RateLimitedSnackbarDismissed -> state.copy(snackbars = state.snackbars.copy(rateLimitedRetryAfter = null))
|
||||
is VerificationCodeScreenEvents.UnableToSendSmsSnackbarDismissed -> state.copy(snackbars = state.snackbars.copy(unableToSendSms = false))
|
||||
is VerificationCodeScreenEvents.CouldNotRequestCodeWithSelectedTransportSnackbarDismissed -> state.copy(snackbars = state.snackbars.copy(couldNotRequestCodeWithSelectedTransport = false))
|
||||
is VerificationCodeScreenEvents.NetworkErrorDialogDismissed -> state.copy(dialogs = state.dialogs.copy(networkError = false))
|
||||
is VerificationCodeScreenEvents.UnknownErrorDialogDismissed -> state.copy(dialogs = state.dialogs.copy(unknownError = false))
|
||||
is VerificationCodeScreenEvents.RateLimitedDialogDismissed -> state.copy(dialogs = state.dialogs.copy(rateLimitedRetryAfter = null))
|
||||
is VerificationCodeScreenEvents.UnableToSendSmsDialogDismissed -> state.copy(dialogs = state.dialogs.copy(unableToSendSms = false))
|
||||
is VerificationCodeScreenEvents.CouldNotRequestCodeWithSelectedTransportDialogDismissed -> state.copy(dialogs = state.dialogs.copy(couldNotRequestCodeWithSelectedTransport = false))
|
||||
is VerificationCodeScreenEvents.ProviderRejectedDialogDismissed -> state.copy(dialogs = state.dialogs.copy(providerRejectedTransport = null))
|
||||
is VerificationCodeScreenEvents.IncorrectVerificationCodeSnackbarDismissed -> state.copy(snackbars = state.snackbars.copy(incorrectVerificationCode = false))
|
||||
is VerificationCodeScreenEvents.RegistrationErrorSnackbarDismissed -> state.copy(snackbars = state.snackbars.copy(registrationError = false))
|
||||
is VerificationCodeScreenEvents.CountdownTick -> applyCountdownTick(state)
|
||||
@@ -467,13 +471,13 @@ class VerificationCodeViewModel(
|
||||
when (val error = result.error) {
|
||||
is RequestVerificationCodeError.InvalidRequest -> {
|
||||
Log.w(TAG, "[RequestCode][$transport] Invalid request: ${error.message}")
|
||||
state.copy(snackbars = state.snackbars.copy(unknownError = true))
|
||||
state.copy(dialogs = state.dialogs.copy(unknownError = true))
|
||||
}
|
||||
is RequestVerificationCodeError.RateLimited -> {
|
||||
Log.w(TAG, "[RequestCode][$transport] Rate limited (retryAfter: ${error.retryAfter}).")
|
||||
parentEventEmitter(RegistrationFlowEvent.SessionUpdated(error.session))
|
||||
state.copy(
|
||||
snackbars = state.snackbars.copy(rateLimitedRetryAfter = error.retryAfter),
|
||||
dialogs = state.dialogs.copy(rateLimitedRetryAfter = error.retryAfter),
|
||||
sessionMetadata = error.session,
|
||||
rateLimits = computeRateLimits(error.session)
|
||||
)
|
||||
@@ -482,7 +486,7 @@ class VerificationCodeViewModel(
|
||||
Log.w(TAG, "[RequestCode][$transport] Could not fulfill with requested transport.")
|
||||
parentEventEmitter(RegistrationFlowEvent.SessionUpdated(error.session))
|
||||
state.copy(
|
||||
snackbars = state.snackbars.copy(couldNotRequestCodeWithSelectedTransport = true),
|
||||
dialogs = state.dialogs.copy(couldNotRequestCodeWithSelectedTransport = true),
|
||||
sessionMetadata = error.session,
|
||||
rateLimits = computeRateLimits(error.session)
|
||||
)
|
||||
@@ -496,7 +500,7 @@ class VerificationCodeViewModel(
|
||||
Log.w(TAG, "[RequestCode][$transport] Missing request information or already verified.")
|
||||
parentEventEmitter(RegistrationFlowEvent.SessionUpdated(error.session))
|
||||
state.copy(
|
||||
snackbars = state.snackbars.copy(unableToSendSms = true),
|
||||
dialogs = state.dialogs.copy(unableToSendSms = true),
|
||||
sessionMetadata = error.session,
|
||||
rateLimits = computeRateLimits(error.session)
|
||||
)
|
||||
@@ -508,17 +512,17 @@ class VerificationCodeViewModel(
|
||||
}
|
||||
is RequestVerificationCodeError.ThirdPartyServiceError -> {
|
||||
Log.w(TAG, "[RequestCode][$transport] Third party service error. ${error.data}")
|
||||
state.copy(snackbars = state.snackbars.copy(unableToSendSms = true))
|
||||
state.copy(dialogs = state.dialogs.copy(providerRejectedTransport = transport))
|
||||
}
|
||||
}
|
||||
}
|
||||
is RequestResult.RetryableNetworkError -> {
|
||||
Log.w(TAG, "[RequestCode][$transport] Network error.", result.networkError)
|
||||
state.copy(snackbars = state.snackbars.copy(networkError = true))
|
||||
state.copy(dialogs = state.dialogs.copy(networkError = true))
|
||||
}
|
||||
is RequestResult.ApplicationError -> {
|
||||
Log.w(TAG, "[RequestCode][$transport] Unknown application error.", result.cause)
|
||||
state.copy(snackbars = state.snackbars.copy(unknownError = true))
|
||||
state.copy(dialogs = state.dialogs.copy(unknownError = true))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -105,6 +105,10 @@
|
||||
<string name="VerificationCodeScreen__unable_to_send_sms">We are unable to send an SMS to your number. Please try again in several hours.</string>
|
||||
<!-- Snackbar shown when the selected transport (SMS/voice) is unavailable -->
|
||||
<string name="VerificationCodeScreen__could_not_send_code_via_selected_method">Could not send code via the selected method. Please try another option.</string>
|
||||
<!-- Dialog shown when the provider rejected placing a verification phone call to the user\'s number -->
|
||||
<string name="VerificationCodeScreen__could_not_call_provider_rejected">We couldn\'t call you with a verification code. Please try again, or request the code via SMS instead.</string>
|
||||
<!-- Dialog shown when the provider rejected sending a verification SMS to the user\'s number -->
|
||||
<string name="VerificationCodeScreen__could_not_sms_provider_rejected">We couldn\'t send you an SMS with a verification code. Please try again, or request the code via a phone call instead.</string>
|
||||
<!-- Snackbar shown for registration errors -->
|
||||
<string name="VerificationCodeScreen__registration_error">Registration failed. Please try again.</string>
|
||||
<!-- Button text for having trouble with verification -->
|
||||
|
||||
+12
-12
@@ -1145,7 +1145,7 @@ class VerificationCodeViewModelTest {
|
||||
|
||||
viewModel.applyEvent(initialState, VerificationCodeScreenEvents.ResendSms, stateEmitter)
|
||||
|
||||
assertThat(emittedStates.last().snackbars.rateLimitedRetryAfter).isEqualTo(45.seconds)
|
||||
assertThat(emittedStates.last().dialogs.rateLimitedRetryAfter).isEqualTo(45.seconds)
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -1160,7 +1160,7 @@ class VerificationCodeViewModelTest {
|
||||
|
||||
viewModel.applyEvent(initialState, VerificationCodeScreenEvents.ResendSms, stateEmitter)
|
||||
|
||||
assertThat(emittedStates.last().snackbars.unknownError).isTrue()
|
||||
assertThat(emittedStates.last().dialogs.unknownError).isTrue()
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -1175,7 +1175,7 @@ class VerificationCodeViewModelTest {
|
||||
|
||||
viewModel.applyEvent(initialState, VerificationCodeScreenEvents.ResendSms, stateEmitter)
|
||||
|
||||
assertThat(emittedStates.last().snackbars.couldNotRequestCodeWithSelectedTransport).isTrue()
|
||||
assertThat(emittedStates.last().dialogs.couldNotRequestCodeWithSelectedTransport).isTrue()
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -1222,11 +1222,11 @@ class VerificationCodeViewModelTest {
|
||||
|
||||
viewModel.applyEvent(initialState, VerificationCodeScreenEvents.ResendSms, stateEmitter)
|
||||
|
||||
assertThat(emittedStates.last().snackbars.unableToSendSms).isTrue()
|
||||
assertThat(emittedStates.last().dialogs.unableToSendSms).isTrue()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `ResendSms with ThirdPartyServiceError returns UnableToSendSms event`() = runTest {
|
||||
fun `ResendSms with ThirdPartyServiceError shows provider rejected dialog for SMS`() = runTest {
|
||||
val sessionMetadata = createSessionMetadata()
|
||||
val initialState = VerificationCodeState(sessionMetadata = sessionMetadata)
|
||||
|
||||
@@ -1239,7 +1239,7 @@ class VerificationCodeViewModelTest {
|
||||
|
||||
viewModel.applyEvent(initialState, VerificationCodeScreenEvents.ResendSms, stateEmitter)
|
||||
|
||||
assertThat(emittedStates.last().snackbars.unableToSendSms).isTrue()
|
||||
assertThat(emittedStates.last().dialogs.providerRejectedTransport).isEqualTo(VerificationCodeTransport.SMS)
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -1252,7 +1252,7 @@ class VerificationCodeViewModelTest {
|
||||
|
||||
viewModel.applyEvent(initialState, VerificationCodeScreenEvents.ResendSms, stateEmitter)
|
||||
|
||||
assertThat(emittedStates.last().snackbars.networkError).isTrue()
|
||||
assertThat(emittedStates.last().dialogs.networkError).isTrue()
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -1265,7 +1265,7 @@ class VerificationCodeViewModelTest {
|
||||
|
||||
viewModel.applyEvent(initialState, VerificationCodeScreenEvents.ResendSms, stateEmitter)
|
||||
|
||||
assertThat(emittedStates.last().snackbars.unknownError).isTrue()
|
||||
assertThat(emittedStates.last().dialogs.unknownError).isTrue()
|
||||
}
|
||||
|
||||
// ==================== applyEvent: CallMe Tests ====================
|
||||
@@ -1307,7 +1307,7 @@ class VerificationCodeViewModelTest {
|
||||
|
||||
viewModel.applyEvent(initialState, VerificationCodeScreenEvents.CallMe, stateEmitter)
|
||||
|
||||
assertThat(emittedStates.last().snackbars.rateLimitedRetryAfter).isEqualTo(90.seconds)
|
||||
assertThat(emittedStates.last().dialogs.rateLimitedRetryAfter).isEqualTo(90.seconds)
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -1322,11 +1322,11 @@ class VerificationCodeViewModelTest {
|
||||
|
||||
viewModel.applyEvent(initialState, VerificationCodeScreenEvents.CallMe, stateEmitter)
|
||||
|
||||
assertThat(emittedStates.last().snackbars.couldNotRequestCodeWithSelectedTransport).isTrue()
|
||||
assertThat(emittedStates.last().dialogs.couldNotRequestCodeWithSelectedTransport).isTrue()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `CallMe with ThirdPartyServiceError returns UnableToSendSms event`() = runTest {
|
||||
fun `CallMe with ThirdPartyServiceError shows provider rejected dialog for VOICE`() = runTest {
|
||||
val sessionMetadata = createSessionMetadata()
|
||||
val initialState = VerificationCodeState(sessionMetadata = sessionMetadata)
|
||||
|
||||
@@ -1339,7 +1339,7 @@ class VerificationCodeViewModelTest {
|
||||
|
||||
viewModel.applyEvent(initialState, VerificationCodeScreenEvents.CallMe, stateEmitter)
|
||||
|
||||
assertThat(emittedStates.last().snackbars.unableToSendSms).isTrue()
|
||||
assertThat(emittedStates.last().dialogs.providerRejectedTransport).isEqualTo(VerificationCodeTransport.VOICE)
|
||||
}
|
||||
|
||||
// ==================== applyEvent: Foregrounded Tests ====================
|
||||
|
||||
Reference in New Issue
Block a user