From ee009c09986aebae3ffef7e04190f80f50cc452e Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Sun, 16 Oct 2022 15:49:18 +0300 Subject: [PATCH 01/24] feat(`Bot`): add shortcut to pass caption to media group closes #3185 add 3 keyword-only arguments to pass caption to entire media group before that a caption for the group had to be manually added to the 1st media item in the group --- AUTHORS.rst | 1 + telegram/_bot.py | 40 +++++++++++++++++++++++++++++- telegram/_chat.py | 6 +++++ telegram/_message.py | 6 +++++ telegram/_user.py | 6 +++++ telegram/ext/_extbot.py | 17 +++++++++++++ tests/test_inputmedia.py | 53 ++++++++++++++++++++++++++++++++++++++++ 7 files changed, 128 insertions(+), 1 deletion(-) diff --git a/AUTHORS.rst b/AUTHORS.rst index bb118e4c7d3..1efaccec0e5 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -34,6 +34,7 @@ The following wonderful people contributed directly or indirectly to this projec - `daimajia `_ - `Daniel Reed `_ - `D David Livingston `_ +- `Dmitry Kolomatskiy `_ - `DonalDuck004 `_ - `Eana Hufwe `_ - `Ehsan Online `_ diff --git a/telegram/_bot.py b/telegram/_bot.py index cb4e00c6b77..abbc71ca39f 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -19,6 +19,7 @@ # along with this program. If not, see [http://www.gnu.org/licenses/]. """This module contains an object that represents a Telegram Bot.""" import asyncio +import copy import functools import logging import pickle @@ -1998,9 +1999,22 @@ async def send_media_group( connect_timeout: ODVInput[float] = DEFAULT_NONE, pool_timeout: ODVInput[float] = DEFAULT_NONE, api_kwargs: JSONDict = None, + group_caption: Optional[str] = None, + # Notes on group_caption_parse_mode: + # 1. To be fully consistent with media items it would be called + # `group_parse_mode`, but that is ambiguous. + # 2. Typing ODVInput[str] = DEFAULT_NONE will make tests for shortcuts fail + # (check_defaults_handling) despite it being used for parse mode for individual captions. + group_caption_parse_mode: Optional[str] = None, + group_caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, ) -> List[Message]: """Use this method to send a group of photos or videos as an album. + Note: + If you supply a :paramref:`group_caption` (along with either + :paramref:`group_parse_mode` or :paramref:`group_caption_entities`), + then items in :paramref:`media` must have no captions, and vice verca. + .. seealso:: :attr:`telegram.Message.reply_media_group`, :attr:`telegram.Chat.send_media_group`, :attr:`telegram.User.send_media_group` @@ -2037,6 +2051,14 @@ async def send_media_group( :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. api_kwargs (:obj:`dict`, optional): Arbitrary keyword arguments to be passed to the Telegram API. + group_caption (:obj:`str`, optional): Text of the caption + that will be shown for the whole media group. + Defaults to :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. + group_caption_parse_mode (:obj:`str` | :obj:`None`, optional): + Parse mode of the caption that will be shown for the whole media group. + Defaults to :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. + group_caption_entities (List[:class:`telegram.MessageEntity`], optional): + List of special entities that appear in the group caption. Returns: List[:class:`telegram.Message`]: An array of the sent Messages. @@ -2044,9 +2066,25 @@ async def send_media_group( Raises: :class:`telegram.error.TelegramError` """ + if group_caption and any(item.caption for item in media): + raise ValueError("You can only supply either group caption or images with captions.") + + # First object could be mutated if group caption has to be applied to it, + # so it's best to copy media to avoid side effects. + # Shallow copy of every media item should be enough. + copied_media = [copy.copy(item) for item in media] + + if group_caption: + # Apply group caption to the first media item. + # This will lead to the group being shown with this caption. + first_item = copied_media[0] + first_item.caption = group_caption + first_item.parse_mode = group_caption_parse_mode + first_item.caption_entities = group_caption_entities + data: JSONDict = { "chat_id": chat_id, - "media": media, + "media": copied_media, "disable_notification": disable_notification, "allow_sending_without_reply": allow_sending_without_reply, "protect_content": protect_content, diff --git a/telegram/_chat.py b/telegram/_chat.py index 39bce15e087..f6a1e35927b 100644 --- a/telegram/_chat.py +++ b/telegram/_chat.py @@ -1129,6 +1129,9 @@ async def send_media_group( connect_timeout: ODVInput[float] = DEFAULT_NONE, pool_timeout: ODVInput[float] = DEFAULT_NONE, api_kwargs: JSONDict = None, + group_caption: Optional[str] = None, + group_caption_parse_mode: Optional[str] = None, + group_caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, ) -> List["Message"]: """Shortcut for:: @@ -1152,6 +1155,9 @@ async def send_media_group( api_kwargs=api_kwargs, allow_sending_without_reply=allow_sending_without_reply, protect_content=protect_content, + group_caption=group_caption, + group_caption_parse_mode=group_caption_parse_mode, + group_caption_entities=group_caption_entities, ) async def send_chat_action( diff --git a/telegram/_message.py b/telegram/_message.py index cf43b456ac6..01e5f2ded34 100644 --- a/telegram/_message.py +++ b/telegram/_message.py @@ -1010,6 +1010,9 @@ async def reply_media_group( connect_timeout: ODVInput[float] = DEFAULT_NONE, pool_timeout: ODVInput[float] = DEFAULT_NONE, api_kwargs: JSONDict = None, + group_caption: Optional[str] = None, + group_caption_parse_mode: Optional[str] = None, + group_caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, ) -> List["Message"]: """Shortcut for:: @@ -1042,6 +1045,9 @@ async def reply_media_group( api_kwargs=api_kwargs, allow_sending_without_reply=allow_sending_without_reply, protect_content=protect_content, + group_caption=group_caption, + group_caption_parse_mode=group_caption_parse_mode, + group_caption_entities=group_caption_entities, ) async def reply_photo( diff --git a/telegram/_user.py b/telegram/_user.py index 52633d9b379..b3a3b2962b9 100644 --- a/telegram/_user.py +++ b/telegram/_user.py @@ -474,6 +474,9 @@ async def send_media_group( connect_timeout: ODVInput[float] = DEFAULT_NONE, pool_timeout: ODVInput[float] = DEFAULT_NONE, api_kwargs: JSONDict = None, + group_caption: Optional[str] = None, + group_caption_parse_mode: Optional[str] = None, + group_caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, ) -> List["Message"]: """Shortcut for:: @@ -497,6 +500,9 @@ async def send_media_group( api_kwargs=api_kwargs, allow_sending_without_reply=allow_sending_without_reply, protect_content=protect_content, + group_caption=group_caption, + group_caption_parse_mode=group_caption_parse_mode, + group_caption_entities=group_caption_entities, ) async def send_audio( diff --git a/telegram/ext/_extbot.py b/telegram/ext/_extbot.py index 246fefb38db..7474b0560cf 100644 --- a/telegram/ext/_extbot.py +++ b/telegram/ext/_extbot.py @@ -2242,7 +2242,21 @@ async def send_media_group( pool_timeout: ODVInput[float] = DEFAULT_NONE, api_kwargs: JSONDict = None, rate_limit_args: RLARGS = None, + group_caption: Optional[str] = None, + group_caption_parse_mode: Optional[str] = None, + group_caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, ) -> List[Message]: + """Shortcut for:: + + await bot.send_media_group(update.effective_chat.id, *args, **kwargs) + + For the documentation of the arguments, please see :meth:`telegram.Bot.send_media_group`. + + Returns: + List[:class:`telegram.Message`]: On success, instance representing the message posted. + + """ + return await super().send_media_group( chat_id=chat_id, media=media, @@ -2255,6 +2269,9 @@ async def send_media_group( connect_timeout=connect_timeout, pool_timeout=pool_timeout, api_kwargs=self._merge_api_rl_kwargs(api_kwargs, rate_limit_args), + group_caption=group_caption, + group_caption_parse_mode=group_caption_parse_mode, + group_caption_entities=group_caption_entities, ) async def send_message( diff --git a/tests/test_inputmedia.py b/tests/test_inputmedia.py index d32ca943e87..58d16bd38ce 100644 --- a/tests/test_inputmedia.py +++ b/tests/test_inputmedia.py @@ -443,6 +443,59 @@ async def test_send_media_group_photo(self, bot, chat_id, media_group): mes.caption_entities == [MessageEntity(MessageEntity.BOLD, 0, 5)] for mes in messages ) + @flaky(3, 1) + async def test_send_media_group_throws_error_with_group_caption_and_individual_captions( + self, bot, chat_id, media_group + ): + with pytest.raises( + ValueError, match="You can only supply either group caption or images with captions." + ): + await bot.send_media_group(chat_id, media_group, group_caption="foo") + + @pytest.mark.parametrize( + "group_caption, group_caption_parse_mode, group_caption_entities", + [ + # same combinations of caption options as in media_group fixture + ("*photo* 1", "Markdown", None), + ("photo 1", "HTML", None), + ("photo 1", None, [MessageEntity(MessageEntity.BOLD, 0, 5)]), + ], + ) + @flaky(3, 1) + async def test_send_media_group_with_group_caption( + self, + bot, + chat_id, + media_group, + group_caption, + group_caption_parse_mode, + group_caption_entities, + ): + # clear media_group of all_captions + + for item in media_group: + item.caption = None + item.parse_mode = None + item.caption_entities = None + + messages = await bot.send_media_group( + chat_id, + media_group, + group_caption=group_caption, + group_caption_parse_mode=group_caption_parse_mode, + group_caption_entities=group_caption_entities, + ) + assert isinstance(messages, list) + assert len(messages) == 3 + assert all(isinstance(mes, Message) for mes in messages) + assert all(mes.media_group_id == messages[0].media_group_id for mes in messages) + + # Make sure first message got the caption, which will lead + # to Telegram displaying its caption as group caption + first_message = messages[0] + assert first_message.caption + assert first_message.caption_entities == [MessageEntity(MessageEntity.BOLD, 0, 5)] + @flaky(3, 1) async def test_send_media_group_all_args(self, bot, chat_id, media_group): m1 = await bot.send_message(chat_id, text="test") From 07b175eae7341ccb6ddf0dd12a1cd03ce3b6c3e0 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Sun, 16 Oct 2022 16:21:51 +0300 Subject: [PATCH 02/24] minor fix(`ExtBot`): remove blank line after docsting --- telegram/ext/_extbot.py | 1 - 1 file changed, 1 deletion(-) diff --git a/telegram/ext/_extbot.py b/telegram/ext/_extbot.py index 7474b0560cf..a3edeacb5e1 100644 --- a/telegram/ext/_extbot.py +++ b/telegram/ext/_extbot.py @@ -2256,7 +2256,6 @@ async def send_media_group( List[:class:`telegram.Message`]: On success, instance representing the message posted. """ - return await super().send_media_group( chat_id=chat_id, media=media, From 0cdfd920cc6904e07c1c78dad8399104a907f1c6 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Mon, 17 Oct 2022 00:40:22 +0300 Subject: [PATCH 03/24] fix(`test_official.py`) add convenience arguments to `ignored` "group_caption", "group_caption_parse_mode", and "group_caption_entities" are not part of API, so exclude them from check of arguments matching API --- tests/test_official.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_official.py b/tests/test_official.py index 798c4a9bdda..0748cd0b67d 100644 --- a/tests/test_official.py +++ b/tests/test_official.py @@ -118,6 +118,9 @@ def check_method(h4): ignored |= {"venue"} # Added for ease of use elif name == "answerInlineQuery": ignored |= {"current_offset"} # Added for ease of use + elif name == "sendMediaGroup": + # Added for ease of use + ignored |= {"group_caption", "group_caption_parse_mode", "group_caption_entities"} assert (sig.parameters.keys() ^ checked) - ignored == set() From 3ff6f19d1929f9140d108d3c5df339266693d9fb Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Tue, 18 Oct 2022 17:39:08 +0300 Subject: [PATCH 04/24] fix(`test_inputmedia.py`) remove `flaky` from test checking ValueError --- tests/test_inputmedia.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_inputmedia.py b/tests/test_inputmedia.py index 58d16bd38ce..59ce19f1b0a 100644 --- a/tests/test_inputmedia.py +++ b/tests/test_inputmedia.py @@ -443,7 +443,6 @@ async def test_send_media_group_photo(self, bot, chat_id, media_group): mes.caption_entities == [MessageEntity(MessageEntity.BOLD, 0, 5)] for mes in messages ) - @flaky(3, 1) async def test_send_media_group_throws_error_with_group_caption_and_individual_captions( self, bot, chat_id, media_group ): From 5af855f5c211f0dae66a4b3e7f97b49c3e10eccb Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Tue, 18 Oct 2022 17:50:25 +0300 Subject: [PATCH 05/24] fix(`send_media_group`) change `ValueError` message, add caption checks - replace "images" with "media" - check not only .caption but also .caption_entities and .parse_mode of individual media items --- telegram/_bot.py | 12 +++++++++--- tests/test_inputmedia.py | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/telegram/_bot.py b/telegram/_bot.py index abbc71ca39f..969cc2c57fa 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -2012,7 +2012,7 @@ async def send_media_group( Note: If you supply a :paramref:`group_caption` (along with either - :paramref:`group_parse_mode` or :paramref:`group_caption_entities`), + :paramref:`group_caption_parse_mode` or :paramref:`group_caption_entities`), then items in :paramref:`media` must have no captions, and vice verca. .. seealso:: :attr:`telegram.Message.reply_media_group`, @@ -2066,8 +2066,14 @@ async def send_media_group( Raises: :class:`telegram.error.TelegramError` """ - if group_caption and any(item.caption for item in media): - raise ValueError("You can only supply either group caption or images with captions.") + if group_caption and any( + [ + any(item.caption for item in media), + any(item.caption_entities for item in media), + any(item.parse_mode is DEFAULT_NONE for item in media), + ] + ): + raise ValueError("You can only supply either group caption or media with captions.") # First object could be mutated if group caption has to be applied to it, # so it's best to copy media to avoid side effects. diff --git a/tests/test_inputmedia.py b/tests/test_inputmedia.py index 59ce19f1b0a..9e7bc64ac57 100644 --- a/tests/test_inputmedia.py +++ b/tests/test_inputmedia.py @@ -447,7 +447,7 @@ async def test_send_media_group_throws_error_with_group_caption_and_individual_c self, bot, chat_id, media_group ): with pytest.raises( - ValueError, match="You can only supply either group caption or images with captions." + ValueError, match="You can only supply either group caption or media with captions." ): await bot.send_media_group(chat_id, media_group, group_caption="foo") From af61aac0fc1e6869241c79d986c91c8fd4617163 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Tue, 18 Oct 2022 18:16:40 +0300 Subject: [PATCH 06/24] fix(`test_send_media_group`) check media except for 1st have no captions --- tests/test_inputmedia.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/test_inputmedia.py b/tests/test_inputmedia.py index 9e7bc64ac57..62dfab20a0b 100644 --- a/tests/test_inputmedia.py +++ b/tests/test_inputmedia.py @@ -471,7 +471,6 @@ async def test_send_media_group_with_group_caption( group_caption_entities, ): # clear media_group of all_captions - for item in media_group: item.caption = None item.parse_mode = None @@ -487,14 +486,21 @@ async def test_send_media_group_with_group_caption( assert isinstance(messages, list) assert len(messages) == 3 assert all(isinstance(mes, Message) for mes in messages) - assert all(mes.media_group_id == messages[0].media_group_id for mes in messages) + + first_message, other_messages = messages[0], messages[1:] + assert all(mes.media_group_id == first_message.media_group_id for mes in messages) # Make sure first message got the caption, which will lead # to Telegram displaying its caption as group caption - first_message = messages[0] assert first_message.caption assert first_message.caption_entities == [MessageEntity(MessageEntity.BOLD, 0, 5)] + # Check that other messages have no captions + assert all(mes.caption is None for mes in other_messages) + assert not any(mes.caption_entities for mes in other_messages) + # .parse_mode must be completely absent + assert not any(getattr(mes, "parse_mode", None) for mes in other_messages) + @flaky(3, 1) async def test_send_media_group_all_args(self, bot, chat_id, media_group): m1 = await bot.send_message(chat_id, text="test") From 20c89ddf714319c21780279445e5b4ca74f19bf6 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Tue, 18 Oct 2022 18:29:37 +0300 Subject: [PATCH 07/24] fix(`send_media_group`) add `:paramref:` to docstring, tweak text --- telegram/_bot.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/telegram/_bot.py b/telegram/_bot.py index 969cc2c57fa..d05896e54b2 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -2051,14 +2051,15 @@ async def send_media_group( :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. api_kwargs (:obj:`dict`, optional): Arbitrary keyword arguments to be passed to the Telegram API. - group_caption (:obj:`str`, optional): Text of the caption - that will be shown for the whole media group. + group_caption (:obj:`str`, optional): Caption that will be added to the + first element of :paramref:`media`, so that it will be used as caption for the + whole media group. Defaults to :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. group_caption_parse_mode (:obj:`str` | :obj:`None`, optional): - Parse mode of the caption that will be shown for the whole media group. + Parse mode for :paramref:`group_caption`. Defaults to :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. group_caption_entities (List[:class:`telegram.MessageEntity`], optional): - List of special entities that appear in the group caption. + List of special entities for :paramref:`group_caption`. Returns: List[:class:`telegram.Message`]: An array of the sent Messages. From 063b2daba79da3cd1f99eadd337dc0d820c6e7b2 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Tue, 18 Oct 2022 18:34:07 +0300 Subject: [PATCH 08/24] fix(`_extbot.py`) remove docstring for `send_media_group()` --- telegram/ext/_extbot.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/telegram/ext/_extbot.py b/telegram/ext/_extbot.py index a3edeacb5e1..10027ca86f8 100644 --- a/telegram/ext/_extbot.py +++ b/telegram/ext/_extbot.py @@ -2246,16 +2246,6 @@ async def send_media_group( group_caption_parse_mode: Optional[str] = None, group_caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, ) -> List[Message]: - """Shortcut for:: - - await bot.send_media_group(update.effective_chat.id, *args, **kwargs) - - For the documentation of the arguments, please see :meth:`telegram.Bot.send_media_group`. - - Returns: - List[:class:`telegram.Message`]: On success, instance representing the message posted. - - """ return await super().send_media_group( chat_id=chat_id, media=media, From e4b7971164c4c08aab6920401bae0bd74397dcf7 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Tue, 18 Oct 2022 18:50:27 +0300 Subject: [PATCH 09/24] fix(`send_media_group`) fix `Defaults`, enhance docstring --- telegram/_bot.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/telegram/_bot.py b/telegram/_bot.py index d05896e54b2..1a500a054e5 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -2054,12 +2054,16 @@ async def send_media_group( group_caption (:obj:`str`, optional): Caption that will be added to the first element of :paramref:`media`, so that it will be used as caption for the whole media group. - Defaults to :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. + Defaults to :obj:`None`. group_caption_parse_mode (:obj:`str` | :obj:`None`, optional): Parse mode for :paramref:`group_caption`. - Defaults to :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. + See the constants in :class:`telegram.constants.ParseMode` for the + available modes. + Defaults to :obj:`None`. group_caption_entities (List[:class:`telegram.MessageEntity`], optional): - List of special entities for :paramref:`group_caption`. + List of special entities for :paramref:`group_caption`, + which can be specified instead of :paramref:`group_caption_parse_mode`. + Defaults to :obj:`None`. Returns: List[:class:`telegram.Message`]: An array of the sent Messages. From c38801fab32cc0fad40c07ef176fef00d98ca2f8 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Tue, 18 Oct 2022 19:47:35 +0300 Subject: [PATCH 10/24] refactor(`send_media_group`) copy just one media item if necessary we should try to copy as little as possible: if none of the `group_*` parameters was passed, we don't need copying. Otherwise, we only need to copy (the list and) the first media item rather than all of them --- telegram/_bot.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/telegram/_bot.py b/telegram/_bot.py index 1a500a054e5..64f6a4e9139 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -2080,22 +2080,23 @@ async def send_media_group( ): raise ValueError("You can only supply either group caption or media with captions.") - # First object could be mutated if group caption has to be applied to it, - # so it's best to copy media to avoid side effects. - # Shallow copy of every media item should be enough. - copied_media = [copy.copy(item) for item in media] - + changed_media = None if group_caption: # Apply group caption to the first media item. # This will lead to the group being shown with this caption. - first_item = copied_media[0] - first_item.caption = group_caption - first_item.parse_mode = group_caption_parse_mode - first_item.caption_entities = group_caption_entities + # Copy first item to avoid mutation of original object + item_to_get_caption = copy.copy(media[0]) + item_to_get_caption.caption = group_caption + item_to_get_caption.parse_mode = group_caption_parse_mode + item_to_get_caption.caption_entities = group_caption_entities + + # copy the list (just the references) to avoid mutating the original list + changed_media = media[:] + changed_media[0] = item_to_get_caption data: JSONDict = { "chat_id": chat_id, - "media": copied_media, + "media": changed_media if group_caption else media, "disable_notification": disable_notification, "allow_sending_without_reply": allow_sending_without_reply, "protect_content": protect_content, From 61af4f491fe5390b3f9e30ac28553366531af759 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Wed, 19 Oct 2022 13:16:33 +0300 Subject: [PATCH 11/24] refactor(`send_media_group`) rename caption arguments rename to match arguments of functions that handle individual media: - group_caption -> caption - group_caption_parse_mode -> parse_mode - group_caption_entities -> caption_entities --- telegram/_bot.py | 40 +++++++++++++++++++--------------------- telegram/_chat.py | 12 ++++++------ telegram/_message.py | 12 ++++++------ telegram/_user.py | 12 ++++++------ telegram/ext/_extbot.py | 12 ++++++------ tests/test_inputmedia.py | 16 ++++++++-------- tests/test_official.py | 2 +- 7 files changed, 52 insertions(+), 54 deletions(-) diff --git a/telegram/_bot.py b/telegram/_bot.py index 64f6a4e9139..f949500ecf3 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -1999,20 +1999,18 @@ async def send_media_group( connect_timeout: ODVInput[float] = DEFAULT_NONE, pool_timeout: ODVInput[float] = DEFAULT_NONE, api_kwargs: JSONDict = None, - group_caption: Optional[str] = None, - # Notes on group_caption_parse_mode: - # 1. To be fully consistent with media items it would be called - # `group_parse_mode`, but that is ambiguous. - # 2. Typing ODVInput[str] = DEFAULT_NONE will make tests for shortcuts fail + caption: Optional[str] = None, + # Note on parse_mode: + # Typing ODVInput[str] = DEFAULT_NONE will make tests for shortcuts fail # (check_defaults_handling) despite it being used for parse mode for individual captions. - group_caption_parse_mode: Optional[str] = None, - group_caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, + parse_mode: Optional[str] = None, + caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, ) -> List[Message]: """Use this method to send a group of photos or videos as an album. Note: - If you supply a :paramref:`group_caption` (along with either - :paramref:`group_caption_parse_mode` or :paramref:`group_caption_entities`), + If you supply a :paramref:`caption` (along with either + :paramref:`parse_mode` or :paramref:`caption_entities`), then items in :paramref:`media` must have no captions, and vice verca. .. seealso:: :attr:`telegram.Message.reply_media_group`, @@ -2051,18 +2049,18 @@ async def send_media_group( :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`. api_kwargs (:obj:`dict`, optional): Arbitrary keyword arguments to be passed to the Telegram API. - group_caption (:obj:`str`, optional): Caption that will be added to the + caption (:obj:`str`, optional): Caption that will be added to the first element of :paramref:`media`, so that it will be used as caption for the whole media group. Defaults to :obj:`None`. - group_caption_parse_mode (:obj:`str` | :obj:`None`, optional): - Parse mode for :paramref:`group_caption`. + parse_mode (:obj:`str` | :obj:`None`, optional): + Parse mode for :paramref:`caption`. See the constants in :class:`telegram.constants.ParseMode` for the available modes. Defaults to :obj:`None`. - group_caption_entities (List[:class:`telegram.MessageEntity`], optional): - List of special entities for :paramref:`group_caption`, - which can be specified instead of :paramref:`group_caption_parse_mode`. + caption_entities (List[:class:`telegram.MessageEntity`], optional): + List of special entities for :paramref:`caption`, + which can be specified instead of :paramref:`parse_mode`. Defaults to :obj:`None`. Returns: @@ -2071,7 +2069,7 @@ async def send_media_group( Raises: :class:`telegram.error.TelegramError` """ - if group_caption and any( + if caption and any( [ any(item.caption for item in media), any(item.caption_entities for item in media), @@ -2081,14 +2079,14 @@ async def send_media_group( raise ValueError("You can only supply either group caption or media with captions.") changed_media = None - if group_caption: + if caption: # Apply group caption to the first media item. # This will lead to the group being shown with this caption. # Copy first item to avoid mutation of original object item_to_get_caption = copy.copy(media[0]) - item_to_get_caption.caption = group_caption - item_to_get_caption.parse_mode = group_caption_parse_mode - item_to_get_caption.caption_entities = group_caption_entities + item_to_get_caption.caption = caption + item_to_get_caption.parse_mode = parse_mode + item_to_get_caption.caption_entities = caption_entities # copy the list (just the references) to avoid mutating the original list changed_media = media[:] @@ -2096,7 +2094,7 @@ async def send_media_group( data: JSONDict = { "chat_id": chat_id, - "media": changed_media if group_caption else media, + "media": changed_media if caption else media, "disable_notification": disable_notification, "allow_sending_without_reply": allow_sending_without_reply, "protect_content": protect_content, diff --git a/telegram/_chat.py b/telegram/_chat.py index f6a1e35927b..f762ca7d1e4 100644 --- a/telegram/_chat.py +++ b/telegram/_chat.py @@ -1129,9 +1129,9 @@ async def send_media_group( connect_timeout: ODVInput[float] = DEFAULT_NONE, pool_timeout: ODVInput[float] = DEFAULT_NONE, api_kwargs: JSONDict = None, - group_caption: Optional[str] = None, - group_caption_parse_mode: Optional[str] = None, - group_caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, + caption: Optional[str] = None, + parse_mode: Optional[str] = None, + caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, ) -> List["Message"]: """Shortcut for:: @@ -1155,9 +1155,9 @@ async def send_media_group( api_kwargs=api_kwargs, allow_sending_without_reply=allow_sending_without_reply, protect_content=protect_content, - group_caption=group_caption, - group_caption_parse_mode=group_caption_parse_mode, - group_caption_entities=group_caption_entities, + caption=caption, + parse_mode=parse_mode, + caption_entities=caption_entities, ) async def send_chat_action( diff --git a/telegram/_message.py b/telegram/_message.py index 01e5f2ded34..9dac0021158 100644 --- a/telegram/_message.py +++ b/telegram/_message.py @@ -1010,9 +1010,9 @@ async def reply_media_group( connect_timeout: ODVInput[float] = DEFAULT_NONE, pool_timeout: ODVInput[float] = DEFAULT_NONE, api_kwargs: JSONDict = None, - group_caption: Optional[str] = None, - group_caption_parse_mode: Optional[str] = None, - group_caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, + caption: Optional[str] = None, + parse_mode: Optional[str] = None, + caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, ) -> List["Message"]: """Shortcut for:: @@ -1045,9 +1045,9 @@ async def reply_media_group( api_kwargs=api_kwargs, allow_sending_without_reply=allow_sending_without_reply, protect_content=protect_content, - group_caption=group_caption, - group_caption_parse_mode=group_caption_parse_mode, - group_caption_entities=group_caption_entities, + caption=caption, + parse_mode=parse_mode, + caption_entities=caption_entities, ) async def reply_photo( diff --git a/telegram/_user.py b/telegram/_user.py index b3a3b2962b9..661a18edf43 100644 --- a/telegram/_user.py +++ b/telegram/_user.py @@ -474,9 +474,9 @@ async def send_media_group( connect_timeout: ODVInput[float] = DEFAULT_NONE, pool_timeout: ODVInput[float] = DEFAULT_NONE, api_kwargs: JSONDict = None, - group_caption: Optional[str] = None, - group_caption_parse_mode: Optional[str] = None, - group_caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, + caption: Optional[str] = None, + parse_mode: Optional[str] = None, + caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, ) -> List["Message"]: """Shortcut for:: @@ -500,9 +500,9 @@ async def send_media_group( api_kwargs=api_kwargs, allow_sending_without_reply=allow_sending_without_reply, protect_content=protect_content, - group_caption=group_caption, - group_caption_parse_mode=group_caption_parse_mode, - group_caption_entities=group_caption_entities, + caption=caption, + parse_mode=parse_mode, + caption_entities=caption_entities, ) async def send_audio( diff --git a/telegram/ext/_extbot.py b/telegram/ext/_extbot.py index 10027ca86f8..2f1c1475f00 100644 --- a/telegram/ext/_extbot.py +++ b/telegram/ext/_extbot.py @@ -2242,9 +2242,9 @@ async def send_media_group( pool_timeout: ODVInput[float] = DEFAULT_NONE, api_kwargs: JSONDict = None, rate_limit_args: RLARGS = None, - group_caption: Optional[str] = None, - group_caption_parse_mode: Optional[str] = None, - group_caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, + caption: Optional[str] = None, + parse_mode: Optional[str] = None, + caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, ) -> List[Message]: return await super().send_media_group( chat_id=chat_id, @@ -2258,9 +2258,9 @@ async def send_media_group( connect_timeout=connect_timeout, pool_timeout=pool_timeout, api_kwargs=self._merge_api_rl_kwargs(api_kwargs, rate_limit_args), - group_caption=group_caption, - group_caption_parse_mode=group_caption_parse_mode, - group_caption_entities=group_caption_entities, + caption=caption, + parse_mode=parse_mode, + caption_entities=caption_entities, ) async def send_message( diff --git a/tests/test_inputmedia.py b/tests/test_inputmedia.py index 62dfab20a0b..739269232fe 100644 --- a/tests/test_inputmedia.py +++ b/tests/test_inputmedia.py @@ -449,10 +449,10 @@ async def test_send_media_group_throws_error_with_group_caption_and_individual_c with pytest.raises( ValueError, match="You can only supply either group caption or media with captions." ): - await bot.send_media_group(chat_id, media_group, group_caption="foo") + await bot.send_media_group(chat_id, media_group, caption="foo") @pytest.mark.parametrize( - "group_caption, group_caption_parse_mode, group_caption_entities", + "caption, parse_mode, caption_entities", [ # same combinations of caption options as in media_group fixture ("*photo* 1", "Markdown", None), @@ -466,9 +466,9 @@ async def test_send_media_group_with_group_caption( bot, chat_id, media_group, - group_caption, - group_caption_parse_mode, - group_caption_entities, + caption, + parse_mode, + caption_entities, ): # clear media_group of all_captions for item in media_group: @@ -479,9 +479,9 @@ async def test_send_media_group_with_group_caption( messages = await bot.send_media_group( chat_id, media_group, - group_caption=group_caption, - group_caption_parse_mode=group_caption_parse_mode, - group_caption_entities=group_caption_entities, + caption=caption, + parse_mode=parse_mode, + caption_entities=caption_entities, ) assert isinstance(messages, list) assert len(messages) == 3 diff --git a/tests/test_official.py b/tests/test_official.py index 0748cd0b67d..9ac1907e21f 100644 --- a/tests/test_official.py +++ b/tests/test_official.py @@ -120,7 +120,7 @@ def check_method(h4): ignored |= {"current_offset"} # Added for ease of use elif name == "sendMediaGroup": # Added for ease of use - ignored |= {"group_caption", "group_caption_parse_mode", "group_caption_entities"} + ignored |= {"caption", "parse_mode", "caption_entities"} assert (sig.parameters.keys() ^ checked) - ignored == set() From 7643dc6633fd82ab96ef9f1bd7490ddb84b7662b Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Wed, 19 Oct 2022 19:49:37 +0300 Subject: [PATCH 12/24] refactor(`send_media_group`) remove redundant auxiliary list variable --- telegram/_bot.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/telegram/_bot.py b/telegram/_bot.py index f949500ecf3..6c7f7b32982 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -2078,7 +2078,6 @@ async def send_media_group( ): raise ValueError("You can only supply either group caption or media with captions.") - changed_media = None if caption: # Apply group caption to the first media item. # This will lead to the group being shown with this caption. @@ -2089,12 +2088,12 @@ async def send_media_group( item_to_get_caption.caption_entities = caption_entities # copy the list (just the references) to avoid mutating the original list - changed_media = media[:] - changed_media[0] = item_to_get_caption + media = media.copy() + media[0] = item_to_get_caption data: JSONDict = { "chat_id": chat_id, - "media": changed_media if caption else media, + "media": media, "disable_notification": disable_notification, "allow_sending_without_reply": allow_sending_without_reply, "protect_content": protect_content, From 7a6ffc2805727cc8521894a3f057dc56bacbcfb8 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Wed, 19 Oct 2022 19:56:23 +0300 Subject: [PATCH 13/24] fix(`test_send_media_group`) remove redundant check for no attribute --- tests/test_inputmedia.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_inputmedia.py b/tests/test_inputmedia.py index 739269232fe..e84bbe63d5c 100644 --- a/tests/test_inputmedia.py +++ b/tests/test_inputmedia.py @@ -498,8 +498,6 @@ async def test_send_media_group_with_group_caption( # Check that other messages have no captions assert all(mes.caption is None for mes in other_messages) assert not any(mes.caption_entities for mes in other_messages) - # .parse_mode must be completely absent - assert not any(getattr(mes, "parse_mode", None) for mes in other_messages) @flaky(3, 1) async def test_send_media_group_all_args(self, bot, chat_id, media_group): From cb8fef64801309637fed4f57e8b36987b04393b8 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Wed, 19 Oct 2022 20:47:01 +0300 Subject: [PATCH 14/24] fix(`test_send_media_group`) check that method caused no side effects make sure the original media group is intact --- tests/test_inputmedia.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test_inputmedia.py b/tests/test_inputmedia.py index e84bbe63d5c..7bd6cccbb8f 100644 --- a/tests/test_inputmedia.py +++ b/tests/test_inputmedia.py @@ -470,6 +470,9 @@ async def test_send_media_group_with_group_caption( parse_mode, caption_entities, ): + # prepare a copy to check later on if calling the method has caused side effects + copied_media_group = media_group.copy() + # clear media_group of all_captions for item in media_group: item.caption = None @@ -483,6 +486,13 @@ async def test_send_media_group_with_group_caption( parse_mode=parse_mode, caption_entities=caption_entities, ) + + # Check that the method had no side effects: + # original group was not changed and 1st item still points to the same object + # (1st item must be copied within the method before adding the caption) + assert media_group == copied_media_group + assert media_group[0] is copied_media_group[0] + assert isinstance(messages, list) assert len(messages) == 3 assert all(isinstance(mes, Message) for mes in messages) From dac8359cb86199226e017060d72cd4f7cbfe7c15 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Wed, 19 Oct 2022 20:56:51 +0300 Subject: [PATCH 15/24] fix(`send_media_group`) fix check of `parse_mode` of individual item --- telegram/_bot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/telegram/_bot.py b/telegram/_bot.py index 6c7f7b32982..ea454f4aeda 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -2073,7 +2073,7 @@ async def send_media_group( [ any(item.caption for item in media), any(item.caption_entities for item in media), - any(item.parse_mode is DEFAULT_NONE for item in media), + any(item.parse_mode for item in media), ] ): raise ValueError("You can only supply either group caption or media with captions.") From bad0128667cbfb128292c57a5149f4ca1bf4db62 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Thu, 20 Oct 2022 21:37:18 +0300 Subject: [PATCH 16/24] fix(`test_send_media_group`) check media w/o `caption`, w/ other attrs check that the error is raised if user passes media that only have `parse_mode` or `caption_entities` --- tests/test_inputmedia.py | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/tests/test_inputmedia.py b/tests/test_inputmedia.py index 7bd6cccbb8f..f147f884d20 100644 --- a/tests/test_inputmedia.py +++ b/tests/test_inputmedia.py @@ -430,6 +430,22 @@ def media_group(photo, thumb): # noqa: F811 ] +@pytest.fixture(scope="function") # noqa: F811 +def media_group_no_caption_only_caption_entities(photo, thumb): # noqa: F811 + return [ + InputMediaPhoto(photo, caption_entities=[MessageEntity(MessageEntity.BOLD, 0, 5)]), + InputMediaPhoto(photo, caption_entities=[MessageEntity(MessageEntity.BOLD, 0, 5)]), + ] + + +@pytest.fixture(scope="function") # noqa: F811 +def media_group_no_caption_only_parse_mode(photo, thumb): # noqa: F811 + return [ + InputMediaPhoto(photo, parse_mode="Markdown"), + InputMediaPhoto(thumb, parse_mode="HTML"), + ] + + class TestSendMediaGroup: @flaky(3, 1) async def test_send_media_group_photo(self, bot, chat_id, media_group): @@ -444,12 +460,23 @@ async def test_send_media_group_photo(self, bot, chat_id, media_group): ) async def test_send_media_group_throws_error_with_group_caption_and_individual_captions( - self, bot, chat_id, media_group + self, + bot, + chat_id, + media_group, + media_group_no_caption_only_caption_entities, + media_group_no_caption_only_parse_mode, ): - with pytest.raises( - ValueError, match="You can only supply either group caption or media with captions." + for group in ( + media_group, + media_group_no_caption_only_caption_entities, + media_group_no_caption_only_parse_mode, ): - await bot.send_media_group(chat_id, media_group, caption="foo") + with pytest.raises( + ValueError, + match="You can only supply either group caption or media with captions.", + ): + await bot.send_media_group(chat_id, group, caption="foo") @pytest.mark.parametrize( "caption, parse_mode, caption_entities", From 4e8007dd766fcdffdf57c6572fcb7f50f9f56633 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Fri, 21 Oct 2022 22:05:18 +0300 Subject: [PATCH 17/24] fix(`send_media_group`) fix check for individual captions Check if the user explicitly set parse_mode for the InputMedia* object. Even if it is set to None, ValueError must be raised. --- telegram/_bot.py | 3 ++- tests/test_inputmedia.py | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/telegram/_bot.py b/telegram/_bot.py index ea454f4aeda..eb0e03f2040 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -2073,7 +2073,8 @@ async def send_media_group( [ any(item.caption for item in media), any(item.caption_entities for item in media), - any(item.parse_mode for item in media), + # if parse_mode was set explicitly, even to None, error must be raised + any(item.parse_mode is not DEFAULT_NONE for item in media), ] ): raise ValueError("You can only supply either group caption or media with captions.") diff --git a/tests/test_inputmedia.py b/tests/test_inputmedia.py index f147f884d20..4c427acbf43 100644 --- a/tests/test_inputmedia.py +++ b/tests/test_inputmedia.py @@ -497,14 +497,16 @@ async def test_send_media_group_with_group_caption( parse_mode, caption_entities, ): + from telegram._utils.defaultvalue import DEFAULT_NONE + # prepare a copy to check later on if calling the method has caused side effects copied_media_group = media_group.copy() # clear media_group of all_captions for item in media_group: item.caption = None - item.parse_mode = None item.caption_entities = None + item.parse_mode = DEFAULT_NONE # parse_mode must not be explicitly set to anything messages = await bot.send_media_group( chat_id, From 6d8954e0827e7b4711691b47a4efa237714a1488 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Fri, 21 Oct 2022 23:17:09 +0300 Subject: [PATCH 18/24] refactor(`test_send_media_group`) use new fixture, don't change existing add fixture for a media group without individual captions rather than taking `media_group` fixtures and setting attributes manually this fixture can be used later when checking defaults --- tests/test_inputmedia.py | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/tests/test_inputmedia.py b/tests/test_inputmedia.py index 4c427acbf43..767971e222c 100644 --- a/tests/test_inputmedia.py +++ b/tests/test_inputmedia.py @@ -430,6 +430,11 @@ def media_group(photo, thumb): # noqa: F811 ] +@pytest.fixture(scope="function") # noqa: F811 +def media_group_no_caption_args(photo, thumb): # noqa: F811 + return [InputMediaPhoto(photo), InputMediaPhoto(thumb), InputMediaPhoto(photo)] + + @pytest.fixture(scope="function") # noqa: F811 def media_group_no_caption_only_caption_entities(photo, thumb): # noqa: F811 return [ @@ -492,25 +497,17 @@ async def test_send_media_group_with_group_caption( self, bot, chat_id, - media_group, + media_group_no_caption_args, caption, parse_mode, caption_entities, ): - from telegram._utils.defaultvalue import DEFAULT_NONE - # prepare a copy to check later on if calling the method has caused side effects - copied_media_group = media_group.copy() - - # clear media_group of all_captions - for item in media_group: - item.caption = None - item.caption_entities = None - item.parse_mode = DEFAULT_NONE # parse_mode must not be explicitly set to anything + copied_media_group = media_group_no_caption_args.copy() messages = await bot.send_media_group( chat_id, - media_group, + media_group_no_caption_args, caption=caption, parse_mode=parse_mode, caption_entities=caption_entities, @@ -519,8 +516,8 @@ async def test_send_media_group_with_group_caption( # Check that the method had no side effects: # original group was not changed and 1st item still points to the same object # (1st item must be copied within the method before adding the caption) - assert media_group == copied_media_group - assert media_group[0] is copied_media_group[0] + assert media_group_no_caption_args == copied_media_group + assert media_group_no_caption_args[0] is copied_media_group[0] assert isinstance(messages, list) assert len(messages) == 3 From b0d20c4dfda80095f644c1ba6542db7fc4674f07 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Sat, 22 Oct 2022 16:23:30 +0300 Subject: [PATCH 19/24] fix: revert `parse_mode` typing to `ODVInput[str] = DEFAULT_NONE`, test * make `parse_mode` default to DEFAULT_NONE in `Bot` and shortcut methods in classes * exclude `parse_mode` from check in conftest.py * add tests for handling of default values --- telegram/_bot.py | 9 +++----- telegram/_chat.py | 2 +- telegram/_message.py | 2 +- telegram/_user.py | 2 +- telegram/ext/_extbot.py | 2 +- tests/conftest.py | 2 +- tests/test_inputmedia.py | 45 ++++++++++++++++++++++++++++++++++++++++ 7 files changed, 53 insertions(+), 11 deletions(-) diff --git a/telegram/_bot.py b/telegram/_bot.py index eb0e03f2040..cfb646db412 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -2000,10 +2000,7 @@ async def send_media_group( pool_timeout: ODVInput[float] = DEFAULT_NONE, api_kwargs: JSONDict = None, caption: Optional[str] = None, - # Note on parse_mode: - # Typing ODVInput[str] = DEFAULT_NONE will make tests for shortcuts fail - # (check_defaults_handling) despite it being used for parse mode for individual captions. - parse_mode: Optional[str] = None, + parse_mode: ODVInput[str] = DEFAULT_NONE, caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, ) -> List[Message]: """Use this method to send a group of photos or videos as an album. @@ -2057,7 +2054,6 @@ async def send_media_group( Parse mode for :paramref:`caption`. See the constants in :class:`telegram.constants.ParseMode` for the available modes. - Defaults to :obj:`None`. caption_entities (List[:class:`telegram.MessageEntity`], optional): List of special entities for :paramref:`caption`, which can be specified instead of :paramref:`parse_mode`. @@ -2085,7 +2081,8 @@ async def send_media_group( # Copy first item to avoid mutation of original object item_to_get_caption = copy.copy(media[0]) item_to_get_caption.caption = caption - item_to_get_caption.parse_mode = parse_mode + if parse_mode is not DEFAULT_NONE: + item_to_get_caption.parse_mode = parse_mode item_to_get_caption.caption_entities = caption_entities # copy the list (just the references) to avoid mutating the original list diff --git a/telegram/_chat.py b/telegram/_chat.py index f762ca7d1e4..8971cf206bb 100644 --- a/telegram/_chat.py +++ b/telegram/_chat.py @@ -1130,7 +1130,7 @@ async def send_media_group( pool_timeout: ODVInput[float] = DEFAULT_NONE, api_kwargs: JSONDict = None, caption: Optional[str] = None, - parse_mode: Optional[str] = None, + parse_mode: ODVInput[str] = DEFAULT_NONE, caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, ) -> List["Message"]: """Shortcut for:: diff --git a/telegram/_message.py b/telegram/_message.py index 9dac0021158..33ccaebc066 100644 --- a/telegram/_message.py +++ b/telegram/_message.py @@ -1011,7 +1011,7 @@ async def reply_media_group( pool_timeout: ODVInput[float] = DEFAULT_NONE, api_kwargs: JSONDict = None, caption: Optional[str] = None, - parse_mode: Optional[str] = None, + parse_mode: ODVInput[str] = DEFAULT_NONE, caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, ) -> List["Message"]: """Shortcut for:: diff --git a/telegram/_user.py b/telegram/_user.py index 661a18edf43..01bf0d93a87 100644 --- a/telegram/_user.py +++ b/telegram/_user.py @@ -475,7 +475,7 @@ async def send_media_group( pool_timeout: ODVInput[float] = DEFAULT_NONE, api_kwargs: JSONDict = None, caption: Optional[str] = None, - parse_mode: Optional[str] = None, + parse_mode: ODVInput[str] = DEFAULT_NONE, caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, ) -> List["Message"]: """Shortcut for:: diff --git a/telegram/ext/_extbot.py b/telegram/ext/_extbot.py index 2f1c1475f00..9771e275a7b 100644 --- a/telegram/ext/_extbot.py +++ b/telegram/ext/_extbot.py @@ -2243,7 +2243,7 @@ async def send_media_group( api_kwargs: JSONDict = None, rate_limit_args: RLARGS = None, caption: Optional[str] = None, - parse_mode: Optional[str] = None, + parse_mode: ODVInput[str] = DEFAULT_NONE, caption_entities: Union[List["MessageEntity"], Tuple["MessageEntity", ...]] = None, ) -> List[Message]: return await super().send_media_group( diff --git a/tests/conftest.py b/tests/conftest.py index 09bdf9abd2e..67cb9935b2f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -712,7 +712,7 @@ async def make_assertion( data = request_data.parameters # Check regular arguments that need defaults - for arg in (dkw for dkw in kwargs_need_default if dkw != "timeout"): + for arg in (dkw for dkw in kwargs_need_default if dkw not in ("timeout", "parse_mode")): # 'None' should not be passed along to Telegram if df_value in [None, DEFAULT_NONE]: if arg in data: diff --git a/tests/test_inputmedia.py b/tests/test_inputmedia.py index 767971e222c..07699e3ab37 100644 --- a/tests/test_inputmedia.py +++ b/tests/test_inputmedia.py @@ -675,6 +675,51 @@ async def test_send_media_group_default_protect_content( ) assert not all(msg.has_protected_content for msg in unprotected) + @flaky(3, 1) + @pytest.mark.parametrize("default_bot", [{"parse_mode": ParseMode.HTML}], indirect=True) + async def test_send_media_group_default_parse_mode( + self, chat_id, media_group_no_caption_args, default_bot + ): + messages = await default_bot.send_media_group( + chat_id, media_group_no_caption_args, caption="*photo* 1" + ) + + first_message, other_messages = messages[0], messages[1:] + assert all(mes.media_group_id == first_message.media_group_id for mes in messages) + + # Make sure first message got the caption, which will lead + # to Telegram displaying its caption as group caption + assert first_message.caption == "photo 1" + assert first_message.caption_entities == [MessageEntity(MessageEntity.BOLD, 0, 5)] + + # Check that other messages have no captions + assert all(mes.caption is None for mes in other_messages) + assert not any(mes.caption_entities for mes in other_messages) + + @flaky(3, 1) + @pytest.mark.parametrize("default_bot", [{"parse_mode": ParseMode.HTML}], indirect=True) + async def test_send_media_group_default_parse_mode_overridden( + self, chat_id, media_group_no_caption_args, default_bot + ): + messages = await default_bot.send_media_group( + chat_id, + media_group_no_caption_args, + caption="*photo* 1", + parse_mode=ParseMode.MARKDOWN_V2, + ) + + first_message, other_messages = messages[0], messages[1:] + assert all(mes.media_group_id == first_message.media_group_id for mes in messages) + + # Make sure first message got the caption, which will lead + # to Telegram displaying its caption as group caption + assert first_message.caption == "photo 1" + assert first_message.caption_entities == [MessageEntity(MessageEntity.BOLD, 0, 5)] + + # Check that other messages have no captions + assert all(mes.caption is None for mes in other_messages) + assert not any(mes.caption_entities for mes in other_messages) + @flaky(3, 1) async def test_edit_message_media(self, bot, chat_id, media_group): messages = await bot.send_media_group(chat_id, media_group) From 008f201ec93fdc466eba2fe6c5c46596b0a0add3 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Sat, 22 Oct 2022 16:42:57 +0300 Subject: [PATCH 20/24] fix typo in test for default parse mode --- tests/test_inputmedia.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_inputmedia.py b/tests/test_inputmedia.py index 07699e3ab37..a2ad5b7bae7 100644 --- a/tests/test_inputmedia.py +++ b/tests/test_inputmedia.py @@ -681,7 +681,7 @@ async def test_send_media_group_default_parse_mode( self, chat_id, media_group_no_caption_args, default_bot ): messages = await default_bot.send_media_group( - chat_id, media_group_no_caption_args, caption="*photo* 1" + chat_id, media_group_no_caption_args, caption="photo 1" ) first_message, other_messages = messages[0], messages[1:] From 86d024b4ca206c7ecd72cd9dfa385e86e4581bb4 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Sat, 22 Oct 2022 17:35:51 +0300 Subject: [PATCH 21/24] fix side effect of handling of `parse_mode` by bot with defaults to allow multiple calls with same media group, I need to copy not only the first item (that gets the caption) but all items because otherwise bot with Defaults will assign its default parse mode to every item, which will cause ValueError upon subsequent calls unless there are other things to be fixed, this should be the commit that closes #3185 --- telegram/_bot.py | 16 +++++++------- tests/test_inputmedia.py | 45 +++++++++++++++------------------------- 2 files changed, 26 insertions(+), 35 deletions(-) diff --git a/telegram/_bot.py b/telegram/_bot.py index cfb646db412..d75666ba884 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -2076,19 +2076,21 @@ async def send_media_group( raise ValueError("You can only supply either group caption or media with captions.") if caption: - # Apply group caption to the first media item. + # Create copies of objects in media and apply group caption to the first item. # This will lead to the group being shown with this caption. - # Copy first item to avoid mutation of original object - item_to_get_caption = copy.copy(media[0]) + + # Only copying of first item should be enough, but the behaviour of Defaults is such + # that a bot with a default parse mode will automatically set `parse_mode` for + # each item to default parse mode of the bot. + # This will throw ValueError at subsequent calls with same media items. + media = [copy.copy(item) for item in media] + + item_to_get_caption = media[0] item_to_get_caption.caption = caption if parse_mode is not DEFAULT_NONE: item_to_get_caption.parse_mode = parse_mode item_to_get_caption.caption_entities = caption_entities - # copy the list (just the references) to avoid mutating the original list - media = media.copy() - media[0] = item_to_get_caption - data: JSONDict = { "chat_id": chat_id, "media": media, diff --git a/tests/test_inputmedia.py b/tests/test_inputmedia.py index a2ad5b7bae7..500b444e348 100644 --- a/tests/test_inputmedia.py +++ b/tests/test_inputmedia.py @@ -519,6 +519,8 @@ async def test_send_media_group_with_group_caption( assert media_group_no_caption_args == copied_media_group assert media_group_no_caption_args[0] is copied_media_group[0] + assert not any(item.parse_mode for item in media_group_no_caption_args) + assert isinstance(messages, list) assert len(messages) == 3 assert all(isinstance(mes, Message) for mes in messages) @@ -680,45 +682,32 @@ async def test_send_media_group_default_protect_content( async def test_send_media_group_default_parse_mode( self, chat_id, media_group_no_caption_args, default_bot ): - messages = await default_bot.send_media_group( + default = await default_bot.send_media_group( chat_id, media_group_no_caption_args, caption="photo 1" ) - first_message, other_messages = messages[0], messages[1:] - assert all(mes.media_group_id == first_message.media_group_id for mes in messages) - - # Make sure first message got the caption, which will lead - # to Telegram displaying its caption as group caption - assert first_message.caption == "photo 1" - assert first_message.caption_entities == [MessageEntity(MessageEntity.BOLD, 0, 5)] - - # Check that other messages have no captions - assert all(mes.caption is None for mes in other_messages) - assert not any(mes.caption_entities for mes in other_messages) + # make sure no parse_mode was set as a side effect + assert not any(item.parse_mode for item in media_group_no_caption_args) - @flaky(3, 1) - @pytest.mark.parametrize("default_bot", [{"parse_mode": ParseMode.HTML}], indirect=True) - async def test_send_media_group_default_parse_mode_overridden( - self, chat_id, media_group_no_caption_args, default_bot - ): - messages = await default_bot.send_media_group( + overridden = await default_bot.send_media_group( chat_id, - media_group_no_caption_args, + media_group_no_caption_args.copy(), caption="*photo* 1", parse_mode=ParseMode.MARKDOWN_V2, ) - first_message, other_messages = messages[0], messages[1:] - assert all(mes.media_group_id == first_message.media_group_id for mes in messages) + for mes_group in (default, overridden): + first_message, other_messages = mes_group[0], mes_group[1:] + assert all(mes.media_group_id == first_message.media_group_id for mes in mes_group) - # Make sure first message got the caption, which will lead - # to Telegram displaying its caption as group caption - assert first_message.caption == "photo 1" - assert first_message.caption_entities == [MessageEntity(MessageEntity.BOLD, 0, 5)] + # Make sure first message got the caption, which will lead + # to Telegram displaying its caption as group caption + assert first_message.caption == "photo 1" + assert first_message.caption_entities == [MessageEntity(MessageEntity.BOLD, 0, 5)] - # Check that other messages have no captions - assert all(mes.caption is None for mes in other_messages) - assert not any(mes.caption_entities for mes in other_messages) + # Check that other messages have no captions + assert all(mes.caption is None for mes in other_messages) + assert not any(mes.caption_entities for mes in other_messages) @flaky(3, 1) async def test_edit_message_media(self, bot, chat_id, media_group): From 7b683154be006b2f591960dbf82c71fad40bc219 Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Sun, 23 Oct 2022 19:10:18 +0300 Subject: [PATCH 22/24] fix(conftest.py) exclude `parse_mode` only for `..._media_group()` also remove `timeout` as a remnant from v13 --- tests/conftest.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 67cb9935b2f..fa78bce92c8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -699,6 +699,10 @@ async def check_defaults_handling( if isinstance(value.default, DefaultValue) and not kwarg.endswith("_timeout") ] + if method.__name__.endswith("_media_group"): + # the parse_mode is applied to the first media item, and we test this elsewhere + kwargs_need_default.remove("parse_mode") + defaults_no_custom_defaults = Defaults() kwargs = {kwarg: "custom_default" for kwarg in inspect.signature(Defaults).parameters.keys()} kwargs["tzinfo"] = pytz.timezone("America/New_York") @@ -712,7 +716,7 @@ async def make_assertion( data = request_data.parameters # Check regular arguments that need defaults - for arg in (dkw for dkw in kwargs_need_default if dkw not in ("timeout", "parse_mode")): + for arg in kwargs_need_default: # 'None' should not be passed along to Telegram if df_value in [None, DEFAULT_NONE]: if arg in data: From b23acc53531dc4ec3f35bea791393d62bd01525d Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Sun, 23 Oct 2022 19:25:40 +0300 Subject: [PATCH 23/24] fix(test) test send_media_group with Defaults bot and `parse_mode=None` --- tests/test_inputmedia.py | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/tests/test_inputmedia.py b/tests/test_inputmedia.py index 500b444e348..7c1f43508a5 100644 --- a/tests/test_inputmedia.py +++ b/tests/test_inputmedia.py @@ -689,23 +689,36 @@ async def test_send_media_group_default_parse_mode( # make sure no parse_mode was set as a side effect assert not any(item.parse_mode for item in media_group_no_caption_args) - overridden = await default_bot.send_media_group( + overridden_markdown_v2 = await default_bot.send_media_group( chat_id, media_group_no_caption_args.copy(), caption="*photo* 1", parse_mode=ParseMode.MARKDOWN_V2, ) - for mes_group in (default, overridden): - first_message, other_messages = mes_group[0], mes_group[1:] - assert all(mes.media_group_id == first_message.media_group_id for mes in mes_group) + overridden_none = await default_bot.send_media_group( + chat_id, + media_group_no_caption_args.copy(), + caption="photo 1", + parse_mode=None, + ) - # Make sure first message got the caption, which will lead - # to Telegram displaying its caption as group caption + # Make sure first message got the caption, which will lead to Telegram + # displaying its caption as group caption + assert overridden_none[0].caption == "photo 1" + assert not overridden_none[0].caption_entities + # First messages in these two groups have to have caption "photo 1" + # because of parse mode (default or explicit) + for mes_group in (default, overridden_markdown_v2): + first_message = mes_group[0] assert first_message.caption == "photo 1" assert first_message.caption_entities == [MessageEntity(MessageEntity.BOLD, 0, 5)] - # Check that other messages have no captions + # This check is valid for all 3 groups of messages + for mes_group in (default, overridden_markdown_v2, overridden_none): + first_message, other_messages = mes_group[0], mes_group[1:] + assert all(mes.media_group_id == first_message.media_group_id for mes in mes_group) + # Check that messages from 2nd message onwards have no captions assert all(mes.caption is None for mes in other_messages) assert not any(mes.caption_entities for mes in other_messages) From 43d2d28232f2c0cecb7e43cfcd0b9a726ff7fd6e Mon Sep 17 00:00:00 2001 From: Dmitry Kolomatskiy <58207913+lemontree210@users.noreply.github.com> Date: Mon, 31 Oct 2022 18:08:05 +0300 Subject: [PATCH 24/24] refactor(`Bot.send_media_group()`): copy only 1st item in media group Closes #3185 Refactoring becomes possible since #3305 was fixed. Refactoring is based on ideas in commit c38801fab32cc0fad40c07ef176fef00d98ca2f8 --- telegram/_bot.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/telegram/_bot.py b/telegram/_bot.py index a9830a3b1bf..956a3d3de53 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -2082,21 +2082,18 @@ async def send_media_group( raise ValueError("You can only supply either group caption or media with captions.") if caption: - # Create copies of objects in media and apply group caption to the first item. + # Copy first item (to avoid mutation of original object), apply group caption to it. # This will lead to the group being shown with this caption. - - # Only copying of first item should be enough, but the behaviour of Defaults is such - # that a bot with a default parse mode will automatically set `parse_mode` for - # each item to default parse mode of the bot. - # This will throw ValueError at subsequent calls with same media items. - media = [copy.copy(item) for item in media] - - item_to_get_caption = media[0] + item_to_get_caption = copy.copy(media[0]) item_to_get_caption.caption = caption if parse_mode is not DEFAULT_NONE: item_to_get_caption.parse_mode = parse_mode item_to_get_caption.caption_entities = caption_entities + # copy the list (just the references) to avoid mutating the original list + media = media[:] + media[0] = item_to_get_caption + data: JSONDict = { "chat_id": chat_id, "media": media,