Add ring notification to UserIntent.StartNewCallDM (#3499)

* Add ring notification to UserIntent.StartNewCallDM

Signed-off-by: Timo K <toger5@hotmail.de>

* Add more tests (refactor to compute + get)

Signed-off-by: Timo K <toger5@hotmail.de>

---------

Signed-off-by: Timo K <toger5@hotmail.de>

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")
+        computeUrlParams(
-          .returnToLobby,
+          "?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:",
+        {},
+      );
     });
   });
 });

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 &&
+    urlParamCache.params
-    !skipCache
   ) {
     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;
 };
 
 /**