Conversation
Walkthrough입고 아이템 삭제 기능을 완전히 구현했습니다. 새로운 DELETE 엔드포인트, 서비스 계층 로직, 응답 DTO, 그리고 포괄적인 테스트 케이스를 추가했으며, 성공 메시지 enum도 확장했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Service
participant Repository
participant DB
Client->>Controller: DELETE /receipt/{receiptItemId}
Controller->>Controller: Extract userId from@AuthenticationPrincipal
Controller->>Service: deleteReceiptItem(receiptItemId, userId)
Service->>Repository: findUserById(userId)
alt User not found
Repository-->>Service: null
Service-->>Controller: throw USER_NOT_FOUND
Controller-->>Client: 404 error
end
Service->>Repository: findReceiptItemById(receiptItemId)
alt Item not found
Repository-->>Service: null
Service-->>Controller: throw RECEIPT_ITEM_NOT_FOUND
Controller-->>Client: 404 error
end
Service->>Service: Verify store access
alt Access denied
Service-->>Controller: throw RECEIPT_ITEM_ACCESS_DENIED
Controller-->>Client: 403 error
end
Service->>Service: Get parent Receipt
Service->>DB: Remove item from receipt.items
Service->>DB: Update receipt.totalBoxCount
Service->>DB: Flush changes
rect rgb(200, 220, 255)
Note over Service,DB: Transaction commits
end
Service-->>Controller: DeleteReceiptItemResponse(true)
Controller-->>Client: 200 OK with DeleteReceiptItemResponse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20분 주의 깊게 검토할 영역:
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/main/java/com/almang/inventory/receipt/dto/response/DeleteReceiptItemResponse.java (1)
1-5: 삭제 응답 DTO를 단일 success 플래그로 단순하게 정의한 점 좋습니다현재는 삭제 성공 여부만 필요하므로
boolean success하나만 가지는 record 구조가 과하지 않고 명확합니다.
향후 클라이언트에서 “어떤 아이템이 삭제됐는지”를 응답으로 활용해야 한다면, 별도receiptItemId필드를 추가한 DTO로 확장하거나, 동일 패턴의DeleteReceiptResponse와 함께 공통 인터페이스/record로 정리하는 것도 고려해볼 수 있습니다.src/test/java/com/almang/inventory/receipt/controller/ReceiptControllerTest.java (1)
22-22: 입고 아이템 삭제 컨트롤러 테스트 구성이 기존 패턴과 잘 정렬되어 있습니다
/api/v1/receipt/receipt/{receiptItemId}경로,SuccessMessage.DELETE_RECEIPT_ITEM_SUCCESS메시지,DeleteReceiptItemResponse의success필드까지 한 번에 검증하고 있어서 회귀를 잡는 데 도움이 되겠습니다. USER_NOT_FOUND / ACCESS_DENIED 에러 케이스도 잘 커버되어 있습니다.다만, 서비스/조회 쪽과 대칭성을 생각하면 컨트롤러 레벨에서도
ErrorCode.RECEIPT_ITEM_NOT_FOUND→ 404 응답이 나가는지 한 케이스를 더 추가해 두면 좋겠습니다. 예를 들면:@Test void 입고_아이템_삭제시_아이템이_존재하지_않으면_예외가_발생한다() throws Exception { Long receiptItemId = 9999L; when(receiptService.deleteReceiptItem(anyLong(), anyLong())) .thenThrow(new BaseException(ErrorCode.RECEIPT_ITEM_NOT_FOUND)); mockMvc.perform(delete("/api/v1/receipt/receipt/{receiptItemId}", receiptItemId) .with(authentication(auth()))) .andExpect(status().isNotFound()) .andExpect(jsonPath("$.status").value(ErrorCode.RECEIPT_ITEM_NOT_FOUND.getHttpStatus().value())) .andExpect(jsonPath("$.message").value(ErrorCode.RECEIPT_ITEM_NOT_FOUND.getMessage())) .andExpect(jsonPath("$.data").doesNotExist()); }이렇게 해두면 삭제 API의 에러 매핑이 조회 API와 완전히 대칭이라, 나중에 ErrorCode 변경 시에도 테스트가 안전망 역할을 해줄 것 같습니다.
Also applies to: 1088-1137
src/main/java/com/almang/inventory/receipt/controller/ReceiptController.java (1)
10-13: 입고 아이템 삭제 엔드포인트가 기존 API 패턴과 자연스럽게 통합되었습니다
- 경로(
/api/v1/receipt/receipt/{receiptItemId}), 인증 방식(@AuthenticationPrincipal CustomUserPrincipal), 로그 포맷,ApiResponse.success+SuccessMessage.DELETE_RECEIPT_ITEM_SUCCESS사용까지 기존 조회/수정/삭제 API들과 일관성이 잘 맞습니다.DeleteReceiptItemResponse를 그대로 노출해 FE가data.success만 확인하면 되는 구조라 사용성도 명확합니다.추가로, Swagger 문서 측면에서만 보면 이 삭제 엔드포인트에도
@ApiResponses등을 활용해USER_NOT_FOUND,RECEIPT_ITEM_NOT_FOUND,RECEIPT_ITEM_ACCESS_DENIED같은 대표 에러 코드와 HTTP Status를 명시해두면, FE/문서 소비자 입장에서 기대 가능한 응답 스펙을 한눈에 파악하기 더 좋아질 것 같습니다.Also applies to: 171-184
src/test/java/com/almang/inventory/receipt/service/ReceiptServiceTest.java (1)
21-23: 입고 아이템 삭제 서비스 테스트가 핵심 시나리오를 잘 커버하고 있습니다
- 성공 케이스에서 2→1개로 아이템 수가 줄어드는지,
totalBoxCount가 5→3으로 재계산되는지,DeleteReceiptItemResponse.success가 true인지까지 함께 검증한 점이 좋습니다.- USER_NOT_FOUND / RECEIPT_ITEM_NOT_FOUND / RECEIPT_ITEM_ACCESS_DENIED를 각각 별도 테스트로 분리해 둔 것도, 에러 원인별로 명확하게 회귀를 잡을 수 있어 깔끔합니다.
조금 더 방어적으로 가고 싶다면, 성공 테스트에서 “남아 있는 아이템이 기대한 item2인지”도 함께 확인해두면 좋습니다. 예를 들어:
Receipt updated = receiptRepository.findById(saved.getId()) .orElseThrow(); assertThat(updated.getItems()).hasSize(1); assertThat(updated.getItems().get(0).getId()) .isEqualTo(item2.getId()); // 삭제되지 않아야 하는 쪽 검증이 정도만 추가해 두면 “다른 아이템을 잘못 지우는” 류의 버그도 더 쉽게 잡을 수 있을 것 같습니다.
Also applies to: 1281-1399
📜 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/receipt/controller/ReceiptController.java(2 hunks)src/main/java/com/almang/inventory/receipt/dto/response/DeleteReceiptItemResponse.java(1 hunks)src/main/java/com/almang/inventory/receipt/service/ReceiptService.java(4 hunks)src/test/java/com/almang/inventory/receipt/controller/ReceiptControllerTest.java(2 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/controller/ReceiptControllerTest.javasrc/main/java/com/almang/inventory/receipt/service/ReceiptService.javasrc/test/java/com/almang/inventory/receipt/service/ReceiptServiceTest.java
🔇 Additional comments (2)
src/main/java/com/almang/inventory/global/api/SuccessMessage.java (1)
50-60: 입고 아이템 삭제용 SuccessMessage 추가가 기존 컨벤션과 잘 맞습니다RECEIPT 섹션 내 다른 상수들과 네이밍, 한글 메시지 톤이 모두 일관적이라 유지보수 시에도 의미가 명확합니다. 별도 수정 없이 이대로 가져가도 좋겠습니다.
src/main/java/com/almang/inventory/receipt/service/ReceiptService.java (1)
16-17: 현재 JPA 설정이 이미 완벽하게 구성되어 있습니다코드 검증 결과,
Receipt.javaLine 50에서@OneToMany(mappedBy = "receipt", cascade = CascadeType.ALL, orphanRemoval = true)가 이미 명시적으로 설정되어 있습니다. 따라서 현재의receipt.getItems().remove(receiptItem)호출만으로도 JPA가 자동으로 orphaned 아이템을 감지하고 데이터베이스 레코드를 삭제합니다.원래 제안 중 첫 번째는 불필요합니다
명시적receiptItemRepository.delete(receiptItem)호출을 추가하는 것은 중복 삭제를 시도할 수 있으므로 현재 구현이 정답입니다.도메인 메서드 캡슐화 제안은 여전히 유효합니다
같은 패턴이 다른 곳에서도 반복될 여지가 있다면,Receipt엔티티에 아래처럼 도메인 메서드를 추가하는 것을 권장합니다:public void removeItemAndRecalculate(ReceiptItem item) { this.items.remove(item); this.updateTotalBoxCount(items.stream() .mapToInt(ReceiptItem::getBoxCount) .sum()); }이렇게 하면 서비스 레이어에서는
receipt.removeItemAndRecalculate(receiptItem)만 호출하면 되고, 나중에 비즈니스 규칙이 변경되어도 한 곳만 수정하면 됩니다.
✨ 작업 내용
📝 적용 범위
/receipt📌 참고 사항
Summary by CodeRabbit
릴리스 노트
새 기능
테스트
✏️ Tip: You can customize this high-level summary in your review settings.