Merge pull request #6066 from element-hq/feature/bma/fix/joinRoomWithSpecialAlias

First try to resolve the room before checking for the alias validity

features/startchat/impl/src/main/kotlin/io/element/android/features/startchat/impl/joinbyaddress/JoinRoomByAddressEvent.kt

@@ -8,8 +8,8 @@
 
 package io.element.android.features.startchat.impl.joinbyaddress
 
-sealed interface JoinRoomByAddressEvents {
+sealed interface JoinRoomByAddressEvent {
-    data object Dismiss : JoinRoomByAddressEvents
+    data object Dismiss : JoinRoomByAddressEvent
-    data object Continue : JoinRoomByAddressEvents
+    data object Continue : JoinRoomByAddressEvent
-    data class UpdateAddress(val address: String) : JoinRoomByAddressEvents
+    data class UpdateAddress(val address: String) : JoinRoomByAddressEvent
 }

features/startchat/impl/src/main/kotlin/io/element/android/features/startchat/impl/joinbyaddress/JoinRoomByAddressPresenter.kt

@@ -49,16 +49,16 @@ class JoinRoomByAddressPresenter(
         var internalAddressState by remember { mutableStateOf<RoomAddressState>(RoomAddressState.Unknown) }
         var validateAddress: Boolean by remember { mutableStateOf(false) }
 
-        fun handleEvent(event: JoinRoomByAddressEvents) {
+        fun handleEvent(event: JoinRoomByAddressEvent) {
             when (event) {
-                JoinRoomByAddressEvents.Continue -> {
+                JoinRoomByAddressEvent.Continue -> {
                     when (val currentState = internalAddressState) {
                         is RoomAddressState.RoomFound -> onRoomFound(currentState)
                         else -> validateAddress = true
                     }
                 }
-                JoinRoomByAddressEvents.Dismiss -> navigator.onDismissJoinRoomByAddress()
+                JoinRoomByAddressEvent.Dismiss -> navigator.onDismissJoinRoomByAddress()
-                is JoinRoomByAddressEvents.UpdateAddress -> {
+                is JoinRoomByAddressEvent.UpdateAddress -> {
                     validateAddress = false
                     address = event.address.trim()
                 }
@@ -113,7 +113,7 @@ class JoinRoomByAddressPresenter(
             // debounce the room address resolution
             delay(300)
             val roomAlias = tryOrNull { RoomAlias(fullAddress) }
-            if (roomAlias != null && roomAliasHelper.isRoomAliasValid(roomAlias)) {
+            if (roomAlias != null) {
                 onChange(RoomAddressState.Resolving)
                 onChange(client.resolveRoomAddress(roomAlias))
             } else {
@@ -130,11 +130,21 @@ class JoinRoomByAddressPresenter(
                         if (resolved.isPresent) {
                             RoomAddressState.RoomFound(resolved.get())
                         } else {
-                            RoomAddressState.RoomNotFound
+                            roomAlias.toInvalidOrNotFound()
                         }
                     },
-                    onFailure = { _ -> RoomAddressState.RoomNotFound }
+                    onFailure = { _ ->
+                        roomAlias.toInvalidOrNotFound()
+                    }
                 )
         } ?: RoomAddressState.RoomNotFound
     }
+
+    private fun RoomAlias.toInvalidOrNotFound(): RoomAddressState {
+        return if (roomAliasHelper.isRoomAliasValid(this)) {
+            RoomAddressState.RoomNotFound
+        } else {
+            RoomAddressState.Invalid
+        }
+    }
 }

features/startchat/impl/src/main/kotlin/io/element/android/features/startchat/impl/joinbyaddress/JoinRoomByAddressState.kt

@@ -14,7 +14,7 @@ import io.element.android.libraries.matrix.api.room.alias.ResolvedRoomAlias
 data class JoinRoomByAddressState(
     val address: String,
     val addressState: RoomAddressState,
-    val eventSink: (JoinRoomByAddressEvents) -> Unit
+    val eventSink: (JoinRoomByAddressEvent) -> Unit
 )
 
 @Immutable

features/startchat/impl/src/main/kotlin/io/element/android/features/startchat/impl/joinbyaddress/JoinRoomByAddressStateProvider.kt

@@ -30,7 +30,7 @@ open class JoinRoomByAddressStateProvider : PreviewParameterProvider<JoinRoomByA
 fun aJoinRoomByAddressState(
     address: String = "",
     addressState: RoomAddressState = RoomAddressState.Unknown,
-    eventSink: (JoinRoomByAddressEvents) -> Unit = {},
+    eventSink: (JoinRoomByAddressEvent) -> Unit = {},
 ) = JoinRoomByAddressState(
     address = address,
     addressState = addressState,

features/startchat/impl/src/main/kotlin/io/element/android/features/startchat/impl/joinbyaddress/JoinRoomByAddressView.kt

@@ -50,7 +50,7 @@ fun JoinRoomByAddressView(
         modifier = modifier,
         sheetState = sheetState,
         onDismissRequest = {
-            state.eventSink(JoinRoomByAddressEvents.Dismiss)
+            state.eventSink(JoinRoomByAddressEvent.Dismiss)
         },
     ) {
         Column(
@@ -64,10 +64,10 @@ fun JoinRoomByAddressView(
                 addressState = state.addressState,
                 requestFocus = sheetState.isVisible,
                 onAddressChange = {
-                    state.eventSink(JoinRoomByAddressEvents.UpdateAddress(it))
+                    state.eventSink(JoinRoomByAddressEvent.UpdateAddress(it))
                 },
                 onContinue = {
-                    state.eventSink(JoinRoomByAddressEvents.Continue)
+                    state.eventSink(JoinRoomByAddressEvent.Continue)
                 },
             )
             Spacer(modifier = Modifier.height(24.dp))
@@ -76,7 +76,7 @@ fun JoinRoomByAddressView(
                 modifier = Modifier.fillMaxWidth(),
                 showProgress = state.addressState is RoomAddressState.Resolving,
                 onClick = {
-                    state.eventSink(JoinRoomByAddressEvents.Continue)
+                    state.eventSink(JoinRoomByAddressEvent.Continue)
                 }
             )
         }

features/startchat/impl/src/test/kotlin/io/element/android/features/startchat/impl/joinbyaddress/JoinBaseRoomByAddressPresenterTest.kt

@@ -21,6 +21,7 @@ import io.element.android.tests.testutils.lambda.lambdaRecorder
 import io.element.android.tests.testutils.test
 import kotlinx.coroutines.test.runTest
 import org.junit.Test
+import java.util.Optional
 
 class JoinBaseRoomByAddressPresenterTest {
     @Test
@@ -43,12 +44,12 @@ class JoinBaseRoomByAddressPresenterTest {
         )
         presenter.test {
             with(awaitItem()) {
-                eventSink(JoinRoomByAddressEvents.UpdateAddress("invalid_address"))
+                eventSink(JoinRoomByAddressEvent.UpdateAddress("invalid_address"))
             }
             with(awaitItem()) {
                 assertThat(address).isEqualTo("invalid_address")
                 assertThat(addressState).isEqualTo(RoomAddressState.Unknown)
-                eventSink(JoinRoomByAddressEvents.Continue)
+                eventSink(JoinRoomByAddressEvent.Continue)
             }
             // The address should be marked as invalid only after the user tries to continue
             with(awaitItem()) {
@@ -58,6 +59,107 @@ class JoinBaseRoomByAddressPresenterTest {
         }
     }
 
+    @Test
+    fun `present - invalid address - but room exists`() = runTest {
+        val presenter = createJoinRoomByAddressPresenter(
+            roomAliasHelper = FakeRoomAliasHelper(
+                isRoomAliasValidLambda = {
+                    // The SDK still return false, but we have a room for this alias
+                    false
+                }
+            )
+        )
+        presenter.test {
+            with(awaitItem()) {
+                eventSink(JoinRoomByAddressEvent.UpdateAddress("#ö:invalid.org"))
+            }
+            with(awaitItem()) {
+                assertThat(address).isEqualTo("#ö:invalid.org")
+                assertThat(addressState).isEqualTo(RoomAddressState.Unknown)
+                eventSink(JoinRoomByAddressEvent.Continue)
+            }
+            // The address should not be marked as valid
+            with(awaitItem()) {
+                assertThat(address).isEqualTo("#ö:invalid.org")
+                assertThat(addressState).isEqualTo(RoomAddressState.Resolving)
+            }
+            with(awaitItem()) {
+                assertThat(address).isEqualTo("#ö:invalid.org")
+                assertThat(addressState).isInstanceOf(RoomAddressState.RoomFound::class.java)
+            }
+        }
+    }
+
+    @Test
+    fun `present - invalid address - room does not exist`() = runTest {
+        val presenter = createJoinRoomByAddressPresenter(
+            roomAliasHelper = FakeRoomAliasHelper(
+                isRoomAliasValidLambda = {
+                    // The SDK return false
+                    false
+                }
+            ),
+            matrixClient = FakeMatrixClient(
+                resolveRoomAliasResult = {
+                    Result.success(Optional.empty())
+                }
+            )
+        )
+        presenter.test {
+            with(awaitItem()) {
+                eventSink(JoinRoomByAddressEvent.UpdateAddress("#ö:invalid.org"))
+            }
+            with(awaitItem()) {
+                assertThat(address).isEqualTo("#ö:invalid.org")
+                assertThat(addressState).isEqualTo(RoomAddressState.Unknown)
+                eventSink(JoinRoomByAddressEvent.Continue)
+            }
+            // The address should not be marked as valid
+            with(awaitItem()) {
+                assertThat(address).isEqualTo("#ö:invalid.org")
+                assertThat(addressState).isEqualTo(RoomAddressState.Resolving)
+            }
+            with(awaitItem()) {
+                assertThat(address).isEqualTo("#ö:invalid.org")
+                assertThat(addressState).isEqualTo(RoomAddressState.Invalid)
+            }
+        }
+    }
+
+    @Test
+    fun `present - invalid address - failure to resolve the room`() = runTest {
+        val presenter = createJoinRoomByAddressPresenter(
+            roomAliasHelper = FakeRoomAliasHelper(
+                isRoomAliasValidLambda = {
+                    // The SDK still return false, but we have a room for this alias
+                    false
+                }
+            ),
+            matrixClient = FakeMatrixClient(
+                resolveRoomAliasResult = { Result.failure(RuntimeException()) }
+            )
+        )
+        presenter.test {
+            with(awaitItem()) {
+                eventSink(JoinRoomByAddressEvent.UpdateAddress("#ö:invalid.org"))
+            }
+            with(awaitItem()) {
+                assertThat(address).isEqualTo("#ö:invalid.org")
+                assertThat(addressState).isEqualTo(RoomAddressState.Unknown)
+                eventSink(JoinRoomByAddressEvent.Continue)
+            }
+            // The address should not be marked as valid
+            with(awaitItem()) {
+                assertThat(address).isEqualTo("#ö:invalid.org")
+                assertThat(addressState).isEqualTo(RoomAddressState.Resolving)
+            }
+            with(awaitItem()) {
+                assertThat(address).isEqualTo("#ö:invalid.org")
+                assertThat(addressState).isEqualTo(RoomAddressState.Invalid)
+            }
+        }
+    }
+
     @Test
     fun `present - room found`() = runTest {
         val openRoomLambda = lambdaRecorder<RoomIdOrAlias, List<String>, Unit> { _, _ -> }
@@ -69,7 +171,7 @@ class JoinBaseRoomByAddressPresenterTest {
         val presenter = createJoinRoomByAddressPresenter(navigator = navigator)
         presenter.test {
             with(awaitItem()) {
-                eventSink(JoinRoomByAddressEvents.UpdateAddress("#room_found:matrix.org"))
+                eventSink(JoinRoomByAddressEvent.UpdateAddress("#room_found:matrix.org"))
             }
             with(awaitItem()) {
                 assertThat(address).isEqualTo("#room_found:matrix.org")
@@ -78,7 +180,7 @@ class JoinBaseRoomByAddressPresenterTest {
             with(awaitItem()) {
                 assertThat(address).isEqualTo("#room_found:matrix.org")
                 assertThat(addressState).isInstanceOf(RoomAddressState.RoomFound::class.java)
-                eventSink(JoinRoomByAddressEvents.Continue)
+                eventSink(JoinRoomByAddressEvent.Continue)
             }
             assert(openRoomLambda).isCalledOnce()
             assert(dismissJoinRoomByAddressLambda).isCalledOnce()
@@ -94,12 +196,12 @@ class JoinBaseRoomByAddressPresenterTest {
         )
         presenter.test {
             with(awaitItem()) {
-                eventSink(JoinRoomByAddressEvents.UpdateAddress("#room_not_found:matrix.org"))
+                eventSink(JoinRoomByAddressEvent.UpdateAddress("#room_not_found:matrix.org"))
             }
             with(awaitItem()) {
                 assertThat(address).isEqualTo("#room_not_found:matrix.org")
                 assertThat(addressState).isEqualTo(RoomAddressState.Unknown)
-                eventSink(JoinRoomByAddressEvents.Continue)
+                eventSink(JoinRoomByAddressEvent.Continue)
             }
             with(awaitItem()) {
                 assertThat(address).isEqualTo("#room_not_found:matrix.org")
@@ -121,7 +223,7 @@ class JoinBaseRoomByAddressPresenterTest {
         val presenter = createJoinRoomByAddressPresenter(navigator = navigator)
         presenter.test {
             with(awaitItem()) {
-                eventSink(JoinRoomByAddressEvents.Dismiss)
+                eventSink(JoinRoomByAddressEvent.Dismiss)
             }
             assert(dismissJoinRoomByAddressLambda).isCalledOnce()
         }

features/startchat/impl/src/test/kotlin/io/element/android/features/startchat/impl/joinbyaddress/JoinBaseRoomByAddressViewTest.kt

@@ -31,7 +31,7 @@ class JoinBaseRoomByAddressViewTest {
 
     @Test
     fun `entering text emits the expected event`() {
-        val eventsRecorder = EventsRecorder<JoinRoomByAddressEvents>()
+        val eventsRecorder = EventsRecorder<JoinRoomByAddressEvent>()
         rule.setJoinRoomByAddressView(
             aJoinRoomByAddressState(
                 eventSink = eventsRecorder,
@@ -39,19 +39,19 @@ class JoinBaseRoomByAddressViewTest {
         )
         val text = rule.activity.getString(R.string.screen_start_chat_join_room_by_address_action)
         rule.onNodeWithText(text).performTextInput("#address:matrix.org")
-        eventsRecorder.assertSingle(JoinRoomByAddressEvents.UpdateAddress("#address:matrix.org"))
+        eventsRecorder.assertSingle(JoinRoomByAddressEvent.UpdateAddress("#address:matrix.org"))
     }
 
     @Test
     fun `clicking on continue emits the expected event`() {
-        val eventsRecorder = EventsRecorder<JoinRoomByAddressEvents>()
+        val eventsRecorder = EventsRecorder<JoinRoomByAddressEvent>()
         rule.setJoinRoomByAddressView(
             aJoinRoomByAddressState(
                 eventSink = eventsRecorder,
             )
         )
         rule.clickOn(CommonStrings.action_continue)
-        eventsRecorder.assertSingle(JoinRoomByAddressEvents.Continue)
+        eventsRecorder.assertSingle(JoinRoomByAddressEvent.Continue)
     }
 }
 

features/startchat/impl/src/test/kotlin/io/element/android/features/startchat/impl/root/StartChatPresenterTest.kt

@@ -9,9 +9,6 @@
 package io.element.android.features.startchat.impl.root
 
 import androidx.compose.runtime.MutableState
-import app.cash.molecule.RecompositionMode
-import app.cash.molecule.moleculeFlow
-import app.cash.turbine.test
 import com.google.common.truth.Truth.assertThat
 import io.element.android.features.invitepeople.test.FakeStartDMAction
 import io.element.android.features.startchat.api.ConfirmingStartDmWithMatrixUser
@@ -33,6 +30,7 @@ import io.element.android.tests.testutils.WarmUpRule
 import io.element.android.tests.testutils.lambda.any
 import io.element.android.tests.testutils.lambda.lambdaRecorder
 import io.element.android.tests.testutils.lambda.value
+import io.element.android.tests.testutils.test
 import kotlinx.coroutines.test.runTest
 import org.junit.Rule
 import org.junit.Test
@@ -49,9 +47,7 @@ class StartChatPresenterTest {
         }
         val startDMAction = FakeStartDMAction(executeResult = executeResult)
         val presenter = createStartChatPresenter(startDMAction)
-        moleculeFlow(RecompositionMode.Immediate) {
+        presenter.test {
-            presenter.present()
-        }.test {
             val initialState = awaitItem()
             assertThat(initialState.startDmAction).isInstanceOf(AsyncAction.Uninitialized::class.java)
             assertThat(initialState.applicationName).isEqualTo(aBuildMeta().applicationName)
@@ -83,9 +79,7 @@ class StartChatPresenterTest {
         }
         val startDMAction = FakeStartDMAction(executeResult = executeResult)
         val presenter = createStartChatPresenter(startDMAction)
-        moleculeFlow(RecompositionMode.Immediate) {
+        presenter.test {
-            presenter.present()
-        }.test {
             val initialState = awaitItem()
             assertThat(initialState.startDmAction).isInstanceOf(AsyncAction.Uninitialized::class.java)
             assertThat(initialState.applicationName).isEqualTo(aBuildMeta().applicationName)
@@ -114,9 +108,7 @@ class StartChatPresenterTest {
         }
         val startDMAction = FakeStartDMAction(executeResult = executeResult)
         val presenter = createStartChatPresenter(startDMAction)
-        moleculeFlow(RecompositionMode.Immediate) {
+        presenter.test {
-            presenter.present()
-        }.test {
             val initialState = awaitItem()
             assertThat(initialState.startDmAction).isInstanceOf(AsyncAction.Uninitialized::class.java)
             initialState.eventSink(StartChatEvents.StartDM(matrixUser))
@@ -144,9 +136,7 @@ class StartChatPresenterTest {
         }
         val startDMAction = FakeStartDMAction(executeResult = executeResult)
         val presenter = createStartChatPresenter(startDMAction)
-        moleculeFlow(RecompositionMode.Immediate) {
+        presenter.test {
-            presenter.present()
-        }.test {
             val initialState = awaitItem()
             assertThat(initialState.startDmAction).isInstanceOf(AsyncAction.Uninitialized::class.java)
             initialState.eventSink(StartChatEvents.StartDM(matrixUser))
@@ -169,9 +159,7 @@ class StartChatPresenterTest {
     @Test
     fun `present - room directory search`() = runTest {
         val presenter = createStartChatPresenter(isRoomDirectorySearchEnabled = true)
-        moleculeFlow(RecompositionMode.Immediate) {
+        presenter.test {
-            presenter.present()
-        }.test {
             skipItems(1)
             awaitItem().let { state ->
                 assertThat(state.isRoomDirectorySearchEnabled).isTrue()

features/startchat/impl/src/test/kotlin/io/element/android/features/startchat/impl/userlist/DefaultUserListPresenterTest.kt

@@ -8,9 +8,6 @@
 
 package io.element.android.features.startchat.impl.userlist
 
-import app.cash.molecule.RecompositionMode
-import app.cash.molecule.moleculeFlow
-import app.cash.turbine.test
 import com.google.common.truth.Truth.assertThat
 import io.element.android.libraries.designsystem.theme.components.SearchBarResultState
 import io.element.android.libraries.matrix.test.FakeMatrixClient
@@ -20,6 +17,7 @@ import io.element.android.libraries.usersearch.api.UserSearchResult
 import io.element.android.libraries.usersearch.api.UserSearchResultState
 import io.element.android.libraries.usersearch.test.FakeUserRepository
 import io.element.android.tests.testutils.WarmUpRule
+import io.element.android.tests.testutils.test
 import kotlinx.collections.immutable.persistentListOf
 import kotlinx.coroutines.test.runTest
 import org.junit.Rule
@@ -40,9 +38,7 @@ class DefaultUserListPresenterTest {
                 UserListDataStore(),
                 FakeMatrixClient(),
             )
-        moleculeFlow(RecompositionMode.Immediate) {
+        presenter.test {
-            presenter.present()
-        }.test {
             skipItems(1)
             val initialState = awaitItem()
             assertThat(initialState.searchQuery).isEmpty()
@@ -62,9 +58,7 @@ class DefaultUserListPresenterTest {
                 UserListDataStore(),
                 FakeMatrixClient(),
             )
-        moleculeFlow(RecompositionMode.Immediate) {
+        presenter.test {
-            presenter.present()
-        }.test {
             skipItems(1)
             val initialState = awaitItem()
             assertThat(initialState.searchQuery).isEmpty()
@@ -84,9 +78,7 @@ class DefaultUserListPresenterTest {
                 UserListDataStore(),
                 FakeMatrixClient(),
             )
-        moleculeFlow(RecompositionMode.Immediate) {
+        presenter.test {
-            presenter.present()
-        }.test {
             skipItems(1)
             val initialState = awaitItem()
 
@@ -121,9 +113,7 @@ class DefaultUserListPresenterTest {
                 UserListDataStore(),
                 FakeMatrixClient(),
             )
-        moleculeFlow(RecompositionMode.Immediate) {
+        presenter.test {
-            presenter.present()
-        }.test {
             skipItems(1)
             val initialState = awaitItem()
 
@@ -174,9 +164,7 @@ class DefaultUserListPresenterTest {
                 UserListDataStore(),
                 FakeMatrixClient(),
             )
-        moleculeFlow(RecompositionMode.Immediate) {
+        presenter.test {
-            presenter.present()
-        }.test {
             skipItems(1)
             val initialState = awaitItem()
 
@@ -200,9 +188,7 @@ class DefaultUserListPresenterTest {
                 UserListDataStore(),
                 FakeMatrixClient(),
             )
-        moleculeFlow(RecompositionMode.Immediate) {
+        presenter.test {
-            presenter.present()
-        }.test {
             skipItems(1)
             val initialState = awaitItem()