Merge branch 'develop' into feature/fga/fix_2692

.github/workflows/quality.yml

@@ -10,7 +10,7 @@ on:
 # Enrich gradle.properties for CI/CD
 env:
   GRADLE_OPTS: -Dorg.gradle.jvmargs="-Xmx6g -Dfile.encoding=UTF-8 -XX:+HeapDumpOnOutOfMemoryError" -Dkotlin.incremental=false -XX:+UseParallelGC
-  CI_GRADLE_ARG_PROPERTIES: --stacktrace -PpreDexEnable=false --max-workers 8 --no-daemon --warn
+  CI_GRADLE_ARG_PROPERTIES: --stacktrace -PpreDexEnable=false --max-workers 8  --no-daemon
 
 jobs:
   checkScript:
@@ -33,12 +33,13 @@ jobs:
       - name: Search for invalid screenshot files
         run: ./tools/test/checkInvalidScreenshots.py
 
-  check:
-    name: Project Check Suite
+  # Code checks
+  konsist:
+    name: Konsist tests
     runs-on: ubuntu-latest
     # Allow all jobs on main and develop. Just one per PR.
     concurrency:
-      group: ${{ github.ref == 'refs/heads/main' && format('check-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('check-develop-{0}', github.sha) || format('check-{0}', github.ref) }}
+      group: ${{ github.ref == 'refs/heads/main' && format('check-konsist-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('check-konsist-develop-{0}', github.sha) || format('check-konsist-{0}', github.ref) }}
       cancel-in-progress: true
     steps:
       - uses: actions/checkout@v4
@@ -55,8 +56,40 @@ jobs:
         uses: gradle/actions/setup-gradle@v3
         with:
           cache-read-only: ${{ github.ref != 'refs/heads/develop' }}
-      - name: Run code quality check suite
-        run: ./gradlew runQualityChecks $CI_GRADLE_ARG_PROPERTIES
+      - name: Run Konsist tests
+        run: ./gradlew :tests:konsist:testDebugUnitTest $CI_GRADLE_ARG_PROPERTIES --no-daemon
+      - name: Upload reports
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: konsist-report
+          path: |
+            **/build/reports/**/*.*
+
+  lint:
+    name: Android lint check
+    runs-on: ubuntu-latest
+    # Allow all jobs on main and develop. Just one per PR.
+    concurrency:
+      group: ${{ github.ref == 'refs/heads/main' && format('check-lint-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('check-lint-develop-{0}', github.sha) || format('check-lint-{0}', github.ref) }}
+      cancel-in-progress: true
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          # Ensure we are building the branch and not the branch after being merged on develop
+          # https://github.com/actions/checkout/issues/881
+          ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.ref }}
+      - name: Use JDK 17
+        uses: actions/setup-java@v4
+        with:
+          distribution: 'temurin' # See 'Supported distributions' for available options
+          java-version: '17'
+      - name: Configure gradle
+        uses: gradle/actions/setup-gradle@v3
+        with:
+          cache-read-only: ${{ github.ref != 'refs/heads/develop' }}
+      - name: Run lint
+        run: ./gradlew :app:lintGplayDebug :app:lintFdroidDebug $CI_GRADLE_ARG_PROPERTIES
       - name: Upload reports
         if: always()
         uses: actions/upload-artifact@v4
@@ -64,6 +97,108 @@ jobs:
           name: linting-report
           path: |
             **/build/reports/**/*.*
+
+  detekt:
+    name: Detekt checks
+    runs-on: ubuntu-latest
+    # Allow all jobs on main and develop. Just one per PR.
+    concurrency:
+      group: ${{ github.ref == 'refs/heads/main' && format('check-detekt-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('check-detekt-develop-{0}', github.sha) || format('check-detekt-{0}', github.ref) }}
+      cancel-in-progress: true
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          # Ensure we are building the branch and not the branch after being merged on develop
+          # https://github.com/actions/checkout/issues/881
+          ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.ref }}
+      - name: Use JDK 17
+        uses: actions/setup-java@v4
+        with:
+          distribution: 'temurin' # See 'Supported distributions' for available options
+          java-version: '17'
+      - name: Configure gradle
+        uses: gradle/actions/setup-gradle@v3
+        with:
+          cache-read-only: ${{ github.ref != 'refs/heads/develop' }}
+      - name: Run Detekt
+        run: ./gradlew detekt $CI_GRADLE_ARG_PROPERTIES --no-daemon
+      - name: Upload reports
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: detekt-report
+          path: |
+            **/build/reports/**/*.*
+
+  ktlint:
+    name: Ktlint checks
+    runs-on: ubuntu-latest
+    # Allow all jobs on main and develop. Just one per PR.
+    concurrency:
+      group: ${{ github.ref == 'refs/heads/main' && format('check-ktlint-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('check-ktlint-develop-{0}', github.sha) || format('check-ktlint-{0}', github.ref) }}
+      cancel-in-progress: true
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          # Ensure we are building the branch and not the branch after being merged on develop
+          # https://github.com/actions/checkout/issues/881
+          ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.ref }}
+      - name: Use JDK 17
+        uses: actions/setup-java@v4
+        with:
+          distribution: 'temurin' # See 'Supported distributions' for available options
+          java-version: '17'
+      - name: Configure gradle
+        uses: gradle/actions/setup-gradle@v3
+        with:
+          cache-read-only: ${{ github.ref != 'refs/heads/develop' }}
+      - name: Run Ktlint check
+        run: ./gradlew ktlintCheck $CI_GRADLE_ARG_PROPERTIES
+      - name: Upload reports
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: ktlint-report
+          path: |
+            **/build/reports/**/*.*
+
+  knit:
+    name: Knit checks
+    runs-on: ubuntu-latest
+    # Allow all jobs on main and develop. Just one per PR.
+    concurrency:
+      group: ${{ github.ref == 'refs/heads/main' && format('check-knit-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('check-knit-develop-{0}', github.sha) || format('check-knit-{0}', github.ref) }}
+      cancel-in-progress: true
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          # Ensure we are building the branch and not the branch after being merged on develop
+          # https://github.com/actions/checkout/issues/881
+          ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.ref }}
+      - name: Use JDK 17
+        uses: actions/setup-java@v4
+        with:
+          distribution: 'temurin' # See 'Supported distributions' for available options
+          java-version: '17'
+      - name: Configure gradle
+        uses: gradle/actions/setup-gradle@v3
+        with:
+          cache-read-only: ${{ github.ref != 'refs/heads/develop' }}
+      - name: Run Knit
+        run: ./gradlew knitCheck $CI_GRADLE_ARG_PROPERTIES
+
+  upload_reports:
+    name: Project Check Suite
+    runs-on: ubuntu-latest
+    needs: [konsist, lint, ktlint, detekt]
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          # Ensure we are building the branch and not the branch after being merged on develop
+          # https://github.com/actions/checkout/issues/881
+          ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.ref }}
+      - name: Download reports from previous jobs
+        uses: actions/download-artifact@v4
       - name: Prepare Danger
         if: always()
         run: |

changelog.d/2291.bugfix

@@ -0,0 +1 @@
+Make sure explicit links in messages take priority over links found by linkification (urls, emails, phone numbers, etc.)

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

@@ -268,12 +268,16 @@ class TimelineItemContentMessageFactory @Inject constructor(
         }
         // Find and set as URLSpans any links present in the text
         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
+        // Restore old spans, remove new ones if there is a conflict
         for ((urlSpan, location) in oldURLSpans) {
             val (start, end) = location
-            if (getSpans<URLSpan>(start, end).isEmpty()) {
-                setSpan(urlSpan, start, end, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE)
+            val addedSpans = getSpans<URLSpan>(start, end).orEmpty()
+            if (addedSpans.isNotEmpty()) {
+                for (span in addedSpans) {
+                    removeSpan(span)
+                }
             }
+            setSpan(urlSpan, start, end, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE)
         }
         return this
     }

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

@@ -16,7 +16,9 @@
 
 package io.element.android.features.messages.impl.timeline.factories.event
 
+import android.net.Uri
 import android.text.SpannableString
+import android.text.SpannableStringBuilder
 import android.text.Spanned
 import android.text.style.URLSpan
 import androidx.core.text.buildSpannedString
@@ -46,6 +48,7 @@ import io.element.android.libraries.matrix.api.media.ImageInfo
 import io.element.android.libraries.matrix.api.media.MediaSource
 import io.element.android.libraries.matrix.api.media.ThumbnailInfo
 import io.element.android.libraries.matrix.api.media.VideoInfo
+import io.element.android.libraries.matrix.api.permalink.PermalinkData
 import io.element.android.libraries.matrix.api.timeline.item.event.AudioMessageType
 import io.element.android.libraries.matrix.api.timeline.item.event.EmoteMessageType
 import io.element.android.libraries.matrix.api.timeline.item.event.FileMessageType
@@ -75,6 +78,7 @@ import org.robolectric.RobolectricTestRunner
 import kotlin.time.Duration
 import kotlin.time.Duration.Companion.minutes
 
+@Suppress("LargeClass")
 @RunWith(RobolectricTestRunner::class)
 class TimelineItemContentMessageFactoryTest {
     @Test
@@ -641,6 +645,31 @@ class TimelineItemContentMessageFactoryTest {
         assertThat((result as TimelineItemEmoteContent).formattedBody).isEqualTo(SpannableString("* Bob formatted"))
     }
 
+    @Test
+    fun `a message with existing URLSpans keeps it after linkification`() = runTest {
+        val expectedSpanned = SpannableStringBuilder().apply {
+            append("Test ")
+            inSpans(URLSpan("https://www.example.org")) {
+                append("me@matrix.org")
+            }
+        }
+        val sut = createTimelineItemContentMessageFactory(
+            htmlConverterTransform = { expectedSpanned },
+            permalinkParser = FakePermalinkParser { PermalinkData.FallbackLink(Uri.EMPTY) }
+        )
+        val result = sut.create(
+            content = createMessageContent(
+                type = TextMessageType(
+                    body = "Test [me@matrix.org](https://www.example.org)",
+                    formatted = FormattedBody(MessageFormat.HTML, "Test <a href=\"https://www.example.org\">me@matrix.org</a>")
+                )
+            ),
+            senderDisambiguatedDisplayName = "Bob",
+            eventId = AN_EVENT_ID,
+        )
+        assertThat((result as TimelineItemTextContent).formattedBody).isEqualTo(expectedSpanned)
+    }
+
     private fun createMessageContent(
         body: String = "Body",
         inReplyTo: InReplyTo? = null,
@@ -660,12 +689,13 @@ class TimelineItemContentMessageFactoryTest {
     private fun createTimelineItemContentMessageFactory(
         featureFlagService: FeatureFlagService = FakeFeatureFlagService(),
         htmlConverterTransform: (String) -> CharSequence = { it },
+        permalinkParser: FakePermalinkParser = FakePermalinkParser(),
     ) = TimelineItemContentMessageFactory(
         fileSizeFormatter = FakeFileSizeFormatter(),
         fileExtensionExtractor = FileExtensionExtractorWithoutValidation(),
         featureFlagService = featureFlagService,
         htmlConverterProvider = FakeHtmlConverterProvider(htmlConverterTransform),
-        permalinkParser = FakePermalinkParser(),
+        permalinkParser = permalinkParser,
     )
 
     private fun createStickerContent(

gradle/libs.versions.toml

@@ -136,7 +136,7 @@ test_arch_core = "androidx.arch.core:core-testing:2.2.0"
 test_junit = "junit:junit:4.13.2"
 test_runner = "androidx.test:runner:1.5.2"
 test_junitext = "androidx.test.ext:junit:1.1.5"
-test_mockk = "io.mockk:mockk:1.13.10"
+test_mockk = "io.mockk:mockk:1.13.11"
 test_konsist = "com.lemonappdev:konsist:0.13.0"
 test_turbine = "app.cash.turbine:turbine:1.1.0"
 test_truth = "com.google.truth:truth:1.4.2"

plugins/src/main/kotlin/extension/KoverExtension.kt

@@ -122,7 +122,6 @@ fun Project.setupKover() {
 
             total {
                 verify {
-                    onCheck = true
                     // General rule: minimum code coverage.
                     rule("Global minimum code coverage.") {
                         groupBy = GroupingEntityType.APPLICATION
@@ -140,7 +139,6 @@ fun Project.setupKover() {
             }
             variant(KoverVariant.Presenters.variantName) {
                 verify {
-                    onCheck = true
                     // Rule to ensure that coverage of Presenters is sufficient.
                     rule("Check code coverage of presenters") {
                         groupBy = GroupingEntityType.CLASS
@@ -171,7 +169,6 @@ fun Project.setupKover() {
             }
             variant(KoverVariant.States.variantName) {
                 verify {
-                    onCheck = true
                     // Rule to ensure that coverage of States is sufficient.
                     rule("Check code coverage of states") {
                         groupBy = GroupingEntityType.CLASS
@@ -214,7 +211,6 @@ fun Project.setupKover() {
             }
             variant(KoverVariant.Views.variantName) {
                 verify {
-                    onCheck = true
                     // Rule to ensure that coverage of Views is sufficient (deactivated for now).
                     rule("Check code coverage of views") {
                         groupBy = GroupingEntityType.CLASS