Skip to content

Conversation

@mlemosf
Copy link
Contributor

@mlemosf mlemosf commented Oct 19, 2020

Feature: Added filter for file endings / file extensions on documents

Added class 'file_ending' that filter documents which end in the
extension passed by the user to Filters.document.file_ending;
Also added name to AUTHORS.rst.

This pull request solves issue #2140

Added class 'file_ending' that filter documents which end in the
extension passed by the user to Filters.document.file_ending;
Added name to AUTHORS.rst.
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 for the PR. I left some comments. Also we'll need unit tests. Please have a look at tests/test_filters.py for inspiration.

def filter(self, message : Message) -> bool:
"""""" # remove method from docs
if message.document:
return message.document.file_name.endswith(self.file_ending)
Copy link
Member

Choose a reason for hiding this comment

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

this way file_extension('jpg') would also allow foo.barjpg. Please adjust for the dot

Changed 'file_ending' to 'file_extension';
Use of fstrings on formatting instead of .format();
Added tests for some possible cases of file extensions.
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 updates. I left a comment. Also please have a look at the failing tests. this coverage report tells you which lines haven't been covered (marked in red)

please also merge from master when #2158 is merged

"""""" # remove method from docs
if message.document:
return message.document.file_name.endswith(self.file_ending)
return message.document.file_name.endswith(self.file_extension)
Copy link
Member

@Bibo-Joshi Bibo-Joshi Oct 23, 2020

Choose a reason for hiding this comment

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

This still needs to account for the dot. please make sure to add a unit test testing that file_extension('jpg') doesn't allow filename.foojpg as filename

@github-actions github-actions bot locked and limited conversation to collaborators Nov 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants