feat: 멘토 승격 api 구현 #532
feat: 멘토 승격 api 구현 #532
Conversation
* feat: 멘티 승격 요청 관련 entity, controller, service, repository 기본셋팅 * fix: MentorApplication 를 prefix 로 클래스명 수정 * feat: MentorApplication 엔티티 필드 확정 - universityId 는 null 을 허용합니다.(서비스에서 제공하지 않는 학교는 universityId가 null) * feat: 멘토 승격 신청 로직 작성 - 요청자의 증명서를 S3에 저장하기 위해 ImgType 클래스에 MENTOR_PROOF 추가 - 멘토 승격 요청자의 수학 상태를 나타내는 ExchangePhase 클래스 추가 * feat: 멘토 승격 중복 요청을 막는 조건 추가 - ErrorCode클래스에 MENTOR_APPLICATION_ALREADY_EXISTED 예외 추가 * style: code 컨벤션 적용 * test: 멘토 승격 요청 api 테스트 코드 추가 * feat: 멘토 지원 테이블 DDL 작성
* fix: MentorApplication 엔티티 수정 - UniversitySelectStatus 필드 추가 - region 필트 삭제 - ExchangeStatus 쓰도록 수정 - 도메인 규칙 추가 * feat: 도메인 규칙에 맞게 ErrorCode 추가 * test: 테스트 코드 에러 수정 * test: 도메인 규칙 테스트 추가
sukangpunch
requested review from
Gyuhyeok99,
Hexeong,
JAEHEE25,
lsy1307,
whqtker and
wibaek
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/main/resources/db/migration/V35__add_mentor_application_table.sql (1)
5-5: country_code 컬럼 타입을 최적화할 수 있습니다.
VARCHAR(255)는 국가 코드 저장에 과도하게 큽니다. ISO 국가 코드는 일반적으로 2-3자(예: 'KR', 'USA')입니다.다음과 같이 변경을 고려해보세요:
- country_code VARCHAR(255), + country_code VARCHAR(10),src/main/java/com/example/solidconnection/mentor/controller/MentorApplicationController.java (1)
26-26: 응답에 생성된 신청 정보를 포함하는 것을 고려해보세요.현재는
ResponseEntity<Void>로 응답 본문이 없지만, 생성된 신청의 ID나 상태를 반환하면 클라이언트가 후속 작업을 하기 편합니다.예시:
-public ResponseEntity<Void> requestMentorApplication( +public ResponseEntity<MentorApplicationResponse> requestMentorApplication( @AuthorizedUser long siteUserId, @Valid @RequestPart("mentorApplicationRequest") MentorApplicationRequest mentorApplicationRequest, @RequestPart("file") MultipartFile file ) { - mentorApplicationService.submitMentorApplication(siteUserId, mentorApplicationRequest, file); - return ResponseEntity.ok().build(); + MentorApplicationResponse response = mentorApplicationService.submitMentorApplication(siteUserId, mentorApplicationRequest, file); + return ResponseEntity.ok(response); }단, 이는 API 설계 결정이므로 팀 컨벤션에 따라 선택하시면 됩니다.
src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java (1)
89-123: 검증 로직 테스트가 정확합니다.
- CATALOG 타입 검증: universityId 필수 체크
- OTHER 타입 검증: universityId null 강제 체크
두 테스트 모두 CustomException과 올바른 에러 메시지를 검증하고 있습니다.
선택적 개선: 검증 실패 테스트에서 S3 모킹 제거
검증 로직이 S3 업로드 전에 실행되므로, lines 96-98과 114-116의 S3Service 모킹은 실행되지 않습니다. 테스트 가독성을 위해 제거를 고려해보세요.
@Test void 대학_선택_타입이_CATALOG_인데_universityId가_null_이면_예외(){ // given UniversitySelectType universitySelectType = UniversitySelectType.CATALOG; Long universityId = null; MentorApplicationRequest request = createMentorApplicationRequest(universitySelectType, universityId); MockMultipartFile file = createMentorProofFile(); - String fileUrl = "/mentor-proof.pdf"; - given(s3Service.uploadFile(file, ImgType.MENTOR_PROOF)) - .willReturn(new UploadedFileUrlResponse(fileUrl)); // when // thensrc/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java (2)
72-86: 대학 선택 검증 로직이 명확합니다.Switch 표현식을 사용하여 각 타입별 검증 규칙을 깔끔하게 표현했습니다.
선택적 개선: Default case 제거 검토
Line 84의
default케이스는UniversitySelectTypeenum이 CATALOG와 OTHER만 가진다면 도달 불가능한 코드입니다. Java의 switch 표현식은 enum의 모든 케이스를 다루면 exhaustive하므로 default가 불필요합니다.private void validateUniversitySelection(UniversitySelectType universitySelectType, Long universityId) { switch (universitySelectType) { case CATALOG -> { if(universityId == null) { throw new CustomException(ErrorCode.UNIVERSITY_ID_REQUIRED_FOR_CATALOG); } } case OTHER -> { if(universityId != null) { throw new CustomException(ErrorCode.UNIVERSITY_ID_MUST_BE_NULL_FOR_OTHER); } } - default -> throw new CustomException(ErrorCode.INVALID_UNIVERSITY_SELECT_TYPE); } }다만 향후 enum에 새 타입이 추가될 때 컴파일 에러로 잡아주기 위해 의도적으로 남겨두는 것도 합리적인 선택입니다.
24-28: siteUserId와 countryCode 설계 검토 및 일관성 제안
siteUserId 타입 일관성
- 현재 primitive
long으로 null 불가능.- 다른 ID 필드(
universityId)는 wrapperLong사용.- 일관성을 위해
Long래퍼 타입 사용 권장.countryCode 설계
- 현재 String으로 ISO 코드만 저장.
- University 엔티티에서 이미
@ManyToOne Country매핑 사용 중.- FK 매핑(
@ManyToOne,@JoinColumn) 적용 시 참조 무결성 보장 가능.- CHECK 제약 또는 어노테이션으로 유효성 검증 추가 검토.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(1 hunks)src/main/java/com/example/solidconnection/mentor/controller/MentorApplicationController.java(1 hunks)src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java(1 hunks)src/main/java/com/example/solidconnection/mentor/domain/MentorApplicationStatus.java(1 hunks)src/main/java/com/example/solidconnection/mentor/domain/UniversitySelectType.java(1 hunks)src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java(1 hunks)src/main/java/com/example/solidconnection/mentor/repository/MentorApplicationRepository.java(1 hunks)src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java(1 hunks)src/main/java/com/example/solidconnection/s3/domain/ImgType.java(1 hunks)src/main/resources/db/migration/V35__add_mentor_application_table.sql(1 hunks)src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java (2)
src/main/java/com/example/solidconnection/mentor/controller/MentorApplicationController.java (1)
RequiredArgsConstructor(12-28)src/main/java/com/example/solidconnection/common/exception/CustomExceptionHandler.java (1)
Slf4j(22-88)
⏰ 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 (16)
src/main/java/com/example/solidconnection/mentor/domain/UniversitySelectType.java (1)
3-7: 깔끔한 enum 정의입니다.대학 선택 방식을 명확히 구분하는 간결한 enum이네요. CATALOG와 OTHER의 의미가 명확하고, 도메인 규칙과 잘 매칭됩니다.
src/main/resources/db/migration/V35__add_mentor_application_table.sql (2)
16-19: CHECK 제약조건이 도메인 규칙을 정확히 반영합니다.
university_select_type에 따라university_id의 필수 여부를 강제하는 로직이 올바르게 구현되어 있습니다.
- CATALOG 선택 시:
university_id NOT NULL보장- OTHER 선택 시:
university_id IS NULL강제
5-6: 1. Country 테이블 확인
- migrations에서 Country 테이블(또는 복수형 countries)이 실제로 존재하는지 살펴보세요.
- 존재한다면V35__add_mentor_application_table.sql의country_code(라인 5)에 FOREIGN KEY 제약조건을 추가하세요.
2. University 테이블 확인
- migrations에서 University 테이블(또는 복수형 universities)이 실제로 존재하는지 살펴보세요.
- 존재한다면V35__add_mentor_application_table.sql의university_id(라인 6)에 FOREIGN KEY 제약조건을 추가하세요.src/main/java/com/example/solidconnection/mentor/domain/MentorApplicationStatus.java (1)
3-8: 멘토 신청 상태를 명확히 표현합니다.PENDING, APPROVED, REJECTED로 신청 생명주기를 잘 표현하고 있습니다. Line 7의 trailing semicolon은 선택사항이지만 일관성을 위해 유지하셔도 좋습니다.
src/main/java/com/example/solidconnection/s3/domain/ImgType.java (1)
7-14: 멘토 증빙 파일 타입 추가가 적절합니다.
MENTOR_PROOF("mentor-proof")추가로 멘토 신청 증빙 파일 업로드를 지원하게 되었습니다. 기존 패턴을 잘 따르고 있고, MentorApplicationService와의 통합도 명확합니다.src/main/java/com/example/solidconnection/mentor/repository/MentorApplicationRepository.java (1)
6-9: 리포지토리 설계가 명확합니다.중복 신청 방지를 위한
existsBySiteUserId메서드가 적절하게 정의되어 있습니다. Spring Data JPA 명명 규칙을 따르고 있어 추가 구현 없이 동작합니다.src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java (1)
36-38: 중복 신청 방지 로직이 올바르게 구현되어 있습니다.
existsBySiteUserId를 통한 중복 체크가 적절하며, 트랜잭션 내에서 실행되어 동시성 문제도 어느 정도 방지됩니다.src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java (2)
7-14: DTO에 유효성 검사 어노테이션 추가 및 universityId 검증 확인 요청
validation 어노테이션 추가
@NotNull(message = "교환 상태는 필수입니다") → exchangeStatus
@NotNull(message = "대학 선택 타입은 필수입니다") → universitySelectType
@notblank(message = "국가 정보는 필수입니다") → countryuniversityId 조건부 검증 로직 구현
universitySelectType에 따라 universityId가 필수/nullable로 동작하도록 커스텀 validator 작성MentorApplication 생성자 또는 서비스 레이어에서 universityId 검증 로직이 제대로 적용되었는지 확인 부탁드립니다.
11-12: Country 및universityId참조 방식 재검토
- Country 타입 변경
- 현재String country대신countryCode(String, 2~3자)로 수정하여 유효한 국가만 입력받도록 설계를 강화해주세요.universityId유효성 제약 반영
- CATALOG 선택 시 필수, OTHER 선택 시null로 허용하는 비즈니스 로직을 DTO 및 서비스 계층에 반영해주세요.- Country/University 조회 API 확인 또는 구현
- 코드베이스에CountryController나UniversityController(또는 유사 엔드포인트)가 존재하는지 확인하고, 없으면GET /countries,GET /universities?countryCode={}엔드포인트를 추가해주세요.현재 관련 컨트롤러가 검색되지 않으니, API 유무를 직접 확인해주시기 바랍니다.
src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java (4)
30-51: 테스트 설정이 적절합니다.
- 통합 테스트 구성: TestContainerSpringBootTest를 사용하여 실제 DB 환경에서 테스트
- 외부 의존성 모킹: S3Service를 MockBean으로 처리하여 테스트 격리
- 테스트 데이터 준비: SiteUserFixture로 일관된 테스트 사용자 생성
53-69: CATALOG 타입 정상 등록 시나리오가 잘 작성되었습니다.테스트가 given-when-then 패턴을 명확하게 따르고 있으며, S3 업로드를 적절히 모킹하여 서비스 로직에만 집중하고 있습니다.
71-87: OTHER 타입 테스트가 적절합니다.universityId가 null인 경우의 정상 처리를 검증하고 있어, CATALOG 타입 테스트와 함께 대학 선택 타입의 양 갈래를 모두 커버합니다.
125-167: 중복 신청 방지 테스트와 헬퍼 메서드가 잘 구현되었습니다.
- 중복 방지 검증: 기존 신청이 있을 때 MENTOR_APPLICATION_ALREADY_EXISTED 예외 발생 확인
- 헬퍼 메서드: 테스트 데이터 생성 로직을 재사용 가능하게 분리
테스트 커버리지가 충분하며, 주요 검증 시나리오를 모두 다루고 있습니다.
src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java (2)
53-70: 생성자와 검증 로직이 잘 설계되었습니다.
- Fail-fast 검증: 생성자에서 즉시 검증하여 잘못된 상태의 엔티티 생성 방지
- SiteUser에서 ID 추출: Line 64에서
siteUser.getId()로 ID를 가져와 저장하여 엔티티 간 결합도 최소화- 순차적 검증: exchangeStatus → universitySelection 순서로 명확하게 검증
50-51: 교환학생 상태 검증이 효율적으로 구현되었습니다.
- EnumSet 사용: Line 51에서
EnumSet.of()로 허용된 상태들을 정의하여 O(1) 검증 성능 확보- Immutable Set:
Collections.unmodifiableSet으로 불변성 보장- 명확한 비즈니스 규칙: STUDYING_ABROAD와 AFTER_EXCHANGE만 멘토 신청 가능
Also applies to: 88-92
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1)
122-126: 새로운 에러 코드들이 적절하게 추가되었습니다.
적절한 HTTP 상태 코드:
- Line 122: 중복 신청은 CONFLICT (409) 사용
- Lines 123-126: 검증 실패는 BAD_REQUEST (400) 사용
명확한 에러 메시지: 각 에러마다 구체적인 실패 이유와 조건을 한글로 설명
일관된 네이밍: 기존 ErrorCode 패턴(대문자 스네이크 케이스, 도메인 접두사)을 따름
에러 코드들이 테스트와 엔티티 검증 로직에서 올바르게 사용되고 있습니다.
| @PostMapping("/mentor-applications") | ||
| public ResponseEntity<Void> requestMentorApplication( | ||
| @AuthorizedUser long siteUserId, | ||
| @Valid @RequestPart("mentorApplicationRequest") MentorApplicationRequest mentorApplicationRequest, | ||
| @RequestParam("file") MultipartFile file | ||
| ) { | ||
| mentorApplicationService.submitMentorApplication(siteUserId, mentorApplicationRequest, file); | ||
| return ResponseEntity.ok().build(); | ||
| } |
There was a problem hiding this comment.
Controller 구조는 명확하지만, 파라미터 어노테이션을 통일해주세요.
Controller는 적절히 service로 위임하고 있으나 다음을 개선하면 좋겠습니다:
-
@Valid 동작 안 함 (Line 22):
MentorApplicationRequest에 validation 어노테이션이 없어@Valid가 작동하지 않습니다.- DTO 리뷰 코멘트를 참고해주세요.
-
@RequestParam vs @RequestPart 불일치 (Line 23):
mentorApplicationRequest는@RequestPart,file은@RequestParam을 사용하고 있습니다.- Multipart 요청에서는 일관성을 위해 둘 다
@RequestPart를 사용하는 것이 좋습니다.
-
파일 검증 부재:
- 파일 크기, 타입 등에 대한 검증이 없습니다.
- 서비스나 컨트롤러 레벨에서 파일 검증을 추가하면 더 안전합니다.
다음과 같이 개선해보세요:
@PostMapping("/mentor-applications")
public ResponseEntity<Void> requestMentorApplication(
@AuthorizedUser long siteUserId,
@Valid @RequestPart("mentorApplicationRequest") MentorApplicationRequest mentorApplicationRequest,
- @RequestParam("file") MultipartFile file
+ @RequestPart("file") MultipartFile file
) {
+ // Optional: Add file validation
+ if (file.isEmpty()) {
+ throw new CustomException(ErrorCode.FILE_REQUIRED);
+ }
+ if (file.getSize() > 10 * 1024 * 1024) { // 10MB limit
+ throw new CustomException(ErrorCode.FILE_TOO_LARGE);
+ }
+
mentorApplicationService.submitMentorApplication(siteUserId, mentorApplicationRequest, file);
return ResponseEntity.ok().build();
}📝 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.
| @PostMapping("/mentor-applications") | |
| public ResponseEntity<Void> requestMentorApplication( | |
| @AuthorizedUser long siteUserId, | |
| @Valid @RequestPart("mentorApplicationRequest") MentorApplicationRequest mentorApplicationRequest, | |
| @RequestParam("file") MultipartFile file | |
| ) { | |
| mentorApplicationService.submitMentorApplication(siteUserId, mentorApplicationRequest, file); | |
| return ResponseEntity.ok().build(); | |
| } | |
| @PostMapping("/mentor-applications") | |
| public ResponseEntity<Void> requestMentorApplication( | |
| @AuthorizedUser long siteUserId, | |
| @Valid @RequestPart("mentorApplicationRequest") MentorApplicationRequest mentorApplicationRequest, | |
| @RequestPart("file") MultipartFile file | |
| ) { | |
| // Optional: Add file validation | |
| if (file.isEmpty()) { | |
| throw new CustomException(ErrorCode.FILE_REQUIRED); | |
| } | |
| if (file.getSize() > 10 * 1024 * 1024) { // 10MB limit | |
| throw new CustomException(ErrorCode.FILE_TOO_LARGE); | |
| } | |
| mentorApplicationService.submitMentorApplication(siteUserId, mentorApplicationRequest, file); | |
| return ResponseEntity.ok().build(); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/mentor/controller/MentorApplicationController.java
around lines 19-27, the endpoint uses @Valid on MentorApplicationRequest which
has no validation annotations, mixes @RequestPart and @RequestParam for a
multipart request, and lacks file validation; fix by adding appropriate
javax/jakarta validation annotations to MentorApplicationRequest fields (so
@Valid takes effect), change the controller to accept both parts as @RequestPart
and set consumes = MediaType.MULTIPART_FORM_DATA_VALUE on the @PostMapping, and
add server-side file checks (size limit, allowed content types and null/empty
checks) either in the controller before delegating or in the service, returning
a 400 response for invalid files.
There was a problem hiding this comment.
파일검증은 serivce 내에서 해줘도 좋을 거 같습니다~
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| @Transactional | ||
| public void submitMentorApplication( | ||
| long siteUserId, | ||
| MentorApplicationRequest mentorApplicationRequest, | ||
| MultipartFile file | ||
| ) { | ||
| if (mentorApplicationRepository.existsBySiteUserId(siteUserId)) { | ||
| throw new CustomException(MENTOR_APPLICATION_ALREADY_EXISTED); | ||
| } | ||
|
|
||
| SiteUser siteUser = siteUserRepository.findById(siteUserId) | ||
| .orElseThrow(() -> new CustomException(USER_NOT_FOUND)); | ||
| UploadedFileUrlResponse uploadedFile = s3Service.uploadFile(file, ImgType.MENTOR_PROOF); | ||
| MentorApplication mentorApplication = new MentorApplication( | ||
| siteUser, | ||
| mentorApplicationRequest.country(), | ||
| mentorApplicationRequest.universityId(), | ||
| mentorApplicationRequest.universitySelectType(), | ||
| uploadedFile.fileUrl(), | ||
| mentorApplicationRequest.exchangeStatus() | ||
| ); | ||
| mentorApplicationRepository.save(mentorApplication); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
서비스 로직 구조는 명확하지만, 실행 순서를 재검토해주세요.
전반적인 흐름은 좋으나, 다음 사항들을 확인해주세요:
-
S3 업로드 시점 (Line 42):
MentorApplication생성자의 validation이 실패하면 이미 업로드된 파일이 고아 파일로 남게 됩니다.- Validation을 먼저 수행하거나, 실패 시 S3 파일을 삭제하는 보상 트랜잭션을 고려해주세요.
-
universityId validation 누락:
universitySelectType에 따른universityId필수/null 검증이 서비스 레이어에 없습니다.MentorApplication생성자에서 검증하는지 확인이 필요합니다.
-
file 파라미터 null 체크 부재:
MultipartFile file파라미터에 대한 null 체크가 없습니다.- Controller에서
@RequestParam으로 받지만 명시적 검증을 추가하면 더 안전합니다.
다음 스크립트로 MentorApplication 생성자의 validation 로직을 확인해보세요:
권장 개선안:
@Transactional
public void submitMentorApplication(
long siteUserId,
MentorApplicationRequest mentorApplicationRequest,
MultipartFile file
) {
+ if (file == null || file.isEmpty()) {
+ throw new CustomException(ErrorCode.FILE_REQUIRED);
+ }
+
if (mentorApplicationRepository.existsBySiteUserId(siteUserId)) {
throw new CustomException(MENTOR_APPLICATION_ALREADY_EXISTED);
}
SiteUser siteUser = siteUserRepository.findById(siteUserId)
.orElseThrow(() -> new CustomException(USER_NOT_FOUND));
- UploadedFileUrlResponse uploadedFile = s3Service.uploadFile(file, ImgType.MENTOR_PROOF);
+
+ // Validate before S3 upload
MentorApplication mentorApplication = new MentorApplication(
siteUser,
mentorApplicationRequest.country(),
mentorApplicationRequest.universityId(),
mentorApplicationRequest.universitySelectType(),
- uploadedFile.fileUrl(),
+ null, // temporary, will be set after upload
mentorApplicationRequest.exchangeStatus()
);
+
+ // Upload file after validation
+ UploadedFileUrlResponse uploadedFile = s3Service.uploadFile(file, ImgType.MENTOR_PROOF);
+ mentorApplication.setMentorProofUrl(uploadedFile.fileUrl());
+
mentorApplicationRepository.save(mentorApplication);
}단, 이 방식은 MentorApplication에 setter 추가가 필요하므로 불변성을 유지하려면 다른 패턴(빌더 등)을 고려해야 합니다.
🏁 Script executed:
#!/bin/bash
# Description: Verify MentorApplication constructor validation logic
ast-grep --pattern $'class MentorApplication {
$$$
MentorApplication($$$) {
$$$
}
$$$
}'Length of output: 0
아래 스크립트를 실행하여 MentorApplication 생성자 검증 로직을 다시 확인해주세요.
- MentorApplication.java 파일 경로 조회
- 생성자 시그니처와 유효성 검사 코드 출력
🏁 Script executed:
#!/bin/bash
# 1. MentorApplication.java 파일 경로 찾기
file=$(rg -lP --type java 'class\s+MentorApplication\b')
echo "[DEBUG] MentorApplication 파일: $file"
# 2. 생성자 시그니처 및 검증 로직 출력
rg -nP '\bMentorApplication\s*\(' "$file" -C3Length of output: 425
아래 스크립트를 실행하여 생성자 본문 전체를 확인해주세요.
🏁 Script executed:
#!/bin/bash
file=src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java
echo "[DEBUG] Dumping lines 50-200 of $file"
sed -n '50,200p' "$file"Length of output: 1830
서비스 로직 개선 제안
서비스 로직은 깔끔하지만, 아래 두 가지를 살펴봐 주세요:
-
S3 업로드 시점 재구성
현재s3Service.uploadFile호출 전에 생성자 검증이 실패하면 업로드된 파일이 고아 파일로 남습니다.
유효성 검증 후 업로드하거나, 실패 시 S3 삭제 보상 로직을 추가하세요. -
파일 파라미터 null/empty 체크 추가
MultipartFile file이 null이거나 비어 있을 경우 예외를 던지도록 검증을 넣어주세요.
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java
around lines 30 to 52, the current flow uploads the file to S3 before completing
validation and creating the entity which can leave orphaned files, and it also
lacks null/empty checks for the MultipartFile; change the flow to first validate
input (throw if file is null or file.isEmpty()), check mentor application
existence and load SiteUser, then call s3Service.uploadFile only after
validations succeed and before constructing the MentorApplication, or
alternatively wrap the upload + save in a try/catch that deletes the uploaded S3
object on any downstream failure; ensure you throw an appropriate
CustomException for missing/empty file and perform S3 cleanup in the catch if
you keep upload before persistence.
There was a problem hiding this comment.
@Gyuhyeok99 네, 알겠습니다! 👍
나중에 개선하시기로 하셨군요. 혹시 이 개선 사항을 추적하기 위해 후속 이슈를 생성해드릴까요?
이슈를 만들어드리면 다음 내용이 포함됩니다:
- S3 업로드 시점 재구성 (validation 후 업로드 또는 실패 시 보상 로직)
- MultipartFile null/empty 체크 추가
- 이 PR과 댓글의 링크
원하시면 말씀해주세요! 😊
There was a problem hiding this comment.
전반적으로 정말 잘 작성해주신 거 같습니다! 리뷰가 늦어진 점 죄송합니다.. 오늘부턴 아주 빨리빨리 답변할 수 있을 거 같습니다!!
고생하셨습니다!!!
멘토 승격 지원서가 가져야할 필드
지금 작성해주신 방식 유지하면 문제 없을 거 같습니다!!
'기타' 처리에 대한 구현
저는 지금 방식도 괜찮다고 생각합니다! 처음에는 그냥 기타라는 대학을 하나 만들어두려고했었던 것인데 괜히 데이터를 오염시키는 거 같기도 해서 지금 방식 유지해도 좋을 거 같습니다~
| import jakarta.validation.Valid; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.web.bind.annotation.*; |
| import jakarta.persistence.*; | ||
| import java.util.Collections; | ||
| import java.util.EnumSet; | ||
| import java.util.Set; | ||
| import lombok.*; | ||
|
|
| if (mentorApplicationRepository.existsBySiteUserId(siteUserId)) { | ||
| throw new CustomException(MENTOR_APPLICATION_ALREADY_EXISTED); | ||
| } |
There was a problem hiding this comment.
이러면 한 번 거절되면 재신청이 안될 거 같은데 맞나요?
그렇다면 거절된 상태가 있을 땐 신청이 계속 가능해야할 거 같습니다!
There was a problem hiding this comment.
기획에 요청해보니 거절된 후에도 재신청 가능하다고 합니다!
| ); | ||
| mentorApplicationRepository.save(mentorApplication); | ||
| } | ||
|
|
| (university_select_type = 'CATALOG' AND university_id IS NOT NULL) OR | ||
| (university_select_type = 'OTHER' AND university_id IS NULL) | ||
| ) | ||
| ) ENGINE=InnoDB No newline at end of file |
| } | ||
|
|
||
| @Test | ||
| void 대학_선택_타입이_CATALOG_인데_universityId가_null_이면_예외(){ |
There was a problem hiding this comment.
예외 관련 테스트명은 ~~하면_예외가_발생한다 입니다!
| Collections.unmodifiableSet(EnumSet.of(ExchangeStatus.STUDYING_ABROAD, ExchangeStatus.AFTER_EXCHANGE)); | ||
|
|
||
| public MentorApplication( | ||
| SiteUser siteUser, |
There was a problem hiding this comment.
크게 중요하지 않지만 이건 그냥 long siteUserId로 보내는 건 어떤가요? 어차피 id만 필요한 거 같아서요!
| // when | ||
| // then | ||
| assertThatCode(() -> mentorApplicationService.submitMentorApplication(user.getId(), request, file)) |
There was a problem hiding this comment.
// when & then 으로 통일해주실 수 있을까요?
| } | ||
|
|
||
| @Test | ||
| void 한_유저가_1회_이상_멘토_신청_요청을_보내면_예외() { |
There was a problem hiding this comment.
거절된 후에 재신청이 가능하다면 수정이 필요할 거 같습니다!
| MentorApplication existsMentorApplication = new MentorApplication( | ||
| user, | ||
| "US", | ||
| 1L, | ||
| UniversitySelectType.CATALOG, | ||
| "/mentor-proof.pdf", | ||
| ExchangeStatus.AFTER_EXCHANGE | ||
| ); | ||
| mentorApplicationRepository.save(existsMentorApplication); |
There was a problem hiding this comment.
MentorApplication과 관련된 Fixture 코드를 만들어보실 수 있을까요?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java (1)
88-102: 대학 선택 검증 로직이 명확합니다.다만,
UniversitySelectType이 enum이므로default케이스는 실제로 도달할 수 없는 코드입니다. enum에 새로운 값이 추가될 경우를 대비한 방어 코드로 보이지만, 현재 CATALOG와 OTHER만 존재하는 상황에서는 선택 사항입니다.필요하다면 아래와 같이 default 케이스를 제거할 수 있습니다:
private void validateUniversitySelection(UniversitySelectType universitySelectType, Long universityId) { switch (universitySelectType) { case CATALOG -> { if(universityId == null) { throw new CustomException(ErrorCode.UNIVERSITY_ID_REQUIRED_FOR_CATALOG); } } case OTHER -> { if(universityId != null) { throw new CustomException(ErrorCode.UNIVERSITY_ID_MUST_BE_NULL_FOR_OTHER); } } - default -> throw new CustomException(ErrorCode.INVALID_UNIVERSITY_SELECT_TYPE); } }하지만 미래에 enum 값이 추가될 가능성을 고려하면 현재 구현도 충분히 합리적입니다.
src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java (2)
58-74: 멘토 승격 신청 성공 시나리오가 잘 테스트되었습니다.정상 케이스(CATALOG + universityId 제공)를 명확하게 검증하고 있습니다.
다만, Line 73의 assertion을 좀 더 구체적으로 개선할 수 있습니다. 단순히
true인지 확인하는 대신, 실제로 저장된 엔티티를 조회하여 필드 값들을 검증하면 더 견고한 테스트가 됩니다.아래와 같이 개선할 수 있습니다:
// then -assertThat(mentorApplicationRepository.existsBySiteUserIdAndMentorApplicationStatusIn(user.getId(), List.of(MentorApplicationStatus.PENDING, MentorApplicationStatus.APPROVED))).isEqualTo(true); +assertThat(mentorApplicationRepository.existsBySiteUserIdAndMentorApplicationStatusIn(user.getId(), List.of(MentorApplicationStatus.PENDING, MentorApplicationStatus.APPROVED))).isTrue();또는 더 상세한 검증:
// then MentorApplication saved = mentorApplicationRepository.findBySiteUserId(user.getId()); assertThat(saved).isNotNull(); assertThat(saved.getMentorApplicationStatus()).isEqualTo(MentorApplicationStatus.PENDING); assertThat(saved.getUniversityId()).isEqualTo(universityId);
76-92: OTHER 타입의 성공 케이스도 잘 검증되었습니다.universityId가 null일 때의 처리를 확인하고 있어 좋습니다. 마찬가지로 assertion을
isTrue()로 간결하게 표현할 수 있습니다.src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixtureBuilder.java (1)
27-29: 빌더 초기화 메서드의 동작을 확인해주세요.
mentorApplication()메서드가 새로운MentorApplicationFixtureBuilder인스턴스를 생성하고 있습니다. 이는 현재 빌더의 상태를 버리는 동작입니다.사용 패턴을 보면
MentorApplicationFixture에서 다음과 같이 호출됩니다:mentorApplicationFixtureBuilder.mentorApplication() .siteUserId(...) .countryCode(...)이 경우
mentorApplicationFixtureBuilder는 주입받은 인스턴스이고,mentorApplication()은 새 인스턴스를 반환하므로 의도대로 동작합니다.다만, 이 메서드명은 일반적으로 static factory method에서 사용되는 패턴입니다. 현재 구조에서는 다음 중 하나로 개선할 수 있습니다:
옵션 1: static factory method로 변경
-public MentorApplicationFixtureBuilder mentorApplication() { - return new MentorApplicationFixtureBuilder(mentorApplicationRepository); -} +public static MentorApplicationFixtureBuilder mentorApplication(MentorApplicationRepository repository) { + return new MentorApplicationFixtureBuilder(repository); +}옵션 2: 메서드 제거하고 직접 생성자 호출
MentorApplicationFixture에서:new MentorApplicationFixtureBuilder(mentorApplicationRepository) .siteUserId(...)현재 구현도 동작하지만, 빌더 패턴의 명확성을 위해 검토를 권장합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(1 hunks)src/main/java/com/example/solidconnection/mentor/controller/MentorApplicationController.java(1 hunks)src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java(1 hunks)src/main/java/com/example/solidconnection/mentor/repository/MentorApplicationRepository.java(1 hunks)src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java(1 hunks)src/main/resources/db/migration/V35__add_mentor_application_table.sql(1 hunks)src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixture.java(1 hunks)src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixtureBuilder.java(1 hunks)src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/resources/db/migration/V35__add_mentor_application_table.sql
- src/main/java/com/example/solidconnection/common/exception/ErrorCode.java
- src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java
- src/main/java/com/example/solidconnection/mentor/controller/MentorApplicationController.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixture.java (2)
src/main/java/com/example/solidconnection/mentor/controller/MentorApplicationController.java (1)
RequiredArgsConstructor(16-32)src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixtureBuilder.java (1)
TestComponent(13-78)
src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixtureBuilder.java (1)
src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixture.java (1)
TestComponent(10-66)
⏰ 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 (13)
src/main/java/com/example/solidconnection/mentor/repository/MentorApplicationRepository.java (1)
8-11: LGTM! 리포지토리 구조가 깔끔합니다.Spring Data JPA의 쿼리 메서드 네이밍 규칙을 잘 따르고 있고, 중복 신청 체크를 위한 존재 여부 확인 메서드가 적절하게 정의되어 있습니다.
src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java (2)
27-33: DB 제약 조건과 애플리케이션 검증 로직의 일관성이 훌륭합니다.Hibernate의
@Check제약 조건을 통해 DB 레벨에서도 대학 선택 규칙을 강제하고 있어, 이중 방어가 잘 구현되어 있습니다.
66-67: 교환 상태 제한이 명확하게 구현되었습니다.
EnumSet을 사용한 불변 컬렉션으로 허용 상태를 관리하고 있어 효율적이고 안전합니다.src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java (6)
32-56: 테스트 설정이 체계적입니다.TestContainer를 사용한 통합 테스트 환경 구성과 픽스처 활용이 잘 되어 있습니다.
94-109: 검증 실패 시나리오가 정확하게 테스트되었습니다.CATALOG 타입인데 universityId가 null인 경우의 예외 처리를 명확하게 검증하고 있습니다.
111-126: OTHER 타입 검증 실패 케이스도 잘 구현되었습니다.universityId가 있으면 안 되는 경우를 정확히 테스트하고 있습니다.
128-158: 중복 신청 방지 로직이 철저하게 테스트되었습니다.PENDING과 APPROVED 상태의 중복 신청을 각각 테스트하고 있어 커버리지가 우수합니다.
160-178: 거절 후 재신청 가능 로직이 올바르게 구현되었습니다.REJECTED 상태인 경우 새로운 신청이 가능하다는 비즈니스 규칙을 정확하게 검증하고 있습니다.
180-196: 테스트 헬퍼 메서드들이 깔끔하게 구현되었습니다.MockMultipartFile과 MentorApplicationRequest 생성 로직을 별도 메서드로 분리하여 가독성과 재사용성을 높였습니다.
src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixture.java (2)
10-18: 테스트 픽스처 구조가 체계적입니다.상태별 편의 메서드를 제공하여 테스트 코드의 가독성을 높이고 있습니다. 기본값 상수를 사용한 것도 좋은 접근입니다.
20-65: 상태별 픽스처 메서드가 명확하게 구현되었습니다.PENDING, APPROVED, REJECTED 각 상태에 대한 테스트 데이터 생성 메서드가 잘 분리되어 있어, 테스트 시나리오별로 필요한 데이터를 쉽게 준비할 수 있습니다.
src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixtureBuilder.java (2)
66-77: 엔티티 생성 및 상태 설정이 적절하게 구현되었습니다.
ReflectionTestUtils를 사용하여 생성자에서 설정할 수 없는mentorApplicationStatus를 테스트 목적으로 변경하고 있습니다. 이는 테스트 픽스처에서 다양한 상태의 엔티티를 만들기 위한 합리적인 접근입니다.
19-64: 빌더 패턴이 깔끔하게 구현되었습니다.각 필드별 설정 메서드가
this를 반환하여 fluent API를 잘 구현하고 있으며, 기본값 설정도 적절합니다.
Gyuhyeok99
left a comment
Gyuhyeok99
left a comment
There was a problem hiding this comment.
고생하셨습니다! 간단한 리뷰사항들만 남아서 선 Approve드리겠습니다!
밑에 남긴 의견들만 반영해주세요!
| package com.example.solidconnection.mentor.domain; | ||
|
|
||
| public enum MentorApplicationStatus { | ||
| PENDING, |
| MENTORING_NOT_FOUND(HttpStatus.NOT_FOUND.value(), "해당 멘토링 신청을 찾을 수 없습니다."), | ||
| UNAUTHORIZED_MENTORING(HttpStatus.FORBIDDEN.value(), "멘토링 권한이 없습니다."), | ||
| MENTORING_ALREADY_CONFIRMED(HttpStatus.BAD_REQUEST.value(), "이미 승인 또는 거절된 멘토링입니다."), | ||
| MENTOR_APPLICATION_ALREADY_EXISTED(HttpStatus.CONFLICT.value(),"멘토 승격 신청이 존재하고 상태가 PENDING,APPROVE 일 때, 중복 신청 할 수 없습니다."), |
There was a problem hiding this comment.
이거 제가 알기로 사용자한테 에러얼럿창으로 나가는 메시지여서 이렇게까지 자세히 나갈 필요 없다고 생각합니다! 그래서 그냥 PENDING,APPROVE 일 때 이런 이야기는 없어도 될 거 같은데 어떻게 생각하시나요?
There was a problem hiding this comment.
아 사용자에게 바로 보여지는 정보라면 디테일한 정보는 숨기는게 좋겠네요!
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.boot.test.context.TestComponent; | ||
| import org.springframework.test.util.ReflectionTestUtils; | ||
|
|
| @@ -0,0 +1,197 @@ | |||
| package com.example.solidconnection.mentor.service; | |||
|
|
|||
|
|
|||
|
다음 PR에선 개행을 조금 더 신경 써보겠습니다!! |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixtureBuilder.java (1)
11-11: 개행 제거 권장이전 리뷰에서 지적된 불필요한 개행이 남아있습니다.
src/main/java/com/example/solidconnection/mentor/domain/MentorApplicationStatus.java (1)
5-5: 개행 제거 권장이전 리뷰에서 요청된 개행 제거가 아직 반영되지 않았습니다. enum 상수들 사이의 일관성을 위해 빈 줄을 제거해 주세요.
🧹 Nitpick comments (5)
src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixtureBuilder.java (1)
26-28: 빌더 패턴 구현 개선 필요현재
mentorApplication()메서드가 매번 새로운 인스턴스를 생성하여 다음과 같은 문제가 있습니다:
- 빌더의 상태가 유지되지 않아 재사용이 불가능합니다
- 인스턴스 필드(lines 18-24)의 초기값이 새 인스턴스에 반영되지 않습니다
두 가지 해결 방안을 제안합니다:
방안 1 (권장): 현재 인스턴스를 리셋하여 반환
public MentorApplicationFixtureBuilder mentorApplication() { - return new MentorApplicationFixtureBuilder(mentorApplicationRepository); + this.siteUserId = 0L; + this.countryCode = "US"; + this.universityId = null; + this.universitySelectType = UniversitySelectType.OTHER; + this.mentorProofUrl = "/mentor-proof.pdf"; + this.exchangeStatus = ExchangeStatus.AFTER_EXCHANGE; + this.mentorApplicationStatus = MentorApplicationStatus.PENDING; + return this; }방안 2: 정적 팩토리 메서드로 변경
-public MentorApplicationFixtureBuilder mentorApplication() { +public static MentorApplicationFixtureBuilder mentorApplication(MentorApplicationRepository repository) { - return new MentorApplicationFixtureBuilder(mentorApplicationRepository); + return new MentorApplicationFixtureBuilder(repository); }방안 2를 선택할 경우
MentorApplicationFixture에서 호출 방식도 수정이 필요합니다.src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java (3)
72-72: 가독성 개선을 위한 줄바꿈 권장assertion 라인이 너무 길어 가독성이 떨어집니다 (약 180자). 다음과 같이 개선할 수 있습니다:
-assertThat(mentorApplicationRepository.existsBySiteUserIdAndMentorApplicationStatusIn(user.getId(), List.of(MentorApplicationStatus.PENDING, MentorApplicationStatus.APPROVED))).isEqualTo(true); +assertThat(mentorApplicationRepository.existsBySiteUserIdAndMentorApplicationStatusIn( + user.getId(), + List.of(MentorApplicationStatus.PENDING, MentorApplicationStatus.APPROVED) +)).isTrue();추가로
.isEqualTo(true)대신.isTrue()를 사용하면 더 명확합니다.
90-90: 가독성 개선을 위한 줄바꿈 권장Line 72와 동일한 패턴으로 긴 assertion 라인을 개선해 주세요:
-assertThat(mentorApplicationRepository.existsBySiteUserIdAndMentorApplicationStatusIn(user.getId(),List.of(MentorApplicationStatus.PENDING, MentorApplicationStatus.APPROVED))).isEqualTo(true); +assertThat(mentorApplicationRepository.existsBySiteUserIdAndMentorApplicationStatusIn( + user.getId(), + List.of(MentorApplicationStatus.PENDING, MentorApplicationStatus.APPROVED) +)).isTrue();
176-176: 가독성 개선을 위한 줄바꿈 권장Line 72, 90과 동일한 패턴입니다:
-assertThat(mentorApplicationRepository.existsBySiteUserIdAndMentorApplicationStatusIn(user.getId(),List.of(MentorApplicationStatus.PENDING, MentorApplicationStatus.APPROVED))).isEqualTo(true); +assertThat(mentorApplicationRepository.existsBySiteUserIdAndMentorApplicationStatusIn( + user.getId(), + List.of(MentorApplicationStatus.PENDING, MentorApplicationStatus.APPROVED) +)).isTrue();src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1)
124-125: 사용자 대상 에러 메시지의 기술 용어 노출 검토 필요이전 리뷰에서 "사용자에게 바로 보여지는 정보라면 디테일한 정보는 숨기는게 좋겠다"는 피드백이 있었습니다.
현재 두 에러 메시지가 다음과 같은 기술적 세부사항을 포함하고 있습니다:
- Line 124:
UniversitySelectType,CATALOG,universityId같은 내부 구현 용어 노출- Line 125: 동일하게
UniversitySelectType,OTHER,universityId,null노출사용자 친화적인 메시지로 개선을 고려해 보세요:
-UNIVERSITY_ID_REQUIRED_FOR_CATALOG(HttpStatus.BAD_REQUEST.value(), "UniversitySelectType이 CATALOG이면 universityId가 필요합니다."), +UNIVERSITY_ID_REQUIRED_FOR_CATALOG(HttpStatus.BAD_REQUEST.value(), "카탈로그에서 대학을 선택하셨다면 대학 정보가 필요합니다."), -UNIVERSITY_ID_MUST_BE_NULL_FOR_OTHER(HttpStatus.BAD_REQUEST.value(), "UniversitySelectType이 OTHER이면 universityId가 null 이어야 합니다."), +UNIVERSITY_ID_MUST_BE_NULL_FOR_OTHER(HttpStatus.BAD_REQUEST.value(), "기타를 선택하신 경우 대학 정보를 입력하실 수 없습니다."),Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(1 hunks)src/main/java/com/example/solidconnection/mentor/domain/MentorApplicationStatus.java(1 hunks)src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixtureBuilder.java(1 hunks)src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixtureBuilder.java (1)
src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixture.java (1)
TestComponent(10-66)
⏰ 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 (3)
src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java (1)
57-177: 테스트 커버리지 및 로직 검증 완료멘토 승격 신청 기능의 핵심 시나리오들이 잘 구현되었습니다:
- 정상 케이스: CATALOG/OTHER 타입별 신청 등록 검증
- 유효성 검사: universityId 필수/null 제약 조건 검증
- 중복 방지: PENDING/APPROVED 상태에서 중복 신청 차단
- 재신청 허용: REJECTED 상태 이후 재신청 가능
특히 거절 후 재신청을 허용하는 비즈니스 로직(lines 160-177)이 명확하게 테스트되어 있어 좋습니다.
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1)
122-126: 에러 코드 추가 확인 완료멘토 승격 기능을 위한 5개의 에러 코드가 적절하게 추가되었습니다:
- 중복 신청 방지 (Line 122): CONFLICT 상태로 적절히 처리
- 교환학생 상태 검증 (Line 123): 명확한 메시지
- 대학 선택 타입 검증 (Lines 124-125): 비즈니스 규칙 반영
- 타입 유효성 (Line 126): 일반적인 검증 메시지
HTTP 상태 코드(CONFLICT, BAD_REQUEST)도 각 에러 상황에 적합하게 선택되었습니다.
src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixtureBuilder.java (1)
14-14: 검토 의견이 부정확함 - 수정 불필요저희가 검증한 결과, 원래 지적하신 오타는 실제로 코드베이스에 존재하지 않습니다.
변수명 확인 완료
- 검색 결과: snake_case 버전
mentor_applicationFixtureBuilder는 전혀 나타나지 않음- 실제 코드: 모든 참조에서 정확한 camelCase
mentorApplicationFixtureBuilder사용 중라인 번호 불일치
- 원래 지적: 30번, 47번 라인이라고 명시
- 실제 위치: 14번, 25번, 40번, 56번 라인에서만 해당 변수 존재
결론
- 현재 코드는 변수명이 올바르게 작성되어 있으며, 추가 수정이 필요하지 않습니다
Likely an incorrect or invalid review comment.
sukangpunch
force-pushed
the
feat/527-mentee-promotion-request
branch
from
94cc59d to
b7c1ca6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/example/solidconnection/mentor/controller/MentorApplicationController.java (1)
26-27: Multipart 요청에서 어노테이션을 통일해주세요.
- 일관성 문제 (Line 27):
mentorApplicationRequest는@RequestPart,file은@RequestParam을 사용하고 있습니다.- Multipart 요청에서는 일관성을 위해 둘 다
@RequestPart를 사용하는 것이 권장됩니다.다음과 같이 수정해주세요:
@PostMapping("/mentor-applications") public ResponseEntity<Void> requestMentorApplication( @AuthorizedUser long siteUserId, @Valid @RequestPart("mentorApplicationRequest") MentorApplicationRequest mentorApplicationRequest, - @RequestParam("file") MultipartFile file + @RequestPart("file") MultipartFile file ) { mentorApplicationService.submitMentorApplication(siteUserId, mentorApplicationRequest, file); return ResponseEntity.ok().build(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(1 hunks)src/main/java/com/example/solidconnection/mentor/controller/MentorApplicationController.java(1 hunks)src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java(1 hunks)src/main/java/com/example/solidconnection/mentor/domain/MentorApplicationStatus.java(1 hunks)src/main/java/com/example/solidconnection/mentor/repository/MentorApplicationRepository.java(1 hunks)src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java(1 hunks)src/main/resources/db/migration/V35__add_mentor_application_table.sql(1 hunks)src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixture.java(1 hunks)src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixtureBuilder.java(1 hunks)src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java
- src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java
- src/main/java/com/example/solidconnection/mentor/domain/MentorApplicationStatus.java
- src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixtureBuilder.java
- src/main/resources/db/migration/V35__add_mentor_application_table.sql
- src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java
- src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixture.java
⏰ 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 (1)
src/main/java/com/example/solidconnection/mentor/repository/MentorApplicationRepository.java (1)
8-10: LGTM! 리포지토리 구현이 적절합니다.Spring Data JPA 네이밍 컨벤션을 올바르게 따르고 있으며, 중복 신청 검증 로직에 적합한 exists 쿼리 메서드입니다.
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1)
123-123: 메시지 표현을 더 명확하게 다듬어보면 좋겠습니다.현재 "멘토 승격 지원 가능한 교환학생 상태가 아닙니다"는 이해 가능하지만, 이중 부정 구조로 인해 약간 어색하게 느껴질 수 있습니다.
다음과 같이 더 직접적인 표현으로 수정하는 것을 고려해보세요:
- INVALID_EXCHANGE_STATUS_FOR_MENTOR(HttpStatus.BAD_REQUEST.value(), "멘토 승격 지원 가능한 교환학생 상태가 아닙니다."), + INVALID_EXCHANGE_STATUS_FOR_MENTOR(HttpStatus.BAD_REQUEST.value(), "현재 교환학생 상태에서는 멘토 승격을 신청할 수 없습니다."),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(1 hunks)
🔇 Additional comments (1)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1)
122-126: 이전 피드백이 잘 반영되었습니다!기술적인 필드명(UniversitySelectType, universityId)을 숨기고 사용자 친화적인 메시지로 개선한 점이 좋습니다.
fix: PostRepository & CommentRepository의 일부 메서드의 정렬 순서 오류 해결 (#522) refactor: 이메일 유니크키 제거 refactor: 엔티티가 BaseEntity를 상속하도록 (#524) fix: BaseEntity 관련 마이그레이션 파일 수정 (#530) feat: 멘토 승격 api 구현 (#532) fix: 대학교 중복 오류 수정 (#510) refactor: 불필요한 로그 삭제 (#543) fix: 운영환경 8081 포트 설정 추가 (#542) fix: dev환경 디비명 변경 (#546) refactor: 불필요한 로그 삭제 (#547) refactor: 학기를 테이블로 관리하도록 변경 (#526) refactor: 모의지원 시 지원한 대학 정보 응답 추가 (#539)
2 participants
관련 이슈
작업 내용
멘토 승격 요청 api 를 구현 하였습니다.
제 생각대로 먼저 구현한 부분과 기획을 설명 듣고 개선한 부분을 커밋으로 나눠 놓았습니다.
기획 적용 내용은 다음과 같습니다.
1. ExchangeEnum 재사용 하기
2. 학교 선택 목록에 파견 학교가 존재하지 않을 시 '기타' 선택
3. 멘토 승격 요청은 취소하거나 수정 불가능
특이 사항
현재 서비스에서 Country 엔티티가 존재합니다. 해당 엔티티에는 code, koreaName, region 필드가 존재합니다. 예: ('BN', '브루나이', 'ASIA')
멘토 승격 지원 에서도 국가 정보를 받습니다.
국가 다음 학교인데 디자인 오타 인듯 합니다
여기서 프론트에서 어떤 api로 국가와 학교데이터를 가져와서 목록을 어떻게 리턴하는지와, 목록 기반으로 멘토 승격 요청 지원서에 어떠한 값을 저장 해야 하는지 명확하게 확신이 들지 않아 의견을 얻고 싶습니다!
리뷰 요구사항 (선택)
1. 멘토 승격 지원서가 가져야할 필드
멘토 승격 지원서에 universityId 와 countryCode 값을 받는 것으로 일단 구현을 해 놓았는데, Country 엔티티가 있다 보니 해당 엔티티를 물고 있어야 하나 고민입니다.
2. '기타' 처리에 대한 구현
서비스에서 제공하는 대학 중, 사용자의 대학이 존재하지 않을 때, 목록에서 기타를 선택하고, 해당 상태 기반으로 추후 대학 추가(어드민 기능)을 한다고 이해하고 있습니다.
이를 구현하기 위해 UniversitrySelectType 과 universityId 를 기반으로 제약을 구현 했는데 더 좋은 방식이 있는지 궁금합니다.