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

'Bot.SendMessage' does not support target channel username #160

Open
liuwb opened this issue May 10, 2024 · 4 comments
Open

'Bot.SendMessage' does not support target channel username #160

liuwb opened this issue May 10, 2024 · 4 comments

Comments

@liuwb
Copy link

liuwb commented May 10, 2024

Description of the problem / feature request

This is the method signature of 'SendMessage' function of Bot:

func (bot *Bot) SendMessage(chatId int64, text string, opts *SendMessageOpts) (*Message, error)

And the description of 'chatId' input parameter:

- chatId (type int64): Unique identifier for the target chat or username of the target channel (in the format @channelusername)

We have no way to pass channel username (which is a string), but only chat ID is supported.

Feature requests

Fix the generation tool to generate correct 'SendMessage' implementation code that accepts 'chatId' as integer or string.

@PaulSonOfLars
Copy link
Owner

Hi! Thank you for raising the issue!

A few things things to note here:

  • Go does not allow for overloading methods in the same way other compiled languages (eg Java) might. This means that if a method is defined, you cannot define a method with the same name but different parameter types/orders.
  • Go does not allow for using generics in methods; only in functions. One can use a generic type with methods that depend on that type, but that would then force that bot instance to only ever use that type; eg, you would be forced to only use strings, or integers, for any chat_id fields.
  • Telegram usernames can change; Telegram IDs cannot. It is therefore much safer to rely on IDs than on usernames.

Regarding how to "fix" the issue (and why it hasnt been/isnt planned to be):

  • Since we can't use generics, we would have to create a new wrapper type, something like a StringOrInt type. This would be a major breaking change for all users, and require a lot of duplicated code everywhere (EVERY method call would end up looking something like bot.SendMessage(stringorint.Int(ctx.EffectiveChat.Id), "text", ...etc)). In my opinion, the extra boilerplate would be a chore.
  • Alternatively, one could think of something like adding XByUsername methods; eg SendMessageByUsername, to allow for sending by string. This seems like an OK idea at first, but rapidly breaks down when you consider methods like forwardMessage, which takes different chat_ids parameters; one for the origin chat, and one for the destination chat. Would you then make 4 methods, to cover each case? What would they be called to separate each parameter? Seems like exponential issues.
  • It also raises an issues with the other Telegram types, not just the methods; how should one handle the BotCommandScope type, which allows for Integer or Username fields too? should we be using multiple types there? that would require an interface for each individual type to make sure that each can be returned correctly, further increasing unmarshalling complexity.

As you can see - I've put in some thought into this issue already 😅. And given the advantages of using IDs over usernames, I think it makes more sense to save ourselves the trouble and stick to IDs for now.

If you really need to do actions via username, you can use the bot.Request(...) method and populate it with your own values; it won't have all the typesafety advantages the rest of the lib has, but it does have much more flexibility.

TL;DR - this is a known "issue" and not likely to be fixed, as I believe it would result in fighting Go's typesystem more than anything else. Use bot.Request if you really need this.

@PaulSonOfLars
Copy link
Owner

I definitely need to update the doc generation to remove the mention of the channelusername though; that seems to be causing more confusion than anything else!

@liuwb
Copy link
Author

liuwb commented May 10, 2024

Hi! Thank you for raising the issue!

A few things things to note here:

  • Go does not allow for overloading methods in the same way other compiled languages (eg Java) might. This means that if a method is defined, you cannot define a method with the same name but different parameter types/orders.
  • Go does not allow for using generics in methods; only in functions. One can use a generic type with methods that depend on that type, but that would then force that bot instance to only ever use that type; eg, you would be forced to only use strings, or integers, for any chat_id fields.
  • Telegram usernames can change; Telegram IDs cannot. It is therefore much safer to rely on IDs than on usernames.

Regarding how to "fix" the issue (and why it hasnt been/isnt planned to be):

  • Since we can't use generics, we would have to create a new wrapper type, something like a StringOrInt type. This would be a major breaking change for all users, and require a lot of duplicated code everywhere (EVERY method call would end up looking something like bot.SendMessage(stringorint.Int(ctx.EffectiveChat.Id), "text", ...etc)). In my opinion, the extra boilerplate would be a chore.
  • Alternatively, one could think of something like adding XByUsername methods; eg SendMessageByUsername, to allow for sending by string. This seems like an OK idea at first, but rapidly breaks down when you consider methods like forwardMessage, which takes different chat_ids parameters; one for the origin chat, and one for the destination chat. Would you then make 4 methods, to cover each case? What would they be called to separate each parameter? Seems like exponential issues.
  • It also raises an issues with the other Telegram types, not just the methods; how should one handle the BotCommandScope type, which allows for Integer or Username fields too? should we be using multiple types there? that would require an interface for each individual type to make sure that each can be returned correctly, further increasing unmarshalling complexity.

As you can see - I've put in some thought into this issue already 😅. And given the advantages of using IDs over usernames, I think it makes more sense to save ourselves the trouble and stick to IDs for now.

If you really need to do actions via username, you can use the bot.Request(...) method and populate it with your own values; it won't have all the typesafety advantages the rest of the lib has, but it does have much more flexibility.

TL;DR - this is a known "issue" and not likely to be fixed, as I believe it would result in fighting Go's typesystem more than anything else. Use bot.Request if you really need this.

I can see your concerns, but the lib loses the full capability of telegram bot api, such as when I want the bot to send message to channel instead of chat ID. Maybe we can have some minor fix in the lib without breaking much.

1)Accept chatId as interface{}, but casting it to integer or string internally, so we can identify whether it is chat ID or channel username. It breaks the api signature but can still compile with old codes.

2)Change the signature to add an optional postProcess parameter:

func (bot *Bot) SendMessage(chatId int64, text string, opts *SendMessageOpts, postProcess ...func(map[string]string) ) (*Message, error)

It saves us from populating many values in the params map if I have to use bot.Request as you suggest. We can replace v["chat_id"] with channel username in the postProcess function and still use most of the logic of 'SendMessage'. It provides almost the same flexibility of bot.Request as well as all the type safety advantages.

I hope it helps.

@PaulSonOfLars
Copy link
Owner

PaulSonOfLars commented May 10, 2024

I can see your concerns, but the lib loses the full capability of telegram bot api, such as when I want the bot to send message to channel instead of chat ID. Maybe we can have some minor fix in the lib without breaking much.

You can use the channel ID instead - you don't HAVE to use usernames to interact with channels.

1)Accept chatId as interface{}, but casting it to integer or string internally, so we can identify whether it is chat ID or channel username. It breaks the api signature but can still compile with old codes.

This removes all the advantages of using a typesafe language; these things should be caught at compile time, not at runtime. This would also be a BIG breaking change which I don't think is justifiable.

2)Change the signature to add an optional postProcess parameter:

I find that using variadic arguments to denote "optional" parameters is a codesmell; if I had done that for all the other optional parameters, we would not have any type safety in the library. Your suggestion also seems to be adding unnecessary complexity and friction points where I don't think any is needed. Finally, this only handles the sending of Integer or String fields, not the receiving.

If your concern is really just using sendMessage to send to a username, and you really want type safety, you can implement your own helper method which wraps around bot.Request; that gives you both the flexibility and the typesafety that you want, without compromising other parts of the library

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

No branches or pull requests

2 participants