Conversation
Walkthrough판매자가 주문 항목에 대해 반품 및 교환 요청을 생성할 수 있도록 API·서비스·도메인·리포지토리·응답/요청 DTO 및 테스트를 추가/수정하고, 도메인에 상태 전환 메서드와 sellerComment 필드를 도입했습니다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Controller as SellerOrderController
participant Service as SellerExchangeService
participant OrderRepo as OrderRepository
participant ItemRepo as OrderItemRepository
participant SellerRepo as SellerRepository
participant ExchangeRepo as ExchangeRequestRepository
Client->>Controller: POST /{orderId}/exchanges (command)
Controller->>Service: createExchange(command)
Service->>OrderRepo: findById(orderId)
OrderRepo-->>Service: Order
Service->>SellerRepo: findStoreIdBySellerId(sellerId)
SellerRepo-->>Service: storeId
Service->>ItemRepo: findByOrderIdAndIdIn(orderId, itemIds)
ItemRepo-->>Service: List<OrderItem>
Service->>OrderItem: requestExchange() for each
alt eligible
OrderItem-->>Service: true
Service->>ExchangeRepo: build/save ExchangeRequest (status REQUESTED)
else not eligible
OrderItem-->>Service: false
Service->>Service: mark failed
end
Service->>ExchangeRepo: saveAll(exchanges)
ExchangeRepo-->>Service: saved
Service-->>Controller: ExchangeCreateResponse (summary, success/failed lists)
Controller-->>Client: SingleResult<ExchangeCreateResponse>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/main/java/com/bbangle/bbangle/claim/seller/service/SellerExchangeService.java (1)
37-109:SellerReturnService.createReturn과 거의 동일한 코드입니다.
SellerReturnService리뷰에서 언급한 것과 같이, 이 메서드의 전체 흐름(검증 → 조회 → 소유권 확인 → 아이템 순회 → 일괄 저장 → 응답 구성)과getStoreIdOrThrow,assertOwnedOrderItems헬퍼가 완전히 중복됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/claim/seller/service/SellerExchangeService.java` around lines 37 - 109, The createExchange method duplicates the entire flow found in SellerReturnService.createReturn; refactor by extracting shared validation and persistence logic into a common helper/service used by both methods: move input validation, order and orderItems retrieval, getStoreIdOrThrow and assertOwnedOrderItems calls, and the loop that builds ExchangeRequest/ReturnRequest plus histories into reusable methods (e.g., validateAndFetchOrderAndItems(command), buildExchangeRequests(orderItems, command) or a generic processItemRequests(orderItems, predicate, buildRequestFn)), then have createExchange call those helpers and keep only exchangeRequestRepository.saveAll(...) and orderItemHistoryRepository.saveAll(...) orchestration and response construction; ensure unique symbols referenced (createExchange, SellerReturnService.createReturn, getStoreIdOrThrow, assertOwnedOrderItems, exchangeRequestRepository.saveAll, orderItemHistoryRepository.saveAll) are updated to use the new shared helper/service.
🧹 Nitpick comments (7)
src/main/java/com/bbangle/bbangle/order/seller/controller/dto/request/SellerOrderRequest.java (1)
61-85:reason필드에 유효성 검증 어노테이션이 없습니다.
ReturnCreateRequest와ExchangeCreateRequest모두reason필드에@NotBlank등의 검증이 없습니다. 반품/교환 사유가 필수 정보라면 검증을 추가하는 것이 좋습니다. 선택 사항이라면 현재 상태로 괜찮습니다.기존
ShipmentRegisterRequest에서는 유사 필드에@NotBlank가 적용되어 있어 일관성 측면에서도 확인이 필요합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/order/seller/controller/dto/request/SellerOrderRequest.java` around lines 61 - 85, The records ReturnCreateRequest and ExchangeCreateRequest are missing validation on the reason field; add `@NotBlank` (import javax.validation.constraints.NotBlank) to the reason parameter in both records (e.g., in ReturnCreateRequest.reason and ExchangeCreateRequest.reason) so the DTO enforces non-empty reasons consistent with ShipmentRegisterRequest, leaving the rest of the toCommand builders unchanged.src/test/java/com/bbangle/bbangle/claim/seller/service/SellerReturnServiceUnitTest.java (1)
113-116: 실패 케이스의 검증이 성공 케이스의 구현 변경과 일치하지 않습니다.성공 경로가
saveAll()을 사용하도록 변경되었지만, 실패 케이스(Line 115, 134, 152)에서는 여전히save(any())만 호출되지 않았는지 검증하고 있습니다. 실패 시saveAll()도 호출되지 않아야 하므로, 두 메서드 모두 검증하는 것이 더 정확합니다.제안 (Line 115 예시, 다른 실패 케이스도 동일 적용)
then(returnRequestRepository).should(never()).findAllById(anyList()); then(orderItemHistoryRepository).should(never()).save(any()); + then(orderItemHistoryRepository).should(never()).saveAll(anyList());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/bbangle/bbangle/claim/seller/service/SellerReturnServiceUnitTest.java` around lines 113 - 116, The failure-case verifications in SellerReturnServiceUnitTest still only assert that save(any()) was not called while the success path now uses saveAll(); update each failing test (the assertions around lines 115, 134, 152) to also assert that saveAll(...) was not invoked. Concretely, for the orderItemHistoryRepository assertions (and any other repository that now uses saveAll in the success path), add then(orderItemHistoryRepository).should(never()).saveAll(anyList()) (and equivalent should(never()).saveAll(...) checks for the other mocked repositories) in the failure-case blocks so both save(...) and saveAll(...) are verified as not called.src/test/java/com/bbangle/bbangle/claim/seller/service/SellerReturnServiceCreateTest.java (2)
310-318:newEntity헬퍼가SellerExchangeServiceCreateTest에도 동일하게 존재합니다.두 테스트 클래스에서 동일한 리플렉션 기반 인스턴스 생성 유틸리티를 사용하고 있습니다. 공통 테스트 유틸리티 클래스로 추출하면 중복을 줄일 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/bbangle/bbangle/claim/seller/service/SellerReturnServiceCreateTest.java` around lines 310 - 318, Extract the duplicate reflection-based helper method newEntity into a shared test utility (e.g., TestEntityFactory.newEntity) and replace the private newEntity implementations in SellerReturnServiceCreateTest and SellerExchangeServiceCreateTest with calls to that shared static method; ensure the utility exposes a generic <T> T newEntity(Class<T>) that uses getDeclaredConstructor(), setAccessible(true) and newInstance(), and update imports/usages in both test classes to reference the new utility.
291-308:orderItemIds가null인 경우에 대한 테스트가 누락되어 있습니다.서비스 레이어(Line 40)에서
command.orderItemIds() == null조건도 체크하고 있지만, 테스트에서는 빈 리스트(List.of())만 검증하고 있습니다.null입력에 대한 테스트 케이스도 추가하면 좋겠습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/bbangle/bbangle/claim/seller/service/SellerReturnServiceCreateTest.java` around lines 291 - 308, Add a new unit test in SellerReturnServiceCreateTest that verifies a null orderItemIds triggers ORDER_ITEM_NOT_FOUND: construct a ReturnCreateCommand with orderItemIds(null) (using the builder), call sut.createReturn(command) inside assertThrows(BbangleException.class), and assert the thrown exception's getBbangleErrorCode() equals BbangleErrorCode.ORDER_ITEM_NOT_FOUND; name the test method e.g. givenNullOrderItemIds_whenCreateReturn_thenThrowsException to mirror the existing empty-list test.src/main/java/com/bbangle/bbangle/claim/seller/service/SellerReturnService.java (2)
16-18: 서비스 레이어에서 컨트롤러 DTO를 직접 생성하고 있습니다.
SellerReturnService(claim.seller.service패키지)가SellerOrderResponse,ReturnContent,ReturnCreateResponse(order.seller.controller.dto.response패키지)를 직접 import하여 구성합니다. 이는 서비스 → 컨트롤러 방향의 의존성으로, 계층 분리 원칙에 어긋납니다.서비스가 자체 결과 모델(예:
ReturnCreateResult)을 반환하고, 컨트롤러에서 응답 DTO로 변환하는 구조를 고려해 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/claim/seller/service/SellerReturnService.java` around lines 16 - 18, SellerReturnService is constructing controller-layer DTOs (SellerOrderResponse, ReturnContent, ReturnCreateResponse) which creates a controller->service dependency; change the service to return a service-level result model (e.g., create ReturnCreateResult in the claim.seller.service or claim.seller.service.dto package), update the methods in SellerReturnService to build and return ReturnCreateResult instead of ReturnCreateResponse/SellerOrderResponse/ReturnContent, remove the controller DTO imports from SellerReturnService, and implement the mapping from ReturnCreateResult to SellerOrderResponse/ReturnCreateResponse inside the controller layer (update controller methods that call SellerReturnService to perform this conversion and adjust method signatures/call sites accordingly).
38-110:SellerExchangeService.createExchange와 거의 동일한 코드 — 공통 로직 추출을 권장합니다.
createReturn과SellerExchangeService.createExchange는 입력 검증, 중복 제거, 주문 조회, 소유권 검증, 아이템 순회, 이력 저장, 응답 구성까지 동일한 흐름을 가지고 있으며 차이는requestReturn()/requestExchange()호출과 엔티티 생성 부분뿐입니다. 현재getStoreIdOrThrow와assertOwnedOrderItems도 양쪽에 중복 정의되어 있습니다.Template Method 또는 공통 헬퍼 클래스로 추출하면 향후 검증 로직 변경 시 두 곳을 동시에 수정해야 하는 위험을 줄일 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/claim/seller/service/SellerReturnService.java` around lines 38 - 110, createReturn duplicates most logic in SellerExchangeService.createExchange (input validation, dedup, order lookup, ownership check, iteration, history/save, response build); extract the common workflow into a shared helper or abstract template (e.g., a new SellerRequestService or abstract SellerRequestTemplate) that encapsulates validation, uniqueOrderItemIds creation, order lookup, getStoreIdOrThrow and assertOwnedOrderItems usage, collecting histories and building the summary/ReturnCreateResponse; have createReturn and createExchange delegate to that helper and supply only the differing bits as lambdas/strategy parameters (the predicate to check requestable: orderItem.requestReturn()/requestExchange() and a factory to create ReturnRequest/ExchangeRequest entities) and reuse orderItemHistoryRepository.saveAll and returnRequestRepository.saveAll call sites via the shared method.src/main/java/com/bbangle/bbangle/claim/seller/service/SellerExchangeService.java (1)
76-90:requestExchange()와requestReturn()메서드가 조건 판단과 상태 변경을 동시에 수행합니다.두 메서드 모두 전형적인 CQS(Command-Query Separation) 위반을 보입니다. 상태 조건을 확인한 후, 유효한 경우
orderStatus를 변경하면서 동시에true를 반환합니다:
requestExchange(): 상태가SHIPPED또는PURCHASE_CONFIRMED가 아니면false반환 (상태 변경 없음)- 상태가 유효하면
orderStatus = EXCHANGE_REQUEST로 설정 후true반환이 패턴은 동작하지만 가독성을 떨어뜨리고, 조건문 내에서의 암묵적인 상태 변경으로 인해 코드 흐름을 따라가기 어렵습니다.
canRequestExchange()(조건 확인만) 과applyExchangeRequest()(상태 변경만) 로 분리하여, 조건 확인과 상태 변경을 명시적으로 분리하는 것을 고려해 보세요. 동일한 문제가SellerReturnService의requestReturn()에도 적용됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/claim/seller/service/SellerExchangeService.java` around lines 76 - 90, The loop currently calls orderItem.requestExchange() which both checks and mutates state; replace this with a pure predicate and a separate mutator: add methods canRequestExchange() (returns boolean, no mutation) and applyExchangeRequest() (sets orderStatus to EXCHANGE_REQUEST) on the OrderItem, then in SellerExchangeService use if (orderItem.canRequestExchange()) { orderItem.applyExchangeRequest(); ... } to build ExchangeRequest, historiesToSave.add(OrderItemHistory.create(orderItem)), and successOrderItemIds; otherwise add to failedOrderItemIds; also apply the same refactor idea to SellerReturnService.requestReturn() by introducing canRequestReturn() and applyReturnRequest() and updating its caller to first check then mutate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/bbangle/bbangle/claim/domain/ExchangeRequest.java`:
- Around line 34-47: The entity ExchangeRequest has a sellerComment field used
by approve/reject but the exchange_request table lacks the seller_comment
column; add a Flyway migration that runs "ALTER TABLE exchange_request ADD
COLUMN seller_comment VARCHAR(255) COMMENT '판매자의 승인/거절 사유';" (new V migration
file) and update the ExchangeRequest class to annotate the field with
`@Column`(name = "seller_comment") on the sellerComment property so the JPA
mapping matches the DB; ensure the migration is applied before deploying changes
that call ExchangeRequest.approve/reject.
In
`@src/main/java/com/bbangle/bbangle/claim/seller/service/SellerReturnService.java`:
- Around line 63-66: The code skips ownership verification when foundIds is
empty, allowing a caller to infer item existence; in SellerReturnService update
the logic so that if foundIds.isEmpty() you either perform an early return (mark
all requested orderItemIds as failed) or call
assertOwnedOrderItems(order.getId(), Collections.emptyList(), storeId) / verify
order ownership first—specifically adjust the block that currently checks "if
(!foundIds.isEmpty()) { assertOwnedOrderItems(...)}" to ensure ownership is
validated even when foundIds is empty (use order.getId(), foundIds/orderItemIds,
and storeId to locate the correct checks).
In `@src/main/java/com/bbangle/bbangle/exception/BbangleErrorCode.java`:
- Around line 161-164: The enum BbangleErrorCode has changed numeric values for
DELIVERY_NOT_FOUND and DELIVERY_MODIFY_NOT_ALLOWED which can break clients
relying on those numeric codes; restore the original numeric values for the
existing constants DELIVERY_NOT_FOUND and DELIVERY_MODIFY_NOT_ALLOWED to
preserve compatibility and, if you need new semantics, add new distinct enum
constants (with new names) appended after the existing entries for any new
numeric values instead of renumbering existing ones so the original numeric
codes remain stable.
---
Duplicate comments:
In
`@src/main/java/com/bbangle/bbangle/claim/seller/service/SellerExchangeService.java`:
- Around line 37-109: The createExchange method duplicates the entire flow found
in SellerReturnService.createReturn; refactor by extracting shared validation
and persistence logic into a common helper/service used by both methods: move
input validation, order and orderItems retrieval, getStoreIdOrThrow and
assertOwnedOrderItems calls, and the loop that builds
ExchangeRequest/ReturnRequest plus histories into reusable methods (e.g.,
validateAndFetchOrderAndItems(command), buildExchangeRequests(orderItems,
command) or a generic processItemRequests(orderItems, predicate,
buildRequestFn)), then have createExchange call those helpers and keep only
exchangeRequestRepository.saveAll(...) and
orderItemHistoryRepository.saveAll(...) orchestration and response construction;
ensure unique symbols referenced (createExchange,
SellerReturnService.createReturn, getStoreIdOrThrow, assertOwnedOrderItems,
exchangeRequestRepository.saveAll, orderItemHistoryRepository.saveAll) are
updated to use the new shared helper/service.
---
Nitpick comments:
In
`@src/main/java/com/bbangle/bbangle/claim/seller/service/SellerExchangeService.java`:
- Around line 76-90: The loop currently calls orderItem.requestExchange() which
both checks and mutates state; replace this with a pure predicate and a separate
mutator: add methods canRequestExchange() (returns boolean, no mutation) and
applyExchangeRequest() (sets orderStatus to EXCHANGE_REQUEST) on the OrderItem,
then in SellerExchangeService use if (orderItem.canRequestExchange()) {
orderItem.applyExchangeRequest(); ... } to build ExchangeRequest,
historiesToSave.add(OrderItemHistory.create(orderItem)), and
successOrderItemIds; otherwise add to failedOrderItemIds; also apply the same
refactor idea to SellerReturnService.requestReturn() by introducing
canRequestReturn() and applyReturnRequest() and updating its caller to first
check then mutate.
In
`@src/main/java/com/bbangle/bbangle/claim/seller/service/SellerReturnService.java`:
- Around line 16-18: SellerReturnService is constructing controller-layer DTOs
(SellerOrderResponse, ReturnContent, ReturnCreateResponse) which creates a
controller->service dependency; change the service to return a service-level
result model (e.g., create ReturnCreateResult in the claim.seller.service or
claim.seller.service.dto package), update the methods in SellerReturnService to
build and return ReturnCreateResult instead of
ReturnCreateResponse/SellerOrderResponse/ReturnContent, remove the controller
DTO imports from SellerReturnService, and implement the mapping from
ReturnCreateResult to SellerOrderResponse/ReturnCreateResponse inside the
controller layer (update controller methods that call SellerReturnService to
perform this conversion and adjust method signatures/call sites accordingly).
- Around line 38-110: createReturn duplicates most logic in
SellerExchangeService.createExchange (input validation, dedup, order lookup,
ownership check, iteration, history/save, response build); extract the common
workflow into a shared helper or abstract template (e.g., a new
SellerRequestService or abstract SellerRequestTemplate) that encapsulates
validation, uniqueOrderItemIds creation, order lookup, getStoreIdOrThrow and
assertOwnedOrderItems usage, collecting histories and building the
summary/ReturnCreateResponse; have createReturn and createExchange delegate to
that helper and supply only the differing bits as lambdas/strategy parameters
(the predicate to check requestable: orderItem.requestReturn()/requestExchange()
and a factory to create ReturnRequest/ExchangeRequest entities) and reuse
orderItemHistoryRepository.saveAll and returnRequestRepository.saveAll call
sites via the shared method.
In
`@src/main/java/com/bbangle/bbangle/order/seller/controller/dto/request/SellerOrderRequest.java`:
- Around line 61-85: The records ReturnCreateRequest and ExchangeCreateRequest
are missing validation on the reason field; add `@NotBlank` (import
javax.validation.constraints.NotBlank) to the reason parameter in both records
(e.g., in ReturnCreateRequest.reason and ExchangeCreateRequest.reason) so the
DTO enforces non-empty reasons consistent with ShipmentRegisterRequest, leaving
the rest of the toCommand builders unchanged.
In
`@src/test/java/com/bbangle/bbangle/claim/seller/service/SellerReturnServiceCreateTest.java`:
- Around line 310-318: Extract the duplicate reflection-based helper method
newEntity into a shared test utility (e.g., TestEntityFactory.newEntity) and
replace the private newEntity implementations in SellerReturnServiceCreateTest
and SellerExchangeServiceCreateTest with calls to that shared static method;
ensure the utility exposes a generic <T> T newEntity(Class<T>) that uses
getDeclaredConstructor(), setAccessible(true) and newInstance(), and update
imports/usages in both test classes to reference the new utility.
- Around line 291-308: Add a new unit test in SellerReturnServiceCreateTest that
verifies a null orderItemIds triggers ORDER_ITEM_NOT_FOUND: construct a
ReturnCreateCommand with orderItemIds(null) (using the builder), call
sut.createReturn(command) inside assertThrows(BbangleException.class), and
assert the thrown exception's getBbangleErrorCode() equals
BbangleErrorCode.ORDER_ITEM_NOT_FOUND; name the test method e.g.
givenNullOrderItemIds_whenCreateReturn_thenThrowsException to mirror the
existing empty-list test.
In
`@src/test/java/com/bbangle/bbangle/claim/seller/service/SellerReturnServiceUnitTest.java`:
- Around line 113-116: The failure-case verifications in
SellerReturnServiceUnitTest still only assert that save(any()) was not called
while the success path now uses saveAll(); update each failing test (the
assertions around lines 115, 134, 152) to also assert that saveAll(...) was not
invoked. Concretely, for the orderItemHistoryRepository assertions (and any
other repository that now uses saveAll in the success path), add
then(orderItemHistoryRepository).should(never()).saveAll(anyList()) (and
equivalent should(never()).saveAll(...) checks for the other mocked
repositories) in the failure-case blocks so both save(...) and saveAll(...) are
verified as not called.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/main/java/com/bbangle/bbangle/claim/domain/ExchangeRequest.javasrc/main/java/com/bbangle/bbangle/claim/domain/ReturnRequest.javasrc/main/java/com/bbangle/bbangle/claim/repository/ExchangeRequestRepository.javasrc/main/java/com/bbangle/bbangle/claim/seller/service/SellerExchangeService.javasrc/main/java/com/bbangle/bbangle/claim/seller/service/SellerReturnService.javasrc/main/java/com/bbangle/bbangle/claim/seller/service/model/ExchangeCreateCommand.javasrc/main/java/com/bbangle/bbangle/claim/seller/service/model/ReturnCreateCommand.javasrc/main/java/com/bbangle/bbangle/exception/BbangleErrorCode.javasrc/main/java/com/bbangle/bbangle/order/domain/OrderItem.javasrc/main/java/com/bbangle/bbangle/order/seller/controller/SellerOrderApi.javasrc/main/java/com/bbangle/bbangle/order/seller/controller/SellerOrderController.javasrc/main/java/com/bbangle/bbangle/order/seller/controller/dto/request/SellerOrderRequest.javasrc/main/java/com/bbangle/bbangle/order/seller/controller/dto/response/SellerOrderResponse.javasrc/test/java/com/bbangle/bbangle/claim/seller/service/SellerExchangeServiceCreateTest.javasrc/test/java/com/bbangle/bbangle/claim/seller/service/SellerReturnServiceCreateTest.javasrc/test/java/com/bbangle/bbangle/claim/seller/service/SellerReturnServiceUnitTest.java
| private String sellerComment; | ||
|
|
||
| @Builder | ||
| public ExchangeRequest( | ||
| OrderItem orderItem, | ||
| String detailReason, | ||
| LocalDateTime decidedAt, | ||
| ExchangeRequestStatus status, | ||
| String sellerComment | ||
| ) { | ||
| super(orderItem, detailReason, decidedAt); | ||
| this.status = status; | ||
| this.sellerComment = sellerComment; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find SQL migration files to check seller_comment column definition
fd -e sql -type f | head -20Repository: eco-dessert-platform/backend
Length of output: 242
🏁 Script executed:
# Search for seller_comment references in SQL files
rg -i 'seller_comment' --type sqlRepository: eco-dessert-platform/backend
Length of output: 178
🏁 Script executed:
# Examine ExchangeRequest.java to see the actual field and parent class
cat -n src/main/java/com/bbangle/bbangle/claim/domain/ExchangeRequest.javaRepository: eco-dessert-platform/backend
Length of output: 2675
🏁 Script executed:
# Check the parent Claim class
fd -e java | xargs rg -l 'class Claim' | head -5Repository: eco-dessert-platform/backend
Length of output: 430
🏁 Script executed:
# Check ReturnRequest.java for comparison
cat -n src/main/java/com/bbangle/bbangle/claim/domain/ReturnRequest.javaRepository: eco-dessert-platform/backend
Length of output: 2697
🏁 Script executed:
# Check the parent Claim class structure
cat -n src/main/java/com/bbangle/bbangle/claim/domain/Claim.java | head -100Repository: eco-dessert-platform/backend
Length of output: 1919
🏁 Script executed:
# Check the database migration to see full context of seller_comment column
cat -n src/main/resources/flyway/V33__app.sql | grep -A5 -B5 'seller_comment'Repository: eco-dessert-platform/backend
Length of output: 385
🏁 Script executed:
# Search for exchange_request table definition and seller_comment column in all migrations
rg -i 'exchange_request' src/main/resources/flyway/ -A10 -B2 | grep -i 'seller_comment\|alter table\|create table'Repository: eco-dessert-platform/backend
Length of output: 204
🏁 Script executed:
# Check all flyway migrations for exchange_request table creation
fd sql -x grep -l 'exchange_request' src/main/resources/flyway/Repository: eco-dessert-platform/backend
Length of output: 1942
🏁 Script executed:
# Check V21__app.sql for exchange_request table definition and seller_comment
cat -n src/main/resources/flyway/V21__app.sql | grep -A30 'CREATE TABLE exchange_request'Repository: eco-dessert-platform/backend
Length of output: 1407
🏁 Script executed:
# Check all flyway migrations in order to see if seller_comment was added to exchange_request after V33
ls -1 src/main/resources/flyway/ | sort -VRepository: eco-dessert-platform/backend
Length of output: 527
🏁 Script executed:
# Search all migration files for any reference to exchange_request and seller_comment
rg 'exchange_request' src/main/resources/flyway/ -A20Repository: eco-dessert-platform/backend
Length of output: 1790
🏁 Script executed:
# Check V34 and later migrations for any changes to exchange_request
cat src/main/resources/flyway/V34__app.sql | grep -i 'exchange_request\|seller_comment'Repository: eco-dessert-platform/backend
Length of output: 54
🏁 Script executed:
# Check if there are any other migrations that modify exchange_request
for file in src/main/resources/flyway/V3{4,5,6,7}__app.sql; do
echo "=== $file ==="
grep -i 'exchange_request\|alter table exchange' "$file" || echo "No matches"
doneRepository: eco-dessert-platform/backend
Length of output: 392
🏁 Script executed:
# Check if there's a migration that adds seller_comment to exchange_request that we might have missed
rg 'ALTER TABLE exchange_request' src/main/resources/flyway/Repository: eco-dessert-platform/backend
Length of output: 54
🏁 Script executed:
# List all migrations and check content of files after V33
for file in src/main/resources/flyway/V36__app.sql src/main/resources/flyway/V37__app.sql; do
[ -f "$file" ] && echo "=== $file ===" && head -100 "$file"
doneRepository: eco-dessert-platform/backend
Length of output: 1816
exchange_request 테이블에 seller_comment 컬럼이 누락되어 있습니다.
ExchangeRequest.java의 sellerComment 필드(34줄)가 코드에서 사용 중이지만(approve/reject 메서드), 데이터베이스 스키마에서 해당 컬럼이 없습니다. V21에서 exchange_request 테이블이 생성될 때 status 컬럼만 포함되었으며, V33에서는 seller_comment가 return_request 테이블에만 추가되었습니다. 이로 인해 엔티티 저장 시 컬럼 불일치 오류가 발생합니다.
Flyway 마이그레이션 파일에 다음을 추가하여 exchange_request 테이블에 seller_comment 컬럼을 추가해야 합니다:
ALTER TABLE exchange_request
ADD COLUMN seller_comment VARCHAR(255) COMMENT '판매자의 승인/거절 사유';또한 sellerComment 필드에 @Column(name = "seller_comment") 어노테이션을 명시적으로 추가하는 것이 권장됩니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/bbangle/bbangle/claim/domain/ExchangeRequest.java` around
lines 34 - 47, The entity ExchangeRequest has a sellerComment field used by
approve/reject but the exchange_request table lacks the seller_comment column;
add a Flyway migration that runs "ALTER TABLE exchange_request ADD COLUMN
seller_comment VARCHAR(255) COMMENT '판매자의 승인/거절 사유';" (new V migration file) and
update the ExchangeRequest class to annotate the field with `@Column`(name =
"seller_comment") on the sellerComment property so the JPA mapping matches the
DB; ensure the migration is applied before deploying changes that call
ExchangeRequest.approve/reject.
|
|
||
| if (!foundIds.isEmpty()) { | ||
| assertOwnedOrderItems(order.getId(), new ArrayList<>(foundIds), storeId); | ||
| } |
There was a problem hiding this comment.
foundIds가 비어있을 때 소유권 검증이 건너뛰어집니다.
요청된 orderItemIds가 모두 해당 주문에 존재하지 않는 경우, 소유권 검증 없이 모든 아이템이 실패로 처리됩니다. 기능적으로 변경이 발생하지 않아 큰 문제는 아니지만, 판매자가 자신의 스토어 소유가 아닌 주문에 대해 아이템 ID 존재 여부를 간접적으로 확인할 수 있는 여지가 있습니다.
foundIds가 비어있더라도 일관성을 위해 early return하거나, 먼저 주문 자체의 소유권을 검증하는 것을 고려해 주세요.
🛡️ 제안: 아이템이 없을 때 early return
if (!foundIds.isEmpty()) {
assertOwnedOrderItems(order.getId(), new ArrayList<>(foundIds), storeId);
+ } else {
+ return ReturnCreateResponse.of(ReturnContent.of(
+ order.getId(),
+ SellerOrderResponse.Summary.of(requestedCount, 0, requestedCount),
+ List.of(),
+ new ArrayList<>(uniqueOrderItemIds)
+ ));
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/bbangle/bbangle/claim/seller/service/SellerReturnService.java`
around lines 63 - 66, The code skips ownership verification when foundIds is
empty, allowing a caller to infer item existence; in SellerReturnService update
the logic so that if foundIds.isEmpty() you either perform an early return (mark
all requested orderItemIds as failed) or call
assertOwnedOrderItems(order.getId(), Collections.emptyList(), storeId) / verify
order ownership first—specifically adjust the block that currently checks "if
(!foundIds.isEmpty()) { assertOwnedOrderItems(...)}" to ensure ownership is
validated even when foundIds is empty (use order.getId(), foundIds/orderItemIds,
and storeId to locate the correct checks).
| RETURN_NOT_ALLOWED(-782, "반품 요청이 불가능한 상태입니다.", BAD_REQUEST), | ||
| EXCHANGE_NOT_ALLOWED(-783, "교환 요청이 불가능한 상태입니다.", BAD_REQUEST), | ||
| DELIVERY_NOT_FOUND(-784, "해당 주문상품의 배송 정보를 찾을 수 없습니다.", NOT_FOUND), | ||
| DELIVERY_MODIFY_NOT_ALLOWED(-785, "현재 배송 상태에서는 운송장을 수정할 수 없습니다.", BAD_REQUEST); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for usages of the old error code numbers (-782, -783)
echo "=== Searching for -782 or -783 usage outside BbangleErrorCode.java ==="
rg -n '\-782|\-783' --type=java -g '!BbangleErrorCode.java' || echo "No matches found"
echo ""
echo "=== Searching for DELIVERY_NOT_FOUND and DELIVERY_MODIFY_NOT_ALLOWED references ==="
rg -n 'DELIVERY_NOT_FOUND|DELIVERY_MODIFY_NOT_ALLOWED' --type=java -g '!BbangleErrorCode.java' || echo "No matches found"
echo ""
echo "=== Checking the file structure around error codes ==="
fd BbangleErrorCode.java --type=fRepository: eco-dessert-platform/backend
Length of output: 1014
🏁 Script executed:
#!/bin/bash
# Get the full context of error code definitions to understand the pattern
if [ -f "src/main/java/com/bbangle/bbangle/exception/BbangleErrorCode.java" ]; then
echo "=== BbangleErrorCode.java content (lines 150-180) ==="
sed -n '150,180p' src/main/java/com/bbangle/bbangle/exception/BbangleErrorCode.java
fiRepository: eco-dessert-platform/backend
Length of output: 1539
🏁 Script executed:
#!/bin/bash
# Search for any API or controller endpoints that might return these error codes
echo "=== Searching for endpoints that might return delivery-related errors ==="
rg -n 'DELIVERY_NOT_FOUND|DELIVERY_MODIFY_NOT_ALLOWED|delivery|delivery' --type=java -A 3 | head -100 || echo "No matches found"Repository: eco-dessert-platform/backend
Length of output: 11347
에러 코드의 숫자 변경으로 인한 API 하위 호환성 문제
DELIVERY_NOT_FOUND(-782 → -784)와 DELIVERY_MODIFY_NOT_ALLOWED(-783 → -785)의 숫자 코드가 변경되었습니다. 현재 코드베이스에는 이 숫자 코드에 대한 하드코딩된 참조가 없으므로 내부적으로는 영향이 없지만, 클라이언트 앱이나 외부 시스템이 이 숫자 코드를 API 응답으로 받아 처리하고 있다면 하위 호환성이 깨질 수 있습니다.
새 에러 코드를 기존 코드 뒤에 배치하여 기존 숫자를 유지하는 것을 권장합니다.
제안
ORDER_INVALID_STATUS(-781, "요청하신 order의 상태로 변경할 수 없습니다", BAD_REQUEST),
- RETURN_NOT_ALLOWED(-782, "반품 요청이 불가능한 상태입니다.", BAD_REQUEST),
- EXCHANGE_NOT_ALLOWED(-783, "교환 요청이 불가능한 상태입니다.", BAD_REQUEST),
- DELIVERY_NOT_FOUND(-784, "해당 주문상품의 배송 정보를 찾을 수 없습니다.", NOT_FOUND),
- DELIVERY_MODIFY_NOT_ALLOWED(-785, "현재 배송 상태에서는 운송장을 수정할 수 없습니다.", BAD_REQUEST);
+ DELIVERY_NOT_FOUND(-782, "해당 주문상품의 배송 정보를 찾을 수 없습니다.", NOT_FOUND),
+ DELIVERY_MODIFY_NOT_ALLOWED(-783, "현재 배송 상태에서는 운송장을 수정할 수 없습니다.", BAD_REQUEST),
+ RETURN_NOT_ALLOWED(-784, "반품 요청이 불가능한 상태입니다.", BAD_REQUEST),
+ EXCHANGE_NOT_ALLOWED(-785, "교환 요청이 불가능한 상태입니다.", BAD_REQUEST);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RETURN_NOT_ALLOWED(-782, "반품 요청이 불가능한 상태입니다.", BAD_REQUEST), | |
| EXCHANGE_NOT_ALLOWED(-783, "교환 요청이 불가능한 상태입니다.", BAD_REQUEST), | |
| DELIVERY_NOT_FOUND(-784, "해당 주문상품의 배송 정보를 찾을 수 없습니다.", NOT_FOUND), | |
| DELIVERY_MODIFY_NOT_ALLOWED(-785, "현재 배송 상태에서는 운송장을 수정할 수 없습니다.", BAD_REQUEST); | |
| DELIVERY_NOT_FOUND(-782, "해당 주문상품의 배송 정보를 찾을 수 없습니다.", NOT_FOUND), | |
| DELIVERY_MODIFY_NOT_ALLOWED(-783, "현재 배송 상태에서는 운송장을 수정할 수 없습니다.", BAD_REQUEST), | |
| RETURN_NOT_ALLOWED(-784, "반품 요청이 불가능한 상태입니다.", BAD_REQUEST), | |
| EXCHANGE_NOT_ALLOWED(-785, "교환 요청이 불가능한 상태입니다.", BAD_REQUEST); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/bbangle/bbangle/exception/BbangleErrorCode.java` around
lines 161 - 164, The enum BbangleErrorCode has changed numeric values for
DELIVERY_NOT_FOUND and DELIVERY_MODIFY_NOT_ALLOWED which can break clients
relying on those numeric codes; restore the original numeric values for the
existing constants DELIVERY_NOT_FOUND and DELIVERY_MODIFY_NOT_ALLOWED to
preserve compatibility and, if you need new semantics, add new distinct enum
constants (with new names) appended after the existing entries for any new
numeric values instead of renumbering existing ones so the original numeric
codes remain stable.
a9e0efc to
289b6d5
Compare
There was a problem hiding this comment.
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/bbangle/bbangle/claim/seller/service/SellerReturnServiceUnitTest.java (1)
115-115:⚠️ Potential issue | 🟠 Major실패 경로 검증이
saveAll이 아닌save를 확인하고 있어 테스트가 무의미합니다.성공 경로(Line 71, 96)에서
saveAll(anyList())로 변경되었지만, 세 개의 실패 경로 테스트(Line 115, 134, 152)는 여전히never().save(any())를 검증합니다. 프로덕션 코드가 이미save()를 사용하지 않도록 변경되었으므로, 이 어서션은 SUT의 동작과 무관하게 항상 통과합니다. 즉, 오류 경로에서saveAll이 실제로 호출되더라도 이 테스트들은 실패하지 않습니다.🐛 실패 경로 검증 수정 제안
- then(orderItemHistoryRepository).should(never()).save(any()); // Line 115 + then(orderItemHistoryRepository).should(never()).saveAll(anyList()); - then(orderItemHistoryRepository).should(never()).save(any()); // Line 134 + then(orderItemHistoryRepository).should(never()).saveAll(anyList()); - then(orderItemHistoryRepository).should(never()).save(any()); // Line 152 + then(orderItemHistoryRepository).should(never()).saveAll(anyList());Also applies to: 134-134, 152-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/bbangle/bbangle/claim/seller/service/SellerReturnServiceUnitTest.java` at line 115, The failure-path assertions currently verify orderItemHistoryRepository.save(any()), which is obsolete because production code now calls saveAll; update the three tests that assert never().save(any()) to assert never().saveAll(anyList()) instead (refer to orderItemHistoryRepository.saveAll and the failing test methods in SellerReturnServiceUnitTest), ensuring you import/use Mockito.anyList() so the tests correctly fail if saveAll is invoked on error paths.
♻️ Duplicate comments (1)
src/main/java/com/bbangle/bbangle/exception/BbangleErrorCode.java (1)
161-164:⚠️ Potential issue | 🟠 Major기존 에러 코드 번호 변경으로 인한 하위 호환성 문제 (미해결)
DELIVERY_NOT_FOUND(-782 → -784)와DELIVERY_MODIFY_NOT_ALLOWED(-783 → -785)의 숫자 코드가 변경되어 있습니다. 이미 배포된 클라이언트(앱, 프론트엔드, 외부 연동 등)가 해당 숫자 코드를 직접 비교·처리하고 있다면 이전 리뷰에서 지적된 대로 하위 호환성이 깨집니다. 새 상수를 기존 코드 앞에 삽입하는 대신 뒤에 추가해 기존 번호를 유지하세요.🔧 제안 수정
ORDER_INVALID_STATUS(-781, "요청하신 order의 상태로 변경할 수 없습니다", BAD_REQUEST), - RETURN_NOT_ALLOWED(-782, "반품 요청이 불가능한 상태입니다.", BAD_REQUEST), - EXCHANGE_NOT_ALLOWED(-783, "교환 요청이 불가능한 상태입니다.", BAD_REQUEST), - DELIVERY_NOT_FOUND(-784, "해당 주문상품의 배송 정보를 찾을 수 없습니다.", NOT_FOUND), - DELIVERY_MODIFY_NOT_ALLOWED(-785, "현재 배송 상태에서는 운송장을 수정할 수 없습니다.", BAD_REQUEST); + DELIVERY_NOT_FOUND(-782, "해당 주문상품의 배송 정보를 찾을 수 없습니다.", NOT_FOUND), + DELIVERY_MODIFY_NOT_ALLOWED(-783, "현재 배송 상태에서는 운송장을 수정할 수 없습니다.", BAD_REQUEST), + RETURN_NOT_ALLOWED(-784, "반품 요청이 불가능한 상태입니다.", BAD_REQUEST), + EXCHANGE_NOT_ALLOWED(-785, "교환 요청이 불가능한 상태입니다.", BAD_REQUEST);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/exception/BbangleErrorCode.java` around lines 161 - 164, The numeric values for DELIVERY_NOT_FOUND and DELIVERY_MODIFY_NOT_ALLOWED in BbangleErrorCode were shifted, breaking compatibility; restore their original numeric codes (set DELIVERY_NOT_FOUND to -782 and DELIVERY_MODIFY_NOT_ALLOWED to -783) so existing clients keep the same values, and if you need new error codes add new enum constants at the end of BbangleErrorCode with new numbers instead of inserting them before existing constants.
🧹 Nitpick comments (1)
src/main/java/com/bbangle/bbangle/order/seller/controller/SellerOrderApi.java (1)
78-90:ReturnContent·ExchangeContent구조 중복 — 공통 타입으로 추출 고려
SellerOrderResponse.java의ReturnContent와ExchangeContent는 필드 구성이 완전히 동일합니다(orderId,summary,successOrderItemIds,failedOrderItemIds). 현재 두 개의 별도 레코드로 정의되어 있어, 향후 필드가 추가될 때 두 곳을 모두 수정해야 하는 유지보수 부담이 있습니다.
SellerOrderResponse.java에서 공통 레코드(ClaimContent등)로 추출하고, 두 응답 DTO가 이를 재사용하도록 리팩터링하는 것을 고려해 보세요.♻️ 리팩터링 예시 (`SellerOrderResponse.java`)
+ `@Builder` + `@Schema`(description = "반품/교환 요청 응답 컨텐츠") + public record ClaimContent( + + `@Schema`(description = "주문 ID") + Long orderId, + + `@Schema`(description = "요청 요약 정보") + Summary summary, + + `@Schema`(description = "성공한 주문상품 ID 목록") + List<Long> successOrderItemIds, + + `@Schema`(description = "실패한 주문상품 ID 목록") + List<Long> failedOrderItemIds + + ) { + public static ClaimContent of( + Long orderId, + Summary summary, + List<Long> successOrderItemIds, + List<Long> failedOrderItemIds + ) { + return ClaimContent.builder() + .orderId(orderId) + .summary(summary) + .successOrderItemIds(successOrderItemIds) + .failedOrderItemIds(failedOrderItemIds) + .build(); + } + } `@Builder` `@Schema`(description = "판매자 반품 요청 생성 응답 DTO") public record ReturnCreateResponse( `@Schema`(description = "반품 요청 결과 컨텐츠") - ReturnContent content + ClaimContent content ) { ... } `@Builder` `@Schema`(description = "판매자 교환 요청 생성 응답 DTO") public record ExchangeCreateResponse( `@Schema`(description = "교환 요청 결과 컨텐츠") - ExchangeContent content + ClaimContent content ) { ... } - // ReturnContent, ExchangeContent 레코드 삭제🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/order/seller/controller/SellerOrderApi.java` around lines 78 - 90, ReturnContent and ExchangeContent in SellerOrderResponse have identical fields causing duplication; extract a shared record (e.g., ClaimContent with fields orderId, summary, successOrderItemIds, failedOrderItemIds) in SellerOrderResponse and have both ReturnCreateResponse and ExchangeCreateResponse reference this shared ClaimContent instead of duplicating records. Update any usages/constructors and imports that refer to ReturnContent or ExchangeContent to use ClaimContent (or alias the existing types to the new record) and run compilation to adjust DTO mappings in controllers/services that construct these response objects.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@src/test/java/com/bbangle/bbangle/claim/seller/service/SellerReturnServiceUnitTest.java`:
- Line 115: The failure-path assertions currently verify
orderItemHistoryRepository.save(any()), which is obsolete because production
code now calls saveAll; update the three tests that assert never().save(any())
to assert never().saveAll(anyList()) instead (refer to
orderItemHistoryRepository.saveAll and the failing test methods in
SellerReturnServiceUnitTest), ensuring you import/use Mockito.anyList() so the
tests correctly fail if saveAll is invoked on error paths.
---
Duplicate comments:
In `@src/main/java/com/bbangle/bbangle/exception/BbangleErrorCode.java`:
- Around line 161-164: The numeric values for DELIVERY_NOT_FOUND and
DELIVERY_MODIFY_NOT_ALLOWED in BbangleErrorCode were shifted, breaking
compatibility; restore their original numeric codes (set DELIVERY_NOT_FOUND to
-782 and DELIVERY_MODIFY_NOT_ALLOWED to -783) so existing clients keep the same
values, and if you need new error codes add new enum constants at the end of
BbangleErrorCode with new numbers instead of inserting them before existing
constants.
---
Nitpick comments:
In
`@src/main/java/com/bbangle/bbangle/order/seller/controller/SellerOrderApi.java`:
- Around line 78-90: ReturnContent and ExchangeContent in SellerOrderResponse
have identical fields causing duplication; extract a shared record (e.g.,
ClaimContent with fields orderId, summary, successOrderItemIds,
failedOrderItemIds) in SellerOrderResponse and have both ReturnCreateResponse
and ExchangeCreateResponse reference this shared ClaimContent instead of
duplicating records. Update any usages/constructors and imports that refer to
ReturnContent or ExchangeContent to use ClaimContent (or alias the existing
types to the new record) and run compilation to adjust DTO mappings in
controllers/services that construct these response objects.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/bbangle/bbangle/exception/BbangleErrorCode.javasrc/main/java/com/bbangle/bbangle/order/seller/controller/SellerOrderApi.javasrc/main/java/com/bbangle/bbangle/order/seller/controller/SellerOrderController.javasrc/main/java/com/bbangle/bbangle/order/seller/controller/dto/request/SellerOrderRequest.javasrc/test/java/com/bbangle/bbangle/claim/seller/service/SellerReturnServiceUnitTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/bbangle/bbangle/order/seller/controller/dto/request/SellerOrderRequest.java
- src/main/java/com/bbangle/bbangle/order/seller/controller/SellerOrderController.java
History
🚀 Major Changes & Explanations
1) 판매자용 반품(Return) 및 교환(Exchange) 생성 기능 구현
SellerReturnService와SellerExchangeService(신규)로 각각 분리하여 구현2) 클레임 생성 검증 로직 및 허용 상태 제한
SHIPPED(배송 중),PURCHASE_CONFIRMED(구매 확정)INVALID_CLAIM_REQUEST로 처리Summary by CodeRabbit
신규 기능
승인/거절 처리
오류 처리
테스트