Merged
Conversation
Walkthrough입고 아이템 조회 기능을 구현했습니다. 새로운 GET 엔드포인트, 서비스 메서드, 성공 메시지를 추가하고 에러 메시지를 개선했으며, 성공/실패 시나리오에 대한 테스트를 작성했습니다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Controller as ReceiptController
participant Service as ReceiptService
participant Repo as ReceiptItemRepository
Client->>Controller: GET /receipt/{receiptItemId}
activate Controller
Note over Controller: 사용자 인증 확인
Controller->>Service: getReceiptItem(receiptItemId, userId)
activate Service
Service->>Service: 사용자 조회
Service->>Service: 상점 파생
Service->>Service: 상점별 아이템 접근 권한 검증
Service->>Repo: findReceiptItemByIdAndValidateStoreAccess()
activate Repo
Repo-->>Service: ReceiptItem
deactivate Repo
Service->>Service: ReceiptItemResponse로 변환
Service-->>Controller: ReceiptItemResponse
deactivate Service
Controller-->>Client: ApiResponse<ReceiptItemResponse>
deactivate Controller
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 추가 검토 항목:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
🧹 Nitpick comments (4)
src/main/java/com/almang/inventory/receipt/service/ReceiptService.java (1)
148-158: 입고 아이템 조회 로직은 적절하고, 로그 메시지만 약간 손보면 좋겠습니다
getReceiptItem/findReceiptItemByIdAndValidateStoreAccess구조는 기존findReceiptByIdAndValidateAccess패턴과 일관되고,Store기준 접근 제어도 올바르게 구현되어 있습니다. 도메인/권한 관점에서 큰 문제는 없습니다.- 다만 Line 156에서 성공 로그 메시지가
"[ReceiptService] 입고 아이템 조회 요청 - receiptId: {}"로 되어 있고, 실제로는receiptItem.getId()를 찍고 있어서 운영 로그에서 혼동이 생길 수 있습니다. 성공 로그 문구와 필드명을 receipt item 기준으로 맞추는 편이 좋겠습니다.예시 수정안입니다:
- log.info("[ReceiptService] 입고 아이템 조회 요청 - receiptId: {}", receiptItem.getId()); + log.info("[ReceiptService] 입고 아이템 조회 성공 - receiptItemId: {}", receiptItem.getId());이렇게 하면 요청/성공 로그가 각각 무엇을 의미하는지, 어떤 ID를 남기는지 한눈에 구분되기 때문에 운영 중 트러블슈팅이 훨씬 수월해집니다.
Also applies to: 311-319
src/test/java/com/almang/inventory/receipt/service/ReceiptServiceTest.java (1)
21-22: 입고 아이템 조회 서비스 테스트 커버리지는 충분하며, 중복만 추후에 정리해도 좋겠습니다
- 성공/실패 4가지 시나리오가 모두 잘 분리되어 있고, 특히 성공 케이스에서 실제
Receipt/ReceiptItem를 영속화한 뒤ReceiptItemResponse의 각 필드를 검증하는 점이 좋습니다. 서비스 로직과 DTO 매핑 양쪽을 동시에 검증하고 있습니다.- 실패 케이스에서 각각
USER_NOT_FOUND,RECEIPT_ITEM_NOT_FOUND,RECEIPT_ITEM_ACCESS_DENIED를 명시적으로 기대하는 것도 ErrorCode 계약을 잘 고정해 줍니다.다만 이 파일 전체가 그렇듯이, 이번에 추가된 테스트도
Store/User/Vendor/Order/Receipt/ReceiptItem생성 코드가 반복되는 편입니다. 당장은 유지보수에 큰 부담은 아니지만, 나중에라도 다음처럼 헬퍼를 한 단계 추상화하면 테스트 가독성이 꽤 좋아질 것 같습니다.
- 예:
Receipt newReceiptWithItems(Store store, Vendor vendor)같은 팩토리 메서드 추가- 또는
createReceiptWithItemsFor(Store store, String username)정도의 상위 수준 DSL 메서드 도입현재 PR 범위를 고려하면 필수는 아니고, 추후 리팩터링 후보로만 봐도 충분해 보입니다.
Also applies to: 982-1088
src/test/java/com/almang/inventory/receipt/controller/ReceiptControllerTest.java (1)
782-869: 입고 아이템 조회 컨트롤러 테스트는 계약을 잘 고정하고 있으며, 엔드포인트 네이밍만 한 번 고민해 볼 만합니다
SuccessMessage.GET_RECEIPT_ITEM_SUCCESS와ErrorCode들(존재하지 않음/권한 없음)에 대해 HTTP 상태 코드와 응답 바디 구조를 모두 검증하고 있어서, 컨트롤러와 전역 예외 핸들러 사이 계약이 잘 고정되어 있습니다. 테스트 구성 자체는 매우 좋습니다.- 한 가지, 엔드포인트가 최종적으로
/api/v1/receipt/receipt/{receiptItemId}형태가 되는데, 동일 리소스 명이 두 번 반복되는 URI는 가독성이 조금 떨어질 수 있습니다.
- 예:
/api/v1/receipt/item/{receiptItemId}또는/api/v1/receipt/items/{receiptItemId}처럼 리소스 계층을 분리하는 것도 선택지입니다.- 지금 구현/테스트는 일관되게 이 경로를 사용하고 있으므로 동작 상 문제는 없고, 향후 API 외부 공개 범위와 버전 전략을 보면서 필요하면 리팩터링(또는 alias 경로 추가)을 고려하셔도 좋겠습니다.
src/main/java/com/almang/inventory/receipt/controller/ReceiptController.java (1)
10-11: 입고 아이템 조회 엔드포인트 구현은 깔끔하며, URI 구조만 약간 더 다듬을 여지가 있습니다
- 구현 패턴이 기존 메서드(
getReceipt,getReceiptFromOrder)와 거의 동일해서, 인증 주체 → userId 추출 → 서비스 호출 →ApiResponse.success흐름이 잘 맞습니다.SuccessMessage.GET_RECEIPT_ITEM_SUCCESS사용도 적절합니다.- 다만 클래스 레벨
@RequestMapping("/api/v1/receipt")+ 메서드@GetMapping("/receipt/{receiptItemId}")조합으로 실제 URI가/api/v1/receipt/receipt/{receiptItemId}가 됩니다. 의미적으로는 맞지만, 리소스명이 이중으로 들어가서 약간 장황하게 느껴질 수 있습니다.
- 향후 API 외부 공개/버전 호환성을 고려하면, 예를 들어
/api/v1/receipt/item/{receiptItemId}또는/api/v1/receipt/items/{receiptItemId}같은 형태로 바꾸는 것도 고민해볼 만합니다.- 지금 단계에서 기능적으로 문제는 없으니, 단기적으로는 그대로 사용하시되 다음 큰 API 리팩터링 타이밍에 URI 구조를 정리하는 후보로 올려두시면 좋겠습니다.
Also applies to: 138-151
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/almang/inventory/global/api/SuccessMessage.java(1 hunks)src/main/java/com/almang/inventory/global/exception/ErrorCode.java(1 hunks)src/main/java/com/almang/inventory/receipt/controller/ReceiptController.java(2 hunks)src/main/java/com/almang/inventory/receipt/service/ReceiptService.java(3 hunks)src/test/java/com/almang/inventory/receipt/controller/ReceiptControllerTest.java(1 hunks)src/test/java/com/almang/inventory/receipt/service/ReceiptServiceTest.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-22T18:12:13.161Z
Learnt from: JoonKyoLee
Repo: almang2/inventory-server PR: 66
File: src/main/java/com/almang/inventory/order/domain/OrderItem.java:41-53
Timestamp: 2025-11-22T18:12:13.161Z
Learning: In the almang2/inventory-server repository, OrderItem entity update methods (updateQuantity, updatePrice in src/main/java/com/almang/inventory/order/domain/OrderItem.java) do not require null checks because OrderService will validate parameters before calling these update methods, following the same pattern as Product entity updates.
<!--
Applied to files:
src/main/java/com/almang/inventory/receipt/controller/ReceiptController.javasrc/test/java/com/almang/inventory/receipt/service/ReceiptServiceTest.javasrc/main/java/com/almang/inventory/receipt/service/ReceiptService.java
🔇 Additional comments (2)
src/main/java/com/almang/inventory/global/exception/ErrorCode.java (1)
54-54: 입고 아이템 권한 에러 메시지 방향성 좋습니다기존 도메인 제약 설명형 문구보다, 지금처럼 권한 중심의 일반화된
"입고 아이템에 접근할 수 없습니다."메시지가 다양한 접근 거부 케이스(다른 상점, 잘못된 연관 등)에 재사용하기 훨씬 좋습니다. 별도 수정 필요 없어 보입니다.src/main/java/com/almang/inventory/global/api/SuccessMessage.java (1)
50-58: 입고 아이템 조회 성공 메시지 추가, 기존 패턴과 잘 맞습니다
GET_RECEIPT_ITEM_SUCCESS네이밍과"입고 아이템 조회 성공"문구가GET_ORDER_ITEM_SUCCESS와 대칭이라, API 응답 레벨에서 도메인 간 일관성이 잘 유지되고 있습니다. 별도 수정 없이 이대로 사용해도 좋겠습니다.
This was referenced Nov 24, 2025
Merged
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
1 participant
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.
✨ 작업 내용
📝 적용 범위
/receipt📌 참고 사항
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항
테스트
✏️ Tip: You can customize this high-level summary in your review settings.