fix: host_university 테이블의 korean_name 필드에 unique key 추가#645
fix: host_university 테이블의 korean_name 필드에 unique key 추가#645whqtker wants to merge 3 commits intosolid-connection:developfrom
Conversation
Walkthrough1. HostUniversity 엔티티 변경. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/main/resources/db/migration/V44__add_unique_constraint_to_host_university_korean_name.sql`:
- Around line 1-2: The migration ADD CONSTRAINT uk_host_university_korean_name
on table host_university will fail if duplicate korean_name values exist; add a
cleanup migration that finds and resolves duplicates in
host_university.korean_name (either delete/merge duplicate rows or keep the
desired row and remove others) before applying the UNIQUE constraint, and
include a pre-deployment duplicate-check SQL query (e.g., SELECT korean_name,
COUNT(*) FROM host_university GROUP BY korean_name HAVING COUNT(*)>1) plus a
brief documented procedure for manual review/merge so the ALTER TABLE in
V44__add_unique_constraint_to_host_university_korean_name.sql succeeds reliably.
src/main/resources/db/migration/V44__add_unique_constraint_to_host_university_korean_name.sql
Show resolved
Hide resolved
sukangpunch
left a comment
There was a problem hiding this comment.
고생하셨습니다!! 궁금한 부분 리뷰 남겼습니다
| import java.util.Optional; | ||
| import org.springframework.data.jpa.repository.JpaRepository; | ||
|
|
||
| public interface HostUniversityRepositoryForTest extends JpaRepository<HostUniversity, Long> { |
There was a problem hiding this comment.
테스트용 Repository를 따로 생성하신 이유가 있나요?
There was a problem hiding this comment.
아마 유니크키를 달면서 중복생성하면 예외가 터지니 존재하면 find하도록 바꾸신 거 같은데 맞나요?
There was a problem hiding this comment.
맞습니다 ! 테스트 코드에서 korean_name이 같은 객체를 생성해서 존재하면 이미 존재하는 객체를 가져오도록 했습니다.
지금 다시 생각해보니, 그냥 애초에 korean_name 이 같은 객체가 존재하지 않도록 변경하는 것이 더 좋을 거 같아 그렇게 수정했습니다.
| import java.util.Optional; | ||
| import org.springframework.data.jpa.repository.JpaRepository; | ||
|
|
||
| public interface HostUniversityRepositoryForTest extends JpaRepository<HostUniversity, Long> { |
There was a problem hiding this comment.
아마 유니크키를 달면서 중복생성하면 예외가 터지니 존재하면 find하도록 바꾸신 거 같은데 맞나요?
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/java/com/example/solidconnection/application/service/ApplicationQueryServiceTest.java (1)
240-253:⚠️ Potential issue | 🔴 Critical필터 테스트의 기대값이 검색 키워드와 맞지 않습니다.
- 검색 키워드: 240-253 범위의 테스트에서
"괌"으로 필터링합니다 (244줄).- 예상되는 결과:
괌대학_A_지원_정보만 포함되어야 합니다.
괌대학_A_지원_정보의 koreanName ="괌대학(A형)"→ "괌"을 포함합니다 ✓버지니아공과대학_지원_정보의 koreanName ="버지니아공과대학"→ "괌"을 포함하지 않습니다 ✗- 현재 문제: 248-253줄의 assertion에서 두 대학 모두를 기대하고 있어 테스트가 실패합니다.
수정 방안
// then assertThat(response.firstChoice()).containsExactlyInAnyOrder( ApplicantsResponse.of(괌대학_A_지원_정보, - List.of(application1), user1), - ApplicantsResponse.of(버지니아공과대학_지원_정보, - List.of(application2), user1) + List.of(application1), user1) );
b9858c8 to
dd0ba99
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/java/com/example/solidconnection/application/service/ApplicationQueryServiceTest.java (1)
240-253:⚠️ Potential issue | 🟡 Minor테스트 이름과 실제 검색 값이 불일치합니다.
테스트 메서드명은
이번_학기_지원자를_대학_국문_이름으로_필터링해서_조회한다인데, 검색 키워드가"미국"(국가명)으로 변경되었습니다. 이전에는 아마"괌"같은 대학 이름 일부로 검색했을 것으로 보이는데, 버지니아공과대학으로 픽스처가 변경되면서 두 대학을 동시에 매칭할 수 있는"미국"으로 바뀐 것 같습니다.테스트의 의도를 정확히 전달하려면 두 가지 중 하나를 고려해주세요:
- 검색 키워드를 대학 이름 일부로 변경 — 예:
"대학"으로 검색하면 괌대학, 버지니아공과대학 모두 매칭- 테스트 이름을 변경 — 국가명으로 필터링하는 것을 반영하도록 수정
관련 이슈
작업 내용
같은 값을 가지는
korean_name은 사실상 없기에 UK를 설정합니다.기존
homeUniversityRepository를 사용한 테스트 코드를 올바르게 동작하도록 수정했습니다. 이전에는 중복 이름이 생성 가능했으나, 해당 변경으로 인해 불가능해졌고, test용 repository를 생성하고 이를 사용하도록 변경했습니다.UK 추가로 인해 깨진 테스트 정합성 문제는 각 테스트가 다른
host_university객체를 사용하게끔 수정했습니다.특이 사항
리뷰 요구사항 (선택)