UrlParams: Intent system update, split into configuration and propreties (#3376)

* refactor UrlParams to use a preset intent system

* change defaults for intend headers

* add: getEnumParam to ParamParser

* remove deprecated url params

* only allow skip lobby in widget (more strict needs test adjustment)

* fix tests that now require the url to be a widget url

Co-authored-by: Robin <robin@robin.town>
---------

Co-authored-by: Robin <robin@robin.town>

src/UrlParams.test.ts

@@ -10,7 +10,7 @@ import { describe, expect, it } from "vitest";
 import {
   getRoomIdentifierFromUrl,
   getUrlParams,
-  UserIntent,
+  HeaderStyle,
 } from "../src/UrlParams";
 
 const ROOM_NAME = "roomNameHere";
@@ -211,24 +211,68 @@ describe("UrlParams", () => {
   });
 
   describe("intent", () => {
-    it("defaults to unknown", () => {
+    const noIntentDefaults = {
-      expect(getUrlParams().intent).toBe(UserIntent.Unknown);
+      confineToRoom: false,
+      appPrompt: true,
+      preload: false,
+      header: HeaderStyle.Standard,
+      showControls: true,
+      hideScreensharing: false,
+      allowIceFallback: false,
+      perParticipantE2EE: false,
+      controlledAudioDevices: false,
+      skipLobby: false,
+      returnToLobby: false,
+      sendNotificationType: undefined,
+    };
+    const startNewCallDefaults = (platform: string): object => ({
+      confineToRoom: true,
+      appPrompt: false,
+      preload: true,
+      header: platform === "desktop" ? HeaderStyle.None : HeaderStyle.AppBar,
+      showControls: true,
+      hideScreensharing: false,
+      allowIceFallback: true,
+      perParticipantE2EE: true,
+      controlledAudioDevices: platform === "desktop" ? false : true,
+      skipLobby: true,
+      returnToLobby: false,
+      sendNotificationType: "notification",
+    });
+    const joinExistingCallDefaults = (platform: string): object => ({
+      confineToRoom: true,
+      appPrompt: false,
+      preload: true,
+      header: platform === "desktop" ? HeaderStyle.None : HeaderStyle.AppBar,
+      showControls: true,
+      hideScreensharing: false,
+      allowIceFallback: true,
+      perParticipantE2EE: true,
+      controlledAudioDevices: platform === "desktop" ? false : true,
+      skipLobby: false,
+      returnToLobby: false,
+      sendNotificationType: "notification",
+    });
+    it("use no-intent-defaults with unknown intent", () => {
+      expect(getUrlParams()).toMatchObject(noIntentDefaults);
     });
 
     it("ignores intent if it is not a valid value", () => {
-      expect(getUrlParams("?intent=foo").intent).toBe(UserIntent.Unknown);
+      expect(getUrlParams("?intent=foo")).toMatchObject(noIntentDefaults);
     });
 
     it("accepts start_call", () => {
-      expect(getUrlParams("?intent=start_call").intent).toBe(
+      expect(
-        UserIntent.StartNewCall,
+        getUrlParams("?intent=start_call&widgetId=1234&parentUrl=parent.org"),
-      );
+      ).toMatchObject(startNewCallDefaults("desktop"));
     });
 
     it("accepts join_existing", () => {
-      expect(getUrlParams("?intent=join_existing").intent).toBe(
+      expect(
-        UserIntent.JoinExistingCall,
+        getUrlParams(
-      );
+          "?intent=join_existing&widgetId=1234&parentUrl=parent.org",
+        ),
+      ).toMatchObject(joinExistingCallDefaults("desktop"));
     });
   });
 
@@ -260,9 +304,5 @@ describe("UrlParams", () => {
       );
       expect(getUrlParams("?header=none&hideHeader=false").header).toBe("none");
     });
-    it("converts hideHeader to the correct header value", () => {
-      expect(getUrlParams("?hideHeader=true").header).toBe("none");
-      expect(getUrlParams("?hideHeader=false").header).toBe("standard");
-    });
   });
 });

src/UrlParams.ts

@@ -9,10 +9,12 @@ import { useMemo } from "react";
 import { useLocation } from "react-router-dom";
 import { logger } from "matrix-js-sdk/lib/logger";
 import { type RTCNotificationType } from "matrix-js-sdk/lib/matrixrtc";
