feat: 어드민 멘토 승격 지원서 승인/거절 기능, 상태 별 지원서 개수 조회 기능 추가 #577
Conversation
sukangpunch
requested review from
Gyuhyeok99,
Hexeong,
JAEHEE25,
lsy1307,
whqtker and
wibaek
as code owners
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
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: 1
🧹 Nitpick comments (2)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1)
132-132: 1) MENTOR_APPLICATION_ALREADY_CONFIRM 네이밍·메시지를 기존 패턴에 더 맞게 다듬는 것을 고려해 주세요.
기존 상수 MENTORING_ALREADY_CONFIRMED와 맞추려면 이름을 MENTOR_APPLICATION_ALREADY_CONFIRMED 정도로 정리하면 읽을 때 통일감이 더 좋아질 것 같습니다.
또한 사용자 메시지의 “요청 입니다.”는 “요청입니다.”로 붙여 쓰는 편이 자연스러우니, 아래처럼 문구만 먼저 정리해 두셔도 좋겠습니다.- MENTOR_APPLICATION_ALREADY_CONFIRM(HttpStatus.BAD_REQUEST.value(), "이미 승인 또는 거절된 멘토 승격 요청 입니다."), + MENTOR_APPLICATION_ALREADY_CONFIRM(HttpStatus.BAD_REQUEST.value(), "이미 승인 또는 거절된 멘토 승격 요청입니다."),src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java (1)
3-6: 1) 멘토 승격 승인/거절 도메인 메서드가 명확하지만, 핵심 규칙을 조금 더 강하게 표현할 여지가 있습니다.
PENDING 상태에서만 approve/reject를 허용하고, 이미 처리된 경우 MENTOR_APPLICATION_ALREADY_CONFIRM 예외를 던지는 흐름은 도메인 규칙을 잘 드러내고 있습니다.
다만 비즈니스 규칙상 APPROVED 상태의 MentorApplication은 universityId가 항상 채워져야 하므로, approve() 내부에서 universityId가 null이면 예외를 던지도록 한 번 더 방어해 두면 상태 일관성을 도메인 레벨에서 확실히 보장할 수 있습니다.
또한 reject(String rejectedReason)는 현재 DTO 검증에 의존하고 있지만, 향후 다른 호출 경로가 생길 가능성을 감안하면 rejectedReason가 null 또는 공백일 때 예외를 던지는 가벼운 방어 코드를 추가하는 것도 고려해 볼 만합니다.
Based on learnings, ...Also applies to: 124-138
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/com/example/solidconnection/admin/controller/AdminMentorApplicationController.java(3 hunks)src/main/java/com/example/solidconnection/admin/dto/MentorApplicationCountResponse.java(1 hunks)src/main/java/com/example/solidconnection/admin/dto/MentorApplicationRejectRequest.java(1 hunks)src/main/java/com/example/solidconnection/admin/service/AdminMentorApplicationService.java(2 hunks)src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(1 hunks)src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java(2 hunks)src/main/java/com/example/solidconnection/mentor/repository/MentorApplicationRepository.java(1 hunks)src/test/java/com/example/solidconnection/admin/service/AdminMentorApplicationServiceTest.java(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: whqtker
Repo: solid-connection/solid-connect-server PR: 362
File: src/main/java/com/example/solidconnection/mentor/service/MentoringQueryService.java:0-0
Timestamp: 2025-07-04T10:41:32.999Z
Learning: 멘토링 관련 기능에는 페이지네이션을 적용하지 않는 것이 해당 프로젝트의 설계 방침이다.
📚 Learning: 2025-11-17T06:30:49.502Z
Learnt from: sukangpunch
Repo: solid-connection/solid-connect-server PR: 562
File: src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java:10-23
Timestamp: 2025-11-17T06:30:49.502Z
Learning: MentorApplication 도메인에서 universityId는 null일 수 있으며, MentorApplicationRequest에서도 이 필드에 대한 NotNull validation을 추가하지 않아야 한다.
Applied to files:
src/main/java/com/example/solidconnection/common/exception/ErrorCode.javasrc/main/java/com/example/solidconnection/mentor/domain/MentorApplication.javasrc/main/java/com/example/solidconnection/admin/service/AdminMentorApplicationService.javasrc/main/java/com/example/solidconnection/admin/dto/MentorApplicationRejectRequest.javasrc/test/java/com/example/solidconnection/admin/service/AdminMentorApplicationServiceTest.java
📚 Learning: 2025-11-20T14:03:56.450Z
Learnt from: sukangpunch
Repo: solid-connection/solid-connect-server PR: 562
File: src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java:76-93
Timestamp: 2025-11-20T14:03:56.450Z
Learning: MentorApplication의 universityId는 PENDING 상태에서는 null일 수 있지만, admin이 승인(APPROVED)할 때 반드시 대학 데이터를 생성하고 universityId를 채운 후 승인하므로, APPROVED 상태의 MentorApplication은 항상 non-null universityId를 가진다는 것이 비즈니스 규칙이다.
Applied to files:
src/main/java/com/example/solidconnection/common/exception/ErrorCode.javasrc/main/java/com/example/solidconnection/mentor/domain/MentorApplication.javasrc/test/java/com/example/solidconnection/admin/service/AdminMentorApplicationServiceTest.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 (12)
src/main/java/com/example/solidconnection/admin/dto/MentorApplicationCountResponse.java (1)
3-7: 1) 멘토 지원서 상태별 카운트 DTO 정의가 단순·명확합니다.
레코드 필드 이름과 타입이 서비스/컨트롤러에서 사용하는 의미와 잘 맞습니다.
현재 요구사항 기준으로는 추가 생성자나 정적 팩토리 없이도 충분해 보여 그대로 유지하셔도 좋겠습니다.src/main/java/com/example/solidconnection/mentor/repository/MentorApplicationRepository.java (1)
16-16: 1) 상태별 카운트용 JPA 메서드 시그니처가 적절합니다.
파생 쿼리 메서드 이름이 mentorApplicationStatus 필드와 일관되고, long 반환 타입도 카운트 용도로 자연스럽습니다.
JPQL 없이도 요구사항을 충족하므로 레포지토리 복잡도를 늘리지 않는 좋은 선택으로 보입니다.src/main/java/com/example/solidconnection/admin/dto/MentorApplicationRejectRequest.java (1)
6-10: 1) 거절 사유 요청 DTO의 검증 범위가 적절합니다.
@notblank와 @SiZe(max = 200) 조합으로 필수 입력·길이 제한이 명확하게 정의되어 있습니다.
DB 기본 컬럼 길이(보통 255)보다 작은 최대 길이를 사용하므로 저장 시 제약과도 안전하게 호환될 것으로 보입니다.src/test/java/com/example/solidconnection/admin/service/AdminMentorApplicationServiceTest.java (4)
50-51: 테스트 검증을 위한 repository 의존성 추가가 적절합니다.승인/거절 후 실제 상태 변경을 확인하기 위해 repository를 직접 조회하는 것은 통합 테스트에서 올바른 접근입니다.
223-272: 승인 기능에 대한 포괄적인 테스트 커버리지입니다.다음 시나리오들을 체계적으로 검증하고 있습니다:
- 대기중인 지원서의 정상 승인 처리
- 이미 승인된 지원서에 대한 중복 승인 방어
- 이미 거절된 지원서에 대한 승인 시도 방어
- 존재하지 않는 지원서 처리
특히 Line 235-237에서 상태 변경과 approvedAt 타임스탬프를 모두 검증하는 것이 좋습니다.
274-327: 거절 기능에 대한 체계적인 테스트입니다.승인 테스트와 일관된 패턴으로 다음을 검증합니다:
- 대기중인 지원서의 정상 거절 처리
- 이미 처리된 지원서들에 대한 방어 로직
- 존재하지 않는 지원서 처리
Line 287-289에서 거절 사유(rejectedReason)를 함께 검증하는 것이 적절합니다.
329-361: 상태별 카운트 기능에 대한 명확한 테스트입니다.두 가지 시나리오를 검증합니다:
- 각 상태(APPROVED, PENDING, REJECTED)별 지원서 개수 집계
- 지원서가 없는 경우 모든 카운트가 0 반환
테스트가 직관적이고 검증 로직이 명확합니다.
src/main/java/com/example/solidconnection/admin/service/AdminMentorApplicationService.java (2)
42-48: 거절 기능의 구현이 명확하고 적절합니다.지원서를 조회하고 도메인 메서드에 거절 사유를 전달하는 로직이 간결합니다. 트랜잭션 처리와 에러 핸들링도 적절합니다.
50-61: 상태별 카운트 조회 방식이 합리적입니다.각 상태별로 개별 쿼리를 실행하는 방식을 선택하신 것에 대해:
- 장점: admin 패키지 DTO를 mentor 패키지 Repository에서 참조하지 않아 역방향 의존성 방지
- 장점: JPA 메서드 명명 규칙으로 유지보수성 향상
- 단점: 3번의 DB 쿼리 실행
멘토 지원서가 대량 데이터가 아니고 어드민 전용 기능이므로, 성능보다 의존성 구조를 우선한 판단이 적절해 보입니다.
src/main/java/com/example/solidconnection/admin/controller/AdminMentorApplicationController.java (3)
43-49: 승인 엔드포인트가 적절하게 구현되었습니다.POST 메서드를 사용한 상태 변경 API 설계가 RESTful 원칙에 부합합니다.
한 가지 확인사항: 현재 코드에 어드민 권한 검증 로직이 보이지 않습니다.
/admin/경로에 대한 권한 검증이 Spring Security 설정이나 인터셉터에서 처리되고 있는지 확인해주세요.
51-58: 거절 엔드포인트의 구현이 적절합니다.다음 설계가 좋습니다:
- PathVariable로 대상 지원서 식별
- RequestBody로 거절 사유 전달
- @Valid 어노테이션으로 입력값 검증
RESTful 설계 원칙에 부합하고 명확한 API입니다.
60-64: 카운트 조회 엔드포인트가 명확하게 구현되었습니다.GET 메서드를 사용한 조회 API로, 상태별 지원서 개수를 담은 DTO를 반환합니다. 설계가 직관적이고 적절합니다.
src/main/java/com/example/solidconnection/admin/service/AdminMentorApplicationService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1)
132-133: 1. 에러 코드 네이밍 일관성 및 오타 수정이 필요합니다.두 가지 개선 사항이 있습니다:
네이밍 일관성: Line 125의
MENTORING_ALREADY_CONFIRMED는 과거형 "CONFIRMED"를 사용하는데, 새로 추가된 코드는 "CONFIRM"을 사용하고 있습니다. 일관성을 위해MENTOR_APPLICATION_ALREADY_CONFIRMED로 수정하는 것이 좋습니다.메시지 오타: "멘토 승격 요청 입니다" → "멘토 승격 요청입니다" (불필요한 공백 제거)
- MENTOR_APPLICATION_ALREADY_CONFIRM(HttpStatus.BAD_REQUEST.value(), "이미 승인 또는 거절된 멘토 승격 요청 입니다."), + MENTOR_APPLICATION_ALREADY_CONFIRMED(HttpStatus.BAD_REQUEST.value(), "이미 승인 또는 거절된 멘토 승격 요청입니다."), MENTOR_APPLICATION_UNIVERSITY_NOT_SELECTED(HttpStatus.BAD_REQUEST.value(), "승인하려는 멘토 신청에 대학교가 선택되지 않았습니다."),src/test/java/com/example/solidconnection/admin/service/AdminMentorApplicationServiceTest.java (1)
224-285: 1. 멘토 승격 지원서 승인 테스트가 잘 구성되어 있습니다.테스트 케이스들이 비즈니스 규칙을 잘 반영하고 있습니다:
- 대기중 → 승인 정상 케이스
- 대학 미선택 시 예외 (learnings의 비즈니스 규칙과 일치)
- 이미 승인/거절된 경우 예외
- 존재하지 않는 지원서 예외
한 가지 작은 제안: Line 236, 300에서
Optional.get()대신orElseThrow()를 사용하면 테스트 실패 시 더 명확한 에러 메시지를 얻을 수 있습니다.// then - MentorApplication result = mentorApplicationRepository.findById(mentorApplication2.getId()).get(); + MentorApplication result = mentorApplicationRepository.findById(mentorApplication2.getId()) + .orElseThrow(() -> new AssertionError("MentorApplication should exist")); assertThat(result.getMentorApplicationStatus()).isEqualTo(MentorApplicationStatus.APPROVED);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/example/solidconnection/admin/service/AdminMentorApplicationService.java(2 hunks)src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(1 hunks)src/test/java/com/example/solidconnection/admin/service/AdminMentorApplicationServiceTest.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/example/solidconnection/admin/service/AdminMentorApplicationService.java
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: sukangpunch
Repo: solid-connection/solid-connect-server PR: 562
File: src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java:76-93
Timestamp: 2025-11-20T14:03:56.450Z
Learning: MentorApplication의 universityId는 PENDING 상태에서는 null일 수 있지만, admin이 승인(APPROVED)할 때 반드시 대학 데이터를 생성하고 universityId를 채운 후 승인하므로, APPROVED 상태의 MentorApplication은 항상 non-null universityId를 가진다는 것이 비즈니스 규칙이다.
Learnt from: sukangpunch
Repo: solid-connection/solid-connect-server PR: 562
File: src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java:10-23
Timestamp: 2025-11-17T06:30:49.502Z
Learning: MentorApplication 도메인에서 universityId는 null일 수 있으며, MentorApplicationRequest에서도 이 필드에 대한 NotNull validation을 추가하지 않아야 한다.
📚 Learning: 2025-11-17T06:30:49.502Z
Learnt from: sukangpunch
Repo: solid-connection/solid-connect-server PR: 562
File: src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java:10-23
Timestamp: 2025-11-17T06:30:49.502Z
Learning: MentorApplication 도메인에서 universityId는 null일 수 있으며, MentorApplicationRequest에서도 이 필드에 대한 NotNull validation을 추가하지 않아야 한다.
Applied to files:
src/test/java/com/example/solidconnection/admin/service/AdminMentorApplicationServiceTest.javasrc/main/java/com/example/solidconnection/common/exception/ErrorCode.java
📚 Learning: 2025-11-20T14:03:56.450Z
Learnt from: sukangpunch
Repo: solid-connection/solid-connect-server PR: 562
File: src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java:76-93
Timestamp: 2025-11-20T14:03:56.450Z
Learning: MentorApplication의 universityId는 PENDING 상태에서는 null일 수 있지만, admin이 승인(APPROVED)할 때 반드시 대학 데이터를 생성하고 universityId를 채운 후 승인하므로, APPROVED 상태의 MentorApplication은 항상 non-null universityId를 가진다는 것이 비즈니스 규칙이다.
Applied to files:
src/test/java/com/example/solidconnection/admin/service/AdminMentorApplicationServiceTest.javasrc/main/java/com/example/solidconnection/common/exception/ErrorCode.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 (2)
src/test/java/com/example/solidconnection/admin/service/AdminMentorApplicationServiceTest.java (2)
287-340: 2. 멘토 승격 지원서 거절 테스트가 체계적으로 작성되었습니다.거절 시나리오에 대한 테스트 커버리지가 충분합니다:
- 정상 거절 및 거절 사유 저장 검증
- 이미 처리된 지원서 재거절 방지
- 존재하지 않는 지원서 처리
rejectedReason필드 검증까지 포함되어 있어 데이터 무결성 확인이 잘 되어 있습니다.
342-374: Based on the verification performed, the review comment identified a legitimate architectural concern about test isolation. The verification revealed that:
Custom JUnit 5 extensions can implement AfterEachCallback to reset database state, which is the pattern used in this codebase via the custom
DatabaseClearExtension.To ensure tests are repeatable and isolated, it is a good practice to ensure tests always start with a clean state, and in integration testing this often means bringing the database to a well-known state.
The annotation does NOT include
@Transactional, instead relying on the customDatabaseClearExtensionextension for isolation.Given that @transactional is invaluable as it automatically rolls back database changes after each test, ensuring data isolation, and the project opts for a custom extension approach instead, the test isolation mechanism depends entirely on the
DatabaseClearExtensionimplementation.테스트 격리 메커니즘 확인 필요
DatabaseClearExtension이AfterEachCallback을 구현하여 각 테스트 후 적절히 정리하는지 확인이 필요합니다.@Transactional을 사용하지 않으므로, 커스텀 확장이 정확하게 데이터베이스를 정리해야만 테스트 격리가 보장됩니다.364번 줄의
deleteAll()호출은@BeforeEach로 설정되어 있어 테스트 실행 순서에 관계없이 각 테스트가 깨끗한 상태에서 시작하므로, 테스트 격리는DatabaseClearExtension의 구현에 따라 결정됩니다.현재 설정에서는
@Nested클래스의@BeforeEach초기화 및DatabaseClearExtension이 함께 작동하면 격리가 유지될 것으로 보이지만, 확장의 실제 동작을 확인하지 않았으므로 수동 검증을 권장합니다.
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
고생하셨습니다 ~ 잘못된 점은 없는 거 같습니다. 개인적 의견만 몇 개 달았습니다 👍
|
|
||
| public record MentorApplicationRejectRequest( | ||
| @NotBlank(message = "거절 사유는 필수입니다") | ||
| @Size(max = 200, message = "거절 사유는 200자를 초과할 수 없습니다") |
There was a problem hiding this comment.
디자인 보고 감으로 작성해 놨었습니다....
명확한 기획 내용 기반으로 작성 한 건 아니여서 빼는게 좋겠네요!
| public MentorApplicationCountResponse getMentorApplicationCount() { | ||
| long approvedCount = mentorApplicationRepository.countByMentorApplicationStatus(MentorApplicationStatus.APPROVED); | ||
| long pendingCount = mentorApplicationRepository.countByMentorApplicationStatus(MentorApplicationStatus.PENDING); | ||
| long rejectedCount = mentorApplicationRepository.countByMentorApplicationStatus(MentorApplicationStatus.REJECTED); |
There was a problem hiding this comment.
하나의 메서드 안에서 세 개의 쿼리를 실행하는데, 중간에 데이터가 변경되면 의도와 다른 개수가 나올 것 같습니다. 그룹바이로 하나의 쿼리로 될 거 같긴 합니다
There was a problem hiding this comment.
그렇겠네요, 도중에 상태가 변경된다면 값의 일관성이 깨질수도 있으니 jpql로 한번에 조회하는 식으로 수정하겠습니다!
There was a problem hiding this comment.
jpql 방식으로 조회를 시도 하면 Object[] 혹은, dto 리스트, queryDsl 을 활용하여 map 으로 받는 방식들이 존재하는데, 셋 다 비용이 있어 보여서 고민이 됩니다...
그리고 @transactional(readOnly = true) + mysql innoDB 의 REPEATABLE_READ 격리 수준 에선, 해당 트랜잭션 시작 시 스냅샷을 생성하고 트랜잭션 내의 쿼리들은 동일한 스냅샷을 읽는다고 합니다!
그래서 우려되는 문제는 발생하지 않지 않을까? 싶어서 답변 다시 드립니다!
@whqtker
There was a problem hiding this comment.
현 격리 수준에서 동일한 스냅샷을 읽기 때문에 정합성 깨짐 문제는 발생하지 않음 + 여러모로 JPQL을 통한 하나의 쿼리보다 현제 쿼리가 효율적인
말씀해주신 내용 전부 동의합니다 ~! 그대로 유지해도 될 거 같습니다.
| UNIVERSITY_ID_MUST_BE_NULL_FOR_OTHER(HttpStatus.BAD_REQUEST.value(), "기타 학교를 선택한 경우 학교 정보를 입력할 수 없습니다."), | ||
| INVALID_UNIVERSITY_SELECT_TYPE(HttpStatus.BAD_REQUEST.value(), "지원하지 않는 학교 선택 방식입니다."), | ||
| MENTOR_ALREADY_EXISTS(HttpStatus.BAD_REQUEST.value(), "이미 존재하는 멘토입니다."), | ||
| MENTOR_APPLICATION_ALREADY_CONFIRM(HttpStatus.BAD_REQUEST.value(), "이미 승인 또는 거절된 멘토 승격 요청 입니다."), |
There was a problem hiding this comment.
사소한 거긴 한데, CONFIRMED 가 더 나을 것 같습니다
* refactor: MENTOR_APPLICATION_ALREADY_CONFIRM -> MENTOR_APPLICATION_ALREADY_CONFIRMED 로 수정 * refactor: 멘토 지원서 거절 사유 관련하여 기획에 명시되지 않은 길이 제한 제거
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/com/example/solidconnection/admin/service/AdminMentorApplicationServiceTest.java (1)
224-285: 잘 구성된 승인 테스트입니다!
테스트 커버리지
- 대기중 승인, 대학 미선택, 이미 승인됨, 이미 거절됨, 존재하지 않음 - 모든 시나리오가 포함되었습니다.
상태 및 타임스탬프 검증 (Line 237-238)
- 상태 전환과
approvedAt설정 여부를 함께 검증하는 것이 좋습니다.선택적 개선 제안 (Line 236)
.get()대신.orElseThrow()를 사용하면 테스트 실패 시 더 명확한 메시지를 얻을 수 있습니다.- MentorApplication result = mentorApplicationRepository.findById(mentorApplication2.getId()).get(); + MentorApplication result = mentorApplicationRepository.findById(mentorApplication2.getId()) + .orElseThrow(() -> new AssertionError("승인된 멘토 지원서를 찾을 수 없습니다."));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/example/solidconnection/admin/dto/MentorApplicationRejectRequest.java(1 hunks)src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(1 hunks)src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java(2 hunks)src/test/java/com/example/solidconnection/admin/service/AdminMentorApplicationServiceTest.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/example/solidconnection/admin/dto/MentorApplicationRejectRequest.java
- src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: sukangpunch
Repo: solid-connection/solid-connect-server PR: 562
File: src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java:76-93
Timestamp: 2025-11-20T14:03:56.462Z
Learning: MentorApplication의 universityId는 PENDING 상태에서는 null일 수 있지만, admin이 승인(APPROVED)할 때 반드시 대학 데이터를 생성하고 universityId를 채운 후 승인하므로, APPROVED 상태의 MentorApplication은 항상 non-null universityId를 가진다는 것이 비즈니스 규칙이다.
Learnt from: sukangpunch
Repo: solid-connection/solid-connect-server PR: 562
File: src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java:10-23
Timestamp: 2025-11-17T06:30:49.502Z
Learning: MentorApplication 도메인에서 universityId는 null일 수 있으며, MentorApplicationRequest에서도 이 필드에 대한 NotNull validation을 추가하지 않아야 한다.
📚 Learning: 2025-11-17T06:30:49.502Z
Learnt from: sukangpunch
Repo: solid-connection/solid-connect-server PR: 562
File: src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java:10-23
Timestamp: 2025-11-17T06:30:49.502Z
Learning: MentorApplication 도메인에서 universityId는 null일 수 있으며, MentorApplicationRequest에서도 이 필드에 대한 NotNull validation을 추가하지 않아야 한다.
Applied to files:
src/test/java/com/example/solidconnection/admin/service/AdminMentorApplicationServiceTest.javasrc/main/java/com/example/solidconnection/common/exception/ErrorCode.java
📚 Learning: 2025-11-20T14:03:56.462Z
Learnt from: sukangpunch
Repo: solid-connection/solid-connect-server PR: 562
File: src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java:76-93
Timestamp: 2025-11-20T14:03:56.462Z
Learning: MentorApplication의 universityId는 PENDING 상태에서는 null일 수 있지만, admin이 승인(APPROVED)할 때 반드시 대학 데이터를 생성하고 universityId를 채운 후 승인하므로, APPROVED 상태의 MentorApplication은 항상 non-null universityId를 가진다는 것이 비즈니스 규칙이다.
Applied to files:
src/test/java/com/example/solidconnection/admin/service/AdminMentorApplicationServiceTest.javasrc/main/java/com/example/solidconnection/common/exception/ErrorCode.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 (4)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1)
132-133: LGTM! 에러 코드가 잘 추가되었습니다.
네이밍 일관성
- 기존
MENTORING_ALREADY_CONFIRMED(Line 125)와 동일한 패턴을 따르고 있습니다.HTTP 상태 코드
- 검증 실패에 대해
BAD_REQUEST를 사용하는 것이 적절합니다.메시지 명확성
- 에러 메시지가 사용자에게 문제 상황을 명확하게 전달합니다.
src/test/java/com/example/solidconnection/admin/service/AdminMentorApplicationServiceTest.java (3)
1-53: LGTM! Import 및 필드 주입이 적절하게 구성되었습니다.
정적 import
- 에러 코드와 AssertJ 헬퍼 메서드가 명확하게 import 되었습니다.
Repository 주입
- 테스트에서 상태 전환 검증을 위해
MentorApplicationRepository를 주입한 것은 좋은 접근입니다.
287-340: 거절 테스트도 잘 구성되었습니다!
거절 사유 검증 (Line 302)
- 거절 사유가 정확히 저장되는지 검증하는 것이 좋습니다.
예외 케이스 커버리지
- 이미 승인/거절된 지원서에 대한 거절 시도, 존재하지 않는 지원서 거절 시도 모두 포함되어 있습니다.
동일한 선택적 개선 (Line 300)
- 승인 테스트와 마찬가지로
.orElseThrow()사용을 고려해 볼 수 있습니다.
342-374: 상태별 개수 조회 테스트가 깔끔합니다!
정상 케이스 검증 (Lines 346-359)
- 각 상태별 개수가 정확히 일치하는지 검증합니다.
빈 데이터 케이스 (Lines 361-373)
deleteAll()후 모든 상태의 개수가 0인지 확인하는 것은 경계 조건 테스트로 적절합니다.트랜잭션 격리
@TestContainerSpringBootTest환경에서 트랜잭션 롤백이 테스트 격리를 보장하므로deleteAll()이 다른 테스트에 영향을 주지 않습니다.
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
변경사항 확인했습니다 !
어드민 기능 담당하신 분 리뷰까지 받고 머지하는 게 좋을 것 같습니다. 고생하셨습니다 !
| long approved, | ||
| long pending, | ||
| long rejected |
There was a problem hiding this comment.
일관성을 위해서 approvedCount, pendingCount, rejectedCount로 수정하면 좋을 것 같습니다!
Gyuhyeok99
left a comment
Gyuhyeok99
left a comment
There was a problem hiding this comment.
공채때문에 너무 바빠서 죄송했습니다.. 코드를 아주 깔끔하게 잘 작성해주시는군요!
요즘 업무처내느라 이런 깔끔한 코드를 못봤는데 반성하게되네요
간단한 코멘트하나 남겼습니다! 고생하셨습니다~
| assertThat(response.approved()).isEqualTo(expectedApprovedCount.size()); | ||
| assertThat(response.pending()).isEqualTo(expectedPendingCount.size()); | ||
| assertThat(response.rejected()).isEqualTo(expectedRejectedCount.size()); | ||
| } |
There was a problem hiding this comment.
이런 것들은 assertAll로 묶어주시면 감사하겠습니다!
| assertThat(response.approved()).isEqualTo(0L); | ||
| assertThat(response.pending()).isEqualTo(0L); | ||
| assertThat(response.rejected()).isEqualTo(0L); |
There was a problem hiding this comment.
이것도요! 그래야 ci 실패했을 때 디버깅하기 편해지더라구요
- refactor: 변수명, 필드명 일관성 맞추기 - test: assertAll 적용
sukangpunch
force-pushed
the
feat/575-admin-mentor-application-approval
branch
from
d239f5b to
c45a67c
Compare
4 participants
관련 이슈
작업 내용
멘토 승격 지원서를 어드민이 승인/거절 하는 기능을 추가하였습니다.
멘토 승격 지원서들의 상태(PENDDING, APPROVED, REJECTED) 별 지원서 개수를 조회합니다.
특이 사항
멘토 승격 지원서의 COUNT를 가져올 때 JPQL 로 한번에 가져오는 방법과 JPA 메서드를 활용하여 상태 별로 따로 조회 하는 방법 을 생각 했었는데, jpa 메서드를 활용하여 상태 별로 따로 조회 하도록 하였습니다.
이유
추후 브루노 작성도 하겠습니다!
리뷰 요구사항 (선택)