Skip to content

Comments

[fix] 페이지 체류 시간 중복 집계 버그 수정#804

Merged
seongwon030 merged 2 commits intodevelop-fefrom
fix/#803-duplicate-duration-tracking-MOA-300
Nov 8, 2025
Merged

[fix] 페이지 체류 시간 중복 집계 버그 수정#804
seongwon030 merged 2 commits intodevelop-fefrom
fix/#803-duplicate-duration-tracking-MOA-300

Conversation

@seongwon030
Copy link
Member

@seongwon030 seongwon030 commented Nov 5, 2025

#️⃣연관된 이슈

ex) #803

📝작업 내용

이전의 trackPageDuration함수

trackPageDuration은 사용자가 특정 페이지에 머무는 시간을 기록하는 함수입니다.

    const trackPageDuration = () => {
      if (isTracked.current) return;

      const duration = Date.now() - startTime.current;
      mixpanel.track(`${pageName} Duration`, {
        url: window.location.href,
        duration: duration,
        duration_seconds: Math.round(duration / 1000),
        clubName,
      });
     isTracked.current = true;
    };

mixpanel.track() 함수는 비동기 네트워크 요청이기에 trackPageDuration함수는 isTracked.current = true;가 수행되기 전까지 기다리지 않습니다.

만약 visibilitychange이벤트와 useEffect의 클린업 함수가 거의 동시에 trackPageDuration함수를 호출하면, 현재 코드는 아래와 같이 동작합니다.

1. [호출 1] (visibilitychange): if (isTracked.current)가 false인지 확인 -> 통과
2. [호출 2] (useEffect Cleanup): if (isTracked.current)가 false인지 확인 -> 또 통과 (아직 [호출 1]이 true로 바꾸기 전)
3. [호출 1]: mixpanel.track() 실행 (Duration 1번째 전송)
4. [호출 2]: mixpanel.track() 실행 (Duration 2번째 중복 전송)
5. [호출 1]: isTracked.current = true;로 설정
6. [호출 2]: isTracked.current = true;로 설정 (이미 늦음)

이러면 Visited는 1번인데 Duration은 2번 전송됩니다.

개선하기

비동기 트래킹 로직 전에 isTracked 플래그를 사용한 중복 트래킹 방지 로직을 배치함으로써 레이스 컨디션을 해결할 수 있습니다.

const trackPageDuration = () => {
  if (isTracked.current) return;

  // 통과했으면 즉시 '잠금'을 설정
  isTracked.current = true; 

  // 그 다음에 안전하게 비동기 작업을 수행
  const duration = Date.now() - startTime.current;
  mixpanel.track(`${pageName} Duration`, {
    url: window.location.href,
    duration: duration,
    duration_seconds: Math.round(duration / 1000),
    clubName,
  });
};

중점적으로 리뷰받고 싶은 부분(선택)

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

ex) 메서드 XXX의 이름을 더 잘 짓고 싶은데 혹시 좋은 명칭이 있을까요?

논의하고 싶은 부분(선택)

논의하고 싶은 부분이 있다면 작성해주세요.

🫡 참고사항

Summary by CodeRabbit

  • 버그 수정
    • 페이지 조회 추적의 안정성을 개선했습니다. 중복 실행을 방지해 한 번만 페이지 체류 시간을 기록하도록 했고, 기록 시점 조정으로 경합 상황에서의 재진입 문제를 막아 조회 정보의 정확도가 향상됩니다.

- trackPageDuration 함수 내에서 mixpanel.track 비동기 호출과
isTracked.current 플래그 설정 사이에 발생할 수 있는 레이스 컨디션을 해결
@seongwon030 seongwon030 self-assigned this Nov 5, 2025
@vercel
Copy link

vercel bot commented Nov 5, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
moadong Ready Ready Preview Comment Nov 7, 2025 5:19pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 5, 2025

Warning

.coderabbit.yaml has a parsing error

The CodeRabbit configuration file in this repository has a parsing error and default settings were used instead. Please fix the error(s) in the configuration file. You can initialize chat with CodeRabbit to get help with the configuration file.

💥 Parsing errors (1)
Validation error: Invalid regex pattern for base branch. Received: "**" at "reviews.auto_review.base_branches[0]"
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Walkthrough

