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

Config option for turning off custom scope argument sanitization #742

Conversation

garettarrowood
Copy link
Contributor

@garettarrowood garettarrowood commented Dec 11, 2016

There has been a lot of discussion about ways to get around Ransack converting scope values to booleans. Here are just a few places: #509, #593, #580, #566. This PR specifically addresses the tests presented in #575.

An easy solution to this would be to simply stop making these conversions. But then we run the risk of breaking backwards compatibility. So instead of changing them, I've added a config option to turn them off.

It'd be easy to modify this to make the config option more granular. We could set it up for the user to specify what exact truthy/falsey values they would like converted, and use the default settings if this config is NOT specified. For example, instead of:

    # Ransack.configure do |config|
    #   # Accept my custom scope values as what they are.
    #   config.sanitize_custom_scope_booleans = false
    # end

we could do this:

    # Ransack.configure do |config|
    #   # Only convert these values to booleans
    #   config.truthy_values_to_convert_in_custom_scopes = ['TRUE', 'true', '1']
    #   config.falsey_values_to_convert_in_custom_scopes = ['FALSE', 'no way no how']
    # end

But I don't know if that level of detail is needed/desired. If being able to disable boolean conversion is not enough, this could be added later on.

@truonglocbinh93
Copy link

@garettarrowood I added your config but error when start server Is there any solution for this problem? Please help me.

@stevebissett
Copy link

This was really helpful today. Thank you @garettarrowood 🏆

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.

4 participants