[Feat] 약 상세 정보 조회 API + 메모장 자동 생성 기능 추가 - #54
Conversation
- MedicationCreateRequest에 memo 필드 추가 (복용방법 + 보관방법)
- MedicationCommandService에서 memo 저장 처리
- MedicationQueryService 신규 생성 — GET /api/medications/{id}/detail
- MedicationDetailResponse 신규 생성 (efcyQesitm, useMethodQesitm, atpnQesitm, seQesitm)
- MedicationConverter에 toDetailResponse 변환 메서드 추가
- SecurityConfig에 GET /api/medications/** permitAll 추가
- DrugInfoController Swagger 설명 보완
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- .claude/rules/git.md 신규 생성 (globs 없이 항상 로드) - 커밋 타이밍, 레이어별 분리 순서, 메시지 작성 규칙 포함 - CLAUDE.md에 git.md 참조 추가 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough약 상세 정보 조회 API( Changes약 상세 조회 및 메모 기능
Estimated code review effort: 2 (Simple) | ~15 minutes 문서 및 설명 문구 업데이트
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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: 2
🧹 Nitpick comments (1)
src/main/java/com/piuda/callcare/domain/medication/controller/MedicationController.java (1)
20-20: 📐 Maintainability & Code Quality | 🔵 Trivial
@Tagdescription이 상세 조회 엔드포인트 추가를 반영하지 않습니다
@Tag(description = "약 등록 API")에 상세 조회 기능이 추가되었으므로 설명 업데이트를 권장합니다.♻️ 제안
-@Tag(name = "Medication", description = "약 등록 API") +@Tag(name = "Medication", description = "약 등록 및 상세 조회 API")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/piuda/callcare/domain/medication/controller/MedicationController.java` at line 20, Update the Swagger/OpenAPI `@Tag` on MedicationController so its description reflects both medication registration and the newly added detail 조회 endpoint. Adjust the `@Tag` annotation near MedicationController to use a broader, accurate description that covers all exposed medication APIs, not just registration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/piuda/callcare/domain/medication/converter/MedicationConverter.java`:
- Around line 16-29: There is a mismatch between the PR goal and
`MedicationConverter.toDetailResponse`: when `drugInfo` is null, the method
currently returns a partially populated `MedicationDetailResponse` with
`drugName`, which conflicts with the stated behavior and the
`MedicationDetailResponse` schema. Please resolve this by choosing one approach
explicitly: either make `toDetailResponse` return null when
`medication.getDrugInfo()` is null, or keep `drugName` populated and update the
`MedicationDetailResponse` schema/PR intent to match that behavior.
In `@src/main/java/com/piuda/callcare/global/config/SecurityConfig.java`:
- Line 47: `GET /api/medications/**` is currently exposed via `permitAll`, which
leaves medication detail lookup vulnerable to IDOR; update `SecurityConfig` to
stop allowing anonymous access here and add the same temporary TODO-style note
used for `/api/home/**` and `/api/conflicts/**` if you need to preserve the
existing pattern. Also make sure `MedicationQueryService.getDetail` and the
corresponding `MedicationController` path enforce `userId`-based ownership
checks so the detail lookup is scoped to the authenticated user rather than a
sequential `medicationId`.
---
Nitpick comments:
In
`@src/main/java/com/piuda/callcare/domain/medication/controller/MedicationController.java`:
- Line 20: Update the Swagger/OpenAPI `@Tag` on MedicationController so its
description reflects both medication registration and the newly added detail 조회
endpoint. Adjust the `@Tag` annotation near MedicationController to use a broader,
accurate description that covers all exposed medication APIs, not just
registration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2a3bc016-c1f2-4131-9a12-ca87214671b4
📒 Files selected for processing (10)
.claude/rules/git.mdCLAUDE.mdsrc/main/java/com/piuda/callcare/domain/druginfo/controller/DrugInfoController.javasrc/main/java/com/piuda/callcare/domain/medication/controller/MedicationController.javasrc/main/java/com/piuda/callcare/domain/medication/converter/MedicationConverter.javasrc/main/java/com/piuda/callcare/domain/medication/dto/request/MedicationCreateRequest.javasrc/main/java/com/piuda/callcare/domain/medication/dto/response/MedicationDetailResponse.javasrc/main/java/com/piuda/callcare/domain/medication/service/command/MedicationCommandService.javasrc/main/java/com/piuda/callcare/domain/medication/service/query/MedicationQueryService.javasrc/main/java/com/piuda/callcare/global/config/SecurityConfig.java
| // Medication → MedicationDetailResponse (약 상세 조회용, DrugInfo 없으면 전 필드 null) | ||
| public MedicationDetailResponse toDetailResponse(Medication medication) { | ||
| DrugInfo drugInfo = medication.getDrugInfo(); | ||
| if (drugInfo == null) { | ||
| return new MedicationDetailResponse(medication.getDrugName(), null, null, null, null); | ||
| } | ||
| return new MedicationDetailResponse( | ||
| medication.getDrugName(), | ||
| drugInfo.getEfcyQesitm(), | ||
| drugInfo.getUseMethodQesitm(), | ||
| drugInfo.getAtpnQesitm(), | ||
| drugInfo.getSeQesitm() | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
PR 목표와 구현이 불일치합니다 — DrugInfo 미연결 시 응답 처리 방식 확인 필요
PR 목표는 "DrugInfo가 연결되지 않은 경우 전체 상세 응답을 null로 반환"이라고 명시하지만, 현재 구현은 drugName을 포함한 non-null 응답을 반환합니다. 또한 MedicationDetailResponse의 @Schema 설명인 "DrugInfo 미연결 시 전 필드 null"과도 일치하지 않습니다 (drugName이 설정됨).
두 가지 해석이 가능합니다:
- PR 목표대로
drugInfo == null일 때null반환 - 현재 구현대로
drugName은 유지하되 Schema/PR 목표 업데이트
💡 옵션 1: PR 목표에 맞추는 경우
if (drugInfo == null) {
- return new MedicationDetailResponse(medication.getDrugName(), null, null, null, null);
+ return null;
}💡 옵션 2: 현재 구현을 유지하는 경우 — Schema 설명 수정
-@Schema(description = "약 상세 정보 응답 — DrugInfo 미연결 시 전 필드 null")
+@Schema(description = "약 상세 정보 응답 — DrugInfo 미연결 시 DrugInfo 관련 필드는 null, drugName은 Medication에서 반환")Based on learnings, if multiple interpretations of a task exist, present the alternatives rather than choosing silently.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/piuda/callcare/domain/medication/converter/MedicationConverter.java`
around lines 16 - 29, There is a mismatch between the PR goal and
`MedicationConverter.toDetailResponse`: when `drugInfo` is null, the method
currently returns a partially populated `MedicationDetailResponse` with
`drugName`, which conflicts with the stated behavior and the
`MedicationDetailResponse` schema. Please resolve this by choosing one approach
explicitly: either make `toDetailResponse` return null when
`medication.getDrugInfo()` is null, or keep `drugName` populated and update the
`MedicationDetailResponse` schema/PR intent to match that behavior.
Source: Learnings
| .requestMatchers(HttpMethod.POST, "/api/conflicts/**").permitAll() | ||
| .requestMatchers(HttpMethod.POST, "/api/ocr/**").permitAll() | ||
| .requestMatchers(HttpMethod.POST, "/api/medications/batch").permitAll() | ||
| .requestMatchers(HttpMethod.GET, "/api/medications/**").permitAll() |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
GET /api/medications/** permitAll — IDOR 위험 및 TODO 주석 누락
GET /api/medications/**가 permitAll로 설정되어 인증 없이 모든 약 상세 정보에 접근 가능합니다. medicationId가 순차적 DB ID이므로 열거 공격으로 타 사용자의 약 정보(drugName 등)가 노출됩니다.
/api/home/**, /api/conflicts/**에는 인증 필터 도입 시 제거하겠다는 TODO 주석이 있지만, medications에는 누락되어 있습니다. 또한 MedicationQueryService.getDetail에 userId 기반 소유권 검증이 없어 인증 도입 시에도 IDOR이 해결되지 않습니다.
🔒 권장 사항
1. TODO 주석 추가 (즉시, 기존 패턴 일관성):
+ // TODO: 인증 필터 도입 시 제거하고, 로그인 사용자의 seniorId 소유권 검증으로 전환
.requestMatchers(HttpMethod.GET, "/api/medications/**").permitAll()2. 인증 도입 시 소유권 검증 추가 (MedicationQueryService + MedicationController):
// MedicationController
public ResponseEntity<ApiResponse<MedicationDetailResponse>> getDetail(
+ `@AuthenticationPrincipal` Long userId,
`@PathVariable` Long medicationId
) {
- return ResponseUtils.ok(medicationQueryService.getDetail(medicationId));
+ return ResponseUtils.ok(medicationQueryService.getDetail(medicationId, userId));
} // MedicationQueryService
-public MedicationDetailResponse getDetail(Long medicationId) {
+public MedicationDetailResponse getDetail(Long medicationId, Long userId) {
Medication medication = medicationRepository.findById(medicationId)
.orElseThrow(() -> new CallCareException(ErrorCode.MEDICATION_NOT_FOUND));
+ if (!medication.getSenior().getUser().getId().equals(userId)) {
+ throw new CallCareException(ErrorCode.FORBIDDEN);
+ }
return medicationConverter.toDetailResponse(medication);
}📝 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.
| .requestMatchers(HttpMethod.GET, "/api/medications/**").permitAll() | |
| // TODO: 인증 필터 도입 시 제거하고, 로그인 사용자의 seniorId 소유권 검증으로 전환 | |
| .requestMatchers(HttpMethod.GET, "/api/medications/**").permitAll() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/piuda/callcare/global/config/SecurityConfig.java` at line
47, `GET /api/medications/**` is currently exposed via `permitAll`, which leaves
medication detail lookup vulnerable to IDOR; update `SecurityConfig` to stop
allowing anonymous access here and add the same temporary TODO-style note used
for `/api/home/**` and `/api/conflicts/**` if you need to preserve the existing
pattern. Also make sure `MedicationQueryService.getDetail` and the corresponding
`MedicationController` path enforce `userId`-based ownership checks so the
detail lookup is scoped to the authenticated user rather than a sequential
`medicationId`.
DrugInfo 미연결 시 의약품 정보 필드만 null, drugName은 항상 반환 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
userId가 non-null인 경우 medication → senior → user 경로로 소유권 검증 null이면 로컬 테스트용으로 통과 처리 (permitAll 유지 구간) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
userId를 service로 전달하여 소유권 검증 수행 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
🔍️ 작업 내용
Closes #52
✨ 상세 설명
약 상세 정보 조회 API 추가
GET /api/medications/{id}/detail 신규 엔드포인트
MedicationQueryService 신규 생성
MedicationDetailResponse 신규 생성 — efcyQesitm(효능), useMethodQesitm(용법), atpnQesitm(주의사항), seQesitm(부작용) 반환
MedicationConverter에 toDetailResponse 변환 메서드 추가
메모장 자동 생성 관련
MedicationCreateRequest에 memo 필드 추가 (복용방법 + 보관방법)
MedicationCommandService에서 memo 저장 처리
기타
DrugInfoController Swagger 설명 보완
🛠️ 추후 리팩토링 및 고도화 계획
memo 자동 생성 로직 고도화 (약물 데이터 존재 여부에 따른 조건부 생성)
📸 스크린샷 (선택)
💬 리뷰 요구사항
Summary by CodeRabbit
New Features
Bug Fixes
Documentation