paginates_per() doesn't work with subclasses #342

Closed
coneybeare opened this Issue Jan 17, 2013 · 12 comments

Projects

None yet

6 participants

@coneybeare

Given:

Kaminari.configure do |config|
  config.default_per_page = 100
end

class ApplicationController < ActionController::Base
  has_scope :page, :default => 1
end

class Foo < Bar
end

class Bar < ActiveRecord::Base
end

...setting the paginates_per does not work. In the console:

>> Foo.default_per_page
=> 100
>> Foo.paginates_per 25
=> 25
>> Foo.default_per_page
=> 25
>> Foo.page 1
  Foo Load (0.9ms)  SELECT `foos`.* FROM `foos` LIMIT 100 OFFSET 0
=> []
>> Bar.default_per_page
=> 100

As you can see, kaminari is using the superclasses default_per_page instead of the subclasses.

I know changing this at runtime is not usually something that is done, but I am getting caught up on this in my rspec tests where I am changing the normally high per page to be a much lower value. Is this a bug or is it something I am doing wrong*.

*Note, when there is no model subclassing this works as intended.

@coneybeare

I think this is because the @ style class variables, used here, are not inheritable, but the cattr_accessor style ones are. with cattr_accessor, changing the subclass variable also changes the superclass variable.

Of course, this may not be the intended behavior here. Perhaps the reading method needs to be tweaked so that when formulating the sql, the subclasses value is read instead of the superclass.

@eitoball

@coneybeare, is the output of Foo.page 1 correct? I think that it should have been like SELECTbars.* FROMbarsLIMIT 100 OFFSET 0 because ActiveRecord uses Single Table Inheritance pattern by default.

@coneybeare

The output above is correct. This is not using STI, but a simple subclass. If it was STI, it would have been something like SELECTbars.* FROMbarsWHEREtype= "Foo" LIMIT 100 OFFSET 0.

@eitoball
eitoball commented Feb 4, 2013

@coneybeare, I could not reproduce this issue. I set up a rails app with the setup you mentioned. The result of Foo.page 1 was:

  Foo Load (0.2ms)  SELECT "bars".* FROM "bars" LIMIT 100 OFFSET 0

on rails console. If I make Bar to be an abstract class, I got the same result as yours. I used Rails 3.2.11 and sqlite3 for database.

@coneybeare

I think you misunderstood how I subclass, or I was not clear.

I have a foos table. I also have a Foo class but this class does not inherit from ActiveRecord, it inherits from "Bar"

class Foo > Bar
...
end

class Bar > ActiveRecord::Base
...
end

There is no bars table that exists in my db. I think that in trying to reproduce, you created a bars table instead of a foos table.

@take
take commented Nov 18, 2013

@coneybeare Hi, I was unable to reproduce this in Rails 4.0.1, and the master of kaminari

the app is https://github.com/take/kaminari_issue_342/commits/master

[master][~/Projects/kaminari/rails_apps/issue_342]$ rails c                                                                                                     rbenv:2.0.0-p247
[1] pry(main)> Foo.default_per_page
=> 100
[2] pry(main)> Foo.paginates_per 25
=> 25
[3] pry(main)> Foo.default_per_page
=> 25
[4] pry(main)> Foo.page 1
  Foo Load (1.1ms)  SELECT "foos".* FROM "foos" LIMIT 25 OFFSET 0
=> []
[5] pry(main)>

what version are u using?

@take
take commented Nov 18, 2013

I also tried rails 3.2.x(and kaminari master), but was unable to reproduce

[master][~/Projects/kaminari/rails_apps/issue_342_with_old_rails]$ rails c                                                                                      rbenv:2.0.0-p247
Loading development environment (Rails 3.2.15)
[1] pry(main)> Foo.default_per_page
=> 100
[2] pry(main)> Foo.paginates_per 25
=> 25
[3] pry(main)> Foo.default_per_page
=> 25
[4] pry(main)> Foo.page 1
  Foo Load (0.1ms)  SELECT "foos".* FROM "foos" LIMIT 25 OFFSET 0
=> []
[5] pry(main)>

app is at https://github.com/take/kaminari_issue_342_with_old_rails/commits/master

@take
take commented Nov 18, 2013

After further investigation, found out that commit ecd2578 fixed this problem.
@coneybeare please update ur kaminari to the master of this repo.

gem 'kaminari', github: 'amatsuda/kaminari'

This will fix ur problem I think 😃

@yuki24
Collaborator
yuki24 commented Nov 18, 2013

Thank you everyone! I'm closing this issue since this has been fixed on master.

@yuki24 yuki24 closed this Nov 18, 2013
@zzak
Collaborator
zzak commented Nov 18, 2013

👏 getting closer to a release!

@take
take commented Nov 18, 2013

😋

@asmatameem

Can this please be fixed for mongoid as well ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment