Merge pull request #2040 from element-hq/feature/bma/fixLinkify

fix linkify

appnav/src/test/kotlin/io/element/android/appnav/loggedin/LoggedInPresenterTest.kt

@@ -60,8 +60,9 @@ class LoggedInPresenterTest {
         }.test {
             val initialState = awaitItem()
             assertThat(initialState.showSyncSpinner).isFalse()
+            roomListService.postSyncIndicator(RoomListService.SyncIndicator.Show)
             consumeItemsUntilPredicate { it.showSyncSpinner }
-            roomListService.postState(RoomListService.State.Running)
+            roomListService.postSyncIndicator(RoomListService.SyncIndicator.Hide)
             consumeItemsUntilPredicate { !it.showSyncSpinner }
         }
     }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactory.kt

@@ -21,6 +21,7 @@ import android.text.style.URLSpan
 import android.text.util.Linkify
 import androidx.core.text.buildSpannedString
 import androidx.core.text.getSpans
+import androidx.core.text.toSpannable
 import androidx.core.text.util.LinkifyCompat
 import io.element.android.features.location.api.Location
 import io.element.android.features.messages.api.timeline.HtmlConverterProvider
@@ -68,12 +69,15 @@ class TimelineItemContentMessageFactory @Inject constructor(
 
     suspend fun create(content: MessageContent, senderDisplayName: String, eventId: EventId?): TimelineItemEventContent {
         return when (val messageType = content.type) {
-            is EmoteMessageType -> TimelineItemEmoteContent(
+            is EmoteMessageType -> {
-                body = "* $senderDisplayName ${messageType.body}",
+                val emoteBody = "* $senderDisplayName ${messageType.body}"
-                htmlDocument = messageType.formatted?.toHtmlDocument(prefix = "* $senderDisplayName"),
+                TimelineItemEmoteContent(
-                formattedBody = parseHtml(messageType.formatted, prefix = "* $senderDisplayName"),
+                    body = emoteBody,
-                isEdited = content.isEdited,
+                    htmlDocument = messageType.formatted?.toHtmlDocument(prefix = "* $senderDisplayName"),
-            )
+                    formattedBody = parseHtml(messageType.formatted, prefix = "* $senderDisplayName") ?: emoteBody.withLinks(),
+                    isEdited = content.isEdited,
+                )
+            }
             is ImageMessageType -> {
                 val aspectRatio = aspectRatioOf(messageType.info?.width, messageType.info?.height)
                 TimelineItemImageContent(
@@ -171,21 +175,21 @@ class TimelineItemContentMessageFactory @Inject constructor(
             is NoticeMessageType -> TimelineItemNoticeContent(
                 body = messageType.body,
                 htmlDocument = messageType.formatted?.toHtmlDocument(),
-                formattedBody = parseHtml(messageType.formatted),
+                formattedBody = parseHtml(messageType.formatted) ?: messageType.body.withLinks(),
                 isEdited = content.isEdited,
             )
             is TextMessageType -> {
                 TimelineItemTextContent(
                     body = messageType.body,
                     htmlDocument = messageType.formatted?.toHtmlDocument(),
-                    formattedBody = parseHtml(messageType.formatted),
+                    formattedBody = parseHtml(messageType.formatted) ?: messageType.body.withLinks(),
                     isEdited = content.isEdited,
                 )
             }
             is OtherMessageType -> TimelineItemTextContent(
                 body = messageType.body,
                 htmlDocument = null,
-                formattedBody = null,
+                formattedBody = messageType.body.withLinks(),
                 isEdited = content.isEdited,
             )
         }
@@ -226,7 +230,7 @@ class TimelineItemContentMessageFactory @Inject constructor(
             Pair(start, end)
         }
         // Find and set as URLSpans any links present in the text
-        LinkifyCompat.addLinks(this, Linkify.WEB_URLS or Linkify.PHONE_NUMBERS)
+        LinkifyCompat.addLinks(this, Linkify.WEB_URLS or Linkify.PHONE_NUMBERS or Linkify.EMAIL_ADDRESSES)
         // Restore old spans if they don't conflict with the new ones
         for ((urlSpan, location) in oldURLSpans) {
             val (start, end) = location
@@ -237,3 +241,11 @@ class TimelineItemContentMessageFactory @Inject constructor(
         return this
     }
 }
+
+@Suppress("USELESS_ELVIS")
+private fun String.withLinks(): CharSequence? {
+    /* Note: toSpannable() can return null when running unit tests */
+    val spannable = toSpannable() ?: return null
+    val addedLinks = LinkifyCompat.addLinks(spannable, Linkify.WEB_URLS or Linkify.PHONE_NUMBERS or Linkify.EMAIL_ADDRESSES)
+    return spannable.takeIf { addedLinks }
+}

features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactoryTest.kt

@@ -17,8 +17,9 @@
 package io.element.android.features.messages.impl.timeline.factories.event
 
 import android.text.SpannableString
-import android.text.SpannableStringBuilder
+import android.text.Spanned
 import android.text.style.URLSpan
+import androidx.core.text.buildSpannedString
 import androidx.core.text.inSpans
 import com.google.common.truth.Truth.assertThat
 import io.element.android.features.location.api.Location
@@ -144,9 +145,40 @@ class TimelineItemContentMessageFactoryTest {
         assertThat(result).isEqualTo(expected)
     }
 
+    @Test
+    fun `test create TextMessageType with simple link`() = runTest {
+        val sut = createTimelineItemContentMessageFactory()
+        val result = sut.create(
+            content = createMessageContent(type = TextMessageType("https://www.example.org", null)),
+            senderDisplayName = "Bob",
+            eventId = AN_EVENT_ID,
+        ) as TimelineItemTextContent
+        val expected = TimelineItemTextContent(
+            body = "https://www.example.org",
+            htmlDocument = null,
+            plainText = "https://www.example.org",
+            isEdited = false,
+            formattedBody = buildSpannedString {
+                inSpans(URLSpan("https://www.example.org")) {
+                    append("https://www.example.org")
+                }
+            }
+        )
+        assertThat(result.body).isEqualTo(expected.body)
+        assertThat(result.htmlDocument).isEqualTo(expected.htmlDocument)
+        assertThat(result.plainText).isEqualTo(expected.plainText)
+        assertThat(result.isEdited).isEqualTo(expected.isEdited)
+        assertThat(result.formattedBody).isInstanceOf(Spanned::class.java)
+        val spanned = result.formattedBody as Spanned
+        assertThat(spanned.toString()).isEqualTo("https://www.example.org")
+        val urlSpans = spanned.getSpans(0, spanned.length, URLSpan::class.java)
+        assertThat(urlSpans).hasLength(1)
+        assertThat(urlSpans[0].url).isEqualTo("https://www.example.org")
+    }
+
     @Test
     fun `test create TextMessageType with HTML formatted body`() = runTest {
-        val expected = SpannableStringBuilder().apply {
+        val expected = buildSpannedString {
             append("link to ")
             inSpans(URLSpan("https://matrix.org")) {
                 append("https://matrix.org")
@@ -160,10 +192,12 @@ class TimelineItemContentMessageFactoryTest {
             htmlConverterTransform = { expected }
         )
         val result = sut.create(
-            content = createMessageContent(type = TextMessageType(
+            content = createMessageContent(
-                body = "body",
+                type = TextMessageType(
-                formatted = FormattedBody(MessageFormat.HTML, expected.toString())
+                    body = "body",
-            )),
+                    formatted = FormattedBody(MessageFormat.HTML, expected.toString())
+                )
+            ),
             senderDisplayName = "Bob",
             eventId = AN_EVENT_ID,
         )
@@ -176,10 +210,12 @@ class TimelineItemContentMessageFactoryTest {
             htmlConverterTransform = { it }
         )
         val result = sut.create(
-            content = createMessageContent(type = TextMessageType(
+            content = createMessageContent(
-                body = "body",
+                type = TextMessageType(
-                formatted = FormattedBody(MessageFormat.UNKNOWN, "formatted")
+                    body = "body",
-            )),
+                    formatted = FormattedBody(MessageFormat.UNKNOWN, "formatted")
+                )
+            ),
             senderDisplayName = "Bob",
             eventId = AN_EVENT_ID,
         )
@@ -520,10 +556,12 @@ class TimelineItemContentMessageFactoryTest {
     fun `test create NoticeMessageType with HTML formatted body`() = runTest {
         val sut = createTimelineItemContentMessageFactory()
         val result = sut.create(
-            content = createMessageContent(type = NoticeMessageType(
+            content = createMessageContent(
-                body = "body",
+                type = NoticeMessageType(
-                formatted = FormattedBody(MessageFormat.HTML, "formatted")
+                    body = "body",
-            )),
+                    formatted = FormattedBody(MessageFormat.HTML, "formatted")
+                )
+            ),
             senderDisplayName = "Bob",
             eventId = AN_EVENT_ID,
         )
@@ -552,10 +590,12 @@ class TimelineItemContentMessageFactoryTest {
     fun `test create EmoteMessageType with HTML formatted body`() = runTest {
         val sut = createTimelineItemContentMessageFactory()
         val result = sut.create(
-            content = createMessageContent(type = EmoteMessageType(
+            content = createMessageContent(
-                body = "body",
+                type = EmoteMessageType(
-                formatted = FormattedBody(MessageFormat.HTML, "formatted")
+                    body = "body",
-            )),
+                    formatted = FormattedBody(MessageFormat.HTML, "formatted")
+                )
+            ),
             senderDisplayName = "Bob",
             eventId = AN_EVENT_ID,
         )

features/poll/impl/src/test/kotlin/io/element/android/features/poll/impl/history/PollHistoryPresenterTest.kt

@@ -52,14 +52,15 @@ class PollHistoryPresenterTest {
     @get:Rule
     val warmUpRule = WarmUpRule()
 
-    private val room = FakeMatrixRoom(
+    private val timeline = aPollTimeline(
-        matrixTimeline = aPollTimeline(
+        polls = mapOf(
-            polls = mapOf(
+            AN_EVENT_ID to anOngoingPollContent(),
-                AN_EVENT_ID to anOngoingPollContent(),
+            AN_EVENT_ID_2 to anEndedPollContent()
-                AN_EVENT_ID_2 to anEndedPollContent()
-            )
         )
     )
+    private val room = FakeMatrixRoom(
+        matrixTimeline = timeline
+    )
 
     @Test
     fun `present - initial states`() = runTest {
@@ -134,10 +135,14 @@ class PollHistoryPresenterTest {
             presenter.present()
         }.test {
             consumeItemsUntilPredicate {
-                it.pollHistoryItems.size == 2 && !it.isLoading
+                it.pollHistoryItems.size == 2
-            }.last().also { state ->
-                state.eventSink(PollHistoryEvents.LoadMore)
             }
+            timeline.updatePaginationState {
+                copy(isBackPaginating = false)
+            }
+            val loadedState = awaitItem()
+            assertThat(loadedState.isLoading).isFalse()
+            loadedState.eventSink(PollHistoryEvents.LoadMore)
             consumeItemsUntilPredicate {
                 it.isLoading
             }

features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfilePresenterTest.kt

@@ -37,6 +37,7 @@ import io.element.android.libraries.permissions.test.FakePermissionsPresenter
 import io.element.android.libraries.permissions.test.FakePermissionsPresenterFactory
 import io.element.android.tests.testutils.WarmUpRule
 import io.element.android.tests.testutils.consumeItemsUntilPredicate
+import io.element.android.tests.testutils.consumeItemsUntilTimeout
 import io.mockk.every
 import io.mockk.mockk
 import io.mockk.mockkStatic
@@ -292,11 +293,10 @@ class EditUserProfilePresenterTest {
             val initialState = awaitItem()
             initialState.eventSink(EditUserProfileEvents.UpdateDisplayName("   Name   "))
             initialState.eventSink(EditUserProfileEvents.Save)
-            consumeItemsUntilPredicate { matrixClient.setDisplayNameCalled && !matrixClient.removeAvatarCalled && !matrixClient.uploadAvatarCalled }
+            consumeItemsUntilTimeout()
             assertThat(matrixClient.setDisplayNameCalled).isFalse()
             assertThat(matrixClient.uploadAvatarCalled).isFalse()
             assertThat(matrixClient.removeAvatarCalled).isFalse()
-            cancelAndIgnoreRemainingEvents()
         }
     }
 

features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTests.kt

@@ -192,17 +192,22 @@ class RoomListPresenterTests {
             presenter.present()
         }.test {
             roomListService.postAllRooms(listOf(aRoomSummaryFilled()))
-            val loadedState = consumeItemsUntilPredicate { state -> state.roomList.size == 1 }.last()
+            skipItems(3)
+            val loadedState = awaitItem()
             // Test filtering with result
+            assertThat(loadedState.roomList.size).isEqualTo(1)
             loadedState.eventSink.invoke(RoomListEvents.UpdateFilter(A_ROOM_NAME.substring(0, 3)))
-            val withFilteredRoomState = consumeItemsUntilPredicate { state -> state.filteredRoomList.size == 1 }.last()
+            skipItems(1)
+            val withFilteredRoomState = awaitItem()
+            assertThat(withFilteredRoomState.filteredRoomList.size).isEqualTo(1)
             assertThat(withFilteredRoomState.filter).isEqualTo(A_ROOM_NAME.substring(0, 3))
             assertThat(withFilteredRoomState.filteredRoomList.size).isEqualTo(1)
             assertThat(withFilteredRoomState.filteredRoomList.first())
                 .isEqualTo(aRoomListRoomSummary)
             // Test filtering without result
             withFilteredRoomState.eventSink.invoke(RoomListEvents.UpdateFilter("tada"))
-            val withNotFilteredRoomState = consumeItemsUntilPredicate { state -> state.filteredRoomList.size == 0 }.last()
+            skipItems(1)
+            val withNotFilteredRoomState = awaitItem()
             assertThat(withNotFilteredRoomState.filter).isEqualTo("tada")
             assertThat(withNotFilteredRoomState.filteredRoomList).isEmpty()
             scope.cancel()

libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/ClickableLinkText.kt

@@ -142,7 +142,7 @@ fun ClickableLinkText(
 fun AnnotatedString.linkify(linkStyle: SpanStyle): AnnotatedString {
     val original = this
     val spannable = SpannableString(this.text)
-    LinkifyCompat.addLinks(spannable, Linkify.WEB_URLS or Linkify.PHONE_NUMBERS)
+    LinkifyCompat.addLinks(spannable, Linkify.WEB_URLS or Linkify.PHONE_NUMBERS or Linkify.EMAIL_ADDRESSES)
 
     val spans = spannable.getSpans(0, spannable.length, URLSpan::class.java)
     return buildAnnotatedString {

libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/roomlist/FakeRoomListService.kt

@@ -52,6 +52,10 @@ class FakeRoomListService : RoomListService {
         roomListStateFlow.emit(state)
     }
 
+    suspend fun postSyncIndicator(value: RoomListService.SyncIndicator) {
+        syncIndicatorStateFlow.emit(value)
+    }
+
     var latestSlidingSyncRange: IntRange? = null
         private set
 

tests/testutils/src/main/kotlin/io/element/android/tests/testutils/ReceiveTurbine.kt

@@ -19,7 +19,7 @@ package io.element.android.tests.testutils
 import app.cash.turbine.Event
 import app.cash.turbine.ReceiveTurbine
 import app.cash.turbine.withTurbineTimeout
-import io.element.android.libraries.core.data.tryOrNull
+import io.element.android.libraries.core.bool.orFalse
 import kotlin.time.Duration
 import kotlin.time.Duration.Companion.milliseconds
 
@@ -29,7 +29,7 @@ import kotlin.time.Duration.Companion.milliseconds
  * @return the list of consumed items.
  */
 suspend fun <T : Any> ReceiveTurbine<T>.consumeItemsUntilTimeout(timeout: Duration = 100.milliseconds): List<T> {
-    return consumeItemsUntilPredicate(timeout) { false }
+    return consumeItemsUntilPredicate(timeout, ignoreTimeoutError = true) { false }
 }
 
 /**
@@ -49,22 +49,29 @@ suspend fun <T : Any> ReceiveTurbine<T>.awaitLastSequentialItem(): T {
  */
 suspend fun <T : Any> ReceiveTurbine<T>.consumeItemsUntilPredicate(
     timeout: Duration = 100.milliseconds,
+    ignoreTimeoutError: Boolean = false,
     predicate: (T) -> Boolean,
 ): List<T> {
     val items = ArrayList<T>()
-    tryOrNull {
+    var exitLoop = false
-        var foundItemOrFinished = false
+    try {
-        while (!foundItemOrFinished) {
+        while (!exitLoop) {
             when (val event = withTurbineTimeout(timeout) { awaitEvent() }) {
                 is Event.Item<T> -> {
                     items.add(event.value)
-                    if (predicate(event.value)) {
+                    exitLoop = predicate(event.value)
-                        foundItemOrFinished = true
-                    }
                 }
-                Event.Complete, is Event.Error -> foundItemOrFinished = true
+                Event.Complete -> error("Unexpected complete")
+                is Event.Error -> throw event.throwable
             }
         }
+    } catch (assertionError: AssertionError) {
+        // TurbineAssertionError is internal :/, so rely on the message
+        if (assertionError.message?.startsWith("No value produced in").orFalse() && ignoreTimeoutError) {
+            // Timeout, ignore
+        } else {
+            throw assertionError
+        }
     }
     return items
 }