-
Notifications
You must be signed in to change notification settings - Fork 8
fix : 동일 멘토 멘티 중복 신청 불가능하도록 수정 #563
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix : 동일 멘토 멘티 중복 신청 불가능하도록 수정 #563
Conversation
- UK 제약조건 추가 - flyway script 추가 - CustomException 추가 - Service 로직 수정 - Test code 추가
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/main/java/com/example/solidconnection/mentor/service/MentoringCommandService.java (1)
40-40: 추출한 변수를 일관되게 사용하는 것이 좋겠습니다.Line 34에서 이미
mentorId변수로 추출했는데, Line 40에서는 다시mentoringApplyRequest.mentorId()를 호출하고 있습니다. 추출한 변수를 재사용하면 코드 일관성이 향상됩니다.다음과 같이 수정할 수 있습니다:
- Mentoring mentoring = new Mentoring(mentoringApplyRequest.mentorId(), siteUserId, VerifyStatus.PENDING); + Mentoring mentoring = new Mentoring(mentorId, siteUserId, VerifyStatus.PENDING);src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1)
121-121: 에러 메시지를 더 명확하게 표현하면 좋겠습니다.현재 메시지 "이미 신청된 멘티, 멘토입니다."보다는 "이미 해당 멘토에게 멘토링을 신청했습니다." 또는 "이미 신청된 멘토링입니다."가 사용자 입장에서 더 이해하기 쉬울 것 같습니다.
다음과 같이 수정을 고려해보세요:
- ALREADY_EXIST_MENTORING(HttpStatus.BAD_REQUEST.value(), "이미 신청된 멘티, 멘토입니다."), + ALREADY_EXIST_MENTORING(HttpStatus.BAD_REQUEST.value(), "이미 신청된 멘토링입니다."),src/test/java/com/example/solidconnection/mentor/service/MentoringCommandServiceTest.java (1)
101-111: 테스트 커버리지를 확장하여 더욱 견고한 검증을 완성하세요.검증 결과, 현재 구현은 데이터베이스 수준의 유니크 제약조건
uk_mentoring_mentor_id_mentee_id로 보호되고 있어서 모든 상태(PENDING, APPROVED, REJECTED)에서 중복 신청을 올바르게 방지하고 있습니다. 다만, 테스트 커버리지를 다음과 같이 강화하면 좋겠습니다:
- APPROVED 상태에서의 중복 방지 검증: 이미 승인된 멘토링이 있을 때 재신청이 차단되는지 테스트
- REJECTED 상태에서의 중복 방지 검증: 거절된 멘토링도 동일하게 재신청이 차단되는지 테스트
현재 테스트는 PENDING 상태만 확인하고 있는데, APPROVED/REJECTED 시나리오를 추가하면 모든 멘토링 상태에 대해 중복 방지 로직이 정상 작동함을 명확하게 보여줄 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(1 hunks)src/main/java/com/example/solidconnection/mentor/domain/Mentoring.java(2 hunks)src/main/java/com/example/solidconnection/mentor/service/MentoringCommandService.java(2 hunks)src/main/resources/db/migration/V37__add_unique_constraint_to_mentor_id_mentee_id.sql(1 hunks)src/test/java/com/example/solidconnection/mentor/service/MentoringCommandServiceTest.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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_unique_constraint_to_mentor_id_mentee_id.sqlsrc/main/java/com/example/solidconnection/mentor/domain/Mentoring.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/service/MentoringCommandService.java (1)
34-38: 멘토링 중복 신청 방지 로직이 잘 구현되었습니다.애플리케이션 레벨에서 중복을 먼저 체크하여 불필요한 DB 작업을 방지하고 있습니다. DB 제약 조건과 함께 이중 방어 체계를 구성하여 동시성 문제도 안전하게 처리됩니다.
src/main/resources/db/migration/V37__add_unique_constraint_to_mentor_id_mentee_id.sql
Outdated
Show resolved
Hide resolved
- column명 camelCase -> snake_case로 변경
- column명 name으로 명시
Hexeong
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
whqtker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인했습니다 ! 고생하셨습니다 ~!
관련 이슈
작업 내용
특이 사항
성혁님 PR과 Mentoring Repository가 겹치는데 merge되는 순서대로 conflict 해결하면 될 것 같습니다!
리뷰 요구사항 (선택)