feat(telegram): add support for chat topics #1125
Conversation
|
As it seems |
internal/notif/telegram/client.go
Outdated
| var chatTopic int64 | ||
| if len(chatTopics) > i { | ||
| chatTopic = chatTopics[i] | ||
| } |
There was a problem hiding this comment.
I don't think we should use a second array to define topics. That's error prone imo.
I think we could either use a separator in chat ID to define topics like 123456789:25 where 123456789 is the chat ID and 25 an optional topic/thread id.
Also I think a chat ID can have multiple topics right?
If that's the case we could either define comma separated list of topic IDs in chat ID like 123456789:25,12 or keep ChatTopics that would be a map like:
chatTopics:
123456789:
- 25
- 12I would tend to reuse chatIDs with 123456789:25,12
There was a problem hiding this comment.
I think we could either use a separator in chat ID to define topics like 123456789:25 where 123456789 is the chat ID and 25 an optional topic/thread id.
Unless I'm misunderstanding something, that would mean we'd have to change the chatID to a string, right? That would be a rather breaking change and I'm not sure if that is the best solution.
I made a draft of using a map for the chat topics in 1915c27. It's working but it has the downside that you have to make the map keys explicitly a string (github.com/crazy-max/gonfig doesn't support int64 as map keys).
Let me know what you think!
This PR adds support for telegram chat topics.
github.com/go-telegram-bot-api/telegram-bot-apidoesn't have support for topics despite several open PRs, so I switched it out with a more up-to-date library.Diun supports sending messages to multiple chats so
ChatTopicsis a 2d slice, which allows diun to send messages to multiple chats and multiple topics.I couldn't quite figure out how the environment variables are parsed, so I'm not 100% sure if the parsing of multi dimensional arrays works. I'll test that before I remove the draft status.
This is my first time contributing to
diunso please let me know if I overlooked something or if have any other comments so far!Closes #948 and #1031