-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/#75 유저 프로필 사진, 학적 정보 변경 기능 구현 #76
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
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
Walkthrough사용자 프로필 이미지 및 학적 정보 변경 기능을 추가합니다. 요청/응답 DTO, 서비스 메서드, 컨트롤러 엔드포인트, 매퍼 매핑, 엔티티 필드(마지막 업데이트 시간) 및 관련 예외/에러코드가 도입되며 학적 정보는 6개월 제한이 적용됩니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant UC as UserController
participant US as UserService
participant UR as UserRepository
participant DB as Database
Client->>UC: PATCH /users/change/profile/image\n(ChangeProfileImageRequest)
UC->>US: updateProfileImage(userId, request)
US->>UR: findById(userId)
UR->>DB: SELECT user
DB-->>UR: User
UR-->>US: User
US->>US: user.updateProfileImage(newProfileImage)\nuser.setLastProfileUpdatedAt(now)
US->>UR: save(user)
UR->>DB: UPDATE user
DB-->>UR: OK
US-->>UC: ChangeProfileImageResponse
UC-->>Client: CommonResponse<ChangeProfileImageResponse>
sequenceDiagram
actor Client
participant UC as UserController
participant US as UserService
participant UR as UserRepository
participant SR as SchoolRepository
participant MR as MajorRepository
participant DB as Database
Client->>UC: PATCH /users/profile/academic\n(ChangeUserAcademicRequest)
UC->>US: updateUserAcademic(userId, request)
US->>UR: findById(userId)
UR->>DB: SELECT user
DB-->>UR: User
UR-->>US: User
alt lastProfileUpdatedAt within 6 months
US-->>UC: AcademicInfoUpdateRestrictionException
UC-->>Client: Error Response (ACADEMIC_INFO_CANNOT_CHANGE)
else
US->>SR: findById(schoolId)
SR->>DB: SELECT school
DB-->>SR: School
SR-->>US: School
US->>MR: findById(majorId)
MR->>DB: SELECT major
DB-->>MR: Major
MR-->>US: Major
US->>US: validate major belongs to school
US->>US: user.updateAcademicInfo(school, major)\nuser.setLastProfileUpdatedAt(now)
US->>UR: save(user)
UR->>DB: UPDATE user
DB-->>UR: OK
US-->>UC: ChangeUserAcademicResponse
UC-->>Client: CommonResponse<ChangeUserAcademicResponse>
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 3
🤖 Fix all issues with AI agents
In
`@src/main/java/com/campus/campus/domain/user/application/exception/AcademicInfoUpdateRestrictionException.java`:
- Around line 1-9: The file AcademicInfoUpdateRestrictionException references
ErrorCode but lacks its import; add the import for ErrorCode (matching other
exceptions) at the top of the file so the constructor call
super(ErrorCode.ACADEMIC_INFO_CANNOT_CHANGE) resolves correctly; locate the
class AcademicInfoUpdateRestrictionException and add the same ErrorCode import
statement used by other exception classes.
In
`@src/main/java/com/campus/campus/domain/user/application/mapper/UserMapper.java`:
- Around line 51-60: In toChangeUserAcademicResponse replace direct use of
user.getCampusNickname() with the same null-fallback used in other mappers (use
campusNickname if not null, otherwise use user.getNickname()), and change the
college name lookup from user.getMajor().getCollege().getCollegeName() to
user.getCollege().getCollegeName() so the method follows the same null-handling
and more direct college access as other mapper methods.
In
`@src/main/java/com/campus/campus/domain/user/presentation/UserController.java`:
- Around line 50-57: The `@PatchMapping` path in UserController.updateProfileImage
uses a trailing slash ("/change/profile-image/") which should be removed for
consistency and to avoid redirect/routing issues; change the mapping to
"/change/profile-image" in the annotation on the updateProfileImage method so
the route matches other endpoints (e.g., /profile, /change/nickname) and update
any related API docs/tests if they relied on the trailing slash.
🧹 Nitpick comments (3)
src/main/java/com/campus/campus/domain/user/application/dto/request/ChangeProfileImageRequest.java (1)
5-9:newProfileImage필드에 유효성 검사 어노테이션 추가를 권장합니다.
ChangeUserAcademicRequest에서는@NotNull을 사용하고 있지만, 이 요청 DTO에는 유효성 검사 어노테이션이 누락되어 있습니다. 빈 문자열이나 null 값이 전달되는 것을 방지하기 위해@NotBlank어노테이션 추가를 고려해 주세요.♻️ 제안된 수정
package com.campus.campus.domain.user.application.dto.request; import io.swagger.v3.oas.annotations.media.Schema; +import jakarta.validation.constraints.NotBlank; public record ChangeProfileImageRequest( `@Schema`(description = "새로운 사용자 프로필 이미지", example = "http://image/img_640x640.jpg") + `@NotBlank` String newProfileImage ) { }src/main/java/com/campus/campus/domain/user/application/service/UserService.java (2)
104-113: 학교/전공 검증 로직 중복이 검증 로직이
writeUserProfile(lines 52-61)과 동일합니다. private 헬퍼 메서드로 추출하면 유지보수성이 향상됩니다.♻️ 리팩토링 제안
private ValidatedAcademicInfo validateAndGetAcademicInfo(Long schoolId, Long majorId) { School school = schoolRepository.findById(schoolId) .orElseThrow(SchoolNotFoundException::new); Major major = majorRepository.findById(majorId) .orElseThrow(MajorNotFoundException::new); if (!major.getSchool().getSchoolId().equals(school.getSchoolId())) { throw new SchoolMajorNotSameException(); } return new ValidatedAcademicInfo(school, major.getCollege(), major); } private record ValidatedAcademicInfo(School school, College college, Major major) {}
97-102:LocalDateTime.now()직접 호출로 인한 테스트 어려움시간 기반 로직을 테스트하기 위해
Clock의존성 주입을 고려해 보세요. 현재 구현도 동작하지만, 단위 테스트 작성 시 시간을 모킹하기 어려울 수 있습니다.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/com/campus/campus/domain/user/application/dto/request/ChangeProfileImageRequest.javasrc/main/java/com/campus/campus/domain/user/application/dto/request/ChangeUserAcademicRequest.javasrc/main/java/com/campus/campus/domain/user/application/dto/response/ChangeProfileImageResponse.javasrc/main/java/com/campus/campus/domain/user/application/dto/response/ChangeUserAcademicResponse.javasrc/main/java/com/campus/campus/domain/user/application/exception/AcademicInfoUpdateRestrictionException.javasrc/main/java/com/campus/campus/domain/user/application/exception/ErrorCode.javasrc/main/java/com/campus/campus/domain/user/application/mapper/UserMapper.javasrc/main/java/com/campus/campus/domain/user/application/service/UserService.javasrc/main/java/com/campus/campus/domain/user/domain/entity/User.javasrc/main/java/com/campus/campus/domain/user/presentation/UserController.javasrc/main/java/com/campus/campus/domain/user/presentation/UserResponseCode.java
🧰 Additional context used
🧬 Code graph analysis (5)
src/main/java/com/campus/campus/domain/user/application/exception/AcademicInfoUpdateRestrictionException.java (7)
src/main/java/com/campus/campus/domain/mail/application/exception/InvalidSchoolEmailException.java (1)
InvalidSchoolEmailException(5-9)src/main/java/com/campus/campus/domain/school/application/exception/SchoolMajorNotSameException.java (1)
SchoolMajorNotSameException(5-9)src/main/java/com/campus/campus/domain/council/application/exception/NewPasswordIsCurrentPasswordException.java (1)
NewPasswordIsCurrentPasswordException(5-9)src/main/java/com/campus/campus/domain/user/application/exception/NicknameAlreadyExistsException.java (1)
NicknameAlreadyExistsException(5-9)src/main/java/com/campus/campus/domain/council/application/exception/CouncilSignupForbiddenException.java (1)
CouncilSignupForbiddenException(5-9)src/main/java/com/campus/campus/domain/school/application/exception/SchoolCollegeNotSameException.java (1)
SchoolCollegeNotSameException(6-8)src/main/java/com/campus/campus/domain/council/application/exception/PrecautionNotAgreeException.java (1)
PrecautionNotAgreeException(5-9)
src/main/java/com/campus/campus/domain/user/application/dto/response/ChangeUserAcademicResponse.java (2)
src/main/java/com/campus/campus/domain/user/application/dto/response/UserInfoResponse.java (1)
UserInfoResponse(5-21)src/main/java/com/campus/campus/domain/user/application/dto/response/UserFirstProfileResponse.java (1)
Builder(6-17)
src/main/java/com/campus/campus/domain/user/domain/entity/User.java (1)
src/main/java/com/campus/campus/global/util/jwt/UserPrincipal.java (1)
Getter(14-77)
src/main/java/com/campus/campus/domain/user/presentation/UserResponseCode.java (7)
src/main/java/com/campus/campus/domain/councilpost/presentation/StudentCouncilPostResponseCode.java (1)
Getter(10-27)src/main/java/com/campus/campus/domain/councilnotice/presentation/NoticeResponseCode.java (1)
Getter(10-23)src/main/java/com/campus/campus/domain/mail/presentation/EmailVerificationResponseCode.java (1)
Getter(10-19)src/main/java/com/campus/campus/domain/stamp/presentation/StampResponseCode.java (1)
Getter(10-19)src/main/java/com/campus/campus/domain/manager/presentation/ManagerResponseCode.java (1)
Getter(10-23)src/main/java/com/campus/campus/global/oci/presentation/PresignedUrlResponseCode.java (1)
Getter(10-19)src/main/java/com/campus/campus/domain/council/presentation/StudentCouncilResponseCode.java (1)
Getter(10-27)
src/main/java/com/campus/campus/domain/user/application/service/UserService.java (4)
src/main/java/com/campus/campus/domain/user/application/exception/AcademicInfoUpdateRestrictionException.java (1)
AcademicInfoUpdateRestrictionException(5-9)src/main/java/com/campus/campus/domain/user/application/exception/UserNotFoundException.java (1)
UserNotFoundException(5-9)src/main/java/com/campus/campus/domain/school/application/exception/SchoolNotFoundException.java (1)
SchoolNotFoundException(5-9)src/main/java/com/campus/campus/domain/school/application/exception/MajorNotFoundException.java (1)
MajorNotFoundException(5-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (11)
src/main/java/com/campus/campus/domain/user/application/dto/request/ChangeUserAcademicRequest.java (1)
6-15: LGTM!DTO 구조가 올바르게 정의되었습니다.
@NotNull유효성 검사와@SchemaAPI 문서화 어노테이션이 적절히 적용되어 있습니다.src/main/java/com/campus/campus/domain/user/application/dto/response/ChangeUserAcademicResponse.java (1)
7-26: LGTM!기존
UserInfoResponse와 일관된 구조를 유지하면서nextUpdateAvailableDate필드를 추가하여 6개월 변경 제한 기능을 지원합니다.LocalDateTime타입 사용이 적절합니다.src/main/java/com/campus/campus/domain/user/application/dto/response/ChangeProfileImageResponse.java (1)
5-15: LGTM!기존 응답 DTO 패턴과 일관되게 구현되었습니다.
@Schema어노테이션을 통한 API 문서화가 적절합니다.src/main/java/com/campus/campus/domain/user/domain/entity/User.java (2)
72-79: LGTM! 학적 정보 변경 추적을 위한 필드 추가가 적절합니다.
lastProfileUpdatedAt필드가 추가되었고,updateProfile메서드에서 업데이트 시 현재 시간으로 설정됩니다. 6개월 제한 검증은 서비스 레이어에서 처리되어야 하며, 엔티티는 상태 변경만 담당하는 것이 적절합니다.
97-100: LGTM! 프로필 이미지 업데이트 메서드가 올바르게 구현되었습니다.src/main/java/com/campus/campus/domain/user/application/mapper/UserMapper.java (1)
43-49: LGTM! 프로필 이미지 응답 매퍼 메서드가 올바르게 구현되었습니다.src/main/java/com/campus/campus/domain/user/presentation/UserResponseCode.java (1)
17-19: LGTM! 새로운 응답 코드가 기존 패턴과 일관성 있게 추가되었습니다.src/main/java/com/campus/campus/domain/user/application/exception/ErrorCode.java (1)
17-18: LGTM! 학적 정보 변경 제한 에러 코드가 올바르게 추가되었습니다.에러 코드 번호가 순차적으로 부여되었고,
BAD_REQUEST상태가 비즈니스 규칙 위반에 적절합니다.src/main/java/com/campus/campus/domain/user/presentation/UserController.java (1)
59-66: LGTM! 학적 정보 변경 엔드포인트가 올바르게 구현되었습니다.6개월 제한에 대한 Swagger 문서화가 적절히 포함되어 있습니다.
src/main/java/com/campus/campus/domain/user/application/service/UserService.java (2)
81-90: LGTM!프로필 이미지 업데이트 로직이 간결하고 명확합니다. 사용자 조회, 이미지 업데이트, 저장, 응답 반환의 흐름이 기존 패턴과 일관성 있게 구현되어 있습니다.
113-115: 이 지적은 부정확합니다.lastProfileUpdatedAt은 올바르게 갱신됩니다.
User.updateProfile()메서드(User.java 79줄)에서 이미this.lastProfileUpdatedAt = LocalDateTime.now();로 타임스탬프를 설정하고 있습니다. 따라서updateUserAcademic()에서 이 메서드를 호출할 때 타임스탬프가 자동으로 갱신되므로, 제안된 추가 코드는 불필요합니다. 6개월 제한은 정상 작동합니다.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| public ChangeUserAcademicResponse toChangeUserAcademicResponse(User user) { | ||
| return new ChangeUserAcademicResponse( | ||
| user.getId(), | ||
| user.getCampusNickname(), | ||
| user.getSchool().getSchoolName(), | ||
| user.getMajor().getCollege().getCollegeName(), | ||
| user.getMajor().getMajorName(), | ||
| user.getLastProfileUpdatedAt() | ||
| ); | ||
| } |
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.
campusNickname null 처리가 다른 매퍼 메서드들과 일관성이 없습니다.
toUserInfoResponse(Line 36)와 toChangeProfileImageResponse(Line 46)에서는 campusNickname이 null일 경우 nickname으로 대체하는 로직이 있지만, toChangeUserAcademicResponse에서는 campusNickname을 직접 사용합니다. 동일한 패턴을 적용해야 합니다.
또한, Line 56에서 user.getMajor().getCollege().getCollegeName() 대신 user.getCollege().getCollegeName()을 사용하는 것이 더 직관적이고 일관성 있습니다.
🔧 수정 제안
public ChangeUserAcademicResponse toChangeUserAcademicResponse(User user) {
return new ChangeUserAcademicResponse(
user.getId(),
- user.getCampusNickname(),
+ user.getCampusNickname() == null ? user.getNickname() : user.getCampusNickname(),
user.getSchool().getSchoolName(),
- user.getMajor().getCollege().getCollegeName(),
+ user.getCollege().getCollegeName(),
user.getMajor().getMajorName(),
user.getLastProfileUpdatedAt()
);
}🤖 Prompt for AI Agents
In
`@src/main/java/com/campus/campus/domain/user/application/mapper/UserMapper.java`
around lines 51 - 60, In toChangeUserAcademicResponse replace direct use of
user.getCampusNickname() with the same null-fallback used in other mappers (use
campusNickname if not null, otherwise use user.getNickname()), and change the
college name lookup from user.getMajor().getCollege().getCollegeName() to
user.getCollege().getCollegeName() so the method follows the same null-handling
and more direct college access as other mapper methods.
| @PatchMapping("/change/profile-image/") | ||
| @Operation(summary = "사용자 프로필 이미지 변경") | ||
| public CommonResponse<ChangeProfileImageResponse> updateProfileImage(@CurrentUserId Long userId, | ||
| @RequestBody @Valid ChangeProfileImageRequest changeProfileImageRequest) { | ||
| ChangeProfileImageResponse response = userService.updateProfileImage(userId, changeProfileImageRequest); | ||
|
|
||
| return CommonResponse.success(UserResponseCode.PROFILE_IMAGE_UPDATE_SUCCESS, response); | ||
| } |
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.
URL 경로에서 trailing slash를 제거해야 합니다.
/change/profile-image/ 경로에 trailing slash가 있어 다른 엔드포인트(/profile, /change/nickname)와 일관성이 없습니다. 서버 설정에 따라 301 리다이렉트가 발생하거나 라우팅 문제가 생길 수 있습니다.
🔧 수정 제안
- `@PatchMapping`("/change/profile-image/")
+ `@PatchMapping`("/change/profile-image")
`@Operation`(summary = "사용자 프로필 이미지 변경")🤖 Prompt for AI Agents
In `@src/main/java/com/campus/campus/domain/user/presentation/UserController.java`
around lines 50 - 57, The `@PatchMapping` path in
UserController.updateProfileImage uses a trailing slash
("/change/profile-image/") which should be removed for consistency and to avoid
redirect/routing issues; change the mapping to "/change/profile-image" in the
annotation on the updateProfileImage method so the route matches other endpoints
(e.g., /profile, /change/nickname) and update any related API docs/tests if they
relied on the trailing slash.
🔀 변경 내용
✅ 작업 항목
📸 스크린샷 (선택)
유저 프로필 사진 변경
유저 학저 정보 변경
📎 참고 이슈
관련 이슈 번호 #75
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.