-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: Member 클래스 gender 필드 Byte -> Enum 변경 #212
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
Conversation
WalkthroughMember 도메인의 gender 표현을 Byte에서 Changes
Sequence Diagram(s)(해당 변경은 데이터 타입·마이그레이션 중심이며 신규 다중 컴포넌트 제어 흐름을 도입하지 않아 시퀀스 다이어그램을 생략합니다.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (2)
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: 2
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/medicare_call/service/auth/AuthServiceTest.java (1)
50-57: Mock 데이터에서 성별이 일치하지 않아요.
AuthService.register()구현을 보니 요청의 성별(req.getGender())을 그대로 Member에 저장하는데(line 37), 테스트의 mock된 savedMember는 요청과 다른 성별(FEMALE)로 설정되어 있습니다.Line 57의
gender(Gender.FEMALE)를gender(Gender.MALE)로 수정해야 mock 데이터가 실제 동작과 일치해요.
🤖 Fix all issues with AI agents
In @src/main/java/com/example/medicare_call/domain/Member.java:
- Around line 42-44: The Member entity uses @Enumerated(EnumType.STRING) on the
gender field, but the V31 migration creates a MySQL ENUM('MALE','FEMALE') column
which conflicts with JPA's STRING mapping; update the V31 migration to create a
VARCHAR(50) (or appropriate length) column for gender instead of ENUM and keep
the same data conversion steps so existing values are preserved, ensuring the
Member.gender field (annotated with @Enumerated(EnumType.STRING)) maps correctly
to the new VARCHAR column.
In @src/main/java/com/example/medicare_call/dto/MemberInfoUpdateRequest.java:
- Line 54: The inline comment on the MemberInfoUpdateRequest constructor
parameter is incorrect: it says "Gender enum을 byte로 변환" but the code passes the
Gender enum value directly. Either remove or update the comment to accurately
describe the behavior (e.g., "Gender enum 전달"), or if you intended to convert to
a byte, implement the conversion where the field is set (reference the gender
parameter and any getter/setter for gender in MemberInfoUpdateRequest) so the
comment matches the code; ensure the comment text and the actual handling of the
gender field are consistent.
🧹 Nitpick comments (3)
src/main/java/com/example/medicare_call/util/TestDataGenerator.java (1)
49-49: 향후 일관성 개선 제안: Elder 엔티티의 gender 필드도 enum으로 전환 고려현재 Line 49에서 Elder 엔티티는 여전히
Gender.MALE.getCode()를 사용하여 byte 값을 전달하고 있습니다. 이는 Elder 엔티티가 아직 byte 타입을 사용하기 때문에 현재로서는 정상적인 코드입니다.다만, Member 엔티티와의 일관성을 위해 향후 Elder 엔티티의 gender 필드도 Gender enum으로 전환하는 것을 고려해보시면 좋을 것 같습니다.
src/test/java/com/example/medicare_call/service/payment/NaverPayServiceTest.java (1)
10-10: 명시적 import 사용을 권장합니다.와일드카드 import (
.*)보다는 사용하는 enum들을 명시적으로 import하는 것이 코드 가독성과 유지보수성 측면에서 더 좋습니다.♻️ 명시적 import로 변경
-import com.example.medicare_call.global.enums.*; +import com.example.medicare_call.global.enums.Gender; +import com.example.medicare_call.global.enums.OrderStatus; +import com.example.medicare_call.global.enums.PaymentMethod; +import com.example.medicare_call.global.enums.SubscriptionPlan; +import com.example.medicare_call.global.enums.SubscriptionStatus;src/test/java/com/example/medicare_call/Integration/CareCallIntegrationTest.java (1)
67-67: Elder 엔티티의 gender 필드도 Gender enum으로 refactoring하면 좋을 것 같습니다.Member는 Gender enum을 사용하도록 개선되었는데, Elder는 여전히 Byte 타입을 사용하고 있네요. 코드 일관성을 위해 Elder도 동일하게 Gender enum으로 refactoring할 것을 제안합니다. (별도로 진행 중이라면 무시해도 됩니다.)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/main/java/com/example/medicare_call/domain/Member.javasrc/main/java/com/example/medicare_call/dto/MemberInfoResponse.javasrc/main/java/com/example/medicare_call/dto/MemberInfoUpdateRequest.javasrc/main/java/com/example/medicare_call/global/enums/Gender.javasrc/main/java/com/example/medicare_call/service/auth/AuthService.javasrc/main/java/com/example/medicare_call/util/TestDataGenerator.javasrc/main/resources/db/migration/V31__member_gender_byte_to_enum.sqlsrc/test/java/com/example/medicare_call/Integration/CareCallIntegrationTest.javasrc/test/java/com/example/medicare_call/controller/CareCallControllerImmediateTest.javasrc/test/java/com/example/medicare_call/controller/ElderControllerTest.javasrc/test/java/com/example/medicare_call/repository/NotificationRepositoryTest.javasrc/test/java/com/example/medicare_call/service/ElderServiceTest.javasrc/test/java/com/example/medicare_call/service/auth/AuthServiceTest.javasrc/test/java/com/example/medicare_call/service/payment/NaverPayServiceTest.java
💤 Files with no reviewable changes (1)
- src/main/java/com/example/medicare_call/global/enums/Gender.java
⏰ 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 (14)
src/test/java/com/example/medicare_call/service/ElderServiceTest.java (1)
61-61: LGTM! Gender enum으로 깔끔하게 전환되었습니다.테스트 데이터에서 Member 생성 시
Gender.MALEenum을 직접 사용하도록 변경되어 타입 안정성이 향상되었습니다.src/main/java/com/example/medicare_call/dto/MemberInfoResponse.java (1)
55-55: LGTM! DTO 응답 매핑이 올바르게 업데이트되었습니다.
member.getGender()를 직접 호출하여 Gender enum을 반환하도록 변경되었습니다. Member 도메인의 gender 필드가 이제 Gender 타입이므로 별도의 변환 없이 직접 사용 가능합니다.src/test/java/com/example/medicare_call/controller/ElderControllerTest.java (1)
92-92: LGTM! 컨트롤러 테스트가 Gender enum으로 잘 업데이트되었습니다.테스트 Member 생성 시
Gender.FEMALEenum을 사용하여 타입 안정성이 개선되었습니다.src/test/java/com/example/medicare_call/repository/NotificationRepositoryTest.java (1)
5-5: LGTM! Repository 테스트가 Gender enum으로 정상적으로 전환되었습니다.Gender enum import를 추가하고 테스트 데이터 생성 시
Gender.FEMALE을 사용하도록 업데이트되었습니다.Also applies to: 35-35
src/main/java/com/example/medicare_call/util/TestDataGenerator.java (1)
35-35: LGTM! Member 테스트 데이터 생성이 Gender enum으로 올바르게 업데이트되었습니다.
Gender.MALEenum을 직접 사용하여 Member 엔티티 생성 로직이 리팩터링된 구조에 맞춰 변경되었습니다.src/main/resources/db/migration/V31__member_gender_byte_to_enum.sql (2)
13-16: 변환 검증 로직이 정상적으로 동작합니다.SELECT COUNT는 실제로 마이그레이션을 중단시키지 않지만, Line 21의 NOT NULL 제약 조건이 무효한 데이터가 있을 경우 마이그레이션을 실패시키므로 안전합니다. 이 SELECT문은 마이그레이션 실패 시 디버깅에 유용합니다.
6-11: 성별 매핑이 enum 정의와 일치합니다.Gender enum에서 MALE=(byte)0, FEMALE=(byte)1로 정의되어 있고, 마이그레이션의 CASE문도 정확하게 일치하게 매핑하고 있습니다. 매핑 방향 문제없습니다.
src/main/java/com/example/medicare_call/service/auth/AuthService.java (1)
37-37: 변경사항이 올바르게 적용되었습니다.
req.getGender()를 직접 전달하도록 수정하여.getCode()호출이 제거되었습니다.Member빌더가 이제Genderenum을 직접 받으므로 더 깔끔한 API가 되었습니다.src/test/java/com/example/medicare_call/service/payment/NaverPayServiceTest.java (1)
100-100: 테스트 데이터가 올바르게 업데이트되었습니다.
Gender.FEMALEenum 값을 사용하도록 변경되어 타입 안정성이 향상되었고, 테스트 코드의 가독성도 개선되었습니다.Also applies to: 530-530, 585-585
src/test/java/com/example/medicare_call/Integration/CareCallIntegrationTest.java (1)
57-57: Member 엔티티 변경이 올바르게 적용되었습니다.
Gender.FEMALEenum을 사용하도록 변경되어 리팩토링 목표와 일치합니다.src/test/java/com/example/medicare_call/controller/CareCallControllerImmediateTest.java (1)
4-4: 테스트 코드가 새로운 Gender enum과 잘 맞춰졌네요.import 추가와
.gender(Gender.FEMALE)사용이 Member 도메인의 리팩터링과 정확히 일치합니다.Also applies to: 86-86
src/test/java/com/example/medicare_call/service/auth/AuthServiceTest.java (1)
106-106: 올바른 enum 사용입니다.
Gender.MALE사용이 적절하고 테스트 컨텍스트와 일치합니다.src/main/java/com/example/medicare_call/domain/Member.java (2)
73-82: 생성자 시그니처가 올바르게 업데이트되었습니다.Lombok
@Builder와 함께Genderenum 타입으로 깔끔하게 변경되었네요.
84-95: updateInfo 메서드 시그니처 변경 완료됨메서드 파라미터가
Genderenum으로 정확히 변경되었으며, 유일한 호출처인MemberInfoUpdateRequest.updateMember()도 이미Genderenum을 제대로 전달하고 있습니다. 모든 구현이 일관성 있게 업데이트되어 있네요.
src/main/java/com/example/medicare_call/dto/MemberInfoUpdateRequest.java
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: 2
🤖 Fix all issues with AI agents
In @src/main/resources/db/migration/V31__member_gender_byte_to_enum.sql:
- Around line 18-21: The migration currently proceeds unconditionally and will
fail at the ALTER TABLE if there are NULLs or values other than 0/1 in the
existing gender column; modify V31__member_gender_byte_to_enum.sql to run
validation SELECTs (checking that gender values are only 0 or 1 and that there
are no NULLs) and abort the migration when invalid rows exist by raising an
error (e.g., using SIGNAL SQLSTATE '45000') instead of continuing to the
DROP/CHANGE block that operates on `gender` and `gender_new`; ensure the script
only reaches the ALTER TABLE ... DROP COLUMN `gender`, CHANGE COLUMN
`gender_new` `gender` ENUM(...) NOT NULL when the validation confirms
invalid_rows = 0.
- Around line 13-16: Current SELECT check for invalid_rows does not halt Flyway,
so the ALTER that adds NOT NULL to `gender_new` can run even when invalid rows
exist and cause a failed migration; instead update the migration to perform a
safe two-step ALTER on table `Member`: first explicitly enforce NOT NULL on
`gender_new` (e.g., ALTER TABLE `Member` MODIFY COLUMN `gender_new` VARCHAR(...)
NOT NULL) so it fails early if NULLs or bad values exist, and only after that
succeeds perform the swap/drop (e.g., ALTER TABLE `Member` DROP COLUMN `gender`,
CHANGE COLUMN `gender_new` `gender` ENUM('MALE','FEMALE') NOT NULL); reference
the `Member` table and the `gender_new`/`gender` column names when making these
changes.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/example/medicare_call/dto/MemberInfoUpdateRequest.javasrc/main/resources/db/migration/V31__member_gender_byte_to_enum.sql
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (1)
src/main/java/com/example/medicare_call/dto/MemberInfoUpdateRequest.java (1)
50-61: Gender enum 전달이 정확하게 구현됨
Member.updateInfo()메서드는 세 번째 매개변수로Gender gender타입을 받고 있으며, 라인 54에서this.gender를 직접 전달하는 것이 올바릅니다. Gender enum 마이그레이션이 일관되게 진행되었습니다.
| -- 3) 변환 실패(=NULL) 체크 | ||
| SELECT COUNT(*) AS invalid_rows | ||
| FROM `Member` | ||
| WHERE `gender_new` IS NULL; |
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.
🧩 Analysis chain
🏁 Script executed:
# First, find the migration file
find . -name "V31__member_gender_byte_to_enum.sql" -type f 2>/dev/nullRepository: Medicare-Call/Medicare-Call-Backend
Length of output: 148
🏁 Script executed:
# Also check if there are other migration files to understand structure
find . -path "*db/migration*" -name "*.sql" -type f 2>/dev/null | head -20Repository: Medicare-Call/Medicare-Call-Backend
Length of output: 1569
🌐 Web query:
Flyway SELECT statement migration behavior does it halt stop check result
💡 Result:
Short answer: Flyway will execute SELECT statements during a migration, it does not stop/halt the migration just because a SELECT returns rows — SELECT results are displayed by default (you can disable this) and the migration continues unless the statement raises an SQL error. Flyway runs each migration in a transaction and will roll back that migration if a statement fails (DB vendor permitting transactional DDL). [1][2]
References:
- Flyway displays query results for statements that return results; can disable with outputQueryResults=false. [1]
- Flyway runs migrations in transactions and rolls back a failed migration if an error occurs (subject to DB support for transactional DDL). [2]
🏁 Script executed:
cat -n ./src/main/resources/db/migration/V31__member_gender_byte_to_enum.sqlRepository: Medicare-Call/Medicare-Call-Backend
Length of output: 788
SELECT 결과가 마이그레이션을 중단시키지 않습니다.
Flyway는 SELECT 문의 결과를 확인하지 않고 계속 진행하므로, invalid_rows가 0보다 크더라도 라인 19-21의 ALTER TABLE이 실행됩니다. 만약 운영 DB에 예상치 못한 gender 값(0, 1이 아닌 값)이나 NULL이 존재한다면, 라인 21의 NOT NULL 제약조건 적용 시 마이그레이션이 실패합니다.
실제로는 NOT NULL 제약 실패로 인한 SQL 에러가 발생하면 Flyway가 트랜잭션을 롤백하겠지만, 이렇게 되면 마이그레이션이 깨지고 수동 개입이 필요합니다. 미리 데이터를 검증하거나, 아래처럼 더 안전한 방식으로 스크립트를 수정하는 게 좋습니다:
-- 먼저 NULL이 없는지 확인하며 NOT NULL 적용
ALTER TABLE `Member`
MODIFY COLUMN `gender_new` VARCHAR(50) NOT NULL;
-- 위가 성공하면 스왑 진행
ALTER TABLE `Member`
DROP COLUMN `gender`,
CHANGE COLUMN `gender_new` `gender` ENUM('MALE','FEMALE') NOT NULL;이렇게 하면 NULL이 있을 경우 첫 번째 ALTER TABLE에서 명시적으로 실패하고, gender 컬럼은 손상되지 않습니다.
🤖 Prompt for AI Agents
In @src/main/resources/db/migration/V31__member_gender_byte_to_enum.sql around
lines 13 - 16, Current SELECT check for invalid_rows does not halt Flyway, so
the ALTER that adds NOT NULL to `gender_new` can run even when invalid rows
exist and cause a failed migration; instead update the migration to perform a
safe two-step ALTER on table `Member`: first explicitly enforce NOT NULL on
`gender_new` (e.g., ALTER TABLE `Member` MODIFY COLUMN `gender_new` VARCHAR(...)
NOT NULL) so it fails early if NULLs or bad values exist, and only after that
succeeds perform the swap/drop (e.g., ALTER TABLE `Member` DROP COLUMN `gender`,
CHANGE COLUMN `gender_new` `gender` ENUM('MALE','FEMALE') NOT NULL); reference
the `Member` table and the `gender_new`/`gender` column names when making these
changes.
| -- invalid_rows = 0 확인 후 4) 스왑 (기존 컬럼 제거 + 새 컬럼을 gender로) | ||
| ALTER TABLE `Member` | ||
| DROP COLUMN `gender`, | ||
| CHANGE COLUMN `gender_new` `gender` ENUM('MALE','FEMALE') NOT NULL; |
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.
🧩 Analysis chain
🏁 Script executed:
cat src/main/resources/db/migration/V31__member_gender_byte_to_enum.sqlRepository: Medicare-Call/Medicare-Call-Backend
Length of output: 641
마이그레이션 실행 전 데이터 검증이 필요합니다.
라인 10-12의 SELECT 쿼리는 참고용일 뿐 자동으로 실행을 중단하지 않습니다. 만약 invalid_rows가 0이 아닌데 스크립트가 계속 진행되면 라인 21의 NOT NULL 제약조건에서 마이그레이션이 실패합니다.
마이그레이션 실행 전에 다음을 확인하는 것을 권장합니다:
- 기존
gender컬럼이 0 또는 1의 값만 가지고 있는지 확인 - NULL 값이 없는지 확인
SELECT 쿼리 결과에서 invalid_rows = 0이 확인되어야 안전하게 진행할 수 있습니다.
🤖 Prompt for AI Agents
In @src/main/resources/db/migration/V31__member_gender_byte_to_enum.sql around
lines 18 - 21, The migration currently proceeds unconditionally and will fail at
the ALTER TABLE if there are NULLs or values other than 0/1 in the existing
gender column; modify V31__member_gender_byte_to_enum.sql to run validation
SELECTs (checking that gender values are only 0 or 1 and that there are no
NULLs) and abort the migration when invalid rows exist by raising an error
(e.g., using SIGNAL SQLSTATE '45000') instead of continuing to the DROP/CHANGE
block that operates on `gender` and `gender_new`; ensure the script only reaches
the ALTER TABLE ... DROP COLUMN `gender`, CHANGE COLUMN `gender_new` `gender`
ENUM(...) NOT NULL when the validation confirms invalid_rows = 0.
faff41a to
1d5023e
Compare
|
@sudo-Terry 코드래빗이 제안한 gender 필드 타입 변경 enum -> varchar(50) 내용은 할루시네이션 같더라구요. 그리고 프로젝트에서도 현재 enum 타입은 DB 스키마 레벨에서 enum으로 적용되어있어서 기존 방식 그대로 가겠습니다.
|
|
@mjkhub 저도 enum 타입을 원해 사용했었는데 이야기를 들어보니 VARCHAR도 괜찮겠다 싶어서 의견이 궁금해서 여쭤봤던 것이에요! 지금까지 프로젝트는 enum타입으로 스키마 타입이 만들어져있는게 맞아요! 다 바꾸자는게 아니라... 단순하게 의견이 궁금했습니다. |
|
@sudo-Terry 물론, hibernate ddl 설정값을 validate -> none으로 바꾸면 지금 버전에서도 varchar로 할 수 있을 것 같긴 합니닷 |
kisusu115
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.
드디어 byte 정상화가.. 마이그레이션 까지 고생하셨습니다👍👍
|
@mjkhub |

Desc
ex) 테스트코드
Summary by CodeRabbit
Refactor
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.