DeleteMessage / EditMessage - #86
Conversation
iamwavecut
left a comment
There was a problem hiding this comment.
Thanks for adding a typed deletion facade. The facade itself is useful, but its interface should follow the conventions already used throughout this library and map the Telegram API result faithfully.
BotAPI facade methods take the corresponding config type so the config remains the complete request contract. Accepting primitive IDs here would create a narrower parallel API and drop capabilities already represented by DeleteMessageConfig, such as non-numeric chat identifiers and compatibility fields. Telegram also returns a Boolean from deleteMessage, so this facade should use requestBool and return (bool, error), consistently with the existing Boolean facades.
Please also add the symmetric batch facade for the existing DeleteMessagesConfig. The library already exposes both NewDeleteMessage and NewDeleteMessages; exposing both typed operations keeps the public surface coherent without introducing another abstraction.
Please add focused tests for the single-message and batch facades. The inline suggestion shows the requested interface.
| // DeleteMessage deletes a message | ||
| func (bot *BotAPI) DeleteMessage(chatID int64, messageID int) error { | ||
| req := NewDeleteMessage(chatID, messageID) | ||
| _, err := bot.Request(req) | ||
| return err | ||
| } |
There was a problem hiding this comment.
Please keep the deletion facade, but make it config-based so it remains consistent with the rest of BotAPI and preserves the full existing request contract. Both Telegram methods return Boolean results, and the existing single/batch config pair should have a symmetric typed facade.
| // DeleteMessage deletes a message | |
| func (bot *BotAPI) DeleteMessage(chatID int64, messageID int) error { | |
| req := NewDeleteMessage(chatID, messageID) | |
| _, err := bot.Request(req) | |
| return err | |
| } | |
| // DeleteMessage deletes a message. | |
| func (bot *BotAPI) DeleteMessage(config DeleteMessageConfig) (bool, error) { | |
| return bot.requestBool(config) | |
| } | |
| // DeleteMessages deletes multiple messages in the same chat. | |
| func (bot *BotAPI) DeleteMessages(config DeleteMessagesConfig) (bool, error) { | |
| return bot.requestBool(config) | |
| } |
iamwavecut
left a comment
There was a problem hiding this comment.
The edit facade family needs a separate correction as well. Unlike deletion, the ordinary edit endpoints do not have a single Telegram result type: most return Message for chat messages and True for inline messages. The current methods avoid that distinction only by narrowing every call to numeric chat/message IDs, which drops capabilities already present in the config types and duplicates the existing NewEdit... + Send/Request flow.
Please remove the edit facades from this PR, or redesign them separately as config-based, endpoint-level methods with an explicit result model for Message | True. If the latter is chosen, add focused local tests for both response shapes and for preservation of the config contract. The existing config/helper tests do not execute any of the new facades.
| return err | ||
| } | ||
|
|
||
| func (bot *BotAPI) EditMessageText(chatID int64, messageID int, text string) (Message, error) { |
There was a problem hiding this comment.
These primitive signatures are not consistent with the library interface. For non-inline edits, bot.Send(EditMessage*Config) already provides the typed Message path; inline edits use Request because Telegram returns True instead of a Message. These wrappers duplicate helper constructors while discarding ChannelUsername/SuperGroupUsername, BusinessConnectionID, InlineMessageID, parse mode, entities, rich content, link preview options, and optional markup. They also create multiple BotAPI methods (Photo, Video, Audio, etc.) for the single Telegram editMessageMedia endpoint, while the rest of the typed surface is organized around endpoint configs. Please remove this family from the PR, or introduce it separately as one config-based facade per Telegram endpoint with an explicit Message | True result and tests for both variants. A complete endpoint-level design would also need to account for the existing live-location edit configs.
| return message, err | ||
| } | ||
|
|
||
| func (bot *BotAPI) EditMessageChecklist(chatID int64, messageID int, checklist InputChecklist) (Message, error) { |
There was a problem hiding this comment.
editMessageChecklist requires business_connection_id, but this method calls NewEditMessageChecklist(chatID, messageID, checklist), which does not set it, and the primitive signature offers no way to supply it. As written, the facade cannot construct a valid Telegram request. If this facade is kept, it should accept EditMessageChecklistConfig and return (Message, error); the config preserves the required business connection and optional reply markup.
Add bot's methods to delete and edit message, inspite of only send new messages by bot send