-
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 #642
Conversation
## Walkthrough
이번 변경사항은 스터디 세부 정보를 업데이트할 수 있는 새로운 API 엔드포인트와 관련된 여러 클래스를 수정하고 추가합니다. `MentorStudyService`는 세부 정보 업데이트 기능을 구현하며, 검증 및 세부 정보 업데이트를 위한 DTO가 포함되었습니다. 이로 인해 스터디 관리 시스템의 기능이 크게 확장되었습니다.
## Changes
| 파일 경로 | 변경 요약 |
|-----------|-----------|
| `src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java`<br>`src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java` | 스터디 세부 정보 업데이트 메소드(`updateStudy`) 추가. |
| `src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyDetailValidator.java` | 업데이트 검증 로직을 위한 새로운 메소드(`validateUpdateStudyDetail`) 추가. |
| `src/main/java/com/gdschongik/gdsc/domain/study/dto/request/StudyUpdateRequest.java` | 스터디 업데이트 요청 DTO 추가. |
| `src/test/java/com/gdschongik/gdsc/domain/study/application/MentorStudyServiceTest.java`<br>`src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyDetailValidatorTest.java` | 스터디 세부 정보 업데이트에 대한 테스트 케이스 추가. |
| `src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java` | 새로운 에러 코드 추가(`STUDY_DETAIL_ID_INVALID`, `STUDY_DETAIL_SESSION_SIZE_MISMATCH`). |
## Assessment against linked issues
| Objective | Addressed | Explanation |
|-----------|-----------|-------------|
| 스터디 세부 정보 저장 기능 구현 (#636) | ✅ | |
| 스터디 노션 링크 및 소개 추가 (#636) | ✅ | |
| 세부 정보 유효성 검증 기능 강화 (#636) | ✅ | |
| 테스트 코드 확장 및 보강 (#636) | ✅ | |
> 🐰 새로 생긴 기능에 기뻐하며,<br>
> 스터디 세부 정보가 한층 더 빛나네!<br>
> 쿼리와 검증이 만나는 곳,<br>
> 마음의 짐을 덜어주네!<br>
> 모든 변화가 반짝여, 이젠 더 완벽해!✨ 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: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (14)
- src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyDetailController.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyDetailService.java (3 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/Study.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyDetail.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyDetailValidator.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/vo/Session.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dto/request/StudyDetailUpdateRequest.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dto/request/StudySessionCreateRequest.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (2 hunks)
- src/test/java/com/gdschongik/gdsc/domain/study/application/MentorStudyDetailServiceTest.java (2 hunks)
- src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyDetailValidatorTest.java (3 hunks)
- src/test/java/com/gdschongik/gdsc/global/common/constant/StudyConstant.java (1 hunks)
- src/test/java/com/gdschongik/gdsc/helper/FixtureHelper.java (1 hunks)
- src/test/java/com/gdschongik/gdsc/helper/IntegrationTest.java (2 hunks)
Additional comments not posted (18)
src/main/java/com/gdschongik/gdsc/domain/study/dto/request/StudyDetailUpdateRequest.java (1)
1-9
: 구현이 적절합니다.Java Record를 사용하여 데이터 전송 객체를 간결하게 정의하였으며, Swagger 주석을 통해 API 문서화를 잘 처리하였습니다.
src/main/java/com/gdschongik/gdsc/domain/study/dto/request/StudySessionCreateRequest.java (1)
1-12
: 구현이 적절합니다.Java Record를 사용하여 스터디 세션 생성 요청을 간결하게 정의하였으며, Swagger 주석을 통해 API 문서화를 잘 처리하였습니다.
src/main/java/com/gdschongik/gdsc/domain/study/domain/vo/Session.java (1)
49-58
: 새로운 메서드 추가가 적절합니다.
generateSession
메서드는 세션 객체를 보다 유연하게 생성할 수 있도록 하며, 빌더 패턴을 효과적으로 사용하고 있습니다.src/test/java/com/gdschongik/gdsc/global/common/constant/StudyConstant.java (1)
29-30
: 새로운 상수 추가 확인 완료.
STUDY_NOTION_LINK
,STUDY_INTRODUCTION
,SESSION_TITLE
,SESSION_DESCRIPTION
상수들이 잘 추가되었습니다. 기존 상수들과 일관성을 유지하고 있습니다.Also applies to: 38-39
src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyDetailValidator.java (1)
62-78
: 새로운 검증 메서드 추가 확인 완료.
validateUpdateStudyDetail
메서드는 두 리스트의 크기와 ID 집합의 일치를 검증하여 데이터 무결성을 보장합니다. 로직이 명확하고 필요한 예외 처리가 잘 되어 있습니다.src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyDetailController.java (1)
31-37
: 새로운 엔드포인트 추가 확인 완료.
updateStudyDetail
메서드는 스터디 상세 정보를 업데이트하기 위한 엔드포인트로 잘 구현되었습니다. 서비스 레이어와의 연동 및 응답 처리가 적절합니다.src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyDetail.java (1)
104-107
: 세션 업데이트 메서드 추가 확인
updateSession
메서드는 세션 정보를 업데이트하는 기능을 추가합니다. 이 메서드는Session.generateSession
을 호출하여 새로운 세션 객체를 생성하고 할당합니다. 메서드의 구현은 명확하며, 기존 기능을 확장하는 데 적절합니다.src/test/java/com/gdschongik/gdsc/helper/FixtureHelper.java (1)
108-110
: 새로운 스터디 세부 정보 생성 메서드 추가 확인
createNewStudyDetail
메서드는 주차를 지정하여StudyDetail
객체를 생성하는 기능을 추가합니다. 이 메서드는StudyDetail.createStudyDetail
을 호출하며, 주어진 매개변수를 적절히 사용하고 있습니다. 테스트에서 유용하게 활용될 수 있는 메서드입니다.src/test/java/com/gdschongik/gdsc/domain/study/application/MentorStudyDetailServiceTest.java (1)
57-108
: 스터디 상세 정보 작성 테스트 클래스 추가 확인
스터디_상세정보_작성시
클래스와성공한다
메서드는 스터디 세부 정보 업데이트 기능을 테스트합니다. 테스트는 멘토 생성, 스터디 생성, 세션 생성 요청 준비, 그리고 업데이트 요청을 통해 시스템이 올바르게 동작하는지를 확인합니다. 테스트는 현실적인 시나리오를 반영하고 있으며, 검증이 잘 이루어지고 있습니다.src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1)
117-118
: 에러 코드 추가 승인새로운 에러 코드
STUDY_DETAIL_SESSION_ID_INVALID
와STUDY_DETAIL_SESSION_SIZE_MISMATCH
는 적절한 예외 상황을 처리할 수 있도록 잘 설계되었습니다. 에러 메시지가 명확하여 디버깅에 도움이 될 것입니다.src/test/java/com/gdschongik/gdsc/helper/IntegrationTest.java (3)
168-172
: 좋습니다!
createMentor
메서드는 멘토 역할을 할당하는 방식으로 잘 구현되었습니다.
232-247
: 좋습니다!
createNewStudy
메서드는 새로운 스터디를 생성하고 저장하는 방식으로 잘 구현되었습니다.
255-259
: 좋습니다!
createNewStudyDetail
메서드는 새로운 스터디 상세 정보를 생성하고 저장하는 방식으로 잘 구현되었습니다.src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyDetailValidatorTest.java (5)
24-29
: 좋습니다!
createStudy
메서드는FixtureHelper
를 사용하여 스터디를 생성하는 방식으로 잘 구현되었습니다.
31-33
: 좋습니다!
createMentor
메서드는FixtureHelper
를 사용하여 멘토를 생성하는 방식으로 잘 구현되었습니다.
35-37
: 좋습니다!
createNewStudyDetail
메서드는FixtureHelper
를 사용하여 스터디 상세 정보를 생성하는 방식으로 잘 구현되었습니다.
39-46
: 좋습니다!
createSessionCreateRequest
메서드는 주어진 개수에 따라 요청을 생성하는 방식으로 잘 구현되었습니다.
188-233
: 테스트 케이스가 잘 작성되었습니다!
스터디_상세정보_작성시
클래스는 스터디 상세 정보 업데이트 시 발생할 수 있는 중요한 검증 시나리오를 잘 다루고 있습니다.추가로, 요청된 상세 정보의 순서가 맞지 않을 경우에 대한 테스트도 고려해보세요.
public void update(String link, String intro) { | ||
notionLink = link; | ||
introduction = intro; | ||
} |
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.
필드 업데이트에 대한 유효성 검토 필요
update
메서드는 notionLink
와 introduction
을 직접 업데이트합니다. 입력 값에 대한 유효성 검사를 추가하여 잘못된 데이터가 저장되는 것을 방지하는 것이 좋습니다.
public void update(String link, String intro) {
if (link == null || link.isEmpty()) {
throw new IllegalArgumentException("Notion link cannot be null or empty");
}
if (intro == null || intro.isEmpty()) {
throw new IllegalArgumentException("Introduction cannot be null or empty");
}
notionLink = link;
introduction = intro;
}
public void updateStudyDetail(Long studyId, StudyDetailUpdateRequest request) { | ||
Member currentMember = memberUtil.getCurrentMember(); | ||
Study study = studyRepository.findById(studyId).orElseThrow(() -> new CustomException(STUDY_NOT_FOUND)); | ||
studyValidator.validateStudyMentor(currentMember, study); | ||
|
||
List<StudyDetail> studyDetails = studyDetailRepository.findAllByStudyId(studyId); | ||
studyDetailValidator.validateUpdateStudyDetail(studyDetails, request.studySessions()); | ||
|
||
// 스터디 저장 | ||
study.update(request.notionLink(), request.introduction()); | ||
studyRepository.save(study); | ||
log.info("[MentorStudyDetailService] 스터디 기본 정보 수정 완료: studyId={}", studyId); | ||
|
||
Map<Long, StudySessionCreateRequest> requestMap = request.studySessions().stream() | ||
.collect(Collectors.toMap(StudySessionCreateRequest::studyDetailId, Function.identity())); | ||
|
||
// StudyDetail을 업데이트하는 작업 | ||
List<StudyDetail> updatedStudyDetails = new ArrayList<>(); | ||
for (StudyDetail studyDetail : studyDetails) { | ||
Long id = studyDetail.getId(); | ||
StudySessionCreateRequest matchingSession = requestMap.get(id); | ||
|
||
studyDetail.updateSession( | ||
studyDetail.getStudy().getPeriod().getStartDate(), | ||
matchingSession.title(), | ||
matchingSession.description(), | ||
matchingSession.difficulty(), | ||
matchingSession.status()); | ||
|
||
updatedStudyDetails.add(studyDetail); | ||
} | ||
studyDetailRepository.saveAll(updatedStudyDetails); | ||
log.info("[MentorStudyDetailService] 스터디 상세정보 작성 완료: studyDetailId={}", studyDetails); | ||
} |
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.
추가 로깅 및 예외 처리 개선
updateStudyDetail
메서드는 여러 단계의 작업을 수행합니다. 각 주요 단계에 대한 추가 로깅을 통해 문제 발생 시 디버깅을 용이하게 할 수 있습니다. 또한, 예외 발생 시 적절한 메시지를 기록하는 것이 좋습니다.
@Transactional
public void updateStudyDetail(Long studyId, StudyDetailUpdateRequest request) {
log.debug("Updating study detail for studyId={}", studyId);
Member currentMember = memberUtil.getCurrentMember();
Study study = studyRepository.findById(studyId)
.orElseThrow(() -> {
log.error("Study not found for studyId={}", studyId);
return new CustomException(STUDY_NOT_FOUND);
});
studyValidator.validateStudyMentor(currentMember, study);
log.debug("Validated mentor for studyId={}", studyId);
List<StudyDetail> studyDetails = studyDetailRepository.findAllByStudyId(studyId);
studyDetailValidator.validateUpdateStudyDetail(studyDetails, request.studySessions());
log.debug("Validated study details for studyId={}", studyId);
study.update(request.notionLink(), request.introduction());
studyRepository.save(study);
log.info("[MentorStudyDetailService] 스터디 기본 정보 수정 완료: studyId={}", studyId);
Map<Long, StudySessionCreateRequest> requestMap = request.studySessions().stream()
.collect(Collectors.toMap(StudySessionCreateRequest::studyDetailId, Function.identity()));
List<StudyDetail> updatedStudyDetails = new ArrayList<>();
for (StudyDetail studyDetail : studyDetails) {
Long id = studyDetail.getId();
StudySessionCreateRequest matchingSession = requestMap.get(id);
if (matchingSession == null) {
log.warn("No matching session found for studyDetailId={}", id);
continue;
}
studyDetail.updateSession(
studyDetail.getStudy().getPeriod().getStartDate(),
matchingSession.title(),
matchingSession.description(),
matchingSession.difficulty(),
matchingSession.status());
updatedStudyDetails.add(studyDetail);
}
studyDetailRepository.saveAll(updatedStudyDetails);
log.info("[MentorStudyDetailService] 스터디 상세정보 작성 완료: studyDetailId={}", studyDetails);
}
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 (3)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyDetailValidator.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (2 hunks)
- src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyDetailValidatorTest.java (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyDetailValidator.java
- src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java
Additional comments not posted (5)
src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyDetailValidatorTest.java (5)
24-29
: 코드 변경 승인됨.
createStudy
메서드는FixtureHelper
를 사용하여Study
객체를 생성하는 올바른 방법을 사용하고 있습니다.
31-33
: 코드 변경 승인됨.
createMentor
메서드는FixtureHelper
를 사용하여 멘토 역할의Member
객체를 생성하는 올바른 방법을 사용하고 있습니다.
35-37
: 코드 변경 승인됨.
createNewStudyDetail
메서드는FixtureHelper
를 사용하여StudyDetail
객체를 생성하는 올바른 방법을 사용하고 있습니다.
39-46
: 코드 변경 승인됨.
createSessionCreateRequest
메서드는 지정된 개수에 따라StudySessionCreateRequest
객체 목록을 효율적으로 생성하여 테스트 설정의 유연성을 높입니다.
188-233
: 코드 변경 승인됨. 예외 메시지 검증 필요.
스터디_상세정보_작성시
클래스는validateUpdateStudyDetail
의 중요한 검증 로직을 테스트하는 두 가지 시나리오를 다룹니다. 예외 메시지가 예상대로 출력되는지 확인하세요.Verification successful
예외 메시지가 올바르게 구현되었습니다.
테스트 클래스
스터디_상세정보_작성시
의 예외 메시지 검증이 정확합니다.STUDY_DETAIL_SESSION_SIZE_MISMATCH
와STUDY_DETAIL_ID_INVALID
메시지가 올바르게 사용되었습니다.
STUDY_DETAIL_SESSION_SIZE_MISMATCH
: "스터디 커리큘럼의 총 개수가 일치하지 않습니다."STUDY_DETAIL_ID_INVALID
: "수정하려는 스터디 상세정보가 서버에 존재하지 않거나 유효하지 않습니다."Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify exception messages for `validateUpdateStudyDetail`. # Test: Search for exception messages. Expect: Correct exception messages are used. rg --type java 'STUDY_DETAIL_SESSION_SIZE_MISMATCH|getMessage' -A 2 rg --type java 'STUDY_DETAIL_ID_INVALID|getMessage' -A 2Length of output: 86279
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
@@ -27,6 +28,14 @@ public class MentorStudyDetailController { | |||
|
|||
private final MentorStudyDetailService mentorStudyDetailService; | |||
|
|||
@Operation(summary = "스터디 상세 정보 작성", description = "스터디 상세 정보를 작성합니다.") | |||
@PatchMapping |
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.
uri에 pathValue 넣어주세요
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.
requestParam은 필요 없지 않나요?
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에 대한 전체 detail을 변경하는거잖아요? 그렇다보니 일단 컨트롤러도 study쪽으로 가야하지 않을까 싶네요
한 study에 대해 수정을 진행하는거니 pathParam으로 studyId를 명시해야 더 명확할거같아요! (uri로 구분함으로써)
추가로 리퀘스트바디가 있는데 RequestParam(쿼리스트링)으로 받을 필요는 없어보여요!!
RequestParam형식은 get메소드에서 사용하는 것이니까요!
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 session -> curriculum 변경 | ||
@Transactional | ||
public void updateStudyDetail(Long studyId, StudyDetailUpdateRequest request) { |
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.
public void updateStudyDetail(Long studyId, StudyDetailUpdateRequest request) { | |
public void updateStudySessions(Long studyId, StudyDetailUpdateRequest request) { |
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.
세션 뿐만아니라 스터디도 수정하고 있습니다!
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.
Session부분은 따로 private메소드로 뺴서 작성해주는게 더 보기 좋을거같긴하네요!
import io.swagger.v3.oas.annotations.media.Schema; | ||
|
||
public record StudySessionCreateRequest( | ||
Long studyDetailId, |
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.
studyDetailId에 @NotNull 필요할거같아요!!
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
|
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
Outside diff range, codebase verification and nitpick comments (1)
src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyDetailService.java (1)
108-138
: 예외 처리 및 로깅 개선 제안.메서드는 여러 작업을 수행하며, 로깅이 적절하게 사용되었습니다. 그러나 예외 발생 시 더 구체적인 메시지를 기록하는 것이 좋습니다. 또한, 각 주요 단계에서 추가 로깅을 통해 디버깅을 용이하게 할 수 있습니다.
@Transactional public void updateStudyDetail(Long studyId, StudyDetailUpdateRequest request) { log.debug("Updating study detail for studyId={}", studyId); Member currentMember = memberUtil.getCurrentMember(); Study study = studyRepository.findById(studyId) .orElseThrow(() -> { log.error("Study not found for studyId={}", studyId); return new CustomException(STUDY_NOT_FOUND); }); studyValidator.validateStudyMentor(currentMember, study); log.debug("Validated mentor for studyId={}", studyId); List<StudyDetail> studyDetails = studyDetailRepository.findAllByStudyId(studyId); studyDetailValidator.validateUpdateStudyDetail(studyDetails, request.studySessions()); log.debug("Validated study details for studyId={}", studyId); study.update(request.notionLink(), request.introduction()); studyRepository.save(study); log.info("[MentorStudyDetailService] 스터디 기본 정보 수정 완료: studyId={}", studyId); Map<Long, StudySessionCreateRequest> requestMap = request.studySessions().stream() .collect(Collectors.toMap(StudySessionCreateRequest::studyDetailId, Function.identity())); List<StudyDetail> updatedStudyDetails = new ArrayList<>(); for (StudyDetail studyDetail : studyDetails) { Long id = studyDetail.getId(); StudySessionCreateRequest matchingSession = requestMap.get(id); if (matchingSession == null) { log.warn("No matching session found for studyDetailId={}", id); continue; } studyDetail.updateSession( studyDetail.getStudy().getPeriod().getStartDate(), matchingSession.title(), matchingSession.description(), matchingSession.difficulty(), matchingSession.status()); updatedStudyDetails.add(studyDetail); } studyDetailRepository.saveAll(updatedStudyDetails); log.info("[MentorStudyDetailService] 스터디 상세정보 커리큘럼 작성 완료: studyDetailId={}", studyDetails); }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyDetailService.java (3 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dto/request/StudySessionCreateRequest.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/gdschongik/gdsc/domain/study/dto/request/StudySessionCreateRequest.java
Additional comments not posted (3)
src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyDetailService.java (3)
7-14
: LGTM! 필요한 import 추가.필요한 import가 적절하게 추가되었습니다.
Also applies to: 21-23
37-38
: LGTM! 새로운 필드 선언.새로운 필드가 적절하게 선언되었습니다.
105-107
: 메서드 선언 적절함, 하지만 이름 확인 필요.메서드 선언이 적절합니다. 하지만 메서드 이름이 세션뿐만 아니라 스터디도 수정하고 있음을 반영해야 합니다.
Verification successful
현재 메서드 이름은 적절하지만, 향후 변경 사항에 주의 필요
updateStudyDetail
메서드는 현재 기능에 적합한 이름입니다. 그러나 "session"에서 "curriculum"으로의 변경이 계획되어 있으므로, 이러한 변경 사항이 구현될 경우 메서드 이름과 기능의 일관성을 확인해야 합니다.
MentorStudyDetailService.java
파일에서 TODO 주석이 이러한 변경을 암시합니다.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify method naming consistency with its functionality. # Test: Check for similar method names and their usage. rg --type java -A 3 'public void updateStudyDetail'Length of output: 734
Script:
#!/bin/bash # Description: Check for interactions with session or curriculum within the updateStudyDetail method. # Test: Search for keywords related to session or curriculum in the method. rg --type java -A 10 'public void updateStudyDetail' | rg 'session|curriculum'Length of output: 80
Script:
#!/bin/bash # Description: Search for TODO comments related to session or curriculum changes in the codebase. # Test: Search for TODO comments mentioning session or curriculum. rg --type java 'TODO.*session.*curriculum'Length of output: 170
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 (3)
- src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (2 hunks)
- src/test/java/com/gdschongik/gdsc/domain/study/application/MentorStudyServiceTest.java (1 hunks)
Additional comments not posted (3)
src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java (1)
22-28
: 스터디 상세 정보 업데이트 엔드포인트 추가
@PatchMapping
을 사용하여 스터디의 상세 정보를 업데이트하는 엔드포인트를 추가했습니다.StudyDetailUpdateRequest
를 통해 요청 본문을 받아 처리합니다. 이 구현은 RESTful 원칙에 부합하며,ResponseEntity.ok()
를 사용하여 성공적인 응답을 반환합니다. 적절한 예외 처리가 서비스 레이어에서 이루어지는지 확인해야 합니다.src/test/java/com/gdschongik/gdsc/domain/study/application/MentorStudyServiceTest.java (1)
30-77
: 스터디 상세 정보 업데이트 테스트이 테스트는 스터디 상세 정보 업데이트의 성공적인 시나리오를 검증합니다.
StudyDetailUpdateRequest
를 사용하여 요청을 생성하고,mentorStudyService.updateStudyDetail
메서드를 호출하여 업데이트를 수행합니다. 그런 다음, 저장된 스터디와 세부 정보가 요청과 일치하는지 확인합니다. 테스트는 적절하게 구성되어 있으며, 다양한 속성을 검증하여 업데이트 로직이 올바르게 작동하는지 확인합니다. 테스트 커버리지를 높이기 위해 실패 시나리오에 대한 테스트도 추가하는 것이 좋습니다.src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (1)
59-92
: 스터디 상세 정보 업데이트 메서드 구현
updateStudyDetail
메서드는 스터디 ID와StudyDetailUpdateRequest
를 받아 스터디의 상세 정보를 업데이트합니다. 멤버 검증, 스터디 존재 여부 확인, 세션 정보 매핑 및 업데이트가 포함되어 있습니다. Java Streams를 사용하여 요청 데이터를 효율적으로 처리하고 있으며, 트랜잭션 관리가 적절하게 이루어지고 있습니다. 예외 처리와 로그 메시지를 통해 디버깅을 용이하게 하고 있습니다. 성능 최적화를 위해 필요에 따라 데이터베이스 접근을 최소화하는 방법을 고려할 수 있습니다.
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
StudySessionCreateRequest matchingSession = requestMap.get(id); | ||
|
||
studyDetail.updateSession( | ||
studyDetail.getStudy().getPeriod().getStartDate(), |
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.
세션 시작시간이 왜 항상 스터디 시작일이어야 하는 건가요?
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.
피그마상에서 세션 시작시간에 대해 요청을 받고 있지 않더라고요
그래서 startAt이 LocalDateTime으로 되어있길래 시작 날짜라고 생각햇습니다
이 부분은 기획분들한테 정확히 물어보겠습니다
|
||
study.update(request.notionLink(), request.introduction()); | ||
studyRepository.save(study); | ||
log.info("[MentorStudyService] 스터디 기본 정보 수정 완료: studyId={}", studyId); |
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.
두 로직을 추출하여 적절히 분리하는게 좋을 것 같습니다.
studyRepository.save(study); | ||
log.info("[MentorStudyService] 스터디 기본 정보 수정 완료: studyId={}", studyId); | ||
|
||
Map<Long, StudySessionCreateRequest> requestMap = request.studySessions().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.
굳이 맵으로 만들 필요 없이 리스트에서 바로 찾아서 반환하면 될듯 합니다.
@Test | ||
void 요청한_상세정보_id와_기존의_상세정보_id가_맞지_않으면_실패한다() { | ||
// given | ||
LocalDateTime now = LocalDateTime.now(); |
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.
저번에 PR에서 논의했던 것처럼 now 대신 고정된 날짜를 사용할 수 있으면 적절히 대체하는 것이 좋아보입니다.
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.
앗 그 부분에 대해서 생각 못했네요
Map<Long, StudySessionCreateRequest> requestMap = request.studySessions().stream() | ||
.collect(Collectors.toMap(StudySessionCreateRequest::studyDetailId, Function.identity())); | ||
|
||
List<StudyDetail> updatedStudyDetails = new ArrayList<>(); |
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.
굳이 updatedStudyDetails를 한번 더 담는 이유가 뭔가요?
@@ -182,4 +182,9 @@ public boolean isStudyOngoing() { | |||
public LocalDate getStartDate() { | |||
return period.getStartDate().toLocalDate(); | |||
} | |||
|
|||
public void update(String link, String intro) { |
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.
public void update(String link, String intro) { | |
public void update(String link, String introduction) { |
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.
위에 필드랑 겹쳐서 오류가 나서 intro로 만들었습니다
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.
그러면 this로 명시해서 넣으면 되지 않나요?
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.
아아 이전에 History쪽에서는 repository를 repo로 하신거 생각하고 이런식으로 처리를 하는건가보다~해서 intro라고 했습니다 다시 보니까 다 this로 바꿔져있군요!
this 쓰겠습니다
assertThat(studyDetail.getId()).isEqualTo(expectedId); | ||
assertThat(studyDetail.getSession().getTitle()).isEqualTo(SESSION_TITLE + expectedId); | ||
assertThat(studyDetail.getSession().getDescription()).isEqualTo(SESSION_DESCRIPTION + expectedId); | ||
assertThat(studyDetail.getSession().getDifficulty()).isEqualTo(Difficulty.HIGH); |
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.
불필요하게 모든 필드를 전부 검증하지 않아도 됩니다.
@@ -54,4 +58,23 @@ private void validateUpdateDeadline( | |||
throw new CustomException(STUDY_DETAIL_ASSIGNMENT_INVALID_UPDATE_DEADLINE); | |||
} | |||
} | |||
|
|||
public void validateUpdateStudyDetail(List<StudyDetail> studyDetails, List<StudySessionCreateRequest> requests) { |
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.
validator는 도메인 계층에 속한 도메인서비스이기 때문에, 외부에 대한 의존성을 가져서는 안됩니다.
표현 계층에서 사용하는 Request를 직접 인자로 받는 것은 적절하지 않습니다.
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.
현재validator에서 만든 Set들을 서비스 레이어에서 만들어서넘기겠습니다
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
|
||
// TODO session -> curriculum 변경 | ||
@Transactional | ||
public void updateStudyDetail(Long studyId, StudyDetailUpdateRequest request) { |
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.
Session부분은 따로 private메소드로 뺴서 작성해주는게 더 보기 좋을거같긴하네요!
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: 1
Outside diff range, codebase verification and nitpick comments (1)
src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (1)
106-127
:updateStudy
메서드의 구현이 잘 되었습니다.이 메서드는 스터디의 기본 정보를 업데이트하고, 요청된 세션 정보를 검증하며, 필요한 경우 예외를 발생시킵니다. 전반적으로 잘 구조화되어 있습니다. 그러나,
log.info
메시지에 좀 더 구체적인 정보가 포함되면 좋을 것 같습니다.- log.info("[MentorStudyService] 스터디 기본 정보 수정 완료: studyId={}", studyId); + log.info("[MentorStudyService] 스터디 기본 정보 수정 완료: studyId={}, notionLink={}, introduction={}", studyId, request.notionLink(), request.introduction());
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java (3 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (3 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyDetailValidator.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/dto/request/StudyUpdateRequest.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (2 hunks)
- src/test/java/com/gdschongik/gdsc/domain/study/application/MentorStudyServiceTest.java (1 hunks)
- src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyDetailValidatorTest.java (3 hunks)
- src/test/java/com/gdschongik/gdsc/helper/FixtureHelper.java (1 hunks)
Files skipped from review as they are similar to previous changes (6)
- src/main/java/com/gdschongik/gdsc/domain/study/api/MentorStudyController.java
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyDetailValidator.java
- src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java
- src/test/java/com/gdschongik/gdsc/domain/study/application/MentorStudyServiceTest.java
- src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyDetailValidatorTest.java
- src/test/java/com/gdschongik/gdsc/helper/FixtureHelper.java
Additional comments not posted (2)
src/main/java/com/gdschongik/gdsc/domain/study/dto/request/StudyUpdateRequest.java (1)
1-9
: 구현이 잘 되었습니다.
StudyUpdateRequest
레코드는 스터디 업데이트 요청을 위한 적절한 구조를 가지고 있습니다. Swagger 주석도 잘 추가되어 있어 API 문서화에 도움이 됩니다.src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (1)
41-42
: 새로운 필드 추가가 적절합니다.
StudyDetailRepository
와StudyDetailValidator
의 추가는 스터디 세부 정보 관리를 강화하는 데 필요한 요소입니다.
private void updateAllStudyDetailSession( | ||
List<StudyDetail> studyDetails, List<StudySessionCreateRequest> studySessions) { | ||
for (StudyDetail studyDetail : studyDetails) { | ||
Long id = studyDetail.getId(); | ||
StudySessionCreateRequest matchingSession = studySessions.stream() | ||
.filter(session -> session.studyDetailId().equals(id)) | ||
.findFirst() | ||
.get(); | ||
|
||
studyDetail.updateSession( | ||
studyDetail.getStudy().getPeriod().getStartDate(), | ||
matchingSession.title(), | ||
matchingSession.description(), | ||
matchingSession.difficulty(), | ||
matchingSession.status()); | ||
} | ||
studyDetailRepository.saveAll(studyDetails); | ||
log.info("[MentorStudyService] 스터디 상세정보 커리큘럼 작성 완료: studyDetailId={}", studyDetails); | ||
} |
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.
updateAllStudyDetailSession
메서드의 예외 처리 추가 필요
현재 findFirst().get()
을 사용하고 있어 매칭되는 세션이 없을 경우 NoSuchElementException
이 발생할 수 있습니다. 이를 방지하기 위해 예외 처리를 추가하는 것이 좋습니다.
StudySessionCreateRequest matchingSession = studySessions.stream()
.filter(session -> session.studyDetailId().equals(id))
.findFirst()
.orElseThrow(() -> new CustomException(ErrorCode.SESSION_NOT_FOUND, "세션이 존재하지 않습니다: studyDetailId=" + id));
@@ -182,4 +182,9 @@ public boolean isStudyOngoing() { | |||
public LocalDate getStartDate() { | |||
return period.getStartDate().toLocalDate(); | |||
} | |||
|
|||
public void update(String link, String intro) { |
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.
그러면 this로 명시해서 넣으면 되지 않나요?
studyDetails.stream().map(StudyDetail::getId).collect(Collectors.toSet()); | ||
|
||
// 요청된 StudySessionCreateRequest의 StudyDetail ID를 추출하여 Set으로 저장 | ||
Set<Long> requestIds = sessionCreateRequest.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.
어차피 validator에서는 id 세트만 받는 거면 given 절에서 다른 엔티티 픽스처를 세팅해줄 필요가 없지 않나요?
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/domain/Study.java (1 hunks)
- src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyDetailValidatorTest.java (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/Study.java
Additional comments not posted (2)
src/test/java/com/gdschongik/gdsc/domain/study/domain/StudyDetailValidatorTest.java (2)
167-178
: 테스트 설정이 적절합니다.이 테스트는 존재하는 스터디 상세 정보 ID의 총개수와 요청된 ID의 총개수가 다를 때
CustomException
이 발생하는지를 검증합니다. 테스트 로직이 예상한 대로 작동합니다.
180-191
: 테스트 설정이 적절합니다.이 테스트는 요청한 상세 정보 ID가 기존의 ID와 맞지 않을 때
CustomException
이 발생하는지를 검증합니다. 테스트 로직이 예상한 대로 작동합니다.
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
Outside diff range, codebase verification and nitpick comments (1)
src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (1)
104-104
: TODO 주석 업데이트 필요: 'session'을 'curriculum'으로 변경해야 합니다.코드베이스에서 'session'이라는 용어가 여전히 사용되고 있습니다. 특히 주요 코드 파일에서 'session'이 사용되고 있어, 'curriculum'으로의 변경이 완료되지 않은 것으로 보입니다. 다음 파일들을 확인하여 변경 사항을 반영해야 합니다:
src/main/java/com/gdschongik/gdsc/domain/study/dto/response/StudySessionResponse.java
src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyDetail.java
Analysis chain
TODO 주석 업데이트 필요.
session
을curriculum
으로 변경하는 주석이 있습니다. 이 부분은 실제 코드에서 반영되었는지 확인이 필요합니다.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the term 'session' has been replaced with 'curriculum' in the codebase. # Test: Search for occurrences of 'session'. Expect: Occurrences should be updated to 'curriculum'. rg --type java 'session'Length of output: 3447
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (3 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyDetail.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/vo/Session.java (4 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyDetail.java
- src/main/java/com/gdschongik/gdsc/domain/study/domain/vo/Session.java
Additional comments not posted (3)
src/main/java/com/gdschongik/gdsc/domain/study/application/MentorStudyService.java (3)
7-7
: 필요한 import 추가 확인.
StudyDetailRepository
와StudyDetailValidator
의 import가 적절히 추가되었습니다.Also applies to: 41-42
106-127
: 예외 처리 추가 필요.
updateAllStudyDetailSession
메서드에서findFirst().get()
을 사용하고 있어 매칭되는 세션이 없을 경우NoSuchElementException
이 발생할 수 있습니다. 이를 방지하기 위해 예외 처리를 추가하는 것이 좋습니다.
129-147
:updateAllStudyDetailSession
메서드의 예외 처리 추가 필요현재
findFirst().get()
을 사용하고 있어 매칭되는 세션이 없을 경우NoSuchElementException
이 발생할 수 있습니다. 이를 방지하기 위해 예외 처리를 추가하는 것이 좋습니다.StudySessionCreateRequest matchingSession = studySessions.stream() .filter(session -> session.studyDetailId().equals(id)) .findFirst() .orElseThrow(() -> new CustomException(ErrorCode.SESSION_NOT_FOUND, "세션이 존재하지 않습니다: studyDetailId=" + id));
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
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit
Summary by CodeRabbit
New Features
notionLink
및introduction
을 업데이트하는 기능 추가.Bug Fixes
Documentation
Tests