-
Notifications
You must be signed in to change notification settings - Fork 8
feat: 파견 대학 테이블명 변경 및 협정 대학 테이블 추가하는 DDL 작성 #620
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: 파견 대학 테이블명 변경 및 협정 대학 테이블 추가하는 DDL 작성 #620
Conversation
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. 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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/main/resources/db/migration/V42__rename_university_to_host_university_and_create_home_university.sql`:
- Around line 21-25: The migration currently adds the foreign key constraint
fk_host_university_home_university on host_university(home_university_id) before
creating the explicit index fk_host_university_home_university_idx, which lets
InnoDB create its own index and causes a duplicate; reorder the statements so
the CREATE INDEX for fk_host_university_home_university_idx on
host_university(home_university_id) runs first, then ADD CONSTRAINT
fk_host_university_home_university to reference home_university(id), ensuring
InnoDB reuses the existing index and avoids duplicate indexes.
🧹 Nitpick comments (1)
src/main/resources/db/migration/V42__rename_university_to_host_university_and_create_home_university.sql (1)
10-16: 마이그레이션 컨벤션과 맞추기 위해IF NOT EXISTS제거를 권장합니다.최근 마이그레이션 파일들(V35, V40 등)을 살펴보면 다음과 같은 패턴이 보입니다:
프로젝트의 진화된 관례
- 초기 마이그레이션(V1):
CREATE TABLE IF NOT EXISTS사용- 최근 마이그레이션(V35~):
CREATE TABLE만 사용안전성 관점
IF NOT EXISTS는 스키마 드리프트를 숨겨 예상치 못한 상태를 감지하기 어렵게 만듭니다- 마이그레이션은 데이터베이스 상태를 명확하고 확정적으로 관리해야 합니다
따라서 이 파일도 프로젝트의 최신 컨벤션에 맞추어
IF NOT EXISTS를 제거하시길 권장합니다.🛠️ 수정 제안
-CREATE TABLE IF NOT EXISTS home_university +CREATE TABLE home_university
...ources/db/migration/V42__rename_university_to_host_university_and_create_home_university.sql
Outdated
Show resolved
Hide resolved
- 또한 목데이터 생성 로직 수정
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java (1)
29-32: 2. 주석 내용이 코드 변경사항과 맞지 않습니다
- 주석에서
대학교(University)라고 명시되어 있지만, 실제 코드는 이제HostUniversity를 사용하고 있어요.- 주석도 함께 업데이트하면 코드와 문서의 일관성이 유지됩니다.
📝 주석 수정 제안
/* * 대학교 상세 정보를 불러온다. - * - 대학교(University) 정보와 대학 지원 정보(UniversityInfoForApply) 정보를 조합하여 반환한다. + * - 파견 대학교(HostUniversity) 정보와 대학 지원 정보(UniversityInfoForApply) 정보를 조합하여 반환한다. * */
🤖 Fix all issues with AI agents
In `@src/main/resources/data.sql`:
- Around line 72-87: The host_university rows for
'university_of_southern_queensland', 'university_of_sydney', and
'curtin_university' have region_code 'AMERICAS' but the country table defines
'AU' (Australia) as 'ASIA'; update the region_code values in the host_university
INSERTs to 'ASIA' to match the country table (and run a quick check that other
host_university rows use the country->region mapping consistently), referencing
the host_university entries by their slug values to locate the exact tuples in
the data.sql.
🧹 Nitpick comments (2)
src/main/java/com/example/solidconnection/university/domain/HomeUniversity.java (1)
14-25: 명시적 테이블명 지정으로 마이그레이션 정합성을 보강해주세요.현재 프로젝트에서는 명시적 naming strategy 설정이 없어 Hibernate 기본 동작에만 의존하고 있습니다. 클래스명의 자동 변환에 의존하기보다 테이블명을 명시적으로 지정하면, 향후 naming 전략을 변경하더라도 데이터베이스 스키마와의 일관성을 안전하게 유지할 수 있습니다.
♻️ 제안 변경
import jakarta.persistence.Id; +import jakarta.persistence.Table; @@ `@Entity` +@Table(name = "home_university") `@AllArgsConstructor`src/main/java/com/example/solidconnection/mentor/service/MentorQueryService.java (1)
49-90: 네이밍을hostUniversity로 맞추면 가독성이 더 좋아집니다.
home/host 공존 맥락이라서 로컬 변수와 맵 이름을 명확히 해두는 편이 안전합니다.
getMentorDetails의university→hostUniversitybuildMentorPreviewsWithBatchQuery의 맵/로컬 변수명 정리🔧 제안 수정안
- HostUniversity university = hostUniversityRepository.findById(mentor.getUniversityId()) + HostUniversity hostUniversity = hostUniversityRepository.findById(mentor.getUniversityId()) .orElseThrow(() -> new CustomException(UNIVERSITY_NOT_FOUND)); ... - return MentorDetailResponse.of(mentor, mentorUser, university, isApplied, term.getName()); + return MentorDetailResponse.of(mentor, mentorUser, hostUniversity, isApplied, term.getName()); ... - Map<Long, HostUniversity> mentorIdToUniversity = mentorBatchQueryRepository.getMentorIdToUniversityMap(mentors); + Map<Long, HostUniversity> mentorIdToHostUniversity = mentorBatchQueryRepository.getMentorIdToUniversityMap(mentors); ... - HostUniversity university = mentorIdToUniversity.get(mentor.getId()); + HostUniversity hostUniversity = mentorIdToHostUniversity.get(mentor.getId()); ... - MentorPreviewResponse response = MentorPreviewResponse.of(mentor, mentorUser, university, isApplied, termName); + MentorPreviewResponse response = MentorPreviewResponse.of(mentor, mentorUser, hostUniversity, isApplied, termName);
| (4, 1, 'AU', 'AMERICAS', 'University of Southern Queensland', 'university_of_southern_queensland', '서던퀸스랜드대학', | ||
| 'https://www.unisq.edu.au/current-students/support/accommodation', | ||
| 'https://www.unisq.edu.au/course/specification/current/', | ||
| 'https://www.unisq.edu.au/international/partnerships/study-abroad-exchange', NULL, | ||
| 'https://solid-connection.s3.ap-northeast-2.amazonaws.com/original/university_of_southern_queensland/logo.png', | ||
| 'https://solid-connection.s3.ap-northeast-2.amazonaws.com/original/university_of_southern_queensland/1.png'), | ||
| (5, 'AU', 'AMERICAS', 'University of Sydney', 'university_of_sydney', '시드니대학', | ||
| (5, 1, 'AU', 'AMERICAS', 'University of Sydney', 'university_of_sydney', '시드니대학', | ||
| 'https://www.sydney.edu.au/study/accommodation.html', 'www.sydney.edu.au/sydney-abroad-units', | ||
| 'https://www.sydney.edu.au/', NULL, | ||
| 'https://solid-connection.s3.ap-northeast-2.amazonaws.com/original/university_of_sydney/logo.png', | ||
| 'https://solid-connection.s3.ap-northeast-2.amazonaws.com/original/university_of_sydney/1.png'), | ||
| (6, 'AU', 'AMERICAS', 'Curtin University', 'curtin_university', '커틴대학', | ||
| (6, 1, 'AU', 'AMERICAS', 'Curtin University', 'curtin_university', '커틴대학', | ||
| 'https://www.curtin.edu.au/study/campus-life/accommodation/#perth', 'https://handbook.curtin.edu.au/', | ||
| 'https://www.curtin.edu.au/', NULL, | ||
| 'https://solid-connection.s3.ap-northeast-2.amazonaws.com/original/curtin_university/logo.png', | ||
| 'https://solid-connection.s3.ap-northeast-2.amazonaws.com/original/curtin_university/1.png'), |
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.
호주 대학들의 region_code 불일치 문제가 있습니다.
country 테이블(Line 17)에서 호주('AU')는 'ASIA' 지역으로 정의되어 있습니다:
('AU', '호주', 'ASIA'),하지만 host_university 테이블에서 호주 대학들은 'AMERICAS' 지역으로 설정되어 있습니다:
| 대학명 | country_code | region_code (현재) | region_code (예상) |
|---|---|---|---|
| 서던퀸스랜드대학 | AU | AMERICAS | ASIA |
| 시드니대학 | AU | AMERICAS | ASIA |
| 커틴대학 | AU | AMERICAS | ASIA |
이로 인해 지역별 대학 조회 시 데이터 불일치가 발생할 수 있습니다.
🔧 수정 제안
- (4, 1, 'AU', 'AMERICAS', 'University of Southern Queensland', ...
+ (4, 1, 'AU', 'ASIA', 'University of Southern Queensland', ...
- (5, 1, 'AU', 'AMERICAS', 'University of Sydney', ...
+ (5, 1, 'AU', 'ASIA', 'University of Sydney', ...
- (6, 1, 'AU', 'AMERICAS', 'Curtin University', ...
+ (6, 1, 'AU', 'ASIA', 'Curtin University', ...🤖 Prompt for AI Agents
In `@src/main/resources/data.sql` around lines 72 - 87, The host_university rows
for 'university_of_southern_queensland', 'university_of_sydney', and
'curtin_university' have region_code 'AMERICAS' but the country table defines
'AU' (Australia) as 'ASIA'; update the region_code values in the host_university
INSERTs to 'ASIA' to match the country table (and run a quick check that other
host_university rows use the country->region mapping consistently), referencing
the host_university entries by their slug values to locate the exact tuples in
the data.sql.
- FK 제약조건을 추가할 때 인덱스가 없다면 알아서 추가하기 때문
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
🤖 Fix all issues with AI agents
In
`@src/main/resources/db/migration/V42__rename_university_to_host_university_and_create_home_university.sql`:
- Around line 10-16: The migration uses "CREATE TABLE IF NOT EXISTS
home_university" which can silently skip creating the table when a conflicting
table exists in another schema; remove the "IF NOT EXISTS" clause so the CREATE
TABLE for home_university fails loudly on conflict, allowing Flyway to surface
and fail the migration; apply the same removal to any other CREATE TABLE
statements in this migration to ensure missing/duplicate schema issues are not
silently ignored.
| CREATE TABLE IF NOT EXISTS home_university | ||
| ( | ||
| id BIGINT AUTO_INCREMENT NOT NULL, | ||
| name VARCHAR(100) NOT NULL, | ||
| CONSTRAINT `PRIMARY` PRIMARY KEY (id), | ||
| CONSTRAINT uk_home_university_name UNIQUE (name) | ||
| ); |
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.
IF NOT EXISTS는 마이그레이션 누락을 조용히 숨길 수 있어요.
기존에 다른 스키마로 테이블이 존재하면 이 단계가 “조용히 통과”해 후속 오류로 번질 수 있으니, Flyway에서는 제거하는 편이 안전합니다.
🛠️ 수정 제안
-CREATE TABLE IF NOT EXISTS home_university
+CREATE TABLE home_university
(
id BIGINT AUTO_INCREMENT NOT NULL,
name VARCHAR(100) NOT NULL,
CONSTRAINT `PRIMARY` PRIMARY KEY (id),
CONSTRAINT uk_home_university_name UNIQUE (name)
);🤖 Prompt for AI Agents
In
`@src/main/resources/db/migration/V42__rename_university_to_host_university_and_create_home_university.sql`
around lines 10 - 16, The migration uses "CREATE TABLE IF NOT EXISTS
home_university" which can silently skip creating the table when a conflicting
table exists in another schema; remove the "IF NOT EXISTS" clause so the CREATE
TABLE for home_university fails loudly on conflict, allowing Flyway to surface
and fail the migration; apply the same removal to any other CREATE TABLE
statements in this migration to ensure missing/duplicate schema issues are not
silently ignored.
관련 이슈
작업 내용
'대학'에 속하는 개념이 추가되었으므로, 기존 파견(수학) 학교 테이블명을
university에서host_university로 변경하였습니다. 협정 학교(수학 학교로 가는 학생의 출신 학교) 테이블명은home_university로 명명했습니다.home_university의name필드에 UK 설정했고, 두 테이블을 FK로 연관관계 설정했습니다.엔티티에서는
Region과Country연관 관계가 설정되어 있어서HomeUniversity와도 양뱡향 연관관계로 구현했습니다. 추후 불필요한 조회 쿼리가 발생한다면 간접 참조하도록 변경하겠습니다.host_university,home_university에 대한 엔티티를 작성했고, 목데이터 생성 로직을 수정 및home_university관련 데이터를 추가했습니다.특이 사항
리뷰 요구사항 (선택)