Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test inserting edit events into the thread timeline "on demand" #3410

Merged
merged 12 commits into from
May 25, 2023
31 changes: 31 additions & 0 deletions spec/test-utils/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,37 @@ export function mkReaction(
);
}

export function mkEdit(
target: MatrixEvent,
client: MatrixClient,
userId: string,
roomId: string,
msg?: string,
ts?: number,
) {
msg = msg ?? `Edit of ${target.getId()}`;
return mkEvent(
{
event: true,
type: EventType.RoomMessage,
user: userId,
room: roomId,
content: {
"body": `* ${msg}`,
"m.new_content": {
body: msg,
},
"m.relates_to": {
rel_type: RelationType.Replace,
event_id: target.getId()!,
},
},
ts,
},
client,
);
}

/**
* A mock implementation of webstorage
*/
Expand Down
87 changes: 1 addition & 86 deletions spec/unit/event-timeline-set.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,10 @@ import {
MatrixClient,
MatrixEvent,
MatrixEventEvent,
RelationType,
Room,
RoomEvent,
} from "../../src";
import { FeatureSupport, Thread } from "../../src/models/thread";
import { Thread } from "../../src/models/thread";
import { ReEmitter } from "../../src/ReEmitter";
import { eventMapperFor } from "../../src/event-mapper";

describe("EventTimelineSet", () => {
const roomId = "!foo:bar";
Expand Down Expand Up @@ -206,88 +203,6 @@ describe("EventTimelineSet", () => {
expect(liveTimeline.getEvents().length).toStrictEqual(0);
});

it("should allow edits to be added to thread timeline", async () => {
jest.spyOn(client, "supportsThreads").mockReturnValue(true);
jest.spyOn(client, "getEventMapper").mockReturnValue(eventMapperFor(client, {}));
Thread.hasServerSideSupport = FeatureSupport.Stable;

const sender = "@alice:matrix.org";

const root = utils.mkEvent({
event: true,
content: {
body: "Thread root",
},
type: EventType.RoomMessage,
sender,
});
room.addLiveEvents([root]);

const threadReply = utils.mkEvent({
event: true,
content: {
"body": "Thread reply",
"m.relates_to": {
event_id: root.getId()!,
rel_type: RelationType.Thread,
},
},
type: EventType.RoomMessage,
sender,
});

root.setUnsigned({
"m.relations": {
[RelationType.Thread]: {
count: 1,
latest_event: {
content: threadReply.getContent(),
origin_server_ts: 5,
room_id: room.roomId,
sender,
type: EventType.RoomMessage,
event_id: threadReply.getId()!,
user_id: sender,
age: 1,
},
current_user_participated: true,
},
},
});

const editToThreadReply = utils.mkEvent({
event: true,
content: {
"body": " * edit",
"m.new_content": {
"body": "edit",
"msgtype": "m.text",
"org.matrix.msc1767.text": "edit",
},
"m.relates_to": {
event_id: threadReply.getId()!,
rel_type: RelationType.Replace,
},
},
type: EventType.RoomMessage,
sender,
});

jest.spyOn(client, "paginateEventTimeline").mockImplementation(async () => {
thread.timelineSet.getLiveTimeline().addEvent(threadReply, { toStartOfTimeline: true });
return true;
});
jest.spyOn(client, "relations").mockResolvedValue({
events: [],
});

const thread = room.createThread(root.getId()!, root, [threadReply, editToThreadReply], false);
thread.once(RoomEvent.TimelineReset, () => {
const lastEvent = thread.timeline.at(-1)!;
expect(lastEvent.getContent().body).toBe(" * edit");
});
});

describe("non-room timeline", () => {
it("Adds event to timeline", () => {
const nonRoomEventTimelineSet = new EventTimelineSet(
Expand Down
138 changes: 134 additions & 4 deletions spec/unit/models/thread.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ limitations under the License.
import { mocked } from "jest-mock";

import { MatrixClient, PendingEventOrdering } from "../../../src/client";
import { Room } from "../../../src/models/room";
import { Room, RoomEvent } from "../../../src/models/room";
import { Thread, THREAD_RELATION_TYPE, ThreadEvent, FeatureSupport } from "../../../src/models/thread";
import { mkThread } from "../../test-utils/thread";
import { makeThreadEvent, mkThread } from "../../test-utils/thread";
import { TestClient } from "../../TestClient";
import { emitPromise, mkMessage, mkReaction, mock } from "../../test-utils/test-utils";
import { Direction, EventStatus, MatrixEvent } from "../../../src";
import { emitPromise, mkEdit, mkMessage, mkReaction, mock } from "../../test-utils/test-utils";
import { Direction, EventStatus, EventType, MatrixEvent } from "../../../src";
import { ReceiptType } from "../../../src/@types/read_receipts";
import { getMockClientWithEventEmitter, mockClientMethodsUser } from "../../test-utils/client";
import { ReEmitter } from "../../../src/ReEmitter";
Expand Down Expand Up @@ -474,4 +474,134 @@ describe("Thread", () => {
return client;
}
});

describe("Editing events", () => {
describe("Given server support for threads", () => {
let previousThreadHasServerSideSupport: FeatureSupport;

beforeAll(() => {
previousThreadHasServerSideSupport = Thread.hasServerSideSupport;
Thread.hasServerSideSupport = FeatureSupport.Stable;
});

afterAll(() => {
Thread.hasServerSideSupport = previousThreadHasServerSideSupport;
});

it("Adds edits from sync to the thread timeline and applies them", async () => {
// Given a thread
const client = createClient();
const user = "@alice:matrix.org";
const room = "!room:z";
const thread = await createThread(client, user, room);

// When a message and an edit are added to the thread
const messageToEdit = createThreadMessage(thread.id, user, room, "Thread reply");
const editEvent = mkEdit(messageToEdit, client, user, room, "edit");
await thread.addEvent(messageToEdit, false);
await thread.addEvent(editEvent, false);

// 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");
});

it("Adds edits fetched on demand to the thread timeline and applies them", async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
it("Adds edits fetched on demand to the thread timeline and applies them", async () => {
it("applies edits fetched on demand", async () => {

I'd be more comfortable if this test's title were honest about what it's really testing at the moment ^^

Copy link
Member Author

Choose a reason for hiding this comment

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

Please forgive me for not doing this.. I promise I'll merge #3398 very very soon.

It was going to topple my tower of PRs :-)

// Given we don't support recursive relations
const client = createClient(new Map([[Feature.RelationsRecursion, ServerSupport.Unsupported]]));
// And we have a thread
const user = "@alice:matrix.org";
const room = "!room:z";
const thread = await createThread(client, user, room);

// When a message is added to the thread, and an edit to it is provided on demand
const messageToEdit = createThreadMessage(thread.id, user, room, "Thread reply");
// (fetchEditsWhereNeeded only applies to encrypted messages for some reason)
messageToEdit.event.type = EventType.RoomMessageEncrypted;
const editEvent = mkEdit(messageToEdit, client, user, room, "edit");
mocked(client.relations).mockImplementation(async (_roomId, eventId) => {
if (eventId === messageToEdit.getId()) {
return { events: [editEvent] };
} else {
return { events: [] };
}
});
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");
});
});
});
});

/**
* Create a message event that lives in a thread
*/
function createThreadMessage(threadId: string, user: string, room: string, msg: string): MatrixEvent {
return makeThreadEvent({
event: true,
user,
room,
msg,
rootEventId: threadId,
replyToEventId: threadId,
});
}

/**
* Create a thread and wait for it to be properly initialised (so you can safely
* add events to it and expect them to appear in the timeline.
*/
async function createThread(client: MatrixClient, user: string, roomId: string): Promise<Thread> {
const root = mkMessage({ event: true, user, room: roomId, msg: "Thread root" });
const room = new Room(roomId, client, "@roomcreator:x");

// Ensure the root is in the room timeline
root.setThreadId(root.getId());
room.addLiveEvents([root]);

// Create the thread and wait for it to be initialised
const thread = room.createThread(root.getId()!, root, [], false);
await new Promise<void>((res) => thread.once(RoomEvent.TimelineReset, () => res()));

return thread;
}

/**
* Create a MatrixClient that supports threads and has all the methods used when
* creating a thread that call out to HTTP endpoints mocked out.
*/
function createClient(canSupport = new Map()): MatrixClient {
const client = mock(MatrixClient, "MatrixClient");
client.reEmitter = mock(ReEmitter, "ReEmitter");
client.canSupport = canSupport;

jest.spyOn(client, "supportsThreads").mockReturnValue(true);
jest.spyOn(client, "getEventMapper").mockReturnValue(eventMapperFor(client, {}));

// Mock methods that call out to HTTP endpoints
jest.spyOn(client, "paginateEventTimeline").mockResolvedValue(true);
jest.spyOn(client, "relations").mockResolvedValue({ events: [] });
jest.spyOn(client, "fetchRoomEvent").mockResolvedValue({});

return client;
}
3 changes: 2 additions & 1 deletion src/models/thread.ts
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,8 @@ export class Thread extends ReadReceipt<EmittedEvents, EventHandlerMap> {
events
.filter((e) => e.isEncrypted())
.map((event: MatrixEvent) => {
if (event.isRelation()) return; // skip - relations don't get edits
// The only type of relation that gets edits is a thread message.
if (event.getThread() === undefined && event.isRelation()) return;
return this.client
.relations(this.roomId, event.getId()!, RelationType.Replace, event.getType(), {
limit: 1,
Expand Down