Feat(project): amplitude event 연결#328
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…eps 수정 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
✅ Storybook chromatic 배포 확인: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough애널리틱스 이벤트(클릭·임프레션·저장) 통합, Amplitude 이벤트/메서드 리팩토링, 여러 파일의 TypeScript 타입 임포트를 Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Client Browser
participant UI as App UI (Card)
participant Analytics as Analytics (ampli)
participant Router as Navigation
UI->>Analytics: Impression event on in-view\n("Impression_Saved_Content", bookmark_type)
UI->>Analytics: Click event on click\n("Clicked_*", {article_id, category_id})
UI->>Router: open article (window.open)
Analytics-->>Analytics: enqueue/send event
sequenceDiagram
participant SW as ServiceWorker
participant FCM as Firebase Messaging
participant Amplitude as Amplitude API
participant Client as Client (focus/navigation)
FCM->>SW: onBackgroundMessage(payload)
SW->>Amplitude: trackAmplitudeEvent('Triggered_Reminder')
SW->>SW: showNotification(...)
SW->>SW: notificationclick
SW->>Amplitude: trackAmplitudeEvent('Clicked_alarm')
SW->>Client: clients.openWindow(notificationTarget)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/client/src/pages/myBookmark/hooks/useMyBookmarkContentData.ts (1)
8-14:⚠️ Potential issue | 🟡 Minor
scrollContainerRef의 타입을RefObject로 변경하세요.
useInfiniteScroll훅의 타입 정의(line 13)에서rootRef?: React.RefObject<HTMLElement | null>로 명시하고 있습니다. 실제 사용처(line 33)에서도rootRef?.current를 읽기만 수행하므로, 파라미터 타입을MutableRefObject에서RefObject로 수정하여 타입 계약을 일치시켜야 합니다.제안 diff 보기
-import { type MutableRefObject } from 'react'; +import { type RefObject } from 'react'; ... - scrollContainerRef: MutableRefObject<HTMLDivElement | null>; + scrollContainerRef: RefObject<HTMLDivElement | null>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/pages/myBookmark/hooks/useMyBookmarkContentData.ts` around lines 8 - 14, The parameter type for scrollContainerRef in the UseMyBookmarkContentDataParams interface is too broad; change it from MutableRefObject<HTMLDivElement | null> to React.RefObject<HTMLDivElement | null> so it matches the useInfiniteScroll signature (rootRef?: React.RefObject<HTMLElement | null>) and usage where only rootRef?.current is read; update the import to include React if necessary and adjust any callers to pass a RefObject.
🧹 Nitpick comments (2)
apps/client/src/shared/components/analyticsCardWrapper/AnalyticsCardWrapper.tsx (1)
27-27: 래퍼div가 레이아웃에 영향을 줄 수 있습니다.추가된
<div>요소가 flex/grid 레이아웃에서 예상치 못한 스타일링 문제를 일으킬 수 있습니다. 필요 시classNameprop을 추가하여 스타일 제어가 가능하도록 하는 것을 고려해 주세요.♻️ 선택적 개선안
interface AnalyticsCardWrapperProps { bookmarkType: ImpressionSavedContentProperties['bookmark_type']; children: React.ReactNode; + className?: string; } const AnalyticsCardWrapper = ({ bookmarkType, children, + className, }: AnalyticsCardWrapperProps) => { const { ref, inView } = useInView({ threshold: 0.5, triggerOnce: true }); useEffect(() => { if (inView) { analytics.track('Impression_Saved_Content', { bookmark_type: bookmarkType, }); } }, [inView, bookmarkType]); - return <div ref={ref}>{children}</div>; + return <div ref={ref} className={className}>{children}</div>; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/shared/components/analyticsCardWrapper/AnalyticsCardWrapper.tsx` at line 27, The wrapper div in AnalyticsCardWrapper may disrupt flex/grid layouts; update the AnalyticsCardWrapper component to accept an optional className prop (e.g., className?: string) and pass it through to the returned <div> (alongside the existing ref and children) so consumers can control layout/styling, ensuring the ref forwarding (ref) remains intact; alternatively consider using a React.Fragment when no extra DOM node is needed, but if keeping the div, add and forward className to it.apps/client/public/firebase-messaging-sw.js (1)
6-21:device_id가 하드코딩되어 사용자 간 이벤트 구분이 불가능합니다.현재
device_id: 'serviceworker'로 고정되어 있어 모든 사용자의 서비스 워커 이벤트가 동일한 디바이스로 집계됩니다. 이로 인해 개별 사용자 행동 분석이 어려워질 수 있습니다.가능하다면 IndexedDB나 다른 저장소에서 사용자 ID 또는 디바이스 ID를 가져와 사용하는 것을 고려해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/public/firebase-messaging-sw.js` around lines 6 - 21, The trackAmplitudeEvent function currently hardcodes device_id as 'serviceworker' so events can't be attributed to individual users; update trackAmplitudeEvent to read a persisted identifier (user ID or device ID) from a storage accessible in the service worker (e.g., IndexedDB key like 'deviceId' or 'userId') and use that value for the events.device_id in the payload sent to AMPLITUDE_API_KEY; if none exists, generate a UUID, persist it to the same storage for future calls, and use it as the device_id; ensure the fetch body uses that retrieved/generated identifier instead of the static 'serviceworker'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/client/src/pages/jobPins/JobPins.tsx`:
- Around line 83-86: The analytics.track call can throw when article.category is
undefined; update the analytics.track('Clicked_Shared_Bookmark', ...) invocation
to defensively access category by using optional chaining or a fallback (e.g.,
String(article.category?.categoryId ?? '')) so category_id is always a string
and no runtime error occurs; locate the analytics.track call in JobPins.tsx and
replace the direct article.category.categoryId access with a safe expression.
In `@apps/extension/src/pages/MainPop.tsx`:
- Around line 217-222: The analytics.track call uses new URL(url).hostname which
can throw and abort the save flow; wrap the hostname extraction in a safe parse
(try/catch or a helper like getHostnameSafe) so that if parsing fails you fall
back to undefined (e.g., const page_domain = safeHostname(url) ?? undefined) and
then call analytics.track({ page_domain, category_id: selected ?? undefined, ...
}) without letting an exception prevent save(...) from being invoked; ensure the
change touches the same analytics.track invocation and that save(...) remains
called even when URL parsing fails.
---
Outside diff comments:
In `@apps/client/src/pages/myBookmark/hooks/useMyBookmarkContentData.ts`:
- Around line 8-14: The parameter type for scrollContainerRef in the
UseMyBookmarkContentDataParams interface is too broad; change it from
MutableRefObject<HTMLDivElement | null> to React.RefObject<HTMLDivElement |
null> so it matches the useInfiniteScroll signature (rootRef?:
React.RefObject<HTMLElement | null>) and usage where only rootRef?.current is
read; update the import to include React if necessary and adjust any callers to
pass a RefObject.
---
Nitpick comments:
In `@apps/client/public/firebase-messaging-sw.js`:
- Around line 6-21: The trackAmplitudeEvent function currently hardcodes
device_id as 'serviceworker' so events can't be attributed to individual users;
update trackAmplitudeEvent to read a persisted identifier (user ID or device ID)
from a storage accessible in the service worker (e.g., IndexedDB key like
'deviceId' or 'userId') and use that value for the events.device_id in the
payload sent to AMPLITUDE_API_KEY; if none exists, generate a UUID, persist it
to the same storage for future calls, and use it as the device_id; ensure the
fetch body uses that retrieved/generated identifier instead of the static
'serviceworker'.
In
`@apps/client/src/shared/components/analyticsCardWrapper/AnalyticsCardWrapper.tsx`:
- Line 27: The wrapper div in AnalyticsCardWrapper may disrupt flex/grid
layouts; update the AnalyticsCardWrapper component to accept an optional
className prop (e.g., className?: string) and pass it through to the returned
<div> (alongside the existing ref and children) so consumers can control
layout/styling, ensuring the ref forwarding (ref) remains intact; alternatively
consider using a React.Fragment when no extra DOM node is needed, but if keeping
the div, add and forward className to it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 567e688a-ca32-4c18-8b47-da337e4f4791
📒 Files selected for processing (33)
.vscode/settings.jsonapps/client/public/firebase-messaging-sw.jsapps/client/src/pages/jobPins/JobPins.tsxapps/client/src/pages/jobPins/apis/axios.tsapps/client/src/pages/jobPins/apis/queries.tsapps/client/src/pages/level/Level.tsxapps/client/src/pages/level/components/LevelInfoCard.tsxapps/client/src/pages/level/components/LevelScene.tsxapps/client/src/pages/myBookmark/MyBookmark.tsxapps/client/src/pages/myBookmark/apis/axios.tsapps/client/src/pages/myBookmark/apis/queries.tsapps/client/src/pages/myBookmark/components/myBookmarkContent/MyBookmarkContent.tsxapps/client/src/pages/myBookmark/hooks/useMyBookmarkContentData.tsapps/client/src/pages/onBoarding/components/funnel/MainCard.tsxapps/client/src/pages/onBoarding/components/funnel/step/job/JobStep.tsxapps/client/src/pages/onBoarding/components/timePicker/TimePicker.tsxapps/client/src/pages/onBoarding/hooks/useOnboardingFunnel.tsapps/client/src/pages/remind/Remind.tsxapps/client/src/shared/apis/axios.tsapps/client/src/shared/apis/queries.tsapps/client/src/shared/components/analyticsCardWrapper/AnalyticsCardWrapper.tsxapps/client/src/shared/components/balloon/Balloon.tsxapps/client/src/shared/components/cardEditModal/CardEditModal.tsxapps/client/src/shared/components/jobSelectionFunnel/step/job/JobStep.tsxapps/client/src/shared/components/sidebar/AccordionItem.tsxapps/client/src/shared/components/sidebar/SideItem.tsxapps/client/src/shared/components/sidebar/hooks/useCategoryActions.tsapps/client/src/shared/utils/fetchOgData.tsapps/client/src/shared/utils/treeLevel.tsapps/extension/src/pages/MainPop.tsxpackages/analytics/index.tspackages/analytics/src/ampli/index.tspackages/eslint-config/react-internal.js
💤 Files with no reviewable changes (1)
- apps/client/src/pages/myBookmark/MyBookmark.tsx
| analytics.track('Clicked_Shared_Bookmark', { | ||
| article_id: String(article.articleId), | ||
| category_id: String(article.category.categoryId), | ||
| }); |
There was a problem hiding this comment.
카테고리 없는 카드 클릭 시 런타임 에러 가능성이 있습니다.
Line 85는 article.category가 없을 때 예외가 발생할 수 있습니다. 이미 다른 props에서 optional 접근을 쓰고 있어 여기서도 동일하게 방어가 필요합니다.
수정 제안
analytics.track('Clicked_Shared_Bookmark', {
article_id: String(article.articleId),
- category_id: String(article.category.categoryId),
+ category_id: article.category
+ ? String(article.category.categoryId)
+ : undefined,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| analytics.track('Clicked_Shared_Bookmark', { | |
| article_id: String(article.articleId), | |
| category_id: String(article.category.categoryId), | |
| }); | |
| analytics.track('Clicked_Shared_Bookmark', { | |
| article_id: String(article.articleId), | |
| category_id: article.category | |
| ? String(article.category.categoryId) | |
| : undefined, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/client/src/pages/jobPins/JobPins.tsx` around lines 83 - 86, The
analytics.track call can throw when article.category is undefined; update the
analytics.track('Clicked_Shared_Bookmark', ...) invocation to defensively access
category by using optional chaining or a fallback (e.g.,
String(article.category?.categoryId ?? '')) so category_id is always a string
and no runtime error occurs; locate the analytics.track call in JobPins.tsx and
replace the direct article.category.categoryId access with a safe expression.
📌 Related Issues
📄 Tasks
ampli pull을 통해Viewed_Saved_Content→Impression_Saved_Content이벤트로 교체@pinback/analytics패키지에서 ampli 타입 전체 re-export (export type *)AnalyticsCardWrapper추가react-intersection-observer의useInView를 활용해 카드가 뷰포트에 노출될 때 이벤트 발화 (triggerOnce: true)Impression_Saved_Content이벤트 연결 (bookmark_type으로 탭 구분)consistent-type-imports룰 추가 및 저장 시 자동 수정 활성화⭐ PR Point (To Reviewer)
Saved_Shared_Bookmark이벤트는 dashboard → 외부 사이트 → extension 저장으로 이어지는 크로스 컨텍스트 구조상 신뢰도 있는 트래킹이 어려워 연결하지 않았습니다.Impression_Saved_Content의article_id,category_id프로퍼티는 유저별 내부 ID라 cross-user 집계가 불가능하여 현재 미사용 처리하였습니다.📷 Screenshot
Summary by CodeRabbit
New Features
Refactor