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

ignore blank strings in numeric filters #52

Open
goravbhootra opened this issue Jun 23, 2018 · 7 comments
Open

ignore blank strings in numeric filters #52

goravbhootra opened this issue Jun 23, 2018 · 7 comments

Comments

@goravbhootra
Copy link

goravbhootra commented Jun 23, 2018

Hi,

I have setup a filter as:

number :status, allow_decimal: false, allowed_values: 1..5

if no status is selected in filters, blank value is passed in params. I am filtering the blank values before sending the filter params to Filtrex.parse_params else it responds with an error. This pattern is so common, I was wondering if it would be good idea to include this as an option in the config - something like:

number :status, allow_decimal: false, allowed_values: 1..5, ignore_blanks: true
@rcdilorenzo
Copy link
Owner

When you say a "blank value" do you mean an empty string or nil? I like this proposal but want to make sure I understand for sure beforehand.

@goravbhootra
Copy link
Author

html form sends empty string. I am applying |> Enum.reject(fn {_, v} -> v == "" end) to filter out the values but it might be good idea to ignore empty string "" and nil both.

@rcdilorenzo
Copy link
Owner

So, does parse_params actually throw an error if you pass an empty string or did you mean your application logic rejects it since it is then filtering using that blank parameter?

@goravbhootra
Copy link
Author

parse_params throws an error since "" is not in the allowed_values

@rcdilorenzo
Copy link
Owner

Oh, I see. Yes, I like this! What do you think about calling it ignore_empty versus ignore_blanks? I'm not sure myself and wonder how those strike you.

@goravbhootra
Copy link
Author

ignore_empty sounds more apt if you are planning to remove only empty strings whereas ignore_blanks, to me, covers empty strings as well as nil. Users will be able to relate to either of them, as long as documentation is clear about what it covers IMHO.

@rcdilorenzo
Copy link
Owner

I agree. Let's go with ignore_blanks then and have that match "" and nil.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants