-
Notifications
You must be signed in to change notification settings - Fork 548
posthog migration part 6 #7374
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
posthog migration part 6 #7374
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
5 Skipped Deployments
|
WalkthroughThe changes refactor analytics reporting for asset-related actions, replacing generic status-based reporting functions with explicit, dedicated functions for success and failure events across asset buying, creation, and import flows. Reporting calls are updated throughout the codebase to use these new functions, with payload structures and event names adjusted for clarity and granularity. Some component APIs are updated to support new analytics callbacks. Changes
Sequence Diagram(s)Asset Buy Reporting (New Flow)sequenceDiagram
participant User
participant UI_Component
participant Analytics
User->>UI_Component: Initiate asset buy
alt Buy fails
UI_Component->>Analytics: reportAssetBuyFailed({chainId, contractType, assetType, error})
else Buy succeeds
UI_Component->>Analytics: reportAssetBuySuccessful({chainId, contractType, assetType})
end
Asset Creation Step ReportingsequenceDiagram
participant User
participant AssetCreationUI
participant Analytics
User->>AssetCreationUI: Configure step (e.g., "collection-info")
AssetCreationUI->>Analytics: reportAssetCreationStepConfigured({assetType, step})
Asset Creation Status ReportingsequenceDiagram
participant User
participant AssetCreationProcess
participant Analytics
User->>AssetCreationProcess: Complete asset creation steps
alt Step fails
AssetCreationProcess->>Analytics: reportAssetCreationFailed({assetType, contractType, step, error})
else All steps succeed
AssetCreationProcess->>Analytics: reportAssetCreationSuccessful({assetType, contractType})
end
Asset Import ReportingsequenceDiagram
participant User
participant ImportModal
participant Analytics
User->>ImportModal: Start import
ImportModal->>Analytics: reportAssetImportStarted()
alt Import succeeds
ImportModal->>Analytics: reportAssetImportSuccessful()
end
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🪧 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 (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 06-17-posthog_migration_part_5 #7374 +/- ##
===============================================================
Coverage 52.35% 52.35%
===============================================================
Files 939 939
Lines 63161 63161
Branches 4217 4214 -3
===============================================================
Hits 33070 33070
Misses 29984 29984
Partials 107 107
🚀 New features to boost your workflow:
|
63d5bfa
to
870e1c9
Compare
|
size-limit report 📦
|
870e1c9
to
3d432ff
Compare
3d432ff
to
6038701
Compare
6038701
to
f38d2fe
Compare
f38d2fe
to
ac429f1
Compare
@coderabbitai review |
✅ Actions performedReview triggered.
|
410b552
into
06-17-posthog_migration_part_5
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
🧹 Nitpick comments (1)
apps/dashboard/src/@/analytics/report.ts (1)
192-192
: Minor: Extra newline added.An additional blank line was added which doesn't affect functionality but should be consistent with the file's formatting style.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/dashboard/src/@/analytics/report.ts
(2 hunks)apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/public-pages/erc20/_components/claim-tokens/claim-tokens-ui.tsx
(4 hunks)apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/public-pages/nft/overview/buy-edition-drop/buy-edition-drop.client.tsx
(5 hunks)apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/public-pages/nft/overview/buy-nft-drop/buy-nft-drop.client.tsx
(4 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/cards.tsx
(3 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/nft/create-nft-page-ui.tsx
(4 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/nft/create-nft-page.tsx
(5 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/nft/launch/launch-nft.tsx
(2 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/token/create-token-page-impl.tsx
(5 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/token/create-token-page.client.tsx
(3 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/token/launch/launch-token.tsx
(3 hunks)apps/dashboard/src/components/contract-components/import-contract/modal.tsx
(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/nft/launch/launch-nft.tsx (1)
Learnt from: MananTank
PR: thirdweb-dev/js#7315
File: apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/nft/launch-nft.tsx:153-226
Timestamp: 2025-06-10T00:50:20.795Z
Learning: In apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/nft/launch-nft.tsx, the updateStatus function correctly expects a complete MultiStepState["status"] object. For pending states, { type: "pending" } is the entire status object. For error states, { type: "error", message: React.ReactNode } is the entire status object. The current code incorrectly spreads the entire step object instead of passing just the status object.
🧬 Code Graph Analysis (10)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/nft/launch/launch-nft.tsx (1)
apps/dashboard/src/@/analytics/report.ts (1)
reportAssetCreationFailed
(361-383)
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/public-pages/nft/overview/buy-edition-drop/buy-edition-drop.client.tsx (1)
apps/dashboard/src/@/analytics/report.ts (2)
reportAssetBuyFailed
(256-268)reportAssetBuySuccessful
(236-246)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/token/create-token-page.client.tsx (1)
apps/dashboard/src/@/analytics/report.ts (1)
reportAssetCreationStepConfigured
(317-332)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/token/launch/launch-token.tsx (1)
apps/dashboard/src/@/analytics/report.ts (2)
reportAssetCreationFailed
(361-383)reportAssetCreationSuccessful
(342-350)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/token/create-token-page-impl.tsx (1)
apps/dashboard/src/@/analytics/report.ts (1)
reportAssetCreationFailed
(361-383)
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/public-pages/nft/overview/buy-nft-drop/buy-nft-drop.client.tsx (1)
apps/dashboard/src/@/analytics/report.ts (2)
reportAssetBuyFailed
(256-268)reportAssetBuySuccessful
(236-246)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/nft/create-nft-page-ui.tsx (1)
apps/dashboard/src/@/analytics/report.ts (1)
reportAssetCreationStepConfigured
(317-332)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/nft/create-nft-page.tsx (1)
apps/dashboard/src/@/analytics/report.ts (1)
reportAssetCreationFailed
(361-383)
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/public-pages/erc20/_components/claim-tokens/claim-tokens-ui.tsx (1)
apps/dashboard/src/@/analytics/report.ts (2)
reportAssetBuyFailed
(256-268)reportAssetBuySuccessful
(236-246)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/cards.tsx (1)
apps/dashboard/src/@/analytics/report.ts (3)
reportAssetImportSuccessful
(295-297)reportAssetCreationStarted
(280-286)reportAssetImportStarted
(306-308)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
🔇 Additional comments (37)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/token/create-token-page.client.tsx (3)
3-3
: LGTM: Import updated correctlyThe import statement has been properly updated to use the new analytics function as part of the broader reporting refactor.
98-101
: LGTM: Analytics call correctly updatedThe function call has been properly updated to use
reportAssetCreationStepConfigured
with the correct payload structure. TheassetType: "coin"
andstep: "coin-info"
parameters match the expected function signature.
118-121
: LGTM: Consistent analytics implementationThis analytics call follows the same correct pattern as the previous one, using the proper function and payload structure with
step: "token-distribution"
which is valid for coin creation.apps/dashboard/src/components/contract-components/import-contract/modal.tsx (3)
42-42
: LGTM: Optional callback prop added correctlyThe addition of the optional
onSuccess
callback follows standard React patterns and maintains backward compatibility.
73-73
: LGTM: Callback prop passed correctlyThe
onSuccess
prop is properly passed to theImportForm
component, maintaining the callback chain.
101-101
: LGTM: Success callback implemented correctlyThe optional
onSuccess
prop is properly added to the component interface and invoked at the right time after successful contract import. The use of optional chaining prevents runtime errors when the callback is not provided.Also applies to: 168-168
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/nft/create-nft-page-ui.tsx (1)
3-3
: LGTM: Analytics migration implemented correctlyAll analytics calls have been properly updated to use
reportAssetCreationStepConfigured
with the correct payload structure. The step values ("collection-info", "upload-assets", "sales-settings") exactly match the expected union types for NFT creation in the function signature.Also applies to: 90-93, 104-107, 123-126
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/nft/launch/launch-nft.tsx (2)
3-3
: LGTM: Import updated for specific failure reportingThe import has been correctly updated from the generic
reportCreateAssetStatus
to the more specificreportAssetCreationFailed
function, improving analytics granularity.
214-219
: LGTM: Failure reporting correctly implementedThe failure analytics call is properly implemented with all required parameters. The
currentStep.id
values from the stepIds object match the expected step types for NFT creation, ensuring type safety. The error context and step information provide valuable debugging information.apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/token/launch/launch-token.tsx (3)
2-5
: LGTM: Imports updated for specific success/failure reportingThe imports have been correctly updated to include both success and failure reporting functions, supporting more granular analytics tracking for token creation.
148-153
: LGTM: Failure reporting correctly implementedThe failure analytics call is properly implemented with the correct parameters for coin creation. The step values from stepIds match the expected union types, and the error context provides valuable debugging information.
164-167
: LGTM: Success reporting correctly implementedThe success analytics call is properly placed after all steps complete successfully and uses the correct parameters for coin creation. This provides valuable completion tracking for the asset creation flow.
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/public-pages/nft/overview/buy-edition-drop/buy-edition-drop.client.tsx (2)
3-6
: LGTM: Import changes align with analytics refactoring.The updated imports correctly replace the generic reporting function with specific success and failure reporting functions, consistent with the broader analytics migration.
116-121
: Analytics reporting implementation is correct and consistent.The reporting calls properly use:
reportAssetBuySuccessful
for successful purchasesreportAssetBuyFailed
for all failure scenarios (approval failures, claim failures, and general errors)- Correct parameters:
chainId
,contractType: "DropERC1155"
,assetType: "nft"
, anderror
messages for failuresThis aligns with the goal of providing more granular analytics while removing intermediate "attempted" status reporting.
Also applies to: 144-148, 159-165, 176-181
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/public-pages/erc20/_components/claim-tokens/claim-tokens-ui.tsx (2)
3-6
: Import changes correctly implement the analytics refactoring.The new imports align with the migration to explicit success/failure reporting functions.
135-140
: Analytics implementation is accurate for token purchases.The reporting correctly distinguishes between:
- Approval failures and claim failures using
reportAssetBuyFailed
- Successful purchases using
reportAssetBuySuccessful
Parameters are appropriate for ERC20 token purchases:
contractType: "DropERC20"
,assetType: "coin"
.Also applies to: 173-178, 186-190
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/token/create-token-page-impl.tsx (2)
4-4
: Import correctly updated for the analytics refactoring.The replacement of
reportCreateAssetStepStatus
withreportAssetCreationFailed
aligns with the migration to explicit failure reporting.
120-125
: Failure reporting implementation is consistent and comprehensive.All failure scenarios are properly captured with:
- Correct asset type (
"coin"
) and contract type ("DropERC20"
)- Appropriate step names that match the function signature:
"deploy-contract"
,"airdrop-tokens"
,"mint-tokens"
,"set-claim-conditions"
- Parsed error messages for better debugging
The removal of "attempted" and "successful" step reporting simplifies the analytics while maintaining critical failure tracking.
Also applies to: 172-177, 238-243, 323-328
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/create/nft/create-nft-page.tsx (3)
4-4
: Import update aligns with analytics migration.The replacement follows the consistent pattern of moving to explicit failure reporting.
154-154
: Improved error logging consistency.The change from logging raw error objects to using
parseError(error)
provides more consistent and readable error messages across all failure scenarios.Also applies to: 193-193, 242-242, 322-322
156-161
: NFT creation failure reporting is comprehensive and accurate.All failure scenarios are properly tracked with:
- Correct asset type (
"nft"
)- Appropriate contract types (
"DropERC721"
,"DropERC1155"
)- Valid step names matching the function signature:
"deploy-contract"
,"mint-nfts"
,"set-claim-conditions"
- Detailed error information
The simplified analytics approach removes intermediate status tracking while maintaining essential failure monitoring.
Also applies to: 195-200, 244-249, 324-329
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/public-pages/nft/overview/buy-nft-drop/buy-nft-drop.client.tsx (2)
2-5
: Import changes complete the analytics migration pattern.The updated imports are consistent with the refactoring across all asset purchase components.
75-80
: NFT purchase analytics implementation is thorough and consistent.The reporting correctly handles:
- All failure scenarios (approval, claim, and general errors) using
reportAssetBuyFailed
- Successful purchases using
reportAssetBuySuccessful
- Proper ERC721 parameters:
contractType: "DropERC721"
,assetType: "nft"
This completes the consistent analytics refactoring across all asset purchase flows in the application.
Also applies to: 102-106, 113-118, 129-134
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/assets/cards.tsx (6)
3-7
: LGTM: Clean import organization for new analytics functions.The imports are well-organized and align with the analytics refactoring to use specific, granular reporting functions instead of generic ones.
35-37
: LGTM: Proper success callback integration.The
onSuccess
callback correctly triggers thereportAssetImportSuccessful
analytics event when the import modal completes successfully, providing clear tracking of successful asset imports.
45-49
: LGTM: Accurate asset creation tracking for coins.The onClick handler properly reports asset creation start with the correct asset type, enabling better analytics on user interactions with coin creation.
57-61
: LGTM: Accurate asset creation tracking for NFTs.The onClick handler properly reports asset creation start with the correct asset type, enabling better analytics on user interactions with NFT creation.
70-70
: LGTM: Consistent import start tracking.The reporting function name is updated to align with the new analytics pattern, providing clear tracking when users initiate asset imports.
94-100
: LGTM: Simplified and accessible click handling.The CardLink component is properly simplified by removing the intermediate tracking logic and delegating analytics responsibility to the parent component. The keyboard accessibility is maintained correctly.
apps/dashboard/src/@/analytics/report.ts (8)
236-246
: LGTM: Clear success reporting for asset purchases.The function is well-structured with proper JSDoc documentation and captures the essential properties for tracking successful asset purchases. The separation from failure cases improves analytics clarity.
256-268
: LGTM: Comprehensive failure reporting for asset purchases.The function properly captures error information along with context properties, which will be valuable for debugging and improving the user experience. The explicit separation of success and failure events is a good architectural choice.
280-286
: LGTM: Clear asset creation start tracking.The function properly captures the asset type at the beginning of the creation flow, providing valuable funnel analytics data.
295-297
: LGTM: Simple and focused import success tracking.The function provides clear tracking of successful asset imports without unnecessary complexity.
306-308
: LGTM: Clear import start tracking.The function provides straightforward tracking when users initiate asset imports.
317-332
: LGTM: Improved step configuration tracking.The renamed function (
reportAssetCreationStepConfigured
) with updated property names (assetType
andstep
) provides clearer semantics than the previous version. The union type properly handles different steps for NFT and coin creation flows.
342-350
: LGTM: Comprehensive success tracking for asset creation.The function captures both asset type and contract type, providing valuable data for understanding successful creation patterns.
361-383
: LGTM: Comprehensive failure tracking with context.The function design is excellent for debugging and analytics:
- Captures error details for troubleshooting
- Includes the specific failing step for funnel analysis
- Maintains type safety with union types for different asset flows
- Provides actionable data for improving user experience
PR-Codex overview
This PR focuses on enhancing the asset creation and import reporting functionality within the application. It refines event tracking for asset creation, import success, and error handling, ensuring better analytics for user interactions with asset-related features.
Detailed summary
onSuccess
prop toImportModal
andImportForm
.reportCreateAssetStepNextClicked
withreportAssetCreationStepConfigured
.reportAssetCreationStarted
,reportAssetImportStarted
,reportAssetImportSuccessful
,reportAssetCreationFailed
, andreportAssetCreationSuccessful
.Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores