From bc9ea1c18ccb7816009a30406156909e5b7f174b Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 26 Sep 2024 15:27:33 +0200 Subject: [PATCH 1/3] Rework: create extension method for cleaner code. --- .../impl/DefaultRoomLastMessageFormatter.kt | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt index c7efd8c0ad..82bb110455 100644 --- a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt +++ b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt @@ -61,7 +61,7 @@ class DefaultRoomLastMessageFormatter @Inject constructor( val isOutgoing = event.isOwn val senderDisambiguatedDisplayName = event.senderProfile.getDisambiguatedDisplayName(event.sender) return when (val content = event.content) { - is MessageContent -> processMessageContents(content, senderDisambiguatedDisplayName, isDmRoom) + is MessageContent -> content.process(senderDisambiguatedDisplayName, isDmRoom) RedactedContent -> { val message = sp.getString(CommonStrings.common_message_removed) if (!isDmRoom) { @@ -71,7 +71,8 @@ class DefaultRoomLastMessageFormatter @Inject constructor( } } is StickerContent -> { - prefixIfNeeded(sp.getString(CommonStrings.common_sticker) + " (" + content.body + ")", senderDisambiguatedDisplayName, isDmRoom) + val message = sp.getString(CommonStrings.common_sticker) + " (" + content.body + ")" + message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom) } is UnableToDecryptContent -> { val message = sp.getString(CommonStrings.common_waiting_for_decryption_key) @@ -92,22 +93,22 @@ class DefaultRoomLastMessageFormatter @Inject constructor( } is PollContent -> { val message = sp.getString(CommonStrings.common_poll_summary, content.question) - prefixIfNeeded(message, senderDisambiguatedDisplayName, isDmRoom) + message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom) } is FailedToParseMessageLikeContent, is FailedToParseStateContent, is UnknownContent -> { - prefixIfNeeded(sp.getString(CommonStrings.common_unsupported_event), senderDisambiguatedDisplayName, isDmRoom) + val message = sp.getString(CommonStrings.common_unsupported_event) + message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom) } is LegacyCallInviteContent -> sp.getString(CommonStrings.common_call_invite) is CallNotifyContent -> sp.getString(CommonStrings.common_call_started) }?.take(MAX_SAFE_LENGTH) } - private fun processMessageContents( - messageContent: MessageContent, + private fun MessageContent.process( senderDisambiguatedDisplayName: String, isDmRoom: Boolean, ): CharSequence { - val internalMessage = when (val messageType: MessageType = messageContent.type) { + val message = when (val messageType: MessageType = type) { // Doesn't need a prefix is EmoteMessageType -> { return "* $senderDisambiguatedDisplayName ${messageType.body}" @@ -143,16 +144,15 @@ class DefaultRoomLastMessageFormatter @Inject constructor( messageType.body } } - return prefixIfNeeded(internalMessage, senderDisambiguatedDisplayName, isDmRoom) + return message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom) } - private fun prefixIfNeeded( - message: String, + private fun String.prefixIfNeeded( senderDisambiguatedDisplayName: String, isDmRoom: Boolean, ): CharSequence = if (isDmRoom) { - message + this } else { - message.prefixWith(senderDisambiguatedDisplayName) + prefixWith(senderDisambiguatedDisplayName) } } From 2468592ec0980625d924acbefbe3fe80d5c6b946 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 26 Sep 2024 15:31:42 +0200 Subject: [PATCH 2/3] Use prefixIfNeeded (equivalent code) --- .../impl/DefaultRoomLastMessageFormatter.kt | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt index 82bb110455..8e69e8e0e6 100644 --- a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt +++ b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt @@ -64,11 +64,7 @@ class DefaultRoomLastMessageFormatter @Inject constructor( is MessageContent -> content.process(senderDisambiguatedDisplayName, isDmRoom) RedactedContent -> { val message = sp.getString(CommonStrings.common_message_removed) - if (!isDmRoom) { - message.prefixWith(senderDisambiguatedDisplayName) - } else { - message - } + message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom) } is StickerContent -> { val message = sp.getString(CommonStrings.common_sticker) + " (" + content.body + ")" @@ -76,11 +72,7 @@ class DefaultRoomLastMessageFormatter @Inject constructor( } is UnableToDecryptContent -> { val message = sp.getString(CommonStrings.common_waiting_for_decryption_key) - if (!isDmRoom) { - message.prefixWith(senderDisambiguatedDisplayName) - } else { - message - } + message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom) } is RoomMembershipContent -> { roomMembershipContentFormatter.format(content, senderDisambiguatedDisplayName, isOutgoing) From 8118ae7ae10ec8179432a88c99d70f9939eaa9c8 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 26 Sep 2024 15:42:43 +0200 Subject: [PATCH 3/3] Prefix with `You` instead of display name #3470 --- .../impl/DefaultRoomLastMessageFormatter.kt | 24 ++++++---- .../DefaultRoomLastMessageFormatterTest.kt | 45 ++++++++++++++----- .../src/main/res/values/localazy.xml | 1 + 3 files changed, 50 insertions(+), 20 deletions(-) diff --git a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt index 8e69e8e0e6..200b93343d 100644 --- a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt +++ b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt @@ -61,18 +61,18 @@ class DefaultRoomLastMessageFormatter @Inject constructor( val isOutgoing = event.isOwn val senderDisambiguatedDisplayName = event.senderProfile.getDisambiguatedDisplayName(event.sender) return when (val content = event.content) { - is MessageContent -> content.process(senderDisambiguatedDisplayName, isDmRoom) + is MessageContent -> content.process(senderDisambiguatedDisplayName, isDmRoom, isOutgoing) RedactedContent -> { val message = sp.getString(CommonStrings.common_message_removed) - message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom) + message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom, isOutgoing) } is StickerContent -> { val message = sp.getString(CommonStrings.common_sticker) + " (" + content.body + ")" - message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom) + message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom, isOutgoing) } is UnableToDecryptContent -> { val message = sp.getString(CommonStrings.common_waiting_for_decryption_key) - message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom) + message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom, isOutgoing) } is RoomMembershipContent -> { roomMembershipContentFormatter.format(content, senderDisambiguatedDisplayName, isOutgoing) @@ -85,11 +85,11 @@ class DefaultRoomLastMessageFormatter @Inject constructor( } is PollContent -> { val message = sp.getString(CommonStrings.common_poll_summary, content.question) - message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom) + message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom, isOutgoing) } is FailedToParseMessageLikeContent, is FailedToParseStateContent, is UnknownContent -> { val message = sp.getString(CommonStrings.common_unsupported_event) - message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom) + message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom, isOutgoing) } is LegacyCallInviteContent -> sp.getString(CommonStrings.common_call_invite) is CallNotifyContent -> sp.getString(CommonStrings.common_call_started) @@ -99,6 +99,7 @@ class DefaultRoomLastMessageFormatter @Inject constructor( private fun MessageContent.process( senderDisambiguatedDisplayName: String, isDmRoom: Boolean, + isOutgoing: Boolean ): CharSequence { val message = when (val messageType: MessageType = type) { // Doesn't need a prefix @@ -136,15 +137,22 @@ class DefaultRoomLastMessageFormatter @Inject constructor( messageType.body } } - return message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom) + return message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom, isOutgoing) } private fun String.prefixIfNeeded( senderDisambiguatedDisplayName: String, isDmRoom: Boolean, + isOutgoing: Boolean, ): CharSequence = if (isDmRoom) { this } else { - prefixWith(senderDisambiguatedDisplayName) + prefixWith( + if (isOutgoing) { + sp.getString(CommonStrings.common_you) + } else { + senderDisambiguatedDisplayName + } + ) } } diff --git a/libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatterTest.kt b/libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatterTest.kt index 36fa8c0fd0..db3377c5e8 100644 --- a/libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatterTest.kt +++ b/libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatterTest.kt @@ -148,7 +148,29 @@ class DefaultRoomLastMessageFormatterTest { @Test @Config(qualifiers = "en") - fun `Message contents`() { + fun `Message contents sent by other user`() { + testMessageContents( + sentByYou = false, + senderName = "Alice", + expectedPrefix = "Alice", + ) + } + + @Test + @Config(qualifiers = "en") + fun `Message contents sent by current user`() { + testMessageContents( + sentByYou = true, + senderName = "Bob", + expectedPrefix = "You", + ) + } + + private fun testMessageContents( + sentByYou: Boolean, + senderName: String, + expectedPrefix: String, + ) { val body = "Shared body" fun createMessageContent(type: MessageType): MessageContent { return MessageContent(body, null, false, false, type) @@ -167,7 +189,6 @@ class DefaultRoomLastMessageFormatterTest { EmoteMessageType(body, null), OtherMessageType(msgType = "a_type", body = body), ) - val senderName = "Someone" val resultsInRoom = mutableListOf>() val resultsInDm = mutableListOf>() @@ -175,7 +196,7 @@ class DefaultRoomLastMessageFormatterTest { sequenceOf(false, true).forEach { isDm -> sharedContentMessagesTypes.forEach { type -> val content = createMessageContent(type) - val message = createRoomEvent(sentByYou = false, senderDisplayName = "Someone", content = content) + val message = createRoomEvent(sentByYou = sentByYou, senderDisplayName = senderName, content = content) val result = formatter.format(message, isDmRoom = isDm) if (isDm) { resultsInDm.add(type to result) @@ -207,16 +228,16 @@ class DefaultRoomLastMessageFormatterTest { for ((type, result) in resultsInRoom) { val string = result.toString() val expectedResult = when (type) { - is VideoMessageType -> "$senderName: Video" - is AudioMessageType -> "$senderName: Audio" - is VoiceMessageType -> "$senderName: Voice message" - is ImageMessageType -> "$senderName: Image" - is StickerMessageType -> "$senderName: Sticker" - is FileMessageType -> "$senderName: File" - is LocationMessageType -> "$senderName: Shared location" + is VideoMessageType -> "$expectedPrefix: Video" + is AudioMessageType -> "$expectedPrefix: Audio" + is VoiceMessageType -> "$expectedPrefix: Voice message" + is ImageMessageType -> "$expectedPrefix: Image" + is StickerMessageType -> "$expectedPrefix: Sticker" + is FileMessageType -> "$expectedPrefix: File" + is LocationMessageType -> "$expectedPrefix: Shared location" is TextMessageType, is NoticeMessageType, - is OtherMessageType -> "$senderName: $body" + is OtherMessageType -> "$expectedPrefix: $body" is EmoteMessageType -> "* $senderName ${type.body}" } val shouldCreateAnnotatedString = when (type) { @@ -821,7 +842,7 @@ class DefaultRoomLastMessageFormatterTest { val pollContent = aPollContent() val mineContentEvent = createRoomEvent(sentByYou = true, senderDisplayName = "Alice", content = pollContent) - assertThat(formatter.format(mineContentEvent, false).toString()).isEqualTo("Alice: Poll: Do you like polls?") + assertThat(formatter.format(mineContentEvent, false).toString()).isEqualTo("You: Poll: Do you like polls?") val contentEvent = createRoomEvent(sentByYou = false, senderDisplayName = "Bob", content = pollContent) assertThat(formatter.format(contentEvent, false).toString()).isEqualTo("Bob: Poll: Do you like polls?") diff --git a/libraries/ui-strings/src/main/res/values/localazy.xml b/libraries/ui-strings/src/main/res/values/localazy.xml index 4ef598a33b..11e1ab922a 100644 --- a/libraries/ui-strings/src/main/res/values/localazy.xml +++ b/libraries/ui-strings/src/main/res/values/localazy.xml @@ -248,6 +248,7 @@ Reason: %1$s." "Voice message" "Waiting…" "Waiting for this message" + "You" "Confirmation" "Error" "Success"