From 3fa6f8cbf0539bb051e241b185d83784638ce005 Mon Sep 17 00:00:00 2001 From: alunturner <56027671+alunturner@users.noreply.github.com> Date: Mon, 10 Apr 2023 13:47:42 +0100 Subject: [PATCH] Handle command completions in RTE (#10521) * pass handleCommand prop down and use it in WysiwygAutocomplete * allow a command to generate a query from buildQuery * port command functionality into the sendMessage util * tidy up comments * remove use of shouldSend and update comments * remove console log * make logic more explicit and amend comment * uncomment replyToEvent block * update util test * remove commented out test * use local text over import from current composer * expand tests * expand tests * handle the FocusAComposer action for the wysiwyg composer * remove TODO comment * remove TODO * test for action dispatch * fix failing tests * tidy up tests * fix TS error and improve typing * fix TS error * amend return types for sendMessage, editMessage * fix null content TS error * fix another null content TS error * use as to correct final TS error * remove undefined argument * try to fix TS errors for editMessage function usage * tidy up * add TODO * improve comments * update comment --- .../DynamicImportWysiwygComposer.tsx | 2 +- .../components/WysiwygAutocomplete.tsx | 20 ++- .../components/WysiwygComposer.tsx | 9 +- .../wysiwyg_composer/hooks/useEditing.ts | 13 +- .../hooks/useWysiwygSendActionHandler.ts | 1 + .../wysiwyg_composer/utils/autocomplete.ts | 3 +- .../rooms/wysiwyg_composer/utils/message.ts | 63 ++++++- .../components/WysiwygAutocomplete-test.tsx | 13 +- .../utils/autocomplete-test.ts | 9 +- .../wysiwyg_composer/utils/message-test.ts | 166 +++++++++++++++++- 10 files changed, 268 insertions(+), 31 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/DynamicImportWysiwygComposer.tsx b/src/components/views/rooms/wysiwyg_composer/DynamicImportWysiwygComposer.tsx index 9df04f90f3..afe8396bbd 100644 --- a/src/components/views/rooms/wysiwyg_composer/DynamicImportWysiwygComposer.tsx +++ b/src/components/views/rooms/wysiwyg_composer/DynamicImportWysiwygComposer.tsx @@ -28,7 +28,7 @@ export const dynamicImportSendMessage = async ( message: string, isHTML: boolean, params: SendMessageParams, -): Promise => { +): Promise => { const { sendMessage } = await import("./utils/message"); return sendMessage(message, isHTML, params); diff --git a/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx b/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx index 3afa409c71..709de5fbbc 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete.tsx @@ -35,6 +35,12 @@ interface WysiwygAutocompleteProps { * a mention in the autocomplete list or pressing enter on a selected item */ handleMention: FormattingFunctions["mention"]; + + /** + * This handler will be called with the display text for a command on clicking + * a command in the autocomplete list or pressing enter on a selected item + */ + handleCommand: FormattingFunctions["command"]; } /** @@ -45,13 +51,23 @@ interface WysiwygAutocompleteProps { * @param props.ref - the ref will be attached to the rendered `` component */ const WysiwygAutocomplete = forwardRef( - ({ suggestion, handleMention }: WysiwygAutocompleteProps, ref: ForwardedRef): JSX.Element | null => { + ( + { suggestion, handleMention, handleCommand }: WysiwygAutocompleteProps, + ref: ForwardedRef, + ): JSX.Element | null => { const { room } = useRoomContext(); const client = useMatrixClientContext(); function handleConfirm(completion: ICompletion): void { // TODO handle all of the completion types // Using this to pick out the ones we can handle during implementation + if (completion.type === "command") { + // TODO determine if utils in SlashCommands.tsx are required + + // trim the completion as some include trailing spaces, but we always insert a + // trailing space in the rust model anyway + handleCommand(completion.completion.trim()); + } if (client && room && completion.href && (completion.type === "room" || completion.type === "user")) { handleMention( completion.href, @@ -61,6 +77,8 @@ const WysiwygAutocomplete = forwardRef( } } + // TODO - determine if we show all of the /command suggestions, there are some options in the + // list which don't seem to make sense in this context, specifically /html and /plain return room ? (
mention.addEventListener("click", handleClick)); } @@ -108,7 +108,12 @@ export const WysiwygComposer = memo(function WysiwygComposer({ onFocus={onFocus} onBlur={onFocus} > - + ; + editMessage(): Promise; endEditing(): void; } { const roomContext = useRoomContext(); @@ -45,11 +45,12 @@ export function useEditing( [initialContent], ); - const editMessageMemoized = useCallback( - () => - !!mxClient && content !== undefined && editMessage(content, { roomContext, mxClient, editorStateTransfer }), - [content, roomContext, mxClient, editorStateTransfer], - ); + const editMessageMemoized = useCallback(async () => { + if (mxClient === undefined || content === undefined) { + return; + } + return editMessage(content, { roomContext, mxClient, editorStateTransfer }); + }, [content, roomContext, mxClient, editorStateTransfer]); const endEditingMemoized = useCallback(() => endEditing(roomContext), [roomContext]); return { onChange, editMessage: editMessageMemoized, endEditing: endEditingMemoized, isSaveDisabled }; diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useWysiwygSendActionHandler.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useWysiwygSendActionHandler.ts index 40b7e8182c..50a229b637 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useWysiwygSendActionHandler.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useWysiwygSendActionHandler.ts @@ -46,6 +46,7 @@ export function useWysiwygSendActionHandler( switch (payload.action) { case "reply_to_event": + case Action.FocusAComposer: case Action.FocusSendMessageComposer: focusComposer(composerElement, context, roomContext, timeoutId); break; diff --git a/src/components/views/rooms/wysiwyg_composer/utils/autocomplete.ts b/src/components/views/rooms/wysiwyg_composer/utils/autocomplete.ts index d1f066a7bc..86dd2791a7 100644 --- a/src/components/views/rooms/wysiwyg_composer/utils/autocomplete.ts +++ b/src/components/views/rooms/wysiwyg_composer/utils/autocomplete.ts @@ -29,9 +29,8 @@ import * as Avatar from "../../../../../Avatar"; * with @ for a user query, # for a room or space query */ export function buildQuery(suggestion: MappedSuggestion | null): string { - if (!suggestion || !suggestion.keyChar || suggestion.type === "command") { + if (!suggestion || !suggestion.keyChar) { // if we have an empty key character, we do not build a query - // TODO implement the command functionality return ""; } diff --git a/src/components/views/rooms/wysiwyg_composer/utils/message.ts b/src/components/views/rooms/wysiwyg_composer/utils/message.ts index 70b65e392e..9753ebae49 100644 --- a/src/components/views/rooms/wysiwyg_composer/utils/message.ts +++ b/src/components/views/rooms/wysiwyg_composer/utils/message.ts @@ -15,7 +15,7 @@ limitations under the License. */ import { Composer as ComposerEvent } from "@matrix-org/analytics-events/types/typescript/Composer"; -import { IEventRelation, MatrixEvent } from "matrix-js-sdk/src/models/event"; +import { IContent, IEventRelation, MatrixEvent } from "matrix-js-sdk/src/models/event"; import { ISendEventResponse, MatrixClient } from "matrix-js-sdk/src/matrix"; import { THREAD_RELATION_TYPE } from "matrix-js-sdk/src/models/thread"; @@ -33,6 +33,11 @@ import { endEditing, cancelPreviousPendingEdit } from "./editing"; import EditorStateTransfer from "../../../../../utils/EditorStateTransfer"; import { createMessageContent } from "./createMessageContent"; import { isContentModified } from "./isContentModified"; +import { CommandCategories, getCommand } from "../../../../../SlashCommands"; +import { runSlashCommand, shouldSendAnyway } from "../../../../../editor/commands"; +import { Action } from "../../../../../dispatcher/actions"; +import { addReplyToMessageContent } from "../../../../../utils/Reply"; +import { attachRelation } from "../../SendMessageComposer"; export interface SendMessageParams { mxClient: MatrixClient; @@ -47,8 +52,8 @@ export async function sendMessage( message: string, isHTML: boolean, { roomContext, mxClient, ...params }: SendMessageParams, -): Promise { - const { relation, replyToEvent } = params; +): Promise { + const { relation, replyToEvent, permalinkCreator } = params; const { room } = roomContext; const roomId = room?.roomId; @@ -71,9 +76,51 @@ export async function sendMessage( }*/ PosthogAnalytics.instance.trackEvent(posthogEvent); - const content = await createMessageContent(message, isHTML, params); + let content: IContent | null = null; - // TODO slash comment + // Functionality here approximates what can be found in SendMessageComposer.sendMessage() + if (message.startsWith("/") && !message.startsWith("//")) { + const { cmd, args } = getCommand(message); + if (cmd) { + // TODO handle /me special case separately, see end of SlashCommands.Commands + const threadId = relation?.rel_type === THREAD_RELATION_TYPE.name ? relation?.event_id : null; + let commandSuccessful: boolean; + [content, commandSuccessful] = await runSlashCommand(cmd, args, roomId, threadId ?? null); + + if (!commandSuccessful) { + return; // errored + } + + if ( + content && + (cmd.category === CommandCategories.messages || cmd.category === CommandCategories.effects) + ) { + 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, + }); + } + } else { + // instead of setting shouldSend to false as in SendMessageComposer, just return + return; + } + } else { + const sendAnyway = await shouldSendAnyway(message); + // re-focus the composer after QuestionDialog is closed + dis.dispatch({ + action: Action.FocusAComposer, + context: roomContext.timelineRenderingType, + }); + // if !sendAnyway bail to let the user edit the composer and try again + if (!sendAnyway) return; + } + } + + // if content is null, we haven't done any slash command processing, so generate some content + content ??= await createMessageContent(message, isHTML, params); // TODO replace emotion end of message ? @@ -92,7 +139,7 @@ export async function sendMessage( const prom = doMaybeLocalRoomAction( roomId, - (actualRoomId: string) => mxClient.sendMessage(actualRoomId, threadId, content), + (actualRoomId: string) => mxClient.sendMessage(actualRoomId, threadId, content as IContent), mxClient, ); @@ -108,7 +155,7 @@ export async function sendMessage( dis.dispatch({ action: "message_sent" }); CHAT_EFFECTS.forEach((effect) => { - if (containsEmoji(content, effect.emojis)) { + if (content && containsEmoji(content, effect.emojis)) { // For initial threads launch, chat effects are disabled // see #19731 const isNotThread = relation?.rel_type !== THREAD_RELATION_TYPE.name; @@ -146,7 +193,7 @@ interface EditMessageParams { export async function editMessage( html: string, { roomContext, mxClient, editorStateTransfer }: EditMessageParams, -): Promise { +): Promise { const editedEvent = editorStateTransfer.getEvent(); PosthogAnalytics.instance.trackEvent({ diff --git a/test/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete-test.tsx b/test/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete-test.tsx index e4de34c269..b666ff1ff9 100644 --- a/test/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/components/WysiwygAutocomplete-test.tsx @@ -69,8 +69,9 @@ describe("WysiwygAutocomplete", () => { }, ]); const mockHandleMention = jest.fn(); + const mockHandleCommand = jest.fn(); - const renderComponent = (props = {}) => { + const renderComponent = (props: Partial> = {}) => { const mockClient = stubClient(); const mockRoom = mkStubRoom("test_room", "test_room", mockClient); const mockRoomContext = getRoomContext(mockRoom, {}); @@ -82,6 +83,7 @@ describe("WysiwygAutocomplete", () => { ref={autocompleteRef} suggestion={null} handleMention={mockHandleMention} + handleCommand={mockHandleCommand} {...props} /> @@ -90,7 +92,14 @@ describe("WysiwygAutocomplete", () => { }; it("does not show the autocomplete when room is undefined", () => { - render(); + render( + , + ); expect(screen.queryByTestId("autocomplete-wrapper")).not.toBeInTheDocument(); }); diff --git a/test/components/views/rooms/wysiwyg_composer/utils/autocomplete-test.ts b/test/components/views/rooms/wysiwyg_composer/utils/autocomplete-test.ts index 2612f037f2..dc89caba65 100644 --- a/test/components/views/rooms/wysiwyg_composer/utils/autocomplete-test.ts +++ b/test/components/views/rooms/wysiwyg_composer/utils/autocomplete-test.ts @@ -53,15 +53,12 @@ describe("buildQuery", () => { expect(buildQuery(noKeyCharSuggestion)).toBe(""); }); - it("returns an empty string when suggestion is a command", () => { - // TODO alter this test when commands are implemented - const commandSuggestion = { keyChar: "/" as const, text: "slash", type: "command" as const }; - expect(buildQuery(commandSuggestion)).toBe(""); - }); - it("combines the keyChar and text of the suggestion in the query", () => { const handledSuggestion = { keyChar: "@" as const, text: "alice", type: "mention" as const }; expect(buildQuery(handledSuggestion)).toBe("@alice"); + + const handledCommand = { keyChar: "/" as const, text: "spoiler", type: "mention" as const }; + expect(buildQuery(handledCommand)).toBe("/spoiler"); }); }); diff --git a/test/components/views/rooms/wysiwyg_composer/utils/message-test.ts b/test/components/views/rooms/wysiwyg_composer/utils/message-test.ts index 49442127d0..58fc6b7184 100644 --- a/test/components/views/rooms/wysiwyg_composer/utils/message-test.ts +++ b/test/components/views/rooms/wysiwyg_composer/utils/message-test.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { EventStatus } from "matrix-js-sdk/src/matrix"; +import { EventStatus, IEventRelation } from "matrix-js-sdk/src/matrix"; import { IRoomState } from "../../../../../../src/components/structures/RoomView"; import { editMessage, sendMessage } from "../../../../../../src/components/views/rooms/wysiwyg_composer/utils/message"; @@ -25,6 +25,11 @@ 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"; +import * as Commands from "../../../../../../src/editor/commands"; +import * as Reply from "../../../../../../src/utils/Reply"; +import { MatrixClientPeg } from "../../../../../../src/MatrixClientPeg"; +import { Action } from "../../../../../../src/dispatcher/actions"; describe("message", () => { const permalinkCreator = { @@ -47,6 +52,9 @@ describe("message", () => { }); const mockClient = createTestClient(); + mockClient.setDisplayName = jest.fn().mockResolvedValue({}); + mockClient.setRoomName = jest.fn().mockResolvedValue({}); + const mockRoom = mkStubRoom("myfakeroom", "myfakeroom", mockClient) as any; mockRoom.findEventById = jest.fn((eventId) => { return eventId === mockEvent.getId() ? mockEvent : null; @@ -56,8 +64,13 @@ describe("message", () => { const spyDispatcher = jest.spyOn(defaultDispatcher, "dispatch"); - afterEach(() => { - jest.resetAllMocks(); + beforeEach(() => { + jest.spyOn(MatrixClientPeg, "get").mockReturnValue(mockClient); + jest.clearAllMocks(); + }); + + afterAll(() => { + jest.restoreAllMocks(); }); describe("sendMessage", () => { @@ -226,6 +239,153 @@ describe("message", () => { // Then expect(spyDispatcher).toHaveBeenCalledWith({ action: "effects.confetti" }); }); + + describe("slash commands", () => { + const getCommandSpy = jest.spyOn(SlashCommands, "getCommand"); + + it("calls getCommand for a message starting with a valid command", async () => { + // When + const validCommand = "/spoiler"; + await sendMessage(validCommand, true, { + roomContext: defaultRoomContext, + mxClient: mockClient, + permalinkCreator, + }); + + // Then + expect(getCommandSpy).toHaveBeenCalledWith(validCommand); + }); + + it("does not call getCommand for valid command with invalid prefix", async () => { + // When + const invalidPrefixCommand = "//spoiler"; + await sendMessage(invalidPrefixCommand, true, { + roomContext: defaultRoomContext, + mxClient: mockClient, + permalinkCreator, + }); + + // Then + expect(getCommandSpy).toHaveBeenCalledTimes(0); + }); + + it("returns undefined when the command is not successful", async () => { + // When + const validCommand = "/spoiler"; + jest.spyOn(Commands, "runSlashCommand").mockResolvedValueOnce([{ content: "mock content" }, false]); + + const result = await sendMessage(validCommand, true, { + roomContext: defaultRoomContext, + mxClient: mockClient, + permalinkCreator, + }); + + // Then + expect(result).toBeUndefined(); + }); + + // /spoiler is a .messages category command, /fireworks is an .effect category command + const messagesAndEffectCategoryTestCases = ["/spoiler text", "/fireworks"]; + it.each(messagesAndEffectCategoryTestCases)( + "does not add relations for a .messages or .effects category command if there is no relation to add", + async (inputText) => { + await sendMessage(inputText, true, { + roomContext: defaultRoomContext, + mxClient: mockClient, + permalinkCreator, + }); + expect(mockClient.sendMessage).toHaveBeenCalledWith( + "myfakeroom", + null, + expect.not.objectContaining({ "m.relates_to": expect.any }), + ); + }, + ); + + it.each(messagesAndEffectCategoryTestCases)( + "adds relations for a .messages or .effects category command if there is a relation", + async (inputText) => { + const mockRelation: IEventRelation = { + rel_type: "mock relation type", + }; + await sendMessage(inputText, true, { + roomContext: defaultRoomContext, + mxClient: mockClient, + permalinkCreator, + relation: mockRelation, + }); + + expect(mockClient.sendMessage).toHaveBeenCalledWith( + "myfakeroom", + null, + expect.objectContaining({ "m.relates_to": expect.objectContaining(mockRelation) }), + ); + }, + ); + + it("calls addReplyToMessageContent when there is an event to reply to", async () => { + const addReplySpy = jest.spyOn(Reply, "addReplyToMessageContent"); + await sendMessage("input", true, { + roomContext: defaultRoomContext, + mxClient: mockClient, + permalinkCreator, + replyToEvent: mockEvent, + }); + + expect(addReplySpy).toHaveBeenCalledTimes(1); + }); + + // these test cases are .action and .admin categories + const otherCategoryTestCases = ["/nick new_nickname", "/roomname new_room_name"]; + it.each(otherCategoryTestCases)( + "returns undefined when the command category is not .messages or .effects", + async (input) => { + const result = await sendMessage(input, true, { + roomContext: defaultRoomContext, + mxClient: mockClient, + permalinkCreator, + replyToEvent: mockEvent, + }); + + expect(result).toBeUndefined(); + }, + ); + + it("if user enters invalid command and then sends it anyway", async () => { + // mock out returning a true value for `shouldSendAnyway` to avoid rendering the modal + jest.spyOn(Commands, "shouldSendAnyway").mockResolvedValueOnce(true); + const invalidCommandInput = "/badCommand"; + + await sendMessage(invalidCommandInput, true, { + roomContext: defaultRoomContext, + mxClient: mockClient, + permalinkCreator, + }); + + // we expect the message to have been sent + // and a composer focus action to have been dispatched + expect(mockClient.sendMessage).toHaveBeenCalledWith( + "myfakeroom", + null, + expect.objectContaining({ body: invalidCommandInput }), + ); + expect(spyDispatcher).toHaveBeenCalledWith(expect.objectContaining({ action: Action.FocusAComposer })); + }); + + it("if user enters invalid command and then does not send, return undefined", async () => { + // mock out returning a false value for `shouldSendAnyway` to avoid rendering the modal + jest.spyOn(Commands, "shouldSendAnyway").mockResolvedValueOnce(false); + const invalidCommandInput = "/badCommand"; + + const result = await sendMessage(invalidCommandInput, true, { + roomContext: defaultRoomContext, + mxClient: mockClient, + permalinkCreator, + }); + + expect(result).toBeUndefined(); + }); + }); }); describe("editMessage", () => {