-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[YS-176] 회원가입 로직 리팩토링 (연구자) #23
Conversation
작업하느라 정말 고생 많으셨습니다!! 제가 테스트를 해봤는데 알려주신 것처럼 회원가입 버튼을 누르면 입력값이 초기화돼서 회원가입 자체가 안되고 있는데.. 이 부분은 다른 이슈에서 해결하실 예정인가요? 👀 아니면 제가 피드백 드리는 방향으로 추가 작업을 하실 계획이신가요? 우선 저는 react-hook-form rule로 유효성 검증을 진행해본 경험이 없어서 현재 방식으로는 직접 구현해보지 않은 이상 명확한 피드백을 드리기 어렵고, 어차피 이후에 zod를 적용할 예정이면 현재 유효성 검증 방식으로 계속 작업을 하는게 일정상 빠듯하고 불필요해보여서요..🥲..! 제 생각엔 현재 validate 코드는 아예 제거하고 우선 회원가입이 동작하도록 두는게 좋을 것 같습니다. 유효성 검증은 이후에 추가하는 방향으로요. 현재 validate 코드를 리뷰하기보단 이후 zod를 사용해서 유효성 검증을 구현한 후에 코드리뷰를 진행하겠습니다. 여기까진 제 생각인데 규한님은 어떤 의견인지 공유주시면 감사하겠습니다! +참고로 blur의 경우 react-hook-form의 useForm mode를 'onBlur'로 설정하면 원하시는대로 처리할 수 있습니다! 일반 input외에 커스텀 인풋의 경우 추가로 처리가 필요할 수 있지만 일단 기본적으로 제공하는 기능 중 하나입니다! |
유효성 검증을 위해 trigger 함수를 사용했는데, 이 함수 때문에 컴포넌트가 리렌더링되면서 처리가 어렵더라구요 그래서 추후에 zod schema 사용해서 validation 하는 것 좋은 것 같아요! 지금은 validation 제거하고, 리팩토링할 때 zod 적용해보겠습니다!! |
개요Walkthrough이 풀 리퀘스트는 회원가입 로직을 전면적으로 리팩토링하는 작업을 포함합니다. Naver 로그인 지원, 회원가입 단계의 퍼널(Funnel) 접근 방식 도입, 컴포넌트 구조 재설계, 그리고 상태 관리 개선에 중점을 두고 있습니다. 코드의 모듈성과 재사용성을 높이기 위해 여러 훅과 컴포넌트를 새롭게 구현했습니다. Changes
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 20
🔭 Outside diff range comments (2)
src/components/Header/Header.tsx (1)
Line range hint
20-20
: 에러 및 로딩 상태 처리 필요
useResearcherInfoQuery
의 에러와 로딩 상태가 처리되지 않았습니다. 사용자 경험 향상을 위해 이러한 상태들을 적절히 처리해야 합니다.다음과 같이 수정하는 것을 제안합니다:
- const { data: myData } = useResearcherInfoQuery(); + const { data: myData, isError, isLoading } = useResearcherInfoQuery(); + if (isLoading) { + return <div>로딩 중...</div>; + } + if (isError) { + return <div>오류가 발생했습니다</div>; + }src/app/home/components/PostCard/PostCard.tsx (1)
Line range hint
45-61
: 모집 상태 표시 로직이 개선되었습니다.모집 상태를
recruitDone
에서recruitStatus
로 변경한 것이 더 명확합니다. 하지만 현재 구현에서는recruitStatus
가 boolean 타입처럼 사용되고 있습니다.아래와 같이 명시적인 상태 비교를 추천드립니다:
- {post.recruitStatus ? ( + {post.recruitStatus === 'CLOSED' ? (
🧹 Nitpick comments (22)
src/app/join/components/JoinInfoStep/JoinInfoStep.styles.ts (1)
3-11
: 스타일 상수 분리를 고려해보세요.스타일링이 전반적으로 잘 구성되어 있습니다만, 매직 넘버(2.8rem, 1.2rem, 3.2rem, 4rem 등)를 의미있는 상수로 분리하면 유지보수성이 향상될 것 같습니다.
다음과 같은 방식으로 상수를 분리하는 것을 제안드립니다:
const SPACING = { GAP: '2.8rem', BORDER_RADIUS: '1.2rem', PADDING: { VERTICAL: '3.2rem', HORIZONTAL: '4rem', }, } as const;src/app/home/hooks/usePostListQuery.ts (1)
10-10
: 쿼리 키 최적화 제안현재 모든 파라미터를 쿼리 키에 포함시키고 있어, 불필요한 리패치가 발생할 수 있습니다. 실제로 변경이 필요한 파라미터만 포함하는 것이 좋습니다.
다음과 같이 필수 파라미터만 포함하도록 수정하는 것을 고려해보세요:
- queryKey: [QUERY_KEY.post, matchType, gender, age, region, areas, recruitStatus], + queryKey: [QUERY_KEY.post, { matchType, recruitStatus }],src/apis/post.ts (1)
43-43
: 기본값 설정에 대한 피드백기본값 설정이 잘 되어있습니다. 다만, 타입 안전성을 위해 상수로 분리하는 것을 고려해보세요.
다음과 같이 수정하는 것을 제안합니다:
+ const DEFAULT_POST_PARAMS: PostListParams = { + recruitStatus: 'ALL' + } as const; - export const fetchPostList = async (params: PostListParams = { recruitStatus: 'ALL' }) => { + export const fetchPostList = async (params: PostListParams = DEFAULT_POST_PARAMS) => {src/app/home/components/PostContainer/PostContainer.tsx (1)
53-54
: 데이터 접근 방식이 일관성 있게 변경되었습니다.
data?.content
를 통한 안전한 접근이 좋습니다. 하지만 데이터가 없을 경우의 처리가 필요해 보입니다.아래와 같이 기본값 처리를 추가하는 것을 추천드립니다:
- <span css={totalPostCount}>총 {data?.content.length}개</span> + <span css={totalPostCount}>총 {data?.content?.length ?? 0}개</span> - <PostCardList postList={data?.content} /> + <PostCardList postList={data?.content ?? []} />src/app/join/components/JoinInput/JoinInput.tsx (1)
16-16
: 컴포넌트의 유연성이 향상되었습니다.
control
prop을 선택적으로 만들고onBlur
이벤트 핸들러를 추가한 것이 좋습니다. 하지만any
타입 사용은 개선이 필요합니다.
control
타입을 명시적으로 지정하는 것을 추천드립니다:- control?: any; + control?: Control<FieldValues>;이를 위해 아래 import를 추가해주세요:
import { Control, FieldValues } from 'react-hook-form';Also applies to: 25-25
src/app/join/hooks/useSendUnivAuthCodeMutation.ts (1)
6-6
: 이미 인증된 이메일에 대한 에러 처리 필요
// TODO: 이미 인증된 메일일 경우 에러 처리
라는 주석이 있습니다. 이미 인증된 이메일에 대해 사용자에게 적절한 에러 메시지를 표시하고 처리하는 로직이 필요합니다.이 부분에 대한 에러 처리 코드를 구현하는 데 도움이 필요하시면, 예시 코드를 제공해 드릴 수 있습니다. 새로운 이슈를 생성하여 추적할까요?
src/app/join/layout.tsx (1)
11-15
: 매직 넘버를 상수로 추출하는 것이 좋습니다.
calc(100vh - 12.2rem)
에서 사용된 12.2rem은 매직 넘버입니다. 이 값의 의미를 명확히 하기 위해 상수로 추출하는 것이 좋습니다.+const HEADER_HEIGHT = '12.2rem'; const joinLayout = (theme: Theme) => css` display: flex; background-color: ${theme.colors.field01}; width: 56rem; margin: 0 auto; - min-height: calc(100vh - 12.2rem); + min-height: calc(100vh - ${HEADER_HEIGHT}); `;src/app/join/hooks/useJoinMutation.ts (1)
1-17
: 커스텀 훅의 재사용성을 높일 수 있습니다.성공/실패 콜백을 외부에서 주입받도록 하면 더 유연하게 사용할 수 있습니다.
-const useJoinMutation = () => { +interface UseJoinMutationOptions { + onSuccess?: (data: { accessToken: string; refreshToken: string; memberInfo: MemberInfo }) => void; + onError?: (error: unknown) => void; +} + +const useJoinMutation = (options?: UseJoinMutationOptions) => { return useMutation({ mutationFn: join, onSuccess: ({ accessToken, refreshToken, memberInfo }) => { API.defaults.headers.common['Authorization'] = `Bearer ${accessToken}`; sessionStorage.setItem('refreshToken', refreshToken); sessionStorage.setItem('role', memberInfo.role); + options?.onSuccess?.({ accessToken, refreshToken, memberInfo }); }, + onError: (error) => { + console.error('회원가입 중 오류가 발생했습니다:', error); + options?.onError?.(error); + }, }); };src/app/join/components/JoinSuccessStep/JoinSuccessStep.tsx (2)
27-28
: 이미지 접근성 개선이 필요합니다.SVG 이미지의 alt 텍스트가 더 구체적일 필요가 있습니다.
- <Image src={JoinSuccess} alt="회원가입 성공" width={160} height={140} css={image} /> + <Image src={JoinSuccess} alt="회원가입 성공을 축하하는 일러스트레이션" width={160} height={140} css={image} />
29-31
: 홈 링크의 접근성 개선이 필요합니다.스크린 리더 사용자를 위한 aria-label 추가를 권장드립니다.
- <Link href="/" css={homeLink}> + <Link href="/" css={homeLink} aria-label="그라밋 홈페이지로 이동">src/app/join/hooks/useServiceAgreeCheck.ts (1)
5-31
: 모든 약관 동의 여부를 확인하는 상태 추가를 제안드립니다.모든 필수 약관에 동의했는지 쉽게 확인할 수 있는 파생 상태를 추가하면 좋을 것 같습니다.
const useServiceAgreeCheck = () => { const [serviceAgreeCheck, setServiceAgreeCheck] = useState<ServiceAgreeCheck>({ isTermOfService: false, isPrivacy: false, isAdvertise: false, }); + const isAllRequiredChecked = serviceAgreeCheck.isTermOfService && + serviceAgreeCheck.isPrivacy; // ... existing code ... - return { serviceAgreeCheck, handleAllCheck, handleChangeCheck }; + return { + serviceAgreeCheck, + handleAllCheck, + handleChangeCheck, + isAllRequiredChecked + }; };src/app/join/hooks/useAuthCodeTimer.ts (1)
22-25
: setInterval 사용에 따른 성능 최적화 필요
setInterval
을 사용한 구현은 다음과 같은 잠재적 문제가 있습니다:
- 컴포넌트가 비활성화된 상태에서도 타이머가 계속 실행됨
- 브라우저 탭이 백그라운드 상태일 때 부정확한 타이밍
- 불필요한 리렌더링 발생
다음과 같은 개선된 구현을 제안합니다:
- timerRef.current = setInterval(() => { - setAuthTimer((prev) => prev - 1); - }, ONE_SEC); + const startTime = Date.now(); + timerRef.current = setInterval(() => { + const elapsedSeconds = Math.floor((Date.now() - startTime) / 1000); + const remainingTime = Math.max(0, TEN_MINUTE_SEC - elapsedSeconds); + setAuthTimer(remainingTime); + }, ONE_SEC);src/app/join/hooks/useFunnel.tsx (1)
34-38
: 컴포넌트 렌더링 최적화 필요현재 구현은 매 렌더링마다
find
연산을 수행합니다. 성능 최적화를 위해 다음과 같은 개선을 제안합니다:const Funnel = ({ children }: FunnelProps) => { - const targetStep = children.find((childStep) => childStep.props.name === currentStep); + const targetStep = useMemo( + () => children.find((childStep) => childStep.props.name === currentStep), + [children, currentStep] + ); return <>{targetStep}</>; };src/app/join/components/JoinEmailStep/JoinCheckboxContainer/JoinCheckboxContainer.tsx (2)
1-4
: Import 순서를 수정해 주세요.eslint 규칙에 따라 import 문의 순서를 다음과 같이 수정하는 것이 좋습니다:
-import { ServiceAgreeCheck } from '../../../JoinPage.types'; -import JoinCheckbox from './JoinCheckbox/JoinCheckbox'; -import { termContainer } from './JoinCheckboxContainer.styles'; +import { termContainer } from './JoinCheckboxContainer.styles'; +import JoinCheckbox from './JoinCheckbox/JoinCheckbox'; +import { ServiceAgreeCheck } from '../../../JoinPage.types';🧰 Tools
🪛 eslint
[error] 1-1:
../../../JoinPage.types
import should occur after import of./JoinCheckboxContainer.styles
(import/order)
11-48
: Form Context를 활용한 상태 관리 개선을 제안합니다.현재 props를 통해 상태와 이벤트 핸들러를 전달하고 있는데, react-hook-form의 Form Context를 활용하면 더 효율적인 상태 관리가 가능할 것 같습니다. PR 목표에서 언급된 것처럼 form 데이터와 유효성 검사 관리를 개선하는 방향과도 일치합니다.
다음과 같은 리팩토링을 제안합니다:
-interface JoinCheckboxContainerProps { - serviceAgreeCheck: ServiceAgreeCheck; - handleAllCheck: (e: React.ChangeEvent<HTMLInputElement>) => void; - handleChange: (e: React.ChangeEvent<HTMLInputElement>, name: string) => void; -} const JoinCheckboxContainer = () => { + const { register, watch, setValue } = useFormContext(); + const values = watch(['isTermOfService', 'isPrivacy', 'isAdvertise']); + + const handleAllCheck = (e: React.ChangeEvent<HTMLInputElement>) => { + setValue('isTermOfService', e.target.checked); + setValue('isPrivacy', e.target.checked); + setValue('isAdvertise', e.target.checked); + }; return ( <div css={termContainer}> <JoinCheckbox label="이용약관에 모두 동의합니다" - isChecked={isAllCheck} - onChange={handleAllCheck} + isChecked={values.every(Boolean)} + onChange={handleAllCheck} isAllCheck={true} /> <JoinCheckbox label="서비스 이용약관 동의" - isChecked={isTermOfService} - onChange={(e) => handleChange(e, 'isTermOfService')} + {...register('isTermOfService')} isRequired /> // ... 나머지 체크박스들도 비슷하게 수정 </div> ); };src/app/login/components/LoginCard.tsx (2)
37-39
: 네이버 로그인 구현이 잘 되어있습니다.OAuth URL 구성이 적절하게 되어 있으며, 환경 변수를 올바르게 사용하고 있습니다.
다만, 다음과 같은 개선사항을 고려해보시면 좋을 것 같습니다:
- URL 생성 로직을 별도의 유틸리티 함수로 분리
- 에러 처리 추가
- 로딩 상태 관리 추가
+const createNaverOAuthURL = (role: Role) => { + const baseURL = 'https://nid.naver.com/oauth2.0/authorize'; + const params = new URLSearchParams({ + client_id: process.env.NEXT_PUBLIC_NAVER_CLIENT_ID ?? '', + response_type: 'code', + redirect_uri: process.env.NEXT_PUBLIC_NAVER_REDIRECT_URI ?? '', + state: `${process.env.NEXT_PUBLIC_NAVER_STATE}|${roleMapper[role]}`, + }); + return `${baseURL}?${params.toString()}`; +}; const goToLoginNaver = () => { + try { + setIsLoading(true); + const naverOauthURL = createNaverOAuthURL(role); router.push(naverOauthURL); + } catch (error) { + console.error('네이버 로그인 중 오류가 발생했습니다:', error); + } finally { + setIsLoading(false); + } };
Line range hint
65-72
: 버튼 컴포넌트의 접근성을 개선해주세요.소셜 로그인 버튼에 aria-label을 추가하여 스크린 리더 사용자의 접근성을 개선할 수 있습니다.
-<button css={loginButton} onClick={goToLoginNaver}> +<button + css={loginButton} + onClick={goToLoginNaver} + aria-label="네이버 계정으로 로그인" +>src/app/join/components/JoinEmailStep/UnivAuthInput/UnivAuthInput.tsx (2)
14-16
: import 구문 순서를 수정해 주세요.컴포넌트의 가독성을 위해 다음과 같은 import 순서로 정리해 주세요:
- 외부 라이브러리
- 스타일
- 커스텀 훅
- 컴포넌트
import EmailToast from '../../EmailToast/EmailToast'; -import AuthCodeInput from './AuthCodeInput/AuthCodeInput'; -import useAuthCodeTimer from '../../../hooks/useAuthCodeTimer'; +import useAuthCodeTimer from '../../../hooks/useAuthCodeTimer'; +import AuthCodeInput from './AuthCodeInput/AuthCodeInput';🧰 Tools
🪛 eslint
[error] 15-15:
./AuthCodeInput/AuthCodeInput
import should occur before import of./UnivAuthInput.styles
(import/order)
[error] 16-16:
../../../hooks/useAuthCodeTimer
import should occur before import of../../../hooks/useSendUnivAuthCodeMutation
(import/order)
79-79
: 불필요한 삼항 연산자를 단순화해 주세요.
fieldState.invalid ? true : false
는fieldState.invalid
로 단순화할 수 있습니다.- aria-invalid={fieldState.invalid ? true : false} + aria-invalid={fieldState.invalid}src/app/join/components/JoinEmailStep/JoinEmailStep.tsx (2)
62-64
: 이메일 유효성 검사 패턴을 상수로 분리해 주세요.동일한 이메일 유효성 검사 패턴이 두 곳에서 중복되어 사용되고 있습니다. 재사용성과 유지보수성을 위해 상수로 분리하는 것이 좋습니다.
+const EMAIL_PATTERN = /^[^\s@ㄱ-ㅎㅏ-ㅣ가-힣]+@[^\s@ㄱ-ㅎㅏ-ㅣ가-힣]+\.[a-zA-Z]{2,}$/; +const EMAIL_ERROR_MESSAGE = '이메일 형식이 올바르지 않아요'; // 사용 시 pattern: { - value: /^[^\s@ㄱ-ㅎㅏ-ㅣ가-힣]+@[^\s@ㄱ-ㅎㅏ-ㅣ가-힣]+\.[a-zA-Z]{2,}$/, - message: '이메일 형식이 올바르지 않아요', + value: EMAIL_PATTERN, + message: EMAIL_ERROR_MESSAGE, }Also applies to: 76-78
33-34
: 조건부 로직을 가독성 있게 개선해 주세요.여러 조건을 하나의 표현식으로 결합하는 것보다, 의미 있는 중간 변수들로 분리하면 코드의 가독성이 향상됩니다.
- const allValid = - isEmailVerified && serviceAgreeCheck.isTermOfService && serviceAgreeCheck.isPrivacy; + const isAgreementComplete = serviceAgreeCheck.isTermOfService && serviceAgreeCheck.isPrivacy; + const allValid = isEmailVerified && isAgreementComplete;src/app/join/components/JoinInfoStep/JoinInfoStep.tsx (1)
46-55
: 입력 필드 유효성 검사 규칙을 상수로 분리해 주세요.각 입력 필드마다 반복되는 유효성 검사 규칙들이 있습니다. 이를 상수로 분리하면 유지보수성이 향상됩니다.
const VALIDATION_RULES = { name: { minLength: { value: 2, message: '이름은 최소 2자 이상이어야 합니다.' }, maxLength: { value: 10, message: '이름은 최대 10자 이하여야 합니다.' } }, univName: { minLength: { value: 2, message: '학교명은 최소 2자 이상이어야 합니다.' }, maxLength: { value: 25, message: '학교명은 최대 25자 이하여야 합니다.' } }, // ... 기타 필드들의 규칙 };Also applies to: 64-73, 82-91
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
src/apis/login.ts
(1 hunks)src/apis/post.ts
(3 hunks)src/app/home/components/PostCard/PostCard.tsx
(1 hunks)src/app/home/components/PostContainer/PostContainer.tsx
(4 hunks)src/app/home/hooks/usePostListQuery.ts
(1 hunks)src/app/home/hooks/useResearcherInfoQuery.ts
(1 hunks)src/app/join/JoinEmailStep.tsx
(0 hunks)src/app/join/JoinInfoStep.tsx
(0 hunks)src/app/join/JoinPage.styles.ts
(1 hunks)src/app/join/JoinPage.types.ts
(1 hunks)src/app/join/JoinPage.utils.ts
(1 hunks)src/app/join/JoinSuccessStep.tsx
(0 hunks)src/app/join/components/JoinCheckboxContainer/JoinCheckboxContainer.tsx
(0 hunks)src/app/join/components/JoinEmailStep/JoinCheckboxContainer/JoinCheckboxContainer.tsx
(1 hunks)src/app/join/components/JoinEmailStep/JoinEmailStep.styles.ts
(1 hunks)src/app/join/components/JoinEmailStep/JoinEmailStep.tsx
(1 hunks)src/app/join/components/JoinEmailStep/UnivAuthInput/AuthCodeInput/AuthCodeInput.tsx
(1 hunks)src/app/join/components/JoinEmailStep/UnivAuthInput/UnivAuthInput.tsx
(5 hunks)src/app/join/components/JoinInfoStep/JoinInfoStep.styles.ts
(1 hunks)src/app/join/components/JoinInfoStep/JoinInfoStep.tsx
(1 hunks)src/app/join/components/JoinInput/JoinInput.styles.ts
(0 hunks)src/app/join/components/JoinInput/JoinInput.tsx
(4 hunks)src/app/join/components/JoinSuccessStep/JoinSuccessStep.styles.ts
(1 hunks)src/app/join/components/JoinSuccessStep/JoinSuccessStep.tsx
(1 hunks)src/app/join/components/UnivAuthInput/AuthCodeInput/AuthCodeInput.tsx
(0 hunks)src/app/join/hooks/useAuthCodeTimer.ts
(1 hunks)src/app/join/hooks/useFunnel.tsx
(1 hunks)src/app/join/hooks/useJoinMutation.ts
(1 hunks)src/app/join/hooks/useSendUnivAuthCodeMutation.ts
(1 hunks)src/app/join/hooks/useServiceAgreeCheck.ts
(1 hunks)src/app/join/hooks/useVerifyUnivAuthCodeMutation.ts
(1 hunks)src/app/join/layout.tsx
(1 hunks)src/app/join/page.tsx
(1 hunks)src/app/login/components/LoginCard.tsx
(2 hunks)src/app/login/hooks/useNaverLoginMutation.ts
(1 hunks)src/app/login/naver/NaverLoginPage.styles.ts
(1 hunks)src/app/login/naver/page.tsx
(1 hunks)src/app/upload/components/RegionPopover/RegionPopover.tsx
(2 hunks)src/components/Header/Header.tsx
(1 hunks)src/constants/url.ts
(1 hunks)src/styles/fonts.ts
(1 hunks)src/types/post.ts
(1 hunks)
💤 Files with no reviewable changes (6)
- src/app/join/components/JoinInput/JoinInput.styles.ts
- src/app/join/JoinSuccessStep.tsx
- src/app/join/components/JoinCheckboxContainer/JoinCheckboxContainer.tsx
- src/app/join/JoinEmailStep.tsx
- src/app/join/JoinInfoStep.tsx
- src/app/join/components/UnivAuthInput/AuthCodeInput/AuthCodeInput.tsx
✅ Files skipped from review due to trivial changes (4)
- src/app/login/naver/NaverLoginPage.styles.ts
- src/app/upload/components/RegionPopover/RegionPopover.tsx
- src/app/join/hooks/useVerifyUnivAuthCodeMutation.ts
- src/app/join/components/JoinSuccessStep/JoinSuccessStep.styles.ts
🧰 Additional context used
🪛 eslint
src/app/join/components/JoinEmailStep/JoinCheckboxContainer/JoinCheckboxContainer.tsx
[error] 1-1: ../../../JoinPage.types
import should occur after import of ./JoinCheckboxContainer.styles
(import/order)
src/app/join/components/JoinEmailStep/UnivAuthInput/UnivAuthInput.tsx
[error] 15-15: ./AuthCodeInput/AuthCodeInput
import should occur before import of ./UnivAuthInput.styles
(import/order)
[error] 16-16: ../../../hooks/useAuthCodeTimer
import should occur before import of ../../../hooks/useSendUnivAuthCodeMutation
(import/order)
🪛 Biome (1.9.4)
src/app/join/components/JoinEmailStep/UnivAuthInput/UnivAuthInput.tsx
[error] 83-84: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
🔇 Additional comments (17)
src/app/join/components/JoinInfoStep/JoinInfoStep.styles.ts (1)
1-1
: 필요한 의존성이 올바르게 임포트되었습니다!Theme 타입을 활용한 타입 안정성이 잘 확보되어 있습니다.
src/app/home/components/PostContainer/PostContainer.tsx (2)
20-22
: 필터 상태가 단순화되었습니다.불필요한 필터들이 제거되고
recruitStatus
만 남은 것이 좋습니다.
33-37
: 모집 상태 토글 로직이 개선되었습니다.
isRecruiting
변수를 통해 상태를 명확하게 표현하고 있습니다.src/styles/fonts.ts (1)
72-78
: 새로운 폰트 스타일이 추가되었습니다.
SB28
폰트 스타일이 기존 패턴을 잘 따르고 있습니다.src/app/home/hooks/useResearcherInfoQuery.ts (1)
3-3
: 깔끔한 import 구문입니다!코드의 가독성이 좋습니다.
src/app/join/JoinPage.styles.ts (1)
8-9
: 레이아웃 개선을 위한padding-top
및flex-grow
추가 확인
padding-top: 8.4rem;
를 사용하여 상단 여백을 패딩으로 설정하고,flex-grow: 1;
을 추가하여 컨테이너가 남은 공간을 채우도록 한 것은 레이아웃 유연성을 높이는 적절한 변경입니다.src/app/join/page.tsx (2)
25-36
:useForm
의 초기값 및 모드 설정 확인
react-hook-form
의useForm
을 사용하여 기본값을 설정하고,mode: 'onBlur'
로 설정한 것은 폼의 유효성 검사 시점을 제어하는 데 효과적입니다. 모든 필드에 대한 기본값도 정확하게 설정되어 있어 폼 관리에 문제가 없어 보입니다.
48-62
:FormProvider
로 폼 컴포넌트 감싸기 확인
<FormProvider {...methods}>
로 모든 폼 컴포넌트를 감싸서react-hook-form
의 컨텍스트를 제공한 것은 올바른 구현입니다. 이를 통해 하위 컴포넌트에서 폼 메서드에 접근할 수 있습니다.src/app/join/hooks/useSendUnivAuthCodeMutation.ts (1)
3-4
:sendUnivAuthCode
함수 임포트 및 사용 확인
sendUnivAuthCode
를 임포트하여 뮤테이션 함수로 사용하는 것이 적절합니다. 이를 통해 대학 인증 코드를 보내는 기능을 구현할 수 있습니다.src/app/join/JoinPage.types.ts (1)
12-15
: 동의 체크 인터페이스가 명확하게 정의되어 있습니다.필수 동의사항(이용약관, 개인정보)과 선택 동의사항(광고)이 잘 구분되어 있습니다.
src/constants/url.ts (1)
5-5
: 네이버 로그인 URL 구현이 적절합니다!기존 Google 로그인 URL 패턴을 일관성 있게 따르고 있어 좋습니다.
src/app/login/hooks/useNaverLoginMutation.ts (1)
14-16
: 민감한 정보의 안전한 저장 방식 검토 필요
sessionStorage
에 토큰과 역할 정보를 저장하는 것은 XSS 공격에 취약할 수 있습니다. 보안을 강화하기 위해 다음 사항들을 고려해주세요:
httpOnly
쿠키 사용- 토큰 암호화
- 민감한 정보의 메모리 내 저장
src/app/join/components/JoinEmailStep/JoinCheckboxContainer/JoinCheckboxContainer.tsx (1)
5-9
: Props 인터페이스가 잘 정의되어 있습니다.타입 안정성을 위해 props 인터페이스가 명확하게 정의되어 있습니다.
src/apis/login.ts (1)
52-56
: 인터페이스 정의가 명확합니다.
NaverLoginParams
인터페이스가 필요한 모든 파라미터를 포함하고 있습니다.src/app/join/components/JoinEmailStep/UnivAuthInput/UnivAuthInput.tsx (2)
44-47
: 이메일 인증 로직 개선이 필요합니다.현재 TODO 주석에서 언급된 대로, 이미 인증된 사용자의 경우에만 verify를 수행해야 하는데, 이 부분이 구현되지 않았습니다.
이 부분의 구현을 도와드릴까요? 다음과 같은 방식으로 개선할 수 있습니다:
- 서버 응답에서 사용자의 인증 상태 확인
- 인증 상태에 따른 조건부 처리
96-96
: 인증 타이머 만료 시 처리가 필요합니다.인증 코드 입력 컴포넌트에 타이머가 전달되고 있지만, 타이머 만료 시의 처리 로직이 누락되어 있습니다.
사용자 경험 향상을 위해 다음 사항들을 고려해 주세요:
- 타이머 만료 시 인증 코드 입력 비활성화
- 재전송 버튼 활성화
- 적절한 안내 메시지 표시
src/app/join/components/JoinInfoStep/JoinInfoStep.tsx (1)
23-23
: TODO 항목 구현이 필요합니다.blur 이벤트 처리와 입력값 초기화 방지에 대한 TODO 항목이 있습니다. 이는 사용자 경험에 직접적인 영향을 미치는 중요한 이슈입니다.
다음과 같은 구현을 제안드립니다:
react-hook-form
의mode
옵션을 'onBlur'로 설정- 폼 상태 보존을 위한 로컬 스토리지 활용
- 컴포넌트 언마운트 시 cleanup 로직 추가
export const joinButton = (theme: Theme) => css` | ||
${theme.fonts.body.normal.SB16}; | ||
background-color: ${theme.colors.primaryMint}; | ||
color: ${theme.colors.text01}; | ||
border-radius: 1.2rem; | ||
padding: 1.2rem 0; | ||
width: 20rem; | ||
align-items: center; | ||
margin-bottom: 5.6rem; | ||
|
||
:disabled { | ||
color: ${theme.colors.text02}; | ||
background-color: ${theme.colors.field04}; | ||
} | ||
`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
마진 설정 위치 재고려가 필요합니다.
버튼 스타일링은 전반적으로 잘 되어 있으나, margin-bottom: 5.6rem
은 컴포넌트의 재사용성을 제한할 수 있습니다. 이러한 여백은 부모 컴포넌트에서 관리하는 것이 더 적절할 것 같습니다.
다음과 같이 수정하는 것을 추천드립니다:
export const joinButton = (theme: Theme) => css`
${theme.fonts.body.normal.SB16};
background-color: ${theme.colors.primaryMint};
color: ${theme.colors.text01};
border-radius: 1.2rem;
padding: 1.2rem 0;
width: 20rem;
align-items: center;
- margin-bottom: 5.6rem;
:disabled {
color: ${theme.colors.text02};
background-color: ${theme.colors.field04};
}
`;
그리고 부모 컴포넌트에서 다음과 같이 처리하시면 됩니다:
const ParentComponent = styled.div`
& > button {
margin-bottom: 5.6rem;
}
`;
@@ -1,6 +1,6 @@ | |||
export interface Post { | |||
postInfo: PostInfo; | |||
recruitDone: boolean; | |||
recruitStatus: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
타입 불일치 문제 해결 필요
recruitStatus
의 타입이 boolean
으로 정의되어 있지만, API (src/apis/post.ts
)에서는 'ALL' | 'OPEN' 문자열을 사용하고 있습니다. 이는 타입 불일치를 야기할 수 있습니다.
다음과 같이 타입을 수정하는 것을 제안합니다:
- recruitStatus: boolean;
+ recruitStatus: 'ALL' | 'OPEN';
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
recruitStatus: boolean; | |
recruitStatus: 'ALL' | 'OPEN'; |
interface PostResponse { | ||
content: Post[]; | ||
isLast: boolean; | ||
page: number; | ||
size: number; | ||
totalCount: number; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
응답 데이터 검증 로직 추가 필요
PostResponse
인터페이스가 추가되어 타입 안전성이 향상되었습니다. 하지만 실제 응답 데이터의 유효성 검증이 없습니다.
zod
스키마를 사용하여 런타임에서도 데이터 유효성을 검증하는 것을 추천드립니다:
import { z } from 'zod';
const PostResponseSchema = z.object({
content: z.array(PostSchema),
isLast: z.boolean(),
page: z.number(),
size: z.number(),
totalCount: z.number()
});
@@ -62,6 +64,7 @@ const JoinInput = ({ | |||
field.onChange(e); | |||
if (onChange) onChange(); | |||
}} | |||
onBlur={onBlur} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
onBlur 이벤트 처리가 추가되었습니다.
입력 필드에 onBlur
이벤트 핸들러가 추가되었지만, textarea에는 누락되어 있습니다.
일관성을 위해 textarea에도 동일하게 적용하는 것을 추천드립니다:
<textarea
{...field}
placeholder={placeholder}
disabled={disabled}
aria-invalid={fieldState.invalid ? true : false}
onChange={(e) => {
field.onChange(e);
if (onChange) onChange();
}}
+ onBlur={onBlur}
rows={3}
maxLength={100}
/>
Committable suggestion skipped: line range outside the PR's diff.
export const useResearcherInfoQuery = () => { | ||
const role = sessionStorage.getItem('role'); | ||
const userRole = role === 'RESEARCHER' ? 'researchers' : 'participants'; | ||
|
||
return useQuery({ | ||
queryKey: ['researcherInfo'], | ||
queryFn: getResearcherInfo, | ||
enabled: !!role, | ||
enabled: !!userRole, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
연구자 정보 쿼리 활성화 조건 수정 필요
현재 구현상 참여자 역할을 가진 사용자도 연구자 정보를 조회할 수 있습니다. 보안과 불필요한 API 호출을 방지하기 위해 다음과 같이 수정하는 것이 좋겠습니다:
- enabled: !!userRole,
+ enabled: userRole === 'researchers',
🔗 Analysis chain
enabled
조건의 의도된 동작 여부를 확인해주세요.
userRole
이 'participants'인 경우에도 쿼리가 활성화될 수 있습니다. 함수 이름이 useResearcherInfoQuery
인 것을 고려할 때, 연구자 정보 조회는 연구자 역할을 가진 사용자에게만 제한되어야 할 것 같습니다.
다음과 같이 수정하는 것을 고려해보세요:
- enabled: !!userRole,
+ enabled: userRole === 'researchers',
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Find the complete implementation of useResearcherInfoQuery
echo "=== useResearcherInfoQuery Implementation ==="
ast-grep --pattern 'export function useResearcherInfoQuery($_) {
$$$
}'
echo -e "\n=== useResearcherInfoQuery Hook Usage ==="
rg "useResearcherInfoQuery" -l
echo -e "\n=== Similar Query Hooks with Role Checks ==="
rg "enabled.*userRole" -A 2 -B 2
Length of output: 860
Script:
#!/bin/bash
echo "=== useResearcherInfoQuery.ts Content ==="
cat src/app/home/hooks/useResearcherInfoQuery.ts
echo -e "\n=== Header.tsx Usage ==="
cat src/components/Header/Header.tsx
echo -e "\n=== Role Type Definitions ==="
rg "type.*Role|enum.*Role|interface.*Role" -A 3 -B 1
echo -e "\n=== Role Constants or Utils ==="
rg "Role.*=.*['\"]researchers['\"]" -A 2 -B 2
Length of output: 2667
export const naverLogin = async ({ code, role, state }: NaverLoginParams) => { | ||
const res = await API.post<LoginResponse>(API_URL.naver(role), { | ||
authorizationCode: code, | ||
state, | ||
}); | ||
|
||
return res.data; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
에러 처리를 추가해주세요.
네이버 로그인 API 호출 시 발생할 수 있는 에러에 대한 처리가 필요합니다.
export const naverLogin = async ({ code, role, state }: NaverLoginParams) => {
+ try {
const res = await API.post<LoginResponse>(API_URL.naver(role), {
authorizationCode: code,
state,
});
return res.data;
+ } catch (error) {
+ if (error instanceof Error) {
+ throw new Error(`네이버 로그인 실패: ${error.message}`);
+ }
+ throw new Error('네이버 로그인 중 알 수 없는 오류가 발생했습니다.');
+ }
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export const naverLogin = async ({ code, role, state }: NaverLoginParams) => { | |
const res = await API.post<LoginResponse>(API_URL.naver(role), { | |
authorizationCode: code, | |
state, | |
}); | |
return res.data; | |
}; | |
export const naverLogin = async ({ code, role, state }: NaverLoginParams) => { | |
try { | |
const res = await API.post<LoginResponse>(API_URL.naver(role), { | |
authorizationCode: code, | |
state, | |
}); | |
return res.data; | |
} catch (error) { | |
if (error instanceof Error) { | |
throw new Error(`네이버 로그인 실패: ${error.message}`); | |
} | |
throw new Error('네이버 로그인 중 알 수 없는 오류가 발생했습니다.'); | |
} | |
}; |
<button type="button" css={sendAgainButton}> | ||
인증번호 재전송 | ||
</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
재전송 버튼의 핸들러가 누락되었습니다.
'인증번호 재전송' 버튼에 클릭 핸들러가 구현되어 있지 않습니다.
const handleVerifyUniv = () => { | ||
const univEmail = getValues('univEmail'); | ||
|
||
verifyEmail( | ||
{ univEmail, inputCode: authCode }, | ||
{ | ||
onSuccess: () => { | ||
setIsToastOpen(true); | ||
handleVerifyEmail(); | ||
}, | ||
}, | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
이메일 인증 실패 처리를 추가해주세요.
현재는 성공 케이스만 처리되어 있습니다. 실패 시 사용자에게 적절한 피드백을 제공해야 합니다.
const handleVerifyUniv = () => {
const univEmail = getValues('univEmail');
verifyEmail(
{ univEmail, inputCode: authCode },
{
onSuccess: () => {
setIsToastOpen(true);
handleVerifyEmail();
},
+ onError: (error) => {
+ setAuthCode('');
+ alert('인증번호가 올바르지 않습니다. 다시 시도해주세요.');
+ },
},
);
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const handleVerifyUniv = () => { | |
const univEmail = getValues('univEmail'); | |
verifyEmail( | |
{ univEmail, inputCode: authCode }, | |
{ | |
onSuccess: () => { | |
setIsToastOpen(true); | |
handleVerifyEmail(); | |
}, | |
}, | |
); | |
}; | |
const handleVerifyUniv = () => { | |
const univEmail = getValues('univEmail'); | |
verifyEmail( | |
{ univEmail, inputCode: authCode }, | |
{ | |
onSuccess: () => { | |
setIsToastOpen(true); | |
handleVerifyEmail(); | |
}, | |
onError: (error) => { | |
setAuthCode(''); | |
alert('인증번호가 올바르지 않습니다. 다시 시도해주세요.'); | |
}, | |
}, | |
); | |
}; |
const handleChangeAuthCode = (e: ChangeEvent<HTMLInputElement>) => { | ||
if (e.target.value.length <= AUTH_CODE_VALID_LENGTH) { | ||
setAuthCode(e.target.value); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
입력값 유효성 검사를 개선해주세요.
현재는 길이만 확인하고 있는데, 숫자만 입력되도록 추가 검증이 필요합니다.
const handleChangeAuthCode = (e: ChangeEvent<HTMLInputElement>) => {
+ const value = e.target.value.replace(/[^0-9]/g, '');
if (e.target.value.length <= AUTH_CODE_VALID_LENGTH) {
- setAuthCode(e.target.value);
+ setAuthCode(value);
}
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const handleChangeAuthCode = (e: ChangeEvent<HTMLInputElement>) => { | |
if (e.target.value.length <= AUTH_CODE_VALID_LENGTH) { | |
setAuthCode(e.target.value); | |
} | |
}; | |
const handleChangeAuthCode = (e: ChangeEvent<HTMLInputElement>) => { | |
const value = e.target.value.replace(/[^0-9]/g, ''); | |
if (e.target.value.length <= AUTH_CODE_VALID_LENGTH) { | |
setAuthCode(value); | |
} | |
}; |
<button css={joinButton} onClick={onNext}> | ||
회원가입 | ||
</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
회원가입 버튼의 상태 관리가 필요합니다.
현재 회원가입 버튼이 폼의 유효성 상태와 관계없이 항상 활성화되어 있습니다. 이는 사용자에게 잘못된 피드백을 줄 수 있습니다.
- <button css={joinButton} onClick={onNext}>
+ <button
+ css={joinButton}
+ onClick={onNext}
+ disabled={!formState.isValid}
+ >
회원가입
</button>
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
유효성 검증은 이후 zod 적용 때 확인하겠습니다!
코드래빗이 적용되었고 참고할만한 리뷰가 있으면 반영해도 좋을 것 같습니다!
고생하셨습니다 🙌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/app/join/components/JoinEmailStep/UnivAuthInput/UnivAuthInput.tsx (2)
Line range hint
64-68
: 이메일 유효성 검사 패턴을 개선해주세요.현재 정규식 패턴이 복잡하고 유지보수가 어려울 수 있습니다. 또한 한글 도메인을 지원하지 않는 문제가 있습니다.
rules={{ required: '학교 이메일을 입력해주세요', pattern: { - value: /^[^\s@ㄱ-ㅎㅏ-ㅣ가-힣]+@[^\s@ㄱ-ㅎㅏ-ㅣ가-힣]+\.[a-zA-Z]{2,}$/, + value: /^[^\s@]+@[^\s@]+\.[^\s@]+$/, message: '이메일 형식이 올바르지 않아요', }, }}
79-79
: 불필요한 삼항 연산자를 제거해주세요.
aria-invalid
속성에 불필요한 삼항 연산자가 사용되었습니다.- aria-invalid={fieldState.invalid ? true : false} + aria-invalid={fieldState.invalid}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/home/components/PostCard/PostCard.tsx
(1 hunks)src/app/join/components/JoinEmailStep/UnivAuthInput/AuthCodeInput/AuthCodeInput.tsx
(1 hunks)src/app/join/components/JoinEmailStep/UnivAuthInput/UnivAuthInput.tsx
(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/home/components/PostCard/PostCard.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/join/components/JoinEmailStep/UnivAuthInput/UnivAuthInput.tsx
[error] 83-84: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
🔇 Additional comments (2)
src/app/join/components/JoinEmailStep/UnivAuthInput/AuthCodeInput/AuthCodeInput.tsx (2)
36-40
: 입력값 유효성 검사를 개선해주세요.현재는 길이 검증만 수행하고 있습니다. 숫자만 입력되도록 추가 검증이 필요합니다.
const handleChangeAuthCode = (e: ChangeEvent<HTMLInputElement>) => { + const value = e.target.value.replace(/[^0-9]/g, ''); if (e.target.value.length <= AUTH_CODE_VALID_LENGTH) { - setAuthCode(e.target.value); + setAuthCode(value); } };
42-54
: 이메일 인증 실패 처리를 추가해주세요.현재는 성공 케이스만 처리되어 있습니다. 실패 시 사용자에게 적절한 피드백을 제공해야 합니다.
const handleVerifyUniv = () => { const univEmail = getValues('univEmail'); verifyEmail( { univEmail, inputCode: authCode }, { onSuccess: () => { setIsToastOpen(true); handleVerifyEmail(); }, + onError: (error) => { + setAuthCode(''); + alert('인증번호가 올바르지 않습니다. 다시 시도해주세요.'); + }, }, ); };
<input | ||
placeholder="인증번호 6자리 입력" | ||
type="number" | ||
disabled={isUnivVerify} | ||
value={authCode} | ||
onChange={handleChangeAuthCode} | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
접근성 개선이 필요합니다.
다음과 같은 접근성 개선이 필요합니다:
- 스크린 리더 사용자를 위한
aria-label
추가 type="number"
는 인증번호 입력에 적합하지 않습니다 (up/down 버튼 노출)
<input
+ aria-label="인증번호 입력"
placeholder="인증번호 6자리 입력"
- type="number"
+ type="text"
+ inputMode="numeric"
+ pattern="[0-9]*"
disabled={isUnivVerify}
value={authCode}
onChange={handleChangeAuthCode}
/>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<input | |
placeholder="인증번호 6자리 입력" | |
type="number" | |
disabled={isUnivVerify} | |
value={authCode} | |
onChange={handleChangeAuthCode} | |
/> | |
<input | |
aria-label="인증번호 입력" | |
placeholder="인증번호 6자리 입력" | |
type="text" | |
inputMode="numeric" | |
pattern="[0-9]*" | |
disabled={isUnivVerify} | |
value={authCode} | |
onChange={handleChangeAuthCode} | |
/> |
// TODO: 이미 인증된 유저인 경우에만 verify | ||
handleVerifyEmail(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이미 인증된 사용자 검증 로직 구현이 필요합니다.
현재 TODO 주석에 표시된 대로, 이미 인증된 사용자인 경우에만 verify를 호출해야 하지만 모든 에러 케이스에서 verify가 호출되고 있습니다. 이는 보안 문제를 일으킬 수 있습니다.
onError: (error) => {
- // TODO: 이미 인증된 유저인 경우에만 verify
- handleVerifyEmail();
+ if (error.code === 'ALREADY_VERIFIED_USER') {
+ handleVerifyEmail();
+ } else {
+ alert(error.message);
+ }
},
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/app/join/JoinPage.utils.ts (2)
8-9
: 상수를 함수 외부로 이동해야 합니다.
GOOGLE_DOMAINS
와NAVER_DOMAINS
상수를 함수 외부로 이동하여 재사용성을 높이고 메모리 사용을 최적화해야 합니다.+const GOOGLE_DOMAINS = ['gmail.com', 'googlemail.com']; +const NAVER_DOMAINS = ['naver.com', 'nate.com']; + export const getProvider = (email: string): 'GOOGLE' | 'NAVER' => { - const GOOGLE_DOMAINS = ['gmail.com', 'googlemail.com']; - const NAVER_DOMAINS = ['naver.com', 'nate.com'];
19-19
: 에러 메시지 국제화가 필요합니다.에러 메시지를 i18n을 통해 관리하여 다국어 지원을 준비해야 합니다.
- throw new Error('지원하지 않는 이메일입니다.'); + throw new Error(t('errors.unsupported_email'));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/join/JoinPage.utils.ts
(1 hunks)
🔇 Additional comments (1)
src/app/join/JoinPage.utils.ts (1)
11-11
:⚠️ Potential issue이메일 유효성 검사가 필요합니다.
이메일 형식 검증 없이 문자열을 분할하면 런타임 오류가 발생할 수 있습니다.
export const getProvider = (email: string): 'GOOGLE' | 'NAVER' => { + if (!email || !email.includes('@')) { + throw new Error('유효하지 않은 이메일 형식입니다.'); + } + const targetDomain = email.split('@')[1].toLowerCase();Likely invalid or redundant comment.
src/app/join/JoinPage.utils.ts
Outdated
if (GOOGLE_DOMAINS.some((domain) => targetDomain.includes(domain))) { | ||
return 'GOOGLE'; | ||
} else if (NAVER_DOMAINS.some((domain) => targetDomain.includes(domain))) { | ||
return 'NAVER'; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
도메인 매칭 로직을 개선해야 합니다.
includes()
사용 시 부분 문자열도 매칭되어 잘못된 결과가 나올 수 있습니다. 예를 들어 'fake-gmail.com'도 매칭됩니다.
- if (GOOGLE_DOMAINS.some((domain) => targetDomain.includes(domain))) {
+ if (GOOGLE_DOMAINS.includes(targetDomain)) {
return 'GOOGLE';
- } else if (NAVER_DOMAINS.some((domain) => targetDomain.includes(domain))) {
+ } else if (NAVER_DOMAINS.includes(targetDomain)) {
return 'NAVER';
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if (GOOGLE_DOMAINS.some((domain) => targetDomain.includes(domain))) { | |
return 'GOOGLE'; | |
} else if (NAVER_DOMAINS.some((domain) => targetDomain.includes(domain))) { | |
return 'NAVER'; | |
} | |
if (GOOGLE_DOMAINS.includes(targetDomain)) { | |
return 'GOOGLE'; | |
} else if (NAVER_DOMAINS.includes(targetDomain)) { | |
return 'NAVER'; | |
} |
Issue Number
close #22
As-Is
To-Be
Check List
Test Screenshot
(Optional) Additional Description
useFunnel.tsx
🚨 form 데이터 관리 & validation 설정
react-hook-form 도 익숙하진 않고, zod 사용도 익숙하지 않아서 validation 처리가 미흡합니다ㅠ rule은 설정하였는데 해당 rule에 따라서 원하는 시점에 validation 하는 게 어렵더라구요ㅜ
hook-form이 기본적으로 uncontrolled로 동작해서 입력값이 변할 때마다 validation 하는 게 어렵던데 어떻게 처리하나요??
예를 들면 연구자 회원가입에서 이름 입력하다가 blur 되었을 때 validation을 하고 싶어서 trigger를 호출하였더니 컴포넌트가 리렌더링되면서 입력값이 모두 지워지는 문제가 있었습니다! 개선 방향이나 키워드 알려주시면 추후에 유효성 검증 리팩토링도 따로 진행하겠습니다:)
Summary by CodeRabbit
릴리즈 노트
새로운 기능
JoinEmailStep
및JoinInfoStep
컴포넌트 추가버그 수정
사용자 경험