fix(desktop): move desktop_opened event to app startup#878
Conversation
Previously, the desktop_opened event only fired when the sign-in page loaded. This meant logged-in users who skip sign-in never triggered the event. Moved the event to PostHogProvider initialization so it fires for all app launches regardless of auth state.
📝 WalkthroughWalkthroughThis PR relocates the "desktop_opened" PostHog analytics event capture from the sign-in page component to the PostHogProvider initialization. The event is now emitted when PostHog is initialized rather than when the user reaches the sign-in page. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~4 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/desktop/src/renderer/providers/PostHogProvider/PostHogProvider.tsx`:
- Around line 13-16: Extract the hardcoded "desktop_opened" string to a
module-level constant (e.g., DESKTOP_OPENED_EVENT) and then wrap the
posthog.capture call in a try-catch so any exception is logged (use existing
logger) and does not prevent state progression; call setIsInitialized(true)
regardless (either in a finally block or after the try/catch) so isInitialized
isn't left false if capture throws—apply these changes around initPostHog(),
posthog.capture(...) and setIsInitialized(...) in PostHogProvider.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/providers/PostHogProvider/PostHogProvider.tsx (1)
15-15: Extract event name to a module constant.Hardcoded event names should be named constants at the module top for reuse and consistency.
♻️ Suggested change
+const POSTHOG_EVENT_DESKTOP_OPENED = "desktop_opened"; + export function PostHogProvider({ children }: PostHogProviderProps) { const [isInitialized, setIsInitialized] = useState(false); useEffect(() => { initPostHog(); - posthog.capture("desktop_opened"); + posthog.capture(POSTHOG_EVENT_DESKTOP_OPENED); setIsInitialized(true); }, []);
| useEffect(() => { | ||
| initPostHog(); | ||
| posthog.capture("desktop_opened"); | ||
| setIsInitialized(true); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find PostHog-related files
fd -i 'posthog' apps/desktop/src/renderer --type fRepository: superset-sh/superset
Length of output: 261
🏁 Script executed:
# Read the PostHogProvider file
cat -n apps/desktop/src/renderer/providers/PostHogProvider/PostHogProvider.tsxRepository: superset-sh/superset
Length of output: 908
🏁 Script executed:
# Search for initPostHog implementation
rg -n 'initPostHog' apps/desktop/src/renderer -B 2 -A 10Repository: superset-sh/superset
Length of output: 3323
🏁 Script executed:
cat -n apps/desktop/src/renderer/lib/posthog.tsRepository: superset-sh/superset
Length of output: 1107
Add error handling and extract hardcoded event name to a constant.
initPostHog() is synchronous, so timing isn't the issue. However, posthog.capture() is not wrapped in error handling—if it throws, setIsInitialized is never called, leaving isInitialized false and blanking the UI indefinitely. Add a try-catch block with logging. Also extract the hardcoded event name "desktop_opened" to a named constant at the module top per guidelines.
🤖 Prompt for AI Agents
In `@apps/desktop/src/renderer/providers/PostHogProvider/PostHogProvider.tsx`
around lines 13 - 16, Extract the hardcoded "desktop_opened" string to a
module-level constant (e.g., DESKTOP_OPENED_EVENT) and then wrap the
posthog.capture call in a try-catch so any exception is logged (use existing
logger) and does not prevent state progression; call setIsInitialized(true)
regardless (either in a finally block or after the try/catch) so isInitialized
isn't left false if capture throws—apply these changes around initPostHog(),
posthog.capture(...) and setIsInitialized(...) in PostHogProvider.
1 participant
Summary
desktop_openedPostHog event from sign-in page toPostHogProviderinitializationTest plan
desktop_openedfiresdesktop_openedfires (previously didn't)Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.