From d7824ce86e10f529e2022e0e5929636d5dea6abe Mon Sep 17 00:00:00 2001 From: Timo <16718859+toger5@users.noreply.github.com> Date: Thu, 18 Sep 2025 17:46:04 +0200 Subject: [PATCH] Backport: Add ring notification to UserIntent.StartNewCallDM (#3498) * Add ring notification to UserIntent.StartNewCallDM Signed-off-by: Timo K * Add more tests (refactor to compute + get) Signed-off-by: Timo K --------- Signed-off-by: Timo K --- src/UrlParams.test.ts | 119 +++++++++++++++++++++++++++++++----------- src/UrlParams.ts | 30 +++++++---- 2 files changed, 108 insertions(+), 41 deletions(-) diff --git a/src/UrlParams.test.ts b/src/UrlParams.test.ts index 56a3797d..850e4091 100644 --- a/src/UrlParams.test.ts +++ b/src/UrlParams.test.ts @@ -5,12 +5,15 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, onTestFinished, vi } from "vitest"; +import { logger } from "matrix-js-sdk/lib/logger"; +import * as PlatformMod from "../src/Platform"; import { getRoomIdentifierFromUrl, - getUrlParams, + computeUrlParams, HeaderStyle, + getUrlParams, } from "../src/UrlParams"; const ROOM_NAME = "roomNameHere"; @@ -103,16 +106,16 @@ describe("UrlParams", () => { describe("preload", () => { it("defaults to false", () => { - expect(getUrlParams().preload).toBe(false); + expect(computeUrlParams().preload).toBe(false); }); it("ignored in SPA mode", () => { - expect(getUrlParams("?preload=true").preload).toBe(false); + expect(computeUrlParams("?preload=true").preload).toBe(false); }); it("respected in widget mode", () => { expect( - getUrlParams( + computeUrlParams( "?preload=true&widgetId=12345&parentUrl=https%3A%2F%2Flocalhost%2Ffoo", ).preload, ).toBe(true); @@ -121,19 +124,20 @@ describe("UrlParams", () => { describe("returnToLobby", () => { it("is false in SPA mode", () => { - expect(getUrlParams("?returnToLobby=true").returnToLobby).toBe(false); + expect(computeUrlParams("?returnToLobby=true").returnToLobby).toBe(false); }); it("defaults to false in widget mode", () => { expect( - getUrlParams("?widgetId=12345&parentUrl=https%3A%2F%2Flocalhost%2Ffoo") - .returnToLobby, + computeUrlParams( + "?widgetId=12345&parentUrl=https%3A%2F%2Flocalhost%2Ffoo", + ).returnToLobby, ).toBe(false); }); it("respected in widget mode", () => { expect( - getUrlParams( + computeUrlParams( "?returnToLobby=true&widgetId=12345&parentUrl=https%3A%2F%2Flocalhost%2Ffoo", ).returnToLobby, ).toBe(true); @@ -142,12 +146,12 @@ describe("UrlParams", () => { describe("userId", () => { it("is ignored in SPA mode", () => { - expect(getUrlParams("?userId=asd").userId).toBe(null); + expect(computeUrlParams("?userId=asd").userId).toBe(null); }); it("is parsed in widget mode", () => { expect( - getUrlParams( + computeUrlParams( "?userId=asd&widgetId=12345&parentUrl=https%3A%2F%2Flocalhost%2Ffoo", ).userId, ).toBe("asd"); @@ -156,12 +160,12 @@ describe("UrlParams", () => { describe("deviceId", () => { it("is ignored in SPA mode", () => { - expect(getUrlParams("?deviceId=asd").deviceId).toBe(null); + expect(computeUrlParams("?deviceId=asd").deviceId).toBe(null); }); it("is parsed in widget mode", () => { expect( - getUrlParams( + computeUrlParams( "?deviceId=asd&widgetId=12345&parentUrl=https%3A%2F%2Flocalhost%2Ffoo", ).deviceId, ).toBe("asd"); @@ -170,12 +174,12 @@ describe("UrlParams", () => { describe("baseUrl", () => { it("is ignored in SPA mode", () => { - expect(getUrlParams("?baseUrl=asd").baseUrl).toBe(null); + expect(computeUrlParams("?baseUrl=asd").baseUrl).toBe(null); }); it("is parsed in widget mode", () => { expect( - getUrlParams( + computeUrlParams( "?baseUrl=asd&widgetId=12345&parentUrl=https%3A%2F%2Flocalhost%2Ffoo", ).baseUrl, ).toBe("asd"); @@ -185,28 +189,28 @@ describe("UrlParams", () => { describe("viaServers", () => { it("is ignored in widget mode", () => { expect( - getUrlParams( + computeUrlParams( "?viaServers=asd&widgetId=12345&parentUrl=https%3A%2F%2Flocalhost%2Ffoo", ).viaServers, ).toBe(null); }); it("is parsed in SPA mode", () => { - expect(getUrlParams("?viaServers=asd").viaServers).toBe("asd"); + expect(computeUrlParams("?viaServers=asd").viaServers).toBe("asd"); }); }); describe("homeserver", () => { it("is ignored in widget mode", () => { expect( - getUrlParams( + computeUrlParams( "?homeserver=asd&widgetId=12345&parentUrl=https%3A%2F%2Flocalhost%2Ffoo", ).homeserver, ).toBe(null); }); it("is parsed in SPA mode", () => { - expect(getUrlParams("?homeserver=asd").homeserver).toBe("asd"); + expect(computeUrlParams("?homeserver=asd").homeserver).toBe("asd"); }); }); @@ -237,7 +241,7 @@ describe("UrlParams", () => { controlledAudioDevices: platform === "desktop" ? false : true, skipLobby: true, returnToLobby: false, - sendNotificationType: "notification", + sendNotificationType: platform === "desktop" ? "notification" : "ring", }); const joinExistingCallDefaults = (platform: string): object => ({ confineToRoom: true, @@ -252,24 +256,55 @@ describe("UrlParams", () => { skipLobby: false, returnToLobby: false, sendNotificationType: "notification", + defaultAudioEnabled: true, + defaultVideoEnabled: true, }); it("use no-intent-defaults with unknown intent", () => { - expect(getUrlParams()).toMatchObject(noIntentDefaults); + expect(computeUrlParams()).toMatchObject(noIntentDefaults); }); it("ignores intent if it is not a valid value", () => { - expect(getUrlParams("?intent=foo")).toMatchObject(noIntentDefaults); + expect(computeUrlParams("?intent=foo")).toMatchObject(noIntentDefaults); }); it("accepts start_call", () => { expect( - getUrlParams("?intent=start_call&widgetId=1234&parentUrl=parent.org"), + computeUrlParams( + "?intent=start_call&widgetId=1234&parentUrl=parent.org", + ), ).toMatchObject(startNewCallDefaults("desktop")); }); + it("accepts start_call_dm mobile", () => { + vi.spyOn(PlatformMod, "platform", "get").mockReturnValue("android"); + onTestFinished(() => { + vi.spyOn(PlatformMod, "platform", "get").mockReturnValue("desktop"); + }); + expect( + computeUrlParams( + "?intent=start_call_dm&widgetId=1234&parentUrl=parent.org", + ), + ).toMatchObject(startNewCallDefaults("android")); + }); + + it("accepts start_call_dm mobile and prioritizes overwritten params", () => { + vi.spyOn(PlatformMod, "platform", "get").mockReturnValue("android"); + onTestFinished(() => { + vi.spyOn(PlatformMod, "platform", "get").mockReturnValue("desktop"); + }); + expect( + computeUrlParams( + "?intent=start_call_dm&widgetId=1234&parentUrl=parent.org&sendNotificationType=notification", + ), + ).toMatchObject({ + ...startNewCallDefaults("android"), + sendNotificationType: "notification", + }); + }); + it("accepts join_existing", () => { expect( - getUrlParams( + computeUrlParams( "?intent=join_existing&widgetId=1234&parentUrl=parent.org", ), ).toMatchObject(joinExistingCallDefaults("desktop")); @@ -278,31 +313,55 @@ describe("UrlParams", () => { describe("skipLobby", () => { it("defaults to false", () => { - expect(getUrlParams().skipLobby).toBe(false); + expect(computeUrlParams().skipLobby).toBe(false); }); it("defaults to false if intent is start_call in SPA mode", () => { - expect(getUrlParams("?intent=start_call").skipLobby).toBe(false); + expect(computeUrlParams("?intent=start_call").skipLobby).toBe(false); }); it("defaults to true if intent is start_call in widget mode", () => { expect( - getUrlParams( + computeUrlParams( "?intent=start_call&widgetId=12345&parentUrl=https%3A%2F%2Flocalhost%2Ffoo", ).skipLobby, ).toBe(true); }); it("default to false if intent is join_existing", () => { - expect(getUrlParams("?intent=join_existing").skipLobby).toBe(false); + expect(computeUrlParams("?intent=join_existing").skipLobby).toBe(false); }); }); describe("header", () => { it("uses header if provided", () => { - expect(getUrlParams("?header=app_bar&hideHeader=true").header).toBe( + expect(computeUrlParams("?header=app_bar&hideHeader=true").header).toBe( "app_bar", ); - expect(getUrlParams("?header=none&hideHeader=false").header).toBe("none"); + expect(computeUrlParams("?header=none&hideHeader=false").header).toBe( + "none", + ); + }); + }); + describe("getUrlParams", () => { + it("uses cached values", () => { + const spy = vi.spyOn(logger, "info"); + // call get once + const params = getUrlParams("?header=app_bar&hideHeader=true", ""); + // call get twice + expect(getUrlParams("?header=app_bar&hideHeader=true", "")).toBe(params); + // expect compute to only be called once + // it will only log when it is computing the values + expect(spy).toHaveBeenCalledExactlyOnceWith( + "UrlParams: final set of url params\n", + "intent:", + "unknown", + "\nproperties:", + expect.any(Object), + "configuration:", + expect.any(Object), + "intentAndPlatformDerivedConfiguration:", + {}, + ); }); }); }); diff --git a/src/UrlParams.ts b/src/UrlParams.ts index 4bd5fced..f6d135d7 100644 --- a/src/UrlParams.ts +++ b/src/UrlParams.ts @@ -322,8 +322,9 @@ let urlParamCache: { hash?: string; params?: UrlParams; } = {}; + /** - * Gets the app parameters for the current URL. + * Gets the url params and loads them from a cache if already computed. * @param search The URL search string * @param hash The URL hash * @returns The app parameters encoded in the URL @@ -331,18 +332,27 @@ let urlParamCache: { export const getUrlParams = ( search = window.location.search, hash = window.location.hash, - /** Skipping the cache might be needed in tests, to allow recomputing based on mocked platform changes. */ - skipCache = false, ): UrlParams => { - // Only run the param configuration if we do not yet have it cached for this url. if ( urlParamCache.search === search && urlParamCache.hash === hash && - urlParamCache.params && - !skipCache + urlParamCache.params ) { return urlParamCache.params; } + const params = computeUrlParams(search, hash); + urlParamCache = { search, hash, params }; + + return params; +}; + +/** + * Gets the app parameters for the current URL. + * @param search The URL search string + * @param hash The URL hash + * @returns The app parameters encoded in the URL + */ +export const computeUrlParams = (search = "", hash = ""): UrlParams => { const parser = new ParamParser(search, hash); const fontScale = parseFloat(parser.getParam("fontScale") ?? ""); @@ -378,7 +388,7 @@ export const getUrlParams = ( controlledAudioDevices: platform === "desktop" ? false : true, skipLobby: true, returnToLobby: false, - sendNotificationType: "notification" as RTCNotificationType, + sendNotificationType: "notification", autoLeaveWhenOthersLeft: false, waitForCallPickup: false, }; @@ -392,6 +402,7 @@ export const getUrlParams = ( break; case UserIntent.StartNewCallDM: intentPreset.skipLobby = true; + intentPreset.sendNotificationType = "ring"; intentPreset.autoLeaveWhenOthersLeft = true; intentPreset.waitForCallPickup = true; @@ -505,15 +516,12 @@ export const getUrlParams = ( intentAndPlatformDerivedConfiguration, ); - const params = { + return { ...properties, ...intentPreset, ...pickBy(configuration, (v?: unknown) => v !== undefined), ...intentAndPlatformDerivedConfiguration, }; - urlParamCache = { search, hash, params }; - - return params; }; /**