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

11539 empty lookup handle non boolean params #11556

Closed
wants to merge 1 commit into from

Conversation

arthanson
Copy link
Collaborator

Fixes: #11539

The issue requested that a 404 be returned, but IMHO this is not consistent with how other filter params work, if you filter on "region_id=ss" it won't 404 but just return no values from the query. This change uses strtobool "True values are y, yes, t, true, on and 1; false values are n, no, f, false, off and 0" for anything else it interprets it as false. Also cleaned up the return string to make it a bit easier to read what exactly is going on. Unfortunately strtobool raises a ValueError if the string doesn't match a valid input, so needed to wrap that.

@jeremystretch
Copy link
Member

Rather than invoking strtobool(), it may be preferable to remain as consistent with Django as possible by replicating the logic of its own BooleanField class. This would effectively treat anything other than the strings "false" or "0" (after calling lower()) as true.

@jeremystretch
Copy link
Member

After digging into this a bit, it's unfortunately going to require a lot more work to resolve. The lookup itself should only accept boolean values; the validation should happen in the filter. I've opened PR #11784 to capture my work toward a resolution, however more work remains to be done.

@jeremystretch jeremystretch deleted the 11539-boolean-filters branch February 17, 2023 20:23
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 20, 2023
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.

Insufficent validation of boolean filters
2 participants