Conversation
|
Caution Review failedThe pull request is closed. 📝 Walkthrough워크스루사용자 정보 조회, 전화번호 변경, 징계 내역 조회 API를 구현합니다. 관련 서비스, 저장소, DTO를 추가하고, 기존 DTO들을 요청/응답 패키지로 재구성합니다. 변경사항
예상 코드 리뷰 노력🎯 3 (보통) | ⏱️ ~25 분 관련 PR
제안 라벨
제안 검토자
축하 시
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/main/java/ssurent/ssurentbe/domain/users/controller/UserController.java`:
- Around line 26-35: The updatePhoneNumber controller method is missing
validation and a proper PATCH response: add javax.validation.Valid to the
`@RequestBody` parameter (i.e. change the parameter to `@RequestBody` `@Valid`
UpdatePhoneNumberRequest request) so UpdatePhoneNumberRequest constraints are
enforced, and return 204 No Content by either annotating the method with
`@ResponseStatus`(HttpStatus.NO_CONTENT) or changing the signature to return
ResponseEntity<Void> and returning ResponseEntity.noContent().build(); update
the UserController.updatePhoneNumber method accordingly.
In
`@src/main/java/ssurent/ssurentbe/domain/users/dto/UpdatePhoneNumberRequest.java`:
- Around line 3-6: UpdatePhoneNumberRequest currently exposes phoneNum without
validation; add Bean Validation annotations to the record component (e.g.,
annotate phoneNum with `@NotBlank` and a suitable `@Pattern` regex for your phone
format or a more permissive pattern) so controllers using `@Valid` will reject
empty or malformed numbers; update imports to use javax.validation.constraints.*
(or jakarta.validation.constraints.* depending on project) and ensure the
pattern matches your expected formats (international/local) and error messages
are set if needed.
In
`@src/main/java/ssurent/ssurentbe/domain/users/repository/UserPanaltyLogRepository.java`:
- Around line 10-12: The repository and related types use the misspelled
"Panalty" instead of "Penalty" (e.g., UserPanaltyLogRepository, UserPanaltyLog,
PanaltyTypes); rename all occurrences to the correct spelling ("Penalty") across
the codebase: rename classes/interfaces (UserPanaltyLogRepository ->
UserPenaltyLogRepository, UserPanaltyLog -> UserPenaltyLog), enums (PanaltyTypes
-> PenaltyTypes), update file names, imports, JPA entity names/annotations,
repository method signatures (findByUserIdOrderByCreatedAtDesc), tests and any
SQL/migration/table names or JSON fields that reference the old name; use a
project-wide refactor/IDE rename to update the ~16 locations and run/build tests
to ensure no references remain.
In `@src/main/java/ssurent/ssurentbe/domain/users/service/UserService.java`:
- Around line 22-27: In UserService.getMyInfo the method calls
UserInfoResponse.from(user) twice and assigns the first result to an unused
local variable response; remove the duplicate call by either returning the
already-created response or eliminating the unused variable and returning the
single UserInfoResponse.from(user) result. Update the getMyInfo implementation
around userRepository.findByStudentNumAndDeletedFalse(...) /
UserInfoResponse.from(user) so only one UserInfoResponse is created and
returned.
- Around line 37-39: getMyPenalties currently uses
userRepository.findByStudentNum and throws RuntimeException with mixed-language
text; change it to use the soft-delete-aware repository method (same as
getMyInfo/updatePhoneNumber) i.e.
userRepository.findByStudentNumAndDeletedFalse(...) and replace the
RuntimeException with IllegalArgumentException using a consistent message style
(match other methods' language/format). Ensure you update the lookup in
getMyPenalties to mirror the soft-delete check and the exception type and
message used by getMyInfo/updatePhoneNumber.
🧹 Nitpick comments (3)
src/main/java/ssurent/ssurentbe/domain/users/repository/UserPanaltyLogRepository.java (1)
8-8: 사용되지 않는 import
java.util.Optional이 import되어 있으나 이 인터페이스에서 사용되지 않습니다.제안 수정사항
-import java.util.Optional;src/main/java/ssurent/ssurentbe/domain/users/controller/UserController.java (1)
10-10: 사용되지 않는 import:Users
Users엔티티가 컨트롤러에서 사용되지 않습니다.-import ssurent.ssurentbe.domain.users.entity.Users;src/main/java/ssurent/ssurentbe/domain/users/service/UserService.java (1)
6-8: "Panalty" 오타 — 전체 코드베이스에 걸친 문제
UserPanaltyLog,UserPanaltyLogRepository등 "Panalty"는 "Penalty"의 오타입니다. 여러 파일에 걸쳐 있어 지금 수정하면 범위가 크지만, 조기에 수정할수록 향후 혼란을 줄일 수 있습니다.Also applies to: 20-20
src/main/java/ssurent/ssurentbe/domain/users/controller/UserController.java
Show resolved
Hide resolved
| public record UpdatePhoneNumberRequest( | ||
| String phoneNum | ||
| ) { | ||
| } |
There was a problem hiding this comment.
전화번호 입력값 검증이 누락되었습니다.
phoneNum 필드에 @NotBlank나 @Pattern 같은 Bean Validation 어노테이션이 없어서, 빈 문자열이나 잘못된 형식의 전화번호가 그대로 저장될 수 있습니다. 컨트롤러에서 @Valid와 함께 사용하려면 여기에 검증 어노테이션이 필요합니다.
제안 수정사항
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.Pattern;
+
public record UpdatePhoneNumberRequest(
- String phoneNum
+ `@NotBlank`(message = "전화번호는 필수 입력값입니다.")
+ `@Pattern`(regexp = "^01[016789]-?\\d{3,4}-?\\d{4}$", message = "올바른 전화번호 형식이 아닙니다.")
+ String phoneNum
) {
}🤖 Prompt for AI Agents
In
`@src/main/java/ssurent/ssurentbe/domain/users/dto/UpdatePhoneNumberRequest.java`
around lines 3 - 6, UpdatePhoneNumberRequest currently exposes phoneNum without
validation; add Bean Validation annotations to the record component (e.g.,
annotate phoneNum with `@NotBlank` and a suitable `@Pattern` regex for your phone
format or a more permissive pattern) so controllers using `@Valid` will reject
empty or malformed numbers; update imports to use javax.validation.constraints.*
(or jakarta.validation.constraints.* depending on project) and ensure the
pattern matches your expected formats (international/local) and error messages
are set if needed.
| public interface UserPanaltyLogRepository extends JpaRepository<UserPanaltyLog, Long> { | ||
|
|
||
| List<UserPanaltyLog> findByUserIdOrderByCreatedAtDesc(Users user); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
find . -name "UserPanaltyLog.java" -type fRepository: SSURENT/SSURENT-BE
Length of output: 135
🏁 Script executed:
cat -n ./src/main/java/ssurent/ssurentbe/domain/users/entity/UserPanaltyLog.javaRepository: SSURENT/SSURENT-BE
Length of output: 1330
🏁 Script executed:
# Check if there are other entities with similar naming patterns
rg "private \w+ \w+Id;" --type java -A 1 -B 1 | head -50Repository: SSURENT/SSURENT-BE
Length of output: 2302
🏁 Script executed:
# Check for "Panalty" vs "Penalty" usage throughout the codebase
rg "Panalty|Penalty" --type java | head -30Repository: SSURENT/SSURENT-BE
Length of output: 2958
🏁 Script executed:
# Verify how Spring Data JPA resolves the method - check if this pattern works elsewhere in the codebase
rg "findBy.*OrderBy.*Desc\(" --type java -A 1Repository: SSURENT/SSURENT-BE
Length of output: 551
🏁 Script executed:
# Check the extent of the typo - count usages
rg "Panalty" --type java | wc -lRepository: SSURENT/SSURENT-BE
Length of output: 64
"Panalty" 오타 수정 필요
클래스명, 엔티티명, 리포지토리에서 Panalty로 표기되어 있으나 올바른 철자는 Penalty입니다. 이 오타는 코드베이스 전반에 걸쳐 16개 위치에서 나타나고 있으므로, 엔티티(UserPanaltyLog), 열거형(PanaltyTypes), 리포지토리 등에서 일관되게 Penalty로 수정하는 것을 권장합니다.
🤖 Prompt for AI Agents
In
`@src/main/java/ssurent/ssurentbe/domain/users/repository/UserPanaltyLogRepository.java`
around lines 10 - 12, The repository and related types use the misspelled
"Panalty" instead of "Penalty" (e.g., UserPanaltyLogRepository, UserPanaltyLog,
PanaltyTypes); rename all occurrences to the correct spelling ("Penalty") across
the codebase: rename classes/interfaces (UserPanaltyLogRepository ->
UserPenaltyLogRepository, UserPanaltyLog -> UserPenaltyLog), enums (PanaltyTypes
-> PenaltyTypes), update file names, imports, JPA entity names/annotations,
repository method signatures (findByUserIdOrderByCreatedAtDesc), tests and any
SQL/migration/table names or JSON fields that reference the old name; use a
project-wide refactor/IDE rename to update the ~16 locations and run/build tests
to ensure no references remain.
src/main/java/ssurent/ssurentbe/domain/users/service/UserQueryService.java
Show resolved
Hide resolved
| public List<UserPenaltyResponse> getMyPenalties(String username) { | ||
| Users user = userRepository.findByStudentNum(username) | ||
| .orElseThrow(() -> new RuntimeException("User not found")); |
There was a problem hiding this comment.
soft-delete 필터링 누락 및 예외 타입 불일치
두 가지 일관성 문제가 있습니다:
- Soft-delete 필터 누락:
getMyInfo와updatePhoneNumber는findByStudentNumAndDeletedFalse를 사용하지만,getMyPenalties는findByStudentNum을 사용하여 삭제된 사용자의 징계 내역도 조회될 수 있습니다. - 예외 타입 불일치: 다른 메서드는
IllegalArgumentException을 던지지만 여기서는RuntimeException을 사용합니다. 에러 메시지도 영어/한국어가 혼재되어 있습니다.
🔧 수정 제안
public List<UserPenaltyResponse> getMyPenalties(String username) {
- Users user = userRepository.findByStudentNum(username)
- .orElseThrow(() -> new RuntimeException("User not found"));
+ Users user = userRepository.findByStudentNumAndDeletedFalse(username)
+ .orElseThrow(() -> new IllegalArgumentException("사용자를 찾을 수 없습니다."));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public List<UserPenaltyResponse> getMyPenalties(String username) { | |
| Users user = userRepository.findByStudentNum(username) | |
| .orElseThrow(() -> new RuntimeException("User not found")); | |
| public List<UserPenaltyResponse> getMyPenalties(String username) { | |
| Users user = userRepository.findByStudentNumAndDeletedFalse(username) | |
| .orElseThrow(() -> new IllegalArgumentException("사용자를 찾을 수 없습니다.")); |
🤖 Prompt for AI Agents
In `@src/main/java/ssurent/ssurentbe/domain/users/service/UserService.java` around
lines 37 - 39, getMyPenalties currently uses userRepository.findByStudentNum and
throws RuntimeException with mixed-language text; change it to use the
soft-delete-aware repository method (same as getMyInfo/updatePhoneNumber) i.e.
userRepository.findByStudentNumAndDeletedFalse(...) and replace the
RuntimeException with IllegalArgumentException using a consistent message style
(match other methods' language/format). Ensure you update the lookup in
getMyPenalties to mirror the soft-delete check and the exception type and
message used by getMyInfo/updatePhoneNumber.
|
PR제목 형식 맞춰주세여 제목에 이슈번호 빼기 |
There was a problem hiding this comment.
우리 CQRS 패턴으로 작업중이라서 Request/Response Dto 패키지 따로 만들어서 관리해야 해.
|
|
||
| public UserInfoResponse getMyInfo(String username) { | ||
| Users user = userRepository.findByStudentNumAndDeletedFalse(username) | ||
| .orElseThrow(() -> new IllegalArgumentException("사용자를 찾을 수 없습니다.")); |
There was a problem hiding this comment.
커스텀 에러 사용하기스텀 에러 사용하기
.orElseThrow(() -> new GeneralException(ErrorStatus.USER_NOT_FOUND)
| private final UserPanaltyLogRepository userPanaltyLogRepository; | ||
|
|
||
| public UserInfoResponse getMyInfo(String username) { | ||
| Users user = userRepository.findByStudentNumAndDeletedFalse(username) |
There was a problem hiding this comment.
해당 문장이 반복되는거 같으니까 함수화해서 적용하면 좋을 듯
| @Transactional | ||
| public void updatePhoneNumber(String username, String phoneNum) { | ||
| Users user = userRepository.findByStudentNumAndDeletedFalse(username) | ||
| .orElseThrow(() -> new IllegalArgumentException("사용자를 찾을 수 없습니다.")); |
There was a problem hiding this comment.
동일하게 커스텀 에러 사용
.orElseThrow(() -> new GeneralException(ErrorStatus.USER_NOT_FOUND)
There was a problem hiding this comment.
서비스도 CQRS 패턴 적용해서
UserCommandService(쓰기)랑
UserQueryService(읽기)로 나누어서 관리해야 함.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/main/java/ssurent/ssurentbe/common/status/SuccessStatus.java`:
- Line 21: Rename the enum constant PANELTY_CHECK_SUCCESS to
PENALTY_CHECK_SUCCESS and fix the associated code string "PANELTY_200" to
"PENALTY_200" in SuccessStatus; update all usages/references of
PANELTY_CHECK_SUCCESS throughout the codebase (imports, switch/case, tests, JSON
mapping, API docs) to the new PENALTY_CHECK_SUCCESS identifier so compilation
and external responses continue to work.
In `@src/main/java/ssurent/ssurentbe/domain/users/controller/UserController.java`:
- Line 47: In UserController.java replace the misspelled enum constant reference
SuccessStatus.PANELTY_CHECK_SUCCESS with the corrected
SuccessStatus.PENALTY_CHECK_SUCCESS (and update any other occurrences in this
class) so it matches the renamed constant in SuccessStatus; ensure
imports/compilation and run a quick search for PANELTY_* usages to update them
similarly.
- Around line 26-29: The getMyInfo endpoint in UserController currently returns
UserInfoResponse directly; change it to return the consistent wrapper type
(BaseResponse<UserInfoResponse>) and wrap the service result
(userQueryService.getMyInfo(userDetails.getUsername())) in a BaseResponse
success payload (use the same static factory or constructor used by other
endpoints like getMyPenalties/AuthController) and update the method signature
and return statement accordingly so the controller consistently returns
BaseResponse across endpoints.
🧹 Nitpick comments (2)
src/main/java/ssurent/ssurentbe/domain/users/service/UserCommandService.java (1)
17-20:getUserInfo메서드가UserQueryService와 중복됩니다.
UserCommandService.getUserInfo와UserQueryService.getUserInfo가 동일한 로직을 가지고 있습니다.UserCommandService.updatePhoneNumber에서UserQueryService에 위임하거나, 공통 헬퍼로 추출하는 것을 고려해 주세요.♻️ 수정 제안: UserQueryService에 위임
`@Service` `@RequiredArgsConstructor` -@Transactional(readOnly = true) public class UserCommandService { - private final UserRepository userRepository; - - public Users getUserInfo(String username) { - return userRepository.findByStudentNumAndDeletedFalse(username) - .orElseThrow(() -> new GeneralException(ErrorStatus.USER_NOT_FOUND)); - } + private final UserQueryService userQueryService; + `@Transactional` public void updatePhoneNumber(String username, String phoneNum) { - Users user = getUserInfo(username); + Users user = userQueryService.getUserInfo(username); user.updatePhoneNumber(phoneNum); } }src/main/java/ssurent/ssurentbe/domain/users/service/UserQueryService.java (1)
38-40:Collectors.toList()대신.toList()사용 권장Java 16+ 환경이라면
Collectors.toList()대신Stream.toList()를 사용하여 간결하게 작성할 수 있습니다. (반환되는 리스트가 unmodifiable인 점은 참고하세요.)♻️ 수정 제안
return logs.stream() .map(UserPenaltyResponse::from) - .collect(Collectors.toList()); + .toList();
| SIGNUP_SUCCESS(HttpStatus.CREATED, "AUTH_201", "회원가입 성공"); | ||
| SIGNUP_SUCCESS(HttpStatus.CREATED, "AUTH_201", "회원가입 성공"), | ||
|
|
||
| PANELTY_CHECK_SUCCESS(HttpStatus.OK, "PANELTY_200", "징계내역 조회 성공"); |
There was a problem hiding this comment.
오타: PANELTY → PENALTY
PANELTY_CHECK_SUCCESS와 코드 "PANELTY_200" 모두 "PENALTY"의 오타입니다. API 응답 코드에 포함되는 값이므로 수정이 필요합니다.
🐛 수정 제안
- PANELTY_CHECK_SUCCESS(HttpStatus.OK, "PANELTY_200", "징계내역 조회 성공");
+ PENALTY_CHECK_SUCCESS(HttpStatus.OK, "PENALTY_200", "징계내역 조회 성공");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| PANELTY_CHECK_SUCCESS(HttpStatus.OK, "PANELTY_200", "징계내역 조회 성공"); | |
| PENALTY_CHECK_SUCCESS(HttpStatus.OK, "PENALTY_200", "징계내역 조회 성공"); |
🤖 Prompt for AI Agents
In `@src/main/java/ssurent/ssurentbe/common/status/SuccessStatus.java` at line 21,
Rename the enum constant PANELTY_CHECK_SUCCESS to PENALTY_CHECK_SUCCESS and fix
the associated code string "PANELTY_200" to "PENALTY_200" in SuccessStatus;
update all usages/references of PANELTY_CHECK_SUCCESS throughout the codebase
(imports, switch/case, tests, JSON mapping, API docs) to the new
PENALTY_CHECK_SUCCESS identifier so compilation and external responses continue
to work.
| @GetMapping | ||
| public UserInfoResponse getMyInfo(@AuthenticationPrincipal UserDetails userDetails) { | ||
| return userQueryService.getMyInfo(userDetails.getUsername()); | ||
| } |
There was a problem hiding this comment.
getMyInfo 응답이 BaseResponse로 래핑되지 않았습니다.
getMyPenalties와 AuthController의 다른 엔드포인트들은 모두 BaseResponse로 래핑하여 반환하는데, getMyInfo만 UserInfoResponse를 직접 반환합니다. API 응답 형식의 일관성을 위해 동일한 패턴을 적용하는 것이 좋습니다.
♻️ 수정 제안
`@GetMapping`
- public UserInfoResponse getMyInfo(`@AuthenticationPrincipal` UserDetails userDetails) {
- return userQueryService.getMyInfo(userDetails.getUsername());
+ public ResponseEntity<BaseResponse<UserInfoResponse>> getMyInfo(`@AuthenticationPrincipal` UserDetails userDetails) {
+ UserInfoResponse data = userQueryService.getMyInfo(userDetails.getUsername());
+ SuccessStatus status = SuccessStatus.COMM_SUCCESS_STATUS;
+ return ResponseEntity.status(status.getHttpStatus())
+ .body(BaseResponse.success(status, data));
}🤖 Prompt for AI Agents
In `@src/main/java/ssurent/ssurentbe/domain/users/controller/UserController.java`
around lines 26 - 29, The getMyInfo endpoint in UserController currently returns
UserInfoResponse directly; change it to return the consistent wrapper type
(BaseResponse<UserInfoResponse>) and wrap the service result
(userQueryService.getMyInfo(userDetails.getUsername())) in a BaseResponse
success payload (use the same static factory or constructor used by other
endpoints like getMyPenalties/AuthController) and update the method signature
and return statement accordingly so the controller consistently returns
BaseResponse across endpoints.
| @AuthenticationPrincipal UserDetails userDetails | ||
| ) { | ||
| List<UserPenaltyResponse> data = userQueryService.getMyPenalties(userDetails.getUsername()); | ||
| SuccessStatus status = SuccessStatus.PANELTY_CHECK_SUCCESS; |
There was a problem hiding this comment.
PANELTY_CHECK_SUCCESS 오타 참조
SuccessStatus.java에서 PANELTY → PENALTY 수정 시 이 참조도 함께 변경해야 합니다.
🤖 Prompt for AI Agents
In `@src/main/java/ssurent/ssurentbe/domain/users/controller/UserController.java`
at line 47, In UserController.java replace the misspelled enum constant
reference SuccessStatus.PANELTY_CHECK_SUCCESS with the corrected
SuccessStatus.PENALTY_CHECK_SUCCESS (and update any other occurrences in this
class) so it matches the renamed constant in SuccessStatus; ensure
imports/compilation and run a quick search for PANELTY_* usages to update them
similarly.
# Conflicts: # src/main/resources/application.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/resources/application.yml`:
- Around line 7-8: The application.yml currently hardcodes the Spring profile to
prod (profiles.active: prod); change this to a safe default (e.g.,
profiles.active: local or dev) or remove the active key entirely so the profile
is driven by SPRING_PROFILES_ACTIVE/CLI at runtime, and ensure deployment/CI
sets SPRING_PROFILES_ACTIVE=prod for production; update the "profiles.active"
entry in application.yml and verify any bootstrap/start scripts or CI pipelines
explicitly set the production profile when deploying.
| profiles: | ||
| active: prod |
There was a problem hiding this comment.
기본 프로파일이 prod로 하드코딩되어 있습니다.
application.yml에서 active: prod를 기본값으로 설정하면, 로컬 개발 및 CI 환경에서도 프로덕션 설정(DB, 외부 서비스 등)으로 연결됩니다. 실수로 프로덕션 데이터를 변경하거나 장애를 유발할 수 있습니다.
기본값은 local 또는 dev로 설정하고, 프로덕션 배포 시 환경 변수(SPRING_PROFILES_ACTIVE=prod) 또는 CLI 인자로 오버라이드하는 것을 권장합니다.
🛡️ 수정 제안
spring:
profiles:
- active: prod
+ active: local📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| profiles: | |
| active: prod | |
| profiles: | |
| active: local |
🤖 Prompt for AI Agents
In `@src/main/resources/application.yml` around lines 7 - 8, The application.yml
currently hardcodes the Spring profile to prod (profiles.active: prod); change
this to a safe default (e.g., profiles.active: local or dev) or remove the
active key entirely so the profile is driven by SPRING_PROFILES_ACTIVE/CLI at
runtime, and ensure deployment/CI sets SPRING_PROFILES_ACTIVE=prod for
production; update the "profiles.active" entry in application.yml and verify any
bootstrap/start scripts or CI pipelines explicitly set the production profile
when deploying.
개요
PR 유형
어떤 변경 사항이 있나요?
PR Checklist
PR이 다음 요구 사항을 충족하는지 확인하세요.
🧩 작업 내용
📸 스크린샷(선택)
📣 To Reviewers
Summary by CodeRabbit
릴리스 노트
New Features
Refactor