Repository: python-telegram-bot/python-telegram-bot
Pull request: Add chat_id in ChatMemberHandler to Filter Specific Chat(s) #4287
closes #4287

Added a `chat_id` param to `ChatMemberHandler` to filter updates only from specified chat(s).

So, now instead of having to filter `chat_id` within the callback methods as the following,
```python
async def member_callback_1(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None:
    if update.chat_member.chat.id != <chat_id_1>:
        return
    ...

async def member_callback_2(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None:
    if update.chat_member.chat.id != <chat_id_2>:
        return
    ...
```

you can now pass `chat_id` while declaring your `ChatMemberHandler`.

```python
application.add_handler(
    ChatMemberHandler(member_callback_1, ChatMemberHandler.CHAT_MEMBER, chat_id=<chat_id_1>)
)

application.add_handler(
    ChatMemberHandler(member_callback_2, ChatMemberHandler.CHAT_MEMBER, chat_id=<chat_id_2>)
)
```


Problem statement:
[FEATURE] Add chat_id in ChatMemberHandler to Filter Specific Chat(s)
### What kind of feature are you missing? Where do you notice a shortcoming of PTB?

ChatMemberHandler can include a `chat_id` param to filter only on specified chat(s).

### Describe the solution you'd like

It would be convenient to be able to do something like this:

``` python
application.add_handler(
    ChatMemberHandler(member_callback_1, ChatMemberHandler.CHAT_MEMBER, chat_id=<chat_id_1>)
)

application.add_handler(
    ChatMemberHandler(member_callback_2, ChatMemberHandler.CHAT_MEMBER, chat_id=<chat_id_2>)
)
```

### Describe alternatives you've considered

``` python
async def member_callback_1(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None:
    if update.chat_member.chat.id != <chat_id_1>:
        return
    ...

async def member_callback_2(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None:
    if update.chat_member.chat.id != <chat_id_2>:
        return
    ...
```

### Additional context

Let me know if there is an alternate / better way to achieve this.

Hints from issue discussion:
Hey there. Thanks for reaching out. The proposal sounds okay to me. If you like to, you're welcome to send a PR. We already have similar functionality in other handlers so that you can probably mostly adopt similar logic. See e.g. [here](https://github.com/python-telegram-bot/python-telegram-bot/blob/6fc45a803dee783df52a8648664a37d08c0981ec/telegram/ext/_handlers/chatjoinrequesthandler.py#L31-L112).
Sure, will send a PR. Thanks.

Commit message: Add chat_id in ChatMemberHandler to Filter Specific Chat(s) #4287

Patch to review:
diff --git a/telegram/ext/_handlers/chatmemberhandler.py b/telegram/ext/_handlers/chatmemberhandler.py
index 87232d84261..362cb77aea1 100644
--- a/telegram/ext/_handlers/chatmemberhandler.py
+++ b/telegram/ext/_handlers/chatmemberhandler.py
@@ -17,12 +17,14 @@
 # You should have received a copy of the GNU Lesser Public License
 # along with this program.  If not, see [http://www.gnu.org/licenses/].
 """This module contains the ChatMemberHandler class."""
+
 from typing import Final, Optional, TypeVar
 
 from telegram import Update
 from telegram._utils.defaultvalue import DEFAULT_TRUE
-from telegram._utils.types import DVType
+from telegram._utils.types import SCT, DVType
 from telegram.ext._handlers.basehandler import BaseHandler
+from telegram.ext._utils._update_parsing import parse_chat_id
 from telegram.ext._utils.types import CCT, HandlerCallback
 
 RT = TypeVar("RT")
@@ -49,10 +51,16 @@ async def callback(update: Update, context: CallbackContext)
 
             The return value of the callback is usually ignored except for the special case of
             :class:`telegram.ext.ConversationHandler`.
+
         chat_member_types (:obj:`int`, optional): Pass one of :attr:`MY_CHAT_MEMBER`,
             :attr:`CHAT_MEMBER` or :attr:`ANY_CHAT_MEMBER` to specify if this handler should handle
             only updates with :attr:`telegram.Update.my_chat_member`,
             :attr:`telegram.Update.chat_member` or both. Defaults to :attr:`MY_CHAT_MEMBER`.
+
+            .. versionadded:: NEXT.VERSION
+        chat_id (:obj:`int` | Collection[:obj:`int`], optional): Filters chat member updates from
+        specified chat ID(s) only.
+
         block (:obj:`bool`, optional): Determines whether the return value of the callback should
             be awaited before processing the next handler in
             :meth:`telegram.ext.Application.process_update`. Defaults to :obj:`True`.
