Skip to content

New Filters.forwarded_from filter #2446

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.

Sign up for GitHub

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

Jump to bottom
Merged
Bibo-Joshi merged 3 commits into from
Mar 28, 2021
Merged

New Filters.forwarded_from filter #2446

Bibo-Joshi merged 3 commits into from
Mar 28, 2021

Conversation

@harshil21
Copy link
Member

@harshil21 harshil21 commented Mar 25, 2021

Closes #2411

@harshil21 harshil21 added this to the v13.5 milestone Mar 25, 2021
Bibo-Joshi
Bibo-Joshi reviewed Mar 26, 2021
View reviewed changes
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.

Thanks for the PR!
Code looks good, but I'm unsure about what we filter: We have forward_from and forward_from_chat as attributes. Can we rule out that both are non-None at the same time? If not, which should we use? Even if we can rule it out, maybe it's just cleaner to provide both Filters.forward_{from, from_chat}. In any case, the docstring should explicitly mention, which attribute of the message is checked.

@harshil21
Copy link
Member Author

harshil21 commented Mar 27, 2021 •
edited
Loading

Can we rule out that both are non-None at the same time?

Yeah, only one of .forward_from or .forward_from_chat is None for a forwarded message, except when the account is hidden by the user (privacy setting), then both those attributes are None. So the filter would fail in that case. We should mention that too in the docs.

maybe it's just cleaner to provide both Filters.forward_{from, from_chat}.

They do have the same function, so as a user I would not like the hassle of switching between the two.

In any case, the docstring should explicitly mention, which attribute of the message is checked.

Agreed.

@Bibo-Joshi
Copy link
Member

Bibo-Joshi commented Mar 27, 2021 •
edited
Loading

Can we rule out that both are non-None at the same time?

Yeah, only one of .forward_from or .forward_from_chat is None for a forwarded message, except when the account is hidden by the user (privacy setting), then both those attributes are None. So the filter would fail in that case. We should mention that too in the docs.

but that not documented, right? So behavior could change … Unlikely, for sure.

@Bibo-Joshi Bibo-Joshi merged commit 9e08fa3 into master Mar 28, 2021
@Bibo-Joshi Bibo-Joshi deleted the filter-forwarded_from branch March 28, 2021 09:50
@github-actions github-actions bot locked and limited conversation to collaborators Mar 29, 2021
@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.

Reviewers

@Bibo-Joshi Bibo-Joshi Bibo-Joshi left review comments

Assignees

No one assigned

Labels

🔌 enhancement pr description: enhancement

Projects

None yet

Milestone

v13.5

Development

Successfully merging this pull request may close these issues.

[FEATURE] new Filters.forwarded_from filter

3 participants

@harshil21 @Bibo-Joshi