feat: 우수 스터디원 처리시 중복된 요청에 대한 검증 추가 #825
Conversation
워크스루이 풀 리퀘스트는 스터디 성과 관리 시스템에 중복된 우수 스터디원 지정을 방지하는 검증 로직을 추가합니다. 새로운 변경 사항
연결된 이슈 평가
관련 가능성 있는 PR
제안된 라벨
제안된 리뷰어
토끼의 시
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAchievementCustomRepository.java (1)
13-14: 매개변수 유효성 검사 추가 제안구현 클래스에서 다음 사항들에 대한 유효성 검사를 추가하는 것이 좋을 것 같습니다:
- studyId가 null이 아닌지 확인
- achievementType이 null이 아닌지 확인
- studentIds가 null이 아니거나 비어있지 않은지 확인
src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyAchievementValidator.java (1)
11-16: 검증 로직이 명확하고 잘 구현되어 있습니다.검증 로직이 단순하고 목적에 맞게 잘 구현되어 있습니다. 다만 다음과 같은 개선사항을 제안드립니다:
- 메서드 파라미터 이름을 더 명확하게 표현하면 좋을 것 같습니다.
- 예외 메시지에 구체적인 정보를 포함하면 디버깅에 도움이 될 것 같습니다.
- public void validateDesignateOutstandingStudent(long countStudyAchievementsAlreadyExist) { + public void validateDesignateOutstandingStudent(long existingAchievementsCount) { // 이미 우수 스터디원으로 지정된 스터디원이 있는 경우 - if (countStudyAchievementsAlreadyExist > 0) { - throw new CustomException(STUDY_ACHIEVEMENT_ALREADY_EXISTS); + if (existingAchievementsCount > 0) { + throw new CustomException(STUDY_ACHIEVEMENT_ALREADY_EXISTS, + String.format("이미 %d개의 우수 스터디원 지정이 존재합니다.", existingAchievementsCount)); } }src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAchievementCustomRepositoryImpl.java (2)
34-42: 새로운 카운트 메서드가 잘 구현되어 있습니다.카운트 쿼리가 명확하게 구현되어 있으며, 기존 코드와 일관성이 있습니다. 다만 다음과 같은 개선사항을 제안드립니다:
- 메서드에 Javadoc을 추가하여 사용 목적과 파라미터를 문서화하면 좋을 것 같습니다.
- 반환값이 null일 수 있는 경우에 대한 처리를 추가하면 좋을 것 같습니다.
+ /** + * 특정 스터디의 우수 스터디원 지정 여부를 확인합니다. + * + * @param studyId 스터디 ID + * @param achievementType 업적 유형 + * @param studentIds 학생 ID 목록 + * @return 지정된 우수 스터디원 수 + */ @Override public long countByStudyIdAndAchievementTypeAndStudentIds( Long studyId, AchievementType achievementType, List<Long> studentIds) { - return (long) queryFactory + Long count = queryFactory .select(studyAchievement.count()) .from(studyAchievement) .where(eqStudyId(studyId), eqAchievementType(achievementType), containsStudentId(studentIds)) .fetchOne(); + return count != null ? count : 0L; }
48-54: 조건 메서드들이 잘 분리되어 있습니다.쿼리 조건들이 재사용 가능한 메서드로 잘 분리되어 있습니다. 다만 다음과 같은 개선사항을 제안드립니다:
각 메서드에 간단한 Javadoc을 추가하여 사용 목적을 문서화하면 좋을 것 같습니다.
+ /** + * 업적 유형 일치 여부를 확인하는 조건을 생성합니다. + */ private BooleanExpression eqAchievementType(AchievementType achievementType) { return achievementType != null ? studyAchievement.achievementType.eq(achievementType) : null; } + /** + * 학생 ID 목록에 포함되는지 확인하는 조건을 생성합니다. + */ private BooleanExpression containsStudentId(List<Long> memberIds) { return memberIds != null ? studyAchievement.student.id.in(memberIds) : null; }src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyAchievementService.java (1)
42-51: 중복 검증 로직이 잘 통합되어 있습니다.검증 로직이 서비스에 잘 통합되어 있습니다. 다만 다음과 같은 개선사항을 제안드립니다:
- 검증 실패 시의 로그 처리를 추가하면 좋을 것 같습니다.
- 검증에 실패한 학생 ID들을 로그에 포함하면 디버깅에 도움이 될 것 같습니다.
long countByStudyIdAndStudentIds = studyHistoryRepository.countByStudyIdAndStudentIds(studyId, request.studentIds()); long countStudyAchievementsAlreadyExist = studyAchievementRepository.countByStudyIdAndAchievementTypeAndStudentIds( studyId, request.achievementType(), request.studentIds()); + log.debug( + "[MentorStudyAchievementService] 우수 스터디원 지정 검증: studyId={}, studentIds={}, existingCount={}", + studyId, request.studentIds(), countStudyAchievementsAlreadyExist); + studyValidator.validateStudyMentor(currentMember, study); studyHistoryValidator.validateAppliedToStudy( countByStudyIdAndStudentIds, request.studentIds().size()); studyAchievementValidator.validateDesignateOutstandingStudent(countStudyAchievementsAlreadyExist);src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1)
131-133: 코드 변경이 적절하며 문서화 추가 제안에러 코드의 추가가 적절하게 이루어졌습니다. HttpStatus.CONFLICT 상태 코드의 사용이 중복 지정 상황을 잘 표현하고 있습니다.
다만, 이 에러 코드가 언제 발생하는지 다른 개발자들이 쉽게 이해할 수 있도록 주석을 추가하면 좋을 것 같습니다:
// StudyAchievement + /** + * 이미 우수 스터디원으로 지정된 회원을 다시 지정하려고 할 때 발생하는 에러 + */ STUDY_ACHIEVEMENT_ALREADY_EXISTS(HttpStatus.CONFLICT, "이미 우수 스터디원으로 지정된 스터디원이 존재합니다."),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyAchievementService.java(3 hunks)src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAchievementCustomRepository.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAchievementCustomRepositoryImpl.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyAchievementValidator.java(1 hunks)src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java(1 hunks)
🔇 Additional comments (2)
src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAchievementCustomRepository.java (1)
13-14: 메서드 구현이 잘 되었습니다!
중복 지정을 방지하기 위한 카운트 메서드가 적절하게 추가되었습니다. 메서드 시그니처가 명확하고 기존 코드 스타일과 일관성이 있습니다.
아래 스크립트를 실행하여 구현 클래스의 존재 여부를 확인하겠습니다:
✅ Verification successful
구현 클래스가 정상적으로 존재하며 메서드가 올바르게 구현되었습니다!
StudyAchievementCustomRepositoryImpl 클래스에서 QueryDSL을 사용하여 적절하게 구현되어 있습니다. 메서드의 구현이 명확하고 기존 코드 스타일과 일관성이 있으며, 요구사항에 맞게 잘 작성되었습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation class exists
# Expected: Find the implementation class that contains the new method
rg -l "implements.*StudyAchievementCustomRepository"
Length of output: 147
Script:
#!/bin/bash
# Check if the implementation class contains the method
rg -A 5 "countByStudyIdAndAchievementTypeAndStudentIds" src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAchievementCustomRepositoryImpl.java
Length of output: 530
src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyAchievementService.java (1)
32-32: 의존성 주입이 적절합니다.
새로운 Validator가 final 필드로 적절하게 주입되어 있습니다.
| studyHistoryRepository.countByStudyIdAndStudentIds(studyId, request.studentIds()); | ||
| long countStudyAchievementsAlreadyExist = |
There was a problem hiding this comment.
변수명이 조금 어색한 것 같네요. xxxCount 와 같은 구조가 되어야 할 것 같습니다.
| @Override | ||
| public long countByStudyIdAndAchievementTypeAndStudentIds( | ||
| Long studyId, AchievementType achievementType, List<Long> studentIds) { | ||
| return (long) queryFactory |
There was a problem hiding this comment.
Long -> long으로 auto unboxing이 수행되기 때문에 명시적으로 캐스팅해주지 않아도 됩니다.
그리고 언박싱 과정에서 IDE에서 NPE 경고가 발생하는데, Objects.requireNonnull 을 사용하면 더 적절히 쓸 수 있을듯 합니다 (실제로는 null일 가능성이 없기 때문에 Optional.ofNullable을 사용하지 않는 점을 참고해주세요)
| @DomainService | ||
| public class StudyAchievementValidator { | ||
|
|
||
| public void validateDesignateOutstandingStudent(long countStudyAchievementsAlreadyExist) { |
There was a problem hiding this comment.
이렇게 놓고 보니 designate라는 워딩이 좀 맘에 안드네요 (별로 안 직관적임). nominated나... candidate 워딩 써도 될 것 같은데... 투두로 파놓고 나중에 이슈로 일괄 변경해보면 어떨까 싶습니다
There was a problem hiding this comment.
nominate는 후보로 올려두는 느낌이 있어서 designate로 했는데 candidate도 마찬가지로 후보의 느낌이 있는 것 같아요.
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
🌱 관련 이슈
📌 작업 내용 및 특이사항
따닥 방지용으로 Unique Constraint 걸어뒀는데, 이게 따닥을 막는 과정에서 500 에러를 던지고 있습니다.
📝 참고사항
📚 기타
Summary by CodeRabbit
새로운 기능
StudyAchievementValidator가 도입되어 우수 학생 지정을 위한 검증 기능 제공.STUDY_ACHIEVEMENT_ALREADY_EXISTS추가.버그 수정