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

[FEATURE] API 7.0 #4033

Closed
33 of 34 tasks
clot27 opened this issue Dec 29, 2023 · 13 comments · Fixed by #4034
Closed
33 of 34 tasks

[FEATURE] API 7.0 #4033

clot27 opened this issue Dec 29, 2023 · 13 comments · Fixed by #4034

Comments

@clot27
Copy link
Member

clot27 commented Dec 29, 2023

What kind of feature are you missing? Where do you notice a shortcoming of PTB?

This is the biggest bot API update ever!, hence opening this issue to keep the track of all changes.

Important

Describe the solution you'd like

Reactions

Replies 2.0

Link Preview Customization - @harshil21

  • Added the class LinkPreviewOptions and replaced the parameter disable_web_page_preview with link_preview_options in the methods sendMessage and editMessageText.
  • Replaced the field disable_web_page_preview with link_preview_options in the class InputTextMessageContent.
  • Added the field link_preview_options to the class Message with information about the link preview options used to send the message.

Block Quotation

Multiple Message Actions @clot27

Request for multiple users @Bibo-Joshi

  • Renamed the class KeyboardButtonRequestUser to KeyboardButtonRequestUsers and added the field max_quantity to it.
  • Renamed the field request_user in the class KeyboardButton to request_users. The old name will still work for backward compatibility.
  • Added the class UsersShared.
  • Replaced the field user_shared in the class Message with the field users_shared.

Chat Boost - @harshil21

Giveaway @Bibo-Joshi

  • Added the class Giveaway and the field giveaway to the class Message for messages about scheduled giveaways.
  • Added the class GiveawayCreated and the field giveaway_created to the class Message for service messages about the creation of a scheduled giveaway.
  • Added the class GiveawayWinners and the field giveaway_winners to the class Message for messages about the completion of a giveaway with public winners.
  • Added the class GiveawayCompleted and the field giveaway_completed to the class Message for service messages about the completion of a giveaway without public winners.

Web App Changes
- [ ] Added the field SettingsButton to the class WebApp.
- [ ] Added the fields header_bg_color, accent_text_color, section_bg_color, section_header_text_color, subtitle_text_color, destructive_text_color to the class ThemeParams.

Other Changes

  • Added support for the fields emoji_status_custom_emoji_id and emoji_status_expiration_date in the class Chat for non-private chats. @Bibo-Joshi
  • Added the fields accent_color_id, background_custom_emoji_id, profile_accent_color_id, and profile_background_custom_emoji_id to the class Chat. @Bibo-Joshi
  • Added the field has_visible_history to the class Chat. @Bibo-Joshi
  • Added the class MessageOrigin and replaced the fields forward_from, forward_from_chat, forward_from_message_id, forward_signature, forward_sender_name, and forward_date with the field forward_origin of type MessageOrigin in the class Message. @clot27
  • Improved documentation for the field message of the class callbackQuery and the field pinned_message of the class Message by adding the classes MaybeInaccessibleMessage and InaccessibleMessage. @Poolitzer

Describe alternatives you've considered

If you want to contribute, you can start from the Contribution Guide.

Additional context

Please comment below before start working on any part of the update so as to keep things in track and avoid double work.

@aelkheir
Copy link
Contributor

Hi! I'd love to work on reactions.

@harshil21
Copy link
Member

Hi! I'd love to work on reactions.

Hey! Thanks for your intent! Unfortunately, the reactions part has already been taken by @Poolitzer as is indicated beside the bullet point.

You may choose any other bullet points which haven't been taken yet

@Poolitzer Poolitzer pinned this issue Dec 29, 2023
@clot27 clot27 mentioned this issue Dec 29, 2023
26 tasks
@aelkheir
Copy link
Contributor

Okay. Then Block quotation.

  • Added support for “blockquote” entities in received messages.
  • Added support for “blockquote” entity parsing in “MarkdownV2” and “HTML” parse modes
  • Allowed to explicitly specify “blockquote” entities in formatted texts.

@Bibo-Joshi
Copy link
Member

Bibo-Joshi commented Dec 29, 2023

@clot27 A tew thoughts after reading the changelog:

• I guess we'll keep reply_to_message_id and allow_sending_without_reply around for backwards compatiblity for now. One can think of keeping them in the the long term as helper so that users don't have to build a ReplyParameters object. However, this would mean additional runtime checks for mutually exclusive arguments. We also need to think about what to do with all the Message.reply_* shortcuts: Should they just get additional arguments chat_id and for the quoted-text stuff or do we want users to supply those via an object as well? If we go for additional arguments, we need to figure out a way to avoid the name clash bewteen ReplyParamaters.quote and our own quote parameter. I guess we should rename the latter in any case to avoid confusion in the future.
• Handling of ext.Defaults for allow_sending_without_reply has to be adjusted and we need a logic for quote_parse_mode - it should use Defaults.parse_mode just like Defaults.explanation_parse_mode already does.
• for disable_web_page_preview, we need to adjust defaults handling as well and one can think about keeping it in the long term as convenience argument
• We'll need new enums in telegram.constants for the accent colors, similar to the existing telegram.constants.ForumIconColor (done)

