Conversation
WalkthroughThis set of changes introduces and extends notification functionality across several service layers, particularly for inquiry answers, payments, refunds, and profile registration. It adds new notification event publishing logic, updates repository query methods, modifies DTOs and projections to include additional fields, and adjusts method signatures to propagate user and context information. Some resource strings and database initialization logic are also updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Controller
participant Service
participant EventTxService
participant EventListener
participant NotifyEventPublisher
User->>Controller: Trigger (e.g., payment, refund, inquiry answer)
Controller->>Service: Call service method (with userId)
Service->>EventTxService: publishSuccessEvent/publishFailureEvent (userId, ...)
EventTxService->>EventListener: Publish event (after commit)
EventListener->>NotifyEventPublisher: sendNotification (NotificationEvent)
NotifyEventPublisher-->>User: Notification sent
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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. 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 (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @chominju02, 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 integrates Alimtalk notifications across various user lifecycle events, including payment completion, refund completion, user sign-up, and inquiry answer updates. This enhances user communication by providing timely and relevant notifications directly through the Alimtalk service. The changes involve propagating user and application context through transactional events and refining data retrieval for notification content.
Highlights
- Alimtalk Integration for Key Events: Implemented automated Alimtalk notifications for payment completion, refund completion, new user sign-ups, and inquiry answer updates, enhancing real-time user communication.
- Enhanced Notification Data Handling: Updated data structures and retrieval logic to include comprehensive details, such as refund amounts and specific exam application information, ensuring richer notification content.
- Improved Transactional Event Propagation: Modified payment and refund transaction services to correctly propagate user and application IDs, enabling precise targeting of notifications post-transaction.
- Refinement of Inquiry Answer Logic: Corrected the logic for checking existing inquiry answers to use
inquiryIdinstead of the primary key, preventing potential data integrity issues.
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 or fill out our survey to provide feedback.
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
This pull request refactors the application to integrate Alimtalk notifications for key events like payment, refund, sign-up, and inquiry answers. The changes are mostly centered around publishing and handling notification events. There are a few issues related to userId being passed around and a factory method that is error prone. Please address these points to ensure the stability and maintainability of the new notification feature.
| private void sendNotification() { | ||
| NotificationEvent event = NotificationEvent.create(NotificationStatus.SIGN_UP_SUCCESS); | ||
| notifier.notify(event); | ||
| } |
There was a problem hiding this comment.
The sendNotification() method creates a NotificationEvent with a null userId because it uses NotificationEvent.create(NotificationStatus.SIGN_UP_SUCCESS). This will cause a NullPointerException in NotifyService, which needs the userId to fetch the user's phone number.
To fix this, the sendNotification method should accept the userId and pass it when creating the event.
| private void sendNotification() { | |
| NotificationEvent event = NotificationEvent.create(NotificationStatus.SIGN_UP_SUCCESS); | |
| notifier.notify(event); | |
| } | |
| private void sendNotification(Long userId) { | |
| NotificationEvent event = NotificationEvent.create(NotificationStatus.SIGN_UP_SUCCESS, userId, null); | |
| notifier.notify(event); | |
| } |
| user.grantUserRole(); | ||
| syncUserInfoFromProfile(user, request); | ||
|
|
||
| sendNotification(); |
There was a problem hiding this comment.
The sendNotification() method should accept the userId and pass it when creating the event. You will also need to update the call to this method on line 42 to pass the userId:
sendNotification(userId);
| sendNotification(); | |
| sendNotification(userId); |
| public static NotificationEvent create(NotificationStatus status) { | ||
| if (!NotificationStatus.SIGN_UP_SUCCESS.equals(status)) { | ||
| throw new IllegalArgumentException("Unknown notification status: " + status); | ||
| } | ||
| return new NotificationEvent(status, null, null); | ||
| } |
There was a problem hiding this comment.
The new factory method create(NotificationStatus status) is error-prone because it creates an event with a null userId. This is an implicit contract that is not enforced by the type system and can easily lead to NullPointerExceptions in services that consume these events, as seen with ProfileService and NotifyService.
It's safer to remove this method and exclusively use the create(NotificationStatus, Long, Long) factory method, always providing a userId explicitly. For cases like sign-up where there's no specific target entity, null can be passed for targetId.
| targetId) | ||
| .orElseThrow( | ||
| () -> new CustomRuntimeException(ErrorCode.APPLICATION_SCHOOL_NOT_FOUND)); |
There was a problem hiding this comment.
The error code ErrorCode.APPLICATION_SCHOOL_NOT_FOUND seems incorrect here. The method is trying to find a refund projection by examApplicationId. A more appropriate error code would be ErrorCode.EXAM_APPLICATION_NOT_FOUND (which is used in createApplicationVariable in the same file) or a new specific error code for "refund data not found". Using a misleading error code can make debugging more difficult.
() -> new CustomRuntimeException(ErrorCode.EXAM_APPLICATION_NOT_FOUND));![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaRepository.java (1)
3-3: Critical: Wrong @param import used.The import is using Lettuce Core's
@Paramannotation instead of Spring Data JPA's annotation. This will cause runtime failures when executing the query.Apply this diff to fix the import:
-import io.lettuce.core.dynamic.annotation.Param; +import org.springframework.data.repository.query.Param;
🧹 Nitpick comments (2)
src/main/java/life/mosu/mosuserver/global/initializer/DatabaseInitializer.java (1)
139-169: Consider parameterizing fixed values for better maintainability.The
createAdditionalUsers()method correctly follows the existing pattern but uses several hardcoded values. Consider extracting constants for repeated values like phone number, school info, and agreement flags to improve maintainability.+ private static final String DEFAULT_PHONE = "010-9161-2960"; + private static final String DEFAULT_SCHOOL_NAME = "모수고등학교"; + private static final String DEFAULT_SCHOOL_ADDRESS = "서울시 모수구 123"; + private static final String DEFAULT_ZIPCODE = "12345"; + private void createAdditionalUsers() { for (int i = 11; i <= 12; i++) { UserJpaEntity user = UserJpaEntity.builder() .loginId("userid" + i) .password(passwordEncoder.encode("Password!" + i)) .gender(i % 2 == 0 ? Gender.MALE : Gender.FEMALE) .name("모수학생" + i) - .phoneNumber("010-9161-2960") + .phoneNumber(DEFAULT_PHONE) // ... other fieldssrc/main/java/life/mosu/mosuserver/presentation/refund/RefundController.java (1)
28-30: Simplify the authorization expression.The
isAuthenticated()check is redundant when checking for a role, ashasRole('USER')already implies authentication.- @PreAuthorize("isAuthenticated() and hasRole('USER')") + @PreAuthorize("hasRole('USER')")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
src/main/java/life/mosu/mosuserver/application/inquiry/InquiryAnswerService.java(2 hunks)src/main/java/life/mosu/mosuserver/application/notify/NotifyVariableFactory.java(2 hunks)src/main/java/life/mosu/mosuserver/application/notify/dto/RefundNotifyRequest.java(2 hunks)src/main/java/life/mosu/mosuserver/application/payment/PaymentEventTxService.java(1 hunks)src/main/java/life/mosu/mosuserver/application/payment/PaymentService.java(3 hunks)src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentContext.java(3 hunks)src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxEventListener.java(3 hunks)src/main/java/life/mosu/mosuserver/application/profile/ProfileService.java(4 hunks)src/main/java/life/mosu/mosuserver/application/refund/RefundEventTxService.java(1 hunks)src/main/java/life/mosu/mosuserver/application/refund/RefundService.java(3 hunks)src/main/java/life/mosu/mosuserver/application/refund/tx/RefundContext.java(1 hunks)src/main/java/life/mosu/mosuserver/application/refund/tx/RefundTxEventListener.java(3 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/inquiryAnswer/InquiryAnswerJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/refund/RefundJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/refund/RefundNotifyProjection.java(1 hunks)src/main/java/life/mosu/mosuserver/global/initializer/DatabaseInitializer.java(2 hunks)src/main/java/life/mosu/mosuserver/infra/notify/dto/NotificationEvent.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/payment/PaymentWidgetController.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/refund/RefundController.java(2 hunks)src/main/resources/messages_ko.properties(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/life/mosu/mosuserver/presentation/refund/RefundController.java (2)
src/main/java/life/mosu/mosuserver/application/refund/RefundEventTxService.java (1)
Slf4j(13-44)src/main/java/life/mosu/mosuserver/application/refund/RefundService.java (1)
Slf4j(20-88)
src/main/java/life/mosu/mosuserver/application/inquiry/InquiryAnswerService.java (2)
src/main/java/life/mosu/mosuserver/application/inquiry/InquiryService.java (1)
Slf4j(24-93)src/main/java/life/mosu/mosuserver/application/inquiry/InquiryAnswerAttachmentService.java (1)
Service(15-77)
🔇 Additional comments (36)
src/main/java/life/mosu/mosuserver/domain/inquiryAnswer/InquiryAnswerJpaRepository.java (1)
10-10: LGTM! Method correctly implements inquiry existence check.The new
existsByInquiryIdmethod properly follows Spring Data JPA naming conventions and fixes the incorrect usage ofexistsById(postId)mentioned in the AI summary. This ensures accurate validation of whether an answer already exists for a specific inquiry.src/main/resources/messages_ko.properties (1)
8-8: Good formatting improvements for Korean notification templates.The formatting changes enhance the AlimTalk message templates:
- Removing spaces after commas in lines 8 and 17 follows proper Korean typography conventions
- Adding the bullet point (■) symbol in line 121 provides visual consistency with other notification formats
These changes improve the user experience for Korean notification messages.
Also applies to: 17-17, 121-121
src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaRepository.java (1)
91-92: LGTM! Parameter annotation correctly added.The explicit
@Param("targetId")annotation ensures proper parameter binding in the JPQL query. This change supports the notification integration by enabling accurate data retrieval for notification variables.src/main/java/life/mosu/mosuserver/domain/refund/RefundNotifyProjection.java (1)
10-10: LGTM! Refund amount field appropriately added.The new
refundAmountfield completes the refund notification data structure. UsingIntegertype is appropriate for monetary amounts in this context, and the field placement maintains logical ordering in the record.src/main/java/life/mosu/mosuserver/application/profile/ProfileService.java (2)
10-12: LGTM! Proper dependency injection for notification functionality.The notification dependencies are correctly injected following the established pattern used in other services within this PR.
Also applies to: 27-27
42-42: Ensure notification failures don’t rollback the main transactionNotifications are correctly published after profile registration, following the existing event‐publishing pattern. However, since
sendNotification()is invoked inside the registration transaction (see src/main/java/life/mosu/mosuserver/application/profile/ProfileService.java:42), any exception thrown there will roll back the entire transaction. The same pattern appears in:
- RefundTxEventListener (src/main/java/life/mosu/mosuserver/application/refund/tx/RefundTxEventListener.java)
- PaymentTxEventListener (src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxEventListener.java)
- InquiryAnswerService (src/main/java/life/mosu/mosuserver/application/inquiry/InquiryAnswerService.java)
If notifications are non-critical, consider:
- Executing them in a separate transaction or asynchronously
- Wrapping the call in error handling so notification failures don’t bubble up and rollback the core transaction
src/main/java/life/mosu/mosuserver/presentation/payment/PaymentWidgetController.java (1)
48-48: LGTM! Proper integration of userId for payment confirmation.The addition of
userIdparameter to thepaymentService.confirm()call correctly propagates user context through the payment flow, enabling user-specific notifications as intended by the 알림톡 integration.src/main/java/life/mosu/mosuserver/application/notify/dto/RefundNotifyRequest.java (1)
15-15: LGTM! Improved type safety and field naming.The change from
String amounttoInteger refundAmountprovides better type safety and more descriptive naming. The corresponding update intoMap()correctly handles the Integer-to-String conversion for serialization.Also applies to: 33-33
src/main/java/life/mosu/mosuserver/application/payment/PaymentService.java (3)
48-48: LGTM! Proper method signature update for user context.The addition of
userIdparameter enables user-specific event publishing and notifications as part of the 알림톡 integration.
70-75: LGTM! Consistent user context propagation in event publishing.Both success and failure event publishing methods correctly receive the
userIdparameter, ensuring consistent user context for notifications regardless of payment outcome.
58-58: Ignore userId parameter suggestionThe
verifyAmountmethod inPaymentAmountCalculatoris declared as:public void verifyAmount(int applicationCount, int lunchAmount, int requestedAmount) { … }It does not accept a
userId, nor is it passed in the call:amountCalculator.verifyAmount(applications.size(), lunchAmount, request.amount());No user context is involved or required. You can safely disregard this review comment.
Likely an incorrect or invalid review comment.
src/main/java/life/mosu/mosuserver/global/initializer/DatabaseInitializer.java (1)
109-109: LGTM! Phone number update for test data.The phone number change for test users is straightforward and maintains consistency.
src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxEventListener.java (3)
4-6: LGTM! Proper dependency injection for notification functionality.The addition of
NotifyEventPublisherdependency and related imports correctly enables the notification functionality for successful payment events.Also applies to: 22-22
33-38: LGTM! Well-structured after-commit event handler.The
afterCommitHandlermethod properly implements the 알림톡 notification functionality:
- Uses correct transaction phase (AFTER_COMMIT)
- Includes appropriate logging for debugging
- Efficiently iterates over exam school IDs
- Maintains clean separation from rollback handling
46-50: LGTM! Clean notification dispatch implementation.The
sendNotificationmethod follows good practices:
- Clear method documentation
- Appropriate use of
APPLICATION_SUCCESSstatus- Proper encapsulation of notification event creation
- Clean separation of concerns
src/main/java/life/mosu/mosuserver/application/refund/tx/RefundTxEventListener.java (1)
29-34: LGTM! Proper transaction phase handling for notifications.The use of
AFTER_COMMITphase ensures notifications are only sent after successful refund transactions, preventing notifications for failed operations.src/main/java/life/mosu/mosuserver/application/refund/RefundService.java (2)
32-32: LGTM! Proper user context integration.The addition of
userIdparameter enables proper user association for refund operations and downstream notifications.
46-47: LGTM! Consistent event publishing with enhanced context.The event publishing calls now properly include
userIdandexamApplicationId, enabling notifications to be sent to the correct user after successful refund processing.src/main/java/life/mosu/mosuserver/presentation/refund/RefundController.java (1)
30-34: LGTM! Proper user authentication integration.The addition of
@UserIdparameter and passing it to the service layer correctly integrates user context into the refund processing flow.src/main/java/life/mosu/mosuserver/application/inquiry/InquiryAnswerService.java (2)
36-36: LGTM! Proper user context extraction.Extracting the user ID from the inquiry entity ensures notifications are sent to the correct user who created the inquiry.
90-90: ExistsByInquiryId method confirmed
InquiryAnswerJpaRepositorydeclaresboolean existsByInquiryId(Long inquiryId);(domain/inquiryAnswer/InquiryAnswerJpaRepository.java:10)InquiryAnswerServiceuses it correctly at line 90 (application/inquiry/InquiryAnswerService.java:90)LGTM!
src/main/java/life/mosu/mosuserver/application/refund/tx/RefundContext.java (3)
8-9: LGTM! Proper context enhancement for notification support.The addition of
userIdandexamApplicationIdfields enables the refund event listeners to access the necessary context for sending notifications to the correct user.
16-19: LGTM! Consistent factory method updates.The
ofSuccessfactory method properly accepts and passes the new parameters to maintain consistency with the record structure.Also applies to: 23-24
31-34: LGTM! Consistent factory method updates.The
ofFailurefactory method properly accepts and passes the new parameters, maintaining consistency with the success case.Also applies to: 38-39
src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentContext.java (3)
6-13: LGTM! Clean addition of userId field to payment context.The addition of the
userIdfield to thePaymentContextrecord is well-positioned and enables user-specific notification functionality. The field placement maintains good readability of the record structure.
15-28: Factory method updated consistently for success context.The
ofSuccessfactory method correctly accepts and passes theuserIdparameter to the constructor, maintaining consistency with the record structure.
30-43: Factory method updated consistently for failure context.The
ofFailurefactory method correctly accepts and passes theuserIdparameter to the constructor, maintaining consistency with the record structure and theofSuccessmethod.src/main/java/life/mosu/mosuserver/domain/refund/RefundJpaRepository.java (1)
15-32: Well-structured query for refund notification data.The JPQL query correctly joins multiple entities (RefundJpaEntity, PaymentJpaEntity, ExamApplicationJpaEntity, ExamJpaEntity) to gather comprehensive refund notification information. The filtering by
examApplicationIdandrefundStatus = 'DONE'is appropriate for completed refunds.src/main/java/life/mosu/mosuserver/application/payment/PaymentEventTxService.java (2)
22-27: Consistent addition of userId parameter to success event publishing.The
publishSuccessEventmethod correctly accepts and propagates theuserIdparameter to thePaymentContext.ofSuccessfactory method, enabling user-specific notification functionality.
29-35: Consistent addition of userId parameter to failure event publishing.The
publishFailureEventmethod correctly accepts and propagates theuserIdparameter to thePaymentContext.ofFailurefactory method, maintaining consistency with the success event method.src/main/java/life/mosu/mosuserver/application/refund/RefundEventTxService.java (2)
22-31: Well-implemented addition of user and exam application context to success events.The
publishSuccessEventmethod correctly accepts and propagates theuserIdandexamApplicationIdparameters to enable targeted refund notifications. The parameter order is logical and consistent.
33-43: Consistent handling of user and exam application context in failure events.The
publishFailureEventmethod maintains consistency with the success event method by accepting and properly propagating the same additional parameters (userIdandexamApplicationId).src/main/java/life/mosu/mosuserver/infra/notify/dto/NotificationEvent.java (2)
16-19: Clean implementation of parameterized factory method.The three-parameter factory method provides a clean API for creating notification events with specific user and target context, supporting the enhanced notification functionality.
9-14: Restrictive Validation Is Safe for Single-Parameter FactoryThe single-parameter
create(NotificationStatus)is only ever called withSIGN_UP_SUCCESS(inProfileService.java). All other statuses use the three-argument overload (create(status, userId, targetId)), so the current restriction will not introduce runtime errors.src/main/java/life/mosu/mosuserver/application/notify/NotifyVariableFactory.java (2)
8-8: LGTM! Import statements correctly added.The new import statements for
RefundNotifyRequest,ExamApplicationNotifyProjection, andRefundNotifyProjectionare properly added to support the implemented methods.Also applies to: 13-13, 18-18
75-81: LGTM! Clean implementation with proper error handling.The implementation correctly:
- Uses the appropriate repository method with proper parameter
- Handles the Optional result with meaningful error code
- Delegates object construction to the DTO's factory method
This follows good practices by keeping the factory method focused and delegating complex object construction to the DTO itself.
| private void sendNotification(Long userId, Long postId) { | ||
| NotificationEvent event = NotificationEvent.create( | ||
| NotificationStatus.INQUIRY_ANSWER_SUCCESS, userId, postId); | ||
| notifier.notify(event); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding error handling for notification failures.
Similar to the refund notification, consider adding error handling to prevent inquiry answer creation from failing due to notification issues.
private void sendNotification(Long userId, Long postId) {
+ try {
NotificationEvent event = NotificationEvent.create(
NotificationStatus.INQUIRY_ANSWER_SUCCESS, userId, postId);
notifier.notify(event);
+ } catch (Exception e) {
+ log.error("문의 답변 완료 알림톡 발송 실패: userId={}, postId={}",
+ userId, postId, e);
+ }
}📝 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.
| private void sendNotification(Long userId, Long postId) { | |
| NotificationEvent event = NotificationEvent.create( | |
| NotificationStatus.INQUIRY_ANSWER_SUCCESS, userId, postId); | |
| notifier.notify(event); | |
| } | |
| private void sendNotification(Long userId, Long postId) { | |
| try { | |
| NotificationEvent event = NotificationEvent.create( | |
| NotificationStatus.INQUIRY_ANSWER_SUCCESS, userId, postId); | |
| notifier.notify(event); | |
| } catch (Exception e) { | |
| log.error("문의 답변 완료 알림톡 발송 실패: userId={}, postId={}", | |
| userId, postId, e); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/inquiry/InquiryAnswerService.java
around lines 95 to 99, the sendNotification method lacks error handling for
notification failures. Wrap the notification sending logic in a try-catch block
to catch any exceptions thrown by notifier.notify(event). Log the error or
handle it appropriately to ensure that notification failures do not cause the
inquiry answer creation process to fail.
| private NotificationVariable createRefundVariable(Long targetId) { | ||
| // RefundNotifyProjection projection = refundJpaRepository.findRefundByApplicationSchoolId( | ||
| // targetId) | ||
| // .orElseThrow( | ||
| // () -> new CustomRuntimeException(ErrorCode.APPLICATION_SCHOOL_NOT_FOUND)); | ||
| // return new RefundNotifyRequest( | ||
| // projection.paymentKey(), projection.examDate(), projection.schoolName(), | ||
| // projection.paymentMethod().getName(), projection.reason() | ||
| // ); | ||
| return null; | ||
| RefundNotifyProjection projection = refundJpaRepository.findRefundByExamApplicationId( | ||
| targetId) | ||
| .orElseThrow( | ||
| () -> new CustomRuntimeException(ErrorCode.APPLICATION_SCHOOL_NOT_FOUND)); | ||
| return new RefundNotifyRequest( | ||
| projection.paymentKey(), projection.examDate(), projection.schoolName(), | ||
| projection.refundAmount(), projection.paymentMethod().getName(), projection.reason() | ||
| ); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify error code consistency and consider null safety.
The implementation looks correct, but there are a couple of points to verify:
-
Error Code Mismatch: The method uses
ErrorCode.APPLICATION_SCHOOL_NOT_FOUNDwhen no refund data is found, but this seems semantically incorrect - it should probably beREFUND_NOT_FOUNDor similar. -
Method Chain Safety: The call
projection.paymentMethod().getName()assumespaymentMethod()is never null. Consider adding null safety if the payment method could be null.
Please verify the following:
Consider this improvement for null safety:
return new RefundNotifyRequest(
projection.paymentKey(), projection.examDate(), projection.schoolName(),
- projection.refundAmount(), projection.paymentMethod().getName(), projection.reason()
+ projection.refundAmount(),
+ projection.paymentMethod() != null ? projection.paymentMethod().getName() : "N/A",
+ projection.reason()
);🏁 Script executed:
#!/bin/bash
# Check if REFUND_NOT_FOUND or similar error code exists in ErrorCode enum
ast-grep --pattern 'enum ErrorCode {
$$$
}'Length of output: 47
🏁 Script executed:
#!/bin/bash
set -e
# Locate the ErrorCode enum definition
file=$(fd ErrorCode.java | head -n1)
echo "Found ErrorCode enum in: $file"
# Dump its contents to list all defined error codes
sed -n '/enum ErrorCode/,/};/p' "$file"Length of output: 6687
🏁 Script executed:
#!/bin/bash
set -e
# Locate and print the RefundNotifyProjection interface to check paymentMethod() return type
proj_file=$(fd RefundNotifyProjection.java | head -n1)
echo "=== $proj_file ==="
sed -n '1,200p' "$proj_file"
# Locate and inspect the Refund entity for the paymentMethod field nullability
entity_file=$(fd Refund.java | head -n1)
echo "=== $entity_file ==="
grep -R -n "paymentMethod" "$entity_file" -n
sed -n '1,200p' "$entity_file"Length of output: 1198
Use a dedicated REFUND_NOT_FOUND error code and guard against null paymentMethod
Your implementation should throw a semantically correct error when no refund is found and only assume paymentMethod is non-null if your DB/schema guarantees it.
• In ErrorCode enum (src/main/java/life/mosu/mosuserver/global/exception/ErrorCode.java), add:
REFUND_NOT_FOUND(HttpStatus.NOT_FOUND, "환불 정보를 찾을 수 없습니다."),• In NotifyVariableFactory#createRefundVariable (src/main/java/life/mosu/mosuserver/application/notify/NotifyVariableFactory.java), update the exception and (optionally) guard the paymentMethod name:
- .orElseThrow(() -> new CustomRuntimeException(ErrorCode.APPLICATION_SCHOOL_NOT_FOUND));
+ .orElseThrow(() -> new CustomRuntimeException(ErrorCode.REFUND_NOT_FOUND));
return new RefundNotifyRequest(
projection.paymentKey(),
projection.examDate(),
projection.schoolName(),
projection.refundAmount(),
- projection.paymentMethod().getName(),
+ projection.paymentMethod() != null
+ ? projection.paymentMethod().getName()
+ : "N/A",
projection.reason()
);• If paymentMethod is declared non-nullable in your JPA mapping and DB schema, you can omit the null check; otherwise keep a safe fallback or throw a different exception.
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/notify/NotifyVariableFactory.java
lines 64-73, replace the current error code APPLICATION_SCHOOL_NOT_FOUND with a
new dedicated REFUND_NOT_FOUND error code that you must add to the ErrorCode
enum in src/main/java/life/mosu/mosuserver/global/exception/ErrorCode.java.
Additionally, add a null check for paymentMethod before accessing its name; if
paymentMethod can be null, either provide a safe fallback value or throw an
appropriate exception to avoid NullPointerException.
| private void sendNotification(Long userId, Long examApplicationId) { | ||
| NotificationEvent notificationEvent = NotificationEvent.create( | ||
| NotificationStatus.REFUND_SUCCESS, | ||
| userId, examApplicationId); | ||
| notifier.notify(notificationEvent); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding error handling for notification failures.
The notification sending could fail and should not cause the refund transaction to fail since it occurs after commit. Consider wrapping the notification logic in a try-catch block to log failures without propagating exceptions.
private void sendNotification(Long userId, Long examApplicationId) {
+ try {
NotificationEvent notificationEvent = NotificationEvent.create(
NotificationStatus.REFUND_SUCCESS,
userId, examApplicationId);
notifier.notify(notificationEvent);
+ } catch (Exception e) {
+ log.error("환불 성공 알림톡 발송 실패: userId={}, examApplicationId={}",
+ userId, examApplicationId, e);
+ }
}📝 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.
| private void sendNotification(Long userId, Long examApplicationId) { | |
| NotificationEvent notificationEvent = NotificationEvent.create( | |
| NotificationStatus.REFUND_SUCCESS, | |
| userId, examApplicationId); | |
| notifier.notify(notificationEvent); | |
| } | |
| private void sendNotification(Long userId, Long examApplicationId) { | |
| try { | |
| NotificationEvent notificationEvent = NotificationEvent.create( | |
| NotificationStatus.REFUND_SUCCESS, | |
| userId, examApplicationId); | |
| notifier.notify(notificationEvent); | |
| } catch (Exception e) { | |
| log.error("환불 성공 알림톡 발송 실패: userId={}, examApplicationId={}", | |
| userId, examApplicationId, e); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/refund/tx/RefundTxEventListener.java
around lines 36 to 41, the sendNotification method currently does not handle
exceptions that may occur during notification sending. To fix this, wrap the
notification logic inside a try-catch block that catches any exceptions, logs
the failure with relevant details, and prevents the exception from propagating
further, ensuring the refund transaction is not affected by notification
failures.
| @@ -1,24 +1,35 @@ | |||
| package life.mosu.mosuserver.domain.refund; | |||
|
|
|||
| import io.lettuce.core.dynamic.annotation.Param; | |||
There was a problem hiding this comment.
Fix incorrect import for @param annotation.
The import is using io.lettuce.core.dynamic.annotation.Param instead of the correct Spring Data annotation.
-import io.lettuce.core.dynamic.annotation.Param;
+import org.springframework.data.repository.query.Param;📝 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.
| import io.lettuce.core.dynamic.annotation.Param; | |
| -import io.lettuce.core.dynamic.annotation.Param; | |
| +import org.springframework.data.repository.query.Param; |
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/domain/refund/RefundJpaRepository.java at
line 3, the import statement for the @Param annotation is incorrect. Replace the
import of io.lettuce.core.dynamic.annotation.Param with the correct Spring Data
annotation import org.springframework.data.repository.query.Param to ensure
proper functionality.
1 participant
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Localization