Skip to content
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

Update algorithm for parsing smart filters #1276

Merged
merged 16 commits into from Nov 6, 2023

Conversation

Dr-Blank
Copy link
Contributor

@Dr-Blank Dr-Blank commented Oct 20, 2023

Description

algorithm for parsing is updated to cover multilevel filter groups parsing

Fixes

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated the docstring for new or existing methods
  • I have added tests when applicable

@Dr-Blank Dr-Blank changed the title Update algorithm for parsing #1274 Update algorithm for parsing smart filters Oct 20, 2023
- `and` is now considered the default operation instead of raising error
- fix algorithm so that the reserved keys are parsed in the filters dictionary instead of filters group when located later in the feed.
- removed some code repetition
If multiple filter groups are parsed they are joined by `and` instead of raising error.
transfers "=" from the value to key
@Dr-Blank Dr-Blank marked this pull request as ready for review October 22, 2023 03:33
@JonnyWong16
Copy link
Collaborator

Thanks. Could you add some tests with some more complex filters? Right now the tests only check that searching with the parsed filters returns the same items.

filters = collection.filters()
assert movies.search(**filters) == collection.items()

filters = playlist.filters()
filters['libtype'] = tvshows.METADATA_TYPE # Override libtype to check playlist items
assert tvshows.search(**filters) == playlist.items()

The new tests should actually check that the filter and parsed filter dictionaries are equal.

parsedFilters = collection.filters()
assert filters == parsedFilters
parsedFilters = playlist.filters()
assert filters == parsedFilters

@Dr-Blank
Copy link
Contributor Author

Thanks. Could you add some tests with some more complex filters? Right now the tests only check that searching with the parsed filters returns the same items.

Could you please guide me to a document where I can learn how to setup plex environment for testing?

Currently running test locally fails all of them except a few. I have to use GH Actions every time, and it would be great to have it run locally.

I know it sets up pms using docker but if there was a script for this to do automatically?

@JonnyWong16
Copy link
Collaborator

See Discord.

@Dr-Blank
Copy link
Contributor Author

@JonnyWong16

I have added 2 tests which will fail for the old algorithm that are passing for this PR.

Copy link
Collaborator

@JonnyWong16 JonnyWong16 left a comment

Choose a reason for hiding this comment

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

The logic looks good. Thanks!

Just a few stylistic changes.

plexapi/mixins.py Outdated Show resolved Hide resolved
plexapi/mixins.py Outdated Show resolved Hide resolved
plexapi/mixins.py Outdated Show resolved Hide resolved
plexapi/mixins.py Outdated Show resolved Hide resolved
adhere to style guide of the project

Co-authored-by: JonnyWong16 <9099342+JonnyWong16@users.noreply.github.com>
@JonnyWong16 JonnyWong16 merged commit 56a8df6 into pkkid:master Nov 6, 2023
3 of 5 checks passed
@Dr-Blank Dr-Blank deleted the fix-filters-parsing branch November 6, 2023 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants