Conversation
Walkthrough발주 목록을 페이지네이션, 벤더 필터링, 상태 필터링, 날짜 범위 필터링을 지원하는 엔드포인트로 제공하는 기능을 구현했습니다. 컨트롤러, 서비스, 리포지토리 레이어에 걸쳐 쿼리 메서드를 추가하고 테스트를 포함했습니다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant OrderController
participant OrderService
participant OrderRepository
participant Database
Client->>OrderController: GET /api/v1/order?page=0&size=10&vendorId=1&orderStatus=PENDING&fromDate=...
OrderController->>OrderService: getOrderList(userId, vendorId, page, size, status, fromDate, toDate)
OrderService->>OrderService: findOrdersByFilter(storeId, vendorId, status, fromDate, toDate, pageable)
OrderService->>OrderRepository: findAllByStoreIdAndVendorIdAndStatusAndCreatedAtBetween(...)
OrderRepository->>Database: SELECT * FROM orders WHERE store_id=? AND vendor_id=? AND status=? AND created_at BETWEEN ? AND ?
Database-->>OrderRepository: Page<Order>
OrderRepository-->>OrderService: Page<Order>
OrderService->>OrderService: map Order entities to OrderResponse
OrderService-->>OrderController: PageResponse<OrderResponse>
OrderController-->>Client: ApiResponse { success: true, data: PageResponse { ... } }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 분 추가 검토 대상:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/test/java/com/almang/inventory/order/service/OrderServiceTest.java (1)
6-6: 발주 목록 조회 테스트 전반적으로 잘 짜여 있습니다.
PageResponse를 직접 사용해서 서비스 레이어 반환 타입과 정확히 매칭해 보는 구조가 좋습니다. 기본 조회 / 발주처 필터 / 상태 필터 / 날짜 필터까지 한 번에 커버해서 회귀 위험을 많이 줄여줍니다. 딱 필요한 시나리오를 잘 골라오신 느낌입니다.- 다만
발주_목록_조회_기본_조회에_성공한다에서 리스트 순서를메시지1,메시지2로 단언할 때, 내부 정렬 기준(createdAtASC)에 의존하고 있어서 아주 드물게라도 생성 시각이 거의 동일할 경우 DB 구현에 따라 순서가 바뀔 여지가 있습니다.
- 개선 아이디어:
content에 두 개가 모두 포함되는지만extracting(OrderResponse::orderMessage)후containsExactlyInAnyOrder로 검증하거나, 정렬 기준을 명시적으로 테스트하는 별도 테스트를 두는 방법도 있습니다.- 날짜 필터링 테스트는
fromDate만 사용하는 happy-path를 잘 확인하고 있는데, 나중에 유지보수성을 높이려면 아래 케이스도 하나 정도 추가해두면 좋겠습니다.
toDate만 지정했을 때 상한이 제대로 적용되는 케이스fromDate와toDate가 모두 지정된 좁은 구간(예: 하루) 필터링 케이스
이렇게 해두면findOrdersByFilter의 날짜 경계 조건 변경이 생겨도 빠르게 감지할 수 있습니다.관련 개념은 Spring Data JPA 공식 문서의 “Query Methods”와 “Paging and Sorting” 섹션을 함께 보면 정렬 + 페이징 테스트 설계에 도움이 됩니다.
Also applies to: 417-595
src/main/java/com/almang/inventory/order/repository/OrderRepository.java (1)
12-30: 필터 조합별 메서드 분리는 현재 요구사항에는 잘 맞습니다.
findAllByStoreId...CreatedAtBetween시그니처가 서비스의 분기(vendorId,status조합)와 1:1로 대응되어 있어서, 가독성과 추적성이 좋습니다. Spring Data JPA 메서드 네이밍 규칙에도 잘 맞습니다.- 다만 필터 조합이 앞으로 더 늘어날 가능성이 있다면, 메서드 수가 급격히 늘어나는 패턴이라 이후에는 Specification/Querydsl 같은 동적 쿼리 방식으로 옮기는 것도 고려해볼 만합니다.
- 운영 측면에서는
store_id,vendor_id,status,created_at조합으로 조회가 집중될 것이므로, 해당 컬럼들에 적절한 인덱스가 잡혀 있는지 DBA·운영 측과 한 번만 맞춰 두시면 대용량 데이터에서 성능을 안정적으로 가져갈 수 있습니다.자세한 패턴은 Spring Data JPA 레퍼런스의 “Query Creation” 및 “Specifications” 섹션을 참고해 보시면 좋습니다.
src/test/java/com/almang/inventory/order/controller/OrderControllerTest.java (2)
11-11: 발주 목록 조회 성공 케이스 테스트 구성이 깔끔합니다.
PageResponse<OrderResponse>를 직접 만들어orderService.getOrderList를 mock 하고, 컨트롤러가 이를 그대로ApiResponse구조로 감싸는지 검증하는 흐름이 자연스럽습니다.- 상태 코드, 메시지(
GET_ORDER_LIST_SUCCESS),totalElements, 각orderId까지 핵심 필드를 정확히 짚어서 API 계약을 잘 고정하고 있습니다.- 여유가 있다면 리스트 항목에 대해
vendorId,orderStatus같은 다른 필드도 한두 개 정도 추가로 단언해 두면, 응답 DTO 변경 시 깨질 지점을 좀 더 촘촘하게 잡아줄 수 있습니다.Spring MVC 테스트 패턴은 공식 문서의 “Testing Spring MVC” 섹션을 참고하시면, 다양한 JSONPath 단언 패턴 예제를 더 볼 수 있습니다.
Also applies to: 319-376
378-414: 예외 매핑 테스트로 에러 처리 계약을 잘 검증하고 있습니다.
USER_NOT_FOUND→ 404,ORDER_ACCESS_DENIED→ 403으로 매핑되는지와,status,message,data부재까지 꼼꼼히 확인해서 에러 응답 포맷이 깨지지 않도록 잘 방어하고 있습니다.- 동일한 패턴이 다른 컨트롤러에도 반복된다면, 나중에 공통 에러 응답에 대한 추상화/헬퍼를 도입해 테스트 코드 중복을 줄이는 것도 한 번 생각해 볼 만합니다.
에러 처리 전반은 Spring 공식 문서의 “Error Handling” 및
@ControllerAdvice관련 섹션을 참고하시면 구조화된 패턴을 설계하는 데 도움이 됩니다.src/main/java/com/almang/inventory/order/controller/OrderController.java (1)
4-5: 발주 목록 조회 엔드포인트 설계가 서비스 레이어와 잘 맞습니다.
- 인증 주체에서
userId를 꺼내고, 나머지 필터를@RequestParam(required = false)로 받는 구조가 명확합니다. 서비스 메서드 시그니처와도 일치해서 흐름 추적이 쉽습니다.- 컨트롤러 로그에서
userId,page,size,vendorId,status,fromDate,endDate를 모두 남겨 주는 것도 운영 측면에서 유용할 것 같습니다.- 한 가지 개선 아이디어로, 페이지네이션 기본 동작(예: 페이지는 1부터 시작, 기본
size=20)이PaginationUtil에 숨겨져 있으니, Swagger 문서나@Operation설명에 이 기본값을 짧게 적어 두면 클라이언트 입장에서 API 사용성이 더 좋아집니다. 필요하다면@RequestParam(defaultValue = "1")/"20"으로 명시해 주는 방법도 있습니다.자세한 패턴은 Spring 공식 문서의 “Web MVC framework” 중
@RequestParam, “Pagination and Sorting” 부분을 참고하시면 좋습니다.Also applies to: 7-8, 14-15, 24-25, 66-86
src/main/java/com/almang/inventory/order/service/OrderService.java (2)
3-7: getOrderList 구현이 요구사항을 잘 충족하지만, 목록 조회 특성상 성능·도메인 측면에서 몇 가지 점을 고려해볼 만합니다.
- 사용자→상점 조회 후
storeId기준으로만 검색하는 패턴이 기존 접근 제어와 자연스럽게 맞습니다.PaginationUtil.createPageRequest(page, size, "createdAt")를 통해 1‑based 페이지 파라미터를 0‑based로 변환하는 것도 재사용성 측면에서 좋습니다.orderPage.map(order -> OrderResponse.of(order, order.getItems()))에서order.getItems()가 LAZY 관계라면, 페이지 크기가 커질수록 N+1 문제가 발생할 수 있습니다.
- 단기적으로는 페이지 크기를 너무 크게 받지 않도록 API 가이드를 두는 방법이 있고,
- 장기적으로는 “목록 조회용 요약 DTO”를 별도로 두고, 필요 필드만 projection/fetch join으로 한 번에 가져오는 쿼리를 만드는 것도 고려해볼 만합니다.
- 반환을
PageResponse.from(mapped)로 감싸는 구조는 컨트롤러와 테스트에서 그대로 재사용되고 있어, 페이징 응답 포맷을 중앙에서 관리한다는 점이 좋습니다.N+1 관련 내용은 JPA 베스트 프랙티스나 Spring Data JPA 문서의 pagination + fetch join 제한 사항을 함께 보시면 설계 선택에 도움이 됩니다.
Also applies to: 23-25, 29-32, 78-93
184-221: findOrdersByFilter의 분기와 날짜 범위 처리가 명확하지만, 경계값·입력 검증을 조금만 더 다듬으면 좋겠습니다.
fromDate/toDate를LocalDate로 받고, 내부에서[startOfDay, endOfDay]형태의LocalDateTime범위로 변환한 뒤...CreatedAtBetween을 사용하는 방식이라 도메인 관점에서 이해하기 쉽습니다.endDate.plusDays(1).atStartOfDay().minusNanos(1)로 상한을 “하루 끝”까지 포함시키는 것도 의도가 분명합니다.- 필터 조합 분기(
hasVendor,hasStatus)가 리포지토리 메서드 네이밍과 정확히 매칭돼 있어서, 나중에 읽을 때 실수할 여지가 적습니다.- 개선해볼 만한 지점은 두 가지 정도입니다.
fromDate가toDate보다 이후인 경우, 현재는 조용히 빈 페이지를 반환하는데, 클라이언트 입장에서는 잘못된 입력임을 알려주는 쪽이 더 친절할 수 있습니다. 이때는BaseException(ErrorCode.INVALID_INPUT_VALUE)같이 명시적으로 던지도록 검증을 추가할 수 있습니다.LocalDate.of(1970, 1, 1)를 매직값으로 쓰고 있으니, 상수로 추출하거나 도메인 정책(예: 서비스 런칭일)과 연결한 의미 있는 최소 일자로 바꿔두면 의도가 더 잘 드러납니다.날짜·시간 필터링 로직은 Java Time API 문서와 Spring Data JPA의 “Query Creation” 섹션을 참고하시면, Between 연산과 경계 포함 여부를 더 명확히 정리하는 데 도움이 됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/almang/inventory/global/api/SuccessMessage.java(1 hunks)src/main/java/com/almang/inventory/order/controller/OrderController.java(3 hunks)src/main/java/com/almang/inventory/order/repository/OrderRepository.java(1 hunks)src/main/java/com/almang/inventory/order/service/OrderService.java(4 hunks)src/test/java/com/almang/inventory/order/controller/OrderControllerTest.java(2 hunks)src/test/java/com/almang/inventory/order/service/OrderServiceTest.java(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-22T18:12:13.161Z
Learnt from: JoonKyoLee
Repo: almang2/inventory-server PR: 66
File: src/main/java/com/almang/inventory/order/domain/OrderItem.java:41-53
Timestamp: 2025-11-22T18:12:13.161Z
Learning: In the almang2/inventory-server repository, OrderItem entity update methods (updateQuantity, updatePrice in src/main/java/com/almang/inventory/order/domain/OrderItem.java) do not require null checks because OrderService will validate parameters before calling these update methods, following the same pattern as Product entity updates.
<!--
Applied to files:
src/test/java/com/almang/inventory/order/service/OrderServiceTest.javasrc/main/java/com/almang/inventory/order/service/OrderService.javasrc/main/java/com/almang/inventory/order/repository/OrderRepository.java
📚 Learning: 2025-11-20T10:43:47.489Z
Learnt from: JoonKyoLee
Repo: almang2/inventory-server PR: 33
File: src/main/java/com/almang/inventory/product/domain/Product.java:62-66
Timestamp: 2025-11-20T10:43:47.489Z
Learning: In the almang2/inventory-server repository, Product entity update methods (e.g., updateVendor in src/main/java/com/almang/inventory/product/domain/Product.java) do not require null checks on vendor parameters because ProductService validates vendor existence via findVendorByIdAndValidateAccess before calling update methods, and the vendor field has nullable=false constraint ensuring this.vendor is never null for persisted entities.
Applied to files:
src/test/java/com/almang/inventory/order/service/OrderServiceTest.javasrc/main/java/com/almang/inventory/order/service/OrderService.java
🧬 Code graph analysis (1)
src/main/java/com/almang/inventory/order/service/OrderService.java (2)
src/main/java/com/almang/inventory/global/util/PaginationUtil.java (1)
PaginationUtil(7-17)src/main/java/com/almang/inventory/order/controller/OrderController.java (1)
Slf4j(27-87)
🔇 Additional comments (2)
src/test/java/com/almang/inventory/order/service/OrderServiceTest.java (1)
597-607: USER_NOT_FOUND 예외 경로 테스트가 서비스 계약을 잘 고정해 줍니다.
getOrderList에서 사용자 조회 실패가 곧바로USER_NOT_FOUND로 매핑되는지 검증해서, 이후 리팩터링 시 보안·접근 제어 쪽 회귀를 방지하는 데 도움이 됩니다. 별도의 개선 포인트는 보이지 않습니다.src/main/java/com/almang/inventory/global/api/SuccessMessage.java (1)
38-45: 성공 메시지 enum 확장이 일관성 있게 잘 추가되었습니다.
GET_ORDER_LIST_SUCCESS("발주 목록 조회 성공")가 기존 ORDER 영역의 네이밍·메시지 스타일과 잘 맞습니다. 컨트롤러·테스트에서도 동일 상수를 사용하므로 하드코딩 문자열을 줄인 점이 좋습니다.
✨ 작업 내용
📝 적용 범위
/order📌 참고 사항
Summary by CodeRabbit
새로운 기능
테스트
✏️ Tip: You can customize this high-level summary in your review settings.