-
Notifications
You must be signed in to change notification settings - Fork 8
feat: 멘토 조회 시 파견대학정보 포함하는 기능, 권역 필터링 기능 구현 #415
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: 멘토 조회 시 파견대학정보 포함하는 기능, 권역 필터링 기능 구현 #415
Conversation
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (10)
⏰ 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)
🔇 Additional comments (4)
✨ 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. 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 (
|
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/com/example/solidconnection/location/region/repository/RegionRepository.java(2 hunks)src/main/java/com/example/solidconnection/mentor/dto/MentorDetailResponse.java(2 hunks)src/main/java/com/example/solidconnection/mentor/dto/MentorMyPageResponse.java(2 hunks)src/main/java/com/example/solidconnection/mentor/dto/MentorPreviewResponse.java(2 hunks)src/main/java/com/example/solidconnection/mentor/repository/MentorBatchQueryRepository.java(3 hunks)src/main/java/com/example/solidconnection/mentor/repository/MentorRepository.java(2 hunks)src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java(3 hunks)src/main/java/com/example/solidconnection/mentor/service/MentorQueryService.java(3 hunks)src/test/java/com/example/solidconnection/mentor/repository/MentorBatchQueryRepositoryTest.java(4 hunks)src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java(3 hunks)src/test/java/com/example/solidconnection/mentor/service/MentorQueryServiceTest.java(7 hunks)
🔇 Additional comments (35)
src/main/java/com/example/solidconnection/location/region/repository/RegionRepository.java (1)
5-5: 새로운 단일 지역 조회 메서드 추가 승인
- Optional 임포트 추가와
findByKoreanName메서드 구현이 적절합니다
- Spring Data JPA 네이밍 컨벤션을 올바르게 따르고 있습니다
- 기존
findByKoreanNames메서드와 잘 보완되는 단일 조회 기능을 제공합니다- Optional 반환 타입으로 null safety를 보장합니다
Also applies to: 15-15
src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java (1)
21-23: 대학교 정보 통합을 위한 테스트 수정 승인
대학교 엔티티 통합을 위한 테스트 구조 개선:
UniversityFixture의존성 추가 및University필드 선언이 적절합니다setUp메서드에서 대학교 생성 후 멘토와 연결하는 로직이 올바릅니다응답 검증 로직 강화:
universityName과country필드에 대한 어서션 추가로 새로운 기능의 정확성을 검증합니다- 실제 대학교 엔티티의 데이터를 사용한 검증으로 테스트의 신뢰성이 향상되었습니다
Also applies to: 49-51, 57-57, 61-63, 82-83
src/main/java/com/example/solidconnection/mentor/repository/MentorRepository.java (1)
3-3: 지역별 멘토 필터링을 위한 레포지토리 메서드 추가 승인
적절한 의존성 임포트:
Region,Query,Param임포트가 새로운 기능에 필요한 요소들을 모두 포함합니다효율적인 JPQL 쿼리 구현:
Mentor와University엔티티를 올바르게 조인하여 지역 필터링을 수행합니다Slice<Mentor>반환 타입과Pageable파라미터로 페이지네이션을 지원합니다- 쿼리 구조가 명확하고 성능상 효율적입니다
Also applies to: 9-10, 20-25
src/main/java/com/example/solidconnection/mentor/dto/MentorMyPageResponse.java (1)
6-6: 대학교 정보를 활용한 응답 DTO 개선 승인
팩토리 메서드 시그니처 개선:
University파라미터 추가로 실제 대학교 데이터를 활용할 수 있게 되었습니다동적 데이터 활용:
- 기존 하드코딩된 플레이스홀더 값 대신
university.getCountry().getKoreanName()과university.getKoreanName()으로 실제 데이터를 사용합니다- 데이터의 정확성과 일관성이 크게 향상되었습니다
Also applies to: 22-22, 28-29
src/test/java/com/example/solidconnection/mentor/repository/MentorBatchQueryRepositoryTest.java (1)
12-13: 대학교 매핑 기능을 위한 테스트 확장 승인
테스트 데이터 구조 개선:
- 단일 대학교 ID 대신 두 개의 서로 다른
University인스턴스를 사용하여 더 현실적인 테스트 환경을 구성했습니다UniversityFixture를 활용한 대학교 생성으로 테스트 데이터의 일관성을 보장합니다새로운 매핑 기능 테스트 추가:
멘토_ID_와_멘토의_파견_대학교를_매핑한다()테스트 메서드가 새로운getMentorIdToUniversityMap기능을 적절히 검증합니다- 각 멘토가 올바른 대학교와 매핑되는지 확인하는 어서션이 정확합니다
테스트 설정 로직 강화:
setUp메서드에서 서로 다른 대학교를 각 멘토에 할당하여 매핑 기능의 정확성을 검증할 수 있는 환경을 조성했습니다Also applies to: 37-40, 49-52, 70-83
src/main/java/com/example/solidconnection/mentor/dto/MentorDetailResponse.java (4)
6-6: 적절한 임포트 추가University 도메인 클래스를 정확하게 임포트했습니다.
24-25: 메서드 시그니처 확장이 적절함University 파라미터를 추가하여 파견대학 정보를 포함하도록 메서드를 확장한 것이 올바릅니다.
31-32: 하드코딩된 값을 동적 데이터로 교체
- 국가명:
university.getCountry().getKoreanName()사용- 대학명:
university.getKoreanName()사용기존 플레이스홀더를 실제 대학 데이터로 교체한 구현이 깔끔합니다.
6-6: 변경사항이 깔끔하고 일관성 있게 구현되었습니다!다음과 같은 개선사항들이 잘 적용되었습니다:
- University 도메인 import 추가 (라인 6)
- 메서드 시그니처 업데이트 (라인 24-25): University 파라미터 추가
- 동적 데이터 활용 (라인 31-32): 하드코딩된 플레이스홀더를 실제 대학교 정보로 교체
특히
university.getCountry().getKoreanName()과university.getKoreanName()을 통해 실제 데이터를 활용하는 부분이 인상적입니다.Also applies to: 24-25, 31-32
src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java (3)
5-5: 의존성 추가가 적절함
- 에러 코드 임포트:
UNIVERSITY_NOT_FOUND추가- 도메인 및 리포지토리 임포트:
University,UniversityRepository추가- 필드 주입:
UniversityRepository의존성 주입필요한 의존성들이 모두 올바르게 추가되었습니다.
Also applies to: 17-18, 34-34
42-44: 대학 정보 조회 로직 구현 완료
- 멘토의 대학 ID로 University 엔티티 조회
- 대학이 존재하지 않을 경우 적절한 예외 처리
- DTO 생성 시 University 객체 전달
에러 핸들링과 데이터 조회 로직이 잘 구현되었습니다.
5-5: 서비스 레이어의 대학교 정보 통합이 체계적으로 구현되었습니다.개선된 부분들을 정리하면:
- 필요한 의존성 추가 (라인 5, 17-18, 34): 에러코드, 도메인, 레포지토리 import 및 의존성 주입
- 대학교 정보 조회 로직 (라인 42-43): 멘토의 대학교 ID로 University 엔티티 조회
- 적절한 예외 처리: UNIVERSITY_NOT_FOUND로 일관성 있는 에러 핸들링
- 응답 객체 생성 업데이트 (라인 44): University 파라미터 포함
특히 예외 처리가 기존 패턴과 일관성을 유지하고 있어 코드의 안정성이 향상되었습니다.
Also applies to: 17-18, 34-34, 42-44
src/main/java/com/example/solidconnection/mentor/dto/MentorPreviewResponse.java (4)
6-6: 일관된 임포트 추가MentorDetailResponse와 동일한 패턴으로 University 클래스를 임포트했습니다.
23-24: 메서드 시그니처 일관성 유지다른 멘토 DTO들과 동일한 방식으로 University 파라미터를 추가하여 일관성을 유지했습니다.
30-31: 동적 대학 정보 활용
- 국가명:
university.getCountry().getKoreanName()- 대학명:
university.getKoreanName()MentorDetailResponse와 동일한 방식으로 실제 대학 데이터를 활용한 구현이 일관성 있습니다.
6-6: MentorDetailResponse와 일관성 있는 구현으로 훌륭합니다!변경사항들이 체계적으로 적용되었습니다:
- University 도메인 import (라인 6)
- 메서드 시그니처 확장 (라인 23-24): University 파라미터 추가로 실제 데이터 활용 가능
- 실제 대학교 정보 활용 (라인 30-31): 플레이스홀더 대신
university.getCountry().getKoreanName(),university.getKoreanName()사용다른 Response DTO들과 동일한 패턴을 따라 코드베이스의 일관성이 유지되고 있습니다.
Also applies to: 23-24, 30-31
src/main/java/com/example/solidconnection/mentor/repository/MentorBatchQueryRepository.java (3)
10-11: 배치 쿼리를 위한 의존성 추가
- University 도메인 임포트
- UniversityRepository 의존성 주입
N+1 문제 해결을 위한 배치 쿼리 구현에 필요한 의존성들이 적절히 추가되었습니다.
Also applies to: 26-26
46-62: 효율적인 배치 쿼리 메서드 구현
- 멘토 리스트에서 대학 ID 추출
- 배치로 University 엔티티들 조회
- University ID를 키로 하는 맵 생성
- 멘토 ID와 University 매핑
- 데이터 정합성 검증 및 예외 처리
기존
getMentorIdToSiteUserMap메서드와 동일한 패턴을 따라 N+1 문제를 효과적으로 방지하는 구현입니다.
10-11: 배치 쿼리 레포지토리에 대학교 매핑 기능이 효율적으로 구현되었습니다!새로 추가된 기능들의 장점:
- 필수 의존성 추가 (라인 10-11, 26): University 도메인과 레포지토리 import 및 의존성 주입
- 일관성 있는 메서드 구조 (라인 46-62): 기존
getMentorIdToSiteUserMap과 동일한 패턴 적용- N+1 문제 방지: 배치로 대학교 정보를 조회하여 성능 최적화
- 데이터 무결성 검증 (라인 56-57): 멘토의 university_id에 해당하는 대학교가 없을 경우 적절한 예외 처리
- 효율적인 스트림 연산: Map 변환과 데이터 매핑이 깔끔하게 구현됨
특히 기존 코드와 동일한 패턴을 유지하면서도 새로운 기능을 추가한 점이 인상적입니다.
Also applies to: 26-26, 46-62
src/main/java/com/example/solidconnection/mentor/service/MentorQueryService.java (9)
4-4: 권역 필터링 기능을 위한 의존성 추가
- 에러 코드:
UNIVERSITY_NOT_FOUND,ErrorCode추가- 권역 관련:
Region,RegionRepository임포트- 대학 관련:
University,UniversityRepository임포트- 서비스 필드: 필요한 리포지토리들 의존성 주입
권역 필터링과 대학 정보 통합을 위한 모든 의존성이 적절히 추가되었습니다.
Also applies to: 8-10, 19-20, 38-39
45-46: 멘토 상세 조회에 대학 정보 통합
- 멘토의 대학 ID로 University 엔티티 조회
- 대학이 존재하지 않을 경우 적절한 예외 처리
- DTO 생성 시 University 객체 전달
멘토 상세 정보에 파견대학 데이터가 올바르게 통합되었습니다.
Also applies to: 51-51
55-55: 권역 필터링 파라미터 추가메서드 시그니처에
regionKoreanName파라미터를 추가하여 권역별 멘토 조회 기능을 지원합니다.
74-74: 배치 조회에 대학 정보 매핑 추가
- 배치 쿼리로 멘토별 University 매핑 조회
- 각 멘토에 대해 University 객체 할당
- DTO 생성 시 University 데이터 전달
기존 배치 쿼리 패턴을 유지하면서 대학 정보가 효율적으로 통합되었습니다.
Also applies to: 80-80, 82-82
63-70: ✅ 에러 코드 정의 확인 및 코드 승인
- 에러 코드 정의 확인
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java에서REGION_NOT_FOUND_BY_KOREAN_NAME이 정상 정의되어 있습니다.- 권역별 필터링 로직 검증
src/main/java/com/example/solidconnection/mentor/service/MentorQueryService.java (63–70라인): 예외 처리 흐름이 적절히 구현되어 있습니다.모든 검토를 마쳤으며 추가 수정 없이 코드 변경을 승인합니다.
4-4: 필요한 의존성들이 체계적으로 추가되었습니다.다음과 같은 import들이 적절히 추가되었습니다:
- 에러 처리: UNIVERSITY_NOT_FOUND, ErrorCode 추가
- 지역 관련: Region 도메인과 RegionRepository
- 대학교 관련: University 도메인과 UniversityRepository
모든 새로운 기능에 필요한 의존성이 빠짐없이 포함되어 있습니다.
Also applies to: 8-10, 19-20, 38-39
45-46: 대학교 정보 조회 로직이 잘 구현되었습니다.변경사항:
- University 엔티티 조회 (라인 45-46): 멘토의 대학교 ID로 조회하고 적절한 예외 처리
- 응답 객체 생성 업데이트 (라인 51): University 파라미터 포함하여 실제 데이터 활용
대학교 정보가 없을 경우 UNIVERSITY_NOT_FOUND 예외로 명확한 에러 처리가 이루어지고 있습니다.
Also applies to: 51-51
55-55: 지역 필터링 기능이 효과적으로 구현되었습니다!새로 추가된 기능들:
- 메서드 시그니처 업데이트 (라인 55): regionKoreanName 파라미터 추가
- 지역 필터링 로직 (라인 63-70):
- null/empty 체크로 전체 멘토 반환
- 지역명으로 Region 조회 및 예외 처리
- 지역별 멘토 필터링 수행
특히 조건부 필터링 로직이 깔끔하게 구현되어 있어 사용성이 좋을 것 같습니다.
Also applies to: 63-70
74-74: 배치 쿼리에 대학교 정보 매핑이 잘 추가되었습니다.개선된 부분들:
- 대학교 매핑 추가 (라인 74):
getMentorIdToUniversityMap배치 쿼리 활용- University 객체 활용 (라인 80): 각 멘토의 대학교 정보 추출
- 응답 생성 업데이트 (라인 82): University 파라미터를 포함한 MentorPreviewResponse 생성
N+1 문제 없이 효율적으로 대학교 정보를 포함한 멘토 미리보기 응답을 생성할 수 있게 되었습니다.
Also applies to: 80-80, 82-82
src/test/java/com/example/solidconnection/mentor/service/MentorQueryServiceTest.java (7)
21-22: 새로운 대학 관련 기능을 위한 import 추가멘토 조회 시 파견대학정보를 포함하는 기능 구현을 위해 필요한 import들이 올바르게 추가되었습니다.
52-60: 테스트 설정을 위한 대학 인스턴스 초기화멘토와 대학 간의 연관관계를 테스트하기 위해 University 인스턴스가 적절히 초기화되었습니다. 픽스처를 활용한 일관된 테스트 데이터 설정이 잘 구현되어 있습니다.
70-70: 멘토 상세 조회 테스트의 대학 정보 검증 추가
- 멘토 생성 시 동적 대학 ID 사용으로 변경
- 대학명(universityName)과 국가명(country) 필드 검증 추가
멘토 조회 시 파견대학정보가 올바르게 포함되는지 검증하는 테스트로 적절히 수정되었습니다.
Also applies to: 81-82, 92-92
126-126: 멘토 미리보기 테스트의 대학 정보 통합
- 중첩 클래스명을 더 명확하게 변경
- 다양한 대학 픽스처를 활용한 테스트 데이터 설정
- 멘토와 대학 간의 연관관계를 반영한 테스트 구조 개선
테스트 데이터 설정이 새로운 기능 요구사항에 맞게 잘 구성되었습니다.
Also applies to: 132-132, 139-143
152-152: 멘토 미리보기 응답의 대학 정보 검증
- 빈 문자열("")을 권역 필터로 사용하여 전체 조회 테스트
- 각 멘토의 대학명과 국가명 필드 검증 추가
멘토 미리보기에서 대학 정보가 올바르게 포함되는지 확인하는 테스트로 적절히 개선되었습니다.
Also applies to: 161-162, 167-169
175-191: 페이지네이션 동작 검증 테스트 추가
- 다음 페이지 번호 반환 확인
- 다음 페이지가 없을 때의 처리 확인
페이지네이션 기능의 정확한 동작을 검증하는 테스트가 잘 구현되었습니다.
193-240: 권역 필터링 기능 테스트 구현
- 아시아와 유럽 대학으로 구분된 멘토 데이터 설정
- 권역별 필터링 동작 검증
- 전체 조회(빈 권역 필터) 동작 검증
권역 필터링 기능이 올바르게 동작하는지 확인하는 포괄적인 테스트가 잘 구현되었습니다. 특히 다양한 지역의 대학을 활용한 테스트 시나리오가 실제 사용 사례를 잘 반영하고 있습니다.
src/main/java/com/example/solidconnection/mentor/service/MentorQueryService.java
Show resolved
Hide resolved
a560152 to
bbdda98
Compare
| university.getCountry().getKoreanName(), | ||
| university.getKoreanName(), |
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.
university는 country 를 eager로 fetch 해오기 때문에
여기에서 initialization exception이 발생하진 않습니다~
(혹시나 아리송 하실까봐 설명 덧붙입니다 ㅎㅎ)
| public Map<Long, University> getMentorIdToUniversityMap(List<Mentor> mentors) { | ||
| List<Long> universityIds = mentors.stream().map(Mentor::getUniversityId).toList(); | ||
| List<University> universities = universityRepository.findAllById(universityIds); | ||
| Map<Long, University> universityIdToUniversityMap = universities.stream() | ||
| .collect(Collectors.toMap(University::getId, Function.identity())); |
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.
Mentor와 University도 객체 참조 관계가 아니기 때문에,
List<Mentor>의 각 원소들에 대한 university를 조회하려면 N+1 문제가 발생합니다.
따라서 기존애 해결하던 방법대로 Batch로 가져오는 함수를 구현하여 사용했습니다~
| return mentorRepository.findAllByRegion(region, pageable); | ||
| } | ||
|
|
||
| private List<MentorPreviewResponse> getMentorPreviewResponses(List<Mentor> mentors, long currentUserId) { // 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.
이 함수는 제가 구현했지만서도 이름이 참 모호한 것 같아서.. todo 표시해뒀습니다 😅
이름만 보면 응답만 생성할 것 같은데
실제로 하는 일을 보면 batch 로 fetch 하여서 이들을 조합하여 응답을 생성하니깐요..
이 PR에서 반드시 수정해야하는것은 아니라 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.
'조합'하여 응답을 만든다는 동작을 강조하고 싶으시다면, buildXXX 는 어떠신가요 ?
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.
오 좋습니다! buildMentorPreviewsWithBatchQuery() 정도면 딱 봐도 동작을 이해할 수 있을까요?
BatchQueryRepository 로부터 데이터를 조회하고 있어서 WithBatchQuery를 뒤에 붙여봤습니다 ㅎㅎ
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.
이름짓는 건 늘 어렵네요.. 뒤에 withBatch라도 붙여야할까요 😂
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.
저도 동의합니다 !
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.
반영했습니다~ 감사합니다😊
4b4f355
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.
고생하셨습니다 !!! 코멘트 남겼습니다
| return mentorRepository.findAllByRegion(region, pageable); | ||
| } | ||
|
|
||
| private List<MentorPreviewResponse> getMentorPreviewResponses(List<Mentor> mentors, long currentUserId) { // 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.
'조합'하여 응답을 만든다는 동작을 강조하고 싶으시다면, buildXXX 는 어떠신가요 ?
| } | ||
|
|
||
| public Map<Long, University> getMentorIdToUniversityMap(List<Mentor> mentors) { | ||
| List<Long> universityIds = mentors.stream().map(Mentor::getUniversityId).toList(); |
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.
여러 멘토가 동일한 대학을 나왔을 수 있으므로, distinct 를 통해 중복 제거하는 것이 메모리 측면에서 좋을 것 같습니다 !
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.
정말 너무 좋은 생각입니다~
바로 적용했습니다! 😁 02a7354
Gyuhyeok99
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.
빠른 구현 고생하셨습니다!
정신없던 적응기간이 좀 끝나서 빨리빨리 리뷰 남기겠습니다 앞으로..
영서님은 늘 커밋으로 설명을 잘해주셔서 리뷰가 빨라지는 거 같아요
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.
변경사항 확인했습니다 !!!!
- 여러 멘토가 동일한 대학을 나왔을 경우, 동일한 대학을 멘토 수만틈 가지고 있기보다는 distinct를 적용하여 하나씩만 가지고 있게 하는 것이 좋다.
4b4f355 to
ac5fba5
Compare
관련 이슈
작업 내용
사실은 두개의 PR로 올리는게 맞으나, 각각의 변경이 크지 않고 빠르게 머지해야한다는 점을 들어 하나의 PR로 올립니다🥹
특이 사항
리뷰 요구사항 (선택)