Migrate website analytics to self-hosted Umami - #22
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe application replaces Plausible with configurable Umami analytics. It sanitizes analytics URLs, limits recordings to public routes, reloads SPA navigation across recording boundaries, marks sensitive fields, and updates the privacy policy and environment configuration. ChangesUmami analytics migration
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The analytics migration and related privacy/configuration updates have no actionable merge-blocking risk identified at the current head; the PR is merge-ready after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Browser
participant Layout
participant initAnalytics
participant Umami
participant Recorder
Browser->>Layout: initialize application
Layout->>initAnalytics: initialize analytics
initAnalytics->>Umami: load tracker with sanitized payload handler
initAnalytics->>Recorder: load recorder for eligible public route
Umami-->>initAnalytics: expose tracker
Layout->>Layout: evaluate beforeNavigate
Layout->>Browser: reload navigation across recording boundary
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 6 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
🤖 review-bot (run started · HEAD Generated by Claude Code |
|
Generated by Claude Code |
There was a problem hiding this comment.
1 issue found across 15 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/lib/analytics-script.js">
<violation number="1" location="src/lib/analytics-script.js:11">
P2: The recorder is loaded on '/signup', which renders password and email inputs and handles OAuth code exchange. The only thing this repo does for those routes is load the recorder; sanitizeAnalyticsPayload rewrites url/referrer and passes Umami 'recorded' captures (the blob) through unchanged, so the password/email/PII protection depends entirely on external recorder masking config that lives outside this PR and can't be audited here. Move '/signup' (and other routes containing identity forms) out of ANALYTICS_RECORDING_PATHS, or add an in-repo defense-in-depth that suppresses 'recorded' payloads for these routes.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| '/demo', | ||
| '/pricing', | ||
| '/privacy', | ||
| '/signup', |
There was a problem hiding this comment.
P2: The recorder is loaded on '/signup', which renders password and email inputs and handles OAuth code exchange. The only thing this repo does for those routes is load the recorder; sanitizeAnalyticsPayload rewrites url/referrer and passes Umami 'recorded' captures (the blob) through unchanged, so the password/email/PII protection depends entirely on external recorder masking config that lives outside this PR and can't be audited here. Move '/signup' (and other routes containing identity forms) out of ANALYTICS_RECORDING_PATHS, or add an in-repo defense-in-depth that suppresses 'recorded' payloads for these routes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/analytics-script.js, line 11:
<comment>The recorder is loaded on '/signup', which renders password and email inputs and handles OAuth code exchange. The only thing this repo does for those routes is load the recorder; sanitizeAnalyticsPayload rewrites url/referrer and passes Umami 'recorded' captures (the blob) through unchanged, so the password/email/PII protection depends entirely on external recorder masking config that lives outside this PR and can't be audited here. Move '/signup' (and other routes containing identity forms) out of ANALYTICS_RECORDING_PATHS, or add an in-repo defense-in-depth that suppresses 'recorded' payloads for these routes.</comment>
<file context>
@@ -0,0 +1,32 @@
+ '/demo',
+ '/pricing',
+ '/privacy',
+ '/signup',
+ '/terms'
+]);
</file context>
|
🤖 review-bot (run started · HEAD Generated by Claude Code |
- Reset the umami promise on load/network failure so a later initAnalytics() call can retry instead of getting stuck forever. - Mark the signup form's email/password inputs analytics-sensitive, matching the pattern already used for other credential fields, since /signup is a recorded route. Addresses cubic review findings on PR #22.
Automated review-bot triage
Trusted review (current HEAD before this push): cubic ✓ (2 issues, both actioned below); CodeRabbit ✓ (no actionable comments); Greptile absent (no check on this PR); Gemini/Copilot absent (no review). Mergeability is the Plan: 1 commit, both cubic findings fixed — reset the tracker on load/error so it retries, and mask the signup email/password inputs the same way the rest of this PR masks credential fields. Pushed as Generated by Claude Code |
|
✅ done — pushed 1 fix commit (2 findings addressed) as Generated by Claude Code |
|
🤖 review-bot (run 00:00 UTC · HEAD Generated by Claude Code |
|
Generated by Claude Code |
|
🤖 review-bot (run 00:00 UTC · HEAD Generated by Claude Code |
✅ Review-bot convergedrun 21:37 UTC · HEAD No bot comment postdates the prior triage (table, 21:32 UTC) — this HEAD is that triage's own fix-push. CodeRabbit's fresh pass on Trusted review (current HEAD): CodeRabbit ✓ (no actionable comments); cubic ✓ (check-run success, no new findings). Greptile absent (no check on this PR); Gemini/Copilot absent (no review). Mergeability is the Loop totals: 1 round, 3 findings (2 do, 0 defer, 0 won't-do, 1 skip). Generated by Claude Code |
Summary
Verification
Summary by cubic
Replaces Plausible with a self-hosted Umami tracker and first-party relay, and limits sampled replay and heatmaps to clean marketing and signup URLs. The old tracker captured every page view; the new setup never records on authenticated, OAuth, repository, or queried pages, and forces a reload when SPA navigation crosses that boundary.
sanitizeAnalyticsPayloadto strip credentials and keep only marketing attribution parameters.analytics-sensitiveclass to mask form controls, email fields, and identity content from recordings, including the signup email/password inputs.PUBLIC_UMAMI_TRACKED_DOMAINandPUBLIC_UMAMI_WEBSITE_ID; removesPUBLIC_PLAUSIBLE_DOMAINand the@plausible-analytics/trackerdependency.Written for commit 940734f. Summary will update on new commits.
Summary by CodeRabbit
New Features
Privacy
Bug Fixes