refactor: unify frontend API contract types#3
Conversation
- API 응답 계약 기준을 src/shared/types/api.ts로 통합 - validations.ts의 API 응답 중복 선언 제거 - ranking user auth 소비자를 backend OpenAPI 기준에 맞춤
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3925415f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/app/sitemap.ts
Outdated
| const payload = await response.json() as ApiResponse<RankingListResponse> | ||
| if (payload.result !== "SUCCESS" || !payload.data) { | ||
| return null |
There was a problem hiding this comment.
This parser now accepts only the { result, data } envelope, so any environment still returning the older shape (raw rankings/pageInfo or success) will make getRankingPage return null, which causes getTopUsers() to return an empty list and drops all dynamic user URLs from the sitemap. The repo still indicates mixed-format support via api-client's fallback (response.data?.success ?? response.data), so removing fallback handling here introduces an SEO-visible regression for those deployments.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
최신 커밋 bb10f60에서 반영했습니다. sitemap 파서를 legacy 호환 형태(result/data, success, raw payload`)까지 다시 처리하도록 바꿨고, 이로 인한 동적 사용자 URL 누락 회귀를 막았습니다.
| const payload = await response.json() as ApiResponse<RegisterUserResponse> | ||
| if (payload.result === 'SUCCESS' && payload.data) { | ||
| user = payload.data | ||
| } |
There was a problem hiding this comment.
The OG image route now treats only payload.result === 'SUCCESS' with payload.data as valid, so a legacy /users/{username} response shape (e.g., success) will leave user as null and render the "User not found" fallback for real users. Since the rest of the client still explicitly supports legacy payloads, this strict parse can break social preview cards in environments that have not fully migrated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
최신 커밋 bb10f60에서 반영했습니다. OG image 라우트도 legacy 응답 형태(result/data, success, raw payload)를 다시 허용하도록 바꿔서 실제 사용자가 User not found`로 떨어지지 않게 했습니다.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/app/sitemap.ts (1)
31-40: ReusefirstPageDatainstead of fetching page 0 twice.
getTopUsers()already has page 0 from the probe request, but thePromise.allblock asks for it again. That adds one avoidable API call to every sitemap generation/revalidation.♻️ Suggested change
- const pages = Array.from({ length: totalPages }, (_, i) => i) - const responses = await Promise.all(pages.map((page) => getRankingPage(page).catch(() => null))) + const pages = Array.from({ length: Math.max(totalPages - 1, 0) }, (_, i) => i + 1) + const responses = [ + firstPageData, + ...(await Promise.all(pages.map((page) => getRankingPage(page).catch(() => null)))), + ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/sitemap.ts` around lines 31 - 40, The code currently fetches page 0 twice by calling getRankingPage(0) for firstPageData and then including page 0 again in the Promise.all; change the logic in getTopUsers()/sitemap generation to reuse firstPageData: compute totalPages from firstPageData.pageInfo, build the pages array for pages 1..totalPages-1 (not including 0), create an array of promises where the first element is a resolved value of firstPageData and the rest are getRankingPage(page).catch(() => null) for pages > 0, then await Promise.all on that array so getRankingPage is not called for page 0 twice.src/shared/constants/tier-styles.ts (1)
1-1: Keep the style maps exhaustive overTier.Deriving
TIER_ORDERfromTIER_VALUESis a good start, but the maps above are still typed asRecord<Tier | string, ...>, which effectively widens them back tostringand removes compile-time coverage for new tiers. The next tier addition can still compile with a missing style entry.♻️ Suggested pattern
-export const TIER_COLOR_STYLES: Record<Tier | string, string> = { +export const TIER_COLOR_STYLES: Record<Tier, string> = { ... } -export const getTierColorClass = (tier: Tier | string) => - TIER_COLOR_STYLES[tier] || TIER_COLOR_STYLES['IRON'] +export const getTierColorClass = (tier: string) => + TIER_COLOR_STYLES[tier as Tier] ?? TIER_COLOR_STYLES.IRONApply the same pattern to
TIER_STYLES,TIER_BADGE_STYLES,TIER_DOT_COLORS, andTIER_TEXT_COLORS.Also applies to: 150-166
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/constants/tier-styles.ts` at line 1, TIER_* maps (TIER_STYLES, TIER_BADGE_STYLES, TIER_DOT_COLORS, TIER_TEXT_COLORS) are typed too loosely as Record<Tier | string,...> which loses compile-time exhaustiveness; change their types to an exact mapped type over Tier (e.g. { [K in Tier]: ... } or Record<Tier, ...>) and construct/annotate the object literals accordingly (use TIER_VALUES/TIER_ORDER for construction but assert the resulting object with the mapped type) so the compiler enforces an entry for every Tier and fails when a new Tier is added without a corresponding style. Ensure TIER_ORDER remains derived from TIER_VALUES and apply the same strict typing pattern to the maps referenced above.
🤖 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/app/auth/callback/page.tsx`:
- Around line 50-51: The /auth/me response is assumed to be successful and
destructured directly, which can pass undefined to getUser; modify the call
sites that use apiClient.get<void, AuthMeResponse>('/auth/me') (e.g., in auth
callback and oauth2 redirect pages) to validate the response shape before
calling getUser: check that the returned object has result === 'SUCCESS' and
that response.data?.username (or the destructured username) is defined; if not,
handle the error path (reject/throw or call the existing error handler) instead
of calling getUser(undefined). Alternatively, if you prefer a centralized fix,
change the API client's response interceptor (the logic around result ===
'SUCCESS') to reject non-success 2xx responses so callers always receive a valid
success shape.
In `@src/app/users/`[username]/opengraph-image.tsx:
- Line 25: The code indexes tierColors using user.tier but user is parsed with a
cast and tier may be missing or unexpected at runtime, so replace direct
indexing with a safe fallback: compute accent = tierColors[user?.tier as keyof
typeof tierColors] ?? tierColors['default'] (or a known default key like 'free')
wherever accent is set (references: user, tierColors, accent, opengraph image
generation lines ~25 and ~41) so an unknown backend tier doesn't yield undefined
and the OG accent/border styling remains intact.
---
Nitpick comments:
In `@src/app/sitemap.ts`:
- Around line 31-40: The code currently fetches page 0 twice by calling
getRankingPage(0) for firstPageData and then including page 0 again in the
Promise.all; change the logic in getTopUsers()/sitemap generation to reuse
firstPageData: compute totalPages from firstPageData.pageInfo, build the pages
array for pages 1..totalPages-1 (not including 0), create an array of promises
where the first element is a resolved value of firstPageData and the rest are
getRankingPage(page).catch(() => null) for pages > 0, then await Promise.all on
that array so getRankingPage is not called for page 0 twice.
In `@src/shared/constants/tier-styles.ts`:
- Line 1: TIER_* maps (TIER_STYLES, TIER_BADGE_STYLES, TIER_DOT_COLORS,
TIER_TEXT_COLORS) are typed too loosely as Record<Tier | string,...> which loses
compile-time exhaustiveness; change their types to an exact mapped type over
Tier (e.g. { [K in Tier]: ... } or Record<Tier, ...>) and construct/annotate the
object literals accordingly (use TIER_VALUES/TIER_ORDER for construction but
assert the resulting object with the mapped type) so the compiler enforces an
entry for every Tier and fails when a new Tier is added without a corresponding
style. Ensure TIER_ORDER remains derived from TIER_VALUES and apply the same
strict typing pattern to the maps referenced above.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 37ef9bbf-19fc-4380-909d-284b6feed825
📒 Files selected for processing (9)
src/app/auth/callback/page.tsxsrc/app/oauth2/redirect/page.tsxsrc/app/ranking/page.tsxsrc/app/sitemap.tssrc/app/users/[username]/opengraph-image.tsxsrc/shared/constants/tier-styles.tssrc/shared/lib/api-client.tssrc/shared/lib/validations.tssrc/shared/types/api.ts
💤 Files with no reviewable changes (1)
- src/shared/lib/validations.ts
- legacy 응답 fallback을 sitemap과 OG image에 복구 - 비정상 2xx API envelope를 ApiError로 처리 - tier 스타일 맵을 exhaustive typing으로 강화
1 participant
1) 요약
src/shared/types/api.ts로 모으고validations.ts의 API 응답 중복 선언을 제거했습니다.EMERALD누락과/ranking,/users/{username},/auth/me직접 소비자의 계약 drift를 정리했습니다.2) 연관 이슈
3) 사용자 관점 결과
4) 영향 범위
/ranking,/users/[username],/auth/callback,/oauth2/redirect, sitemap, OG imageApiResponse,AuthMeResponse,RankingListResponse,RegisterUserResponse를 사용하도록 정리imgalt 누락 warning 1건만 함께 정리5) 스크린샷 또는 영상
6) 검증 증거
npm run lintnpm run buildJetBrains MonoGoogle Fonts fetch 실패와middlewaredeprecated warning 확인npx tsc --noEmit미구축(GRC-04에서 도입 예정)미실행(UI 동작 변경 없음)7) API / 백엔드 연동 확인
/api/v1/ranking,/api/v1/users/{username},/api/v1/auth/megit-ranker/docs/openapi/openapi.json과 수동 대조로 정합성 확인8) 리스크 및 롤백
9) 체크리스트
Summary by CodeRabbit
Bug Fixes
Refactor