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

feat(telegram): add support for message_thread_id #362

Merged
merged 5 commits into from
Jun 13, 2023

Conversation

yegle
Copy link
Contributor

@yegle yegle commented Jun 11, 2023

@codecov
Copy link

codecov bot commented Jun 11, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.03 🎉

Comparison is base (048e61a) 78.92% compared to head (01ad967) 78.95%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #362      +/-   ##
==========================================
+ Coverage   78.92%   78.95%   +0.03%     
==========================================
  Files         101      101              
  Lines        4431     4438       +7     
==========================================
+ Hits         3497     3504       +7     
  Misses        756      756              
  Partials      178      178              
Impacted Files Coverage Δ
pkg/services/telegram/telegram_json.go 94.28% <100.00%> (+1.42%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@piksel
Copy link
Member

piksel commented Jun 11, 2023

Looks good, but there are no tests. Also, how do you retrieve the thread ID?

@yegle
Copy link
Contributor Author

yegle commented Jun 11, 2023

Test added, info on how to obtain the message_thread_id added.

Copy link
Member

@piksel piksel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! I'm not sure how I feel about linking a SO question in the docs, but perhaps it's better than nothing. We could always update the docs at a later time.

@yegle
Copy link
Contributor Author

yegle commented Jun 12, 2023

Is it okay to keep the $chat_id:$message_thread_id format? I only proposed it because it's easy to implement, but we can change to something more formal.

@piksel
Copy link
Member

piksel commented Jun 13, 2023

Yeah, I think it's a good way to do it. Normally I would prefer to add another parameter, but since this is directly relating to a target it makes sense to use this. It also doesn't need to be escaped which is a plus!

@piksel piksel changed the title Add support for Telegram message_thread_id. feat(telegram): add support for message_thread_id Jun 13, 2023
@piksel piksel merged commit 04c6ea8 into containrrr:main Jun 13, 2023
7 checks passed
@yegle yegle deleted the telegram-thread-id branch July 24, 2023 19:34
@instantdreams
Copy link

Documentation needs to be updated to reflect use and examples of Topic ID

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Telegram thread ID
3 participants