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

1123 sub document validate filters #1125

Merged

Conversation

lmoretto
Copy link
Contributor

This PR fixes issue #1123

Also, commit bae6757 fixes the behavior of ALLOWED_FILTERS (and the related resource-specific allowed_fillters) settings in the same scenario as the one defined in #1123, i.e. filtering on a sub-document field.

Copy link
Member

@nicolaiarocci nicolaiarocci left a comment

Choose a reason for hiding this comment

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

Great job. Make sure VALIDATE_FILTERS documentation is updated with the contents of that code comment, then we're all set for the merge.

eve/utils.py Outdated
return "filter on '%s' not allowed" % key
if '*' not in allowed:
def recursive_check_allowed(filter_key, allowed_filters):
# Filter key can be a plain key (e.g. "foo") or a dotted
Copy link
Member

Choose a reason for hiding this comment

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

I would rather move this comment to the documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @nicolaiarocci , thanks for your review!
Just to be sure, did you mean ALLOWED_FILTERS? Also, do you want me to remove this comment from the code once it will be present in the documentation?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, remove it from the code. And yes that's what I meant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I updated the PR with the requested changes ;)

@nicolaiarocci nicolaiarocci added this to the 0.8 milestone Mar 26, 2018
@nicolaiarocci nicolaiarocci merged commit d5659c2 into pyeve:master Mar 27, 2018
nicolaiarocci added a commit that referenced this pull request Mar 27, 2018
@lmoretto lmoretto deleted the 1123_sub-document_VALIDATE_FILTERS branch March 27, 2018 08:23
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.

2 participants