feat: 어드민 전체 스터디 조회 V2 API 구현 #875
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Caution Review failedThe pull request is closed. Walkthrough이번 PR은 어드민 전체 스터디 조회 V2 API 구현을 위한 기능을 추가합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant AC as AdminStudyControllerV2
participant AS as AdminStudyServiceV2
participant R as StudyV2RepositoryImpl
C->>AC: GET /admin/studies
AC->>AS: getAllStudies()
AS->>R: findFetchAll()
R-->>AS: List<StudyV2>
AS-->>AC: Map to StudyManagerResponse
AC-->>C: ResponseEntity(List<StudyManagerResponse>)
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✨ 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 (
|
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: 1
🧹 Nitpick comments (9)
src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/response/StudyStudentResponse.java (1)
1-7: 문서화 개선이 필요합니다.레코드의 목적과 각 필드에 대한 설명이 포함된 JavaDoc 문서화가 필요합니다.
다음과 같이 문서화를 추가하는 것을 제안합니다:
package com.gdschongik.gdsc.domain.studyv2.dto.response; import com.gdschongik.gdsc.domain.studyv2.dto.dto.StudySessionStudentDto; import com.gdschongik.gdsc.domain.studyv2.dto.dto.StudyStudentDto; import java.util.List; +/** + * 학생 관점에서의 스터디 정보와 해당 스터디의 세션 목록을 포함하는 응답 DTO입니다. + * + * @param study 스터디 기본 정보 + * @param studySessions 스터디 세션 목록 + */ public record StudyStudentResponse(StudyStudentDto study, List<StudySessionStudentDto> studySessions) {}src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/dto/StudySessionStudentDto.java (2)
11-13: 상태 필드에 Enum 사용을 고려해주세요.
lessonAttendanceStatus와assignmentStatus는 String 대신 Enum을 사용하면 타입 안전성을 보장할 수 있습니다.예시:
public enum LessonAttendanceStatus { PRESENT, LATE, ABSENT } public enum AssignmentStatus { SUBMITTED, NOT_SUBMITTED, UNDER_REVIEW }
3-5: JavaDoc 문서화를 보강해주세요.현재 문서화는 출결번호가 포함되지 않았다는 점만 언급하고 있습니다. 각 필드의 의미와 용도에 대한 설명도 추가하면 좋겠습니다.
다음과 같이 보강하는 것을 제안합니다:
/** * 스터디 회차 학생 DTO입니다. 출결번호가 포함되어 있지 않습니다. + * + * @param studySessionId 스터디 세션 ID + * @param position 회차 번호 + * @param title 세션 제목 + * @param description 세션 설명 + * @param lessonAttendanceStatus 수업 출석 상태 + * @param assignmentDescriptionLink 과제 설명 링크 + * @param assignmentStatus 과제 제출 상태 + * @param studyId 스터디 ID */src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/dto/StudyStudentDto.java (2)
9-11: JavaDoc 문서화를 보강해주세요.현재 문서화는 디스코드 ID가 포함되지 않았다는 점만 언급하고 있습니다. 각 필드의 의미와 용도에 대한 설명도 추가하면 좋겠습니다.
다음과 같이 보강하는 것을 제안합니다:
/** * 스터디 학생 DTO입니다. 디스코드 관련 ID가 포함되어 있지 않습니다. + * + * @param studyId 스터디 ID + * @param type 스터디 유형 + * @param title 스터디 제목 + * @param description 스터디 설명 + * @param descriptionNotionLink 노션 설명 링크 + * @param semester 학기 + * @param totalRound 총 회차 수 + * @param dayOfWeek 요일 + * @param startTime 시작 시간 + * @param endTime 종료 시간 + * @param applicationPeriod 신청 기간 + * @param mentorId 멘토 ID */
12-24: 필드 구성을 논리적으로 그룹화하는 것을 고려해주세요.현재 필드들이 연관성에 따라 그룹화되어 있지 않습니다. 관련 필드들을 함께 배치하면 코드의 가독성이 향상될 것 같습니다.
다음과 같이 필드를 재구성하는 것을 제안합니다:
public record StudyStudentDto( Long studyId, + Long mentorId, StudyType type, String title, String description, String descriptionNotionLink, Semester semester, Integer totalRound, DayOfWeek dayOfWeek, LocalTime startTime, LocalTime endTime, - Period applicationPeriod, - Long mentorId) {} + Period applicationPeriod) {}src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyV2RepositoryImpl.java (1)
26-33: 페이지네이션 도입을 고려해보세요.fetch join을 통한 성능 최적화는 좋습니다. 하지만 데이터가 많아질 경우를 대비하여 페이지네이션 구현을 고려해보시는 것이 좋겠습니다.
예시 구현:
@Override - public List<StudyV2> findFetchAll() { + public Page<StudyV2> findFetchAll(Pageable pageable) { return queryFactory .selectFrom(studyV2) .join(studyV2.studySessions) .fetchJoin() - .fetch(); + .offset(pageable.getOffset()) + .limit(pageable.getPageSize()) + .fetchResults(); }src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/dto/StudySessionManagerDto.java (1)
19-30: 필드 유효성 검증 추가를 고려해보세요.from 메서드에서 null 체크나 기타 유효성 검증을 추가하면 더 안전한 코드가 될 것 같습니다.
예시 구현:
public static StudySessionManagerDto from(StudySessionV2 studySession) { + Objects.requireNonNull(studySession, "studySession must not be null"); + Objects.requireNonNull(studySession.getStudyV2(), "studyV2 must not be null"); return new StudySessionManagerDto( studySession.getId(), studySession.getPosition(), studySession.getTitle(), studySession.getDescription(), studySession.getLessonAttendanceNumber(), studySession.getLessonPeriod(), studySession.getAssignmentDescriptionLink(), studySession.getAssignmentPeriod(), studySession.getStudyV2().getId()); }src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/dto/StudyManagerDto.java (1)
28-44: Optional 필드에 대한 안전한 처리가 필요합니다.descriptionNotionLink, discordChannelId, discordRoleId와 같은 선택적 필드들에 대해 Optional을 사용하여 null 안전성을 보장하는 것이 좋겠습니다.
예시 구현:
public record StudyManagerDto( Long studyId, StudyType type, String title, String description, - String descriptionNotionLink, + Optional<String> descriptionNotionLink, Semester semester, Integer totalRound, DayOfWeek dayOfWeek, LocalTime startTime, LocalTime endTime, Period applicationPeriod, - String discordChannelId, - String discordRoleId, + Optional<String> discordChannelId, + Optional<String> discordRoleId, Long mentorId) { public static StudyManagerDto from(StudyV2 study) { return new StudyManagerDto( study.getId(), study.getType(), study.getTitle(), study.getDescription(), - study.getDescriptionNotionLink(), + Optional.ofNullable(study.getDescriptionNotionLink()), study.getSemester(), study.getTotalRound(), study.getDayOfWeek(), study.getStartTime(), study.getEndTime(), study.getApplicationPeriod(), - study.getDiscordChannelId(), - study.getDiscordRoleId(), + Optional.ofNullable(study.getDiscordChannelId()), + Optional.ofNullable(study.getDiscordRoleId()), study.getMentor().getId()); } }src/main/java/com/gdschongik/gdsc/domain/studyv2/application/AdminStudyServiceV2.java (1)
59-64: getAllStudies 메소드가 효율적으로 구현되었습니다.다음과 같은 점들이 잘 구현되어 있습니다:
- readOnly 트랜잭션 설정으로 성능 최적화
- Stream API를 활용한 간결한 엔티티-DTO 변환
- 메소드 체이닝을 통한 가독성 있는 코드
성능 최적화를 위해 페이징 처리 추가를 고려해보시기 바랍니다.
다음과 같이 페이징 처리를 추가하는 것을 제안합니다:
@Transactional(readOnly = true) - public List<StudyManagerResponse> getAllStudies() { + public Page<StudyManagerResponse> getAllStudies(Pageable pageable) { - return studyV2Repository.findFetchAll().stream() + return studyV2Repository.findFetchAll(pageable).stream() .map(StudyManagerResponse::from) - .toList(); + .collect(Collectors.toPage(pageable)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/com/gdschongik/gdsc/domain/studyv2/api/AdminStudyControllerV2.java(2 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/application/AdminStudyServiceV2.java(2 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyV2CustomRepository.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyV2RepositoryImpl.java(2 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/dto/StudyManagerDto.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/dto/StudySessionManagerDto.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/dto/StudySessionStudentDto.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/dto/StudyStudentDto.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/response/StudyManagerResponse.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/response/StudyStudentResponse.java(1 hunks)
🧰 Additional context used
📓 Learnings (2)
src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyV2RepositoryImpl.java (1)
Learnt from: Sangwook02
PR: GDSC-Hongik/gdsc-server#431
File: src/main/java/com/gdschongik/gdsc/domain/study/application/StudyService.java:50-57
Timestamp: 2024-11-12T14:06:41.465Z
Learning: Consider using Stream API for creating lists in a more concise and potentially performant manner compared to traditional for-loops.
src/main/java/com/gdschongik/gdsc/domain/studyv2/application/AdminStudyServiceV2.java (1)
Learnt from: Sangwook02
PR: GDSC-Hongik/gdsc-server#431
File: src/main/java/com/gdschongik/gdsc/domain/study/application/StudyService.java:50-57
Timestamp: 2024-11-12T14:06:41.465Z
Learning: Consider using Stream API for creating lists in a more concise and potentially performant manner compared to traditional for-loops.
🔇 Additional comments (5)
src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyV2CustomRepository.java (2)
1-11: 인터페이스가 잘 설계되었습니다!기존의
findFetchById메서드와 일관된 네이밍 패턴을 유지하면서 전체 스터디 조회 기능이 잘 추가되었습니다.Fetch접두사를 통해 즉시 로딩(eager loading)을 사용한다는 것을 명확히 표현하고 있습니다.
10-10: 성능 최적화 관련 검토가 필요할 수 있습니다.
findFetchAll()은 모든 스터디를 즉시 로딩으로 가져오는 메서드입니다. 데이터가 많아질 경우 성능 이슈가 발생할 수 있으므로, 다음 사항들을 고려해보시기 바랍니다:
- 페이징 처리 도입
- 필요한 필드만 선택적으로 조회하는 방식 검토
성능 최적화가 필요한지 확인하기 위해 현재 데이터 규모와 향후 예상되는 데이터 증가량을 검토해주시겠습니까?
src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/response/StudyManagerResponse.java (1)
8-16: 구현이 깔끔하고 적절합니다!레코드를 사용하여 불변성을 보장하고, Stream API를 활용하여 세션 매핑을 효율적으로 처리했습니다.
src/main/java/com/gdschongik/gdsc/domain/studyv2/api/AdminStudyControllerV2.java (1)
5-5: 새로운 import 문이 적절하게 추가되었습니다.StudyManagerResponse와 List의 import가 새로운 기능 구현에 필요한 최소한의 의존성만을 추가하고 있습니다.
Also applies to: 9-9
src/main/java/com/gdschongik/gdsc/domain/studyv2/application/AdminStudyServiceV2.java (1)
12-12: 필요한 import 문이 적절하게 추가되었습니다.StudyManagerResponse와 List의 import가 새로운 getAllStudies 메소드 구현에 필요한 최소한의 의존성만을 추가하고 있습니다.
Also applies to: 14-14
Comment on lines
+33
to
+38
| @Operation(summary = "전체 스터디 조회", description = "모든 스터디를 조회합니다. 코어멤버만 접근 가능합니다.") | ||
| @GetMapping | ||
| public ResponseEntity<List<StudyManagerResponse>> getStudies() { | ||
| var response = adminStudyServiceV2.getAllStudies(); | ||
| return ResponseEntity.ok(response); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
API 엔드포인트가 잘 구현되었습니다.
다음과 같은 점들이 잘 구현되어 있습니다:
- Operation 어노테이션을 통한 명확한 API 문서화
- 코어멤버 접근 제한에 대한 명시
- 간결하고 명확한 응답 처리
하지만 보안 관련 어노테이션이 누락되어 있습니다.
다음과 같이 보안 어노테이션을 추가하는 것을 권장합니다:
@Operation(summary = "전체 스터디 조회", description = "모든 스터디를 조회합니다. 코어멤버만 접근 가능합니다.")
+ @PreAuthorize("hasRole('CORE')")
@GetMapping
public ResponseEntity<List<StudyManagerResponse>> getStudies() {📝 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.
Suggested change
| @Operation(summary = "전체 스터디 조회", description = "모든 스터디를 조회합니다. 코어멤버만 접근 가능합니다.") | |
| @GetMapping | |
| public ResponseEntity<List<StudyManagerResponse>> getStudies() { | |
| var response = adminStudyServiceV2.getAllStudies(); | |
| return ResponseEntity.ok(response); | |
| } | |
| @Operation(summary = "전체 스터디 조회", description = "모든 스터디를 조회합니다. 코어멤버만 접근 가능합니다.") | |
| @PreAuthorize("hasRole('CORE')") | |
| @GetMapping | |
| public ResponseEntity<List<StudyManagerResponse>> getStudies() { | |
| var response = adminStudyServiceV2.getAllStudies(); | |
| return ResponseEntity.ok(response); | |
| } |
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
This was referenced Feb 9, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit