Merge pull request #3053 from element-hq/robin/posthog-logout

Enable analytics only while authenticated

src/ClientContext.tsx

@@ -50,7 +50,7 @@ export type ValidClientState = {
     reactions: boolean;
     thumbnails: boolean;
   };
-  setClient: (params?: SetClientParams) => void;
+  setClient: (client: MatrixClient, session: Session) => void;
 };
 
 export type AuthenticatedClient = {
@@ -65,11 +65,6 @@ export type ErrorState = {
   error: Error;
 };
 
-export type SetClientParams = {
-  client: MatrixClient;
-  session: Session;
-};
-
 const ClientContext = createContext<ClientState | undefined>(undefined);
 
 export const ClientContextProvider = ClientContext.Provider;
@@ -79,7 +74,7 @@ export const useClientState = (): ClientState | undefined =>
 
 export function useClient(): {
   client?: MatrixClient;
-  setClient?: (params?: SetClientParams) => void;
+  setClient?: (client: MatrixClient, session: Session) => void;
 } {
   let client;
   let setClient;
@@ -96,7 +91,7 @@ export function useClient(): {
 // Plain representation of the `ClientContext` as a helper for old components that expected an object with multiple fields.
 export function useClientLegacy(): {
   client?: MatrixClient;
-  setClient?: (params?: SetClientParams) => void;
+  setClient?: (client: MatrixClient, session: Session) => void;
   passwordlessUser: boolean;
   loading: boolean;
   authenticated: boolean;
@@ -160,7 +155,11 @@ export const ClientProvider: FC<Props> = ({ children }) => {
     initializing.current = true;
 
     loadClient()
-      .then(setInitClientState)
+      .then((initResult) => {
+        setInitClientState(initResult);
+        if (PosthogAnalytics.instance.isEnabled())
+          PosthogAnalytics.instance.startListeningToSettingsChanges();
+      })
       .catch((err) => logger.error(err))
       .finally(() => (initializing.current = false));
   }, []);
@@ -196,24 +195,20 @@ export const ClientProvider: FC<Props> = ({ children }) => {
   );
 
   const setClient = useCallback(
-    (clientParams?: SetClientParams) => {
+    (client: MatrixClient, session: Session) => {
       const oldClient = initClientState?.client;
-      const newClient = clientParams?.client;
-      if (oldClient && oldClient !== newClient) {
+      if (oldClient && oldClient !== client) {
         oldClient.stopClient();
       }
 
-      if (clientParams) {
-        saveSession(clientParams.session);
-        setInitClientState({
-          widgetApi: null,
-          client: clientParams.client,
-          passwordlessUser: clientParams.session.passwordlessUser,
-        });
-      } else {
-        clearSession();
-        setInitClientState(null);
-      }
+      saveSession(session);
+      setInitClientState({
+        widgetApi: null,
+        client,
+        passwordlessUser: session.passwordlessUser,
+      });
+      if (PosthogAnalytics.instance.isEnabled())
+        PosthogAnalytics.instance.startListeningToSettingsChanges();
     },
     [initClientState?.client],
   );
@@ -229,6 +224,7 @@ export const ClientProvider: FC<Props> = ({ children }) => {
     clearSession();
     setInitClientState(null);
     await navigate("/");
+    PosthogAnalytics.instance.logout();
     PosthogAnalytics.instance.setRegistrationType(RegistrationType.Guest);
   }, [navigate, initClientState?.client]);
 

src/analytics/PosthogAnalytics.ts

@@ -13,6 +13,7 @@ import posthog, {
 import { logger } from "matrix-js-sdk/src/logger";
 import { type MatrixClient } from "matrix-js-sdk/src/matrix";
 import { Buffer } from "buffer";
+import { type Subscription } from "rxjs";
 
 import { widget } from "../widget";
 import {
@@ -96,6 +97,7 @@ export class PosthogAnalytics {
   private anonymity = Anonymity.Disabled;
   private platformSuperProperties = {};
   private registrationType: RegistrationType = RegistrationType.Guest;
+  private optInListener: Subscription | null = null;
 
   public static hasInstance(): boolean {
     return Boolean(this.internalInstance);
@@ -144,7 +146,6 @@ export class PosthogAnalytics {
       );
       this.enabled = false;
     }
-    this.startListeningToSettingsChanges(); // Triggers maybeIdentifyUser
   }
 
   private sanitizeProperties = (
@@ -326,6 +327,8 @@ export class PosthogAnalytics {
     if (this.enabled) {
       this.posthog.reset();
     }
+    this.optInListener?.unsubscribe();
+    this.optInListener = null;
     this.setAnonymity(Anonymity.Disabled);
   }
 
@@ -404,7 +407,7 @@ export class PosthogAnalytics {
     }
   }
 
-  private startListeningToSettingsChanges(): void {
+  public startListeningToSettingsChanges(): void {
     // Listen to account data changes from sync so we can observe changes to relevant flags and update.
     // This is called -
     //  * On page load, when the account data is first received by sync
@@ -413,7 +416,7 @@ export class PosthogAnalytics {
     //  * When the user changes their preferences on this device
     // Note that for new accounts, pseudonymousAnalyticsOptIn won't be set, so updateAnonymityFromSettings
     // won't be called (i.e. this.anonymity will be left as the default, until the setting changes)
-    optInAnalytics.value$.subscribe((optIn) => {
+    this.optInListener ??= optInAnalytics.value$.subscribe((optIn) => {
       this.setAnonymity(optIn ? Anonymity.Pseudonymous : Anonymity.Disabled);
       this.maybeIdentifyUser().catch(() =>
         logger.log("Could not identify user"),

src/auth/LoginPage.tsx

@@ -53,7 +53,7 @@ export const LoginPage: FC = () => {
             return;
           }
 
-          setClient({ client, session });
+          setClient(client, session);
 
           const locationState = location.state;
           // eslint-disable-next-line @typescript-eslint/ban-ts-comment

src/auth/RegisterPage.tsx

@@ -95,7 +95,7 @@ export const RegisterPage: FC = () => {
           }
         }
 
-        setClient?.({ client: newClient, session });
+        setClient?.(newClient, session);
         PosthogAnalytics.instance.eventSignup.cacheSignupEnd(new Date());
       };
 

src/auth/useRegisterPasswordlessUser.ts

@@ -47,7 +47,7 @@ export function useRegisterPasswordlessUser(): UseRegisterPasswordlessUserType {
           recaptchaResponse,
           true,
         );
-        setClient({ client, session });
+        setClient(client, session);
       } catch (e) {
         reset();
         throw e;

src/home/UnauthenticatedView.tsx

@@ -89,7 +89,7 @@ export const UnauthenticatedView: FC = () => {
           // @ts-ignore
           if (error.errcode === "M_ROOM_IN_USE") {
             setOnFinished(() => {
-              setClient({ client, session });
+              setClient(client, session);
               const aliasLocalpart = roomAliasLocalpartFromRoomName(roomName);
               navigate(`/${aliasLocalpart}`)?.catch((error) => {
                 logger.error("Failed to navigate to alias localpart", error);
@@ -111,7 +111,7 @@ export const UnauthenticatedView: FC = () => {
         if (!createRoomResult.password)
           throw new Error("Failed to create room with shared secret");
 
-        setClient({ client, session });
+        setClient(client, session);
         await navigate(
           getRelativeRoomUrl(
             createRoomResult.roomId,

src/rtcSessionHelpers.ts

@@ -166,7 +166,6 @@ const widgetPostHangupProcedure = async (
       logger.error("Failed to send close action", e);
     }
     widget.api.transport.stop();
-    PosthogAnalytics.instance.logout();
   }
 };