-
Notifications
You must be signed in to change notification settings - Fork 1
prod : 가상 계좌 부분 삭제 및 휴대폰 번호 UID 변경 #366
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
Conversation
MOSU-358 refactor: 아이디 찾기 휴대폰 번호 비교 시 U Prefix 추가
…nd improve method formatting
MOSU-360 refactor: 문의 답변 시 "관리자" 고정적으로 명시
…ionValidator and related files
Feat/test: application 및 infra 단위 테스트 진행
feat: add application status update on deposit success event handling
|
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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!
This pull request significantly refactors and enhances the codebase by introducing CI/CD workflow improvements, critical bug fixes, and extensive code cleanup. It also corrects naming inconsistencies and adds a comprehensive suite of new unit tests, improving the overall maintainability and reliability of the application.
Highlights
- CI/CD Workflow Refinement: The GitHub Actions build process has been streamlined by renaming the job, separating build and test steps, and enabling artifact uploads for better build management.
- User Service Enhancements: Logic has been implemented in
MyUserServiceto prevent Kakao-registered users from attempting to retrieve a login ID, accompanied by a new error code for such conflicts. - Payment Status Automation: The
DepositSuccessEventHandlernow automatically updates the application status toAPPROVEDupon successful deposit, ensuring data consistency. - Code Quality Improvements: Significant refactoring in
ApplicationContextenhances readability and maintainability, including the introduction of a static factory method. - Typo Correction & Cleanup: A persistent typo in the
vaildatorpackage and file name has been corrected tovalidator, and numerous commented-out or unused code sections have been removed across the repository. - Comprehensive Test Coverage: A large number of new unit tests have been added for various services, processors, and infrastructure components, drastically improving the test coverage and stability of the application.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
전반적으로 코드 품질을 크게 향상시키는 훌륭한 PR입니다. 특히 테스트 커버리지를 대폭 확장하고, Flyway를 도입하여 데이터베이스 스키마 관리를 체계화하며, 환경별 설정을 분리한 점이 인상적입니다. 코드 정리와 네이밍 수정 등 세세한 부분까지 신경 쓴 점도 좋습니다. 몇 가지 추가 개선점을 제안드립니다.
| var application = applicationJpaRepository.findById(log.getApplicationId()) | ||
| .orElseThrow( | ||
| () -> new CustomRuntimeException(ErrorCode.APPLICATION_LIST_NOT_FOUND)); | ||
| if (application.getStatus() == ApplicationStatus.PENDING) { | ||
| application.changeStatus(ApplicationStatus.APPROVED); | ||
| } |
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.
가상계좌 입금 성공 시 신청(Application)의 상태를 PENDING에서 APPROVED로 변경하는 로직이 추가되어 데이터 정합성을 높이는 좋은 개선입니다.
다만, 이 로직이 추가되면서 바로 뒤에 이어지는 알림 생성 로직(LunaNotificationEvent.create(...))에서 잠재적인 문제가 발생할 수 있어 보입니다.
- 알림 유형 고정: 현재
LunaNotificationStatus.APPLICATION_GUEST_SUCCESS로 알림 유형이 고정되어 있습니다. 만약 로그인한 일반 사용자가 가상계좌로 결제하는 경우에도 게스트용 알림이 발송될 수 있어 사용자에게 혼란을 줄 수 있습니다. - 단일 시험 정보만 사용: 하나의 신청(application)에 여러 시험(exam)이 포함될 수 있지만,
findFirst()를 사용하여 첫 번째 시험 정보(exam.getId())만 알림에 사용하고 있습니다. 이로 인해 사용자는 전체 신청 건 중 일부에 대한 정보만 받게 될 수 있습니다.
추가하신 로직과 함께 주변 코드의 상호작용을 고려하여, 사용자의 유형(게스트/일반 회원)을 구분하고 신청에 포함된 모든 시험 정보를 올바르게 처리하도록 알림 로직을 개선하는 것을 검토해 보시는 것이 좋겠습니다.
| .content(content) | ||
| .userId(user.getId()) | ||
| .author(user.getName()) | ||
| .author("관리자") |
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.
"관리자"라는 문자열이 하드코딩되어 있습니다. 유지보수성을 높이기 위해, 이와 같은 상수는 별도의 상수 클래스나 final static 필드로 정의하여 사용하는 것이 좋습니다. 이렇게 하면 나중에 작성자명을 변경해야 할 때 한 곳만 수정하면 되므로 실수를 줄일 수 있습니다.
예시:
public final class AdminConstants {
public static final String ADMIN_AUTHOR_NAME = "관리자";
private AdminConstants() {}
}사용:
.author(AdminConstants.ADMIN_AUTHOR_NAME)
This pull request introduces several improvements and fixes across the codebase, including workflow enhancements, bug fixes, code cleanup, and naming corrections. The most important changes are grouped below by theme.
CI/CD Workflow Improvements:
build-and-deploytobuild-and-test, separated test execution from the build step, and added artifact upload for build outputs in.github/workflows/docker-pr-build.yaml. [1] [2]Bug Fixes and Feature Enhancements:
MyUserService, added logic to prevent users registered via Kakao from retrieving a login ID, and introduced a new error codeKAKAO_ACCOUNT_DUPLICATEDinErrorCode. [1] [2]DepositSuccessEventHandler, now updates the application status fromPENDINGtoAPPROVEDwhen a deposit succeeds. This ensures application state consistency after payment. [1] [2] [3]Code Cleanup and Refactoring:
ApplicationContextby refactoring method signatures and return statements, and added a static factory methodoffor easier instantiation. [1] [2] [3] [4]Naming and Structure Corrections:
vaildatortovalidatorforApplicationValidator, and updated all references accordingly. [1] [2]Codebase Maintenance:
EventAttachmentService.java,FaqServiceTest.java, andMosuServerApplicationTests.javato keep the repository clean. [1] [2] [3]