From 7813e12eb0806b5eff1facd73b3b09323c6afa13 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Tue, 7 Nov 2023 13:41:33 +0000 Subject: [PATCH] Revert code moving deleted messages to main timeline (#3858) * Revert "Move the redaction event to main at the same time we move redacted" This reverts commit 378a776815f63fdd1e4d507af35046c0ba88153c. Context: https://github.com/vector-im/element-web/issues/26498 * Revert "Don't remove thread info from a thread root when it is redacted" This reverts commit 17b61a69c20677e39e4f3b1b4ed5903421eaee6b. Context: https://github.com/vector-im/element-web/issues/26498 * Revert "Move all related messages into main timeline on redaction" This reverts commit d8fc1795f1319b6a77175c5c584ae03be53c457c. Context: https://github.com/vector-im/element-web/issues/26498 * Revert "Factor out the code for moving an event to the main timeline" This reverts commit 942dfcb84b8aef6ea84a419d73e845d3611bd91c. Context: https://github.com/vector-im/element-web/issues/26498 * Revert "Factor out utils in redaction tests" This reverts commit 43a0dc56e130f75ef695b52cd23945e10393119a. Context: https://github.com/vector-im/element-web/issues/26498 * Revert "Move redaction event tests into their own describe block" This reverts commit 9b0ea80f93fe944da03c27df26064cae1765c94d. Context: https://github.com/vector-im/element-web/issues/26498 * Revert "Move redacted messages out of any thread, into main timeline." This reverts commit b94d137398d600ad208450732b6f12aed177221b. Context: https://github.com/vector-im/element-web/issues/26498 --- spec/unit/models/event.spec.ts | 300 +++------------------------------ spec/unit/room-state.spec.ts | 9 +- spec/unit/room.spec.ts | 2 +- src/models/event.ts | 44 +---- src/models/room.ts | 16 +- src/models/thread.ts | 4 +- 6 files changed, 31 insertions(+), 344 deletions(-) diff --git a/spec/unit/models/event.spec.ts b/spec/unit/models/event.spec.ts index 5ff9d745da1..efb2404f6a6 100644 --- a/spec/unit/models/event.spec.ts +++ b/spec/unit/models/event.spec.ts @@ -14,19 +14,10 @@ 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, - MatrixClient, - PushRuleActionName, - Room, - THREAD_RELATION_TYPE, - TweakName, -} from "../../../src"; +import { IAnnotatedPushRule, PushRuleActionName, TweakName } from "../../../src"; describe("MatrixEvent", () => { it("should create copies of itself", () => { @@ -70,264 +61,31 @@ describe("MatrixEvent", () => { expect(a.toSnapshot().isEquivalentTo(b)).toBe(false); }); - 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; - 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(); - }); - - 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"); - - 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()]); - }); - - 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 prune clearEvent when being redacted", () => { + const ev = new MatrixEvent({ + type: "m.room.message", + content: { + body: "Test", + }, + event_id: "$event1:server", }); - 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()); - }); + 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 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()); + const redaction = new MatrixEvent({ + type: "m.room.redaction", + redacts: ev.getId(), }); - function createMockClient(): MatrixClient { - return { - supportsThreads: jest.fn().mockReturnValue(true), - decryptEventIfNeeded: jest.fn().mockReturnThis(), - getUserId: jest.fn().mockReturnValue("@user:server"), - } as unknown as MockedObject; - } - - 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, - }); - } + ev.makeRedacted(redaction); + expect(ev.getContent().body).toBeUndefined(); + expect(ev.getWireContent().body).toBeUndefined(); + expect(ev.getWireContent().ciphertext).toBeUndefined(); }); describe("applyVisibilityEvent", () => { @@ -572,19 +330,3 @@ describe("MatrixEvent", () => { expect(stateEvent.threadRootId).toBeUndefined(); }); }); - -function mainTimelineLiveEventIds(room: Room): Array { - return room - .getLiveTimeline() - .getEvents() - .map((e) => e.getId()!); -} - -function threadLiveEventIds(room: Room, threadIndex: number): Array { - return room - .getThreads() - [threadIndex].getUnfilteredTimelineSet() - .getLiveTimeline() - .getEvents() - .map((e) => e.getId()!); -} diff --git a/spec/unit/room-state.spec.ts b/spec/unit/room-state.spec.ts index 6c60d08f908..435d2e33ddc 100644 --- a/spec/unit/room-state.spec.ts +++ b/spec/unit/room-state.spec.ts @@ -27,7 +27,6 @@ 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"; @@ -363,11 +362,9 @@ describe("RoomState", function () { }); it("does not add redacted beacon info events to state", () => { - const mockClient = {} as unknown as MockedObject; const redactedBeaconEvent = makeBeaconInfoEvent(userA, roomId); const redactionEvent = new MatrixEvent({ type: "m.room.redaction" }); - const room = new Room(roomId, mockClient, userA); - redactedBeaconEvent.makeRedacted(redactionEvent, room); + redactedBeaconEvent.makeRedacted(redactionEvent); const emitSpy = jest.spyOn(state, "emit"); state.setStateEvents([redactedBeaconEvent]); @@ -397,13 +394,11 @@ describe("RoomState", function () { }); it("destroys and removes redacted beacon events", () => { - const mockClient = {} as unknown as MockedObject; 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() }); - const room = new Room(roomId, mockClient, userA); - redactedBeaconEvent.makeRedacted(redactionEvent, room); + redactedBeaconEvent.makeRedacted(redactionEvent); state.setStateEvents([beaconEvent]); const beaconInstance = state.beacons.get(getBeaconInfoIdentifier(beaconEvent)); diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index 481ccdcafe0..f32cf5fb6ca 100644 --- a/spec/unit/room.spec.ts +++ b/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, room); + pollStartEvent.makeRedacted(redactedEvent); await flushPromises(); diff --git a/src/models/event.ts b/src/models/event.ts index 8768c0cd906..66c410ba220 100644 --- a/src/models/event.ts +++ b/src/models/event.ts @@ -45,8 +45,6 @@ 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"; @@ -1152,19 +1150,13 @@ export class MatrixEvent extends TypedEventEmitter void; + [RoomEvent.Redaction]: (event: MatrixEvent, room: Room) => void; /** * Fires when an event that was previously redacted isn't anymore. * This happens when the redaction couldn't be sent and @@ -2114,12 +2113,6 @@ export class Room extends ReadReceipt { * 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, @@ -2336,8 +2329,7 @@ export class Room extends ReadReceipt { // 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); + redactedEvent.makeRedacted(event); // If this is in the current state, replace it with the redacted version if (redactedEvent.isState()) { @@ -2350,7 +2342,7 @@ export class Room extends ReadReceipt { } } - this.emit(RoomEvent.Redaction, event, this, threadRootId); + this.emit(RoomEvent.Redaction, event, this); // TODO: we stash user displaynames (among other things) in // RoomMember objects which are then attached to other events @@ -2503,7 +2495,7 @@ export class Room extends ReadReceipt { } if (redactedEvent) { redactedEvent.markLocallyRedacted(event); - this.emit(RoomEvent.Redaction, event, this, redactedEvent.threadRootId); + this.emit(RoomEvent.Redaction, event, this); } } } else { diff --git a/src/models/thread.ts b/src/models/thread.ts index 6b8069c9a5f..1da3fca558b 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -228,8 +228,8 @@ export class Thread extends ReadReceipt => { - if (threadRootId !== this.id) return; // ignore redactions for other timelines + private onRedaction = async (event: MatrixEvent): Promise => { + if (event.threadRootId !== this.id) return; // ignore redactions for other timelines if (this.replyCount <= 0) { for (const threadEvent of this.timeline) { this.clearEventMetadata(threadEvent);