Skip to content
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

Telegram Handler: support additional API parameters #1451

Merged
merged 5 commits into from
May 22, 2020
Merged

Telegram Handler: support additional API parameters #1451

merged 5 commits into from
May 22, 2020

Conversation

Pascal-So
Copy link
Contributor

The telegram API for sendMessage supports some additional parameters that where not yet covered with the current implementation. See the API documentation here:

https://core.telegram.org/bots/api#sendmessage

In my use case I'm mostly just interested in the parse_mode parameter but I thought that while I'm on it I could also add support for disable_web_page_preview and disable_notification.

I didn't add support for the other remaining options because in my opinion they will never make sense when using the API for logging. reply_to_message_id can only be set for replies, but when logging we're not replying to a message but rather sending independent messages. reply_markup lets the bot display an interactive interface for the user receiving the message, but the log handler can't deal with replies anyway.

Something that I'm not sure about whether it's okay like this is the function argument order to the constructor. Should I move the new arguments to the end?

@Seldaek Seldaek added this to the 2.x milestone May 11, 2020
@Pascal-So
Copy link
Contributor Author

I've added some simple validation for the parse mode in the setter, plus tests for the validation.

@Seldaek
Copy link
Owner

Seldaek commented May 22, 2020

Thanks for the updates!

@Seldaek Seldaek merged commit 7a801dd into Seldaek:master May 22, 2020
@lyrixx lyrixx mentioned this pull request Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants