From c951174863f2d49546e11c8e2aacaf142604fa34 Mon Sep 17 00:00:00 2001 From: Beatrice Guerra Date: Fri, 18 Oct 2024 09:18:45 +0200 Subject: [PATCH] feat(posthog): disable autocapture and track pageview on location change - Disable autocapture to not send any event anymore. - Disable autocapture of pageview in favor of TrackerPageView, which sends a $pageview event each time the location change. - Move the TrackerProvider under the Router in order to receive updates on the location. - Keep autocapture of pageleave enable. - Configure PostHog client to use POSTHOG_API_HOST env variable if set, and fallback to stat.zextrsa.tools. This configuration allows using a different PostHog instance in development. Be aware that the host env is not set in Jenkins, meaning that for the packages build through it only the key will be set, while the host will always use the fallback. - Move each component/hook in a specific file, under the tracker folder. Refs: SHELL-251 (#529) --- __mocks__/posthog-js/react.tsx | 3 +- carbonio.webpack.ts | 3 +- jest.config.ts | 1 + src/boot/app/app-direct-exports.ts | 3 +- src/boot/app/default-views.ts | 1 - src/boot/bootstrapper.tsx | 18 +++--- src/boot/loader.test.tsx | 6 +- src/boot/loader.tsx | 2 +- src/globals.d.ts | 1 + src/tracker/page-view.test.tsx | 55 +++++++++++++++++++ src/tracker/page-view.tsx | 22 ++++++++ src/tracker/provider.test.tsx | 38 +++++++++++++ src/tracker/provider.tsx | 36 ++++++++++++ .../tracker.test.ts} | 30 +--------- src/{boot/posthog.tsx => tracker/tracker.tsx} | 29 ++-------- src/utility-bar/bar.tsx | 2 +- 16 files changed, 179 insertions(+), 71 deletions(-) create mode 100644 src/tracker/page-view.test.tsx create mode 100644 src/tracker/page-view.tsx create mode 100644 src/tracker/provider.test.tsx create mode 100644 src/tracker/provider.tsx rename src/{boot/posthog.test.tsx => tracker/tracker.test.ts} (86%) rename src/{boot/posthog.tsx => tracker/tracker.tsx} (76%) diff --git a/__mocks__/posthog-js/react.tsx b/__mocks__/posthog-js/react.tsx index 21697b5ab..507782d8e 100644 --- a/__mocks__/posthog-js/react.tsx +++ b/__mocks__/posthog-js/react.tsx @@ -17,7 +17,8 @@ const postHog = { has_opted_in_capturing: (): boolean => false, setPersonProperties: (): void => undefined, set_config: (): void => undefined, - config: {} as PostHogConfig + config: {} as PostHogConfig, + capture: (): undefined => undefined } satisfies Partial>; export const usePostHog: (typeof PostHogReact)['usePostHog'] = () => diff --git a/carbonio.webpack.ts b/carbonio.webpack.ts index 825d5b343..20ae22b63 100644 --- a/carbonio.webpack.ts +++ b/carbonio.webpack.ts @@ -70,7 +70,8 @@ const configFn = ( new DefinePlugin({ COMMIT_ID: JSON.stringify(commitHash.toString().trim()), BASE_PATH: JSON.stringify(baseStaticPath), - POSTHOG_API_KEY: JSON.stringify(process.env.POSTHOG_API_KEY) + POSTHOG_API_KEY: JSON.stringify(process.env.POSTHOG_API_KEY), + POSTHOG_API_HOST: JSON.stringify(process.env.POSTHOG_API_HOST) }), new HtmlWebpackPlugin({ inject: true, diff --git a/jest.config.ts b/jest.config.ts index 17b68733d..d6f3d79a3 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -82,6 +82,7 @@ const config: Config = { // A set of global variables that need to be available in all test environments globals: { BASE_PATH: '', + POSTHOG_API_HOST: '', POSTHOG_API_KEY: '' }, diff --git a/src/boot/app/app-direct-exports.ts b/src/boot/app/app-direct-exports.ts index 6de55263d..11a29f07d 100644 --- a/src/boot/app/app-direct-exports.ts +++ b/src/boot/app/app-direct-exports.ts @@ -117,6 +117,5 @@ export const { export { useIsCarbonioCE } from '../../store/login/hooks'; -export { useTracker } from '../posthog'; - export type { NewAction } from '../../shell/creation-button'; +export { useTracker } from '../../tracker/tracker'; diff --git a/src/boot/app/default-views.ts b/src/boot/app/default-views.ts index de9c8d588..310be908c 100644 --- a/src/boot/app/default-views.ts +++ b/src/boot/app/default-views.ts @@ -3,7 +3,6 @@ * * SPDX-License-Identifier: AGPL-3.0-only */ -/* eslint-disable no-param-reassign */ import { useEffect, useMemo } from 'react'; diff --git a/src/boot/bootstrapper.tsx b/src/boot/bootstrapper.tsx index 0cb61563f..70e75e9e2 100644 --- a/src/boot/bootstrapper.tsx +++ b/src/boot/bootstrapper.tsx @@ -13,13 +13,13 @@ import AppLoaderMounter from './app/app-loader-mounter'; import { DefaultViewsRegister } from './app/default-views'; import { ContextBridge } from './context-bridge'; import { Loader } from './loader'; -import { TrackerProvider } from './posthog'; import { ShellI18nextProvider } from './shell-i18n-provider'; import { ThemeProvider } from './theme-provider'; import { BASENAME, IS_FOCUS_MODE } from '../constants'; import { NotificationPermissionChecker } from '../notification/NotificationPermissionChecker'; import ShellView from '../shell/shell-view'; import { useAppStore } from '../store/app/store'; +import { TrackerProvider } from '../tracker/provider'; const FocusModeListener = (): null => { const { route } = useParams<{ route?: string }>(); @@ -30,10 +30,10 @@ const FocusModeListener = (): null => { }; const Bootstrapper = (): React.JSX.Element => ( - - - - + + + + {IS_FOCUS_MODE && ( @@ -49,10 +49,10 @@ const Bootstrapper = (): React.JSX.Element => ( - - - - + + + + ); export default Bootstrapper; diff --git a/src/boot/loader.test.tsx b/src/boot/loader.test.tsx index 86538a1bf..eaa97ce20 100644 --- a/src/boot/loader.test.tsx +++ b/src/boot/loader.test.tsx @@ -11,7 +11,6 @@ import { EventEmitter } from 'node:events'; import type * as loadAppsModule from './app/load-apps'; import { Loader } from './loader'; -import * as posthog from './posthog'; import { LOGIN_V3_CONFIG_PATH } from '../constants'; import { getGetInfoRequest } from '../mocks/handlers/getInfoRequest'; import server from '../mocks/server'; @@ -20,6 +19,7 @@ import { useLoginConfigStore } from '../store/login/store'; import { TIMERS } from '../tests/constants'; import { spyOnPosthog } from '../tests/posthog-utils'; import { controlConsoleError, setup, screen } from '../tests/utils'; +import * as tracker from '../tracker/tracker'; import type { AccountSettingsPrefs } from '../types/account'; import * as utils from '../utils/utils'; @@ -120,7 +120,7 @@ describe('Loader', () => { ) ); jest - .spyOn(posthog, 'useTracker') + .spyOn(tracker, 'useTracker') .mockReturnValue({ enableTracker: enableTrackerFn, reset: jest.fn(), capture: jest.fn() }); setup( @@ -180,7 +180,7 @@ describe('Loader', () => { ) ); jest - .spyOn(posthog, 'useTracker') + .spyOn(tracker, 'useTracker') .mockReturnValue({ enableTracker: enableTrackerFn, reset: jest.fn(), capture: jest.fn() }); setup( diff --git a/src/boot/loader.tsx b/src/boot/loader.tsx index fa1b3ef13..c80a1c423 100644 --- a/src/boot/loader.tsx +++ b/src/boot/loader.tsx @@ -11,7 +11,6 @@ import { find } from 'lodash'; import { useTranslation } from 'react-i18next'; import { loadApps, unloadAllApps } from './app/load-apps'; -import { useTracker } from './posthog'; import { IS_FOCUS_MODE } from '../constants'; import { getComponents } from '../network/get-components'; import { getInfo } from '../network/get-info'; @@ -20,6 +19,7 @@ import { logout } from '../network/logout'; import { goToLogin } from '../network/utils'; import { useAccountStore } from '../store/account'; import { useAppStore } from '../store/app'; +import { useTracker } from '../tracker/tracker'; export function isPromiseRejectedResult( promiseSettledResult: PromiseSettledResult diff --git a/src/globals.d.ts b/src/globals.d.ts index d7b7d0e93..b0c4ac6e1 100644 --- a/src/globals.d.ts +++ b/src/globals.d.ts @@ -9,6 +9,7 @@ import type { ComponentType } from 'react'; declare global { const BASE_PATH: string; const POSTHOG_API_KEY: string; + const POSTHOG_API_HOST: string; interface Window { __ZAPP_SHARED_LIBRARIES__?: { '@zextras/carbonio-shell-ui': { diff --git a/src/tracker/page-view.test.tsx b/src/tracker/page-view.test.tsx new file mode 100644 index 000000000..8708ac2c5 --- /dev/null +++ b/src/tracker/page-view.test.tsx @@ -0,0 +1,55 @@ +/* + * SPDX-FileCopyrightText: 2024 Zextras + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import React from 'react'; + +import { Link } from 'react-router-dom'; + +import { TrackerPageView } from './page-view'; +import * as useTracker from './tracker'; +import type { Tracker } from './tracker'; +import { screen, setup } from '../tests/utils'; + +describe('TrackerPageView', () => { + it('should capture pageview event when pathname change', async () => { + const tracker: Tracker = { + capture: jest.fn(), + enableTracker: jest.fn(), + reset: jest.fn() + }; + jest.spyOn(useTracker, 'useTracker').mockReturnValue(tracker); + const { user } = setup( + <> + + Go to different path + , + { initialRouterEntries: ['/initial-path'] } + ); + await user.click(screen.getByRole('link')); + expect(tracker.capture).toHaveBeenLastCalledWith('$pageview', { + $current_url: `${window.origin}/different-path` + }); + }); + + it('should capture pageview event when search params change', async () => { + const tracker: Tracker = { + capture: jest.fn(), + enableTracker: jest.fn(), + reset: jest.fn() + }; + jest.spyOn(useTracker, 'useTracker').mockReturnValue(tracker); + const { user } = setup( + <> + + Go to different path + , + { initialRouterEntries: ['/initial-path?param=1'] } + ); + await user.click(screen.getByRole('link')); + expect(tracker.capture).toHaveBeenLastCalledWith('$pageview', { + $current_url: `${window.origin}/initial-path?param=2` + }); + }); +}); diff --git a/src/tracker/page-view.tsx b/src/tracker/page-view.tsx new file mode 100644 index 000000000..9949181c1 --- /dev/null +++ b/src/tracker/page-view.tsx @@ -0,0 +1,22 @@ +/* + * SPDX-FileCopyrightText: 2024 Zextras + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { useEffect } from 'react'; + +import { useLocation } from 'react-router-dom'; + +import { useTracker } from './tracker'; + +export const TrackerPageView = (): null => { + const tracker = useTracker(); + const { pathname, search } = useLocation(); + useEffect(() => { + tracker.capture('$pageview', { + $current_url: window.origin + pathname + search + }); + }, [pathname, search, tracker]); + + return null; +}; diff --git a/src/tracker/provider.test.tsx b/src/tracker/provider.test.tsx new file mode 100644 index 000000000..f3b1d8f7e --- /dev/null +++ b/src/tracker/provider.test.tsx @@ -0,0 +1,38 @@ +/* + * SPDX-FileCopyrightText: 2024 Zextras + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import React from 'react'; + +import * as posthogJsReact from 'posthog-js/react'; +import type * as PostHogReact from 'posthog-js/react'; + +import { TrackerProvider } from './provider'; +import { setup } from '../tests/utils'; +import * as utils from '../utils/utils'; + +beforeEach(() => { + jest.spyOn(utils, 'getCurrentLocationHost').mockReturnValue('differentHost'); +}); + +describe('TrackerProvider', () => { + it('should invoke tracker provider with trackers disabled by default', () => { + const mockProvider = jest.spyOn(posthogJsReact, 'PostHogProvider'); + setup(); + type PostHogProviderProps = React.ComponentPropsWithoutRef< + (typeof PostHogReact)['PostHogProvider'] + >; + expect(mockProvider).toHaveBeenLastCalledWith( + expect.objectContaining({ + options: expect.objectContaining>({ + opt_out_capturing_by_default: true, + disable_session_recording: true, + disable_surveys: true + }) + }), + expect.anything() + ); + }); +}); diff --git a/src/tracker/provider.tsx b/src/tracker/provider.tsx new file mode 100644 index 000000000..21d57b67b --- /dev/null +++ b/src/tracker/provider.tsx @@ -0,0 +1,36 @@ +/* + * SPDX-FileCopyrightText: 2024 Zextras + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import React, { useMemo } from 'react'; + +import type { PostHogConfig } from 'posthog-js'; +import { PostHogProvider } from 'posthog-js/react'; + +import { TrackerPageView } from './page-view'; + +export const TrackerProvider = ({ + children +}: React.PropsWithChildren>): React.JSX.Element => { + const options = useMemo( + (): Partial => ({ + api_host: POSTHOG_API_HOST || 'https://stats.zextras.tools', + person_profiles: 'identified_only', + opt_out_capturing_by_default: true, + disable_session_recording: true, + mask_all_text: true, + disable_surveys: true, + capture_pageview: false, + capture_pageleave: true, + autocapture: false + }), + [] + ); + return ( + + {children} + + + ); +}; diff --git a/src/boot/posthog.test.tsx b/src/tracker/tracker.test.ts similarity index 86% rename from src/boot/posthog.test.tsx rename to src/tracker/tracker.test.ts index 451c38c8e..6e3e1ced4 100644 --- a/src/boot/posthog.test.tsx +++ b/src/tracker/tracker.test.ts @@ -3,27 +3,21 @@ * * SPDX-License-Identifier: AGPL-3.0-only */ - -import React from 'react'; - -import { act, waitFor, renderHook } from '@testing-library/react'; +import { act, renderHook, waitFor } from '@testing-library/react'; import type { CaptureOptions } from 'posthog-js'; -import * as posthogJsReact from 'posthog-js/react'; -import type * as PostHogReact from 'posthog-js/react'; -import { TrackerProvider, useTracker } from './posthog'; +import { useTracker } from './tracker'; import { useAccountStore } from '../store/account'; import { useLoginConfigStore } from '../store/login/store'; import { mockedAccount } from '../tests/account-utils'; import { spyOnPosthog } from '../tests/posthog-utils'; -import { setup } from '../tests/utils'; import * as utils from '../utils/utils'; beforeEach(() => { jest.spyOn(utils, 'getCurrentLocationHost').mockReturnValue('differentHost'); }); -describe('Posthog', () => { +describe('useTracker', () => { it('should opt-in posthog if host is not localhost and enableTracker is called with true value', () => { const posthog = spyOnPosthog(); const { result } = renderHook(() => useTracker()); @@ -67,24 +61,6 @@ describe('Posthog', () => { expect(posthog.capture).toHaveBeenCalledWith(eventName, properties, options); }); - it('should invoke posthog provider with trackers disabled by default', () => { - const mockProvider = jest.spyOn(posthogJsReact, 'PostHogProvider'); - setup(); - type PostHogProviderProps = React.ComponentPropsWithoutRef< - (typeof PostHogReact)['PostHogProvider'] - >; - expect(mockProvider).toHaveBeenLastCalledWith( - expect.objectContaining({ - options: expect.objectContaining>({ - opt_out_capturing_by_default: true, - disable_session_recording: true, - disable_surveys: true - }) - }), - expect.anything() - ); - }); - it('should enable surveys if user is opted in and Carbonio is CE', () => { useLoginConfigStore.setState({ isCarbonioCE: true }); const posthog = spyOnPosthog(); diff --git a/src/boot/posthog.tsx b/src/tracker/tracker.tsx similarity index 76% rename from src/boot/posthog.tsx rename to src/tracker/tracker.tsx index 2724893b5..deadf2fe7 100644 --- a/src/boot/posthog.tsx +++ b/src/tracker/tracker.tsx @@ -3,37 +3,16 @@ * * SPDX-License-Identifier: AGPL-3.0-only */ -import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import { useCallback, useEffect, useState } from 'react'; -import type { CaptureOptions, PostHogConfig, Properties } from 'posthog-js'; -import { PostHogProvider, usePostHog } from 'posthog-js/react'; +import type { CaptureOptions, Properties } from 'posthog-js'; +import { usePostHog } from 'posthog-js/react'; import { useAccountStore } from '../store/account'; import { useIsCarbonioCE } from '../store/login/hooks'; import { getCurrentLocationHost } from '../utils/utils'; -export const TrackerProvider = ({ - children -}: React.PropsWithChildren>): React.JSX.Element => { - const options = useMemo( - (): Partial => ({ - api_host: 'https://stats.zextras.tools', - person_profiles: 'identified_only', - opt_out_capturing_by_default: true, - disable_session_recording: true, - mask_all_text: true, - disable_surveys: true - }), - [] - ); - return ( - - {children} - - ); -}; - -interface Tracker { +export interface Tracker { enableTracker: (enable: boolean) => void; reset: () => void; capture: ( diff --git a/src/utility-bar/bar.tsx b/src/utility-bar/bar.tsx index a77f04889..a7c65f001 100644 --- a/src/utility-bar/bar.tsx +++ b/src/utility-bar/bar.tsx @@ -11,11 +11,11 @@ import { map, noop } from 'lodash'; import { useUtilityBarStore } from './store'; import { useUtilityViews } from './utils'; -import { useTracker } from '../boot/posthog'; import { CUSTOM_EVENTS } from '../constants'; import { logout } from '../network/logout'; import { useAccountStore } from '../store/account'; import { getT } from '../store/i18n/hooks'; +import { useTracker } from '../tracker/tracker'; import type { UtilityView } from '../types/apps'; export interface UtilityBarItemProps {