feat: 스터디 개설 V2 API 구현 #863
Conversation
Walkthrough이 풀 리퀘스트는 스터디 V2 API 개설을 위한 새로운 컨트롤러, 서비스, 저장소, DTO 및 관련 구성 요소를 도입합니다. 주요 기능은 멘토가 새로운 스터디를 생성할 수 있는 API 엔드포인트를 제공하는 것으로, 스터디 세션 생성, 멘토 역할 할당, 출석 번호 생성 등의 로직을 포함합니다. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 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: 3
🧹 Nitpick comments (6)
src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/RandomAttendanceNumberGenerator.java (1)
Line range hint
17-23: 예외 처리 방식 개선을 제안드립니다.
@SneakyThrows를 사용하면 예외가 숨겨져 디버깅이 어려울 수 있습니다. 명시적인 예외 처리를 통해 코드의 안정성을 높일 것을 제안드립니다.다음과 같이 수정하는 것을 고려해보세요:
- @SneakyThrows public String generate() { + try { return String.valueOf(SecureRandom.getInstanceStrong() .ints(MIN_ORIGIN, MAX_BOUND) .findFirst() .orElseThrow()); + } catch (Exception e) { + throw new RuntimeException("출석 번호 생성 중 오류가 발생했습니다.", e); + } }src/main/java/com/gdschongik/gdsc/domain/studyv2/application/AdminStudyServiceV2.java (2)
33-33: mentor.assignToMentor() 호출 시점과 로직을 점검하세요
이미 멘토로 지정된 사용자의 경우 예외 처리가 필요할 수 있습니다. 중복 역할 부여로 인한 예기치 않은 상태를 방지하세요.
54-54: 로그에 멘토 정보도 함께 남기는 것을 고려해보세요
스터디 ID뿐 아니라 멘토 ID나 멘토 이름 등을 함께 로깅하면 추적과 분석이 더욱 용이합니다.src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyV2CustomRepository.java (1)
7-7: 메서드 명이 의도를 명확히 드러내는지 검토해보세요
“findFetchById”라는 이름은 fetch join을 한다는 의도를 어느 정도 나타내지만, 실제 사용하는 쿼리 전략에 따라 더 구체적인 명명(예: findWithSessionsById)이 필요할 수 있습니다.src/test/java/com/gdschongik/gdsc/domain/studyv2/application/AdminStudyServiceV2Test.java (2)
34-47: 테스트 데이터 생성 중복을 제거하고 엣지 케이스 테스트를 추가해야 합니다.테스트 코드에서 다음과 같은 개선이 필요합니다:
- StudyCreateRequest 생성 코드가 중복됨
- 경계값 테스트가 누락됨 (최소/최대 라운드 수, 유효하지 않은 기간 등)
테스트 데이터 생성을 위한 팩토리 메서드를 추가하고 경계값 테스트를 추가하는 것을 제안합니다:
private StudyCreateRequest createDefaultStudyRequest(Long mentorId, Integer totalRound) { return new StudyCreateRequest( mentorId, StudyType.OFFLINE, STUDY_TITLE, STUDY_DESCRIPTION, STUDY_DESCRIPTION_NOTION_LINK, STUDY_SEMESTER, totalRound, DAY_OF_WEEK, STUDY_START_TIME, STUDY_END_TIME, STUDY_APPLICATION_PERIOD, STUDY_DISCORD_CHANNEL_ID, STUDY_DISCORD_ROLE_ID); } @Test void 최소_라운드_수로_스터디를_생성할_수_있다() { // given createRegularMember(); var request = createDefaultStudyRequest(1L, 1); // when adminStudyService.createStudy(request); // then StudyV2 study = studyV2Repository.findFetchById(1L).orElseThrow(); assertThat(study.getStudySessions()).hasSize(1); }Also applies to: 64-77, 93-106
82-87: null 체크 대신 더 구체적인 검증이 필요합니다.출석 번호가 null이 아닌지만 확인할 것이 아니라, 출석 번호의 유효성도 함께 검증해야 합니다.
다음과 같이 수정하는 것을 제안합니다:
- assertThat(study.getStudySessions()) - .extracting(StudySessionV2::getLessonAttendanceNumber) - .doesNotContainNull(); + assertThat(study.getStudySessions()) + .extracting(StudySessionV2::getLessonAttendanceNumber) + .allMatch(number -> number >= 1000 && number <= 9999);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/com/gdschongik/gdsc/domain/studyv2/api/AdminStudyControllerV2.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/application/AdminStudyServiceV2.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyV2CustomRepository.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyV2Repository.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyV2RepositoryImpl.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/RandomAttendanceNumberGenerator.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/request/StudyCreateRequest.java(1 hunks)src/test/java/com/gdschongik/gdsc/domain/studyv2/application/AdminStudyServiceV2Test.java(1 hunks)
🔇 Additional comments (3)
src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/RandomAttendanceNumberGenerator.java (1)
5-5: Spring 컴포넌트 등록이 적절하게 구현되었습니다.
@Component어노테이션을 통한 Spring Bean 등록이 잘 되어있습니다. 이를 통해 다른 서비스에서 의존성 주입이 가능해졌습니다.Also applies to: 10-10
src/main/java/com/gdschongik/gdsc/domain/studyv2/application/AdminStudyServiceV2.java (1)
30-31: 멘토 존재 여부 이외에도 추가 검증이 필요할 수 있습니다
Member 엔티티가 이미 멘토 역할을 가지고 있는지 여부 등을 사전에 확인하여, 불필요한 로직 호출을 줄이고 데이터 무결성을 강화할 수 있는지 고려해보세요.src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudyV2Repository.java (1)
6-6: JpaRepository 및 CustomRepository를 함께 사용하는 설계가 좋아 보입니다
현재 설계로 기본 CRUD와 커스텀 로직을 구분하여 유지보수를 쉽게 할 수 있을 것으로 보입니다.
| public Optional<StudyV2> findFetchById(Long id) { | ||
| return Optional.ofNullable(queryFactory | ||
| .selectFrom(studyV2) | ||
| .join(studyV2.studySessions) | ||
| .fetchJoin() | ||
| .fetchOne()); | ||
| } |
There was a problem hiding this comment.
where 절이 누락되어 잘못된 결과를 반환할 수 있습니다.
findFetchById 메서드에서 id를 기준으로 필터링하는 where 절이 누락되었습니다. 현재 구현은 데이터베이스의 첫 번째 레코드를 반환할 수 있습니다.
다음과 같이 수정하는 것을 제안합니다:
public Optional<StudyV2> findFetchById(Long id) {
return Optional.ofNullable(queryFactory
.selectFrom(studyV2)
.join(studyV2.studySessions)
.fetchJoin()
+ .where(studyV2.id.eq(id))
.fetchOne());
}📝 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.
| public Optional<StudyV2> findFetchById(Long id) { | |
| return Optional.ofNullable(queryFactory | |
| .selectFrom(studyV2) | |
| .join(studyV2.studySessions) | |
| .fetchJoin() | |
| .fetchOne()); | |
| } | |
| public Optional<StudyV2> findFetchById(Long id) { | |
| return Optional.ofNullable(queryFactory | |
| .selectFrom(studyV2) | |
| .join(studyV2.studySessions) | |
| .fetchJoin() | |
| .where(studyV2.id.eq(id)) | |
| .fetchOne()); | |
| } |
| DayOfWeek dayOfWeek, | ||
| LocalTime startTime, | ||
| LocalTime endTime, | ||
| @NotNull Period applicationPeriod, | ||
| String discordChannelId, | ||
| String discordRoleId) {} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
시간 관련 필드와 Discord ID에 대한 유효성 검증이 필요합니다.
현재 구현에서 몇 가지 중요한 유효성 검증이 누락되어 있습니다:
- 시작 시간이 종료 시간보다 이전인지 검증
- Discord 채널 ID와 역할 ID의 형식 검증
다음과 같은 커스텀 유효성 검증 어노테이션 추가를 제안합니다:
@interface ValidTimeRange {
String message() default "시작 시간은 종료 시간보다 이전이어야 합니다";
Class<?>[] groups() default {};
Class<? extends Payload>[] payload() default {};
}
@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
@Constraint(validatedBy = TimeRangeValidator.class)
public @interface ValidTimeRange {
String message() default "시작 시간은 종료 시간보다 이전이어야 합니다";
Class<?>[] groups() default {};
Class<? extends Payload>[] payload() default {};
}그리고 DTO에 다음과 같이 적용:
+ @ValidTimeRange
public record StudyCreateRequest(
@NotNull @Positive Long mentorId,
@NotNull StudyType type,
@NotNull String title,
String description,
String descriptionNotionLink,
@NotNull Semester semester,
@NotNull @Positive Integer totalRound,
DayOfWeek dayOfWeek,
LocalTime startTime,
LocalTime endTime,
@NotNull Period applicationPeriod,
- String discordChannelId,
- String discordRoleId) {}
+ @Pattern(regexp = "^\\d{17,19}$", message = "올바른 Discord 채널 ID 형식이 아닙니다")
+ String discordChannelId,
+ @Pattern(regexp = "^\\d{17,19}$", message = "올바른 Discord 역할 ID 형식이 아닙니다")
+ String discordRoleId) {}| @Operation(summary = "스터디 개설", description = "스터디를 개설합니다. 빈 스터디회차를 함께 생성합니다.") | ||
| @PostMapping | ||
| public ResponseEntity<Void> createStudy(@Valid @RequestBody StudyCreateRequest request) { | ||
| adminStudyServiceV2.createStudy(request); | ||
| return ResponseEntity.ok().build(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
리소스 생성에 대한 응답을 개선해야 합니다.
현재 구현은 다음과 같은 REST API 모범 사례를 따르지 않고 있습니다:
- 생성된 리소스의 ID를 응답하지 않음
- 리소스 생성 시 적절한 HTTP 상태 코드(201 Created)를 사용하지 않음
다음과 같이 수정하는 것을 제안합니다:
- public ResponseEntity<Void> createStudy(@Valid @RequestBody StudyCreateRequest request) {
- adminStudyServiceV2.createStudy(request);
- return ResponseEntity.ok().build();
+ public ResponseEntity<StudyCreateResponse> createStudy(@Valid @RequestBody StudyCreateRequest request) {
+ StudyV2 study = adminStudyServiceV2.createStudy(request);
+ return ResponseEntity
+ .created(URI.create("/admin/studies/v2/" + study.getId()))
+ .body(new StudyCreateResponse(study.getId()));
}Committable suggestion skipped: line range outside the PR's diff.
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit
새로운 기능
개선 사항
테스트