-
Notifications
You must be signed in to change notification settings - Fork 1
fix: Slack Webhook의 허용 형식에 맞게 VO 재구성 완료 #12
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
Walkthrough이 변경사항은 Slack 메시지 전송을 위한 VO(Value Object) 구조를 전면적으로 재구성하고, 메시지 포맷을 Slack Webhook의 허용 형식에 맞게 수정합니다. 기존의 Changes
Sequence Diagram(s)sequenceDiagram
participant SentryEventDetail
participant SlackNotificationService
participant SlackMessage
participant SlackWebhook
SentryEventDetail->SlackNotificationService: from(JsonNode eventNode)
SlackNotificationService->SlackMessage: buildSlackMessage(team, type, stage, sentryEventDetail)
SlackMessage->SlackWebhook: send()
Assessment against linked issues
Suggested labels
Poem
✨ Finishing Touches
🪧 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 (5)
src/main/java/org/sopt/makers/vo/slack/text/MarkdownText.java (2)
3-3: *와일드카드(static ) 임포트 자제 권장
import static org.sopt.makers.global.constant.SlackConstant.*;형태의 와일드카드는 어떤 상수가 실제로 사용되는지 파악하기 어렵고, 추후 동일 이름 상수가 추가될 때 충돌 위험이 있습니다. 사용되는 상수(TEXT_TYPE_MARKDOWN)만 명시적으로 임포트하는 것이 가독성과 안전성 측면에서 좋습니다.
5-11: 레코드 생성자에 상수 검증·null 검사 추가 제안현재 레코드의 Canonical 생성자가 그대로 노출되어 있어, 외부에서 잘못된
type값을 넣어도 객체가 생성됩니다. 불변 VO의 의도를 확실히 하려면 다음과 같이 Canonical 생성자(컴팩트 생성자)를 오버라이드하여 상수 검증과textnull 검사를 수행하는 편이 좋습니다.+import java.util.Objects; + public record MarkdownText( String type, String text ) implements Text { public static MarkdownText newInstance(String text) { return new MarkdownText(TEXT_TYPE_MARKDOWN, text); } + + // Canonical constructor + public MarkdownText { + if (!TEXT_TYPE_MARKDOWN.equals(type)) { + throw new IllegalArgumentException("type must be TEXT_TYPE_MARKDOWN"); + } + Objects.requireNonNull(text, "text must not be null"); + } }이렇게 하면 VO 무결성이 컴파일 타임·런타임 모두에서 보장됩니다.
src/main/java/org/sopt/makers/vo/slack/text/PlainText.java (2)
3-3: *와일드카드(static ) 임포트 자제 권장
SlackConstant의 상수를 명시적으로 적어주면 사용되는 심볼을 한눈에 파악할 수 있습니다.
5-12: Canonical 생성자 보호 및 편의 메서드 확장
PlainText역시 외부에서 임의의type값을 넣을 수 있습니다. 또한emoji기본값을 지정할 방법이 없습니다. 아래와 같이 보완을 고려해 보세요.+import java.util.Objects; + public record PlainText( String type, String text, boolean emoji ) implements Text { - public static PlainText newInstance(String text, boolean emoji) { - return new PlainText(TEXT_TYPE_PLAIN, text, emoji); - } + public static PlainText newInstance(String text, boolean emoji) { + return new PlainText(TEXT_TYPE_PLAIN, text, emoji); + } + + // 기본 이모지 false 편의 메서드 + public static PlainText newInstance(String text) { + return new PlainText(TEXT_TYPE_PLAIN, text, false); + } + + // Canonical constructor + public PlainText { + if (!TEXT_TYPE_PLAIN.equals(type)) { + throw new IllegalArgumentException("type must be TEXT_TYPE_PLAIN"); + } + Objects.requireNonNull(text, "text must not be null"); + } }무결성 유지와 동시에 호출 측 편의성을 높일 수 있습니다.
src/main/java/org/sopt/makers/vo/slack/text/Text.java (1)
3-5:emoji처리가 필요한 경우 인터페이스에 기본 메서드 추가 고려현재
PlainText에만 존재하는emoji정보를 외부에서 공통적으로 다뤄야 한다면, 다음과 같이Text인터페이스에 기본 메서드로 제공해 Optional 성격을 명확히 할 수 있습니다.default boolean emoji() { return false; // MarkdownText 등은 기본값 false }필요 없다면 그대로 두셔도 무방하지만, 메시지 빌더 단계에서 instanceof 검사 대신 다형성을 활용할 수 있어 유지보수성이 올라갑니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/org/sopt/makers/dto/SentryEventDetail.java(2 hunks)src/main/java/org/sopt/makers/service/SlackNotificationService.java(6 hunks)src/main/java/org/sopt/makers/vo/slack/block/HeaderBlock.java(1 hunks)src/main/java/org/sopt/makers/vo/slack/block/SectionBlock.java(1 hunks)src/main/java/org/sopt/makers/vo/slack/element/Button.java(1 hunks)src/main/java/org/sopt/makers/vo/slack/text/MarkdownText.java(1 hunks)src/main/java/org/sopt/makers/vo/slack/text/PlainText.java(1 hunks)src/main/java/org/sopt/makers/vo/slack/text/Text.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/org/sopt/makers/service/SlackNotificationService.java (1)
src/main/java/org/sopt/makers/global/exception/checked/SlackSendException.java (1)
SlackSendException(5-13)
🔇 Additional comments (16)
src/main/java/org/sopt/makers/dto/SentryEventDetail.java (2)
14-15: SentryEventDetail 레코드에 title 필드 추가에 대한 리뷰Sentry 이벤트 세부 정보에 title 필드를 추가하여 에러의 제목 정보를 포함할 수 있게 되었습니다. 이는 Slack 알림에서 더 명확한 오류 식별을 가능하게 합니다.
24-24: factory 메서드에 title 추출 로직 추가 확인JSON 노드에서 title 값을 추출하여 SentryEventDetail 객체를 생성하는 로직이 적절하게 구현되었습니다.
src/main/java/org/sopt/makers/vo/slack/block/SectionBlock.java (1)
7-8: Text 관련 임포트 구문 변경 확인Text 인터페이스의 임포트 위치가 변경되었습니다. 이는 Text 타입이 레코드에서 sealed 인터페이스로 리팩토링된 것과 연관되어 있습니다. 코드 기능에는 영향이 없습니다.
src/main/java/org/sopt/makers/vo/slack/block/HeaderBlock.java (2)
5-7: Text 관련 임포트 업데이트 확인Text와 함께 새로운 PlainText 구현체를 명시적으로 임포트하였습니다. 이는 Text 타입이 sealed 인터페이스로 변경됨에 따른 적절한 업데이트입니다.
13-13: HeaderBlock 팩토리 메서드 개선
- 경고 이모지(🚨)를 추가하여 Slack 메시지의 가시성이 향상되었습니다.
- PlainText.newInstance() 메서드를 사용하도록 변경되어 Text 인터페이스의 새로운 구현체 구조와 일치하게 되었습니다.
이러한 변경은 사용자 경험을 개선하고 코드 구조의 일관성을 유지하는 데 도움이 됩니다.
src/main/java/org/sopt/makers/vo/slack/element/Button.java (2)
5-7: Text 관련 임포트 업데이트 확인Text와 함께 새로운 PlainText 구현체를 명시적으로 임포트하였습니다. 이는 Text 타입이 sealed 인터페이스로 변경됨에 따른 적절한 업데이트입니다.
15-15: Button 팩토리 메서드 개선Text.newPlainInstance 대신 PlainText.newInstance를 사용하도록 변경되었습니다. 이는 Text 인터페이스의 새로운 구현체 구조와 일치하며, 코드의 일관성을 유지합니다.
src/main/java/org/sopt/makers/service/SlackNotificationService.java (9)
29-31: 슬랙 메시지 구현체 변경을 위한 임포트 수정이 적절합니다.VO 구조 변경에 맞게 임포트 문이 적절하게 수정되었습니다.
MarkdownText클래스를 추가하고SlackMessage임포트 경로를 수정한 것이 좋습니다.
58-59: 로깅 메시지 포맷팅이 잘 정리되었습니다.로그 메시지의 줄 바꿈과 들여쓰기가 일관되게 적용되었습니다.
81-82: 에러 메시지 포맷팅이 잘 정리되었습니다.가독성을 위해 에러 메시지 포맷팅이 개선되었습니다.
87-88: 로그 메시지에 이슈 ID 정보가 추가되었습니다.슬랙 전송 완료 로그에
sentryEventDetail.issueId()가 추가되어 로그 추적이 더 용이해졌습니다.
100-104: Slack 메시지 블록 구성 파라미터 업데이트가 적절합니다.메시지 블록 구성이 새로운 VO 구조에 맞게 잘 수정되었습니다. 헤더 블록이 이제 message를 직접 사용하고, details 블록에 issueId와 level이 추가되었으며, message 블록이 title을 사용하도록 변경된 것은 모두 적절한 개선입니다.
112-113: 헤더 블록 구성 방식이 간소화되었습니다.헤더 블록이 이벤트의 message 문자열을 직접 사용하도록 변경되어 코드가 더 간결해졌습니다.
119-126: 상세 정보 블록이 영어로 통일되고 마크다운 형식이 적용되었습니다.필드 레이블이 영어로 통일되고, 마크다운 형식이 적용되어 가독성이 향상되었습니다. 또한
issueId와level필드가 추가되어 더 풍부한 정보를 제공합니다.
133-142: 에러 상세 정보 포맷팅이 개선되었습니다.에러 메시지가 "Error Details:" 레이블과 함께 코드 블록 안에 표시되도록 개선되었습니다. 멀티라인 문자열("""...""")을 사용하여 가독성이 향상되었습니다.
156-156: 날짜 시간 변환 로직이 잘 정리되었습니다.코드 포맷팅 관련 변경으로, 기능적 차이는 없습니다.
Related issue 🛠
Work Description ✏️
Trouble Shooting ⚽️