mirror of
https://github.com/signalapp/Signal-Android.git
synced 2026-08-04 20:34:14 +01:00
Fix issue with null nextSms/Call times.
This commit is contained in:
committed by
Michelle Tang
parent
682e5e4a01
commit
01a2599c3c
+13
-10
@@ -78,7 +78,7 @@ fun VerificationCodeScreen(
|
||||
val resources = LocalResources.current
|
||||
|
||||
LaunchedEffect(state.rateLimits) {
|
||||
if (state.rateLimits.smsResendTimeRemaining > 0.seconds || state.rateLimits.callRequestTimeRemaining > 0.seconds) {
|
||||
if (state.smsResendCountdown() != null || state.callRequestCountdown() != null) {
|
||||
while (true) {
|
||||
delay(1000)
|
||||
onEvent(VerificationCodeScreenEvents.CountdownTick)
|
||||
@@ -362,8 +362,10 @@ private fun CodeField(
|
||||
|
||||
@Composable
|
||||
private fun AlternateCodeOptions(state: VerificationCodeState, onEvent: (VerificationCodeScreenEvents) -> Unit) {
|
||||
val canResendSms = state.canResendSms()
|
||||
val disabledColor = MaterialTheme.colorScheme.onSurface.copy(alpha = 0.38f)
|
||||
|
||||
val canResendSms = state.canResendSms()
|
||||
val smsCountdown = state.smsResendCountdown()
|
||||
TextButton(
|
||||
onClick = { onEvent(VerificationCodeScreenEvents.ResendSms) },
|
||||
enabled = canResendSms,
|
||||
@@ -371,14 +373,14 @@ private fun AlternateCodeOptions(state: VerificationCodeState, onEvent: (Verific
|
||||
.testTag(TestTags.VERIFICATION_CODE_RESEND_SMS_BUTTON)
|
||||
) {
|
||||
Text(
|
||||
text = if (canResendSms) {
|
||||
stringResource(R.string.VerificationCodeScreen__resend_code)
|
||||
} else {
|
||||
val totalSeconds = state.rateLimits.smsResendTimeRemaining.inWholeSeconds.toInt()
|
||||
text = if (smsCountdown != null) {
|
||||
val totalSeconds = smsCountdown.inWholeSeconds.toInt()
|
||||
val minutes = totalSeconds / 60
|
||||
val seconds = totalSeconds % 60
|
||||
stringResource(R.string.VerificationCodeScreen__resend_code) + " " +
|
||||
stringResource(R.string.VerificationCodeScreen__countdown_format, minutes, seconds)
|
||||
} else {
|
||||
stringResource(R.string.VerificationCodeScreen__resend_code)
|
||||
},
|
||||
color = if (canResendSms) MaterialTheme.colorScheme.primary else disabledColor,
|
||||
textAlign = TextAlign.Center,
|
||||
@@ -389,6 +391,7 @@ private fun AlternateCodeOptions(state: VerificationCodeState, onEvent: (Verific
|
||||
Spacer(modifier = Modifier.width(8.dp))
|
||||
|
||||
val canRequestCall = state.canRequestCall()
|
||||
val callCountdown = state.callRequestCountdown()
|
||||
TextButton(
|
||||
onClick = { onEvent(VerificationCodeScreenEvents.CallMe) },
|
||||
enabled = canRequestCall,
|
||||
@@ -396,13 +399,13 @@ private fun AlternateCodeOptions(state: VerificationCodeState, onEvent: (Verific
|
||||
.testTag(TestTags.VERIFICATION_CODE_CALL_ME_BUTTON)
|
||||
) {
|
||||
Text(
|
||||
text = if (canRequestCall) {
|
||||
stringResource(R.string.VerificationCodeScreen__call_me_instead)
|
||||
} else {
|
||||
val totalSeconds = state.rateLimits.callRequestTimeRemaining.inWholeSeconds.toInt()
|
||||
text = if (callCountdown != null) {
|
||||
val totalSeconds = callCountdown.inWholeSeconds.toInt()
|
||||
val minutes = totalSeconds / 60
|
||||
val seconds = totalSeconds % 60
|
||||
stringResource(R.string.VerificationCodeScreen__call_me_available_in, minutes, seconds)
|
||||
} else {
|
||||
stringResource(R.string.VerificationCodeScreen__call_me_instead)
|
||||
},
|
||||
color = if (canRequestCall) MaterialTheme.colorScheme.primary else disabledColor,
|
||||
textAlign = TextAlign.Center,
|
||||
|
||||
+24
-6
@@ -54,14 +54,28 @@ data class VerificationCodeState(
|
||||
)
|
||||
|
||||
/**
|
||||
* Returns true if the user can resend SMS (timer has expired)
|
||||
* Returns true if the user can resend SMS right now (a timer exists and has expired). False while a timer is still
|
||||
* counting down, and also when SMS resend is unavailable ([SmsAndCallRateLimits.smsResendTimeRemaining] is null).
|
||||
*/
|
||||
fun canResendSms(): Boolean = rateLimits.smsResendTimeRemaining <= 0.seconds
|
||||
fun canResendSms(): Boolean = rateLimits.smsResendTimeRemaining.let { it != null && it <= 0.seconds }
|
||||
|
||||
/**
|
||||
* Returns true if the user can request a call (timer has expired)
|
||||
* Returns true if the user can request a call right now (a timer exists and has expired). False while a timer is
|
||||
* still counting down, and also when calls are unavailable ([SmsAndCallRateLimits.callRequestTimeRemaining] is null).
|
||||
*/
|
||||
fun canRequestCall(): Boolean = rateLimits.callRequestTimeRemaining <= 0.seconds
|
||||
fun canRequestCall(): Boolean = rateLimits.callRequestTimeRemaining.let { it != null && it <= 0.seconds }
|
||||
|
||||
/**
|
||||
* The SMS resend cooldown to display as a countdown, or null when there is nothing to count down — either because
|
||||
* resend is available now or because it is unavailable.
|
||||
*/
|
||||
fun smsResendCountdown(): Duration? = rateLimits.smsResendTimeRemaining?.takeIf { it > 0.seconds }
|
||||
|
||||
/**
|
||||
* The call request cooldown to display as a countdown, or null when there is nothing to count down — either because
|
||||
* a call can be requested now or because it is unavailable.
|
||||
*/
|
||||
fun callRequestCountdown(): Duration? = rateLimits.callRequestTimeRemaining?.takeIf { it > 0.seconds }
|
||||
|
||||
/**
|
||||
* Returns true if the "Having Trouble" button should be shown.
|
||||
@@ -72,8 +86,12 @@ data class VerificationCodeState(
|
||||
|
||||
/**
|
||||
* Rate limit data for SMS resend and phone call request countdown timers.
|
||||
*
|
||||
* For each transport: `null` means the server won't allow that request at all (its button is disabled with no
|
||||
* countdown), [kotlin.time.Duration.ZERO] means it can be requested now, and a positive value is the countdown until
|
||||
* it becomes available.
|
||||
*/
|
||||
data class SmsAndCallRateLimits(
|
||||
val smsResendTimeRemaining: Duration = 0.seconds,
|
||||
val callRequestTimeRemaining: Duration = 0.seconds
|
||||
val smsResendTimeRemaining: Duration? = 0.seconds,
|
||||
val callRequestTimeRemaining: Duration? = 0.seconds
|
||||
)
|
||||
|
||||
+17
-18
@@ -103,9 +103,6 @@ class VerificationCodeViewModel(
|
||||
private val _state = MutableStateFlow(VerificationCodeState())
|
||||
val state: StateFlow<VerificationCodeState> = _state.asStateFlow()
|
||||
|
||||
private var nextSmsAvailableAt: Duration = 0.seconds
|
||||
private var nextCallAvailableAt: Duration = 0.seconds
|
||||
|
||||
init {
|
||||
_state
|
||||
.onEach { Log.d(TAG, "[State] $it") }
|
||||
@@ -196,8 +193,8 @@ class VerificationCodeViewModel(
|
||||
private fun applyCountdownTick(state: VerificationCodeState): VerificationCodeState {
|
||||
return state.copy(
|
||||
rateLimits = SmsAndCallRateLimits(
|
||||
smsResendTimeRemaining = (state.rateLimits.smsResendTimeRemaining - 1.seconds).coerceAtLeast(0.seconds),
|
||||
callRequestTimeRemaining = (state.rateLimits.callRequestTimeRemaining - 1.seconds).coerceAtLeast(0.seconds)
|
||||
smsResendTimeRemaining = state.rateLimits.smsResendTimeRemaining?.minus(1.seconds)?.coerceAtLeast(0.seconds),
|
||||
callRequestTimeRemaining = state.rateLimits.callRequestTimeRemaining?.minus(1.seconds)?.coerceAtLeast(0.seconds)
|
||||
)
|
||||
)
|
||||
}
|
||||
@@ -526,38 +523,40 @@ class VerificationCodeViewModel(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Builds the countdowns from a freshly-returned session. A null [SessionMetadata.nextSms]/[SessionMetadata.nextCall]
|
||||
* means the server won't permit that request, which we surface as a null remaining time (an unavailable button)
|
||||
* rather than a countdown.
|
||||
*/
|
||||
private fun computeRateLimits(session: SessionMetadata): SmsAndCallRateLimits {
|
||||
val now = clock().milliseconds
|
||||
nextSmsAvailableAt = now + (session.nextSms?.seconds ?: nextSmsAvailableAt)
|
||||
nextCallAvailableAt = now + (session.nextCall?.seconds ?: nextCallAvailableAt)
|
||||
|
||||
return SmsAndCallRateLimits(
|
||||
smsResendTimeRemaining = (nextSmsAvailableAt - clock().milliseconds).coerceAtLeast(0.seconds),
|
||||
callRequestTimeRemaining = (nextCallAvailableAt - clock().milliseconds).coerceAtLeast(0.seconds)
|
||||
smsResendTimeRemaining = session.nextSms?.seconds?.coerceAtLeast(0.seconds),
|
||||
callRequestTimeRemaining = session.nextCall?.seconds?.coerceAtLeast(0.seconds)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Seeds the resend countdowns when we first see a session. Prefers the absolute timestamps recorded when the codes
|
||||
* were actually requested (which remain accurate across leaving and re-entering this screen), falling back to
|
||||
* anchoring the session's relative nextSms/nextCall values to now.
|
||||
* anchoring the session's relative nextSms/nextCall values to now. A null value with no recorded request means the
|
||||
* transport is unavailable, surfaced as a null remaining time.
|
||||
*/
|
||||
private fun initializeRateLimits(session: SessionMetadata, parentState: RegistrationFlowState): SmsAndCallRateLimits {
|
||||
val now = clock().milliseconds
|
||||
|
||||
nextSmsAvailableAt = parentState.lastSmsVerificationCodeRequest
|
||||
val nextSmsAvailableAt: Duration? = parentState.lastSmsVerificationCodeRequest
|
||||
?.takeIf { it.e164 == parentState.sessionE164 }
|
||||
?.nextAllowedRequestTime?.milliseconds
|
||||
?: (now + (session.nextSms?.seconds ?: 0.seconds))
|
||||
?: session.nextSms?.let { now + it.seconds }
|
||||
|
||||
nextCallAvailableAt = parentState.lastCallVerificationCodeRequest
|
||||
val nextCallAvailableAt: Duration? = parentState.lastCallVerificationCodeRequest
|
||||
?.takeIf { it.e164 == parentState.sessionE164 }
|
||||
?.nextAllowedRequestTime?.milliseconds
|
||||
?: (now + (session.nextCall?.seconds ?: 0.seconds))
|
||||
?: session.nextCall?.let { now + it.seconds }
|
||||
|
||||
return SmsAndCallRateLimits(
|
||||
smsResendTimeRemaining = (nextSmsAvailableAt - now).coerceAtLeast(0.seconds),
|
||||
callRequestTimeRemaining = (nextCallAvailableAt - now).coerceAtLeast(0.seconds)
|
||||
smsResendTimeRemaining = nextSmsAvailableAt?.minus(now)?.coerceAtLeast(0.seconds),
|
||||
callRequestTimeRemaining = nextCallAvailableAt?.minus(now)?.coerceAtLeast(0.seconds)
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
+32
-2
@@ -967,6 +967,36 @@ class VerificationCodeViewModelTest {
|
||||
assertThat(emittedStates.last().sessionMetadata).isEqualTo(updatedSession)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `ResendSms success with a null nextCall marks calls unavailable instead of a bogus countdown`() = runTest {
|
||||
val updatedSession = createSessionMetadata(nextSms = 0L, nextCall = null)
|
||||
val initialState = VerificationCodeState(sessionMetadata = createSessionMetadata(), e164 = "+15551234567")
|
||||
|
||||
coEvery { mockRepository.requestVerificationCode(any(), any(), eq(VerificationCodeTransport.SMS)) } returns
|
||||
RequestResult.Success(updatedSession)
|
||||
|
||||
viewModel.applyEvent(initialState, VerificationCodeScreenEvents.ResendSms, stateEmitter)
|
||||
|
||||
assertThat(emittedStates.last().rateLimits).isEqualTo(
|
||||
SmsAndCallRateLimits(smsResendTimeRemaining = 0.seconds, callRequestTimeRemaining = null)
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `ResendSms success with a null nextSms marks SMS resend unavailable instead of a bogus countdown`() = runTest {
|
||||
val updatedSession = createSessionMetadata(nextSms = null, nextCall = 30L)
|
||||
val initialState = VerificationCodeState(sessionMetadata = createSessionMetadata(), e164 = "+15551234567")
|
||||
|
||||
coEvery { mockRepository.requestVerificationCode(any(), any(), eq(VerificationCodeTransport.SMS)) } returns
|
||||
RequestResult.Success(updatedSession)
|
||||
|
||||
viewModel.applyEvent(initialState, VerificationCodeScreenEvents.ResendSms, stateEmitter)
|
||||
|
||||
assertThat(emittedStates.last().rateLimits).isEqualTo(
|
||||
SmsAndCallRateLimits(smsResendTimeRemaining = null, callRequestTimeRemaining = 30.seconds)
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `ResendSms with success emits VerificationCodeRequested with the next allowed timestamp`() = runTest {
|
||||
val fixedNow = 1_000_000L
|
||||
@@ -1060,7 +1090,7 @@ class VerificationCodeViewModelTest {
|
||||
clockedViewModel.applyEvent(VerificationCodeState(), VerificationCodeScreenEvents.ParentStateChanged(parentFlowState), stateEmitter)
|
||||
|
||||
assertThat(emittedStates.last().rateLimits).isEqualTo(
|
||||
SmsAndCallRateLimits(smsResendTimeRemaining = 5.seconds, callRequestTimeRemaining = 0.seconds)
|
||||
SmsAndCallRateLimits(smsResendTimeRemaining = 5.seconds, callRequestTimeRemaining = null)
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1079,7 +1109,7 @@ class VerificationCodeViewModelTest {
|
||||
clockedViewModel.applyEvent(VerificationCodeState(), VerificationCodeScreenEvents.ParentStateChanged(parentFlowState), stateEmitter)
|
||||
|
||||
assertThat(emittedStates.last().rateLimits).isEqualTo(
|
||||
SmsAndCallRateLimits(smsResendTimeRemaining = 0.seconds, callRequestTimeRemaining = 0.seconds)
|
||||
SmsAndCallRateLimits(smsResendTimeRemaining = 0.seconds, callRequestTimeRemaining = null)
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user