-
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
Feat/#9 닉네임 중복 체크 기능 구현 #40
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,11 +44,12 @@ public String getAuthCodeRequestUrl(OAuthProvider oAuthProvider) { | |
public AuthTokens signup(SignupRequest request) { | ||
OAuthInfoResponse response = oauthMemberClientComposite.fetch(request.oAuthProvider(), | ||
request.authCode()); | ||
checkDuplicateUser(response); | ||
checkDuplicatedUser(response); | ||
String profileImage = checkProfileImage(response.profileImage()); | ||
String nickname = checkDuplicatedNickname(request.nickname()); | ||
|
||
User user = new User(new OAuthInfo(response.oAuthProvider(), response.oAuthProviderId()), | ||
Profile.builder().name(response.name()).nickname(request.nickname()).email(response.email()) | ||
Profile.builder().name(response.name()).nickname(nickname).email(response.email()) | ||
.profileImageUrl(profileImage).birth(response.birth()).build()); | ||
|
||
User newUser = userRepository.save(user); | ||
|
@@ -80,7 +81,7 @@ public void logout(Long userId) { | |
refreshTokenRepository.deleteById(userId); | ||
} | ||
|
||
private void checkDuplicateUser(OAuthInfoResponse response) { | ||
private void checkDuplicatedUser(OAuthInfoResponse response) { | ||
if (userRepository.existsUserByOauthInfo_oauthProviderAndOauthInfo_oauthProviderId( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이거 OAuthInfo 에 OAuthProvider oauthProvider, String oauthProviderId 이렇게 두 개만 있고 조회도 그 두 개로 해서 userRepository.existsUserByOauthInfo 까지맨 해도 될 것 같은데 안되나용? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 메서드명을 단순화하면 스프링 데이터 JPA가 어떤 필드를 기준으로 사용자를 조회할지 알 수 없어 해당 이름은 줄이기 어렵습니다 ! |
||
response.oAuthProvider(), response.oAuthProviderId())) { | ||
throw new IllegalArgumentException(ALREADY_REGISTERED_MESSAGE); | ||
|
@@ -94,6 +95,13 @@ private String checkProfileImage(String profileImage) { | |
return profileImage; | ||
} | ||
|
||
private String checkDuplicatedNickname(String nickname) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 굳이 nickname을 반환할 필요 있나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. checkDuplicatedUser 반환값이 void라 검증 메서드들을 void로 통일시키는게 좋을 것 같네요 ! 🙆🏻♀️🙆🏻♀️ |
||
if (userRepository.findUserByProfileNickname(nickname).isPresent()) { | ||
throw new IllegalArgumentException("중복된 닉네임입니다."); | ||
} | ||
return nickname; | ||
} | ||
|
||
Comment on lines
+98
to
+104
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 부분 UserService에 checkDuplicatedNickname 랑 중복인데 UserService를 필드로 갖고 쓰는거 어때요? |
||
private void generateInterests(SignupRequest request, User newUser) { | ||
List<Interest> interests = new ArrayList<>(); | ||
for (Keyword value : request.keywords()) { | ||
|
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.
이건 AuthService보다 UserService에 두는게 어때요?
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.
그러면 AuthService가 너무 많은 클래스에 의존하게 되어 AuthService에 해당 로직을 넣었는데, 그만큼 많은 책임을 가지고 있다는 의미같아서 말씀하신대로 해당 메서드는 UserService에 두고, 추후에 AuthService의 책임을 분리하려고 합니다 ! 🙆🏻♀️🫡