@Bibo-Joshi
Copy link
Member

Bibo-Joshi commented Dec 31, 2023

Some insights for the reply_parameters:

  • quote_position is not an alternative to quote, but an optional addition for cases where the quoted text appears multiple times in the message. If you leave it out, the first ocurrence is highlighted when clicking on the reply (see https://t.me/pythontelegrambottalk/248433)
  • [allow_sending_without_reply] can be used only for replies in the same chat and forum topic. This has to be considered for inserting defaults, i.e. the specified default value must be inserted only if ReplyParameters.chat_id is not specified (I guess?)

@Bibo-Joshi
Copy link
Member

Bibo-Joshi commented Jan 1, 2024

More thoughts on reply_parameters and Message.reply_*

  • IMO it would be good if the default behavior of Message.reply_* would stay as is, i.e. send a standalone new message in private chats & a proper reply in all other chats.
  • ReplyParameters.chat_id is the chat_id where the replied to messages originates from. For Message.reply_* we already have that info available, so it would make more sense to allow the user to specify the target chat_id instead, i.e. the chat where the messages is supposed to be sent to
  • RP.message_id is the only mandatory argument of that class. However, that info is already available in Message.reply_*. So if we were to add the argument reply_parameters to Message.reply_*, it would be weird to have the RP.message_id parameter positional. One could make tg.RP.message_id optional and implement additional runtime checks in Bot.send_message or one could add something like tg.auxil.RP where message_id is optional, but neither one seems clean and both will probably lead to confusion
  • OTOH, if we add only the optional parameters of RP as direct parameters to Message.reply_*, we'll have a rather large discrepancy in the signatures of Bot.send_* and Message.reply_*, which I don't like
  • As stated in [FEATURE] API 7.0 #4033 (comment) I would vote to rename the quote argument of Message.reply_text and Defaults in any case to avoid confusion. Suggestions for alternative names: direct_reply, make_reply, do_quote

Independently of that, @harshil proposed a helper function for the Message class that helps to build the quote_* arguments. I guess this method should receive as arguments

  • The text to quote. Here one should discuss if we want to receive the plain text or if we want to allow receiving text including markup formatting (i.e. substrings of text_(html|markdown_2)). I would tend to allow only the first one b/c there could be weird situations like "<i>ABC</i> ABC", i.e. passing <i>ABC</i> wouldn't tell us if this is plain text or text including formatting
  • Optionally an argument that tells us which occurrence of the text should be used (in case the same substring appears multiple times). RP.quote_position expects a UTF-16 offset. This will be in general harder to compute for users, so I would suggest that we instead accept a simple index number here, i.e. passing "2" means "quote the 2nd occurence of the substring"

Initially I thought that computing the entities would be tricky, but I guess with quote_position and quote given, we have the offset & length of the quote so we only need to check which entities overlap and clip the MessageEntity.{offset, length} if necessary …
This method should search both Message.text and Message.caption.

Building up on Harshils idea, I had the thought that one could extend this method to something like Message.build_reply_arguments, i.e. a method that returns a typed-dict such that you can do bot.send_message(**update.effective_message.bulid_reply_arguments(…), …). This would also include the target chat id (as explained above) and allow_sending_without_reply (with handling as in #4033 (comment))


If we have such a method, this might also give other options to handle the Message.reply_* issue:

  • One could make the (do_)quote argument accept the output of Message.build_reply_arguments and merge the arguments internally
  • One could skip special new handling for the ReplyParameters in Message.reply_* and tell the users to do bot.send_message(**update.effective_message.bulid_reply_arguments(…), …) if they want to use convenience functionality. We could still add the parameter Message.reply_*.reply_parameters for completeness.

For interenal reference, let me link to a message in the dev chat about defaults handling: https://t.me/c/1494805131/39152


In summary, integrating the update into the existing PTB convenience functionality is unfortunately not straight forward. I would appreciate feedback from the dev team on my write-up.

@Poolitzer
Copy link
Member

IMO it would be good if the default behavior of Message.reply_* would stay as is, i.e. send a standalone new message in private chats & a proper reply in all other chats.

Agreed

ReplyParameters.chat_id is the chat_id where the replied to messages originates from. For Message.reply_* we already have that info available, so it would make more sense to allow the user to specify the target chat_id instead, i.e. the chat where the messages is supposed to be sent to

Into ReplyParameters? I don't like changing input classes like that that much, and would people actually use this?

OTOH, if we add only the optional parameters of RP as direct parameters to Message.reply_*, we'll have a rather large discrepancy in the signatures of Bot.send_* and Message.reply_*, which I don't like

I like this way more then the other ideas


Not allowing formatted strings, users might think there is only one occurrence of the text but there are several, just differentiated by formatting. So I would definitely also allow formatted strings. I like the index part.

not sure I see the upside of build_reply_arguments instead of simply passing them

@Bibo-Joshi
Copy link
Member

ReplyParameters.chat_id is the chat_id where the replied to messages originates from. For Message.reply_* we already have that info available, so it would make more sense to allow the user to specify the target chat_id instead, i.e. the chat where the messages is supposed to be sent to

Into ReplyParameters? I don't like changing input classes like that that much, and would people actually use this?

No, I rather had an additional argument for Message.reply_* in mind. Something like effective_message.reply_text(target_chat_id=123, reply_parameters=RP(quote="abc"))

Not allowing formatted strings, users might think there is only one occurrence of the text but there are several, just differentiated by formatting.

That's exactly my example though: "<i>ABC</i> ABC" contains the string "ABC" twice. The firts occurence is unformatted, the second one is formatted in italic. If the user now passes <i>ABC</i>, we still don't know which occurence they mean.

not sure I see the upside of build_reply_arguments instead of simply passing them

Sorry I don't understand. What do you mean by "simply passing them"?

@Poolitzer
Copy link
Member

No, I rather had an additional argument for Message.reply_* in mind. Something like effective_message.reply_text(target_chat_id=123, reply_parameters=RP(quote="abc"))

... but that is already there, as reply_text(chat_id=. Why do you want it again?

If the user now passes ABC, we still don't know which occurence they mean.

Ohhhh double the markdown, twice the fall. Yeah I see that. Okay I think non formatted text it is then, hope that doesn't confuse users too much...

Sorry I don't understand. What do you mean by "simply passing them"?

I get the feeling you are trying to solve a problem I am not seeing as one yet. What is the improvement for a user to have the builder instead of initiating the class like any other?


Btw I gave the "how do we avoid the double passing of message_id" some thought, and I think the best way is to make a class without the parameter, which can only be used in the shortcuts (so we raise if someone uses it in a non shortcut) and in the shortcuts we change it to the actual class.

@Bibo-Joshi
Copy link
Member

... but that is already there, as reply_text(chat_id=. Why do you want it again?

No, Message.reply_* doesn't have a chat_id argument at the moment. Not having to pass the chat_id (and message_id) has been the main selling point of these methods so far. The problem now is that we want to allow to send the reply not only to message.chat_id but to an entirely different chat, while still needing message.{chat_id, message_id} for ReplyParameters.{chat_id, message_id}.

Ohhhh double the markdown, twice the fall. Yeah I see that. Okay I think non formatted text it is then, hope that doesn't confuse users too much...

👍

I get the feeling you are trying to solve a problem I am not seeing as one yet. What is the improvement for a user to have the builder instead of initiating the class like any other?

  • If the helper method computes only the quote and quote_entities parameters, then the user will still have to manually build the RP object with this info, e.g. something like RP(message_id=message.message_id, **message.compute_quote("text")). But we already have most of the relevant information available within the message object, so IMO it would be a nicer user experience if I don't have to create the RP object manually - reply_parameters = message.build_rp(text="abc") would be better IMO
  • Additionally we have the distinction between "which chat is the replied to message in" vs "which chat do we send the reply to" and - depending on that - the possible values for allow_sending_without_reply. If the helper method computes only the quote and quote_entities parameters, the user will have to handle those by themself. With a Message.build_reply_arguments, this burden could be taken from them:

User has set Defaults.allow_sending_without_reply = True

message.bulid_reply_arguments(text="abc"){"chat_id": message.chat_id, "reply_parameters": RP(message_id=message.message_id, allow_sending_without_reply=True, quote="text", quote_entities=[…])}

RP.aswr not present b/c not allowed, also mind the chat_id values
message.bulid_reply_arguments(text="abc", target_chat_id=123){"chat_id": 123, "reply_parameters": RP(message_id=message.message_id, chat_id=message.chat_id, quote="text", quote_entities=[…])}

Btw I gave the "how do we avoid the double passing of message_id" some thought, and I think the best way is to make a class without the parameter, which can only be used in the shortcuts (so we raise if someone uses it in a non shortcut) and in the shortcuts we change it to the actual class.

This would be my 3rd bullet point in #4033 (comment). the problem that I see with this approach is that it can cause a lot of confusion about which class to use where. Moreover, it's unclear to me where we could cleanly place such a class within the telegram module (it can't be telegram.ext. b/c Message.reply_text should work in ptb-raw as well). And it causes more maintenance overhead.

@Bibo-Joshi
Copy link
Member

Provided a PoC implementation draft in #4058

@dikirilov

This comment was marked as off-topic.

@Poolitzer
Copy link
Member

hi, please make a separate issue for that, don't comment in the one for API 7.0 changes

@Bibo-Joshi Bibo-Joshi unpinned this issue Feb 7, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Feb 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants