From 6f1a3af895eb5dc8d75758dfc8492b1569833e4d Mon Sep 17 00:00:00 2001 From: Kerry Date: Mon, 3 Apr 2023 09:32:12 +1200 Subject: [PATCH] Properly generate mentions when editing a reply with MSC3952 (#10486) * remove redundant feature_intentional_mentions settings check * tests * pass replytoevent to attachmmentions in editmessagecomposer * lint * strict fix --- .../views/rooms/EditMessageComposer.tsx | 15 +- .../views/rooms/EditMessageComposer-test.tsx | 394 +++++++++++++++++- test/test-utils/room.ts | 11 +- 3 files changed, 409 insertions(+), 11 deletions(-) diff --git a/src/components/views/rooms/EditMessageComposer.tsx b/src/components/views/rooms/EditMessageComposer.tsx index 368a29b015..16c9c66e8d 100644 --- a/src/components/views/rooms/EditMessageComposer.tsx +++ b/src/components/views/rooms/EditMessageComposer.tsx @@ -70,7 +70,7 @@ function getTextReplyFallback(mxEvent: MatrixEvent): string { } // exported for tests -export function createEditContent(model: EditorModel, editedEvent: MatrixEvent): IContent { +export function createEditContent(model: EditorModel, editedEvent: MatrixEvent, replyToEvent?: MatrixEvent): IContent { const isEmote = containsEmote(model); if (isEmote) { model = stripEmoteCommand(model); @@ -108,11 +108,7 @@ export function createEditContent(model: EditorModel, editedEvent: MatrixEvent): } // Build the mentions properties for both the content and new_content. - // - // TODO If this is a reply we need to include all the users from it. - if (SettingsStore.getValue("feature_intentional_mentions")) { - attachMentions(editedEvent.sender!.userId, contentBody, model, undefined, editedEvent.getContent()); - } + attachMentions(editedEvent.sender!.userId, contentBody, model, replyToEvent, editedEvent.getContent()); attachRelation(contentBody, { rel_type: "m.replace", event_id: editedEvent.getId() }); return contentBody; @@ -132,6 +128,7 @@ class EditMessageComposer extends React.Component(); private readonly dispatcherRef: string; + private readonly replyToEvent?: MatrixEvent; private model: EditorModel; public constructor(props: IEditMessageComposerProps, context: React.ContextType) { @@ -141,7 +138,9 @@ class EditMessageComposer extends React.Component", () => { + const userId = "@alice:server.org"; + const roomId = "!room:server.org"; + const mockClient = getMockClientWithEventEmitter({ + ...mockClientMethodsUser(userId), + getRoom: jest.fn(), + sendMessage: jest.fn(), + }); + const room = new Room(roomId, mockClient, userId); + const editedEvent = mkEvent({ type: "m.room.message", user: "@alice:test", @@ -29,6 +59,95 @@ describe("", () => { event: true, }); + const eventWithMentions = mkEvent({ + type: "m.room.message", + user: userId, + room: roomId, + content: { + "msgtype": "m.text", + "body": "hey Bob and Charlie", + "format": "org.matrix.custom.html", + "formatted_body": + 'hey Bob and Charlie', + "org.matrix.msc3952.mentions": { + user_ids: ["@bob:server.org", "@charlie:server.org"], + }, + }, + event: true, + }); + + // message composer emojipicker uses this + // which would require more irrelevant mocking + jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined); + + const defaultRoomContext = getRoomContext(room, {}); + + const getComponent = (editState: EditorStateTransfer, roomContext: IRoomState = defaultRoomContext) => + render(, { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + beforeEach(() => { + mockClient.getRoom.mockReturnValue(room); + mockClient.sendMessage.mockClear(); + + userEvent.setup(); + + DMRoomMap.makeShared(); + + jest.spyOn(Autocompleter.prototype, "getCompletions").mockResolvedValue([ + { + completions: [ + { + completion: "@dan:server.org", + completionId: "@dan:server.org", + type: "user", + suffix: " ", + component: Dan, + }, + ], + command: { + command: ["@d"], + }, + provider: new NotifProvider(room), + } as unknown as IProviderCompletions, + ]); + }); + + const editText = async (text: string, shouldClear?: boolean): Promise => { + const input = screen.getByRole("textbox"); + if (shouldClear) { + await userEvent.clear(input); + } + await userEvent.type(input, text); + }; + + it("should edit a simple message", async () => { + const editState = new EditorStateTransfer(editedEvent); + getComponent(editState); + await editText(" + edit"); + + fireEvent.click(screen.getByText("Save")); + + const expectedBody = { + ...editedEvent.getContent(), + "body": " * original message + edit", + "m.new_content": { + body: "original message + edit", + msgtype: "m.text", + }, + "m.relates_to": { + event_id: editedEvent.getId(), + rel_type: "m.replace", + }, + }; + expect(mockClient.sendMessage).toHaveBeenCalledWith(editedEvent.getRoomId()!, null, expectedBody); + }); + describe("createEditContent", () => { it("sends plaintext messages correctly", () => { const model = new EditorModel([], createPartCreator()); @@ -147,4 +266,275 @@ describe("", () => { }); }); }); + + describe("with feature_intentional_mentions enabled", () => { + const mockSettings = (mockValues: Record = {}) => { + const defaultMockValues = { + feature_intentional_mentions: true, + }; + jest.spyOn(SettingsStore, "getValue") + .mockClear() + .mockImplementation((settingName) => { + return { ...defaultMockValues, ...mockValues }[settingName]; + }); + }; + + beforeEach(() => { + mockSettings(); + }); + + describe("when message is not a reply", () => { + it("should attach an empty mentions object for a message with no mentions", async () => { + const editState = new EditorStateTransfer(editedEvent); + getComponent(editState); + const editContent = " + edit"; + await editText(editContent); + + fireEvent.click(screen.getByText("Save")); + + const messageContent = mockClient.sendMessage.mock.calls[0][2]; + + // both content.mentions and new_content.mentions are empty + expect(messageContent["org.matrix.msc3952.mentions"]).toEqual({}); + expect(messageContent["m.new_content"]["org.matrix.msc3952.mentions"]).toEqual({}); + }); + + it("should retain mentions in the original message that are not removed by the edit", async () => { + const editState = new EditorStateTransfer(eventWithMentions); + getComponent(editState); + // Remove charlie from the message + const editContent = "{backspace}{backspace}friends"; + await editText(editContent); + + fireEvent.click(screen.getByText("Save")); + + const messageContent = mockClient.sendMessage.mock.calls[0][2]; + + // no new mentions were added, so nothing in top level mentions + expect(messageContent["org.matrix.msc3952.mentions"]).toEqual({}); + // bob is still mentioned, charlie removed + expect(messageContent["m.new_content"]["org.matrix.msc3952.mentions"]).toEqual({ + user_ids: ["@bob:server.org"], + }); + }); + + it("should remove mentions that are removed by the edit", async () => { + const editState = new EditorStateTransfer(eventWithMentions); + getComponent(editState); + const editContent = "new message!"; + // clear the original message + await editText(editContent, true); + + fireEvent.click(screen.getByText("Save")); + + const messageContent = mockClient.sendMessage.mock.calls[0][2]; + + // no new mentions were added, so nothing in top level mentions + expect(messageContent["org.matrix.msc3952.mentions"]).toEqual({}); + // bob is not longer mentioned in the edited message, so empty mentions in new_content + expect(messageContent["m.new_content"]["org.matrix.msc3952.mentions"]).toEqual({}); + }); + + it("should add mentions that were added in the edit", async () => { + const editState = new EditorStateTransfer(editedEvent); + getComponent(editState); + const editContent = " and @d"; + await editText(editContent); + + // submit autocomplete for mention + await editText("{enter}"); + + fireEvent.click(screen.getByText("Save")); + + const messageContent = mockClient.sendMessage.mock.calls[0][2]; + + // new mention in the edit + expect(messageContent["org.matrix.msc3952.mentions"]).toEqual({ + user_ids: ["@dan:server.org"], + }); + expect(messageContent["m.new_content"]["org.matrix.msc3952.mentions"]).toEqual({ + user_ids: ["@dan:server.org"], + }); + }); + + it("should add and remove mentions from the edit", async () => { + const editState = new EditorStateTransfer(eventWithMentions); + getComponent(editState); + // Remove charlie from the message + await editText("{backspace}{backspace}"); + // and replace with @room + await editText("@d"); + // submit autocomplete for @dan mention + await editText("{enter}"); + + fireEvent.click(screen.getByText("Save")); + + const messageContent = mockClient.sendMessage.mock.calls[0][2]; + + // new mention in the edit + expect(messageContent["org.matrix.msc3952.mentions"]).toEqual({ + user_ids: ["@dan:server.org"], + }); + // all mentions in the edited version of the event + expect(messageContent["m.new_content"]["org.matrix.msc3952.mentions"]).toEqual({ + user_ids: ["@bob:server.org", "@dan:server.org"], + }); + }); + }); + + describe("when message is replying", () => { + const originalEvent = mkEvent({ + type: "m.room.message", + user: "@ernie:test", + room: roomId, + content: { body: "original message", msgtype: "m.text" }, + event: true, + }); + + const replyEvent = mkEvent({ + type: "m.room.message", + user: "@bert:test", + room: roomId, + content: { + "body": "reply with plain message", + "msgtype": "m.text", + "m.relates_to": { + "m.in_reply_to": { + event_id: originalEvent.getId(), + }, + }, + "org.matrix.msc3952.mentions": { + user_ids: [originalEvent.getSender()!], + }, + }, + event: true, + }); + + const replyWithMentions = mkEvent({ + type: "m.room.message", + user: "@bert:test", + room: roomId, + content: { + "body": 'reply that mentions Bob', + "msgtype": "m.text", + "m.relates_to": { + "m.in_reply_to": { + event_id: originalEvent.getId(), + }, + }, + "org.matrix.msc3952.mentions": { + user_ids: [ + // sender of event we replied to + originalEvent.getSender()!, + // mentions from this event + "@bob:server.org", + ], + }, + }, + event: true, + }); + + beforeEach(() => { + setupRoomWithEventsTimeline(room, [originalEvent, replyEvent]); + }); + + it("should retain parent event sender in mentions when editing with plain text", async () => { + const editState = new EditorStateTransfer(replyEvent); + getComponent(editState); + const editContent = " + edit"; + await editText(editContent); + + fireEvent.click(screen.getByText("Save")); + + const messageContent = mockClient.sendMessage.mock.calls[0][2]; + + // no new mentions from edit + expect(messageContent["org.matrix.msc3952.mentions"]).toEqual({}); + // edited reply still mentions the parent event sender + expect(messageContent["m.new_content"]["org.matrix.msc3952.mentions"]).toEqual({ + user_ids: [originalEvent.getSender()], + }); + }); + + it("should retain parent event sender in mentions when adding a mention", async () => { + const editState = new EditorStateTransfer(replyEvent); + getComponent(editState); + await editText(" and @d"); + // submit autocomplete for @dan mention + await editText("{enter}"); + + fireEvent.click(screen.getByText("Save")); + + const messageContent = mockClient.sendMessage.mock.calls[0][2]; + + // new mention in edit + expect(messageContent["org.matrix.msc3952.mentions"]).toEqual({ + user_ids: ["@dan:server.org"], + }); + // edited reply still mentions the parent event sender + // plus new mention @dan + expect(messageContent["m.new_content"]["org.matrix.msc3952.mentions"]).toEqual({ + user_ids: [originalEvent.getSender(), "@dan:server.org"], + }); + }); + + it("should retain parent event sender in mentions when removing all mentions from content", async () => { + const editState = new EditorStateTransfer(replyWithMentions); + getComponent(editState); + // replace text to remove all mentions + await editText("no mentions here", true); + + fireEvent.click(screen.getByText("Save")); + + const messageContent = mockClient.sendMessage.mock.calls[0][2]; + + // no mentions in edit + expect(messageContent["org.matrix.msc3952.mentions"]).toEqual({}); + // edited reply still mentions the parent event sender + // existing @bob mention removed + expect(messageContent["m.new_content"]["org.matrix.msc3952.mentions"]).toEqual({ + user_ids: [originalEvent.getSender()], + }); + }); + + it("should retain parent event sender in mentions when removing mention of said user", async () => { + const replyThatMentionsParentEventSender = mkEvent({ + type: "m.room.message", + user: "@bert:test", + room: roomId, + content: { + "body": `reply that mentions the sender of the message we replied to Ernie`, + "msgtype": "m.text", + "m.relates_to": { + "m.in_reply_to": { + event_id: originalEvent.getId(), + }, + }, + "org.matrix.msc3952.mentions": { + user_ids: [ + // sender of event we replied to + originalEvent.getSender()!, + ], + }, + }, + event: true, + }); + const editState = new EditorStateTransfer(replyThatMentionsParentEventSender); + getComponent(editState); + // replace text to remove all mentions + await editText("no mentions here", true); + + fireEvent.click(screen.getByText("Save")); + + const messageContent = mockClient.sendMessage.mock.calls[0][2]; + + // no mentions in edit + expect(messageContent["org.matrix.msc3952.mentions"]).toEqual({}); + // edited reply still mentions the parent event sender + expect(messageContent["m.new_content"]["org.matrix.msc3952.mentions"]).toEqual({ + user_ids: [originalEvent.getSender()], + }); + }); + }); + }); }); diff --git a/test/test-utils/room.ts b/test/test-utils/room.ts index af2487a604..501afb947d 100644 --- a/test/test-utils/room.ts +++ b/test/test-utils/room.ts @@ -15,7 +15,7 @@ limitations under the License. */ import { MockedObject } from "jest-mock"; -import { MatrixClient, MatrixEvent, EventType, Room } from "matrix-js-sdk/src/matrix"; +import { MatrixClient, MatrixEvent, EventType, Room, EventTimeline } from "matrix-js-sdk/src/matrix"; import { IRoomState } from "../../src/components/structures/RoomView"; import { TimelineRenderingType } from "../../src/contexts/RoomContext"; @@ -91,3 +91,12 @@ export function getRoomContext(room: Room, override: Partial): IRoom ...override, }; } + +export const setupRoomWithEventsTimeline = (room: Room, events: MatrixEvent[] = []): void => { + const timelineSet = room.getUnfilteredTimelineSet(); + const getTimelineForEventSpy = jest.spyOn(timelineSet, "getTimelineForEvent"); + const eventTimeline = { + getEvents: jest.fn().mockReturnValue(events), + } as unknown as EventTimeline; + getTimelineForEventSpy.mockReturnValue(eventTimeline); +};