Skip to content

refactor(common): 코드 리뷰 반영 및 테스트 페이지 정리#35

Merged
Lilium0422 merged 1 commit into
developfrom
#30-feat-error-dashboard
Jun 4, 2026
Merged

refactor(common): 코드 리뷰 반영 및 테스트 페이지 정리#35
Lilium0422 merged 1 commit into
developfrom
#30-feat-error-dashboard

Conversation

@Lilium0422
Copy link
Copy Markdown
Contributor

@Lilium0422 Lilium0422 commented Jun 4, 2026

🛠️ 설명 (Description)

이전 PR 코드 리뷰에서 반영하지 못했던 유틸 분리 및 테스트 페이지 정리를 진행했습니다.

📝 변경 사항 요약 (Summary)

  • errorStatsUtils.ts: buildDailySlots, formatIssueSummary 유틸 함수 분리
  • stats/route.ts: 24슬롯 생성 로직을 buildDailySlots 유틸로 리팩토링
  • ErrorDetailView.tsx: 시간/횟수 포맷팅을 formatIssueSummary 유틸로 리팩토링
  • sentry-test/page.tsx: 에러 전송 테스트 버튼만 남기고 API 조회 UI 제거
  • views/sentry/page.tsx: 불필요 파일 삭제

💁 변경 사항 이유 (Why)

  • 팀원 코드 리뷰 반영: 시간/횟수 계산 로직을 유틸로 분리하여 재사용성 확보
  • 에러 대시보드(/dashboard/errors) 완성으로 sentry-test 페이지의 API 조회 기능 불필요
  • 테스트 버튼은 개발 중 에러 수집 확인용으로 유지

✅ 테스트 계획 (Test Plan)

  • /dashboard/errors 에러 목록 정상 렌더링 확인
  • /dashboard/errors/{id} 상세 페이지에서 formatIssueSummary 정상 표시 확인
  • /sentry-test 에러 전송 버튼 정상 동작 확인
  • pnpm run type-check 통과 확인

🔗 관련 이슈 (Related Issues)

☑️ 체크리스트 (Checklist)

  • 코드가 프로젝트 코딩 컨벤션을 따릅니다.
  • 변경 사항에 대한 문서화가 완료되었습니다.
  • 필요한 경우, 다른 팀원에게 리뷰를 요청했습니다.

Summary by CodeRabbit

릴리스 노트

  • Refactor

    • Sentry 에러 통계 처리 로직을 최적화하여 일일 데이터 슬롯 생성 성능 개선
    • 에러 요약 정보 포맷팅 로직을 통합하여 코드 유지보수성 향상
    • 에러 상세 페이지의 이슈 정보 표시 로직 단순화
  • New Features

    • Sentry 에러/메시지 전송 테스트를 위한 전용 테스트 페이지 추가

- errorStatsUtils: buildDailySlots, formatIssueSummary 유틸 분리
- stats/route: buildDailySlots 유틸 사용으로 리팩토링
- ErrorDetailView: formatIssueSummary 유틸 사용으로 리팩토링
- sentry-test/page: 에러 전송 버튼만 남기고 api 조회 ui 제거
- views/sentry/page.tsx: 불필요 파일 삭제

Resolves: #30
See also: None
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 4, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

이 PR은 Sentry 통계 처리를 재사용 가능한 유틸로 추출하고 통합한 리팩토링입니다. errorStatsUtilsbuildDailySlots (24시간 슬롯 자동 생성) 및 formatIssueSummary (이슈 정보 포맷팅) 함수를 추가한 후, 이를 API 라우트와 ErrorDetailView에서 활용하도록 변경했습니다. 동시에 app/sentry-test/page.tsx를 클라이언트 컴포넌트로 전환하여 Sentry 기능 테스트용 UI를 제공하고, src/views/sentry/page.tsx를 제거했습니다.

코드 리뷰 코칭

🎯 강점

  1. 로직의 적절한 추상화 — 24시간 슬롯 생성과 이슈 요약 포맷팅이 errorStatsUtils로 분리되어, 다른 곳에서도 재사용할 수 있는 구조가 완성되었습니다. 👍

  2. 단방향 의존성 — 유틸 → API/뷰 순의 명확한 의존성이 유지되어 순환 참조 위험이 없습니다.

🤔 리뷰 포인트

1. buildDailySlots의 시간대 기준

// 현재 로컬 시간의 0시~23시 슬롯을 고정으로 생성
const today = dayjs().startOf('day');

질문: 이 함수가 서버의 로컬 타임존을 기준으로 슬롯을 생성하는데, API 호출자의 타임존과 불일치할 가능성이 있습니다. 예를 들어, UTC 서버에서 한국 시간대 사용자가 통계를 조회하면 시간 차이가 발생합니다.

  • 개선 제안:
    • API 레이어에서 클라이언트의 타임존을 쿼리 파라미터로 받거나,
    • dayjs.tz()를 사용해 명시적으로 타임존을 지정하는 방식을 검토하세요.
    • 참고: Dayjs Timezone Plugin

2. formatIssueSummary의 정적 템플릿

`최초: ${firstSeen} | 마지막: ${lastSeen} | 발생: ${count} | 사용자: ${userCount}`

