From 9597fe8410c24c98592a28842439e1443b8b8bba Mon Sep 17 00:00:00 2001 From: Robbie Date: Wed, 27 Nov 2024 10:59:15 +0000 Subject: [PATCH] feat(web-analytics): Add warning when conversion goal is a custom action with no session id (#26441) --- frontend/src/lib/constants.tsx | 1 + .../web-analytics/WebAnalyticsHealthCheck.tsx | 28 +++++++- .../web-analytics/webAnalyticsLogic.tsx | 72 +++++++++++++++++-- 3 files changed, 94 insertions(+), 7 deletions(-) diff --git a/frontend/src/lib/constants.tsx b/frontend/src/lib/constants.tsx index 3bc00eb8ddf34..9c41f441e73f4 100644 --- a/frontend/src/lib/constants.tsx +++ b/frontend/src/lib/constants.tsx @@ -232,6 +232,7 @@ export const FEATURE_FLAGS = { SELF_SERVE_CREDIT_OVERRIDE: 'self-serve-credit-override', // owner: @zach EXPERIMENTS_MIGRATION_DISABLE_UI: 'experiments-migration-disable-ui', // owner: @jurajmajerik #team-experiments CUSTOM_CSS_THEMES: 'custom-css-themes', // owner: @daibhin + WEB_ANALYTICS_WARN_CUSTOM_EVENT_NO_SESSION: 'web-analytics-warn-custom-event-no-session', // owner: @robbie-c #team-web-analytics } as const export type FeatureFlagKey = (typeof FEATURE_FLAGS)[keyof typeof FEATURE_FLAGS] diff --git a/frontend/src/scenes/web-analytics/WebAnalyticsHealthCheck.tsx b/frontend/src/scenes/web-analytics/WebAnalyticsHealthCheck.tsx index 10b2948b856db..b5374e075f9bf 100644 --- a/frontend/src/scenes/web-analytics/WebAnalyticsHealthCheck.tsx +++ b/frontend/src/scenes/web-analytics/WebAnalyticsHealthCheck.tsx @@ -1,10 +1,34 @@ import { useValues } from 'kea' +import { useFeatureFlag } from 'lib/hooks/useFeatureFlag' import { LemonBanner } from 'lib/lemon-ui/LemonBanner' import { Link } from 'lib/lemon-ui/Link' -import { webAnalyticsLogic } from 'scenes/web-analytics/webAnalyticsLogic' +import { ConversionGoalWarning, webAnalyticsLogic } from 'scenes/web-analytics/webAnalyticsLogic' export const WebAnalyticsHealthCheck = (): JSX.Element | null => { - const { statusCheck } = useValues(webAnalyticsLogic) + const { statusCheck, conversionGoalWarning } = useValues(webAnalyticsLogic) + const isFlagConversionGoalWarningsSet = useFeatureFlag('WEB_ANALYTICS_WARN_CUSTOM_EVENT_NO_SESSION') + + if (conversionGoalWarning && isFlagConversionGoalWarningsSet) { + switch (conversionGoalWarning) { + case ConversionGoalWarning.CustomEventWithNoSessionId: + return ( + +

+ A custom event has been set as a conversion goal, but it has been seen with no{' '} + $session_id. This means that some queries will not be able to include these + events. +

+

+ To fix this, please see{' '} + + documentation for custom session IDs + + . +

+ + ) + } + } // No need to show loading or error states for this warning if (!statusCheck) { diff --git a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx index 4d65ceb7f1222..c0f97927dc01f 100644 --- a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx +++ b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx @@ -1,4 +1,4 @@ -import { actions, afterMount, connect, kea, listeners, path, reducers, selectors } from 'kea' +import { actions, afterMount, BreakPointFunction, connect, kea, listeners, path, reducers, selectors } from 'kea' import { loaders } from 'kea-loaders' import { actionToUrl, urlToAction } from 'kea-router' import { windowValues } from 'kea-window-values' @@ -6,11 +6,12 @@ import api from 'lib/api' import { FEATURE_FLAGS, RETENTION_FIRST_TIME, STALE_EVENT_SECONDS } from 'lib/constants' import { dayjs } from 'lib/dayjs' import { Link, PostHogComDocsURL } from 'lib/lemon-ui/Link/Link' -import { featureFlagLogic } from 'lib/logic/featureFlagLogic' +import { featureFlagLogic, FeatureFlagsSet } from 'lib/logic/featureFlagLogic' import { getDefaultInterval, isNotNil, objectsEqual, updateDatesWithInterval } from 'lib/utils' import { errorTrackingQuery } from 'scenes/error-tracking/queries' import { urls } from 'scenes/urls' +import { hogqlQuery } from '~/queries/query' import { ActionConversionGoal, ActionsNode, @@ -194,6 +195,10 @@ export interface WebAnalyticsStatusCheck { isSendingPageLeavesScroll: boolean } +export enum ConversionGoalWarning { + CustomEventWithNoSessionId = 'CustomEventWithNoSessionId', +} + export const GEOIP_PLUGIN_URLS = [ 'https://github.com/PostHog/posthog-plugin-geoip', 'https://www.npmjs.com/package/@posthog/geoip-plugin', @@ -269,6 +274,7 @@ export const webAnalyticsLogic = kea([ openAsNewInsight: (tileId: TileId, tabId?: string) => { return { tileId, tabId } }, + setConversionGoalWarning: (warning: ConversionGoalWarning | null) => ({ warning }), }), reducers({ webAnalyticsFilters: [ @@ -455,6 +461,12 @@ export const webAnalyticsLogic = kea([ setConversionGoal: (_, { conversionGoal }) => conversionGoal, }, ], + conversionGoalWarning: [ + null as ConversionGoalWarning | null, + { + setConversionGoalWarning: (_, { warning }) => warning, + }, + ], }), selectors(({ actions, values }) => ({ graphsTab: [(s) => [s._graphsTab], (graphsTab: string | null) => graphsTab || GraphsTab.UNIQUE_USERS], @@ -1489,18 +1501,68 @@ export const webAnalyticsLogic = kea([ } } } + return { setGraphsTab: ({ tab }) => { checkGraphsTabIsCompatibleWithConversionGoal(tab, values.conversionGoal) }, - setConversionGoal: ({ conversionGoal }) => { - checkGraphsTabIsCompatibleWithConversionGoal(values.graphsTab, conversionGoal) - }, + setConversionGoal: [ + ({ conversionGoal }) => { + checkGraphsTabIsCompatibleWithConversionGoal(values.graphsTab, conversionGoal) + }, + ({ conversionGoal }, breakpoint) => + checkCustomEventConversionGoalHasSessionIdsHelper( + conversionGoal, + breakpoint, + actions.setConversionGoalWarning, + values.featureFlags + ), + ], } }), + afterMount(({ actions, values }) => { + checkCustomEventConversionGoalHasSessionIdsHelper( + values.conversionGoal, + undefined, + actions.setConversionGoalWarning, + values.featureFlags + ).catch(() => { + // ignore, this warning is just a nice-to-have, no point showing an error to the user + }) + }), ]) const isDefinitionStale = (definition: EventDefinition | PropertyDefinition): boolean => { const parsedLastSeen = definition.last_seen_at ? dayjs(definition.last_seen_at) : null return !!parsedLastSeen && dayjs().diff(parsedLastSeen, 'seconds') > STALE_EVENT_SECONDS } + +const checkCustomEventConversionGoalHasSessionIdsHelper = async ( + conversionGoal: WebAnalyticsConversionGoal | null, + breakpoint: BreakPointFunction | undefined, + setConversionGoalWarning: (warning: ConversionGoalWarning | null) => void, + featureFlags: FeatureFlagsSet +): Promise => { + if (!featureFlags[FEATURE_FLAGS.WEB_ANALYTICS_WARN_CUSTOM_EVENT_NO_SESSION]) { + return + } + + if (!conversionGoal || !('customEventName' in conversionGoal) || !conversionGoal.customEventName) { + setConversionGoalWarning(null) + return + } + const { customEventName } = conversionGoal + // check if we have any conversion events from the last week without sessions ids + + const response = await hogqlQuery( + `select count() from events where timestamp >= (now() - toIntervalHour(24)) AND ($session_id IS NULL OR $session_id = '') AND event = {event}`, + { event: customEventName } + ) + breakpoint?.() + const row = response.results[0] + if (row[0]) { + setConversionGoalWarning(ConversionGoalWarning.CustomEventWithNoSessionId) + } else { + setConversionGoalWarning(null) + } +}