Conversation
Summary of ChangesHello @2yunseong, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 회원가입 과정에 이메일 인증 절차를 도입하여 사용자 계정의 보안을 강화하고, 가입 시 이메일 주소의 유효성을 확인하도록 개선합니다. 이를 통해 스팸 계정 생성을 방지하고, 사용자에게 더욱 신뢰할 수 있는 서비스 환경을 제공하는 것을 목표로 합니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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
회원가입 시 이메일 인증 기능을 추가하는 PR이네요. 전반적으로 기능 구현은 잘 되었지만, 코드의 유지보수성과 견고함을 높이기 위해 몇 가지 개선점을 제안합니다. 주요 제안 사항은 API 함수의 에러 처리 방식을 개선하여 react-query의 onError를 활용하는 것, SignUpForm 컴포넌트로 전달되는 많은 수의 props를 그룹화하는 것, 그리고 여러 useState로 관리되는 인증 관련 상태들을 useReducer나 단일 상태 객체로 통합하여 관리하는 것입니다. 자세한 내용은 각 파일의 리뷰 코멘트를 참고해주세요.
| const { mutate: sendEmailCode, isLoading: isSendingEmailCode } = useMutation({ | ||
| mutationFn: verifySignUpEmail, | ||
| onSuccess: (result) => { | ||
| if (result) { | ||
| setIsVerificationRequested(true); | ||
| setIsEmailCodeWrong(false); | ||
| alert("인증번호가 발송되었습니다."); | ||
| return; | ||
| } | ||
| alert("인증번호 발송에 실패했습니다. 다시 시도해주세요."); | ||
| }, | ||
| onError: () => { | ||
| alert("인증번호 발송에 실패했습니다. 다시 시도해주세요."); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
현재 useMutation의 onSuccess 콜백에서 API 호출 결과(result)에 따라 성공과 실패를 모두 처리하고 있으며, onError 콜백과 실패 처리 로직이 중복됩니다. 이는 verifySignUpEmail 함수가 try-catch로 에러를 감싸고 false를 반환하기 때문입니다.
react-query의 디자인에 맞게 API 함수(verifySignUpEmail)가 에러 발생 시 false를 반환하는 대신 에러를 그대로 throw하도록 수정하는 것을 권장합니다. 이렇게 하면 useMutation의 onError 콜백에서만 실패 로직을 처리하게 되어 코드가 더 명확하고 간결해집니다.
1. API 함수 수정 제안 (frontend/src/apis/user/index.ts)
export const verifySignUpEmail = async ({ email }: VerifyEmailReq) => {
// try-catch를 제거하여 에러가 발생하면 useMutation의 onError에서 처리되도록 합니다.
await https.post("/signup/verify", {
email,
});
};2. useMutation 수정 제안
위와 같이 API를 수정하면, useMutation은 다음과 같이 간소화할 수 있습니다. 이 패턴은 signUp, confirmEmailCode 등 다른 뮤테이션에도 일관되게 적용할 수 있어 프로젝트 전체의 코드 품질을 높일 수 있습니다.
const { mutate: sendEmailCode, isLoading: isSendingEmailCode } = useMutation({
mutationFn: verifySignUpEmail,
onSuccess: () => {
setIsVerificationRequested(true);
setIsEmailCodeWrong(false);
alert("인증번호가 발송되었습니다.");
},
onError: () => {
alert("인증번호 발송에 실패했습니다. 다시 시도해주세요.");
},
});
| try { | ||
| await https.post("/signup/verify", { | ||
| email, | ||
| }); | ||
| return true; | ||
| } catch (error) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
try-catch 블록에서 에러를 잡아서 false를 반환하는 대신, 에러를 그대로 발생시키는 것이 react-query와 함께 사용하기에 더 좋은 패턴입니다. 이렇게 하면 useMutation의 onError 콜백에서 에러를 일관되게 처리할 수 있으며, 컴포넌트의 onSuccess 콜백은 성공 케이스만 다루게 되어 코드가 더 간결하고 명확해집니다. 또한, 에러에 대한 자세한 정보를 onError로 전달할 수 있어 디버깅 및 사용자 피드백에 유리합니다.
이러한 변경은 frontend/app/signup/page.tsx의 useMutation 로직을 단순화하는 데 도움이 됩니다.
await https.post("/signup/verify", {
email,
});
return true;| const [isEmailVerified, setIsEmailVerified] = useState(false); | ||
| const [isEmailCodeWrong, setIsEmailCodeWrong] = useState(false); | ||
| const [isVerificationRequested, setIsVerificationRequested] = useState(false); |
There was a problem hiding this comment.
이메일 인증과 관련된 여러 상태(isEmailVerified, isEmailCodeWrong, isVerificationRequested)를 개별 useState로 관리하고 있습니다. 기능이 더 복잡해지면 상태 간의 의존성으로 인해 관리가 어려워질 수 있습니다. 이와 같은 관련 상태들은 하나의 객체로 묶어 useState로 관리하거나, 상태 전이가 명확한 useReducer를 사용하여 관리하는 것을 고려해볼 수 있습니다. 이렇게 하면 상태 업데이트 로직을 중앙에서 관리할 수 있고, 유효하지 않은 상태 조합을 방지하는 데 도움이 됩니다.
예시:
type VerificationStatus = "IDLE" | "REQUESTED" | "VERIFIED" | "ERROR";
interface VerificationState {
status: VerificationStatus;
}
// useReducer나 useState<VerificationState>를 사용하여 상태 관리(위 코드는 예시이며, 실제 구현은 프로젝트의 컨텍스트에 맞게 조정이 필요합니다.)
| emailCode: string; | ||
| onChangeEmailCode: (value: string) => void; | ||
| isEmailVerified: boolean; | ||
| isEmailCodeWrong: boolean; | ||
| isVerificationRequested: boolean; | ||
| isSendingEmailCode: boolean; | ||
| isCheckingEmailCode: boolean; | ||
| isLoading?: boolean; |
There was a problem hiding this comment.
SignUpForm 컴포넌트로 전달되는 props의 개수가 많아지고 있습니다. 특히 이메일 인증 관련 props가 다수 추가되었습니다. 관련 있는 props들을 하나의 객체로 묶어서 전달하면 prop drilling을 줄이고 코드 가독성을 높일 수 있습니다. 예를 들어, 이메일 인증과 관련된 상태와 핸들러들을 emailVerification과 같은 이름의 객체 prop으로 그룹화할 수 있습니다.
이렇게 하면 SignUpForm을 사용할 때 <SignUpForm {...} emailVerification={emailVerificationProps} /> 와 같이 더 깔끔하게 props를 전달할 수 있습니다.
emailVerification: {
emailCode: string;
onChangeEmailCode: (value: string) => void;
onSendVerificationCode: () => void;
onVerifyEmailCode: () => void;
isEmailVerified: boolean;
isEmailCodeWrong: boolean;
isVerificationRequested: boolean;
isSendingEmailCode: boolean;
isCheckingEmailCode: boolean;
};
isLoading?: boolean;
1 participant
관련 이슈
작업 분류
PR을 통해 해결하려는 문제가 무엇인가요? 🚀
PR에서 핵심적으로 변경된 부분이 어떤 부분인가요? 👀
핵심 변경사항 이외 추가적으로 변경된 사항이 있나요? ➕
추가적으로, 리뷰어가 리뷰하며 알아야 할 정보가 있나요? 🙌
이런 부분을 신경써서 봐주셨으면 좋겠어요. 🙋🏻♂️
체크리스트 ✅
reviewers설정assignees설정label설정