feat: slack 자동화 메세지 기능 개발 #190
Conversation
|
Warning Rate limit exceeded@capDoYeonLee has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 32 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes involve modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ScheduleController
participant SlackService
participant ScheduleService
participant ScheduleJpaRepository
User->>ScheduleController: Call slackTest()
ScheduleController->>ScheduleService: makeSlackMessage()
ScheduleService->>ScheduleJpaRepository: findWeekPublic(startDate, endDate)
ScheduleJpaRepository-->>ScheduleService: List<ScheduleEntity>
ScheduleService-->>ScheduleController: Formatted message
ScheduleController->>SlackService: sendSlackMessage(message, "test")
SlackService-->>User: Message sent to Slack
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
capDoYeonLee
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (5)
BE/error/src/main/java/com/example/demo/schedule/infrastructure/SlackService.java (2)
55-72: Consider pagination for large schedule listsThe
makeSlackMessagemethod fetches all schedules without pagination, which could be problematic with large datasets.Consider:
- Implementing pagination in the
findWeekendSchedulemethod- Adding a limit to the number of schedules included in a single message
- Breaking large messages into multiple smaller ones to comply with Slack's message size limits
74-84: Enhance schedule formatting for better readabilityThe current formatting is basic and could be enhanced for better readability in Slack.
Consider:
- Adding emoji icons for visual appeal
- Including duration if available
- Using Slack's block kit for better formatting
public String formatSchedule(ScheduleEntity schedule) { LocalDateTime startDate = schedule.getEventStartDate(); String formattedDate = startDate.format(DateTimeFormatter.ofPattern("M월 d일 (E) HH:mm", Locale.KOREAN)); - return String.format("(%s)\n- 일시: %s\n- 장소: %s", + return String.format("📅 *%s*\n⏰ 일시: %s\n📍 장소: %s", schedule.getEventName(), formattedDate, schedule.getEventPlace()); }BE/error/src/main/java/com/example/demo/schedule/application/service/ScheduleService.java (1)
77-83: Remove commented codeThe commented code should be removed as it's no longer needed and adds noise to the codebase.
- //List<WeekendSchedule> schedules = new ArrayList<>(); - -// List<ScheduleEntity> test = scheduleJpaRepository.findWeekendPublicSchedule(); -// if (test.size() == 0) { -// System.out.println("empty"); -// } -// return test;BE/error/src/main/java/com/example/demo/schedule/infrastructure/persistence/ScheduleJpaRepository.java (2)
37-45: Consider consolidating duplicate schedule query logicThere's duplicate logic between
findWeekPublicandfindWeekendPublicSchedulemethods. Consider refactoring to reuse the query logic.Here's a suggested refactor:
- public List<ScheduleEntity> findWeekPublic(LocalDateTime startDate, LocalDateTime endDate) { + private List<ScheduleEntity> findSchedulesBetweenDates(LocalDateTime startDate, LocalDateTime endDate, ScheduleType type) { if (startDate == null || endDate == null) { throw new IllegalArgumentException("Date parameters cannot be null"); } if (startDate.isAfter(endDate)) { throw new IllegalArgumentException("Start date must be before or equal to end date"); } return em.createQuery("SELECT s FROM ScheduleEntity s " + "WHERE s.scheduleType = :scheduleType " + "AND s.eventStartDate BETWEEN :startDate AND :endDate", ScheduleEntity.class) .setParameter("scheduleType", type.name()) .setParameter("startDate", startDate) .setParameter("endDate", endDate) .getResultList(); } + public List<ScheduleEntity> findWeekPublic(LocalDateTime startDate, LocalDateTime endDate) { + return findSchedulesBetweenDates(startDate, endDate, ScheduleType.PUBLIC); + } public List<ScheduleEntity> findWeekendPublicSchedule() { - List<ScheduleEntity> entity = em.createQuery("SELECT s FROM ScheduleEntity s \n" + - "WHERE s.scheduleType = 'PUBLIC' \n" + - "AND s.eventStartDate BETWEEN CURRENT_DATE AND CURRENT_DATE + 5\n", ScheduleEntity.class).getResultList(); - return entity; + LocalDateTime now = LocalDateTime.now(); + return findSchedulesBetweenDates(now, now.plusDays(5), ScheduleType.PUBLIC); }Also applies to: 48-53
35-36: Remove unnecessary blank linesThere are multiple consecutive blank lines that don't improve readability.
- - public List<ScheduleEntity> findWeekPublic(LocalDateTime startDate, LocalDateTime endDate) { // ... implementation ... } - - - public List<ScheduleEntity> findWeekendPublicSchedule() {Also applies to: 46-48
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
BE/error/src/main/java/com/example/demo/schedule/application/service/ScheduleService.java(2 hunks)BE/error/src/main/java/com/example/demo/schedule/infrastructure/SlackService.java(1 hunks)BE/error/src/main/java/com/example/demo/schedule/infrastructure/persistence/ScheduleJpaRepository.java(1 hunks)BE/error/src/main/java/com/example/demo/schedule/presentation/ScheduleController.java(3 hunks)
| @Value("${spring.config.activate.on-profile.oauth.provider.slack.token}") | ||
| private String slackToken; | ||
| private final ScheduleService scheduleService; | ||
| private final static String CHANNEL_NAME = "에코노베이션-일정-소개-페이지"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving token and channel configuration management
The Slack token and channel name configuration could be improved:
- The token path seems unusual:
spring.config.activate.on-profile.oauth.provider.slack.token - The channel name is hardcoded as a constant
Consider:
- Moving configurations to a dedicated configuration class
- Using a more standard property path for the token
- Making the channel name configurable via properties
- @Value("${spring.config.activate.on-profile.oauth.provider.slack.token}")
+ @Value("${slack.api.token}")
private String slackToken;
- private final static String CHANNEL_NAME = "에코노베이션-일정-소개-페이지";
+ @Value("${slack.channel.name}")
+ private String channelName;Committable suggestion skipped: line range outside the PR's diff.
| public void sendSlackMessage(String message, String channel){ | ||
|
|
||
|
|
||
| try{ | ||
| MethodsClient methods = Slack.getInstance().methods(slackToken); | ||
|
|
||
| ChatPostMessageRequest request = ChatPostMessageRequest.builder() | ||
| .channel(CHANNEL_NAME) | ||
| .text(message) | ||
| .build(); | ||
|
|
||
| methods.chatPostMessage(request); | ||
|
|
||
| log.info("Slack " + channel + " 에 메시지 보냄"); | ||
|
|
||
| } catch (SlackApiException | IOException e) { | ||
| log.error(e.getMessage()); | ||
| } catch (com.slack.api.methods.SlackApiException e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } |
There was a problem hiding this comment.
Improve error handling and logging in sendSlackMessage
Current error handling has several issues:
- Catches both SlackApiException and IOException but only logs the message
- Different handling for com.slack.api.methods.SlackApiException
- Limited error information in logs
- No validation of input parameters
Consider this improved implementation:
public void sendSlackMessage(String message, String channel) {
+ if (message == null || message.trim().isEmpty()) {
+ throw new IllegalArgumentException("Message cannot be null or empty");
+ }
+
try {
MethodsClient methods = Slack.getInstance().methods(slackToken);
ChatPostMessageRequest request = ChatPostMessageRequest.builder()
- .channel(CHANNEL_NAME)
+ .channel(channelName)
.text(message)
.build();
- methods.chatPostMessage(request);
+ var response = methods.chatPostMessage(request);
+ if (!response.isOk()) {
+ throw new SlackApiException("Failed to send message: " + response.getError());
+ }
- log.info("Slack " + channel + " 에 메시지 보냄");
+ log.info("Successfully sent message to Slack channel: {}", channelName);
- } catch (SlackApiException | IOException e) {
- log.error(e.getMessage());
- } catch (com.slack.api.methods.SlackApiException e) {
- throw new RuntimeException(e);
+ } catch (Exception e) {
+ log.error("Failed to send Slack message", e);
+ throw new SlackApiException("Failed to send Slack message", e);
}
}Committable suggestion skipped: line range outside the PR's diff.
| @GetMapping("slack/test") | ||
| public void slackTest() { | ||
|
|
||
| String event = slackService.makeSlackMessage(); | ||
| slackService.sendSlackMessage(event, "test"); | ||
| } |
There was a problem hiding this comment.
Enhance slackTest endpoint implementation
The current implementation has several issues:
- No security controls
- No error handling
- No proper response structure
- Test endpoint in production code
Consider this improved implementation:
- @GetMapping("slack/test")
- public void slackTest() {
- String event = slackService.makeSlackMessage();
- slackService.sendSlackMessage(event, "test");
- }
+ @PostMapping("/slack/notify")
+ @PreAuthorize("hasRole('ADMIN')")
+ public ApiResponse<SuccessBody<Void>> sendSlackNotification() {
+ try {
+ String event = slackService.makeSlackMessage();
+ slackService.sendSlackMessage(event, "test");
+ return ApiResponseGenerator.success(HttpStatus.OK, MessageCode.SUCCESS);
+ } catch (Exception e) {
+ log.error("Failed to send Slack notification", e);
+ throw new ApiException(ErrorCode.SLACK_NOTIFICATION_FAILED);
+ }
+ }Committable suggestion skipped: line range outside the PR's diff.
| LocalDateTime startDate = LocalDateTime.now(); | ||
| LocalDateTime endDate = startDate.plusDays(5); | ||
|
|
||
| List<ScheduleEntity> schedules = scheduleJpaRepository.findWeekPublic(startDate, endDate); | ||
| return schedules; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve weekend schedule retrieval logic
The current implementation has several issues:
- Hard-coded 5-day range without explanation
- No validation of date range
- Direct entity return from service layer
Consider this improved implementation:
+ private static final int SCHEDULE_LOOKUP_DAYS = 5;
+
public List<ScheduleEntity> findWeekendSchedule() {
LocalDateTime startDate = LocalDateTime.now();
- LocalDateTime endDate = startDate.plusDays(5);
+ LocalDateTime endDate = startDate.plusDays(SCHEDULE_LOOKUP_DAYS);
+
+ if (startDate.isAfter(endDate)) {
+ throw new IllegalStateException("Start date must be before end date");
+ }
List<ScheduleEntity> schedules = scheduleJpaRepository.findWeekPublic(startDate, endDate);
+ log.debug("Found {} schedules between {} and {}", schedules.size(), startDate, endDate);
return schedules;
}Committable suggestion skipped: line range outside the PR's diff.
| public List<ScheduleEntity> findWeekPublic(LocalDateTime startDate, LocalDateTime endDate) { | ||
| return em.createQuery("SELECT s FROM ScheduleEntity s " + | ||
| "WHERE s.scheduleType = :scheduleType " + | ||
| "AND s.eventStartDate BETWEEN :startDate AND :endDate", ScheduleEntity.class) | ||
| .setParameter("scheduleType", "PUBLIC") // scheduleType 파라미터 설정 | ||
| .setParameter("startDate", startDate) // startDate 파라미터 설정 | ||
| .setParameter("endDate", endDate) // endDate 파라미터 설정 | ||
| .getResultList(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using an enum for schedule types and add input validation
The implementation could be improved in several ways:
- Replace the hard-coded "PUBLIC" string with an enum constant to prevent typos and improve maintainability
- Add input validation for date parameters
- Remove redundant comments that explain obvious parameter settings
Here's a suggested improvement:
+ public enum ScheduleType {
+ PUBLIC, PRIVATE
+ }
- public List<ScheduleEntity> findWeekPublic(LocalDateTime startDate, LocalDateTime endDate) {
+ public List<ScheduleEntity> findWeekPublic(LocalDateTime startDate, LocalDateTime endDate) {
+ if (startDate == null || endDate == null) {
+ throw new IllegalArgumentException("Date parameters cannot be null");
+ }
+ if (startDate.isAfter(endDate)) {
+ throw new IllegalArgumentException("Start date must be before or equal to end date");
+ }
+
return em.createQuery("SELECT s FROM ScheduleEntity s " +
"WHERE s.scheduleType = :scheduleType " +
"AND s.eventStartDate BETWEEN :startDate AND :endDate", ScheduleEntity.class)
- .setParameter("scheduleType", "PUBLIC") // scheduleType 파라미터 설정
- .setParameter("startDate", startDate) // startDate 파라미터 설정
- .setParameter("endDate", endDate) // endDate 파라미터 설정
+ .setParameter("scheduleType", ScheduleType.PUBLIC.name())
+ .setParameter("startDate", startDate)
+ .setParameter("endDate", endDate)
.getResultList();
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
BE/error/src/main/java/com/example/demo/schedule/application/service/ScheduleService.java (2)
30-30: Document the business logic behind SCHEDULE_LOOKUP_DAYSWhile extracting the magic number to a constant is good, consider:
- Adding a Javadoc comment explaining why 5 days was chosen
- Making this value configurable through application properties
+ /** + * Defines the number of days to look ahead for weekend schedules. + * This window ensures we capture upcoming events for Slack notifications. + * TODO: Consider making this configurable via application.properties + */ private static final int SCHEDULE_LOOKUP_DAYS = 5;
76-81: Maintain architectural consistency across the serviceThe
findWeekendSchedulemethod deviates from the established pattern in other methods (likegetPublicScheduleandgetPrivateSchedule) which properly use domain models and converters. This inconsistency could lead to maintenance issues.Consider:
- Using
ScheduleModelconsistently across all methods- Applying the same converter pattern used in other methods
- Creating a dedicated response DTO for weekend schedules if needed
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
BE/error/src/main/java/com/example/demo/schedule/application/service/ScheduleService.java(3 hunks)BE/error/src/main/java/com/example/demo/schedule/presentation/ScheduleController.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- BE/error/src/main/java/com/example/demo/schedule/presentation/ScheduleController.java
🔇 Additional comments (2)
BE/error/src/main/java/com/example/demo/schedule/application/service/ScheduleService.java (2)
15-15: LGTM!
The LocalDateTime import is appropriately added for the new date handling functionality.
76-81:
Refactor to follow service layer best practices
The current implementation has several architectural and robustness issues:
- Returns repository entities directly, violating layer separation
- Lacks date validation
- Missing error handling
- No logging for monitoring
Consider this improved implementation:
- public List<ScheduleEntity> findWeekendSchedule() {
+ public List<AllPublicCalendarResponse> findWeekendSchedule() {
+ try {
+ LocalDateTime startDate = LocalDateTime.now();
+ LocalDateTime endDate = startDate.plusDays(SCHEDULE_LOOKUP_DAYS);
+
+ if (startDate.isAfter(endDate)) {
+ throw new IllegalStateException("Invalid date range calculation");
+ }
+
+ List<ScheduleEntity> schedules = scheduleJpaRepository.findWeekPublic(startDate, endDate);
+ log.debug("Found {} weekend schedules between {} and {}",
+ schedules.size(), startDate, endDate);
+
+ return schedules.stream()
+ .map(responseConverter::toPublicModel)
+ .collect(Collectors.toList());
+ } catch (Exception e) {
+ log.error("Error fetching weekend schedules: {}", e.getMessage());
+ throw new ScheduleServiceException("Failed to fetch weekend schedules", e);
+ }
- LocalDateTime startDate = LocalDateTime.now();
- LocalDateTime endDate = startDate.plusDays(SCHEDULE_LOOKUP_DAYS);
-
- List<ScheduleEntity> schedules = scheduleJpaRepository.findWeekPublic(startDate, endDate);
- return schedules;
}Let's verify the usage of this method in the Slack integration:
✅ Verification successful
Let's examine how the method is used in SlackService to ensure our proposed changes align with its usage.
Let's check the existing AllPublicCalendarResponse and responseConverter implementation to ensure our suggestion is compatible.
Based on the gathered information, I can now provide the final response:
Refactor to align with existing service patterns
The current implementation should be updated to match the established patterns in the codebase:
- public List<ScheduleEntity> findWeekendSchedule() {
+ public List<AllPublicCalendarResponse> findWeekendSchedule() {
LocalDateTime startDate = LocalDateTime.now();
LocalDateTime endDate = startDate.plusDays(SCHEDULE_LOOKUP_DAYS);
+ if (startDate.isAfter(endDate)) {
+ throw new IllegalArgumentException("Invalid date range calculation");
+ }
+
List<ScheduleEntity> schedules = scheduleJpaRepository.findWeekPublic(startDate, endDate);
- return schedules;
+ return schedules.stream()
+ .map(schedule -> responseConverter.toPublicModel(schedule))
+ .collect(Collectors.toList());
}- The method should return
AllPublicCalendarResponseto match other public methods likegetPublicSchedule() - Use the existing
responseConverterwhich is already injected and used throughout the service - Simple date validation is sufficient as other service methods don't show complex error handling
- Logging can be omitted as it's not used in similar methods in the service
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the method is used in SlackService and how it handles the response
rg -l "findWeekendSchedule" | grep -i "slack"
Length of output: 125
Script:
#!/bin/bash
# Check the implementation in SlackService
ast-grep --pattern 'findWeekendSchedule' -A 10
Length of output: 2510
Script:
#!/bin/bash
# Look for any DTO/Response classes that might already exist
fd -e java AllPublicCalendarResponse
Length of output: 134
Script:
#!/bin/bash
# Check if responseConverter exists in the codebase
rg -l "responseConverter" --type java
Length of output: 213
Script:
#!/bin/bash
# Check for any existing ScheduleServiceException
fd ScheduleServiceException
Length of output: 29
Script:
#!/bin/bash
# Check AllPublicCalendarResponse implementation
cat BE/error/src/main/java/com/example/demo/schedule/application/dto/AllPublicCalendarResponse.java
Length of output: 748
Script:
#!/bin/bash
# Check responseConverter usage in ScheduleService
ast-grep --pattern 'responseConverter' -A 5 BE/error/src/main/java/com/example/demo/schedule/application/service/ScheduleService.java
Length of output: 4534
Script:
#!/bin/bash
# Look for any existing exception classes in the schedule module
fd -e java -p ".*Exception.java" BE/error/src/main/java/com/example/demo/schedule
Length of output: 83
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
BE/error/src/main/java/com/example/demo/schedule/presentation/ScheduleController.java (1)
77-84: Remove test endpoint from production codeThe
GET /slack/testendpoint appears to be for testing purposes. It's advisable to remove or secure test endpoints to prevent unauthorized access in a production environment.BE/error/src/main/java/com/example/demo/schedule/application/service/ScheduleService.java (1)
76-83: Consider returning DTOs instead of entitiesReturning entities from the service layer can lead to tight coupling between layers and potential data exposure. Consider mapping
ScheduleEntityobjects to DTOs before returning them.Implement a converter or mapper to transform entities to DTOs:
public List<ScheduleEntity> findWeekendSchedule() { LocalDateTime startDate = LocalDateTime.now(); LocalDateTime endDate = startDate.plusDays(SCHEDULE_LOOKUP_DAYS); List<ScheduleEntity> schedules = scheduleJpaRepository.findWeekPublic(startDate, endDate); - return schedules; + return schedules.stream() + .map(scheduleConverter::toDto) + .collect(Collectors.toList()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
BE/error/src/main/java/com/example/demo/ErrorApplication.java(1 hunks)BE/error/src/main/java/com/example/demo/schedule/application/service/ScheduleService.java(3 hunks)BE/error/src/main/java/com/example/demo/schedule/infrastructure/SlackService.java(1 hunks)BE/error/src/main/java/com/example/demo/schedule/presentation/ScheduleController.java(3 hunks)
🔇 Additional comments (4)
BE/error/src/main/java/com/example/demo/schedule/infrastructure/SlackService.java (2)
29-32: Consider externalizing Slack token and channel configuration
The Slack token is retrieved using a non-standard property path, and the channel name is hardcoded. It's better to use standard configuration properties and make both the Slack token and channel name configurable through the application.properties file.
34-54: 🛠️ Refactor suggestion
Fix inconsistent exception handling and unused parameter
There are issues in the sendSlackMessage method:
- The
channelparameter is not used in theChatPostMessageRequestbuilder; instead,CHANNEL_NAMEis hardcoded. This can lead to confusion and prevent flexibility in specifying different channels. - Exception handling is inconsistent. The
SlackApiExceptionis caught twice, and exceptions are either logged or rethrown without a clear strategy.
Consider applying the following changes:
public void sendSlackMessage(String message, String channel){
+ if (message == null || message.trim().isEmpty()) {
+ throw new IllegalArgumentException("Message cannot be null or empty");
+ }
+ if (channel == null || channel.trim().isEmpty()) {
+ throw new IllegalArgumentException("Channel cannot be null or empty");
+ }
try{
MethodsClient methods = Slack.getInstance().methods(slackToken);
ChatPostMessageRequest request = ChatPostMessageRequest.builder()
- .channel(CHANNEL_NAME)
+ .channel(channel)
.text(message)
.build();
methods.chatPostMessage(request);
- log.info("Slack " + channel + " 에 메시지 보냄");
+ log.info("Slack {} 에 메시지 보냄", channel);
- } catch (SlackApiException | IOException e) {
- log.error(e.getMessage());
- } catch (com.slack.api.methods.SlackApiException e) {
- throw new RuntimeException(e);
+ } catch (IOException | com.slack.api.methods.SlackApiException e) {
+ log.error("Failed to send Slack message", e);
+ throw new RuntimeException("Failed to send Slack message", e);
}
}Likely invalid or redundant comment.
BE/error/src/main/java/com/example/demo/ErrorApplication.java (1)
12-19: Ensure @EnableScheduling is necessary in the main application class
Adding @EnableScheduling enables scheduling throughout the application. Confirm that this annotation is required here and that scheduled tasks are properly configured elsewhere in the application.
BE/error/src/main/java/com/example/demo/schedule/application/service/ScheduleService.java (1)
76-83: Improve weekend schedule retrieval logic
The findWeekendSchedule method has potential issues:
- The
SCHEDULE_LOOKUP_DAYSconstant is set to 5 without context or explanation. - There's no validation to ensure that
startDateis beforeendDate. - Returning
ScheduleEntitydirectly from the service layer can expose internal entity details.
| @GetMapping("slack/test") | ||
| //@Scheduled(cron = "0 0 9 * * MON") | ||
| @Scheduled(cron = "0 * * * * *") | ||
| public void sendSlackMessage() { | ||
|
|
||
|
|
||
| // 일정 조회를 어떻게 리팩토링 할 수 있을까? | ||
| // 하나의 요청 uri를 가지고 내부 | ||
| // 토큰이 존재한다면? 토근에서 값을 추출 후 memberId에 맞는 private 일정 + public 일정 응답 | ||
| // 토큰이 존재하지 않다면? public 일정만 응답 | ||
| String event = slackService.makeSlackMessage(); | ||
| slackService.sendSlackMessage(event, "test"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid scheduling controller methods
Applying @Scheduled to a controller method annotated with @GetMapping is not a good practice. Controller methods are intended to handle HTTP requests, while scheduled tasks should reside in separate components or service classes.
Consider moving the scheduled task to a dedicated scheduler class:
- @GetMapping("slack/test")
- //@Scheduled(cron = "0 0 9 * * MON")
- @Scheduled(cron = "0 * * * * *")
- public void sendSlackMessage() {
+ // Removed from controller
String event = slackService.makeSlackMessage();
slackService.sendSlackMessage(event, "test");
}
+
+ // Create a new scheduler component
+ @Component
+ public class SlackScheduler {
+
+ private final SlackService slackService;
+
+ public SlackScheduler(SlackService slackService) {
+ this.slackService = slackService;
+ }
+
+ @Scheduled(cron = "0 * * * * *")
+ public void sendSlackMessage() {
+ String event = slackService.makeSlackMessage();
+ slackService.sendSlackMessage(event, "test");
+ }
+ }Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores