Improve username discrimator reservation for numberless reg.

feature/registration/src/main/java/org/signal/registration/screens/addusername/AddUsernameScreen.kt

@@ -33,13 +33,18 @@ import androidx.compose.material3.TextField
 import androidx.compose.material3.TextFieldDefaults
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.LaunchedEffect
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
+import androidx.compose.runtime.setValue
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.focus.FocusRequester
 import androidx.compose.ui.focus.focusRequester
+import androidx.compose.ui.focus.onFocusChanged
 import androidx.compose.ui.graphics.SolidColor
 import androidx.compose.ui.platform.LocalDensity
+import androidx.compose.ui.platform.LocalFocusManager
 import androidx.compose.ui.platform.testTag
 import androidx.compose.ui.res.painterResource
 import androidx.compose.ui.res.stringResource
@@ -334,6 +339,8 @@ private fun DiscriminatorField(
 ) {
   val textStyle = LocalTextStyle.current.copy(color = MaterialTheme.colorScheme.onSurface)
   val textMeasurer = rememberTextMeasurer()
+  val focusManager = LocalFocusManager.current
+  var wasFocused by remember { mutableStateOf(false) }
 
   val width = with(LocalDensity.current) {
     val content = textMeasurer.measure(state.discriminator, textStyle).size.width
@@ -354,13 +361,21 @@ private fun DiscriminatorField(
     ),
     keyboardActions = KeyboardActions(
       onDone = {
-        if (state.isSubmittable) {
+        if (state.discriminator.isBlank()) {
+          focusManager.clearFocus()
+        } else if (state.isSubmittable) {
           onEvent(AddUsernameScreenEvents.NextClicked)
         }
       }
     ),
     modifier = Modifier
       .width(width)
+      .onFocusChanged { focusState ->
+        if (wasFocused && !focusState.isFocused) {
+          onEvent(AddUsernameScreenEvents.DiscriminatorFocusLost)
+        }
+        wasFocused = focusState.isFocused
+      }
       .testTag(TestTags.ADD_USERNAME_DISCRIMINATOR_FIELD)
   )
 }

feature/registration/src/main/java/org/signal/registration/screens/addusername/AddUsernameScreenEvents.kt

@@ -21,6 +21,9 @@ sealed class AddUsernameScreenEvents {
     override fun toString(): String = "DiscriminatorChanged(value=${value.censor()})"
   }
 
+  /** The discriminator field lost focus, settling whatever the user left in it. */
+  data object DiscriminatorFocusLost : AddUsernameScreenEvents()
+
   /**
    * Internal: the user paused typing long enough for the entered username to be validated and reserved. A null
    * [discriminator] means the service should assign one.

feature/registration/src/main/java/org/signal/registration/screens/addusername/AddUsernameViewModel.kt

@@ -17,7 +17,6 @@ import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.asStateFlow
 import kotlinx.coroutines.flow.debounce
-import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.flow.receiveAsFlow
@@ -70,7 +69,6 @@ class AddUsernameViewModel(
       .launchIn(viewModelScope)
 
     entryChanges
-      .distinctUntilChanged()
       .debounce(ENTRY_DEBOUNCE)
       .onEach { onEvent(it) }
       .launchIn(viewModelScope)
@@ -90,6 +88,7 @@ class AddUsernameViewModel(
     when (event) {
       is AddUsernameScreenEvents.UsernameChanged -> applyUsernameChanged(state, event.value, stateEmitter)
       is AddUsernameScreenEvents.DiscriminatorChanged -> applyDiscriminatorChanged(state, event.value, stateEmitter)
+      is AddUsernameScreenEvents.DiscriminatorFocusLost -> applyDiscriminatorFocusLost(state)
       is AddUsernameScreenEvents.EntrySettled -> applyEntrySettled(state, event, stateEmitter)
       is AddUsernameScreenEvents.ReservationCompleted -> applyReservationCompleted(state, event, stateEmitter)
       is AddUsernameScreenEvents.LearnMoreClicked -> _actions.trySend(AddUsernameScreenActions.OpenLearnMoreArticle)
@@ -124,8 +123,9 @@ class AddUsernameViewModel(
   }
 
   /**
-   * Non-digits are dropped as they're typed, since a discriminator can only ever be digits. A blank discriminator hands
-   * control back to the service, matching the behavior of clearing the field in the app's username editor.
+   * Non-digits are dropped as they're typed, since a discriminator can only ever be digits.
+   *
+   * Emptying the field hands control back to the service once the field loses focus (see [applyDiscriminatorFocusLost]).
    */
   private fun applyDiscriminatorChanged(state: AddUsernameState, discriminator: String, stateEmitter: (AddUsernameState) -> Unit) {
     val digitsOnly = discriminator.filter { it in '0'..'9' }
@@ -141,14 +141,31 @@ class AddUsernameViewModel(
       discriminator = digitsOnly,
       isDiscriminatorUserSet = isUserSet,
       validationError = null,
-      reservation = null,
+      reservation = if (isUserSet) null else state.reservation,
       isReserving = false
     )
 
     stateEmitter(updated)
-    scheduleReservation(updated)
+
+    if (isUserSet) {
+      scheduleReservation(updated)
+    }
   }
 
+  /**
+   * Reserves right away rather than through [scheduleReservation], since focus loss already means the user is done
+   * typing and there is nothing left to debounce.
+   */
+  private fun applyDiscriminatorFocusLost(state: AddUsernameState) {
+    if (state.discriminator.isBlank() && state.username.isNotBlank()) {
+      onEvent(AddUsernameScreenEvents.EntrySettled(state.username, null))
+    }
+  }
+
+  /**
+   * Deliberately not de-duplicated: the entry can travel away from a pair and back to it, and those repeats still need
+   * a fresh reservation.
+   */
   private fun scheduleReservation(state: AddUsernameState) {
     if (state.username.isNotBlank()) {
       entryChanges.tryEmit(AddUsernameScreenEvents.EntrySettled(state.username, state.requestedDiscriminator))

feature/registration/src/test/java/org/signal/registration/screens/addusername/AddUsernameScreenTest.kt

@@ -12,10 +12,12 @@ import androidx.compose.ui.test.onNodeWithTag
 import androidx.compose.ui.test.performClick
 import androidx.compose.ui.test.performTextInput
 import androidx.compose.ui.test.performTextReplacement
+import androidx.compose.ui.test.requestFocus
 import androidx.test.core.app.ApplicationProvider
 import assertk.assertThat
 import assertk.assertions.contains
 import assertk.assertions.containsExactly
+import assertk.assertions.doesNotContain
 import assertk.assertions.isEqualTo
 import assertk.assertions.isNotNull
 import org.junit.Rule
@@ -87,6 +89,37 @@ class AddUsernameScreenTest {
     assertThat(emittedEvent).isEqualTo(AddUsernameScreenEvents.DiscriminatorChanged("77"))
   }
 
+  @Test
+  fun `showing the discriminator field does not emit DiscriminatorFocusLost`() {
+    val emittedEvents = mutableListOf<AddUsernameScreenEvents>()
+
+    composeTestRule.setContent {
+      SignalTheme {
+        AddUsernameScreen(state = reservedState, onEvent = { emittedEvents.add(it) })
+      }
+    }
+
+    composeTestRule.onNodeWithTag(TestTags.ADD_USERNAME_DISCRIMINATOR_FIELD).assertIsDisplayed()
+
+    assertThat(emittedEvents).doesNotContain(AddUsernameScreenEvents.DiscriminatorFocusLost)
+  }
+
+  @Test
+  fun `when the discriminator field loses focus, DiscriminatorFocusLost is emitted`() {
+    val emittedEvents = mutableListOf<AddUsernameScreenEvents>()
+
+    composeTestRule.setContent {
+      SignalTheme {
+        AddUsernameScreen(state = reservedState, onEvent = { emittedEvents.add(it) })
+      }
+    }
+
+    composeTestRule.onNodeWithTag(TestTags.ADD_USERNAME_DISCRIMINATOR_FIELD).requestFocus()
+    composeTestRule.onNodeWithTag(TestTags.ADD_USERNAME_FIELD).requestFocus()
+
+    assertThat(emittedEvents).contains(AddUsernameScreenEvents.DiscriminatorFocusLost)
+  }
+
   @Test
   fun `when typing in the username field, UsernameChanged is emitted`() {
     var emittedEvent: AddUsernameScreenEvents? = null

feature/registration/src/test/java/org/signal/registration/screens/addusername/AddUsernameViewModelTest.kt

@@ -14,6 +14,7 @@ import assertk.assertions.isFalse
 import assertk.assertions.isNull
 import assertk.assertions.isTrue
 import io.mockk.coEvery
+import io.mockk.coVerify
 import io.mockk.mockk
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -138,6 +139,8 @@ class AddUsernameViewModelTest {
     advanceUntilIdle()
     viewModel.onEvent(AddUsernameScreenEvents.DiscriminatorChanged(""))
     advanceUntilIdle()
+    viewModel.onEvent(AddUsernameScreenEvents.DiscriminatorFocusLost)
+    advanceUntilIdle()
 
     assertThat(viewModel.state.value.isDiscriminatorUserSet).isFalse()
     assertThat(viewModel.state.value.discriminator).isEqualTo("45")
@@ -175,8 +178,8 @@ class AddUsernameViewModelTest {
   }
 
   @Test
-  fun `clearing a service-assigned discriminator empties the field`() = runTest(testDispatcher) {
-    coEvery { mockRepository.reserveUsername("maya") } returns RequestResult.Success(Username("maya.45"))
+  fun `emptying the discriminator while still focused does not reserve`() = runTest(testDispatcher) {
+    coEvery { mockRepository.reserveUsername("maya") } returns RequestResult.Success(Username("maya.45")) andThen RequestResult.Success(Username("maya.99"))
 
     viewModel.onEvent(AddUsernameScreenEvents.UsernameChanged("maya"))
     advanceUntilIdle()
@@ -185,8 +188,51 @@ class AddUsernameViewModelTest {
 
     assertThat(viewModel.state.value.discriminator).isEmpty()
     assertThat(viewModel.state.value.isDiscriminatorUserSet).isFalse()
-    assertThat(viewModel.state.value.requestedDiscriminator).isNull()
     assertThat(viewModel.state.value.validationError).isNull()
+    coVerify(exactly = 1) { mockRepository.reserveUsername("maya") }
+  }
+
+  @Test
+  fun `an emptied discriminator keeps the entry submittable while the user decides`() = runTest(testDispatcher) {
+    coEvery { mockRepository.reserveUsername("maya") } returns RequestResult.Success(Username("maya.45"))
+
+    viewModel.onEvent(AddUsernameScreenEvents.UsernameChanged("maya"))
+    advanceUntilIdle()
+    viewModel.onEvent(AddUsernameScreenEvents.DiscriminatorChanged(""))
+    advanceUntilIdle()
+
+    assertThat(viewModel.state.value.reservation).isEqualTo(Username("maya.45"))
+    assertThat(viewModel.state.value.isSubmittable).isTrue()
+  }
+
+  @Test
+  fun `losing focus with an empty discriminator hands it back to the service`() = runTest(testDispatcher) {
+    coEvery { mockRepository.reserveUsername("maya") } returns RequestResult.Success(Username("maya.45")) andThen RequestResult.Success(Username("maya.99"))
+
+    viewModel.onEvent(AddUsernameScreenEvents.UsernameChanged("maya"))
+    advanceUntilIdle()
+    viewModel.onEvent(AddUsernameScreenEvents.DiscriminatorChanged(""))
+    advanceUntilIdle()
+    viewModel.onEvent(AddUsernameScreenEvents.DiscriminatorFocusLost)
+    advanceUntilIdle()
+
+    assertThat(viewModel.state.value.discriminator).isEqualTo("99")
+    assertThat(viewModel.state.value.isDiscriminatorUserSet).isFalse()
+    assertThat(viewModel.state.value.reservation).isEqualTo(Username("maya.99"))
+    assertThat(viewModel.state.value.isSubmittable).isTrue()
+  }
+
+  @Test
+  fun `losing focus with a discriminator still in the field reserves nothing new`() = runTest(testDispatcher) {
+    coEvery { mockRepository.reserveUsername("maya") } returns RequestResult.Success(Username("maya.45"))
+
+    viewModel.onEvent(AddUsernameScreenEvents.UsernameChanged("maya"))
+    advanceUntilIdle()
+    viewModel.onEvent(AddUsernameScreenEvents.DiscriminatorFocusLost)
+    advanceUntilIdle()
+
+    assertThat(viewModel.state.value.discriminator).isEqualTo("45")
+    coVerify(exactly = 1) { mockRepository.reserveUsername("maya") }
   }
 
   @Test