prod : 알림톡 factory, 내에 알림톡 변수를 만들 때, 가상계좌에 대한 부분으로 join 하도록 변경#325
prod : 알림톡 factory, 내에 알림톡 변수를 만들 때, 가상계좌에 대한 부분으로 join 하도록 변경#325polyglot-k merged 2 commits intoprodfrom
Conversation
…licationJpaRepository
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Summary of Changes
Hello @polyglot-k, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
이 PR은 알림톡 변수 생성 로직을 개선하여, 시험 신청에 대한 알림톡 변수를 생성할 때 일반 결제 정보 대신 가상 계좌 정보를 사용하도록 변경합니다. 이는 가상 계좌 로그와 조인하고 입금 상태가 'DONE'인 경우만 고려함으로써 이루어집니다.
Highlights
- NotifyVariableFactory 변경: 알림톡 변수 생성을 위해 시험 신청 데이터를 조회하는 메서드가 findExamAndPaymentByExamApplicationId에서 findExamAndVirtualAccountByExamApplicationId로 변경되었습니다. 이는 일반 결제 데이터에서 특정 가상 계좌 데이터로의 전환을 의미합니다.
- 새로운 JPA Repository 쿼리 추가: ExamApplicationJpaRepository에 findExamAndVirtualAccountByExamApplicationId라는 새로운 JPA 쿼리가 추가되었습니다. 이 쿼리는 알림톡 알림을 위한 시험 및 가상 계좌 세부 정보를 특별히 가져오며, 완료된 가상 계좌 입금만 고려하도록 보장합니다.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
안녕하세요. 코드 변경 사항을 검토했습니다. 알림톡 변수 생성 시 가상계좌 정보를 사용하도록 쿼리를 수정한 점이 인상 깊습니다.
다만, 새로 추가된 JPQL 쿼리가 여러 개의 결과를 반환할 가능성이 있어 IncorrectResultSizeDataAccessException이 발생할 수 있는 점을 발견했습니다. 이로 인해 서비스 장애가 발생할 수 있으므로 수정이 필요해 보입니다.
관련하여 두 파일에 수정 제안을 포함한 리뷰 코멘트를 남겼습니다. 확인 부탁드립니다.
| Optional<ExamApplicationNotifyProjection> findExamAndVirtualAccountByExamApplicationId( | ||
| @Param("examApplicationId") Long examApplicationId); |
There was a problem hiding this comment.
현재 쿼리는 applicationId 당 여러 개의 VirtualAccountLogJpaEntity가 DONE 상태로 존재할 경우 IncorrectResultSizeDataAccessException을 발생시킬 수 있습니다. Optional을 반환 타입으로 사용하는 경우, 쿼리 결과가 2개 이상이면 예외가 발생합니다.
데이터 무결성이 깨지는 예외적인 상황에 대한 방어 코드를 추가하는 것이 좋습니다. 반환 타입을 List<ExamApplicationNotifyProjection>으로 변경하여 예상치 못한 다중 결과에 대비하는 것을 제안합니다.
| Optional<ExamApplicationNotifyProjection> findExamAndVirtualAccountByExamApplicationId( | |
| @Param("examApplicationId") Long examApplicationId); | |
| List<ExamApplicationNotifyProjection> findExamAndVirtualAccountByExamApplicationId( | |
| @Param("examApplicationId") Long examApplicationId); |
| ExamApplicationNotifyProjection projection = examApplicationRepository.findExamAndVirtualAccountByExamApplicationId( | ||
| targetId) | ||
| .orElseThrow( | ||
| () -> new CustomRuntimeException(ErrorCode.EXAM_APPLICATION_NOT_FOUND)); |
There was a problem hiding this comment.
리포지토리 메서드의 반환 타입이 List로 변경됨에 따라, 여기서도 결과를 리스트로 받고 첫 번째 요소를 사용하도록 수정해야 합니다. 리스트가 비어있는 경우 기존과 동일하게 예외를 발생시킵니다.
이렇게 하면 IncorrectResultSizeDataAccessException 발생을 방지하고, 비정상적인 데이터가 존재하더라도 첫 번째 유효한 데이터를 기준으로 알림을 보낼 수 있어 시스템 안정성을 높일 수 있습니다.
List<ExamApplicationNotifyProjection> projections = examApplicationRepository.findExamAndVirtualAccountByExamApplicationId(
targetId);
if (projections.isEmpty()) {
throw new CustomRuntimeException(ErrorCode.EXAM_APPLICATION_NOT_FOUND);
}
ExamApplicationNotifyProjection projection = projections.get(0);
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타