-
Notifications
You must be signed in to change notification settings - Fork 8
refactor: 멘토의 멘토링 조회 시, confirm 여부 포함하여 응답하도록 #441
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
refactor: 멘토의 멘토링 조회 시, confirm 여부 포함하여 응답하도록 #441
Conversation
- getMentoringsForMentee와 getMentoringsForMentor가 같은 구조를 갖도록
Walkthrough
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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 (5)
src/main/java/com/example/solidconnection/mentor/service/MentoringQueryService.java (3)
60-76: [2] 이슈 목적상 ‘미확정’만 노출 필요합니다. 서비스 레이어 필터링 적용 여부 확인 부탁드립니다.
현재는 isConfirmed 필드만 추가되어 있습니다.
이슈 #425의 의도는 멘토 화면에 confirmed 된 멘토링을 숨기는 것입니다.
본 PR 범위를 DTO 확장으로 한정한 것으로 이해합니다. 다음 단계로 서버 필터를 적용할지 확인 부탁드립니다.
옵션 A. 레포지토리 쿼리로 필터링합니다.
- 예: findAllByMentorIdAndConfirmedAtIsNull(...) 형태로 확장합니다.
옵션 B. 서비스에서 스트림 필터링을 적용합니다.
- 아래는 최소 변경 예시입니다.
- Map<Mentoring, SiteUser> mentoringToPartnerUser = mapMentoringToPartnerUserWithBatchQuery( - mentoringSlice.toList(), - Mentoring::getMenteeId - ); + List<Mentoring> mentorings = mentoringSlice.toList().stream() + .filter(m -> m.getConfirmedAt() == null) + .toList(); + Map<Mentoring, SiteUser> mentoringToPartnerUser = mapMentoringToPartnerUserWithBatchQuery( + mentorings, + Mentoring::getMenteeId + );
90-93: [3] 응답 순서 보존을 위해 LinkedHashMap 사용을 권장합니다.
- 현재 HashMap 수집으로 페이지 내 순서가 보장되지 않습니다.
- Slice 순서를 그대로 유지하려면 LinkedHashMap 수집을 권장합니다.
- return mentorings.stream().collect(Collectors.toMap( - Function.identity(), - mentoring -> partnerIdToPartnerUsermap.get(getPartnerId.apply(mentoring)) - )); + return mentorings.stream().collect(Collectors.toMap( + Function.identity(), + mentoring -> partnerIdToPartnerUserMap.get(getPartnerId.apply(mentoring)), + (a, b) -> a, + java.util.LinkedHashMap::new + ));
- 추가로, 상단에 import java.util.LinkedHashMap; 를 추가해 주세요.
87-88: [4] 변수명 오타 정정 제안입니다.
- partnerIdToPartnerUsermap → partnerIdToPartnerUserMap 으로 카멜케이스를 맞추면 가독성이 좋아집니다.
- Map<Long, SiteUser> partnerIdToPartnerUsermap = partnerUsers.stream() - .collect(Collectors.toMap(SiteUser::getId, Function.identity())); + Map<Long, SiteUser> partnerIdToPartnerUserMap = partnerUsers.stream() + .collect(Collectors.toMap(SiteUser::getId, Function.identity()));
- 아래 참조부도 함께 변경이 필요합니다.
src/main/java/com/example/solidconnection/mentor/dto/MentoringForMentorResponse.java (2)
16-25: [4] 단위 테스트 보강을 제안드립니다.
- 케이스 1. confirmedAt != null → isConfirmed = true 직렬화 검증.
- 케이스 2. confirmedAt == null → isConfirmed = false 직렬화 검증.
- 스냅샷 테스트로 JSON 키 포함 여부까지 확인하면 회귀 방지에 유용합니다.
12-12: Jackson 버전 및 직렬화 키 검증 요청안녕하세요! 즐겁게 아래 두 가지를 점검해 주세요. 😄
- Jackson 라이브러리 버전 확인
build.gradle 또는 pom.xml에서 com.fasterxml.jackson 의존성 버전을 찾아주세요. (record boolean 컴포넌트의 JSON 키 노출 방식은 버전마다 달라질 수 있습니다.)- @JsonProperty 적용 검증
isConfirmed 필드가 실제로 JSON에서"isConfirmed"로 직렬화되는지 확인하고, 필요 시
@JsonProperty("isConfirmed")애노테이션을 추가해 안정성을 확보하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/example/solidconnection/mentor/dto/MentoringForMentorResponse.java(2 hunks)src/main/java/com/example/solidconnection/mentor/service/MentoringQueryService.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
src/main/java/com/example/solidconnection/mentor/service/MentoringQueryService.java (2)
47-49: [1] toList 인라인으로 간결해졌습니다.
- 성능 영향은 없습니다.
- 이 메서드 내에서 한 번만 사용하므로 가독성도 좋아졌습니다.
- 아래 멘토용 메서드와 형태가 더 잘 맞춰졌습니다.
92-93: [5] 파트너 사용자 누락 시 NPE 가능성이 있습니다. 가드 추가를 검토해 주세요.
- partnerIdToPartnerUserMap.get(...) 이 null이면 이후 DTO 생성 시 NPE가 발생할 수 있습니다.
- 데이터 정합성이 100% 보장되지 않는 환경이라면 가드를 넣는 편이 안전합니다.
- 예: Objects.requireNonNull(...) 또는 null 시 필터링/로깅 처리.
src/main/java/com/example/solidconnection/mentor/dto/MentoringForMentorResponse.java (2)
22-22: [2] confirmedAt 존재 여부로 매핑하는 로직이 명확합니다.
- mentoring.getConfirmedAt() != null 조건은 비즈니스 의미와 일치합니다.
7-14: 레코드 시그니처 변경 후 직접 생성 호출점 확인 결과, 문제 없습니다
- new 호출 위치 확인
- ‘src/main/java/com/example/solidconnection/mentor/dto/MentoringForMentorResponse.java’ 내부의 정적 팩토리 메소드(
of) 구현부에서만new MentoringForMentorResponse(...)호출이 발견되었습니다.- 외부 호출 현황
- 서비스 레이어(
MentoringQueryService) 등 파일 외부에서는 오직MentoringForMentorResponse.of(...)정적 팩토리만 사용 중입니다.위 검증 결과 직접 생성(new) 호출은 DTO 내부에 국한되어 있어 컴파일 오류나 의도치 않은 부작용은 발생하지 않습니다. 안심하고 머지 진행해주세요!
whqtker
left a comment
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.
확인했습니다 !
관련 이슈
작업 내용
멘토가 보는 멘토링 목록에서 위 스크린샷의 기능이 가능하도록
isConfirmedJSON key 를 추가로 응답하게 했습니다.MentoringForMentorResponse 클래스에서 (Mentoring mentoring, SiteUser partner) 을
인자로 받고 있으므로, 서비스 코드 변경 없이 dto 내부만 변경할 수 있었습니다. 👍
특이 사항
추가로, getMentoringsForMentee와 getMentoringsForMentor의 구조가 거의 동일한데
세부적으로 코드가 다른 부분이 있어 통일해주었습니다.
불필요한 인지 부하를 줄이는게 좋다 판단했습니다!