-
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 구현 #911
Conversation
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
📝 WalkthroughWalkthrough새로운 API 기능이 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Controller
participant S as Service
participant R as StudyHistoryRepository
participant V as Validator
participant G as GithubClient
participant M as MemberUtil
C->>S: updateMyRepository(studyId, request)
S->>M: getCurrentMember()
S->>R: find study by studyId
S->>R: find StudyHistory by (member, study)
S->>G: getOwnerId(repositoryLink)
G-->>S: ownerId
S->>V: validateUpdateRepository(ownerId, currentMember)
V-->>S: 검증 완료 혹은 예외 발생
S->>R: save 업데이트된 StudyHistory
S-->>C: HTTP 200 OK
Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
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 (
|
✅ Actions performedReview triggered.
|
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
🧹 Nitpick comments (4)
src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/StudyHistoryValidatorV2.java (1)
20-20
: 한글 주석을 영문으로 변경하는 것이 좋겠습니다.국제화 및 일관성을 위해 영문 주석 사용을 권장드립니다.
- // 레포지토리 소유자가 현 멤버가 아닌 경우 + // Throw exception if repository owner is not the current membersrc/main/java/com/gdschongik/gdsc/domain/studyv2/api/StudentStudyHistoryControllerV2.java (1)
26-30
: 응답 처리를 더 구체적으로 개선하면 좋겠습니다.현재는 단순히 200 OK를 반환하고 있습니다. 다음과 같은 개선을 제안드립니다:
- 성공 시 업데이트된 리소스의 위치를 반환
- 적절한 응답 본문 추가
- public ResponseEntity<Void> updateRepository( + public ResponseEntity<StudyHistoryResponse> updateRepository( @PathVariable Long studyId, @Valid @RequestBody StudyHistoryRepositoryUpdateRequest request) { studentStudyHistoryServiceV2.updateRepository(studyId, request); - return ResponseEntity.ok().build(); + return ResponseEntity + .ok() + .location(URI.create("/v2/study-history/" + studyId)) + .body(new StudyHistoryResponse(studyId, request.repositoryLink())); }src/main/java/com/gdschongik/gdsc/domain/studyv2/application/StudentStudyHistoryServiceV2.java (1)
31-49
: 레포지토리 업데이트 로직에 대한 개선 제안현재 구현은 기능적으로는 문제가 없으나, 다음 사항들을 고려해보시기 바랍니다:
- GitHub 레포지토리 링크 유효성 검사가 필요합니다
- 예외 처리 시 더 구체적인 메시지가 도움될 수 있습니다
다음과 같이 개선해보시는 것을 제안드립니다:
@Transactional public void updateRepository(Long studyId, StudyHistoryRepositoryUpdateRequest request) { Member currentMember = memberUtil.getCurrentMember(); StudyV2 study = studyV2Repository.findById(studyId).orElseThrow(() -> new CustomException(STUDY_NOT_FOUND)); StudyHistoryV2 studyHistory = studyHistoryV2Repository .findByStudentAndStudy(currentMember, study) .orElseThrow(() -> new CustomException(STUDY_HISTORY_NOT_FOUND)); + if (!isValidGithubRepositoryUrl(request.repositoryLink())) { + throw new CustomException(INVALID_GITHUB_REPOSITORY_URL); + } String repositoryOwnerId = githubClient.getOwnerId(request.repositoryLink()); studyHistoryValidatorV2.validateUpdateRepository(repositoryOwnerId, currentMember); studyHistory.updateRepositoryLink(request.repositoryLink()); studyHistoryV2Repository.save(studyHistory); log.info( "[료StudentStudyHistoryServiceV2] 레포지토리 입력 완료: studyHistoryId={}, repositoryLink={}", studyHistory.getId(), request.repositoryLink()); } +private boolean isValidGithubRepositoryUrl(String url) { + return url != null && url.matches("^https://github\\.com/[\\w-]+/[\\w-]+$"); +}src/main/java/com/gdschongik/gdsc/infra/github/client/GithubClient.java (1)
42-48
: getOwnerId 메소드에 대한 개선 제안메소드가 기본적인 기능은 잘 수행하고 있으나, 다음과 같은 개선사항을 제안드립니다:
- Javadoc 문서화 추가
- null 체크 추가
다음과 같이 개선해보시는 것을 제안드립니다:
+/** + * GitHub 레포지토리의 소유자 ID를 조회합니다. + * + * @param repo GitHub 레포지토리 URL + * @return 레포지토리 소유자의 ID + * @throws CustomException GITHUB_REPOSITORY_NOT_FOUND (레포지토리를 찾을 수 없는 경우) + */ public String getOwnerId(String repo) { + if (repo == null || repo.trim().isEmpty()) { + throw new CustomException(INVALID_REPOSITORY_URL); + } try { return String.valueOf(getRepository(repo).getOwner().getId()); } catch (IOException e) { throw new CustomException(GITHUB_REPOSITORY_NOT_FOUND); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/gdschongik/gdsc/domain/studyv2/api/StudentStudyHistoryControllerV2.java
(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/application/StudentStudyHistoryServiceV2.java
(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyHistoryV2Repository.java
(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/StudyHistoryValidatorV2.java
(2 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/request/StudyHistoryRepositoryUpdateRequest.java
(1 hunks)src/main/java/com/gdschongik/gdsc/infra/github/client/GithubClient.java
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyHistoryV2Repository.java (1)
Learnt from: uwoobeat
PR: GDSC-Hongik/gdsc-server#889
File: src/main/java/com/gdschongik/gdsc/domain/studyv2/api/MentorStudyControllerV2.java:35-39
Timestamp: 2025-02-12T11:11:19.196Z
Learning: In the GDSC server project's V2 domain, following DDD principles, StudyV2 is an aggregate root and StudySession is its child entity. Therefore, StudySession should only be accessed through StudyV2 aggregate root using StudyV2Repository, not directly via a separate StudySessionRepository.
🔇 Additional comments (5)
src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/request/StudyHistoryRepositoryUpdateRequest.java (1)
7-8
: 깔끔한 DTO 구현을 확인했습니다!Record를 사용한 불변 DTO 구현과 적절한 유효성 검증 애노테이션 적용이 잘 되어있습니다.
src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyHistoryV2Repository.java (1)
10-10
: 리포지토리 메서드 구현이 적절합니다!JPA 명명 규칙을 잘 따르고 있으며, DDD 원칙에 맞게 구현되어 있습니다.
src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/StudyHistoryValidatorV2.java (1)
19-26
: 검증 로직이 명확하게 구현되어 있습니다!소유권 검증 로직이 명확하고 예외 처리가 적절합니다.
src/main/java/com/gdschongik/gdsc/domain/studyv2/api/StudentStudyHistoryControllerV2.java (1)
16-16
: API 문서화가 잘 되어있습니다!Swagger 애노테이션을 통한 API 문서화가 명확하게 작성되어 있습니다.
src/main/java/com/gdschongik/gdsc/domain/studyv2/application/StudentStudyHistoryServiceV2.java (1)
1-30
: 클래스 구조가 잘 설계되어 있습니다.의존성 주입과 트랜잭션 관리가 적절하게 구현되어 있습니다.
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.
한가지 남겼는데 병목 있다고 하셔서 일단 어푸루브 했어요~
|
||
@Tag(name = "Student Study History V2", description = "학생 스터디 수강이력 API입니다.") | ||
@RestController | ||
@RequestMapping("/v2/study-history") |
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.
이건 histories로 안 해도 되려나요?
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
log.info( | ||
"[료StudentStudyHistoryServiceV2] 레포지토리 입력 완료: studyHistoryId={}, 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.
오타 있어요!
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: 1
🧹 Nitpick comments (1)
src/main/java/com/gdschongik/gdsc/domain/studyv2/api/StudentStudyHistoryControllerV2.java (1)
23-28
: 응답 타입과 문서화를 개선할 수 있습니다.
- 현재
ResponseEntity<Void>
를 반환하고 있는데, 업데이트된 리소스의 정보를 포함하는 것이 RESTful API 설계에 더 적합할 수 있습니다.- OpenAPI 문서화에 가능한 응답 코드와 예외 상황도 추가하면 좋을 것 같습니다.
예시:
@Operation( summary = "내 레포지토리 입력", description = "나의 과제 제출 레포지토리를 입력합니다.", responses = { @ApiResponse(responseCode = "200", description = "성공적으로 업데이트됨"), @ApiResponse(responseCode = "404", description = "스터디 또는 수강이력을 찾을 수 없음"), @ApiResponse(responseCode = "400", description = "잘못된 요청") } )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/gdschongik/gdsc/domain/studyv2/api/StudentStudyHistoryControllerV2.java
(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/application/StudentStudyHistoryServiceV2.java
(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/request/StudyHistoryRepositoryUpdateRequest.java
(1 hunks)
🔇 Additional comments (2)
src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/request/StudyHistoryRepositoryUpdateRequest.java (1)
9-11
: 코드가 깔끔하고 잘 구현되어 있습니다!Record를 사용한 DTO 구현과 적절한 validation 제약 조건이 잘 적용되어 있습니다.
src/main/java/com/gdschongik/gdsc/domain/studyv2/application/StudentStudyHistoryServiceV2.java (1)
25-29
: 의존성 주입이 잘 구현되어 있습니다.필요한 의존성들이 명확하게 정의되어 있고,
@RequiredArgsConstructor
를 통한 생성자 주입이 잘 구현되어 있습니다.
@Transactional | ||
public void updateMyRepository(StudyHistoryRepositoryUpdateRequest request) { | ||
Member currentMember = memberUtil.getCurrentMember(); | ||
StudyV2 study = | ||
studyV2Repository.findById(request.studyId()).orElseThrow(() -> new CustomException(STUDY_NOT_FOUND)); | ||
StudyHistoryV2 studyHistory = studyHistoryV2Repository | ||
.findByStudentAndStudy(currentMember, study) | ||
.orElseThrow(() -> new CustomException(STUDY_HISTORY_NOT_FOUND)); | ||
String repositoryOwnerId = githubClient.getOwnerId(request.repositoryLink()); | ||
|
||
studyHistoryValidatorV2.validateUpdateRepository(repositoryOwnerId, currentMember); | ||
|
||
studyHistory.updateRepositoryLink(request.repositoryLink()); | ||
studyHistoryV2Repository.save(studyHistory); | ||
|
||
log.info( | ||
"[StudentStudyHistoryServiceV2] 내 레포지토리 입력 완료: studyHistoryId={}, repositoryLink={}", | ||
studyHistory.getId(), | ||
request.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.
🛠️ Refactor suggestion
예외 처리와 로깅을 개선할 수 있습니다.
- GitHub API 호출 시 발생할 수 있는 예외에 대한 처리가 필요합니다.
- 로그 메시지에 예외 상황에 대한 정보도 포함하면 좋을 것 같습니다.
다음과 같이 개선해보세요:
@Transactional
public void updateMyRepository(StudyHistoryRepositoryUpdateRequest request) {
Member currentMember = memberUtil.getCurrentMember();
+ log.debug("[StudentStudyHistoryServiceV2] 레포지토리 업데이트 시작: memberId={}, studyId={}",
+ currentMember.getId(), request.studyId());
StudyV2 study =
studyV2Repository.findById(request.studyId()).orElseThrow(() -> new CustomException(STUDY_NOT_FOUND));
StudyHistoryV2 studyHistory = studyHistoryV2Repository
.findByStudentAndStudy(currentMember, study)
.orElseThrow(() -> new CustomException(STUDY_HISTORY_NOT_FOUND));
- String repositoryOwnerId = githubClient.getOwnerId(request.repositoryLink());
+
+ String repositoryOwnerId;
+ try {
+ repositoryOwnerId = githubClient.getOwnerId(request.repositoryLink());
+ } catch (Exception e) {
+ log.error("[StudentStudyHistoryServiceV2] GitHub API 호출 실패: {}", e.getMessage());
+ throw new CustomException(GITHUB_API_ERROR);
+ }
studyHistoryValidatorV2.validateUpdateRepository(repositoryOwnerId, currentMember);
studyHistory.updateRepositoryLink(request.repositoryLink());
studyHistoryV2Repository.save(studyHistory);
log.info(
"[StudentStudyHistoryServiceV2] 내 레포지토리 입력 완료: studyHistoryId={}, repositoryLink={}",
studyHistory.getId(),
request.repositoryLink());
}
📝 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.
@Transactional | |
public void updateMyRepository(StudyHistoryRepositoryUpdateRequest request) { | |
Member currentMember = memberUtil.getCurrentMember(); | |
StudyV2 study = | |
studyV2Repository.findById(request.studyId()).orElseThrow(() -> new CustomException(STUDY_NOT_FOUND)); | |
StudyHistoryV2 studyHistory = studyHistoryV2Repository | |
.findByStudentAndStudy(currentMember, study) | |
.orElseThrow(() -> new CustomException(STUDY_HISTORY_NOT_FOUND)); | |
String repositoryOwnerId = githubClient.getOwnerId(request.repositoryLink()); | |
studyHistoryValidatorV2.validateUpdateRepository(repositoryOwnerId, currentMember); | |
studyHistory.updateRepositoryLink(request.repositoryLink()); | |
studyHistoryV2Repository.save(studyHistory); | |
log.info( | |
"[StudentStudyHistoryServiceV2] 내 레포지토리 입력 완료: studyHistoryId={}, repositoryLink={}", | |
studyHistory.getId(), | |
request.repositoryLink()); | |
} | |
@Transactional | |
public void updateMyRepository(StudyHistoryRepositoryUpdateRequest request) { | |
Member currentMember = memberUtil.getCurrentMember(); | |
log.debug("[StudentStudyHistoryServiceV2] 레포지토리 업데이트 시작: memberId={}, studyId={}", | |
currentMember.getId(), request.studyId()); | |
StudyV2 study = | |
studyV2Repository.findById(request.studyId()) | |
.orElseThrow(() -> new CustomException(STUDY_NOT_FOUND)); | |
StudyHistoryV2 studyHistory = studyHistoryV2Repository | |
.findByStudentAndStudy(currentMember, study) | |
.orElseThrow(() -> new CustomException(STUDY_HISTORY_NOT_FOUND)); | |
String repositoryOwnerId; | |
try { | |
repositoryOwnerId = githubClient.getOwnerId(request.repositoryLink()); | |
} catch (Exception e) { | |
log.error("[StudentStudyHistoryServiceV2] GitHub API 호출 실패: {}", e.getMessage()); | |
throw new CustomException(GITHUB_API_ERROR); | |
} | |
studyHistoryValidatorV2.validateUpdateRepository(repositoryOwnerId, currentMember); | |
studyHistory.updateRepositoryLink(request.repositoryLink()); | |
studyHistoryV2Repository.save(studyHistory); | |
log.info( | |
"[StudentStudyHistoryServiceV2] 내 레포지토리 입력 완료: studyHistoryId={}, repositoryLink={}", | |
studyHistory.getId(), | |
request.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.
lgtm
🌱 관련 이슈
📌 작업 내용 및 특이사항
기존과 동일하긴 한데, 사소한 변경사항이 있습니다.
GithubClient
로부터GHRepository
를 직접 가져오는 대신 레포지터리 오우너 정보만 가져오도록 개선했습니다.Objects.equals()
사용하도록 변경했습니다.0223 20:11 업데이트 사항
/study-histories/{studyId}
와 같은 식이었는데요, 스터디 수강이력은 스터디 수강이력 ID로 식별해야 하며 studyId로 검색하고 싶은 경우/study-histories?studyId={studyId}
와 같은 식으로 설계되어야 합니다./me
suffix를 붙여주는 것이 적절합니다.PUT /study-histories/repositories/me
가 적절하다고 판단했습니다.updateRepository()
를updateMyRepository()
로 변경했습니다.📝 참고사항
📚 기타
Summary by CodeRabbit