-
Notifications
You must be signed in to change notification settings - Fork 11.8k
feat: posthog version upgrade and added trackings #24401
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis change updates PostHog dependencies (posthog-js to ^1.274.1 and posthog-node to ^5.9.5), removes the PostHog initialization option persistence: "memory", and adds analytics event capture across several UI components. BuyNumberDialog now captures calai_phone_number_purchased on successful purchase and calai_buy_number_button_clicked with agentId, workflowId, and teamId on click. PhoneNumberTab captures a Buy button click event when not readOnly. CalAiBanner captures events on banner dismissal and on “Try Now.” No exported/public API changes were made. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
| api_host: process.env.NEXT_PUBLIC_POSTHOG_HOST || "https://us.i.posthog.com", | ||
| autocapture: false, | ||
| person_profiles: "never", | ||
| persistence: "memory", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incase of memory, it tracks as a different person every time you refresh the page/open in new tab. Not good for analysing funnels / tracking movements.
now it stores it as localStorage + cookie.
For more info: https://posthog.com/docs/libraries/js/persistence
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/features/ee/workflows/components/agent-configuration/components/dialogs/BuyNumberDialog.tsx (1)
31-31: Consider capturing context with the purchase event.The purchase success event currently doesn't include context properties like
agentId,workflowId, andteamId, which could be valuable for analytics and understanding the purchase flow. These properties are available in the component and are captured in the button click event (line 122).Apply this diff to add context to the purchase event:
- posthog.capture("calai_phone_number_purchased"); + posthog.capture("calai_phone_number_purchased", { agentId, workflowId, teamId });packages/features/ee/workflows/components/agent-configuration/components/tabs/PhoneNumberTab.tsx (1)
279-283: Consider capturing context with the modal opened event.For consistency with the
calai_buy_number_button_clickedevent in BuyNumberDialog.tsx (line 122) and to provide better analytics insights, consider capturingagentId,workflowId, andteamIdwith this event as well. These properties are available in the component scope.Apply this diff to add context:
- posthog.capture("calai_buy_number_modal_opened"); + posthog.capture("calai_buy_number_modal_opened", { agentId, workflowId, teamId });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
apps/web/package.json(1 hunks)packages/features/ee/event-tracking/lib/posthog/provider.tsx(0 hunks)packages/features/ee/workflows/components/agent-configuration/components/dialogs/BuyNumberDialog.tsx(3 hunks)packages/features/ee/workflows/components/agent-configuration/components/tabs/PhoneNumberTab.tsx(2 hunks)packages/features/shell/CalAiBanner.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- packages/features/ee/event-tracking/lib/posthog/provider.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/shell/CalAiBanner.tsxpackages/features/ee/workflows/components/agent-configuration/components/dialogs/BuyNumberDialog.tsxpackages/features/ee/workflows/components/agent-configuration/components/tabs/PhoneNumberTab.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/shell/CalAiBanner.tsxpackages/features/ee/workflows/components/agent-configuration/components/dialogs/BuyNumberDialog.tsxpackages/features/ee/workflows/components/agent-configuration/components/tabs/PhoneNumberTab.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/shell/CalAiBanner.tsxpackages/features/ee/workflows/components/agent-configuration/components/dialogs/BuyNumberDialog.tsxpackages/features/ee/workflows/components/agent-configuration/components/tabs/PhoneNumberTab.tsx
🧬 Code graph analysis (1)
packages/features/shell/CalAiBanner.tsx (1)
packages/lib/webstorage.ts (1)
localStorage(6-36)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
packages/features/shell/CalAiBanner.tsx (1)
4-4: LGTM! Analytics tracking added for banner interactions.The PostHog event tracking is correctly implemented for the banner dismissal and "Try Now" actions. The event names follow a consistent naming convention with the
calai_prefix.Also applies to: 20-20, 26-26
packages/features/ee/workflows/components/agent-configuration/components/dialogs/BuyNumberDialog.tsx (2)
1-2: LGTM! PostHog import added.
122-122: LGTM! Button click event properly captures context.The event correctly includes
agentId,workflowId, andteamIdfor analytics tracking, and it's appropriately guarded by the conditional check on lines 119-121.packages/features/ee/workflows/components/agent-configuration/components/tabs/PhoneNumberTab.tsx (1)
1-1: LGTM! PostHog import added.
E2E results are ready! |
|
Waiting for @emrysal to merge |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 6 files
…ack_events_for_posthog
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
…ack_events_for_posthog
…ack_events_for_posthog
…lcom/cal.com into feat/track_events_for_posthog
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5 issues found across 34 files
Prompt for AI agents (all 5 issues)
Understand the root cause of the following 5 issues and fix them.
<file name="apps/web/modules/signup-view.tsx">
<violation number="1" location="apps/web/modules/signup-view.tsx:514">
Rule violated: **Avoid Logging Sensitive Information**
The new PostHog event logs the user's email address, which violates the "Avoid Logging Sensitive Information" rule. Please remove the email (and any other direct PII) from this analytics payload before shipping.</violation>
</file>
<file name="apps/web/modules/auth/verify-email-view.tsx">
<violation number="1" location="apps/web/modules/auth/verify-email-view.tsx:27">
Rule violated: **Avoid Logging Sensitive Information**
PostHog capture is logging `session?.user?.email`, which is PII. To comply with our "Avoid Logging Sensitive Information" rule, remove the email from the analytics payload before capturing the event.</violation>
</file>
<file name="packages/features/ee/teams/components/MemberInvitationModal.tsx">
<violation number="1" location="packages/features/ee/teams/components/MemberInvitationModal.tsx:238">
posthog.capture is invoked here but posthog isn’t imported anywhere in this component, so clicking the invite button will throw a ReferenceError at runtime.</violation>
</file>
<file name="packages/features/ee/event-tracking/lib/posthog/web/PostHogPageView.tsx">
<violation number="1" location="packages/features/ee/event-tracking/lib/posthog/web/PostHogPageView.tsx:13">
The TRACKED_ROUTES entry uses "/settings/teams/", but usePathname() returns the base settings route as "/settings/teams" (no trailing slash). Because the guard checks startsWith(), the base settings page now skips PostHog capture, so you lose analytics on that screen.</violation>
</file>
<file name="packages/ui/components/card/Card.tsx">
<violation number="1" location="packages/ui/components/card/Card.tsx:7">
Importing the PostHog SDK directly in the shared Card component bloats every bundle that uses Card. Please keep analytics concerns at the banner/feature level instead of adding a heavy SDK to this generic UI primitive.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
packages/features/ee/event-tracking/lib/posthog/web/PostHogPageView.tsx
Outdated
Show resolved
Hide resolved
Co-Authored-By: amit@cal.com <samit91848@gmail.com>
…ionModal Co-Authored-By: amit@cal.com <samit91848@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 issues found across 38 files
Prompt for AI agents (all 3 issues)
Understand the root cause of the following 3 issues and fix them.
<file name="packages/features/ee/event-tracking/lib/posthog/web/PostHogPageView.tsx">
<violation number="1" location="packages/features/ee/event-tracking/lib/posthog/web/PostHogPageView.tsx:13">
The base /settings/teams page is never tracked because TRACKED_ROUTES only lists the trailing-slash version; `pathname.startsWith("/settings/teams/")` fails for the real `/settings/teams` pathname, so analytics for that page are silently skipped.</violation>
</file>
<file name="apps/web/modules/signup-view.tsx">
<violation number="1" location="apps/web/modules/signup-view.tsx:511">
Rule violated: **Avoid Logging Sensitive Information**
`signup_saml_submit_button_clicked` logs the user’s email address to PostHog, which violates the ban on logging PII. Remove the email (or anonymize it) before capturing the event.</violation>
</file>
<file name="apps/web/modules/auth/verify-email-view.tsx">
<violation number="1" location="apps/web/modules/auth/verify-email-view.tsx:28">
Rule violated: **Avoid Logging Sensitive Information**
`posthog.capture` should not send `session?.user?.email`, as this exposes PII in analytics logs. Remove the email field or anonymize it before tracking.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
packages/features/ee/event-tracking/lib/posthog/web/PostHogPageView.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 39 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="apps/web/modules/signup-view.tsx">
<violation number="1" location="apps/web/modules/signup-view.tsx:511">
Rule violated: **Avoid Logging Sensitive Information**
Remove the `email` field from the PostHog payload so that user email addresses are never logged or transmitted to analytics systems.</violation>
</file>
<file name="apps/web/modules/auth/verify-email-view.tsx">
<violation number="1" location="apps/web/modules/auth/verify-email-view.tsx:28">
Rule violated: **Avoid Logging Sensitive Information**
Both PostHog tracking events add the raw `session?.user?.email`, which logs PII to an external analytics service. Remove or anonymize the email before tracking to comply with the rule against logging sensitive information.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 39 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="packages/ui/components/card/Card.tsx">
<violation number="1" location="packages/ui/components/card/Card.tsx:138">
Type the new `learnMore.onClick` handler with the anchor mouse event so consumers can access the event object when the link is clicked.</violation>
</file>
<file name="apps/web/modules/signup-view.tsx">
<violation number="1" location="apps/web/modules/signup-view.tsx:510">
Rule violated: **Avoid Logging Sensitive Information**
Do not send the actual username to PostHog analytics; usernames are PII and must not be logged per the "Avoid Logging Sensitive Information" rule.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
…ack_events_for_posthog
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 42 files
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/modules/signup-view.tsx">
<violation number="1" location="apps/web/modules/signup-view.tsx:566">
Switch these inline ESLint directives to `eslint-disable-next-line @next/next/no-img-element` (or re‑enable immediately) so the suppression is limited to the intended `<img>` tag instead of disabling the rule for the rest of the file.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| alt="Continue with Google Icon" | ||
| /> | ||
| <> | ||
| {/* eslint-disable @next/next/no-img-element */} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switch these inline ESLint directives to eslint-disable-next-line @next/next/no-img-element (or re‑enable immediately) so the suppression is limited to the intended <img> tag instead of disabling the rule for the rest of the file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/modules/signup-view.tsx, line 566:
<comment>Switch these inline ESLint directives to `eslint-disable-next-line @next/next/no-img-element` (or re‑enable immediately) so the suppression is limited to the intended `<img>` tag instead of disabling the rule for the rest of the file.</comment>
<file context>
@@ -538,15 +562,24 @@ export default function Signup({
- alt="Continue with Google Icon"
- />
+ <>
+ {/* eslint-disable @next/next/no-img-element */}
+ <img
+ className={classNames("text-subtle mr-2 h-4 w-4", premiumUsername && "opacity-50")}
</file context>
| {/* eslint-disable @next/next/no-img-element */} | |
| {/* eslint-disable-next-line @next/next/no-img-element */} |
| <Icon name="chevron-right" className="h-4 w-4" /> | ||
| )} | ||
| </div> | ||
| { } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| { } |
i guess this was unintended
eunjae-lee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good !
TODO before merge/release
NEXT_PUBLIC_POSTHOG_KEYNEXT_PUBLIC_POSTHOG_HOSTWhat does this PR do?
Upgrades the posthog package and adds tracking a bunch of pages
Mandatory Tasks (DO NOT REMOVE)