-
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
fix: 제출 이력이 없는 과제도 포함하도록 수정 #674
fix: 제출 이력이 없는 과제도 포함하도록 수정 #674
Conversation
Walkthrough변경 사항은 Changes
Assessment against linked issues
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
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.
lgtm
List<AssignmentSubmittableDto> submittableAssignments = studyDetails.stream() | ||
.map(studyDetail -> AssignmentSubmittableDto.of( | ||
studyDetail, | ||
assignmentHistories.stream() |
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.
stream 안의 stream은 가독성이 좋지 않아서, extract하면 좋을듯 합니다
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.
좋아요.
다시보니 슬기님이 이미 만들어두신 private 메서드가 있었네요
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
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyDetailService.java (3 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dto/response/AssignmentSubmittableDto.java (2 hunks)
Additional context used
Learnings (1)
src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyDetailService.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-07-07T15:32:34.451Z Learning: Consider using Stream API for creating lists in a more concise and potentially performant manner compared to traditional for-loops.
Additional comments not posted (6)
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/AssignmentSubmittableDto.java (3)
Line range hint
23-43
: 메서드 시그니처 변경 및 로직 추가 확인
of
메서드의 시그니처가 변경되어StudyDetail
을 추가로 받도록 수정되었습니다.assignmentHistory
가 null인 경우를 처리하는 로직이 추가되었습니다. 이러한 변경은 PR의 목표와 일치하며, 과제 제출 이력이 없는 경우를 적절히 처리합니다.
48-50
: 변경 사항 없음
cancelledAssignment
메서드에는 변경 사항이 없습니다.
51-62
: 새로운 메서드 추가 확인
notSubmittedAssignment
메서드는 과제 제출 이력이 없는 경우를 처리하기 위해 새롭게 추가되었습니다. 메서드 구조가 명확하며 PR의 목표와 잘 맞습니다.src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyDetailService.java (3)
42-49
: 로직 수정 및 Stream API 사용
getSubmittableAssignments
메서드는studyRepository
의존성을 제거하고studyDetailRepository
를 사용하여 로직을 수정했습니다. 이러한 변경은 PR의 목표와 일치하며, Stream API를 사용한 필터링이 적절하게 이루어졌습니다. 이전 학습에 따라 Stream API 사용을 권장합니다.
Line range hint
54-65
: 변경 사항 없음
getStudySessions
메서드에는 변경 사항이 없습니다.
Line range hint
67-72
: 변경 사항 없음
getSubmittedAssignment
메서드에는 변경 사항이 없습니다.
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyDetailService.java (3 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/vo/Assignment.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyDetailService.java
Additional comments not posted (1)
src/main/java/com/gdschongik/gdsc/domain/study/domain/vo/Assignment.java (1)
80-82
:isOpen
메서드가 잘 구현되었습니다.이 메서드는 과제 상태가
OPEN
인지 확인하는 간단한 논리를 올바르게 구현하고 있습니다.
🌱 관련 이슈
📌 작업 내용 및 특이사항
AssignmentHistory가 수강신청을 하는 시점에 생성될 것이라는 생각에 선택한 방식이었으나, 최초 제출 시 생성되도록 구현되었기 때문에 로직 변경이 필요합니다.
📝 참고사항
📚 기타
Summary by CodeRabbit
새로운 기능
AssignmentSubmittableDto
의 기능을 확장했습니다.OPEN
인지 확인할 수 있는isOpen()
메서드를 추가했습니다.버그 수정