feat: more tracking#1262
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
Request changes: add portfolio→vault click attribution. Two events (matching the per-surface convention like VAULT_CLICK_LIST_ROW / LANDER_VAULT_CLICK):
-
VAULT_CLICK_PORTFOLIO_LIST_ROW— holdings click.VaultsListRowhardcodesVAULT_CLICK_LIST_ROW(VaultsListRow.tsx:502); add an optionalclickEventNameprop and pass this from the portfolio holdings. -
VAULT_CLICK_PORTFOLIO_SUGGESTED— suggested click.SuggestedVaultCardhas no tracking and is shared withTrendingVaultson /vaults — pass the event as a prop set only from the portfolio, don't hardcode it.
Reviewed using the review-pr skill.
murderteeth
left a comment
There was a problem hiding this comment.
Summary
Previous review items are resolved — clickEventName works as requested on both VaultsListRow and SuggestedVaultCard. Verified on the Vercel preview.
But the branch now fails bun run test.
Issue
-
SuggestedVaultCard.test.tsxfails
This PR makes SuggestedVaultCard import usePlausible, which pulls the real @plausible-analytics/tracker package into the test. Vitest can't resolve that package (it ships no main/exports), so the test file crashes before running.
Fix by stubbing the hook, same as the other mocks already in the file (verified working):
vi.mock('@hooks/usePlausible', () => ({
usePlausible: () => vi.fn()
}))Verdict
REQUEST_CHANGES
How This Was Reviewed
This review was conducted using the review-pr skill.
New events to track:
plausible was discussed here - #1201 (review)