0

With the following code I'm ordering the products whichever one was created or update last:

@products = Product.where(:id=> @account.id).order('greatest(created_at, updated_at) desc').page(params[:page]).per(12)

It works just fine, but since I'm getting the following message in the console I was wondering if there is a better way to implement the above??

DEPRECATION WARNING: Dangerous query method (method whose arguments 
are used as raw SQL) called with non-attribute argument(s): 
"greatest(created_at, updated_at) desc". 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().
Dev
  • 437
  • 6
  • 25
  • 1
    [See](https://stackoverflow.com/questions/48897070/deprecation-warning-dangerous-query-method-random-record-in-activerecord-5) – Sebastián Palma Apr 25 '19 at 12:35
  • thanks @Sebastian Palma, I wrapped it around `Arel.sql` and the message doesnt appear anymore – Dev Apr 25 '19 at 12:45
  • 3
    Your created_at should never be greater than your updated_at, and ActiveRecord always sets an updated_at, so you should just be able to `.order(updated_at: :desc)`. I don’t see a reason to use `greatest` at all unless you’re doing something really weird with how you store your data. – Nate Apr 25 '19 at 12:49
  • @Nate you have a point.. it might be better to just use `.order(updated_at: :desc)`. I'm not doing anything weird, I just want whatever record was created or updated most recently on the top. – Dev Apr 25 '19 at 12:59
  • Also, are you sure your where clause is correct? Typically `id` is unique, which means you’d only ever get 0-1 products back from your query. I’m wondering if you actually want `Product.where(:account_id => @account.id)`? If so, the more “Rails” way would be to say that Account has many products, and do `@products = @account.products.order(updated_at: :desc).page(params[:page]).per(12)`. – Nate Apr 25 '19 at 13:07
  • 1
    @Dev Ordering by updated_at descending should suffice then. And it should be a little faster for your database to do the sorting since it doesn’t need to take created_at into account or figure out which is bigger. – Nate Apr 25 '19 at 13:10
  • I think this qualifies as a duplicate but @Nate is also right about not needing `greatest` at all due to the way ActiveRecord handles `created_at` and `updated_at`. – mu is too short Apr 25 '19 at 17:51

1 Answers1

0

Wrap it in Arel.sql call as deprecation message suggest:

@products = Product.where(:id=> @account.id).order(Arel.sql('greatest(created_at, updated_at) desc')).page(params[:page]).per(12)

If curious check out this issue in rails repo. There are links to original implementation and why it was made

Martin
  • 4,042
  • 2
  • 19
  • 29