From b5414ea914d7626e368bb1d3f6273cf8661f305e Mon Sep 17 00:00:00 2001
From: Andy Balaam <andy.balaam@matrix.org>
Date: Thu, 25 May 2023 16:48:48 +0100
Subject: [PATCH] Fix bug where original event was inserted into timeline
 instead of the edit event (#3398)

* Fix an existing test for editing messages in threads

While attempting to test a new change, I discovered that the test
"should allow edits to be added to thread timeline" did not actually
fail if its assertions failed. Further, those assertions were incorrect.

So this change fixes the test to create the thread, wait for it to be
initialised, and then add events to it. This simplifies the flow and
ensures the test fails if something unexpected happens.

* Move editing test into thread.spec.ts

* Isolate Thread global modification in beforeAll()

* Delete unneeded setUnsigned call

* Use standard message-creation methods

* Rename event variables

* Rename sender->user

* Remove unneeded variables

* Extract distractions into functions

* Fetch edits for thread messages

Modifies fetchEditsWhereNeeded to allow edits of threaded messages. The
code before prevented any relations from fetching edits, but of course
events in threads are relations.

We definitely want thread messages to get their edits fetched, and I
assume this is working in the real code, probably because the event
being looked at is some kind of eventmapped thing that doesn't have
proper relations visible on it.

In tests, if we don't make this change, we can't see edits getting
fetched.

* Add a test for fetching edits on demand in a thread

This test demonstrates the current behaviour, which contains a bug - we
don't actually add the right event to the timeline.

* Fix bug where original event was inserted into timeline instead of the edit event
---
 spec/unit/models/thread.spec.ts | 19 ++++++-------------
 src/models/thread.ts            |  5 +++--
 2 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/spec/unit/models/thread.spec.ts b/spec/unit/models/thread.spec.ts
index 480e28f64..6aa1d5c86 100644
--- a/spec/unit/models/thread.spec.ts
+++ b/spec/unit/models/thread.spec.ts
@@ -660,21 +660,14 @@ describe("Thread", () => {
                 });
                 await thread.addEvent(messageToEdit, false);
 
-                // THIS IS THE CORRECT BEHAVIOUR
                 // Then both events end up in the timeline
-                //const lastEvent = thread.timeline.at(-1)!;
-                //const secondLastEvent = thread.timeline.at(-2)!;
-                //expect(lastEvent).toBe(editEvent);
-                //expect(secondLastEvent).toBe(messageToEdit);
-
-                //// And the first message has been edited
-                //expect(secondLastEvent.getContent().body).toEqual("edit");
-
-                // TODO: For now, we incorrecly DON'T add the event to the timeline
                 const lastEvent = thread.timeline.at(-1)!;
-                expect(lastEvent).toBe(messageToEdit);
-                // But the original is edited, as expected
-                expect(lastEvent.getContent().body).toEqual("edit");
+                const secondLastEvent = thread.timeline.at(-2)!;
+                expect(lastEvent).toBe(editEvent);
+                expect(secondLastEvent).toBe(messageToEdit);
+
+                // And the first message has been edited
+                expect(secondLastEvent.getContent().body).toEqual("edit");
             });
         });
     });
diff --git a/src/models/thread.ts b/src/models/thread.ts
index b04ea63dc..11727d970 100644
--- a/src/models/thread.ts
+++ b/src/models/thread.ts
@@ -530,8 +530,9 @@ export class Thread extends ReadReceipt<EmittedEvents, EventHandlerMap> {
                             })
                             .then((relations) => {
                                 if (relations.events.length) {
-                                    event.makeReplaced(relations.events[0]);
-                                    this.insertEventIntoTimeline(event);
+                                    const editEvent = relations.events[0];
+                                    event.makeReplaced(editEvent);
+                                    this.insertEventIntoTimeline(editEvent);
                                 }
                             })
                             .catch((e) => {