-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ Feature/#28 - 4.5 영수증 리뷰 등록하기 API 구현 #37
Conversation
WalkthroughThis pull request introduces a new API endpoint for registering receipt reviews and modifies an existing endpoint related to receipt OCR processing. It adds a method to the Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
🧹 Outside diff range and nitpick comments (7)
src/main/java/com/daon/onjung/account/repository/mysql/StoreRepository.java (1)
12-12
: Consider implementing fuzzy matching for OCR data.Direct string matching with OCR-extracted data might be too strict and could fail to find matching stores due to OCR errors or slight differences in formatting. Consider:
- Implementing fuzzy matching using techniques like Levenshtein distance
- Normalizing store names and addresses before comparison
- Adding additional query methods with partial matches
Example implementation approach:
// Add these methods for more flexible matching Optional<Store> findFirstByOcrStoreNameContainingAndOcrStoreAddressContaining( String ocrStoreName, String ocrStoreAddress); @Query("SELECT s FROM Store s " + "WHERE LEVENSHTEIN(s.ocrStoreName, :ocrStoreName) <= :threshold " + "AND LEVENSHTEIN(s.ocrStoreAddress, :ocrStoreAddress) <= :threshold " + "ORDER BY (LEVENSHTEIN(s.ocrStoreName, :ocrStoreName) + " + " LEVENSHTEIN(s.ocrStoreAddress, :ocrStoreAddress)) ASC " + "LIMIT 1") Optional<Store> findClosestMatch( String ocrStoreName, String ocrStoreAddress, int threshold);src/main/java/com/daon/onjung/onjung/application/dto/request/CreateReceiptRequestDto.java (1)
5-5
: Add class-level documentationMissing Javadoc documentation for the DTO class.
Add documentation explaining the purpose and usage:
+/** + * Data Transfer Object for creating a receipt review. + * Used to capture receipt details from the client during review registration. + * + * @param storeName The name of the store where the purchase was made + * @param storeAddress The address of the store + * @param paymentDate The date and time when the payment was made + * @param paymentAmount The total amount paid + */ public record CreateReceiptRequestDto(src/main/java/com/daon/onjung/onjung/domain/service/ReceiptService.java (1)
13-25
: Consider adding audit fields to the Receipt entity.For better tracking and maintenance, consider implementing audit fields (created_at, updated_at) in the Receipt entity. This can be achieved using Spring Data JPA's
@EntityListeners(AuditingEntityListener.class)
and relevant annotations (@CreatedDate
,@LastModifiedDate
).src/main/java/com/daon/onjung/onjung/application/controller/command/OnjungCommandV1Controller.java (2)
36-38
: Documentation could be more descriptive.Consider enhancing the documentation with:
- Expected request/response format
- Possible error scenarios
- Any validation rules
Example documentation:
/** * 4.5 영수증 리뷰 등록하기 * * @param accountId User's account identifier * @param requestDto Receipt review details * @return Empty response with HTTP 201 on success * @throws InvalidRequestException if validation fails * @throws StoreNotFoundException if referenced store doesn't exist */
40-46
: Consider enhancing error handling and response type.A few suggestions for improvement:
- Consider returning the created receipt's ID or details instead of void
- Explicit error handling might be needed for business logic failures
Example implementation:
- public ResponseDto<Void> reviewReceipt( + public ResponseDto<ReceiptDto> reviewReceipt( @AccountID UUID accountId, @RequestBody @Valid CreateReceiptRequestDto requestDto ) { - createReceiptUseCase.execute(accountId, requestDto); - return ResponseDto.created(null); + ReceiptDto receipt = createReceiptUseCase.execute(accountId, requestDto); + return ResponseDto.created(receipt); }src/main/java/com/daon/onjung/onjung/application/service/CreateReceiptService.java (1)
22-30
: Consider extracting store lookup logic to a separate service.The service currently has multiple repository dependencies. Consider extracting the store lookup logic into a dedicated service (e.g.,
StoreSearchService
) to better adhere to the Single Responsibility Principle and improve maintainability.@Service @RequiredArgsConstructor public class CreateReceiptService implements CreateReceiptUseCase { private final UserRepository userRepository; - private final StoreRepository storeRepository; private final ReceiptRepository receiptRepository; + private final StoreSearchService storeSearchService; private final ReceiptService receiptService;src/main/java/com/daon/onjung/core/exception/error/ErrorCode.java (1)
17-17
: Consider reordering the error codes for better maintainability.The new error code (40410) breaks the numerical sequence of error codes. Consider moving it after
NOT_FOUND_RESOURCE
(40404) and before the "Invalid Argument Error" section (40000) to maintain a logical grouping and sequence.NOT_FOUND_END_POINT(40400, HttpStatus.NOT_FOUND, "존재하지 않는 API 엔드포인트입니다."), NOT_FOUND_RESOURCE(40404, HttpStatus.NOT_FOUND, "해당 리소스가 존재하지 않습니다."), + NOT_FOUND_MATCHED_STORE(40410, HttpStatus.NOT_FOUND, "영수증 내용과 일치하는 매장이 존재하지 않습니다."), - NOT_FOUND_MATCHED_STORE(40410, HttpStatus.NOT_FOUND, "영수증 내용과 일치하는 매장이 존재하지 않습니다."), // Invalid Argument Error MISSING_REQUEST_PARAMETER(40000, HttpStatus.BAD_REQUEST, "필수 요청 파라미터가 누락되었습니다."),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
http/onjung/OnjungControllerHttpRequest.http
(1 hunks)src/main/java/com/daon/onjung/account/repository/mysql/StoreRepository.java
(1 hunks)src/main/java/com/daon/onjung/core/exception/error/ErrorCode.java
(1 hunks)src/main/java/com/daon/onjung/onjung/application/controller/command/OnjungCommandV1Controller.java
(1 hunks)src/main/java/com/daon/onjung/onjung/application/dto/request/CreateReceiptRequestDto.java
(1 hunks)src/main/java/com/daon/onjung/onjung/application/service/CreateReceiptService.java
(1 hunks)src/main/java/com/daon/onjung/onjung/application/usecase/CreateReceiptUseCase.java
(1 hunks)src/main/java/com/daon/onjung/onjung/domain/service/ReceiptService.java
(1 hunks)
🔇 Additional comments (8)
src/main/java/com/daon/onjung/onjung/application/usecase/CreateReceiptUseCase.java (1)
1-7
: LGTM! Clean package structure and imports.
The package structure and imports follow clean architecture principles, properly separating the application layer concerns.
src/main/java/com/daon/onjung/account/repository/mysql/StoreRepository.java (1)
12-12
: LGTM! Clean implementation following JPA conventions.
The method signature is well-structured and follows Spring Data JPA naming conventions. Using Optional as the return type is a good practice for handling potential null cases.
src/main/java/com/daon/onjung/onjung/domain/service/ReceiptService.java (2)
3-9
: LGTM! Imports and class structure are well-organized.
The necessary imports are correctly included, and the class is properly annotated as a Spring service.
13-25
: Verify business rules for store-user relationship.
Please confirm if there are any business rules regarding which users can create receipts for which stores. If such rules exist, they should be validated before receipt creation.
http/onjung/OnjungControllerHttpRequest.http (1)
28-39
: Verify API integration with Docker environment
The PR objectives mention that Docker testing is incomplete. Please ensure the API endpoint is tested in the Docker environment before merging.
Run the following script to check Docker-related configurations:
src/main/java/com/daon/onjung/onjung/application/controller/command/OnjungCommandV1Controller.java (2)
3-24
: LGTM! Clean dependency management.
The new imports and field additions follow consistent patterns with proper dependency injection through constructor injection.
26-29
: LGTM! Documentation improvement.
The updated documentation clearly specifies the purpose of the OCR endpoint for store visit authentication.
src/main/java/com/daon/onjung/core/exception/error/ErrorCode.java (1)
17-17
: LGTM! The new error code is well-defined.
The implementation follows the established patterns:
- Appropriate error code range (404xx) for "Not Found" scenarios
- Consistent HTTP status code usage
- Clear and descriptive message
@UseCase | ||
public interface CreateReceiptUseCase { | ||
void execute(UUID accountId, CreateReceiptRequestDto requestDto); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Based on the analysis of the codebase, I can now provide a final response:
The suggestion for error handling enhancement is valid but should align with existing patterns
The codebase shows a robust error handling pattern using:
- A
ResponseDto<T>
wrapper class for all responses - A global exception handler (
HttpGlobalExceptionHandler
) that handles various exceptions - Custom exceptions like
CommonException
andHttpSecurityException
The interface should be updated as follows:
@UseCase
/**
* Use case for creating a new receipt in the system.
*
* @throws CommonException if business rules are violated or validation fails
*/
public interface CreateReceiptUseCase {
- void execute(UUID accountId, CreateReceiptRequestDto requestDto);
+ ResponseDto<Void> execute(UUID accountId, CreateReceiptRequestDto requestDto);
}
This aligns with:
- The project's consistent use of
ResponseDto<T>
for responses - The established exception handling through
CommonException
- The pattern seen in other use cases like
ReceiptOCRUseCase
andReadOnjungBriefUseCase
🔗 Analysis chain
Consider enhancing error handling and validation.
The interface design is clean and follows the Single Responsibility Principle. However, consider the following suggestions:
- The void return type might make it difficult to handle errors effectively. Consider returning a result object that can encapsulate success/failure states.
- The interface would benefit from documentation explaining the expected behavior, possible error conditions, and parameter constraints.
Consider this enhancement:
@UseCase
+/**
+ * Use case for creating a new receipt in the system.
+ *
+ * @throws IllegalArgumentException if the requestDto contains invalid data
+ * @throws EntityNotFoundException if the account is not found
+ * @throws BusinessException if business rules are violated
+ */
public interface CreateReceiptUseCase {
- void execute(UUID accountId, CreateReceiptRequestDto requestDto);
+ ReceiptResult execute(UUID accountId, CreateReceiptRequestDto requestDto);
}
Let's verify the error handling patterns used in other use cases:
Let me gather more specific information about the use cases and their return types.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check error handling patterns in other use cases
# Expected: Find similar use cases to understand the project's error handling conventions
# Search for other UseCase interfaces
ast-grep --pattern 'interface $_ {
$$$
$_($$_) $$$;
$$$
}'
# Search for exception handling patterns
rg -l "throws \w+Exception"
Length of output: 1149
Script:
#!/bin/bash
# Search for UseCase interfaces and their method signatures
ast-grep --pattern 'interface $_ {
$$$
$_ execute($$$);
}'
# Search for any Result/Response type returns in the codebase
rg "Result|Response" --type java -A 2 -B 2
# Look for existing exception classes
fd -e java -x grep -l "Exception"
Length of output: 92223
String storeAddress, | ||
|
||
@JsonProperty("payment_date") | ||
String paymentDate, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Convert date and amount fields to appropriate data types
The current implementation uses String for both payment date and amount, which could lead to parsing issues and data inconsistency.
Consider applying these changes:
@JsonProperty("payment_date")
- String paymentDate,
+ @JsonFormat(pattern = "yyyy-MM-dd HH:mm:ss")
+ LocalDateTime paymentDate,
@JsonProperty("payment_amount")
- String paymentAmount
+ BigDecimal paymentAmount
Also add these imports:
import java.time.LocalDateTime;
import java.math.BigDecimal;
import com.fasterxml.jackson.annotation.JsonFormat;
Also applies to: 15-16
public record CreateReceiptRequestDto( | ||
@JsonProperty("store_name") | ||
String storeName, | ||
|
||
@JsonProperty("store_address") | ||
String storeAddress, | ||
|
||
@JsonProperty("payment_date") | ||
String paymentDate, | ||
|
||
@JsonProperty("payment_amount") | ||
String paymentAmount | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add input validation constraints
The DTO lacks validation annotations which could lead to invalid data being processed.
Add these validations:
public record CreateReceiptRequestDto(
@JsonProperty("store_name")
+ @NotBlank(message = "Store name is required")
+ @Size(max = 100, message = "Store name must be less than 100 characters")
String storeName,
@JsonProperty("store_address")
+ @NotBlank(message = "Store address is required")
+ @Size(max = 200, message = "Store address must be less than 200 characters")
String storeAddress,
@JsonProperty("payment_date")
+ @NotNull(message = "Payment date is required")
LocalDateTime paymentDate,
@JsonProperty("payment_amount")
+ @NotNull(message = "Payment amount is required")
+ @Positive(message = "Payment amount must be positive")
BigDecimal paymentAmount
)
Add these imports:
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Size;
import jakarta.validation.constraints.Positive;
@JsonProperty("store_name") | ||
String storeName, | ||
|
||
@JsonProperty("store_address") | ||
String storeAddress, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Input validation and sanitization needed for store fields
The codebase has a validation framework in place (SelfValidating
base class and custom validators), but CreateReceiptRequestDto
lacks proper validation for storeName
and storeAddress
fields. Consider:
- The DTO should extend
SelfValidating<CreateReceiptRequestDto>
- Add
@Pattern
or custom validation annotations to prevent XSS and injection attacks - Implement length constraints to prevent unreasonably long inputs
Example locations needing attention:
src/main/java/com/daon/onjung/onjung/application/dto/request/CreateReceiptRequestDto.java
🔗 Analysis chain
Consider adding store data sanitization
The store name and address fields might need sanitization to prevent XSS or other injection attacks.
Let's check if there's any input sanitization in place:
Let's gather more information about the security configuration and any existing input validation/sanitization mechanisms.
Let's check the validation mechanisms and the SelfValidating class that was found in the search results.
Let's check for any string validation mechanisms and if CreateReceiptRequestDto uses any validation annotations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any existing input sanitization utilities or security configurations
rg -l "XssFilter|HtmlSanitizer|SecurityConfig"
# Check if there are any other DTOs with similar fields to see how they handle sanitization
ast-grep --pattern 'record $_(
$$$
@JsonProperty("store_name")
$$$
)'
Length of output: 192
Script:
#!/bin/bash
# Check the SecurityConfig implementation
cat src/main/java/com/daon/onjung/security/config/SecurityConfig.java
# Look for any input validation or sanitization utilities
rg -g '*.java' -l "validate|sanitize|clean|escape"
# Check for any existing validators or filters
fd -e java validator filter sanitizer
Length of output: 6740
Script:
#!/bin/bash
# Check the SelfValidating class implementation
cat src/main/java/com/daon/onjung/core/dto/SelfValidating.java
# Check if CreateReceiptRequestDto extends SelfValidating
ast-grep --pattern 'record CreateReceiptRequestDto$_($$$) extends SelfValidating'
# Check custom validation annotations
cat src/main/java/com/daon/onjung/core/annotation/validation/TimeValue.java
cat src/main/java/com/daon/onjung/core/annotation/validation/EnumValue.java
cat src/main/java/com/daon/onjung/core/annotation/validation/DateValue.java
Length of output: 3285
Script:
#!/bin/bash
# Check for string validators
fd -e java -x cat {} \; -p validator
# Check the complete CreateReceiptRequestDto implementation
rg -g "CreateReceiptRequestDto.java" -A 20 "record CreateReceiptRequestDto"
# Look for any string sanitization or validation patterns in the codebase
ast-grep --pattern '@Pattern($$$)
String $_;'
Length of output: 5655
public Receipt createReceipt( | ||
LocalDate paymentDate, | ||
Integer paymentAmount, | ||
User user, | ||
Store store | ||
) { | ||
return Receipt.builder() | ||
.paymentAmount(paymentAmount) | ||
.paymentDate(paymentDate) | ||
.user(user) | ||
.store(store) | ||
.build(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add input validation and documentation to ensure data integrity.
The method implementation is clean and uses the builder pattern correctly. However, there are several important improvements needed:
- Add parameter validation to ensure data integrity
- Add method documentation
- Consider adding transaction management
Consider applying these improvements:
+ /**
+ * Creates a new Receipt entity with the provided payment details.
+ *
+ * @param paymentDate The date when the payment was made
+ * @param paymentAmount The amount paid (must be positive)
+ * @param user The user who made the payment
+ * @param store The store where the payment was made
+ * @return A new Receipt entity
+ * @throws IllegalArgumentException if paymentAmount is negative or any parameter is null
+ */
+ @Transactional
public Receipt createReceipt(
LocalDate paymentDate,
Integer paymentAmount,
User user,
Store store
) {
+ if (paymentDate == null || paymentAmount == null || user == null || store == null) {
+ throw new IllegalArgumentException("All parameters are required");
+ }
+ if (paymentAmount <= 0) {
+ throw new IllegalArgumentException("Payment amount must be positive");
+ }
return Receipt.builder()
.paymentAmount(paymentAmount)
.paymentDate(paymentDate)
.user(user)
.store(store)
.build();
}
📝 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.
public Receipt createReceipt( | |
LocalDate paymentDate, | |
Integer paymentAmount, | |
User user, | |
Store store | |
) { | |
return Receipt.builder() | |
.paymentAmount(paymentAmount) | |
.paymentDate(paymentDate) | |
.user(user) | |
.store(store) | |
.build(); | |
} | |
/** | |
* Creates a new Receipt entity with the provided payment details. | |
* | |
* @param paymentDate The date when the payment was made | |
* @param paymentAmount The amount paid (must be positive) | |
* @param user The user who made the payment | |
* @param store The store where the payment was made | |
* @return A new Receipt entity | |
* @throws IllegalArgumentException if paymentAmount is negative or any parameter is null | |
*/ | |
@Transactional | |
public Receipt createReceipt( | |
LocalDate paymentDate, | |
Integer paymentAmount, | |
User user, | |
Store store | |
) { | |
if (paymentDate == null || paymentAmount == null || user == null || store == null) { | |
throw new IllegalArgumentException("All parameters are required"); | |
} | |
if (paymentAmount <= 0) { | |
throw new IllegalArgumentException("Payment amount must be positive"); | |
} | |
return Receipt.builder() | |
.paymentAmount(paymentAmount) | |
.paymentDate(paymentDate) | |
.user(user) | |
.store(store) | |
.build(); | |
} |
@PostMapping("/api/v1/receipts") | ||
public ResponseDto<Void> reviewReceipt( | ||
@AccountID UUID accountId, | ||
@RequestBody @Valid CreateReceiptRequestDto requestDto |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Add validation constraints to CreateReceiptRequestDto fields
The DTO lacks necessary validation constraints. While @JsonProperty
annotations are present for JSON mapping, there are no validation annotations to ensure data integrity. Consider adding:
@NotBlank
forstoreName
andstoreAddress
@Pattern
or custom validation forpaymentDate
to ensure date format@Pattern
or numeric validation forpaymentAmount
🔗 Analysis chain
Verify request DTO validation.
The @Valid
annotation is present, but we should verify that the CreateReceiptRequestDto
has appropriate validation constraints.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for CreateReceiptRequestDto definition and its validation annotations
ast-grep --pattern 'class CreateReceiptRequestDto {
$$$
}'
Length of output: 62
Script:
#!/bin/bash
# Try to find the CreateReceiptRequestDto file first
fd CreateReceiptRequestDto
# Then search for validation annotations in files containing CreateReceiptRequestDto
rg -l "CreateReceiptRequestDto" | xargs rg -A 10 "class CreateReceiptRequestDto"
Length of output: 199
Script:
#!/bin/bash
# Now that we found the file, let's examine its contents
cat src/main/java/com/daon/onjung/onjung/application/dto/request/CreateReceiptRequestDto.java
Length of output: 520
public void execute(UUID accountId, CreateReceiptRequestDto requestDto) { | ||
|
||
// 유저 조회 | ||
User user = userRepository.findById(accountId) | ||
.orElseThrow(() -> new CommonException(ErrorCode.NOT_FOUND_RESOURCE)); | ||
|
||
// 매장 조회 | ||
Store store = storeRepository.findByOcrStoreNameAndOcrStoreAddress(requestDto.storeName(), requestDto.storeAddress()) | ||
.orElseThrow(() -> new CommonException(ErrorCode.NOT_FOUND_MATCHED_STORE)); | ||
|
||
// paymentDate 형변환 | ||
LocalDate paymentDate = DateTimeUtil.convertStringToLocalDate(requestDto.paymentDate()); | ||
|
||
// paymentAmount 쉼표 제거 및 형변환 | ||
Integer paymentAmount = Integer.parseInt(requestDto.paymentAmount().replaceAll(",", "")); | ||
|
||
// 영수증 생성 | ||
Receipt receipt = receiptRepository.save(receiptService.createReceipt( | ||
paymentDate, | ||
paymentAmount, | ||
user, | ||
store | ||
)); | ||
receiptRepository.save(receipt); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add input validation for request DTO.
Consider adding validation annotations to the CreateReceiptRequestDto
and enabling validation in this service to ensure all required fields are present and properly formatted before processing.
Example validation for the DTO:
public record CreateReceiptRequestDto(
@NotBlank(message = "Store name is required")
String storeName,
@NotBlank(message = "Store address is required")
String storeAddress,
@Pattern(regexp = "\\d{4}-\\d{2}-\\d{2}", message = "Payment date must be in format YYYY-MM-DD")
String paymentDate,
@Pattern(regexp = "^[0-9,]+$", message = "Payment amount must contain only digits and commas")
String paymentAmount
) {}
Then add validation to the service:
@Override
@Transactional
- public void execute(UUID accountId, CreateReceiptRequestDto requestDto) {
+ public void execute(UUID accountId, @Valid CreateReceiptRequestDto requestDto) {
user, | ||
store | ||
)); | ||
receiptRepository.save(receipt); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant save operation.
The receipt is already saved on line 51. The second save operation on line 57 is redundant and should be removed.
Receipt receipt = receiptRepository.save(receiptService.createReceipt(
paymentDate,
paymentAmount,
user,
store
));
- receiptRepository.save(receipt);
Committable suggestion skipped: line range outside the PR's diff.
LocalDate paymentDate = DateTimeUtil.convertStringToLocalDate(requestDto.paymentDate()); | ||
|
||
// paymentAmount 쉼표 제거 및 형변환 | ||
Integer paymentAmount = Integer.parseInt(requestDto.paymentAmount().replaceAll(",", "")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add proper error handling for payment amount parsing.
The current implementation directly parses the string without proper error handling. Consider adding validation and error handling for the payment amount.
- Integer paymentAmount = Integer.parseInt(requestDto.paymentAmount().replaceAll(",", ""));
+ Integer paymentAmount;
+ try {
+ String sanitizedAmount = requestDto.paymentAmount().replaceAll(",", "").trim();
+ paymentAmount = Integer.parseInt(sanitizedAmount);
+ if (paymentAmount <= 0) {
+ throw new CommonException(ErrorCode.INVALID_PAYMENT_AMOUNT);
+ }
+ } catch (NumberFormatException e) {
+ throw new CommonException(ErrorCode.INVALID_PAYMENT_AMOUNT);
+ }
📝 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.
Integer paymentAmount = Integer.parseInt(requestDto.paymentAmount().replaceAll(",", "")); | |
Integer paymentAmount; | |
try { | |
String sanitizedAmount = requestDto.paymentAmount().replaceAll(",", "").trim(); | |
paymentAmount = Integer.parseInt(sanitizedAmount); | |
if (paymentAmount <= 0) { | |
throw new CommonException(ErrorCode.INVALID_PAYMENT_AMOUNT); | |
} | |
} catch (NumberFormatException e) { | |
throw new CommonException(ErrorCode.INVALID_PAYMENT_AMOUNT); | |
} |
// paymentDate 형변환 | ||
LocalDate paymentDate = DateTimeUtil.convertStringToLocalDate(requestDto.paymentDate()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add date validation.
The payment date should be validated to ensure it falls within a reasonable range (e.g., not in the future and not too far in the past).
- LocalDate paymentDate = DateTimeUtil.convertStringToLocalDate(requestDto.paymentDate());
+ LocalDate paymentDate = DateTimeUtil.convertStringToLocalDate(requestDto.paymentDate());
+ LocalDate now = LocalDate.now();
+ if (paymentDate.isAfter(now)) {
+ throw new CommonException(ErrorCode.INVALID_PAYMENT_DATE);
+ }
+ if (paymentDate.isBefore(now.minusYears(1))) {
+ throw new CommonException(ErrorCode.PAYMENT_DATE_TOO_OLD);
+ }
📝 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.
// paymentDate 형변환 | |
LocalDate paymentDate = DateTimeUtil.convertStringToLocalDate(requestDto.paymentDate()); | |
// paymentDate 형변환 | |
LocalDate paymentDate = DateTimeUtil.convertStringToLocalDate(requestDto.paymentDate()); | |
LocalDate now = LocalDate.now(); | |
if (paymentDate.isAfter(now)) { | |
throw new CommonException(ErrorCode.INVALID_PAYMENT_DATE); | |
} | |
if (paymentDate.isBefore(now.minusYears(1))) { | |
throw new CommonException(ErrorCode.PAYMENT_DATE_TOO_OLD); | |
} |
Related issue 🛠
closed #28
어떤 변경사항이 있었나요?
CheckPoint ✅
PR이 다음 요구 사항을 충족하는지 확인하세요.
Work Description ✏️
Uncompleted Tasks 😅
N/A
To Reviewers 📢
Summary by CodeRabbit