refactor: BloodSugarMeasurementType byte->enum 비즈니스 로직 수정 - #227
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughBloodSugarData DTO의 필드를 mealTime (String)에서 measurementType (BloodSugarMeasurementType)로 변경했습니다. 관련 enum을 단순화하고, 서비스 로직과 테스트를 이 변경에 맞게 업데이트했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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🧪 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: 2
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/medicare_call/dto/data_processor/HealthDataExtractionResponse.java (1)
106-111:⚠️ Potential issue | 🟠 Major
measurementType스키마 설명이 실제 enum 값과 불일치합니다현재 타입은
BloodSugarMeasurementType인데, 스키마 예시/허용값은"식전","식후"로 남아 있어 API 계약이 어긋납니다. 문서/프롬프트/클라이언트가 잘못된 값을 생성할 가능성이 큽니다. enum 값 기준으로 스키마를 맞추거나, 별도 매핑 계층을 명시해 주세요.🔧 제안 diff
`@Schema`( description = "식전/식후 여부", - example = "식후", - allowableValues = {"식전", "식후"} + example = "AFTER_MEAL", + allowableValues = {"BEFORE_MEAL", "AFTER_MEAL"} ) private BloodSugarMeasurementType measurementType;🤖 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/data_processor/HealthDataExtractionResponse.java` around lines 106 - 111, The `@Schema` on HealthDataExtractionResponse.measurementType is inconsistent with the actual BloodSugarMeasurementType enum; update the schema to reflect the enum constants (or add an explicit mapping) so API docs match runtime values: locate the measurementType field in HealthDataExtractionResponse and either replace the example/allowableValues ("식전","식후") with the actual BloodSugarMeasurementType constant names (or their string representations returned by the enum), or add a clear mapping layer (e.g., a DTO/string field or a custom `@Schema` description) that documents the enum-to-localized value mapping; ensure the chosen approach updates example, allowableValues and description to reference BloodSugarMeasurementType so clients and prompts receive correct values.
🧹 Nitpick comments (1)
src/main/java/com/example/medicare_call/service/health_data/BloodSugarService.java (1)
54-55: 로그 필드명mealTime은 현재 의미와 맞지 않습니다실제 출력 값은
measurementType인데 키가mealTime으로 남아 있어 운영 로그 해석이 헷갈릴 수 있습니다.🧹 제안 diff
- log.info("혈당 데이터 저장 완료: value={}, mealTime={}, status={}", - bloodSugarData.getBloodSugarValue(), bloodSugarData.getMeasurementType(), status); + log.info("혈당 데이터 저장 완료: value={}, measurementType={}, status={}", + bloodSugarData.getBloodSugarValue(), bloodSugarData.getMeasurementType(), status);🤖 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/health_data/BloodSugarService.java` around lines 54 - 55, The log field name "mealTime" is incorrect for the value being logged; update the log message in BloodSugarService so the placeholder key matches the actual value: change the key from mealTime to measurementType (or another accurate name) in the log.info call that uses bloodSugarData.getMeasurementType() and status, ensuring the call in the BloodSugarService class still passes bloodSugarData.getBloodSugarValue(), bloodSugarData.getMeasurementType(), status in the same order; also scan for other occurrences of "mealTime" in BloodSugarService to keep logging consistent.
🤖 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/health_data/BloodSugarService.java`:
- Around line 35-36: BloodSugarMeasurementType returned by
bloodSugarData.getMeasurementType() can be null so calling
measurementType.name() may throw NPE; update BloodSugarService to guard this by
checking measurementType for null before using name() (either skip
processing/return early with a logged warning or substitute a sensible default
enum value), e.g., in the code paths around where measurementType is used (the
getMeasurementType() call and subsequent measurementType.name() usages) add a
null-check/Optional handling and ensure you handle/save accordingly.
In
`@src/test/java/com/example/medicare_call/service/carecall/analysis/HealthDataExtractionIntegrationTest.java`:
- Line 121: The test only asserts measurementTime and the mock JSON still uses
the old mealTime field, so update the test and mock payload to exercise the new
enum-based field: change the mock JSON to include "measurementType" with the
appropriate enum string (instead of "mealTime"), and in
HealthDataExtractionIntegrationTest update the assertion on bloodSugar to assert
the measurementType (e.g., compare to the expected MeasurementType enum value)
rather than or in addition to measurementTime so the enum mapping is validated
(reference the bloodSugar variable and the measurementType
property/MeasurementType enum in your changes).
---
Outside diff comments:
In
`@src/main/java/com/example/medicare_call/dto/data_processor/HealthDataExtractionResponse.java`:
- Around line 106-111: The `@Schema` on
HealthDataExtractionResponse.measurementType is inconsistent with the actual
BloodSugarMeasurementType enum; update the schema to reflect the enum constants
(or add an explicit mapping) so API docs match runtime values: locate the
measurementType field in HealthDataExtractionResponse and either replace the
example/allowableValues ("식전","식후") with the actual BloodSugarMeasurementType
constant names (or their string representations returned by the enum), or add a
clear mapping layer (e.g., a DTO/string field or a custom `@Schema` description)
that documents the enum-to-localized value mapping; ensure the chosen approach
updates example, allowableValues and description to reference
BloodSugarMeasurementType so clients and prompts receive correct values.
---
Nitpick comments:
In
`@src/main/java/com/example/medicare_call/service/health_data/BloodSugarService.java`:
- Around line 54-55: The log field name "mealTime" is incorrect for the value
being logged; update the log message in BloodSugarService so the placeholder key
matches the actual value: change the key from mealTime to measurementType (or
another accurate name) in the log.info call that uses
bloodSugarData.getMeasurementType() and status, ensuring the call in the
BloodSugarService class still passes bloodSugarData.getBloodSugarValue(),
bloodSugarData.getMeasurementType(), status in the same order; also scan for
other occurrences of "mealTime" in BloodSugarService to keep logging consistent.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/example/medicare_call/dto/data_processor/HealthDataExtractionResponse.javasrc/main/java/com/example/medicare_call/global/enums/BloodSugarMeasurementType.javasrc/main/java/com/example/medicare_call/service/health_data/BloodSugarService.javasrc/test/java/com/example/medicare_call/global/enums/BloodSugarMeasurementTypeTest.javasrc/test/java/com/example/medicare_call/service/carecall/analysis/HealthDataExtractionIntegrationTest.javasrc/test/java/com/example/medicare_call/service/health_data/BloodSugarServiceTest.java
💤 Files with no reviewable changes (1)
- src/test/java/com/example/medicare_call/global/enums/BloodSugarMeasurementTypeTest.java
| BloodSugarMeasurementType measurementType = bloodSugarData.getMeasurementType(); | ||
|
|
There was a problem hiding this comment.
measurementType null 가드가 없어 런타임 NPE가 발생할 수 있습니다
measurementType이 비어 있으면 name() 호출 시 바로 예외가 납니다. 저장 스킵/기본값 처리 중 하나를 명시적으로 넣어 주세요.
🛠️ 제안 diff
- // measurementType 결정 (식전/식후)
- BloodSugarMeasurementType measurementType = bloodSugarData.getMeasurementType();
+ // measurementType 결정 (식전/식후)
+ BloodSugarMeasurementType measurementType = bloodSugarData.getMeasurementType();
+ if (measurementType == null) {
+ log.warn("혈당 측정 유형이 없어서 저장하지 않습니다. measurementTime={}", bloodSugarData.getMeasurementTime());
+ continue;
+ }
@@
BloodSugarRecord bloodSugarRecord = BloodSugarRecord.builder()
@@
.responseSummary(String.format("측정시각: %s, 식전/식후: %s",
- bloodSugarData.getMeasurementTime(), bloodSugarData.getMeasurementType().name()))
+ bloodSugarData.getMeasurementTime(), measurementType.name()))
.build();Also applies to: 49-50
🤖 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/health_data/BloodSugarService.java`
around lines 35 - 36, BloodSugarMeasurementType returned by
bloodSugarData.getMeasurementType() can be null so calling
measurementType.name() may throw NPE; update BloodSugarService to guard this by
checking measurementType for null before using name() (either skip
processing/return early with a logged warning or substitute a sensible default
enum value), e.g., in the code paths around where measurementType is used (the
getMeasurementType() call and subsequent measurementType.name() usages) add a
null-check/Optional handling and ensure you handle/save accordingly.
| HealthDataExtractionResponse.BloodSugarData bloodSugar = result.getBloodSugarData().get(0); | ||
| assertThat(bloodSugar.getBloodSugarValue()).isEqualTo(120); | ||
| assertThat(bloodSugar.getMealTime()).isEqualTo("식후"); | ||
| assertThat(bloodSugar.getMeasurementTime()).isEqualTo("아침"); |
There was a problem hiding this comment.
핵심 변경점인 measurementType 검증이 빠져 회귀를 놓칠 수 있습니다
지금은 measurementTime만 확인하고, mock JSON도 구 필드(mealTime)를 사용하고 있어 enum 전환이 실제로 동작하는지 보장하지 못합니다. payload와 assertion을 measurementType 기준으로 맞춰 주세요.
✅ 제안 diff
+import com.example.medicare_call.global.enums.BloodSugarMeasurementType;
@@
- "mealTime": "식후",
+ "measurementType": "AFTER_MEAL",
@@
assertThat(bloodSugar.getBloodSugarValue()).isEqualTo(120);
assertThat(bloodSugar.getMeasurementTime()).isEqualTo("아침");
+ assertThat(bloodSugar.getMeasurementType()).isEqualTo(BloodSugarMeasurementType.AFTER_MEAL);📝 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.
| assertThat(bloodSugar.getMeasurementTime()).isEqualTo("아침"); | |
| assertThat(bloodSugar.getMeasurementTime()).isEqualTo("아침"); | |
| assertThat(bloodSugar.getMeasurementType()).isEqualTo(BloodSugarMeasurementType.AFTER_MEAL); |
🤖 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/carecall/analysis/HealthDataExtractionIntegrationTest.java`
at line 121, The test only asserts measurementTime and the mock JSON still uses
the old mealTime field, so update the test and mock payload to exercise the new
enum-based field: change the mock JSON to include "measurementType" with the
appropriate enum string (instead of "mealTime"), and in
HealthDataExtractionIntegrationTest update the assertion on bloodSugar to assert
the measurementType (e.g., compare to the expected MeasurementType enum value)
rather than or in addition to measurementTime so the enum mapping is validated
(reference the bloodSugar variable and the measurementType
property/MeasurementType enum in your changes).
개인적으로는 mealTime -> measurementType으로 Enum 이름하고 통일해서 |
631aa2c to
b74c2c7
Compare
Desc
HealthDataExtractionResponse 클래스의 BloodSugarData 에서 변수명이
위와 같이 되어있는데 테스트 수정하다보니 헷갈리는것 같네요. 어떤 식으로 수정하면 좋을까요?