From cf6891d528a2b063e335832251f5bdbfcbd4f190 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Wed, 2 Oct 2024 14:52:22 -0500 Subject: [PATCH 1/4] Reimplementing PostHog support for the Next.JS version of this app - Now pull the API key and host from `process.env` - Now pull API host - we weren't doing this before; you probably don't want to specify this, but if you want to self-host Pos tHog (it is an option!) you'd need to be able to specify this. - Roll feature flag processing into the new Webpack Config - Minor update to the LearningResourceDrawer to make the custom `lrd_view` hook there work Going forward, things that use PostHog through the usePostHog hook (or one of their components) should work as expected. (The lrd_view one did some checking to make sure PostHog was turned on, hence it needed a bit more work.) --- frontends/main/next.config.js | 17 +++++++ frontends/main/src/app/providers.tsx | 23 ++++++++- .../LearningResourceDrawer.tsx | 50 +++++++++---------- frontends/main/validateEnv.js | 5 ++ 4 files changed, 68 insertions(+), 27 deletions(-) diff --git a/frontends/main/next.config.js b/frontends/main/next.config.js index 44af6220cc..8942cd4596 100644 --- a/frontends/main/next.config.js +++ b/frontends/main/next.config.js @@ -3,6 +3,22 @@ const { validateEnv } = require("./validateEnv") validateEnv() +const processFeatureFlags = () => { + const featureFlagPrefix = + process.env.NEXT_PUBLIC_POSTHOG_FEATURE_PREFIX || "FEATURE_" + const bootstrapFeatureFlags = {} + + for (const [key, value] of Object.entries(process.env)) { + if (key.startsWith(`NEXT_PUBLIC_${featureFlagPrefix}`)) { + bootstrapFeatureFlags[ + key.replace(`NEXT_PUBLIC_${featureFlagPrefix}`, "") + ] = value === "True" ? true : JSON.stringify(value) + } + } + + return { FEATURE_FLAGS: JSON.stringify(bootstrapFeatureFlags) } +} + /** @type {import('next').NextConfig} */ const nextConfig = { webpack: (config, { webpack }) => { @@ -13,6 +29,7 @@ const nextConfig = { new webpack.IgnorePlugin({ resourceRegExp: /mockAxios\.ts/, }), + new webpack.EnvironmentPlugin(processFeatureFlags()), ) // Do not do this. Added to fix "import type", but causes a strage issue where diff --git a/frontends/main/src/app/providers.tsx b/frontends/main/src/app/providers.tsx index 80b9632231..000a14deef 100644 --- a/frontends/main/src/app/providers.tsx +++ b/frontends/main/src/app/providers.tsx @@ -5,11 +5,24 @@ import { getQueryClient } from "./getQueryClient" import { QueryClientProvider } from "@tanstack/react-query" import { ThemeProvider, NextJsAppRouterCacheProvider } from "ol-components" import { Provider as NiceModalProvider } from "@ebay/nice-modal-react" +import { PostHogProvider } from "posthog-js/react" export default function Providers({ children }: { children: React.ReactNode }) { const queryClient = getQueryClient() - return ( + const apiKey = process.env.NEXT_PUBLIC_POSTHOG_PROJECT_API_KEY || "" + const apiHost = + process.env.NEXT_PUBLIC_POSTHOG_API_HOST || "https://us.i.posthog.com" + const featureFlags = JSON.parse(process.env.FEATURE_FLAGS || "") + + const postHogOptions = { + api_host: apiHost, + bootstrap: { + featureFlags: featureFlags, + }, + } + + const interiorElements = ( @@ -18,4 +31,12 @@ export default function Providers({ children }: { children: React.ReactNode }) { ) + + return apiKey.length > 0 ? ( + + {interiorElements} + + ) : ( + interiorElements + ) } diff --git a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.tsx b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.tsx index 4d6ba70e75..fbc8f2d857 100644 --- a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.tsx +++ b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.tsx @@ -1,4 +1,4 @@ -import React, { Suspense, useCallback, useMemo } from "react" +import React, { Suspense, useCallback, useEffect, useMemo } from "react" import { RoutedDrawer, LearningResourceExpanded, @@ -23,38 +23,35 @@ import { AddToUserListDialog, } from "../Dialogs/AddToListDialog" import { SignupPopover } from "../SignupPopover/SignupPopover" +import { usePostHog } from "posthog-js/react" const RESOURCE_DRAWER_PARAMS = [RESOURCE_DRAWER_QUERY_PARAM] as const -/* const useCapturePageView = (resourceId: number) => { const { data, isSuccess } = useLearningResourcesDetail(Number(resourceId)) const posthog = usePostHog() - - // TODO Provide POSTHOG env vars - - // const { POSTHOG } = APP_SETTINGS - - // useEffect(() => { - // if (!POSTHOG?.api_key || POSTHOG.api_key.length < 1) return - // if (!isSuccess) return - // posthog.capture("lrd_view", { - // resourceId: data?.id, - // readableId: data?.readable_id, - // platformCode: data?.platform?.code, - // resourceType: data?.resource_type, - // }) - // }, [ - // isSuccess, - // posthog, - // data?.id, - // data?.readable_id, - // data?.platform?.code, - // data?.resource_type, - // POSTHOG?.api_key, - // ]) + const apiKey = process.env.NEXT_PUBLIC_POSTHOG_PROJECT_API_KEY + + useEffect(() => { + if (!apiKey || apiKey.length < 1) return + if (!isSuccess) return + posthog.capture("lrd_view", { + resourceId: data?.id, + readableId: data?.readable_id, + platformCode: data?.platform?.code, + resourceType: data?.resource_type, + }) + }, [ + isSuccess, + posthog, + data?.id, + data?.readable_id, + data?.platform?.code, + data?.resource_type, + apiKey, + ]) } -*/ + /** * Convert HTML to plaintext, removing any HTML tags. * This conversion method has some issues: @@ -93,6 +90,7 @@ const DrawerContent: React.FC<{ NiceModal.show(AddToUserListDialog, { resourceId }) } }, [user]) + useCapturePageView(Number(resourceId)) return ( <> diff --git a/frontends/main/validateEnv.js b/frontends/main/validateEnv.js index 74f01e2cf7..c0326fe082 100644 --- a/frontends/main/validateEnv.js +++ b/frontends/main/validateEnv.js @@ -21,6 +21,11 @@ const schema = yup.object().shape({ .string() .oneOf(["true", "false"]), NEXT_PUBLIC_CSRF_COOKIE_NAME: yup.string().required(), + NEXT_PUBLIC_POSTHOG_PROJECT_ID: yup.string(), + NEXT_PUBLIC_POSTHOG_PERSONAL_API_KEY: yup.string(), + NEXT_PUBLIC_POSTHOG_PROJECT_API_KEY: yup.string(), + NEXT_PUBLIC_POSTHOG_FEATURE_PREFIX: yup.string(), + NEXT_PUBLIC_POSTHOG_API_HOST: yup.string(), }) const validateEnv = () => schema.validateSync(process.env) From bf97da6829e9719da21acd47853587b0895a968f Mon Sep 17 00:00:00 2001 From: James Kachel Date: Wed, 2 Oct 2024 15:00:31 -0500 Subject: [PATCH 2/4] fixing test --- .../LearningResourceDrawer.test.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.test.tsx b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.test.tsx index 97eaa142d3..f09800d5b4 100644 --- a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.test.tsx +++ b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.test.tsx @@ -37,17 +37,16 @@ jest.mock("posthog-js/react", () => ({ })) describe("LearningResourceDrawer", () => { - it.skip.each([ + it.each([ { descriptor: "is enabled", enablePostHog: true }, { descriptor: "is not enabled", enablePostHog: false }, ])( "Renders drawer content when resource=id is in the URL and captures the view if PostHog $descriptor", async ({ enablePostHog }) => { setMockResponse.get(urls.userMe.get(), {}) - // @ts-expect-error reinstante posthog - APP_SETTINGS.POSTHOG = { - api_key: enablePostHog ? "test1234" : "", // pragma: allowlist secret - } + process.env.NEXT_PUBLIC_POSTHOG_PROJECT_API_KEY = enablePostHog + ? "12345abcdef" // pragma: allowlist secret + : "" const resource = factories.learningResources.resource() setMockResponse.get( urls.learningResources.details({ id: resource.id }), From b6805f8c995347a5f6685fa667cf0630b27628bc Mon Sep 17 00:00:00 2001 From: James Kachel Date: Thu, 3 Oct 2024 11:14:54 -0500 Subject: [PATCH 3/4] Move the PostHog bootstrapping stuff into its own wrapper component --- frontends/main/next.config.js | 7 +++- frontends/main/src/app/providers.tsx | 40 +++++-------------- .../ConfiguredPostHogProvider.tsx | 28 +++++++++++++ 3 files changed, 44 insertions(+), 31 deletions(-) create mode 100644 frontends/main/src/components/ConfiguredPostHogProvider/ConfiguredPostHogProvider.tsx diff --git a/frontends/main/next.config.js b/frontends/main/next.config.js index 8942cd4596..11658fe0cc 100644 --- a/frontends/main/next.config.js +++ b/frontends/main/next.config.js @@ -16,7 +16,7 @@ const processFeatureFlags = () => { } } - return { FEATURE_FLAGS: JSON.stringify(bootstrapFeatureFlags) } + return bootstrapFeatureFlags } /** @type {import('next').NextConfig} */ @@ -29,7 +29,6 @@ const nextConfig = { new webpack.IgnorePlugin({ resourceRegExp: /mockAxios\.ts/, }), - new webpack.EnvironmentPlugin(processFeatureFlags()), ) // Do not do this. Added to fix "import type", but causes a strage issue where @@ -74,6 +73,10 @@ const nextConfig = { }, ], }, + + env: { + FEATURE_FLAGS: JSON.stringify(processFeatureFlags()), + }, } module.exports = nextConfig diff --git a/frontends/main/src/app/providers.tsx b/frontends/main/src/app/providers.tsx index 000a14deef..f0e66a07ff 100644 --- a/frontends/main/src/app/providers.tsx +++ b/frontends/main/src/app/providers.tsx @@ -5,38 +5,20 @@ import { getQueryClient } from "./getQueryClient" import { QueryClientProvider } from "@tanstack/react-query" import { ThemeProvider, NextJsAppRouterCacheProvider } from "ol-components" import { Provider as NiceModalProvider } from "@ebay/nice-modal-react" -import { PostHogProvider } from "posthog-js/react" +import ConfiguredPostHogProvider from "@/components/ConfiguredPostHogProvider/ConfiguredPostHogProvider" export default function Providers({ children }: { children: React.ReactNode }) { const queryClient = getQueryClient() - const apiKey = process.env.NEXT_PUBLIC_POSTHOG_PROJECT_API_KEY || "" - const apiHost = - process.env.NEXT_PUBLIC_POSTHOG_API_HOST || "https://us.i.posthog.com" - const featureFlags = JSON.parse(process.env.FEATURE_FLAGS || "") - - const postHogOptions = { - api_host: apiHost, - bootstrap: { - featureFlags: featureFlags, - }, - } - - const interiorElements = ( - - - - {children} - - - - ) - - return apiKey.length > 0 ? ( - - {interiorElements} - - ) : ( - interiorElements + return ( + + + + + {children} + + + + ) } diff --git a/frontends/main/src/components/ConfiguredPostHogProvider/ConfiguredPostHogProvider.tsx b/frontends/main/src/components/ConfiguredPostHogProvider/ConfiguredPostHogProvider.tsx new file mode 100644 index 0000000000..78b3e9cb8e --- /dev/null +++ b/frontends/main/src/components/ConfiguredPostHogProvider/ConfiguredPostHogProvider.tsx @@ -0,0 +1,28 @@ +import React from "react" +import { PostHogProvider } from "posthog-js/react" + +const ConfiguredPostHogProvider: React.FC<{ children: React.ReactNode }> = ({ + children, +}) => { + const apiKey = process.env.NEXT_PUBLIC_POSTHOG_PROJECT_API_KEY || "" + const apiHost = + process.env.NEXT_PUBLIC_POSTHOG_API_HOST || "https://us.i.posthog.com" + const featureFlags = JSON.parse(process.env.FEATURE_FLAGS || "") + + const postHogOptions = { + api_host: apiHost, + bootstrap: { + featureFlags: featureFlags, + }, + } + + return apiKey ? ( + + {children} + + ) : ( + children + ) +} + +export default ConfiguredPostHogProvider From 90ad4861e294b221cc3fdf990f03e99ef4be773d Mon Sep 17 00:00:00 2001 From: James Kachel Date: Thu, 3 Oct 2024 11:16:17 -0500 Subject: [PATCH 4/4] pull a couple settings that weren't being used --- frontends/main/validateEnv.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/frontends/main/validateEnv.js b/frontends/main/validateEnv.js index c0326fe082..ed1ff36222 100644 --- a/frontends/main/validateEnv.js +++ b/frontends/main/validateEnv.js @@ -21,8 +21,6 @@ const schema = yup.object().shape({ .string() .oneOf(["true", "false"]), NEXT_PUBLIC_CSRF_COOKIE_NAME: yup.string().required(), - NEXT_PUBLIC_POSTHOG_PROJECT_ID: yup.string(), - NEXT_PUBLIC_POSTHOG_PERSONAL_API_KEY: yup.string(), NEXT_PUBLIC_POSTHOG_PROJECT_API_KEY: yup.string(), NEXT_PUBLIC_POSTHOG_FEATURE_PREFIX: yup.string(), NEXT_PUBLIC_POSTHOG_API_HOST: yup.string(),