Merge pull request #2221 from element-hq/feature/bma/displayNameAmbiguous

Display name disambiguation

changelog.d/2215.misc

@@ -0,0 +1 @@
+Disambiguate display name in the timeline.

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

@@ -24,13 +24,13 @@ import io.element.android.libraries.matrix.api.timeline.item.event.FailedToParse
 import io.element.android.libraries.matrix.api.timeline.item.event.MessageContent
 import io.element.android.libraries.matrix.api.timeline.item.event.PollContent
 import io.element.android.libraries.matrix.api.timeline.item.event.ProfileChangeContent
-import io.element.android.libraries.matrix.api.timeline.item.event.ProfileTimelineDetails
 import io.element.android.libraries.matrix.api.timeline.item.event.RedactedContent
 import io.element.android.libraries.matrix.api.timeline.item.event.RoomMembershipContent
 import io.element.android.libraries.matrix.api.timeline.item.event.StateContent
 import io.element.android.libraries.matrix.api.timeline.item.event.StickerContent
 import io.element.android.libraries.matrix.api.timeline.item.event.UnableToDecryptContent
 import io.element.android.libraries.matrix.api.timeline.item.event.UnknownContent
+import io.element.android.libraries.matrix.api.timeline.item.event.getDisambiguatedDisplayName
 import javax.inject.Inject
 
 class TimelineItemContentFactory @Inject constructor(
@@ -50,7 +50,7 @@ class TimelineItemContentFactory @Inject constructor(
             is FailedToParseMessageLikeContent -> failedToParseMessageFactory.create(itemContent)
             is FailedToParseStateContent -> failedToParseStateFactory.create(itemContent)
             is MessageContent -> {
-                val senderDisplayName = (eventTimelineItem.senderProfile as? ProfileTimelineDetails.Ready)?.displayName ?: eventTimelineItem.sender.value
+                val senderDisplayName = eventTimelineItem.senderProfile.getDisambiguatedDisplayName(eventTimelineItem.sender)
                 messageFactory.create(itemContent, senderDisplayName, eventTimelineItem.eventId)
             }
             is ProfileChangeContent -> profileChangeFactory.create(eventTimelineItem)

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

@@ -33,6 +33,7 @@ import io.element.android.libraries.matrix.api.MatrixClient
 import io.element.android.libraries.matrix.api.room.RoomMember
 import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
 import io.element.android.libraries.matrix.api.timeline.item.event.ProfileTimelineDetails
+import io.element.android.libraries.matrix.api.timeline.item.event.getDisambiguatedDisplayName
 import kotlinx.collections.immutable.toImmutableList
 import java.text.DateFormat
 import java.util.Date
@@ -63,7 +64,7 @@ class TimelineItemEventFactory @Inject constructor(
                 senderAvatarUrl = null
             }
             is ProfileTimelineDetails.Ready -> {
-                senderDisplayName = senderProfile.displayName
+                senderDisplayName = senderProfile.getDisambiguatedDisplayName(currentSender)
                 senderAvatarUrl = senderProfile.avatarUrl
             }
         }

libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt

@@ -69,6 +69,8 @@ class DefaultRoomLastMessageFormatter @Inject constructor(
 
     override fun format(event: EventTimelineItem, isDmRoom: Boolean): CharSequence? {
         val isOutgoing = event.isOwn
+        // Note: we do not use disambiguated display name here, see
+        // https://github.com/element-hq/element-x-ios/issues/1845#issuecomment-1888707428
         val senderDisplayName = (event.senderProfile as? ProfileTimelineDetails.Ready)?.displayName ?: event.sender.value
         return when (val content = event.content) {
             is MessageContent -> processMessageContents(content, senderDisplayName, isDmRoom)

libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultTimelineEventFormatter.kt

@@ -27,13 +27,13 @@ import io.element.android.libraries.matrix.api.timeline.item.event.FailedToParse
 import io.element.android.libraries.matrix.api.timeline.item.event.MessageContent
 import io.element.android.libraries.matrix.api.timeline.item.event.PollContent
 import io.element.android.libraries.matrix.api.timeline.item.event.ProfileChangeContent
-import io.element.android.libraries.matrix.api.timeline.item.event.ProfileTimelineDetails
 import io.element.android.libraries.matrix.api.timeline.item.event.RedactedContent
 import io.element.android.libraries.matrix.api.timeline.item.event.RoomMembershipContent
 import io.element.android.libraries.matrix.api.timeline.item.event.StateContent
 import io.element.android.libraries.matrix.api.timeline.item.event.StickerContent
 import io.element.android.libraries.matrix.api.timeline.item.event.UnableToDecryptContent
 import io.element.android.libraries.matrix.api.timeline.item.event.UnknownContent
+import io.element.android.libraries.matrix.api.timeline.item.event.getDisambiguatedDisplayName
 import io.element.android.libraries.ui.strings.CommonStrings
 import io.element.android.services.toolbox.api.strings.StringProvider
 import javax.inject.Inject
@@ -48,7 +48,7 @@ class DefaultTimelineEventFormatter @Inject constructor(
 ) : TimelineEventFormatter {
     override fun format(event: EventTimelineItem): CharSequence? {
         val isOutgoing = event.isOwn
-        val senderDisplayName = (event.senderProfile as? ProfileTimelineDetails.Ready)?.displayName ?: event.sender.value
+        val senderDisplayName = event.senderProfile.getDisambiguatedDisplayName(event.sender)
         return when (val content = event.content) {
             is RoomMembershipContent -> {
                 roomMembershipContentFormatter.format(content, senderDisplayName, isOutgoing)

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/ProfileTimelineDetails.kt

@@ -17,6 +17,7 @@
 package io.element.android.libraries.matrix.api.timeline.item.event
 
 import androidx.compose.runtime.Immutable
+import io.element.android.libraries.matrix.api.core.UserId
 
 @Immutable
 sealed interface ProfileTimelineDetails {
@@ -34,3 +35,20 @@ sealed interface ProfileTimelineDetails {
         val message: String
     ) : ProfileTimelineDetails
 }
+
+/**
+ * Returns a disambiguated display name for the user.
+ * If the display name is null, or profile is not Ready, the user ID is returned.
+ * If the display name is ambiguous, the user ID is appended in parentheses.
+ * Otherwise, the display name is returned.
+ */
+fun ProfileTimelineDetails.getDisambiguatedDisplayName(userId: UserId): String {
+    return when (this) {
+        is ProfileTimelineDetails.Ready -> when {
+            displayName == null -> userId.value
+            displayNameAmbiguous -> "$displayName ($userId)"
+            else -> displayName
+        }
+        else -> userId.value
+    }
+}

libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/ProfileTimelineDetailsTest.kt

@@ -0,0 +1,74 @@
+/*
+ * Copyright (c) 2024 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.element.android.libraries.matrix.api.timeline.item.event
+
+import com.google.common.truth.Truth.assertThat
+import io.element.android.libraries.matrix.api.core.UserId
+import org.junit.Test
+
+private const val A_USER_ID = "@foo:example.org"
+private val aUserId = UserId(A_USER_ID)
+
+class ProfileTimelineDetailsTest {
+    @Test
+    fun `getDisambiguatedDisplayName of Unavailable should be equal to userId`() {
+        assertThat(ProfileTimelineDetails.Unavailable.getDisambiguatedDisplayName(aUserId)).isEqualTo(A_USER_ID)
+    }
+
+    @Test
+    fun `getDisambiguatedDisplayName of Error should be equal to userId`() {
+        assertThat(ProfileTimelineDetails.Error("An error").getDisambiguatedDisplayName(aUserId)).isEqualTo(A_USER_ID)
+    }
+
+    @Test
+    fun `getDisambiguatedDisplayName of Pending should be equal to userId`() {
+        assertThat(ProfileTimelineDetails.Pending.getDisambiguatedDisplayName(aUserId)).isEqualTo(A_USER_ID)
+    }
+
+    @Test
+    fun `getDisambiguatedDisplayName of Ready without display name should be equal to userId`() {
+        assertThat(
+            ProfileTimelineDetails.Ready(
+                displayName = null,
+                displayNameAmbiguous = false,
+                avatarUrl = null,
+            ).getDisambiguatedDisplayName(aUserId)
+        ).isEqualTo(A_USER_ID)
+    }
+
+    @Test
+    fun `getDisambiguatedDisplayName of Ready with display name should be equal to display name`() {
+        assertThat(
+            ProfileTimelineDetails.Ready(
+                displayName = "Alice",
+                displayNameAmbiguous = false,
+                avatarUrl = null,
+            ).getDisambiguatedDisplayName(aUserId)
+        ).isEqualTo("Alice")
+    }
+
+    @Test
+    fun `getDisambiguatedDisplayName of Ready with display name and ambiguous should be equal to display name with user id`() {
+        assertThat(
+            ProfileTimelineDetails.Ready(
+                displayName = "Alice",
+                displayNameAmbiguous = true,
+                avatarUrl = null,
+            ).getDisambiguatedDisplayName(aUserId)
+        ).isEqualTo("Alice ($A_USER_ID)")
+    }
+}