[Refactor/#239] 지표 메타데이터 Registry 중앙화 및 지표 포맷 통합 - #258
Merged
Conversation
정수형 카운트 → toLocaleString, 비율/퍼센트 → toFixed(2)%, 금액 → Math.round + ₩, 증감률 → toFixed(2)% 절댓값으로 기준 통일. formatCompact, formatTableRow, formatTableTotal, formatPercentDeltaCompact 제거 후 format으로 대체.
5 tasks
📝 WalkthroughWalkthrough대시보드 지표 포맷/메타 레지스트리( ChangesDashboard METRIC_REGISTRY 중앙화 및 isLoading prop 제거
Auth 유틸 경로 재구성 및 기타 정리
추정 코드 리뷰 노력🎯 4 (Complex) | ⏱️ ~60 minutes 관련 가능성이 있는 이슈
관련 가능성이 있는 PR
추천 리뷰어
🚥 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)
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 |
📚 Storybook 배포 완료
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/dashboard/platform/SinglePlatformView.tsx (1)
151-154: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift단일 플랫폼 트래픽 차트의 데이터 소스를 mock에서 실데이터로 전환해주세요.
PlatformTrafficChart에platformTrafficMock를 주입하면, 부모의 로딩 드릴링 제거 이후에도 트래픽 섹션만 실서버 상태와 분리된 채 동작합니다. 서버 상태 훅 기반 데이터로 교체해 섹션 간 일관된 로딩/오류/데이터 표시를 맞춰주세요.
As per coding guidelines, "**/*.mock.ts: Mock data: separate into*.mock.tsfiles. Forbidden: mixing mock data into production APIs and hooks."
As per path instructions, "src/**: 상태 관리: 서버 상태(React Query)와 전역 상태(Zustand)의 분리 여부 확인."🤖 Prompt for 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. In `@src/components/dashboard/platform/SinglePlatformView.tsx` around lines 151 - 154, The PlatformTrafficChart component in the SinglePlatformView is currently using mock data from platformTrafficMock instead of real server state data, which causes inconsistency with the parent component's loading and error handling. Replace the platformTrafficMock[platform] data source with an appropriate server state hook (such as a React Query hook) that fetches the actual platform traffic data for the given platform prop. Ensure the data passed to PlatformTrafficChart comes from the same server state management pattern used elsewhere in the component for consistency in loading, error, and data display states.Sources: Coding guidelines, Path instructions
🧹 Nitpick comments (1)
src/hooks/dashboard/useOverviewRoasRankings.ts (1)
19-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
toProviderType정규화 로직을 공용 유틸로 합쳐주세요.동일한 정규화 함수가
src/components/dashboard/platform/PlatformRoasTable.tsx에도 있어요. 매핑 규칙이 바뀔 때 한쪽만 수정되면 랭킹-메트릭 결합과 표시 로직이 쉽게 어긋날 수 있어서, 공용 유틸로 추출해 두 군데에서 재사용하는 편이 안전합니다.🤖 Prompt for 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. In `@src/hooks/dashboard/useOverviewRoasRankings.ts` around lines 19 - 23, The `toProviderType` function currently exists as a duplicate in both useOverviewRoasRankings.ts and PlatformRoasTable.tsx, which creates a maintenance risk when provider mapping rules change. Extract the `toProviderType` function into a shared utility module (create a new utility file if one doesn't exist for provider-related utilities), then import and use it in both files. Remove the duplicate function definition from PlatformRoasTable.tsx and replace its usage with the imported utility function to ensure consistent normalization logic across the codebase.
🤖 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 `@src/components/dashboard/platform/AllPlatformTrafficChart.tsx`:
- Around line 18-20: The AllPlatformTrafficChart component is directly importing
and using platformTrafficMock, which violates the separation between mock and
production code and prevents server state updates from being reflected. Remove
the import of platformTrafficMock from the mock file and refactor the
AllPlatformTrafficChart component to either accept real data as props from a
parent component or consume data directly through a react-query hook. This
ensures the chart displays actual server data and updates reactively based on
server state changes rather than relying on static mock data.
In `@src/utils/dashboard/metricRegistry.ts`:
- Around line 53-56: The currencyFormat object definition uses formatCurrency
instead of formatCurrencyRounded, which causes decimal places to be exposed for
spend/revenue/adSpend metrics inconsistent with the file's rounding convention.
Replace the formatCurrency reference with formatCurrencyRounded in the
currencyFormat object (lines 53-56). Additionally, apply the same change to all
other similar currency format definitions throughout the file as indicated by
the "Also applies to" comment spanning lines 95-124 to ensure consistent
currency formatting across all metrics.
In `@src/utils/dashboard/metricsToKpis.ts`:
- Around line 5-9: Replace the relative import path `./metricRegistry` in the
import statement for getKpiMetric, getMetricKpiTitle, and OVERVIEW_KPI_BINDINGS
with the `@/` alias path to follow the project's import conventions. Update the
import statement to use the full path starting with `@/` instead of the relative
path notation to maintain consistency with project coding guidelines.
---
Outside diff comments:
In `@src/components/dashboard/platform/SinglePlatformView.tsx`:
- Around line 151-154: The PlatformTrafficChart component in the
SinglePlatformView is currently using mock data from platformTrafficMock instead
of real server state data, which causes inconsistency with the parent
component's loading and error handling. Replace the
platformTrafficMock[platform] data source with an appropriate server state hook
(such as a React Query hook) that fetches the actual platform traffic data for
the given platform prop. Ensure the data passed to PlatformTrafficChart comes
from the same server state management pattern used elsewhere in the component
for consistency in loading, error, and data display states.
---
Nitpick comments:
In `@src/hooks/dashboard/useOverviewRoasRankings.ts`:
- Around line 19-23: The `toProviderType` function currently exists as a
duplicate in both useOverviewRoasRankings.ts and PlatformRoasTable.tsx, which
creates a maintenance risk when provider mapping rules change. Extract the
`toProviderType` function into a shared utility module (create a new utility
file if one doesn't exist for provider-related utilities), then import and use
it in both files. Remove the duplicate function definition from
PlatformRoasTable.tsx and replace its usage with the imported utility function
to ensure consistent normalization logic across the codebase.
🪄 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: 7584819a-ed9b-40b9-8766-649073e05aae
📒 Files selected for processing (41)
src/components/auth/common/CommonAuthInput.tsxsrc/components/auth/common/PasswordForm.tsxsrc/components/auth/flows/find-email/EnterPhoneStep.tsxsrc/components/auth/flows/find-email/ShowEmailResultStep.tsxsrc/components/auth/flows/signup/ProfileSetupStep.tsxsrc/components/dashboard/charts/BudgetGaugeChart.tsxsrc/components/dashboard/charts/PerformanceEfficiencyChart.tsxsrc/components/dashboard/charts/TrafficChart.tsxsrc/components/dashboard/charts/budgetGaugeChart.mock.tssrc/components/dashboard/charts/performanceEfficiencyChart.config.tssrc/components/dashboard/charts/trafficChart.config.tssrc/components/dashboard/platform/AllPlatformTrafficChart.tsxsrc/components/dashboard/platform/AllPlatformView.tsxsrc/components/dashboard/platform/PlatformDetailCard.tsxsrc/components/dashboard/platform/PlatformDetailTable.tsxsrc/components/dashboard/platform/PlatformRoasTable.tsxsrc/components/dashboard/platform/PlatformTrafficChart.tsxsrc/components/dashboard/platform/SinglePlatformView.tsxsrc/components/dashboard/platform/TopPerformanceList.tsxsrc/components/dashboard/platform/skeleton/PlatformSkeleton.tsxsrc/components/workspace/InviteMemberModal.tsxsrc/hooks/auth/useEmailVerification.tssrc/hooks/dashboard/useOverviewRoasRankings.tssrc/hooks/dashboard/usePlatformMetrics.tssrc/hooks/dashboard/usePlatformPerformance.tssrc/lib/loadable.tsxsrc/pages/auth/Login.tsxsrc/pages/dashboard/overview/sections/OverviewKpiSection.tsxsrc/pages/dashboard/platform/PlatformDashboard.tsxsrc/pages/dashboard/platform/platformDashboard.mock.tssrc/pages/integration/platformIntegrations.mock.tssrc/routes/AuthRoutes.tsxsrc/routes/MainRoutes.tsxsrc/types/dashboard/overview.tssrc/utils/auth/formatPhoneNumber.tssrc/utils/auth/maskEmail.tssrc/utils/auth/validation.tssrc/utils/dashboard/downloadChart.tssrc/utils/dashboard/metricRegistry.tssrc/utils/dashboard/metricsToKpis.tssrc/utils/dashboard/platformMetricsQuery.ts
💤 Files with no reviewable changes (4)
- src/components/dashboard/platform/skeleton/PlatformSkeleton.tsx
- src/components/dashboard/charts/budgetGaugeChart.mock.ts
- src/pages/integration/platformIntegrations.mock.ts
- src/utils/dashboard/downloadChart.ts
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.
🚨 관련 이슈
#239
✨ 변경사항
✏️ 작업 내용
핵심 아키텍처 - metricRegistry 도입
src/utils/dashboard/metricRegistry.ts- 정수 카운트 → toLocaleString
- 비율/퍼센트 → toFixed(2)%
- KRW 금액 → Math.round + 천 단위 콤마 + ₩
- 증감률 → toFixed(2)% + 절댓값
KPI 변환 — metricsToKpis 리팩터
컴포넌트별 Registry 연동
데이터·훅 레이어
src/utils/dashboard/platformMetricsQuery.ts를 추가해 조회 로직과 캐시 키를 한곳으로 모음utils 파일 구조 정리
mock / dead code 정리
😅 미완성 작업
N/A
📢 논의 사항 및 참고 사항
📝 작업 정리 및 회고
🔗 작업 내용 정리 및 회고
Summary by CodeRabbit
변경 사항
신규 기능
개선 사항
버그 수정