feat: 지원서가 APPROVED 인 유저의 멘토 생성 기능 추가 #562
Conversation
sukangpunch
requested review from
Gyuhyeok99,
Hexeong,
JAEHEE25,
lsy1307,
whqtker and
wibaek
as code owners
Walkthrough이 풀 리퀘스트는 멘토 마이페이지 생성 기능을 추가하는 기능 단위의 변경입니다. 주요 내용은 다음과 같습니다.
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60-75 minutes 주의가 필요한 영역:
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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: 4
🧹 Nitpick comments (2)
src/main/java/com/example/solidconnection/mentor/dto/MentorMyPageCreateRequest.java (1)
7-18: 새로운 DTO가 추가되었습니다. 코드 중복을 고려해주세요.
MentorMyPageCreateRequest가MentorMyPageUpdateRequest와 완전히 동일한 구조를 가지고 있습니다:
- 동일한 필드:
introduction,passTip,channels- 동일한 validation 규칙
개선 제안:
- 옵션 1 (권장): 두 DTO를 하나로 통합하고
MentorMyPageRequest같은 이름을 사용합니다.- 옵션 2: 공통 필드를 추출하여 베이스 레코드나 인터페이스를 만듭니다.
- 옵션 3 (현상유지): Create와 Update의 의미론적 분리가 중요하고, 향후 각각 다른 필드가 추가될 가능성이 높다면 현재 구조를 유지합니다.
현재 PR에서는 기능 구현에 집중하고, 필요시 별도 리팩토링 이슈로 처리하는 것도 좋습니다.
만약 통합을 선택한다면:
public record MentorMyPageRequest( @NotBlank(message = "자기소개를 입력해주세요.") String introduction, @NotBlank(message = "합격 레시피를 입력해주세요.") String passTip, @Valid List<ChannelRequest> channels ) { }그리고 컨트롤러에서
MentorMyPageCreateRequest와MentorMyPageUpdateRequest대신 이것을 사용합니다.src/main/java/com/example/solidconnection/mentor/domain/Mentor.java (1)
56-68: 새로운 생성자가 추가되었습니다.5개 매개변수를 받는 생성자가 추가되었습니다:
introduction,passTip,siteUserId,universityId,termId관찰사항:
universityId가Long(nullable) 타입입니다. 이는 "기타 학교" 선택 시 null이 될 수 있음을 의미하며, 비즈니스 로직상 적절합니다.id,menteeCount,hasBadge는 자동으로 초기화됩니다.channels는 필드 초기화 (line 54)에서 빈 리스트로 설정됩니다.선택적 개선사항:
매개변수에 대한 null 체크를 추가하여 방어적 프로그래밍을 강화할 수 있습니다 (특히introduction,passTip은 DB에서 NOT NULL이므로). 하지만 현재 코드도 JPA 제약조건에 의해 보호되므로 큰 문제는 없습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(3 hunks)src/main/java/com/example/solidconnection/mentor/controller/MentorMyPageController.java(3 hunks)src/main/java/com/example/solidconnection/mentor/domain/Mentor.java(2 hunks)src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java(5 hunks)src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java(1 hunks)src/main/java/com/example/solidconnection/mentor/dto/MentorMyPageCreateRequest.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(3 hunks)src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java(4 hunks)src/main/java/com/example/solidconnection/term/repository/TermRepository.java(1 hunks)src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixture.java(5 hunks)src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixtureBuilder.java(3 hunks)src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java(3 hunks)src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-11T14:39:49.462Z
Learnt from: Gyuhyeok99
Repo: solid-connection/solid-connect-server PR: 443
File: src/main/resources/db/migration/V28__alter_mentor_introduction_pass_tip_not_null.sql:1-5
Timestamp: 2025-08-11T14:39:49.462Z
Learning: The mentor table currently has no data in production/staging environment, so NULL backfill operations are not needed when adding NOT NULL constraints to the introduction and pass_tip columns in the mentor table.
Applied to files:
src/main/java/com/example/solidconnection/mentor/domain/Mentor.java
📚 Learning: 2025-08-02T21:44:08.950Z
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 426
File: src/main/java/com/example/solidconnection/mentor/domain/Mentor.java:80-80
Timestamp: 2025-08-02T21:44:08.950Z
Learning: Mentor.updateChannels() 메서드에서 채널 삭제 시 originalChannelSize를 메서드 시작 시점에 고정하여 사용하므로, 반복문 내에서 리스트 크기가 변경되어도 올바른 수의 채널이 삭제된다. i < originalChannelSize 조건으로 넘치는 채널을 정확히 제거할 수 있다.
Applied to files:
src/main/java/com/example/solidconnection/mentor/domain/Mentor.java
📚 Learning: 2025-07-05T17:54:42.475Z
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 375
File: src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java:47-53
Timestamp: 2025-07-05T17:54:42.475Z
Learning: MentorMyPageService에서 PUT 메서드 구현 시 전체 채널을 새로 생성하여 교체하는 방식을 사용하는 것이 PUT의 의미론적 특성과 일치하며, 트랜잭션 로킹 관점에서도 합리적인 접근이다.
Applied to files:
src/main/java/com/example/solidconnection/mentor/controller/MentorMyPageController.javasrc/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java
🧬 Code graph analysis (2)
src/main/java/com/example/solidconnection/mentor/dto/MentorMyPageCreateRequest.java (1)
src/main/java/com/example/solidconnection/mentor/dto/MentorMyPageUpdateRequest.java (1)
MentorMyPageUpdateRequest(8-18)
src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java (1)
src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java (1)
TestContainerSpringBootTest(33-204)
🔇 Additional comments (4)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1)
53-53: 새로운 에러 코드가 적절하게 추가되었습니다.두 가지 에러 코드가 추가되었습니다:
MENTOR_APPLICATION_NOT_FOUND(404): 멘토 지원서를 찾을 수 없는 경우MENTOR_ALREADY_EXISTS(400): 이미 멘토가 존재하는 경우HTTP 상태 코드와 메시지가 적절하게 설정되어 있습니다.
Also applies to: 130-130
src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixture.java (1)
7-7: 테스트 픽스처가 term 지원을 위해 올바르게 업데이트되었습니다.변경사항:
TermFixture의존성 추가- 세 가지 빌더 메서드에
termId설정 추가:
대기중_멘토신청승인된_멘토신청거절된_멘토신청모든 메서드가 일관되게
termFixture.현재_학기("2025-1").getId()를 사용하여 termId를 설정하고 있습니다.Also applies to: 16-16, 33-33, 49-49, 66-66
src/main/java/com/example/solidconnection/term/repository/TermRepository.java (1)
11-11:findByName메서드는 문제없이 정상 구현되었습니다.검증을 완료했으니 원래 우려사항들을 정리해드립니다:
Unique 제약조건 확인됨
Term 엔티티에uk_term_name이라는 이름의 unique constraint가name컬럼에 적용되어 있으므로, 중복된 name은 데이터베이스 수준에서 방지됩니다.Optional 반환 타입이 적절함
name 필드가 unique하므로findByName은 최대 1개의 결과만 반환하게 되며, Optional으로 감싸는 것이 정확한 설계입니다.구현 방식이 안전함
Spring Data JPA가 자동으로 생성하는 쿼리는 name이 unique하다는 데이터베이스 제약조건을 존중하므로, 예상치 못한 동작이 발생할 위험이 없습니다.src/main/java/com/example/solidconnection/mentor/repository/MentorApplicationRepository.java (1)
13-13: 메서드 사용 패턴 분석 결과 추가 조치가 필요하지 않습니다.초기 검토에서 지적된 여러 결과 반환 가능성에 대한 우려를 검증한 결과, 다음과 같은 이유로 현재 구현이 안전함을 확인했습니다:
비즈니스 로직에 의한 보호
ensureNoPendingOrApprovedMentorApplication()메서드가 PENDING/APPROVED 상태의 중복 지원서를 사전에 차단하고 있습니다- 새로운 메서드는 APPROVED 상태만으로 호출되며, 이 상태는 유저당 최대 1개로 보장됩니다
데이터베이스 무결성
- 만약 DB에서 예상치 못하게 여러 건이 발생한다면, Spring Data JPA는
IncorrectResultSizeDataAccessException예외를 발생시키므로 조용한 실패가 없습니다현재 사용 패턴
MentorMyPageService에서 APPROVED 상태로만 호출되며.orElseThrow()로 감싸져 있어 명시적 예외 처리를 하고 있습니다코드 변경이 불필요합니다.
| @RequireRoleAccess(roles = Role.MENTOR) | ||
| @PostMapping | ||
| public ResponseEntity<Void> createMentorMyPage( | ||
| @AuthorizedUser long siteUserId, | ||
| @Valid @RequestBody MentorMyPageCreateRequest request | ||
| ) { | ||
| mentorMyPageService.createMentorMyPage(siteUserId, request); | ||
| return ResponseEntity.ok().build(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
역할 기반 접근 제어에 잠재적 문제가 있습니다.
이 엔드포인트는 @RequireRoleAccess(roles = Role.MENTOR)를 사용하고 있는데, 이는 모순을 야기합니다:
문제:
- 멘토 마이페이지를 생성하는 엔드포인트인데, 이미
MENTOR역할을 가진 사용자만 접근할 수 있습니다. - PR 목표에 따르면, "지원서가 APPROVED 인 유저의 멘토 생성"이므로, 아직 MENTOR가 아닌 사용자가 이 엔드포인트를 호출해야 합니다.
고려사항:
- 옵션 1 (권장): 역할 체크를 제거하거나 다른 역할(예:
Role.USER)로 변경하고, 서비스 레이어에서 APPROVED 지원서 존재 여부를 확인합니다. - 옵션 2: 어드민이 승인 시 자동으로 역할을 MENTOR로 변경한 후, 사용자가 이 엔드포인트를 호출하여 마이페이지를 완성하는 플로우인 경우, 현재 설계가 맞을 수 있습니다. 하지만 이 경우 엔드포인트명이
createMentorMyPage가 아니라completeMentorMyPage같은 이름이 더 적절합니다.
현재 비즈니스 플로우를 명확히 하고, 그에 맞게 접근 제어를 조정해주세요.
현재 플로우를 확인하기 위해 다음을 검토해주세요:
🏁 Script executed:
#!/bin/bash
# Description: Verify the role assignment flow
# Check where Role.MENTOR is assigned
rg -n "Role\.MENTOR|setRole.*MENTOR|role.*=.*MENTOR" --type=java -C3
# Check MentorMyPageService.createMentorMyPage implementation
ast-grep --pattern $'public $_ createMentorMyPage($$$) {
$$$
}'Length of output: 11628
🏁 Script executed:
#!/bin/bash
# Find MentorMyPageService implementation
fd -e java "MentorMyPageService" --exec grep -l "createMentorMyPage"
# Check service implementation
rg -n "createMentorMyPage" --type=java -A 10 -B 2
# Search for where roles are actually assigned/updated
rg -n "\.setRole\(|updateRole|Role\.MENTOR" --type=java -g '!**/test/**' -g '!**/fixture/**' -A 2
# Look for mentor application approval flow
fd -e java ".*Application.*" | head -20
rg -n "APPROVED|application.*approved" --type=java -g '!**/test/**' -i -C 2Length of output: 34228
🏁 Script executed:
#!/bin/bash
# Search for mentor application approval logic
rg -n "MentorApplicationService|approveMentorApplication|approveApplication" --type=java -A 5 -B 2
# Look for any place where role is updated/assigned
rg -n "siteUser\.setRole|updateRole|\.role\s*=" --type=java -g '!**/test/**' -g '!**/fixture/**' -A 2 -B 2
# Check if there's a separate Admin/Approval controller
fd -e java ".*Approval.*|.*Admin.*" --type=java | grep -i mentorLength of output: 6720
@RequireRoleAccess 데코레이터가 서비스 로직과 모순됩니다 — 엔드포인트에 도달 불가능한 상태입니다.
코드베이스를 검증한 결과, 핵심 문제가 확인되었습니다.
1. 역할 할당 코드가 존재하지 않음
- 프로덕션 코드 어디에도 사용자 역할을
MENTOR으로 업데이트하는 로직이 없습니다. MentorMyPageService.createMentorMyPage()는 Mentor 엔티티만 생성하며 역할을 변경하지 않습니다.MentorApplicationService에서도 신청 승인 시 역할 할당 코드가 없습니다.
2. 해결 불가능한 접근 제어 체크
컨트롤러: @RequireRoleAccess(roles = Role.MENTOR) ← 사용자가 이미 MENTOR 역할을 가져야 함
서비스: MentorApplicationStatus.APPROVED 확인 ← 승인된 신청만 허용
결과: 역할 할당 코드가 없으므로 누구도 접근 불가
필수 조치:
-
멘토 신청 승인 시 역할 업데이트
MentorApplicationService또는 별도의 admin 승인 서비스에서 신청 승인 시 사용자 역할을Role.MENTOR으로 변경하세요.
-
컨트롤러 가드 재검토
- 역할 할당 후에 이 엔드포인트를 호출하도록 의도했다면, 현재 설계가 맞습니다.
- 하지만 역할을 먼저 확인하고 나서 멘토 마이페이지를 생성하는 흐름이라는 점을 명확히 문서화하세요.
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/mentor/controller/MentorMyPageController.java
around lines 46 to 54, the @RequireRoleAccess(roles = Role.MENTOR) guard makes
this endpoint unreachable because nowhere in the codebase users are assigned
Role.MENTOR (MentorMyPageService.createMentorMyPage only creates Mentor entity
and MentorApplicationService does not update roles). Fix by adding role
assignment when a mentor application is approved (update the user's roles to
include Role.MENTOR in MentorApplicationService or the admin approval flow)
and/or remove or adjust the controller guard to match the actual flow (e.g.,
require approved application status instead of MENTOR role) and document the
intended sequence so callers first get the role then call this endpoint.
There was a problem hiding this comment.
해당 기능은, 아직 admin에서 멘토 지원서 approve 기능을 추가하지 않았기 때문이야, 다음 pr에 해당 내용을 추가할 예정
There was a problem hiding this comment.
@sukangpunch, 명확한 설명 감사합니다! 단계적 구현 방식을 이해했습니다.
현재 상황 확인:
- 이번 PR: 멘토 마이페이지 생성 엔드포인트 추가 (역할 가드 포함)
- 다음 PR: 어드민 승인 기능 +
Role.MENTOR할당 로직 추가
다음 PR에서 확인해주세요:
- 멘토 지원서를 APPROVED로 변경할 때, 해당 사용자의 역할을
Role.MENTOR로 업데이트하는 로직이 포함되어야 합니다. - 그래야 승인받은 사용자가 이 엔드포인트를 호출하여 마이페이지를 생성할 수 있습니다.
현재 설계는 최종 플로우를 고려한 것으로 보이므로, 다음 PR에서 역할 할당 로직만 추가되면 완벽하게 동작할 것 같습니다! 👍
| public void createChannels(List<Channel> channels) { | ||
| for(Channel channel : channels) { | ||
| channel.updateMentor(this); | ||
| this.channels.add(channel); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
createChannels 메서드가 추가되었습니다. Null 안전성을 고려해주세요.
새로운 createChannels 메서드는 채널 리스트를 받아 멘토와 연결하고 내부 리스트에 추가합니다.
잠재적 이슈:
channels 매개변수가 null인 경우 NullPointerException이 발생합니다.
Null 체크를 추가하여 안전성을 높이는 것을 권장합니다:
public void createChannels(List<Channel> channels) {
+ if (channels == null) {
+ return; // 또는 throw new IllegalArgumentException("channels must not be null");
+ }
for(Channel channel : channels) {
channel.updateMentor(this);
this.channels.add(channel);
}
}또는 더 간결하게:
public void createChannels(List<Channel> channels) {
+ if (channels == null || channels.isEmpty()) {
+ return;
+ }
for(Channel channel : channels) {
channel.updateMentor(this);
this.channels.add(channel);
}
}📝 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 void createChannels(List<Channel> channels) { | |
| for(Channel channel : channels) { | |
| channel.updateMentor(this); | |
| this.channels.add(channel); | |
| } | |
| } | |
| public void createChannels(List<Channel> channels) { | |
| if (channels == null) { | |
| return; | |
| } | |
| for(Channel channel : channels) { | |
| channel.updateMentor(this); | |
| this.channels.add(channel); | |
| } | |
| } |
| public void createChannels(List<Channel> channels) { | |
| for(Channel channel : channels) { | |
| channel.updateMentor(this); | |
| this.channels.add(channel); | |
| } | |
| } | |
| public void createChannels(List<Channel> channels) { | |
| if (channels == null || channels.isEmpty()) { | |
| return; | |
| } | |
| for(Channel channel : channels) { | |
| channel.updateMentor(this); | |
| this.channels.add(channel); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/example/solidconnection/mentor/domain/Mentor.java around
lines 100 to 105, the new createChannels(List<Channel> channels) method can
throw a NullPointerException if the channels parameter is null; add a
null-safety guard such as returning immediately when channels is null or
replacing it with Collections.emptyList(), then iterate over the safe list and
for each channel call channel.updateMentor(this) and add to this.channels;
ensure the method handles an empty list gracefully and does not modify state
when input is null.
There was a problem hiding this comment.
프론트에서 무조건 리스트를 보내준다 가정 해도 null 체크를 하는것이 좋아?
There was a problem hiding this comment.
@sukangpunch, 좋은 질문입니다. 프론트엔드에서 무조건 리스트를 보낸다고 해도 null 체크를 하는 것이 더 안전합니다.
Null 체크를 권장하는 이유:
-
다양한 호출 경로: 이 메서드는 컨트롤러를 통한 API 호출뿐만 아니라, 백엔드 내부의 다른 서비스나 테스트 코드에서도 호출될 수 있습니다.
-
미래의 변경사항: 나중에 코드가 리팩토링되거나 새로운 기능이 추가될 때, 다른 경로에서 이 메서드를 호출할 수 있습니다.
-
방어적 프로그래밍: 도메인 엔티티의 public 메서드는 잘못된 입력으로부터 자신을 보호하는 것이 좋습니다.
대안:
Null 체크가 과하다고 생각되신다면, 서비스 레이어에서 검증하거나 @NonNull 어노테이션을 사용하는 방법도 있습니다:
public void createChannels(@NonNull List<Channel> channels) {
for(Channel channel : channels) {
channel.updateMentor(this);
this.channels.add(channel);
}
}다만 도메인 엔티티 레벨에서 간단한 null 체크를 추가하는 것이 가장 안전하고 명시적입니다. 최종 결정은 팀의 코딩 컨벤션에 따라 하시면 됩니다!
🧠 Learnings used
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 426
File: src/main/java/com/example/solidconnection/mentor/domain/Mentor.java:80-80
Timestamp: 2025-08-02T21:44:08.950Z
Learning: Mentor.updateChannels() 메서드에서 채널 삭제 시 originalChannelSize를 메서드 시작 시점에 고정하여 사용하므로, 반복문 내에서 리스트 크기가 변경되어도 올바른 수의 채널이 삭제된다. i < originalChannelSize 조건으로 넘치는 채널을 정확히 제거할 수 있다.
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 375
File: src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java:47-53
Timestamp: 2025-07-05T17:54:42.475Z
Learning: MentorMyPageService에서 PUT 메서드 구현 시 전체 채널을 새로 생성하여 교체하는 방식을 사용하는 것이 PUT의 의미론적 특성과 일치하며, 트랜잭션 로킹 관점에서도 합리적인 접근이다.
src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java
Show resolved
Hide resolved
src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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(3 hunks)src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java(1 hunks)src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java(4 hunks)src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/example/solidconnection/common/exception/ErrorCode.java
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 375
File: src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java:47-53
Timestamp: 2025-07-05T17:54:42.475Z
Learning: MentorMyPageService에서 PUT 메서드 구현 시 전체 채널을 새로 생성하여 교체하는 방식을 사용하는 것이 PUT의 의미론적 특성과 일치하며, 트랜잭션 로킹 관점에서도 합리적인 접근이다.
📚 Learning: 2025-07-05T17:54:42.475Z
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 375
File: src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java:47-53
Timestamp: 2025-07-05T17:54:42.475Z
Learning: MentorMyPageService에서 PUT 메서드 구현 시 전체 채널을 새로 생성하여 교체하는 방식을 사용하는 것이 PUT의 의미론적 특성과 일치하며, 트랜잭션 로킹 관점에서도 합리적인 접근이다.
Applied to files:
src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java
📚 Learning: 2025-08-02T21:44:08.950Z
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 426
File: src/main/java/com/example/solidconnection/mentor/domain/Mentor.java:80-80
Timestamp: 2025-08-02T21:44:08.950Z
Learning: Mentor.updateChannels() 메서드에서 채널 삭제 시 originalChannelSize를 메서드 시작 시점에 고정하여 사용하므로, 반복문 내에서 리스트 크기가 변경되어도 올바른 수의 채널이 삭제된다. i < originalChannelSize 조건으로 넘치는 채널을 정확히 제거할 수 있다.
Applied to files:
src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java
🧬 Code graph analysis (1)
src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java (1)
src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java (1)
TestContainerSpringBootTest(33-204)
⏰ 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 (7)
src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java (1)
6-7: 필요한 validation import가 올바르게 추가되었습니다.아래 레코드 필드에서 사용하는
@NotBlank,@NotNull어노테이션을 위한 import 문이 정확하게 추가되었습니다.src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java (3)
3-33: 테스트 준비 작업이 탄탄합니다.새로운 에러 코드 import와 Term, MentorApplicationFixture 설정이 잘 구성되어 있고, BeforeEach에서 테스트에 필요한 term과 siteUser를 적절히 초기화하고 있습니다.
179-215: 멘토 생성 성공 케이스 검증이 꼼꼼합니다.
- 지원서 승인 상태를 먼저 생성하고
- 멘토 생성을 수행한 뒤
- 멘토 정보와 채널 정보를 모두 검증하는 흐름이 명확합니다.
Line 198의
createMentor.getId()는 이전 리뷰 코멘트에서 지적된 부분이 이미 수정되어 올바르게 동작하고 있습니다.
217-280: 예외 상황 테스트 커버리지가 훌륭합니다.
- 멘토 정보 중복 생성 차단 (MENTOR_ALREADY_EXISTS)
- 채널 등록 제한 초과 검증 (CHANNEL_REGISTRATION_LIMIT_EXCEEDED)
- 멘토 지원서 부재 검증 (MENTOR_APPLICATION_NOT_FOUND)
- REJECTED 상태 지원서 차단
- PENDING 상태 지원서 차단
모든 예외 경로가 적절히 테스트되어 있어 신뢰할 수 있는 코드입니다.
src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java (3)
77-94: 멘토 생성 로직이 요구사항을 정확히 구현했습니다.
- 사용자 검증 (존재 여부, 멘토 중복 여부)
- 채널 등록 제한 검증
- APPROVED 상태의 지원서 조회
- 지원서 데이터로 멘토 생성
- 채널 생성 및 저장
각 단계가 명확하게 분리되어 있고, 지원서의 universityId와 termId를 활용해 멘토를 생성하는 흐름이 깔끔합니다.
72-75: 채널 생성/수정 로직이 명확하게 분리되었습니다.
updateChannel은 기존 멘토의 채널을 수정할 때mentor.updateChannels()호출createChannels는 신규 멘토의 채널을 생성할 때mentor.createChannels()호출- 공통 로직인
buildChannels를 추출하여 중복 제거각 메서드의 역할이 분명하고, 학습된 내용에 따르면 채널 전체 교체 방식이 의미론적으로도 적절합니다.
Based on learnings
Also applies to: 116-128
106-114: DTO의 channels 필드에는 @NotNull이 없으므로, 리뷰 코멘트의 전제가 맞지 않습니다.
검증 결과
MentorMyPageCreateRequest의channels필드는@Valid어노테이션만 있고@NotNull이 없습니다.@Valid는 중첩된 객체들을 검증하기만 할 뿐 null 체크는 하지 않습니다.null 검증의 필요성
validateChannelRegistrationLimit()메서드의 null 체크는 중복이 아니라 필수적인 방어 로직입니다.- 서비스 계층에서 null 가능성에 대한 명시적인 검증을 하는 것이 좋은 설계입니다.
- 현재 구현은 유지하셔도 됩니다.
Likely an incorrect or invalid review comment.
| @NotNull(message = "교환 상태를 입력해주세요.") | ||
| @JsonProperty("preparationStatus") | ||
| ExchangeStatus exchangeStatus, | ||
|
|
||
| @NotNull(message = "대학교 선택 유형을 입력해주세요.") | ||
| UniversitySelectType universitySelectType, | ||
|
|
||
| @NotNull(message = "국가를 입력해주세요") | ||
| String country, | ||
| Long universityId | ||
|
|
||
| Long universityId, | ||
|
|
||
| @NotBlank(message = "학기를 입력해주세요.") | ||
| String term |
There was a problem hiding this comment.
🧩 Analysis chain
Validation 어노테이션이 적절히 추가되었습니다. universityId 검증 여부를 확인해주세요.
변경사항을 정리하면 다음과 같습니다:
-
이전 리뷰 코멘트 반영 완료
term필드에@NotBlank어노테이션이 추가되어 이전 리뷰 요청사항이 해결되었습니다.
-
기존 필드에 validation 추가
exchangeStatus:@NotNull추가 ✓universitySelectType:@NotNull추가 ✓country:@NotNull추가 ✓
-
신규 필드 validation
term:@NotBlank추가 ✓
다만, universityId 필드만 validation 어노테이션이 없는데, 이것이 의도된 것인지 확인이 필요합니다. 만약 "대학교 미정" 같은 케이스를 허용하는 것이라면 현재 구현이 맞지만, 필수 입력이어야 한다면 @NotNull 추가를 고려해주세요.
🏁 Script executed:
#!/bin/bash
# universityId가 필수 필드인지 확인하기 위해 서비스 로직과 도메인 모델을 검토
# MentorApplication 도메인에서 universityId 사용 패턴 확인
ast-grep --pattern $'class MentorApplication {
$$$
universityId
$$$
}'
# MentorApplicationService에서 universityId 검증 로직 확인
rg -n -A5 -B5 "universityId" --type=java -g '**/MentorApplicationService.java'
# universitySelectType enum 정의 확인 (UNDECIDED 같은 옵션이 있는지)
fd -e java UniversitySelectType.java --exec cat {}Length of output: 1688
universityId 필드에 @NotNull 어노테이션을 추가해주세요.
검증 결과를 정리하면 다음과 같습니다:
-
이전 리뷰 요청사항은 완벽하게 반영됨
term필드에@NotBlank어노테이션이 올바르게 추가되었습니다.
-
기존 필드 검증은 적절함
exchangeStatus,universitySelectType,country에@NotNull어노테이션이 모두 추가되었습니다.
-
universityId 필수 검증 추가 필요
- MentorApplicationService에서 universityId가 null 체크 없이 직접 사용됩니다.
- UniversitySelectType 열거형에 "미정" 옵션이 없으므로, 항상 대학교 선택이 필수입니다.
- 따라서
universityId에도@NotNull어노테이션을 추가해야 합니다.
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java
around lines 10 to 23, the universityId field is missing validation and must be
required; add the @NotNull annotation (with an appropriate message like "대학교를
선택해주세요.") to the universityId field so requests are validated before
MentorApplicationService uses it without null checks.
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!
There was a problem hiding this comment.
@coderabbitai MentorApplication 에서 universityId 는 null 일수 있어
There was a problem hiding this comment.
@sukangpunch, 명확한 설명 감사합니다!
universityId가 null일 수 있다는 점을 확인했습니다. 제가 이전에 @NotNull 추가를 제안한 것은 잘못된 제안이었습니다. 멘토 지원 시 대학교가 미정이거나 선택되지 않은 경우를 허용하는 설계가 맞네요.
현재 구현이 올바르며, universityId에는 validation 어노테이션을 추가하지 않는 것이 맞습니다. 🐰
/) /)
( •_• )
/>📝 noted!
✏️ Learnings added
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.484Z
Learning: MentorApplication 도메인에서 universityId는 null일 수 있으며, MentorApplicationRequest에서도 이 필드에 대한 NotNull validation을 추가하지 않아야 한다.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 486
File: src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java:166-178
Timestamp: 2025-08-26T05:00:56.556Z
Learning: UnivApplyInfo의 텍스트 검색 기능에서는 University.koreanName이 아닌 UnivApplyInfo.koreanName을 검색 대상으로 사용하는 것이 의도된 설계이다.
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
현재 MentorApplicationRequest를 보면, 멘토 지원 시 termName 을 입력 받아, findByName으로 term 을 찾도록 구현하였습니다.
해당 방식에서 다른 방식(termId) 과 큰 차이를 못느껴서 name을 사용했는데 의견 주시면 감사하겠습니다.
DTO에서는 termId 보다 현재 작성하신대로 name으로 접근하는 것이 좋다고 생각합니다
고생하셨습니다 ! 몇 가지 코멘트 남겼는데 확인 부탁드립니다 ~
추가로 MentorApplication 엔티티에 컬럼(termId) 추가하셨는데 관련 flyway 스크립트도 작성해주세요 !
그리고 하나의 커밋이 조금 큰 거 같습니다. 기능 별로 조금 세분화해주시면 더욱 수월하게 리뷰 가능할 것 같습니다 !
| private final int code; | ||
| private final String message; | ||
| } | ||
| } |
There was a problem hiding this comment.
제가 컨벤션을 잘못 이해하고 있었네요
추가하겠습니다!
| mentorMyPageService.createMentorMyPage(siteUserId, request); | ||
| return ResponseEntity.ok().build(); | ||
| } | ||
| } |
| } | ||
| } | ||
| } | ||
| } |
| } | ||
|
|
||
| Optional<MentorApplication> findBySiteUserIdAndMentorApplicationStatus(long siteUserId, MentorApplicationStatus mentorApplicationStatus); | ||
| } |
| throw new CustomException(MENTOR_APPLICATION_ALREADY_EXISTED); | ||
| } | ||
| } | ||
| } |
| private void updateChannel(List<ChannelRequest> channelRequests, Mentor mentor) { | ||
| List<Channel> newChannels = buildChannels(channelRequests); | ||
| mentor.updateChannels(newChannels); | ||
| } |
There was a problem hiding this comment.
updateChannel 메서드와 createChannels 메서드가 거의 동일한 로직을 가지고 있는 거 같은데, 메서드 분리하지 않고 상위 메서드에 직접 작성하는 건 어떻게 생각하시나요 ?
There was a problem hiding this comment.
그 방식도 괜찮은 듯 합니다!
하지만, 상위 메서드로 넣으면, buildChannel 이라는 메서드를 직접 호출 해야 하는데, 추상화 레벨이 조금 낮아지는 느낌이라, 혹시 상위 메서드로 올려야 하는 추가적인 이유가 있을까요?
There was a problem hiding this comment.
이 부분은 정말 개인의 성향에 따라 느끼는 바가 다른 거 같아요
사실 전 buildChannel을 상위 메서드에 올려도 추상화 레벨이 그렇게 낮아진다고 느껴지지 않는 거 같습니다. update/createChannel 메서드 또한 단순히 변환하고 넘겨주는 역할만 수행하고 있어서, 분리할 정도까지는 아니라고 생각했습니다.
코멘트 의도가 단순 궁금해서였고, 구현하신 근거가 분명하셔서 굳이 바꿀 필요는 없을 거 같습니다 !
| private void validateUserCanCreateMentor(long siteUserId) { | ||
| if (!siteUserRepository.existsById(siteUserId)) { | ||
| throw new CustomException(USER_NOT_FOUND); | ||
| } |
There was a problem hiding this comment.
siteUserRepository.existsById 검증은 불필요하다고 생각합니다. 컨트롤러의 @AuthorizedUser 어노테이션을 통해 이미 존재하는 SiteUser 인 것은 분명하니까요 !
There was a problem hiding this comment.
알겠습니다! 빼는게 맞는 듯 합니다.
| ); | ||
| } | ||
| } | ||
| } |
| .hasMessage(MENTOR_APPLICATION_NOT_FOUND.getMessage()); | ||
| } | ||
| } | ||
| } |
| if(channelRequests == null){ | ||
| throw new CustomException(CHANNEL_NULL_NOT_ALLOWED); | ||
| } | ||
|
|
There was a problem hiding this comment.
무결성 관련 검증이라 .. 작성하신 것처럼 서비스 계층에서 검증해도 되지만 개인적으론 DTO 단에서 처리하는 게 더 좋을 거 같습니다 ..!
음 전 termId가 키이니 termId가 좋다고 생각했는데 성혁님은 혹시 왜 name으로 하는 게 좋다고 생각하셨는지 알 수 있을까요~? @whqtker |
* refactor: submitMentorApplication 메서드의 멘토 지원 유효 검증 부분을 메서드로 추출 * refactor: MentorMyPage 생성, 수정 부분의 channel 생성, 업데이트 부분 중복 제거 * test: Mentor 생성 관련 테스트 추가
* refactor: 멘토 생성 시 channelRequest가 null 일 떄 예외 처리 * feat: MentorApplicationRequest 필드에 유효성 어노테이션 추가 * test: 채널 검색 시 siteUserId로 조회하는 문제 해결
* fix: 파일 끝에 개행 추가 * refactor: 멘토 생성 메서드에서 siteUser의 검증 제외 * refactor: dto 단에서 채널 리스트 null 검증
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java (1)
42-42: 헬퍼 메서드 추출로 코드 가독성이 개선되었습니다.
- 중복 검증 로직을
ensureNoPendingOrApprovedMentorApplication메서드로 추출- 메서드명이 의도를 명확하게 표현하여 가독성 향상
선택 사항: 메서드명을 더 간결하게 할 수 있습니다:
-private void ensureNoPendingOrApprovedMentorApplication(long siteUserId) { +private void validateNoDuplicateApplication(long siteUserId) {Also applies to: 61-68
src/main/java/com/example/solidconnection/mentor/dto/MentorMyPageCreateRequest.java (1)
8-18: MentorMyPageCreateRequest 검증 설계가 깔끔합니다.
introduction/passTip에 대한@NotBlank와 한글 메시지가 명확해서 사용자가 어떤 필드를 채워야 하는지 잘 드러납니다.channels에@NotNull과@Valid를 붙여서 채널 리스트 자체와 내부 원소 검증을 분리한 것도 좋은 선택입니다.- 다만 비즈니스적으로 “채널은 최소 1개 이상이어야 한다”는 요구가 생긴다면, 나중에
channels필드에@Size(min = 1)정도를 추가해서 DTO 단계에서 빠르게 막는 것도 한 가지 옵션으로 보입니다(지금 상태도 충분히 괜찮아서 필수는 아닙니다).src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java (1)
80-81: termName 기반 조회 설계에 대한 의견 (termId를 이미 잘 활용하고 있습니다).
- 이 서비스 레이어에서는
MentorApplication에서 가져온termId만 사용하고 있어서, Admin 쪽 DTO에서termName을 받아findByName으로termId를 채우는 현재 구조는 동작 상으로는 크게 문제가 없어 보입니다.- 다만
termName이 절대적으로 유일하다는 보장이 약하거나, 운영 중 이름 변경 가능성이 있다면, 나중에는 Admin 요청 DTO도termId를 직접 받도록 맞추는 편이 더 안전하고, API 간 계약도 명확해질 것 같습니다.- 개인적으로는 FE ↔ BE 간에는 식별자(id)로 통신하고, 화면 표시용 이름은 별도 조회로 가져오는 패턴을 선호하는 편이라, 장기적으로는 termId 기반으로 정렬하는 방향을 고려해 보셔도 좋겠다는 정도의 의견입니다.
Also applies to: 87-89
📜 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(2 hunks)src/main/java/com/example/solidconnection/mentor/controller/MentorMyPageController.java(3 hunks)src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java(4 hunks)src/main/java/com/example/solidconnection/mentor/dto/MentorMyPageCreateRequest.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(3 hunks)src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java(4 hunks)src/main/resources/db/migration/V37__add_term_id_to_mentor_application.sql(1 hunks)src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java(3 hunks)src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java(3 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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-07-05T17:54:42.475Z
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 375
File: src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java:47-53
Timestamp: 2025-07-05T17:54:42.475Z
Learning: MentorMyPageService에서 PUT 메서드 구현 시 전체 채널을 새로 생성하여 교체하는 방식을 사용하는 것이 PUT의 의미론적 특성과 일치하며, 트랜잭션 로킹 관점에서도 합리적인 접근이다.
Applied to files:
src/main/java/com/example/solidconnection/mentor/controller/MentorMyPageController.javasrc/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java
📚 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/mentor/domain/MentorApplication.javasrc/main/java/com/example/solidconnection/mentor/repository/MentorApplicationRepository.javasrc/main/java/com/example/solidconnection/common/exception/ErrorCode.javasrc/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.javasrc/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.javasrc/main/java/com/example/solidconnection/mentor/dto/MentorMyPageCreateRequest.java
📚 Learning: 2025-08-11T14:39:49.462Z
Learnt from: Gyuhyeok99
Repo: solid-connection/solid-connect-server PR: 443
File: src/main/resources/db/migration/V28__alter_mentor_introduction_pass_tip_not_null.sql:1-5
Timestamp: 2025-08-11T14:39:49.462Z
Learning: The mentor table currently has no data in production/staging environment, so NULL backfill operations are not needed when adding NOT NULL constraints to the introduction and pass_tip columns in the mentor table.
Applied to files:
src/main/resources/db/migration/V37__add_term_id_to_mentor_application.sql
📚 Learning: 2025-08-02T21:44:08.950Z
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 426
File: src/main/java/com/example/solidconnection/mentor/domain/Mentor.java:80-80
Timestamp: 2025-08-02T21:44:08.950Z
Learning: Mentor.updateChannels() 메서드에서 채널 삭제 시 originalChannelSize를 메서드 시작 시점에 고정하여 사용하므로, 반복문 내에서 리스트 크기가 변경되어도 올바른 수의 채널이 삭제된다. i < originalChannelSize 조건으로 넘치는 채널을 정확히 제거할 수 있다.
Applied to files:
src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.javasrc/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java
🧬 Code graph analysis (1)
src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java (1)
src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java (1)
TestContainerSpringBootTest(33-204)
🪛 GitHub Actions: CI with Gradle
src/main/resources/db/migration/V37__add_term_id_to_mentor_application.sql
[error] 1-1: Flyway validation failed: Found more than one migration with version 37. Offenders: V37__add_term_id_to_mentor_application.sql
🔇 Additional comments (9)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1)
53-53: 새로운 에러 코드가 적절하게 추가되었습니다.
MENTOR_APPLICATION_NOT_FOUND: data-not-found 섹션에 올바르게 배치MENTOR_ALREADY_EXISTS: mentor 관련 섹션에 올바르게 배치에러 메시지도 명확하고 일관성 있게 작성되었습니다.
Also applies to: 130-130
src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java (1)
46-47: Term을 termName으로 조회하는 방식에 대한 검증이 필요합니다.PR 목표에서 리뷰어가 termId vs termName 사용에 대해 피드백을 요청했습니다. 현재 구현은 termName으로 Term을 조회하고 있습니다.
고려사항:
termName 사용 (현재 방식):
- 장점: 클라이언트가 termId를 미리 알 필요 없음
- 단점: 문자열 매칭으로 인한 오류 가능성, 추가 DB 조회
termId 사용:
- 장점: PK 기반 조회로 성능 및 안정성 향상
- 단점: 클라이언트가 termId를 미리 조회해야 함
현재 프론트엔드 및 API 설계와의 일관성을 고려하여 어떤 방식이 적합한지 팀 논의가 필요합니다. termName을 사용하는 명확한 이유가 있다면 주석으로 문서화하는 것을 권장합니다.
Also applies to: 55-55
src/main/java/com/example/solidconnection/mentor/controller/MentorMyPageController.java (1)
46-54: 멘토 마이페이지 생성 엔드포인트가 일관성 있게 구현되었습니다.
- 기존 엔드포인트들과 동일한 패턴 사용
- Validation 처리 (
@Valid) 적용- 응답 형식 통일 (
ResponseEntity.ok().build())참고: 이전 리뷰에서 논의된
@RequireRoleAccess(roles = Role.MENTOR)이슈는 다음 PR에서 admin 승인 기능 추가 시 해결될 예정입니다.src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java (1)
46-47: 테스트가 Term 지원을 일관성 있게 반영하도록 업데이트되었습니다.
- TermFixture 의존성 추가
- setUp에서 Term 초기화
- 테스트 헬퍼 메서드가 termName을 전달하도록 수정
모든 기존 테스트가 새로운 파라미터를 올바르게 사용하고 있습니다.
Also applies to: 56-56, 61-61, 200-201
src/main/java/com/example/solidconnection/mentor/repository/MentorApplicationRepository.java (1)
13-13: 새로운 repository 메서드가 적절하게 추가되었습니다.
findBySiteUserIdAndMentorApplicationStatus메서드는 APPROVED 상태의 멘토 지원서를 조회하는 데 사용되며, Spring Data JPA naming convention을 준수합니다.src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java (1)
56-57: termId 필드가 올바르게 추가되고 생성자 로직이 개선되었습니다.
termId필드 추가: NOT NULL 제약조건과 함께 DB migration과 일치- 생성자 파라미터로
termId추가: 불변성 보장mentorApplicationStatus초기화를 생성자로 이동: 도메인 로직의 응집도 향상Also applies to: 78-78, 89-91
src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java (1)
176-281: 멘토 마이페이지 생성 기능에 대한 포괄적인 테스트가 추가되었습니다.테스트 커버리지가 우수합니다:
성공 케이스 (Line 179-215):
- 멘토 정보 생성 검증
- 채널 정보 생성 검증
- 모든 필드 값 확인
예외 케이스:
- 이미 존재하는 멘토 (Line 217-227)
- 채널 등록 제한 초과 (Line 229-245)
- 멘토 승격 요청 부재 (Line 247-256)
- REJECTED 상태 (Line 258-268)
- PENDING 상태 (Line 270-280)
테스트 구조가 기존 nested 테스트와 일관성을 유지하며, 이전 리뷰 코멘트도 반영되었습니다 (Line 198:
createMentor.getId()사용).src/main/resources/db/migration/V37__add_term_id_to_mentor_application.sql (1)
1-6: 현재 저장소 상태에는 V37 파일 중복이 없습니다.검증 결과:
V37 파일 검색 결과
- 저장소 전체 검색: V37__add_term_id_to_mentor_application.sql 파일 1개만 존재
- 마이그레이션 디렉토리: 중복 파일 없음
마이그레이션 버전 순서 확인
- V36 (migrate_term_to_table) → V37 (add_term_id_to_mentor_application)
- 버전 간 간격 없음, 정상적인 순차 구성
결론
원본 리뷰 코멘트에서 보고한 "Found more than one migration with version 37" 오류는 현재 코드베이스에 존재하지 않습니다. V37 파일명과 위치는 올바르며, 중복된 마이그레이션도 발견되지 않습니다.Likely an incorrect or invalid review comment.
src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java (1)
71-74: 채널 빌드/업데이트 로직 분리가 읽기 좋습니다.
updateChannel/createChannels에서 공통 부분을buildChannels로 모아서, 채널 엔티티 생성 책임이 한 군데로 모인 점이 유지보수에 유리해 보입니다.CHANNEL_SEQUENCE_START_NUMBER상수를 통해 시퀀스를 명시적으로 관리하는 덕분에, 추후 요구사항(예: 시작 번호 변경)이 생겨도 변경 지점이 명확합니다.updateMentorMyPage에서 전체 채널을 새로 만들어 교체하는 방식은 이전 러닝에서 언급된 것처럼 PUT 의미론과도 잘 맞고, 락 관점에서도 단순해서 좋은 선택으로 느껴집니다.Also applies to: 107-119
| @Transactional | ||
| public void createMentorMyPage(long siteUserId, MentorMyPageCreateRequest request) { | ||
| validateUserCanCreateMentor(siteUserId); | ||
| validateChannelRegistrationLimit(request.channels()); | ||
| MentorApplication mentorApplication = mentorApplicationRepository.findBySiteUserIdAndMentorApplicationStatus(siteUserId, MentorApplicationStatus.APPROVED) | ||
| .orElseThrow(() -> new CustomException(MENTOR_APPLICATION_NOT_FOUND)); | ||
|
|
||
| Mentor mentor = new Mentor( | ||
| request.introduction(), | ||
| request.passTip(), | ||
| siteUserId, | ||
| mentorApplication.getUniversityId(), | ||
| mentorApplication.getTermId() | ||
| ); | ||
|
|
||
| createChannels(request.channels(), mentor); | ||
| mentorRepository.save(mentor); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
APPROVED 지원서에서 universityId가 null인 경우에 대한 처리 검토가 필요해 보입니다.
createMentorMyPage에서mentorApplication.getUniversityId()를 그대로Mentor생성자에 넘기고 있는데, 이전 러닝에 따르면MentorApplication의universityId는 null이 될 수 있는 설계입니다.- 이 상태로 null인
universityId가Mentor에 저장되면, 이후getMentorMyPage에서universityRepository.findById(mentor.getUniversityId())호출 시 null id로 인한 런타임 문제(예: NPE 또는 잘못된 쿼리)가 발생할 수 있습니다. - 비즈니스적으로 “APPROVED 가능한 지원서는 반드시 universityId를 가져야 한다”는 규칙이 있다면,
- (선호) 승인 시점(어드민 플로우)에서 universityId를 강제/검증하거나,
- (대안) 여기
createMentorMyPage에서 universityId가 null이면 별도 예외(예:UNIVERSITY_NOT_FOUND또는 새로운 에러 코드)를 던지는 식으로 방어 코드를 두는 편이 안전할 것 같습니다.
- 어떤 단계에서 이 무결성을 보장할지 한 번만 정리해 두시면, 이후 다른 멘토 생성 경로가 추가되더라도 일관성을 유지하기 쉬울 것 같습니다.
Based on learnings
Also applies to: 95-99
🏁 Script executed:
#!/bin/bash
# Mentor 및 MentorApplication에서 universityId의 null 가능성과 사용 패턴을 빠르게 훑어보기 위한 스크립트입니다.
# 1) Mentor.universityId 필드 선언과 생성자/팩토리에서의 사용 위치 확인
rg -n "universityId" src/main/java/com/example/solidconnection/mentor -C3
# 2) MentorApplication.universityId가 어디에서 세팅/검증되는지 확인
rg -n "universityId" src/main/java/com/example/solidconnection/mentor -C3
# 3) MentoryMyPageService에서 universityId 관련 사용 위치 재확인
rg -n "getUniversityId" src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java -C3Length of output: 23853
Mentor.universityId 필드 타입 불일치로 인한 null 언박싱 위험
코드 검증 결과, 다음 문제들을 발견했습니다:
-
타입 불일치 및 NPE 위험
Mentor.universityId필드는 기본형long이지만, 생성자 매개변수는Long(nullable wrapper)mentorApplication.getUniversityId()가 null일 경우, 기본형 long에 할당될 때 자동 언박싱되면서NullPointerException발생
-
비즈니스 로직 간극
MentorApplication의 설계에서universitySelectType = OTHER일 때는universityId가 null로 유지되도록 함 (line 102-104)- 하지만 승인 단계에서
OTHER타입 지원서가APPROVED되는 것을 막는 로직이 없음 - 결과적으로 OTHER 타입 지원서가 승인된 후
createMentorMyPage호출 시 null 언박싱 NPE 발생 가능
-
추가 영향 범위
getMentorMyPage(line 55):universityRepository.findById(mentor.getUniversityId())도 동일한 위험
해결 권장사항 (선택 사항)
- (선호)
Mentor.universityId를Long(nullable)으로 변경하고,getMentorMyPage에서 null 체크 추가 - (대안) 승인 시점에서
universitySelectType == CATALOG만 승인 가능하도록 비즈니스 규칙 강제 - (방어)
createMentorMyPage에서 null universityId 시 명확한 예외 던지기 (예:UNIVERSITY_ID_REQUIRED)
현재 MentorBatchQueryRepository (line 59-60)에서 이미 null 체크를 하고 있는 것으로 보아, 이 문제를 인식한 코드가 존재합니다. 한 곳에서만 일관성 있게 처리하면 향후 유지보수가 용이할 것입니다.
There was a problem hiding this comment.
@coderabbitai 다음 pr에 admin 멘토 지원서 approve/reject 하는 기능을 추가 할건데, 거기서 대학 증명 자료는 있지만, 현재 서버가 가지고 있지 않은 대학에 대해서는(universityId가 null 인경우) 대학 데이터를 만들고 지원서에 넣은 다음 approve 하려고 해
There was a problem hiding this comment.
@sukangpunch 아, 다음 PR에서 admin이 승인 시점에 대학 데이터를 생성하고 universityId를 채운 다음 APPROVED로 전환하는 구조군요! 그렇다면 제가 우려했던 부분이 비즈니스 로직 수준에서 해결되는 것으로 이해했습니다.
설계 방향에 대한 정리:
- APPROVED 상태의 지원서는 항상 universityId를 가진다는 불변 조건(invariant)이 admin 승인 플로우에서 보장됨
- null universityId는 PENDING 상태에서만 일시적으로 존재
- 현재 PR은 인프라 구축, 다음 PR에서 검증 로직 추가
이 접근은 합리적이고 관심사의 분리(separation of concerns)가 잘 되어 있다고 생각합니다!
참고로 추가 고려사항:
만약 혹시라도 admin 플로우에서 예외가 발생하거나 데이터 정합성 문제가 생길 경우를 대비하여, createMentorMyPage 메서드에 방어 코드를 추가하는 것도 고려해볼 수 있습니다:
MentorApplication mentorApplication = mentorApplicationRepository
.findBySiteUserIdAndMentorApplicationStatus(siteUserId, MentorApplicationStatus.APPROVED)
.orElseThrow(() -> new CustomException(MENTOR_APPLICATION_NOT_FOUND));
// 방어적 검증 (선택 사항)
if (mentorApplication.getUniversityId() == null) {
throw new CustomException(MENTOR_APPLICATION_INVALID); // 또는 적절한 에러 코드
}하지만 이는 선택 사항이며, admin 승인 플로우에서 충분히 보장된다면 불필요한 코드일 수 있습니다. 팀의 방어적 프로그래밍 정책에 따라 결정하시면 될 것 같습니다!
✏️ Learnings added
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를 가진다는 것이 비즈니스 규칙이다.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 375
File: src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java:47-53
Timestamp: 2025-07-05T17:54:42.475Z
Learning: MentorMyPageService에서 PUT 메서드 구현 시 전체 채널을 새로 생성하여 교체하는 방식을 사용하는 것이 PUT의 의미론적 특성과 일치하며, 트랜잭션 로킹 관점에서도 합리적인 접근이다.
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을 추가하지 않아야 한다.
09048fd to
38b3ad3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixtureBuilder.java (1)
23-79: termId 기본값이 0으로 들어가는 것을 개선해야 합니다.현재
termId가 primitivelong이므로,termId(...)를 호출하지 않고create()를 호출하면 자동으로0이 들어가게 됩니다. 실제 도메인에서는 항상 유효한 학기 FK가 필요하다는 제약이 있다면, 테스트에서 조용히0이 들어가는 것은 아래와 같은 위험이 있습니다:
모든 사용처에서 termId()를 호출하는지 확인하고 누락된 곳 추가
누락된 호출이 없다면, 이 옵션으로 문제를 해결할 수 있습니다.Long으로 변경하고 create()에서 null 검증 추가
long대신Long으로 바꾼 후,create()메서드에서 null 체크를 추가해서 설정되지 않은 termId가 있으면 예외를 던지도록 하면 더 안전합니다.TermFixture를 사용해 기본값 제공
현재exchangeStatus처럼 기본값을 설정하는 방식으로, 테스트용 기본 학기를 미리 세팅하는 헬퍼를 두면 테스트 기본값 관리가 더 명확해집니다.조용히 기본값(0)이 들어가는 것보다는 NullPointerException이 발생하는 것이 낫습니다. 위 옵션 중 하나를 선택해서 도메인 제약을 테스트에 반영하면 버그를 조기에 잡을 수 있습니다.
♻️ Duplicate comments (1)
src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java (1)
83-89: Mentor 생성자의 universityId null 안전성 문제 재확인 - 수정 필수검증 결과, 이전 리뷰에서 지적한 문제가 실제로 존재함을 확인했습니다.
1. 문제의 원인
Mentor생성자 매개변수:Long universityId(null 가능)Mentor필드:private long universityId(기본형)MentorApplication.getUniversityId():Long반환 (null 가능)- MentorMyPageService 라인 87:
mentorApplication.getUniversityId()를 직접 전달하면서 null을 기본형으로 auto-unboxing 시도 →NullPointerException2. 실제 위험성
MentorApplication의 CHECK 제약조건:universitySelectType = 'OTHER'일 때universityId IS NULL- 코드 파일을 살펴보면, APPROVED 상태의 지원서도
universitySelectType = 'OTHER'인 경우가 존재 가능- 테스트 픽스쳐도
universityId = null인 케이스를 기본값으로 설정3. 해결 방안 (다음 중 선택)
- 라인 83-89: null 체크를 추가하여
universityId가 null인 경우 명시적으로 처리if (mentorApplication.getUniversityId() == null) { throw new CustomException(/* 적절한 에러 코드 */); }- 또는
Mentor필드를Long universityId(nullable)로 변경하되, 다른 참조 코드들도 함께 검토 필요- 또는 어드민 승인 시점에
APPROVED상태가 되려면 반드시universitySelectType = 'CATALOG'만 허용하도록 비즈니스 로직 강화
🧹 Nitpick comments (5)
src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java (1)
6-23: term을 name(String)으로 받는 설계에 대한 피드백과 함께, 몇 가지 보완 아이디어를 공유드립니다.
이 설계의 장점은 분명합니다.
- 프론트가 내부 termId를 몰라도 되고, 사람이 읽을 수 있는 값으로 바로 주고받을 수 있어서 API가 직관적입니다.
- 별도의 “term 목록 → id 선택” 단계 없이도, 화면에 표시되는 그대로를 서버에 보낼 수 있습니다.
다만 아래 세 가지를 함께 보완/확인해 두면 더 안전할 것 같습니다.
- ① Term.name 유일성 보장: DB 제약이나 엔티티 레벨에서
name이 unique 인지 확인해두면,findByName사용 시 모호성이 없어집니다.- ② 이름 변경 전략: 학기 이름을 운영 중에 변경할 수 있는지, 변경 시 과거 요청/이력 조회에 영향이 없는지(예: name은 바뀌어도 id는 그대로라면, 마이그레이션 전략)를 한 번 정리해 두면 좋겠습니다.
- ③ 도메인 키 관점: 시스템 내부적으로 termId가 진짜 키라면, 외부에는 name을 받되 내부에서는 즉시 id로 치환하는 지금 구조(이 PR의 구조)가 “외부 표현은 name / 내부 키는 id”인 anti-corruption layer 역할을 잘 하고 있어서, 이 방향성을 명시적으로 문서에 남겨두면 리뷰어분들의 의문도 자연스럽게 해소될 것 같습니다.
Validation 측면에서는 거의 정리가 잘 되어 있습니다.
exchangeStatus,universitySelectType,country에@NotNull,term에@NotBlank가 붙어 있어 기본적인 필수값 검증이 잘 들어가 있습니다.- 만약
country도 빈 문자열을 허용하지 않으려는 의도라면, 나중에 여유 있을 때@NotBlank로 맞추는 것도 살짝 고려해 볼 만합니다(현재도 큰 문제는 아닙니다).전반적으로 term을 name으로 받는 선택은 합리해 보이고, 위 세 가지를 정리해 두면 장점은 살리면서 장기적인 유지보수 리스크는 줄일 수 있을 것 같습니다.
src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixture.java (1)
7-67: 테스트에서 사용하는 학기 이름 문자열을 상수로 한 번만 정의해 두면 유지보수에 도움이 될 것 같습니다.현재
"2025-1"리터럴이 세 메서드에서 반복되는데, 나중에 테스트 기본 학기를 바꾸고 싶을 때 실수 가능성이 살짝 있습니다.
예를 들어private static final String DEFAULT_TERM_NAME = "2025-1";정도로 상수를 두고termFixture.현재_학기(DEFAULT_TERM_NAME)형태로 쓰면, 한 곳만 수정하면 되어 테스트 유지보수가 조금 더 편해질 것 같습니다.src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1)
53-131: 새로 추가된 에러 코드들의 의미 범위를 살짝 더 명시해 두면 나중에 헷갈림을 줄일 수 있을 것 같습니다.
MENTOR_APPLICATION_NOT_FOUND를 “멘토 지원서 조회 실패(멘토 마이페이지 생성 등에서 사용하는 용도)”로 두신 것은 위치와 메시지 모두 자연스럽습니다.- 다만 멘토 관련해서는
ALREADY_MENTOR(이미 멘토로 등록된 사용자)와MENTOR_ALREADY_EXISTS(이미 존재하는 멘토) 두 코드가 의미상 조금 겹쳐 보일 수 있습니다.현재 PR에서 각각을 어느 상황에서 사용할지(예:
ALREADY_MENTOR는 “현재 로그인 유저가 이미 멘토인지” 검사할 때,MENTOR_ALREADY_EXISTS는 “특정 멘토 엔티티(예: siteUserId + term 조합)가 이미 DB에 존재할 때” 등으로 역할을 분리
하는 식으로) 간단히 주석이나 컨벤션을 남겨 두면, 이후 기능 추가 시 코드 선택에 고민이 줄어들 것 같습니다.src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java (1)
13-68: 멘토 지원서 제출 플로우 정리가 잘 되어 있고, 몇 가지 보완 아이디어만 살짝 제안드립니다.
변경된 플로우 자체는 요구사항과 잘 맞습니다.
ensureNoPendingOrApprovedMentorApplication로 PENDING/APPROVED 중복을 한 번에 막고, 바로 그 다음에만 S3 업로드를 수행해서 불필요한 업로드를 줄인 구조가 자연스럽습니다.- REJECTED 상태에서는 다시 신청할 수 있도록 열어 둔 것도 의도한 정책으로 보입니다.
Term 연동 부분도 읽기 좋게 정리되어 있습니다.
termRepository.findByName(mentorApplicationRequest.term())→ 없으면TERM_NOT_FOUND예외로 이어지는 흐름이 명확하고, DTO에서 받은 term 문자열을 내부적으로 termId로 치환하는 계층 분리가 잘 드러납니다.동시성까지 고려하면, DB 레벨에서의 중복 방지도 한 번 고민해 볼 수 있습니다.
- 극단적인 케이스지만, 동일 유저가 거의 동시에 멘토 신청을 두 번 보내면, 둘 다
existsBy...검사 통과 후 insert로 이어질 여지가 있습니다.- 나중에 트래픽/요구사항이 늘어나면,
(site_user_id, mentor_application_status in (PENDING, APPROVED))조합에 대해 유니크 제약(또는 부분 인덱스)을 두는 것도 하나의 선택지입니다. 지금 단계에서는 운영 상황을 보면서 추후 도입해도 될 것 같고, 코드 구조 자체는 현재도 충분히 명료합니다.현재 구현만으로도 기능 요구사항은 잘 충족하고 있고, 위 DB 제약은 “강화 옵션” 정도로 참고하시면 될 것 같습니다.
src/main/java/com/example/solidconnection/mentor/dto/MentorMyPageCreateRequest.java (1)
15-17: channels 검증 일관성 확인
MentorMyPageCreateRequest에서는channels에@NotNull이 있지만,MentorMyPageUpdateRequest에는 없네요.
생성 시에는 빈 리스트라도 명시적으로 제공하도록 강제하는 의도로 보이는데, 비즈니스 요구사항상 이 차이가 의도된 것인지 확인해보면 좋을 것 같습니다.테스트 코드(line 220, 250)를 보면 빈 리스트
List.of()를 허용하고 있으므로,@NotNull은 적절해 보입니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(2 hunks)src/main/java/com/example/solidconnection/mentor/controller/MentorMyPageController.java(3 hunks)src/main/java/com/example/solidconnection/mentor/domain/Mentor.java(2 hunks)src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java(4 hunks)src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java(1 hunks)src/main/java/com/example/solidconnection/mentor/dto/MentorMyPageCreateRequest.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(3 hunks)src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java(4 hunks)src/main/java/com/example/solidconnection/term/repository/TermRepository.java(1 hunks)src/main/resources/db/migration/V38__add_term_id_to_mentor_application.sql(1 hunks)src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixture.java(5 hunks)src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixtureBuilder.java(3 hunks)src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java(3 hunks)src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/java/com/example/solidconnection/mentor/domain/Mentor.java
- src/main/java/com/example/solidconnection/term/repository/TermRepository.java
- src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java
- src/main/java/com/example/solidconnection/mentor/controller/MentorMyPageController.java
- src/main/java/com/example/solidconnection/mentor/repository/MentorApplicationRepository.java
🧰 Additional context used
🧠 Learnings (6)
📓 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-08-02T21:44:08.950Z
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 426
File: src/main/java/com/example/solidconnection/mentor/domain/Mentor.java:80-80
Timestamp: 2025-08-02T21:44:08.950Z
Learning: Mentor.updateChannels() 메서드에서 채널 삭제 시 originalChannelSize를 메서드 시작 시점에 고정하여 사용하므로, 반복문 내에서 리스트 크기가 변경되어도 올바른 수의 채널이 삭제된다. i < originalChannelSize 조건으로 넘치는 채널을 정확히 제거할 수 있다.
Applied to files:
src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.javasrc/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java
📚 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/mentor/fixture/MentorApplicationFixtureBuilder.javasrc/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixture.javasrc/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.javasrc/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.javasrc/main/java/com/example/solidconnection/mentor/dto/MentorMyPageCreateRequest.javasrc/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.javasrc/main/java/com/example/solidconnection/mentor/domain/MentorApplication.javasrc/main/java/com/example/solidconnection/common/exception/ErrorCode.java
📚 Learning: 2025-08-26T05:00:56.556Z
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 486
File: src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java:166-178
Timestamp: 2025-08-26T05:00:56.556Z
Learning: UnivApplyInfo의 텍스트 검색 기능에서는 University.koreanName이 아닌 UnivApplyInfo.koreanName을 검색 대상으로 사용하는 것이 의도된 설계이다.
Applied to files:
src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java
📚 Learning: 2025-08-11T14:39:49.462Z
Learnt from: Gyuhyeok99
Repo: solid-connection/solid-connect-server PR: 443
File: src/main/resources/db/migration/V28__alter_mentor_introduction_pass_tip_not_null.sql:1-5
Timestamp: 2025-08-11T14:39:49.462Z
Learning: The mentor table currently has no data in production/staging environment, so NULL backfill operations are not needed when adding NOT NULL constraints to the introduction and pass_tip columns in the mentor table.
Applied to files:
src/main/resources/db/migration/V38__add_term_id_to_mentor_application.sql
📚 Learning: 2025-07-05T17:54:42.475Z
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 375
File: src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java:47-53
Timestamp: 2025-07-05T17:54:42.475Z
Learning: MentorMyPageService에서 PUT 메서드 구현 시 전체 채널을 새로 생성하여 교체하는 방식을 사용하는 것이 PUT의 의미론적 특성과 일치하며, 트랜잭션 로킹 관점에서도 합리적인 접근이다.
Applied to files:
src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java
🧬 Code graph analysis (2)
src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java (1)
src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java (1)
TestContainerSpringBootTest(33-204)
src/main/java/com/example/solidconnection/mentor/dto/MentorMyPageCreateRequest.java (1)
src/main/java/com/example/solidconnection/mentor/dto/MentorMyPageUpdateRequest.java (1)
MentorMyPageUpdateRequest(8-18)
🔇 Additional comments (4)
src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java (1)
56-92: termId와 mentorApplicationStatus 초기화를 생성자에서 함께 처리한 구조가 도메인 관점에서 깔끔해 보입니다.
termId를 non-null 컬럼으로 두고, 생성자에서만 세팅하도록 한 덕분에 “학기 없는 MentorApplication”이 생성되는 경로가 차단되어 일관성이 좋습니다.- 새로 생성되는 지원서는 생성자에서 무조건
MentorApplicationStatus.PENDING으로 시작하게 해서, 상태 초기값을 한 곳에서 통제하는 것도 도메인 규칙을 읽기 쉽게 해 줍니다(JPA 로드 시에는 필드 주입이라 영향 없음).현재까지 보이는 변경 범위 내에서는 도메인 모델링이 요구사항과 잘 맞게 정리된 것 같습니다.
src/main/resources/db/migration/V38__add_term_id_to_mentor_application.sql (1)
1-6: mentor_application에 term_id NOT NULL 추가 시 기존 데이터 검증이 필수입니다.검증 결과, V36 마이그레이션에서 다른 테이블들(application, mentor, university_info_for_apply)은 다음과 같은 신중한 2단계 전략을 사용했습니다:
- 먼저 term_id를 NULL로 추가
- 기존 데이터를 UPDATE로 backfill
- NOT NULL로 변경 후 FK 생성
하지만 V38에서는 mentor_application에 기본값이나 backfill 없이 직접
term_id BIGINT NOT NULL을 추가하려고 하므로, V35 이후 mentor_application에 쌓인 기존 데이터가 있다면 마이그레이션이 실패할 수 있습니다. 배포 전에 다음을 검토해 주세요:
- 운영 환경의 mentor_application 행 수 확인
- 데이터가 있다면 ① NULL → backfill → NOT NULL 방식으로 V38을 분리하거나, ② 임시로 DEFAULT 값(예: 현재 학기 term_id)을 설정한 후 필요시 수정하는 전략 검토
⛔ Skipped due to learnings
Learnt from: Gyuhyeok99 Repo: solid-connection/solid-connect-server PR: 443 File: src/main/resources/db/migration/V28__alter_mentor_introduction_pass_tip_not_null.sql:1-5 Timestamp: 2025-08-11T14:39:49.462Z Learning: The mentor table currently has no data in production/staging environment, so NULL backfill operations are not needed when adding NOT NULL constraints to the introduction and pass_tip columns in the mentor table.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을 추가하지 않아야 한다.src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java (1)
176-281: 포괄적인 테스트 커버리지 확인멘토 마이페이지 생성 기능에 대한 테스트 시나리오가 매우 잘 구성되어 있습니다:
- 정상 케이스: 4개 채널과 함께 멘토 정보 생성
- 중복 생성 방지:
MENTOR_ALREADY_EXISTS- 채널 개수 제한:
CHANNEL_REGISTRATION_LIMIT_EXCEEDED- 지원서 미존재:
MENTOR_APPLICATION_NOT_FOUND- 거절된 지원서:
MENTOR_APPLICATION_NOT_FOUND- 대기 중 지원서:
MENTOR_APPLICATION_NOT_FOUNDAPPROVED 상태의 지원서만 멘토 생성이 가능하다는 비즈니스 로직이 명확하게 검증되고 있습니다.
src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java (1)
76-93: 멘토 마이페이지 생성 로직 검토새로 추가된
createMentorMyPage메서드의 구조가 깔끔합니다:
- 멘토 중복 생성 방지 검증
- 채널 개수 제한 검증
- APPROVED 상태의 지원서 조회
- 지원서 정보로 멘토 생성
- 채널 생성 및 저장
비즈니스 요구사항대로 APPROVED 지원서가 있어야만 멘토를 생성할 수 있도록 제한하고 있어 좋습니다.
3 participants
관련 이슈
작업 내용
멘토 승격 요청 이후 어드민이 해당 지원서를 승낙(APPROVED) 한 상황에서 멘토 마이페이지 생성 기능을 추가합니다.
멘토 마이페이지를 생성 하기 위해선,
이 필수적입니다.
다음 PR에 어드민 관련 기능을 추가하겠습니다.
특이 사항
리뷰 요구사항 (선택)
현재 MentorApplicationRequest를 보면, 멘토 지원 시 termName 을 입력 받아, findByName으로 term 을 찾도록 구현하였습니다.
해당 방식에서 다른 방식(termId) 과 큰 차이를 못느껴서 name을 사용했는데 의견 주시면 감사하겠습니다.