diff --git a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeScreen.kt b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeScreen.kt index 281ee71711..4d5aa83654 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeScreen.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeScreen.kt @@ -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, diff --git a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeState.kt b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeState.kt index 4c07ea900f..8a8f9c8228 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeState.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeState.kt @@ -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 ) diff --git a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModel.kt b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModel.kt index 52f5f0bb17..80dab4128b 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModel.kt @@ -103,9 +103,6 @@ class VerificationCodeViewModel( private val _state = MutableStateFlow(VerificationCodeState()) val state: StateFlow = _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) ) } diff --git a/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModelTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModelTest.kt index 3418389ab5..bca40c9e10 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModelTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModelTest.kt @@ -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) ) }