@@ -70,7 +78,11 @@ async def callback(update: Update, context: CallbackContext)
 
     """
 
-    __slots__ = ("chat_member_types",)
+    __slots__ = (
+        "_chat_ids",
+        "chat_member_types",
+    )
+
     MY_CHAT_MEMBER: Final[int] = -1
     """:obj:`int`: Used as a constant to handle only :attr:`telegram.Update.my_chat_member`."""
     CHAT_MEMBER: Final[int] = 0
@@ -83,11 +95,13 @@ def __init__(
         self,
         callback: HandlerCallback[Update, CCT, RT],
         chat_member_types: int = MY_CHAT_MEMBER,
+        chat_id: Optional[SCT[int]] = None,
         block: DVType[bool] = DEFAULT_TRUE,
     ):
         super().__init__(callback, block=block)
 
         self.chat_member_types: Optional[int] = chat_member_types
+        self._chat_ids = parse_chat_id(chat_id)
 
     def check_update(self, update: object) -> bool:
         """Determines whether an update should be passed to this handler's :attr:`callback`.
@@ -99,12 +113,31 @@ def check_update(self, update: object) -> bool:
             :obj:`bool`
 
         """
-        if isinstance(update, Update):
-            if not (update.my_chat_member or update.chat_member):
-                return False
-            if self.chat_member_types == self.ANY_CHAT_MEMBER:
-                return True
-            if self.chat_member_types == self.CHAT_MEMBER:
-                return bool(update.chat_member)
-            return bool(update.my_chat_member)
-        return False
+        if not isinstance(update, Update):
+            return False
+
+        if not (update.my_chat_member or update.chat_member):
+            return False
+
+        if self.__is_chat_restricted(update):
+            return False
+
+        if self.chat_member_types == self.ANY_CHAT_MEMBER:
+            return True
+
+        if self.chat_member_types == self.CHAT_MEMBER:
+            return bool(update.chat_member)
+
+        return bool(update.my_chat_member)
+
+    def __is_chat_restricted(self, update: Update) -> bool:
+        """Checks if the handler is chat ID restricted and doesn't match with update's chat ID."""
+        if not self._chat_ids:
+            return False
+        chat_id = None
+        if update.chat_member:
+            chat_id = update.chat_member.chat.id
+        elif update.my_chat_member:
+            chat_id = update.my_chat_member.chat.id
+
+        return chat_id is not None and chat_id not in self._chat_ids
diff --git a/tests/ext/test_chatmemberhandler.py b/tests/ext/test_chatmemberhandler.py
index 9182b2a1124..ada95288cfc 100644
--- a/tests/ext/test_chatmemberhandler.py
+++ b/tests/ext/test_chatmemberhandler.py
@@ -144,6 +144,66 @@ async def test_chat_member_types(
             await app.process_update(chat_member)
             assert self.test_flag == result_2
 
+    @pytest.mark.parametrize(
+        argnames=["allowed_types", "chat_id", "expected"],
+        argvalues=[
+            (ChatMemberHandler.MY_CHAT_MEMBER, None, (True, False)),
+            (ChatMemberHandler.CHAT_MEMBER, None, (False, True)),
+            (ChatMemberHandler.ANY_CHAT_MEMBER, None, (True, True)),
+            (ChatMemberHandler.MY_CHAT_MEMBER, 1, (True, False)),
+            (ChatMemberHandler.CHAT_MEMBER, 1, (False, True)),
+            (ChatMemberHandler.ANY_CHAT_MEMBER, 1, (True, True)),
+            (ChatMemberHandler.MY_CHAT_MEMBER, [1], (True, False)),
+            (ChatMemberHandler.CHAT_MEMBER, [1], (False, True)),
+            (ChatMemberHandler.ANY_CHAT_MEMBER, [1], (True, True)),
+            (ChatMemberHandler.MY_CHAT_MEMBER, 2, (False, False)),
+            (ChatMemberHandler.CHAT_MEMBER, 2, (False, False)),
+            (ChatMemberHandler.ANY_CHAT_MEMBER, 2, (False, False)),
+            (ChatMemberHandler.MY_CHAT_MEMBER, [2], (False, False)),
+            (ChatMemberHandler.CHAT_MEMBER, [2], (False, False)),
+            (ChatMemberHandler.ANY_CHAT_MEMBER, [2], (False, False)),
+        ],
+        ids=[
+            "MY_CHAT_MEMBER",
+            "CHAT_MEMBER",
+            "ANY_CHAT_MEMBER",
+            "MY_CHAT_MEMBER, CHAT=1 ",
+            "CHAT_MEMBER, CHAT=1",
+            "ANY_CHAT_MEMBER, CHAT=1",
+            "MY_CHAT_MEMBER, CHAT=[1] ",
+            "CHAT_MEMBER, CHAT=[1]",
+            "ANY_CHAT_MEMBER, CHAT=[1]",
+            "MY_CHAT_MEMBER, CHAT=2 ",
+            "CHAT_MEMBER, CHAT=2",
+            "ANY_CHAT_MEMBER, CHAT=2",
+            "MY_CHAT_MEMBER, CHAT=[2] ",
+            "CHAT_MEMBER, CHAT=[2]",
+            "ANY_CHAT_MEMBER, CHAT=[2]",
+        ],
+    )
+    async def test_chat_member_types_with_chat_id(
+        self, app, chat_member_updated, chat_member, expected, allowed_types, chat_id
+    ):
+        result_1, result_2 = expected
+
+        handler = ChatMemberHandler(
+            self.callback, chat_member_types=allowed_types, chat_id=chat_id
+        )
+        app.add_handler(handler)
+
+        async with app:
+            assert handler.check_update(chat_member) == result_1
+            await app.process_update(chat_member)
+            assert self.test_flag == result_1
+
+            self.test_flag = False
+            chat_member.my_chat_member = None
+            chat_member.chat_member = chat_member_updated
+
+            assert handler.check_update(chat_member) == result_2
+            await app.process_update(chat_member)
+            assert self.test_flag == result_2
+
     def test_other_update_types(self, false_update):
         handler = ChatMemberHandler(self.callback)
         assert not handler.check_update(false_update)


Code review comments:
diff_hunk: @@ -83,11 +95,13 @@ def __init__(
         self,
         callback: HandlerCallback[Update, CCT, RT],
         chat_member_types: int = MY_CHAT_MEMBER,
+        chat_id: Optional[SCT[int]] = None,
path: telegram/ext/_handlers/chatmemberhandler.py
text: @user1:
please move this to the end of the parameter list - otherwise `CMH(callback, MY_CHAT_MEMBER, True)` will break

diff_hunk: @@ -99,12 +113,31 @@ def check_update(self, update: object) -> bool:
             :obj:`bool`
 
         """
