MOSU-28 feat: 알림톡 기능 구현 #104
Conversation
WalkthroughThis update introduces a comprehensive notification system, including event-driven notification strategies, DTOs, and infrastructure for sending notifications via external services. It adds new repository interfaces and implementations for batch and projection queries, message templating with localization, and configuration for WebClient and message sources. Several new domain projections and DTOs support notification scenarios for applications, exams, inquiries, and refunds. Changes
Sequence Diagram(s)sequenceDiagram
participant EventSource as Event Source
participant NotifyEventListener
participant NotifyService
participant ApplicationContext
participant ProfileRepository
participant NotifyStrategy
participant NotifyClientAdapter
EventSource->>NotifyEventListener: Publish NotifyEvent
NotifyEventListener->>NotifyService: notify(event)
NotifyService->>ApplicationContext: getNotifyStrategy(event.status)
ApplicationContext-->>NotifyService: NotifyStrategy
NotifyService->>ProfileRepository: findByUserId(event.userId)
ProfileRepository-->>NotifyService: Profile (with phone number)
NotifyService->>NotifyStrategy: apply(phoneNumber, event)
NotifyStrategy-->>NotifyService: NotifyEventRequest
NotifyService->>NotifyClientAdapter: send(NotifyEventRequest)
NotifyClientAdapter-->>NotifyService: (async send)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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 introduces a new Alimtalk notification feature, enabling the system to send automated messages for various user interactions and important events. It establishes an event-driven notification framework, integrates with an external messaging API using WebClient, and lays the groundwork for future performance optimizations in application data handling.
Highlights
- Alimtalk (KakaoTalk Notification) Integration: I've implemented a comprehensive Alimtalk notification system, allowing the application to send various types of messages such as application confirmations, exam reminders (1-week, 3-day, 1-day before), refund confirmations, inquiry answer notifications, and sign-up welcome messages. This is achieved through a new event-driven architecture and a strategy pattern for message generation.
- Asynchronous Event Handling: A new
@ReactiveEventListenerannotation has been introduced, combining@Asyncand@EventListener, to ensure that notification events are processed asynchronously, preventing blocking operations and improving application responsiveness. - WebClient and External API Communication: The
spring-boot-starter-webfluxdependency was added, andWebClientis now configured to handle external API calls to the Alimtalk service, facilitating efficient and non-blocking communication for sending notifications. - Application Service Refactoring (Initial Stage): I've included a commented-out section in
ApplicationServicethat outlines a refactored approach for handling school applications using a newSchoolQueryRepositorywith QueryDSL. This indicates a future direction for optimizing data retrieval, though the bulk save functionality is not yet fully implemented.
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 is currently in preview and 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 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 introduces a notification feature using Alimtalk. The implementation leverages the Strategy pattern for handling different notification types and employs asynchronous event handling. I've identified a few areas for improvement, including a critical bug in a repository query that will cause runtime failures. I've also noted some opportunities to improve error handling, enhance maintainability by cleaning up work-in-progress code, and boost efficiency by using projections for database queries. Addressing these points will significantly increase the robustness and quality of this new feature.
| @Query(""" | ||
| SELECT | ||
| p.paymentKey AS paymentKey, | ||
| a.examDate AS examDate, | ||
| p.paymentMethod AS paymentMethod, | ||
| r.reason AS reason | ||
| FROM RefundJpaEntity r | ||
| LEFT JOIN PaymentJpaEntity p ON r.applicationSchoolId = p.applicationSchoolId | ||
| LEFT JOIN ApplicationSchoolJpaEntity a ON r.applicationSchoolId = a.id | ||
| WHERE r.applicationSchoolId = :applicationSchoolId | ||
| """) |
There was a problem hiding this comment.
The query for findRefundByApplicationSchoolId is missing a.schoolName in the SELECT clause. The RefundNotifyProjection record expects this field, and its absence will cause a mapping error at runtime. Please add a.schoolName AS schoolName to the projection.
@Query("""
SELECT
p.paymentKey AS paymentKey,
a.examDate AS examDate,
a.schoolName AS schoolName,
p.paymentMethod AS paymentMethod,
r.reason AS reason
FROM RefundJpaEntity r
LEFT JOIN PaymentJpaEntity p ON r.applicationSchoolId = p.applicationSchoolId
LEFT JOIN ApplicationSchoolJpaEntity a ON r.applicationSchoolId = a.id
WHERE r.applicationSchoolId = :applicationSchoolId
""")| inquiryEntity.updateStatusToComplete(); | ||
|
|
||
| ProfileJpaEntity profile = profileJpaRepository.findByUserId(inquiryEntity.getUserId()) | ||
| .orElseThrow(() -> new RuntimeException("")); |
There was a problem hiding this comment.
Throwing a generic RuntimeException with an empty message is not informative. It's better to use the existing CustomRuntimeException with a specific ErrorCode to provide more context. Consider creating and using an appropriate error code (e.g., PROFILE_NOT_FOUND in ErrorCode.java) to represent this specific failure case.
| .orElseThrow(() -> new RuntimeException("")); | |
| .orElseThrow(() -> new CustomRuntimeException(ErrorCode.PROFILE_NOT_FOUND)); |
| RefundNotifyProjection projection = refundJpaRepository.findRefundByApplicationSchoolId( | ||
| event.id()); | ||
|
|
||
| //TODO: 가격 추가 |
There was a problem hiding this comment.
There's a TODO here to add the price, and the RefundNotifyVariablesDto has the amount field commented out. However, the message template in messages_ko.properties uses a #{refundAmount} placeholder. This will lead to an incomplete message being sent to the user. Please implement the logic to fetch and pass the refund amount to ensure the notification is correct.
src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java
Show resolved
Hide resolved
| ApplicationSchoolJpaEntity applicationSchool = applicationSchoolJpaRepository.findById( | ||
| event.id()).orElseThrow( | ||
| () -> new CustomRuntimeException(ErrorCode.APPLICATION_SCHOOL_NOT_FOUND)); |
There was a problem hiding this comment.
Fetching the entire ApplicationSchoolJpaEntity object when only a few fields are needed is inefficient and can lead to performance issues. Consider creating a dedicated projection to fetch only the required fields (examDate, examinationNumber, schoolName). This would also make the code's intent clearer.
This feedback also applies to Exam3DayBeforeNotifyStrategy.
|
|
||
| private final MessageSource messageSource; | ||
| private final NotifyEventTemplate template; | ||
| private final InquiryJpaRepository inquiryJpaRepository; |
There was a problem hiding this comment.
The InquiryJpaRepository dependency is injected but not used within this class. Please remove it to keep the code clean and avoid unnecessary dependencies.
| String alimTalkContent = messageSource.getMessage("notify.signup.complete.alimtalk", null, | ||
| Locale.KOREA); |
There was a problem hiding this comment.
This strategy uses MessageSource directly to get the message content, while other strategies use the NotifyEventTemplate which encapsulates MessageSource logic. For consistency across strategies, it would be better to use template.getProcessedMessage(...) here as well, even if there are no variables to replace.
| String alimTalkContent = messageSource.getMessage("notify.signup.complete.alimtalk", null, | |
| Locale.KOREA); | |
| String alimTalkContent = template.getProcessedMessage("notify.signup.complete.alimtalk", | |
| java.util.Collections.emptyMap()); |
| public void saveAllApplicationSchools(List<ApplicationSchoolJpaEntity> applicationSchools) { | ||
| String sql = """ | ||
| INSERT INTO application_school | ||
| (user_id, application_id, school_id, school_name, area, exam_date) | ||
| VALUES (?, ?, ?, ?, ?, ?) | ||
| """; | ||
| } |
There was a problem hiding this comment.
The saveAllApplicationSchools method is incomplete. It defines an SQL string but doesn't execute any database operation with JdbcTemplate. This is currently dead code. Please either complete the implementation (e.g., using jdbcTemplate.batchUpdate) or remove this method if it's not intended to be used yet.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
🧹 Nitpick comments (28)
src/main/java/life/mosu/mosuserver/domain/refund/RefundNotifyProjection.java (1)
6-13: Remove TODO or create a tracked issueThe inline
// TODO: 환불 금액 추가will be easy to forget. Convert to a tracked issue or implement the field now to avoid another projection change (which triggers recompilation & migration work).Example:
- PaymentMethod paymentMethod, - String reason - // TODO: 환불 금액 추가 + PaymentMethod paymentMethod, + String reason, + BigDecimal refundAmount // new fieldsrc/main/java/life/mosu/mosuserver/presentation/notify/dto/NotifyButtonUrls.java (1)
14-16: Consider usingList.of()for better performance.While the current implementation works correctly,
List.of()provides better performance thanArrays.asList()for creating immutable lists in modern Java.public static NotifyButtonUrls of(NotifyButtonUrl... btns) { - return new NotifyButtonUrls(Arrays.asList(btns)); + return new NotifyButtonUrls(List.of(btns)); }src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyEventSuccessMessageDto.java (1)
16-27: Remove redundant static factory method.The
create()method is unnecessary since Java records automatically provide a constructor with the same signature. This adds unnecessary code complexity.-public static NotifyEventSuccessMessageDto create( - Integer no, - String telNum, - String msgContent, - String smsContent, - String useSms, - NotifyButtonUrls btnUrls -) { - return new NotifyEventSuccessMessageDto( - no, telNum, msgContent, smsContent, useSms, btnUrls - ); -}Simply use the record constructor directly:
var dto = new NotifyEventSuccessMessageDto(no, telNum, msgContent, smsContent, useSms, btnUrls);src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyEventRemindMessageDto.java (1)
17-33: Remove redundant static factory method.Similar to
NotifyEventSuccessMessageDto, this static factory method is unnecessary since Java records provide constructors automatically.-public static NotifyEventRemindMessageDto create( - Integer no, - String telNum, - LocalDateTime reserveTime, - String msgContent, - String smsContent, - NotifyButtonUrls btnUrls -) { - return new NotifyEventRemindMessageDto( - no, - telNum, - reserveTime, - msgContent, - smsContent, - btnUrls - ); -}src/main/resources/application.yml (1)
84-84: Fix missing newline at end of file.Add a newline character at the end of the file to comply with YAML formatting standards.
kakao: channel-id: ${KAKAO_CHANNEL_ID} +src/main/java/life/mosu/mosuserver/presentation/notify/dto/NotifyEventRequest.java (1)
8-11: Ensure consistent naming conventions in JSON properties.The JSON property names use inconsistent naming conventions:
userIduses camelCaseapi_keyandtemplate_iduse snake_caseConsider standardizing to either camelCase or snake_case:
public record NotifyEventRequest( - @JsonProperty("userId") String userId, - @JsonProperty("api_key") String apiKey, - @JsonProperty("template_id") Integer templateId, + @JsonProperty("user_id") String userId, + @JsonProperty("api_key") String apiKey, + @JsonProperty("template_id") Integer templateId, @JsonProperty("messages") List<Map<String, Object>> messages ) {Or alternatively, use camelCase throughout:
public record NotifyEventRequest( @JsonProperty("userId") String userId, - @JsonProperty("api_key") String apiKey, - @JsonProperty("template_id") Integer templateId, + @JsonProperty("apiKey") String apiKey, + @JsonProperty("templateId") Integer templateId, @JsonProperty("messages") List<Map<String, Object>> messages ) {src/main/java/life/mosu/mosuserver/domain/school/SchoolQueryRepository.java (1)
7-12: Consider adding method documentation and parameter naming.The interface would benefit from documentation explaining the method's purpose and parameter structure.
Add JavaDoc documentation:
public interface SchoolQueryRepository { + /** + * Finds school IDs that match the given conditions. + * + * @param conditions List of triples containing (schoolName, area, examDate) + * @return List of school IDs matching the conditions + */ List<Long> findSchoolsByConditions( List<Triple<String, Area, LocalDate>> conditions);Also consider using a more descriptive parameter name:
List<Long> findSchoolsByConditions( - List<Triple<String, Area, LocalDate>> conditions); + List<Triple<String, Area, LocalDate>> schoolConditions);src/main/java/life/mosu/mosuserver/infra/notify/dto/Exam3DayBeforeNotifyVariablesDto.java (1)
6-10: Consider creating a base interface for notification variable DTOs.Since multiple DTOs follow the same pattern with a
toMap()method, consider creating a common interface to ensure consistency across the notification system.Create a base interface:
package life.mosu.mosuserver.infra.notify.dto; import java.util.Map; public interface NotifyVariablesDto { Map<String, String> toMap(); }Then implement it in your DTOs:
-public record Exam3DayBeforeNotifyVariablesDto( +public record Exam3DayBeforeNotifyVariablesDto( LocalDate examDate, String examinationNumber, String schoolName -) { +) implements NotifyVariablesDto {This would provide better type safety and consistency across all notification variable DTOs.
src/main/java/life/mosu/mosuserver/infra/notify/dto/Exam1WeekBeforeNotifyVariablesDto.java (1)
12-18: Consider date formatting and null safety in toMap() method.The implementation is functionally correct, but consider these improvements:
examDate.toString()produces ISO format (YYYY-MM-DD) which might not be user-friendly- No null checks could cause NPE if any field is null
Consider this enhanced version:
public Map<String, String> toMap() { + if (examDate == null || paymentKey == null || schoolName == null) { + throw new IllegalStateException("All fields must be non-null"); + } return Map.of( - "examDate", examDate.toString(), + "examDate", examDate.format(DateTimeFormatter.ofPattern("yyyy년 MM월 dd일")), "paymentKey", paymentKey, "schoolName", schoolName ); }src/main/java/life/mosu/mosuserver/infra/notify/dto/Exam1DayBeforeNotifyVariablesDto.java (1)
12-18: Same date formatting and null safety concerns as Exam1WeekBeforeNotifyVariablesDto.This DTO has identical issues to the previous one regarding date formatting and null safety.
Consider applying the same improvements as suggested for
Exam1WeekBeforeNotifyVariablesDto:public Map<String, String> toMap() { + if (examDate == null || examinationNumber == null || schoolName == null) { + throw new IllegalStateException("All fields must be non-null"); + } return Map.of( - "examDate", examDate.toString(), + "examDate", examDate.format(DateTimeFormatter.ofPattern("yyyy년 MM월 dd일")), "examinationNumber", examinationNumber, "schoolName", schoolName ); }Additionally, consider creating a common interface or abstract class for notification DTOs to ensure consistency:
public interface NotifyVariablesDto { Map<String, String> toMap(); }src/main/java/life/mosu/mosuserver/global/config/MessageSourceConfig.java (1)
11-17: Standard MessageSource configuration looks good.The implementation follows Spring best practices for internationalization. The configuration is correct and appropriate for Korean message support.
Optional enhancement - consider adding cache seconds for better performance:
@Bean public MessageSource messageSource() { ReloadableResourceBundleMessageSource messageSource = new ReloadableResourceBundleMessageSource(); messageSource.setBasename("classpath:messages"); messageSource.setDefaultEncoding("UTF-8"); + messageSource.setCacheSeconds(60); // Cache for 1 minute return messageSource; }src/main/java/life/mosu/mosuserver/infra/notify/dto/RefundNotifyVariablesDto.java (1)
15-24: Consider null safety for the toMap method.The current implementation doesn't handle potential null values. Consider adding null checks or using a more robust approach to handle null fields.
public Map<String, String> toMap() { - return Map.of( - "paymentKey", paymentKey, - "examDate", examDate.toString(), - "schoolName", schoolName, - "paymentMethod", paymentMethod, - "reason", reason - ); + Map<String, String> map = new HashMap<>(); + if (paymentKey != null) map.put("paymentKey", paymentKey); + if (examDate != null) map.put("examDate", examDate.toString()); + if (schoolName != null) map.put("schoolName", schoolName); + if (paymentMethod != null) map.put("paymentMethod", paymentMethod); + if (reason != null) map.put("reason", reason); + return Map.copyOf(map); }Alternatively, if null values are not expected, consider adding
@NonNullannotations to the record fields.src/main/java/life/mosu/mosuserver/domain/school/SchoolQueryRepositoryImpl.java (2)
22-28: Add null safety for individual condition elements.The method doesn't validate that the Triple elements are not null, which could cause issues in the query building.
for (Triple<String, Area, LocalDate> cond : conditions) { + if (cond.getLeft() == null || cond.getMiddle() == null || cond.getRight() == null) { + continue; // Skip invalid conditions + } builder.or( QSchoolJpaEntity.schoolJpaEntity.schoolName.eq(cond.getLeft()) .and(QSchoolJpaEntity.schoolJpaEntity.area.eq(cond.getMiddle())) .and(QSchoolJpaEntity.schoolJpaEntity.examDate.eq(cond.getRight())) ); }
30-35: Consider performance implications for large condition sets.For very large condition lists, this approach might result in performance issues due to the large number of OR conditions. Consider alternative approaches for bulk operations.
For better performance with large datasets, consider:
- Using
INclauses with composite keys if the database supports it- Implementing pagination for large result sets
- Adding database indexes on the combination of (schoolName, area, examDate)
- Using EXISTS subqueries for better performance in some cases
You might also want to add logging or monitoring to track query performance:
+ log.debug("Querying schools with {} conditions", conditions.size()); return queryFactory .select(QSchoolJpaEntity.schoolJpaEntity.id) .from(QSchoolJpaEntity.schoolJpaEntity) .where(builder) .fetch();src/main/java/life/mosu/mosuserver/infra/notify/service/NotifyService.java (1)
36-38: Consider the implications of fire-and-forget pattern.The
subscribe()call makes this a fire-and-forget operation. While this might be appropriate for notifications, consider if you need to handle completion or errors differently.For better error handling and monitoring, consider:
.doOnTerminate(() -> log.info("알림톡 전송 완료: {}", request)) - .subscribe(); + .subscribe( + response -> log.debug("알림톡 전송 성공: {}", response), + error -> { + log.error("알림톡 전송 실패: {}", error.getMessage(), error); + // Consider adding metrics or alerting here + } + );Or if you need to handle the result:
- .subscribe(); + .subscribe( + response -> handleSuccess(response, request), + error -> handleError(error, request) + );src/main/java/life/mosu/mosuserver/infra/notify/strategy/InquiryAnswerNotifyStrategy.java (1)
28-47: Consider optimizing and verify SMS content handling.The implementation is functionally correct, but has a few considerations:
- SMS content is empty: Line 45 sets SMS content to an empty string. Verify this is intentional.
- Template processing: The alimtalk template processing looks correct.
- Button URL configuration: Using the same URL for both button text and URL might be confusing.
Consider this improvement for the button URL:
- NotifyButtonUrls btnUrls = NotifyButtonUrls.of( - NotifyButtonUrl.of(INQUIRY, INQUIRY) - ); + NotifyButtonUrls btnUrls = NotifyButtonUrls.of( + NotifyButtonUrl.of("문의 확인", INQUIRY) + );Verify that empty SMS content is the intended behavior for inquiry answer notifications.
src/main/java/life/mosu/mosuserver/domain/applicationschool/ApplicationSchoolJpaRepository.java (2)
11-11: Remove commented-out code or implement it properly.The commented-out method
existsByUserIdAndSchoolIdsshould either be removed if not needed or implemented if it's part of the planned refactoring mentioned in ApplicationService.java.
24-30: Consider JOIN consistency and null handling.The query uses
LEFT JOINwhich allows for null payment keys, but this might not be the intended behavior for notification purposes. Consider whether anINNER JOINwould be more appropriate if payments are required for notifications.- @Query(""" - SELECT p.paymentKey, a.examDate, a.schoolName, a.lunch - FROM ApplicationSchoolJpaEntity a - LEFT JOIN PaymentJpaEntity p ON a.id = p.applicationSchoolId - WHERE a.id = :applicationSchoolId - """) + @Query(""" + SELECT p.paymentKey, a.examDate, a.schoolName, a.lunch + FROM ApplicationSchoolJpaEntity a + INNER JOIN PaymentJpaEntity p ON a.id = p.applicationSchoolId + WHERE a.id = :applicationSchoolId + """)src/main/java/life/mosu/mosuserver/infra/notify/strategy/SignUpNotifyStrategy.java (2)
29-30: Consider externalizing the locale configuration.The hardcoded
Locale.KOREAshould be extracted to a configuration property or constant to improve maintainability and internationalization support.- String alimTalkContent = messageSource.getMessage("notify.signup.complete.alimtalk", null, - Locale.KOREA); + String alimTalkContent = messageSource.getMessage("notify.signup.complete.alimtalk", null, + Locale.getDefault());
32-34: Verify button URL appropriateness.Using
HOMEPAGEfor both the URL and button text might not provide the best user experience. Consider using a more descriptive button text.- NotifyButtonUrls btnUrls = NotifyButtonUrls.of( - NotifyButtonUrl.of(HOMEPAGE, HOMEPAGE) - ); + NotifyButtonUrls btnUrls = NotifyButtonUrls.of( + NotifyButtonUrl.of("홈페이지", HOMEPAGE) + );src/main/java/life/mosu/mosuserver/infra/notify/strategy/RefundNotifyStrategy.java (1)
30-30: Address the TODO comment.The TODO comment indicates that refund amount should be added to the notification. This is crucial information for users and should be prioritized.
Would you like me to help implement the refund amount field in the projection and DTO, or should this be tracked as a separate issue?
src/main/java/life/mosu/mosuserver/presentation/notify/dto/NotifyStatus.java (1)
19-20: Consider adding getter methods for better encapsulation.While Lombok's
@RequiredArgsConstructorcreates the constructor, consider adding explicit getter methods for better API clarity and potential future enhancements.+ public EventStatus getStatus() { + return status; + } + + public String getMessage() { + return message; + }src/main/java/life/mosu/mosuserver/infra/notify/NotifyEventTemplate.java (2)
25-31: Improve variable substitution performance and add null safety checks.The current implementation uses
StringUtils.replacein a loop which can be inefficient with many variables. Additionally, there's no null safety for the variables map or validation that all placeholders are replaced.Consider this improved implementation:
public String getProcessedMessage(String code, Map<String, String> variables) { String message = messageSource.getMessage(code, null, Locale.KOREA); + if (variables == null || variables.isEmpty()) { + return message; + } + for (Map.Entry<String, String> entry : variables.entrySet()) { + if (entry.getKey() == null || entry.getValue() == null) { + continue; + } message = StringUtils.replace(message, "#{" + entry.getKey() + "}", entry.getValue()); } return message; }
26-26: Consider making locale configurable instead of hardcoded.The locale is hardcoded to
Locale.KOREA. Consider making this configurable or parameterizing it to support internationalization.-String message = messageSource.getMessage(code, null, Locale.KOREA); +String message = messageSource.getMessage(code, null, Locale.getDefault());Or inject a configurable locale property.
src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam3DayBeforeNotifyStrategy.java (1)
46-48: Consider making notification scheduling time configurable.The hardcoded 8:00 AM scheduling time should be configurable through application properties to allow for operational flexibility.
+@Value("${notification.reminder.time.hour:8}") +private int reminderHour; +@Value("${notification.reminder.time.minute:0}") +private int reminderMinute; LocalDateTime reserveTime = applicationSchool.getExamDate() .minusDays(3) - .atTime(8, 0); + .atTime(reminderHour, reminderMinute);src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam1DayBeforeNotifyStrategy.java (1)
46-48: Same scheduling time configurability issue as other strategies.The hardcoded 8:00 AM scheduling time should be configurable. This issue appears across multiple exam reminder strategies.
Apply the same configuration approach as suggested for
Exam3DayBeforeNotifyStrategy.src/main/java/life/mosu/mosuserver/infra/notify/strategy/ApplicationNotifyStrategy.java (1)
31-33: Consider adding null safety for projection fields.The projection fields (
paymentKey(),examDate(),schoolName(),lunch().getLunchName()) should be validated for null values before use, especially the nestedlunch().getLunchName()call.+if (projection == null) { + throw new CustomRuntimeException(ErrorCode.APPLICATION_SCHOOL_NOT_FOUND); +} + ApplicationNotifyVariablesDto dto = new ApplicationNotifyVariablesDto( projection.paymentKey(), projection.examDate(), projection.schoolName(), - projection.lunch().getLunchName()); + projection.lunch() != null ? projection.lunch().getLunchName() : "");src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam1WeekBeforeNotifyStrategy.java (1)
42-44: Apply same scheduling time configurability as other strategies.The hardcoded 8:00 AM scheduling time should be configurable, consistent with the other exam reminder strategies.
Apply the same configuration approach as suggested for the other exam reminder strategies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
build.gradle(1 hunks)src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java(3 hunks)src/main/java/life/mosu/mosuserver/application/inquiry/InquiryAnswerService.java(3 hunks)src/main/java/life/mosu/mosuserver/domain/applicationschool/ApplicationSchoolJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/applicationschool/ApplicationSchoolNotifyProjection.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/applicationschool/OneWeekNotifyProjection.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/domain/school/SchoolQueryRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/school/SchoolQueryRepositoryImpl.java(1 hunks)src/main/java/life/mosu/mosuserver/global/annotation/ReactiveEventListener.java(1 hunks)src/main/java/life/mosu/mosuserver/global/config/MessageSourceConfig.java(1 hunks)src/main/java/life/mosu/mosuserver/global/config/WebClientConfig.java(1 hunks)src/main/java/life/mosu/mosuserver/global/exception/ErrorCode.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/NotifyEvent.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/NotifyEventPublisher.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/NotifyEventTemplate.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/constant/NotifyConstants.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/dto/ApplicationNotifyVariablesDto.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/dto/Exam1DayBeforeNotifyVariablesDto.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/dto/Exam1WeekBeforeNotifyVariablesDto.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/dto/Exam3DayBeforeNotifyVariablesDto.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/dto/InquiryAnswerNotifyVariablesDto.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyEventRemindMessageDto.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyEventSuccessMessageDto.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/dto/RefundNotifyVariablesDto.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/service/NotifyService.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/strategy/ApplicationNotifyStrategy.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam1DayBeforeNotifyStrategy.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam1WeekBeforeNotifyStrategy.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam3DayBeforeNotifyStrategy.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/strategy/InquiryAnswerNotifyStrategy.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/strategy/NotifyStrategy.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/strategy/NotifyStrategyMap.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/strategy/RefundNotifyStrategy.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/strategy/SignUpNotifyStrategy.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/respository/BulkRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/notify/NotifyEventListener.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/notify/dto/EventStatus.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/notify/dto/NotifyButtonUrl.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/notify/dto/NotifyButtonUrls.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/notify/dto/NotifyEventRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/notify/dto/NotifyStatus.java(1 hunks)src/main/resources/application.yml(2 hunks)src/main/resources/messages_ko.properties(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
src/main/java/life/mosu/mosuserver/presentation/notify/NotifyEventListener.java (1)
src/main/java/life/mosu/mosuserver/infra/notify/NotifyEventPublisher.java (1)
Component(7-16)
src/main/java/life/mosu/mosuserver/domain/school/SchoolQueryRepositoryImpl.java (1)
src/main/java/life/mosu/mosuserver/infra/respository/BulkRepository.java (1)
Repository(10-25)
src/main/java/life/mosu/mosuserver/infra/notify/strategy/SignUpNotifyStrategy.java (2)
src/main/java/life/mosu/mosuserver/infra/notify/constant/NotifyConstants.java (1)
NotifyConstants(3-23)src/main/java/life/mosu/mosuserver/infra/notify/strategy/InquiryAnswerNotifyStrategy.java (1)
Component(20-51)
src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam3DayBeforeNotifyStrategy.java (4)
src/main/java/life/mosu/mosuserver/infra/notify/constant/NotifyConstants.java (1)
NotifyConstants(3-23)src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam1DayBeforeNotifyStrategy.java (1)
Component(22-62)src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam1WeekBeforeNotifyStrategy.java (1)
Component(20-57)src/main/java/life/mosu/mosuserver/infra/notify/strategy/ApplicationNotifyStrategy.java (1)
Component(19-53)
src/main/java/life/mosu/mosuserver/infra/notify/strategy/ApplicationNotifyStrategy.java (3)
src/main/java/life/mosu/mosuserver/infra/notify/constant/NotifyConstants.java (1)
NotifyConstants(3-23)src/main/java/life/mosu/mosuserver/infra/notify/strategy/SignUpNotifyStrategy.java (1)
Component(18-42)src/main/java/life/mosu/mosuserver/infra/notify/strategy/RefundNotifyStrategy.java (1)
Component(18-55)
src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam1DayBeforeNotifyStrategy.java (3)
src/main/java/life/mosu/mosuserver/infra/notify/constant/NotifyConstants.java (1)
NotifyConstants(3-23)src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam1WeekBeforeNotifyStrategy.java (1)
Component(20-57)src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam3DayBeforeNotifyStrategy.java (1)
Component(22-64)
src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam1WeekBeforeNotifyStrategy.java (2)
src/main/java/life/mosu/mosuserver/infra/notify/constant/NotifyConstants.java (1)
NotifyConstants(3-23)src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam1DayBeforeNotifyStrategy.java (1)
Component(22-62)
src/main/java/life/mosu/mosuserver/infra/notify/strategy/InquiryAnswerNotifyStrategy.java (4)
src/main/java/life/mosu/mosuserver/infra/notify/constant/NotifyConstants.java (1)
NotifyConstants(3-23)src/main/java/life/mosu/mosuserver/infra/notify/strategy/ApplicationNotifyStrategy.java (1)
Component(19-53)src/main/java/life/mosu/mosuserver/infra/notify/strategy/SignUpNotifyStrategy.java (1)
Component(18-42)src/main/java/life/mosu/mosuserver/infra/notify/strategy/RefundNotifyStrategy.java (1)
Component(18-55)
🪛 YAMLlint (1.37.1)
src/main/resources/application.yml
[error] 84-84: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (26)
build.gradle (1)
88-90: Beware of dual‐stack servlet & reactive startersAdding
spring-boot-starter-webfluxwhile keepingspring-boot-starter-webpulls in both the servlet (Tomcat) and reactive (Netty) stacks.
Spring Boot will default to servlet and silently skip Netty auto-configuration, so the extra dependency:
- Bloats the artifact size.
- Creates the risk of classpath conflicts (e.g.,
WebFiltervsFilter,WebExceptionHandlervsExceptionHandlerExceptionResolver).- Provides little benefit if you only need
WebClient, becauseWebClientcan be obtained fromspring-boot-starter-webfluxalone.Consider one of the following:
- implementation 'org.springframework.boot:spring-boot-starter-web' - ... - implementation 'org.springframework.boot:spring-boot-starter-webflux' + // Option A – servlet stack only, use RestTemplate or bring `spring-webflux` as *compileOnly* for WebClient + implementation 'org.springframework.boot:spring-boot-starter-web' + compileOnly 'org.springframework:spring-webflux' + + // Option B – go fully reactive (preferred if you plan to use WebFlux controllers) + // implementation 'org.springframework.boot:spring-boot-starter-webflux'Pick one stack to avoid surprises at runtime.
Run
./gradlew dependencies | grep "spring-boot-starter-web"to confirm duplicate transitive trees.src/main/java/life/mosu/mosuserver/domain/applicationschool/ApplicationSchoolNotifyProjection.java (1)
6-11: Confirm projection aliases match your JPQLSpring projection mapping is name-based. Ensure the JPQL in
ApplicationSchoolJpaRepository#findPaymentByApplicationSchoolIdaliases columns exactly to:
paymentKey, examDate, schoolName, lunchOtherwise Spring will fail to bind the record constructor at runtime.
src/main/java/life/mosu/mosuserver/presentation/notify/dto/NotifyButtonUrls.java (1)
18-20: Clean implementation using streams.The method properly delegates to
NotifyButtonUrl.toMap()and uses streams effectively for conversion.src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyEventSuccessMessageDto.java (1)
30-39: Proper serialization implementation.The
toMap()method correctly handles field name mapping and nested object conversion for external API integration.src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyEventRemindMessageDto.java (1)
42-42: Clarify hardcoded SMS usage value.The hardcoded
"use_sms": "1"differs fromNotifyEventSuccessMessageDtowhich has a configurableuseSmsfield. This inconsistency suggests different SMS behavior for reminders.Please verify if this hardcoded value is intentional for reminder messages or if it should be configurable like success messages.
src/main/java/life/mosu/mosuserver/presentation/notify/dto/EventStatus.java (1)
3-5: Clean enum implementation.The enum provides clear, descriptive values for event status tracking and follows Java naming conventions.
src/main/resources/application.yml (2)
49-51: Proper internationalization configuration.The Spring messages configuration correctly sets up the message source for the notification templates.
77-84: Good security practice with environment variables.The configuration properly externalizes sensitive information like API keys and user credentials using environment variables.
src/main/java/life/mosu/mosuserver/infra/notify/strategy/NotifyStrategy.java (1)
6-9: LGTM! Clean strategy interface design.The interface follows the strategy pattern effectively, providing a clear contract for different notification strategies. The method signature is well-defined and the package structure is logical.
src/main/java/life/mosu/mosuserver/domain/applicationschool/OneWeekNotifyProjection.java (1)
5-11: LGTM! Well-designed projection record.The record effectively encapsulates the necessary data for one-week exam notifications. The field types are appropriate and the immutable nature of records is perfect for data projections.
src/main/java/life/mosu/mosuserver/infra/notify/dto/InquiryAnswerNotifyVariablesDto.java (1)
5-14: LGTM! Effective DTO design for notification templating.The record provides a clean abstraction for inquiry answer notification variables. The
toMap()method is well-implemented and the key mapping is consistent with the domain context.src/main/java/life/mosu/mosuserver/infra/notify/NotifyEvent.java (1)
5-15: LGTM! Clean event record design.The record effectively encapsulates notification event data with appropriate field types. The static factory method follows established patterns and maintains consistency with the codebase conventions.
src/main/java/life/mosu/mosuserver/infra/notify/NotifyEventPublisher.java (1)
7-16: LGTM! Well-structured event publisher component.The class effectively wraps Spring's ApplicationEventPublisher, providing a clean abstraction for notification event publishing. The use of dependency injection and the single responsibility principle is commendable.
src/main/java/life/mosu/mosuserver/global/exception/ErrorCode.java (1)
86-90: LGTM! Well-structured error codes for notification system.The new error codes follow the existing naming conventions and provide clear Korean messages. The HTTP status codes are appropriate for the error scenarios.
src/main/java/life/mosu/mosuserver/domain/refund/RefundJpaRepository.java (1)
8-19: Add missingschoolNameto query and verify single‐result expectationThe custom JPQL projection omits the
schoolNamefield declared inRefundNotifyProjection, and there’s no uniqueness constraint onapplicationSchoolIdinRefundJpaEntity, so this method may return multiple rows and throw an exception.• File to update:
src/main/java/life/mosu/mosuserver/domain/refund/RefundJpaRepository.java(lines 8–19)
• Projection interface exists:RefundNotifyProjectiondefinesString getSchoolName()
• Entity check:applicationSchoolIdhas nounique = trueor@UniqueConstraintApply this diff to include the missing field:
@Query(""" SELECT p.paymentKey AS paymentKey, a.examDate AS examDate, + a.schoolName AS schoolName, p.paymentMethod AS paymentMethod, r.reason AS reason FROM RefundJpaEntity r LEFT JOIN PaymentJpaEntity p ON r.applicationSchoolId = p.applicationSchoolId LEFT JOIN ApplicationSchoolJpaEntity a ON r.applicationSchoolId = a.id WHERE r.applicationSchoolId = :applicationSchoolId """) RefundNotifyProjection findRefundByApplicationSchoolId(Long applicationSchoolId);Next steps:
- Confirm in your domain that each
applicationSchoolIdcan have at most one refund.- If multiple refunds per application are possible, change the return type to
List<RefundNotifyProjection>or add a dialect-specific limit clause.src/main/java/life/mosu/mosuserver/infra/notify/strategy/InquiryAnswerNotifyStrategy.java (1)
22-26: LGTM: Clean strategy implementation with proper dependency injection.The class follows the strategy pattern correctly with appropriate Spring annotations and constructor injection.
src/main/java/life/mosu/mosuserver/application/inquiry/InquiryAnswerService.java (1)
7-9: LGTM: Proper dependency injection for notification functionality.The new dependencies are correctly added to support the notification publishing feature.
Also applies to: 12-13, 18-18, 31-33
src/main/java/life/mosu/mosuserver/infra/notify/constant/NotifyConstants.java (1)
3-23: LGTM: Well-structured constants class with proper design.The class follows Java best practices for utility classes:
- Final class with private constructor
- Constants are logically grouped (URLs vs Template IDs)
- Consistent naming conventions
- Proper public static final declarations
src/main/java/life/mosu/mosuserver/infra/notify/strategy/NotifyStrategyMap.java (1)
10-42: Excellent strategy pattern implementation with optimal performance.This implementation demonstrates several best practices:
- Performance optimization: Uses
EnumMapfor optimal enum key performance- Comprehensive coverage: Maps all notification statuses to their corresponding strategies
- Proper error handling: Throws appropriate custom exception when strategy not found
- Clean design: Constructor injection ensures all strategies are available at initialization
The strategy pattern is well-implemented and provides a clean, maintainable way to handle different notification types.
src/main/resources/messages_ko.properties (1)
1-132: LGTM: Comprehensive Korean message templates with proper formatting.The message templates are well-structured with:
- Consistent placeholder syntax: Uses
#{variable}format throughout- Proper line continuation: Backslash continuation for long messages
- Unicode encoding: Proper Korean character encoding
- Complete coverage: Templates for all notification scenarios (exam, signup, inquiry, refund)
- Channel-specific formatting: Separate templates for alimtalk and SMS where appropriate
The templates provide clear, informative content that would be helpful for users receiving notifications.
src/main/java/life/mosu/mosuserver/domain/applicationschool/ApplicationSchoolJpaRepository.java (1)
32-38: LGTM - Consistent JOIN usage.The
INNER JOINusage here is consistent with the expected behavior where payment is required for the one-week notification.src/main/java/life/mosu/mosuserver/infra/notify/strategy/RefundNotifyStrategy.java (1)
36-41: LGTM - Dual content creation is appropriate.Creating both AlimTalk and SMS content is good practice for notification fallback scenarios.
src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java (1)
47-74: LGTM - Improved code documentation.The inline comments significantly improve code readability and make the application process clearer for future maintainers.
src/main/java/life/mosu/mosuserver/presentation/notify/dto/NotifyStatus.java (1)
8-17: Well-structured enum with clear categorization.The enum is well-organized with logical grouping of SUCCESS and INFO statuses. The descriptive messages are appropriate and provide clear context for each notification type.
src/main/java/life/mosu/mosuserver/infra/notify/NotifyEventTemplate.java (1)
33-57: LGTM - Template creation methods are well-structured.The template creation methods for success and reminder notifications are clean and follow a consistent pattern. The use of builder pattern with DTOs promotes maintainability.
src/main/java/life/mosu/mosuserver/infra/notify/strategy/ApplicationNotifyStrategy.java (1)
42-45: LGTM - Button URL configuration is appropriate for application notifications.The button URLs (WARNING and MYPAGE) are appropriate for application completion notifications, providing users with relevant next steps.
| @Target(ElementType.METHOD) | ||
| @Retention(RetentionPolicy.RUNTIME) | ||
| @Async | ||
| @EventListener | ||
| public @interface ReactiveEventListener { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Annotation name & meta-configuration are misleading
The composed annotation wires @Async + @EventListener, but nothing here is “reactive” in the Project-Reactor sense. This may confuse future maintainers who’ll expect a Flux/Mono contract.
Suggestions:
-@Target(ElementType.METHOD)
-@Retention(RetentionPolicy.RUNTIME)
-@Async
-@EventListener
-public @interface ReactiveEventListener {
+@Target(ElementType.METHOD)
+@Retention(RetentionPolicy.RUNTIME)
+@Documented
+@Async
+@EventListener
+public @interface AsyncEventListener { // clearer intent
}• Add @Documented for Javadoc visibility.
• Rename to AsyncEventListener (or similar) unless you truly require Reactor types.
Remember to @EnableAsync in a configuration class; otherwise the async executor is never activated.
📝 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.
| @Target(ElementType.METHOD) | |
| @Retention(RetentionPolicy.RUNTIME) | |
| @Async | |
| @EventListener | |
| public @interface ReactiveEventListener { | |
| @Target(ElementType.METHOD) | |
| @Retention(RetentionPolicy.RUNTIME) | |
| @Documented | |
| @Async | |
| @EventListener | |
| public @interface AsyncEventListener { // clearer intent | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/global/annotation/ReactiveEventListener.java
around lines 10 to 14, the annotation name and meta-configuration are misleading
because it combines @Async and @EventListener but does not involve any
Project-Reactor reactive types like Flux or Mono. Rename the annotation to
AsyncEventListener to better reflect its behavior, add the @Documented
annotation for Javadoc visibility, and ensure that @EnableAsync is present in a
configuration class to activate the async executor.
| @Bean | ||
| public WebClient webClient() { | ||
| return WebClient.builder() | ||
| .defaultHeader("Content-Type", MediaType.APPLICATION_JSON_VALUE) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Set sensible timeouts & use header constants
A bare WebClient without timeouts risks hanging threads under network issues.
- return WebClient.builder()
- .defaultHeader("Content-Type", MediaType.APPLICATION_JSON_VALUE)
- .build();
+ return WebClient.builder()
+ .defaultHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)
+ .clientConnector(new ReactorClientHttpConnector(
+ HttpClient.create()
+ .responseTimeout(Duration.ofSeconds(5))
+ .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, 3000)
+ ))
+ .build();• Use HttpHeaders.CONTENT_TYPE constant.
• Configure connect & read timeouts (values adjustable).
This prevents resource leaks and speeds up failure recovery.
📝 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.
| @Bean | |
| public WebClient webClient() { | |
| return WebClient.builder() | |
| .defaultHeader("Content-Type", MediaType.APPLICATION_JSON_VALUE) | |
| .build(); | |
| } | |
| @Bean | |
| public WebClient webClient() { | |
| return WebClient.builder() | |
| .defaultHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) | |
| .clientConnector(new ReactorClientHttpConnector( | |
| HttpClient.create() | |
| .responseTimeout(Duration.ofSeconds(5)) | |
| .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, 3000) | |
| )) | |
| .build(); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/global/config/WebClientConfig.java around
lines 11 to 16, the WebClient bean is created without any timeout settings and
uses a hardcoded string for the Content-Type header. To fix this, replace the
hardcoded "Content-Type" string with the HttpHeaders.CONTENT_TYPE constant, and
configure sensible connect and read timeouts on the WebClient builder using
appropriate timeout values to prevent hanging threads and improve failure
handling.
| public Map<String, String> toMap() { | ||
| return Map.of( | ||
| "paymentKey", paymentKey, | ||
| "examDate", examDate.toString(), | ||
| "schoolName", schoolName, | ||
| "lunch", lunch | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider date formatting and null safety in toMap() method.
The current implementation has two potential issues:
LocalDate.toString()uses ISO format (yyyy-MM-dd) which might not be user-friendly for notificationsMap.of()will throwNullPointerExceptionif any parameter is null
Consider this safer implementation:
public Map<String, String> toMap() {
return Map.of(
- "paymentKey", paymentKey,
- "examDate", examDate.toString(),
- "schoolName", schoolName,
- "lunch", lunch
+ "paymentKey", Objects.requireNonNullElse(paymentKey, ""),
+ "examDate", examDate.format(DateTimeFormatter.ofPattern("yyyy년 MM월 dd일")),
+ "schoolName", Objects.requireNonNullElse(schoolName, ""),
+ "lunch", Objects.requireNonNullElse(lunch, "")
);
}Add these imports:
+import java.time.format.DateTimeFormatter;
+import java.util.Objects;📝 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 Map<String, String> toMap() { | |
| return Map.of( | |
| "paymentKey", paymentKey, | |
| "examDate", examDate.toString(), | |
| "schoolName", schoolName, | |
| "lunch", lunch | |
| ); | |
| } | |
| // Add at the top of the file: | |
| import java.time.format.DateTimeFormatter; | |
| import java.util.Objects; | |
| public Map<String, String> toMap() { | |
| return Map.of( | |
| "paymentKey", Objects.requireNonNullElse(paymentKey, ""), | |
| "examDate", examDate.format(DateTimeFormatter.ofPattern("yyyy년 MM월 dd일")), | |
| "schoolName", Objects.requireNonNullElse(schoolName, ""), | |
| "lunch", Objects.requireNonNullElse(lunch, "") | |
| ); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/infra/notify/dto/ApplicationNotifyVariablesDto.java
around lines 13 to 20, the toMap() method uses LocalDate.toString() which
outputs ISO format and may not be user-friendly, and Map.of() which throws
NullPointerException if any value is null. To fix this, format examDate using a
user-friendly date formatter like DateTimeFormatter.ofPattern with a suitable
pattern, and replace Map.of() with a map construction approach that handles null
values safely, such as using a HashMap and conditionally putting entries or
using Map.ofEntries with Objects.toString(value, "") to avoid nulls.
| public record NotifyEventRequest( | ||
| @JsonProperty("userId") String userId, | ||
| @JsonProperty("api_key") String apiKey, | ||
| @JsonProperty("template_id") Integer templateId, | ||
| @JsonProperty("messages") List<Map<String, Object>> messages | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding validation annotations for required fields.
The record lacks validation which could lead to runtime errors when processing notification requests.
Add validation annotations:
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.NotNull;
+import jakarta.validation.constraints.Positive;
public record NotifyEventRequest(
- @JsonProperty("userId") String userId,
- @JsonProperty("api_key") String apiKey,
- @JsonProperty("template_id") Integer templateId,
- @JsonProperty("messages") List<Map<String, Object>> messages
+ @JsonProperty("userId") @NotBlank String userId,
+ @JsonProperty("api_key") @NotBlank String apiKey,
+ @JsonProperty("template_id") @NotNull @Positive Integer templateId,
+ @JsonProperty("messages") @NotNull List<Map<String, Object>> messages
) {📝 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 record NotifyEventRequest( | |
| @JsonProperty("userId") String userId, | |
| @JsonProperty("api_key") String apiKey, | |
| @JsonProperty("template_id") Integer templateId, | |
| @JsonProperty("messages") List<Map<String, Object>> messages | |
| ) { | |
| import jakarta.validation.constraints.NotBlank; | |
| import jakarta.validation.constraints.NotNull; | |
| import jakarta.validation.constraints.Positive; | |
| public record NotifyEventRequest( | |
| @JsonProperty("userId") @NotBlank String userId, | |
| @JsonProperty("api_key") @NotBlank String apiKey, | |
| @JsonProperty("template_id") @NotNull @Positive Integer templateId, | |
| @JsonProperty("messages") @NotNull List<Map<String, Object>> messages | |
| ) { | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/presentation/notify/dto/NotifyEventRequest.java
lines 7 to 12, the record fields lack validation annotations which may cause
runtime errors. Add appropriate validation annotations such as @NotNull or
@NotEmpty to required fields like userId, apiKey, templateId, and messages to
ensure they are present and valid when processing notification requests.
| public Map<String, String> toMap() { | ||
| return Map.of( | ||
| "examDate", examDate.toString(), | ||
| "examinationNumber", examinationNumber, | ||
| "schoolName", schoolName | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply consistent date formatting and null safety (same as ApplicationNotifyVariablesDto).
This DTO has the same issues as ApplicationNotifyVariablesDto:
LocalDate.toString()uses ISO format which might not be user-friendlyMap.of()will throwNullPointerExceptionif any parameter is null
For consistency across the notification system, apply the same fix:
public Map<String, String> toMap() {
return Map.of(
- "examDate", examDate.toString(),
- "examinationNumber", examinationNumber,
- "schoolName", schoolName
+ "examDate", examDate.format(DateTimeFormatter.ofPattern("yyyy년 MM월 dd일")),
+ "examinationNumber", Objects.requireNonNullElse(examinationNumber, ""),
+ "schoolName", Objects.requireNonNullElse(schoolName, "")
);
}Add these imports:
+import java.time.format.DateTimeFormatter;
+import java.util.Objects;🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/infra/notify/dto/Exam3DayBeforeNotifyVariablesDto.java
around lines 12 to 18, replace the direct use of examDate.toString() with a
formatted date string using a DateTimeFormatter consistent with
ApplicationNotifyVariablesDto. Also, modify the toMap method to handle potential
null values safely, avoiding NullPointerException from Map.of by using a map
builder or a method that supports null values. Add necessary imports for
DateTimeFormatter and null checks as done in ApplicationNotifyVariablesDto.
src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java
Show resolved
Hide resolved
| @Override | ||
| public NotifyEventRequest create(NotifyEvent event) { | ||
| ApplicationSchoolJpaEntity applicationSchool = applicationSchoolJpaRepository.findById( | ||
| event.id()).orElseThrow( | ||
| () -> new CustomRuntimeException(ErrorCode.APPLICATION_SCHOOL_NOT_FOUND)); | ||
|
|
||
| Exam3DayBeforeNotifyVariablesDto dto = new Exam3DayBeforeNotifyVariablesDto( | ||
| applicationSchool.getExamDate(), applicationSchool.getExaminationNumber(), | ||
| applicationSchool.getSchoolName()); | ||
|
|
||
| String alimTalkContent = template.getProcessedMessage( | ||
| "notify.exam.threeday.reminder.alimtalk", | ||
| dto.toMap()); | ||
|
|
||
| String smsContent = template.getProcessedMessage("notify.exam.threeday.reminder.sms", | ||
| dto.toMap()); | ||
|
|
||
| LocalDateTime reserveTime = applicationSchool.getExamDate() | ||
| .minusDays(3) | ||
| .atTime(8, 0); | ||
|
|
||
| NotifyButtonUrls btnUrls = NotifyButtonUrls.of( | ||
| NotifyButtonUrl.of(INQUIRY, INQUIRY), | ||
| NotifyButtonUrl.of(MYPAGE, MYPAGE) | ||
| ); | ||
|
|
||
| NotifyEventRemindMessageDto eventMessage = NotifyEventRemindMessageDto.create(1, | ||
| event.telNum(), reserveTime, | ||
| alimTalkContent, | ||
| smsContent, btnUrls); | ||
|
|
||
| return template.getNotifyEventRemindTemplate(EXAM_3DAY_BEFORE_TEMPLATE_ID, eventMessage); | ||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify exception handling and consider extracting common notification logic.
The strategy follows a consistent pattern but has some areas for improvement:
-
Exception handling: The AI summary mentions
NOTICE_NOT_FOUNDerror code, but the code usesAPPLICATION_SCHOOL_NOT_FOUND. Please verify this is the correct error code. -
Code duplication: This strategy shares significant structure with other exam reminder strategies. Consider extracting common logic into a base class or utility method.
-
Configuration: The hardcoded scheduling time (8:00 AM) should be configurable.
Let me verify the correct error code usage:
🏁 Script executed:
#!/bin/bash
# Description: Check for the correct error code usage across notification strategies
# Search for NOTICE_NOT_FOUND error code usage
rg -A 2 -B 2 "NOTICE_NOT_FOUND"
# Search for APPLICATION_SCHOOL_NOT_FOUND error code usage in notify strategies
rg -A 2 -B 2 "APPLICATION_SCHOOL_NOT_FOUND" --type java --glob "**/notify/strategy/**"Length of output: 2676
Refactor common notification logic & externalize scheduling time
The error code usage (APPLICATION_SCHOOL_NOT_FOUND) is correct—no change needed. However, the following improvements are still recommended:
- Extract shared behavior from each
*DayBeforeNotifyStrategyinto a common base class or utility:
• Exam1DayBeforeNotifyStrategy.java
• Exam3DayBeforeNotifyStrategy.java
(and any other “XDayBeforeNotifyStrategy” implementations) - Make the hard-coded reminder time (
.minusDays(3).atTime(8, 0)) configurable rather than fixed in code.
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam3DayBeforeNotifyStrategy.java
lines 29 to 63, refactor by extracting the common notification creation logic
shared with other *DayBeforeNotifyStrategy classes into a common base class or
utility method to avoid duplication. Additionally, replace the hard-coded
reminder time calculation (.minusDays(3).atTime(8, 0)) with a configurable
parameter, such as reading the number of days before and reminder time from
configuration or constructor parameters, to make the scheduling time flexible
and maintainable.
| public NotifyEventRequest create(NotifyEvent event) { | ||
| ApplicationSchoolJpaEntity applicationSchool = applicationSchoolJpaRepository.findById( | ||
| event.id()).orElseThrow( | ||
| () -> new CustomRuntimeException(ErrorCode.APPLICATION_SCHOOL_NOT_FOUND)); | ||
|
|
||
| Exam1DayBeforeNotifyVariablesDto dto = new Exam1DayBeforeNotifyVariablesDto( | ||
| applicationSchool.getExamDate(), applicationSchool.getExaminationNumber(), | ||
| applicationSchool.getSchoolName()); | ||
|
|
||
| String alimTalkContent = template.getProcessedMessage( | ||
| "notify.exam.oneday.reminder.alimtalk", | ||
| dto.toMap()); | ||
|
|
||
| String smsContent = template.getProcessedMessage("notify.exam.oneday.reminder.sms", | ||
| dto.toMap()); | ||
|
|
||
| LocalDateTime reserveTime = applicationSchool.getExamDate() | ||
| .minusDays(1) | ||
| .atTime(8, 0); | ||
|
|
||
| NotifyButtonUrls btnUrls = NotifyButtonUrls.of( | ||
| NotifyButtonUrl.of(INQUIRY, INQUIRY), | ||
| NotifyButtonUrl.of(MYPAGE, MYPAGE) | ||
| ); | ||
|
|
||
| NotifyEventRemindMessageDto eventMessage = NotifyEventRemindMessageDto.create(1, | ||
| event.telNum(), reserveTime, | ||
| alimTalkContent, | ||
| smsContent, btnUrls); | ||
|
|
||
| return template.getNotifyEventRemindTemplate(EXAM_1DAY_BEFORE_TEMPLATE_ID, eventMessage); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract common exam reminder logic to reduce code duplication.
This strategy is nearly identical to Exam3DayBeforeNotifyStrategy except for the number of days (1 vs 3) and message template keys. Consider extracting common logic into a base class or utility method.
Example approach:
+public abstract class BaseExamReminderStrategy implements NotifyStrategy {
+ protected final NotifyEventTemplate template;
+ protected final ApplicationSchoolJpaRepository applicationSchoolJpaRepository;
+
+ protected NotifyEventRequest createExamReminder(NotifyEvent event, int daysBefore,
+ String alimTalkTemplate, String smsTemplate, Integer templateId) {
+ // Common logic here
+ }
+}
-@Component
-@RequiredArgsConstructor
-public class Exam1DayBeforeNotifyStrategy implements NotifyStrategy {
+public class Exam1DayBeforeNotifyStrategy extends BaseExamReminderStrategy {
+ @Override
+ public NotifyEventRequest create(NotifyEvent event) {
+ return createExamReminder(event, 1,
+ "notify.exam.oneday.reminder.alimtalk",
+ "notify.exam.oneday.reminder.sms",
+ EXAM_1DAY_BEFORE_TEMPLATE_ID);
+ }
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam1DayBeforeNotifyStrategy.java
lines 30 to 61, the create method duplicates logic found in
Exam3DayBeforeNotifyStrategy with only differences in days offset and template
keys. Refactor by extracting the shared logic into a base class or utility
method that accepts parameters for the number of days before the exam and the
template keys. Then have Exam1DayBeforeNotifyStrategy and
Exam3DayBeforeNotifyStrategy call this common method with their specific values
to reduce code duplication and improve maintainability.
| public NotifyEventRequest create(NotifyEvent event) { | ||
| ApplicationSchoolNotifyProjection projection = applicationSchoolJpaRepository.findPaymentByApplicationSchoolId( | ||
| event.id()); | ||
|
|
||
| ApplicationNotifyVariablesDto dto = new ApplicationNotifyVariablesDto( | ||
| projection.paymentKey(), projection.examDate(), projection.schoolName(), | ||
| projection.lunch().getLunchName()); | ||
|
|
||
| String alimTalkContent = template.getProcessedMessage( | ||
| "notify.exam.application.complete.alimtalk", | ||
| dto.toMap()); | ||
|
|
||
| String smsContent = template.getProcessedMessage("notify.exam.application.complete.sms", | ||
| dto.toMap()); | ||
|
|
||
| NotifyButtonUrls btnUrls = NotifyButtonUrls.of( | ||
| NotifyButtonUrl.of(WARNING, WARNING), | ||
| NotifyButtonUrl.of(MYPAGE, MYPAGE) | ||
| ); | ||
|
|
||
| NotifyEventSuccessMessageDto eventMessage = NotifyEventSuccessMessageDto.create(1, | ||
| event.telNum(), alimTalkContent, | ||
| smsContent, "1", btnUrls); | ||
| return template.getNotifyEventSuccessTemplate(APPLICATION_TEMPLATE_ID, eventMessage); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add null safety for repository projection and verify projection method behavior.
The strategy uses a repository projection method that may return null if the application school is not found. Unlike the exam reminder strategies, this doesn't use orElseThrow() for null handling.
Let me verify the projection method behavior:
🏁 Script executed:
#!/bin/bash
# Description: Check how the projection method handles missing data
# Search for the projection method implementation
ast-grep --pattern 'findPaymentByApplicationSchoolId($_) {
$$$
}'
# Search for other projection method usages to see error handling patterns
rg -A 5 -B 2 "findPaymentByApplicationSchoolId" --type javaLength of output: 2414
Ensure projection result is non-null before use
The JPA query method
ApplicationSchoolNotifyProjection findPaymentByApplicationSchoolId(Long applicationSchoolId);returns null if no record is found. In create(...), dereferencing a null projection will cause an NPE. To match other strategies’ error handling, add a null check or throw immediately after fetching the projection:
• File: src/main/java/life/mosu/mosuserver/infra/notify/strategy/ApplicationNotifyStrategy.java
ApplicationSchoolNotifyProjection projection =
- applicationSchoolJpaRepository.findPaymentByApplicationSchoolId(event.id());
+ applicationSchoolJpaRepository.findPaymentByApplicationSchoolId(event.id());
+ Objects.requireNonNull(projection,
+ "No application school found for id " + event.id());Alternatively, update the repository signature to return
Optional<ApplicationSchoolNotifyProjection>
findPaymentByApplicationSchoolId(Long id);and then use .orElseThrow(...) here.
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/infra/notify/strategy/ApplicationNotifyStrategy.java
between lines 27 and 51, the projection fetched by
findPaymentByApplicationSchoolId can be null, leading to a NullPointerException
when accessed. Fix this by adding a null check after fetching the projection and
throw an appropriate exception if it is null, or alternatively, modify the
repository method to return an Optional and use orElseThrow here to handle the
absence of data safely.
| public NotifyEventRequest create(NotifyEvent event) { | ||
| OneWeekNotifyProjection projection = applicationSchoolJpaRepository.findOneWeekNotifyByApplicationSchoolId( | ||
| event.id()); | ||
|
|
||
| Exam1WeekBeforeNotifyVariablesDto dto = new Exam1WeekBeforeNotifyVariablesDto( | ||
| projection.examDate(), projection.paymentKey(), projection.schoolName()); | ||
|
|
||
| String alimTalkContent = template.getProcessedMessage( | ||
| "notify.exam.oneweek.reminder.alimtalk", | ||
| dto.toMap()); | ||
|
|
||
| String smsContent = template.getProcessedMessage("notify.exam.oneweek.reminder.sms", | ||
| dto.toMap()); | ||
|
|
||
| LocalDateTime reserveTime = projection.examDate() | ||
| .minusDays(7) | ||
| .atTime(8, 0); | ||
|
|
||
| NotifyButtonUrls btnUrls = NotifyButtonUrls.of( | ||
| NotifyButtonUrl.of(INQUIRY, INQUIRY) | ||
| ); | ||
|
|
||
| NotifyEventRemindMessageDto eventMessage = NotifyEventRemindMessageDto.create(1, | ||
| event.telNum(), reserveTime, | ||
| alimTalkContent, | ||
| smsContent, btnUrls); | ||
|
|
||
| return template.getNotifyEventRemindTemplate(EXAM_1WEEK_BEFORE_TEMPLATE_ID, eventMessage); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null safety for projection result and ensure consistency with other strategies.
This strategy uses a projection method that may return null, but doesn't include null checking like the other exam reminder strategies. Additionally, it only includes one button URL while others include two.
OneWeekNotifyProjection projection = applicationSchoolJpaRepository.findOneWeekNotifyByApplicationSchoolId(
event.id());
+if (projection == null) {
+ throw new CustomRuntimeException(ErrorCode.APPLICATION_SCHOOL_NOT_FOUND);
+}Consider adding the MYPAGE button for consistency with other exam reminders:
NotifyButtonUrls btnUrls = NotifyButtonUrls.of(
- NotifyButtonUrl.of(INQUIRY, INQUIRY)
+ NotifyButtonUrl.of(INQUIRY, INQUIRY),
+ NotifyButtonUrl.of(MYPAGE, MYPAGE)
);📝 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 NotifyEventRequest create(NotifyEvent event) { | |
| OneWeekNotifyProjection projection = applicationSchoolJpaRepository.findOneWeekNotifyByApplicationSchoolId( | |
| event.id()); | |
| Exam1WeekBeforeNotifyVariablesDto dto = new Exam1WeekBeforeNotifyVariablesDto( | |
| projection.examDate(), projection.paymentKey(), projection.schoolName()); | |
| String alimTalkContent = template.getProcessedMessage( | |
| "notify.exam.oneweek.reminder.alimtalk", | |
| dto.toMap()); | |
| String smsContent = template.getProcessedMessage("notify.exam.oneweek.reminder.sms", | |
| dto.toMap()); | |
| LocalDateTime reserveTime = projection.examDate() | |
| .minusDays(7) | |
| .atTime(8, 0); | |
| NotifyButtonUrls btnUrls = NotifyButtonUrls.of( | |
| NotifyButtonUrl.of(INQUIRY, INQUIRY) | |
| ); | |
| NotifyEventRemindMessageDto eventMessage = NotifyEventRemindMessageDto.create(1, | |
| event.telNum(), reserveTime, | |
| alimTalkContent, | |
| smsContent, btnUrls); | |
| return template.getNotifyEventRemindTemplate(EXAM_1WEEK_BEFORE_TEMPLATE_ID, eventMessage); | |
| } | |
| public NotifyEventRequest create(NotifyEvent event) { | |
| OneWeekNotifyProjection projection = applicationSchoolJpaRepository.findOneWeekNotifyByApplicationSchoolId( | |
| event.id()); | |
| if (projection == null) { | |
| throw new CustomRuntimeException(ErrorCode.APPLICATION_SCHOOL_NOT_FOUND); | |
| } | |
| Exam1WeekBeforeNotifyVariablesDto dto = new Exam1WeekBeforeNotifyVariablesDto( | |
| projection.examDate(), projection.paymentKey(), projection.schoolName()); | |
| String alimTalkContent = template.getProcessedMessage( | |
| "notify.exam.oneweek.reminder.alimtalk", | |
| dto.toMap()); | |
| String smsContent = template.getProcessedMessage("notify.exam.oneweek.reminder.sms", | |
| dto.toMap()); | |
| LocalDateTime reserveTime = projection.examDate() | |
| .minusDays(7) | |
| .atTime(8, 0); | |
| NotifyButtonUrls btnUrls = NotifyButtonUrls.of( | |
| NotifyButtonUrl.of(INQUIRY, INQUIRY), | |
| NotifyButtonUrl.of(MYPAGE, MYPAGE) | |
| ); | |
| NotifyEventRemindMessageDto eventMessage = NotifyEventRemindMessageDto.create(1, | |
| event.telNum(), reserveTime, | |
| alimTalkContent, | |
| smsContent, btnUrls); | |
| return template.getNotifyEventRemindTemplate(EXAM_1WEEK_BEFORE_TEMPLATE_ID, eventMessage); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam1WeekBeforeNotifyStrategy.java
between lines 28 and 56, add a null check after retrieving the projection from
applicationSchoolJpaRepository to handle cases where it may return null,
returning null or an appropriate fallback if so. Also, update the
NotifyButtonUrls to include both the INQUIRY and MYPAGE buttons to maintain
consistency with other exam reminder strategies.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (4)
src/main/java/life/mosu/mosuserver/infra/notify/strategy/SignUpNotifyStrategy.java (1)
27-28: Use template generator for consistency with other strategies.This strategy uses
MessageSourcedirectly while other strategies useNotifyEventTemplateGenerator. For consistency across the codebase, consider using the template generator pattern.- String alimTalkContent = messageSource.getMessage("notify.signup.complete.alimtalk", null, - Locale.KOREA); + String alimTalkContent = template.getProcessedMessage("notify.signup.complete.alimtalk", + java.util.Collections.emptyMap());src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam1DayBeforeNotifyStrategy.java (2)
31-37: Create a projection to fetch only required fieldsFetching the entire entity when only
examDate,examinationNumber, andschoolNameare needed is inefficient.
22-62: Extract common exam reminder logic to reduce duplicationThis strategy shares significant logic with
Exam3DayBeforeNotifyStrategyand likelyExam1WeekBeforeNotifyStrategy. The only differences are the number of days and template keys.src/main/java/life/mosu/mosuserver/infra/notify/strategy/ApplicationNotifyStrategy.java (1)
28-33: Add null check for projection to prevent NPEThe repository method
findPaymentByApplicationSchoolIdcan return null if no record is found. Dereferencing it without a null check will cause a NullPointerException.
🧹 Nitpick comments (3)
src/main/java/life/mosu/mosuserver/infra/notify/NotifyProperties.java (1)
6-11: Consider using @ConfigurationProperties for better type safety and validation.The current approach using
@Valueannotations works but has limitations. Consider using Spring Boot's@ConfigurationPropertiesfor better type safety, validation, and IDE support.+import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.validation.annotation.Validated; +import jakarta.validation.constraints.NotBlank; -@Component -public class NotifyProperties { - @Value("${alimtalk.api.base-url}") String alimTalkUrl; - @Value("${alimtalk.user-id}") String userId; - @Value("${alimtalk.api-key}") String apiKey; -} +@ConfigurationProperties(prefix = "alimtalk") +@Validated +@Component +public class NotifyProperties { + @NotBlank + private String apiBaseUrl; + @NotBlank + private String userId; + @NotBlank + private String apiKey; + + // Getters and setters + public String getApiBaseUrl() { return apiBaseUrl; } + public void setApiBaseUrl(String apiBaseUrl) { this.apiBaseUrl = apiBaseUrl; } + public String getUserId() { return userId; } + public void setUserId(String userId) { this.userId = userId; } + public String getApiKey() { return apiKey; } + public void setApiKey(String apiKey) { this.apiKey = apiKey; } +}src/main/java/life/mosu/mosuserver/infra/notify/LunaSoftNotifier.java (1)
32-42: Consider adding timeout and error handling improvements.The reactive chain looks good but could benefit from timeout configuration and better error handling for production use.
webClient.post() .uri(properties.alimTalkUrl) .bodyValue(lunaRequest) .retrieve() .bodyToMono(String.class) + .timeout(Duration.ofSeconds(30)) .publishOn(Schedulers.boundedElastic()) .doOnSuccess(response -> log.info("알림톡 응답: {}", response)) .doOnError(error -> log.error("알림톡 전송 실패", error)) .doOnTerminate(() -> log.info("알림톡 전송 완료: {}", request)) + .onErrorComplete() .subscribe();The
onErrorComplete()ensures that errors don't propagate and crash the application, andtimeout()prevents hanging requests.src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam1DayBeforeNotifyStrategy.java (1)
46-48: Consider making notification time configurableThe notification time is hardcoded to 8:00 AM. This might need to be configurable based on business requirements or user preferences.
+@Value("${notify.exam.reminder.hour:8}") +private int reminderHour; + +@Value("${notify.exam.reminder.minute:0}") +private int reminderMinute; LocalDateTime reserveTime = applicationSchool.getExamDate() .minusDays(1) - .atTime(8, 0); + .atTime(reminderHour, reminderMinute);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
src/main/java/life/mosu/mosuserver/application/inquiry/InquiryAnswerService.java(3 hunks)src/main/java/life/mosu/mosuserver/application/notify/NotifyService.java(1 hunks)src/main/java/life/mosu/mosuserver/global/initializer/DatabaseInitializer.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/LunaSoftNotifier.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/NotifyClientAdapter.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/NotifyEventPublisher.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/NotifyEventTemplateGenerator.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/NotifyProperties.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/constant/NotifyConstants.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/dto/EventStatus.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyButtonUrl.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyButtonUrls.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyEvent.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyEventRemindMessageDto.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyEventRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyEventSuccessMessageDto.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyStatus.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/strategy/ApplicationNotifyStrategy.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam1DayBeforeNotifyStrategy.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam1WeekBeforeNotifyStrategy.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam3DayBeforeNotifyStrategy.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/strategy/InquiryAnswerNotifyStrategy.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/strategy/NotifyStrategy.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/strategy/RefundNotifyStrategy.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/notify/strategy/SignUpNotifyStrategy.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/notify/NotifyEventListener.java(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- src/main/java/life/mosu/mosuserver/infra/notify/dto/EventStatus.java
- src/main/java/life/mosu/mosuserver/infra/notify/NotifyClientAdapter.java
- src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyEventRequest.java
- src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyButtonUrls.java
- src/main/java/life/mosu/mosuserver/presentation/notify/NotifyEventListener.java
- src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyButtonUrl.java
- src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyEvent.java
- src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyStatus.java
🚧 Files skipped from review as they are similar to previous changes (10)
- src/main/java/life/mosu/mosuserver/infra/notify/NotifyEventPublisher.java
- src/main/java/life/mosu/mosuserver/infra/notify/strategy/NotifyStrategy.java
- src/main/java/life/mosu/mosuserver/application/inquiry/InquiryAnswerService.java
- src/main/java/life/mosu/mosuserver/infra/notify/strategy/InquiryAnswerNotifyStrategy.java
- src/main/java/life/mosu/mosuserver/infra/notify/constant/NotifyConstants.java
- src/main/java/life/mosu/mosuserver/infra/notify/strategy/RefundNotifyStrategy.java
- src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam3DayBeforeNotifyStrategy.java
- src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyEventSuccessMessageDto.java
- src/main/java/life/mosu/mosuserver/infra/notify/strategy/Exam1WeekBeforeNotifyStrategy.java
- src/main/java/life/mosu/mosuserver/infra/notify/dto/NotifyEventRemindMessageDto.java
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/life/mosu/mosuserver/infra/notify/LunaSoftNotifier.java (1)
src/main/java/life/mosu/mosuserver/infra/notify/NotifyProperties.java (1)
Component(6-11)
src/main/java/life/mosu/mosuserver/infra/notify/NotifyProperties.java (1)
src/main/java/life/mosu/mosuserver/infra/notify/LunaSoftNotifier.java (1)
Component(13-65)
🔇 Additional comments (4)
src/main/java/life/mosu/mosuserver/global/initializer/DatabaseInitializer.java (1)
110-110: LGTM! Fixed phone number for notification testing support.The change to use a fixed phone number instead of dynamically generated ones makes sense for testing the new notification system infrastructure. This ensures consistent test data for the 알림톡 feature implementation.
src/main/java/life/mosu/mosuserver/infra/notify/LunaSoftNotifier.java (1)
53-64: LGTM! Well-structured record with proper JSON mapping.The
LunaNotifyRequestrecord is well-designed with proper JSON property mapping and a clean factory method.src/main/java/life/mosu/mosuserver/infra/notify/strategy/SignUpNotifyStrategy.java (1)
34-38: LGTM! Clean implementation of the notification strategy.The strategy implementation follows the pattern correctly and properly constructs the notification request with button URLs and message content.
src/main/java/life/mosu/mosuserver/infra/notify/strategy/ApplicationNotifyStrategy.java (1)
42-45: Button URLs are correctly configured with actual URL constants
TheWARNING,MYPAGE,HOMEPAGE, andINQUIRYconstants are defined in NotifyConstants.java as full URLs (e.g.,"https://www.mosuedu.com/warning"). The callNotifyButtonUrl.of(WARNING, WARNING)passes the same URL for both PC and mobile views, which is intentional and valid. You can ignore the earlier concern about using labels in place of URLs.
Likely an incorrect or invalid review comment.
| @Value("${alimtalk.api.base-url}") String alimTalkUrl; | ||
| @Value("${alimtalk.user-id}") String userId; | ||
| @Value("${alimtalk.api-key}") String apiKey; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make fields private and add proper encapsulation.
The fields should be private with getter methods to follow proper encapsulation principles.
- @Value("${alimtalk.api.base-url}") String alimTalkUrl;
- @Value("${alimtalk.user-id}") String userId;
- @Value("${alimtalk.api-key}") String apiKey;
+ @Value("${alimtalk.api.base-url}") private String alimTalkUrl;
+ @Value("${alimtalk.user-id}") private String userId;
+ @Value("${alimtalk.api-key}") private String apiKey;
+
+ public String getAlimTalkUrl() { return alimTalkUrl; }
+ public String getUserId() { return userId; }
+ public String getApiKey() { return apiKey; }📝 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.
| @Value("${alimtalk.api.base-url}") String alimTalkUrl; | |
| @Value("${alimtalk.user-id}") String userId; | |
| @Value("${alimtalk.api-key}") String apiKey; | |
| @Value("${alimtalk.api.base-url}") private String alimTalkUrl; | |
| @Value("${alimtalk.user-id}") private String userId; | |
| @Value("${alimtalk.api-key}") private String apiKey; | |
| public String getAlimTalkUrl() { | |
| return alimTalkUrl; | |
| } | |
| public String getUserId() { | |
| return userId; | |
| } | |
| public String getApiKey() { | |
| return apiKey; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/infra/notify/NotifyProperties.java lines 8
to 10, the fields alimTalkUrl, userId, and apiKey are package-private but should
be made private to enforce encapsulation. Change the field access modifiers to
private and add public getter methods for each field to allow controlled access
while keeping the fields encapsulated.
|
|
||
| ProfileJpaEntity profile = profileJpaRepository.findByUserId(event.targetId()) | ||
| .orElseThrow(() -> new CustomRuntimeException(ErrorCode.PROFILE_NOT_FOUND)); | ||
| String parsedPhoneNumber = profile.getPhoneNumber().replaceAll("-", ""); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for phone number processing.
The current implementation assumes the phone number always contains hyphens and doesn't validate the format. Consider adding validation.
- String parsedPhoneNumber = profile.getPhoneNumber().replaceAll("-", "");
+ String phoneNumber = profile.getPhoneNumber();
+ if (phoneNumber == null || phoneNumber.trim().isEmpty()) {
+ throw new CustomRuntimeException(ErrorCode.INVALID_PHONE_NUMBER);
+ }
+ String parsedPhoneNumber = phoneNumber.replaceAll("-", "");📝 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.
| String parsedPhoneNumber = profile.getPhoneNumber().replaceAll("-", ""); | |
| String phoneNumber = profile.getPhoneNumber(); | |
| if (phoneNumber == null || phoneNumber.trim().isEmpty()) { | |
| throw new CustomRuntimeException(ErrorCode.INVALID_PHONE_NUMBER); | |
| } | |
| String parsedPhoneNumber = phoneNumber.replaceAll("-", ""); |
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/application/notify/NotifyService.java at
line 30, the code removes hyphens from the phone number without validating its
format. Add validation before processing the phone number to ensure it matches
the expected pattern (e.g., digits with optional hyphens). If the format is
invalid, handle it appropriately, such as throwing an exception or returning an
error, to prevent processing incorrect phone numbers.
| private NotifyStrategy getNotifyStrategy(String strategyName) { | ||
| return (NotifyStrategy) applicationContext.getBean(strategyName); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for missing strategy beans.
The current implementation will throw a NoSuchBeanDefinitionException if the strategy bean is not found. Consider adding proper error handling.
private NotifyStrategy getNotifyStrategy(String strategyName) {
- return (NotifyStrategy) applicationContext.getBean(strategyName);
+ try {
+ return (NotifyStrategy) applicationContext.getBean(strategyName);
+ } catch (NoSuchBeanDefinitionException e) {
+ throw new CustomRuntimeException(ErrorCode.STRATEGY_NOT_FOUND);
+ }
}📝 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 NotifyStrategy getNotifyStrategy(String strategyName) { | |
| return (NotifyStrategy) applicationContext.getBean(strategyName); | |
| } | |
| private NotifyStrategy getNotifyStrategy(String strategyName) { | |
| try { | |
| return (NotifyStrategy) applicationContext.getBean(strategyName); | |
| } catch (NoSuchBeanDefinitionException e) { | |
| throw new CustomRuntimeException(ErrorCode.STRATEGY_NOT_FOUND); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/application/notify/NotifyService.java
around lines 37 to 39, the method getNotifyStrategy currently retrieves a bean
by name without handling the case where the bean does not exist, which throws
NoSuchBeanDefinitionException. Modify the method to catch this exception and
handle it gracefully, such as by logging an error and returning null or throwing
a custom exception with a clear message, to improve robustness.
| public NotifyEventRequest getNotifyEventSuccessTemplate( | ||
| Integer templateId, | ||
| NotifyEventSuccessMessageDto message | ||
| ) { | ||
|
|
||
| return new NotifyEventRequest( | ||
| templateId, | ||
| List.of(message.toMap()) | ||
| ); | ||
| } | ||
|
|
||
| public NotifyEventRequest getNotifyEventRemindTemplate( | ||
| Integer templateId, | ||
| NotifyEventRemindMessageDto message | ||
| ) { | ||
|
|
||
| return new NotifyEventRequest( | ||
| templateId, | ||
| List.of(message.toMap()) | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor duplicate code in template methods
Both methods have identical logic except for the parameter type. Consider using a common interface or generic method.
+public interface NotifyMessage {
+ Map<String, Object> toMap();
+}
-public NotifyEventRequest getNotifyEventSuccessTemplate(
- Integer templateId,
- NotifyEventSuccessMessageDto message
-) {
-
- return new NotifyEventRequest(
- templateId,
- List.of(message.toMap())
- );
-}
-
-public NotifyEventRequest getNotifyEventRemindTemplate(
- Integer templateId,
- NotifyEventRemindMessageDto message
-) {
-
- return new NotifyEventRequest(
- templateId,
- List.of(message.toMap())
- );
-}
+public <T extends NotifyMessage> NotifyEventRequest getNotifyEventTemplate(
+ Integer templateId,
+ T message
+) {
+ if (templateId == null || message == null) {
+ throw new IllegalArgumentException("Template ID and message must not be null");
+ }
+
+ return new NotifyEventRequest(
+ templateId,
+ List.of(message.toMap())
+ );
+}Then update NotifyEventSuccessMessageDto and NotifyEventRemindMessageDto to implement the NotifyMessage interface.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/infra/notify/NotifyEventTemplateGenerator.java
between lines 28 and 48, both getNotifyEventSuccessTemplate and
getNotifyEventRemindTemplate methods contain duplicate code differing only by
parameter type. To fix this, create a common interface NotifyMessage with a
toMap method, have NotifyEventSuccessMessageDto and NotifyEventRemindMessageDto
implement this interface, then refactor these methods into a single generic
method that accepts a NotifyMessage parameter and returns the NotifyEventRequest
accordingly.
| public String getProcessedMessage(String code, Map<String, String> variables) { | ||
| String message = messageSource.getMessage(code, null, Locale.KOREA); | ||
| for (Map.Entry<String, String> entry : variables.entrySet()) { | ||
| message = StringUtils.replace(message, "#{" + entry.getKey() + "}", entry.getValue()); | ||
| } | ||
| return message; | ||
| } |
There was a problem hiding this comment.
Add error handling and make locale configurable
The method has several issues:
- No error handling if the message code doesn't exist
- Hardcoded
Locale.KOREAlimits internationalization - Multiple string replacements in a loop can be inefficient
-public String getProcessedMessage(String code, Map<String, String> variables) {
- String message = messageSource.getMessage(code, null, Locale.KOREA);
- for (Map.Entry<String, String> entry : variables.entrySet()) {
- message = StringUtils.replace(message, "#{" + entry.getKey() + "}", entry.getValue());
- }
- return message;
+public String getProcessedMessage(String code, Map<String, String> variables, Locale locale) {
+ if (code == null || variables == null) {
+ throw new IllegalArgumentException("Code and variables must not be null");
+ }
+
+ String message;
+ try {
+ message = messageSource.getMessage(code, null, locale != null ? locale : Locale.KOREA);
+ } catch (NoSuchMessageException e) {
+ throw new CustomRuntimeException(ErrorCode.MESSAGE_NOT_FOUND, "Message not found for code: " + code);
+ }
+
+ // Use StringBuilder for better performance with multiple replacements
+ StringBuilder result = new StringBuilder(message);
+ for (Map.Entry<String, String> entry : variables.entrySet()) {
+ String placeholder = "#{" + entry.getKey() + "}";
+ String value = entry.getValue() != null ? entry.getValue() : "";
+ int index = 0;
+ while ((index = result.indexOf(placeholder, index)) != -1) {
+ result.replace(index, index + placeholder.length(), value);
+ index += value.length();
+ }
+ }
+ return result.toString();
+}You'll also need to update the calling code to pass the locale parameter.
📝 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 String getProcessedMessage(String code, Map<String, String> variables) { | |
| String message = messageSource.getMessage(code, null, Locale.KOREA); | |
| for (Map.Entry<String, String> entry : variables.entrySet()) { | |
| message = StringUtils.replace(message, "#{" + entry.getKey() + "}", entry.getValue()); | |
| } | |
| return message; | |
| } | |
| public String getProcessedMessage(String code, Map<String, String> variables, Locale locale) { | |
| if (code == null || variables == null) { | |
| throw new IllegalArgumentException("Code and variables must not be null"); | |
| } | |
| String message; | |
| try { | |
| message = messageSource.getMessage(code, null, locale != null ? locale : Locale.KOREA); | |
| } catch (NoSuchMessageException e) { | |
| throw new CustomRuntimeException(ErrorCode.MESSAGE_NOT_FOUND, "Message not found for code: " + code); | |
| } | |
| // Use StringBuilder for better performance with multiple replacements | |
| StringBuilder result = new StringBuilder(message); | |
| for (Map.Entry<String, String> entry : variables.entrySet()) { | |
| String placeholder = "#{" + entry.getKey() + "}"; | |
| String value = entry.getValue() != null ? entry.getValue() : ""; | |
| int index = 0; | |
| while ((index = result.indexOf(placeholder, index)) != -1) { | |
| result.replace(index, index + placeholder.length(), value); | |
| index += value.length(); | |
| } | |
| } | |
| return result.toString(); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/infra/notify/NotifyEventTemplateGenerator.java
around lines 20 to 26, the getProcessedMessage method lacks error handling for
missing message codes, uses a hardcoded Locale.KOREA, and performs inefficient
string replacements in a loop. To fix this, add a Locale parameter to the method
to make it configurable, handle exceptions or check for missing messages when
retrieving the message from messageSource, and optimize the replacement logic by
using a more efficient approach such as a single pass replacement or a
StringBuilder. Also, update all calling code to pass the appropriate Locale
argument.
wlgns12370
left a comment
wlgns12370
left a comment
There was a problem hiding this comment.
알림 디자인 패턴 매우 만족스럽네요! 메시지는 인코딩해서 저장해두는 것까지 좋습니다!
| admissionTicketImageJpaRepository.save( | ||
| createAdmissionTicketImageIfPresent(request.admissionTicket(), applicationId)); | ||
|
|
||
| for (ApplicationSchoolRequest schoolRequest : schoolRequests) { | ||
| //해당 학교가 존재하는 학교인지 | ||
| Long schoolId = schoolJpaRepository.findBySchoolNameAndAreaAndExamDate( | ||
| schoolRequest.schoolName(), | ||
| schoolRequest.validatedArea(schoolRequest.area()), | ||
| schoolRequest.examDate()) | ||
| .orElseThrow(() -> new CustomRuntimeException(ErrorCode.SCHOOL_NOT_FOUND)) | ||
| .getId(); | ||
|
|
||
| //해당 학교를 이미 신청하였는지 | ||
| if (applicationSchoolJpaRepository.existsByUserIdAndSchoolId(userId, schoolId)) { | ||
| throw new CustomRuntimeException(ErrorCode.APPLICATION_SCHOOL_ALREADY_APPLIED); | ||
| } | ||
|
|
||
| //해당 학교를 찾을 수 있는지 | ||
| SchoolJpaEntity school = schoolJpaRepository.findById(schoolId) | ||
| .orElseThrow(() -> new CustomRuntimeException(ErrorCode.SCHOOL_NOT_FOUND)); |
There was a problem hiding this comment.
학교 존재 확인과 기존의 신청이 있는지 검증을 하고 수험표 저장 로직을 수행해야 할 것 같은데 혹시 다른 의도가 있는 건가요?
There was a problem hiding this comment.
수정 중에 있습니다! 확인했어요
3 participants
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation