Align widget_build_url_ignore_dm with call behaviour switch between 1:1 and Widget (#12760)

* Align `widget_build_url_ignore_dm` with call behaviour switch between 1:1 and Widget

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Add tests

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Improve coverage

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

---------

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

src/LegacyCallHandler.tsx

@@ -861,6 +861,12 @@ export default class LegacyCallHandler extends EventEmitter {
 
     public async placeCall(roomId: string, type: CallType, transferee?: MatrixCall): Promise<void> {
         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)) {
+        if (isManagedHybridWidgetEnabled(room)) {
-            await addManagedHybridWidget(roomId);
+            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.
 

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 =
+    let hideVoiceCallButton = isManagedHybridWidgetEnabled(room) || !callOptions.includes(PlatformCallType.LegacyCall);
-        isManagedHybridWidgetEnabled(room.roomId) || !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)) {

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 {
+function getWidgetBuildUrl(room: Room): string | undefined {
-    const isDm = !!DMRoomMap.shared().getUserIdForRoomId(roomId);
+    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 {
+export function isManagedHybridWidgetEnabled(room: Room): boolean {
-    return !!getWidgetBuildUrl(roomId);
+    return !!getWidgetBuildUrl(room);
 }
 
-export async function addManagedHybridWidget(roomId: string): Promise<void> {
+export async function addManagedHybridWidget(room: Room): Promise<void> {
-    const cli = MatrixClientPeg.safeGet();
-    const room = cli.getRoom(roomId);
-    if (!room) {
-        return;
-    }
-
     // Check for permission
-    if (!WidgetUtils.canUserModifyWidgets(cli, roomId)) {
+    if (!WidgetUtils.canUserModifyWidgets(room.client, room.roomId)) {
-        logger.error(`User not allowed to modify widgets in ${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<void> {
     const { widget_id: widgetId, widget: widgetContent, layout } = widgetData;
 
     // Ensure the widget is not already present in the room
-    let widgets = WidgetStore.instance.getApps(roomId);
+    let widgets = WidgetStore.instance.getApps(room.roomId);
-    const existing = widgets.some((w) => w.id === widgetId) || WidgetEchoStore.roomHasPendingWidgets(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<void> {
     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;

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;
 

test/widgets/ManagedHybrid-test.ts

@@ -14,38 +14,91 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import { isManagedHybridWidgetEnabled } from "../../src/widgets/ManagedHybrid";
+import { Room } from "matrix-js-sdk/src/matrix";
-import DMRoomMap from "../../src/utils/DMRoomMap";
+import { logger } from "matrix-js-sdk/src/logger";
+import fetchMock from "fetch-mock-jest";
+
+import { addManagedHybridWidget, isManagedHybridWidgetEnabled } from "../../src/widgets/ManagedHybrid";
 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]),
+}));
 
 describe("isManagedHybridWidgetEnabled", () => {
-    let dmRoomMap: DMRoomMap;
+    let room: Room;
 
     beforeEach(() => {
-        stubClient();
+        const client = stubClient();
-        dmRoomMap = {
+        room = new Room("!room:server", client, client.getSafeUserId());
-            getUserIdForRoomId: jest.fn().mockReturnValue("@user:server"),
-        } as unknown as DMRoomMap;
-        DMRoomMap.setShared(dmRoomMap);
     });
 
     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();
+    });
+});
+
+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);
+        jest.spyOn(WidgetLayoutStore.instance, "canCopyLayoutToRoom").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,
+        });
     });
 });