-
Notifications
You must be signed in to change notification settings - Fork 1
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: 레포지토리 입력 API path variable 수정 #735
fix: 레포지토리 입력 API path variable 수정 #735
Conversation
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
|
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: 1
Outside diff range, codebase verification and nitpick comments (2)
src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java (2)
24-27
: 코드베이스에서studyHistoryId
사용 확인 필요
studyHistoryId
가 여전히 사용되고 있는 두 곳이 발견되었습니다. 이 변경이 일관되게 적용되었는지 확인하기 위해 아래 파일들을 검토해야 합니다:
src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyService.java
src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java
이 파일들에서
studyHistoryId
의 사용이 의도된 것인지, 아니면studyId
로 변경되어야 하는지 확인해 주세요.Analysis chain
코드베이스 전체에서 일관성 확인 필요
이 변경이 다른 부분에 영향을 미칠 수 있으므로, 코드베이스 전체에서
studyHistoryId
에서studyId
로의 변경이 일관되게 적용되었는지 확인해야 합니다.다음 스크립트를 실행하여
studyHistoryId
의 사용을 확인하세요:Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: 코드베이스에서 'studyHistoryId'의 사용을 확인합니다. # Test: 'studyHistoryId'가 여전히 사용되고 있는지 확인합니다. 예상 결과: 일치하는 항목이 없어야 합니다. rg --type java 'studyHistoryId' # Test: 'studyId'가 올바르게 사용되고 있는지 확인합니다. 예상 결과: 변경된 부분에서 'studyId'가 사용되어야 합니다. rg --type java 'studyId'Length of output: 17831
24-27
: 로그 메시지에서studyHistoryId
업데이트 필요API 문서나 Swagger 주석에서는
studyHistoryId
가 사용되지 않지만, 다음의 로그 메시지에서 여전히 사용되고 있습니다. 코드베이스의 일관성을 위해 이 부분도 업데이트하는 것이 좋습니다.
src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java
: 로그 메시지에서studyHistoryId
사용src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyService.java
: 로그 메시지에서studyHistoryId
사용Analysis chain
API 문서 업데이트 확인
studyHistoryId
에서studyId
로의 변경으로 인해 API 문서나 Swagger 주석 등이 업데이트되어야 할 수 있습니다.다음 스크립트를 실행하여 API 문서나 Swagger 주석을 확인하세요:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: API 문서나 Swagger 주석에서 'studyHistoryId'의 사용을 확인합니다. # Test: API 문서나 Swagger 주석에서 'studyHistoryId'가 여전히 사용되고 있는지 확인합니다. # 예상 결과: 일치하는 항목이 없거나, 있다면 업데이트가 필요한 부분입니다. rg --type java -C 3 'studyHistoryId' | rg '(@Operation|@ApiOperation|@ApiParam)'Length of output: 80
Script:
#!/bin/bash # Description: 전체 코드베이스에서 'studyHistoryId'의 사용을 확인합니다. # Test: 코드베이스에서 'studyHistoryId'가 여전히 사용되고 있는지 확인합니다. # 예상 결과: 일치하는 항목이 없거나, 있다면 업데이트가 필요한 부분입니다. rg 'studyHistoryId'Length of output: 368
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java (2 hunks)
Additional comments not posted (4)
src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java (1)
24-27
: 변경 사항이 올바르게 구현되었습니다.
studyHistoryId
에서studyId
로의 변경이 API 경로와 메서드 시그니처에 모두 적용되었습니다. 이는 PR 목표와 연결된 이슈 #734의 요구사항을 충족합니다.src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java (3)
10-10
: import 문 추가 승인
StudyRepository
import 문이 적절히 추가되었습니다.
47-47
: StudyRepository 필드 추가 승인
StudyRepository
필드가 적절히 추가되었습니다.final
키워드 사용으로 불변성이 보장됩니다.
50-50
: updateRepository 메서드 시그니처 변경 승인
studyHistoryId
에서studyId
로의 매개변수 변경이 PR 목표와 일치합니다.
Study study = studyRepository.findById(studyId).orElseThrow(() -> new CustomException(STUDY_NOT_FOUND)); | ||
StudyHistory studyHistory = studyHistoryRepository | ||
.findById(studyHistoryId) | ||
.findByStudentAndStudy(currentMember, study) |
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.
updateRepository 메서드 구현 변경 승인 및 개선 제안
studyId
를 사용하여 Study
객체를 조회하고, 이를 통해 StudyHistory
를 가져오는 변경이 적절합니다.
개선 제안:
Study
조회 실패 시 로그 추가StudyHistory
조회 실패 시 더 자세한 예외 메시지 제공
다음과 같이 코드를 개선할 수 있습니다:
- Study study = studyRepository.findById(studyId).orElseThrow(() -> new CustomException(STUDY_NOT_FOUND));
+ Study study = studyRepository.findById(studyId)
+ .orElseThrow(() -> {
+ log.error("Study not found with id: {}", studyId);
+ return new CustomException(STUDY_NOT_FOUND);
+ });
StudyHistory studyHistory = studyHistoryRepository
.findByStudentAndStudy(currentMember, study)
- .orElseThrow(() -> new CustomException(STUDY_HISTORY_NOT_FOUND));
+ .orElseThrow(() -> new CustomException(STUDY_HISTORY_NOT_FOUND,
+ String.format("StudyHistory not found for member %s and study %d", currentMember.getId(), studyId)));
Committable suggestion was skipped due to low confidence.
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
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
🌱 관련 이슈
studyId
를 받도록 수정 #734📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit
기능 개선
studyHistoryId
에서studyId
로 변경하여 명확성을 향상시켰습니다.updateRepository
메서드가 이제StudyRepository
와 직접 상호작용하여 데이터 무결성을 개선합니다.버그 수정