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

Fix a bug in form field filtering #330

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ponychicken
Copy link
Contributor

Fixes #204

The original implementation of the inputs_for_filters function indiscriminately zipped together the fields requested for filtering with their corresponding options without verifying if each field was actually permitted for filtering based on the form's schema.

If the fields requested were [1,2,3] and to_form returned only [2,3] then these two lists would be zipped together resulting in something like [[1,2], [2,3]] instead of [1,1],[2,2].

To fix loop through the fields individually, call to_form on them and only combine them with the opts if to_form returns something.

@ponychicken ponychicken requested a review from woylie as a code owner March 24, 2024 18:24
@ponychicken
Copy link
Contributor Author

no, there is a problem yet, since calling to_form individually will result in all fields having index=0.
Maybe have a look how you would fix the issue with zip best.

@woylie
Copy link
Owner

woylie commented Mar 25, 2024

Thank you for opening the PR. I didn't have a close look at your changes yet, but one way to solve this might be to pass the index to the to_form function as an option.

@ponychicken
Copy link
Contributor Author

Thank you for opening the PR. I didn't have a close look at your changes yet, but one way to solve this might be to pass the index to the to_form function as an option.

Yes, that was my first thought to just use the offset parameter, but then there will be still gaps in the indexes.
So I was too tired to continue looking, but in the end I thought that probably there is no way around restructuring the to_form function.

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.

filters not associated correctly if filters are added for fields that are not filterable
2 participants