Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix duplicate EventListSummarys #7952

Merged
merged 2 commits into from
Mar 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions src/components/structures/MessagePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1290,12 +1290,11 @@ class MainGrouper extends BaseGrouper {
const keyEvent = this.events.find(e => this.panel.grouperKeyMap.get(e));
const key = keyEvent ? this.panel.grouperKeyMap.get(keyEvent) : this.generateKey();
if (!keyEvent) {
// Populate the weak map with the key that we are using for all related events.
this.events.forEach(e => {
if (!this.panel.grouperKeyMap.has(e)) {
this.panel.grouperKeyMap.set(e, key);
}
});
// Populate the weak map with the key.
// Note that we only set the key on the specific event it refers to, since this group might get
// split up in the future by other intervening events. If we were to set the key on all events
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would this split up happen?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't able to find the specific cause, but I think what's happening is that when restarting Element, it pre-populates the timeline with state events that it knows about. Fetching the actual messages for the room can then split up these summaries.

// currently in the group, we would risk later giving the same key to multiple groups.
this.panel.grouperKeyMap.set(this.events[0], key);
}

let highlightInSummary = false;
Expand Down
29 changes: 29 additions & 0 deletions test/components/structures/MessagePanel-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -572,4 +572,33 @@ describe('MessagePanel', function() {
expect(els.key()).toEqual("eventlistsummary-" + events[0].getId());
expect(els.prop("events").length).toEqual(11);
});

it('assigns different keys to summaries that get split up', () => {
const events = mkMelsEvents().slice(1, 11);

const res = mount(<WrappedMessagePanel events={events} />);
let els = res.find("EventListSummary");
expect(els.length).toEqual(1);
expect(els.key()).toEqual("eventlistsummary-" + events[0].getId());
expect(els.prop("events").length).toEqual(10);

res.setProps({
events: [
...events.slice(0, 5),
TestUtilsMatrix.mkMessage({
event: true,
room: "!room:id",
user: "@user:id",
msg: "Hello!",
}),
...events.slice(5, 10),
],
});

els = res.find("EventListSummary");
expect(els.length).toEqual(2);
expect(els.first().key()).not.toEqual(els.last().key());
expect(els.first().prop("events").length).toEqual(5);
expect(els.last().prop("events").length).toEqual(5);
});
});