[Feat] 배송지·주문·결제 API 구현#705
Open
CUCU7103 wants to merge 8 commits into
Open
Conversation
- POST /api/v1/customer/orders/preview: 스토어별 상품 그룹, Board 단위 배송비 계산, 기본 배송지 포함 - POST /api/v1/customer/orders: 다중 판매자 지원, 스토어별 Order 엔티티 분리 생성 - Flyway V59: member_delivery_address 테이블 추가 - MemberDeliveryAddress 도메인 및 레포지토리 추가 - ProductRepository/ProductImgRepository: N+1 방지 JOIN FETCH 쿼리 추가 - BbangleErrorCode: DELIVERY_ADDRESS_NOT_FOUND, ORDER_PRODUCT_EMPTY 추가 - CustomerApiPath: /api/v1/customer/orders/** 인증 경로 추가
- POST /api/v1/customer/delivery-addresses: 배송지 등록
- GET /api/v1/customer/delivery-addresses: 배송지 목록 조회
- PUT /api/v1/customer/delivery-addresses/{id}: 배송지 수정
- DELETE /api/v1/customer/delivery-addresses/{id}: 배송지 삭제
- PATCH /api/v1/customer/delivery-addresses/{id}/default: 기본 배송지 설정
- MemberDeliveryAddress: update(), setDefault(), unsetDefault() 메서드 추가
- BbangleErrorCode: DELIVERY_ADDRESS_ACCESS_DENIED(-858) 추가
- CustomerApiPath: /api/v1/customer/delivery-addresses/** 인증 경로 추가
- 기본 배송지 동시 설정 시 Member 행 비관적 잠금으로 경쟁 조건 해결 - DeliveryAddressSaveRequest.isDefault boolean → Boolean 변환으로 null 처리 - findByIdAndMemberIdAndIsDeletedFalse 단일 쿼리로 배송지 ID 열거 방지 (항상 404) - CustomerOrderController RequestMapping 누락 슬래시 추가 - MemberDeliveryAddressRepository orphan 메서드 제거 - CustomerOrderService dead null check 제거 - PaymentStatus.PENDING → COMPLETED 수정 (OrderStatus.PAYMENT_COMPLETED 일관성) - DeliveryAddressServiceUnitTest 단위 테스트 11개 추가 - DeliveryAddressServiceIntegrationTest 통합 테스트 8개 추가 - CustomerOrderServiceUnitTest 단위 테스트 4개 추가
Walkthrough고객용 배송지 CRUD(기본 배송지 비관적 락 포함), 주문 미리보기/생성(스토어별 그룹핑, 배송비/할인 계산, Order/OrderItem/OrderDelivery/Payment 일괄 생성), 결제 승인(그룹 결제 락, PG 클라이언트 추상화) 기능을 신규 추가하고, 관련 단위/통합 테스트 및 Flyway 마이그레이션 3건을 함께 추가한다. Changes배송지 관리 기능
주문 미리보기/생성 기능
결제 승인 기능
테스트 픽스처 및 데이터 정책 업데이트
Sequence Diagram(s)sequenceDiagram
participant Client as 클라이언트
participant DeliveryCtrl as DeliveryAddressController
participant DeliveryService as DeliveryAddressService
participant OrderCtrl as CustomerOrderController
participant OrderService as CustomerOrderService
participant PaymentCtrl as CustomerPaymentController
participant PaymentService as CustomerPaymentService
participant PaymentClient as PaymentClient
rect rgba(100, 149, 237, 0.5)
Note over Client,DeliveryService: 배송지 관리
Client->>DeliveryCtrl: POST /delivery-addresses (isDefault=true)
DeliveryCtrl->>DeliveryService: addDeliveryAddress(memberId, request)
DeliveryService->>DeliveryService: findByIdWithLock(멤버 비관적 락)
DeliveryService->>DeliveryService: unsetDefault(기존 기본 배송지)
DeliveryService-->>DeliveryCtrl: DeliveryAddressResponse
DeliveryCtrl-->>Client: 201 Created
end
rect rgba(70, 130, 180, 0.5)
Note over Client,OrderService: 주문 미리보기
Client->>OrderCtrl: POST /orders/preview
OrderCtrl->>OrderService: getOrderPreview(memberId, request)
OrderService->>OrderService: 상품 수량 집계, JOIN FETCH, 썸네일 수집
OrderService->>OrderService: 배송비/할인 계산, 스토어 그룹핑
OrderService-->>OrderCtrl: OrderPreviewResponse
OrderCtrl-->>Client: SingleResult
end
rect rgba(60, 179, 113, 0.5)
Note over Client,OrderService: 주문 생성
Client->>OrderCtrl: POST /orders
OrderCtrl->>OrderService: createOrders(memberId, request)
OrderService->>OrderService: 배송지 해석, Order/OrderItem/OrderDelivery/Payment 생성
OrderService-->>OrderCtrl: OrderCreateResponse(orderIds, orderNumber)
OrderCtrl-->>Client: SingleResult
end
rect rgba(210, 105, 30, 0.5)
Note over Client,PaymentClient: 결제 승인
Client->>PaymentCtrl: POST /payments/confirm
PaymentCtrl->>PaymentService: confirm(memberId, request)
PaymentService->>PaymentService: 주문 소유권 검증
PaymentService->>PaymentService: findByOrderGroupIdWithLock(결제 비관적 락)
PaymentService->>PaymentService: 결제 상태/금액 검증
PaymentService->>PaymentClient: confirm(paymentKey, orderNumber, amount)
PaymentClient-->>PaymentService: PaymentConfirmResult
PaymentService->>PaymentService: Payment/OrderItem COMPLETED 전환
PaymentService-->>PaymentCtrl: PaymentConfirmResponse(COMPLETED)
PaymentCtrl-->>Client: 200 OK
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~90 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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)
⚔️ Resolve merge conflicts
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
src/test/java/com/bbangle/bbangle/order/customer/service/CustomerOrderServiceUnitTest.java (1)
188-219: ⚡ Quick win
createOrders성공 경로 검증 케이스를 추가해 주세요.현재 성공 검증이
getOrderPreview에만 집중되어 있어deliveryRequest/결제 생성/주문 그룹 생성 같은 핵심 생성 플로우 회귀를 놓치기 쉽습니다.createOrders성공 케이스 1개를 추가해 계약을 고정하는 편이 좋습니다.🤖 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/bbangle/bbangle/order/customer/service/CustomerOrderServiceUnitTest.java` around lines 188 - 219, Add a success test case for the createOrders method in the OrderPreviewSuccess nested class to verify the complete order creation flow. The test should mock the necessary dependencies (productRepository, productImgRepository, memberDeliveryAddressRepository, and other services for payment/delivery/order creation), create an OrderCreateRequest, call customerOrderService.createOrders with appropriate parameters, and assert that the response contains the expected order data and that the core creation operations (deliveryRequest creation, payment creation, and order group creation) are properly executed. This will ensure regression coverage for the full createOrders contract beyond just the getOrderPreview preview calculations.src/test/java/com/bbangle/bbangle/payment/customer/service/CustomerPaymentServiceIntegrationTest.java (1)
89-187: 🏗️ Heavy lift비관적 락 핵심 경로(동시 승인) 통합 테스트를 추가하는 것을 권장합니다.
현재는 단일 요청 기준 검증만 있어, 동일
orderNumber동시 승인 시 “한 건만 성공” 계약을 회귀로부터 보호하기 어렵습니다. 2개 스레드 동시 호출 테스트를 추가해 한 건 성공 + 한 건PAYMENT_ALREADY_COMPLETED를 검증해 주세요.🤖 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/bbangle/bbangle/payment/customer/service/CustomerPaymentServiceIntegrationTest.java` around lines 89 - 187, Add a new integration test method in the ConfirmFail nested test class to verify the pessimistic locking behavior during concurrent payment confirmation. Create a test that spawns two threads making simultaneous calls to customerPaymentService.confirm() with the same orderNumber and PaymentConfirmRequest, ensuring proper thread synchronization so both threads execute the call at nearly the same time. Verify that one thread succeeds (the first to acquire the lock) and the other thread receives a BbangleException with BbangleErrorCode.PAYMENT_ALREADY_COMPLETED, confirming that the "only one success per orderNumber" contract is protected against concurrent confirmation attempts.src/main/resources/flyway/V60__app.sql (1)
1-17: ⚡ Quick win기본 배송지 유일성을 DB 레벨에서도 보장하는 것을 검토해보세요.
현재 스키마는 동일
member_id에 대해 여러 개의is_default=1행을 허용합니다. 서비스 레이어에서 비관적 락으로 동시성을 제어하고 있지만, DB 직접 접근이나 애플리케이션 버그 시 데이터 무결성이 깨질 수 있습니다.MySQL 8.0+에서 partial unique index(filtered index)를 활용하거나, 또는 별도의 CHECK 제약이나 트리거로 보강할 수 있습니다.
💡 부분 유니크 인덱스 예시
MySQL 8.0.13+ 부터 functional index를 사용할 수 있습니다:
CREATE UNIQUE INDEX idx_mda_member_default ON member_delivery_address (member_id, (CASE WHEN is_default = 1 THEN 1 END));또는 애플리케이션 레벨 제어만으로 충분하다고 판단하시면 현재 상태를 유지하셔도 됩니다.
🤖 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/main/resources/flyway/V60__app.sql` around lines 1 - 17, The member_delivery_address table creation lacks database-level enforcement of the uniqueness constraint for default delivery addresses per member. Currently, multiple rows with is_default=1 can exist for the same member_id, which could compromise data integrity if accessed directly or through application bugs. Add a partial unique index on the member_delivery_address table that enforces uniqueness of the combination of member_id and is_default when is_default equals 1. This can be accomplished using a functional index with a CASE expression in MySQL 8.0.13+, placed after the existing index creation statement in the same migration file.
🤖 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
`@src/main/java/com/bbangle/bbangle/member/customer/service/DeliveryAddressService.java`:
- Around line 75-84: The member lock acquisition in the DeliveryAddressService
is conditional on both makeDefault being true AND the address not already being
default (the !address.isDefault() condition on line 77), which creates a race
condition. When an address is already the default delivery address and an
isDefault=true update request comes in, no lock is acquired, allowing concurrent
transactions to change the default address and potentially violate the invariant
of having exactly one default delivery address per member. Remove the
!address.isDefault() condition from the lock path check so that the
memberRepository.findByIdWithLock(memberId) call and subsequent default address
handling is always executed whenever makeDefault is true, regardless of the
current state of the address.
In
`@src/main/java/com/bbangle/bbangle/order/customer/service/CustomerOrderService.java`:
- Around line 270-275: The OrderDelivery.create() method call in
CustomerOrderService is passing null as the third parameter instead of the
deliveryRequest data from the OrderCreateRequest. Replace the null value with
the appropriate deliveryRequest field from the order creation request object to
ensure delivery request information is preserved during order creation.
In `@src/main/java/com/bbangle/bbangle/order/domain/model/OrderStatus.java`:
- Line 16: The new `PAYMENT_PENDING` status is not included in any of the screen
aggregation groups (lines 54-83), which means it will not be properly counted in
the seller dashboard tab aggregation performed by
`SellerOrderService.buildStatusCounts()`. Determine whether `PAYMENT_PENDING`
should be added to the `PAYMENT_COMPLETED_GROUP` constant or if a separate
aggregation group should be created for it based on the intended behavior. Once
decided, add `PAYMENT_PENDING` to the appropriate Set constant, and consider
adding an explicit comment similar to those in the return/exchange status groups
to clarify when statuses should be included in this group to prevent future
oversights.
In `@src/main/java/com/bbangle/bbangle/order/domain/Order.java`:
- Around line 43-44: The orderGroupId field lacks a not-null constraint, which
could allow null values to be persisted and break the payment group inquiry
contract. Add a `@NotNull` (or `@Column` with nullable=false) constraint annotation
to the orderGroupId field declaration in the Order class to enforce that this
payment group key cannot be null. Additionally, ensure any creation paths or
constructors that initialize the Order entity also validate that orderGroupId is
not null before object creation.
In `@src/main/java/com/bbangle/bbangle/order/repository/OrderRepository.java`:
- Around line 10-12: The findByOrderNumber method in OrderRepository is designed
to return a single Optional result, but the database schema does not enforce
uniqueness on the order_number column, risking
IncorrectResultSizeDataAccessException if duplicates occur. Add a UNIQUE
constraint to the order_number column definition in the migration file
src/main/resources/flyway/V21__app.sql, or alternatively change the
findByOrderNumber method signature to return List<Order> instead of
Optional<Order> to match the actual database behavior. The former approach
(adding UNIQUE constraint) is preferred to maintain data integrity and match the
method's single-result contract.
In
`@src/main/java/com/bbangle/bbangle/payment/customer/service/CustomerPaymentService.java`:
- Around line 41-65: The pessimistic lock acquired by
paymentRepository.findByOrderGroupIdWithLock(orderGroupId) is being held
throughout the entire method including the external paymentClient.confirm(...)
network call, which can cause prolonged blocking if the PG experiences delays.
Refactor to separate the lock-based validation phase from the external call
phase: first execute all validation logic (payment status check, amount
verification, etc.) within the locked transaction to ensure data consistency,
then release the lock and make the paymentClient.confirm(...) call in a separate
transaction outside the lock scope. This will reduce lock contention for
concurrent requests on the same orderGroupId.
In `@src/main/java/com/bbangle/bbangle/payment/domain/Payment.java`:
- Around line 60-68: The confirm() and fail() methods in the Payment class lack
state validation guards, allowing invalid state transitions that can corrupt
payment history. Add validation checks at the start of both methods to ensure
the current paymentStatus is PENDING before allowing the transition. If the
status is not PENDING, throw an appropriate exception (such as
IllegalStateException) with a descriptive message indicating the invalid state
transition. This ensures that payment status can only transition from PENDING to
either COMPLETED (via confirm) or FAILED (via fail), preventing accidental state
overwrites.
In `@src/main/java/com/bbangle/bbangle/payment/repository/PaymentRepository.java`:
- Around line 16-18: The findByOrderGroupIdWithLock method currently only JOIN
FETCHes the order but not the orderItems collection, causing N+1 queries when
service code accesses payment.getOrder().getOrderItems() to modify OrderItem
statuses. Modify the `@Query` annotation to add JOIN FETCH for the orderItems
collection alongside the existing order fetch (e.g., JOIN FETCH p.order o JOIN
FETCH o.orderItems), and add DISTINCT to the SELECT clause to eliminate
duplicate rows that collection JOIN FETCH can create.
In
`@src/test/java/com/bbangle/bbangle/member/customer/service/DeliveryAddressServiceIntegrationTest.java`:
- Around line 113-114: The thread pool executor is not being properly shut down
in the concurrency test, which can cause interference between tests. In the
DeliveryAddressServiceIntegrationTest class at lines 113-114 and lines 173-174,
the executor.shutdown() calls are not waiting for the thread pool to actually
terminate before the test completes. After calling executor.shutdown() at both
locations, add executor.awaitTermination() with an appropriate timeout (e.g., a
few seconds), and if it returns false, call executor.shutdownNow() to forcefully
terminate any remaining threads. This ensures proper cleanup of worker threads
and prevents test interference.
In
`@src/test/java/com/bbangle/bbangle/payment/customer/service/CustomerPaymentServiceUnitTest.java`:
- Around line 205-245: Both test methods canceledPaymentRejected() and
refundedPaymentRejected() are missing verification that the
paymentClient.confirm() method is not invoked when payments in CANCELED or
REFUNDED status are rejected. Add a verify statement using Mockito to assert
that paymentClient.confirm() is never called in each of these test methods,
similar to how other failure case tests validate that the payment confirmation
endpoint is not called. This verification should be added in the when & then
section after the assertThatThrownBy assertion.
---
Nitpick comments:
In `@src/main/resources/flyway/V60__app.sql`:
- Around line 1-17: The member_delivery_address table creation lacks
database-level enforcement of the uniqueness constraint for default delivery
addresses per member. Currently, multiple rows with is_default=1 can exist for
the same member_id, which could compromise data integrity if accessed directly
or through application bugs. Add a partial unique index on the
member_delivery_address table that enforces uniqueness of the combination of
member_id and is_default when is_default equals 1. This can be accomplished
using a functional index with a CASE expression in MySQL 8.0.13+, placed after
the existing index creation statement in the same migration file.
In
`@src/test/java/com/bbangle/bbangle/order/customer/service/CustomerOrderServiceUnitTest.java`:
- Around line 188-219: Add a success test case for the createOrders method in
the OrderPreviewSuccess nested class to verify the complete order creation flow.
The test should mock the necessary dependencies (productRepository,
productImgRepository, memberDeliveryAddressRepository, and other services for
payment/delivery/order creation), create an OrderCreateRequest, call
customerOrderService.createOrders with appropriate parameters, and assert that
the response contains the expected order data and that the core creation
operations (deliveryRequest creation, payment creation, and order group
creation) are properly executed. This will ensure regression coverage for the
full createOrders contract beyond just the getOrderPreview preview calculations.
In
`@src/test/java/com/bbangle/bbangle/payment/customer/service/CustomerPaymentServiceIntegrationTest.java`:
- Around line 89-187: Add a new integration test method in the ConfirmFail
nested test class to verify the pessimistic locking behavior during concurrent
payment confirmation. Create a test that spawns two threads making simultaneous
calls to customerPaymentService.confirm() with the same orderNumber and
PaymentConfirmRequest, ensuring proper thread synchronization so both threads
execute the call at nearly the same time. Verify that one thread succeeds (the
first to acquire the lock) and the other thread receives a BbangleException with
BbangleErrorCode.PAYMENT_ALREADY_COMPLETED, confirming that the "only one
success per orderNumber" contract is protected against concurrent confirmation
attempts.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b52a6a15-9cc9-450a-a17b-717463c1b055
📒 Files selected for processing (39)
src/main/java/com/bbangle/bbangle/board/repository/ProductImgRepository.javasrc/main/java/com/bbangle/bbangle/board/repository/ProductRepository.javasrc/main/java/com/bbangle/bbangle/config/security/CustomerApiPath.javasrc/main/java/com/bbangle/bbangle/exception/BbangleErrorCode.javasrc/main/java/com/bbangle/bbangle/member/customer/controller/DeliveryAddressController.javasrc/main/java/com/bbangle/bbangle/member/customer/controller/dto/request/DeliveryAddressSaveRequest.javasrc/main/java/com/bbangle/bbangle/member/customer/controller/dto/response/DeliveryAddressResponse.javasrc/main/java/com/bbangle/bbangle/member/customer/service/DeliveryAddressService.javasrc/main/java/com/bbangle/bbangle/member/domain/MemberDeliveryAddress.javasrc/main/java/com/bbangle/bbangle/member/repository/MemberDeliveryAddressRepository.javasrc/main/java/com/bbangle/bbangle/member/repository/MemberRepository.javasrc/main/java/com/bbangle/bbangle/order/customer/controller/CustomerOrderController.javasrc/main/java/com/bbangle/bbangle/order/customer/dto/request/OrderCreateRequest.javasrc/main/java/com/bbangle/bbangle/order/customer/dto/request/OrderPreviewRequest.javasrc/main/java/com/bbangle/bbangle/order/customer/dto/response/OrderCreateResponse.javasrc/main/java/com/bbangle/bbangle/order/customer/dto/response/OrderPreviewResponse.javasrc/main/java/com/bbangle/bbangle/order/customer/service/CustomerOrderService.javasrc/main/java/com/bbangle/bbangle/order/domain/Order.javasrc/main/java/com/bbangle/bbangle/order/domain/OrderItem.javasrc/main/java/com/bbangle/bbangle/order/domain/model/OrderStatus.javasrc/main/java/com/bbangle/bbangle/order/repository/OrderRepository.javasrc/main/java/com/bbangle/bbangle/payment/client/PaymentClient.javasrc/main/java/com/bbangle/bbangle/payment/client/StubPaymentClient.javasrc/main/java/com/bbangle/bbangle/payment/client/dto/PaymentConfirmResult.javasrc/main/java/com/bbangle/bbangle/payment/customer/controller/CustomerPaymentController.javasrc/main/java/com/bbangle/bbangle/payment/customer/dto/request/PaymentConfirmRequest.javasrc/main/java/com/bbangle/bbangle/payment/customer/dto/response/PaymentConfirmResponse.javasrc/main/java/com/bbangle/bbangle/payment/customer/service/CustomerPaymentService.javasrc/main/java/com/bbangle/bbangle/payment/domain/Payment.javasrc/main/java/com/bbangle/bbangle/payment/repository/PaymentRepository.javasrc/main/resources/flyway/V60__app.sqlsrc/main/resources/flyway/V61__app.sqlsrc/main/resources/flyway/V62__app.sqlsrc/test/java/com/bbangle/bbangle/fixture/payment/domain/PaymentFixture.javasrc/test/java/com/bbangle/bbangle/member/customer/service/DeliveryAddressServiceIntegrationTest.javasrc/test/java/com/bbangle/bbangle/member/customer/service/DeliveryAddressServiceUnitTest.javasrc/test/java/com/bbangle/bbangle/order/customer/service/CustomerOrderServiceUnitTest.javasrc/test/java/com/bbangle/bbangle/payment/customer/service/CustomerPaymentServiceIntegrationTest.javasrc/test/java/com/bbangle/bbangle/payment/customer/service/CustomerPaymentServiceUnitTest.java
Comment on lines
+113
to
+114
| executor.shutdown(); | ||
|
|
There was a problem hiding this comment.
동시성 테스트에서 스레드풀이 완전히 종료되지 않아 테스트 간 간섭이 생길 수 있습니다.
Line 113, Line 173에서 shutdown()만 호출하고 종료를 기다리지 않아, 타임아웃/실패 케이스에서 워커가 남을 수 있습니다. awaitTermination과 타임아웃 시 shutdownNow()를 추가해 정리를 보장해 주세요.
🔧 제안 수정안
- executor.shutdown();
+ executor.shutdown();
+ if (!executor.awaitTermination(5, TimeUnit.SECONDS)) {
+ executor.shutdownNow();
+ }
@@
- executor.shutdown();
+ executor.shutdown();
+ if (!executor.awaitTermination(5, TimeUnit.SECONDS)) {
+ executor.shutdownNow();
+ }Also applies to: 173-174
🤖 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/bbangle/bbangle/member/customer/service/DeliveryAddressServiceIntegrationTest.java`
around lines 113 - 114, The thread pool executor is not being properly shut down
in the concurrency test, which can cause interference between tests. In the
DeliveryAddressServiceIntegrationTest class at lines 113-114 and lines 173-174,
the executor.shutdown() calls are not waiting for the thread pool to actually
terminate before the test completes. After calling executor.shutdown() at both
locations, add executor.awaitTermination() with an appropriate timeout (e.g., a
few seconds), and if it returns false, call executor.shutdownNow() to forcefully
terminate any remaining threads. This ensures proper cleanup of worker threads
and prevents test interference.
- Shipping.withMemo() 팩토리 추가 및 주문 생성 시 deliveryRequest null 전달 수정 - Order.orderGroupId nullable = false 제약 추가 - Payment.confirm() / fail() PENDING 상태 전이 가드 추가 - PaymentRepository N+1 쿼리 해결: LEFT JOIN FETCH o.orderItems + DISTINCT 적용
- OrderFixture.defaultOrder()에 orderGroupId 기본값 추가 - SellerSettlementItem 통합 테스트 @SQL INSERT에 order_group_id 컬럼 추가
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/test/java/com/bbangle/bbangle/fixture/order/domain/OrderFixture.java (1)
16-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win기본 픽스처의
orderGroupId고정값은 테스트 간 의도치 않은 그룹 결합을 만들 수 있습니다.
defaultOrder()에서 동일한 그룹 ID를 계속 재사용하면, 한 테스트 내 다중 주문 시 그룹 기반 로직 검증이 왜곡될 수 있습니다. 기본값은 생성 시 유니크하게 만들거나, 호출자가 반드시 주입하도록 분리하는 쪽이 더 안전합니다.🤖 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/bbangle/bbangle/fixture/order/domain/OrderFixture.java` at line 16, The hardcoded orderGroupId value in the defaultOrder() method causes unintended side effects when multiple orders are created in the same test, as they will share the same group ID. This can distort validation of group-based logic. Either generate a unique orderGroupId for each invocation of defaultOrder() (using UUID or timestamp-based generation), or refactor the method to accept orderGroupId as a parameter so callers can explicitly provide or override the value instead of relying on the fixed default.
🤖 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 `@src/main/resources/flyway/V63__app.sql`:
- Around line 2-5: The migration adds NOT NULL and UNIQUE constraints directly
to the orders table without first ensuring existing data complies with these
constraints, risking Flyway migration failure. Before applying the ALTER TABLE
statements that modify order_group_id to NOT NULL and add the UNIQUE constraint
on order_number, first add preparatory SQL statements to clean the operational
data: include a statement to identify and correct any NULL values in the
order_group_id column (backfill with appropriate defaults or remove problematic
rows), and include a statement to identify and resolve any duplicate
order_number values (detect duplicates and either remove or normalize them).
Only after ensuring data integrity should you apply the ALTER TABLE MODIFY and
ALTER TABLE ADD UNIQUE KEY statements.
---
Nitpick comments:
In `@src/test/java/com/bbangle/bbangle/fixture/order/domain/OrderFixture.java`:
- Line 16: The hardcoded orderGroupId value in the defaultOrder() method causes
unintended side effects when multiple orders are created in the same test, as
they will share the same group ID. This can distort validation of group-based
logic. Either generate a unique orderGroupId for each invocation of
defaultOrder() (using UUID or timestamp-based generation), or refactor the
method to accept orderGroupId as a parameter so callers can explicitly provide
or override the value instead of relying on the fixed default.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3cf7545e-893f-4107-8518-f4c9f0409784
📒 Files selected for processing (9)
src/main/java/com/bbangle/bbangle/delivery/domain/Shipping.javasrc/main/java/com/bbangle/bbangle/order/customer/service/CustomerOrderService.javasrc/main/java/com/bbangle/bbangle/order/domain/Order.javasrc/main/java/com/bbangle/bbangle/payment/domain/Payment.javasrc/main/java/com/bbangle/bbangle/payment/repository/PaymentRepository.javasrc/main/resources/flyway/V63__app.sqlsrc/test/java/com/bbangle/bbangle/fixture/order/domain/OrderFixture.javasrc/test/java/com/bbangle/bbangle/settlement/seller/controller/SellerSettlementItemControllerIntegrationTest.javasrc/test/java/com/bbangle/bbangle/settlement/seller/excel/controller/SellerSettlementItemExcelIntegrationTest.java
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/com/bbangle/bbangle/order/domain/Order.java
- src/main/java/com/bbangle/bbangle/payment/repository/PaymentRepository.java
- src/main/java/com/bbangle/bbangle/payment/domain/Payment.java
- src/main/java/com/bbangle/bbangle/order/customer/service/CustomerOrderService.java
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.
History
🚀 Major Changes & Explanations
배송지 CRUD API
MemberDeliveryAddress엔티티 및MemberDeliveryAddressRepository추가DeliveryAddressController)주문 API
CustomerOrderController,CustomerOrderService)OrderStatus.PAYMENT_PENDING상태 추가,Order.orderGroupId필드 추가 (다중 스토어 주문 그룹 관리)결제 승인 API
PaymentClient인터페이스 및StubPaymentClient구현체 추가 (로컬/테스트 환경용 스텁)CustomerPaymentService에 비관적 락 기반 동시성 제어 적용Flyway 마이그레이션
member_delivery_address테이블 생성payment테이블에payment_key컬럼 추가orders테이블에order_group_id컬럼 및 인덱스 추가📷 Test Image
💡 ETC
Summary by CodeRabbit
릴리스 노트
New Features
Tests