feat: 학점 카드에 모학교명 표시#587
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/web/src/app/university/application/apply/ApplyPageContent.tsx (2)
40-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTODO로 남긴 GPA 조회 실패 fallback, 함께 마무리해 드릴까요?
서버의 모학교 미인증 에러 코드가 확정되면 인증 화면으로 보내는 fallback이 필요하다는 메모가 잘 남아 있습니다. 현재는
shouldFetchGpaScore로만 진입을 막고 있어, 인증은 됐지만 서버가 미인증 에러를 주는 경계 상황은 아직 비어 있는 상태입니다.원하시면 에러 코드 분기와 리다이렉트 처리를 추가하는 초안을 만들거나, 추적용 이슈를 열어 드리겠습니다.
🤖 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 `@apps/web/src/app/university/application/apply/ApplyPageContent.tsx` around lines 40 - 42, The GPA fetch path in ApplyPageContent still lacks the fallback for server-side “home university not authenticated” failures. Update the useGetMyGpaScore flow in ApplyPageContent so that when the API returns the agreed authentication-related error code, the user is redirected to the school-authentication screen instead of leaving the state unresolved. Use the existing shouldFetchGpaScore gating and the useGetMyGpaScore result handling to add the error-code branch and redirect behavior, and keep the fallback logic colocated with the GPA query so it is easy to find later.
54-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win같은 "인증 게이트 + 리다이렉트" 패턴이 세 화면에 중복되고 있습니다.
shouldFetchGpaScore계산,useEffect리다이렉트, 그리고homeUniversityId === null일 때의 early-return 조합이 이 파일과ScoreScreen.tsx,GpaSubmitForm.tsx에 거의 동일하게 반복됩니다. 지금은 잘 맞아떨어지지만, 한 곳만 조건이 바뀌면 화면마다 동작이 미묘하게 어긋날 위험이 있습니다. 공용 훅(예:useRequireHomeUniversity(returnTo))으로 모아 두면 한 번의 수정으로 세 화면이 함께 안전해집니다.🤖 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 `@apps/web/src/app/university/application/apply/ApplyPageContent.tsx` around lines 54 - 60, The authentication gate and redirect logic is duplicated across ApplyPageContent, ScoreScreen, and GpaSubmitForm, including the homeUniversityId null early-return, shouldFetchGpaScore-style checks, and the useEffect redirect. Extract this shared behavior into a reusable hook such as useRequireHomeUniversity(returnTo) and have each screen call it so the redirect target and gate conditions are defined in one place. Keep the screen-specific rendering logic, but remove the repeated auth/redirect branching from each component and rely on the shared hook instead.apps/web/src/app/university/score/submit/gpa/GpaSubmitForm.tsx (1)
41-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff인증 게이트 로직 자체는 정확하고 안전합니다.
모든 훅(
useState,useForm,useEffect)이 early return 위에 위치해 훅 규칙을 위반하지 않으며, 리다이렉트 조건도ApplyPageContent·ScoreScreen과 동일한 패턴이라 일관성이 좋습니다.다만 동일한 "인증 초기화 + 인증됨 + homeUniversityId null → verification 경로로 리다이렉트 후
null반환" 패턴이 세 화면에 그대로 복제되어 있습니다. 작은 커스텀 훅으로 묶으면 분기 조건이 한 곳으로 모여 향후 정책 변경 시 누락을 줄일 수 있습니다.♻️ 공통 훅 추출 예시
// apps/web/src/app/my/school-email/_lib/useHomeUniversityGate.ts import { useEffect } from "react"; import { useRouter } from "next/navigation"; import useAuthStore from "`@/lib/zustand/useAuthStore`"; import { getSchoolEmailVerificationPath, type SchoolEmailReturnTo } from "./returnTo"; export const useHomeUniversityGate = (returnTo: SchoolEmailReturnTo) => { const router = useRouter(); const homeUniversityId = useAuthStore((s) => s.homeUniversityId); const isAuthInitialized = useAuthStore((s) => s.isInitialized); const isAuthenticated = useAuthStore((s) => s.isAuthenticated); const shouldRedirect = isAuthInitialized && isAuthenticated && homeUniversityId === null; useEffect(() => { if (shouldRedirect) router.replace(getSchoolEmailVerificationPath(returnTo)); }, [shouldRedirect, returnTo, router]); return { shouldRedirect }; };- useEffect(() => { - if (!isAuthInitialized || !isAuthenticated || homeUniversityId !== null) { - return; - } - router.replace(getSchoolEmailVerificationPath("gpaSubmit")); - }, [homeUniversityId, isAuthInitialized, isAuthenticated, router]); - - if (isAuthInitialized && isAuthenticated && homeUniversityId === null) { - return null; - } + const { shouldRedirect } = useHomeUniversityGate("gpaSubmit"); + if (shouldRedirect) { + return null; + }🤖 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 `@apps/web/src/app/university/score/submit/gpa/GpaSubmitForm.tsx` around lines 41 - 51, The authentication gate logic in GpaSubmitForm is duplicated across multiple screens, so extract the shared “initialized + authenticated + homeUniversityId === null” redirect/return-null flow into a reusable custom hook. Move the logic currently in useEffect and the null-return guard into a hook like useHomeUniversityGate, keep the router.replace(getSchoolEmailVerificationPath(...)) behavior unchanged, and reuse it alongside the existing ApplyPageContent and ScoreScreen patterns to centralize future policy changes.
🤖 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 `@apps/web/src/app/university/score/ScoreScreen.tsx`:
- Around line 49-52: The early-return in ScoreScreen is only guarding after auth
initialization, so before isAuthInitialized becomes true the component can
briefly render an empty GPA state and then update. Update the conditional flow
in ScoreScreen to treat the unauthenticated-initializing state as loading/null
as well, using isAuthInitialized, isAuthenticated, and homeUniversityId together
so the screen does not flash a blank state before data is ready.
---
Nitpick comments:
In `@apps/web/src/app/university/application/apply/ApplyPageContent.tsx`:
- Around line 40-42: The GPA fetch path in ApplyPageContent still lacks the
fallback for server-side “home university not authenticated” failures. Update
the useGetMyGpaScore flow in ApplyPageContent so that when the API returns the
agreed authentication-related error code, the user is redirected to the
school-authentication screen instead of leaving the state unresolved. Use the
existing shouldFetchGpaScore gating and the useGetMyGpaScore result handling to
add the error-code branch and redirect behavior, and keep the fallback logic
colocated with the GPA query so it is easy to find later.
- Around line 54-60: The authentication gate and redirect logic is duplicated
across ApplyPageContent, ScoreScreen, and GpaSubmitForm, including the
homeUniversityId null early-return, shouldFetchGpaScore-style checks, and the
useEffect redirect. Extract this shared behavior into a reusable hook such as
useRequireHomeUniversity(returnTo) and have each screen call it so the redirect
target and gate conditions are defined in one place. Keep the screen-specific
rendering logic, but remove the repeated auth/redirect branching from each
component and rely on the shared hook instead.
In `@apps/web/src/app/university/score/submit/gpa/GpaSubmitForm.tsx`:
- Around line 41-51: The authentication gate logic in GpaSubmitForm is
duplicated across multiple screens, so extract the shared “initialized +
authenticated + homeUniversityId === null” redirect/return-null flow into a
reusable custom hook. Move the logic currently in useEffect and the null-return
guard into a hook like useHomeUniversityGate, keep the
router.replace(getSchoolEmailVerificationPath(...)) behavior unchanged, and
reuse it alongside the existing ApplyPageContent and ScoreScreen patterns to
centralize future policy changes.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a2be88fe-a7cc-46ae-98c6-06c08c453b9e
📒 Files selected for processing (9)
apps/web/src/apis/Scores/api.tsapps/web/src/apis/Scores/getGpaList.tsapps/web/src/app/my/school-email/_lib/returnTo.tsapps/web/src/app/my/school-email/_ui/SchoolEmailVerificationContent/index.tsxapps/web/src/app/university/application/apply/ApplyPageContent.tsxapps/web/src/app/university/application/apply/GpaStep.tsxapps/web/src/app/university/score/ScoreScreen.tsxapps/web/src/app/university/score/submit/gpa/GpaSubmitForm.tsxapps/web/src/types/score.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
1 participant
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.
작업 내용
인하대학교문구를 제거하고,/scores/gpas응답의homeUniversityName을 표시하도록 변경했습니다.homeUniversityName과gpaScoreStatusResponseList를 함께 사용할 수 있도록 훅 반환 구조를 변경했습니다.returnTomap으로 관리하도록 분리했습니다./university/application/apply/university/score/university/score/submit/gpa