Feat/order status update event#35
Merged
Merged
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughSummary by CodeRabbit릴리스 노트
WalkthroughPR은 주문 상태 업데이트 이벤트를 adminId에서 buyerId로 전환하고, 이를 위해 저장소에 buyer를 즉시 로드하는 메서드를 추가하고 서비스와 이벤트 리스너를 수정합니다. CI에 Redis 서비스가 추가되고 테스트 구성과 테스트 코드(Testcontainers, Redisson mock, 테스트 YAML)가 업데이트됩니다. 변경사항주문 상태 업데이트 이벤트 흐름 리팩토링
예상되는 코드 리뷰 노력🎯 3 (보통) | ⏱️ ~25분 관련 가능성 있는 PR
제안된 리뷰어
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…t/mvp-admin-server into feat/orderStatusUpdateEvent
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/java/com/example/allinmarket/admin/order/service/AdminOrderServiceTest.java (1)
174-255: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win상태 변경 테스트에 이벤트 발행 내용 검증을 추가해 주세요.
Line 188-193은 반환값만 확인하고 있어, PR 핵심인
buyerId기반 이벤트 발행이 깨져도 테스트가 통과할 수 있습니다. 성공 케이스에서publishEventpayload를 검증하고, 실패 케이스에서는 발행이 없음을 함께 확인하는 편이 안전합니다.테스트 보강 예시
+import com.example.allinmarket.domain.order.event.OrderStatusUpdateEvent; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; @@ void 관리자_주문상태_변경_성공_테스트() { @@ OrderDetailResponse result = adminOrderService.updateStatus(adminId, orderId, request); // then assertNotNull(result); assertEquals(orderId, result.orderId()); + verify(eventPublisher).publishEvent(argThat(event -> + event instanceof OrderStatusUpdateEvent e + && e.buyerId().equals(1L) + && e.orderId().equals(orderId) + && e.status() == OrderStatus.SHIPPED + )); } @@ void 관리자_주문상태_변경_관리자없음_실패_테스트() { @@ assertEquals(ErrorEnum.ADMIN_NOT_FOUND, exception.getErrorEnum()); + verifyNoInteractions(eventPublisher); } @@ void 관리자_주문상태_변경_주문없음_실패_테스트() { @@ assertEquals(ErrorEnum.ORDER_NOT_FOUND, exception.getErrorEnum()); + verifyNoInteractions(eventPublisher); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/example/allinmarket/admin/order/service/AdminOrderServiceTest.java` around lines 174 - 255, Add assertions that the event publisher's publishEvent is called with the correct payload in the success test and not called in the failure tests: in 관리자_주문상태_변경_성공_테스트 after calling adminOrderService.updateStatus(adminId, orderId, request) verify the mocked event publisher's publishEvent was invoked once with an event whose payload contains the expected buyerId (use createOrderMock(orderId)'s buyer id to compare, e.g. verify(eventPublisher).publishEvent(argThat(event -> /* payload contains buyerId */))); and in 관리자_주문상태_변경_관리자없음_실패_테스트, 관리자_주문상태_변경_주문없음_실패_테스트 and 관리자_주문상태_변경_불가상태_실패_테스트 verify(eventPublisher, never()).publishEvent(any()) so no event is emitted on failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 76-77: Update the CI env var for connecting to the Redis service:
change SPRING_REDIS_HOST from "redis" to "localhost" (or "127.0.0.1") so the job
running on runs-on: ubuntu-latest can reach the service container; keep
SPRING_REDIS_PORT: 6379 unchanged and ensure any code that reads
SPRING_REDIS_HOST uses the new value.
In
`@src/main/java/com/example/allinmarket/domain/order/event/OrderStatusUpdateEventListener.java`:
- Around line 73-75: The catch in OrderStatusUpdateEventListener that swallows
JsonProcessingException should instead propagate the failure so the missing
order-status notification is observable: in the catch for
JsonProcessingException (inside the method handling the event), log the error as
you do but then rethrow a runtime exception (or wrap and throw) so the error
surfaces to the caller/monitoring; update the catch block around the
serialization call that currently logs "주문 상태 변경 이벤트 직렬화 실패..." to both log and
throw (e.g., new IllegalStateException or custom unchecked) preserving the
original exception as the cause.
In
`@src/test/java/com/example/allinmarket/admin/refund/service/AdminRefundOptimisticLockServiceTest.java`:
- Around line 63-66: The MySQL TestContainer declaration using the floating tag
"mysql:8.0" reduces CI reproducibility; update the static MySQLContainer<?>
mysql declaration in AdminRefundOptimisticLockServiceTest to reference a fixed
image tag or digest (e.g., "mysql:8.0.36" or use
DockerImageName.parse("mysql:8.0.36") or a sha256 digest) so the container uses
an immutable image version across runs.
---
Outside diff comments:
In
`@src/test/java/com/example/allinmarket/admin/order/service/AdminOrderServiceTest.java`:
- Around line 174-255: Add assertions that the event publisher's publishEvent is
called with the correct payload in the success test and not called in the
failure tests: in 관리자_주문상태_변경_성공_테스트 after calling
adminOrderService.updateStatus(adminId, orderId, request) verify the mocked
event publisher's publishEvent was invoked once with an event whose payload
contains the expected buyerId (use createOrderMock(orderId)'s buyer id to
compare, e.g. verify(eventPublisher).publishEvent(argThat(event -> /* payload
contains buyerId */))); and in 관리자_주문상태_변경_관리자없음_실패_테스트, 관리자_주문상태_변경_주문없음_실패_테스트
and 관리자_주문상태_변경_불가상태_실패_테스트 verify(eventPublisher, never()).publishEvent(any())
so no event is emitted on failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b88c0f3d-7de0-4b6f-8a06-b1306f2db574
📒 Files selected for processing (10)
.github/workflows/ci.ymlsrc/main/java/com/example/allinmarket/admin/order/service/AdminOrderService.javasrc/main/java/com/example/allinmarket/domain/order/event/OrderStatusUpdateEvent.javasrc/main/java/com/example/allinmarket/domain/order/event/OrderStatusUpdateEventListener.javasrc/main/java/com/example/allinmarket/domain/order/event/dto/OrderStatusUpdateEventRequest.javasrc/main/java/com/example/allinmarket/domain/order/repository/OrderRepository.javasrc/test/java/com/example/allinmarket/admin/order/service/AdminOrderServiceTest.javasrc/test/java/com/example/allinmarket/admin/refund/service/AdminRefundOptimisticLockServiceTest.javasrc/test/java/com/example/allinmarket/common/config/TestRedisConfig.javasrc/test/resources/application-test.yml
📜 Review details
🧰 Additional context used
🪛 Checkov (3.2.526)
src/test/resources/application-test.yml
[low] 24-25: Base64 High Entropy String
(CKV_SECRET_6)
.github/workflows/ci.yml
[low] 75-76: Base64 High Entropy String
(CKV_SECRET_6)
🪛 PMD (7.24.0)
src/main/java/com/example/allinmarket/domain/order/event/OrderStatusUpdateEventListener.java
[Low] 71-71: Too many arguments, expected 3 arguments but found 4 (InvalidLogMessageFormat)
(Error Prone)
[Low] 74-74: Too many arguments, expected 2 arguments but found 3 (InvalidLogMessageFormat)
(Error Prone)
🔇 Additional comments (8)
src/main/java/com/example/allinmarket/domain/order/repository/OrderRepository.java (1)
22-26:findByIdWithBuyerfetch join 적용이 적절합니다.Line 22-26 쿼리로
buyer를 함께 로딩해 이벤트 발행 시 추가 조회를 피한 점이 좋습니다.src/main/java/com/example/allinmarket/domain/order/event/OrderStatusUpdateEvent.java (1)
6-9: 이벤트 계약 변경(buyerId)이 일관됩니다.Line 6-9 변경이 상태 변경 이벤트의 실제 수신자 식별자 기준과 잘 맞습니다.
src/main/java/com/example/allinmarket/domain/order/event/dto/OrderStatusUpdateEventRequest.java (1)
6-8: 요청 DTO 필드 변경이 이벤트 스키마와 맞습니다.Line 6 기준
buyerId전환으로 발행/전송 데이터 모델이 정렬되었습니다.src/main/java/com/example/allinmarket/admin/order/service/AdminOrderService.java (1)
42-47: 주문 조회 방식과 이벤트 payload 전환이 일관됩니다.Line 42, Line 47에서
findByIdWithBuyer+buyerId발행 흐름으로 맞춘 점이 좋습니다.src/test/java/com/example/allinmarket/common/config/TestRedisConfig.java (1)
23-26: RedissonClient 목 빈 추가 방향이 적절합니다.테스트 컨텍스트에서 Redisson 의존성을 안전하게 대체해, 통합 테스트 기동 안정성에 도움이 됩니다.
.github/workflows/ci.yml (1)
40-49: Redis 서비스 컨테이너/헬스체크 구성은 좋습니다.테스트 단계에서 인프라 의존성을 명시적으로 준비하는 방향이 CI 신뢰성을 높입니다.
src/test/resources/application-test.yml (1)
2-7: 테스트 프로필에서의 Redis 관련 비활성화/오버라이딩 설정이 일관적입니다.테스트 컨텍스트 충돌을 줄이려는 이번 PR 목적과 맞는 구성입니다.
src/test/java/com/example/allinmarket/admin/refund/service/AdminRefundOptimisticLockServiceTest.java (1)
110-110:impUid를 테스트마다 유니크하게 만든 변경은 충돌 방지에 유효합니다.결제 식별자 중복으로 인한 간헐적 테스트 실패 가능성을 잘 줄였습니다.
Also applies to: 126-127, 406-407
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
4 participants
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.
개요
주문 상태 변경 이벤트 발행 및 알림 서버에 전송
RedisAutoConfiguration 오버라이딩하여 테스트 코드 실패 해결
작업 내용
기술적 고려사항
주문 조회 시 주문자(buyer)를 fetch join하여 알림 서버에서는 별도 조회 없이 바로 발송할 수 있도록 함
참고사항
Closes #
체크리스트