fix: 출석체크 여러번 가능한 문제 해결 #745
Conversation
|
Warning Rate limit exceeded@seulgi99 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 5 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Walkthrough변경 사항은 Changes
Assessment against linked issues
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
|
src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java
Outdated
Show resolved
Hide resolved
![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
Outside diff range, codebase verification and nitpick comments (4)
src/main/java/com/gdschongik/gdsc/domain/study/dao/AttendanceRepository.java (1)
6-8: 변경 사항이 적절해 보입니다.새로운 메서드
existsByStudentIdAndStudyDetailId는 PR의 목적인 다중 출석 체크 문제를 해결하는 데 도움이 될 것 같습니다.메서드의 목적을 명확히 하기 위해 다음과 같은 주석을 추가하는 것이 좋겠습니다:
public interface AttendanceRepository extends JpaRepository<Attendance, Long>, AttendanceCustomRepository { + /** + * 특정 학생이 특정 스터디 세부 사항에 대해 이미 출석했는지 확인합니다. + * @param studentId 학생 ID + * @param studyDetailId 스터디 세부 사항 ID + * @return 출석 기록이 존재하면 true, 그렇지 않으면 false + */ boolean existsByStudentIdAndStudyDetailId(Long studentId, Long studyDetailId); }src/main/java/com/gdschongik/gdsc/domain/study/domain/AttendanceValidator.java (2)
11-12: 메서드 시그니처 변경이 적절합니다.
isAlreadyAttended매개변수를 추가한 것은 중복 출석 체크 문제를 해결하는 데 도움이 됩니다.메서드의 Javadoc을 업데이트하여 새로운 매개변수에 대한 설명을 추가하는 것이 좋겠습니다. 다음과 같이 Javadoc을 추가할 수 있습니다:
/** * 출석을 검증합니다. * @param studyDetail 학습 세부 정보 * @param attendanceNumber 출석 번호 * @param date 출석 날짜 * @param isAlreadyAttended 이미 출석했는지 여부 * @throws CustomException 출석 검증에 실패한 경우 */ public void validateAttendance( StudyDetail studyDetail, String attendanceNumber, LocalDate date, boolean isAlreadyAttended) { // ... 메서드 본문 ... }
23-27: 새로운 검증 로직이 적절히 추가되었습니다.이미 출석한 경우에 대한 검증 로직이 잘 구현되었습니다.
주석을 더 명확하게 수정하는 것이 좋겠습니다. 다음과 같이 변경할 수 있습니다:
// 중복 출석 검증 if (isAlreadyAttended) { throw new CustomException(ALREADY_ATTENDED_STUDY_DETAIL); }src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1)
133-133: 새로운 에러 코드 추가 승인새로운 에러 코드
ALREADY_ATTENDED_STUDY_DETAIL가 적절하게 추가되었습니다. 이는 PR의 목적인 다중 출석 체크 문제 해결에 부합합니다.가독성을 위해 에러 코드를 관련 그룹에 배치하는 것이 좋습니다. 예를 들어,
ATTENDANCE_DATE_INVALID와ATTENDANCE_NUMBER_MISMATCH바로 다음에 배치하는 것을 고려해 보세요.ATTENDANCE_DATE_INVALID(HttpStatus.CONFLICT, "강의일이 아니면 출석체크할 수 없습니다."), ATTENDANCE_NUMBER_MISMATCH(HttpStatus.CONFLICT, "출석번호가 일치하지 않습니다."), + ALREADY_ATTENDED_STUDY_DETAIL(HttpStatus.CONFLICT, "이미 출석 처리된 스터디입니다."), // Order ORDER_NOT_FOUND(HttpStatus.NOT_FOUND, "주문이 존재하지 않습니다."),
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyService.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dao/AttendanceRepository.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/AttendanceValidator.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1 hunks)
- src/test/java/com/gdschongik/gdsc/domain/study/domain/AttendanceValidatorTest.java (3 hunks)
Additional comments not posted (5)
src/test/java/com/gdschongik/gdsc/domain/study/domain/AttendanceValidatorTest.java (5)
4-4: 새로운 에러 코드 import 추가
ALREADY_ATTENDED_STUDY_DETAIL에러 코드가 추가되었습니다. 이는 새로운 테스트 케이스와 관련이 있어 보입니다.
59-62: 기존 테스트 케이스 수정: 출석번호 검증여기서도
validateAttendance메서드 호출에 새로운 boolean 매개변수false가 추가되었습니다. 이는 일관성 있는 변경으로 보입니다.
64-83: 새로운 테스트 케이스 추가: 중복 출석 검증이 새로운 테스트 케이스는 사용자가 이미 출석한 경우를 검증합니다. 다음 사항들이 적절히 구현되었습니다:
- 테스트 설정이 기존 테스트들과 일관성 있게 구성되었습니다.
validateAttendance메서드 호출 시 마지막 인자로true를 전달하여 이미 출석한 상태를 시뮬레이션합니다.ALREADY_ATTENDED_STUDY_DETAIL에러 코드를 사용하여 예외를 검증합니다.이 테스트 케이스는 PR의 목적인 중복 출석 방지 기능을 직접적으로 검증하고 있어 매우 중요합니다.
Line range hint
1-83: 전반적인 변경 사항 검토이 파일의 변경 사항들은 PR의 목적인 "출석체크 여러번 가능한 문제 해결"을 잘 반영하고 있습니다:
validateAttendance메서드에 새로운 boolean 매개변수가 추가되어 이미 출석했는지 여부를 확인할 수 있게 되었습니다.- 기존 테스트 케이스들이 새로운 메서드 시그니처에 맞게 업데이트되었습니다.
- 중복 출석을 검증하는 새로운 테스트 케이스가 추가되었습니다.
이러한 변경 사항들은 출석 체크 시스템의 견고성을 향상시키고, 중복 출석 문제를 효과적으로 해결할 것으로 보입니다.
40-42: 기존 테스트 케이스 수정: 출석일자 검증
validateAttendance메서드 호출에 새로운 boolean 매개변수false가 추가되었습니다. 이는 아직 출석하지 않은 상태를 나타내는 것으로 보입니다.다음 스크립트를 실행하여
validateAttendance메서드의 새로운 시그니처가 일관되게 적용되었는지 확인하세요:Verification successful
validateAttendance메서드 시그니처 일관성 확인 완료
validateAttendance메서드가 모든 호출에서 새로운 시그니처(4개의 인자)를 사용하고 있음을 확인했습니다. 코드베이스 전반에 걸쳐 일관성이 유지되고 있습니다.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: validateAttendance 메서드의 새로운 시그니처 확인 # Test: validateAttendance 메서드 호출 검색. 예상: 모든 호출이 4개의 인자를 가짐 rg --type java -A 5 $'validateAttendance\('Length of output: 4026
| final boolean isAlreadyAttended = | ||
| attendanceRepository.existsByStudentIdAndStudyDetailId(currentMember.getId(), studyDetailId); | ||
| final StudyHistory studyHistory = studyHistoryRepository | ||
| .findByStudentAndStudy(currentMember, study) | ||
| .orElseThrow(() -> new CustomException(STUDY_HISTORY_NOT_FOUND)); | ||
|
|
||
| attendanceValidator.validateAttendance(studyDetail, request.attendanceNumber(), LocalDate.now()); | ||
| attendanceValidator.validateAttendance( | ||
| studyDetail, request.attendanceNumber(), LocalDate.now(), isAlreadyAttended); |
There was a problem hiding this comment.
출석 중복 체크 로직이 잘 구현되었습니다.
변경사항이 이슈 #741을 해결하는 데 효과적으로 보입니다. 다음과 같은 개선 사항을 고려해 보시기 바랍니다:
- 에러 처리: 이미 출석한 경우에 대한 명확한 예외를 추가하는 것이 좋겠습니다.
- 코드 구조:
isAlreadyAttended체크 로직을 별도의 메서드로 분리하여 가독성을 높일 수 있습니다.
다음과 같은 리팩토링을 제안합니다:
@Transactional
public void attend(Long studyDetailId, StudyAttendCreateRequest request) {
final StudyDetail studyDetail = studyDetailRepository
.findById(studyDetailId)
.orElseThrow(() -> new CustomException(STUDY_DETAIL_NOT_FOUND));
final Member currentMember = memberUtil.getCurrentMember();
final Study study = studyDetail.getStudy();
- final boolean isAlreadyAttended =
- attendanceRepository.existsByStudentIdAndStudyDetailId(currentMember.getId(), studyDetailId);
+ final boolean isAlreadyAttended = checkAlreadyAttended(currentMember.getId(), studyDetailId);
final StudyHistory studyHistory = studyHistoryRepository
.findByStudentAndStudy(currentMember, study)
.orElseThrow(() -> new CustomException(STUDY_HISTORY_NOT_FOUND));
attendanceValidator.validateAttendance(
studyDetail, request.attendanceNumber(), LocalDate.now(), isAlreadyAttended);
+ if (isAlreadyAttended) {
+ throw new CustomException(ALREADY_ATTENDED);
+ }
Attendance attendance = Attendance.create(currentMember, studyDetail);
attendanceRepository.save(attendance);
log.info("[StudyService] 스터디 출석: attendanceId={}", attendance.getId());
}
+private boolean checkAlreadyAttended(Long memberId, Long studyDetailId) {
+ return attendanceRepository.existsByStudentIdAndStudyDetailId(memberId, studyDetailId);
+}이렇게 하면 코드의 가독성이 향상되고 에러 처리가 더 명확해집니다.
Committable suggestion was skipped due to low confidence.
src/test/java/com/gdschongik/gdsc/domain/study/domain/AttendanceValidatorTest.java
Outdated
Show resolved
Hide resolved
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit
New Features
Bug Fixes
Tests