-
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: 레포지토리 입력 API 추가 #594
feat: 레포지토리 입력 API 추가 #594
Conversation
Walkthrough이번 변경 사항은 학생의 과제 제출 기록을 관리하는 RESTful API를 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Service
participant Repository
Client->>Controller: PUT /study-history/{studyHistoryId}/repository
Controller->>Service: updateRepository(studyHistoryId, request)
Service->>Repository: findStudyHistory(studyHistoryId)
Repository-->>Service: StudyHistory
Service->>Service: validateUpdateRepository(isAnyAssignmentSubmitted)
Service->>Repository: updateRepositoryLink(newRepositoryLink)
Repository-->>Service: Success
Service-->>Controller: Response 200 OK
Controller-->>Client: Success response
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
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.
LGTM 코멘트하나 남겨놨습니다!
@@ -42,4 +46,12 @@ public ResponseEntity<Void> cancelStudyApply(@PathVariable Long studyId) { | |||
studyService.cancelStudyApply(studyId); | |||
return ResponseEntity.noContent().build(); | |||
} | |||
|
|||
@Operation(summary = "레포지토리 입력", description = "레포지토리를 입력합니다. 이미 제출한 과제가 있다면 수정할 수 없습니다.") | |||
@PatchMapping("/{studyId}/repository") |
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.
이건 궁금증인데 혹시 이 플젝에서 Patch Put을 쓰는 기준이 있나요? 어떤건 PUT인거같고 어떤건 PATCH인거같아서요!
저는 이전 플젝들 할때는 PATCH로 고정해서 사용했었거든요!
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.
테이블에 있는 필드들을 전반적으로 수정할 때는 put을 사용하고 있고요. 특정 필드만 수정할 때는 patch 사용하고 있습니다!
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.
저는 이것도 put이 맞는 것 같네요
patch -> 해당 endpoint가 나타내는 리소스의 일부분만 수정
put -> 해당 endpoint가 나타내는 리소스의 전체를 수정 + 덮어쓰기
그리고 레포지터리는 수강이력에서 관리되는 리소스이기 때문에 endpoint도 수정되어야 합니다
@@ -42,4 +46,12 @@ public ResponseEntity<Void> cancelStudyApply(@PathVariable Long studyId) { | |||
studyService.cancelStudyApply(studyId); | |||
return ResponseEntity.noContent().build(); | |||
} | |||
|
|||
@Operation(summary = "레포지토리 입력", description = "레포지토리를 입력합니다. 이미 제출한 과제가 있다면 수정할 수 없습니다.") | |||
@PatchMapping("/{studyId}/repository") |
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.
저는 이것도 put이 맞는 것 같네요
patch -> 해당 endpoint가 나타내는 리소스의 일부분만 수정
put -> 해당 endpoint가 나타내는 리소스의 전체를 수정 + 덮어쓰기
그리고 레포지터리는 수강이력에서 관리되는 리소스이기 때문에 endpoint도 수정되어야 합니다
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
일단 어프루브 드릴게요!
다른 분들이 수정해달라고 하신것만 보시면 될듯합니다
githubClient.getRepository(ownerRepo); | ||
} | ||
|
||
private String getOwnerRepo(String repositoryLink) { |
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.
REpository라고 끝까지 치는게 더 좋지 않을까용
…into feature/560-add-repository-link-to-study-history
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/application/StudyHistoryService.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/application/StudyService.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryCustomRepository.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryCustomRepositoryImpl.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryRepository.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistory.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidator.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dto/request/RepositoryUpdateRequest.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/global/common/constant/GithubConstant.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/infra/client/github/GithubClient.java (2 hunks)
- src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidatorTest.java (1 hunks)
Files skipped from review due to trivial changes (2)
- src/main/java/com/gdschongik/gdsc/domain/study/application/StudyService.java
- src/main/java/com/gdschongik/gdsc/global/common/constant/GithubConstant.java
Additional comments not posted (18)
src/main/java/com/gdschongik/gdsc/domain/study/dto/request/RepositoryUpdateRequest.java (1)
1-5
: 코드가 잘 작성되었습니다!
RepositoryUpdateRequest
클래스는@NotBlank
를 사용하여repositoryLink
필드에 유효성 검사를 추가하여 빈 문자열이 입력되지 않도록 하고 있습니다. 이는 적절한 유효성 검사입니다.src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryCustomRepository.java (1)
1-9
: 인터페이스가 명확하게 정의되었습니다!
existsSubmittedAssignmentByMemberAndStudy
메서드는 명확한 목적을 가지고 있으며, 과제가 제출되었는지 확인하는 데 유용할 것입니다.src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryRepository.java (1)
1-7
: 레포지토리 인터페이스가 잘 구성되었습니다!
AssignmentHistoryRepository
는JpaRepository
와AssignmentHistoryCustomRepository
를 확장하여 표준 CRUD 연산과 사용자 정의 쿼리 메서드를 모두 활용할 수 있습니다.src/main/java/com/gdschongik/gdsc/infra/client/github/GithubClient.java (2)
3-4
: 필요한 예외 클래스 임포트
CustomException
과ErrorCode
클래스를 임포트하여 예외 처리를 강화했습니다. 이는 코드의 견고성을 높이는 좋은 방법입니다.
17-23
: 예외 처리 개선
getRepository
메서드는IOException
을CustomException
으로 변환하여 명확한 오류 메시지를 제공합니다. 이는 사용자 경험을 향상시키는 좋은 접근 방식입니다.src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java (2)
16-20
: API 태그 및 설명 추가
@Tag
와@Operation
을 사용하여 API의 목적과 기능을 명확히 설명하는 것은 문서화 측면에서 매우 유용합니다.
24-30
: 유효성 검사 및 서비스 호출
updateRepository
메서드는 유효성 검사를 수행하고 서비스 계층을 호출하여 비즈니스 로직을 처리합니다. 이는 좋은 설계 패턴을 따르고 있습니다.src/main/java/com/gdschongik/gdsc/domain/study/dao/AssignmentHistoryCustomRepositoryImpl.java (4)
18-26
: 과제 제출 여부 확인 로직
existsSubmittedAssignmentByMemberAndStudy
메서드는 특정 회원과 스터디에 대한 과제 제출 여부를 정확히 확인합니다. QueryDSL을 활용한 효율적인 쿼리 작성이 돋보입니다.
28-30
: BooleanExpression 메서드: eqMember
eqMember
메서드는 회원이 null일 경우를 처리하여 잠재적인 NPE를 방지합니다. 이는 좋은 방어적 프로그래밍의 예입니다.
32-34
: BooleanExpression 메서드: eqStudy
eqStudy
메서드는 스터디가 null일 경우를 처리하여 잠재적인 NPE를 방지합니다. 이는 좋은 방어적 프로그래밍의 예입니다.
36-38
: BooleanExpression 메서드: isSubmitted
isSubmitted
메서드는 과제 제출 상태를 확인하는 데 사용되며, 실패 및 성공 상태를 모두 포함하여 정확한 상태 확인이 가능합니다.src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidator.java (1)
41-46
: 새로운 메서드 추가:validateUpdateRepository
이 메서드는 과제가 이미 제출된 경우 저장소 업데이트를 방지하기 위해
CustomException
을 던집니다. 이 검증은 과제 제출 후 저장소 변경을 막기 위한 것으로, PR의 목적과 잘 맞습니다. 코드의 논리와 예외 처리가 적절하게 구현되었습니다.src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistory.java (2)
36-36
: 새로운 필드 추가:repositoryLink
repositoryLink
필드는 저장소 링크를 저장하기 위해 추가되었습니다. 이 필드는private
으로 선언되어 캡슐화가 잘 되어 있습니다.
48-53
: 새로운 메서드 추가:updateRepositoryLink
이 메서드는
repositoryLink
필드를 업데이트하는 기능을 제공합니다. 메서드가public
으로 선언되어 외부 클래스에서 접근이 가능합니다. 메서드의 기능과 목적이 명확하게 주석으로 설명되어 있습니다.src/main/java/com/gdschongik/gdsc/domain/study/application/StudyHistoryService.java (2)
24-49
: 새로운 클래스 및 메서드:StudyHistoryService
StudyHistoryService
클래스는 저장소 업데이트를 처리하며,validateUpdateRepository
를 통해 검증을 수행합니다.@Transactional
을 사용하여 데이터 일관성을 유지하고 있으며, 로깅을 통해 중요한 작업을 기록합니다. 전체적으로 서비스 클래스의 구조와 메서드 구현이 명확하고 목적에 부합합니다.
51-59
: 저장소 링크 검증 메서드:validateRepositoryLink
및getOwnerRepo
validateRepositoryLink
메서드는 저장소 링크의 유효성을 검사하며,GithubClient
를 사용하여 저장소 정보를 가져옵니다.getOwnerRepo
메서드는 링크에서 소유자와 저장소 정보를 추출합니다. 두 메서드 모두 기능이 명확하고 잘 구현되어 있습니다.src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidatorTest.java (1)
100-113
: 테스트 케이스 추가가 적절합니다.
레포지토리_입력시
클래스의 테스트 메서드이미_제출한_과제가_있다면_실패한다
는 제출된 과제가 있을 때 예외가 발생하는지를 잘 검증하고 있습니다. 이 테스트는 유효성 검사를 강화하는 데 기여합니다.src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1)
120-120
: 에러 코드 추가가 적절합니다.
STUDY_HISTORY_REPOSITORY_NOT_UPDATABLE
와GITHUB_REPOSITORY_NOT_FOUND
에러 코드는 각각 레포지토리 수정 불가 상황과 GitHub 레포지토리를 찾을 수 없는 상황에 대한 명확한 피드백을 제공합니다. 이는 사용자 경험을 향상시키는 데 기여합니다.Also applies to: 146-149
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
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
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 (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java
Additional comments not posted (3)
src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java (3)
16-23
: Lombok 사용이 적절합니다.
@RequiredArgsConstructor
와@Slf4j
를 사용하여 코드의 간결성을 높였습니다.
32-49
:updateRepository
메서드 구현이 적절합니다.과제 제출 전 레포지토리 링크를 수정할 수 있도록 유효성 검사를 수행합니다. 로깅도 적절하게 사용되었습니다.
유효성 검사 로직이 다른 부분에서 올바르게 작동하는지 확인하세요.
Verification successful
유효성 검사 로직이 올바르게 구현되었습니다.
updateRepository
메서드에서validateUpdateRepository
가 적절하게 호출되고 있으며, 테스트 코드에서 과제 제출 시CustomException
이 발생하는지를 확인하고 있습니다. 이로 인해 유효성 검사 로직이 올바르게 작동함을 확인할 수 있습니다.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the validation logic for repository updates. # Test: Ensure the `validateUpdateRepository` method is correctly invoked. rg --type java -A 3 'validateUpdateRepository'Length of output: 1778
51-59
: 레포지토리 링크 유효성 검사 메서드가 적절합니다.
GithubClient
를 사용하여 레포지토리의 존재를 확인하는 로직이 잘 구현되었습니다.getOwnerRepo
의 문자열 처리도 간결하고 효과적입니다.
getOwnerRepo
메서드가 다양한 링크 형식에서 올바르게 작동하는지 확인하세요.
/** | ||
* 레포지토리 링크를 업데이트합니다. | ||
*/ | ||
public void updateRepositoryLink(String repositoryLink) { |
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.
한번 등록시 변경 불가능하도록 정책 설정하면 좋을듯 합니다
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.
이 부분은 피그마에 '과제 최초 제출 시'까지 변경 가능하다고 써있더라고요.
validator에서 과제 제출 이력이 있는지 검증하는 것도 같은 이유입니다!
public void updateRepositoryLink(String repositoryLink) { | ||
this.repositoryLink = repositoryLink; | ||
} | ||
|
||
// 데이터 전달 로직 | ||
public boolean isStudyOngoing() { |
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.
이건 해당 PR건은 아니지만 올바른 책임의 메서드가 아닌 것 같네요
현재 스터디가 진행중인지의 여부를 반환하는 것에 대한 책임은 스터디 클래스가 가져야 합니다
스터디 히스토리가 아니라요
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.
그렇네요
별도 이슈로 처리할게요
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.
++ todo 달아두겠습니다~
|
||
boolean isAnyAssignmentSubmitted = | ||
assignmentHistoryRepository.existsSubmittedAssignmentByMemberAndStudy(currentMember, study); | ||
studyHistoryValidator.validateUpdateRepository(isAnyAssignmentSubmitted); |
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.
깃허브 클라이언트로 해당 레포의 정보를 가져오는 것으로 보이는데요
단순 존재 여부 말고도, 클라이언트를 통해 해당 레포지터리의 소유자가 멤버 정보에 저장된 깃허브 oauthid와 일치하는지 검증하는 로직이 필요할 것 같습니다
studyHistory.updateRepositoryLink(request.repositoryLink()); | ||
studyHistoryRepository.save(studyHistory); | ||
|
||
log.info("[StudyHistoryService] 레포지토리 입력: studyHistoryId={}", studyHistory.getId()); |
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.
입력된 레포지터리 url에 대해서도 로그를 남기면 좋을듯 합니다
…into feature/560-add-repository-link-to-study-history
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- 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/domain/StudyHistoryValidator.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (2 hunks)
- src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidatorTest.java (2 hunks)
Files skipped from review as they are similar to previous changes (5)
- src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java
- src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidator.java
- src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java
- src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidatorTest.java
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
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit
새로운 기능
버그 수정
문서화
테스트