-
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
[FEAT] 로그인 응답에 유저 스탯, 아바타 정보 추가 #85
Conversation
- UserProgressionResult - 이름 변경: -> UserStat
### LoginRes - 유저 정보 추가(avatarId, userStat: 레벨, 경험치, 포인트 정보) ### AuthService - getSignedUser - authservice -> userservice 이전 - getSignedUser 메소드가 User를 반환하도록 수정 - 유저 기본 아바타 설정 메소드 UserAvatarService로 이전 ### UserService - disconnectApple 메소드 실행을 UserService에서 UserController로 이전: 메소드 책임 분리
- getSignedUser에 생성한 유저가 존재하는지 검사하는 로직과 user 저장 로직이 같이 있어서 안에 넣음. 책임 분리하는 쪽으로 리팩토링이 필요해보임.
authService.disconnectApple(appleUserDeleteReq.getAuthorizationCode()); | ||
userService.deleteById(userId); |
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.
p5 :순환참조때문에 컨트롤러로 옮겼군요!
두 메소드를 @transactional로 묶어놓았는데 옮기면서 트랜재션이 안될것 같아요
저번 리뷰에 달아주신 것처럼 후에 리팩토링하면서 (facade 적용) 수정되면 좋을 것 같아요!
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.
transactional을 생각 못했네요! 말씀하신대로 이후 리팩토링 하면 좋을것같습니다
@AllArgsConstructor | ||
public class UserProgressionResult { | ||
public class UserStat { |
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.
vo 이름이 저희 둘이 통일을 하면 좋을 것 같아요!
저는 ~~Vo로 사용하고 있어서요!
의견부탁드려요!
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.
넵 반영하겠습니다!
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.
아 ~~Vo 통일하자는 것이 아니라 어떤것으로 통일하면 좋을지 여쭈어본것이였습니다!
선호하시는 명명규칙이 궁금해서요!
@@ -52,16 +47,19 @@ public LoginRes socialLogin(SocialLoginReq socialLoginReq) { | |||
} | |||
|
|||
//저장된 유저 or 저장한유저의 id 가져오기 | |||
Integer userId = this.getSignedUserId(socialUserRes, socialLoginReq.getLoginType()); | |||
User user = userService.getSignedUser(socialUserRes, socialLoginReq.getLoginType()); |
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.
메소드 이전 좋은 것 같아요!
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.
고생하셨습니다~~
☀️ 작업 사항
☀️ 참고 사항
구현하는 과정에서 UserController, AuthService, UserService, UserAvatarService에 변경이 있었습니다. 각 서비스에 맞는 메소드를 작성하는 도중 순환참조 문제가 생겼고 기존의 코드를 수정하지 않으면 실행이 불가능하여 서비스의 책임에 맞게 메소드를 이전하거나 수정하였습니다. 로직에는 변경이 없고 메소드 분리와 이전만 적용하였습니다.
확인해보시고 이상이 있다면 가감없이 답글 달아주시면 감사하겠습니다!
그리고 커밋 메시지에 변경사항을 상세하게 적어두었으니 확인 부탁드립니다.