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 building sortable query from ActionController::Parameters in Rails 5 #10

Conversation

karwank
Copy link
Contributor

@karwank karwank commented Jan 13, 2017

I had to add this metthod in Rails 5 because ActionController::Parameters object doesn't return simple hash as default. I don't know how it works with previous versions of Rails.

@hunterae
Copy link
Owner

Hey @karwank . Was this breaking in Rails 5, or just generating the wrong sort url (i.e. one that had the controller and action as url params)? Is it the parameters.delete call that wasn't working properly. This code will not work in Rails 3, and possibly Rails 4 if strong params is not turned on, as that #to_unsafe_h call does not seem to be available to normal hashes.

@scottrobertson
Copy link

@hunterae just to confirm, this is breaking in rails 5. I have had to remove sortable: true for now. I am getting:

ruby-2.2.3/gems/table-for-3.6.0/app/views/table_for/_table_for.html.erb where line #29 raised:
wrong number of arguments (0 for 1)

@hunterae
Copy link
Owner

@scottrobertson : Can you please provide some details about how to reproduce? Is it just by adding sortable: true to a table_for call that renders a collection of ActiveRecord objects?

@lafeber
Copy link

lafeber commented May 12, 2017

@hunterae yes, just by adding sortable: true it breaks. The reason the tests break is due to byebug... the forked version works for me!

hunterae added a commit that referenced this pull request May 13, 2017
ActionController::Parameters#to_query was being called without any arguments
as reported and fixed by @karwank here #10.
Thanks @karwank.
@hunterae
Copy link
Owner

@karwank / @scottrobertson / @lafeber : I have merge the essence of this PR and it can be found by upgrading to version 3.6.1. Thanks everybody.

@hunterae hunterae closed this May 13, 2017
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