Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| .email(null) | ||
| .nickname(kakaoAccount.getProfile().getNickname()) | ||
| .profileImage(kakaoAccount.getProfile().getProfileImageUrl()) | ||
| .build(); |
There was a problem hiding this comment.
이 부분의 코드가 삭제되었습니다. Kakao 로그인 응답에서 이메일, 닉네임, 프로필 이미지를 더 이상 가져오지 않도록 변경된 것 같은데, 그 이유가 무엇인가요? 만약 의도적인 변경이라면, 삭제된 코드 부분에 대한 주석을 추가하여 변경 사유를 명확히 설명해주세요. 혹시 다른 곳에서 이 정보를 가져오도록 변경되었는지 확인해야 합니다.
| .email(null) | ||
| .nickname(response.getName()) | ||
| .profileImage(null) | ||
| .build(); |
There was a problem hiding this comment.
KakaoUserInfoResponse와 마찬가지로 Naver 로그인 응답에서도 이메일, 닉네임, 프로필 이미지를 가져오는 코드가 삭제되었습니다. 변경 사유를 명확히 설명하는 주석을 추가해주세요. 그리고 다른 곳에서 이 정보를 가져오도록 변경되었는지 확인해야 합니다.
| private String email; | ||
| private String nickname; | ||
| private String profileImage; | ||
| private String socialId; |
There was a problem hiding this comment.
OAuthUserInfoResponse에 socialType 필드가 추가되었습니다. 이는 좋은 변경입니다. 기존의 email, nickname, profileImage 필드가 제거된 것과 관련하여, 이 필드를 통해 소셜 로그인 타입을 구분하고, 필요한 정보를 다른 곳에서 가져오는 방식으로 변경한 것으로 예상됩니다. 하지만 socialId 필드에 대한 주석이 삭제된 것은 좋지 않습니다. socialId 필드에 대한 설명을 다시 추가해주세요. 그리고 전체적인 설계 변경에 대한 자세한 설명이 필요합니다.
| boolean needAdditionalInfo = (user.getStatus() == UserStatus.UNREGISTERED); | ||
|
|
||
| return new AuthResponse(accessToken, refreshToken, needAdditionalInfo, user.getEmail()); | ||
| return new AuthResponse( |
There was a problem hiding this comment.
AuthResponse 생성자에 user.getEmail() 대신 null을 전달하도록 변경되었습니다. 이는 이메일 정보를 더 이상 AuthResponse에 포함하지 않겠다는 의미로 해석됩니다. KakaoUserInfoResponse 와 NaverUserInfoResponse에서 이메일 정보를 가져오지 않는 것과 일관성을 유지하기 위한 변경으로 보입니다. 하지만 이 변경으로 인해 다른 부분에서 문제가 발생할 가능성이 있습니다. 이 변경의 영향을 철저히 분석하고, 관련된 모든 부분을 검토해야 합니다.
| @@ -1,7 +1,6 @@ | |||
| package com.frend.planit.domain.user.dto.request; | |||
|
|
|||
| import com.frend.planit.domain.user.enums.Gender; | |||
There was a problem hiding this comment.
UserFirstInfoRequest에서 이메일 관련 필드와 @NotBlank 어노테이션이 제거되고, mailingType 필드가 추가되었습니다. 이메일이 필수 정보가 아니게 된 것으로 보이며, 이메일 수집 동의 여부를 추가한 것 같습니다. 이 변경은 이메일을 OAuth 로그인 시점이 아닌, 추후 사용자 정보 입력 시점에서 받도록 설계를 변경한 것을 반영하는 것으로 보입니다. 하지만 이메일 수집 동의가 필수인지 선택인지 명확히 해야 합니다. mailingType 필드에 대한 설명을 추가해주세요. 또한 이메일 정보 수집과 관련된 다른 부분들에 대한 검토가 필요합니다.
|
|
||
| public void updateFirstInfo(String email, String nickname, String phone, LocalDate birthDate, | ||
| Gender gender) { | ||
| Gender gender, Boolean mailingType) { |
There was a problem hiding this comment.
User 엔티티에 mailingType 필드가 추가되었습니다. UserFirstInfoRequest의 변경과 일관성을 유지하는 부분입니다. mailingType 필드의 데이터 타입과 null 허용 여부를 확인해야 합니다. Boolean 타입으로 설정된 것은 적절해 보입니다. 데이터베이스 스키마 변경이 필요하며, 추가된 필드에 대한 데이터베이스 컬럼의 설정 또한 검토해야 합니다.
| .socialType(socialType) | ||
| .email(userInfo.getEmail()) | ||
| .profileImageUrl(userInfo.getProfileImage()) | ||
| .email(null) |
There was a problem hiding this comment.
UserMapper에서 이메일과 프로필 이미지 매핑 부분이 null로 변경되었습니다. 이는 OAuthUserInfoResponse에서 이메일과 프로필 이미지를 가져오지 않도록 변경된 것과 연관되어 있습니다. 이 변경으로 인해, 사용자 정보가 불완전하게 저장될 가능성이 있습니다. 이에 대한 충분한 검토와 대안이 필요합니다. 만약 다른 곳에서 사용자 정보를 보완하여 저장하는 로직이 있다면, 그 부분을 명확히 해주어야 합니다.
| request.getPhone(), | ||
| request.getBirthDate(), | ||
| request.getGender() | ||
| request.getGender(), |
There was a problem hiding this comment.
UserService의 saveUser 메서드에 request.getMailingType()이 추가되었습니다. 이것은 UserFirstInfoRequest와 User 엔티티 변경과 일관성을 유지하기 위한 변경입니다. 하지만 mailingType이 null일 경우의 예외처리에 대한 고려가 필요합니다. null 값 처리 로직을 추가해야 합니다.
| "01012345678", | ||
| LocalDate.of(1998, 1, 1), | ||
| Gender.MALE | ||
| Gender.MALE, |
There was a problem hiding this comment.
UserControllerTest에서도 mailingType에 대한 값이 추가되었습니다. 테스트 케이스가 변경 사항을 반영하도록 수정된 것입니다. 테스트 코드가 변경된 로직을 완벽히 커버하고 있는지 확인해야 합니다. 다른 테스트 케이스에서 mailingType에 대한 다양한 시나리오(true, false, null)를 테스트해야 합니다.
1 participant
🔘 Part
🔎 PR Type
🔧 작업 내용 상세 작성
Bug/UB-03 #178
최초정보 입력시 회원가입 안되던 오류 해결
✔️ PR Checklist
커밋 메세지를 컨벤션에 맞게 잘 적용 하였나요?
테스트 코드 작성 / 단위 테스트 or 통합 테스트 진행 하셨나요?