Subclasses are including ActiveRecordModelExtension too fast #262

Closed
wants to merge 1 commit into
from

Projects

None yet

4 participants

@rindek

Hi,

In regard of this issue: #119
Everything works fine, unless you want to change the configuration of Kaminari (for example change the page_method_name)

I'm using paper_trail in project, but I'm also kind of forced to change the page_method_name.
First paper_trail is loaded, then kaminari after ActiveSupport loads, then all 'subclasses' are having ActiveRecordModelExtension included, right with Version model which is provided by paper_trail, but unfortunately the extensions are loaded before the configuration file from project is loaded, therefore, Version model has default kaminari config, while other projects model have proper config.

I've changed active_record_extension to load subclasses when first model is 'loaded' in project.

@rindek

Bump. Any updates on this @amatsuda?

@zzak
Collaborator

I'm opposed to this change, the proposal is unclear. Please file a bug if you have an issue that is more specific.

@zzak zzak closed this Aug 7, 2013
@ktaragorn

@rindek I tried to merge this with HEAD in my fork, however it broke with undefined method `per_page_kaminari' for ALL models. Is it possible to check if it is possible to fix this for the latest version of kaminari?

@zzak
Collaborator

I understand this may still be a bug that we can fix, so I will re-open this ticket.

We should consider a way to lazy-load this feature maybe.

@zzak zzak reopened this Nov 24, 2013
@ktaragorn

Small update to what I said earlier, I discovered that master without the fix was also giving me the same problem

@yuki24
Collaborator

I just tried to reproduce this issue and confirmed that PaperTrail::Version class is loaded by Bundler. As a result, it always has page even when we have config/initializers/kaminari_config.rb.

However, a better fix could be to make paper_trail gem a Rails engine and move PaperTrail::Version into app/models/paper_trail/version.rb. If it is under app/models/.., #inherited_with_kaminari will get fired when it's loaded so everything will start working fine. And we don't want to add extra code to kaminari just for fixing edge cases like this since it seems like putting a bandaid.

I'll close this issue and file a new issue on paper_trail. Thanks guys for raising this issue!

@yuki24 yuki24 closed this Mar 19, 2014
@yuki24 yuki24 referenced this pull request in airblade/paper_trail Mar 20, 2014
Closed

Make PaperTrail a Rails engine #347

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