From bf57fcf663a4fc8dcb7fdb34f30b76e9bfa2ee41 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 7 Nov 2024 13:30:38 +0000 Subject: [PATCH 1/3] Remove reply fallbacks as per merged MSC2781 Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/ContentMessages.ts | 4 +- .../views/rooms/MessageComposer.tsx | 8 +- .../views/rooms/SendMessageComposer.tsx | 22 +-- .../views/rooms/VoiceRecordComposerTile.tsx | 9 +- .../utils/createMessageContent.ts | 19 +- .../rooms/wysiwyg_composer/utils/message.ts | 11 +- src/utils/Reply.ts | 173 +----------------- test/unit-tests/Notifier-test.ts | 2 +- test/unit-tests/Reply-test.ts | 82 +-------- .../views/rooms/SendMessageComposer-test.tsx | 14 +- .../rooms/VoiceRecordComposerTile-test.tsx | 3 - .../utils/createMessageContent-test.ts | 28 ++- .../wysiwyg_composer/utils/message-test.ts | 27 +-- 13 files changed, 36 insertions(+), 366 deletions(-) diff --git a/src/ContentMessages.ts b/src/ContentMessages.ts index c669fa4567f..895e168f3b8 100644 --- a/src/ContentMessages.ts +++ b/src/ContentMessages.ts @@ -536,9 +536,7 @@ export default class ContentMessages { attachMentions(matrixClient.getSafeUserId(), content, null, replyToEvent); attachRelation(content, relation); if (replyToEvent) { - addReplyToMessageContent(content, replyToEvent, { - includeLegacyFallback: false, - }); + addReplyToMessageContent(content, replyToEvent); } if (SettingsStore.getValue("Performance.addSendMessageTimingMetadata")) { diff --git a/src/components/views/rooms/MessageComposer.tsx b/src/components/views/rooms/MessageComposer.tsx index 69139fae5bc..62029f46c36 100644 --- a/src/components/views/rooms/MessageComposer.tsx +++ b/src/components/views/rooms/MessageComposer.tsx @@ -414,7 +414,7 @@ export class MessageComposer extends React.Component { this.messageComposerInput.current?.sendMessage(); if (this.state.isWysiwygLabEnabled) { - const { permalinkCreator, relation, replyToEvent } = this.props; + const { relation, replyToEvent } = this.props; const composerContent = this.state.composerContent; this.setState({ composerContent: "", initialComposerContent: "" }); dis.dispatch({ @@ -424,7 +424,6 @@ export class MessageComposer extends React.Component { await sendMessage(composerContent, this.state.isRichTextEnabled, { mxClient: this.props.mxClient, roomContext: this.context, - permalinkCreator, relation, replyToEvent, }); @@ -582,7 +581,6 @@ export class MessageComposer extends React.Component { key="controls_input" room={this.props.room} placeholder={this.renderPlaceholderText()} - permalinkCreator={this.props.permalinkCreator} relation={this.props.relation} replyToEvent={this.props.replyToEvent} onChange={this.onChange} @@ -597,7 +595,6 @@ export class MessageComposer extends React.Component { key="controls_voice_record" ref={this.voiceRecordingButton} room={this.props.room} - permalinkCreator={this.props.permalinkCreator} relation={this.props.relation} replyToEvent={this.props.replyToEvent} />, @@ -642,8 +639,6 @@ export class MessageComposer extends React.Component { ); } - let recordingTooltip: JSX.Element | undefined; - const isTooltipOpen = Boolean(this.state.recordingTimeLeftSeconds); const secondsLeft = this.state.recordingTimeLeftSeconds ? Math.round(this.state.recordingTimeLeftSeconds) : 0; @@ -673,7 +668,6 @@ export class MessageComposer extends React.Component { return (
- {recordingTooltip}
void; } @@ -258,10 +250,6 @@ export class SendMessageComposer extends React.Component) { super(props, context); @@ -500,11 +488,7 @@ export class SendMessageComposer extends React.Component e instan export async function createMessageContent( message: string, isHTML: boolean, - { - relation, - replyToEvent, - permalinkCreator, - includeReplyLegacyFallback = true, - editedEvent, - }: CreateMessageContentParams, + { relation, replyToEvent, editedEvent }: CreateMessageContentParams, ): Promise { const isEditing = isMatrixEvent(editedEvent); const isReply = isEditing ? Boolean(editedEvent.replyEventId) : isMatrixEvent(replyToEvent); @@ -126,11 +118,8 @@ export async function createMessageContent( // TODO Handle editing? attachRelation(content, newRelation); - if (!isEditing && replyToEvent && permalinkCreator) { - addReplyToMessageContent(content, replyToEvent, { - permalinkCreator, - includeLegacyFallback: includeReplyLegacyFallback, - }); + if (!isEditing && replyToEvent) { + addReplyToMessageContent(content, replyToEvent); } return content; diff --git a/src/components/views/rooms/wysiwyg_composer/utils/message.ts b/src/components/views/rooms/wysiwyg_composer/utils/message.ts index d368d150e00..44e10e3cc53 100644 --- a/src/components/views/rooms/wysiwyg_composer/utils/message.ts +++ b/src/components/views/rooms/wysiwyg_composer/utils/message.ts @@ -19,7 +19,6 @@ import { RoomMessageEventContent } from "matrix-js-sdk/src/types"; import { PosthogAnalytics } from "../../../../../PosthogAnalytics"; import SettingsStore from "../../../../../settings/SettingsStore"; import { decorateStartSendingTime, sendRoundTripMetric } from "../../../../../sendTimePerformanceMetrics"; -import { RoomPermalinkCreator } from "../../../../../utils/permalinks/Permalinks"; import { doMaybeLocalRoomAction } from "../../../../../utils/local-room"; import { CHAT_EFFECTS } from "../../../../../effects"; import { containsEmoji } from "../../../../../effects/utils"; @@ -41,8 +40,6 @@ export interface SendMessageParams { relation?: IEventRelation; replyToEvent?: MatrixEvent; roomContext: IRoomState; - permalinkCreator?: RoomPermalinkCreator; - includeReplyLegacyFallback?: boolean; } export async function sendMessage( @@ -50,7 +47,7 @@ export async function sendMessage( isHTML: boolean, { roomContext, mxClient, ...params }: SendMessageParams, ): Promise { - const { relation, replyToEvent, permalinkCreator } = params; + const { relation, replyToEvent } = params; const { room } = roomContext; const roomId = room?.roomId; @@ -95,11 +92,7 @@ export async function sendMessage( ) { attachRelation(content, relation); if (replyToEvent) { - addReplyToMessageContent(content, replyToEvent, { - permalinkCreator, - // Exclude the legacy fallback for custom event types such as those used by /fireworks - includeLegacyFallback: content.msgtype?.startsWith("m.") ?? true, - }); + addReplyToMessageContent(content, replyToEvent); } } else { // instead of setting shouldSend to false as in SendMessageComposer, just return diff --git a/src/utils/Reply.ts b/src/utils/Reply.ts index e693d16a4af..a1baffa2c99 100644 --- a/src/utils/Reply.ts +++ b/src/utils/Reply.ts @@ -7,23 +7,10 @@ * Please see LICENSE files in the repository root for full details. */ -import { - IContent, - IEventRelation, - MatrixEvent, - MsgType, - THREAD_RELATION_TYPE, - M_BEACON_INFO, - M_POLL_END, - M_POLL_START, -} from "matrix-js-sdk/src/matrix"; +import { IContent, IEventRelation, MatrixEvent, THREAD_RELATION_TYPE } from "matrix-js-sdk/src/matrix"; import sanitizeHtml from "sanitize-html"; -import escapeHtml from "escape-html"; -import { PollStartEvent } from "matrix-js-sdk/src/extensible_events_v1/PollStartEvent"; import { PERMITTED_URL_SCHEMES } from "./UrlUtils"; -import { makeUserPermalink, RoomPermalinkCreator } from "./permalinks/Permalinks"; -import { isSelfLocation } from "./location"; export function getParentEventId(ev?: MatrixEvent): string | undefined { if (!ev || ev.isRedacted()) return; @@ -62,137 +49,6 @@ export function stripHTMLReply(html: string): string { }); } -// Part of Replies fallback support -export function getNestedReplyText( - ev: MatrixEvent, - permalinkCreator?: RoomPermalinkCreator, -): { body: string; html: string } | null { - if (!ev) return null; - - let { - body, - formatted_body: html, - msgtype, - } = ev.getContent<{ - body: string; - msgtype?: string; - formatted_body?: string; - }>(); - if (getParentEventId(ev)) { - if (body) body = stripPlainReply(body); - } - - if (!body) body = ""; // Always ensure we have a body, for reasons. - - if (html) { - // sanitize the HTML before we put it in an - html = stripHTMLReply(html); - } else { - // Escape the body to use as HTML below. - // We also run a nl2br over the result to fix the fallback representation. We do this - // after converting the text to safe HTML to avoid user-provided BR's from being converted. - html = escapeHtml(body).replace(/\n/g, "
"); - } - - // dev note: do not rely on `body` being safe for HTML usage below. - - const evLink = permalinkCreator?.forEvent(ev.getId()!); - const userLink = makeUserPermalink(ev.getSender()!); - const mxid = ev.getSender(); - - if (M_BEACON_INFO.matches(ev.getType())) { - const aTheir = isSelfLocation(ev.getContent()) ? "their" : "a"; - return { - html: - `
In reply to ${mxid}` + - `
shared ${aTheir} live location.
`, - body: `> <${mxid}> shared ${aTheir} live location.\n\n`, - }; - } - - if (M_POLL_START.matches(ev.getType())) { - const extensibleEvent = ev.unstableExtensibleEvent as PollStartEvent; - const question = extensibleEvent?.question?.text; - return { - html: - `
In reply to ${mxid}` + - `
Poll: ${question}
`, - body: `> <${mxid}> started poll: ${question}\n\n`, - }; - } - if (M_POLL_END.matches(ev.getType())) { - return { - html: - `
In reply to ${mxid}` + - `
Ended poll
`, - body: `> <${mxid}>Ended poll\n\n`, - }; - } - - // This fallback contains text that is explicitly EN. - switch (msgtype) { - case MsgType.Text: - case MsgType.Notice: { - html = - `
In reply to ${mxid}` + - `
${html}
`; - const lines = body.trim().split("\n"); - if (lines.length > 0) { - lines[0] = `<${mxid}> ${lines[0]}`; - body = lines.map((line) => `> ${line}`).join("\n") + "\n\n"; - } - break; - } - case MsgType.Image: - html = - `
In reply to ${mxid}` + - `
sent an image.
`; - body = `> <${mxid}> sent an image.\n\n`; - break; - case MsgType.Video: - html = - `
In reply to ${mxid}` + - `
sent a video.
`; - body = `> <${mxid}> sent a video.\n\n`; - break; - case MsgType.Audio: - html = - `
In reply to ${mxid}` + - `
sent an audio file.
`; - body = `> <${mxid}> sent an audio file.\n\n`; - break; - case MsgType.File: - html = - `
In reply to ${mxid}` + - `
sent a file.
`; - body = `> <${mxid}> sent a file.\n\n`; - break; - case MsgType.Location: { - const aTheir = isSelfLocation(ev.getContent()) ? "their" : "a"; - html = - `
In reply to ${mxid}` + - `
shared ${aTheir} location.
`; - body = `> <${mxid}> shared ${aTheir} location.\n\n`; - break; - } - case MsgType.Emote: { - html = - `
In reply to * ` + - `${mxid}
${html}
`; - const lines = body.trim().split("\n"); - if (lines.length > 0) { - lines[0] = `* <${mxid}> ${lines[0]}`; - body = lines.map((line) => `> ${line}`).join("\n") + "\n\n"; - } - break; - } - default: - return null; - } - - return { body, html }; -} - export function makeReplyMixIn(ev?: MatrixEvent): IEventRelation { if (!ev) return {}; @@ -227,34 +83,9 @@ export function shouldDisplayReply(event: MatrixEvent): boolean { return !!inReplyTo.event_id; } -interface AddReplyOpts { - permalinkCreator?: RoomPermalinkCreator; - includeLegacyFallback: false; -} - -interface IncludeLegacyFeedbackOpts { - permalinkCreator?: RoomPermalinkCreator; - includeLegacyFallback: true; -} - -export function addReplyToMessageContent( - content: IContent, - replyToEvent: MatrixEvent, - opts: AddReplyOpts | IncludeLegacyFeedbackOpts, -): void { +export function addReplyToMessageContent(content: IContent, replyToEvent: MatrixEvent): void { content["m.relates_to"] = { ...(content["m.relates_to"] || {}), ...makeReplyMixIn(replyToEvent), }; - - if (opts.includeLegacyFallback) { - // Part of Replies fallback support - prepend the text we're sending with the text we're replying to - const nestedReply = getNestedReplyText(replyToEvent, opts.permalinkCreator); - if (nestedReply) { - if (content.formatted_body) { - content.formatted_body = nestedReply.html + content.formatted_body; - } - content.body = nestedReply.body + content.body; - } - } } diff --git a/test/unit-tests/Notifier-test.ts b/test/unit-tests/Notifier-test.ts index 7bfde2afb3b..2fe7fdec0b0 100644 --- a/test/unit-tests/Notifier-test.ts +++ b/test/unit-tests/Notifier-test.ts @@ -351,7 +351,7 @@ describe("Notifier", () => { user: mockClient.getSafeUserId(), room: testRoom.roomId, }); - addReplyToMessageContent(reply.getContent(), event, { includeLegacyFallback: true }); + addReplyToMessageContent(reply.getContent(), event); Notifier.displayPopupNotification(reply, testRoom); expect(MockPlatform.displayNotification).toHaveBeenCalledWith( "@bob:example.org (!room1:server)", diff --git a/test/unit-tests/Reply-test.ts b/test/unit-tests/Reply-test.ts index 65c0d3c154d..d9e1b02a021 100644 --- a/test/unit-tests/Reply-test.ts +++ b/test/unit-tests/Reply-test.ts @@ -6,42 +6,10 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only Please see LICENSE files in the repository root for full details. */ -import { - IContent, - MatrixEvent, - MsgType, - M_BEACON_INFO, - LocationAssetType, - M_ASSET, - M_POLL_END, - Room, -} from "matrix-js-sdk/src/matrix"; +import { Room } from "matrix-js-sdk/src/matrix"; -import { - getNestedReplyText, - getParentEventId, - shouldDisplayReply, - stripHTMLReply, - stripPlainReply, -} from "../../src/utils/Reply"; -import { makePollStartEvent, mkEvent, stubClient } from "../test-utils"; -import { RoomPermalinkCreator } from "../../src/utils/permalinks/Permalinks"; - -function makeTestEvent(type: string, content: IContent): MatrixEvent { - return mkEvent({ - event: true, - type: type, - user: "@user1:server", - room: "!room1:server", - content, - }); -} - -const mockPermalinkGenerator = { - forEvent(eventId: string): string { - return "$$permalink$$"; - }, -} as RoomPermalinkCreator; +import { getParentEventId, shouldDisplayReply, stripHTMLReply, stripPlainReply } from "../../src/utils/Reply"; +import { mkEvent, stubClient } from "../test-utils"; // don't litter test console with logs jest.mock("matrix-js-sdk/src/logger"); @@ -122,50 +90,6 @@ But this is not }); }); - describe("getNestedReplyText", () => { - it("Returns valid reply fallback text for m.text msgtypes", () => { - const event = makeTestEvent(MsgType.Text, { - body: "body", - msgtype: "m.text", - }); - - expect(getNestedReplyText(event, mockPermalinkGenerator)).toMatchSnapshot(); - }); - - ( - [ - ["m.room.message", MsgType.Location, LocationAssetType.Pin], - ["m.room.message", MsgType.Location, LocationAssetType.Self], - [M_BEACON_INFO.name, undefined, LocationAssetType.Pin], - [M_BEACON_INFO.name, undefined, LocationAssetType.Self], - ] as const - ).forEach(([type, msgType, assetType]) => { - it(`should create the expected fallback text for ${assetType} ${type}/${msgType}`, () => { - const event = makeTestEvent(type, { - body: "body", - msgtype: msgType, - [M_ASSET.name]: { type: assetType }, - }); - - expect(getNestedReplyText(event, mockPermalinkGenerator)).toMatchSnapshot(); - }); - }); - - it("should create the expected fallback text for poll end events", () => { - const event = makeTestEvent(M_POLL_END.name, { - body: "body", - }); - - expect(getNestedReplyText(event, mockPermalinkGenerator)).toMatchSnapshot(); - }); - - it("should create the expected fallback text for poll start events", () => { - const event = makePollStartEvent("Will this test pass?", "@user:server.org"); - - expect(getNestedReplyText(event, mockPermalinkGenerator)).toMatchSnapshot(); - }); - }); - describe("shouldDisplayReply", () => { it("Returns false for redacted events", () => { const event = mkEvent({ diff --git a/test/unit-tests/components/views/rooms/SendMessageComposer-test.tsx b/test/unit-tests/components/views/rooms/SendMessageComposer-test.tsx index a41c221616a..1b90cba14d3 100644 --- a/test/unit-tests/components/views/rooms/SendMessageComposer-test.tsx +++ b/test/unit-tests/components/views/rooms/SendMessageComposer-test.tsx @@ -27,7 +27,6 @@ import defaultDispatcher from "../../../../../src/dispatcher/dispatcher"; import DocumentOffset from "../../../../../src/editor/offset"; import { Layout } from "../../../../../src/settings/enums/Layout"; import { IRoomState, MainSplitContentType } from "../../../../../src/components/structures/RoomView"; -import { RoomPermalinkCreator } from "../../../../../src/utils/permalinks/Permalinks"; import { mockPlatformPeg } from "../../../../test-utils/platform"; import { doMaybeLocalRoomAction } from "../../../../../src/utils/local-room"; import { addTextToComposer } from "../../../../test-utils/composer"; @@ -80,14 +79,12 @@ describe("", () => { viewRoomOpts: { buttons: [] }, }; describe("createMessageContent", () => { - const permalinkCreator = jest.fn() as any; - it("sends plaintext messages correctly", () => { const model = new EditorModel([], createPartCreator()); const documentOffset = new DocumentOffset(11, true); model.update("hello world", "insertText", documentOffset); - const content = createMessageContent("@alice:test", model, undefined, undefined, permalinkCreator); + const content = createMessageContent("@alice:test", model, undefined, undefined); expect(content).toEqual({ "body": "hello world", @@ -101,7 +98,7 @@ describe("", () => { const documentOffset = new DocumentOffset(13, true); model.update("hello *world*", "insertText", documentOffset); - const content = createMessageContent("@alice:test", model, undefined, undefined, permalinkCreator); + const content = createMessageContent("@alice:test", model, undefined, undefined); expect(content).toEqual({ "body": "hello *world*", @@ -117,7 +114,7 @@ describe("", () => { const documentOffset = new DocumentOffset(22, true); model.update("/me blinks __quickly__", "insertText", documentOffset); - const content = createMessageContent("@alice:test", model, undefined, undefined, permalinkCreator); + const content = createMessageContent("@alice:test", model, undefined, undefined); expect(content).toEqual({ "body": "blinks __quickly__", @@ -134,7 +131,7 @@ describe("", () => { model.update("/me ✨sparkles✨", "insertText", documentOffset); expect(model.parts.length).toEqual(4); // Emoji count as non-text - const content = createMessageContent("@alice:test", model, undefined, undefined, permalinkCreator); + const content = createMessageContent("@alice:test", model, undefined, undefined); expect(content).toEqual({ "body": "✨sparkles✨", @@ -149,7 +146,7 @@ describe("", () => { model.update("//dev/null is my favourite place", "insertText", documentOffset); - const content = createMessageContent("@alice:test", model, undefined, undefined, permalinkCreator); + const content = createMessageContent("@alice:test", model, undefined, undefined); expect(content).toEqual({ "body": "/dev/null is my favourite place", @@ -364,7 +361,6 @@ describe("", () => { const defaultProps = { room: mockRoom, toggleStickerPickerOpen: jest.fn(), - permalinkCreator: new RoomPermalinkCreator(mockRoom), }; const getRawComponent = (props = {}, roomContext = defaultRoomContext, client = mockClient) => ( diff --git a/test/unit-tests/components/views/rooms/VoiceRecordComposerTile-test.tsx b/test/unit-tests/components/views/rooms/VoiceRecordComposerTile-test.tsx index 31bec798791..a8d4b022a6b 100644 --- a/test/unit-tests/components/views/rooms/VoiceRecordComposerTile-test.tsx +++ b/test/unit-tests/components/views/rooms/VoiceRecordComposerTile-test.tsx @@ -15,7 +15,6 @@ import VoiceRecordComposerTile from "../../../../../src/components/views/rooms/V import { doMaybeLocalRoomAction } from "../../../../../src/utils/local-room"; import { MatrixClientPeg } from "../../../../../src/MatrixClientPeg"; import { IUpload, VoiceMessageRecording } from "../../../../../src/audio/VoiceMessageRecording"; -import { RoomPermalinkCreator } from "../../../../../src/utils/permalinks/Permalinks"; import { VoiceRecordingStore } from "../../../../../src/stores/VoiceRecordingStore"; import { PlaybackClock } from "../../../../../src/audio/PlaybackClock"; import { mkEvent } from "../../../../test-utils"; @@ -57,7 +56,6 @@ describe("", () => { const props = { room, ref: voiceRecordComposerTile, - permalinkCreator: new RoomPermalinkCreator(room), }; mockUpload = { mxc: "mxc://example.com/voice", @@ -142,7 +140,6 @@ describe("", () => { const props = { room, ref: voiceRecordComposerTile, - permalinkCreator: new RoomPermalinkCreator(room), replyToEvent, }; render(); diff --git a/test/unit-tests/components/views/rooms/wysiwyg_composer/utils/createMessageContent-test.ts b/test/unit-tests/components/views/rooms/wysiwyg_composer/utils/createMessageContent-test.ts index eae10f2e3da..d20969a222f 100644 --- a/test/unit-tests/components/views/rooms/wysiwyg_composer/utils/createMessageContent-test.ts +++ b/test/unit-tests/components/views/rooms/wysiwyg_composer/utils/createMessageContent-test.ts @@ -8,18 +8,12 @@ Please see LICENSE files in the repository root for full details. import { MsgType } from "matrix-js-sdk/src/matrix"; import { filterConsole, mkEvent } from "../../../../../../test-utils"; -import { RoomPermalinkCreator } from "../../../../../../../src/utils/permalinks/Permalinks"; import { createMessageContent, EMOTE_PREFIX, } from "../../../../../../../src/components/views/rooms/wysiwyg_composer/utils/createMessageContent"; describe("createMessageContent", () => { - const permalinkCreator = { - forEvent(eventId: string): string { - return "$$permalink$$"; - }, - } as RoomPermalinkCreator; const message = "hello world"; const mockEvent = mkEvent({ type: "m.room.message", @@ -42,12 +36,12 @@ describe("createMessageContent", () => { // Warm up by creating the component once, with a long timeout. // This prevents tests timing out because of the time spent loading // the WASM component. - await createMessageContent(message, true, { permalinkCreator }); + await createMessageContent(message, true, {}); }, 10000); it("Should create html message", async () => { // When - const content = await createMessageContent(message, true, { permalinkCreator }); + const content = await createMessageContent(message, true, {}); // Then expect(content).toEqual({ @@ -60,7 +54,7 @@ describe("createMessageContent", () => { it("Should add reply to message content", async () => { // When - const content = await createMessageContent(message, true, { permalinkCreator, replyToEvent: mockEvent }); + const content = await createMessageContent(message, true, { replyToEvent: mockEvent }); // Then expect(content).toEqual({ @@ -85,7 +79,7 @@ describe("createMessageContent", () => { rel_type: "m.thread", event_id: "myFakeThreadId", }; - const content = await createMessageContent(message, true, { permalinkCreator, relation }); + const content = await createMessageContent(message, true, { relation }); // Then expect(content).toEqual({ @@ -118,7 +112,7 @@ describe("createMessageContent", () => { }, event: true, }); - const content = await createMessageContent(message, true, { permalinkCreator, editedEvent }); + const content = await createMessageContent(message, true, { editedEvent }); // Then expect(content).toEqual({ @@ -141,20 +135,20 @@ describe("createMessageContent", () => { it("Should strip the /me prefix from a message", async () => { const textBody = "some body text"; - const content = await createMessageContent(EMOTE_PREFIX + textBody, true, { permalinkCreator }); + const content = await createMessageContent(EMOTE_PREFIX + textBody, true, {}); expect(content).toMatchObject({ body: textBody, formatted_body: textBody }); }); it("Should strip single / from message prefixed with //", async () => { - const content = await createMessageContent("//twoSlashes", true, { permalinkCreator }); + const content = await createMessageContent("//twoSlashes", true, {}); expect(content).toMatchObject({ body: "/twoSlashes", formatted_body: "/twoSlashes" }); }); it("Should set the content type to MsgType.Emote when /me prefix is used", async () => { const textBody = "some body text"; - const content = await createMessageContent(EMOTE_PREFIX + textBody, true, { permalinkCreator }); + const content = await createMessageContent(EMOTE_PREFIX + textBody, true, {}); expect(content).toMatchObject({ msgtype: MsgType.Emote }); }); @@ -164,14 +158,14 @@ describe("createMessageContent", () => { it("Should replace at-room mentions with `@room` in body", async () => { const messageComposerState = `@room `; - const content = await createMessageContent(messageComposerState, false, { permalinkCreator }); + const content = await createMessageContent(messageComposerState, false, {}); expect(content).toMatchObject({ body: "@room " }); }); it("Should replace user mentions with user name in body", async () => { const messageComposerState = `a test user `; - const content = await createMessageContent(messageComposerState, false, { permalinkCreator }); + const content = await createMessageContent(messageComposerState, false, {}); expect(content).toMatchObject({ body: "a test user " }); }); @@ -179,7 +173,7 @@ describe("createMessageContent", () => { it("Should replace room mentions with room mxid in body", async () => { const messageComposerState = `a test room `; - const content = await createMessageContent(messageComposerState, false, { permalinkCreator }); + const content = await createMessageContent(messageComposerState, false, {}); expect(content).toMatchObject({ body: "#test_room:element.io ", diff --git a/test/unit-tests/components/views/rooms/wysiwyg_composer/utils/message-test.ts b/test/unit-tests/components/views/rooms/wysiwyg_composer/utils/message-test.ts index 7c77912206d..06218a55e89 100644 --- a/test/unit-tests/components/views/rooms/wysiwyg_composer/utils/message-test.ts +++ b/test/unit-tests/components/views/rooms/wysiwyg_composer/utils/message-test.ts @@ -17,7 +17,6 @@ import { createTestClient, getRoomContext, mkEvent, mkStubRoom } from "../../../ import defaultDispatcher from "../../../../../../../src/dispatcher/dispatcher"; import SettingsStore from "../../../../../../../src/settings/SettingsStore"; import { SettingLevel } from "../../../../../../../src/settings/SettingLevel"; -import { RoomPermalinkCreator } from "../../../../../../../src/utils/permalinks/Permalinks"; import EditorStateTransfer from "../../../../../../../src/utils/EditorStateTransfer"; import * as ConfirmRedactDialog from "../../../../../../../src/components/views/dialogs/ConfirmRedactDialog"; import * as SlashCommands from "../../../../../../../src/SlashCommands"; @@ -27,11 +26,6 @@ import { MatrixClientPeg } from "../../../../../../../src/MatrixClientPeg"; import { Action } from "../../../../../../../src/dispatcher/actions"; describe("message", () => { - const permalinkCreator = { - forEvent(eventId: string): string { - return "$$permalink$$"; - }, - } as RoomPermalinkCreator; const message = "hello world"; const mockEvent = mkEvent({ type: "m.room.message", @@ -71,7 +65,7 @@ describe("message", () => { describe("sendMessage", () => { it("Should not send empty html message", async () => { // When - await sendMessage("", true, { roomContext: defaultRoomContext, mxClient: mockClient, permalinkCreator }); + await sendMessage("", true, { roomContext: defaultRoomContext, mxClient: mockClient }); // Then expect(mockClient.sendMessage).toHaveBeenCalledTimes(0); @@ -86,7 +80,6 @@ describe("message", () => { await sendMessage(message, true, { roomContext: mockRoomContextWithoutId, mxClient: mockClient, - permalinkCreator, }); // Then @@ -100,7 +93,6 @@ describe("message", () => { await sendMessage(message, true, { roomContext: defaultRoomContext, mxClient: mockClient, - permalinkCreator, }); // Then @@ -111,7 +103,6 @@ describe("message", () => { await sendMessage(message, true, { roomContext: defaultRoomContext, mxClient: mockClient, - permalinkCreator, relation: {}, }); @@ -123,7 +114,6 @@ describe("message", () => { await sendMessage(message, true, { roomContext: defaultRoomContext, mxClient: mockClient, - permalinkCreator, relation: { event_id: "valid_id", rel_type: "m.does_not_match", @@ -139,7 +129,6 @@ describe("message", () => { await sendMessage(message, true, { roomContext: defaultRoomContext, mxClient: mockClient, - permalinkCreator, relation: { event_id: "valid_id", rel_type: "m.thread", @@ -156,7 +145,6 @@ describe("message", () => { await sendMessage(message, true, { roomContext: defaultRoomContext, mxClient: mockClient, - permalinkCreator, }); // Then @@ -183,7 +171,6 @@ describe("message", () => { await sendMessage(message, true, { roomContext: defaultRoomContext, mxClient: mockClient, - permalinkCreator, replyToEvent: mockReplyEvent, }); @@ -217,7 +204,6 @@ describe("message", () => { await sendMessage(message, true, { roomContext: defaultRoomContext, mxClient: mockClient, - permalinkCreator, }); // Then @@ -229,7 +215,7 @@ describe("message", () => { it("Should handle emojis", async () => { // When - await sendMessage("🎉", false, { roomContext: defaultRoomContext, mxClient: mockClient, permalinkCreator }); + await sendMessage("🎉", false, { roomContext: defaultRoomContext, mxClient: mockClient }); // Then expect(spyDispatcher).toHaveBeenCalledWith({ action: "effects.confetti" }); @@ -244,7 +230,6 @@ describe("message", () => { await sendMessage(validCommand, true, { roomContext: defaultRoomContext, mxClient: mockClient, - permalinkCreator, }); // Then @@ -257,7 +242,6 @@ describe("message", () => { await sendMessage(invalidPrefixCommand, true, { roomContext: defaultRoomContext, mxClient: mockClient, - permalinkCreator, }); // Then @@ -275,7 +259,6 @@ describe("message", () => { const result = await sendMessage(validCommand, true, { roomContext: defaultRoomContext, mxClient: mockClient, - permalinkCreator, }); // Then @@ -290,7 +273,6 @@ describe("message", () => { await sendMessage(inputText, true, { roomContext: defaultRoomContext, mxClient: mockClient, - permalinkCreator, }); expect(mockClient.sendMessage).toHaveBeenCalledWith( "myfakeroom", @@ -309,7 +291,6 @@ describe("message", () => { await sendMessage(inputText, true, { roomContext: defaultRoomContext, mxClient: mockClient, - permalinkCreator, relation: mockRelation, }); @@ -326,7 +307,6 @@ describe("message", () => { await sendMessage("input", true, { roomContext: defaultRoomContext, mxClient: mockClient, - permalinkCreator, replyToEvent: mockEvent, }); @@ -341,7 +321,6 @@ describe("message", () => { const result = await sendMessage(input, true, { roomContext: defaultRoomContext, mxClient: mockClient, - permalinkCreator, replyToEvent: mockEvent, }); @@ -357,7 +336,6 @@ describe("message", () => { await sendMessage(invalidCommandInput, true, { roomContext: defaultRoomContext, mxClient: mockClient, - permalinkCreator, }); // we expect the message to have been sent @@ -378,7 +356,6 @@ describe("message", () => { const result = await sendMessage(invalidCommandInput, true, { roomContext: defaultRoomContext, mxClient: mockClient, - permalinkCreator, }); expect(result).toBeUndefined(); From 0258ff1bb582cb60867ea70230126079265de272 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 7 Nov 2024 13:44:16 +0000 Subject: [PATCH 2/3] Update tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .../__snapshots__/Reply-test.ts.snap | 64 ------------------- .../utils/createMessageContent-test.ts | 28 -------- .../wysiwyg_composer/utils/message-test.ts | 7 +- 3 files changed, 2 insertions(+), 97 deletions(-) delete mode 100644 test/unit-tests/__snapshots__/Reply-test.ts.snap diff --git a/test/unit-tests/__snapshots__/Reply-test.ts.snap b/test/unit-tests/__snapshots__/Reply-test.ts.snap deleted file mode 100644 index 242366574da..00000000000 --- a/test/unit-tests/__snapshots__/Reply-test.ts.snap +++ /dev/null @@ -1,64 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Reply getNestedReplyText Returns valid reply fallback text for m.text msgtypes 1`] = ` -{ - "body": "> <@user1:server> body - -", - "html": "
In reply to @user1:server
body
", -} -`; - -exports[`Reply getNestedReplyText should create the expected fallback text for m.pin m.room.message/m.location 1`] = ` -{ - "body": "> <@user1:server> shared a location. - -", - "html": "
In reply to @user1:server
shared a location.
", -} -`; - -exports[`Reply getNestedReplyText should create the expected fallback text for m.pin org.matrix.msc3672.beacon_info/undefined 1`] = ` -{ - "body": "> <@user1:server> shared a live location. - -", - "html": "
In reply to @user1:server
shared a live location.
", -} -`; - -exports[`Reply getNestedReplyText should create the expected fallback text for m.self m.room.message/m.location 1`] = ` -{ - "body": "> <@user1:server> shared their location. - -", - "html": "
In reply to @user1:server
shared their location.
", -} -`; - -exports[`Reply getNestedReplyText should create the expected fallback text for m.self org.matrix.msc3672.beacon_info/undefined 1`] = ` -{ - "body": "> <@user1:server> shared their live location. - -", - "html": "
In reply to @user1:server
shared their live location.
", -} -`; - -exports[`Reply getNestedReplyText should create the expected fallback text for poll end events 1`] = ` -{ - "body": "> <@user1:server>Ended poll - -", - "html": "
In reply to @user1:server
Ended poll
", -} -`; - -exports[`Reply getNestedReplyText should create the expected fallback text for poll start events 1`] = ` -{ - "body": "> <@user:server.org> started poll: Will this test pass? - -", - "html": "
In reply to @user:server.org
Poll: Will this test pass?
", -} -`; diff --git a/test/unit-tests/components/views/rooms/wysiwyg_composer/utils/createMessageContent-test.ts b/test/unit-tests/components/views/rooms/wysiwyg_composer/utils/createMessageContent-test.ts index d20969a222f..f0d9ed73c16 100644 --- a/test/unit-tests/components/views/rooms/wysiwyg_composer/utils/createMessageContent-test.ts +++ b/test/unit-tests/components/views/rooms/wysiwyg_composer/utils/createMessageContent-test.ts @@ -15,13 +15,6 @@ import { describe("createMessageContent", () => { const message = "hello world"; - const mockEvent = mkEvent({ - type: "m.room.message", - room: "myfakeroom", - user: "myfakeuser", - content: { msgtype: "m.text", body: "Replying to this" }, - event: true, - }); afterEach(() => { jest.resetAllMocks(); @@ -52,27 +45,6 @@ describe("createMessageContent", () => { }); }); - it("Should add reply to message content", async () => { - // When - const content = await createMessageContent(message, true, { replyToEvent: mockEvent }); - - // Then - expect(content).toEqual({ - "body": "> Replying to this\n\n*__hello__ world*", - "format": "org.matrix.custom.html", - "formatted_body": - '
In reply to' + - ' myfakeuser' + - "
Replying to this
hello world", - "msgtype": "m.text", - "m.relates_to": { - "m.in_reply_to": { - event_id: mockEvent.getId(), - }, - }, - }); - }); - it("Should add relation to message", async () => { // When const relation = { diff --git a/test/unit-tests/components/views/rooms/wysiwyg_composer/utils/message-test.ts b/test/unit-tests/components/views/rooms/wysiwyg_composer/utils/message-test.ts index 06218a55e89..c339978f92c 100644 --- a/test/unit-tests/components/views/rooms/wysiwyg_composer/utils/message-test.ts +++ b/test/unit-tests/components/views/rooms/wysiwyg_composer/utils/message-test.ts @@ -182,12 +182,9 @@ describe("message", () => { }); const expectedContent = { - "body": "> My reply\n\n*__hello__ world*", + "body": "*__hello__ world*", "format": "org.matrix.custom.html", - "formatted_body": - '
In reply to' + - ' myfakeuser2' + - "
My reply
hello world", + "formatted_body": "hello world", "msgtype": "m.text", "m.relates_to": { "m.in_reply_to": { From d507e4cef9cfc4cdc4b9cf6112320bb2ff3dfea1 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 7 Nov 2024 16:05:59 +0000 Subject: [PATCH 3/3] Improve coverage Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .../views/rooms/SendMessageComposer-test.tsx | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/test/unit-tests/components/views/rooms/SendMessageComposer-test.tsx b/test/unit-tests/components/views/rooms/SendMessageComposer-test.tsx index 1b90cba14d3..e423d03ea9f 100644 --- a/test/unit-tests/components/views/rooms/SendMessageComposer-test.tsx +++ b/test/unit-tests/components/views/rooms/SendMessageComposer-test.tsx @@ -478,6 +478,44 @@ describe("", () => { }); }); + it("correctly sends a reply using a slash command", async () => { + stubClient(); + mocked(doMaybeLocalRoomAction).mockImplementation( + (roomId: string, fn: (actualRoomId: string) => Promise, _client?: MatrixClient) => { + return fn(roomId); + }, + ); + + const replyToEvent = mkEvent({ + type: "m.room.message", + user: "@bob:test", + room: "!abc:test", + content: { "m.mentions": {} }, + event: true, + }); + + mockPlatformPeg({ overrideBrowserShortcuts: jest.fn().mockReturnValue(false) }); + const { container } = getComponent({ replyToEvent }); + + addTextToComposer(container, "/tableflip"); + fireEvent.keyDown(container.querySelector(".mx_SendMessageComposer")!, { key: "Enter" }); + + await waitFor(() => + expect(mockClient.sendMessage).toHaveBeenCalledWith("myfakeroom", null, { + "body": "(╯°□°)╯︵ ┻━┻", + "msgtype": MsgType.Text, + "m.mentions": { + user_ids: ["@bob:test"], + }, + "m.relates_to": { + "m.in_reply_to": { + event_id: replyToEvent.getId(), + }, + }, + }), + ); + }); + it("shows chat effects on message sending", () => { mocked(doMaybeLocalRoomAction).mockImplementation( (roomId: string, fn: (actualRoomId: string) => Promise, _client?: MatrixClient) => {