Skip to content

Commit

Permalink
Don't remove thread info from a thread root when it is redacted (#3814)
Browse files Browse the repository at this point in the history
* Don't remove thread info from a thread root when it is redacted

* Move the redaction event to main at the same time we move redacted

Since the redacted event is moving to the main timeline, the redaction
belongs there too, since its relationship to the redacted event is the
only thing making it part of the thread.
  • Loading branch information
andybalaam authored Oct 20, 2023
1 parent c026495 commit 0643f38
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 7 deletions.
23 changes: 23 additions & 0 deletions spec/unit/models/event.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,29 @@ describe("MatrixEvent", () => {
expect(mainTimelineLiveEventIds(room)).toEqual([ev.getId()]);
});

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();
Expand Down
6 changes: 4 additions & 2 deletions src/models/event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1193,9 +1193,11 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
}
}

// If the redacted event was in a thread
if (room && this.threadRootId && this.threadRootId !== this.getId()) {
// 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();
Expand Down
14 changes: 11 additions & 3 deletions src/models/room.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -2329,6 +2336,7 @@ 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) {
const threadRootId = redactedEvent.threadRootId;
redactedEvent.makeRedacted(event, this);

// If this is in the current state, replace it with the redacted version
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions src/models/thread.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 0643f38

Please sign in to comment.