-
Notifications
You must be signed in to change notification settings - Fork 10
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
Bugfix for filter listing when items is null #15
Bugfix for filter listing when items is null #15
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.
First of sorry for modifying your original PR and breaking it once more. (This does show it's important to include the criterion(s) / graphql result in the bug so I'm not making the wrong assumptions about criterions which are broken. So if you notice any other issues please try to include the criterion / graphql result.)
I've added two small comments. If you can make those changes I think it will be fine to merge.
@mrbokster, I made the changes (on your branch), can you check these work for you? I did test they work for me |
ec37964
to
179bbf7
Compare
* Fix handling of both missing and null/None values for includes / excludes * Handle criterions without values
Looks good to me and all works on my end as well. No worries about breaking it, im very thankful you created it to begin with and I like coding but have no experience with Python so was fun to have a look at that as well. Cheers |
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 cant approve it so just leaving a comment, looks good and works on my end.
Well, apparently I still didn't test properly. These changes broke all other (simple?) is_null filters like "URL is null" or "date is null" etc. So I implemented a separate fix for Furthermore the "list" handling only worked when depth was set, and didn't properly handle scenarios where it wasn't set (i.e.: it did work for all variants of "tags", but didn't work for any variant of "performer" anymore). All of this should be fixed on the develop branch now. (Which furthermore fixes an older bug where "created / updated at" containing a time didn't work properly. But that's an "old" issue). So after some confirmation of this working I will make another release. |
Fix for listing of filter with items None
Fix for opening of filters that have no value