-
Notifications
You must be signed in to change notification settings - Fork 3.1k
feat(server): Add messageThreadId handling in super groups for Telegram notifications #5409
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
base: master
Are you sure you want to change the base?
Changes from all commits
a84b8af
7e91ea8
353bed5
1541313
866e868
19154c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -47,6 +47,11 @@ public class TelegramNotifier extends AbstractContentNotifier { | |||||||||
| */ | ||||||||||
| @Nullable private String chatId; | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Unique identifier for the target topic of the target super group | ||||||||||
| */ | ||||||||||
| private Integer messageThreadId; | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * The token identifying und authorizing your Telegram bot (e.g. | ||||||||||
| * `123456:ABC-DEF1234ghIkl-zyx57W2v1u123ew11`) | ||||||||||
|
|
@@ -75,14 +80,17 @@ protected Mono<Void> doNotify(InstanceEvent event, Instance instance) { | |||||||||
| .fromRunnable(() -> restTemplate.getForObject(buildUrl(), Void.class, createMessage(event, instance))); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| protected String buildUrl() { | ||||||||||
| 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() { | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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:
|
||||||||||
| return String.format( | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||||||||||
| "%s/bot%s/sendmessage?chat_id={chat_id}&message_thread_id={message_thread_id}&text={text}&parse_mode={parse_mode}&disable_notification={disable_notification}", | ||||||||||
| this.apiUrl, this.authToken | ||||||||||
| ); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| 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); | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This matches the Telegram Bot API specification where message_thread_id is explicitly documented as optional. |
||||||||||
| parameters.put("parse_mode", this.parseMode); | ||||||||||
| parameters.put("disable_notification", this.disableNotify); | ||||||||||
| parameters.put("text", createContent(event, instance)); | ||||||||||
|
|
@@ -114,6 +122,14 @@ public void setChatId(@Nullable String chatId) { | |||||||||
| this.chatId = chatId; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public Integer getMessageThreadId() { | ||||||||||
| return messageThreadId; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public void setMessageThreadId(Integer messageThreadId) { | ||||||||||
| this.messageThreadId = messageThreadId; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| @Nullable public String getAuthToken() { | ||||||||||
| return authToken; | ||||||||||
| } | ||||||||||
|
|
||||||||||
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.
Please mark it with
@Nullablefor consistency.