Merged
Conversation
Walkthrough입고를 CONFIRMED 상태로 전환하는 PATCH /{receiptId}/confirm 엔드포인트와 관련 서비스·도메인·응답 DTO·메시지·예외 코드를 추가하고, 해당 흐름에 대한 단위/컨트롤러 테스트를 포함합니다. (입고 확정 성공 메시지 및 상태 검증/예외 추가) Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Controller as ReceiptController
participant Service as ReceiptService
participant UserRepo as UserRepository
participant ReceiptRepo as ReceiptRepository
participant Domain as Receipt (Domain)
User->>Controller: PATCH /{receiptId}/confirm
Controller->>Controller: 인증 확인 (`@AuthenticationPrincipal`)
Controller->>Service: confirmReceipt(receiptId, userId)
Service->>UserRepo: findById(userId)
alt USER_NOT_FOUND
UserRepo-->>Service: 없음
Service-->>Controller: UserNotFoundException
Controller-->>User: 404
else 사용자 존재
UserRepo-->>Service: User
Service->>ReceiptRepo: findById(receiptId)
alt RECEIPT_NOT_FOUND
ReceiptRepo-->>Service: 없음
Service-->>Controller: ReceiptNotFoundException
Controller-->>User: 404
else 입고 존재
ReceiptRepo-->>Service: Receipt
Service->>Service: storeId 비교 (접근 검증)
alt RECEIPT_ACCESS_DENIED
Service-->>Controller: AccessDeniedException
Controller-->>User: 403
else 접근 허용
Service->>Domain: receipt.confirm()
Domain->>Domain: status = CONFIRMED
Service->>ReceiptRepo: save()
Service-->>Controller: ConfirmReceiptResponse(true)
Controller-->>User: 200 + CONFIRM_RECEIPT_SUCCESS
end
end
end
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: 2
🧹 Nitpick comments (1)
src/test/java/com/almang/inventory/receipt/service/ReceiptServiceTest.java (1)
1437-1489: 기본 에러 케이스 테스트가 충실합니다.향후 비즈니스 검증 로직 추가 시 다음 테스트 케이스들도 추가하시길 권장합니다:
- 이미 확정된 입고 재확정 시도
- 취소된 입고 확정 시도
- 비활성화된 입고 확정 시도
이러한 엣지 케이스들을 미리 테스트하면 회귀 버그를 방지할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
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/domain/Receipt.java(1 hunks)src/main/java/com/almang/inventory/receipt/dto/response/ConfirmReceiptResponse.java(1 hunks)src/main/java/com/almang/inventory/receipt/service/ReceiptService.java(2 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/test/java/com/almang/inventory/receipt/controller/ReceiptControllerTest.javasrc/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 (10)
src/main/java/com/almang/inventory/receipt/dto/response/ConfirmReceiptResponse.java (1)
3-5: 깔끔한 응답 DTO 구현입니다.기존 DeleteReceiptResponse 패턴과 일관성 있게 구현되었습니다.
src/main/java/com/almang/inventory/global/api/SuccessMessage.java (1)
60-60: 성공 메시지가 적절하게 추가되었습니다.기존 패턴과 일관성 있게 RECEIPT 섹션에 배치되었습니다.
src/main/java/com/almang/inventory/receipt/service/ReceiptService.java (1)
16-16: 필요한 import가 올바르게 추가되었습니다.src/test/java/com/almang/inventory/receipt/controller/ReceiptControllerTest.java (3)
22-22: 필요한 테스트 의존성이 추가되었습니다.
1140-1158: 확정 성공 케이스 테스트가 잘 작성되었습니다.응답 상태, 메시지, 데이터를 모두 검증하고 있습니다.
1160-1212: 주요 에러 케이스들이 잘 커버되었습니다.사용자 없음, 입고 없음, 접근 거부 케이스가 모두 테스트되고 있습니다. 다만, 향후 비즈니스 검증 로직이 추가되면(취소된 입고 확정 시도, 이미 확정된 입고 재확정 등) 해당 케이스들에 대한 테스트도 추가해야 합니다.
src/test/java/com/almang/inventory/receipt/service/ReceiptServiceTest.java (2)
21-21: 테스트 의존성이 올바르게 추가되었습니다.
1402-1435: 통합 테스트가 견고하게 작성되었습니다.데이터베이스 상태 변경을 실제로 검증하고 있어 신뢰도가 높습니다.
src/main/java/com/almang/inventory/receipt/controller/ReceiptController.java (2)
10-10: 컨트롤러 의존성이 적절히 추가되었습니다.
141-154: 확정 엔드포인트가 RESTful 하게 잘 구현되었습니다.PATCH 메서드 선택이 적절하며, 인증 처리와 로깅이 기존 패턴과 일관되게 구현되었습니다. Swagger 문서화도 갖추어져 있어 API 사용성이 좋습니다.
Comment on lines
+150
to
+161
| @Transactional | ||
| public ConfirmReceiptResponse confirmReceipt(Long receiptId, Long userId) { | ||
| User user = findUserById(userId); | ||
| Store store = user.getStore(); | ||
|
|
||
| log.info("[ReceiptService] 입고 확정 요청 - userId: {}, storeId: {}", userId, store.getId()); | ||
| Receipt receipt = findReceiptByIdAndValidateAccess(receiptId, store); | ||
| receipt.confirm(); | ||
|
|
||
| log.info("[ReceiptService] 입고 확정 성공 - receiptId: {}", receipt.getId()); | ||
| return new ConfirmReceiptResponse(true); | ||
| } |
There was a problem hiding this comment.
입고 확정 전 비즈니스 검증 로직이 부족합니다.
현재 구현은 사용자 권한만 확인하고 입고의 상태는 검증하지 않습니다. 다음 사항들을 추가 검증해야 합니다:
- activated 플래그 확인: 삭제된(비활성화된) 입고를 확정할 수 없어야 합니다
- 현재 상태 확인: PENDING 상태의 입고만 확정 가능해야 합니다
- 데이터 완전성: 입고 아이템이 존재하는지 확인이 필요할 수 있습니다
개선 제안:
@Transactional
public ConfirmReceiptResponse confirmReceipt(Long receiptId, Long userId) {
User user = findUserById(userId);
Store store = user.getStore();
log.info("[ReceiptService] 입고 확정 요청 - userId: {}, storeId: {}", userId, store.getId());
Receipt receipt = findReceiptByIdAndValidateAccess(receiptId, store);
+
+ if (!receipt.isActivated()) {
+ throw new BaseException(ErrorCode.RECEIPT_ALREADY_DELETED);
+ }
+ if (receipt.getStatus() == ReceiptStatus.CONFIRMED) {
+ throw new BaseException(ErrorCode.RECEIPT_ALREADY_CONFIRMED);
+ }
+ if (receipt.getStatus() == ReceiptStatus.CANCELED) {
+ throw new BaseException(ErrorCode.RECEIPT_ALREADY_CANCELED);
+ }
+
receipt.confirm();
log.info("[ReceiptService] 입고 확정 성공 - receiptId: {}", receipt.getId());
return new ConfirmReceiptResponse(true);
}참고: ErrorCode enum에 해당 에러 코드들을 추가해야 합니다. 비즈니스 검증은 도메인 계층에서 할 수도 있고 서비스 계층에서 할 수도 있습니다만, 일관성을 위해 기존 패턴을 따르세요.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/com/almang/inventory/receipt/service/ReceiptService.java around
lines 150-161, add business validation before calling receipt.confirm(): verify
receipt.isActivated() (reject if false), verify receipt.getStatus() ==
ReceiptStatus.PENDING (reject otherwise), and verify receipt.getItems() is not
null/empty (reject if empty). For each failure throw the existing application
exception with an appropriate ErrorCode (add ErrorCode entries if missing, e.g.,
RECEIPT_NOT_ACTIVE, RECEIPT_INVALID_STATUS, RECEIPT_NO_ITEMS) following the
project’s existing exception pattern; perform these checks in the service method
(or delegate to domain methods on Receipt if that matches existing conventions)
and only call receipt.confirm() after all validations pass.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/com/almang/inventory/global/exception/ErrorCode.java (1)
48-56: 입고 상태 전용 에러코드 분리, 방향 좋습니다.
- 입고 취소/확정에 대해 별도 에러코드를 둔 덕분에 서비스/컨트롤러 단에서 상태별 분기와 테스트가 훨씬 명확해집니다.
- 기존 RECEIPT_* 코드들과 HttpStatus, 메시지 스타일이 일관되어 유지보수성도 좋습니다. 짧게 말해, 나중에 상태 다이어그램 그려도 잘 맞을 설계네요. 😄
추가로, HTTP 400/409 중 어떤 것을 비즈니스 밸리데이션에 쓸지 팀 컨벤션을 한번 정리해 두면, 이후 다른 도메인(ORDER, PRODUCT 등) 상태 전이 에러 추가할 때도 기준이 더 분명해질 것 같습니다. (공식 HTTP 스펙이나 Spring REST Best Practice 문서들을 참고해 보셔도 좋겠습니다.)
src/main/java/com/almang/inventory/receipt/domain/Receipt.java (1)
85-91: 이미 취소된 입고에 대한 재취소 방어 로직이 명확합니다.
- 이미
ReceiptStatus.CANCELED인 경우 예외(RECEIPT_ALREADY_CANCELED)를 던지도록 해서, “취소 → 취소”와 같은 잘못된 상태 전이를 도메인 차원에서 차단하고 있습니다.- 이 덕분에 API 계층에서 굳이 상태를 이중으로 검사하지 않아도 되어, 상태 전이 규칙이 한 곳에서 유지됩니다.
참고로, REST API 관점에서는 “취소 요청을 여러 번 보내도 항상 동일한 결과(멱등)”를 선호하는 경우도 있습니다. 현재처럼 두 번째 호출을 에러로 보는지, 아니면 그냥 성공(예: 200/204)으로 간주할지는 팀의 API UX 정책에 따라 갈릴 수 있으니, 클라이언트 사용 패턴을 보면서 한 번쯤 논의해 보셔도 좋겠습니다. (멱등성은 HTTP RFC 및 REST API 설계 Best Practice 문서들에 좋은 논의가 많습니다.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/almang/inventory/global/exception/ErrorCode.java(1 hunks)src/main/java/com/almang/inventory/receipt/domain/Receipt.java(3 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/domain/Receipt.java
🔇 Additional comments (2)
src/main/java/com/almang/inventory/receipt/domain/Receipt.java (2)
4-6: 도메인 레벨에서 BaseException을 사용하도록 정리한 것 좋습니다.
- Receipt 엔티티가 직접 BaseException + ErrorCode를 사용하면서, “입고 상태 전이 규칙”이 한 곳(도메인 모델)에서 관리됩니다.
- 이렇게 하면 서비스/컨트롤러는 상태 전이 로직 대신 “언제 confirm()/deactivate()를 부를지”만 신경 쓰면 되기 때문에 레이어 간 관심사가 잘 분리됩니다.
DDD에서 말하는 애그리거트 불변식(invariant)을 엔티티 메서드로 캡슐화하는 전형적인 패턴이라, 지금 방향을 계속 유지하시면 좋겠습니다. 필요하다면 나중에 Receipt 상태 전이 다이어그램을 간단히 문서화해 두는 것도 추천드립니다.
93-101: 입고 확정(confirm) 상태 전이 검증이 잘 정리되었습니다.
!activated || status == CANCELED일 때RECEIPT_ALREADY_CANCELED예외를 던져서, “취소된/비활성 입고를 다시 확정”하는 시나리오를 도메인에서 차단하고 있습니다.status == CONFIRMED일 때RECEIPT_ALREADY_CONFIRMED를 던지는 것도, “재확정”을 명시적으로 에러로 처리해 API 응답을 분명하게 만들어 줍니다.- 마지막에
this.status = ReceiptStatus.CONFIRMED;만 변경하고 activated는 건드리지 않아, “활성 + 확정”이라는 상태 조합이 유지되는 점도 자연스럽습니다.이전 커밋에서 지적되었던 “상태 전이 검증 누락” 이슈가, 지금은 엔티티 메서드 안에서 잘 해결된 형태입니다. 앞으로 ReceiptStatus에 상태가 더 늘어난다면(예: PARTIAL_CONFIRMED 등), 이 메서드 초반 조건문만 확장하면 되니 유지보수도 수월할 것 같습니다. 상태 전이 시나리오는 단위 테스트(성공/취소/이미 확정/이미 취소 케이스)를 충분히 커버해 두시면 더욱 안심할 수 있겠네요. 👍
This was referenced Nov 25, 2025
4 tasks
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.