Repository: python-telegram-bot/python-telegram-bot
Pull request: Add new shortcuts to Chat
closes #2284

on the fly replaces some `.format(…)` left in `User`

Problem statement:
[FEATURE] Shortcuts for Chat
Add `Chat.{promote, restrict}_member`.
We could also think about adding `ChatMember.{kick, unban, promote, restrict}`, though the usecase is small, as a `ChatMember` is only returned by `get_chat_member`, so you'd have to do something like

```pyhon
update.effective_chat.get_member(user_id).kick()
```
which is longer than

```python
update.effective_chat.kick_member(user_id)
```

Hints from issue discussion:
While at it, we might as well add `Chat.full_name`

Commit message: Add shortcuts

Patch to review:
diff --git a/telegram/chat.py b/telegram/chat.py
index e049f0114a4..72c17016e60 100644
--- a/telegram/chat.py
+++ b/telegram/chat.py
@@ -189,6 +189,20 @@ def __init__(
         self.bot = bot
         self._id_attrs = (self.id,)
 
+    @property
+    def full_name(self) -> Optional[str]:
+        """
+        :obj:`str`: Convenience property. If :attr:`first_name` is not :obj:`None` gives,
+        :attr:`first_name` followed by (if available) :attr:`last_name`.
+
+        .. versionadded:: 13.2
+        """
+        if not self.first_name:
+            return None
+        if self.last_name:
+            return f'{self.first_name} {self.last_name}'
+        return self.first_name
+
     @property
     def link(self) -> Optional[str]:
         """:obj:`str`: Convenience property. If the chat has a :attr:`username`, returns a t.me
@@ -350,6 +364,80 @@ def unban_member(
             only_if_banned=only_if_banned,
         )
 
+    def promote_member(
+        self,
+        user_id: Union[str, int],
+        can_change_info: bool = None,
+        can_post_messages: bool = None,
+        can_edit_messages: bool = None,
+        can_delete_messages: bool = None,
+        can_invite_users: bool = None,
+        can_restrict_members: bool = None,
+        can_pin_messages: bool = None,
+        can_promote_members: bool = None,
+        timeout: float = None,
+        api_kwargs: JSONDict = None,
+        is_anonymous: bool = None,
+    ) -> bool:
+        """Shortcut for::
+
+            bot.promote_chat_member(update.effective_chat.id, *args, **kwargs)
+
+        For the documentation of the arguments, please see
+        :meth:`telegram.Bot.promote_chat_member`.
+
+        .. versionadded:: 13.2
+
+        Returns:
+            :obj:`bool`: If the action was sent successfully.
+
+        """
+        return self.bot.promote_chat_member(
+            chat_id=self.id,
+            user_id=user_id,
+            can_change_info=can_change_info,
+            can_post_messages=can_post_messages,
+            can_edit_messages=can_edit_messages,
+            can_delete_messages=can_delete_messages,
+            can_invite_users=can_invite_users,
+            can_restrict_members=can_restrict_members,
+            can_pin_messages=can_pin_messages,
+            can_promote_members=can_promote_members,
+            timeout=timeout,
+            api_kwargs=api_kwargs,
+            is_anonymous=is_anonymous,
+        )
+
+    def restrict_member(
+        self,
+        user_id: Union[str, int],
+        permissions: ChatPermissions,
+        until_date: Union[int, datetime] = None,
+        timeout: float = None,
+        api_kwargs: JSONDict = None,
+    ) -> bool:
+        """Shortcut for::
+
+            bot.restrict_chat_member(update.effective_chat.id, *args, **kwargs)
+
+        For the documentation of the arguments, please see
+        :meth:`telegram.Bot.restrict_chat_member`.
+
+        .. versionadded:: 13.2
+
+        Returns:
+            :obj:`bool`: If the action was sent successfully.
+
+        """
+        return self.bot.restrict_chat_member(
+            chat_id=self.id,
+            user_id=user_id,
+            permissions=permissions,
+            until_date=until_date,
+            timeout=timeout,
+            api_kwargs=api_kwargs,
+        )
+
     def set_permissions(
         self,
         permissions: ChatPermissions,
diff --git a/telegram/user.py b/telegram/user.py
index 404b30026fb..e0e6642978e 100644
--- a/telegram/user.py
+++ b/telegram/user.py
@@ -135,7 +135,7 @@ def full_name(self) -> str:
         available) :attr:`last_name`."""
 
         if self.last_name:
-            return u'{} {}'.format(self.first_name, self.last_name)
+            return f'{self.first_name} {self.last_name}'
         return self.first_name
 
     @property
diff --git a/telegram/utils/helpers.py b/telegram/utils/helpers.py
index 8e9a82c7975..522a9940488 100644
--- a/telegram/utils/helpers.py
+++ b/telegram/utils/helpers.py
@@ -317,7 +317,7 @@ def mention_html(user_id: Union[int, str], name: str) -> str:
     Returns:
         :obj:`str`: The inline mention for the user as html.
     """
-    return u'<a href="tg://user?id={}">{}</a>'.format(user_id, escape(name))
+    return f'<a href="tg://user?id={user_id}">{escape(name)}</a>'
 
 
 def mention_markdown(user_id: Union[int, str], name: str, version: int = 1) -> str:
@@ -331,7 +331,7 @@ def mention_markdown(user_id: Union[int, str], name: str, version: int = 1) -> s
     Returns:
         :obj:`str`: The inline mention for the user as markdown.
     """
-    return u'[{}](tg://user?id={})'.format(escape_markdown(name, version=version), user_id)
+    return f'[{escape_markdown(name, version=version)}](tg://user?id={user_id})'
 
 
 def effective_message_type(entity: Union['Message', 'Update']) -> Optional[str]:
diff --git a/tests/test_chat.py b/tests/test_chat.py
index f980eb1ee82..02453109203 100644
--- a/tests/test_chat.py
+++ b/tests/test_chat.py
@@ -112,6 +112,19 @@ def test_link(self, chat):
         chat.username = None
         assert chat.link is None
 
+    def test_full_name(self):
+        chat = Chat(
+            id=1, type=Chat.PRIVATE, first_name=u'first\u2022name', last_name=u'last\u2022name'
+        )
+        assert chat.full_name == u'first\u2022name last\u2022name'
+        chat = Chat(id=1, type=Chat.PRIVATE, first_name=u'first\u2022name')
+        assert chat.full_name == u'first\u2022name'
+        chat = Chat(
+            id=1,
+            type=Chat.PRIVATE,
+        )
+        assert chat.full_name is None
+
     def test_send_action(self, monkeypatch, chat):
         send_chat_action = chat.bot.send_chat_action
 
@@ -211,6 +224,44 @@ def make_assertion(*_, **kwargs):
         monkeypatch.setattr(chat.bot, 'unban_chat_member', make_assertion)
         assert chat.unban_member(user_id=42, only_if_banned=only_if_banned)
 
+    @pytest.mark.parametrize('is_anonymous', [True, False, None])
+    def test_promote_member(self, monkeypatch, chat, is_anonymous):
+        promote_chat_member = chat.bot.promote_chat_member
+
+        def make_assertion(*_, **kwargs):
+            chat_id = kwargs['chat_id'] == chat.id
+            user_id = kwargs['user_id'] == 42
+            o_i_b = kwargs.get('is_anonymous', None) == is_anonymous
+            return (
+                chat_id and user_id and o_i_b and check_shortcut_call(kwargs, promote_chat_member)
+            )
+
+        assert check_shortcut_signature(
+            Chat.promote_member, Bot.promote_chat_member, ['chat_id'], []
+        )
+
+        monkeypatch.setattr(chat.bot, 'promote_chat_member', make_assertion)
+        assert chat.promote_member(user_id=42, is_anonymous=is_anonymous)
+
+    def test_restrict_member(self, monkeypatch, chat):
+        restrict_chat_member = chat.bot.restrict_chat_member
+        permissions = ChatPermissions(True, False, True, False, True, False, True, False)
+
+        def make_assertion(*_, **kwargs):
+            chat_id = kwargs['chat_id'] == chat.id
+            user_id = kwargs['user_id'] == 42
+            o_i_b = kwargs.get('permissions', None) == permissions
+            return (
+                chat_id and user_id and o_i_b and check_shortcut_call(kwargs, restrict_chat_member)
+            )
+
+        assert check_shortcut_signature(
+            Chat.restrict_member, Bot.restrict_chat_member, ['chat_id'], []
+        )
+
+        monkeypatch.setattr(chat.bot, 'restrict_chat_member', make_assertion)
+        assert chat.restrict_member(user_id=42, permissions=permissions)
+
     def test_set_permissions(self, monkeypatch, chat):
         set_chat_permissions = chat.bot.set_chat_permissions
 


Code review comments:
diff_hunk: @@ -189,6 +189,20 @@ def __init__(
         self.bot = bot
         self._id_attrs = (self.id,)
 
+    @property
+    def full_name(self) -> Optional[str]:
+        """
+        :obj:`str`: Convenience property. If :attr:`first_name` is not :obj:`None` gives,
+        :attr:`first_name` followed by (if available) :attr:`last_name`.
+
+        .. versionadded:: 13.2
+        """
+        if not self.first_name:
+            return None
path: telegram/chat.py
text: @user1:
wait, so this will always be None for not user chats? I think the docstring should reflect this, I wouldn't know as a first time API user that first_name is user only. Im not sure why people would use this over .effective_user, since that syntax is way clearer, but okay

@author:
My use case would be `get_chat(…).full_name`, as `get_chat(…)` doesn't give the `User` object. But yeah, for completeness, I'll add a note what `first_name = None` means

@user1:
yay