-
Notifications
You must be signed in to change notification settings - Fork 3
refactor : language response #182
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
WalkthroughUserInfoResponse의 language 타입을 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant C as UserController
participant S as UserService
participant D as UserRepository
participant M as LanguageResponse
rect rgba(200,235,255,0.25)
U->>C: GET /users/info
C->>S: getUserInfo()
S->>D: findById(...)
D-->>S: User(entity)
note over S,M: 도메인 Language → DTO 변환
S->>M: LanguageResponse.from(user.language)
M-->>S: LanguageResponse
S-->>C: UserInfoResponse(language=LanguageResponse, ...)
C-->>U: 200 OK (JSON)
end
sequenceDiagram
autonumber
actor U as User
participant C as UserController
participant S as UserService
participant D as UserRepository
participant M as LanguageResponse
rect rgba(220,255,220,0.25)
U->>C: PUT /users
C->>S: modifyUserInfo(request)
S->>D: save/update User
D-->>S: User(entity)
note over S,M: 도메인 Language → DTO 변환
S->>M: LanguageResponse.from(user.language)
M-->>S: LanguageResponse
S-->>C: UserInfoResponse(language=LanguageResponse, ...)
C-->>U: 200 OK (JSON)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 1
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/org/ezcode/codetest/application/usermanagement/user/service/UserService.java (1)
103-106: 신규 프로필 업로드 후 ‘새 이미지’를 삭제하는 역삭제 버그.oldImageUrl을 삭제해야 하는데, 변경 후의 user.getProfileImageUrl()을 삭제하고 있습니다.
적용 diff:
- user.modifyProfileImage(profileImageUrl); - if (oldImageUrl!=null) { - s3Uploader.delete(user.getProfileImageUrl(), "profile"); - } + user.modifyProfileImage(profileImageUrl); + if (oldImageUrl != null) { + s3Uploader.delete(oldImageUrl, "profile"); + }
🧹 Nitpick comments (3)
src/main/java/org/ezcode/codetest/presentation/usermanagement/UserController.java (1)
48-56: consumes 변경에 따른 호환성 점검 필요.이 변경으로 해당 엔드포인트는 multipart/form-data만 수용합니다. 기존에 application/json으로만 요청하던 클라이언트가 있다면 415가 발생할 수 있습니다. 영향 범위 확인 부탁드립니다. 필요 시 JSON 전용 오버로드를 추가하는 방안을 고려하세요.
예시:
@PutMapping(path = "/users", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) public ResponseEntity<UserInfoResponse> modifyUserInfo(...) +@PutMapping(path = "/users", consumes = MediaType.APPLICATION_JSON_VALUE) +public ResponseEntity<UserInfoResponse> modifyUserInfoJson( + @AuthenticationPrincipal AuthUser authUser, + @Valid @RequestBody ModifyUserInfoRequest request +){ + return ResponseEntity.status(HttpStatus.OK).body(userService.modifyUserInfo(authUser, request, null)); +}src/main/java/org/ezcode/codetest/application/usermanagement/user/service/UserService.java (2)
81-83: null-safe 매핑 보강 제안.user.getLanguage()가 null일 수 있다면 NPE 위험이 있습니다. from(...)에서 null을 허용하지 않는다면 방어 로직을 추가하세요.
적용 diff:
- .language(LanguageResponse.from(user.getLanguage())) + .language(user.getLanguage() != null ? LanguageResponse.from(user.getLanguage()) : null)두 위치 동일 적용.
Also applies to: 121-123
89-89: 오타: 변수명 findLangauge → findLanguage.가독성과 일관성을 위해 수정 권장.
적용 diff:
- Language findLangauge = languageDomainService.getLanguage(request.languageId()); + Language findLanguage = languageDomainService.getLanguage(request.languageId()); ... - findLangauge); + findLanguage);Also applies to: 97-97
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/main/java/org/ezcode/codetest/application/usermanagement/user/dto/response/UserInfoResponse.java(3 hunks)src/main/java/org/ezcode/codetest/application/usermanagement/user/service/UserService.java(3 hunks)src/main/java/org/ezcode/codetest/presentation/usermanagement/UserController.java(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
src/main/java/org/ezcode/codetest/presentation/usermanagement/UserController.java
[error] 49-49: cannot find symbol: variable MediaType in @PutMapping(consumes = MediaType.MULTIPART_FORM_DATA_VALUE); ensure org.springframework.http.MediaType is imported.
🔇 Additional comments (3)
src/main/java/org/ezcode/codetest/application/usermanagement/user/service/UserService.java (1)
13-13: LanguageResponse import 추가 적절.응답 계층 의존으로 정리된 방향성 좋습니다.
src/main/java/org/ezcode/codetest/application/usermanagement/user/dto/response/UserInfoResponse.java (2)
5-5: LanguageResponse로의 의존 전환 적절.DTO에서 도메인 엔티티 분리를 명확히 했습니다.
54-56: 공용 API 스키마 변경에 따른 소비자 영향 점검.language 필드 타입 변경은 응답 스키마를 바꿉니다. 프론트/외부 소비자 호환성 및 문서 업데이트 확인 바랍니다.
Also applies to: 63-65
작업 내용
변경 사항
UserService.createUser()메서드 추가@Email유효성 검증 적용트러블 슈팅
@Transactional이 적용되지 않음this.→AopProxyUtils.사용)해결해야 할 문제
UserController에서 비즈니스 로직 일부 처리 → 서비스로 이전 고려 필요참고 사항
코드 리뷰 전 확인 체크리스트
type :)Summary by CodeRabbit