Conversation
Walkthrough발주 삭제 기능을 추가합니다. DELETE 엔드포인트, 서비스 메서드, DTO, 성공 메시지 상수, 관련 테스트를 포함합니다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Controller as OrderController
participant Service as OrderService
participant DB as Order Entity
Client->>Controller: DELETE /orders/{orderId}
activate Controller
Note over Controller: Extract userId<br/>from Principal
Controller->>Service: deleteOrder(orderId, userId)
activate Service
Service->>Service: Validate user exists
Service->>Service: Validate order exists
Service->>Service: Validate user/store match
Service->>DB: Set status=CANCELED<br/>Set deactivate=false
activate DB
DB-->>Service: Updated Order
deactivate DB
Service-->>Controller: DeleteOrderResponse(true)
deactivate Service
Controller-->>Client: ApiResponse.success<br/>DELETE_ORDER_SUCCESS
deactivate Controller
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 분 주의 깊게 검토할 영역:
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 (5)
src/main/java/com/almang/inventory/order/dto/response/DeleteOrderResponse.java (1)
3-5: 삭제 응답 DTO 구조 적절함
DeleteOrderResponse를 단일success필드로 둔 선택이 현재 사용처(성공 여부만 확인)와 잘 맞습니다.
향후 삭제된 발주의 ID나 상태 등을 추가로 내려줘야 할 일이 생기면 이 레코드에 필드를 확장하는 방향으로도 쉽게 진화시킬 수 있겠습니다.src/test/java/com/almang/inventory/order/controller/OrderControllerTest.java (1)
7-7: 삭제 컨트롤러 테스트 커버리지 충분함
delete("/api/v1/order/{orderId}")호출, 응답 status/message/jsonPath 검증이 기존 생성/수정 테스트 패턴과 잘 맞습니다.- 성공 케이스에서
$.data.success == true까지 확인해서 DTO 매핑까지 커버하는 점이 좋습니다.- 에러 케이스 3종(User 없음, Order 없음, 다른 상점)도 각각 적절한 HTTP 상태 코드와
ErrorCode메시지를 검증하고 있어, 예외 매핑에 대한 회귀 테스트 역할을 잘 하고 있습니다.추가로, 여유가 된다면
orderService.deleteOrder가 인증된 사용자 ID와orderId로 정확히 한 번 호출되는지도verify(orderService)...로 검증해 두면 컨트롤러 수준 계약이 더 명확해질 수 있습니다 (필수는 아닙니다).Also applies to: 24-24, 728-796
src/test/java/com/almang/inventory/order/service/OrderServiceTest.java (1)
16-16: 서비스 레벨 삭제 시나리오 테스트 좋음 (DTO 값 검증만 살짝 보강 제안)
발주_삭제에_성공한다테스트에서 실제 생성→삭제→DB 재조회 후OrderStatus.CANCELED,activated=false를 확인하는 흐름이 도메인 로직 검증에 충실합니다.- 사용자/주문 미존재, 다른 상점 케이스에서 각각
USER_NOT_FOUND,ORDER_NOT_FOUND,ORDER_ACCESS_DENIED메시지를 기대하는 부분도 서비스 단 예외 설계와 잘 맞습니다.개선 여지 한 가지는, 성공 케이스에서
DeleteOrderResponse response에 대해assertThat(response.success()).isTrue();정도를 추가하면 DTO 계약이 변경될 때(예: 삭제 실패 시 false를 돌리도록 확장하는 경우) 빠르게 깨지는 안전망이 될 수 있습니다.Also applies to: 1116-1200
src/main/java/com/almang/inventory/order/service/OrderService.java (1)
14-15: 삭제 로직(JPA 트랜잭션 + 소프트 삭제) 구현이 안정적으로 보입니다
findUserById와findOrderByIdAndValidateAccess를 그대로 재사용해서USER_NOT_FOUND,ORDER_NOT_FOUND,ORDER_ACCESS_DENIED케이스를 자연스럽게 처리하는 구조가 깔끔합니다.order.updateStatus(OrderStatus.CANCELED)와order.updateMessageAndActivated(null, false)만 호출하고 별도save없이 반환하는 방식은, 현재 메서드가@Transactional이므로 JPA 더티 체킹에 의해 정상 반영됩니다. (혹시 더티 체킹이 익숙치 않다면 Spring Data JPA Reference - Transactions 섹션을 한 번 같이 보는 걸 추천드립니다.)- 이미 취소된 주문에 대해 다시
deleteOrder를 호출해도 동일한 상태(CANCELED, 비활성화)를 재설정할 뿐이라, REST DELETE의 멱등(idempotent) 특성과도 잘 맞습니다.도메인 관점에서 한 가지만 고민해볼 만한 지점은, 삭제 시 주문 메시지를
null로 지우는 것이 맞는지입니다. 나중에 감사/이력 조회에 메시지가 필요할 수 있다면, 메시지는 보존하되status/activated와 조회 필터로 노출 여부를 제어하는 식으로 조정할 여지가 있습니다.Also applies to: 151-163
src/main/java/com/almang/inventory/order/controller/OrderController.java (1)
11-12: 삭제 엔드포인트 설계가 기존 컨벤션과 잘 맞습니다
@DeleteMapping("/{orderId}")를 사용해 조회/수정과 동일한 리소스 경로를 공유하면서 HTTP 메서드로 동작을 구분한 점이 REST 관점에서 자연스럽습니다.ApiResponse.success(SuccessMessage.DELETE_ORDER_SUCCESS.getMessage(), response)패턴이 다른 메서드들과 완전히 동일해서, 클라이언트 입장에서도 일관된 응답 스키마(JSONstatus/message/data.success)를 기대할 수 있습니다.- Swagger
@Operation(summary = "발주 삭제", description = "발주를 삭제합니다.")까지 선언되어 있어서 문서화 측면에서도 누락이 없습니다.HTTP 코드 관점에서는 204 No Content를 선택하는 것도 한 가지 옵션이지만, 현재처럼 삭제 결과 DTO를 내려주기 때문에 200 OK + body 선택도 합리적입니다.
Also applies to: 23-23, 110-123
📜 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(3 hunks)src/main/java/com/almang/inventory/order/dto/response/DeleteOrderResponse.java(1 hunks)src/main/java/com/almang/inventory/order/service/OrderService.java(2 hunks)src/test/java/com/almang/inventory/order/controller/OrderControllerTest.java(3 hunks)src/test/java/com/almang/inventory/order/service/OrderServiceTest.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/test/java/com/almang/inventory/order/service/OrderServiceTest.javasrc/test/java/com/almang/inventory/order/controller/OrderControllerTest.javasrc/main/java/com/almang/inventory/order/controller/OrderController.javasrc/main/java/com/almang/inventory/order/service/OrderService.java
🔇 Additional comments (1)
src/main/java/com/almang/inventory/global/api/SuccessMessage.java (1)
38-49: 성공 메시지 상수 추가 일관성 좋음
DELETE_ORDER_SUCCESS("발주 삭제 성공")가 ORDER 관련 메시지 그룹과 잘 정렬되어 있고, 컨트롤러의 삭제 API 동작과 의미도 정확히 일치합니다. 별도의 수정 필요 없어 보입니다.
✨ 작업 내용
📝 적용 범위
/order📌 참고 사항
Summary by CodeRabbit
릴리스 노트
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.