From d695cd8b5a29b0f4979f5754e0ee6a86418c9ea8 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Tue, 7 Jul 2026 17:53:03 +0000 Subject: [PATCH] Fix regV5 SMS auto-fill and paste. --- .../VerificationCodeScreen.kt | 4 +- .../VerificationCodeScreenEvents.kt | 3 +- .../VerificationCodeViewModel.kt | 45 ++++++++----- .../VerificationCodeScreenTest.kt | 10 +-- .../VerificationCodeViewModelTest.kt | 64 ++++++++++++++++--- 5 files changed, 93 insertions(+), 33 deletions(-) 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 fb10eddacf..28213d4d7c 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 @@ -89,9 +89,7 @@ fun VerificationCodeScreen( val code = state.autoFillCode ?: return@LaunchedEffect if (code.length == VerificationCodeState.CODE_LENGTH && code.all { it.isDigit() } && !state.isSubmittingCode) { - code.forEachIndexed { index, digit -> - onEvent(VerificationCodeScreenEvents.DigitChanged(index, digit.toString())) - } + onEvent(VerificationCodeScreenEvents.DigitChanged(0, code)) } onEvent(VerificationCodeScreenEvents.ConsumeAutoFillCode) } diff --git a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeScreenEvents.kt b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeScreenEvents.kt index 64aa8a3913..b8fcdcd117 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeScreenEvents.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeScreenEvents.kt @@ -15,7 +15,8 @@ sealed class VerificationCodeScreenEvents { /** * The raw [value] of the digit field at [index] changed. The view model interprets it: a single digit is recorded * (submitting once the full code is present), an empty [value] is a backspace (deleting a digit and shifting the - * following ones left), and multi-character input (e.g. a pasted "123-456") is treated as a pasted code. + * following ones left), and multi-character input (e.g. a pasted "123-456" or an auto-filled SMS code) populates + * every field at once and submits. */ data class DigitChanged(val index: Int, val value: String) : VerificationCodeScreenEvents() { override fun toString(): String = "DigitChanged(index=$index)" 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 f44a2c1039..0c06daec94 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 @@ -191,7 +191,8 @@ class VerificationCodeViewModel( * * - an empty [value] is a backspace, deleting a digit and moving focus back * - a single digit is recorded and focus advances, submitting once the full code is present - * - multi-character input (e.g. a pasted "123-456") is treated as a pasted code + * - multi-character input (e.g. a pasted "123-456" or an auto-filled SMS code) populates every field at once and + * submits, all in this single reducer pass */ private suspend fun applyDigitChanged( state: VerificationCodeState, @@ -225,7 +226,33 @@ class VerificationCodeViewModel( } } - else -> applyPastedCode(state, remainder) + else -> applyFullCode(state, addedDigits, stateEmitter) + } + } + + /** + * Populates every digit field from a full pasted or auto-filled [code] in a single reducer pass and submits it. + * Multi-character input that isn't a complete code is ignored. + */ + private suspend fun applyFullCode( + state: VerificationCodeState, + code: String, + stateEmitter: (VerificationCodeState) -> Unit + ): VerificationCodeState { + if (code.length != CODE_LENGTH) { + Log.w(TAG, "[DigitChanged] Ignoring multi-character input containing ${code.length} digits.") + return state + } + + val updated = state.copy( + digits = code.map { it.toString() }, + focusedDigitIndex = CODE_LENGTH - 1 + ) + + return if (!updated.isSubmittingCode) { + submitCode(updated, updated.code, stateEmitter) + } else { + updated } } @@ -257,20 +284,6 @@ class VerificationCodeViewModel( return applyCodeEntered(state, code).copy(isSubmittingCode = false) } - /** - * Strips any formatting (e.g. a hyphen) from pasted text and, if what remains is a full code, populates the fields - * by reusing the [VerificationCodeState.autoFillCode] path. Pasted text that doesn't contain a full code is ignored. - */ - private fun applyPastedCode(state: VerificationCodeState, rawCode: String): VerificationCodeState { - val digits = rawCode.filter { it.isDigit() } - if (digits.length != CODE_LENGTH) { - Log.w(TAG, "[DigitChanged] Ignoring pasted text containing ${digits.length} digits.") - return state - } - - return state.copy(autoFillCode = digits) - } - private suspend fun applyCodeEntered(inputState: VerificationCodeState, code: String): VerificationCodeState { var state = inputState var sessionMetadata = state.sessionMetadata ?: return state.also { diff --git a/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeScreenTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeScreenTest.kt index 4501c52b34..3e678b13e1 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeScreenTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeScreenTest.kt @@ -216,7 +216,7 @@ class VerificationCodeScreenTest { } @Test - fun `autoFillCode emits a DigitChanged for each digit`() { + fun `autoFillCode emits a single DigitChanged with the full code`() { // Given val emittedEvents = mutableListOf() @@ -231,13 +231,13 @@ class VerificationCodeScreenTest { // When - the auto-fill effect populates the fields composeTestRule.waitUntil(timeoutMillis = 5_000) { - emittedEvents.filterIsInstance().size == 6 + emittedEvents.any { it is VerificationCodeScreenEvents.DigitChanged } } - // Then + // Then - a single event carries the whole code, rather than a burst of per-digit events val digitChanges = emittedEvents.filterIsInstance() - assert(digitChanges == (0 until 6).map { VerificationCodeScreenEvents.DigitChanged(it, "${it + 1}") }) { - "Expected a DigitChanged per digit but got $digitChanges" + assert(digitChanges == listOf(VerificationCodeScreenEvents.DigitChanged(0, "123456"))) { + "Expected a single DigitChanged(0, 123456) but got $digitChanges" } } 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 78dab1ae95..5a22e381da 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 @@ -197,8 +197,17 @@ class VerificationCodeViewModelTest { } @Test - fun `DigitChanged with pasted hyphenated text stores the stripped code in autoFillCode`() = runTest { - val initialState = VerificationCodeState() + fun `DigitChanged with pasted hyphenated text populates all digits and submits`() = runTest { + val sessionMetadata = createSessionMetadata() + val initialState = VerificationCodeState( + sessionMetadata = sessionMetadata, + e164 = "+15551234567" + ) + + coEvery { mockRepository.submitVerificationCode(any(), any()) } returns + RequestResult.NonSuccess( + NetworkController.SubmitVerificationCodeError.InvalidSessionIdOrVerificationCode("Wrong code") + ) viewModel.applyEvent( initialState, @@ -206,12 +215,23 @@ class VerificationCodeViewModelTest { stateEmitter ) - assertThat(emittedStates.last().autoFillCode).isEqualTo("123456") + coVerify { mockRepository.submitVerificationCode(sessionMetadata.id, "123456") } + assertThat(emittedStates.first().digits).isEqualTo(listOf("1", "2", "3", "4", "5", "6")) + assertThat(emittedStates.first().isSubmittingCode).isTrue() } @Test - fun `DigitChanged with a pasted plain code stores it in autoFillCode`() = runTest { - val initialState = VerificationCodeState() + fun `DigitChanged with a pasted plain code populates all digits and submits`() = runTest { + val sessionMetadata = createSessionMetadata() + val initialState = VerificationCodeState( + sessionMetadata = sessionMetadata, + e164 = "+15551234567" + ) + + coEvery { mockRepository.submitVerificationCode(any(), any()) } returns + RequestResult.NonSuccess( + NetworkController.SubmitVerificationCodeError.InvalidSessionIdOrVerificationCode("Wrong code") + ) viewModel.applyEvent( initialState, @@ -219,12 +239,17 @@ class VerificationCodeViewModelTest { stateEmitter ) - assertThat(emittedStates.last().autoFillCode).isEqualTo("123456") + coVerify { mockRepository.submitVerificationCode(sessionMetadata.id, "123456") } + assertThat(emittedStates.first().digits).isEqualTo(listOf("1", "2", "3", "4", "5", "6")) + assertThat(emittedStates.first().isSubmittingCode).isTrue() } @Test fun `DigitChanged with pasted text of the wrong length is ignored`() = runTest { - val initialState = VerificationCodeState() + val initialState = VerificationCodeState( + sessionMetadata = createSessionMetadata(), + e164 = "+15551234567" + ) viewModel.applyEvent( initialState, @@ -232,7 +257,8 @@ class VerificationCodeViewModelTest { stateEmitter ) - assertThat(emittedStates.last().autoFillCode).isNull() + coVerify(exactly = 0) { mockRepository.submitVerificationCode(any(), any()) } + assertThat(emittedStates.last().digits).isEqualTo(listOf("", "", "", "", "", "")) } @Test @@ -262,6 +288,28 @@ class VerificationCodeViewModelTest { assertThat(vm.state.value.autoFillCode).isEqualTo("123456") } + @Test + fun `DigitChanged with a full code dispatched through the event channel submits it in a single pass`() = runTest(testDispatcher) { + val sessionMetadata = createSessionMetadata() + parentState.value = RegistrationFlowState( + sessionMetadata = sessionMetadata, + sessionE164 = "+15551234567" + ) + + coEvery { mockRepository.submitVerificationCode(any(), any()) } returns + RequestResult.NonSuccess( + NetworkController.SubmitVerificationCodeError.InvalidSessionIdOrVerificationCode("Wrong code") + ) + + backgroundScope.launch { viewModel.state.collect {} } + advanceUntilIdle() + + viewModel.onEvent(VerificationCodeScreenEvents.DigitChanged(0, "123456")) + advanceUntilIdle() + + coVerify { mockRepository.submitVerificationCode(sessionMetadata.id, "123456") } + } + // ==================== applyEvent: DigitChanged Tests ==================== @Test