-
Notifications
You must be signed in to change notification settings - Fork 21
Open
Labels
A-notifsSlack, Email, or other notification methodsSlack, Email, or other notification methodsD-easyBeginner-friendly tasks.Beginner-friendly tasks.P-lowLow-priority or non-urgent tasksLow-priority or non-urgent tasksT-taskGeneral tasks or chores (e.g., refactoring, cleanup)General tasks or chores (e.g., refactoring, cleanup)cla: allowlistgood-first-issueBeginner-friendly, low-complexity issues to help new contributorsBeginner-friendly, low-complexity issues to help new contributorshelp-wantedIssues where community contributions are welcomeIssues where community contributions are welcome
Description
There are few problems with current implementation:
-
Slack
,Telegram
,Discord
are almost identical in structure and logic. Seems like a lot of duplication -
Notifier
trait is not very useful for webhook-based notifiers, becausenotify
method just delegates tonotify_with_payload
of inner Webhook notifier.Slack
,Telegram
,Discord
are essentially wrappers aroundWebhook
notifier. -
Also, there is issue with signing in
Webhook
: currently it signs a genericWebhookMessage
containing only atitle
andbody
, actual JSON payload containing other fields is not singed.
Proposed solution:
- Keep unified
Webhook
notifier, eliminate separateDiscordNotifier
,SlackNotifier
, andTelegramNotifier
structs - Introduce
PayloadBuilder
trait, which will take message and necessary variables and produce JSON payload. Each notification type will have own implementation. Something like this:
trait WebhookPayloadBuilder {
fn build_payload(&self, title: &str, message: &str) -> serde_json::Value;
}
struct SlackPayloadBuilder;
impl WebhookPayloadBuilder for SlackPayloadBuilder {
fn build_payload(&self, title: &str, message: &str) -> serde_json::Value {
// ... logic to create payload JSON for Slack
}
}
NotificationService
will create multipleWebhookNotifier
instances and choose relevant payload builder based onTriggerType
- Builder will also ensure that the final payload is signed before sending
shahnami and NicoMolinaOZ
Metadata
Metadata
Assignees
Labels
A-notifsSlack, Email, or other notification methodsSlack, Email, or other notification methodsD-easyBeginner-friendly tasks.Beginner-friendly tasks.P-lowLow-priority or non-urgent tasksLow-priority or non-urgent tasksT-taskGeneral tasks or chores (e.g., refactoring, cleanup)General tasks or chores (e.g., refactoring, cleanup)cla: allowlistgood-first-issueBeginner-friendly, low-complexity issues to help new contributorsBeginner-friendly, low-complexity issues to help new contributorshelp-wantedIssues where community contributions are welcomeIssues where community contributions are welcome