질문: 요약 문구가 한국어로 고정되어 있어 다국어 지원이 어렵습니다.

  • 개선 제안:
    • i18n 라이브러리(예: next-intl)를 활용해 템플릿을 국제화하거나,
    • 컴포넌트 레벨에서 렌더링하도록 리팩토링하세요.

3. 동적 import의 필요성 재검토

app/api/sentry/issues/[id]/stats/route.ts에서 buildDailySlots를 동적 import합니다:

const { buildDailySlots } = await import('`@/entities/error/model/errorStatsUtils`');

질문: 이 API 엔드포인트가 자주 호출되는 경우, 매 요청마다 모듈을 동적으로 로드하는 오버헤드가 있을까요?

  • 개선 제안: 정적 import로 변경하는 것을 검토해보세요. 동적 import는 주로 조건부 로딩이나 코드 분할 목적일 때 유용합니다.

4. 테스트 페이지에서 사용자 정보 처리

const handleSetUserAndCaptureError = () => {
  setSentryUser({ id: 'test-user', email: 'test@example.com' });
  captureError(new Error('Test error with user context'));
};

질문: 테스트 후 사용자 정보가 초기화되지 않으면, 이후 요청들이 계속 테스트 사용자로 기록될 우려가 있습니다.

  • 개선 제안: "Clear User" 버튼을 추가하거나, 페이지 언마운트 시 setSentryUser(null)을 호출하세요.

✅ 검증 체크리스트

  • buildDailySlots 함수가 실제 edge case(월 경계, 타임존 차이, 빈 통계)에서 정상 동작하는지 단위 테스트 확인
  • ErrorDetailView 통합 후 UI에서 포맷된 텍스트가 레이아웃을 초과하지 않는지 확인
  • 제거된 src/views/sentry/page.tsx의 기능이 다른 곳에서 완전히 대체되었는지 확인

🎯 Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

분석 근거:

  • 논리 밀도: 유틸 함수 2개의 새로운 로직 + API/뷰 통합점 (중간 수준)
  • 파일 다양성: 5개 파일에 걸친 변경 (중간 분산)
  • 변경 패턴: 유틸 추출 → 소비 통합의 일관된 패턴 (예측 가능)
  • 타임존/포맷팅 검토 필요: 서버/클라이언트 타임존 불일치, i18n 미지원 등 비즈니스 로직 세부사항 확인 필요
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR 제목은 코드 리뷰 반영 및 테스트 페이지 정리라는 실제 변경 내용을 요약하고 있으나, 구체적 변경의 핵심(유틸 분리, 슬롯 생성 로직 리팩토링, 테스트 페이지 통합)이 명확하지 않습니다.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch #30-feat-error-dashboard

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.

@Lilium0422 Lilium0422 self-assigned this Jun 4, 2026
@Lilium0422 Lilium0422 added the refactor 리팩토링 label Jun 4, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/sentry-test/page.tsx`:
- Around line 20-25: The onClick handler currently calls captureError(...) then
immediately shows alert, which can mislead because captureError is asynchronous;
change the onClick function to async, call captureError(new Error(...), { page:
"sentry-test" }), then await Sentry.flush(2000) (or an appropriate timeout)
before showing the success alert, or alternatively change the alert text to "전송
요청 완료" if you prefer not to await; update the onClick handler in
app/sentry-test/page.tsx accordingly (references: onClick handler, captureError,
Sentry.flush).
- Around line 44-47: The Sentry test button leaks a persistent user because you
call setSentryUser(...) globally and then immediately show a success alert;
instead isolate the test user to a single capture by using
Sentry.withScope(scope => { scope.setUser(...); captureError(...) }) or clear it
after capture with Sentry.setUser(null), and update the alert text to reflect
that the error was only "sent/requested" (e.g., "전송 요청 완료") or, if you need a
guaranteed delivery, await Sentry.flush(timeout) before notifying; modify the
handler around setSentryUser, captureError/captureMessage, and alert accordingly
(or add a one-shot helper in shared/lib/sentry.ts).

In `@src/entities/error/model/errorStatsUtils.ts`:
- Around line 16-39: The buildDailySlots function is matching timestamps using
server-local Date methods, causing bucket misalignment across time zones/DST;
update buildDailySlots to use a single canonical timezone (UTC) via dayjs with
the UTC plugin (and move logic into the shared date util), compute slot
boundaries and match allStats timestamps consistently in UTC, and also adjust
buildEmptyBreakdown to use the same shared date util so both generation and
matching use the same UTC bucket boundaries.
🪄 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 Plus

Run ID: 2dcc8b4e-e564-4558-8004-0130b3c8bb46

📥 Commits

Reviewing files that changed from the base of the PR and between f0dc9fb and 2c71714.

📒 Files selected for processing (5)
  • app/api/sentry/issues/[id]/stats/route.ts
  • app/sentry-test/page.tsx
  • src/entities/error/model/errorStatsUtils.ts
  • src/views/sentry/ErrorDetailView.tsx
  • src/views/sentry/page.tsx
💤 Files with no reviewable changes (1)
  • src/views/sentry/page.tsx

Comment thread app/sentry-test/page.tsx
Comment thread app/sentry-test/page.tsx
Comment thread src/entities/error/model/errorStatsUtils.ts
@Lilium0422 Lilium0422 merged commit 2fb0d44 into develop Jun 4, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor 리팩토링

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant