Merged
Conversation
Walkthrough발주 상세 조회 기능을 구현합니다. 컨트롤러에 Changes
Sequence DiagramsequenceDiagram
participant Client
participant Controller as OrderController
participant Service as OrderService
participant Repo as Repository
participant DB as Database
Client->>Controller: GET /api/v1/order/item/{orderItemId}
Controller->>Controller: Extract userId from principal
Controller->>Service: getOrderItem(orderItemId, userId)
Service->>Repo: findOrderItemById(orderItemId)
Repo->>DB: Query OrderItem
DB-->>Repo: OrderItem or empty
alt Item not found
Repo-->>Service: empty
Service-->>Controller: throw ORDER_ITEM_NOT_FOUND
Controller-->>Client: 404 Not Found
else Item found
Repo-->>Service: OrderItem
Service->>Repo: findStoreByUserId(userId)
Repo->>DB: Query Store
DB-->>Repo: Store
Service->>Service: validateOrderItemAccess(orderItem, store)
alt Access denied
Service-->>Controller: throw ORDER_ITEM_ACCESS_DENIED
Controller-->>Client: 403 Forbidden
else Access granted
Service->>Service: OrderItemResponse.from(orderItem)
Service-->>Controller: OrderItemResponse
Controller-->>Client: 200 OK + GET_ORDER_ITEM_SUCCESS
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 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/order/controller/OrderController.java (1)
10-10: 발주 아이템 조회 엔드포인트가 기존 패턴과 잘 맞습니다
- 인증 정보에서
userId추출 → 서비스 위임 →ApiResponse.success로 감싸는 흐름이 기존getOrder/getOrderList와 동일해서 유지보수성이 좋습니다.- 새
SuccessMessage.GET_ORDER_ITEM_SUCCESS를 바로 사용한 것도 응답 일관성 측면에서 좋습니다.Swagger 메타데이터만 살짝 다듬으면 더 읽기 좋아집니다:
- @Operation(summary = "발주 아이템 조회", description = "발주 아이템 조회합니다.") + @Operation(summary = "발주 아이템 조회", description = "발주 아이템을 조회합니다.")크게 중요한 부분은 아니므로, 문구가 마음에 들 때 반영하시면 됩니다.
Also applies to: 107-120
src/main/java/com/almang/inventory/order/service/OrderService.java (1)
14-14: 발주 아이템 조회 서비스 플로우가 안정적으로 설계되었습니다좋은 점부터 정리하면:
getOrderItem에서 공통 패턴(사용자 조회 → store 획득 → 접근 검증 → DTO 매핑)을 그대로 따라가서, 다른 메서드들과 읽는 경험이 통일되어 있습니다.ORDER_ITEM_NOT_FOUND/ORDER_ITEM_ACCESS_DENIED를 명확히 분리해서 예외를 던지고 있고, 테스트와도 정확히 매칭됩니다.OrderItemResponse.from(orderItem)으로 DTO 매핑을 위임한 것도 응집도 측면에서 좋습니다.다만, 몇 가지 작은 리팩터를 고려해 볼 수 있습니다.
- 로그 메시지 용어를 엔드포인트와 맞추기
현재 로그는"발주 상세 조회"인데, 컨트롤러/성공 메시지는"발주 아이템 조회"를 쓰고 있습니다. 나중에 로그만 보고 추적할 때 용어가 통일되어 있는 편이 이해가 더 빠릅니다.- log.info("[OrderService] 발주 상세 조회 요청 - userId: {}, storeId: {}", userId, store.getId()); + log.info("[OrderService] 발주 아이템 조회 요청 - userId: {}, storeId: {}", userId, store.getId()); ... - log.info("[OrderService] 발주 상세 조회 성공 - orderItemId: {}", orderItem.getId()); + log.info("[OrderService] 발주 아이템 조회 성공 - orderItemId: {}", orderItem.getId());
- ORDER_ITEM 조회/검증 로직 중복을 살짝 줄이기
이미findOrderItemByIdAndValidateAccess(Long orderItemId, Order order)가 있고, 새로findOrderItemById+validateOrderItemAccess가 추가되어 두 군데에서 비슷한 검증을 하게 됩니다.
필수는 아니지만, 아래처럼 기존 메서드가 새 헬퍼를 재사용하게 하면 규칙 변경 시 수정 지점이 줄어듭니다.- private OrderItem findOrderItemByIdAndValidateAccess(Long orderItemId, Order order) { - OrderItem orderItem = orderItemRepository.findById(orderItemId) - .orElseThrow(() -> new BaseException(ErrorCode.ORDER_ITEM_NOT_FOUND)); - - if (!orderItem.getOrder().getId().equals(order.getId())) { - throw new BaseException(ErrorCode.ORDER_ITEM_ACCESS_DENIED); - } - return orderItem; - } + private OrderItem findOrderItemByIdAndValidateAccess(Long orderItemId, Order order) { + OrderItem orderItem = findOrderItemById(orderItemId); + if (!orderItem.getOrder().getId().equals(order.getId())) { + throw new BaseException(ErrorCode.ORDER_ITEM_ACCESS_DENIED); + } + return orderItem; + }이 두 가지는 전부 가독성과 유지보수성을 위한 선택 사항이라, 팀 코딩 스타일에 맞춰 여유 있을 때 적용하셔도 좋겠습니다.
Also applies to: 119-130, 288-297
src/test/java/com/almang/inventory/order/service/OrderServiceTest.java (1)
16-16: getOrderItem 서비스 테스트 커버리지가 깔끔합니다
발주_상세_조회에_성공한다에서orderItemId,productId,quantity,unitPrice,amount,note까지 모두 검증하고 있어 DTO 매핑 이상 여부를 잘 잡아낼 수 있습니다.- 존재하지 않는 ID와, 다른 상점에 속한 아이템에 대한
ORDER_ITEM_NOT_FOUND/ORDER_ITEM_ACCESS_DENIED예외 플로우도 각각 분리해서 테스트한 점이 좋습니다.작은 개선 아이디어 한 가지 정도만 제안드리면:
- 컨트롤러/성공 메시지에서 쓰는 용어는 “발주 아이템 조회”인데, 테스트 메서드 이름은 “발주_상세_조회…”로 되어 있습니다. 나중에 검색/정리할 때 혼선을 줄이려면 예를 들어 다음처럼 통일하는 것도 고려해볼 수 있습니다.
- void 발주_상세_조회에_성공한다() { + void 발주_아이템_상세_조회에_성공한다() {테스트 자체는 이미 충분히 견고하니, 네이밍은 팀 내 용어 정책에 맞춰 여유 있을 때 정리하셔도 됩니다.
Also applies to: 893-973
src/test/java/com/almang/inventory/order/controller/OrderControllerTest.java (1)
23-23: 발주 아이템 조회 컨트롤러 테스트 구성이 적절합니다
- 성공/존재하지 않음/다른 상점 아이템 접근 거부까지 세 가지 플로우를 모두 검증해서, HTTP 상태 코드와
ErrorCode매핑이 잘 보호되고 있습니다.SuccessMessage.GET_ORDER_ITEM_SUCCESS메시지까지 검증해 둔 점도 응답 규격 변경을 잘 탐지할 수 있어서 좋습니다.조금 더 방어적으로 가고 싶다면, 성공 케이스에서
amount,note까지도 한 줄씩만 추가로 확인해 두면 직렬화 레벨에서의 필드 누락/타이포도 잡아낼 수 있습니다.- .andExpect(jsonPath("$.data.productId").value(10L)); + .andExpect(jsonPath("$.data.productId").value(10L)) + .andExpect(jsonPath("$.data.amount").value(5000)) + .andExpect(jsonPath("$.data.note").value("비고입니다"));이미 서비스 테스트에서 값 검증을 하고 있어 필수는 아니고, 컨트롤러 단에서 어디까지 검증할지에 대한 팀 기준에 따라 선택하시면 되겠습니다.
Also applies to: 568-634
📜 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/order/controller/OrderController.java(2 hunks)src/main/java/com/almang/inventory/order/dto/response/OrderItemResponse.java(2 hunks)src/main/java/com/almang/inventory/order/service/OrderService.java(3 hunks)src/test/java/com/almang/inventory/order/controller/OrderControllerTest.java(2 hunks)src/test/java/com/almang/inventory/order/service/OrderServiceTest.java(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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/order/dto/response/OrderItemResponse.javasrc/main/java/com/almang/inventory/order/service/OrderService.javasrc/test/java/com/almang/inventory/order/controller/OrderControllerTest.javasrc/test/java/com/almang/inventory/order/service/OrderServiceTest.javasrc/main/java/com/almang/inventory/order/controller/OrderController.java
📚 Learning: 2025-11-20T10:43:47.489Z
Learnt from: JoonKyoLee
Repo: almang2/inventory-server PR: 33
File: src/main/java/com/almang/inventory/product/domain/Product.java:62-66
Timestamp: 2025-11-20T10:43:47.489Z
Learning: In the almang2/inventory-server repository, Product entity update methods (e.g., updateVendor in src/main/java/com/almang/inventory/product/domain/Product.java) do not require null checks on vendor parameters because ProductService validates vendor existence via findVendorByIdAndValidateAccess before calling update methods, and the vendor field has nullable=false constraint ensuring this.vendor is never null for persisted entities.
Applied to files:
src/test/java/com/almang/inventory/order/service/OrderServiceTest.java
🔇 Additional comments (2)
src/main/java/com/almang/inventory/global/api/SuccessMessage.java (1)
46-46: 발주 아이템 성공 메시지 추가 적절합니다
GET_ORDER_ITEM_SUCCESS네이밍과 한글 메시지가 기존 ORDER 관련 상수들과 일관되고, 컨트롤러 응답에도 잘 매핑될 수 있는 형태입니다. 별도 수정 필요 없어 보입니다.src/main/java/com/almang/inventory/order/dto/response/OrderItemResponse.java (1)
13-15: OrderItem 응답 DTO 확장이 자연스럽게 잘 되었습니다
amount와note를 record 컴포넌트로 추가하고,from(OrderItem)에서 엔티티의getAmount(),getNote()를 그대로 매핑한 구조가 명확합니다.ORDER/PRODUCT널 방어 로직도 그대로 유지되어 있어, 매핑 시 불변 조건을 잘 지키고 있습니다.현재 형태만으로도 도메인 모델과 응답 스키마가 잘 동기화되어 있어서, 추가 수정이 필요해 보이지 않습니다.
Also applies to: 26-27
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.
✨ 작업 내용
📝 적용 범위
/order📌 참고 사항
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.