-
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
feat: 스터디 과제 레포지터리를 수정할 수 있도록 개선 #822
base: develop
Are you sure you want to change the base?
feat: 스터디 과제 레포지터리를 수정할 수 있도록 개선 #822
Conversation
Walkthrough이 변경 사항은 Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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: 0
🧹 Outside diff range and nitpick comments (2)
src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java (2)
68-68
: 테스트 용이성 개선이 필요합니다.TODO 주석에서 언급된 대로 GHRepository를 래퍼 클래스로 감싸서 테스트 가능하도록 개선이 필요합니다. 이는 단위 테스트의 신뢰성과 유지보수성을 높이는 중요한 개선사항입니다.
다음과 같은 래퍼 인터페이스와 구현체를 제안드립니다. 작성을 도와드릴까요?
public interface RepositoryWrapper { String getOwnerOAuthId(); String getRepositoryUrl(); } public class GHRepositoryWrapper implements RepositoryWrapper { private final GHRepository repository; // implementation }
Line range hint
71-75
: 로깅 메시지 개선을 제안드립니다.현재 로깅 메시지에 studyId를 포함하면 디버깅에 더 도움이 될 것 같습니다.
- log.info( - "[StudyHistoryService] 레포지토리 입력: studyHistoryId={}, repositoryLink={}", - studyHistory.getId(), - request.repositoryLink()); + log.info( + "[StudyHistoryService] 레포지토리 입력: studyId={}, studyHistoryId={}, repositoryLink={}", + studyId, + studyHistory.getId(), + request.repositoryLink());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java
(1 hunks)src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java
(1 hunks)src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryCustomRepository.java
(0 hunks)src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryCustomRepositoryImpl.java
(0 hunks)src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidator.java
(1 hunks)src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidatorTest.java
(1 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryCustomRepository.java
- src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryCustomRepositoryImpl.java
🔇 Additional comments (4)
src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidator.java (1)
Line range hint 42-47
: 과제 제출 여부 검증 제거에 대한 검토가 필요합니다.
validateUpdateRepository 메서드에서 과제 제출 여부 검증이 제거되었습니다. 이는 PR의 목적에 부합하지만, 다음 사항들을 확인해 주시기 바랍니다:
- 과제가 제출된 후에도 레포지토리를 수정할 수 있도록 하는 것이 비즈니스 요구사항에 맞는지
- 과제 평가나 이력 관리에 영향이 없는지
현재 구현은 레포지토리 소유자 검증만을 수행하여 코드가 더 명확해졌습니다.
✅ Verification successful
과제 제출 여부와 관계없이 레포지토리 수정이 가능하도록 변경된 것이 확인되었습니다.
코드베이스 분석 결과, 다음과 같은 사항들이 확인되었습니다:
StudentStudyDetailService
에서isAnySubmitted
상태는 단순히 대시보드 표시 용도로만 사용되고 있습니다.StudyHistoryValidator
의validateUpdateRepository
메서드는 레포지토리 소유자 검증만을 수행하도록 의도적으로 변경되었습니다.- 과제 제출 상태(
AssignmentHistory.isSubmitted()
)는 레포지토리 수정과 독립적으로 관리됩니다.
이는 학생들이 과제 제출 후에도 레포지토리를 자유롭게 수정할 수 있도록 하는 의도적인 설계 변경으로 보입니다. 과제 평가는 AssignmentHistory
를 통해 독립적으로 관리되므로, 이력 관리에도 영향이 없습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 과제 제출 후 레포지토리 수정과 관련된 비즈니스 로직 검증
# 관련된 서비스 레이어에서 과제 제출 관련 로직 확인
rg -A 5 "Assignment.*Submit"
# 테스트 코드에서 관련 시나리오 확인
rg -A 5 "validateUpdateRepository.*test"
Length of output: 13290
Script:
#!/bin/bash
# 과제 제출 관련 로직과 레포지토리 수정 관련 코드 검색
# StudyHistory 클래스에서 레포지토리 수정 관련 로직 확인
ast-grep --pattern 'class StudyHistory {
$$$
updateRepository($$$) {
$$$
}
$$$
}'
# 과제 제출 상태와 레포지토리 수정 관련 테스트 코드 검색
rg -A 10 "class.*StudyHistoryTest|class.*StudyHistoryValidatorTest"
# AssignmentHistory 클래스에서 과제 제출 상태 관련 로직 확인
ast-grep --pattern 'class AssignmentHistory {
$$$
isSubmitted($$$) {
$$$
}
$$$
}'
Length of output: 2581
Script:
#!/bin/bash
# 과제 제출과 레포지토리 수정 관련 코드 추가 검색
# StudyHistory 클래스의 전체 내용 확인
rg -A 20 "class StudyHistory"
# 과제 제출 상태 변경 관련 로직 검색
rg -A 10 "updateRepository|setSubmitted|isSubmitted"
# 과제 제출 후 레포지토리 수정 관련 테스트 케이스 검색
rg -A 10 "test.*Repository.*Update|test.*Assignment.*Submit"
Length of output: 24031
src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java (1)
24-24
: API 설명이 개선되었습니다만, 관련 검증 로직을 확인해주세요.
API 설명이 "과제 제출 레포지토리를 입력합니다"로 명확하게 변경된 것은 좋은 개선입니다. 하지만 기존의 제출된 과제 검증 로직이 제거된 것으로 보이는데, 이로 인해 발생할 수 있는 영향을 확인해주시기 바랍니다.
관련 검증 로직의 변경사항을 확인하기 위해 다음 스크립트를 실행합니다:
✅ Verification successful
검증 로직이 적절히 구현되어 있습니다.
레포지토리 업데이트 관련 검증 로직을 확인한 결과, 다음과 같은 중요한 검증이 유지되고 있습니다:
- 레포지토리 소유자와 현재 사용자의 일치 여부 확인
- GitHub 레포지토리 존재 여부 검증
- 레포지토리 링크 업데이트 기능의 정상 동작
API 설명 변경은 현재 구현된 검증 로직과 일관성이 있으며, 안전하게 처리되고 있습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 관련된 검증 로직의 변경사항 확인
# StudyHistoryValidator 클래스에서 검증 로직 확인
ast-grep --pattern 'class StudyHistoryValidator {
$$$
validateUpdateRepository($$$) {
$$$
}
$$$
}'
# StudentStudyHistoryService 클래스에서 updateRepository 메서드 확인
ast-grep --pattern 'class StudentStudyHistoryService {
$$$
updateRepository($$$) {
$$$
}
$$$
}'
Length of output: 222
Script:
#!/bin/bash
# Let's try a different approach to find the relevant files and their content
# Find all Java files related to study history
fd -e java -x rg -l "StudyHistory" {}
# Search for repository update related code
rg -A 10 "updateRepository|validateRepository" --type java
Length of output: 10017
Script:
#!/bin/bash
# Let's check the StudyHistoryValidator implementation
rg -A 10 "class StudyHistoryValidator" --type java
# Also check for any validation-related error codes
rg "REPOSITORY|VALIDATION" ./src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java
Length of output: 2963
Script:
#!/bin/bash
# Let's check the implementation of validateUpdateRepository method
rg -A 10 "validateUpdateRepository" --type java
# And check for any recent changes to validation logic
git diff HEAD~1 -- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidator.java
Length of output: 4369
src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidatorTest.java (1)
110-110
: 과제 제출 검증 로직 제거에 대한 검토 필요
레포지토리 소유권 검증을 단순화한 것은 좋은 변경이지만, 과제 제출 여부 확인 로직이 제거된 것이 안전한지 검토가 필요합니다. 이전 버전에서는 과제가 이미 제출된 경우를 체크했는데, 이 검증이 제거됨으로써 발생할 수 있는 부작용이 없는지 확인이 필요합니다.
다음 사항들을 고려해보시기 바랍니다:
- 과제 제출 검증이 다른 계층으로 이동되었는지
- 이 변경으로 인해 발생할 수 있는 비즈니스 로직상의 문제는 없는지
- 추가적인 테스트 케이스가 필요한지
src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java (1)
67-67
: 검증 로직이 단순화되어 개선되었습니다.
레포지토리 소유권 검증이 더 명확하고 간단해졌습니다. 불필요한 과제 제출 여부 확인을 제거한 것이 좋은 변경사항입니다.
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
🌱 관련 이슈
📌 작업 내용 및 특이사항
isAnyAssignmentSubmitted
변수 제거 및 관련 레포지토리 코드 삭제📝 참고사항
📚 기타
Summary by CodeRabbit
버그 수정
updateRepository
메서드의 설명이 명확하게 수정되었습니다.StudyHistoryValidator
의validateUpdateRepository
메서드에서 불필요한 매개변수가 제거되었습니다.기능 변경
AssignmentHistoryCustomRepository
에서 제출된 과제가 있는지 확인하는 메서드가 제거되었습니다.StudyHistoryValidatorTest
에서 관련 테스트 케이스가 삭제되었습니다.