Merge pull request #3869 from matrix-org/andybalaam/unrevert-deletion-move-prs

Unrevert "Move redacted messages out of any thread, into main thread"

spec/unit/models/event.spec.ts

@@ -14,10 +14,19 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+import { MockedObject } from "jest-mock";
+
 import { MatrixEvent, MatrixEventEvent } from "../../../src/models/event";
 import { emitPromise } from "../../test-utils/test-utils";
 import { Crypto, IEventDecryptionResult } from "../../../src/crypto";
-import { IAnnotatedPushRule, PushRuleActionName, TweakName } from "../../../src";
+import {
+    IAnnotatedPushRule,
+    MatrixClient,
+    PushRuleActionName,
+    Room,
+    THREAD_RELATION_TYPE,
+    TweakName,
+} from "../../../src";
 
 describe("MatrixEvent", () => {
     it("should create copies of itself", () => {
@@ -61,31 +70,264 @@ describe("MatrixEvent", () => {
         expect(a.toSnapshot().isEquivalentTo(b)).toBe(false);
     });
 
-    it("should prune clearEvent when being redacted", () => {
-        const ev = new MatrixEvent({
-            type: "m.room.message",
-            content: {
-                body: "Test",
-            },
-            event_id: "$event1:server",
+    describe("redaction", () => {
+        it("should prune clearEvent when being redacted", () => {
+            const ev = createEvent("$event1:server", "Test");
+
+            expect(ev.getContent().body).toBe("Test");
+            expect(ev.getWireContent().body).toBe("Test");
+            ev.makeEncrypted("m.room.encrypted", { ciphertext: "xyz" }, "", "");
+            expect(ev.getContent().body).toBe("Test");
+            expect(ev.getWireContent().body).toBeUndefined();
+            expect(ev.getWireContent().ciphertext).toBe("xyz");
+
+            const mockClient = {} as unknown as MockedObject<MatrixClient>;
+            const room = new Room("!roomid:e.xyz", mockClient, "myname");
+            const redaction = createRedaction(ev.getId()!);
+
+            ev.makeRedacted(redaction, room);
+            expect(ev.getContent().body).toBeUndefined();
+            expect(ev.getWireContent().body).toBeUndefined();
+            expect(ev.getWireContent().ciphertext).toBeUndefined();
         });
 
-        expect(ev.getContent().body).toBe("Test");
-        expect(ev.getWireContent().body).toBe("Test");
-        ev.makeEncrypted("m.room.encrypted", { ciphertext: "xyz" }, "", "");
-        expect(ev.getContent().body).toBe("Test");
-        expect(ev.getWireContent().body).toBeUndefined();
-        expect(ev.getWireContent().ciphertext).toBe("xyz");
+        it("should remain in the main timeline when redacted", async () => {
+            // Given an event in the main timeline
+            const mockClient = createMockClient();
+            const room = new Room("!roomid:e.xyz", mockClient, "myname");
+            const ev = createEvent("$event1:server");
 
-        const redaction = new MatrixEvent({
-            type: "m.room.redaction",
-            redacts: ev.getId(),
+            await room.addLiveEvents([ev]);
+            await room.createThreadsTimelineSets();
+            expect(ev.threadRootId).toBeUndefined();
+            expect(mainTimelineLiveEventIds(room)).toEqual([ev.getId()]);
+
+            // When I redact it
+            const redaction = createRedaction(ev.getId()!);
+            ev.makeRedacted(redaction, room);
+
+            // Then it remains in the main timeline
+            expect(ev.threadRootId).toBeUndefined();
+            expect(mainTimelineLiveEventIds(room)).toEqual([ev.getId()]);
         });
 
-        ev.makeRedacted(redaction);
-        expect(ev.getContent().body).toBeUndefined();
-        expect(ev.getWireContent().body).toBeUndefined();
-        expect(ev.getWireContent().ciphertext).toBeUndefined();
+        it("should keep thread roots in both timelines when redacted", async () => {
+            // Given a thread exists
+            const mockClient = createMockClient();
+            const room = new Room("!roomid:e.xyz", mockClient, "myname");
+            const threadRoot = createEvent("$threadroot:server");
+            const ev = createThreadedEvent("$event1:server", threadRoot.getId()!);
+
+            await room.addLiveEvents([threadRoot, ev]);
+            await room.createThreadsTimelineSets();
+            expect(threadRoot.threadRootId).toEqual(threadRoot.getId());
+            expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId()]);
+            expect(threadLiveEventIds(room, 0)).toEqual([threadRoot.getId(), ev.getId()]);
+
+            // When I redact the thread root
+            const redaction = createRedaction(ev.getId()!);
+            threadRoot.makeRedacted(redaction, room);
+
+            // Then it remains in the main timeline and the thread
+            expect(threadRoot.threadRootId).toEqual(threadRoot.getId());
+            expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId()]);
+            expect(threadLiveEventIds(room, 0)).toEqual([threadRoot.getId(), ev.getId()]);
+        });
+
+        it("should move into the main timeline when redacted", async () => {
+            // Given an event in a thread
+            const mockClient = createMockClient();
+            const room = new Room("!roomid:e.xyz", mockClient, "myname");
+            const threadRoot = createEvent("$threadroot:server");
+            const ev = createThreadedEvent("$event1:server", threadRoot.getId()!);
+
+            await room.addLiveEvents([threadRoot, ev]);
+            await room.createThreadsTimelineSets();
+            expect(ev.threadRootId).toEqual(threadRoot.getId());
+            expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId()]);
+            expect(threadLiveEventIds(room, 0)).toEqual([threadRoot.getId(), ev.getId()]);
+
+            // When I redact it
+            const redaction = createRedaction(ev.getId()!);
+            ev.makeRedacted(redaction, room);
+
+            // Then it disappears from the thread and appears in the main timeline
+            expect(ev.threadRootId).toBeUndefined();
+            expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId(), ev.getId()]);
+            expect(threadLiveEventIds(room, 0)).not.toContain(ev.getId());
+        });
+
+        it("should move reactions to a redacted event into the main timeline", async () => {
+            // Given an event in a thread with a reaction
+            const mockClient = createMockClient();
+            const room = new Room("!roomid:e.xyz", mockClient, "myname");
+            const threadRoot = createEvent("$threadroot:server");
+            const ev = createThreadedEvent("$event1:server", threadRoot.getId()!);
+            const reaction = createReactionEvent("$reaction:server", ev.getId()!);
+
+            await room.addLiveEvents([threadRoot, ev, reaction]);
+            await room.createThreadsTimelineSets();
+            expect(reaction.threadRootId).toEqual(threadRoot.getId());
+            expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId()]);
+            expect(threadLiveEventIds(room, 0)).toEqual([threadRoot.getId(), ev.getId(), reaction.getId()]);
+
+            // When I redact the event
+            const redaction = createRedaction(ev.getId()!);
+            ev.makeRedacted(redaction, room);
+
+            // Then the reaction moves into the main timeline
+            expect(reaction.threadRootId).toBeUndefined();
+            expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId(), ev.getId(), reaction.getId()]);
+            expect(threadLiveEventIds(room, 0)).not.toContain(reaction.getId());
+        });
+
+        it("should move edits of a redacted event into the main timeline", async () => {
+            // Given an event in a thread with a reaction
+            const mockClient = createMockClient();
+            const room = new Room("!roomid:e.xyz", mockClient, "myname");
+            const threadRoot = createEvent("$threadroot:server");
+            const ev = createThreadedEvent("$event1:server", threadRoot.getId()!);
+            const edit = createEditEvent("$edit:server", ev.getId()!);
+
+            await room.addLiveEvents([threadRoot, ev, edit]);
+            await room.createThreadsTimelineSets();
+            expect(edit.threadRootId).toEqual(threadRoot.getId());
+            expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId()]);
+            expect(threadLiveEventIds(room, 0)).toEqual([threadRoot.getId(), ev.getId(), edit.getId()]);
+
+            // When I redact the event
+            const redaction = createRedaction(ev.getId()!);
+            ev.makeRedacted(redaction, room);
+
+            // Then the edit moves into the main timeline
+            expect(edit.threadRootId).toBeUndefined();
+            expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId(), ev.getId(), edit.getId()]);
+            expect(threadLiveEventIds(room, 0)).not.toContain(edit.getId());
+        });
+
+        it("should move reactions to replies to replies a redacted event into the main timeline", async () => {
+            // Given an event in a thread with a reaction
+            const mockClient = createMockClient();
+            const room = new Room("!roomid:e.xyz", mockClient, "myname");
+            const threadRoot = createEvent("$threadroot:server");
+            const ev = createThreadedEvent("$event1:server", threadRoot.getId()!);
+            const reply1 = createReplyEvent("$reply1:server", ev.getId()!);
+            const reply2 = createReplyEvent("$reply2:server", reply1.getId()!);
+            const reaction = createReactionEvent("$reaction:server", reply2.getId()!);
+
+            await room.addLiveEvents([threadRoot, ev, reply1, reply2, reaction]);
+            await room.createThreadsTimelineSets();
+            expect(reaction.threadRootId).toEqual(threadRoot.getId());
+            expect(mainTimelineLiveEventIds(room)).toEqual([threadRoot.getId()]);
+            expect(threadLiveEventIds(room, 0)).toEqual([
+                threadRoot.getId(),
+                ev.getId(),
+                reply1.getId(),
+                reply2.getId(),
+                reaction.getId(),
+            ]);
+
+            // When I redact the event
+            const redaction = createRedaction(ev.getId()!);
+            ev.makeRedacted(redaction, room);
+
+            // Then the replies move to the main thread and the reaction disappears
+            expect(reaction.threadRootId).toBeUndefined();
+            expect(mainTimelineLiveEventIds(room)).toEqual([
+                threadRoot.getId(),
+                ev.getId(),
+                reply1.getId(),
+                reply2.getId(),
+                reaction.getId(),
+            ]);
+            expect(threadLiveEventIds(room, 0)).not.toContain(reply1.getId());
+            expect(threadLiveEventIds(room, 0)).not.toContain(reply2.getId());
+            expect(threadLiveEventIds(room, 0)).not.toContain(reaction.getId());
+        });
+
+        function createMockClient(): MatrixClient {
+            return {
+                supportsThreads: jest.fn().mockReturnValue(true),
+                decryptEventIfNeeded: jest.fn().mockReturnThis(),
+                getUserId: jest.fn().mockReturnValue("@user:server"),
+            } as unknown as MockedObject<MatrixClient>;
+        }
+
+        function createEvent(eventId: string, body?: string): MatrixEvent {
+            return new MatrixEvent({
+                type: "m.room.message",
+                content: {
+                    body: body ?? eventId,
+                },
+                event_id: eventId,
+            });
+        }
+
+        function createThreadedEvent(eventId: string, threadRootId: string): MatrixEvent {
+            return new MatrixEvent({
+                type: "m.room.message",
+                content: {
+                    "body": eventId,
+                    "m.relates_to": {
+                        rel_type: THREAD_RELATION_TYPE.name,
+                        event_id: threadRootId,
+                    },
+                },
+                event_id: eventId,
+            });
+        }
+
+        function createEditEvent(eventId: string, repliedToId: string): MatrixEvent {
+            return new MatrixEvent({
+                type: "m.room.message",
+                content: {
+                    "body": "Edited",
+                    "m.new_content": {
+                        body: "Edited",
+                    },
+                    "m.relates_to": {
+                        event_id: repliedToId,
+                        rel_type: "m.replace",
+                    },
+                },
+                event_id: eventId,
+            });
+        }
+
+        function createReplyEvent(eventId: string, repliedToId: string): MatrixEvent {
+            return new MatrixEvent({
+                type: "m.room.message",
+                content: {
+                    "m.relates_to": {
+                        event_id: repliedToId,
+                        key: "x",
+                        rel_type: "m.in_reply_to",
+                    },
+                },
+                event_id: eventId,
+            });
+        }
+
+        function createReactionEvent(eventId: string, reactedToId: string): MatrixEvent {
+            return new MatrixEvent({
+                type: "m.reaction",
+                content: {
+                    "m.relates_to": {
+                        event_id: reactedToId,
+                        key: "x",
+                        rel_type: "m.annotation",
+                    },
+                },
+                event_id: eventId,
+            });
+        }
+
+        function createRedaction(redactedEventid: string): MatrixEvent {
+            return new MatrixEvent({
+                type: "m.room.redaction",
+                redacts: redactedEventid,
+            });
+        }
     });
 
     describe("applyVisibilityEvent", () => {
@@ -330,3 +572,19 @@ describe("MatrixEvent", () => {
         expect(stateEvent.threadRootId).toBeUndefined();
     });
 });
