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

apply_finder_options deprecated in Rails 4 #322

Closed
ghost opened this issue Jul 13, 2013 · 13 comments
Closed

apply_finder_options deprecated in Rails 4 #322

ghost opened this issue Jul 13, 2013 · 13 comments
Labels

Comments

@ghost
Copy link

ghost commented Jul 13, 2013

If you call will_paginate with AR finder options:

@suppliers = Suppliers.paginate :order => 'name'

You'll end up with a warning from Rails 4:

DEPRECATION WARNING: #apply_finder_options is deprecated. (called from paginate at .../lib/will_paginate/active_record.rb:154)

Easily fixable by changing the original call:

@suppliers = Suppliers.order("name").paginate

The problem is that in Rails 4.1, the original call is going to fail unless the user includes the activerecord_deprecated_finders gem in their project, and this isn't obvious from the error.

Not sure the best way forward here, or I'd submit a PR. I don't think I'd be in favor of will_paginate dragging in activerecord_deprecated_finders itself though. I'd rather have it reject finder options and warn the user to rewrite, but that feels like it might break a lot of existing code.

@mislav
Copy link
Owner

mislav commented Jul 13, 2013

The solution should be to deprecate the paginate() method in favor of page(). This would result in the same kind of deprecation warning but the error message would at least come from us and could be more helpful.

Patches welcome

@ghost
Copy link

ghost commented Jan 28, 2014

These guys got it straight:
http://stackoverflow.com/questions/18667615/deprecation-warning-apply-finder-options

Just put the :order out...
Worked for me. Cheers.

@cristim
Copy link

cristim commented May 13, 2014

apply_finder_options is now gone. It has been moved to 'activerecord-deprecated_finders' in Rails 4.0 and since 4.1 it's no longer a dependency of Rails, but it's still used inside the will_paginate code.

Could we explicitly use 'activerecord-deprecated_finders' on Rails 4.x?

@mislav
Copy link
Owner

mislav commented May 14, 2014

Users who need this functionality should add that gem to their Gemfile. However, I would advise simpley switching all pagination calls to the Arel syntax to avoid this dependency altogether.

@cristim
Copy link

cristim commented May 16, 2014

It looks like just the Gemfile doesn't help, will_paginate/lib/will_paginate/active_record.rb still needs to explicitly require the 'activerecord-deprecated_finders' on Rails 4.1.

@nathany nathany mentioned this issue Jun 17, 2014
5 tasks
@mislav mislav added the todo label Jun 18, 2014
@graaff
Copy link

graaff commented Aug 1, 2014

The way in which this is handled now is to catch a LoadError that may occur when trying to load activerecord-deprecated_finders, but this isn't a robust method, and it may fail when e.g. bundler is not used or activerecord-deprecated_finders is in the load path for another reason. Would it not be better to test the activerecord version instead? Something like:

if ActiveRecord::VERSION::MAJOR >= 4
  require 'active_record/deprecated_finders'
end

This will also produce a more explicit failure with Rails 4.1 applications without activerecord-deprecated_finders in the Gemfile.

@deanpcmad
Copy link

Any reason why this hasn't been fixed yet? I shouldn't have to add the activerecord-deprecated_finders gem to use this gem in Rails 4+

@mislav
Copy link
Owner

mislav commented Dec 21, 2014

@deanperry And you don't need to add the gem to use will_paginate in Rails 4+. Is something giving you trouble? Are you getting exceptions? Can we see the invocation code?

@deanpcmad
Copy link

I've sorted it. I'm upgrading a Rails 3.2 app to 4.2 and turns out it was some old conditions: model code that was causing the issues. Sorry!

@mislav
Copy link
Owner

mislav commented Dec 21, 2014

Correct, because apply_finder_options was deprecated and now unavailable, you can't have calls like

User.paginate(per_page: 30, conditions: "...", order: "created_at DESC")

That's alright, because since Arel we can all just do

User.where(...).order("created_at DESC").limit(30).page(params[:page])

@deanpcmad
Copy link

Yep, I didn't write the original code and it was a bit confusing. I ended up scrapping it and using the gem Ransack for searching :)

@dtwconsulting
Copy link

I am having a hard time trying to understand how to rewrite the following so as not to get this error. In my case I have multiple search fields. The above only shows a single condition. Sorry for my lack of knowledge.

My current code is:
@search = Customer.search(params[:q])
@customers = @search.result.paginate(page: params[:page], :per_page => 30, :order => 'last_name ASC')

I get the .order, .limit and .page, just not sure what needs to go into the .where section since there are 3 different fields that can be searched on in the customers.

@mislav
Copy link
Owner

mislav commented Jan 3, 2016

I've dropped use of apply_finder_options in master.

@mislav mislav closed this as completed Jan 3, 2016
rakvium added a commit to rakvium/blog that referenced this issue Sep 22, 2017
As far as #apply_finder_options deprecated in Rails 4.1
(See mislav/will_paginate#322)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants