-
Notifications
You must be signed in to change notification settings - Fork 1
fix: Slack 알림 헤더가 150자 초과될 경우 말줄임표(...)로 잘라서 전송되도록 수정 #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Summary by CodeRabbit
Summary by CodeRabbit
Walkthrough이 변경사항은 Slack 알림의 헤더 메시지 길이 제한을 처리하기 위해 새로운 상수를 추가하고, 헤더 메시지 생성 로직을 수정합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant SlackNotificationService
participant SlackConstant
participant HeaderBlock
SlackNotificationService->>SlackConstant: Get EMOJI_PREFIX, MAX_HEADER_LENGTH, TRUNCATION_SUFFIX
SlackNotificationService->>SlackNotificationService: 메시지 길이와 접두사 고려하여 자르기/생략 표시 추가
SlackNotificationService->>HeaderBlock: newInstance(최종 메시지)
HeaderBlock->>HeaderBlock: PlainText.newInstance(메시지)
Assessment against linked issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/org/sopt/makers/service/SlackNotificationService.java (1)
112-120: 메시지 길이 제한 처리 로직이 잘 구현되었습니다.SlackConstant에 정의된 상수들을 활용하여 헤더 메시지의 길이를 적절히 제한하고, 이모지 접두사와 생략 표시를 일관되게 처리하는 로직이 잘 구현되었습니다. 다만, 아래와 같은 개선 사항을 고려해 볼 수 있습니다:
- 메시지가 null인 경우에 대한 방어 코드 추가
- 문자열 연산 대신 StringBuilder 사용 고려 (성능 최적화)
아래와 같이 리팩토링할 수 있습니다:
private Block buildHeaderBlock(String message) { + if (message == null) { + message = ""; + } String fullMessage = EMOJI_PREFIX + message; if (fullMessage.length() > MAX_HEADER_LENGTH) { int maxLength = MAX_HEADER_LENGTH - EMOJI_PREFIX.length() - TRUNCATION_SUFFIX.length(); fullMessage = EMOJI_PREFIX + message.substring(0, maxLength) + TRUNCATION_SUFFIX; } return HeaderBlock.newInstance(fullMessage); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/org/sopt/makers/global/constant/SlackConstant.java(1 hunks)src/main/java/org/sopt/makers/service/SlackNotificationService.java(1 hunks)src/main/java/org/sopt/makers/vo/slack/block/HeaderBlock.java(1 hunks)
🔇 Additional comments (2)
src/main/java/org/sopt/makers/global/constant/SlackConstant.java (1)
44-47: 상수 추가가 적절하게 되었습니다!헤더 관련 상수들을 추가하여 코드의 가독성과 유지보수성이 향상되었습니다. MAX_HEADER_LENGTH, EMOJI_PREFIX, TRUNCATION_SUFFIX 상수를 통해 메시지 길이 제한 및 형식이 일관되게 적용될 수 있습니다.
src/main/java/org/sopt/makers/vo/slack/block/HeaderBlock.java (1)
13-13: 관심사 분리가 잘 이루어졌습니다.이모지 접두사를 HeaderBlock 내부에서 처리하는 대신 외부(SlackNotificationService)에서 처리하도록 변경한 점이 좋습니다. 이를 통해 HeaderBlock은 단순히 텍스트를 표시하는 역할에 집중할 수 있게 되었습니다.
Related issue 🛠
Work Description ✏️
Trouble Shooting ⚽️