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
This commit is contained in:
Bruno Windels 2021-04-08 15:33:21 +02:00
parent f67ccc18f4
commit 813be758d7
2 changed files with 29 additions and 14 deletions

View file

@ -73,8 +73,7 @@ export class MemberWriter {
} }
} }
async lookupSenderMember(event, timelineEvents, txn) { async lookupMember(userId, event, timelineEvents, txn) {
const userId = event.sender;
let member = this._cache.get(userId); let member = this._cache.get(userId);
if (!member) { if (!member) {
const memberData = await txn.roomMembers.get(this._roomId, userId); const memberData = await txn.roomMembers.get(this._roomId, userId);
@ -84,23 +83,38 @@ export class MemberWriter {
} }
} }
if (!member) { if (!member) {
let memberEvent;
// sometimes the member event isn't included in state, but rather in the timeline, // 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 // 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 foundEvent = false;
let memberEventBefore;
let firstMemberEvent;
for (let i = timelineEvents.length - 1; i >= 0; i -= 1) { for (let i = timelineEvents.length - 1; i >= 0; i -= 1) {
const e = timelineEvents[i]; const e = timelineEvents[i];
if (!foundEvent && e.event_id === event.event_id) { let matchingEvent;
if (e.type === MEMBER_EVENT_TYPE && e.state_key === userId) {
matchingEvent = e;
firstMemberEvent = matchingEvent;
}
if (!foundEvent) {
if (e.event_id === event.event_id) {
foundEvent = true; foundEvent = true;
} }
if (foundEvent && e.type === MEMBER_EVENT_TYPE && e.state_key === userId) { } else if (matchingEvent) {
memberEvent = e; memberEventBefore = matchingEvent;
break; break;
} }
} }
if (memberEvent) { // first see if we found a member event before the event we're looking up the sender for
member = RoomMember.fromMemberEvent(this._roomId, memberEvent); 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; return member;
@ -233,17 +247,18 @@ export function tests() {
const event = createMemberEvent("join", alice, "Alice"); const event = createMemberEvent("join", alice, "Alice");
const writer = new MemberWriter(roomId); const writer = new MemberWriter(roomId);
const txn = createStorage(); const txn = createStorage();
const member = await writer.lookupSenderMember(event, [event], txn); const member = await writer.lookupMember(event.sender, event, [event], txn);
assert(member); assert(member);
const change = await writer.writeTimelineMemberEvent(event, txn); const change = await writer.writeTimelineMemberEvent(event, txn);
assert(change); 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 event1 = createMemberEvent("join", alice, "Alice");
const event2 = createMemberEvent("join", alice, "Alies"); const event2 = createMemberEvent("join", alice, "Alies");
const event3 = createMemberEvent("join", alice, "Alys");
const writer = new MemberWriter(roomId); const writer = new MemberWriter(roomId);
const txn = createStorage(); 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"); assert.equal(member.displayName, "Alies");
}, },
}; };

View file

@ -162,7 +162,7 @@ export class SyncWriter {
// store event in timeline // store event in timeline
currentKey = currentKey.nextKey(); currentKey = currentKey.nextKey();
const entry = createEventEntry(currentKey, this._roomId, event); 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) { if (member) {
entry.displayName = member.displayName; entry.displayName = member.displayName;
entry.avatarUrl = member.avatarUrl; entry.avatarUrl = member.avatarUrl;