Support for will_paginate

[balexand]

This pull-request adds support for the will_paginate gem. Please let me know if you'd like me to change anything. Thanks!

[balexand]

If desired, I can remove this development dependency and instead stub WillPaginate::CollectionMethods.

[karmi]

It's perfectly fine to have it as a development dependency.

[balexand]

I just squashed commits after adding documentation to the README.

[karmi]

Brian, this looks nice, I think we should add also an integration test for this? That would probably mean renaming https://github.com/elasticsearch/elasticsearch-rails/blob/master/elasticsearch-model/test/integration/active_record_pagination_test.rb into active_record_pagination_with_kaminari_test.rb and adding a similar one with will_paginate.

[karmi]

Should be "If Kaminari or WillPaginate is loaded (...)"?

[andey]

Does anyone know of a work around to make will_paginate work with elasticsearch-rails for the interim? I see this pull request, and it makes me resist the urge to change the whole app to Kaminari.

[karmi]

@andey I need to wrap up something I'm working on right now, as soon as possible I'll process @balexand's PR, sorry for the delay.

[balexand]

@karmi I'll try to respond to your suggestions shortly. I've been busy and traveling lately.

[karmi]

@balexand No stress -- I'm tied up in the "persistence" gem within this project at the moment anyway.

[karmi]

@balexand Would you be so kind and revisit the comments I've added to commits, Brian? Please also check the current master, there are couple of changes and fixes to the Kaminari pagination.

[sobrinho]

@balexand I'm interested on this, did you had time to work on this?

[balexand]

@karmi Getting an integration test running is proving problematic due to conflicts. If both Elasticsearch::Model::Response::Pagination::Kaminari and Elasticsearch::Model::Response::Pagination::WillPaginate are included in Elasticsearch::Model::Response::Response, then conflicts occur.

Do you have any advice on how to proceed?

In a real app that includes both kaminari and will_paginate, then only the Kaminari integration will be used thanks to this code.

In the unit test, I did this to avoid conflicts. Even though this works, it has potential for conflicts since methods are being added to Results and Records here.

[balexand]

A possible solution would be to break out kaminari and will_paginate into separate test gemfiles like you do with activemodel 3.x and 4.x. Then we could have a single pagination integration test that would run against both gems.

[karmi]

It all depends on how much painful we wanna make this for us. My main takeaway is that Kaminari has already won that battle, and WillPaginate is used mostly in legacy apps. Therefore, I think it makes sense to have WillPaginate support, but not break our legs twisting everything so it fits.

Separate gemfile is one possible solution, though would muddy the separation there. In practice, getting all these gems run smoothly in CI environments is a delicate task... Not sure it's worth complicating it more with another gemfile?

Another possible solution would be to add the support into the elasticsearch-extensions gem, so people would require something like elasticsearch/model/will_paginate from there...

Yet another solution would be do deal with it programatically, and use some Ruby tricks like remove_const. What if you just pushed the commits with the (failing/erroring) integration tests so I can check it out and try locally?

[balexand]

@karmi I've rebased the branch and squashed all commits.

I didn't make it far enough with the integration tests to have anything useful enough to check in. If you want to try any tricks, then it would probably be easiest to just start from this branch.

Kaminari has already won that battle, and WillPaginate is used mostly in legacy apps

Are you basing this statement on anything? They seem to be equally popular. Also, I've had bad experiences with Kaminari. I've found 2 bugs and when I've submitted pull requests, then maintainer has refused to address them without explanation. WillPaginate has always worked perfectly for me, and thus is my pagination gem of choice.

[karmi]

@balexand Thanks!, will look into it.

Kaminari has already won that battle
Are you basing this statement on anything?

Ah, just totally unscientific guess based on my experiences when helping people with Tire, and also based on Rubygems downloads and general noise. I loved WillPaginate when I was doing lots of Rails apps, and there's absolutely nothing wrong with it, at all :)

[karmi]

@balexand Took a ridiculous amount of time, but it is in, thanks! :)

(I've fooled a bit with an integration test, but the auto-discovery makes it obviously a bit tricky... I think we can skip it for now, it's functionally equivalent to Kaminari, and we can add it if we would hit some problems not discovered by unit tests.)