From 42a74e28870205f8febe643d3b63fcf25171dae2 Mon Sep 17 00:00:00 2001 From: Justin Hernandez Date: Tue, 30 Dec 2025 15:21:29 -0800 Subject: [PATCH] SELF-1155: Address missing screen views (#1539) * use track events for screen views * don't use factory pattern for analytics imports * nice clean up * simplify screen event logic and add tests * fix test and agent feedback --- .../recovery/DocumentDataNotFoundScreen.tsx | 4 +- .../scanning/DocumentCameraTroubleScreen.tsx | 4 +- app/src/screens/shared/ComingSoonScreen.tsx | 4 +- app/src/services/analytics.ts | 180 ++++++++++-------- app/tests/src/services/analytics.test.ts | 116 ++++++++++- 5 files changed, 215 insertions(+), 93 deletions(-) diff --git a/app/src/screens/account/recovery/DocumentDataNotFoundScreen.tsx b/app/src/screens/account/recovery/DocumentDataNotFoundScreen.tsx index 017d926e3..45a4a6a20 100644 --- a/app/src/screens/account/recovery/DocumentDataNotFoundScreen.tsx +++ b/app/src/screens/account/recovery/DocumentDataNotFoundScreen.tsx @@ -21,9 +21,7 @@ import { import useHapticNavigation from '@/hooks/useHapticNavigation'; import { ExpandableBottomLayout } from '@/layouts/ExpandableBottomLayout'; -import analytics from '@/services/analytics'; - -const { flush: flushAnalytics } = analytics(); +import { flush as flushAnalytics } from '@/services/analytics'; const DocumentDataNotFoundScreen: React.FC = () => { const selfClient = useSelfClient(); diff --git a/app/src/screens/documents/scanning/DocumentCameraTroubleScreen.tsx b/app/src/screens/documents/scanning/DocumentCameraTroubleScreen.tsx index 5534819b8..e9d0a2c10 100644 --- a/app/src/screens/documents/scanning/DocumentCameraTroubleScreen.tsx +++ b/app/src/screens/documents/scanning/DocumentCameraTroubleScreen.tsx @@ -16,9 +16,7 @@ import type { TipProps } from '@/components/Tips'; import Tips from '@/components/Tips'; import useHapticNavigation from '@/hooks/useHapticNavigation'; import SimpleScrolledTitleLayout from '@/layouts/SimpleScrolledTitleLayout'; -import analytics from '@/services/analytics'; - -const { flush: flushAnalytics } = analytics(); +import { flush as flushAnalytics } from '@/services/analytics'; const tips: TipProps[] = [ { diff --git a/app/src/screens/shared/ComingSoonScreen.tsx b/app/src/screens/shared/ComingSoonScreen.tsx index f3959fa85..2636e06a2 100644 --- a/app/src/screens/shared/ComingSoonScreen.tsx +++ b/app/src/screens/shared/ComingSoonScreen.tsx @@ -25,11 +25,9 @@ import useHapticNavigation from '@/hooks/useHapticNavigation'; import { notificationError } from '@/integrations/haptics'; import { ExpandableBottomLayout } from '@/layouts/ExpandableBottomLayout'; import type { SharedRoutesParamList } from '@/navigation/types'; -import analytics from '@/services/analytics'; +import { flush as flushAnalytics } from '@/services/analytics'; import { sendCountrySupportNotification } from '@/services/email'; -const { flush: flushAnalytics } = analytics(); - type ComingSoonScreenProps = NativeStackScreenProps< SharedRoutesParamList, 'ComingSoon' diff --git a/app/src/services/analytics.ts b/app/src/services/analytics.ts index 9bb65e99c..86fda5704 100644 --- a/app/src/services/analytics.ts +++ b/app/src/services/analytics.ts @@ -13,9 +13,19 @@ import type { TrackEventParams } from '@selfxyz/mobile-sdk-alpha'; import { createSegmentClient } from '@/config/segment'; import { PassportReader } from '@/integrations/nfc/passportReader'; +// ============================================================================ +// Constants +// ============================================================================ + +const MIXPANEL_AUTO_FLUSH_THRESHOLD = 5; +const MAX_EVENT_QUEUE_SIZE = 100; + +// ============================================================================ +// State Management +// ============================================================================ + const segmentClient = createSegmentClient(); -// --- Analytics flush strategy --- let mixpanelConfigured = false; let eventCount = 0; let isConnected = true; @@ -25,6 +35,10 @@ const eventQueue: Array<{ properties?: Record; }> = []; +// ============================================================================ +// Internal Helpers - JSON Coercion +// ============================================================================ + function coerceToJsonValue( value: unknown, seen = new WeakSet(), @@ -93,22 +107,31 @@ function validateParams( return cleanParams(validatedProps); } +// ============================================================================ +// Internal Helpers - Event Tracking +// ============================================================================ + /** * Internal tracking function used by trackEvent and trackScreenView * Records analytics events and screen views * In development mode, events are logged to console instead of being sent to Segment + * + * NOTE: Screen views are tracked as 'Screen Viewed' events for Mixpanel compatibility */ function _track( type: 'event' | 'screen', eventName: string, properties?: Record, ) { + // Transform screen events for Mixpanel compatibility + const finalEventName = type === 'screen' ? `Viewed ${eventName}` : eventName; + // Validate and clean properties const validatedProps = validateParams(properties); if (__DEV__) { console.log(`[DEV: Analytics ${type.toUpperCase()}]`, { - name: eventName, + name: finalEventName, properties: validatedProps, }); return; @@ -117,18 +140,21 @@ function _track( if (!segmentClient) { return; } - const trackMethod = (e: string, p?: JsonMap) => - type === 'screen' ? segmentClient.screen(e, p) : segmentClient.track(e, p); + // Always use track() for both events and screen views (Mixpanel compatibility) if (!validatedProps) { // you may need to remove the catch when debugging - return trackMethod(eventName).catch(console.info); + return segmentClient.track(finalEventName).catch(console.info); } // you may need to remove the catch when debugging - trackMethod(eventName, validatedProps).catch(console.info); + segmentClient.track(finalEventName, validatedProps).catch(console.info); } +// ============================================================================ +// Public API - Segment Analytics +// ============================================================================ + /** * Cleanup function to clear event queues */ @@ -137,13 +163,12 @@ export const cleanupAnalytics = () => { eventCount = 0; }; -// --- Mixpanel NFC Analytics --- +// ============================================================================ +// Public API - Mixpanel NFC Analytics +// ============================================================================ export const configureNfcAnalytics = async () => { if (!MIXPANEL_NFC_PROJECT_TOKEN || mixpanelConfigured) return; - const enableDebugLogs = - String(ENABLE_DEBUG_LOGS ?? '') - .trim() - .toLowerCase() === 'true'; + const enableDebugLogs = ENABLE_DEBUG_LOGS; // Check if PassportReader and configure method exist (Android doesn't have configure) if (PassportReader && typeof PassportReader.configure === 'function') { @@ -171,21 +196,6 @@ export const flush = () => { } }; -/** - * @deprecated Use named exports (trackEvent, trackScreenView, flush) instead - * Factory function that returns analytics methods - * Kept for backward compatibility - */ -const analytics = () => { - return { - trackEvent, - trackScreenView, - flush, - }; -}; - -export default analytics; - /** * Consolidated analytics flush function that flushes both Segment and Mixpanel events * This should be called when you want to ensure all analytics events are sent immediately @@ -200,6 +210,70 @@ export const flushAllAnalytics = () => { } }; +/** + * Set NFC scanning state to prevent analytics flush interference + */ +export const setNfcScanningActive = (active: boolean) => { + isNfcScanningActive = active; + if (__DEV__) + console.log( + `[NFC Analytics] Scanning state: ${active ? 'active' : 'inactive'}`, + ); + + // Flush queued events when scanning completes + if (!active && eventQueue.length > 0) { + flushMixpanelEvents().catch(console.warn); + } +}; + +/** + * Track an analytics event + * @param eventName - Name of the event to track + * @param properties - Optional properties to attach to the event + */ +export const trackEvent = ( + eventName: string, + properties?: TrackEventParams, +) => { + _track('event', eventName, properties); +}; + +export const trackNfcEvent = async ( + name: string, + properties?: Record, +) => { + if (!MIXPANEL_NFC_PROJECT_TOKEN) return; + if (!mixpanelConfigured) await configureNfcAnalytics(); + + if (!isConnected || isNfcScanningActive) { + if (eventQueue.length >= MAX_EVENT_QUEUE_SIZE) { + if (__DEV__) + console.warn('[Mixpanel] Event queue full, dropping oldest event'); + eventQueue.shift(); + } + eventQueue.push({ name, properties }); + return; + } + + try { + if (PassportReader && PassportReader.trackEvent) { + await Promise.resolve(PassportReader.trackEvent(name, properties)); + } + eventCount++; + // Prevent automatic flush during NFC scanning + if (eventCount >= MIXPANEL_AUTO_FLUSH_THRESHOLD && !isNfcScanningActive) { + flushMixpanelEvents().catch(console.warn); + } + } catch { + if (eventQueue.length >= MAX_EVENT_QUEUE_SIZE) { + if (__DEV__) + console.warn('[Mixpanel] Event queue full, dropping oldest event'); + eventQueue.shift(); + } + eventQueue.push({ name, properties }); + } +}; + const setupFlushPolicies = () => { AppState.addEventListener('change', (state: AppStateStatus) => { // Never flush during active NFC scanning to prevent interference @@ -261,60 +335,6 @@ const flushMixpanelEvents = async () => { } }; -/** - * Set NFC scanning state to prevent analytics flush interference - */ -export const setNfcScanningActive = (active: boolean) => { - isNfcScanningActive = active; - if (__DEV__) - console.log( - `[NFC Analytics] Scanning state: ${active ? 'active' : 'inactive'}`, - ); - - // Flush queued events when scanning completes - if (!active && eventQueue.length > 0) { - flushMixpanelEvents().catch(console.warn); - } -}; - -/** - * Track an analytics event - * @param eventName - Name of the event to track - * @param properties - Optional properties to attach to the event - */ -export const trackEvent = ( - eventName: string, - properties?: TrackEventParams, -) => { - _track('event', eventName, properties); -}; - -export const trackNfcEvent = async ( - name: string, - properties?: Record, -) => { - if (!MIXPANEL_NFC_PROJECT_TOKEN) return; - if (!mixpanelConfigured) await configureNfcAnalytics(); - - if (!isConnected || isNfcScanningActive) { - eventQueue.push({ name, properties }); - return; - } - - try { - if (PassportReader && PassportReader.trackEvent) { - await Promise.resolve(PassportReader.trackEvent(name, properties)); - } - eventCount++; - // Prevent automatic flush during NFC scanning - if (eventCount >= 5 && !isNfcScanningActive) { - flushMixpanelEvents().catch(console.warn); - } - } catch { - eventQueue.push({ name, properties }); - } -}; - /** * Track a screen view * @param screenName - Name of the screen to track diff --git a/app/tests/src/services/analytics.test.ts b/app/tests/src/services/analytics.test.ts index a588b9f58..e0d599a8b 100644 --- a/app/tests/src/services/analytics.test.ts +++ b/app/tests/src/services/analytics.test.ts @@ -7,8 +7,8 @@ import { trackEvent, trackScreenView } from '@/services/analytics'; // Mock the Segment client jest.mock('@/config/segment', () => ({ createSegmentClient: jest.fn(() => ({ - track: jest.fn(), - screen: jest.fn(), + track: jest.fn().mockResolvedValue(undefined), + flush: jest.fn().mockResolvedValue(undefined), })), })); @@ -36,7 +36,7 @@ describe('analytics', () => { }); it('should handle event tracking with null properties', () => { - expect(() => trackEvent('test_event', null)).not.toThrow(); + expect(() => trackEvent('test_event', null as any)).not.toThrow(); }); it('should handle event tracking with undefined properties', () => { @@ -85,7 +85,7 @@ describe('analytics', () => { it('should handle invalid duration values gracefully', () => { const properties = { - duration_seconds: 'not_a_number', + duration_seconds: 'not_a_number' as any, }; expect(() => trackEvent('test_event', properties)).not.toThrow(); @@ -142,6 +142,22 @@ describe('analytics', () => { expect(() => trackEvent('test_event', properties)).not.toThrow(); }); + + it('should NOT transform regular event names (only screen views get "Viewed" prefix)', () => { + const consoleSpy = jest.spyOn(console, 'log').mockImplementation(); + + trackEvent('user_login', { method: 'google' }); + + expect(consoleSpy).toHaveBeenCalledWith( + '[DEV: Analytics EVENT]', + expect.objectContaining({ + name: 'user_login', // No "Viewed" prefix for regular events + properties: expect.objectContaining({ method: 'google' }), + }), + ); + + consoleSpy.mockRestore(); + }); }); describe('trackScreenView', () => { @@ -174,6 +190,98 @@ describe('analytics', () => { expect(() => trackScreenView('test_screen', properties)).not.toThrow(); }); + + it('should transform screen views to "Viewed ScreenName" format', () => { + // Mock console.log to capture dev mode output + const consoleSpy = jest.spyOn(console, 'log').mockImplementation(); + + trackScreenView('SplashScreen', { user_id: 123 }); + + expect(consoleSpy).toHaveBeenCalledWith( + '[DEV: Analytics SCREEN]', + expect.objectContaining({ + name: 'Viewed SplashScreen', + properties: expect.objectContaining({ user_id: 123 }), + }), + ); + + consoleSpy.mockRestore(); + }); + + it('should transform screen names correctly without properties', () => { + const consoleSpy = jest.spyOn(console, 'log').mockImplementation(); + + trackScreenView('DocumentNFCScanScreen'); + + expect(consoleSpy).toHaveBeenCalledWith( + '[DEV: Analytics SCREEN]', + expect.objectContaining({ + name: 'Viewed DocumentNFCScanScreen', + properties: undefined, + }), + ); + + consoleSpy.mockRestore(); + }); + + it('should pass through properties unchanged', () => { + const consoleSpy = jest.spyOn(console, 'log').mockImplementation(); + const properties = { + referrer: 'home', + user_id: 456, + navigation_method: 'swipe', + }; + + trackScreenView('SettingsScreen', properties); + + expect(consoleSpy).toHaveBeenCalledWith( + '[DEV: Analytics SCREEN]', + expect.objectContaining({ + name: 'Viewed SettingsScreen', + properties: expect.objectContaining(properties), + }), + ); + + consoleSpy.mockRestore(); + }); + + it('should call segment client with transformed event name in production', () => { + // Temporarily mock __DEV__ to false for production testing + const originalDev = (global as any).__DEV__; + (global as any).__DEV__ = false; + + try { + // Reset modules first to clear the cache + jest.resetModules(); + + // Get the mocked segment client factory after reset + const segmentModule = require('@/config/segment'); + const mockTrack = jest.fn().mockResolvedValue(undefined); + + // Set up the mock implementation before re-requiring analytics + // This ensures the mock is properly configured when analytics module loads + segmentModule.createSegmentClient.mockImplementation(() => ({ + track: mockTrack, + flush: jest.fn().mockResolvedValue(undefined), + })); + + // Now re-require analytics to get a fresh segmentClient instance + // that uses our mocked createSegmentClient + const analyticsModule = require('@/services/analytics'); + + analyticsModule.trackScreenView('HomeScreen', { user_type: 'premium' }); + + expect(mockTrack).toHaveBeenCalledWith('Viewed HomeScreen', { + user_type: 'premium', + }); + } finally { + // Restore original __DEV__ value + (global as any).__DEV__ = originalDev; + + // Reset modules again to restore original state for other tests + jest.resetModules(); + } + }); }); describe('edge cases', () => {