refactor: 멘토 조회 시, 파견 상태 대신 '파견 학기'를 응답하도록 #412
Conversation
- 멘터 상세 페이지, 미리보기, 멘토 마이페이지에서 변경이 일어남
- sliceResponse 로 대체됨
nayonsoso
requested review from
Gyuhyeok99,
lsy1307,
whqtker and
wibaek
as code owners
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/example/solidconnection/mentor/domain/Mentor.java (1)
47-48: 새로운 term 필드 추가가 적절합니다.
- 데이터베이스 제약조건과 일치:
length = 50, nullable = false설정이 마이그레이션 스크립트와 정확히 일치합니다.- 불변성 설계: setter 메서드가 없어 생성 후 변경이 불가능한 설계로, 파견 학기 정보의 일관성을 보장합니다.
다만 학기 형식 검증을 고려해보시는 것은 어떨까요? 예를 들어 "2025-1", "2025-2" 같은 패턴 검증을 추가하면 데이터 품질을 더욱 향상시킬 수 있을 것 같습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/com/example/solidconnection/mentor/domain/Mentor.java(1 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/dto/MentorPreviewsResponse.java(0 hunks)src/main/resources/db/migration/V26__add_term_column_to_mentor.sql(1 hunks)src/test/java/com/example/solidconnection/mentor/fixture/MentorFixtureBuilder.java(3 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/example/solidconnection/mentor/dto/MentorPreviewsResponse.java
🔇 Additional comments (10)
src/main/resources/db/migration/V26__add_term_column_to_mentor.sql (1)
1-2: 기존 mentor 레코드 확인 필요
- NOT NULL 제약조건 추가
– 기존 데이터가 있는 상태에서 컬럼을 NOT NULL로 추가하면 마이그레이션이 실패할 수 있습니다.- 기본값 미지정
– 새로 추가된 컬럼에 대한 기본값 또는 데이터 초기화 로직이 포함되어 있지 않아 기존 레코드는 NULL을 허용하지 않습니다.아래 스크립트를 실제 DB 접속 정보로 수정해 실행하시고, mentor 테이블의 레코드 수를 확인해주세요:
#!/bin/bash echo "기존 mentor 레코드 수 조회..." mysql -u <username> -p<password> <database_name> -e "SELECT COUNT(*) AS mentor_count FROM mentor;"확인 결과가 0이 아니라면, 기본값 설정 또는 별도 데이터 마이그레이션 스크립트를 추가로 작성하는 방안을 고려해 주세요.
src/test/java/com/example/solidconnection/mentor/fixture/MentorFixtureBuilder.java (3)
20-20: 기본값 설정이 현실적입니다.테스트용 기본값
"2025-1"이 현재 학기를 반영하여 적절합니다.
56-59: Builder 패턴 일관성이 잘 유지되었습니다.기존 setter 메서드들과 동일한 패턴으로 구현되어 일관성이 좋습니다.
70-70: 생성자 파라미터 순서가 올바릅니다.term 필드가 적절한 위치에 추가되어 Mentor 엔티티 생성 시 올바른 값이 전달됩니다.
src/main/java/com/example/solidconnection/mentor/dto/MentorMyPageResponse.java (2)
13-13: DTO 필드 타입 변경이 적절합니다.
ExchangeStatus exchangeStatus에서String term으로의 변경이 요구사항과 정확히 일치합니다.
27-27: 데이터 소스 변경이 올바르게 적용되었습니다.
- 기존:
siteUser.getExchangeStatus()에서 상태 정보 조회- 변경:
mentor.getTerm()에서 파견 학기 정보 직접 조회이제 멘토 엔티티에서 직접 파견 학기 정보를 관리하므로 더 명확한 데이터 구조입니다.
src/main/java/com/example/solidconnection/mentor/dto/MentorPreviewResponse.java (2)
13-13: 일관된 DTO 구조 변경입니다.다른 DTO들과 동일한 패턴으로
ExchangeStatus에서String term으로 변경되어 일관성이 잘 유지되었습니다.
28-28: 데이터 소스 통일이 완료되었습니다.
- MentorMyPageResponse와 동일한 패턴으로
mentor.getTerm()사용- 일관성: 모든 멘토 관련 DTO에서 동일한 방식으로 파견 학기 정보 조회
전체 시스템에서 파견 학기 정보의 출처가 통일되어 유지보수성이 향상되었습니다.
src/main/java/com/example/solidconnection/mentor/dto/MentorDetailResponse.java (2)
13-13: 필드 변경사항이 적절합니다.
ExchangeStatus exchangeStatus에서String term으로의 변경이 PR 목표와 일치합니다.- 파견 상태 대신 파견 학기 정보를 제공하도록 개선되었습니다.
- String 타입 선택으로 유연성을 확보했습니다.
29-29: 팩토리 메서드 업데이트가 올바릅니다.
mentor.getTerm()호출로 새로운 필드와 일관성을 유지합니다.- 이전
SiteUser에서exchangeStatus를 가져오던 방식에서 변경되었습니다.- Mentor 엔티티의 새로운
term필드를 적절히 활용하고 있습니다.
Gyuhyeok99
left a comment
Gyuhyeok99
left a comment
There was a problem hiding this comment.
Flyway 잘 작성되어있습니다!(앞으로 저도 꼼꼼히 확인하겠습니다..)
저는 이렇게 주기적으로 추가되는 데이터는 별도 테이블로 관리하는 게 더 좋다고 생각합니다.
저희는 단일 서버를 사용하고 있어서, 학기 하나 추가할 때마다 서버를 재배포하는 건 효율적이지 않은 것 같습니다..
물론 join이 한 번 더 발생하긴 하지만, 전체적인 trade-off를 고려했을 때는 확장성과 운영 편의성 측면에서 테이블 분리 쪽이 더 유리하다고 생각합니다!
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
음 저도 나중에는 테이블로 관리하는 편이 더 좋을 것 같습니다 !
|
@Gyuhyeok99 @whqtker 의견 감사드립니다😊
특히 규혁님이 말씀하신 이 부분 덕분에 저도 table로 마음이 더 기울었습니다. |
3 participants
관련 이슈
작업 내용
이걸 계속 string 으로 관리해도 되는걸까? 테이블로 분리? 아니면 enum으로?
여기에 대해 의견 주시면 감사하겠습니다 👯♀️
특이 사항
리뷰 요구사항 (선택)