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

remove base class from belongs_to polymorphic association #21385

Closed
wants to merge 1 commit into from

Conversation

dilpreet92
Copy link
Contributor

Fix polymorphic true with inheritance.
Before this PR, it is used to store in type column the class which is directly inherited from the AR::Base.

Example:

 class Asset < ActiveRecord::Base
   belongs_to :attachable, polymorphic: true
 end

 class Post < ActiveRecord::Base
   # Post doesn't have a type column and is not implemented as STI
  has_many :assets, as: :attachable, dependent: :destroy
 end

 class GuestPost < Post
  #Guest Post is inherited From Post
 end

 class MemberPost < Post
  #Member Post is inherited From Post
 end

Example: asset = Asset.new(attachable: GuestPost.new)
                asset.attachable_type would always be equal to 'Post'

After this change, type column can have any class (descendant of the AR::Base)
Now attachable_type can be Post, GuestPost or MemberPost
Example:

asset = Asset.new(attachable: GuestPost.new)
asset.attachable_type would  be equal to 'GuestPost'

@al2o3cr
Copy link
Contributor

al2o3cr commented Oct 7, 2015

This is related to (but not the same issue as) the whole gang of issues referenced here on #20893.

It's different because Post is not STI, so the subclass of Post that a particular row in the posts table represents is determined by how the object is referenced, not by the row alone. In the example given, an Asset with post_id: 42, post_type: 'GuestPost' will get a different flavor of Post object than an Asset with post_id: 42, post_type: 'MemberPost'.

@opsydev
Copy link

opsydev commented Nov 4, 2015

+1 Would love to see this be merged in.

@maclover7
Copy link
Contributor

Please rebase.

@dilpreet92
Copy link
Contributor Author

Done

@arthurnn
Copy link
Member

@rafaelfranca WDYT ? I think the use case make sense. I am curious tho if you can think a situation why this would not work.

@dilpreet92
Copy link
Contributor Author

@maclover7 @rafaelfranca - Any suggestions on this.?

@njakobsen
Copy link
Contributor

njakobsen commented Sep 30, 2016

Was just bitten by this today. The behaviour suggested in the PR seems like it's the most obvious. You build an association off of a subclass, the type column should be that subclass. This is especially useful if the original record is no longer available to read the type from (I'm restoring deleted records)

@chrisdpeters
Copy link

I was also bitten by this this week. I have a model class that extends a class from a gem. The base class name from the gem is being inserted into the polymorphic _type column instead of the real class name.

@boblail
Copy link
Contributor

boblail commented May 23, 2017

+1 😁

@ajays1991
Copy link

+1 👍

@ajays1991
Copy link

any update on this if this is planned to be merged in near future

@rafaelfranca
Copy link
Member

Can you rebase the branch?

@ajays1991
Copy link

@dilpreet92 sorry i am not able to see build error on codeclimate. looks like i its not an open port. let me know if you need spare hand to fix this and getting see in master like me

@dilpreet92
Copy link
Contributor Author

@ajays1991 sure, thanks for the help

@rails-bot
Copy link

rails-bot bot commented Dec 18, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Dec 18, 2019
@rails-bot rails-bot bot closed this Dec 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants