Use base_class instead of superclass in parent_class_name

[brchristian]

I was having a problem supporting polymorphic models in my application, and traced it back to this line.

If there are multiple levels of inheritance, this will go back one level -- but only exactly one.

For instance, let's say I have

Fruit
Fruit::Apple
Fruit::Apple::Fuji

When I follow a Fruit, it's stored in the db as follower_type = "Fruit".
When I follow a Fruit::Apple, it's also stored in the db as follower_type = "Fruit".
But, when I follow a Fruit, it's stored in the db as follower_type = "Fruit::Apple". This causes all kinds of things to break in my app.

Using base_class instead of superclass appears to give the intended behavior here.

I believe this PR may also address the issues described in #8.

I believe this may also be a more general solution for the use of abstract classes than the one given in #42, as the docs for the base_class method say "Returns the class descending directly from ActiveRecord::Base, or an abstract class" (my emphasis). That said, I haven't looked into it more deeply than that.

[bricesanchez]

Any news about this PR? i need it in my app :)

[amoludage]

Hi @bricesanchez I think the @Owners of this repository is not maintaining this repository, because I have seen lots of PR's pending into this repo and no one is replying on them.

So if you want to use this functionality into your app you can add this https://github.com/brchristian/acts_as_follower/tree/patch-1 forked repository into your Gemfile.

Example below

gem "acts_as_follower", github: "brchristian/patch-1"

Thanks.

[jeremylynch]

@jcoyne any chance we could get this merged? Its necessary for Rails 5 compatibility. Thanks

[jcoyne]

@mrjlynch Sorry, I'm no longer maintaining this as my employer has stopped using it.

[jcoyne]

Is there a test for this behavior? Does it work under Rails 4 as well as Rails 5?

[brchristian]

@jcoyne This area of the code was modified when #76 was merged, so I would need to look at the changes and rebase. I’ll work on getting a test and follow up. Should be compatible with both Rails 4 and 5.

[brchristian]

@jcoyne In my opinion, #76 introduced a large amount of complexity which is actually not necessary, for either Rails 5-compatibility or for STI-compatibility. As discussed in #42, the base_class method handles abstract classes elegantly, and so no extra shenanigans are necessary when dealing with the ApplicationRecord abstract class introduced in Rails 5.

My suggested PR here is based on the idea that supporting "custom parent classes" is not something acts_as_follower needs to do by default (I have a hard time thinking of a use case for this), and that base_class handles all of the Rails 5 issues and the issues discussed in #42 and in this thread. The upshot is a dramatically simpler codebase, compatible with Rails 4 and 5, and with no STI hiccups.

I have rebased this branch on the current master, and following the "red, green, refactor" pattern have included:

1. a failing test (845b273)
2. a patch applying base_class to fix the test (81833d0)
3. a refactor that simplifies to the minimal changes needed to make the tests pass (48b22a6)

Let me know if there’s anything else I can do to help out here, or any issues I’ve overlooked. Cheers!

[jeremylynch]

Hi @jcoyne, any chance we could get this merged? Thanks

[jcoyne]

Looks good. Just a few issues about changing the public API without depreciations

[jcoyne]

This change is not backward compatible. This method should be retained and a deprecation should be emitted

[jcoyne]

This change is not backward compatible. This method should be retained and a deprecation should be emitted

[brchristian]

@jcoyne Restored the custom_parent_classes functionality and added deprecation warnings. 👍

[brchristian]

@jcoyne Anything else needed in this branch, or we good to merge?

[jcoyne]

@brchristian thanks for the reminder. Good work.

[brchristian]

Welcome! Cheers!

[brchristian]

@johnmpotter I saw that you forked this PR branch of my repo. Now that it is merged, it looks like you can get back on to the master branch here if you like!

[johnmpotter]

@brchristian Awesome, that makes life easier, thanks for your work on this! 😃

[PelagicDev]

Just to be clear, this PR now deprecates the use of user.following_users & user.following_by_type('Book'); is this correct?

Going forward, we should avoid using these methods?

[brchristian]

@jegodwin The deprecated behavior is simply the use of explicit custom "parent classes" via things like

ActsAsFollower.custom_parent_classes = [CustomRecord]

or

ActsAsFollower.setup do |c|
  c.custom_parent_classes = [CustomRecord]
end

Your examples of following_users and following_by_type should be unaffected by this PR.

[PelagicDev]

@brchristian okay, got it. I was seeing the deprecation warning in the rails server logs whenever I called the following_users and following_by_type('Model') methods. So, I thought that it was referring to those methods.

Thanks for clarification.