-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Add chat_id in ChatMemberHandler to Filter Specific Chat(s) #4287 #4290
Add chat_id in ChatMemberHandler to Filter Specific Chat(s) #4287 #4290
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR :) I left some comments below. Please also have a look at the failing doc build test. We can check the other tests after that if needed.
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.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally find the negation a bit confusing. I would find it more intuitive to have a function __is_chat_allowed
…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates, LGTM now :)
before merging, let me just check if someone else from the dev team wants to review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thank you for the nice contribution! |
closes #4287
Added a
chat_id
param toChatMemberHandler
to filter updates only from specified chat(s).So, now instead of having to filter
chat_id
within the callback methods as the following,you can now pass
chat_id
while declaring yourChatMemberHandler
.