-
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: 스터디 수강신청 취소 API 추가 #536
Conversation
Caution Review failedThe pull request is closed. Walkthrough이 변경 사항은 사용자가 스터디 신청을 취소할 수 있는 새로운 API 엔드포인트를 도입하여, 스터디 신청 기간 동안만 취소가 가능하도록 하는 기능을 구현합니다. 이를 통해 사용자 경험을 향상시키고, 스터디 신청과 관련된 유효성 검사를 강화하였습니다. 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
|
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 (6)
- src/main/java/com/gdschongik/gdsc/domain/study/api/StudyController.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/application/StudyService.java (4 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyHistoryRepository.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidator.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1 hunks)
- src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidatorTest.java (2 hunks)
Additional context used
Learnings (1)
src/main/java/com/gdschongik/gdsc/domain/study/application/StudyService.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 (10)
src/main/java/com/gdschongik/gdsc/domain/study/dao/StudyHistoryRepository.java (2)
12-12
: 이 메서드는 적절합니다!
findAllByMentee
메서드는 특정Member
와 관련된StudyHistory
객체 목록을 반환합니다. 변경 사항이 없으므로 이 메서드는 적절합니다.
14-14
: 이 메서드는 적절합니다!
findByMenteeAndStudy
메서드는 특정Member
와Study
와 관련된StudyHistory
객체를Optional
로 반환합니다. 이 메서드는 잘 정의되어 있으며,Optional
을 사용하여 해당 매개변수에 대한StudyHistory
가 존재하지 않을 경우를 처리합니다.src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidator.java (2)
Line range hint
10-32
:
이 메서드는 적절합니다!
validateApplyStudy
메서드는 여러 조건에 따라 스터디 신청이 가능한지 검증합니다. 이 메서드는 잘 정의되어 있으며, 여러 유효성 검사를 처리합니다. 변경 사항이 없으므로 이 메서드는 적절합니다.
34-39
: 이 메서드는 적절합니다!
validateCancelStudyApply
메서드는 스터디의 적용 가능 상태에 따라 스터디 신청을 취소할 수 있는지 검증합니다. 이 메서드는 잘 정의되어 있으며, 스터디 신청 취소가 불가능한 경우CustomException
을 던집니다.src/main/java/com/gdschongik/gdsc/domain/study/api/StudyController.java (1)
39-44
: 이 메서드는 적절합니다!
cancelStudyApply
메서드는 사용자가 스터디 신청을 취소할 수 있도록 합니다. 이 메서드는 잘 정의되어 있으며, API 문서를 위한 적절한 주석을 사용합니다. 취소 로직을 수행하기 위해studyService.cancelStudyApply
메서드를 호출합니다.src/main/java/com/gdschongik/gdsc/domain/study/application/StudyService.java (2)
40-40
: 코드 변경 승인됨: 예외 처리 단순화예외 처리를 단순화하여 코드 가독성과 일관성이 향상되었습니다.
54-66
: 코드 변경 승인됨: 새로운 메서드 추가새로운 메서드
cancelStudyApply
가 올바르게 구현되었습니다. 스터디를 ID로 조회하고, 취소를 검증한 후, 해당 스터디 기록을 삭제합니다.src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidatorTest.java (1)
81-98
: 코드 변경 승인됨: 새로운 테스트 클래스 및 메서드 추가새로운 테스트 클래스
스터디_수강신청_취소시
와 메서드해당_스터디의_신청기간이_아니라면_실패한다
가 올바르게 구현되었습니다. 신청 기간 외에 취소 시도 시 예외가 발생하는지 검증합니다.src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (2)
109-109
: 코드 변경 승인됨: 새로운 오류 코드 추가새로운 오류 코드
STUDY_APPLY_NOT_CANCELABLE
가 올바르게 정의되었습니다. 지정된 신청 기간 외에 스터디 신청을 취소할 수 없음을 나타냅니다.
112-112
: 코드 변경 승인됨: 새로운 오류 코드 추가새로운 오류 코드
STUDY_HISTORY_NOT_FOUND
가 올바르게 정의되었습니다. 요청된 스터디 수강 기록이 존재하지 않음을 나타냅니다.
|
||
public void validateCancelStudyApply(Study study) { | ||
// 스터디 수강신청 기간이 아닌 경우 | ||
if (!study.isApplicable()) { |
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.
이거 취소 가능 기간은 본신청기간 + 1주일(정정기간) 아닌가요?
수강신청 기간이랑은 다른걸로 알고있어서요!
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.
정정기간에는 신청도 할 수 있어야 해서 본 신청기간에 포함하던지 Study 자체에 정정기간 나타내는 Period를 하나 더 넣어야 할 것 같습니다.
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.
음 그건 그럼 이후에 추가로 이슈파서 작업하면 되겠네요! TODO만 달아주세요!
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.
따로 정정기간을 나타내는 개념을 넣는 것보다는
신청기간 = 수강신청, 취소 자유롭게 가능한 기간 이렇게 풀어가는게 좋을 것 같아요
어차피 OT는 따로 디스코드로 공지 나갈 거고, 내부 정책만 세팅해두면 되니까요
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 (1)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidator.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistoryValidator.java
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
|
||
public void validateCancelStudyApply(Study study) { | ||
// 스터디 수강신청 기간이 아닌 경우 | ||
if (!study.isApplicable()) { |
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.
따로 정정기간을 나타내는 개념을 넣는 것보다는
신청기간 = 수강신청, 취소 자유롭게 가능한 기간 이렇게 풀어가는게 좋을 것 같아요
어차피 OT는 따로 디스코드로 공지 나갈 거고, 내부 정책만 세팅해두면 되니까요
@@ -106,8 +106,10 @@ public enum ErrorCode { | |||
ASSIGNMENT_STUDY_CAN_NOT_INPUT_STUDY_TIME(HttpStatus.CONFLICT, "과제 스터디는 스터디 시간을 입력할 수 없습니다."), | |||
STUDY_NOT_FOUND(HttpStatus.NOT_FOUND, "존재하지 않는 스터디입니다."), | |||
STUDY_NOT_APPLICABLE(HttpStatus.CONFLICT, "스터디 신청기간이 아닙니다."), | |||
STUDY_APPLY_NOT_CANCELABLE(HttpStatus.CONFLICT, "스터디 신청기간이 아니라면 취소할 수 없습니다."), |
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.
STUDY_APPLY_NOT_CANCELABLE(HttpStatus.CONFLICT, "스터디 신청기간이 아니라면 취소할 수 없습니다."), | |
STUDY_NOT_CANCELABLE_APPLICATION_PERIOD(HttpStatus.CONFLICT, "스터디 신청기간이 아니라면 취소할 수 없습니다."), |
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.
가능하다면 도메인_허용할수없는행위_사유
구조로 작성하는게 좋을듯 합니다
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit
신규 기능
버그 수정
문서화