Skip to content

Conversation

@miles170
Copy link
Contributor

@miles170 miles170 commented Sep 28, 2022

chat_id and username parameters should allow filtering requests to allow only those from a specified chat ID or username.

Closes #3215.

Checklist for PRs

  • Added .. versionadded:: version, .. versionchanged:: version or .. deprecated:: version to the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)
  • Created new or adapted existing unit tests
  • Documented code changes according to the CSI standard
  • Added myself alphabetically to AUTHORS.rst (optional)
  • Added new classes & modules to the docs and all suitable __all__ s

Copy link
Member

@Poolitzer Poolitzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, good job implementing the tests.

@miles170 miles170 force-pushed the filter-chat-join-request branch from d96596d to 9a4eb95 Compare September 28, 2022 08:01
@miles170 miles170 changed the title Add chat_id(s) parameter to ChatJoinRequestHandler Add chat_id(s) and username(s) parameters to ChatJoinRequestHandler Sep 28, 2022
@miles170 miles170 requested a review from Poolitzer September 28, 2022 08:02
`chat_id` and `username` parameters should allow filtering requests
to allow only those from a specified chat ID or username.
@miles170 miles170 force-pushed the filter-chat-join-request branch from 9a4eb95 to 0123635 Compare September 28, 2022 08:20
@miles170
Copy link
Contributor Author

The CI failure is not related to code changes.

Copy link
Member

@Poolitzer Poolitzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, could you please not rebase/force push? It makes the review process much worse. Just make a new commit, its fine, all our PRs have lots of commits. We just squash when we merge.

Copy link
Member

@Poolitzer Poolitzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks also from my side for the PR!

Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good work!

@harshil21 harshil21 added this to the v20.0a5 milestone Sep 28, 2022
Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except the suggested reformulation, this LGTM :)

Copy link
Member

@Poolitzer Poolitzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last comment, code is good!

Copy link
Member

@Poolitzer Poolitzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Bibo-Joshi Bibo-Joshi merged commit 26e7cd2 into python-telegram-bot:master Oct 2, 2022
@Bibo-Joshi
Copy link
Member

Thank you for your contribution @miles170 and thanks also for your patience during the review process :)

@miles170 miles170 deleted the filter-chat-join-request branch October 3, 2022 01:08
@github-actions github-actions bot locked and limited conversation to collaborators Oct 10, 2022
@Bibo-Joshi Bibo-Joshi added 🔌 enhancement pr description: enhancement and removed enhancement labels Nov 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

🔌 enhancement pr description: enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Suggestion] Add chat(s) parameter to ChatJoinRequestHandler

4 participants