From 813be758d7b69e7f42d5fbf99bd8df6a8b0888a8 Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@element.io>
Date: Thu, 8 Apr 2021 15:33:21 +0200
Subject: [PATCH] we should prefer to not pick the event itself if it's a
 member event

but still fall back to that if it's a new join
---
 .../room/timeline/persistence/MemberWriter.js | 41 +++++++++++++------
 .../room/timeline/persistence/SyncWriter.js   |  2 +-
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/src/matrix/room/timeline/persistence/MemberWriter.js b/src/matrix/room/timeline/persistence/MemberWriter.js
index 73da64cd..db649f68 100644
--- a/src/matrix/room/timeline/persistence/MemberWriter.js
+++ b/src/matrix/room/timeline/persistence/MemberWriter.js
@@ -73,8 +73,7 @@ export class MemberWriter {
         }
     }
 
-    async lookupSenderMember(event, timelineEvents, txn) {
-        const userId = event.sender;
+    async lookupMember(userId, event, timelineEvents, txn) {
         let member = this._cache.get(userId);
         if (!member) {
             const memberData = await txn.roomMembers.get(this._roomId, userId);
@@ -84,23 +83,38 @@ export class MemberWriter {
             }
         }
         if (!member) {
-            let memberEvent;
             // sometimes the member event isn't included in state, but rather in the timeline,
             // even if it is not the first event in the timeline. In this case, go look for
-            // the last one before the event
+            // the last one before the event, or if none is found,
+            // the least recent matching member event in the timeline.
+            // The latter is needed because of new joins picking up their own display name
             let foundEvent = false;
+            let memberEventBefore;
+            let firstMemberEvent;
             for (let i = timelineEvents.length - 1; i >= 0; i -= 1) {
                 const e = timelineEvents[i];
-                if (!foundEvent && e.event_id === event.event_id) {
-                    foundEvent = true;
+                let matchingEvent;
+                if (e.type === MEMBER_EVENT_TYPE && e.state_key === userId) {
+                    matchingEvent = e;
+                    firstMemberEvent = matchingEvent;
                 }
-                if (foundEvent && e.type === MEMBER_EVENT_TYPE && e.state_key === userId) {
-                    memberEvent = e;
+                if (!foundEvent) {
+                    if (e.event_id === event.event_id) {
+                        foundEvent = true;
+                    } 
+                } else if (matchingEvent) {
+                    memberEventBefore = matchingEvent;
                     break;
                 }
             }
-            if (memberEvent) {
-                member = RoomMember.fromMemberEvent(this._roomId, memberEvent);
+            // first see if we found a member event before the event we're looking up the sender for
+            if (memberEventBefore) {
+                member = RoomMember.fromMemberEvent(this._roomId, memberEventBefore);
+            }
+            // and only if we didn't, fall back to the first member event,
+            // regardless of where it is positioned relative to the lookup event
+            else if (firstMemberEvent) {
+                member = RoomMember.fromMemberEvent(this._roomId, firstMemberEvent);
             }
         }
         return member;
@@ -233,17 +247,18 @@ export function tests() {
             const event = createMemberEvent("join", alice, "Alice");
             const writer = new MemberWriter(roomId);
             const txn = createStorage();
-            const member = await writer.lookupSenderMember(event, [event], txn);
+            const member = await writer.lookupMember(event.sender, event, [event], txn);
             assert(member);
             const change = await writer.writeTimelineMemberEvent(event, txn);
             assert(change);
         },
-        "lookupSenderMember returns closest member in the past": async assert => {
+        "lookupMember returns closest member in the past": async assert => {
             const event1 = createMemberEvent("join", alice, "Alice");
             const event2 = createMemberEvent("join", alice, "Alies");
+            const event3 = createMemberEvent("join", alice, "Alys");
             const writer = new MemberWriter(roomId);
             const txn = createStorage();
-            const member = await writer.lookupSenderMember(event2, [event1, event2], txn);
+            const member = await writer.lookupMember(event3.sender, event3, [event1, event2, event3], txn);
             assert.equal(member.displayName, "Alies");
         },
     };
diff --git a/src/matrix/room/timeline/persistence/SyncWriter.js b/src/matrix/room/timeline/persistence/SyncWriter.js
index 1f251fce..4913ac53 100644
--- a/src/matrix/room/timeline/persistence/SyncWriter.js
+++ b/src/matrix/room/timeline/persistence/SyncWriter.js
@@ -162,7 +162,7 @@ export class SyncWriter {
                 // store event in timeline
                 currentKey = currentKey.nextKey();
                 const entry = createEventEntry(currentKey, this._roomId, event);
-                let member = await this._memberWriter.lookupSenderMember(event, events, txn);
+                let member = await this._memberWriter.lookupMember(event.sender, event, events, txn);
                 if (member) {
                     entry.displayName = member.displayName;
                     entry.avatarUrl = member.avatarUrl;