+import { pickBy } from "lodash-es";
 
 import { Config } from "./config/Config";
 import { type EncryptionSystem } from "./e2ee/sharedKeyManagement";
 import { E2eeType } from "./e2ee/e2eeType";
+import { platform } from "./Platform";
 
 interface RoomIdentifier {
   roomAlias: string | null;
@@ -21,6 +23,7 @@ interface RoomIdentifier {
 }
 
 export enum UserIntent {
+  // TODO: add DM vs room call
   StartNewCall = "start_call",
   JoinExistingCall = "join_existing",
   Unknown = "unknown",
@@ -32,12 +35,12 @@ export enum HeaderStyle {
   AppBar = "app_bar",
 }
 
-// If you need to add a new flag to this interface, prefer a name that describes
+/**
-// a specific behavior (such as 'confineToRoom'), rather than one that describes
+ * The UrlProperties are used to pass required data to the widget.
-// the situations that call for this behavior ('isEmbedded'). This makes it
+ * Those are different in different rooms, users, devices. They do not configure the behavior of the
-// clearer what each flag means, and helps us avoid coupling Element Call's
+ * widget but provide the required data to the widget.
-// behavior to the needs of specific consumers.
+ */
-export interface UrlParams {
+export interface UrlProperties {
   // Widget api related params
   widgetId: string | null;
   parentUrl: string | null;
@@ -49,45 +52,11 @@ export interface UrlParams {
    * is also not validated, where it is in useRoomIdentifier().
    */
   roomId: string | null;
-  /**
-   * Whether the app should keep the user confined to the current call/room.
-   */
-  confineToRoom: boolean;
-  /**
-   * Whether upon entering a room, the user should be prompted to launch the
-   * native mobile app. (Affects only Android and iOS.)
-   *
-   * The app prompt must also be enabled in the config for this to take effect.
-   */
-  appPrompt: boolean;
-  /**
-   * Whether the app should pause before joining the call until it sees an
-   * io.element.join widget action, allowing it to be preloaded.
-   */
-  preload: boolean;
-  /**
-   * The style of headers to show. "standard" is the default arrangement, "none"
-   * hides the header entirely, and "app_bar" produces a header with a back
-   * button like you might see in mobile apps. The callback for the back button
-   * is window.controls.onBackButtonPressed.
-   */
-  header: HeaderStyle;
-  /**
-   * Whether the controls should be shown. For screen recording no controls can be desired.
-   */
-  showControls: boolean;
-  /**
-   * Whether to hide the screen-sharing button.
-   */
-  hideScreensharing: boolean;
-  /**
-   * Whether to use end-to-end encryption.
-   */
-  e2eEnabled: boolean;
   /**
    * The user's ID (only used in matryoshka mode).
    */
   userId: string | null;
+
   /**
    * The display name to use for auto-registration.
    */
@@ -125,14 +94,96 @@ export interface UrlParams {
    */
   posthogApiKey: string | null;
   /**
-   * Whether the app is allowed to use fallback STUN servers for ICE in case the
+   * Whether to use end-to-end encryption.
-   * user's homeserver doesn't provide any.
    */
-  allowIceFallback: boolean;
+  e2eEnabled: boolean;
   /**
    * E2EE password
    */
   password: string | null;
+  /** This defines the homeserver that is going to be used when joining a room.
+   * It has to be set to a non default value for links to rooms
+   * that are not on the default homeserver,
+   * that is in use for the current user.
+   */
+  viaServers: string | null;
+
+  /**
+   * This defines the homeserver that is going to be used when registering
+   * a new (guest) user.
+   * This can be user to configure a non default guest user server when
+   * creating a spa link.
+   */
+  homeserver: string | null;
+
+  /**
+   * The rageshake submit URL. This is only used in the embedded package of Element Call.
+   */
+  rageshakeSubmitUrl: string | null;
+
+  /**
+   * The Sentry DSN. This is only used in the embedded package of Element Call.
+   */
+  sentryDsn: string | null;
+
+  /**
+   * The Sentry environment. This is only used in the embedded package of Element Call.
+   */
+  sentryEnvironment: string | null;
+  /**
+   * The theme to use for element call.
+   * can be "light", "dark", "light-high-contrast" or "dark-high-contrast".
+   */
+  theme: string | null;
+}
+
+/**
+ * The configuration for the app, which can be set via URL parameters.
+ * Those property are different to the UrlProperties, since they are all optional
+ * and configure the behavior of the app. Their value is the same if EC is used in
+ * the same context but with different accounts/users.
+ *
+ * Their defaults can be controlled by the `intent` property.
+ */
+export interface UrlConfiguration {
+  /**
+   * Whether the app should keep the user confined to the current call/room.
+   */
+  confineToRoom: boolean;
+  /**
+   * Whether upon entering a room, the user should be prompted to launch the
+   * native mobile app. (Affects only Android and iOS.)
+   *
+   * The app prompt must also be enabled in the config for this to take effect.
+   */
+  appPrompt: boolean;
+  /**
+   * Whether the app should pause before joining the call until it sees an
+   * io.element.join widget action, allowing it to be preloaded.
+   */
+  preload: boolean;
+  /**
+   * The style of headers to show. "standard" is the default arrangement, "none"
+   * hides the header entirely, and "app_bar" produces a header with a back
+   * button like you might see in mobile apps. The callback for the back button
+   * is window.controls.onBackButtonPressed.
+   */
+  header: HeaderStyle;
+  /**
+   * Whether the controls should be shown. For screen recording no controls can be desired.
+   */
+  showControls: boolean;
+  /**
+   * Whether to hide the screen-sharing button.
+   */
+  hideScreensharing: boolean;
+
+  /**
+   * Whether the app is allowed to use fallback STUN servers for ICE in case the
+   * user's homeserver doesn't provide any.
+   */
+  allowIceFallback: boolean;
+
   /**
    * Whether the app should use per participant keys for E2EE.
    */
@@ -154,52 +205,19 @@ export interface UrlParams {
    * This is useful for video rooms.
    */
   returnToLobby: boolean;
-  /**
-   * The theme to use for element call.
-   * can be "light", "dark", "light-high-contrast" or "dark-high-contrast".
-   */
-  theme: string | null;
-  /** This defines the homeserver that is going to be used when joining a room.
-   * It has to be set to a non default value for links to rooms
-   * that are not on the default homeserver,
-   * that is in use for the current user.
-   */
-  viaServers: string | null;
-  /**
-   * This defines the homeserver that is going to be used when registering
-   * a new (guest) user.
-   * This can be user to configure a non default guest user server when
-   * creating a spa link.
-   */
-  homeserver: string | null;
-
-  /**
-   * The user's intent with respect to the call.
-   * e.g. if they clicked a Start Call button, this would be `start_call`.
-   * If it was a Join Call button, it would be `join_existing`.
-   */
-  intent: string | null;
-
-  /**
-   * The rageshake submit URL. This is only used in the embedded package of Element Call.
-   */
-  rageshakeSubmitUrl: string | null;
-
-  /**
-   * The Sentry DSN. This is only used in the embedded package of Element Call.
-   */
-  sentryDsn: string | null;
-
-  /**
-   * The Sentry environment. This is only used in the embedded package of Element Call.
-   */
-  sentryEnvironment: string | null;
   /**
    * Whether and what type of notification EC should send, when the user joins the call.
    */
   sendNotificationType?: RTCNotificationType;
 }
 
+// If you need to add a new flag to this interface, prefer a name that describes
+// a specific behavior (such as 'confineToRoom'), rather than one that describes
+// the situations that call for this behavior ('isEmbedded'). This makes it
+// clearer what each flag means, and helps us avoid coupling Element Call's
+// behavior to the needs of specific consumers.
+export interface UrlParams extends UrlProperties, UrlConfiguration {}
+
 // This is here as a stopgap, but what would be far nicer is a function that
 // takes a UrlParams and returns a query string. That would enable us to
 // consolidate all the data about URL parameters and their meanings to this one
@@ -240,6 +258,17 @@ class ParamParser {
     return this.fragmentParams.get(name) ?? this.queryParams.get(name);
   }
 
+  public getEnumParam<T extends string>(
+    name: string,
+    type: { [s: string]: T } | ArrayLike<T>,
+  ): T | undefined {
+    const value = this.getParam(name);
+    if (value !== null && Object.values(type).includes(value as T)) {
+      return value as T;
+    }
+    return undefined;
+  }
+
   public getAllParams(name: string): string[] {
     return [
       ...this.fragmentParams.getAll(name),
@@ -251,6 +280,10 @@ class ParamParser {
     const param = this.getParam(name);
     return param === null ? defaultValue : param !== "false";
   }
+  public getFlag(name: string): boolean | undefined {
+    const param = this.getParam(name);
+    return param !== null ? param !== "false" : undefined;
+  }
 }
 
 /**
@@ -267,46 +300,79 @@ export const getUrlParams = (
 
   const fontScale = parseFloat(parser.getParam("fontScale") ?? "");
 
-  let intent = parser.getParam("intent");
-  if (!intent || !Object.values(UserIntent).includes(intent as UserIntent)) {
-    intent = UserIntent.Unknown;
-  }
-
-  // Check hideHeader for backwards compatibility. If header is set, hideHeader
-  // is ignored.
-  const header =
-    parser.getParam("header") ??
-    (parser.getFlagParam("hideHeader")
-      ? HeaderStyle.None
-      : HeaderStyle.Standard);
-
-  const sendNotificationType = ["ring", "notification"].includes(
-    parser.getParam("sendNotificationType") ?? "",
-  )
-    ? (parser.getParam("sendNotificationType") as RTCNotificationType)
-    : undefined;
   const widgetId = parser.getParam("widgetId");
   const parentUrl = parser.getParam("parentUrl");
   const isWidget = !!widgetId && !!parentUrl;
 
-  return {
+  /**
+   * The user's intent with respect to the call.
+   * e.g. if they clicked a Start Call button, this would be `start_call`.
+   * If it was a Join Call button, it would be `join_existing`.
+   * This is a platform specific default set of parameters, that allows to minize the configuration
+   * needed to start a call. And empowers the EC codebase to control the platform/intent behavior in
+   * a central place.
+   *
+   * In short: either provide url query parameters of UrlConfiguration or set the intent
+   * (or the global defaults will be used).
+   */
+  const intent = !isWidget
+    ? UserIntent.Unknown
+    : (parser.getEnumParam("intent", UserIntent) ?? UserIntent.Unknown);
+  // Here we only use constants and `platform` to determine the intent preset.
+  let intentPreset: UrlConfiguration;
+  const inAppDefault = {
+    confineToRoom: true,
+    appPrompt: false,
+    preload: true,
+    header: platform === "desktop" ? HeaderStyle.None : HeaderStyle.AppBar,
+    showControls: true,
+    hideScreensharing: false,
+    allowIceFallback: true,
+    perParticipantE2EE: true,
+    controlledAudioDevices: platform === "desktop" ? false : true,
+    skipLobby: true,
+    returnToLobby: false,
+    sendNotificationType: "notification" as RTCNotificationType,
+  };
+  switch (intent) {
+    case UserIntent.StartNewCall:
+      intentPreset = {
+        ...inAppDefault,
+        skipLobby: true,
+      };
+      break;
+    case UserIntent.JoinExistingCall:
+      intentPreset = {
+        ...inAppDefault,
+        skipLobby: false,
+      };
+      break;
+    // Non widget usecase defaults
+    default:
+      intentPreset = {
+        confineToRoom: false,
+        appPrompt: true,
+        preload: false,
+        header: HeaderStyle.Standard,
+        showControls: true,
+        hideScreensharing: false,
+        allowIceFallback: false,
+        perParticipantE2EE: false,
+        controlledAudioDevices: false,
+        skipLobby: false,
+        returnToLobby: false,
+        sendNotificationType: undefined,
+      };
+  }
+
+  const properties: UrlProperties = {
     widgetId,
     parentUrl,
-
     // NB. we don't validate roomId here as we do in getRoomIdentifierFromUrl:
     // what would we do if it were invalid? If the widget API says that's what
     // the room ID is, then that's what it is.
     roomId: parser.getParam("roomId"),
     password: parser.getParam("password"),
-    // This flag has 'embed' as an alias for historical reasons
-    confineToRoom:
-      parser.getFlagParam("confineToRoom") || parser.getFlagParam("embed"),
-    appPrompt: parser.getFlagParam("appPrompt", true),
-    preload: isWidget ? parser.getFlagParam("preload") : false,
-    header: header as HeaderStyle,
-    showControls: parser.getFlagParam("showControls", true),
-    hideScreensharing: parser.getFlagParam("hideScreensharing"),
-    e2eEnabled: parser.getFlagParam("enableE2EE", true),
     userId: isWidget ? parser.getParam("userId") : null,
     displayName: parser.getParam("displayName"),
     deviceId: isWidget ? parser.getParam("deviceId") : null,
@@ -314,24 +380,9 @@ export const getUrlParams = (
     lang: parser.getParam("lang"),
     fonts: parser.getAllParams("font"),
     fontScale: Number.isNaN(fontScale) ? null : fontScale,
-    allowIceFallback: parser.getFlagParam("allowIceFallback"),
-    perParticipantE2EE: parser.getFlagParam("perParticipantE2EE"),
-    controlledAudioDevices: parser.getFlagParam(
-      "controlledAudioDevices",
-      // the deprecated property name
-      parser.getFlagParam("controlledMediaDevices"),
-    ),
-    skipLobby: parser.getFlagParam(
-      "skipLobby",
-      isWidget && intent === UserIntent.StartNewCall,
-    ),
-    // In SPA mode the user should always exit to the home screen when hanging
-    // up, rather than being sent back to the lobby
-    returnToLobby: isWidget ? parser.getFlagParam("returnToLobby") : false,
     theme: parser.getParam("theme"),
     viaServers: !isWidget ? parser.getParam("viaServers") : null,
     homeserver: !isWidget ? parser.getParam("homeserver") : null,
-    intent,
     posthogApiHost: parser.getParam("posthogApiHost"),
     posthogApiKey: parser.getParam("posthogApiKey"),
     posthogUserId:
@@ -339,7 +390,35 @@ export const getUrlParams = (
     rageshakeSubmitUrl: parser.getParam("rageshakeSubmitUrl"),
     sentryDsn: parser.getParam("sentryDsn"),
     sentryEnvironment: parser.getParam("sentryEnvironment"),
-    sendNotificationType,
+    e2eEnabled: parser.getFlagParam("enableE2EE", true),
+  };
+
+  const configuration: Partial<UrlConfiguration> = {
+    confineToRoom: parser.getFlag("confineToRoom"),
+    appPrompt: parser.getFlag("appPrompt"),
+    preload: isWidget ? parser.getFlag("preload") : undefined,
+    // Check hideHeader for backwards compatibility. If header is set, hideHeader
+    // is ignored.
+    header: parser.getEnumParam("header", HeaderStyle),
+    showControls: parser.getFlag("showControls"),
+    hideScreensharing: parser.getFlag("hideScreensharing"),
+    allowIceFallback: parser.getFlag("allowIceFallback"),
+    perParticipantE2EE: parser.getFlag("perParticipantE2EE"),
+    controlledAudioDevices: parser.getFlag("controlledAudioDevices"),
+    skipLobby: isWidget ? parser.getFlag("skipLobby") : false,
+    // In SPA mode the user should always exit to the home screen when hanging
+    // up, rather than being sent back to the lobby
+    returnToLobby: isWidget ? parser.getFlag("returnToLobby") : false,
+    sendNotificationType: parser.getEnumParam("sendNotificationType", [
+      "ring",
+      "notification",
+    ]),
+  };
+
+  return {
+    ...properties,
+    ...intentPreset,
+    ...pickBy(configuration, (v) => v !== undefined),
   };
 };
 

src/room/MuteStates.test.tsx

@@ -191,7 +191,11 @@ describe("useMuteStates", () => {
     mockConfig();
 
     render(
-      <MemoryRouter initialEntries={["/room/?skipLobby=true"]}>
+      <MemoryRouter
+        initialEntries={[
+          "/room/?skipLobby=true&widgetId=1234&parentUrl=www.parent.org",
+        ]}
+      >
         <MediaDevicesContext value={mockMediaDevices()}>
           <TestComponent />
         </MediaDevicesContext>