-
Notifications
You must be signed in to change notification settings - Fork 1
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
Fix: #291 소셜로그인 callback URL 오류 수정 #294
Fix: #291 소셜로그인 callback URL 오류 수정 #294
Conversation
Walkthrough이 변경 사항은 소셜 로그인 기능의 제공자 이름을 대문자에서 소문자로 변경하는 것을 포함합니다. Changes
Possibly related PRs
Suggested reviewers
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: 2
🧹 Outside diff range and nitpick comments (3)
src/components/user/auth-form/SocialButton.tsx (1)
Line range hint
23-33
: 로그인 실패 시 디버깅을 위한 로깅 추가 제안에러 처리 로직에 디버깅을 위한 로깅을 추가하면 좋을 것 같습니다.
try { window.location.href = url; } catch (error) { + console.error('소셜 로그인 리다이렉션 실패:', error); toastError('로그인에 실패했습니다. 다시 시도해 주세요.'); setTimeout(() => { navigate('/signin', { replace: true }); }, 2000); }
src/mocks/services/authServiceHandler.ts (2)
Line range hint
98-110
: Provider 설정이 체계적으로 구성되어 있습니다.각 소셜 로그인 provider별 설정이 잘 구조화되어 있으며, 필요한 모든 설정 값들이 포함되어 있습니다.
리팩토링 제안:
- 설정 객체를 별도의 상수 파일로 분리하여 관리하면 유지보수가 더욱 용이할 것 같습니다.
+ // src/constants/socialLoginConfig.ts + export const SOCIAL_LOGIN_CONFIG = { + kakao: { + tokenUrl: `https://kauth.kakao.com/oauth/token`, + // ... 나머지 설정 + }, + google: { + tokenUrl: `https://oauth2.googleapis.com/token`, + // ... 나머지 설정 + }, + };
168-168
: 이메일 추출 로직이 provider에 맞게 수정되었습니다.provider 값 비교 시 소문자 문자열을 사용하도록 수정된 것이 적절합니다.
추가 제안:
- 에러 발생 시 더 자세한 로그를 추가하면 디버깅에 도움이 될 것 같습니다.
- const email = provider === 'kakao' ? userInfo.kakao_account.email : userInfo.email; + const email = provider === 'kakao' ? userInfo.kakao_account?.email : userInfo.email; + console.log(`[Social Login] Provider: ${provider}, Email extraction result:`, { email, userInfo });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
src/components/user/auth-form/SocialButton.tsx
(1 hunks)src/mocks/services/authServiceHandler.ts
(3 hunks)src/pages/user/SignInPage.tsx
(3 hunks)src/routes/MainRouter.tsx
(1 hunks)src/types/UserType.tsx
(1 hunks)
🧰 Additional context used
📓 Learnings (1)
src/types/UserType.tsx (1)
Learnt from: Yoonyesol
PR: GU-99/grow-up-fe#191
File: src/types/UserType.tsx:3-9
Timestamp: 2024-11-10T03:54:59.892Z
Learning: provider별 분기가 필요한 부분에서는 'KAKAO'와 'GOOGLE' 문자열 리터럴을 직접 사용하며, 이를 'SocialLoginProvider' 타입으로 치환할 수 없다.
🔇 Additional comments (4)
src/types/UserType.tsx (1)
3-3
: 타입 정의 변경에 따른 영향도 검토 필요
SocialLoginProvider
타입의 리터럴 값이 소문자로 변경되었습니다. 이전 학습 내용에 따르면 대문자 리터럴이 필요했던 것으로 보이는데, 이 변경으로 인한 영향을 검토해야 합니다.
src/pages/user/SignInPage.tsx (1)
105-106
: 소셜 로그인 제공자 속성값 변경 확인
provider 속성값이 소문자로 변경되었습니다. 타입 정의 변경과 일관성 있게 적용되었습니다.
src/routes/MainRouter.tsx (1)
49-50
: 소셜 로그인 provider 값이 올바르게 수정되었습니다.
provider 값을 대문자에서 소문자로 변경한 것이 적절합니다. 이는 소셜 로그인 callback URL 오류를 해결하는데 도움이 될 것입니다.
다만, 다음 사항을 확인해주시기 바랍니다:
- 배포 환경에서 callback URL이 정상적으로 작동하는지 테스트
- 관련 환경 변수(VITE_KAKAO_REDIRECT_URI, VITE_GOOGLE_REDIRECT_URI)가 올바르게 설정되어 있는지 확인
src/mocks/services/authServiceHandler.ts (1)
91-91
: 유효한 provider 목록이 올바르게 수정되었습니다.
소문자로 된 provider 값들이 일관성 있게 정의되어 있습니다.
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.
수정 내용 확인했습니다. ai가 지적한대로 mockData쪽 더미데이터 내용도 수정해야할 것 같아요. 수정해서 다시 올리시면 approve 드릴게요.
@Seok93 mockData를 깜빡했네요;;ㅎ 해당 부분 수정했습니다! |
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.
고생하셨습니다!
PR Type
What kind of change does this PR introduce?
Related Issues
What does this PR do?
Other information
로그인 데이터
Other information
로컬에서는 두 기능 모두 잘 돌아가는 것을 확인했는데 배포 환경에서는 어떨지 잘 모르겠습니다🥲
테스트해보고 원인을 찾아서 수정하도록 하겠습니다!