trackPageDuration에서 재진입 방지를 위해 isTracked.current 플래그를 함수 시작 직후에 설정하도록 이동하여 동일 페이지 체류 시간 중복 집계 가능성을 제거했습니다.

Changes

코호트 / 파일(s) 변경 요약
페이지 트래킹 플래그 조정
frontend/src/hooks/useTrackPageView.ts
trackPageDuration 내에서 if (isTracked.current) return; 검사 추가 및 isTracked.current = true를 기간 계산 전에 설정하도록 위치 이동; 중복 할당 제거 및 레이스 컨디션 방지 주석 추가

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Page
    participant useTrackPageView as Hook
    participant Analytics

    rect rgb(230,245,255)
    Note over Page,Hook: 언로드/visibility 변경 발생
    Page->>Hook: onbeforeunload / visibilitychange
    Hook->>Hook: if (isTracked.current) return
    Hook->>Hook: isTracked.current = true
    Hook->>Analytics: compute & send duration
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • 단일 파일, 위치 변경 및 간단한 논리 추가이므로 검토 난이도 낮음
  • 주의할 부분:
    • isTracked 플래그가 다른 시점(예: 페이지 진입 시 리셋)과 일관되게 관리되는지 확인
    • 이벤트 리스너(visibilitychange, beforeunload) 다중 등록/제거 관련 부작용 여부

Possibly related issues

  • MOA-300: 페이지 체류 시간(Duration) 중복 집계 버그 수정 — 본 변경은 중복 집계 문제를 직접적으로 목표로 함.
  • Moadong/moadong#802: 동일 훅의 isTracked 초기화 위치 변경 관련 이슈 — 같은 로직 수정과 연관됨.

Possibly related PRs

Suggested labels

💻 FE

Suggested reviewers

  • oesnuj
  • PororoAndFriends
  • suhyun113

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR 제목이 변경 사항의 주요 목적을 명확하게 설명하고 있습니다. '페이지 체류 시간 중복 집계 버그 수정'은 코드 변경의 핵심 내용을 정확하게 반영합니다.
Linked Issues check ✅ Passed 코드 변경이 연결된 이슈 MOA-300의 목표와 일치합니다. trackPageDuration 함수에서 isTracked.current 플래그를 함수 시작 시점에 설정하여 레이스 컨디션을 제거하는 구현이 이슈의 요구사항을 충족합니다.
Out of Scope Changes check ✅ Passed 모든 변경 사항이 useTrackPageView.ts 파일의 trackPageDuration 함수 내에서만 발생하며, 연결된 이슈 MOA-300의 범위 내에 있습니다. 범위를 벗어나는 변경 사항이 없습니다.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/#803-duplicate-duration-tracking-MOA-300

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 373c8af and ed888ea.

📒 Files selected for processing (1)
  • frontend/src/hooks/useTrackPageView.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
frontend/**/*.{ts,tsx}

📄 CodeRabbit inference engine (frontend/.cursorrules)

frontend/**/*.{ts,tsx}: Replace magic numbers with named constants for clarity.
Replace complex or nested ternary operators with if/else statements or IIFEs for readability.
Assign complex boolean conditions to named variables.
Use consistent return types for similar functions and hooks.
Avoid hidden side effects; functions should only perform actions implied by their signature (Single Responsibility Principle).
Use unique, descriptive names for custom wrappers and functions to avoid ambiguity.
Define constants near related logic or ensure names link them clearly.

Files:

  • frontend/src/hooks/useTrackPageView.ts
🔇 Additional comments (1)
frontend/src/hooks/useTrackPageView.ts (1)

19-22: 잠금 순서 조정으로 중복 트래킹 방지 확인 완료
체크 직후 바로 isTracked.current를 true로 고정해 레이스 컨디션을 차단한 흐름이 의도대로 잘 동작하겠습니다.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

trackPageDuration 함수에서 isTracked 플래그를 사용한
중복 트래킹 방지 로직에 대한 설명 주석 추가
Copy link
Contributor

@lepitaaar lepitaaar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다! 바뀐건 위치지만 위치하나로 중복전송을 막을 수 있었네요~

@seongwon030 seongwon030 merged commit ece0370 into develop-fe Nov 8, 2025
5 checks passed
@seongwon030 seongwon030 deleted the fix/#803-duplicate-duration-tracking-MOA-300 branch November 11, 2025 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants