[FEAT] Amplitude + GA4 듀얼 프로바이더 기반 사용자 행동 분석 시스템 구축#444
Open
[FEAT] Amplitude + GA4 듀얼 프로바이더 기반 사용자 행동 분석 시스템 구축#444
Conversation
Walkthrough애널리틱스 인프라와 이벤트 추적이 추가되었습니다. 다중 공급자 관리, Amplitude/GA4/Noop 공급자, 이벤트/유형 정의, 페이지·타이머·로그인·템플릿·투표 흐름의 트래킹 훅 및 관련 테스트와 문서가 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant App as 애플리케이션
participant AM as AnalyticsManager
participant Provider as Provider(Amplitude/GA4/Noop)
participant SDK as Analytics SDK
rect rgba(100,150,200,0.5)
Note over App,AM: 초기화
App->>AM: addProvider(Provider)
App->>AM: init()
AM->>Provider: init()
Provider->>SDK: initialize(apiKey)
end
rect rgba(150,100,200,0.5)
Note over App,AM: 사용자 식별/속성 설정
App->>AM: setUserId(memberId)
AM->>Provider: setUserId(memberId)
Provider->>SDK: setUserId(...)
App->>AM: setUserProperties({user_type, language})
AM->>Provider: setUserProperties(...)
Provider->>SDK: identify(...)
end
rect rgba(200,150,100,0.5)
Note over App,AM: 이벤트 전송 흐름
App->>AM: trackPageView({...})
AM->>AM: enrichPayload(globalProps)
AM->>Provider: trackPageView(enriched)
Provider->>SDK: track('page_view', enriched)
App->>AM: trackEvent(eventName, props)
AM->>AM: enrichPayload(globalProps)
AM->>Provider: trackEvent(eventName, enriched)
Provider->>SDK: track(eventName, enriched)
end
sequenceDiagram
participant User as 사용자
participant Page as 페이지 컴포넌트
participant PT as usePageTracking
participant AM as AnalyticsManager
User->>Page: 방문
Page->>PT: hook 실행 (normalizedPath 계산)
PT->>AM: trackPageView({page_title, previous_path, ...})
Note over PT: entryTime 기록
User->>Page: 떠남 또는 라우트 변경
PT->>PT: duration 계산
PT->>AM: trackEvent('page_leave', {duration_ms,...})
PT->>AM: flush()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 분 Possibly related PRs
Suggested reviewers
🎉 토끼의 축시
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a robust user analytics system using an Adapter pattern to integrate GA4 and Amplitude, featuring an AnalyticsManager, dedicated providers, and custom hooks for tracking page views and user interactions. Feedback highlights a critical security risk concerning potentially malicious dependency versions. Other recommendations focus on improving maintainability by using defined constants for language fallbacks and correcting inconsistencies between the documentation, technical contracts, and the implementation.
Comment on lines
+22
to
25
| "@amplitude/analytics-browser": "^2.39.0", | ||
| "@tanstack/eslint-plugin-query": "^5.91.4", | ||
| "@tanstack/react-query": "^5.90.20", | ||
| "axios": "^1.13.4", |
There was a problem hiding this comment.
패키지 버전이 의심스럽습니다. package.json과 package-lock.json에 명시된 @amplitude/analytics-browser의 버전 2.39.0과 axios의 버전 1.13.4는 공식 npm 레지스트리에 존재하지 않는 것으로 보입니다. 이는 오타일 수도 있지만, typosquatting을 통한 악성 패키지 설치의 위험이 있습니다.
보안을 위해 공식 레지스트리에서 각 패키지의 최신 안정 버전을 확인하고 수정해 주시기 바랍니다. 예를 들어, @amplitude/analytics-browser의 최신 버전은 2.9.0입니다.
References
- When a dependency version is flagged as suspicious or too high, verify its authenticity and latest version on the official package registry (e.g., NPM) before taking action, as the flagging tool's data may be outdated.
Comment on lines
+297
to
+298
| - `timer_save_prompt`: 타이머에서 저장 유도로 로그인 | ||
| - `share_prompt`: 공유 기능 사용 시 로그인 유도 |
There was a problem hiding this comment.
문서에 기재된 trigger_context 값이 실제 구현과 다릅니다. 이로 인해 대시보드 사용자가 데이터를 해석할 때 혼란을 겪을 수 있습니다.
timer_save_prompt는 코드에서timer_modal로 사용되고 있습니다.share_prompt는 코드에서share_save로 정의되어 있습니다.
문서의 내용을 실제 구현과 일치시켜 주시기 바랍니다.
Suggested change
| - `timer_save_prompt`: 타이머에서 저장 유도로 로그인 | |
| - `share_prompt`: 공유 기능 사용 시 로그인 유도 | |
| - timer_modal: 타이머에서 저장 유도로 로그인 | |
| - share_save: 공유 기능 사용 시 로그인 유도 |
|
|
||
| /** 시간표 공유 이벤트 속성 */ | ||
| export interface TableSharedProperties { | ||
| table_id: number | string; |
There was a problem hiding this comment.
타입 정의가 구현과 일치하지 않습니다. table_id의 타입을 number | string으로 정의하셨지만, 실제 구현(src/util/analytics/types.ts)과 데이터 모델에서는 비회원 케이스를 위해 'guest'라는 특정 문자열만 사용하므로 number | 'guest'로 더 명확하게 정의하는 것이 좋습니다. 계약 파일의 타입을 더 구체적으로 수정하면 타입 안정성과 코드 명확성을 높일 수 있습니다.
이 변경은 TableSharedProperties, TimerStartedProperties, DebateCompletedProperties, DebateAbandonedProperties, TemplateUsedProperties 인터페이스에 모두 적용되어야 합니다.
Suggested change
| table_id: number | string; | |
| table_id: number | 'guest'; |
| | 환경 게이팅 | 조건부 import / No-op Provider | No-op Provider | 코드 경로 단일화, 테스트 용이 | NoopProvider로 dev 환경 테스트 | | ||
| | 이벤트 발화 위치 | 컴포넌트 직접 / 커스텀 훅 | 커스텀 훅 (useAnalytics 등) | 관심사 분리, 재사용성, 테스트 격리 | renderHook으로 훅 단독 테스트 | | ||
| | GA4 기존 코드 | 유지 / Adapter로 래핑 | Adapter로 래핑 | 기존 `setupGoogleAnalytics`와 `router.subscribe` 로직을 GA4Provider로 이관 | 일관된 테스트 방식 | | ||
| | 체류 시간 | 자체 구현 / SDK 기본 | SDK 기본 (Amplitude 세션 관리) | FR-002 요구사항 + Amplitude 자동 세션 트래킹 활용 | 별도 테스트 불필요 | |
There was a problem hiding this comment.
구현 계획 문서의 내용이 실제 구현과 다릅니다. 체류 시간 측정 방식에 대해 'SDK 기본'을 사용한다고 기재되어 있지만, data-model.md와 실제 구현 코드(usePageTracking.ts)에서는 화면별 체류 시간 측정을 위해 duration_ms를 직접 계산하는 방식을 사용하고 있습니다. 문서의 일관성을 위해 이 부분을 실제 구현 내용에 맞게 수정해 주세요.
Suggested change
| | 체류 시간 | 자체 구현 / SDK 기본 | SDK 기본 (Amplitude 세션 관리) | FR-002 요구사항 + Amplitude 자동 세션 트래킹 활용 | 별도 테스트 불필요 | | |
| | 체류 시간 | 자체 구현 / SDK 기본 | 자체 구현 | FR-002: 화면별 체류 시간 측정을 위해 page_leave 이벤트에 duration_ms를 직접 계산하여 전송 | usePageTracking 훅 테스트 필요 | |
| analyticsManager.setUserId(String(data.id)); | ||
| analyticsManager.setUserProperties({ | ||
| user_type: 'member', | ||
| language: document.documentElement.lang || 'ko', |
There was a problem hiding this comment.
하드코딩된 폴백(fallback) 언어 사용은 유지보수성을 저해할 수 있습니다. 'ko' 대신 src/util/languageRouting.ts에 정의된 DEFAULT_LANG 상수를 사용하면, 향후 기본 언어 변경 시 한 곳만 수정하면 되므로 코드의 일관성과 유지보수성이 향상됩니다. 이 패턴이 프로젝트의 다른 파일에서도 반복되고 있으니 함께 수정하는 것을 권장합니다.
Suggested change
| language: document.documentElement.lang || 'ko', | |
| language: document.documentElement.lang || DEFAULT_LANG, |
| analyticsManager.setUserId(String(memberId)); | ||
| analyticsManager.setUserProperties({ | ||
| user_type: 'member', | ||
| language: document.documentElement.lang || 'ko', |
There was a problem hiding this comment.
하드코딩된 폴백(fallback) 언어 사용은 유지보수성을 저해할 수 있습니다. 'ko' 대신 src/util/languageRouting.ts에 정의된 DEFAULT_LANG 상수를 사용하면, 향후 기본 언어 변경 시 한 곳만 수정하면 되므로 코드의 일관성과 유지보수성이 향상됩니다.
Suggested change
| language: document.documentElement.lang || 'ko', | |
| language: document.documentElement.lang || DEFAULT_LANG, |
| analyticsManager.setUserId(memberId); | ||
| analyticsManager.setUserProperties({ | ||
| user_type: 'member', | ||
| language: document.documentElement.lang || 'ko', |
There was a problem hiding this comment.
하드코딩된 폴백(fallback) 언어 사용은 유지보수성을 저해할 수 있습니다. 'ko' 대신 src/util/languageRouting.ts에 정의된 DEFAULT_LANG 상수를 사용하면, 향후 기본 언어 변경 시 한 곳만 수정하면 되므로 코드의 일관성과 유지보수성이 향상됩니다.
Suggested change
| language: document.documentElement.lang || 'ko', | |
| language: document.documentElement.lang || DEFAULT_LANG, |
| private getGlobalProperties(): GlobalEventProperties { | ||
| return { | ||
| user_type: isLoggedIn() ? 'member' : 'guest', | ||
| language: document.documentElement.lang || 'ko', |
There was a problem hiding this comment.
하드코딩된 폴백(fallback) 언어 사용은 유지보수성을 저해할 수 있습니다. 'ko' 대신 src/util/languageRouting.ts에 정의된 DEFAULT_LANG 상수를 사용하면, 향후 기본 언어 변경 시 한 곳만 수정하면 되므로 코드의 일관성과 유지보수성이 향상됩니다.
Suggested change
| language: document.documentElement.lang || 'ko', | |
| language: document.documentElement.lang || DEFAULT_LANG, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (7)
src/routes/ProtectedRoute.tsx (1)
22-27: 렌더 단계의setLoginTrigger호출은 effect로 분리하는 것을 권장합니다.현재는 렌더 중
sessionStorage를 변경하고 있어, 렌더 순수성 측면에서useEffect로 이동하는 편이 안전합니다.♻️ 제안 수정안
-import { Navigate, useLocation } from 'react-router-dom'; -import { PropsWithChildren } from 'react'; +import { Navigate, useLocation } from 'react-router-dom'; +import { PropsWithChildren, useEffect } from 'react'; @@ - if (!isAuthenticated) { - setLoginTrigger({ - trigger_page: location.pathname, - trigger_context: 'protected_route', - }); - return <Navigate to={homePath} state={{ from: location }} replace />; - } + useEffect(() => { + if (!isAuthenticated) { + setLoginTrigger({ + trigger_page: location.pathname, + trigger_context: 'protected_route', + }); + } + }, [isAuthenticated, location.pathname]); + + if (!isAuthenticated) { + return <Navigate to={homePath} state={{ from: location }} replace />; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/ProtectedRoute.tsx` around lines 22 - 27, The render currently calls setLoginTrigger inside the ProtectedRoute render when !isAuthenticated; move that side-effect into a useEffect to preserve render purity. Inside the ProtectedRoute component add a useEffect that watches [isAuthenticated, location.pathname] (or just [isAuthenticated, location.pathname]) and when !isAuthenticated calls setLoginTrigger({ trigger_page: location.pathname, trigger_context: 'protected_route' }); then remove the setLoginTrigger call from the JSX branch and still return <Navigate to={homePath} state={{ from: location }} replace /> when not authenticated. Ensure the effect guards against running when isAuthenticated is true so the trigger is only set during unauthenticated transitions.src/page/LandingPage/hooks/useLandingPageHandlers.ts (1)
40-47:trackEvent와setLoginTrigger호출 간 중복을 추출할 수 있습니다.두 핸들러에서 동일한 페이로드로
trackEvent와setLoginTrigger를 연속 호출하고 있습니다. 이 패턴을 헬퍼 함수로 추출하면 중복을 줄이고 일관성을 보장할 수 있습니다.♻️ 선택적 리팩터링 제안
+ const startLoginFlow = useCallback( + (context: 'landing_header' | 'landing_table_section') => { + const triggerData = { trigger_page: '/home', trigger_context: context }; + trackEvent('login_started', triggerData); + setLoginTrigger(triggerData); + oAuthLogin(); + }, + [trackEvent], + ); + const handleTableSectionLoginButtonClick = useCallback(() => { if (!isLoggedIn()) { - trackEvent('login_started', { - trigger_page: '/home', - trigger_context: 'landing_table_section', - }); - setLoginTrigger({ - trigger_page: '/home', - trigger_context: 'landing_table_section', - }); - oAuthLogin(); + startLoginFlow('landing_table_section'); } else { navigate(rootPath); } - }, [navigate, rootPath, trackEvent]); + }, [navigate, rootPath, startLoginFlow]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/LandingPage/hooks/useLandingPageHandlers.ts` around lines 40 - 47, The repeated pattern of calling trackEvent(...) immediately followed by setLoginTrigger(...) should be extracted into a small helper to remove duplication; create a helper function (e.g. handleLoginTrigger or logAndSetLoginTrigger) inside useLandingPageHandlers.ts that accepts the payload object and calls trackEvent(payload) then setLoginTrigger(payload), and replace the direct consecutive calls in all handlers with a single call to that helper (ensure you reference the same payload keys: trigger_page and trigger_context).src/util/analytics/providers/amplitudeProvider.test.ts (1)
52-55:setUserProperties테스트에서 전달된 속성을 검증하면 좋겠습니다.현재
amplitude.identify가 호출되었는지만 확인하고 있습니다. 전달된Identify객체에 예상 속성이 설정되었는지도 검증하면 테스트 신뢰도가 높아집니다.♻️ 선택적 개선 제안
test('setUserProperties 호출 시 amplitude.identify가 호출된다', () => { provider.setUserProperties({ user_type: 'member', language: 'ko' }); - expect(amplitude.identify).toHaveBeenCalled(); + expect(amplitude.identify).toHaveBeenCalledWith( + expect.objectContaining({ + _propertySet: expect.objectContaining({ + user_type: 'member', + language: 'ko', + }), + }), + ); });참고: Amplitude
Identify객체의 내부 구조에 따라 matcher 조정이 필요할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util/analytics/providers/amplitudeProvider.test.ts` around lines 52 - 55, Update the test for setUserProperties to not only assert amplitude.identify was called but also verify the Identify object passed contains the expected user properties; when calling provider.setUserProperties({ user_type: 'member', language: 'ko' }) assert that amplitude.identify was invoked with an Identify-like argument that includes those keys/values (e.g., check for an object containing the user properties or the Identify's set/operations that hold user_type and language) so the test validates the actual payload as well as the call.src/page/TimerPage/TimerPage.tsx (1)
79-84:template_label조합 로직 중복 검토 권장Line 82에서
template_label을${origin.organization_name} - ${origin.template_name}형식으로 직접 조합하고 있습니다.setTemplateOrigin호출 시 이미template_label이 저장되어 있다면,origin.template_label을 그대로 사용하는 것이 일관성 측면에서 더 안전합니다.♻️ 제안된 수정
if (origin) { trackEvent('template_used', { organization_name: origin.organization_name, template_name: origin.template_name, - template_label: `${origin.organization_name} - ${origin.template_name}`, + template_label: origin.template_label, table_id: isGuestFlow() ? 'guest' : tableId, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/TimerPage/TimerPage.tsx` around lines 79 - 84, When calling trackEvent('template_used') in TimerPage (the origin object and template_label assembly), avoid reconstructing template_label from origin.organization_name and origin.template_name; instead check if origin.template_label exists and use it, falling back to `${origin.organization_name} - ${origin.template_name}` only if absent. Update the trackEvent payload construction (where table_id and other fields are set) to reference origin.template_label when present to keep consistency with setTemplateOrigin and stored state.specs/feat/443-user-analytics/data-model.md (1)
97-115: 코드 블록에 언어 지정 추가 권장마크다운 린터(MD040)가 코드 블록에 언어가 지정되지 않았음을 경고합니다. 엔티티 관계도이므로
text또는plaintext로 지정하면 됩니다.📝 제안된 수정
-``` +```text User (Amplitude ID) ├── User Properties: { user_type, language }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/feat/443-user-analytics/data-model.md` around lines 97 - 115, The markdown code block that begins with "User (Amplitude ID)" is missing a language specifier which triggers linter MD040; update the opening triple-backtick for that block to include a language like text or plaintext (e.g., change ``` to ```text) and ensure the closing triple-backtick remains, so the block is treated as plain text by the linter and the warning is resolved.specs/feat/443-user-analytics/contracts/analytics-adapter.ts (1)
48-88: 계약 타입이 구현보다 넓어서 벌써 drift가 생겼습니다.여기서는
table_id를number | string으로 열어 두었는데, 실제 구현src/util/analytics/types.ts는number | 'guest'만 허용합니다. 이 상태면 계약/문서 기준으로는 임의 문자열도 유효해 보여서 후속 테스트나 구현이 잘못된 payload를 받아들이게 됩니다. 최소한 union을 구현과 동일하게 맞추고, 가능하면 이런 계약 정의는 한 곳에서만 유지하는 편이 안전합니다.정렬 예시
export interface TableSharedProperties { - table_id: number | string; + table_id: number | 'guest'; } export interface TimerStartedProperties { - table_id: number | string; + table_id: number | 'guest'; total_rounds: number; } export interface DebateCompletedProperties { - table_id: number | string; + table_id: number | 'guest'; total_rounds: number; } export interface DebateAbandonedProperties { - table_id: number | string; + table_id: number | 'guest'; current_round: number; total_rounds: number; abandon_type: 'navigation' | 'unload' | 'visibility'; } export interface TemplateUsedProperties extends TemplateSelectedProperties { - table_id: number | string; + table_id: number | 'guest'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/feat/443-user-analytics/contracts/analytics-adapter.ts` around lines 48 - 88, The interface contracts (TableSharedProperties, TimerStartedProperties, DebateCompletedProperties, DebateAbandonedProperties, TemplateUsedProperties) are too permissive for table_id (number | string) and drift from the implementation which only allows number | 'guest'; update each interface's table_id union to number | 'guest' to match src/util/analytics/types.ts and ensure type safety, and consider centralizing the table_id type into a single exported alias (e.g., TableId) used by these interfaces to prevent future drift.specs/feat/443-user-analytics/test-contracts/analytics.md (1)
125-139:loginTrigger는 이번 변경면에서 테스트로 고정해 두는 편이 안전합니다.이 유틸은 OAuth 리다이렉트 사이에서
login_completed의 출처 attribution을 보존하므로, 저장/복원/만료/force overwrite가 깨지면 전환 퍼널이 조용히 오염됩니다. 문서상 핵심 케이스가 전부 TODO라면 최소한 이 파일의 계약 범위만큼은 바로 테스트를 추가하는 게 좋겠습니다.원하시면 이 계약표 기준으로
src/util/analytics/loginTrigger.test.ts뼈대를 바로 정리해 드릴게요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/feat/443-user-analytics/test-contracts/analytics.md` around lines 125 - 139, The tests for the loginTrigger utility are missing; implement a new test suite file that covers the contract table cases by adding tests for setLoginTrigger, consumeLoginTrigger, clearLoginTrigger, hasLoginTrigger, force overwrite, non-overwrite behavior, sessionStorage persistence across simulated reload, and expiry after 5 minutes; use the function names setLoginTrigger, consumeLoginTrigger, clearLoginTrigger, and hasLoginTrigger to find the implementation, mock or manipulate sessionStorage to simulate persistence and time (advance timers or stub Date.now) to validate the 5-minute expiry, and ensure each test asserts that consumeLoginTrigger returns the expected object then deletes it (or returns null) per the contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/analytics-dashboard.md`:
- Around line 295-299: Update the docs' trigger_context examples to match the
code by replacing `timer_save_prompt` with `timer_modal` and `share_prompt` with
`share_save`, ensure existing `landing_header` remains, and add the missing
code-defined triggers `landing_table_section` and `protected_route` so the
example list matches the constants defined in
src/util/analytics/loginTrigger.ts.
In `@specs/feat/443-user-analytics/plan.md`:
- Around line 156-160: The fenced code blocks in
specs/feat/443-user-analytics/plan.md are missing language tags and trigger
markdownlint MD040; update each triple-backtick block (including the ones around
the snippets at the shown ranges and the additional ranges 164-169, 173-176,
182-185, 192-195, 214-217) to include a language tag such as text (e.g., replace
``` with ```text) so markdownlint passes; search for all occurrences of ``` in
plan.md and add the tag consistently.
In `@specs/feat/443-user-analytics/research.md`:
- Around line 8-19: The Amplitude plan limits are incorrect: replace every
instance that states "Starter 플랜: 50,000 MTU" (and related mentions in the same
sections at lines noted in the review) with the correct "Starter 플랜: 10K MTU",
update any consequential statements (e.g., the assessment that 50K MTU is
sufficient) to reflect the 10K limit, and correct any references to free vs.
Starter plan capabilities accordingly; also add a verification date and cite the
official Amplitude pricing page as the source for these changes so future
readers can verify the claim.
In `@src/hooks/usePageTracking.ts`:
- Around line 53-72: Multiple handlers (window 'pagehide' and 'beforeunload'
plus the SPA cleanup) can cause duplicate page_leave events; add a guard/ref
(e.g., hasTrackedPageLeaveRef or isPageLeaveTracked) checked and set inside
handlePageHide and in the SPA cleanup code path so trackEvent('page_leave', ...)
and analyticsManager.flush() only run once per page leave, and reset that flag
when a new page is entered (where entryTimeRef is updated or in the page_view
tracking logic) so future navigations can fire again; update references to
handlePageHide, the useEffect that registers pagehide/beforeunload, and the
existing cleanup routine to use this guard.
In `@src/page/DebateEndPage/DebateEndPage.tsx`:
- Around line 20-24: The hook useGetDebateTableData is being called with tableId
before URL validation, causing requests with NaN; fix by deriving a validated id
(e.g., parse id from useParams into parsedId and ensure
Number.isFinite(parsedId) or !Number.isNaN(parsedId)) and only invoke the query
(or pass enabled: Boolean(validatedId)) when the id is valid — update the call
to useGetDebateTableData to be conditional or use its enabled option so getTable
is never called with NaN.
In `@src/page/DebateVoteResultPage/DebateVoteResultPage.tsx`:
- Around line 65-70: The event is fired too early; move the
trackEvent('poll_result_viewed', { poll_id: pollId }) call out of the current
useEffect that only checks isPollIdValid and instead fire it only after the poll
data successfully loads (e.g., in the API success handler or a useEffect that
watches the poll data/loading state). Specifically, remove the trackEvent from
the useEffect that references isPollIdValid and pollId, and invoke trackEvent in
the code path that confirms successful data load (for example when pollData !==
null or isLoaded === true) so it only executes once per successful render; keep
references to trackEvent and pollId unchanged.
In `@src/page/LandingPage/hooks/useLandingPageHandlers.ts`:
- Line 41: The hook is building a language-aware homePath via
buildLangPath('/home', lang) (homePath) but several analytics payloads still
hardcode trigger_page: '/home'; update all occurrences of trigger_page in
useLandingPageHandlers (the four places currently using '/home') to use the
homePath variable instead so analytics record the language-prefixed path
consistently (e.g., replace trigger_page: '/home' with trigger_page: homePath).
In `@src/page/TableSharingPage/TableSharingPage.tsx`:
- Around line 71-87: The code currently saves template origin as soon as
isTemplateEntry is true, which can persist for invalid decodedData and
misattribute later template_used events; update the effect so that template
origin is only set when decodedData is successfully validated (i.e., after
decodedData exists/passes the same checks you use elsewhere) or, alternatively,
ensure you clear the stored origin when decodedData validation fails; target the
same logic around encodedData/isTemplateEntry and the setTemplateOrigin call in
TableSharingPage (and any sessionStorage usage) so the origin is only persisted
on a successful decode and removed on decode failure.
In `@src/page/TimerPage/hooks/useDebateTracking.ts`:
- Around line 82-86: The current handleVisibilityChange fires
sendAbandonEvent('visibility') immediately on document.visibilityState ===
'hidden', which falsely marks brief tab switches as abandon; modify
handleVisibilityChange to start a timeout (e.g., 10s) when visibility becomes
'hidden' and only call sendAbandonEvent('visibility') and set
isDebateActiveRef.current = false when that timeout elapses, and cancel/clear
that timeout if visibility returns to 'visible' before the delay so the debate
remains active; reference handleVisibilityChange, sendAbandonEvent, and
isDebateActiveRef to implement the delayed-abandon logic and ensure the timer is
cleared on unmount.
In `@src/util/analytics/providers/ga4Provider.ts`:
- Around line 73-75: In the reset() method of ga4Provider (the function named
reset), replace the incorrect call ReactGA.set({ userId: undefined }) with
ReactGA.set({ user_id: null }) so the GA4 user ID key and value are correctly
reset (use the snake_case key "user_id" and null as the value).
---
Nitpick comments:
In `@specs/feat/443-user-analytics/contracts/analytics-adapter.ts`:
- Around line 48-88: The interface contracts (TableSharedProperties,
TimerStartedProperties, DebateCompletedProperties, DebateAbandonedProperties,
TemplateUsedProperties) are too permissive for table_id (number | string) and
drift from the implementation which only allows number | 'guest'; update each
interface's table_id union to number | 'guest' to match
src/util/analytics/types.ts and ensure type safety, and consider centralizing
the table_id type into a single exported alias (e.g., TableId) used by these
interfaces to prevent future drift.
In `@specs/feat/443-user-analytics/data-model.md`:
- Around line 97-115: The markdown code block that begins with "User (Amplitude
ID)" is missing a language specifier which triggers linter MD040; update the
opening triple-backtick for that block to include a language like text or
plaintext (e.g., change ``` to ```text) and ensure the closing triple-backtick
remains, so the block is treated as plain text by the linter and the warning is
resolved.
In `@specs/feat/443-user-analytics/test-contracts/analytics.md`:
- Around line 125-139: The tests for the loginTrigger utility are missing;
implement a new test suite file that covers the contract table cases by adding
tests for setLoginTrigger, consumeLoginTrigger, clearLoginTrigger,
hasLoginTrigger, force overwrite, non-overwrite behavior, sessionStorage
persistence across simulated reload, and expiry after 5 minutes; use the
function names setLoginTrigger, consumeLoginTrigger, clearLoginTrigger, and
hasLoginTrigger to find the implementation, mock or manipulate sessionStorage to
simulate persistence and time (advance timers or stub Date.now) to validate the
5-minute expiry, and ensure each test asserts that consumeLoginTrigger returns
the expected object then deletes it (or returns null) per the contract.
In `@src/page/LandingPage/hooks/useLandingPageHandlers.ts`:
- Around line 40-47: The repeated pattern of calling trackEvent(...) immediately
followed by setLoginTrigger(...) should be extracted into a small helper to
remove duplication; create a helper function (e.g. handleLoginTrigger or
logAndSetLoginTrigger) inside useLandingPageHandlers.ts that accepts the payload
object and calls trackEvent(payload) then setLoginTrigger(payload), and replace
the direct consecutive calls in all handlers with a single call to that helper
(ensure you reference the same payload keys: trigger_page and trigger_context).
In `@src/page/TimerPage/TimerPage.tsx`:
- Around line 79-84: When calling trackEvent('template_used') in TimerPage (the
origin object and template_label assembly), avoid reconstructing template_label
from origin.organization_name and origin.template_name; instead check if
origin.template_label exists and use it, falling back to
`${origin.organization_name} - ${origin.template_name}` only if absent. Update
the trackEvent payload construction (where table_id and other fields are set) to
reference origin.template_label when present to keep consistency with
setTemplateOrigin and stored state.
In `@src/routes/ProtectedRoute.tsx`:
- Around line 22-27: The render currently calls setLoginTrigger inside the
ProtectedRoute render when !isAuthenticated; move that side-effect into a
useEffect to preserve render purity. Inside the ProtectedRoute component add a
useEffect that watches [isAuthenticated, location.pathname] (or just
[isAuthenticated, location.pathname]) and when !isAuthenticated calls
setLoginTrigger({ trigger_page: location.pathname, trigger_context:
'protected_route' }); then remove the setLoginTrigger call from the JSX branch
and still return <Navigate to={homePath} state={{ from: location }} replace />
when not authenticated. Ensure the effect guards against running when
isAuthenticated is true so the trigger is only set during unauthenticated
transitions.
In `@src/util/analytics/providers/amplitudeProvider.test.ts`:
- Around line 52-55: Update the test for setUserProperties to not only assert
amplitude.identify was called but also verify the Identify object passed
contains the expected user properties; when calling provider.setUserProperties({
user_type: 'member', language: 'ko' }) assert that amplitude.identify was
invoked with an Identify-like argument that includes those keys/values (e.g.,
check for an object containing the user properties or the Identify's
set/operations that hold user_type and language) so the test validates the
actual payload as well as the call.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ba803028-af24-48a2-b662-c3e756d20d73
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (48)
docs/analytics-dashboard.mdpackage.jsonspecs/feat/443-user-analytics/checklists/requirements.mdspecs/feat/443-user-analytics/contracts/analytics-adapter.tsspecs/feat/443-user-analytics/data-model.mdspecs/feat/443-user-analytics/plan.mdspecs/feat/443-user-analytics/research.mdspecs/feat/443-user-analytics/spec.mdspecs/feat/443-user-analytics/tasks.mdspecs/feat/443-user-analytics/test-contracts/analytics.mdsrc/apis/axiosInstance.tssrc/hooks/mutations/useLogout.tssrc/hooks/mutations/usePostUser.tssrc/hooks/useAnalytics.test.tssrc/hooks/useAnalytics.tssrc/hooks/usePageTracking.test.tsxsrc/hooks/usePageTracking.tssrc/main.tsxsrc/page/DebateEndPage/DebateEndPage.tsxsrc/page/DebateVoteResultPage/DebateVoteResultPage.tsxsrc/page/LandingPage/components/TemplateCard.tsxsrc/page/LandingPage/hooks/useLandingPageHandlers.tssrc/page/OAuthPage/OAuth.tsxsrc/page/TableOverviewPage/TableOverviewPage.tsxsrc/page/TableSharingPage/TableSharingPage.tsxsrc/page/TimerPage/TimerPage.tsxsrc/page/TimerPage/components/LoginAndStoreModal.tsxsrc/page/TimerPage/hooks/useDebateTracking.test.tssrc/page/TimerPage/hooks/useDebateTracking.tssrc/page/VoteParticipationPage/VoteParticipationPage.tsxsrc/routes/LanguageWrapper.tsxsrc/routes/ProtectedRoute.tsxsrc/routes/routes.tsxsrc/util/accessToken.tssrc/util/analytics/analyticsManager.test.tssrc/util/analytics/analyticsManager.tssrc/util/analytics/constants.tssrc/util/analytics/index.tssrc/util/analytics/loginTrigger.tssrc/util/analytics/providers/amplitudeProvider.test.tssrc/util/analytics/providers/amplitudeProvider.tssrc/util/analytics/providers/ga4Provider.test.tssrc/util/analytics/providers/ga4Provider.tssrc/util/analytics/providers/noopProvider.tssrc/util/analytics/templateOrigin.tssrc/util/analytics/types.tssrc/util/setupGoogleAnalytics.tsxtsconfig.app.json
💤 Files with no reviewable changes (2)
- src/util/setupGoogleAnalytics.tsx
- src/routes/routes.tsx
Comment on lines
+156
to
+160
| ``` | ||
| RED: analyticsManager.test.ts — 8개 테스트 (팬아웃, 에러 격리, 빈 provider) | ||
| GREEN: analyticsManager.ts — 최소 구현 | ||
| REFACTOR: 불필요한 중복 제거 | ||
| ``` |
There was a problem hiding this comment.
펜스 코드 블록에 언어 태그를 붙여 주세요.
여기 코드 블록들은 ```만 써서 markdownlint MD040이 발생합니다. text 정도만 붙여도 문서 lint가 정리됩니다.
Also applies to: 164-169, 173-176, 182-185, 192-195, 214-217
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 156-156: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@specs/feat/443-user-analytics/plan.md` around lines 156 - 160, The fenced
code blocks in specs/feat/443-user-analytics/plan.md are missing language tags
and trigger markdownlint MD040; update each triple-backtick block (including the
ones around the snippets at the shown ranges and the additional ranges 164-169,
173-176, 182-185, 192-195, 214-217) to include a language tag such as text
(e.g., replace ``` with ```text) so markdownlint passes; search for all
occurrences of ``` in plan.md and add the tag consistently.
Comment on lines
+8
to
+19
| - Starter 플랜: 50,000 MTU (Monthly Tracked Users), 무제한 이벤트, 기본 퍼널 분석 | ||
| - 토론 타이머 서비스의 예상 사용자 규모는 50K MTU 이하로 충분 | ||
| - Identity stitching (비회원→회원 연결): 무료 플랜에서 기본 지원 | ||
| - Amplitude는 device ID로 익명 사용자를 추적하고, `setUserId()` 호출 시 자동으로 이전 이벤트를 병합 | ||
| - 커스텀 이벤트, 사용자 속성, 세션 추적: 모두 무료 플랜에서 지원 | ||
| - 기본 대시보드, 차트, 코호트 분석: 무료 플랜에서 사용 가능 | ||
|
|
||
| **무료 플랜 제한 사항 (현재 요구사항에 영향 없음)**: | ||
| - 고급 행동 코호트 제한 (Plus 플랜부터) | ||
| - 데이터 보존 기간 제한 가능 | ||
| - 고급 세그멘테이션 일부 제한 | ||
| - MTU 기반 과금이므로 50K 초과 시 Plus 플랜 ($49/월) 필요 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Amplitude Starter plan MTU limit, included features, Browser SDK 2 recommendation/deprecation status, and Plus plan pricing as of April 2026 site:amplitude.com
💡 Result:
Amplitude Starter plan: 10K MTUs (up to 10M events). Included features: Out-of-the-box analytics and templates, Session Replay, Unlimited feature flags, Web Experimentation, AI Feedback, Unlimited sources and destinations, Access to community and academy. Browser SDK 2: Recommended and actively supported (GA phase, detailed docs, migration guides from legacy SDKs, built-in autocapture since v2.10+). No deprecation; legacy SDKs like Javascript SDK and Marketing Analytics SDK are deprecated/maintenance. Plus plan pricing: Starting at $49/mo (pay annually), scales with MTUs up to 300K at ~$0.049 per MTU (e.g., $582 annual for 1k MTUs). As of April 2026, no changes indicated in current official docs.
Citations:
- 1: https://amplitude.com/pricing
- 2: https://amplitude.com/docs/admin/billing-use/mtu-guide
- 3: https://amplitude.com/plus
- 4: https://amplitude.com/docs/sdks/sdk-maintenance-and-support
- 5: https://amplitude.com/docs/sdks/analytics/browser/browser-sdk-2
- 6: https://amplitude.com/docs/sdks/analytics/browser/migrate-from-javascript-sdk-to-browser-sdk-2-0
- 7: https://amplitude.com/docs/sdks/analytics/browser/marketing-analytics-sdk
Amplitude 요금제 정보가 정확하지 않습니다 - 즉시 수정이 필요합니다.
문서의 "Starter 플랜: 50,000 MTU"는 틀렸습니다. 현재 Amplitude Starter 플랜의 실제 한도는 10K MTU입니다. 이는 의사결정(예: "토론 타이머 서비스의 예상 사용자 규모는 50K MTU 이하로 충분")에 직접 영향을 미칩니다. Plus 플랜 가격($49/월)은 정확하지만, 무료 플랜 개념과 Starter 플랜의 실제 제한 사항을 다시 확인하고 수정하세요. 향후 업데이트 시에는 정보 검증 날짜와 공식 출처를 함께 기재하십시오.
8-19, 22-25, 31-39, 76-83, 104-107 라인도 마찬가지입니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@specs/feat/443-user-analytics/research.md` around lines 8 - 19, The Amplitude
plan limits are incorrect: replace every instance that states "Starter 플랜:
50,000 MTU" (and related mentions in the same sections at lines noted in the
review) with the correct "Starter 플랜: 10K MTU", update any consequential
statements (e.g., the assessment that 50K MTU is sufficient) to reflect the 10K
limit, and correct any references to free vs. Starter plan capabilities
accordingly; also add a verification date and cite the official Amplitude
pricing page as the source for these changes so future readers can verify the
claim.
userId: undefined는 GA4 spec과 맞지 않아 로그아웃 후에도 사용자 ID가 초기화되지 않는 버그가 있었음. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pagehide, beforeunload, SPA cleanup이 동시에 발화될 때 page_leave가 중복 기록되어 체류 시간 지표가 오염되는 문제 수정. hasTrackedLeaveRef로 페이지당 1회만 발송되도록 보장. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
탭 전환 즉시 abandon으로 기록하면 짧은 탭 전환도 이탈로 집계되고 이후 정상 완료 시 debate_completed가 누락됨. 10초 후에도 hidden 상태일 때만 abandon 이벤트를 발화하도록 수정. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
API 실패, 존재하지 않는 투표 등에서도 이벤트가 기록되어 지표가 오염되는 문제 수정. data 로드 성공 조건 추가 및 isError 선언을 useEffect 위로 이동하여 TDZ 크래시 방지. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
decodedData 검증 전에 origin을 저장하면 이후 정상 플로우에서 잘못된 template 귀속이 발생하는 문제 수정. decodedData를 useMemo로 안정화하고 effect를 분리해 share_link_entered가 모달 상태 변화 시 재발화되지 않도록 보장. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
/home 하드코딩으로 영어 사용자(/en/home)의 로그인 진입 경로가 분석 데이터에 잘못 기록되는 문제 수정. homePath 변수로 대체. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
기본 언어 변경 시 한 곳만 수정하면 되도록 유지보수성 개선. useAnalytics, usePostUser, main, analyticsManager 4개 파일 적용. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
id 검증 전에 useGetDebateTableData가 실행되어 NaN으로 API를 호출하는 문제 수정. isTableIdValid로 enabled 옵션 제어. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
timer_save_prompt → timer_modal, share_prompt → share_save로 정정. 코드에 정의된 landing_table_section, protected_route 값도 추가. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/page/DebateEndPage/DebateEndPage.tsx (1)
25-25:useGetDebateTableData반환값이 사용되지 않습니다.현재 훅 호출 결과를 사용하지 않고 있습니다. 데이터 프리페칭이 목적이라면 문제없지만, 로딩/에러 상태 처리가 필요하다면 반환값을 활용하세요. 의도적인 프리페칭이라면 주석으로 명시하는 것이 좋습니다.
♻️ 의도 명시를 위한 주석 추가
- useGetDebateTableData(tableId, isTableIdValid); + // 후속 페이지에서 사용할 데이터를 미리 캐싱한다. + useGetDebateTableData(tableId, isTableIdValid);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/DebateEndPage/DebateEndPage.tsx` at line 25, The call to useGetDebateTableData in DebateEndPage currently ignores its return value; either capture and use the hook's returned tuple/object (e.g., const { data, isLoading, error } = useGetDebateTableData(tableId, isTableIdValid)) and add proper loading/error handling in the component, or if the hook is intentionally invoked only for prefetching, add an inline comment above the call stating that intent (e.g., "prefetch only — return value intentionally unused") so future readers know this is deliberate; reference the useGetDebateTableData call in DebateEndPage when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/page/DebateEndPage/DebateEndPage.tsx`:
- Line 25: The call to useGetDebateTableData in DebateEndPage currently ignores
its return value; either capture and use the hook's returned tuple/object (e.g.,
const { data, isLoading, error } = useGetDebateTableData(tableId,
isTableIdValid)) and add proper loading/error handling in the component, or if
the hook is intentionally invoked only for prefetching, add an inline comment
above the call stating that intent (e.g., "prefetch only — return value
intentionally unused") so future readers know this is deliberate; reference the
useGetDebateTableData call in DebateEndPage when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5a759655-e0f2-404a-924c-b22467e8d6b7
📒 Files selected for processing (12)
docs/analytics-dashboard.mdsrc/hooks/mutations/usePostUser.tssrc/hooks/useAnalytics.tssrc/hooks/usePageTracking.tssrc/main.tsxsrc/page/DebateEndPage/DebateEndPage.tsxsrc/page/DebateVoteResultPage/DebateVoteResultPage.tsxsrc/page/LandingPage/hooks/useLandingPageHandlers.tssrc/page/TableSharingPage/TableSharingPage.tsxsrc/page/TimerPage/hooks/useDebateTracking.tssrc/util/analytics/analyticsManager.tssrc/util/analytics/providers/ga4Provider.ts
✅ Files skipped from review due to trivial changes (1)
- docs/analytics-dashboard.md
🚧 Files skipped from review as they are similar to previous changes (5)
- src/hooks/mutations/usePostUser.ts
- src/page/TableSharingPage/TableSharingPage.tsx
- src/hooks/usePageTracking.ts
- src/page/TimerPage/hooks/useDebateTracking.ts
- src/util/analytics/providers/ga4Provider.ts
jaeml06
changed the title
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
useon
requested changes
Apr 28, 2026
Contributor
useon
left a comment
useon
left a comment
There was a problem hiding this comment.
치코 ~!! 그동안 알고 싶었던 사용자 행동 분석에 대해 추가적으로 알 수 있게 됐네요. 감사합니다 !! 추가로 몇가지 코멘트 달았는데 확인부탁드려요 ~!
| }); | ||
| }); | ||
|
|
||
| test('resetUser를 호출하면 reset이 호출된다', () => { |
Contributor
There was a problem hiding this comment.
전체적으로 테스트가 구현에 대한 테스트여서 의도가 드러나는 항목이면 좋을 것 같아요!
Contributor
Author
There was a problem hiding this comment.
넵 테스트 의도가 드러나도록 문구를 수정했습니다.
| test('resetUser를 호출하면 reset이 호출된다', () => { | ||
| const { result } = renderHook(() => useAnalytics()); | ||
| result.current.resetUser(); | ||
| expect(analyticsManager.reset).toHaveBeenCalled(); |
Contributor
There was a problem hiding this comment.
테스트 중에서 실패하는 것들이 있어서 함께 확인해 주세용 ~!!
| * 파라미터는 받지 않으며, 반환값은 없다. | ||
| */ | ||
| reset(): void { | ||
| ReactGA.set({ user_id: null }); |
Contributor
There was a problem hiding this comment.
혹시 set은 userId이고 reset은 user_id라서 불일치해서 생기는 문제는 없나요?
Contributor
Author
There was a problem hiding this comment.
아 실수했네요. GA4의 표준이 user_id라고 하니 이것으로 통일하겠습니다.
| data: LoginTriggerData, | ||
| options?: { force?: boolean }, | ||
| ): void { | ||
| if (!options?.force && hasLoginTrigger()) { |
Contributor
There was a problem hiding this comment.
혹시 유효 시간이 지났으면 다시 덮어써야 되지 않나요??
Contributor
Author
There was a problem hiding this comment.
확인했습니다. 기존에 유효시간이 지났을 때, 다시 로그인을 시도하는 경우에, 만료가 되었기 때문에 덮어썻어야했습니다. 자세한 확인 감사해요. hasLoginTrigger에서 만료 시간을 검증해서 시간이 만료되면 덮어쓸 수 있도록 조건문을 수정했습니다.
setUserId는 { userId }, reset은 { user_id: null }로 키가 달라
reset이 setUserId가 설정한 식별자를 실제로 지우지 못하는 문제 수정.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
hasLoginTrigger가 단순히 storage 값 존재 여부만 검사해 만료된(5분 초과) trigger가 남아있으면 setLoginTrigger가 새 값을 저장하지 못하고, 결국 consumeLoginTrigger도 만료로 null을 반환해 trigger_context가 유실되는 문제가 있었음. timestamp를 파싱해 EXPIRY_MS 초과 시 false를 반환하도록 변경하여 만료된 trigger는 자연스럽게 새 값으로 덮어써지도록 함. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
identifyUser 테스트가 language 속성 누락으로 실패하던 문제를
expect.objectContaining으로 핵심 속성만 검증하도록 변경해 해결.
함께 테스트 이름과 assertion을 구현 호출 검증("trackEvent를
호출할 수 있다")에서 의도 표현("이벤트 정보를 분석 매니저로
그대로 전달한다")으로 재작성. template_label 필수 필드 누락도
보강.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
usePageTracking이 사용하는 useMatches는 data router context를 요구하는데 기존 테스트는 MemoryRouter를 사용해 모든 케이스가 'useMatches must be used within a data router' 에러로 실패. createMemoryRouter + RouterProvider로 wrapper를 교체해 실제 런타임과 동일한 라우터 환경에서 검증하도록 변경. 테스트 이름도 의도 중심으로 정리. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/hooks/usePageTracking.test.tsx (1)
83-85: 실패 원인 가독성을 위해 mock 호출 존재를 먼저 단정해 주세요.Line [83]에서
mock.calls[0]를 바로 접근하면 호출이 없을 때 타입 에러로 깨져, 실제 실패 원인(이벤트 미발행)이 흐려집니다.제안 패치
- const call = vi.mocked(analyticsManager.trackEvent).mock.calls[0]; - const properties = call[1] as { duration_ms: number }; + expect(analyticsManager.trackEvent).toHaveBeenCalledTimes(1); + const [, properties] = vi.mocked(analyticsManager.trackEvent).mock + .calls[0] as [string, { duration_ms: number }]; expect(properties.duration_ms).toBeGreaterThanOrEqual(100);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/usePageTracking.test.tsx` around lines 83 - 85, The test accesses vi.mocked(analyticsManager.trackEvent).mock.calls[0] directly which throws when the mock was never called; first assert the mock was invoked (e.g. expect(analyticsManager.trackEvent).toHaveBeenCalled() or check mock.calls.length) before reading calls[0], then proceed to extract properties and assert duration_ms; update the test in usePageTracking.test.tsx around the analyticsManager.trackEvent usage to add that precondition check so failures show "event not emitted" instead of a type error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useAnalytics.test.ts`:
- Line 16: 테스트 설명이 영어로 작성되어 규칙을 위반하고 있습니다; describe('useAnalytics', ...)의 설명
문자열을 한국어로 바꿔주세요(예: "useAnalytics 훅 테스트" 또는 "useAnalytics 동작 테스트") so that the
describe block uses Korean; 또한 같은 파일 내 다른 describe/it 문자열도 규칙에 맞게 한국어로 통일했는지
확인하세요.
In `@src/util/analytics/providers/ga4Provider.ts`:
- Around line 33-38: trackPageView currently only sends page_path and
page_title, dropping shared context fields from
EnrichedEventProperties<PageViewProperties>; update the trackPageView
implementation to include the full enriched context when calling ReactGA.send by
merging the received properties (including user_type, language,
previous_page_path, referrer and any other common/enrichment fields) into the
payload so ReactGA receives both page fields and the common context; locate the
trackPageView method in ga4Provider.ts and ensure it forwards the entire
properties object (or explicitly maps those context fields) alongside page and
title in the ReactGA.send call.
---
Nitpick comments:
In `@src/hooks/usePageTracking.test.tsx`:
- Around line 83-85: The test accesses
vi.mocked(analyticsManager.trackEvent).mock.calls[0] directly which throws when
the mock was never called; first assert the mock was invoked (e.g.
expect(analyticsManager.trackEvent).toHaveBeenCalled() or check
mock.calls.length) before reading calls[0], then proceed to extract properties
and assert duration_ms; update the test in usePageTracking.test.tsx around the
analyticsManager.trackEvent usage to add that precondition check so failures
show "event not emitted" instead of a type error.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: be419ab9-3091-4d15-bf95-e261347be2bd
📒 Files selected for processing (4)
src/hooks/useAnalytics.test.tssrc/hooks/usePageTracking.test.tsxsrc/util/analytics/loginTrigger.tssrc/util/analytics/providers/ga4Provider.ts
✅ Files skipped from review due to trivial changes (1)
- src/util/analytics/loginTrigger.ts
| }, | ||
| })); | ||
|
|
||
| describe('useAnalytics', () => { |
There was a problem hiding this comment.
describe 설명도 한국어로 통일해 주세요.
Line 16의 describe('useAnalytics', ...)는 영어 설명이라 테스트 설명 한국어 작성 규칙과 어긋납니다.
수정 제안
-describe('useAnalytics', () => {
+describe('useAnalytics 훅', () => {As per coding guidelines, "Test descriptions must be written in Korean".
📝 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.
Suggested change
| describe('useAnalytics', () => { | |
| describe('useAnalytics 훅', () => { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useAnalytics.test.ts` at line 16, 테스트 설명이 영어로 작성되어 규칙을 위반하고 있습니다;
describe('useAnalytics', ...)의 설명 문자열을 한국어로 바꿔주세요(예: "useAnalytics 훅 테스트" 또는
"useAnalytics 동작 테스트") so that the describe block uses Korean; 또한 같은 파일 내 다른
describe/it 문자열도 규칙에 맞게 한국어로 통일했는지 확인하세요.
Comment on lines
+33
to
+38
| trackPageView(properties: EnrichedEventProperties<PageViewProperties>): void { | ||
| ReactGA.send({ | ||
| hitType: 'pageview', | ||
| page: properties.page_path, | ||
| title: properties.page_title, | ||
| }); |
There was a problem hiding this comment.
trackPageView에서 공통/유입 컨텍스트 속성이 유실됩니다.
EnrichedEventProperties<PageViewProperties>를 받지만 실제 전송에는 page_path/page_title만 사용하고 있습니다. 이로 인해 user_type, language, previous_page_path, referrer가 페이지뷰 분석에서 빠져 회원/게스트 분해 및 유입 경로 분석 정확도가 떨어질 수 있습니다. trackPageView에서도 전달받은 속성을 보존해 전송하도록 맞춰주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/util/analytics/providers/ga4Provider.ts` around lines 33 - 38,
trackPageView currently only sends page_path and page_title, dropping shared
context fields from EnrichedEventProperties<PageViewProperties>; update the
trackPageView implementation to include the full enriched context when calling
ReactGA.send by merging the received properties (including user_type, language,
previous_page_path, referrer and any other common/enrichment fields) into the
payload so ReactGA receives both page fields and the common context; locate the
trackPageView method in ga4Provider.ts and ensure it forwards the entire
properties object (or explicitly maps those context fields) alongside page and
title in the ReactGA.send call.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚩 연관 이슈
closed #443
📝 작업 내용
개요
회원/비회원 플로우별 유저 유입 및 전환 흐름을 분석하기 위한 이벤트 트래킹 시스템을 구축했습니다.
기존의 GA4 단독 방식에서 벗어나, Amplitude + GA4 듀얼 프로바이더 구조로 전환했습니다.
전체 구조 한눈에 보기
이 구조 덕분에 어떤 분석 도구를 추가하거나 교체해도 각 페이지 코드는 전혀 바꿀 필요가 없습니다.
핵심 구현:
src/util/analytics/analytics의 핵심 로직이 모두 여기에 있습니다.
analyticsManager.ts— 중앙 허브모든 이벤트에는 공통 속성이 자동으로 붙습니다:
user_type:member(로그인) /guest(비로그인)language: 현재 서비스 언어 (ko,en등)page_path: 이벤트 발생 시점의 URL 경로providers/— 실제 전송 담당amplitudeProvider.tsga4Provider.tsgtag)로 이벤트 전송noopProvider.tsloginTrigger.ts— 로그인 유발 맥락 추적로그인 버튼은 여러 곳에 있습니다 (헤더, 타이머 모달, 공유 저장 등).
사용자가 어느 맥락에서 로그인을 시작했는지 기억해두었다가, OAuth 완료 후
login_completed이벤트에 함께 전송합니다.templateOrigin.ts— 템플릿 경로 추적템플릿 카드를 클릭하면 → 미리보기 → 커스터마이즈 → 타이머 시작까지 여러 페이지를 거칩니다.
어떤 템플릿에서 시작했는지 기억해두어야
template_used이벤트를 정확히 남길 수 있습니다.수집하는 이벤트 목록
page_viewpage_leavelogin_startedlogin_completedtable_sharedshare_link_enteredtimer_starteddebate_completeddebate_abandonedtemplate_selectedtemplate_usedpoll_createdpoll_votedpoll_result_viewedfeedback_timer_started훅 구조
usePageTracking— 자동 페이지 추적LanguageWrapper에 마운트되어 모든 페이지에서 자동으로 작동합니다.별도 설정 없이 SPA 라우팅 변경을 감지해
page_view/page_leave를 전송합니다.useDebateTracking— 토론 타이머 전용토론 진행 상태를 추적합니다. 토론이 완주되면
debate_completed, 중간에 이탈하면debate_abandoned를 전송합니다.abandon_type으로 이탈 방식도 구분합니다:navigation(다른 페이지 이동) /unload(탭 닫기) /visibility(백그라운드 전환)useAnalytics— 편의 훅analyticsManager를 직접 import하지 않아도 컴포넌트에서track을 쉽게 사용할 수 있는 wrapper 훅입니다.인증 흐름 연동 (
src/main.tsx,usePostUser,useLogout,axiosInstance)localStorage의memberId+accessToken확인 후 identity 복원setUserId(memberId),user_type: member설정analyticsManager.reset()으로 사용자 정보 초기화axiosInstance인터셉터에서 자동으로reset()호출대시보드 해석 가이드
Amplitude 대시보드를 처음 보는 팀원을 위한 해석 가이드를
docs/analytics-dashboard.md에 정리했습니다.어떤 차트가 어떤 질문에 답하는지, 수치를 어떻게 읽어야 하는지 설명되어 있습니다.
🗣️ 리뷰 요구사항 (선택)
Summary by CodeRabbit
릴리스 노트
New Features
Documentation
Tests
Chores