-
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: 스터디 과제 상세 조회 V2 API 구현 #889
base: develop
Are you sure you want to change the base?
Conversation
Walkthrough이 PR은 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as MentorStudyControllerV2
participant Service as CommonStudyServiceV2
participant Repository as StudySessionV2Repository
Client->>Controller: GET /{studyId}/sessions/{studySessionId}/assignment
Controller->>Service: getAssignment(studySessionId)
Service->>Repository: findById(studySessionId)
Repository-->>Service: StudySessionV2 객체 반환
Service-->>Controller: AssignmentResponse 생성
Controller-->>Client: ResponseEntity<AssignmentResponse> 반환
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
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: 4
🧹 Nitpick comments (2)
src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/response/AssignmentResponse.java (1)
8-14
: Swagger 문서를 더 자세히 작성해주세요API 문서의 품질을 높이기 위해 각 필드에 대한 예시값과 상세한 설명이 필요합니다.
다음과 같이 수정하는 것을 제안합니다:
public record AssignmentResponse( @Schema(description = "스터디 세션 ID") Long studySessionId, - @Schema(description = "스터디 이름") String studyTitle, + @Schema(description = "스터디 이름", example = "Spring Boot 스터디") String studyTitle, - @Schema(description = "과제 제목") String title, + @Schema(description = "과제 제목", example = "JPA 실습하기") String title, - @Schema(description = "회차") Integer position, + @Schema(description = "회차 (1부터 시작)", example = "1", minimum = "1") Integer position, - @Schema(description = "과제 명세 링크") String descriptionLink, + @Schema(description = "과제 명세 링크", example = "https://github.com/...") String descriptionLink, - @Schema(description = "과제 시작일") LocalDateTime assignmentStartDate, + @Schema(description = "과제 시작일", example = "2024-02-20T00:00:00") LocalDateTime assignmentStartDate, - @Schema(description = "과제 종료일") LocalDateTime assignmentEndDate + @Schema(description = "과제 종료일", example = "2024-02-27T23:59:59") LocalDateTime assignmentEndDatesrc/main/java/com/gdschongik/gdsc/domain/studyv2/api/MentorStudyControllerV2.java (1)
33-34
: Swagger 문서를 보완해주세요API 엔드포인트에 대한 더 자세한 설명이 필요합니다.
다음과 같이 수정하는 것을 제안합니다:
- @Operation(summary = "스터디 과제 상세 조회", description = "멘토가 자신의 스터디 과제를 상세 조회합니다.") + @Operation( + summary = "스터디 과제 상세 조회", + description = "멘토가 자신의 스터디 과제를 상세 조회합니다. 과제의 제목, 기간, 설명 링크 등의 정보를 반환합니다.", + responses = { + @ApiResponse(responseCode = "200", description = "과제 조회 성공"), + @ApiResponse(responseCode = "404", description = "과제를 찾을 수 없음"), + @ApiResponse(responseCode = "403", description = "접근 권한 없음") + } + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/gdschongik/gdsc/domain/studyv2/api/MentorStudyControllerV2.java
(2 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/application/CommonStudyServiceV2.java
(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudySessionV2Repository.java
(1 hunks)src/main/java/com/gdschongik/gdsc/domain/studyv2/dto/response/AssignmentResponse.java
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/gdschongik/gdsc/domain/studyv2/dao/StudySessionV2Repository.java
StudySessionV2 studySessionV2 = | ||
studySessionV2Repository.findById(studySessionId).orElseThrow(); |
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
예외 처리를 구체화해야 합니다
orElseThrow()
는 구체적인 예외 메시지를 제공하지 않습니다. 사용자 정의 예외를 사용하여 더 명확한 오류 메시지를 제공하는 것이 좋습니다.
다음과 같이 수정하는 것을 제안합니다:
- studySessionV2Repository.findById(studySessionId).orElseThrow();
+ studySessionV2Repository.findById(studySessionId)
+ .orElseThrow(() -> new StudySessionNotFoundException(
+ String.format("스터디 세션을 찾을 수 없습니다. ID: %d", studySessionId)));
📝 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.
StudySessionV2 studySessionV2 = | |
studySessionV2Repository.findById(studySessionId).orElseThrow(); | |
StudySessionV2 studySessionV2 = | |
studySessionV2Repository.findById(studySessionId) | |
.orElseThrow(() -> new StudySessionNotFoundException( | |
String.format("스터디 세션을 찾을 수 없습니다. ID: %d", studySessionId))); |
public AssignmentResponse getAssignment(Long studySessionId) { | ||
StudySessionV2 studySessionV2 = | ||
studySessionV2Repository.findById(studySessionId).orElseThrow(); | ||
return AssignmentResponse.from(studySessionV2); | ||
} |
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
입력값 검증이 필요합니다
studySessionId
에 대한 유효성 검사가 누락되었습니다. null 체크와 음수값 체크가 필요합니다.
다음과 같이 수정하는 것을 제안합니다:
@Transactional(readOnly = true)
public AssignmentResponse getAssignment(Long studySessionId) {
+ if (studySessionId == null) {
+ throw new IllegalArgumentException("스터디 세션 ID는 null일 수 없습니다.");
+ }
+ if (studySessionId <= 0) {
+ throw new IllegalArgumentException("스터디 세션 ID는 양수여야 합니다.");
+ }
StudySessionV2 studySessionV2 =
studySessionV2Repository.findById(studySessionId).orElseThrow();
return AssignmentResponse.from(studySessionV2);
}
📝 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 AssignmentResponse getAssignment(Long studySessionId) { | |
StudySessionV2 studySessionV2 = | |
studySessionV2Repository.findById(studySessionId).orElseThrow(); | |
return AssignmentResponse.from(studySessionV2); | |
} | |
@Transactional(readOnly = true) | |
public AssignmentResponse getAssignment(Long studySessionId) { | |
if (studySessionId == null) { | |
throw new IllegalArgumentException("스터디 세션 ID는 null일 수 없습니다."); | |
} | |
if (studySessionId <= 0) { | |
throw new IllegalArgumentException("스터디 세션 ID는 양수여야 합니다."); | |
} | |
StudySessionV2 studySessionV2 = | |
studySessionV2Repository.findById(studySessionId).orElseThrow(); | |
return AssignmentResponse.from(studySessionV2); | |
} |
public static AssignmentResponse from(StudySessionV2 studySession) { | ||
return new AssignmentResponse( | ||
studySession.getId(), | ||
studySession.getStudy().getTitle(), | ||
studySession.getTitle(), | ||
studySession.getPosition(), | ||
studySession.getAssignmentDescriptionLink(), | ||
studySession.getAssignmentPeriod().getStartDate(), | ||
studySession.getAssignmentPeriod().getEndDate()); | ||
} |
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
null 체크가 필요합니다
from
메소드에서 StudySessionV2
의 필드들에 대한 null 체크가 없어 NPE가 발생할 수 있습니다.
다음과 같이 수정하는 것을 제안합니다:
public static AssignmentResponse from(StudySessionV2 studySession) {
+ if (studySession == null) {
+ throw new IllegalArgumentException("스터디 세션은 null일 수 없습니다.");
+ }
+ if (studySession.getStudy() == null) {
+ throw new IllegalStateException("스터디 정보가 없습니다.");
+ }
+ if (studySession.getAssignmentPeriod() == null) {
+ throw new IllegalStateException("과제 기간 정보가 없습니다.");
+ }
return new AssignmentResponse(
studySession.getId(),
studySession.getStudy().getTitle(),
studySession.getTitle(),
studySession.getPosition(),
studySession.getAssignmentDescriptionLink(),
studySession.getAssignmentPeriod().getStartDate(),
studySession.getAssignmentPeriod().getEndDate());
}
📝 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 static AssignmentResponse from(StudySessionV2 studySession) { | |
return new AssignmentResponse( | |
studySession.getId(), | |
studySession.getStudy().getTitle(), | |
studySession.getTitle(), | |
studySession.getPosition(), | |
studySession.getAssignmentDescriptionLink(), | |
studySession.getAssignmentPeriod().getStartDate(), | |
studySession.getAssignmentPeriod().getEndDate()); | |
} | |
public static AssignmentResponse from(StudySessionV2 studySession) { | |
if (studySession == null) { | |
throw new IllegalArgumentException("스터디 세션은 null일 수 없습니다."); | |
} | |
if (studySession.getStudy() == null) { | |
throw new IllegalStateException("스터디 정보가 없습니다."); | |
} | |
if (studySession.getAssignmentPeriod() == null) { | |
throw new IllegalStateException("과제 기간 정보가 없습니다."); | |
} | |
return new AssignmentResponse( | |
studySession.getId(), | |
studySession.getStudy().getTitle(), | |
studySession.getTitle(), | |
studySession.getPosition(), | |
studySession.getAssignmentDescriptionLink(), | |
studySession.getAssignmentPeriod().getStartDate(), | |
studySession.getAssignmentPeriod().getEndDate()); | |
} |
public ResponseEntity<AssignmentResponse> getStudyAssignment( | ||
@PathVariable Long studyId, @PathVariable Long studySessionId) { | ||
var response = commonStudyServiceV2.getAssignment(studySessionId); | ||
return ResponseEntity.ok(response); | ||
} |
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.
권한 검증과 studyId 활용이 필요합니다
- 멘토 권한 검증이 누락되었습니다.
studyId
파라미터가 실제로 사용되지 않고 있습니다.
다음과 같이 수정하는 것을 제안합니다:
public ResponseEntity<AssignmentResponse> getStudyAssignment(
@PathVariable Long studyId, @PathVariable Long studySessionId) {
+ // 멘토 권한 검증
+ mentorStudyServiceV2.validateMentorAuthority(studyId);
+
+ // 스터디 세션이 해당 스터디에 속하는지 검증
+ commonStudyServiceV2.validateStudySessionBelongsToStudy(studyId, studySessionId);
+
var response = commonStudyServiceV2.getAssignment(studySessionId);
return ResponseEntity.ok(response);
}
📝 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 ResponseEntity<AssignmentResponse> getStudyAssignment( | |
@PathVariable Long studyId, @PathVariable Long studySessionId) { | |
var response = commonStudyServiceV2.getAssignment(studySessionId); | |
return ResponseEntity.ok(response); | |
} | |
public ResponseEntity<AssignmentResponse> getStudyAssignment( | |
@PathVariable Long studyId, @PathVariable Long studySessionId) { | |
// 멘토 권한 검증 | |
mentorStudyServiceV2.validateMentorAuthority(studyId); | |
// 스터디 세션이 해당 스터디에 속하는지 검증 | |
commonStudyServiceV2.validateStudySessionBelongsToStudy(studyId, studySessionId); | |
var response = commonStudyServiceV2.getAssignment(studySessionId); | |
return ResponseEntity.ok(response); | |
} |
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.
🧩 Analysis chain
@kckc0608 스터디와 세션 간의 계층 구조를 명확하게 표현하기 위해 현재의 URL 구조를 유지하는 것이 좋을 것 같습니다. 다음과 같은 이유들이 있습니다:
- REST API 설계 관례상 리소스 간의 계층 관계를 URL 경로에 반영하는 것이 일반적입니다
- API의 의도가 더 명확해지고 자체 문서화 효과가 있습니다
- 향후 스터디 레벨에서의 권한 검증이 필요할 때 자연스럽게 확장할 수 있습니다
- 컨트롤러 내의 다른 엔드포인트들과의 일관성을 유지할 수 있습니다
🏁 Script executed:
#!/bin/bash
# Search for StudySessionV2 entity definition
ast-grep --pattern 'class StudySessionV2 {
$$$
}'
Length of output: 53
🏁 Script executed:
#!/bin/bash
# First find files containing StudySessionV2
rg "StudySessionV2" -l
# Then look for the actual class definition with context
rg "class StudySessionV2" -A 10
Length of output: 1873
🏁 Script executed:
#!/bin/bash
# Get the full content of StudySessionV2 to see study relationship
cat src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/StudySessionV2.java
# Also check StudyV2 entity for the other side of the relationship
cat src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/StudyV2.java
Length of output: 9666
@kckc0608 StudySessionV2가 StudyV2의 애그리거트 루트에 종속된 엔티티임을 확인했습니다. 도메인 모델에서도 다음과 같이 강한 연관 관계가 있습니다:
@ManyToOne
관계로 StudySessionV2가 StudyV2에 종속cascade = CascadeType.ALL, orphanRemoval = true
로 생명주기가 Study에 완전히 종속- 생성 시에도 반드시 Study 엔티티가 필요
따라서 현재의 계층적 URL 구조 /{studyId}/sessions/{studySessionId}/assignment
가 도메인 모델을 정확히 반영하고 있어 유지하는 것이 좋겠습니다.
✏️ Learnings added
Learnt from: kckc0608
PR: GDSC-Hongik/gdsc-server#889
File: src/main/java/com/gdschongik/gdsc/domain/studyv2/api/MentorStudyControllerV2.java:35-39
Timestamp: 2025-02-11T12:26:14.620Z
Learning: In the GDSC server project, REST API endpoints should maintain hierarchical resource relationships in their URL structure (e.g., `/studies/{studyId}/sessions/{sessionId}`) even if the parent resource ID is not currently used in the implementation. This ensures better API documentation, consistent design, and future extensibility.
Learnt from: kckc0608
PR: GDSC-Hongik/gdsc-server#889
File: src/main/java/com/gdschongik/gdsc/domain/studyv2/api/MentorStudyControllerV2.java:35-39
Timestamp: 2025-02-11T12:26:14.620Z
Learning: In the GDSC server project's V2 domain model, StudySessionV2 is a child entity in the StudyV2 aggregate root, with a strict parent-child relationship enforced through JPA annotations (@ManyToOne, cascade = CascadeType.ALL, orphanRemoval = true). This relationship should be reflected in the REST API URL structure.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
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
🧹 Nitpick comments (1)
src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1)
112-114
: LGTM! 스터디 세션 관련 에러 코드가 잘 추가되었습니다.새로운 에러 코드가 기존 코드의 패턴과 일관성을 잘 유지하고 있으며, HTTP 상태 코드도 적절하게 선택되었습니다. 다만, 다른 섹션들과의 일관성을 위해 다음과 같이 한 줄의 공백을 추가하는 것을 제안합니다:
// StudySession STUDY_SESSION_NOT_FOUND(HttpStatus.NOT_FOUND, "존재하지 않는 스터디 세션입니다."), +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/gdschongik/gdsc/domain/studyv2/application/CommonStudyServiceV2.java
(1 hunks)src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/gdschongik/gdsc/domain/studyv2/application/CommonStudyServiceV2.java
🌱 관련 이슈
📌 작업 내용 및 특이사항
CommonStudyServiceV2
를 만들어서 사용했습니다.📝 참고사항
📚 기타
Summary by CodeRabbit