feat: 스터디 수료 내역 조회 API #798
Conversation
Walkthrough이 변경 사항은 Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (7)
src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAchievementRepository.java (2)
8-8: 인터페이스 선언이 적절합니다.JpaRepository를 확장하여 StudyAchievement 엔티티에 대한 CRUD 작업을 제공하는 것이 좋습니다.
인터페이스에 간단한 JavaDoc 주석을 추가하여 이 리포지토리의 목적을 설명하는 것이 좋을 것 같습니다. 예를 들어:
/** * Repository interface for managing StudyAchievement entities. * Provides CRUD operations and custom queries for StudyAchievement. */ public interface StudyAchievementRepository extends JpaRepository<StudyAchievement, Long> { // ... }
10-10: 커스텀 메서드 선언이 적절합니다.
findAllByStudent메서드는 Spring Data JPA 쿼리 메서드 명명 규칙을 잘 따르고 있으며, PR의 목적인 학습 완료 이력 조회 기능을 잘 구현하고 있습니다.메서드에 JavaDoc 주석을 추가하여 메서드의 목적과 반환값을 설명하는 것이 좋을 것 같습니다. 예를 들어:
/** * 주어진 학생의 모든 학습 성취를 조회합니다. * * @param student 조회할 학생 * @return 학생의 모든 StudyAchievement 목록 */ List<StudyAchievement> findAllByStudent(Member student);src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudentMyCompleteStudyResponse.java (2)
10-21: 레코드 선언이 잘 구조화되어 있습니다.레코드의 구조가 완료된 학습을 표현하기에 적절해 보입니다. 각 필드에 대한
@Schema주석은 API 문서화에 도움이 될 것입니다.
achievements필드의 설명을 더 명확하게 할 수 있습니다. 현재 "우수 스터디원 여부"로 되어 있는데, 이는 여러 유형의 성취를 포함할 수 있음을 나타내지 않습니다. 다음과 같이 변경하는 것이 어떨까요?-@Schema(description = "우수 스터디원 여부") List<AchievementType> achievements +@Schema(description = "획득한 성취 유형 목록") List<AchievementType> achievements
23-36: 팩토리 메서드가 잘 구현되어 있습니다.
of메서드는StudyHistory와achievements를 이용해StudentMyCompleteStudyResponse인스턴스를 생성하는 편리한 방법을 제공합니다. 도메인 객체에서 응답 DTO로의 매핑이 잘 이루어져 있습니다.null 안전성을 개선하기 위해 몇 가지 null 체크를 추가하는 것이 좋을 것 같습니다. 예를 들어:
public static StudentMyCompleteStudyResponse of(StudyHistory studyHistory, List<AchievementType> achievements) { if (studyHistory == null || studyHistory.getStudy() == null) { throw new IllegalArgumentException("StudyHistory or Study cannot be null"); } return new StudentMyCompleteStudyResponse( studyHistory.getStudy().getId(), studyHistory.getStudy().getAcademicYear(), studyHistory.getStudy().getSemesterType(), studyHistory.getStudy().getTitle(), studyHistory.getStudy().getStudyType() != null ? studyHistory.getStudy().getStudyType().getValue() : null, studyHistory.getStudy().getNotionLink(), studyHistory.getStudy().getIntroduction(), studyHistory.getStudy().getMentor() != null ? studyHistory.getStudy().getMentor().getName() : null, studyHistory.getStudy().getTotalWeek(), studyHistory.getStudyHistoryStatus(), achievements != null ? achievements : List.of()); }이렇게 하면 예상치 못한 null 포인터 예외를 방지할 수 있습니다.
src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java (1)
47-52: 새로운 엔드포인트가 성공적으로 구현되었습니다.
getMyCompleteStudy()메서드가 PR의 목표에 맞게 잘 구현되었습니다. API 문서화를 위한@Operation어노테이션, 적절한 HTTP 메서드와 경로, 그리고 서비스 계층으로의 로직 위임이 모두 잘 되어 있습니다.한 가지 제안사항:
에러 처리를 개선하기 위해 try-catch 블록을 추가하는 것을 고려해보세요. 예를 들어:
@GetMapping("/me/complete") public ResponseEntity<?> getMyCompleteStudy() { try { List<StudentMyCompleteStudyResponse> response = studentStudyHistoryService.getMyCompleteStudies(); return ResponseEntity.ok(response); } catch (Exception e) { // 로그 기록 return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body("An error occurred while fetching the study history."); } }이렇게 하면 예기치 않은 오류가 발생했을 때 더 우아하게 처리할 수 있습니다.
src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java (2)
6-7: import 문 변경 사항 검토와일드카드 import를 사용하여
dao와domain패키지의 import를 간소화했습니다. 이는 코드를 더 간결하게 만들지만, 명시성이 떨어질 수 있습니다. 새로 추가된 import들은 새 메서드 구현에 필요한 것으로 보입니다.와일드카드 import 사용에 대한 팀의 코딩 규칙을 확인하고, 필요하다면 명시적인 import로 변경하는 것을 고려해 보세요.
Also applies to: 10-10, 16-16, 18-18, 20-20
105-125: 새 메서드getMyCompleteStudies()검토새로운 메서드
getMyCompleteStudies()가 PR 목표에 맞게 잘 구현되었습니다. Java 스트림과 컬렉터를 효율적으로 사용하여 데이터를 처리하고 있습니다.코드가 잘 구조화되어 있고 좋은 관행을 따르고 있습니다.
achievementTypes에 대한 null 체크를 통해 NullPointerException을 방지하는 것도 좋습니다.성능 최적화를 위해 다음과 같은 작은 개선을 고려해 보세요:
- if (achievementTypes == null) { - achievementTypes = new ArrayList<>(); - } + achievementTypes = achievementTypes == null ? List.of() : achievementTypes;이렇게 하면 불필요한
ArrayList생성을 피하고 불변 빈 리스트를 사용할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java (3 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAchievementRepository.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudentMyCompleteStudyResponse.java (1 hunks)
🧰 Additional context used
🔇 Additional comments (6)
src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyAchievementRepository.java (2)
1-6: 패키지 선언과 임포트가 적절합니다.패키지 구조가 프로젝트 구조를 잘 반영하고 있으며, 필요한 모든 클래스가 올바르게 임포트되어 있습니다.
1-11: 전반적인 구현이 PR 목표와 잘 부합합니다.이
StudyAchievementRepository인터페이스는 PR의 주요 목표인 학습 완료 이력 조회 API 구현을 위한 중요한 구성 요소입니다. JpaRepository를 확장하여 기본적인 CRUD 작업을 제공하면서,findAllByStudent메서드를 통해 특정 학생의 학습 성취를 효율적으로 조회할 수 있게 해줍니다.이 구현은 Spring Data JPA의 모범 사례를 잘 따르고 있으며, 멘티 페이지에서 학습 완료 기록을 쉽게 접근할 수 있게 하는 PR의 목표를 잘 지원합니다.
src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java (2)
6-6: 새로운 import 문이 적절히 추가되었습니다.
StudentMyCompleteStudyResponse클래스의 import 문이 올바르게 추가되었습니다. 이는 새로 추가된 메서드에서 사용되는 반환 타입을 위해 필요합니다.
Line range hint
1-52: 전반적인 변경 사항이 PR 목표와 잘 부합합니다.이 PR의 주요 목표인 학습 완료 이력을 조회하는 API 구현이 성공적으로 이루어졌습니다. 새로운 엔드포인트
/me/complete가 추가되어 사용자가 자신의 학습 성과를 조회할 수 있게 되었습니다.코드 구조와 스타일이 기존 코드베이스와 일관성을 유지하고 있으며, 새로운 기능이 기존 기능들과 잘 통합되어 있습니다. 또한, API 문서화를 위한 어노테이션이 적절히 사용되어 있어 API 사용자들에게 명확한 정보를 제공할 수 있습니다.
다만, 새로운 기능과 관련된 단위 테스트가 추가되었는지 확인이 필요합니다. 다음 스크립트를 실행하여 관련 테스트 파일의 존재 여부를 확인해 주세요:
테스트 파일이 존재하지 않는다면, 새로운 기능에 대한 단위 테스트를 추가하는 것을 고려해 주세요.
src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java (2)
41-41: 새 필드 추가 승인
studyAchievementRepository필드가 적절하게 추가되었습니다. 이는 새로운 메서드에서 학습 성취를 처리하는 데 필요한 것으로 보입니다.필드가
private final로 올바르게 선언되었고, 클래스의 기존 패턴을 따르고 있어 좋습니다.
Line range hint
1-126: 전체 변경 사항 요약이 PR은 학습 수료 내역 조회 API 구현이라는 목표를 성공적으로 달성했습니다.
getMyCompleteStudies()메서드를 통해 사용자의 학습 성취를 효과적으로 조회할 수 있게 되었습니다. 코드는 전반적으로 잘 구조화되어 있고, Java의 최신 기능들을 적절히 활용하고 있습니다.변경 사항들이 PR의 목표와 잘 부합하며, 코드 품질도 우수합니다. 제안된 작은 개선 사항들을 검토하고 반영하면 더욱 좋을 것 같습니다.
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java (1)
118-138: 새로운 메서드getMyCompleteStudies()가 잘 구현되었습니다.이 메서드는 PR의 목표인 학습 완료 내역 조회 API 구현을 잘 달성하고 있습니다. 트랜잭션 처리, 현재 사용자 조회, 그리고 스트림 API를 사용한 데이터 처리가 적절히 이루어졌습니다.
다만, 성능 최적화를 위해 다음과 같은 작은 개선을 제안합니다:
- Map<Study, List<AchievementType>> achievementsByStudy = studyAchievements.stream() - .collect(Collectors.groupingBy( - StudyAchievement::getStudy, - Collectors.mapping(StudyAchievement::getAchievementType, Collectors.toList()))); + Map<Study, List<AchievementType>> achievementsByStudy = studyAchievements.stream() + .collect(Collectors.groupingBy( + StudyAchievement::getStudy, + Collectors.mapping(StudyAchievement::getAchievementType, Collectors.toCollection(ArrayList::new))));이렇게 하면 내부적으로
ArrayList를 직접 사용하여 약간의 성능 향상을 얻을 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java (3 hunks)
🧰 Additional context used
🔇 Additional comments (2)
src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java (2)
7-7: 새로운 import 문이 적절히 추가되었습니다.새로 추가된 import 문들이 새로운 기능 구현에 필요한 요소들을 적절히 포함하고 있습니다. 이는 코드의 가독성과 유지보수성을 향상시킵니다.
Also applies to: 11-11, 23-23, 29-29, 31-31, 33-33
54-54: 새로운 필드가 적절히 추가되었습니다.
studyAchievementRepository필드가 올바르게 선언되었습니다. 이는 private final로 선언되어 있어 불변성을 보장하며, 명명 규칙을 따르고 있습니다. 이 필드는 새로운 기능 구현에 필요한 StudyAchievement 엔티티에 대한 데이터 접근을 제공할 것으로 보입니다.
|
Pr 제목도 컨벤션 있어요! |
src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java
Outdated
Show resolved
Hide resolved
kckc0608
changed the title
|
PR 제목 컨벤션을 잊었네요.. 감사합니다. |
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java (1)
47-52: LGTM: 새로운 엔드포인트가 올바르게 구현되었습니다.새로 추가된
getMyCompletedStudy메서드는 전반적으로 잘 구현되었습니다. API 문서화를 위한@Operation어노테이션이 적절히 사용되었고, 엔드포인트 매핑과 반환 타입이 올바르게 설정되었습니다.다만, 메서드 이름의 일관성을 위해 다음과 같은 개선을 제안합니다:
컨트롤러의 메서드 이름을 서비스 메서드 이름과 일치시키는 것이 좋겠습니다. 다음과 같이 변경하는 것을 고려해보세요:
- public ResponseEntity<List<StudentMyCompleteStudyResponse>> getMyCompletedStudy() { + public ResponseEntity<List<StudentMyCompleteStudyResponse>> getMyCompletedStudies() {이렇게 하면 컨트롤러와 서비스 계층 간의 일관성이 향상됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java (3 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudentMyCompleteStudyResponse.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java
- src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudentMyCompleteStudyResponse.java
🧰 Additional context used
🔇 Additional comments (2)
src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java (2)
6-6: LGTM: 새로운 import 문이 올바르게 추가되었습니다.
StudentMyCompleteStudyResponse클래스의 import 문이 적절하게 추가되었습니다. 이는 새로 추가된 메서드의 반환 타입에 필요한 것으로 보입니다.
49-49: 이전 리뷰 코멘트 관련: 메서드 이름 변경 제안이전 리뷰에서 Sangwook02님이 제안한 메서드 이름 변경에 대해 고려해 보시기 바랍니다. 제안된 이름은 다음과 같습니다:
public ResponseEntity<List<StudentMyCompleteStudyResponse>> getMyCompletedStudy() {이 제안은 현재 구현과 일치하므로, 이미 반영된 것으로 보입니다. 하지만 앞서 언급한 서비스 메서드와의 일관성을 위해
getMyCompletedStudies()로 변경하는 것이 더 좋을 수 있습니다.
| @Schema(description = "멘토 이름") String mentorName, | ||
| @Schema(description = "총 주차수") Long totalWeek, | ||
| @Schema(description = "수료 여부") StudyHistoryStatus studyHistoryStatus, | ||
| @Schema(description = "우수 스터디원 여부") List<AchievementType> achievements) { |
There was a problem hiding this comment.
널러블 어노테이션도 달아주면 좋을 것 같아요~
| @Schema(description = "우수 스터디원 여부") List<AchievementType> achievements) { | |
| @Nullable @Schema(description = "우수 스터디원 여부") List<AchievementType> achievements) { |
| studyHistory.getStudy().getMentor().getName(), | ||
| studyHistory.getStudy().getTotalWeek(), | ||
| studyHistory.getStudyHistoryStatus(), | ||
| (achievements == null) ? new ArrayList<>() : achievements); |
There was a problem hiding this comment.
| (achievements == null) ? new ArrayList<>() : achievements); | |
| (achievements == null) ? null : achievements); |
There was a problem hiding this comment.
이렇게 작성한다면 삼항 연산자를 사용하지 않아도 괜찮은 것 같아서, 삼항 연산자 빼고 @nullable만 추가할게요
src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java
Outdated
Show resolved
Hide resolved
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudentMyCompleteStudyResponse.java
Outdated
Show resolved
Hide resolved
| @Schema(description = "멘토 이름") String mentorName, | ||
| @Schema(description = "총 주차수") Long totalWeek, | ||
| @Schema(description = "수료 여부") StudyHistoryStatus studyHistoryStatus, | ||
| @Nullable @Schema(description = "우수 스터디원 여부") List<AchievementType> achievements) { |
There was a problem hiding this comment.
'우수 스터디원 여부' 보다 더 적절한 네이밍이 있을 것 같습니다.
There was a problem hiding this comment.
'우수 스터디원 선정 내역' 정도로 고쳐봤는데, 다른 아이디어가 잘 안떠오르네요. 혹시 좋은 아이디어 있을까요?
There was a problem hiding this comment.
넹 그정도면 될 것 같아요
'여부'는 뭔가 boolean과 어울리는 느낌이어서...
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
쿼리로 해결해볼까 하다가, 하나의 스터디에 StudyAchievement 가 여러개 있을 수 있어서 (1차 우수 스터디원, 2차 우수 스터디원 모두 선발) Collectors를 사용하여 리스트로 AchievementType을 받도록 구현하였습니다.
피그마에서 봤을 때는 1차, 2차 구분이 안되어있는 것 같아서 우수 스터디원 선발 횟수로 반환해볼까도 고민해봤는데 어떤 게 더 좋은지 의견 있으시면 부탁드립니다.
📚 기타
Summary by CodeRabbit
신규 기능
getMyCompletedStudies()메서드).StudentMyCompleteStudyResponse레코드 추가.문서화