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

DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s): "max(taggings.created_at)" #892

Closed
aalbagarcia opened this issue Apr 20, 2018 · 3 comments

Comments

@aalbagarcia
Copy link

Hi,

I'm trying to upgrade our rails application to version 5.2. I'm using the fork from @Fodoj mentioned in pull request #887.

Using his pull request, the tests in our test suite which are related to acts-as-taggable-on are all back to green but we are getting a ton of deprecation warnings like the following:

DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) 
called with non-attribute argument(s): "max(taggings.created_at)". Non-attribute arguments will be 
disallowed in Rails 6.0. This method should not be called with user-provided values, such as request 
parameters or model attributes. Known-safe values can be passed by wrapping them in Arel.sql(). 
(called from all_tags_on at /opt/rubies/ruby-2.5.0/lib/ruby/gems/2.5.0/bundler/gems/acts-as-
taggable-on-8a4d41bc3107/lib/acts_as_taggable_on/taggable/core.rb:179)

I gave it a try so I replaced line 179 in lib/acts_as_taggable_on/taggable/core.rb:179

scope.order("max(#{tagging_table_name}.created_at)").group(group_columns)

with

scope.order(Arel.sql("max(#{tagging_table_name}.created_at)")).group(group_columns)

and the deprecation warnings went away.

@Fodoj
Copy link
Contributor

Fodoj commented Apr 22, 2018

I will try to add your fix today/tomorrow to my PR, thanks a lot. I will also updates deps to Rails 5.2, so it can be merged finally.

@aalbagarcia
Copy link
Author

@Fodoj You are the one to thank!! Looking forward to see your PR finally merged. Thanks!!

@Fodoj
Copy link
Contributor

Fodoj commented Apr 24, 2018

@aalbagarcia I've introduced your fix, thanks a lot - #887.

@seuros seuros closed this as completed Jun 17, 2018
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

No branches or pull requests

3 participants