Skip to content
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

Merged
merged 1 commit into from
Nov 19, 2024

Conversation

dongkyeomjang
Copy link
Collaborator

@dongkyeomjang dongkyeomjang commented Nov 19, 2024

Related issue 🛠

closed #28

어떤 변경사항이 있었나요?

  • 🐞 BugFix Something isn't working
  • 💻 CrossBrowsing Browser compatibility
  • 🌏 Deploy Deploy
  • 🎨 Design Markup & styling
  • 📃 Docs Documentation writing and editing (README.md, etc.)
  • ✨ Feature Feature
  • 🔨 Refactor Code refactoring
  • ⚙️ Setting Development environment setup
  • ✅ Test Test related (storybook, jest, etc.)

CheckPoint ✅

PR이 다음 요구 사항을 충족하는지 확인하세요.

  • PR 컨벤션에 맞게 작성했습니다. (필수)
  • Docker Container 환경으로 변경 사항에 대한 테스트를 했습니다.(버그 수정/기능에 대한 테스트). (필수)
  • Feature의 경우, API 문서 업데이트를 완료했습니다. (선택)
  • BugFix의 경우, 버그의 원인을 파악하였습니다. (선택)

Work Description ✏️

  • 4.5 영수증 리뷰 등록하기

Uncompleted Tasks 😅

N/A

To Reviewers 📢

Summary by CodeRabbit

  • New Features
    • Introduced a new endpoint for registering receipt reviews via a POST request.
    • Added functionality to retrieve store information based on OCR data.
    • Enhanced error handling with a specific error code for unmatched stores.
  • Bug Fixes
    • Updated documentation for existing receipt OCR functionality to reflect recent changes.
  • Chores
    • Added a data transfer object for creating receipt requests, facilitating API interactions.
    • Implemented a service class for managing receipt creation processes.

@dongkyeomjang dongkyeomjang added ✨ Feature 기능 개발 dongkyeomjang assigned to dongkyeomjang labels Nov 19, 2024
@dongkyeomjang dongkyeomjang self-assigned this Nov 19, 2024
@dongkyeomjang dongkyeomjang linked an issue Nov 19, 2024 that may be closed by this pull request
1 task
Copy link

coderabbitai bot commented Nov 19, 2024

Walkthrough

This 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 StoreRepository for retrieving stores based on OCR data, introduces a new error code for unmatched stores, and establishes a service and DTO for handling receipt creation. The changes enhance the functionality of the application by allowing users to submit receipt reviews and improving error handling related to store lookups.

Changes

File Change Summary
http/onjung/OnjungControllerHttpRequest.http Added new endpoint POST /api/v1/receipts for receipt reviews with JSON payload.
src/main/java/com/daon/onjung/account/repository/mysql/StoreRepository.java Added method findByOcrStoreNameAndOcrStoreAddress(String ocrStoreName, String ocrStoreAddress).
src/main/java/com/daon/onjung/core/exception/error/ErrorCode.java Added error code NOT_FOUND_MATCHED_STORE with code 40410 for unmatched stores.
src/main/java/com/daon/onjung/onjung/application/controller/command/OnjungCommandV1Controller.java Added method reviewReceipt for handling receipt review submissions and updated OCR method documentation.
src/main/java/com/daon/onjung/onjung/application/dto/request/CreateReceiptRequestDto.java Introduced CreateReceiptRequestDto class for receipt review requests.
src/main/java/com/daon/onjung/onjung/application/service/CreateReceiptService.java Added CreateReceiptService class with method execute for processing receipt creation.
src/main/java/com/daon/onjung/onjung/application/usecase/CreateReceiptUseCase.java Introduced CreateReceiptUseCase interface defining method execute for receipt creation.
src/main/java/com/daon/onjung/onjung/domain/service/ReceiptService.java Added method createReceipt for constructing and returning a Receipt object.

Assessment against linked issues

Objective Addressed Explanation
Implement receipt review registration API (#28)

Possibly related issues

Possibly related PRs

Suggested reviewers

  • bianbbc87

🐰 In the fields where receipts lay,
A new endpoint joins the fray.
With reviews to share, and stores to find,
Our API grows, oh so refined!
Let's hop along and celebrate,
For receipts now hold a brighter fate! 🌟


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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:

  1. Implementing fuzzy matching using techniques like Levenshtein distance
  2. Normalizing store names and addresses before comparison
  3. 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 documentation

Missing 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:

  1. Consider returning the created receipt's ID or details instead of void
  2. 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

📥 Commits

Reviewing files that changed from the base of the PR and between 6ebd1bd and 785c325.

📒 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

Comment on lines +8 to +11
@UseCase
public interface CreateReceiptUseCase {
void execute(UUID accountId, CreateReceiptRequestDto requestDto);
}
Copy link

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:

  1. A ResponseDto<T> wrapper class for all responses
  2. A global exception handler (HttpGlobalExceptionHandler) that handles various exceptions
  3. Custom exceptions like CommonException and HttpSecurityException

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 and ReadOnjungBriefUseCase
🔗 Analysis chain

Consider enhancing error handling and validation.

The interface design is clean and follows the Single Responsibility Principle. However, consider the following suggestions:

  1. The void return type might make it difficult to handle errors effectively. Consider returning a result object that can encapsulate success/failure states.
  2. 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,
Copy link

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

Comment on lines +5 to +17
public record CreateReceiptRequestDto(
@JsonProperty("store_name")
String storeName,

@JsonProperty("store_address")
String storeAddress,

@JsonProperty("payment_date")
String paymentDate,

@JsonProperty("payment_amount")
String paymentAmount
) {
Copy link

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;

Comment on lines +6 to +10
@JsonProperty("store_name")
String storeName,

@JsonProperty("store_address")
String storeAddress,
Copy link

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

Comment on lines +13 to +25
public Receipt createReceipt(
LocalDate paymentDate,
Integer paymentAmount,
User user,
Store store
) {
return Receipt.builder()
.paymentAmount(paymentAmount)
.paymentDate(paymentDate)
.user(user)
.store(store)
.build();
}
Copy link

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:

  1. Add parameter validation to ensure data integrity
  2. Add method documentation
  3. 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.

Suggested change
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
Copy link

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 for storeName and storeAddress
  • @Pattern or custom validation for paymentDate to ensure date format
  • @Pattern or numeric validation for paymentAmount
🔗 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

Comment on lines +34 to +58
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);

Copy link

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);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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(",", ""));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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);
}

Comment on lines +44 to +45
// paymentDate 형변환
LocalDate paymentDate = DateTimeUtil.convertStringToLocalDate(requestDto.paymentDate());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
// 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);
}

@dongkyeomjang dongkyeomjang merged commit 8b12877 into dev Nov 19, 2024
3 checks passed
@dongkyeomjang dongkyeomjang deleted the Feature/#28 branch November 19, 2024 07:51
dongkyeomjang added a commit that referenced this pull request Nov 22, 2024
* ✨ Feature/#27 - 4.4 가게 방문 인증용 영수증 OCR 조회하기 API 구현

* ✨ Feature/#28 - 4.5 영수증 리뷰 등록하기 API 구현
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dongkyeomjang assigned to dongkyeomjang ✨ Feature 기능 개발
Projects
None yet
Development

Successfully merging this pull request may close these issues.

✨ Feature - 4.5 영수증 리뷰 등록하기 API 구현
1 participant