From 6bf1806ed4f4853a52312e57dfcb6c82bbf029e5 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 25 Jan 2024 18:33:48 +0100 Subject: [PATCH] Disambiguate display name in notifications #2224 --- libraries/matrix/api/build.gradle.kts | 1 + .../api/notification/NotificationData.kt | 15 +++- .../api/notification/NotificationDataTest.kt | 73 +++++++++++++++++++ .../impl/notification/NotificationMapper.kt | 1 + .../notifications/NotifiableEventResolver.kt | 7 +- .../NotifiableEventResolverTest.kt | 1 + 6 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/notification/NotificationDataTest.kt diff --git a/libraries/matrix/api/build.gradle.kts b/libraries/matrix/api/build.gradle.kts index f8a600df1e..b9467af169 100644 --- a/libraries/matrix/api/build.gradle.kts +++ b/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) } diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationData.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationData.kt index 8275b8ee5f..3fff2dd468 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationData.kt +++ b/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 diff --git a/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/notification/NotificationDataTest.kt b/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/notification/NotificationDataTest.kt new file mode 100644 index 0000000000..77a4188a68 --- /dev/null +++ b/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, + ) + } +} diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/NotificationMapper.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/NotificationMapper.kt index 6285ba393f..4b7c08f9b4 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/NotificationMapper.kt +++ b/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, diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventResolver.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventResolver.kt index ebf8ac6a89..31e71dc6c9 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventResolver.kt +++ b/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, diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventResolverTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventResolverTest.kt index d824a193d2..7b31afc01e 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventResolverTest.kt +++ b/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,