Merge pull request #648 from vector-im/feature/fix_#498

Feature/fix #498

CHANGES.md

@@ -11,6 +11,7 @@ Other changes:
  -
 
 Bugfix 🐛:
+ - Fix issues with some member events rendering (#498)
  - Passphrase does not match (Export room keys) (#644)
 
 Translations 🗣:

matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/timeline/TimelineEvent.kt

@@ -62,16 +62,12 @@ data class TimelineEvent(
     }
 
     fun getDisambiguatedDisplayName(): String {
-        return if (isUniqueDisplayName) {
+        return when {
-            senderName
+            senderName.isNullOrBlank() -> root.senderId ?: ""
-        } else {
+            isUniqueDisplayName        -> senderName
-            senderName?.let { name ->
+            else                       -> "$senderName (${root.senderId})"
-                "$name (${root.senderId})"
         }
     }
-               ?: root.senderId
-               ?: ""
-    }
 
     /**
      * Get the metadata associated with a key.
@@ -113,7 +109,8 @@ fun TimelineEvent.getLastMessageBody(): String? {
     val lastMessageContent = getLastMessageContent()
 
     if (lastMessageContent != null) {
-        return lastMessageContent.newContent?.toModel<MessageContent>()?.body ?: lastMessageContent.body
+        return lastMessageContent.newContent?.toModel<MessageContent>()?.body
+                ?: lastMessageContent.body
     }
 
     return null

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/database/helper/TimelineEventEntityHelper.kt

@@ -39,14 +39,17 @@ internal fun TimelineEventEntity.updateSenderData() {
     val isUnlinked = chunkEntity.isUnlinked()
     var senderMembershipEvent: EventEntity?
     var senderRoomMemberContent: String?
+    var senderRoomMemberPrevContent: String?
     when {
         stateIndex <= 0 -> {
             senderMembershipEvent = chunkEntity.timelineEvents.buildQuery(senderId, isUnlinked).next(from = stateIndex)?.root
             senderRoomMemberContent = senderMembershipEvent?.prevContent
+            senderRoomMemberPrevContent = senderMembershipEvent?.content
         }
         else            -> {
             senderMembershipEvent = chunkEntity.timelineEvents.buildQuery(senderId, isUnlinked).prev(since = stateIndex)?.root
             senderRoomMemberContent = senderMembershipEvent?.content
+            senderRoomMemberPrevContent = senderMembershipEvent?.prevContent
         }
     }
 
@@ -58,11 +61,27 @@ internal fun TimelineEventEntity.updateSenderData() {
                 .equalTo(EventEntityFields.TYPE, EventType.STATE_ROOM_MEMBER)
                 .prev(since = stateIndex)
         senderRoomMemberContent = senderMembershipEvent?.content
+        senderRoomMemberPrevContent = senderMembershipEvent?.prevContent
+    }
+
+    ContentMapper.map(senderRoomMemberContent).toModel<RoomMember>()?.also {
+        this.senderAvatar = it.avatarUrl
+        this.senderName = it.displayName
+        this.isUniqueDisplayName = RoomMembers(realm, roomId).isUniqueDisplayName(it.displayName)
+    }
+
+    // We try to fallback on prev content if we got a room member state events with null fields
+    if (root?.type == EventType.STATE_ROOM_MEMBER) {
+        ContentMapper.map(senderRoomMemberPrevContent).toModel<RoomMember>()?.also {
+            if (this.senderAvatar == null && it.avatarUrl != null) {
+                this.senderAvatar = it.avatarUrl
+            }
+            if (this.senderName == null && it.displayName != null) {
+                this.senderName = it.displayName
+                this.isUniqueDisplayName = RoomMembers(realm, roomId).isUniqueDisplayName(it.displayName)
+            }
+        }
     }
-    val senderRoomMember: RoomMember? = ContentMapper.map(senderRoomMemberContent).toModel()
-    this.senderAvatar = senderRoomMember?.avatarUrl
-    this.senderName = senderRoomMember?.displayName
-    this.isUniqueDisplayName = RoomMembers(realm, roomId).isUniqueDisplayName(senderRoomMember?.displayName)
     this.senderMembershipEvent = senderMembershipEvent
 }
 

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt

@@ -118,7 +118,7 @@ internal class LocalEchoEventFactory @Inject constructor(@UserId private val use
                 permalink,
                 stringProvider.getString(R.string.message_reply_to_prefix),
                 userLink,
-                originalEvent.senderName ?: originalEvent.root.senderId,
+                originalEvent.getDisambiguatedDisplayName(),
                 body.takeFormatted(),
                 createTextContent(newBodyText, newBodyAutoMarkdown).takeFormatted()
         )

vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt

@@ -409,7 +409,7 @@ class RoomDetailFragment :
         composerLayout.composerRelatedMessageActionIcon.setImageDrawable(ContextCompat.getDrawable(requireContext(), iconRes))
         composerLayout.sendButton.setContentDescription(getString(descriptionRes))
 
-        avatarRenderer.render(event.senderAvatar, event.root.senderId ?: "", event.senderName, composerLayout.composerRelatedMessageAvatar)
+        avatarRenderer.render(event.senderAvatar, event.root.senderId ?: "", event.getDisambiguatedDisplayName(), composerLayout.composerRelatedMessageAvatar)
         composerLayout.expand {
             // need to do it here also when not using quick reply
             focusComposerAndShowKeyboard()

vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/EncryptionItemFactory.kt

@@ -27,8 +27,6 @@ import im.vector.riotx.core.resources.StringProvider
 import im.vector.riotx.features.home.AvatarRenderer
 import im.vector.riotx.features.home.room.detail.timeline.TimelineEventController
 import im.vector.riotx.features.home.room.detail.timeline.helper.AvatarSizeProvider
-import im.vector.riotx.features.home.room.detail.timeline.helper.senderAvatar
-import im.vector.riotx.features.home.room.detail.timeline.helper.senderName
 import im.vector.riotx.features.home.room.detail.timeline.item.MessageInformationData
 import im.vector.riotx.features.home.room.detail.timeline.item.NoticeItem
 import im.vector.riotx.features.home.room.detail.timeline.item.NoticeItem_
@@ -41,13 +39,13 @@ class EncryptionItemFactory @Inject constructor(private val stringProvider: Stri
     fun create(event: TimelineEvent,
                highlight: Boolean,
                callback: TimelineEventController.Callback?): NoticeItem? {
-        val text = buildNoticeText(event.root, event.senderName) ?: return null
+        val text = buildNoticeText(event.root, event.getDisambiguatedDisplayName()) ?: return null
         val informationData = MessageInformationData(
                 eventId = event.root.eventId ?: "?",
                 senderId = event.root.senderId ?: "",
                 sendState = event.root.sendState,
-                avatarUrl = event.senderAvatar(),
+                avatarUrl = event.senderAvatar,
-                memberName = event.senderName(),
+                memberName = event.getDisambiguatedDisplayName(),
                 showInformation = false
         )
         val attributes = NoticeItem.Attributes(

vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/factory/MergedHeaderItemFactory.kt

@@ -64,12 +64,12 @@ class MergedHeaderItemFactory @Inject constructor(private val sessionHolder: Act
                     if (!showReadMarker && mergedEvent.hasReadMarker && readMarkerVisible) {
                         showReadMarker = true
                     }
-                    val senderAvatar = mergedEvent.senderAvatar()
+                    val senderAvatar = mergedEvent.senderAvatar
-                    val senderName = mergedEvent.senderName()
+                    val senderName = mergedEvent.getDisambiguatedDisplayName()
                     val data = MergedHeaderItem.Data(
                             userId = mergedEvent.root.senderId ?: "",
                             avatarUrl = senderAvatar,
-                            memberName = senderName ?: "",
+                            memberName = senderName,
                             localId = mergedEvent.localId,
                             eventId = mergedEvent.root.eventId ?: ""
                     )

vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/format/NoticeEventFormatter.kt

@@ -19,13 +19,17 @@ package im.vector.riotx.features.home.room.detail.timeline.format
 import im.vector.matrix.android.api.session.events.model.Event
 import im.vector.matrix.android.api.session.events.model.EventType
 import im.vector.matrix.android.api.session.events.model.toModel
-import im.vector.matrix.android.api.session.room.model.*
+import im.vector.matrix.android.api.session.room.model.Membership
+import im.vector.matrix.android.api.session.room.model.RoomHistoryVisibility
+import im.vector.matrix.android.api.session.room.model.RoomHistoryVisibilityContent
+import im.vector.matrix.android.api.session.room.model.RoomMember
+import im.vector.matrix.android.api.session.room.model.RoomNameContent
+import im.vector.matrix.android.api.session.room.model.RoomTopicContent
 import im.vector.matrix.android.api.session.room.model.call.CallInviteContent
 import im.vector.matrix.android.api.session.room.timeline.TimelineEvent
 import im.vector.riotx.R
 import im.vector.riotx.core.di.ActiveSessionHolder
 import im.vector.riotx.core.resources.StringProvider
-import im.vector.riotx.features.home.room.detail.timeline.helper.senderName
 import timber.log.Timber
 import javax.inject.Inject
 
@@ -36,7 +40,7 @@ class NoticeEventFormatter @Inject constructor(private val sessionHolder: Active
         return when (val type = timelineEvent.root.getClearType()) {
             EventType.STATE_ROOM_NAME          -> formatRoomNameEvent(timelineEvent.root, timelineEvent.getDisambiguatedDisplayName())
             EventType.STATE_ROOM_TOPIC         -> formatRoomTopicEvent(timelineEvent.root, timelineEvent.getDisambiguatedDisplayName())
-            EventType.STATE_ROOM_MEMBER        -> formatRoomMemberEvent(timelineEvent.root, timelineEvent.senderName())
+            EventType.STATE_ROOM_MEMBER        -> formatRoomMemberEvent(timelineEvent.root, timelineEvent.getDisambiguatedDisplayName())
             EventType.STATE_HISTORY_VISIBILITY -> formatRoomHistoryVisibilityEvent(timelineEvent.root, timelineEvent.getDisambiguatedDisplayName())
             EventType.STATE_ROOM_TOMBSTONE     -> formatRoomTombstoneEvent(timelineEvent.getDisambiguatedDisplayName())
             EventType.CALL_INVITE,
@@ -96,7 +100,8 @@ class NoticeEventFormatter @Inject constructor(private val sessionHolder: Active
     }
 
     private fun formatRoomHistoryVisibilityEvent(event: Event, senderName: String?): CharSequence? {
-        val historyVisibility = event.getClearContent().toModel<RoomHistoryVisibilityContent>()?.historyVisibility ?: return null
+        val historyVisibility = event.getClearContent().toModel<RoomHistoryVisibilityContent>()?.historyVisibility
+                                ?: return null
 
         val formattedVisibility = when (historyVisibility) {
             RoomHistoryVisibility.SHARED         -> stringProvider.getString(R.string.notice_room_visibility_shared)
@@ -135,7 +140,7 @@ class NoticeEventFormatter @Inject constructor(private val sessionHolder: Active
         }
     }
 
-    private fun buildProfileNotice(event: Event, senderName: String?, eventContent: RoomMember?, prevEventContent: RoomMember?): String? {
+    private fun buildProfileNotice(event: Event, senderName: String?, eventContent: RoomMember?, prevEventContent: RoomMember?): String {
         val displayText = StringBuilder()
         // Check display name has been changed
         if (eventContent?.displayName != prevEventContent?.displayName) {
@@ -160,6 +165,11 @@ class NoticeEventFormatter @Inject constructor(private val sessionHolder: Active
             }
             displayText.append(displayAvatarText)
         }
+        if (displayText.isEmpty()) {
+            displayText.append(
+                    stringProvider.getString(R.string.notice_member_no_changes, senderName)
+            )
+        }
         return displayText.toString()
     }
 
@@ -171,7 +181,8 @@ class NoticeEventFormatter @Inject constructor(private val sessionHolder: Active
                 val selfUserId = sessionHolder.getSafeActiveSession()?.myUserId
                 when {
                     eventContent.thirdPartyInvite != null -> {
-                        val userWhoHasAccepted = eventContent.thirdPartyInvite?.signed?.mxid ?: event.stateKey
+                        val userWhoHasAccepted = eventContent.thirdPartyInvite?.signed?.mxid
+                                                 ?: event.stateKey
                         stringProvider.getString(R.string.notice_room_third_party_registered_invite,
                                                  userWhoHasAccepted, eventContent.thirdPartyInvite?.displayName)
                     }

vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/helper/TimelineDisplayableEvents.kt

@@ -17,8 +17,6 @@
 package im.vector.riotx.features.home.room.detail.timeline.helper
 
 import im.vector.matrix.android.api.session.events.model.EventType
-import im.vector.matrix.android.api.session.events.model.toModel
-import im.vector.matrix.android.api.session.room.model.RoomMember
 import im.vector.matrix.android.api.session.room.timeline.TimelineEvent
 import im.vector.riotx.core.extensions.localDateTime
 
@@ -47,25 +45,6 @@ object TimelineDisplayableEvents {
     )
 }
 
-fun TimelineEvent.senderAvatar(): String? {
-    // We might have no avatar when user leave, so we try to get it from prevContent
-    return senderAvatar
-            ?: if (root.type == EventType.STATE_ROOM_MEMBER) {
-                root.prevContent.toModel<RoomMember>()?.avatarUrl
-            } else {
-                null
-            }
-}
-
-fun TimelineEvent.senderName(): String? {
-    // We might have no senderName when user leave, so we try to get it from prevContent
-    return when {
-        senderName != null                       -> getDisambiguatedDisplayName()
-        root.type == EventType.STATE_ROOM_MEMBER -> root.prevContent.toModel<RoomMember>()?.displayName
-        else                                     -> null
-    }
-}
-
 fun TimelineEvent.canBeMerged(): Boolean {
     return root.getClearType() == EventType.STATE_ROOM_MEMBER
 }

vector/src/main/java/im/vector/riotx/features/home/room/list/RoomSummaryItemFactory.kt

@@ -30,7 +30,6 @@ import im.vector.riotx.core.resources.DateProvider
 import im.vector.riotx.core.resources.StringProvider
 import im.vector.riotx.features.home.AvatarRenderer
 import im.vector.riotx.features.home.room.detail.timeline.format.NoticeEventFormatter
-import im.vector.riotx.features.home.room.detail.timeline.helper.senderName
 import me.gujun.android.span.span
 import javax.inject.Inject
 
@@ -97,10 +96,10 @@ class RoomSummaryItemFactory @Inject constructor(private val noticeEventFormatte
                     && latestEvent.root.mxDecryptionResult == null) {
                 stringProvider.getString(R.string.encrypted_message)
             } else if (latestEvent.root.getClearType() == EventType.MESSAGE) {
-                val senderName = latestEvent.senderName() ?: latestEvent.root.senderId
+                val senderName = latestEvent.getDisambiguatedDisplayName()
                 val content = latestEvent.root.getClearContent()?.toModel<MessageContent>()
                 val message = content?.body ?: ""
-                if (roomSummary.isDirect.not() && senderName != null) {
+                if (roomSummary.isDirect.not()) {
                     span {
                         text = senderName
                         textColor = colorProvider.getColorFromAttribute(R.attr.riotx_text_primary)

vector/src/main/java/im/vector/riotx/features/notifications/NotifiableEventResolver.kt

@@ -94,7 +94,7 @@ class NotifiableEventResolver @Inject constructor(private val stringProvider: St
                     event.getLastMessageBody()
                             ?: stringProvider.getString(R.string.notification_unknown_new_event)
             val roomName = stringProvider.getString(R.string.notification_unknown_room_name)
-            val senderDisplayName = event.senderName ?: event.root.senderId
+            val senderDisplayName = event.getDisambiguatedDisplayName()
 
             val notifiableEvent = NotifiableMessageEvent(
                     eventId = event.root.eventId!!,
@@ -128,7 +128,7 @@ class NotifiableEventResolver @Inject constructor(private val stringProvider: St
             val body = event.getLastMessageBody()
                     ?: stringProvider.getString(R.string.notification_unknown_new_event)
             val roomName = room.roomSummary()?.displayName ?: ""
-            val senderDisplayName = event.senderName ?: event.root.senderId
+            val senderDisplayName = event.getDisambiguatedDisplayName()
 
             val notifiableEvent = NotifiableMessageEvent(
                     eventId = event.root.eventId!!,

vector/src/main/res/values/strings_riotX.xml

@@ -2,6 +2,6 @@
 <resources>
 
     <!-- Strings not defined in Riot -->
-
+    <string name="notice_member_no_changes">"%1$s made no changes"</string>
 
 </resources>