-
Notifications
You must be signed in to change notification settings - Fork 0
[BUG] 유저 닉네임 변경 조건 강화 및 isPublic 필드 제거 #234
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
Conversation
📝 WalkthroughWalkthrough사용자 정보 업데이트 API에서 사용하지 않는 Changes
Sequence Diagram(s)(생략 — 변경이 DTO 제거 및 단일 서비스 흐름 검증 강화 중심이므로 시퀀스 다이어그램을 생성하지 않습니다.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (1)
365-376: PR 목표와 구현 로직이 불일치합니다.PR 설명에서는 "
isNicknameChanged가 true이고 닉네임이 null인 경우INVALID_INPUT_VALUE예외를 발생시켜 데이터 무결성을 보장"한다고 명시했으나, 현재 코드는 해당 상황을 조용히 무시합니다.클라이언트가 명시적으로 닉네임 변경 의사(
isNicknameChanged=true)를 밝혔음에도 null 값을 전달하는 것은 잘못된 요청이므로, 예외를 발생시켜 명확히 알려주는 것이 데이터 무결성 측면에서 더 안전합니다.🔧 PR 목표에 맞춘 수정 제안
- // isNicknameChanged가 true이고 nickname이 null이 아닐 때만 처리 - if (Boolean.TRUE.equals(requestDto.getIsNicknameChanged()) && requestDto.getNickname() != null) { + // isNicknameChanged가 true일 때 닉네임 검증 및 업데이트 + if (Boolean.TRUE.equals(requestDto.getIsNicknameChanged())) { + // 닉네임 변경 의사를 밝혔는데 null이면 예외 발생 + if (requestDto.getNickname() == null) { + throw new GlobalException(ErrorCode.INVALID_INPUT_VALUE); + } + String nickname = normalize(requestDto.getNickname()); // 자기 닉네임과 다를 때만 중복 체크 및 업데이트
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/hrr/backend/domain/user/dto/UpdateUserInfoRequestDto.javasrc/main/java/com/hrr/backend/domain/user/dto/UpdateUserInfoResponseDto.javasrc/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java
💤 Files with no reviewable changes (2)
- src/main/java/com/hrr/backend/domain/user/dto/UpdateUserInfoResponseDto.java
- src/main/java/com/hrr/backend/domain/user/dto/UpdateUserInfoRequestDto.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-06T08:06:38.145Z
Learnt from: yerinchun
Repo: Hrr-HabbitRoutineResult/hrr-server PR: 26
File: src/main/java/com/hrr/backend/domain/user/entity/User.java:42-44
Timestamp: 2025-11-06T08:06:38.145Z
Learning: In the hrr-server project User entity (src/main/java/com/hrr/backend/domain/user/entity/User.java), the password field is nullable because the application uses social login (OAuth), and social login users do not have passwords stored in the database.
Applied to files:
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java
🔇 Additional comments (2)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (2)
370-375: 중복 체크 최적화가 잘 되어 있습니다! 👍자신의 현재 닉네임과 동일한 경우 불필요한 DB 중복 체크를 스킵하는 로직이 효율적입니다.
367-367: DTO 레벨의 닉네임 검증이 이미 적절하게 구현되어 있습니다.
UpdateUserInfoRequestDto에@Size(min = 1, max = 20)애노테이션이 설정되어 있으므로, 빈 문자열이나 공백만 있는 닉네임은 DTO 검증 단계에서 이미 차단됩니다. 이후normalize()함수에서 trim을 수행하는 것은 공백 처리를 위한 적절한 설계입니다. 현재 코드는 검증-정규화의 책임 분리가 명확하고 안전한 상태이므로, 추가 조치는 불필요합니다. 👍Likely an incorrect or invalid review comment.
yc3697
left a comment
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.
isNicknameChanged=true인데 nickname이 null인 경우에 명시적으로 예외를 던져줘야 할 것 같아요
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: 0
🧹 Nitpick comments (1)
src/main/java/com/hrr/backend/global/response/ErrorCode.java (1)
15-15: 좋습니다! 범용 입력 검증 에러 코드 추가 👍
INVALID_INPUT_VALUE에러 코드가 닉네임 검증 강화 요구사항에 잘 부합합니다. 네이밍도 명확하고 BAD_REQUEST 상태도 적절합니다.다만 에러 코드 번호가
COMMON400에서COMMON600으로 건너뛰는데, 이것이 의도적인 번호 체계인지 확인해 보시면 좋겠습니다. 일반적으로는 순차적 증가(COMMON401, 402...)를 사용하거나, 카테고리별 번호 범위를 문서화하는 것이 유지보수에 도움이 됩니다.📚 참고: 에러 코드 번호 체계 예시
만약 카테고리별로 번호 범위를 나누는 전략이라면, 팀 내 문서화를 권장합니다:
- COMMON400번대: 요청 관련 에러
- COMMON600번대: 입력값 검증 에러
- 기타...
또는 순차적 증가 방식:
-INVALID_INPUT_VALUE(HttpStatus.BAD_REQUEST, "COMMON600", "잘못된 입력값입니다."), +INVALID_INPUT_VALUE(HttpStatus.BAD_REQUEST, "COMMON401", "잘못된 입력값입니다."),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.javasrc/main/java/com/hrr/backend/global/response/ErrorCode.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (1)
365-384: 닉네임 변경 로직의 공백/빈 문자열 검증 보완 필요null 체크는 추가되었으나, 정규화 이후 공백 또는 빈 문자열에 대한 명시적 검증이 누락되어 있습니다.
근본 원인:
UpdateUserInfoRequestDto의@Size(min=1, max=20)는 정규화 이전의 문자열 길이만 검증- 클라이언트가
" "(공백만 3자)를 전송하면@Size검증을 통과- 이후
normalize(rawNickname)가 이를""(빈 문자열)로 변환- 변환된 값에 대한 재검증 없이 DB에 저장됨
문제:
- 사용자 닉네임이 빈 문자열로 저장될 수 있음
setNickname()메서드는@NotBlank로 더 강한 검증을 수행하는데,updateUserInfo()는 약한 검증만 함 (일관성 부족)권장 수정:
공백 문자열을 미리 차단하려면 정규화 이후에 검증하세요:// 변경 플래그는 true인데 값이 null인 경우 if (rawNickname == null || rawNickname.isBlank()) { throw new GlobalException(ErrorCode.INVALID_INPUT_VALUE); } // 값이 있는 경우 업데이트 진행 String nickname = normalize(rawNickname);이렇게 수정하면
setNickname()메서드의 검증 수준과 일치시킬 수 있습니다.
|
|
||
| // 자기 닉네임과 다를 때만 중복 체크 및 업데이트 | ||
| // 변경 플래그는 true인데 값이 null인 경우 | ||
| if (rawNickname == null) { |
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.
코드래빗 리뷰처럼 isBlank 조건 추가해서 빈 문자열 필터링이 필요할 것 같아요!
yc3697
left a comment
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.
확인했습니다 수고하셨어요!
#️⃣ 연관된 이슈
✨ 작업 내용 (Summary)
✅ 변경 사항 체크리스트
🧪 테스트 결과
📸 스크린샷
isNicknameChanged = true && nickname = null

💬 리뷰 요구사항
📎 참고 자료
Summary by CodeRabbit
릴리스 노트
Bug Fixes
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.