refactor: verify_status의 타입을 통일한다. #472
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Column 어노테이션에 name 옵션을 사용하지 않으면 인덱스 생성 시 인식하지 못해 에러가 발생한다.
whqtker
requested review from
Gyuhyeok99,
lsy1307,
nayonsoso and
wibaek
as code owners
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
whqtker
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
src/main/java/com/example/solidconnection/score/domain/GpaScore.java (3)
34-36: @column(name 지정)으로 네이밍 전략 의존도를 낮춰 주세요.
- 현재는 @column(nullable = false)로만 지정되어 있어 물리 이름 매핑이 네이밍 전략에 의존합니다.
- 마이그레이션 스크립트는 컬럼명을 verify_status로 고정하므로, 엔티티도 명시적으로 name="verify_status"를 지정하면 안전합니다.
- 기본값 초기화(= VerifyStatus.PENDING)와 @Enumerated(EnumType.STRING)은 방향성이 올바릅니다.
다음과 같이 보완을 제안드립니다.
- @Column(nullable = false) + @Column(name = "verify_status", nullable = false) @Enumerated(EnumType.STRING) private VerifyStatus verifyStatus = VerifyStatus.PENDING;
42-47: 중복 초기화 제거로 코드 간결화가 가능합니다.
- 필드에서 이미 verifyStatus를 PENDING으로 초기화하고 있습니다.
- 생성자에서 동일 값을 한 번 더 세팅하는 부분은 제거해도 동작에 영향이 없습니다.
- 가독성과 유지보수성을 위해 중복을 줄여 주세요.
아래와 같이 한 줄을 제거해도 됩니다.
public GpaScore(Gpa gpa, SiteUser siteUser) { this.gpa = gpa; this.siteUserId = siteUser.getId(); - this.verifyStatus = VerifyStatus.PENDING; this.rejectedReason = null; }
33-36: verifyStatus Setter 제거 및 도메인 메서드 전환 제안현재 GpaScore.verifyStatus의 Lombok @Setter는 오직 테스트 픽스처에서만(setVerifyStatus) 호출되고 있어, 프로덕션 코드에선 사용되지 않습니다. 도메인 무결성을 강화하기 위해 아래와 같이 리팩토링을 권장드립니다:
Lombok @Setter 제거
GpaScore 클래스에서@Setter애노테이션을 삭제해 외부 변경을 원천 차단합니다.도메인 메서드(updateVerifyStatus) 도입/활용
verifyStatus를 안전하게 전이할 수 있는updateVerifyStatus(VerifyStatus newStatus)메서드를 GpaScore에 추가하고, 상태 전이 규칙을 내부에서 검증하도록 구현합니다.테스트 픽스처 수정
LanguageTestScoreFixtureBuilder, GpaScoreFixtureBuilder 등에서 직접setVerifyStatus(...)호출을 제거하고, 대신 위 도메인 메서드를 통해 상태를 설정하도록 코드를 변경합니다.src/main/java/com/example/solidconnection/score/domain/LanguageTestScore.java (3)
34-36: @column(name 지정)으로 스키마와의 결합을 명시적으로 맞춰 주세요.
- 마이그레이션은 verify_status 컬럼을 생성/변경합니다.
- 엔티티는 네이밍 전략에 기대지 말고 name="verify_status"로 명시하는 편이 안전합니다.
- 기본값 초기화와 EnumType.STRING 매핑은 적절합니다.
다음과 같이 보완을 제안드립니다.
- @Column(nullable = false) + @Column(name = "verify_status", nullable = false) @Enumerated(EnumType.STRING) private VerifyStatus verifyStatus = VerifyStatus.PENDING;
42-46: 중복 초기화 제거로 단순화할 수 있습니다.
- 필드 기본값으로 이미 PENDING을 세팅했으므로 생성자 내 동일 대입은 중복입니다.
- 한 곳에서만 초기화해 유지보수 비용을 줄여 주세요.
- 동작에는 변화가 없습니다.
다음과 같이 정리 가능합니다.
public LanguageTestScore(LanguageTest languageTest, SiteUser siteUser) { this.languageTest = languageTest; - this.verifyStatus = VerifyStatus.PENDING; this.siteUserId = siteUser.getId(); }
23-24: LanguageTestScore: @AllArgsConstructor 제거 및 제약 추가 검토 요청다음 사항을 검토해 주세요.
우회 경로 위험
- @AllArgsConstructor가 모든 필드를 받는 생성자를 생성하여 verifyStatus에 null 또는 임의 값을 주입할 수 있습니다.
권장 조치
- @AllArgsConstructor 제거를 고려하세요.
- verifyStatus 필드에 @nonnull 어노테이션 추가를 검토할 수 있습니다.
- 전용 팩토리 메서드나 빌더에서 기본값을 강제하는 방식을 대안으로 사용해 보세요.
마이그레이션 가이드
- 서비스(src/main/java/.../ScoreService.java)와 테스트(src/test/java/.../LanguageTestScoreFixtureBuilder.java)에서 명시적 all-args 생성자 호출은 없습니다.
- 제거 후에도 existing two-args 생성자 및 no-args 생성자가 정상 동작하는지 확인해주세요.
src/main/java/com/example/solidconnection/application/domain/Application.java (1)
53-53: 스타일 일관성: static import(PENDING)를 필드 초기화에도 활용해 주세요.
- 파일 상단에 static import가 있고, 생성자에서는 PENDING을 사용 중입니다.
- 필드 초기화에서도 동일 스타일을 쓰면 가독성이 좋아집니다.
- 기능 변화는 없습니다.
다음과 같이 한 줄만 수정하면 됩니다.
- private VerifyStatus verifyStatus = VerifyStatus.PENDING; + private VerifyStatus verifyStatus = PENDING;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/main/java/com/example/solidconnection/application/domain/Application.java(1 hunks)src/main/java/com/example/solidconnection/score/domain/GpaScore.java(1 hunks)src/main/java/com/example/solidconnection/score/domain/LanguageTestScore.java(1 hunks)src/main/resources/db/migration/V30__modify_verify_status_from_varchar_to_enum.sql(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 (2)
src/main/resources/db/migration/V30__modify_verify_status_from_varchar_to_enum.sql (1)
1-8: 운영 락/다운타임 영향 검토를 부탁드립니다.
- ENUM으로의 MODIFY COLUMN은 MySQL/InnoDB에서 테이블 리빌드가 발생할 수 있어 잠금 시간이 길어질 수 있습니다.
- 대용량 테이블이라면 배포 윈도우, 배치 분할, 혹은 온라인 스키마 변경 도구(예: pt-online-schema-change) 검토를 권장합니다.
- 사전 리허설로 소요 시간과 락 범위를 확인해 주세요.
src/main/java/com/example/solidconnection/application/domain/Application.java (1)
51-53: LGTM: 컬럼 null 금지 + 자바 측 기본값(PENDING) 초기화가 일관성을 확보합니다.
- @column(nullable = false)와 EnumType.STRING 매핑이 DB ENUM('PENDING','REJECTED','APPROVED')와 잘 호응합니다.
- 필드 기본값을 통해 DB 기본값 의존을 줄이면서도, DB 스키마의 DEFAULT와도 일치해 이중 안전장치가 됩니다.
- 인덱스(columnList = "verify_status, ...")와도 네이밍이 맞습니다.
Comment on lines
+1
to
+8
| ALTER TABLE application | ||
| MODIFY COLUMN verify_status ENUM('PENDING', 'REJECTED', 'APPROVED') NOT NULL DEFAULT 'PENDING'; | ||
|
|
||
| ALTER TABLE gpa_score | ||
| MODIFY COLUMN verify_status ENUM('PENDING', 'REJECTED', 'APPROVED') NOT NULL DEFAULT 'PENDING'; | ||
|
|
||
| ALTER TABLE language_test_score | ||
| MODIFY COLUMN verify_status ENUM('PENDING', 'REJECTED', 'APPROVED') NOT NULL DEFAULT 'PENDING'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
데이터 정합성 선처리 없이 ENUM으로 직접 변경하면 장애 위험이 큽니다. 사전 UPDATE 절을 추가해 주세요.
-
- 기존 데이터에 null 혹은 허용 집합 외 값이 있으면 ALTER TABLE 시 실패하거나 빈 문자열로 강제 변환될 수 있습니다.
-
- 마이그레이션 전에 null/비허용 값은 'PENDING'으로, 소문자는 대문자로 정규화하는 절차를 넣어 주세요.
-
- 아래 패치를 적용하면 수월하게 타입 전환이 가능합니다.
다음 diff로 선처리 쿼리를 추가해 주세요.
+-- 1) 선처리: 소문자 → 대문자 정규화
+UPDATE application SET verify_status = UPPER(verify_status) WHERE verify_status IN ('pending','rejected','approved');
+UPDATE gpa_score SET verify_status = UPPER(verify_status) WHERE verify_status IN ('pending','rejected','approved');
+UPDATE language_test_score SET verify_status = UPPER(verify_status) WHERE verify_status IN ('pending','rejected','approved');
+
+-- 2) 선처리: null 및 허용 집합 외 값 보정
+UPDATE application SET verify_status = 'PENDING' WHERE verify_status IS NULL OR verify_status NOT IN ('PENDING','REJECTED','APPROVED');
+UPDATE gpa_score SET verify_status = 'PENDING' WHERE verify_status IS NULL OR verify_status NOT IN ('PENDING','REJECTED','APPROVED');
+UPDATE language_test_score SET verify_status = 'PENDING' WHERE verify_status IS NULL OR verify_status NOT IN ('PENDING','REJECTED','APPROVED');
+
ALTER TABLE application
MODIFY COLUMN verify_status ENUM('PENDING', 'REJECTED', 'APPROVED') NOT NULL DEFAULT 'PENDING';
ALTER TABLE gpa_score
MODIFY COLUMN verify_status ENUM('PENDING', 'REJECTED', 'APPROVED') NOT NULL DEFAULT 'PENDING';
ALTER TABLE language_test_score
MODIFY COLUMN verify_status ENUM('PENDING', 'REJECTED', 'APPROVED') NOT NULL DEFAULT 'PENDING';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ALTER TABLE application | |
| MODIFY COLUMN verify_status ENUM('PENDING', 'REJECTED', 'APPROVED') NOT NULL DEFAULT 'PENDING'; | |
| ALTER TABLE gpa_score | |
| MODIFY COLUMN verify_status ENUM('PENDING', 'REJECTED', 'APPROVED') NOT NULL DEFAULT 'PENDING'; | |
| ALTER TABLE language_test_score | |
| MODIFY COLUMN verify_status ENUM('PENDING', 'REJECTED', 'APPROVED') NOT NULL DEFAULT 'PENDING'; | |
| -- 1) 선처리: 소문자 → 대문자 정규화 | |
| UPDATE application SET verify_status = UPPER(verify_status) WHERE verify_status IN ('pending','rejected','approved'); | |
| UPDATE gpa_score SET verify_status = UPPER(verify_status) WHERE verify_status IN ('pending','rejected','approved'); | |
| UPDATE language_test_score SET verify_status = UPPER(verify_status) WHERE verify_status IN ('pending','rejected','approved'); | |
| -- 2) 선처리: null 및 허용 집합 외 값 보정 | |
| UPDATE application SET verify_status = 'PENDING' WHERE verify_status IS NULL OR verify_status NOT IN ('PENDING','REJECTED','APPROVED'); | |
| UPDATE gpa_score SET verify_status = 'PENDING' WHERE verify_status IS NULL OR verify_status NOT IN ('PENDING','REJECTED','APPROVED'); | |
| UPDATE language_test_score SET verify_status = 'PENDING' WHERE verify_status IS NULL OR verify_status NOT IN ('PENDING','REJECTED','APPROVED'); | |
| ALTER TABLE application | |
| MODIFY COLUMN verify_status ENUM('PENDING', 'REJECTED', 'APPROVED') NOT NULL DEFAULT 'PENDING'; | |
| ALTER TABLE gpa_score | |
| MODIFY COLUMN verify_status ENUM('PENDING', 'REJECTED', 'APPROVED') NOT NULL DEFAULT 'PENDING'; | |
| ALTER TABLE language_test_score | |
| MODIFY COLUMN verify_status ENUM('PENDING', 'REJECTED', 'APPROVED') NOT NULL DEFAULT 'PENDING'; |
Collaborator
There was a problem hiding this comment.
prod와 stage에서 모두 아래 쿼리 실행 시 0으로 나오므로,
이 코더레빗 리뷰는 반영하지 않아도 될 것 같습니다!
SELECT COUNT(*) AS cnt
FROM language_test_score
WHERE verify_status NOT IN ('PENDING', 'REJECTED', 'APPROVED');
SELECT COUNT(*) AS cnt
FROM gpa_score
WHERE verify_status NOT IN ('PENDING', 'REJECTED', 'APPROVED');
SELECT COUNT(*) AS cnt
FROM application
WHERE verify_status NOT IN ('PENDING', 'REJECTED', 'APPROVED');
There was a problem hiding this comment.
@nayonsoso 정말 좋은 검증 작업입니다! 🎉
1. 데이터 상태 확인 완료
- prod와 stage 환경에서 모든 테이블의 verify_status 값이 이미 올바른 상태임을 확인하셨네요
- 허용되지 않는 값이 0건이라는 것은 안전하게 ENUM 변경이 가능하다는 의미입니다
2. 마이그레이션 진행 가능
- 기존 데이터가 모두 ('PENDING', 'REJECTED', 'APPROVED') 범위 내에 있으므로
- 제가 제안한 전처리 UPDATE 쿼리들은 불필요합니다
- 원본 ALTER TABLE 구문만으로 충분히 안전하게 변경 가능합니다
3. 검증 방식이 모범적
- 스키마 변경 전 실제 데이터 상태를 확인하는 접근이 매우 바람직합니다
- 이런 식으로 사전 검증하시면 마이그레이션 리스크를 크게 줄일 수 있어요
원본 마이그레이션 파일 그대로 진행하셔도 될 것 같습니다! 👍
nayonsoso
approved these changes
Aug 23, 2025
Collaborator
nayonsoso
left a comment
nayonsoso
left a comment
There was a problem hiding this comment.
호호 제가 하겠다고 하고 안했네요 🐒
반영해주셔서 감사합니다!
Gyuhyeok99
approved these changes
Aug 24, 2025
lsy1307
approved these changes
Aug 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
관련 이슈
작업 내용
현재 ENUM 으로 선언된 VerifyState 컬럼 관련하여 JPA 엔티티 명세와 DB 스키마 간 불일치가 발생합니다.
application, gpa_score, language_test_score 테이블에서 varchar 로 설정되어 있습니다.
특이 사항
Mentoring 제외 나머지 세 엔티티들은 verifyStatus에 Setter 어노테이션이 붙어있습니다. 그러나 정작 setter 메서드로 값을 변경하는 도메인은 Application 뿐이고, 나머지 score 관련 엔티티들은 updateXXX 메서드를 통해 verifyStatus 를 변경하고 있습니다. 필요 시 이 PR에서 불필요한 Setter는 제거하겠습니다.
리뷰 요구사항 (선택)
ddl-autovalidate로 확인하긴 했는데, 잘못된 점 있다면 말씀해주세요 !