From df9b5df2703fb906057bcdf013d3aa90d9124b3d Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 10 Jul 2024 08:58:18 +0100 Subject: [PATCH 1/3] Align `widget_build_url_ignore_dm` with call behaviour switch between 1:1 and Widget Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/LegacyCallHandler.tsx | 16 ++++++------ src/hooks/room/useRoomCall.ts | 3 +-- src/widgets/ManagedHybrid.ts | 42 ++++++++++++++---------------- test/widgets/ManagedHybrid-test.ts | 26 +++++++++--------- 4 files changed, 42 insertions(+), 45 deletions(-) diff --git a/src/LegacyCallHandler.tsx b/src/LegacyCallHandler.tsx index c8ee1d5c74a..145784fe0c4 100644 --- a/src/LegacyCallHandler.tsx +++ b/src/LegacyCallHandler.tsx @@ -861,6 +861,12 @@ export default class LegacyCallHandler extends EventEmitter { public async placeCall(roomId: string, type: CallType, transferee?: MatrixCall): Promise { const cli = MatrixClientPeg.safeGet(); + const room = cli.getRoom(roomId); + if (!room) { + logger.error(`Room ${roomId} does not exist.`); + return; + } + // Pause current broadcast, if any SdkContextClass.instance.voiceBroadcastPlaybacksStore.getCurrent()?.pause(); @@ -871,8 +877,8 @@ export default class LegacyCallHandler extends EventEmitter { } // We might be using managed hybrid widgets - if (isManagedHybridWidgetEnabled(roomId)) { - await addManagedHybridWidget(roomId); + if (isManagedHybridWidgetEnabled(room)) { + await addManagedHybridWidget(room); return; } @@ -902,12 +908,6 @@ export default class LegacyCallHandler extends EventEmitter { return; } - const room = cli.getRoom(roomId); - if (!room) { - logger.error(`Room ${roomId} does not exist.`); - return; - } - // We leave the check for whether there's already a call in this room until later, // otherwise it can race. diff --git a/src/hooks/room/useRoomCall.ts b/src/hooks/room/useRoomCall.ts index fd2ed9c7502..8dc18040a12 100644 --- a/src/hooks/room/useRoomCall.ts +++ b/src/hooks/room/useRoomCall.ts @@ -271,8 +271,7 @@ export const useRoomCall = ( }, [isViewingCall, room.roomId]); // We hide the voice call button if it'd have the same effect as the video call button - let hideVoiceCallButton = - isManagedHybridWidgetEnabled(room.roomId) || !callOptions.includes(PlatformCallType.LegacyCall); + let hideVoiceCallButton = isManagedHybridWidgetEnabled(room) || !callOptions.includes(PlatformCallType.LegacyCall); let hideVideoCallButton = false; // We hide both buttons if they require widgets but widgets are disabled. if (memberCount > 2 && !SettingsStore.getValue(UIFeature.Widgets)) { diff --git a/src/widgets/ManagedHybrid.ts b/src/widgets/ManagedHybrid.ts index ff06c295e6e..6617933d97e 100644 --- a/src/widgets/ManagedHybrid.ts +++ b/src/widgets/ManagedHybrid.ts @@ -16,6 +16,7 @@ limitations under the License. import { IWidget } from "matrix-widget-api"; import { logger } from "matrix-js-sdk/src/logger"; +import { Room } from "matrix-js-sdk/src/matrix"; import { MatrixClientPeg } from "../MatrixClientPeg"; import { getCallBehaviourWellKnown } from "../utils/WellKnownUtils"; @@ -24,7 +25,7 @@ import { IStoredLayout, WidgetLayoutStore } from "../stores/widgets/WidgetLayout import WidgetEchoStore from "../stores/WidgetEchoStore"; import WidgetStore, { IApp } from "../stores/WidgetStore"; import SdkConfig from "../SdkConfig"; -import DMRoomMap from "../utils/DMRoomMap"; +import { getJoinedNonFunctionalMembers } from "../utils/room/getJoinedNonFunctionalMembers"; /* eslint-disable camelcase */ interface IManagedHybridWidgetData { @@ -34,8 +35,9 @@ interface IManagedHybridWidgetData { } /* eslint-enable camelcase */ -function getWidgetBuildUrl(roomId: string): string | undefined { - const isDm = !!DMRoomMap.shared().getUserIdForRoomId(roomId); +function getWidgetBuildUrl(room: Room): string | undefined { + const functionalMembers = getJoinedNonFunctionalMembers(room); + const isDm = functionalMembers.length === 2; if (SdkConfig.get().widget_build_url) { if (isDm && SdkConfig.get().widget_build_url_ignore_dm) { return undefined; @@ -51,35 +53,29 @@ function getWidgetBuildUrl(roomId: string): string | undefined { return wellKnown?.widget_build_url; } -export function isManagedHybridWidgetEnabled(roomId: string): boolean { - return !!getWidgetBuildUrl(roomId); +export function isManagedHybridWidgetEnabled(room: Room): boolean { + return !!getWidgetBuildUrl(room); } -export async function addManagedHybridWidget(roomId: string): Promise { - const cli = MatrixClientPeg.safeGet(); - const room = cli.getRoom(roomId); - if (!room) { - return; - } - +export async function addManagedHybridWidget(room: Room): Promise { // Check for permission - if (!WidgetUtils.canUserModifyWidgets(cli, roomId)) { - logger.error(`User not allowed to modify widgets in ${roomId}`); + if (!WidgetUtils.canUserModifyWidgets(room.client, room.roomId)) { + logger.error(`User not allowed to modify widgets in ${room.roomId}`); return; } // Get widget data /* eslint-disable-next-line camelcase */ - const widgetBuildUrl = getWidgetBuildUrl(roomId); + const widgetBuildUrl = getWidgetBuildUrl(room); if (!widgetBuildUrl) { return; } let widgetData: IManagedHybridWidgetData; try { - const response = await fetch(`${widgetBuildUrl}?roomId=${roomId}`); + const response = await fetch(`${widgetBuildUrl}?roomId=${room.roomId}`); widgetData = await response.json(); } catch (e) { - logger.error(`Managed hybrid widget builder failed for room ${roomId}`, e); + logger.error(`Managed hybrid widget builder failed for room ${room.roomId}`, e); return; } if (!widgetData) { @@ -88,21 +84,21 @@ export async function addManagedHybridWidget(roomId: string): Promise { const { widget_id: widgetId, widget: widgetContent, layout } = widgetData; // Ensure the widget is not already present in the room - let widgets = WidgetStore.instance.getApps(roomId); - const existing = widgets.some((w) => w.id === widgetId) || WidgetEchoStore.roomHasPendingWidgets(roomId, []); + let widgets = WidgetStore.instance.getApps(room.roomId); + const existing = widgets.some((w) => w.id === widgetId) || WidgetEchoStore.roomHasPendingWidgets(room.roomId, []); if (existing) { - logger.error(`Managed hybrid widget already present in room ${roomId}`); + logger.error(`Managed hybrid widget already present in room ${room.roomId}`); return; } // Add the widget try { - await WidgetUtils.setRoomWidgetContent(cli, roomId, widgetId, { + await WidgetUtils.setRoomWidgetContent(room.client, room.roomId, widgetId, { ...widgetContent, "io.element.managed_hybrid": true, }); } catch (e) { - logger.error(`Unable to add managed hybrid widget in room ${roomId}`, e); + logger.error(`Unable to add managed hybrid widget in room ${room.roomId}`, e); return; } @@ -110,7 +106,7 @@ export async function addManagedHybridWidget(roomId: string): Promise { if (!WidgetLayoutStore.instance.canCopyLayoutToRoom(room)) { return; } - widgets = WidgetStore.instance.getApps(roomId); + widgets = WidgetStore.instance.getApps(room.roomId); const installedWidget = widgets.find((w) => w.id === widgetId); if (!installedWidget) { return; diff --git a/test/widgets/ManagedHybrid-test.ts b/test/widgets/ManagedHybrid-test.ts index b91db09dc19..99b7f8c704b 100644 --- a/test/widgets/ManagedHybrid-test.ts +++ b/test/widgets/ManagedHybrid-test.ts @@ -14,38 +14,40 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { Room } from "matrix-js-sdk/src/matrix"; + import { isManagedHybridWidgetEnabled } from "../../src/widgets/ManagedHybrid"; -import DMRoomMap from "../../src/utils/DMRoomMap"; import { stubClient } from "../test-utils"; import SdkConfig from "../../src/SdkConfig"; +jest.mock("../../src/utils/room/getJoinedNonFunctionalMembers", () => ({ + getJoinedNonFunctionalMembers: jest.fn().mockReturnValue([1, 2]), +})); + describe("isManagedHybridWidgetEnabled", () => { - let dmRoomMap: DMRoomMap; + let room: Room; beforeEach(() => { - stubClient(); - dmRoomMap = { - getUserIdForRoomId: jest.fn().mockReturnValue("@user:server"), - } as unknown as DMRoomMap; - DMRoomMap.setShared(dmRoomMap); + const client = stubClient(); + room = new Room("!room:server", client, client.getSafeUserId()); }); it("should return false if widget_build_url is unset", () => { - expect(isManagedHybridWidgetEnabled("!room:server")).toBeFalsy(); + expect(isManagedHybridWidgetEnabled(room)).toBeFalsy(); }); - it("should return true for DMs when widget_build_url_ignore_dm is unset", () => { + it("should return true for 1-1 rooms when widget_build_url_ignore_dm is unset", () => { SdkConfig.put({ widget_build_url: "https://url", }); - expect(isManagedHybridWidgetEnabled("!room:server")).toBeTruthy(); + expect(isManagedHybridWidgetEnabled(room)).toBeTruthy(); }); - it("should return false for DMs when widget_build_url_ignore_dm is true", () => { + it("should return false for 1-1 rooms when widget_build_url_ignore_dm is true", () => { SdkConfig.put({ widget_build_url: "https://url", widget_build_url_ignore_dm: true, }); - expect(isManagedHybridWidgetEnabled("!room:server")).toBeFalsy(); + expect(isManagedHybridWidgetEnabled(room)).toBeFalsy(); }); }); From 7f9675ea6e1fa6382f8128c2144bb4cf0a167f51 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 10 Jul 2024 11:53:24 +0100 Subject: [PATCH 2/3] Add tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- test/widgets/ManagedHybrid-test.ts | 51 +++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/test/widgets/ManagedHybrid-test.ts b/test/widgets/ManagedHybrid-test.ts index 99b7f8c704b..5ead3239457 100644 --- a/test/widgets/ManagedHybrid-test.ts +++ b/test/widgets/ManagedHybrid-test.ts @@ -15,10 +15,13 @@ limitations under the License. */ import { Room } from "matrix-js-sdk/src/matrix"; +import { logger } from "matrix-js-sdk/src/logger"; +import fetchMock from "fetch-mock-jest"; -import { isManagedHybridWidgetEnabled } from "../../src/widgets/ManagedHybrid"; +import { addManagedHybridWidget, isManagedHybridWidgetEnabled } from "../../src/widgets/ManagedHybrid"; import { stubClient } from "../test-utils"; import SdkConfig from "../../src/SdkConfig"; +import WidgetUtils from "../../src/utils/WidgetUtils"; jest.mock("../../src/utils/room/getJoinedNonFunctionalMembers", () => ({ getJoinedNonFunctionalMembers: jest.fn().mockReturnValue([1, 2]), @@ -51,3 +54,49 @@ describe("isManagedHybridWidgetEnabled", () => { expect(isManagedHybridWidgetEnabled(room)).toBeFalsy(); }); }); + +describe("addManagedHybridWidget", () => { + let room: Room; + + beforeEach(() => { + const client = stubClient(); + room = new Room("!room:server", client, client.getSafeUserId()); + }); + + it("should noop if user lacks permission", async () => { + const logSpy = jest.spyOn(logger, "error").mockImplementation(); + jest.spyOn(WidgetUtils, "canUserModifyWidgets").mockReturnValue(false); + + fetchMock.mockClear(); + await addManagedHybridWidget(room); + expect(logSpy).toHaveBeenCalledWith("User not allowed to modify widgets in !room:server"); + expect(fetchMock).toHaveBeenCalledTimes(0); + }); + + it("should noop if no widget_build_url", async () => { + jest.spyOn(WidgetUtils, "canUserModifyWidgets").mockReturnValue(true); + + fetchMock.mockClear(); + await addManagedHybridWidget(room); + expect(fetchMock).toHaveBeenCalledTimes(0); + }); + + it("should add the widget successfully", async () => { + fetchMock.get("https://widget-build-url/?roomId=!room:server", { + widget_id: "WIDGET_ID", + widget: { key: "value" }, + }); + jest.spyOn(WidgetUtils, "canUserModifyWidgets").mockReturnValue(true); + const setRoomWidgetContentSpy = jest.spyOn(WidgetUtils, "setRoomWidgetContent").mockResolvedValue(); + SdkConfig.put({ + widget_build_url: "https://widget-build-url", + }); + + await addManagedHybridWidget(room); + expect(fetchMock).toHaveBeenCalledWith("https://widget-build-url?roomId=!room:server"); + expect(setRoomWidgetContentSpy).toHaveBeenCalledWith(room.client, room.roomId, "WIDGET_ID", { + "key": "value", + "io.element.managed_hybrid": true, + }); + }); +}); From a7dca33f8ff40f44356737d8475fbe3edd19b2e0 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 10 Jul 2024 12:20:26 +0100 Subject: [PATCH 3/3] Improve coverage Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- test/LegacyCallHandler-test.ts | 9 +++++++++ test/widgets/ManagedHybrid-test.ts | 2 ++ 2 files changed, 11 insertions(+) diff --git a/test/LegacyCallHandler-test.ts b/test/LegacyCallHandler-test.ts index 005119cb7be..c310db55abc 100644 --- a/test/LegacyCallHandler-test.ts +++ b/test/LegacyCallHandler-test.ts @@ -52,6 +52,7 @@ import { mkVoiceBroadcastInfoStateEvent } from "./voice-broadcast/utils/test-uti import { SdkContextClass } from "../src/contexts/SDKContext"; import Modal from "../src/Modal"; import { createAudioContext } from "../src/audio/compat"; +import * as ManagedHybrid from "../src/widgets/ManagedHybrid"; jest.mock("../src/Modal"); @@ -315,6 +316,7 @@ describe("LegacyCallHandler", () => { document.body.removeChild(audioElement); SdkConfig.reset(); + jest.restoreAllMocks(); }); it("should look up the correct user and start a call in the room when a phone number is dialled", async () => { @@ -403,6 +405,13 @@ describe("LegacyCallHandler", () => { expect(callHandler.getCallForRoom(NATIVE_ROOM_CHARLIE)).toBe(fakeCall); }); + it("should place calls using managed hybrid widget if enabled", async () => { + const spy = jest.spyOn(ManagedHybrid, "addManagedHybridWidget"); + jest.spyOn(ManagedHybrid, "isManagedHybridWidgetEnabled").mockReturnValue(true); + await callHandler.placeCall(NATIVE_ROOM_ALICE, CallType.Voice); + expect(spy).toHaveBeenCalledWith(MatrixClientPeg.safeGet().getRoom(NATIVE_ROOM_ALICE)); + }); + describe("when listening to a voice broadcast", () => { let voiceBroadcastPlayback: VoiceBroadcastPlayback; diff --git a/test/widgets/ManagedHybrid-test.ts b/test/widgets/ManagedHybrid-test.ts index 5ead3239457..05093ed0d4c 100644 --- a/test/widgets/ManagedHybrid-test.ts +++ b/test/widgets/ManagedHybrid-test.ts @@ -22,6 +22,7 @@ import { addManagedHybridWidget, isManagedHybridWidgetEnabled } from "../../src/ import { stubClient } from "../test-utils"; import SdkConfig from "../../src/SdkConfig"; import WidgetUtils from "../../src/utils/WidgetUtils"; +import { WidgetLayoutStore } from "../../src/stores/widgets/WidgetLayoutStore"; jest.mock("../../src/utils/room/getJoinedNonFunctionalMembers", () => ({ getJoinedNonFunctionalMembers: jest.fn().mockReturnValue([1, 2]), @@ -87,6 +88,7 @@ describe("addManagedHybridWidget", () => { widget: { key: "value" }, }); jest.spyOn(WidgetUtils, "canUserModifyWidgets").mockReturnValue(true); + jest.spyOn(WidgetLayoutStore.instance, "canCopyLayoutToRoom").mockReturnValue(true); const setRoomWidgetContentSpy = jest.spyOn(WidgetUtils, "setRoomWidgetContent").mockResolvedValue(); SdkConfig.put({ widget_build_url: "https://widget-build-url",