-
Notifications
You must be signed in to change notification settings - Fork 6
add posthog #466
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
base: main
Are you sure you want to change the base?
add posthog #466
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis change integrates PostHog analytics into the application. It adds environment configuration, initializes PostHog in the app, introduces a custom React hook for analytics tied to wallet events, and tracks blockchain transactions. Supporting code includes new tests for analytics and transaction hooks, dependency updates, and Next.js rewrite rules for PostHog API endpoints. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant PostHogProvider
participant useManifestPostHog
participant PostHog
User->>App: Connects wallet
App->>useManifestPostHog: Wallet state changes
useManifestPostHog->>PostHog: Identify user, capture "wallet_connected"
User->>App: Broadcasts transaction
App->>useManifestPostHog: trackTransaction(metadata)
useManifestPostHog->>PostHog: Capture "transaction_success" or "transaction_failed"
User->>App: Disconnects wallet
App->>useManifestPostHog: Wallet state changes
useManifestPostHog->>PostHog: Capture "wallet_disconnected", reset user
Suggested reviewers
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #466 +/- ##
==========================================
+ Coverage 58.66% 59.80% +1.14%
==========================================
Files 237 238 +1
Lines 16354 16445 +91
==========================================
+ Hits 9594 9835 +241
+ Misses 6760 6610 -150 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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: 0
🔭 Outside diff range comments (1)
hooks/useTx.tsx (1)
122-185
: 🛠️ Refactor suggestionRefactor to reduce code duplication and add error handling.
The tracking logic for success and failure cases contains significant duplication, particularly in fee object construction and data mapping. Additionally, there's no error handling for the tracking calls.
Consider extracting the common tracking logic:
+ // Helper function to build tracking data + const buildTrackingData = (success: boolean, errorMessage?: string) => ({ + success, + transactionHash: res.transactionHash, + chainId: chainName, + messageTypes: msgs.map(msg => msg.typeUrl), + fee: fee + ? { + amount: fee.amount[0]?.amount || '0', + denom: fee.amount[0]?.denom || 'unknown', + } + : undefined, + memo: options.memo, + gasUsed: res.gasUsed?.toString(), + gasWanted: res.gasWanted?.toString(), + height: res.height?.toString(), + ...(errorMessage && { error: errorMessage }), + }); + + // Helper function to safely track transaction + const safeTrackTransaction = (data: any) => { + try { + trackTransaction(data); + } catch (error) { + console.warn('Failed to track transaction:', error); + } + }; if (isDeliverTxSuccess(res)) { if (options.onSuccess) options.onSuccess(); - // Track successful transaction - trackTransaction({ - success: true, - transactionHash: res.transactionHash, - chainId: chainName, - messageTypes: msgs.map(msg => msg.typeUrl), - fee: fee - ? { - amount: fee.amount[0]?.amount || '0', - denom: fee.amount[0]?.denom || 'unknown', - } - : undefined, - memo: options.memo, - gasUsed: res.gasUsed?.toString(), - gasWanted: res.gasWanted?.toString(), - height: res.height?.toString(), - }); + safeTrackTransaction(buildTrackingData(true)); // ... existing success toast logic ... } else { - // Track failed transaction - trackTransaction({ - success: false, - transactionHash: res.transactionHash, - chainId: chainName, - messageTypes: msgs.map(msg => msg.typeUrl), - fee: fee - ? { - amount: fee.amount[0]?.amount || '0', - denom: fee.amount[0]?.denom || 'unknown', - } - : undefined, - memo: options.memo, - error: res?.rawLog || 'Unknown error', - gasUsed: res.gasUsed?.toString(), - gasWanted: res.gasWanted?.toString(), - height: res.height?.toString(), - }); + safeTrackTransaction(buildTrackingData(false, res?.rawLog || 'Unknown error'));🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 122-139: hooks/useTx.tsx#L122-L139
Added lines #L122 - L139 were not covered by tests
[warning] 167-185: hooks/useTx.tsx#L167-L185
Added lines #L167 - L185 were not covered by tests
🧹 Nitpick comments (2)
hooks/useTx.tsx (1)
122-139
: Add test coverage for transaction tracking.The static analysis correctly identifies that the new tracking code lacks test coverage. This is important functionality that should be tested.
Consider adding tests that verify:
trackTransaction
is called with correct data on successful transactionstrackTransaction
is called with error details on failed transactions- Transaction flow continues normally even if tracking fails
Would you like me to help generate test cases for the transaction tracking functionality?
Also applies to: 167-185
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 122-139: hooks/useTx.tsx#L122-L139
Added lines #L122 - L139 were not covered by testshooks/usePostHog.tsx (1)
1-120
: Add test coverage for the PostHog analytics hook.The static analysis indicates missing test coverage for this new hook. While the implementation is solid, adding tests would ensure reliability of the analytics tracking functionality.
Would you like me to generate comprehensive unit tests for the
useManifestPostHog
hook to improve test coverage?🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 32-32: hooks/usePostHog.tsx#L32
Added line #L32 was not covered by tests
[warning] 35-43: hooks/usePostHog.tsx#L35-L43
Added lines #L35 - L43 were not covered by tests
[warning] 46-52: hooks/usePostHog.tsx#L46-L52
Added lines #L46 - L52 were not covered by tests
[warning] 55-57: hooks/usePostHog.tsx#L55-L57
Added lines #L55 - L57 were not covered by tests
[warning] 65-69: hooks/usePostHog.tsx#L65-L69
Added lines #L65 - L69 were not covered by tests
[warning] 71-73: hooks/usePostHog.tsx#L71-L73
Added lines #L71 - L73 were not covered by tests
[warning] 77-111: hooks/usePostHog.tsx#L77-L111
Added lines #L77 - L111 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lock
is excluded by!**/*.lock
📒 Files selected for processing (7)
config/env.ts
(1 hunks)hooks/index.tsx
(1 hunks)hooks/usePostHog.tsx
(1 hunks)hooks/useTx.tsx
(5 hunks)next.config.js
(1 hunks)package.json
(1 hunks)pages/_app.tsx
(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
hooks/useTx.tsx (1)
hooks/usePostHog.tsx (1)
useManifestPostHog
(23-120)
🪛 GitHub Check: codecov/patch
hooks/useTx.tsx
[warning] 122-139: hooks/useTx.tsx#L122-L139
Added lines #L122 - L139 were not covered by tests
[warning] 167-185: hooks/useTx.tsx#L167-L185
Added lines #L167 - L185 were not covered by tests
[warning] 199-199: hooks/useTx.tsx#L199
Added line #L199 was not covered by tests
hooks/usePostHog.tsx
[warning] 32-32: hooks/usePostHog.tsx#L32
Added line #L32 was not covered by tests
[warning] 35-43: hooks/usePostHog.tsx#L35-L43
Added lines #L35 - L43 were not covered by tests
[warning] 46-52: hooks/usePostHog.tsx#L46-L52
Added lines #L46 - L52 were not covered by tests
[warning] 55-57: hooks/usePostHog.tsx#L55-L57
Added lines #L55 - L57 were not covered by tests
[warning] 65-69: hooks/usePostHog.tsx#L65-L69
Added lines #L65 - L69 were not covered by tests
[warning] 71-73: hooks/usePostHog.tsx#L71-L73
Added lines #L71 - L73 were not covered by tests
[warning] 77-111: hooks/usePostHog.tsx#L77-L111
Added lines #L77 - L111 were not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (17)
package.json (1)
79-80
:✅ Verification successful
Verify PostHog package versions and security status.
Please ensure these PostHog package versions are current and free from known vulnerabilities.
🌐 Web query:
What are the latest versions of posthog-js and posthog-node npm packages? Are there any known security vulnerabilities in versions 1.249.5 and 4.18.0 respectively?
💡 Result:
As of June 9, 2025, the latest versions of the PostHog JavaScript libraries are:
posthog-js: Version 1.249.5, published 4 hours ago.
posthog-node: Version 4.18.0, published 13 days ago.
Regarding security vulnerabilities:
posthog-js: The latest version, 1.249.5, does not have any known security vulnerabilities. Previous versions, such as 1.57.2 and earlier, had a medium-severity Cross-Site Scripting (XSS) vulnerability. However, this issue was addressed in version 1.57.2. (security.snyk.io)
posthog-node: The latest version, 4.18.0, does not have any known security vulnerabilities. Earlier versions, including 4.10.1, also do not have reported vulnerabilities. (security.snyk.io)
Therefore, both versions 1.249.5 of posthog-js and 4.18.0 of posthog-node are secure and free from known vulnerabilities.
Citations:
- 1: https://security.snyk.io/package/npm/posthog-js?utm_source=openai
- 2: https://security.snyk.io/package/npm/posthog-node?utm_source=openai
PostHog dependencies are up to date and secure.
Both [email protected] and [email protected] are the latest published versions (as of June 9, 2025) and have no known security vulnerabilities.
config/env.ts (1)
59-61
: LGTM! Environment variables follow established patterns.The PostHog configuration variables are properly structured with appropriate defaults and follow the existing naming conventions.
hooks/index.tsx (1)
11-11
: LGTM! Export follows established pattern.The new PostHog hook export is consistent with the existing barrel export structure.
hooks/useTx.tsx (2)
10-10
: LGTM! PostHog integration import is appropriate.The import follows the established pattern and integrates well with the existing hook structure.
49-49
: LGTM! Hook destructuring is clean and follows patterns.The trackTransaction function is properly extracted from the PostHog hook.
pages/_app.tsx (4)
4-6
: LGTM: PostHog imports are correctly added.The imports for PostHog client and React provider are properly structured.
19-34
: LGTM: PostHog initialization is properly configured.The client-side initialization correctly:
- Checks for environment variable before initializing
- Uses
/ingest
as api_host which aligns with the proxy configuration- Disables autocapture for better performance and privacy control
- Enables debug mode only in development
- Captures exceptions for error tracking
39-48
: LGTM: PostHogProvider wrapper is correctly implemented.The PostHogProvider properly wraps the app to provide analytics context throughout the application.
76-76
: LGTM: Minor formatting improvement for readability.The backslash continuation improves the className formatting without affecting functionality.
next.config.js (2)
82-97
: LGTM: PostHog API proxy configuration is correctly implemented.The rewrites configuration properly:
- Routes static assets to the PostHog assets endpoint
- Routes API calls to the PostHog ingestion endpoint
- Includes the specific
/decide
endpoint for PostHog functionality- Orders rules correctly with more specific patterns first
99-99
: LGTM: Required setting for PostHog API compatibility.The
skipTrailingSlashRedirect
setting is necessary to support PostHog's API request patterns.hooks/usePostHog.tsx (6)
7-21
: LGTM: Comprehensive transaction event interface.The
TransactionEvent
interface captures all relevant transaction metadata including success/failure states, gas usage, fees, and error information.
23-28
: LGTM: Proper hook setup with state management.The hook correctly integrates PostHog with Cosmos wallet state and uses refs to track identification state efficiently.
30-59
: LGTM: Efficient user identification logic.The identification logic properly:
- Only identifies users when address or wallet changes
- Includes comprehensive wallet and chain metadata
- Tracks wallet connection events
- Uses refs to prevent redundant API calls
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 32-32: hooks/usePostHog.tsx#L32
Added line #L32 was not covered by tests
[warning] 35-43: hooks/usePostHog.tsx#L35-L43
Added lines #L35 - L43 were not covered by tests
[warning] 46-52: hooks/usePostHog.tsx#L46-L52
Added lines #L46 - L52 were not covered by tests
[warning] 55-57: hooks/usePostHog.tsx#L55-L57
Added lines #L55 - L57 were not covered by tests
62-75
: LGTM: Proper cleanup on wallet disconnect.The disconnect handling correctly:
- Tracks disconnection events with previous address context
- Resets PostHog state
- Clears identification refs
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 65-69: hooks/usePostHog.tsx#L65-L69
Added lines #L65 - L69 were not covered by tests
[warning] 71-73: hooks/usePostHog.tsx#L71-L73
Added lines #L71 - L73 were not covered by tests
77-113
: LGTM: Comprehensive transaction tracking implementation.The
trackTransaction
function properly:
- Guards against missing PostHog instance
- Captures detailed transaction metadata
- Uses groups for analytics segmentation
- Updates user properties efficiently
- Only updates when user is properly identified
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 77-111: hooks/usePostHog.tsx#L77-L111
Added lines #L77 - L111 were not covered by tests
115-120
: LGTM: Clean hook return interface.The return object provides all necessary functionality with a clear
isReady
flag for consumers.
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: 7
🧹 Nitpick comments (2)
hooks/__tests__/useTx.test.tsx (2)
114-117
: Consider using proper types instead ofany
for mock variablesWhile using
any
is common in test files, defining proper types or interfaces for your mocks would improve type safety and make the tests more maintainable.For example:
- let mockClient: any; - let mockSetToastMessage: any; - let mockTrackTransaction: any; - let mockWeb3AuthContext: any; + let mockClient: ReturnType<typeof jest.fn> & { + simulate: jest.Mock; + sign: jest.Mock; + broadcastTx: jest.Mock; + }; + let mockSetToastMessage: jest.Mock; + let mockTrackTransaction: jest.Mock; + let mockWeb3AuthContext: { + isSigning: boolean; + setIsSigning: jest.Mock; + setPromptId: jest.Mock; + };
199-553
: Consider organizing tests into logical groupsThe test cases provide excellent coverage. Consider grouping related tests using nested
describe
blocks for better organization and readability.For example:
describe('useTx', () => { // ... setup code ... + describe('wallet connection', () => { test('handles wallet not connected', async () => { // ... existing test ... }); + }); + describe('simulation', () => { test('handles successful simulation', async () => { // ... existing test ... }); test('handles simulation error with message extraction', async () => { // ... existing test ... }); // ... other simulation tests ... + }); + describe('transaction execution', () => { test('handles fee function', async () => { // ... existing test ... }); // ... other transaction tests ... + }); + describe('analytics tracking', () => { test('tracks successful transaction', async () => { // ... existing test ... }); // ... other tracking tests ... + }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
components/react/__tests__/__snapshots__/ModalDialog.test.tsx.snap
is excluded by!**/*.snap
📒 Files selected for processing (2)
hooks/__tests__/usePostHog.test.tsx
(1 hunks)hooks/__tests__/useTx.test.tsx
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
hooks/__tests__/usePostHog.test.tsx (3)
hooks/usePostHog.tsx (1)
useManifestPostHog
(23-120)tests/mock.ts (3)
mockRouter
(70-78)mockModule
(23-60)clearAllMocks
(11-15)tests/render.tsx (1)
renderWithChainProvider
(22-35)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (5)
hooks/__tests__/useTx.test.tsx (2)
22-111
: Well-structured test componentThe TestComponent provides comprehensive coverage of different transaction scenarios with clear, focused handlers for each test case. The implementation follows testing best practices.
310-331
: Excellent PostHog analytics integration testingThe test properly verifies that transaction events are tracked with all relevant metadata including success status, transaction hash, chain ID, message types, fee details, and gas usage. This aligns well with the PR's objective of integrating PostHog analytics.
hooks/__tests__/usePostHog.test.tsx (3)
1-8
: Imports and setup look good.The test file uses appropriate testing utilities and follows good practices by importing the hook under test.
16-57
: TestComponent implementation is well-structured.The test component provides a clean interface for testing the hook's functionality with appropriate test IDs and user interactions.
63-97
: Comprehensive mock setup with proper dependencies.The beforeEach setup correctly mocks all necessary dependencies including PostHog, wallet state, router, and environment configuration.
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
hooks/useTx.tsx
(6 hunks)next.config.mjs
(1 hunks)package.json
(1 hunks)pages/_app.tsx
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- pages/_app.tsx
- hooks/useTx.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (1)
next.config.mjs (1)
35-35
: GlobalskipTrailingSlashRedirect
may introduce duplicate-URL permutationsDisabling trailing-slash redirects for the whole site can create both
/page
and/page/
URLs, harming SEO and cache hit-ratio.
If the waiver is required only for PostHog endpoints, consider:async redirects() { return [ // keep default behaviour site-wide { source: '/(.*)/', destination: '/$1', permanent: false }, ]; },Or handle it at the rewrite pattern level instead of a global flag.
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 (4)
.env.test (4)
36-38
: Remove extra blank lines.The blank lines before the PostHog section create unnecessary gaps. Removing them improves readability.
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 37-37: [ExtraBlankLine] Extra blank line detected
39-40
: Alphabetical order of PostHog keys.Per dotenv-linter,
NEXT_PUBLIC_POSTHOG_ASSETS_HOST
should precedeNEXT_PUBLIC_POSTHOG_HOST
for consistent ordering.Apply:
- NEXT_PUBLIC_POSTHOG_HOST=https://us.i.posthog.com - NEXT_PUBLIC_POSTHOG_ASSETS_HOST=https://us-assets.i.posthog.com + NEXT_PUBLIC_POSTHOG_ASSETS_HOST=https://us-assets.i.posthog.com + NEXT_PUBLIC_POSTHOG_HOST=https://us.i.posthog.com🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 40-40: [UnorderedKey] The NEXT_PUBLIC_POSTHOG_ASSETS_HOST key should go before the NEXT_PUBLIC_POSTHOG_HOST key
46-49
: Avoid duplicate environment keys.Defining
NEXT_PUBLIC_POSTHOG_ASSETS_HOST
in multiple sections of the same file can lead to unpredictable overrides. Consider splitting environment-specific variables into separate files (e.g.,.env.production
,.env.development
,.env.test
) to keep each file focused and prevent duplication.🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 48-48: [DuplicatedKey] The NEXT_PUBLIC_POSTHOG_ASSETS_HOST key is duplicated
52-52
: Add trailing newline.Ensure the file ends with a newline to satisfy POSIX standards and suppress linter warnings.
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 52-52: [EndingBlankLine] No blank line at the end of the file
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.env.test
(1 hunks)next.config.mjs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- next.config.mjs
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.test
[warning] 37-37: [ExtraBlankLine] Extra blank line detected
[warning] 40-40: [UnorderedKey] The NEXT_PUBLIC_POSTHOG_ASSETS_HOST key should go before the NEXT_PUBLIC_POSTHOG_HOST key
[warning] 48-48: [DuplicatedKey] The NEXT_PUBLIC_POSTHOG_ASSETS_HOST key is duplicated
[warning] 52-52: [EndingBlankLine] No blank line at the end of the file
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (1)
.env.test (1)
46-49
: Verify self-hosted PostHog configuration.The self-hosted
NEXT_PUBLIC_POSTHOG_HOST
is commented out, whileNEXT_PUBLIC_POSTHOG_ASSETS_HOST
is active. Confirm if this asymmetry is intentional; otherwise, you may end up fetching assets from your host but sending events to the default PostHog cloud.🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 48-48: [DuplicatedKey] The NEXT_PUBLIC_POSTHOG_ASSETS_HOST key is duplicated
This PR adds posthog for analytics tracking
Summary by CodeRabbit