Disambiguate display name in notifications #2224

libraries/matrix/api/build.gradle.kts

@@ -46,4 +46,5 @@ dependencies {
     testImplementation(libs.test.truth)
     testImplementation(libs.test.robolectric)
     testImplementation(projects.tests.testutils)
+    testImplementation(projects.libraries.matrix.test)
 }

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationData.kt

@@ -27,7 +27,9 @@ data class NotificationData(
     val roomId: RoomId,
     // mxc url
     val senderAvatarUrl: String?,
-    val senderDisplayName: String?,
+    // private, must use `getSenderName`
+    private val senderDisplayName: String?,
+    private val senderIsNameAmbiguous: Boolean,
     val roomAvatarUrl: String?,
     val roomDisplayName: String?,
     val isDirect: Boolean,
@@ -36,7 +38,13 @@ data class NotificationData(
     val timestamp: Long,
     val content: NotificationContent,
     val hasMention: Boolean,
-)
+) {
+    fun getSenderName(userId: UserId): String = when {
+        senderDisplayName.isNullOrBlank() -> userId.value
+        senderIsNameAmbiguous -> "$senderDisplayName ($userId)"
+        else -> senderDisplayName
+    }
+}
 
 sealed interface NotificationContent {
     sealed interface MessageLike : NotificationContent {
@@ -54,11 +62,13 @@ sealed interface NotificationContent {
         data class ReactionContent(
             val relatedEventId: String
         ) : MessageLike
+
         data object RoomEncrypted : MessageLike
         data class RoomMessage(
             val senderId: UserId,
             val messageType: MessageType
         ) : MessageLike
+
         data object RoomRedaction : MessageLike
         data object Sticker : MessageLike
         data class Poll(
@@ -83,6 +93,7 @@ sealed interface NotificationContent {
             val userId: String,
             val membershipState: RoomMembershipState
         ) : StateEvent
+
         data object RoomName : StateEvent
         data object RoomPinnedEvents : StateEvent
         data object RoomPowerLevels : StateEvent

libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/notification/NotificationDataTest.kt

@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) 2023 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.notification
+
+import com.google.common.truth.Truth.assertThat
+import io.element.android.libraries.matrix.test.AN_EVENT_ID
+import io.element.android.libraries.matrix.test.A_ROOM_ID
+import io.element.android.libraries.matrix.test.A_USER_ID
+import org.junit.Test
+
+class NotificationDataTest {
+    @Test
+    fun `getSenderName should return user id if there is no sender name`() {
+        val sut = aNotificationData(
+            senderDisplayName = null,
+            senderIsNameAmbiguous = false,
+        )
+        assertThat(sut.getSenderName(A_USER_ID)).isEqualTo("@alice:server.org")
+    }
+
+    @Test
+    fun `getSenderName should return sender name if defined`() {
+        val sut = aNotificationData(
+            senderDisplayName = "Alice",
+            senderIsNameAmbiguous = false,
+        )
+        assertThat(sut.getSenderName(A_USER_ID)).isEqualTo("Alice")
+    }
+
+    @Test
+    fun `getSenderName should return sender name and user id in case of ambiguous display name`() {
+        val sut = aNotificationData(
+            senderDisplayName = "Alice",
+            senderIsNameAmbiguous = true,
+        )
+        assertThat(sut.getSenderName(A_USER_ID)).isEqualTo("Alice (@alice:server.org)")
+    }
+
+    private fun aNotificationData(
+        senderDisplayName: String?,
+        senderIsNameAmbiguous: Boolean,
+    ): NotificationData {
+        return NotificationData(
+            eventId = AN_EVENT_ID,
+            roomId = A_ROOM_ID,
+            senderAvatarUrl = null,
+            senderDisplayName = senderDisplayName,
+            senderIsNameAmbiguous = senderIsNameAmbiguous,
+            roomAvatarUrl = null,
+            roomDisplayName = null,
+            isDirect = false,
+            isEncrypted = false,
+            isNoisy = false,
+            timestamp = 0L,
+            content = NotificationContent.MessageLike.RoomEncrypted,
+            hasMention = false,
+        )
+    }
+}

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/NotificationMapper.kt

@@ -45,6 +45,7 @@ class NotificationMapper(
                 roomId = roomId,
                 senderAvatarUrl = item.senderInfo.avatarUrl,
                 senderDisplayName = item.senderInfo.displayName,
+                senderIsNameAmbiguous = item.senderInfo.isNameAmbiguous,
                 roomAvatarUrl = item.roomInfo.avatarUrl ?: item.senderInfo.avatarUrl.takeIf { item.roomInfo.isDirect },
                 roomDisplayName = item.roomInfo.displayName,
                 isDirect = item.roomInfo.isDirect,

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventResolver.kt

@@ -91,7 +91,8 @@ class NotifiableEventResolver @Inject constructor(
     ): NotifiableEvent? {
         return when (val content = this.content) {
             is NotificationContent.MessageLike.RoomMessage -> {
-                val messageBody = descriptionFromMessageContent(content, senderDisplayName ?: content.senderId.value)
+                val senderName = getSenderName(content.senderId)
+                val messageBody = descriptionFromMessageContent(content, senderName)
                 val notificationBody = if (hasMention) {
                     stringProvider.getString(R.string.notification_mentioned_you_body, messageBody)
                 } else {
@@ -104,7 +105,7 @@ class NotifiableEventResolver @Inject constructor(
                     eventId = eventId,
                     noisy = isNoisy,
                     timestamp = this.timestamp,
-                    senderName = senderDisplayName,
+                    senderName = senderName,
                     body = notificationBody,
                     imageUriString = fetchImageIfPresent(client)?.toString(),
                     roomName = roomDisplayName,
@@ -161,7 +162,7 @@ class NotifiableEventResolver @Inject constructor(
                     eventId = eventId,
                     noisy = isNoisy,
                     timestamp = this.timestamp,
-                    senderName = senderDisplayName,
+                    senderName = getSenderName(content.senderId),
                     body = stringProvider.getString(CommonStrings.common_poll_summary, content.question),
                     imageUriString = null,
                     roomName = roomDisplayName,

libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventResolverTest.kt

@@ -527,6 +527,7 @@ class NotifiableEventResolverTest {
             roomId = A_ROOM_ID,
             senderAvatarUrl = null,
             senderDisplayName = "Bob",
+            senderIsNameAmbiguous = false,
             roomAvatarUrl = null,
             roomDisplayName = null,
             isDirect = isDirect,