-        if isinstance(update, Update):
-            if not (update.my_chat_member or update.chat_member):
-                return False
-            if self.chat_member_types == self.ANY_CHAT_MEMBER:
-                return True
-            if self.chat_member_types == self.CHAT_MEMBER:
-                return bool(update.chat_member)
-            return bool(update.my_chat_member)
-        return False
+        if not isinstance(update, Update):
+            return False
+
+        if not (update.my_chat_member or update.chat_member):
+            return False
+
+        if self.__is_chat_restricted(update):
+            return False
+
+        if self.chat_member_types == self.ANY_CHAT_MEMBER:
+            return True
+
+        if self.chat_member_types == self.CHAT_MEMBER:
+            return bool(update.chat_member)
+
+        return bool(update.my_chat_member)
+
+    def __is_chat_restricted(self, update: Update) -> bool:
+        """Checks if the handler is chat ID restricted and doesn't match with update's chat ID."""
path: telegram/ext/_handlers/chatmemberhandler.py
text: @user1:
I personally find the negation a bit confusing. I would find it more intuitive to have a function `__is_chat_allowed` …

@author:
Had to introduce this function only because pylint was failing with so many `if` conditions in `check_update`. Your suggestion to use `update.effective_chat` instead resolved it. So moved the condition back to `check_update` and removed this function altogether. Fixed the other issues as well. Thanks for your review and suggestions. Let me know if everything looks okay now :)

diff_hunk: @@ -99,12 +113,31 @@ def check_update(self, update: object) -> bool:
             :obj:`bool`
 
         """
-        if isinstance(update, Update):
-            if not (update.my_chat_member or update.chat_member):
-                return False
-            if self.chat_member_types == self.ANY_CHAT_MEMBER:
-                return True
-            if self.chat_member_types == self.CHAT_MEMBER:
-                return bool(update.chat_member)
-            return bool(update.my_chat_member)
-        return False
+        if not isinstance(update, Update):
+            return False
+
+        if not (update.my_chat_member or update.chat_member):
+            return False
+
+        if self.__is_chat_restricted(update):
+            return False
+
+        if self.chat_member_types == self.ANY_CHAT_MEMBER:
+            return True
+
+        if self.chat_member_types == self.CHAT_MEMBER:
+            return bool(update.chat_member)
+
+        return bool(update.my_chat_member)
+
+    def __is_chat_restricted(self, update: Update) -> bool:
+        """Checks if the handler is chat ID restricted and doesn't match with update's chat ID."""
+        if not self._chat_ids:
+            return False
+        chat_id = None
+        if update.chat_member:
+            chat_id = update.chat_member.chat.id
+        elif update.my_chat_member:
+            chat_id = update.my_chat_member.chat.id
path: telegram/ext/_handlers/chatmemberhandler.py
text: @user1:
this can be shortened by using `update.effective_chat`