Conversation
Review Summary by Qodo비밀번호 재설정 API 연동 구현
WalkthroughsDescription• 비밀번호 재설정 API 연동 완료 • 이메일 전송 및 인증번호 검증 훅 추가 • 비밀번호 변경 요청 API 통합 • 로딩 상태 및 에러 처리 개선 Diagramflowchart LR
A["이메일 입력"] -->|useSendPasswordCode| B["인증번호 전송"]
B -->|성공| C["인증번호 입력"]
C -->|저장| D["새 비밀번호 입력"]
D -->|useResetPassword| E["비밀번호 변경"]
E -->|성공| F["완료 페이지"]
File Changes1. hooks/useResetPassword.ts
|
Enabling\disabling automation
meaning the
the tool will replace every marker of the form
Note that when markers are enabled, if the original PR description does not contain any markers, the tool will not alter the description at all. |
Custom labelsThe default labels of the If you specify custom labels in the repo's labels page or via configuration file, you can get tailored labels for your use cases.
The list above is eclectic, and aims to give an idea of different possibilities. Define custom labels that are relevant for your repo and use cases. |
Inline File Walkthrough 💎For enhanced user experience, the To enable inline file summary, set
|
Utilizing extra instructionsThe Be specific, clear, and concise in the instructions. With extra instructions, you are the prompter. Notice that the general structure of the description is fixed, and cannot be changed. Extra instructions can change the content or style of each sub-section of the PR description. Examples for extra instructions: Use triple quotes to write multi-line instructions. Use bullet points to make the instructions more readable. |
More PR-Agent commands
|
See the describe usage page for a comprehensive guide on using this tool.
Code Review by Qodo
1. Bypass shared axios instance
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이번 풀 리퀘스트는 비밀번호 재설정 프로세스에 실제 API 연동을 위해 useSendPasswordCode 및 useResetPassword 커스텀 훅을 도입하고, 이메일 발송 및 비밀번호 변경 로직을 구현했습니다. 코드 리뷰 결과, 보안 및 성능 최적화를 위해 axios 대신 Server Actions 사용이 권장되며, useMutation을 중복으로 래핑한 로직의 단순화가 필요합니다. 또한, 가독성을 위해 200줄이 넘는 컴포넌트를 서브 컴포넌트로 분리하고, 사용자 경험을 개선하기 위해 인증 번호를 세션에 저장하는 대신 즉시 서버에서 검증하는 로직을 추가할 것을 제안합니다.
| const resetPassword = async ( | ||
| payload: ResetPasswordRequest, | ||
| ): Promise<ResetPasswordResponse> => { | ||
| const { data } = await axios.patch<ResetPasswordResponse>( | ||
| `${process.env.NEXT_PUBLIC_API_URL}/api/auth/password/code`, | ||
| payload, | ||
| ); | ||
| return data; | ||
| }; |
There was a problem hiding this comment.
Repository Style Guide (2.2 - 43)에 따라 데이터 변이(Mutation)는 클라이언트 측 axios 호출 대신 Server Actions를 사용하여 처리하는 것이 권장됩니다. 이는 보안성을 높이고 클라이언트 번들 크기를 최적화하는 데 도움이 됩니다.
References
- API Route 대신 Server Actions를 사용하여 데이터 변이(Mutation)를 처리해야 함 (link)
| const reset = ( | ||
| payload: ResetPasswordRequest, | ||
| options: { onSuccess: () => void; onError: (msg?: string) => void }, | ||
| ) => { | ||
| mutation.mutate(payload, { | ||
| onSuccess: (data) => { | ||
| if (data.status === 200) { | ||
| options.onSuccess(); | ||
| } else { | ||
| options.onError(data.message); | ||
| } | ||
| }, | ||
| onError: (error) => { | ||
| if (isAxiosError(error) && error.response?.data?.message) { | ||
| options.onError(error.response.data.message); | ||
| } else { | ||
| options.onError(); | ||
| } | ||
| }, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
useMutation의 mutate 함수는 이미 onSuccess와 onError 콜백을 지원하므로, 이를 래핑하는 별도의 reset 함수를 정의하는 것은 다소 중복적입니다. 또한 Axios는 기본적으로 2xx 이외의 응답에 대해 에러를 발생시키므로 로직을 단순화할 수 있으나, 에러 핸들링 시 컨텍스트 보존을 위해 원래의 에러 객체를 그대로 던져야 합니다.
References
- API 에러 핸들링 시 컨텍스트(상태 코드 등)를 보존하기 위해 새로운 일반 에러를 생성하는 대신 원래의 에러 객체를 던져야 함
| import { Check, Eye, EyeOff, X } from "lucide-react"; | ||
| import { useResetPassword } from "@/hooks/useResetPassword"; | ||
|
|
||
| const ScreenNewPasswordPage = () => { |
There was a problem hiding this comment.
컴포넌트 파일이 200줄을 초과하고 있습니다. 가독성과 유지보수성을 위해 입력 필드 등을 별도의 서브 컴포넌트로 분리하는 것을 권장합니다. 특히 비밀번호 조건 체크와 같은 검증 로직은 순수 함수이므로, 공통 유틸리티 파일로 추출하여 관심사를 분리하고 재사용성을 높여야 합니다.
References
- 컴포넌트 파일이 200줄을 초과하면 더 작은 서브 컴포넌트로 분리 권장 (link)
- 검증 로직과 같은 순수 함수는 재사용성과 관심사 분리를 위해 공통 유틸리티 파일로 추출해야 함
| const handleVerify = (e: React.FormEvent) => { | ||
| e.preventDefault(); | ||
| setIsVerifying(true); | ||
|
|
||
| // TODO: 인증번호 확인 API 연동 | ||
| console.log("Verifying code:", verificationCode); | ||
|
|
||
| if (verificationCode === "123456") { | ||
| // 성공 시 세션 저장 후 비밀번호 재설정 입력 페이지로 이동 | ||
| sessionStorage.setItem("reset_verified", "true"); | ||
| // 인증 완료 시 이전 단계 정보는 유지(new 페이지 접근용)하되, success 도달 시 파기할 예정 | ||
| alert("인증에 성공했습니다."); | ||
| router.replace("/reset/password/new"); | ||
| } else { | ||
| setErrorMessage("인증번호를 다시 확인해 주세요."); | ||
| setIsVerifying(false); | ||
| } | ||
| // 인증코드와 이메일을 sessionStorage에 저장만 하고 새 비밀번호 페이지에서 PATCH API 호출 | ||
| // (PATCH /api/auth/password/code 는 email+code+newPassword 를 한 번에 요청) | ||
| sessionStorage.setItem("reset_verified", "true"); | ||
| sessionStorage.setItem("reset_code", verificationCode); | ||
| router.replace("/reset/password/new"); | ||
| }; |
There was a problem hiding this comment.
인증번호를 서버에서 즉시 검증하지 않고 sessionStorage에 저장한 뒤 다음 페이지로 넘기는 방식은 사용자 경험 측면에서 개선이 필요합니다. 사용자가 잘못된 번호를 입력했을 때 alert() 등을 통해 즉각적인 피드백을 줄 수 있도록 이 단계에서 검증 API를 호출하는 로직을 추가하는 것이 좋습니다.
References
- 에러 메시지 표시를 위해 토스트 알림 대신 네이티브 alert() 함수 사용을 선호함
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| const { data } = await axios.post<SendPasswordCodeResponse>( | ||
| `${process.env.NEXT_PUBLIC_API_URL}/api/auth/password/code`, | ||
| { email }, | ||
| ); |
There was a problem hiding this comment.
1. Bypass shared axios instance 🐞 Bug ☼ Reliability
useSendPasswordCode/useResetPassword가 프로젝트 공통 axios 인스턴스(api)를 쓰지 않고 raw axios + NEXT_PUBLIC_API_URL을 직접 조합해 호출하여 withCredentials/401 재발급 인터셉터/공통 설정이 적용되지 않습니다. 또한 NEXT_PUBLIC_API_URL이 비어있을 경우 "undefined/api/..." 같은 잘못된 URL로 요청이 나갈 수 있습니다.
Agent Prompt
## Issue description
`useSendPasswordCode` / `useResetPassword`가 `axios`를 직접 사용하면서 프로젝트 공통 설정(`baseURL`, `withCredentials`, 401 재발급 인터셉터)을 우회합니다. 또한 `NEXT_PUBLIC_API_URL` 누락 시 문자열 결합으로 잘못된 URL("undefined/api/...")이 만들어질 수 있습니다.
## Issue Context
레포에는 `lib/axios.ts`에서 `api` 인스턴스를 만들고, 다른 훅들은 `api.post('/api/...')` 형태로 호출합니다.
## Fix Focus Areas
- hooks/useSendPasswordCode.ts[1-18]
- hooks/useResetPassword.ts[1-25]
- lib/axios.ts[3-42]
## What to change
- 두 훅에서 `import axios from "axios"` 대신 `import { api } from "@/lib/axios"` 사용
- URL은 `${process.env...}` 조합 대신 상대경로로 호출:
- `api.post("/api/auth/password/code", { email })`
- `api.patch("/api/auth/password/code", payload)`
- (선택) 필요 시 에러 메시지 파싱은 기존 `useVerifyEmail` 패턴 유지
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const email = sessionStorage.getItem("reset_email_to_verify") ?? ""; | ||
| const code = sessionStorage.getItem("reset_code") ?? ""; | ||
|
|
||
| reset( | ||
| { email, code, newPassword: password }, | ||
| { | ||
| onSuccess: () => { |
There was a problem hiding this comment.
2. Reset payload may be empty 🐞 Bug ≡ Correctness
ScreenNewPasswordPage가 sessionStorage에서 email/code를 못 읽으면 빈 문자열로 대체한 뒤 그대로 PATCH 요청을 보내 invalid request를 발생시킵니다. reset_verified만 검사하므로 sessionStorage가 중간에 지워지거나 불일치할 때 사용자는 복구 없이 실패 메시지만 보게 됩니다.
Agent Prompt
## Issue description
`/reset/password/new`에서 `reset_email_to_verify` 또는 `reset_code`가 없을 때도 빈 문자열로 PATCH 요청을 보내 플로우가 깨집니다.
## Issue Context
현재는 `reset_verified === 'true'`만 확인하고, `email/code` 유효성은 보장되지 않습니다(사용자 스토리지 삭제/브라우저 정책/이전 플로우 잔재 등).
## Fix Focus Areas
- app/reset/password/new/_components/ScreenNewPasswordPage.tsx[26-65]
## What to change
- `handleSubmit`에서 `email`/`code`가 비어있으면:
- 에러 메시지 표시(예: "세션이 만료되었습니다. 처음부터 다시 시도해 주세요")
- `reset_verified`, `reset_email_to_verify`, `reset_code` 정리 후 `/reset/password`로 이동
- `code.length === 6` 같은 최소 검증도 함께 적용
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
1 participant
No description provided.