+
+function mainTimelineLiveEventIds(room: Room): Array<string> {
+    return room
+        .getLiveTimeline()
+        .getEvents()
+        .map((e) => e.getId()!);
+}
+
+function threadLiveEventIds(room: Room, threadIndex: number): Array<string> {
+    return room
+        .getThreads()
+        [threadIndex].getUnfilteredTimelineSet()
+        .getLiveTimeline()
+        .getEvents()
+        .map((e) => e.getId()!);
+}

spec/unit/room-state.spec.ts

@@ -27,6 +27,7 @@ import { M_BEACON } from "../../src/@types/beacon";
 import { MatrixClient } from "../../src/client";
 import { DecryptionError } from "../../src/crypto/algorithms";
 import { defer } from "../../src/utils";
+import { Room } from "../../src/models/room";
 
 describe("RoomState", function () {
     const roomId = "!foo:bar";
@@ -362,9 +363,11 @@ describe("RoomState", function () {
         });
 
         it("does not add redacted beacon info events to state", () => {
+            const mockClient = {} as unknown as MockedObject<MatrixClient>;
             const redactedBeaconEvent = makeBeaconInfoEvent(userA, roomId);
             const redactionEvent = new MatrixEvent({ type: "m.room.redaction" });
-            redactedBeaconEvent.makeRedacted(redactionEvent);
+            const room = new Room(roomId, mockClient, userA);
+            redactedBeaconEvent.makeRedacted(redactionEvent, room);
             const emitSpy = jest.spyOn(state, "emit");
 
             state.setStateEvents([redactedBeaconEvent]);
@@ -394,11 +397,13 @@ describe("RoomState", function () {
         });
 
         it("destroys and removes redacted beacon events", () => {
+            const mockClient = {} as unknown as MockedObject<MatrixClient>;
             const beaconId = "$beacon1";
             const beaconEvent = makeBeaconInfoEvent(userA, roomId, { isLive: true }, beaconId);
             const redactedBeaconEvent = makeBeaconInfoEvent(userA, roomId, { isLive: true }, beaconId);
             const redactionEvent = new MatrixEvent({ type: "m.room.redaction", redacts: beaconEvent.getId() });
-            redactedBeaconEvent.makeRedacted(redactionEvent);
+            const room = new Room(roomId, mockClient, userA);
+            redactedBeaconEvent.makeRedacted(redactionEvent, room);
 
             state.setStateEvents([beaconEvent]);
             const beaconInstance = state.beacons.get(getBeaconInfoIdentifier(beaconEvent));

spec/unit/room.spec.ts

@@ -3654,7 +3654,7 @@ describe("Room", function () {
             expect(room.polls.get(pollStartEvent.getId()!)).toBeTruthy();
 
             const redactedEvent = new MatrixEvent({ type: "m.room.redaction" });
-            pollStartEvent.makeRedacted(redactedEvent);
+            pollStartEvent.makeRedacted(redactedEvent, room);
 
             await flushPromises();
 

src/models/event.ts

@@ -45,6 +45,8 @@ import { DecryptionError } from "../crypto/algorithms";
 import { CryptoBackend } from "../common-crypto/CryptoBackend";
 import { WITHHELD_MESSAGES } from "../crypto/OlmDevice";
 import { IAnnotatedPushRule } from "../@types/PushRules";
+import { Room } from "./room";
+import { EventTimeline } from "./event-timeline";
 
 export { EventStatus } from "./event-status";
 
@@ -1135,13 +1137,19 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
         return this.visibility;
     }
 
+    /**
+     * @deprecated In favor of the overload that includes a Room argument
+     */
+    public makeRedacted(redactionEvent: MatrixEvent): void;
     /**
      * Update the content of an event in the same way it would be by the server
      * if it were redacted before it was sent to us
      *
      * @param redactionEvent - event causing the redaction
+     * @param room - the room in which the event exists
      */
-    public makeRedacted(redactionEvent: MatrixEvent): void {
+    public makeRedacted(redactionEvent: MatrixEvent, room: Room): void;
+    public makeRedacted(redactionEvent: MatrixEvent, room?: Room): void {
         // quick sanity-check
         if (!redactionEvent.event) {
             throw new Error("invalid redactionEvent in makeRedacted");
@@ -1185,9 +1193,43 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
             }
         }
 
+        // If the redacted event was in a thread (but not thread root), move it
+        // to the main timeline. This will change if MSC3389 is merged.
+        if (room && !this.isThreadRoot && this.threadRootId && this.threadRootId !== this.getId()) {
+            this.moveAllRelatedToMainTimeline(room);
+            redactionEvent.moveToMainTimeline(room);
+        }
+
         this.invalidateExtensibleEvent();
     }
 
+    private moveAllRelatedToMainTimeline(room: Room): void {
+        const thread = this.thread;
+        this.moveToMainTimeline(room);
+
+        // If we dont have access to the thread, we can only move this
+        // event, not things related to it.
+        if (thread) {
+            for (const event of thread.events) {
+                if (event.getRelation()?.event_id === this.getId()) {
+                    event.moveAllRelatedToMainTimeline(room);
+                }
+            }
+        }
+    }
+
+    private moveToMainTimeline(room: Room): void {
+        // Remove it from its thread
+        this.thread?.timelineSet.removeEvent(this.getId()!);
+        this.setThread(undefined);
+
+        // And insert it into the main timeline
+        const timeline = room.getLiveTimeline();
+        // We use insertEventIntoTimeline to insert it in timestamp order,
+        // because we don't know where it should go (until we have MSC4033).
+        timeline.getTimelineSet().insertEventIntoTimeline(this, timeline, timeline.getState(EventTimeline.FORWARDS)!);
+    }
+
     /**
      * Check if this event has been redacted
      *

src/models/room.ts

@@ -236,8 +236,9 @@ export type RoomEventHandlerMap = {
      *
      * @param event - The matrix redaction event
      * @param room - The room containing the redacted event
+     * @param threadId - The thread containing the redacted event (before it was redacted)
      */
-    [RoomEvent.Redaction]: (event: MatrixEvent, room: Room) => void;
+    [RoomEvent.Redaction]: (event: MatrixEvent, room: Room, threadId?: string) => void;
     /**
      * Fires when an event that was previously redacted isn't anymore.
      * This happens when the redaction couldn't be sent and
@@ -2113,6 +2114,12 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
      * Relations (other than m.thread), redactions, replies to a thread root live only in the main timeline
      * Relations, redactions, replies where the parent cannot be found live in no timelines but should be aggregated regardless.
      * Otherwise, the event lives in the main timeline only.
+     *
+     * Note: when a redaction is applied, the redacted event, events relating
+     * to it, and the redaction event itself, will all move to the main thread.
+     * This method classifies them as inside the thread of the redacted event.
+     * They are moved later as part of makeRedacted.
+     * This will change if MSC3389 is merged.
      */
     public eventShouldLiveIn(
         event: MatrixEvent,
@@ -2329,7 +2336,8 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
             // if we know about this event, redact its contents now.
             const redactedEvent = redactId ? this.findEventById(redactId) : undefined;
             if (redactedEvent) {
-                redactedEvent.makeRedacted(event);
+                const threadRootId = redactedEvent.threadRootId;
+                redactedEvent.makeRedacted(event, this);
 
                 // If this is in the current state, replace it with the redacted version
                 if (redactedEvent.isState()) {
@@ -2342,7 +2350,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
                     }
                 }
 
-                this.emit(RoomEvent.Redaction, event, this);
+                this.emit(RoomEvent.Redaction, event, this, threadRootId);
 
                 // TODO: we stash user displaynames (among other things) in
                 // RoomMember objects which are then attached to other events
@@ -2495,7 +2503,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
                 }
                 if (redactedEvent) {
                     redactedEvent.markLocallyRedacted(event);
-                    this.emit(RoomEvent.Redaction, event, this);
+                    this.emit(RoomEvent.Redaction, event, this, redactedEvent.threadRootId);
                 }
             }
         } else {

src/models/thread.ts

@@ -228,8 +228,8 @@ export class Thread extends ReadReceipt<ThreadEmittedEvents, ThreadEventHandlerM
         }
     };
 
-    private onRedaction = async (event: MatrixEvent): Promise<void> => {
-        if (event.threadRootId !== this.id) return; // ignore redactions for other timelines
+    private onRedaction = async (event: MatrixEvent, room: Room, threadRootId?: string): Promise<void> => {
+        if (threadRootId !== this.id) return; // ignore redactions for other timelines
         if (this.replyCount <= 0) {
             for (const threadEvent of this.timeline) {
                 this.clearEventMetadata(threadEvent);