Refactor/byte to enum migration - #226
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough바이트 기반의 성별(Gender)과 구독 플랜(SubscriptionPlan) 표현을 Java 열거형(enum)으로 전환하는 리팩토링. 도메인 모델, 서비스 레이어, DTO, 데이터베이스 스키마, 테스트 코드 전반에 걸쳐 타입 시스템을 일관되게 업데이트함. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/main/resources/db/migration/V36__fix_gender_enum_migration.sql (1)
1-5:⚠️ Potential issue | 🔴 CriticalV35와 중복 – V36 파일 삭제 필요
V35의 Line 12가 이미
gender_new → genderrename을 수행하므로, 이 V36 스크립트는 V35가 적용된 환경에서 반드시 실패합니다. V35 리뷰 코멘트에서 언급한 충돌 이슈의 근본 원인입니다. V36 파일 전체를 삭제하는 것이 올바른 해결책입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/V36__fix_gender_enum_migration.sql` around lines 1 - 5, This migration V36__fix_gender_enum_migration.sql duplicates the rename already performed in V35 (the ALTER TABLE `Elder` CHANGE COLUMN `gender_new` `gender` ... statement) and will fail when V35 is present; remove the entire V36__fix_gender_enum_migration.sql file from the migrations directory so only the V35 migration performs the rename and to avoid the conflicting ALTER TABLE operation at runtime.
🧹 Nitpick comments (4)
src/main/java/com/example/medicare_call/util/TestDataGenerator.java (1)
4-4: 와일드카드 임포트 사용 – 선택적 개선 사항Java에서 와일드카드 임포트(
import com.example.medicare_call.global.enums.*)는 어떤 타입이 실제로 사용되는지 파악하기 어렵게 만들 수 있습니다. 명시적 임포트로 변경하는 것이 일반적으로 권장됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/example/medicare_call/util/TestDataGenerator.java` at line 4, Replace the wildcard import in TestDataGenerator (import com.example.medicare_call.global.enums.*) with explicit imports of only the enum types actually referenced in the TestDataGenerator class: scan the class for enum usages (e.g., any references to enum types like SomeEnumName) and add individual import lines for each (import com.example.medicare_call.global.enums.YourEnum;). Remove the wildcard import and ensure the class compiles and IDE/static analysis no longer flags unused/wildcard imports.src/test/java/com/example/medicare_call/controller/CareCallControllerImmediateTest.java (1)
83-91:testMember가 어떤 테스트에서도 사용되지 않아요.
@BeforeEach에서 열심히 만들어줬는데 두 테스트 메서드 모두testMember를 참조하지 않습니다. 향후 혼란을 줄이기 위해 실제로 쓰이는 시점에 로컬로 선언하거나, 아니면 필요 없다면 제거하는 게 좋을 것 같아요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/example/medicare_call/controller/CareCallControllerImmediateTest.java` around lines 83 - 91, testMember initialized in the `@BeforeEach` of CareCallControllerImmediateTest is unused by any test; remove the unused field or move its creation into tests that actually need it. Either delete the testMember field and its builder code from the setup method, or refactor tests to declare and build a local Member where needed (referencing testMember and Member.builder() in the class) so no unused test fixture remains.src/test/java/com/example/medicare_call/controller/ElderControllerTest.java (2)
198-207:gender필드 JSON 경로 검증이 빠져 있어요.
expectedResponses에Gender.MALE/Gender.FEMALE를 세팅했는데 정작andExpect체인에는gender검증이 없네요. 직렬화 결과까지 한 번 확인해주면 더 든든할 것 같습니다.✅ 추가 제안
.andExpect(jsonPath("$[0].name").value("홍길동")) + .andExpect(jsonPath("$[0].gender").value("MALE")) .andExpect(jsonPath("$[1].name").value("김영희")); + .andExpect(jsonPath("$[1].gender").value("FEMALE"));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/example/medicare_call/controller/ElderControllerTest.java` around lines 198 - 207, The test in ElderControllerTest misses assertions for the serialized gender field: update the mockMvc.perform(...).andExpect(...) chain to assert the JSON path "$[0].gender" equals the expected first enum value and "$[1].gender" equals the expected second enum value (matching how Gender is serialized, e.g., "MALE"/"FEMALE"), so the response array's gender values match the expectedResponses you set earlier.
113-115:convertGenderToByte메서드가 더 이상 사용되지 않아요.byte 기반 API 시절 잔재인 이 헬퍼는 현재 파일 내 어디서도 호출되지 않습니다. 슬쩍 지워주시면 깔끔할 것 같아요 😊
🧹 제거 제안
- private byte convertGenderToByte(Gender gender) { - return (byte) (gender == Gender.MALE ? 0 : 1); - } -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/example/medicare_call/controller/ElderControllerTest.java` around lines 113 - 115, Remove the unused helper method convertGenderToByte from the ElderControllerTest class: delete the private byte convertGenderToByte(Gender gender) { ... } method, then run a build/tests to ensure there are no remaining references and clean up any now-unused imports (e.g., Gender) in ElderControllerTest.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/example/medicare_call/service/auth/AuthService.java`:
- Line 39: Remove the automatic assignment of SubscriptionPlan.PREMIUM in the
signup flow by deleting the .plan(SubscriptionPlan.PREMIUM) call in the member
creation logic (the builder/creation code in AuthService), so the Member entity
is created with a null plan (either omit the plan setter or explicitly set
plan(null)); verify Member.java’s plan mapping remains nullable and do not
change SubscriptionPlan enum unless you intend to make PREMIUM the default (in
which case update Member.java comments and the nullable constraint accordingly).
In `@src/main/java/com/example/medicare_call/service/ElderService.java`:
- Around line 52-53: The current flow calls getElderOrThrow(elderId) before
performing the access check, which leaks elder existence via different errors;
swap the calls so getManageRelationOrThrow(memberId, elderId) runs first and
only after successful authorization call getElderOrThrow(elderId). Update both
occurrences (the block with getElderOrThrow/getManageRelationOrThrow at the
start and the second occurrence around lines 69-70) so authorization is
performed prior to any resource lookup.
In `@src/main/resources/db/migration/V35__refactor_byte_to_enum.sql`:
- Around line 4-9: The UPDATE leaves gender_new NULL for rows where Elder.gender
is not 0 or 1, which will cause the subsequent NOT NULL constraint to fail;
before adding the NOT NULL constraint in V35__refactor_byte_to_enum.sql, add a
defensive check and remediation: query the Elder table for rows where gender NOT
IN (0,1) OR gender IS NULL, and either update those rows to a safe default enum
(e.g., 'MALE' or 'FEMALE') or raise a clear error and abort the migration so the
data can be fixed; reference the Elder table and the gender → gender_new
transformation in your changes and ensure the migration explicitly handles or
fails on invalid gender values prior to applying NOT NULL.
- Around line 11-12: V35 already performs the rename/change ("ALTER TABLE
`Elder` CHANGE COLUMN `gender_new` `gender` ENUM('MALE','FEMALE') NOT NULL"), so
the subsequent V36 migration tries the same rename and fails with "Unknown
column 'gender_new'"; fix by either (A, recommended) deleting the V36 migration
file entirely so only V35 remains, or (B) remove the duplicate CHANGE COLUMN
line from V35__refactor_byte_to_enum.sql and let V36 perform the final
rename—ensure you reference the SQL statement "CHANGE COLUMN `gender_new`
`gender` ENUM('MALE','FEMALE') NOT NULL" and the V36 migration when making the
change.
---
Duplicate comments:
In `@src/main/resources/db/migration/V36__fix_gender_enum_migration.sql`:
- Around line 1-5: This migration V36__fix_gender_enum_migration.sql duplicates
the rename already performed in V35 (the ALTER TABLE `Elder` CHANGE COLUMN
`gender_new` `gender` ... statement) and will fail when V35 is present; remove
the entire V36__fix_gender_enum_migration.sql file from the migrations directory
so only the V35 migration performs the rename and to avoid the conflicting ALTER
TABLE operation at runtime.
---
Nitpick comments:
In `@src/main/java/com/example/medicare_call/util/TestDataGenerator.java`:
- Line 4: Replace the wildcard import in TestDataGenerator (import
com.example.medicare_call.global.enums.*) with explicit imports of only the enum
types actually referenced in the TestDataGenerator class: scan the class for
enum usages (e.g., any references to enum types like SomeEnumName) and add
individual import lines for each (import
com.example.medicare_call.global.enums.YourEnum;). Remove the wildcard import
and ensure the class compiles and IDE/static analysis no longer flags
unused/wildcard imports.
In
`@src/test/java/com/example/medicare_call/controller/CareCallControllerImmediateTest.java`:
- Around line 83-91: testMember initialized in the `@BeforeEach` of
CareCallControllerImmediateTest is unused by any test; remove the unused field
or move its creation into tests that actually need it. Either delete the
testMember field and its builder code from the setup method, or refactor tests
to declare and build a local Member where needed (referencing testMember and
Member.builder() in the class) so no unused test fixture remains.
In `@src/test/java/com/example/medicare_call/controller/ElderControllerTest.java`:
- Around line 198-207: The test in ElderControllerTest misses assertions for the
serialized gender field: update the mockMvc.perform(...).andExpect(...) chain to
assert the JSON path "$[0].gender" equals the expected first enum value and
"$[1].gender" equals the expected second enum value (matching how Gender is
serialized, e.g., "MALE"/"FEMALE"), so the response array's gender values match
the expectedResponses you set earlier.
- Around line 113-115: Remove the unused helper method convertGenderToByte from
the ElderControllerTest class: delete the private byte
convertGenderToByte(Gender gender) { ... } method, then run a build/tests to
ensure there are no remaining references and clean up any now-unused imports
(e.g., Gender) in ElderControllerTest.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/main/java/com/example/medicare_call/controller/ElderController.javasrc/main/java/com/example/medicare_call/domain/Elder.javasrc/main/java/com/example/medicare_call/domain/Member.javasrc/main/java/com/example/medicare_call/dto/ElderRegisterResponse.javasrc/main/java/com/example/medicare_call/global/enums/Gender.javasrc/main/java/com/example/medicare_call/service/ElderService.javasrc/main/java/com/example/medicare_call/service/auth/AuthService.javasrc/main/java/com/example/medicare_call/service/carecall/CareCallTestService.javasrc/main/java/com/example/medicare_call/util/TestDataGenerator.javasrc/main/resources/db/migration/V35__refactor_byte_to_enum.sqlsrc/main/resources/db/migration/V36__fix_gender_enum_migration.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/service/ElderServiceTest.javasrc/test/java/com/example/medicare_call/service/auth/AuthServiceTest.javasrc/test/java/com/example/medicare_call/service/payment/NaverPayServiceTest.javasrc/test/java/com/example/medicare_call/service/report/MealRecordServiceTest.javasrc/test/java/com/example/medicare_call/service/report/SleepRecordServiceTest.java
| .birthDate(req.getBirthDate()) | ||
| .gender(req.getGender()) | ||
| .plan((byte) 0) | ||
| .plan(SubscriptionPlan.PREMIUM) |
There was a problem hiding this comment.
신규 회원가입 시 SubscriptionPlan.PREMIUM 자동 부여 – 의도한 비즈니스 로직인지 확인 필요
기존 코드는 (byte)0을 사용했는데, V35 마이그레이션에서 0은 ELSE NULL 분기로 NULL 처리됩니다. 즉 기존에는 가입 시 플랜이 미설정 상태였음을 의미합니다. Member.java의 주석도 "처음 회원가입 시 plan이 없으므로 nullable=false 조건 삭제" 라고 명시하고 있어, plan을 null로 유지하는 것이 원래 설계 의도처럼 보입니다.
모든 신규 가입자에게 PREMIUM을 자동 부여하는 것이 맞다면 해당 주석과 nullable 설계를 함께 업데이트하고, 그렇지 않다면 아래와 같이 null로 남겨야 합니다.
🤔 원래 의도대로 null 유지하는 경우 수정 예시
Member member = Member.builder()
.phone(phone)
.name(req.getName())
.birthDate(req.getBirthDate())
.gender(req.getGender())
- .plan(SubscriptionPlan.PREMIUM)
.termsAgreedAt(LocalDateTime.now())
.fcmToken(req.getFcmToken())
.build();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/example/medicare_call/service/auth/AuthService.java` at
line 39, Remove the automatic assignment of SubscriptionPlan.PREMIUM in the
signup flow by deleting the .plan(SubscriptionPlan.PREMIUM) call in the member
creation logic (the builder/creation code in AuthService), so the Member entity
is created with a null plan (either omit the plan setter or explicitly set
plan(null)); verify Member.java’s plan mapping remains nullable and do not
change SubscriptionPlan enum unless you intend to make PREMIUM the default (in
which case update Member.java comments and the nullable constraint accordingly).
Contributor
There was a problem hiding this comment.
현재도 Plan 검사는 Subscription 테이블 조회해서 찾고 있어서 plan 두는게 애매해보이긴 하네요
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the Subscription entity/model
fd -e java Subscription | head -20
Length of output: 677
🏁 Script executed:
#!/bin/bash
# Look for Subscription entity definition
ast-grep --pattern 'class Subscription {
$$$
}'
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for how subscription plan checking is implemented
rg -n -C5 --type=java 'Subscription.*find|findBy.*Subscription|subscriptionRepository' | head -100
Length of output: 11868
🏁 Script executed:
#!/bin/bash
# Check Member.java for the plan field and its comment
rg -n -C3 --type=java '처음 회원가입|plan.*nullable' src/main/java/com/example/medicare_call/domain/Member.java
Length of output: 236
🏁 Script executed:
#!/bin/bash
# Look at the Subscription entity structure
cat src/main/java/com/example/medicare_call/domain/Subscription.java
Length of output: 1758
🏁 Script executed:
#!/bin/bash
# Check SubscriptionService to see how plan checking is done
cat src/main/java/com/example/medicare_call/service/SubscriptionService.java
Length of output: 1184
🏁 Script executed:
#!/bin/bash
# Search for actual usages of Member.plan field in the codebase
rg -n -C3 --type=java 'member\.getPlan\(\)|member\.plan|\.getPlan\(\)' | head -80
Length of output: 4599
@kisusu115 정확한 지적이십니다 👍
코드를 확인해보니 실제로 플랜 체크는 Subscription 테이블을 통해 이루어지고 있네요:
현재 아키텍처:
Subscription엔티티에plan필드가 있고 (NOT NULL, Elder당 하나)SubscriptionService는subscriptionRepository.findByMemberId()로 구독 정보를 조회Member.plan필드는 비즈니스 로직에서 실제로 사용되지 않는 것으로 보입니다
문제점:
- 데이터 중복 -
Member.plan과Subscription.plan이 동시에 존재 - 불일치 가능성 - 회원가입 시
Member.plan = PREMIUM을 설정하지만 실제Subscription레코드는 아직 없음 - Single Source of Truth 위반 - 어느 것이 진실의 원천인지 불명확
제안:
이 리팩토링 PR을 계기로 아키텍처를 정리하는 것이 좋을 것 같습니다:
-
Option A:
Member.plan필드 제거 (추천)- 실제로 사용되지 않는다면 완전히 제거
- Subscription 테이블만 플랜의 단일 진실 원천으로 유지
-
Option B: 용도 명확화 후 유지
- 만약 "기본 플랜" 같은 다른 용도가 있다면 필드명/주석으로 명확히 하고 유지
- 예:
defaultPlan또는initialPlan으로 리네이밍
현재 PR에서는 일단 PREMIUM 할당을 제거하고 null로 두는 것이 안전해 보입니다. 필드 자체를 제거할지는 별도 이슈로 논의하시는게 어떨까요?
Member member = Member.builder()
.phone(phone)
.name(req.getName())
.birthDate(req.getBirthDate())
.gender(req.getGender())
- .plan(SubscriptionPlan.PREMIUM)
.termsAgreedAt(LocalDateTime.now())
.fcmToken(req.getFcmToken())
.build();✏️ Learnings added
Learnt from: kisusu115
Repo: Medicare-Call/Medicare-Call-Backend PR: 226
File: src/main/java/com/example/medicare_call/service/auth/AuthService.java:39-39
Timestamp: 2026-02-26T07:52:19.932Z
Learning: In the Medicare Call Backend project, the actual subscription plan checking and management is done through the Subscription table (one subscription per Elder), not through the Member.plan field. The Member.plan field appears to be redundant with the Subscription.plan field.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Comment on lines
+52
to
+53
| Elder elder = getElderOrThrow(elderId); | ||
| getManageRelationOrThrow(memberId, elderId); |
There was a problem hiding this comment.
권한 검증보다 대상 조회를 먼저 하면 elderId 존재 여부가 노출될 수 있어요.
Line 52-53, Line 69-70에서 먼저 getElderOrThrow를 호출하면, 비인가 사용자에게도 ELDER_NOT_FOUND와 HANDLE_ACCESS_DENIED 차이로 리소스 존재 여부가 드러날 수 있습니다.
🔧 제안 수정안
@@
`@Transactional`
public ElderResponse updateElder(Integer memberId, Integer elderId, ElderUpdateRequest req) {
- Elder elder = getElderOrThrow(elderId);
- getManageRelationOrThrow(memberId, elderId);
+ getManageRelationOrThrow(memberId, elderId);
+ Elder elder = getElderOrThrow(elderId);
@@
`@Transactional`
public void deleteElder(Integer memberId, Integer elderId) {
- Elder elder = getElderOrThrow(elderId);
- getManageRelationOrThrow(memberId, elderId);
+ getManageRelationOrThrow(memberId, elderId);
+ Elder elder = getElderOrThrow(elderId);
elderRepository.delete(elder);
}Also applies to: 69-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/example/medicare_call/service/ElderService.java` around
lines 52 - 53, The current flow calls getElderOrThrow(elderId) before performing
the access check, which leaks elder existence via different errors; swap the
calls so getManageRelationOrThrow(memberId, elderId) runs first and only after
successful authorization call getElderOrThrow(elderId). Update both occurrences
(the block with getElderOrThrow/getManageRelationOrThrow at the start and the
second occurrence around lines 69-70) so authorization is performed prior to any
resource lookup.
Comment on lines
+4
to
+9
| UPDATE `Elder` | ||
| SET `gender_new` = CASE `gender` | ||
| WHEN 0 THEN 'MALE' | ||
| WHEN 1 THEN 'FEMALE' | ||
| ELSE NULL | ||
| END; |
There was a problem hiding this comment.
Elder.gender ELSE NULL → NOT NULL 제약 위반 가능성
UPDATE 시 gender 값이 0 또는 1이 아닌 행(예: 레거시 NULL 또는 기타 값)이 존재하면 gender_new가 NULL로 남습니다. 이후 Line 12에서 NOT NULL 제약을 걸면 MySQL/MariaDB는 즉시 오류를 반환합니다.
마이그레이션 전 데이터를 점검하거나, NOT NULL 제약 적용 전에 NULL 잔존 여부를 검증하는 로직을 추가하는 것을 권장합니다.
🛡️ 방어적 검증 추가 예시
UPDATE `Elder`
SET `gender_new` = CASE `gender`
WHEN 0 THEN 'MALE'
WHEN 1 THEN 'FEMALE'
ELSE NULL
END;
+-- 예상치 못한 gender 값이 없는지 검증 (선택적 safeguard)
+-- 아래 SELECT가 0건을 반환해야 정상입니다.
+-- SELECT COUNT(*) FROM `Elder` WHERE `gender_new` IS NULL;
ALTER TABLE `Elder` DROP COLUMN `gender`;
ALTER TABLE `Elder` CHANGE COLUMN `gender_new` `gender` ENUM('MALE','FEMALE') NOT NULL;📝 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
| UPDATE `Elder` | |
| SET `gender_new` = CASE `gender` | |
| WHEN 0 THEN 'MALE' | |
| WHEN 1 THEN 'FEMALE' | |
| ELSE NULL | |
| END; | |
| UPDATE `Elder` | |
| SET `gender_new` = CASE `gender` | |
| WHEN 0 THEN 'MALE' | |
| WHEN 1 THEN 'FEMALE' | |
| ELSE NULL | |
| END; | |
| -- 예상치 못한 gender 값이 없는지 검증 (선택적 safeguard) | |
| -- 아래 SELECT가 0건을 반환해야 정상입니다. | |
| -- SELECT COUNT(*) FROM `Elder` WHERE `gender_new` IS NULL; | |
| ALTER TABLE `Elder` DROP COLUMN `gender`; | |
| ALTER TABLE `Elder` CHANGE COLUMN `gender_new` `gender` ENUM('MALE','FEMALE') NOT NULL; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/db/migration/V35__refactor_byte_to_enum.sql` around lines
4 - 9, The UPDATE leaves gender_new NULL for rows where Elder.gender is not 0 or
1, which will cause the subsequent NOT NULL constraint to fail; before adding
the NOT NULL constraint in V35__refactor_byte_to_enum.sql, add a defensive check
and remediation: query the Elder table for rows where gender NOT IN (0,1) OR
gender IS NULL, and either update those rows to a safe default enum (e.g.,
'MALE' or 'FEMALE') or raise a clear error and abort the migration so the data
can be fixed; reference the Elder table and the gender → gender_new
transformation in your changes and ensure the migration explicitly handles or
fails on invalid gender values prior to applying NOT NULL.
github-actions
Bot
added
the
refactoring
kisusu115
approved these changes
Feb 26, 2026
| .birthDate(req.getBirthDate()) | ||
| .gender(req.getGender()) | ||
| .plan((byte) 0) | ||
| .plan(SubscriptionPlan.PREMIUM) |
Contributor
There was a problem hiding this comment.
현재도 Plan 검사는 Subscription 테이블 조회해서 찾고 있어서 plan 두는게 애매해보이긴 하네요
Comment on lines
-34
to
-53
| Member guardian = memberRepository.findById(memberId) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.MEMBER_NOT_FOUND)); | ||
| Elder elder = Elder.builder() | ||
| .name(request.getName()) | ||
| .birthDate(request.getBirthDate()) | ||
| .gender((byte) (request.getGender() == Gender.MALE ? 0 : 1)) | ||
| .phone(request.getPhone()) | ||
| .relationship(request.getRelationship()) | ||
| .residenceType(request.getResidenceType()) | ||
| .build(); | ||
| Elder savedElder = elderRepository.save(elder); | ||
| MemberElder relation = MemberElder.builder() | ||
| .guardian(guardian) | ||
| .elder(savedElder) | ||
| .authority(MemberElderAuthority.MANAGE) | ||
| .build(); | ||
| memberElderRepository.save(relation); | ||
| savedElder.addMemberElder(relation); | ||
| guardian.addMemberElder(relation); | ||
| return relation; |
# Conflicts: # src/main/java/com/example/medicare_call/service/carecall/CareCallTestService.java
jyun-KIM
force-pushed
the
refactor/byte-to-enum-migration
branch
from
March 10, 2026 04:37
7f4462c to
d96dafd
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
src/main/java/com/example/medicare_call/service/auth/AuthService.java (1)
34-42:⚠️ Potential issue | 🟠 Major신규 가입자에게
PREMIUM을 고정하면 실제 구독 상태와 어긋납니다.이건 단순 enum 치환이 아니라 회원가입 기본값을 바꾸는 로직입니다.
Member.plan은 nullable로 열어두셨고, 실제 플랜 판정이Subscription기준이면 여기서PREMIUM을 박아 두는 순간 가입 직후부터 중복 상태가 생깁니다. 기본값이 정말 필요하면 같은 트랜잭션에서 실제Subscription까지 함께 만들고, 아니라면 여기서는 비워 두는 쪽이 맞습니다.수정 예시
Member member = Member.builder() .phone(phone) .name(req.getName()) .birthDate(req.getBirthDate()) .gender(req.getGender()) - .plan(SubscriptionPlan.PREMIUM) .termsAgreedAt(LocalDateTime.now()) .fcmToken(req.getFcmToken()) .build();Based on learnings, the actual subscription plan checking and management is done through the Subscription table (one subscription per Elder), not through the Member.plan field. The Member.plan field appears to be redundant with the Subscription.plan field.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/example/medicare_call/service/auth/AuthService.java` around lines 34 - 42, The code is incorrectly hard-setting Member.plan to SubscriptionPlan.PREMIUM in the Member.builder() inside AuthService, causing divergence from the real Subscription state; instead, remove setting Member.plan (leave it null) or, if you truly need a default, create the corresponding Subscription entity in the same transaction and set Member.plan from that Subscription; update the Member.builder() invocation to omit .plan(SubscriptionPlan.PREMIUM) and ensure subscription creation/assignment logic uses the Subscription entity (class Subscription and enum SubscriptionPlan) so Member.plan is not used as the source of truth.src/main/resources/db/migration/V35__refactor_byte_to_enum.sql (1)
4-12:⚠️ Potential issue | 🟠 Major예상 밖
gender값이 하나라도 있으면 이 마이그레이션이 배포 중에 멈춥니다.Line 8에서
0/1이외 값은NULL로 남는데, 곧바로 Line 12에서NOT NULL을 걸고 있습니다. 레거시 데이터에NULL이나 다른 코드가 한 건이라도 있으면 Flyway가 실패하니, 최소한 drop/rename 전에 잔존 건을 검증하거나 정리하는 step이 필요합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/V35__refactor_byte_to_enum.sql` around lines 4 - 12, The migration can fail if `Elder.gender` contains NULL or values other than 0/1 because you set ELSE to NULL then make `gender` NOT NULL; fix by adding a pre-change validation or cleaning step: before dropping/changing columns, add a check using a COUNT of rows where `gender` IS NULL OR `gender` NOT IN (0,1) (referencing `Elder` and `gender`) and either SIGNAL an explanatory error to stop the migration or update those rows to a safe default (e.g., map unexpected to 'MALE' or include an 'UNKNOWN' enum), then continue with the existing UPDATE that sets `gender_new` and the ALTER TABLE DROP/CHANGE for `gender_new` -> `gender`.src/main/java/com/example/medicare_call/service/ElderService.java (1)
51-53:⚠️ Potential issue | 🟠 Major권한 체크보다 elder 조회가 먼저라 존재 여부가 노출돼요.
이 순서면 비인가 사용자가
ELDER_NOT_FOUND와HANDLE_ACCESS_DENIED차이로elderId존재 여부를 구분할 수 있습니다. 두 메서드 모두 권한 검증을 먼저 하고, 그다음에 엔티티를 조회하는 순서로 바꿔주세요.🔧 제안 수정안
`@Transactional` public ElderResponse updateElder(Integer memberId, Integer elderId, ElderUpdateRequest req) { - Elder elder = getElderOrThrow(elderId); getManageRelationOrThrow(memberId, elderId); + Elder elder = getElderOrThrow(elderId); elder.applySettings( req.name(), @@ `@Transactional` public void deleteElder(Integer memberId, Integer elderId) { - Elder elder = getElderOrThrow(elderId); getManageRelationOrThrow(memberId, elderId); + Elder elder = getElderOrThrow(elderId); elderRepository.delete(elder); }Also applies to: 68-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/example/medicare_call/service/ElderService.java` around lines 51 - 53, The current updateElder flow calls getElderOrThrow before getManageRelationOrThrow, leaking elder existence; swap the calls so authorization runs first: call getManageRelationOrThrow(memberId, elderId) before getElderOrThrow(elderId) in updateElder, then proceed with the update logic. Apply the same change to the other method that currently does getElderOrThrow before getManageRelationOrThrow (the one around lines 68-70) so authorization is always checked prior to fetching the Elder entity.
🧹 Nitpick comments (2)
src/test/java/com/example/medicare_call/service/auth/AuthServiceTest.java (1)
53-60: Auth 테스트가Member.plan구현 디테일에 좀 과하게 묶여 있어요.이 두 시나리오는 plan을 읽지 않는데 fixture마다
Member.plan을 직접 채우고 있어서, 나중에 중복 필드 정리할 때 인증 테스트까지 같이 흔들릴 수 있어요. 가능하면 공용 fixture/helper 뒤로 숨기거나, plan이 진짜 의미 있는 테스트에서만 드러내는 편이 덜 끈적합니다.Based on learnings, in the Medicare Call Backend project, the actual subscription plan checking and management is done through the Subscription table (one subscription per Elder), not through the Member.plan field. The Member.plan field appears to be redundant with the Subscription.plan field.
Also applies to: 102-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/example/medicare_call/service/auth/AuthServiceTest.java` around lines 53 - 60, The AuthServiceTest currently hardcodes Member.plan via Member.builder() (in the Member fixture used around the Member creation block and similar code later) which couples auth tests to plan implementation; remove the plan population from those Member fixtures and either (a) move default Member creation into a shared test helper/fixture (e.g., TestFixtures.createMember()/createDefaultMember()) that does not set plan, or (b) only set Member.plan in tests that explicitly validate plan-related behavior; ensure any tests that need subscription state create a Subscription object (the real source of truth) instead of relying on Member.plan, and update AuthServiceTest to use the shared helper or explicit Subscription builders where appropriate.src/test/java/com/example/medicare_call/controller/ElderControllerTest.java (1)
120-142: JSONgender계약 검증이 아직 비어 있어요.이번 PR 핵심이 enum 기반 응답으로 바뀌는 거라서, 성공 케이스에서
$.gender도 같이 고정해두는 게 좋아 보여요. 지금은name/guardian만 확인해서gender가 다시 숫자나 다른 형태로 직렬화돼도 테스트가 그냥 지나갑니다.Also applies to: 170-206
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/example/medicare_call/controller/ElderControllerTest.java` around lines 120 - 142, The test currently asserts only guardianId and guardianName, leaving the gender serialization contract unverified; update ElderControllerTest to also assert the response gender equals the expected enum string (derived from testElderRequest1.getGender() or the Elder instance) after the mockMvc.perform POST, and make the same addition to the second test block (lines ~170-206) that validates the registerElder flow; ensure the mocked elder/relation returned by elderService.registerElder(...) has the correct gender value so the jsonPath("$.gender") assertion checks the enum-based string representation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/example/medicare_call/dto/ElderRegisterResponse.java`:
- Line 15: ElderRegisterResponse의 gender 필드를 그대로 enum으로 노출하면 JSON이
"MALE"/"FEMALE"로 직렬화되어 기존 클라이언트와 호환성 문제가 발생할 수 있으므로,
ElderRegisterResponse.gender를 기대되는 응답 포맷(예: 정수 코드 또는 커스텀 문자열)으로 변환하여 직렬화하도록 수정하고
관련 테스트를 추가하세요; 구체적으로는 ElderRegisterResponse 클래스에서 gender 타입을 String/Integer로
변경하거나 Gender enum에 `@JsonValue` 또는 커스텀 직렬화 로직을 추가(예:
getGenderCode()/getGenderValue() 직렬화용 접근자)하여 원하는 포맷을 반환하도록 구현하고,
bulkRegisterElders 테스트에 gender 필드의 JSON 표현(숫자 코드 또는 커스텀 코드)을 검증하는 assertions를
추가해 API 계약을 보호하세요.
In
`@src/main/java/com/example/medicare_call/service/carecall/CareCallTestService.java`:
- Around line 15-29: CareCallTestService fails to compile because some imports
and package references are incorrect; update the CareCallClient import to its
actual package (e.g.
com.example.medicare_call.service.carecall.outbound.client.CareCallClient) and
add or correct imports for ElderRepository, CareCallSettingService, and
CareCallRequestSenderService from their real packages so the fields in
CareCallTestService resolve; ensure the file's package declaration matches the
directory and run a compile to confirm all symbols (CareCallTestService,
ElderRepository, CareCallSettingService, CareCallRequestSenderService,
CareCallClient) are correctly imported.
- Around line 49-62: The sendTestCall method currently calls
careCallClient.requestCall with hardcoded IDs (settingId 100 and
testElder.getId()) which can target real records; change sendTestCall to avoid
using real IDs by introducing a dedicated sandbox/test identifier scheme and a
runtime environment guard: replace the literal 100 and the hardcoded testElder
id with a constant like SANDBOX_SETTING_ID and SANDBOX_ELDER_ID (or generate
negative/UUID test IDs) and ensure a check (e.g., isNonProdProfile or
req.isSandbox()) prevents execution unless running in a non-production/sandbox
mode; additionally, if careCallClient supports a sandbox flag or endpoint, pass
that through the requestCall invocation or add validation in CareCallTestService
to enforce sandbox routing and reject attempts to send test calls in production.
- Around line 37-45: The catch-all currently wraps every Exception into a new
CustomException with INTERNAL_SERVER_ERROR and exposes e.getMessage(); update
CareCallTestService so you rethrow existing CustomException unchanged, and only
catch other unexpected Exceptions to wrap as new
CustomException(ErrorCode.INTERNAL_SERVER_ERROR, "케어콜 발송 중 오류가 발생했습니다.") without
leaking internal messages; ensure you still log the full exception (use
log.error with the exception object) for
careCallSettingService.getOrCreateImmediateSetting /
careCallRequestSenderService.sendCall failures before throwing the sanitized
CustomException.
---
Duplicate comments:
In `@src/main/java/com/example/medicare_call/service/auth/AuthService.java`:
- Around line 34-42: The code is incorrectly hard-setting Member.plan to
SubscriptionPlan.PREMIUM in the Member.builder() inside AuthService, causing
divergence from the real Subscription state; instead, remove setting Member.plan
(leave it null) or, if you truly need a default, create the corresponding
Subscription entity in the same transaction and set Member.plan from that
Subscription; update the Member.builder() invocation to omit
.plan(SubscriptionPlan.PREMIUM) and ensure subscription creation/assignment
logic uses the Subscription entity (class Subscription and enum
SubscriptionPlan) so Member.plan is not used as the source of truth.
In `@src/main/java/com/example/medicare_call/service/ElderService.java`:
- Around line 51-53: The current updateElder flow calls getElderOrThrow before
getManageRelationOrThrow, leaking elder existence; swap the calls so
authorization runs first: call getManageRelationOrThrow(memberId, elderId)
before getElderOrThrow(elderId) in updateElder, then proceed with the update
logic. Apply the same change to the other method that currently does
getElderOrThrow before getManageRelationOrThrow (the one around lines 68-70) so
authorization is always checked prior to fetching the Elder entity.
In `@src/main/resources/db/migration/V35__refactor_byte_to_enum.sql`:
- Around line 4-12: The migration can fail if `Elder.gender` contains NULL or
values other than 0/1 because you set ELSE to NULL then make `gender` NOT NULL;
fix by adding a pre-change validation or cleaning step: before dropping/changing
columns, add a check using a COUNT of rows where `gender` IS NULL OR `gender`
NOT IN (0,1) (referencing `Elder` and `gender`) and either SIGNAL an explanatory
error to stop the migration or update those rows to a safe default (e.g., map
unexpected to 'MALE' or include an 'UNKNOWN' enum), then continue with the
existing UPDATE that sets `gender_new` and the ALTER TABLE DROP/CHANGE for
`gender_new` -> `gender`.
---
Nitpick comments:
In `@src/test/java/com/example/medicare_call/controller/ElderControllerTest.java`:
- Around line 120-142: The test currently asserts only guardianId and
guardianName, leaving the gender serialization contract unverified; update
ElderControllerTest to also assert the response gender equals the expected enum
string (derived from testElderRequest1.getGender() or the Elder instance) after
the mockMvc.perform POST, and make the same addition to the second test block
(lines ~170-206) that validates the registerElder flow; ensure the mocked
elder/relation returned by elderService.registerElder(...) has the correct
gender value so the jsonPath("$.gender") assertion checks the enum-based string
representation.
In `@src/test/java/com/example/medicare_call/service/auth/AuthServiceTest.java`:
- Around line 53-60: The AuthServiceTest currently hardcodes Member.plan via
Member.builder() (in the Member fixture used around the Member creation block
and similar code later) which couples auth tests to plan implementation; remove
the plan population from those Member fixtures and either (a) move default
Member creation into a shared test helper/fixture (e.g.,
TestFixtures.createMember()/createDefaultMember()) that does not set plan, or
(b) only set Member.plan in tests that explicitly validate plan-related
behavior; ensure any tests that need subscription state create a Subscription
object (the real source of truth) instead of relying on Member.plan, and update
AuthServiceTest to use the shared helper or explicit Subscription builders where
appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8a85b4e1-c801-4dcf-9775-c589e1972921
📒 Files selected for processing (18)
src/main/java/com/example/medicare_call/controller/ElderController.javasrc/main/java/com/example/medicare_call/domain/Elder.javasrc/main/java/com/example/medicare_call/domain/Member.javasrc/main/java/com/example/medicare_call/dto/ElderRegisterResponse.javasrc/main/java/com/example/medicare_call/global/enums/Gender.javasrc/main/java/com/example/medicare_call/service/ElderService.javasrc/main/java/com/example/medicare_call/service/auth/AuthService.javasrc/main/java/com/example/medicare_call/service/carecall/CareCallTestService.javasrc/main/java/com/example/medicare_call/util/TestDataGenerator.javasrc/main/resources/db/migration/V35__refactor_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/service/ElderServiceTest.javasrc/test/java/com/example/medicare_call/service/auth/AuthServiceTest.javasrc/test/java/com/example/medicare_call/service/health_data/MealRecordServiceTest.javasrc/test/java/com/example/medicare_call/service/payment/NaverPayServiceTest.javasrc/test/java/com/example/medicare_call/service/report/SleepRecordServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (4)
- src/test/java/com/example/medicare_call/service/payment/NaverPayServiceTest.java
- src/test/java/com/example/medicare_call/service/report/SleepRecordServiceTest.java
- src/main/java/com/example/medicare_call/global/enums/Gender.java
- src/main/java/com/example/medicare_call/domain/Elder.java
| private LocalDate birthDate; | ||
| private String phone; | ||
| private String gender; | ||
| private Gender gender; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# ElderRegisterResponse가 실제 응답 계약으로 어떻게 검증되고 있는지 확인합니다.
rg -n -C2 --type=java 'ElderRegisterResponse|jsonPath\\(.*gender|\"gender\"' src/main src/testRepository: Medicare-Call/Medicare-Call-Backend
Length of output: 239
🏁 Script executed:
#!/bin/bash
# ElderRegisterResponse 관련 참조 검색
echo "=== ElderRegisterResponse 사용 검색 ==="
rg -n 'ElderRegisterResponse' src/main src/test --type=java
echo ""
echo "=== Gender enum 정의 확인 ==="
rg -n 'enum Gender' src/main --type=java -A 15
echo ""
echo "=== 테스트에서 gender 필드 검증 ==="
rg -n 'gender' src/test --type=java -B 2 -A 2Repository: Medicare-Call/Medicare-Call-Backend
Length of output: 16531
🏁 Script executed:
#!/bin/bash
echo "=== ElderRegisterResponse 전체 내용 ==="
cat -n src/main/java/com/example/medicare_call/dto/ElderRegisterResponse.java
echo ""
echo "=== Gender enum 전체 내용 (Jackson 애노테이션 확인) ==="
cat -n src/main/java/com/example/medicare_call/global/enums/Gender.java
echo ""
echo "=== ElderControllerTest의 gender 검증 부분 (160-200줄) ==="
sed -n '160,200p' src/test/java/com/example/medicare_call/controller/ElderControllerTest.java
echo ""
echo "=== ElderControllerTest 전체에서 gender 검증 ==="
rg -n 'gender.*value|\.gender\(' src/test/java/com/example/medicare_call/controller/ElderControllerTest.java -B2 -A2Repository: Medicare-Call/Medicare-Call-Backend
Length of output: 4262
🏁 Script executed:
#!/bin/bash
echo "=== ElderControllerTest bulkRegisterElders 테스트 전체 (160-250줄) ==="
sed -n '160,250p' src/test/java/com/example/medicare_call/controller/ElderControllerTest.java
echo ""
echo "=== mockMvc 응답 검증 패턴 찾기 ==="
rg -n 'andExpect.*gender|\.gender.*value' src/test/java/com/example/medicare_call/controller/ElderControllerTest.java -B3 -A3Repository: Medicare-Call/Medicare-Call-Backend
Length of output: 3943
gender 필드 응답 형식이 변경됩니다.
Gender enum을 DTO에 노출하면 Jackson의 기본 동작으로 "MALE", "FEMALE" 문자열로 직렬화됩니다. 기존 클라이언트가 다른 형식(예: 숫자 0, 1 또는 커스텀 코드)을 기대했다면 이번 PR에서 깨질 수 있습니다.
더해서, bulkRegisterElders 테스트가 gender 필드를 검증하지 않아서 응답 계약이 제대로 보호되지 않고 있습니다. 의도된 변경이라면 JSON 응답 format을 명시적으로 테스트하고 API 계약을 확정하는 게 좋습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/example/medicare_call/dto/ElderRegisterResponse.java` at
line 15, ElderRegisterResponse의 gender 필드를 그대로 enum으로 노출하면 JSON이
"MALE"/"FEMALE"로 직렬화되어 기존 클라이언트와 호환성 문제가 발생할 수 있으므로,
ElderRegisterResponse.gender를 기대되는 응답 포맷(예: 정수 코드 또는 커스텀 문자열)으로 변환하여 직렬화하도록 수정하고
관련 테스트를 추가하세요; 구체적으로는 ElderRegisterResponse 클래스에서 gender 타입을 String/Integer로
변경하거나 Gender enum에 `@JsonValue` 또는 커스텀 직렬화 로직을 추가(예:
getGenderCode()/getGenderValue() 직렬화용 접근자)하여 원하는 포맷을 반환하도록 구현하고,
bulkRegisterElders 테스트에 gender 필드의 JSON 표현(숫자 코드 또는 커스텀 코드)을 검증하는 assertions를
추가해 API 계약을 보호하세요.
Comment on lines
+15
to
+29
| import com.example.medicare_call.service.carecall.client.CareCallClient; | ||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| @Slf4j | ||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class CareCallTestService { | ||
|
|
||
| private final ElderRepository elderRepository; | ||
| private final CareCallSettingService careCallSettingService; | ||
| private final CareCallRequestSenderService careCallRequestSenderService; | ||
| private final CareCallClient careCallClient; |
There was a problem hiding this comment.
지금 상태로는 이 파일이 컴파일되지 않습니다.
CI 로그 그대로 CareCallClient import 경로가 실제 위치와 맞지 않고, CareCallRequestSenderService/CareCallSettingService도 현재 스코프에서 해석되지 않습니다. 관련 스니펫 기준으로 CareCallClient는 service.carecall.outbound.client 쪽에 있어 보여서, import와 패키지 정리를 먼저 맞춰야 합니다.
🧰 Tools
🪛 GitHub Actions: CI with Gradle
[error] 15-15: import com.example.medicare_call.service.carecall.client.CareCallClient; package does not exist
[error] 23-23: @RequiredArgsConstructor: symbol not found or annotation processor issue due to missing dependencies
[error] 27-29: cannot find symbol: CareCallSettingService, CareCallRequestSenderService, CareCallClient
[error] 27-29: Compilation failed; see the compiler output below.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/example/medicare_call/service/carecall/CareCallTestService.java`
around lines 15 - 29, CareCallTestService fails to compile because some imports
and package references are incorrect; update the CareCallClient import to its
actual package (e.g.
com.example.medicare_call.service.carecall.outbound.client.CareCallClient) and
add or correct imports for ElderRepository, CareCallSettingService, and
CareCallRequestSenderService from their real packages so the fields in
CareCallTestService resolve; ensure the file's package declaration matches the
directory and run a compile to confirm all symbols (CareCallTestService,
ElderRepository, CareCallSettingService, CareCallRequestSenderService,
CareCallClient) are correctly imported.
Comment on lines
+37
to
+45
| try { | ||
| CareCallSetting setting = careCallSettingService.getOrCreateImmediateSetting(elder); | ||
| CallType callType = convertOptionToCallType(careCallOption); | ||
|
|
||
| careCallRequestSenderService.sendCall(setting.getId(), elderId.intValue(), callType); | ||
| return String.format("%s 어르신께 즉시 케어콜 발송이 완료되었습니다.", elder.getName()); | ||
| } catch (Exception e) { | ||
| log.error("즉시 케어콜 발송 실패 - elderId: {}, error: {}", elder.getId(), e.getMessage()); | ||
| throw new CustomException(ErrorCode.INTERNAL_SERVER_ERROR, "케어콜 발송 중 오류가 발생했습니다: " + e.getMessage()); |
There was a problem hiding this comment.
예외를 전부 500으로 덮어쓰면 원래 에러 의미가 사라집니다.
여기서 CustomException까지 한 번 더 감싸면 원래의 ErrorCode가 사라지고, 응답 메시지에 내부 예외 문자열도 그대로 노출됩니다. CustomException은 그대로 재던지고, 예상 못 한 예외만 500으로 변환하는 쪽이 안전해요.
가능한 수정 예시
try {
CareCallSetting setting = careCallSettingService.getOrCreateImmediateSetting(elder);
CallType callType = convertOptionToCallType(careCallOption);
careCallRequestSenderService.sendCall(setting.getId(), elderId.intValue(), callType);
return String.format("%s 어르신께 즉시 케어콜 발송이 완료되었습니다.", elder.getName());
+ } catch (CustomException e) {
+ throw e;
} catch (Exception e) {
- log.error("즉시 케어콜 발송 실패 - elderId: {}, error: {}", elder.getId(), e.getMessage());
- throw new CustomException(ErrorCode.INTERNAL_SERVER_ERROR, "케어콜 발송 중 오류가 발생했습니다: " + e.getMessage());
+ log.error("즉시 케어콜 발송 실패 - elderId: {}", elder.getId(), e);
+ throw new CustomException(ErrorCode.INTERNAL_SERVER_ERROR, "케어콜 발송 중 오류가 발생했습니다.");
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/example/medicare_call/service/carecall/CareCallTestService.java`
around lines 37 - 45, The catch-all currently wraps every Exception into a new
CustomException with INTERNAL_SERVER_ERROR and exposes e.getMessage(); update
CareCallTestService so you rethrow existing CustomException unchanged, and only
catch other unexpected Exceptions to wrap as new
CustomException(ErrorCode.INTERNAL_SERVER_ERROR, "케어콜 발송 중 오류가 발생했습니다.") without
leaking internal messages; ensure you still log the full exception (use
log.error with the exception object) for
careCallSettingService.getOrCreateImmediateSetting /
careCallRequestSenderService.sendCall failures before throwing the sanitized
CustomException.
Comment on lines
+49
to
+62
| public void sendTestCall(CareCallTestRequest req) { | ||
| // 테스트용이라 하드코딩된 더미 데이터 사용, DB 저장 안함 | ||
| Elder testElder = Elder.builder() | ||
| .id(100) | ||
| .name("김옥자") // 테스트 이름 | ||
| .phone("01011111111") | ||
| .gender(Gender.MALE) | ||
| .relationship(ElderRelation.CHILD) | ||
| .residenceType(ResidenceType.ALONE) | ||
| .build(); | ||
|
|
||
| String testPrompt = req.prompt(); | ||
| // 테스트 호출은 settingId 100으로 가정 | ||
| careCallClient.requestCall(100, testElder.getId(), req.phoneNumber(), testPrompt); |
There was a problem hiding this comment.
하드코딩된 settingId/elderId로 실제 외부 호출을 보내는 건 위험합니다.
requestCall()은 이 값을 검증 없이 payload에 넣어 바로 전송합니다. 100이 실제 레코드와 겹치면 테스트 콜이 실데이터에 귀속될 수 있어서, 최소한 비운영 프로필로 제한하거나 sandbox 전용 식별자 체계를 분리해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/example/medicare_call/service/carecall/CareCallTestService.java`
around lines 49 - 62, The sendTestCall method currently calls
careCallClient.requestCall with hardcoded IDs (settingId 100 and
testElder.getId()) which can target real records; change sendTestCall to avoid
using real IDs by introducing a dedicated sandbox/test identifier scheme and a
runtime environment guard: replace the literal 100 and the hardcoded testElder
id with a constant like SANDBOX_SETTING_ID and SANDBOX_ELDER_ID (or generate
negative/UUID test IDs) and ensure a check (e.g., isNonProdProfile or
req.isSandbox()) prevents execution unless running in a non-production/sandbox
mode; additionally, if careCallClient supports a sandbox flag or endpoint, pass
that through the requestCall invocation or add validation in CareCallTestService
to enforce sandbox routing and reject attempts to send test calls in production.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Docs
Gender와 SubscriptionPlan 타입을 기존 Byte(숫자) 기반에서 순수 Enum(String) 기반으로 리팩토링 완료
Enum 클래스 정비: Enum 내부의 불필요한 byte code, 생성자, fromCode() 메서드를 모두 제거
엔티티 매핑: @Enumerated(EnumType.STRING)을 적용해서 DB에 숫자 대신 'MALE', 'STANDARD' 같은 문자열이 저장되도록 바꿈
DB 마이그레이션: Flyway(V35)를 통해 기존 TINYINT 데이터를 ENUM 타입으로 안전하게 전환
테스트 코드: 기존에 (byte) 0 처럼 숫자로 밀어넣던 테스트 케이스들을 전부 Enum 상수를 사용하도록 수정
리팩 내용이 많아서 pr나눠서 올립니다
Summary by CodeRabbit
릴리스 노트