From 19ca18cf9920abcc4db5fe2647eb220f8e0316eb Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 13 Jul 2023 16:54:28 +0100 Subject: [PATCH 1/5] Split join and goto slash commands, the latter shouldn't auto_join --- src/SlashCommands.tsx | 116 +------------------------- src/slash-commands/join.ts | 163 +++++++++++++++++++++++++++++++++++++ 2 files changed, 166 insertions(+), 113 deletions(-) create mode 100644 src/slash-commands/join.ts diff --git a/src/SlashCommands.tsx b/src/SlashCommands.tsx index 929d6e6e6f9..0770ae18a1c 100644 --- a/src/SlashCommands.tsx +++ b/src/SlashCommands.tsx @@ -36,7 +36,6 @@ import { textToHtmlRainbow } from "./utils/colour"; import { AddressType, getAddressType } from "./UserAddress"; import { abbreviateUrl } from "./utils/UrlUtils"; import { getDefaultIdentityServerUrl, setToDefaultIdentityServer } from "./utils/IdentityServerUtils"; -import { isPermalinkHost, parsePermalink } from "./utils/permalinks/Permalinks"; import { WidgetType } from "./widgets/WidgetType"; import { Jitsi } from "./widgets/Jitsi"; import BugReportDialog from "./components/views/dialogs/BugReportDialog"; @@ -66,6 +65,7 @@ import { isCurrentLocalRoom, reject, singleMxcUpload, success, successSync } fro import { deop, op } from "./slash-commands/op"; import { CommandCategories } from "./slash-commands/interface"; import { Command } from "./slash-commands/command"; +import { goto, join } from "./slash-commands/join"; export { CommandCategories, Command }; @@ -458,118 +458,8 @@ export const Commands = [ category: CommandCategories.actions, renderingTypes: [TimelineRenderingType.Room], }), - new Command({ - command: "join", - aliases: ["j", "goto"], - args: "", - description: _td("Joins room with given address"), - runFn: function (cli, roomId, threadId, args) { - if (args) { - // Note: we support 2 versions of this command. The first is - // the public-facing one for most users and the other is a - // power-user edition where someone may join via permalink or - // room ID with optional servers. Practically, this results - // in the following variations: - // /join #example:example.org - // /join !example:example.org - // /join !example:example.org altserver.com elsewhere.ca - // /join https://matrix.to/#/!example:example.org?via=altserver.com - // The command also supports event permalinks transparently: - // /join https://matrix.to/#/!example:example.org/$something:example.org - // /join https://matrix.to/#/!example:example.org/$something:example.org?via=altserver.com - const params = args.split(" "); - if (params.length < 1) return reject(this.getUsage()); - - let isPermalink = false; - if (params[0].startsWith("http:") || params[0].startsWith("https:")) { - // It's at least a URL - try and pull out a hostname to check against the - // permalink handler - const parsedUrl = new URL(params[0]); - const hostname = parsedUrl.host || parsedUrl.hostname; // takes first non-falsey value - - // if we're using a Element permalink handler, this will catch it before we get much further. - // see below where we make assumptions about parsing the URL. - if (isPermalinkHost(hostname)) { - isPermalink = true; - } - } - if (params[0][0] === "#") { - let roomAlias = params[0]; - if (!roomAlias.includes(":")) { - roomAlias += ":" + cli.getDomain(); - } - - dis.dispatch({ - action: Action.ViewRoom, - room_alias: roomAlias, - auto_join: true, - metricsTrigger: "SlashCommand", - metricsViaKeyboard: true, - }); - return success(); - } else if (params[0][0] === "!") { - const [roomId, ...viaServers] = params; - - dis.dispatch({ - action: Action.ViewRoom, - room_id: roomId, - via_servers: viaServers, // for the rejoin button - auto_join: true, - metricsTrigger: "SlashCommand", - metricsViaKeyboard: true, - }); - return success(); - } else if (isPermalink) { - const permalinkParts = parsePermalink(params[0]); - - // This check technically isn't needed because we already did our - // safety checks up above. However, for good measure, let's be sure. - if (!permalinkParts) { - return reject(this.getUsage()); - } - - // If for some reason someone wanted to join a user, we should - // stop them now. - if (!permalinkParts.roomIdOrAlias) { - return reject(this.getUsage()); - } - - const entity = permalinkParts.roomIdOrAlias; - const viaServers = permalinkParts.viaServers; - const eventId = permalinkParts.eventId; - - const dispatch: ViewRoomPayload = { - action: Action.ViewRoom, - auto_join: true, - metricsTrigger: "SlashCommand", - metricsViaKeyboard: true, - }; - - if (entity[0] === "!") dispatch["room_id"] = entity; - else dispatch["room_alias"] = entity; - - if (eventId) { - dispatch["event_id"] = eventId; - dispatch["highlighted"] = true; - } - - if (viaServers) { - // For the join, these are passed down to the js-sdk's /join call - dispatch["opts"] = { viaServers }; - - // For if the join fails (rejoin button) - dispatch["via_servers"] = viaServers; - } - - dis.dispatch(dispatch); - return success(); - } - } - return reject(this.getUsage()); - }, - category: CommandCategories.actions, - renderingTypes: [TimelineRenderingType.Room], - }), + goto, + join, new Command({ command: "part", args: "[]", diff --git a/src/slash-commands/join.ts b/src/slash-commands/join.ts new file mode 100644 index 00000000000..fad925716db --- /dev/null +++ b/src/slash-commands/join.ts @@ -0,0 +1,163 @@ +/* +Copyright 2015, 2016 OpenMarket Ltd +Copyright 2018 New Vector Ltd +Copyright 2019 Michael Telatynski <7t3chguy@gmail.com> +Copyright 2020 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { MatrixClient } from "matrix-js-sdk/src/matrix"; + +import { _td } from "../languageHandler"; +import { reject, success } from "./utils"; +import { isPermalinkHost, parsePermalink } from "../utils/permalinks/Permalinks"; +import dis from "../dispatcher/dispatcher"; +import { ViewRoomPayload } from "../dispatcher/payloads/ViewRoomPayload"; +import { Action } from "../dispatcher/actions"; +import { TimelineRenderingType } from "../contexts/RoomContext"; +import { Command } from "./command"; +import { CommandCategories, RunResult } from "./interface"; + +function openRoom(cli: MatrixClient, args: string | undefined, autoJoin: boolean): RunResult | undefined { + if (!args) return; + const params = args.split(" "); + if (params.length < 1) return; + + let isPermalink = false; + if (params[0].startsWith("http:") || params[0].startsWith("https:")) { + // It's at least a URL - try and pull out a hostname to check against the + // permalink handler + const parsedUrl = new URL(params[0]); + const hostname = parsedUrl.host || parsedUrl.hostname; // takes first non-falsey value + + // if we're using a Element permalink handler, this will catch it before we get much further. + // see below where we make assumptions about parsing the URL. + if (isPermalinkHost(hostname)) { + isPermalink = true; + } + } + + if (params[0][0] === "#") { + let roomAlias = params[0]; + if (!roomAlias.includes(":")) { + roomAlias += ":" + cli.getDomain(); + } + + dis.dispatch({ + action: Action.ViewRoom, + room_alias: roomAlias, + auto_join: autoJoin, + metricsTrigger: "SlashCommand", + metricsViaKeyboard: true, + }); + return success(); + } + + if (params[0][0] === "!") { + const [roomId, ...viaServers] = params; + + dis.dispatch({ + action: Action.ViewRoom, + room_id: roomId, + via_servers: viaServers, // for the rejoin button + auto_join: autoJoin, + metricsTrigger: "SlashCommand", + metricsViaKeyboard: true, + }); + return success(); + } + + if (isPermalink) { + const permalinkParts = parsePermalink(params[0]); + + // This check technically isn't needed because we already did our + // safety checks up above. However, for good measure, let's be sure. + if (!permalinkParts) { + return; + } + + // If for some reason someone wanted to join a user, we should + // stop them now. + if (!permalinkParts.roomIdOrAlias) { + return; + } + + const entity = permalinkParts.roomIdOrAlias; + const viaServers = permalinkParts.viaServers; + const eventId = permalinkParts.eventId; + + const dispatch: ViewRoomPayload = { + action: Action.ViewRoom, + auto_join: autoJoin, + metricsTrigger: "SlashCommand", + metricsViaKeyboard: true, + }; + + if (entity[0] === "!") dispatch["room_id"] = entity; + else dispatch["room_alias"] = entity; + + if (eventId) { + dispatch["event_id"] = eventId; + dispatch["highlighted"] = true; + } + + if (viaServers) { + // For the join, these are passed down to the js-sdk's /join call + dispatch["opts"] = { viaServers }; + + // For if the join fails (rejoin button) + dispatch["via_servers"] = viaServers; + } + + dis.dispatch(dispatch); + return success(); + } +} + +// Note: we support 2 versions of this command. The first is +// the public-facing one for most users and the other is a +// power-user edition where someone may join via permalink or +// room ID with optional servers. Practically, this results +// in the following variations: +// /join #example:example.org +// /join !example:example.org +// /join !example:example.org altserver.com elsewhere.ca +// /join https://matrix.to/#/!example:example.org?via=altserver.com +// The command also supports event permalinks transparently: +// /join https://matrix.to/#/!example:example.org/$something:example.org +// /join https://matrix.to/#/!example:example.org/$something:example.org?via=altserver.com +export const join = new Command({ + command: "join", + aliases: ["j"], + args: "", + description: _td("Joins room with given address"), + runFn: function (cli, roomId, threadId, args) { + return openRoom(cli, args, true) ?? reject(this.getUsage()); + }, + category: CommandCategories.actions, + renderingTypes: [TimelineRenderingType.Room], +}); + +// Similar to join but doesn't auto join the room if you aren't already joined to it +export const goto = new Command({ + command: "goto", + aliases: ["view"], + args: "", + description: _td("Views room with given address"), + runFn: function (cli, roomId, threadId, args) { + return openRoom(cli, args, false) ?? reject(this.getUsage()); + }, + category: CommandCategories.actions, + renderingTypes: [TimelineRenderingType.Room], +}); From f7af88f7d406352b3c679d0c245decd6ed944cf5 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 13 Jul 2023 16:58:49 +0100 Subject: [PATCH 2/5] i18n --- src/i18n/strings/en_EN.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 4b3d180041f..b54b78cf7e2 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -435,7 +435,6 @@ "Continue": "Continue", "Use an identity server to invite by email. Manage in Settings.": "Use an identity server to invite by email. Manage in Settings.", "User (%(user)s) did not end up as invited to %(roomId)s but no error was given from the inviter utility": "User (%(user)s) did not end up as invited to %(roomId)s but no error was given from the inviter utility", - "Joins room with given address": "Joins room with given address", "Leave room": "Leave room", "Unrecognised room address: %(roomAlias)s": "Unrecognised room address: %(roomAlias)s", "Removes user with given id from this room": "Removes user with given id from this room", @@ -934,6 +933,8 @@ "Advanced": "Advanced", "Effects": "Effects", "Other": "Other", + "Joins room with given address": "Joins room with given address", + "Views room with given address": "Views room with given address", "Command failed: Unable to find room (%(roomId)s": "Command failed: Unable to find room (%(roomId)s", "Could not find user in room": "Could not find user in room", "Define the power level of a user": "Define the power level of a user", From 9fd7dca5d88f8d0ffce45a2f8e65c3566647dafb Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 14 Jul 2023 08:51:26 +0100 Subject: [PATCH 3/5] Add tests --- test/SlashCommands-test.tsx | 39 +++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/test/SlashCommands-test.tsx b/test/SlashCommands-test.tsx index 26820386e25..235a152d331 100644 --- a/test/SlashCommands-test.tsx +++ b/test/SlashCommands-test.tsx @@ -27,6 +27,7 @@ import Modal from "../src/Modal"; import WidgetUtils from "../src/utils/WidgetUtils"; import { WidgetType } from "../src/widgets/WidgetType"; import { warnSelfDemote } from "../src/components/views/right_panel/UserInfo"; +import dispatcher from "../src/dispatcher/dispatcher"; jest.mock("../src/components/views/right_panel/UserInfo"); @@ -345,4 +346,42 @@ describe("SlashCommands", () => { ); }); }); + + describe("/join", () => { + beforeEach(() => { + jest.spyOn(dispatcher, "dispatch"); + command = findCommand("join")!; + }); + + it("should handle matrix.org permalinks", () => { + command.run(client, roomId, null, "https://matrix.to/#/#test:server"); + expect(dispatcher.dispatch).toHaveBeenCalledWith( + expect.objectContaining({ + action: "view_room", + room_alias: "#test:server", + }), + ); + }); + + it("should handle room aliases", () => { + command.run(client, roomId, null, "#test:server"); + expect(dispatcher.dispatch).toHaveBeenCalledWith( + expect.objectContaining({ + action: "view_room", + room_alias: "#test:server", + }), + ); + }); + + it("should handle room IDs and via servers", () => { + command.run(client, roomId, null, "!foo:bar serv1.com serv2.com"); + expect(dispatcher.dispatch).toHaveBeenCalledWith( + expect.objectContaining({ + action: "view_room", + room_id: "!foo:bar", + via_servers: ["serv1.com", "serv2.com"], + }), + ); + }); + }); }); From b0afdfb73017c6fde19afd9b91d2e19cc6855138 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 14 Jul 2023 11:13:15 +0100 Subject: [PATCH 4/5] Iterate --- src/slash-commands/join.ts | 3 +++ test/SlashCommands-test.tsx | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/slash-commands/join.ts b/src/slash-commands/join.ts index fad925716db..c54025150fa 100644 --- a/src/slash-commands/join.ts +++ b/src/slash-commands/join.ts @@ -29,6 +29,7 @@ import { TimelineRenderingType } from "../contexts/RoomContext"; import { Command } from "./command"; import { CommandCategories, RunResult } from "./interface"; +// A return of undefined here signals a usage error, where the command should return `reject(this.getUsage());` function openRoom(cli: MatrixClient, args: string | undefined, autoJoin: boolean): RunResult | undefined { if (!args) return; const params = args.split(" "); @@ -123,6 +124,8 @@ function openRoom(cli: MatrixClient, args: string | undefined, autoJoin: boolean dis.dispatch(dispatch); return success(); } + + // Otherwise, it's a usage error. Return `undefined`. } // Note: we support 2 versions of this command. The first is diff --git a/test/SlashCommands-test.tsx b/test/SlashCommands-test.tsx index 235a152d331..c40ffd25706 100644 --- a/test/SlashCommands-test.tsx +++ b/test/SlashCommands-test.tsx @@ -354,11 +354,13 @@ describe("SlashCommands", () => { }); it("should handle matrix.org permalinks", () => { - command.run(client, roomId, null, "https://matrix.to/#/#test:server"); + command.run(client, roomId, null, "https://matrix.to/#/!roomId:server/$eventId"); expect(dispatcher.dispatch).toHaveBeenCalledWith( expect.objectContaining({ action: "view_room", - room_alias: "#test:server", + room_id: "!roomId:server", + event_id: "$eventId", + highlighted: true, }), ); }); From 4d0cd7fc1f74385aca212a17be326cc38f82222b Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 14 Jul 2023 11:54:37 +0100 Subject: [PATCH 5/5] Improve coverage --- test/SlashCommands-test.tsx | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/SlashCommands-test.tsx b/test/SlashCommands-test.tsx index c40ffd25706..0580aa86e05 100644 --- a/test/SlashCommands-test.tsx +++ b/test/SlashCommands-test.tsx @@ -353,6 +353,10 @@ describe("SlashCommands", () => { command = findCommand("join")!; }); + it("should return usage if no args", () => { + expect(command.run(client, roomId, null, undefined).error).toBe(command.getUsage()); + }); + it("should handle matrix.org permalinks", () => { command.run(client, roomId, null, "https://matrix.to/#/!roomId:server/$eventId"); expect(dispatcher.dispatch).toHaveBeenCalledWith( @@ -375,6 +379,16 @@ describe("SlashCommands", () => { ); }); + it("should handle room aliases with no server component", () => { + command.run(client, roomId, null, "#test"); + expect(dispatcher.dispatch).toHaveBeenCalledWith( + expect.objectContaining({ + action: "view_room", + room_alias: `#test:${client.getDomain()}`, + }), + ); + }); + it("should handle room IDs and via servers", () => { command.run(client, roomId, null, "!foo:bar serv1.com serv2.com"); expect(dispatcher.dispatch).toHaveBeenCalledWith(