feat: 경매장 거래 내역 옵션 검색 기능 구현#75
Conversation
…into feat/auction-option-search
…into feat/auction-option-search
✅ 테스트 결과 for PRBuild: success 🧪 테스트 실행 with Gradle |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the auction history search functionality by introducing structured search request DTOs and type-safe enums for sorting and filtering. The changes improve API design by replacing String-based parameters with strongly-typed enums and nested request objects for better validation and maintainability.
Key changes:
- Introduced type-safe enums (
SortField,SortDirection,SearchStandard) to replace String-based parameters - Refactored search request structure into nested DTOs for better organization (price, date, item options)
- Enhanced query logic with complex option filtering using subqueries and GROUP BY/HAVING clauses
- Replaced wildcard import with specific Mockito imports in test files
Reviewed Changes
Copilot reviewed 32 out of 33 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
PageRequestDto.java |
Changed sortBy and direction from String to enum types (SortField, SortDirection) |
SortField.java |
New enum for type-safe sort field values with Jackson JSON mapping |
SortDirection.java |
New enum for ASC/DESC direction with Spring Data conversion |
SearchStandard.java |
New enum for UP/DOWN/EQUAL comparison operators |
AuctionHistorySearchRequest.java |
Restructured to use nested request DTOs instead of flat String fields |
PriceSearchRequest.java |
Fixed parameter naming (PriceTo/PriceFrom → priceFrom/priceTo) and changed primitive to Long |
Multiple *SearchRequest.java |
New request DTOs for structured item option search criteria |
AuctionHistoryQueryDslRepository.java |
Major refactoring with subquery-based option filtering and dynamic sorting |
AuctionHistoryController.java |
Added @ParameterObject annotation for Swagger documentation |
MetalwareInfoServiceTest.java |
Replaced wildcard Mockito import with specific imports |
AuctionHistoryServiceTest.java |
Updated test to match new AuctionHistorySearchRequest constructor |
Comments suppressed due to low confidence (1)
src/main/java/until/the/eternity/auctionhistory/interfaces/rest/dto/enums/SortDirection.java:1
- [nitpick] The
frommethod handles null input by returning a default value, but this duplicates the null handling logic that exists inPageRequestDto.toPageable()(lines 31-32). Consider removing the null check here since the calling code already handles null values, making this method's responsibility clearer - it should only parse non-null strings.
package until.the.eternity.auctionhistory.interfaces.rest.dto.enums;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public String getDescription() { | ||
| return description; | ||
| } | ||
|
|
There was a problem hiding this comment.
The from method serves as both a @JsonCreator for JSON deserialization and a general factory method. Consider adding a JavaDoc comment to clarify this dual purpose and document the behavior when an invalid code is provided (returns DESC as default).
| /** | |
| * Factory method for converting a string code to a {@link SortDirection} enum. | |
| * <p> | |
| * This method serves as both a general factory and as a {@link JsonCreator} for JSON deserialization. | |
| * If the provided code does not match any known direction (case-insensitive), {@code DESC} is returned as the default. | |
| * | |
| * @param code the string code representing the sort direction | |
| * @return the corresponding {@link SortDirection}, or {@code DESC} if the code is invalid | |
| */ |
| @JsonCreator | ||
| public static SortDirection from(String code) { | ||
| if (code == null) { | ||
| return DESC; // 기본값: 내림차순 | ||
| } | ||
| return Arrays.stream(SortDirection.values()) | ||
| .filter(direction -> direction.code.equalsIgnoreCase(code)) | ||
| .findFirst() | ||
| .orElse(DESC); | ||
| } |
There was a problem hiding this comment.
This is a duplicate enum class. Both until.the.eternity.common.enums.SortDirection and until.the.eternity.auctionhistory.interfaces.rest.dto.enums.SortDirection exist with identical implementations. The common enum should be used throughout the codebase to avoid duplication and potential inconsistencies.
| if (date.dateAuctionBuyFrom() != null && !date.dateAuctionBuyFrom().isBlank()) { | ||
| Instant fromInstant = | ||
| LocalDate.parse(date.dateAuctionBuyFrom()) | ||
| .atStartOfDay(ZoneId.systemDefault()) |
There was a problem hiding this comment.
Using ZoneId.systemDefault() can lead to inconsistent behavior across different server environments. Consider using a fixed timezone (e.g., ZoneId.of(\"UTC\") or ZoneId.of(\"Asia/Seoul\")) for consistent date handling across deployments.
| Instant toInstant = | ||
| LocalDate.parse(date.dateAuctionBuyTo()) | ||
| .plusDays(1) | ||
| .atStartOfDay(ZoneId.systemDefault()) |
There was a problem hiding this comment.
Using ZoneId.systemDefault() can lead to inconsistent behavior across different server environments. Consider using a fixed timezone (e.g., ZoneId.of(\"UTC\") or ZoneId.of(\"Asia/Seoul\")) for consistent date handling across deployments.
| return Expressions.numberTemplate( | ||
| Integer.class, "COALESCE({0}, {1}, 0)", aio.optionValue2, aio.optionValue); |
There was a problem hiding this comment.
The castOptionValueToInt method name is misleading - it doesn't cast but rather uses COALESCE to select the first non-null value. Consider renaming to getOptionValueAsInt or coalesceOptionValue to better reflect its actual behavior. Also add a JavaDoc explaining why it checks optionValue2 first before falling back to optionValue.
| // 에르그는 레벨/랭크 통합하여 1개로 카운트 | ||
| if (!ergConditionAdded) { | ||
| conditionCount++; | ||
| ergConditionAdded = true; | ||
| } |
There was a problem hiding this comment.
[nitpick] The logic for preventing duplicate counting of erg conditions (level and rank) requires a boolean flag tracked across multiple if-blocks. Consider extracting erg condition building into a separate method that returns an Optional to encapsulate this logic and make the code more maintainable.
📋 상세 설명
📊 체크리스트
📆 마감일