feat(server): Add messageThreadId handling in super groups for Telegram notifications#5409
feat(server): Add messageThreadId handling in super groups for Telegram notifications#5409AliArtukov wants to merge 6 commits into
Conversation
Add messageThreadId for sending alerts to specific topic of telegram group.
Updated messageThreadId to default to '0' and removed nullable annotations.
|
Hi guys. Changes
WhyTelegram supergroups support topic-based discussions, where messages can be posted into specific topics using the Backward Compatibility
Use CaseThis allows users to route notifications into dedicated topics inside Telegram supergroups (for example, separate topics for environments, services, or alert categories and our company is real case). |
| * Unique identifier for the target topic of the target super group | ||
| * 0 is an ID of general topic | ||
| */ | ||
| private String messageThreadId = "0"; |
There was a problem hiding this comment.
According to https://core.telegram.org/bots/api, this should be an Integer and not a String.
Moreover, I can't find anywhere stated that 0 is the default value. On the contrary, many implementations use 1 instead.
Finally, the parameter is optional, so not passing it at all if not provided is way safer than relying on some not really documented default value.
There was a problem hiding this comment.
I agree with you.
Currently changed attribute type to Integer and added message_thread_id to request param.
There was a problem hiding this comment.
The way you changed the things may break.
If no value is set it may either fail with NPE (if toString is called on it) or pass null as value (if valueOf is called with it).
Both the cases are wrong.
| /** | ||
| * Unique identifier for the target topic of the target super group | ||
| */ | ||
| private Integer messageThreadId; |
There was a problem hiding this comment.
Please mark it with @Nullable for consistency.
| + "&disable_notification={disable_notification}", this.apiUrl, this.authToken); | ||
| } | ||
| protected String buildUrl() { | ||
| return String.format( |
There was a problem hiding this comment.
Only include the parameter when messageThreadId != null.
Otherwise the message_thread_id parameter is always appended to the URL. You could pass null to the Telegram API (undefined behavior, likely a 400 error), or cause an NPE if Spring's UriComponentsBuilder tries to resolve {message_thread_id} with a null map value.
| return String.format("%s/bot%s/sendmessage?chat_id={chat_id}&text={text}&parse_mode={parse_mode}" | ||
| + "&disable_notification={disable_notification}", this.apiUrl, this.authToken); | ||
| } | ||
| protected String buildUrl() { |
There was a problem hiding this comment.
The existing tests verify exact URL strings and parameter map contents. Adding messageThreadId will change behavior that the tests should cover. Two test cases are needed:
- When messageThreadId is set: URL includes &message_thread_id=... and parameter map contains the value
- When messageThreadId is NOT set (default): URL does NOT include message_thread_id and parameter map does NOT contain it (backward compatibility)
| private Map<String, Object> createMessage(InstanceEvent event, Instance instance) { | ||
| Map<String, Object> parameters = new HashMap<>(); | ||
| parameters.put("chat_id", this.chatId); | ||
| parameters.put("message_thread_id", this.messageThreadId); |
There was a problem hiding this comment.
| parameters.put("message_thread_id", this.messageThreadId); | |
| if (this.messageThreadId != null) { | |
| parameters.put("message_thread_id", this.messageThreadId); | |
| } |
This matches the Telegram Bot API specification where message_thread_id is explicitly documented as optional.
3 participants
No description provided.