feat: 우수 스터디원 지정 및 철회 V2 API 구현 #909
Conversation
📝 WalkthroughWalkthrough이번 PR은 멘토의 우수 학생 성과 관리를 위한 새로운 기능을 도입합니다. 새로 추가된 REST 컨트롤러와 서비스 클래스는 우수 학생 지정 및 철회 기능을 제공하며, 관련 데이터 처리를 위한 커스텀 레포지토리와 검증 로직이 구현되었습니다. 또한, StudyAchievement 및 StudyHistory 관련 DAO 인터페이스와 구현체가 추가되어 데이터베이스 쿼리 처리 로직이 정리되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant Ctrl as MentorStudyAchievementControllerV2
participant Srv as MentorStudyAchievementServiceV2
participant Repo as Repository Layer
%% Designate Outstanding Student Flow
C->>Ctrl: HTTP POST /v2/mentor/study-achievements<br>(studyId, OutstandingStudentRequest)
Ctrl->>Srv: designateOutstandingStudent(studyId, request)
Srv->>Repo: 학생 존재 및 중복 여부 검증<br>데이터 조회/저장
Repo-->>Srv: 결과 반환
Srv-->>Ctrl: 성공 응답
Ctrl-->>C: HTTP 200 OK
%% Withdraw Outstanding Student Flow
C->>Ctrl: HTTP DELETE /v2/mentor/study-achievements<br>(studyId, OutstandingStudentRequest)
Ctrl->>Srv: withdrawOutstandingStudent(studyId, request)
Srv->>Repo: 학생 존재 검증 및 성과 삭제<br>쿼리 실행
Repo-->>Srv: 삭제 결과 반환
Srv-->>Ctrl: 성공 응답
Ctrl-->>C: HTTP 200 OK
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 3
🧹 Nitpick comments (8)
src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyHistoryV2CustomRepository.java (2)
8-8: 메서드 문서화 및 매개변수 유효성 검사 추가 필요메서드의 목적과 매개변수에 대한 명확한 이해를 위해 다음 사항들을 고려해주세요:
- JavaDoc을 추가하여 메서드의 목적, 매개변수, 반환값을 문서화
@NonNull어노테이션을 사용하여 null 허용 여부를 명시- 빈
studentIds리스트 처리 방법 명시다음과 같이 개선해보세요:
+ /** + * 특정 스터디의 학생들에 대한 스터디 이력 수를 조회합니다. + * + * @param studyId 조회할 스터디 ID + * @param studentIds 조회할 학생 ID 목록 + * @return 스터디 이력 수 + * @throws IllegalArgumentException studentIds가 비어있는 경우 + */ + @NonNull long countByStudyIdAndStudentIds( + @NonNull Long studyId, + @NonNull List<Long> studentIds );
10-10: 기존 메서드와의 일관성 유지 필요
findAllByStudyIdAndStudentIds메서드도 동일한 패턴으로 문서화와 유효성 검사를 적용하면 좋겠습니다.다음과 같이 개선해보세요:
+ /** + * 특정 스터디의 학생들에 대한 스터디 이력 목록을 조회합니다. + * + * @param studyId 조회할 스터디 ID + * @param studentIds 조회할 학생 ID 목록 + * @return 스터디 이력 목록 + * @throws IllegalArgumentException studentIds가 비어있는 경우 + */ + @NonNull List<StudyHistoryV2> findAllByStudyIdAndStudentIds( + @NonNull Long studyId, + @NonNull List<Long> studentIds );src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyHistoryV2CustomRepositoryImpl.java (1)
17-24: count 쿼리 결과의 null 처리 방식을 개선하면 좋겠습니다.현재 구현은 count 쿼리의 결과가 null인 경우 NullPointerException을 발생시킵니다. count 쿼리의 결과는 일반적으로 null이 아닌 0을 반환해야 합니다.
다음과 같이 개선하는 것을 제안드립니다:
- return Objects.requireNonNull(queryFactory + return queryFactory .select(studyHistoryV2.count()) .from(studyHistoryV2) .where(eqStudyId(studyId), studyHistoryV2.student.id.in(studentIds)) - .fetchOne()); + .fetchOne() != null ? queryFactory + .select(studyHistoryV2.count()) + .from(studyHistoryV2) + .where(eqStudyId(studyId), studyHistoryV2.student.id.in(studentIds)) + .fetchOne() : 0L;src/main/java/com/gdschongik/gdsc/domain/studyv2/application/MentorStudyAchievementServiceV2.java (1)
65-83: 중복 코드 최소화를 고려해 보세요.
withdrawOutstandingStudent메서드에서도studyValidator와studyHistoryValidator호출 로직이designateOutstandingStudent와 유사하게 반복됩니다. 공통되는 로직을 별도의 유틸 메서드로 추출하면 가독성과 유지보수성이 향상될 수 있습니다.src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/StudyAchievementValidatorV2.java (1)
11-16: 검증 로직 개선 제안검증 메서드가 단순히 카운트만 확인하고 있습니다. 다음과 같은 추가 검증을 고려해보세요:
- 입력값 유효성 검사 (음수 카운트 체크)
- 구체적인 에러 메시지 포함
public void validateDesignateOutstandingStudent(long countStudyAchievementsAlreadyExist) { + if (countStudyAchievementsAlreadyExist < 0) { + throw new CustomException(INVALID_STUDY_ACHIEVEMENT_COUNT); + } // 이미 우수 스터디원으로 지정된 스터디원이 있는 경우 if (countStudyAchievementsAlreadyExist > 0) { - throw new CustomException(STUDY_ACHIEVEMENT_ALREADY_EXISTS); + throw new CustomException(STUDY_ACHIEVEMENT_ALREADY_EXISTS, + String.format("이미 %d명의 우수 스터디원이 지정되어 있습니다.", countStudyAchievementsAlreadyExist)); } }src/main/java/com/gdschongik/gdsc/domain/studyv2/api/MentorStudyAchievementControllerV2.java (1)
34-38: 삭제 API 응답 개선 필요삭제 작업의 결과를 명확하게 전달하기 위해 응답 본문을 포함하는 것이 좋습니다.
@DeleteMapping public ResponseEntity<Void> withdrawOutstandingStudent( - @RequestParam(name = "studyId") Long studyId, @RequestBody OutstandingStudentRequest request) { + @RequestParam(name = "studyId") Long studyId, @Valid @RequestBody OutstandingStudentRequest request) { mentorStudyAchievementService.withdrawOutstandingStudent(studyId, request); - return ResponseEntity.ok().build(); + return ResponseEntity.ok() + .body(new ApiResponse<>("우수 스터디원 지정이 철회되었습니다.")); }src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyAchievementV2CustomRepositoryImpl.java (2)
26-34: 카운트 쿼리 성능 최적화 필요카운트 쿼리의 성능을 개선하고 null 체크를 보완할 필요가 있습니다.
@Override public long countByStudyIdAndAchievementTypeAndStudentIds( Long studyId, AchievementType achievementType, List<Long> studentIds) { + if (studentIds == null || studentIds.isEmpty()) { + return 0L; + } - return Objects.requireNonNull(queryFactory + Long count = queryFactory .select(studyAchievementV2.count()) .from(studyAchievementV2) .where(eqStudyId(studyId), eqAchievementType(achievementType), containsStudentId(studentIds)) - .fetchOne()); + .fetchOne(); + return count != null ? count : 0L; }
44-46: 리스트 검증 로직 개선 필요memberIds 리스트의 유효성을 더 엄격하게 검증할 필요가 있습니다.
private BooleanExpression containsStudentId(List<Long> memberIds) { - return memberIds != null ? studyAchievementV2.student.id.in(memberIds) : null; + return (memberIds != null && !memberIds.isEmpty()) ? + studyAchievementV2.student.id.in(memberIds.stream() + .filter(Objects::nonNull) + .collect(Collectors.toList())) : + null; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/com/gdschongik/gdsc/domain/studyv2/api/MentorStudyAchievementControllerV2.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/application/MentorStudyAchievementServiceV2.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyAchievementV2CustomRepository.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyAchievementV2CustomRepositoryImpl.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyAchievementV2Repository.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyHistoryV2CustomRepository.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyHistoryV2CustomRepositoryImpl.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/StudyAchievementValidatorV2.java(1 hunks)
🔇 Additional comments (6)
src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyHistoryV2CustomRepositoryImpl.java (3)
6-6: 새로운 임포트가 적절히 추가되었습니다!BooleanExpression과 Objects 클래스의 추가는 새로운 기능 구현에 필요한 적절한 변경사항입니다.
Also applies to: 9-9
26-32: 조건식 추출이 잘 이루어졌습니다!studyId 비교 로직을 별도의 메서드로 추출하여 코드의 재사용성과 가독성이 향상되었습니다.
34-36: 도메인 로직이 잘 캡슐화되었습니다!eqStudyId 메서드는 간단하지만 도메인 로직을 잘 캡슐화하고 있으며, 메서드 이름이 그 목적을 명확하게 전달합니다.
src/main/java/com/gdschongik/gdsc/domain/studyv2/application/MentorStudyAchievementServiceV2.java (1)
29-37: 필드 주입 체계가 명확합니다.
필요한 의존성을 명확히 주입하고 있어 가독성이 높습니다. 다만, 향후 확장성을 고려하여 너무 많은 필드를 주입하게 되면 서비스 역할이 비대해질 우려가 있으니 모듈화를 고려하셔도 좋겠습니다.src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyAchievementV2Repository.java (1)
1-8: 커스텀 리포지토리와 함께 JpaRepository를 사용해 확장성이 높습니다.
StudyAchievementV2CustomRepository의 메서드들과 표준 JpaRepository 함수가 결합되므로, 구현 유연성이 좋아 보입니다. 추후 복잡한 쿼리를 더 많이 추가해야 할 때도 확장하기에 용이한 구조입니다.src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyAchievementV2CustomRepository.java (1)
6-13: 메서드 시그니처가 명확해 이해하기 쉽습니다.
deleteByStudyAndAchievementTypeAndMemberIds와countByStudyIdAndAchievementTypeAndStudentIds모두 파라미터의 의미가 직관적이라 코드 사용 시 혼동이 적을 것으로 보입니다. 대규모 쿼리 실행 시 성능 검증만 병행한다면 문제없이 활용 가능할 것으로 판단됩니다.
| @Transactional | ||
| public void designateOutstandingStudent(Long studyId, OutstandingStudentRequest request) { | ||
| Member currentMember = memberUtil.getCurrentMember(); | ||
| StudyV2 study = studyRepository.findById(studyId).orElseThrow(() -> new CustomException(STUDY_NOT_FOUND)); | ||
| long countByStudyIdAndStudentIds = | ||
| studyHistoryRepository.countByStudyIdAndStudentIds(studyId, request.studentIds()); | ||
| long studyAchievementsAlreadyExistCount = | ||
| studyAchievementRepository.countByStudyIdAndAchievementTypeAndStudentIds( | ||
| studyId, request.achievementType(), request.studentIds()); | ||
|
|
||
| studyValidator.validateStudyMentor(currentMember, study); | ||
| studyHistoryValidator.validateAppliedToStudy( | ||
| countByStudyIdAndStudentIds, request.studentIds().size()); | ||
| studyAchievementValidator.validateDesignateOutstandingStudent(studyAchievementsAlreadyExistCount); | ||
|
|
||
| List<Member> outstandingStudents = memberRepository.findAllById(request.studentIds()); | ||
| List<StudyAchievementV2> studyAchievements = outstandingStudents.stream() | ||
| .map(member -> StudyAchievementV2.create(request.achievementType(), member, study)) | ||
| .toList(); | ||
| studyAchievementRepository.saveAll(studyAchievements); | ||
|
|
||
| log.info( | ||
| "[MentorStudyAchievementServiceV2] 우수 스터디원 지정: studyId={}, studentIds={}", | ||
| studyId, | ||
| request.studentIds()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
메서드의 예외 처리 및 유효성 검증이 체계적으로 잘 구성되었습니다.
studyValidator, studyHistoryValidator, studyAchievementValidator를 통해 역할별 검증이 철저히 이루어지고 있습니다. 다만, memberRepository.findAllById(request.studentIds()) 중 일부 ID가 실제로 존재하지 않는 경우도 고려하여 누락된 Member가 있을 시 추가적인 예외 처리를 추가하면 데이터 무결성 확보에 도움이 될 것으로 보입니다.
| public ResponseEntity<Void> designateOutstandingStudent( | ||
| @RequestParam(name = "studyId") Long studyId, @RequestBody OutstandingStudentRequest request) { | ||
| mentorStudyAchievementService.designateOutstandingStudent(studyId, request); | ||
| return ResponseEntity.ok().build(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
API 응답 및 유효성 검사 개선 필요
다음 개선사항을 고려해보세요:
- 입력값 유효성 검사 (
@Valid어노테이션 추가) - 성공 시 201 Created 상태 코드 사용
- 작업 결과를 포함하는 응답 본문 추가
@PostMapping
public ResponseEntity<Void> designateOutstandingStudent(
- @RequestParam(name = "studyId") Long studyId, @RequestBody OutstandingStudentRequest request) {
+ @RequestParam(name = "studyId") Long studyId, @Valid @RequestBody OutstandingStudentRequest request) {
mentorStudyAchievementService.designateOutstandingStudent(studyId, request);
- return ResponseEntity.ok().build();
+ return ResponseEntity.status(HttpStatus.CREATED)
+ .body(new ApiResponse<>("우수 스터디원 지정이 완료되었습니다."));
}Committable suggestion skipped: line range outside the PR's diff.
| @Override | ||
| public void deleteByStudyAndAchievementTypeAndMemberIds( | ||
| Long studyId, AchievementType achievementType, List<Long> memberIds) { | ||
| queryFactory | ||
| .delete(studyAchievementV2) | ||
| .where(eqStudyId(studyId), eqAchievementType(achievementType), containsStudentId(memberIds)) | ||
| .execute(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
벌크 삭제 작업 트랜잭션 관리 필요
벌크 삭제 작업은 트랜잭션 내에서 수행되어야 하며, 영속성 컨텍스트와의 동기화가 필요합니다.
+@Transactional
@Override
public void deleteByStudyAndAchievementTypeAndMemberIds(
Long studyId, AchievementType achievementType, List<Long> memberIds) {
+ if (memberIds == null || memberIds.isEmpty()) {
+ return;
+ }
queryFactory
.delete(studyAchievementV2)
.where(eqStudyId(studyId), eqAchievementType(achievementType), containsStudentId(memberIds))
.execute();
+ // 영속성 컨텍스트 초기화
+ entityManager.flush();
+ entityManager.clear();
}📝 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.
| @Override | |
| public void deleteByStudyAndAchievementTypeAndMemberIds( | |
| Long studyId, AchievementType achievementType, List<Long> memberIds) { | |
| queryFactory | |
| .delete(studyAchievementV2) | |
| .where(eqStudyId(studyId), eqAchievementType(achievementType), containsStudentId(memberIds)) | |
| .execute(); | |
| } | |
| @Transactional | |
| @Override | |
| public void deleteByStudyAndAchievementTypeAndMemberIds( | |
| Long studyId, AchievementType achievementType, List<Long> memberIds) { | |
| if (memberIds == null || memberIds.isEmpty()) { | |
| return; | |
| } | |
| queryFactory | |
| .delete(studyAchievementV2) | |
| .where(eqStudyId(studyId), eqAchievementType(achievementType), containsStudentId(memberIds)) | |
| .execute(); | |
| // 영속성 컨텍스트 초기화 | |
| entityManager.flush(); | |
| entityManager.clear(); | |
| } |
| public void designateOutstandingStudent(Long studyId, OutstandingStudentRequest request) { | ||
| Member currentMember = memberUtil.getCurrentMember(); | ||
| StudyV2 study = studyRepository.findById(studyId).orElseThrow(() -> new CustomException(STUDY_NOT_FOUND)); | ||
| long countByStudyIdAndStudentIds = |
There was a problem hiding this comment.
count가 suffix가 되어야 할 것 같네요. 아래는 그렇게 되어있고요
There was a problem hiding this comment.
간단한 로직이라면 상단에서 전부 조회하고 -> 도메인 로직을 호출해도 되겠지만...
수행하는 로직들이 많기 때문에 필요한 로직에 맞게 조회 호출 위치를 조정해야 할 것 같네요
- studyValidator + studyHistoryValidator 호출할 때는 현재 멤버, 스터디만 필요하니 여기까지만 조회하고 검증 로직 호출
- studyAchievementValidator 호출하기 전에 studyAchievementAlreadyExistCount 조회해오기
- 그리고 지정 대상으로 학생들 조회해오고 + 우수내역 만들고 + 저장
이렇게 세 단으로 나눠서 조회 + 로직 수행을 구분하면 좋을듯요
There was a problem hiding this comment.
count가 suffix가 되어야 할 것 같네요. 아래는 그렇게 되어있고요
ByStudyIdAndStudentIds는 메서드명으로도 알 수 있으니 studyHistoryCount로 수정했습니다.
There was a problem hiding this comment.
- vementVali
리뷰 내용대로 전반적인 메서드 호출 순서를 조정했어요.
아래에 있는 철회 메서드도 같이 수정했습니다~
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
| @DomainService | ||
| public class StudyAchievementValidatorV2 { | ||
|
|
||
| public void validateDesignateOutstandingStudent(long countStudyAchievementsAlreadyExist) { |
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit