Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

"Polymorphic has_one and has_many should set proper polymorphic_type in case of STI", patch ported to Rails 2.3

#2435

As you asked on irc, here's the patch to Rails 2.3.2

Polymorphic has_one and has_many should set proper polymorphic_type in case of STI http://dev.rubyonrails.org/ticke...

"Right now, it always sets the polymorphic_type as that of base class in case of STI is used. The worse part is, even the tests cases check for the wrong type.

This patch fixes the issue and also correct the test case, in addition to adding new test cases. "

Reported by Esteban · April 6th, 2009 @ 11:44 PM

State: stale
Milestone: 3.x
Assigned to: nobody
Importance: none

Activity

  1. Matt Jones
    Matt Jones

    This is missing the test cases; also, the issue raised in the ticket you linked to specifically occurs and causes problems only in the non-STI case.

    June 8th, 2009 @ 09:28 PM

  2. Mike Wyatt
    Mike Wyatt

    I've updated the diff with passing test cases, only I'm not sure why this is the expected behaviour. in the application I'm working on there are Events and Shows. both are virtually identical except that Shows have bands and venues and there needs to be a distinction between the two, so STI is being attempted

    the problem came up when bringing in a permission system. so saying these are our models:

    class Event < ActiveRecord::Base
    end
    
    class Show < Event
      belongs_to :venue
      has_many :bands
    end
    
    class Permission < ActiveRecord::Base
      belongs_to :user
      belongs_to :object, :polymorphic => true
    end
    

    when creating a show and adding it to the permissions table the object_type column is set to Event, which seems undesirable in this case. is this a problem with the application's design or a problem within ActiveRecord?

    October 21st, 2009 @ 05:52 AM

  3. Mike Wyatt
    Mike Wyatt

    whoops, here's a patch more like the one found in the ticket linked above

    October 21st, 2009 @ 06:14 AM

  4. Matt Jones
    Matt Jones

    The behavior in the current code is correct; putting the STI type into object_type essentially denormalizes the database - you've got the same value in object_type and the association target's type field. Changing (in your example) a Show to another subclass of Event would mean that you'd need to change every polymorphic association key referring to that record. Not a common case, but it happens.

    October 21st, 2009 @ 07:00 AM

  5. Mike Wyatt
    Mike Wyatt

    thanks for the explanation. I also didn't know this little ActiveRecord tidbit:

    Event.find(3)
    -> #<Event ...>
    
    Event.find(4)
    -> #<Show ...>
    

    so when actually iterating through a collection of permissions, their classes are still respected. though it does make you use object.class.base_class.name.to_s when checking for permission

    October 21st, 2009 @ 08:10 AM

  6. juozasgaigalas (at gmail)
    juozasgaigalas (at gmail)
    • Tag set to belongs_to, polymorphic_association, sti
    • Assigned user cleared.

    I have a case where I think this patch makes sense.

    Votables:

    class Poll < ActiveRecord::Base
    has_many :possibles end

    class RankingPoll < Poll
    has_many :votes, :as => :votable, :class_name => "RankingVote" end

    class RatingPoll < Poll
    has_many :votes, :as => :votable, :class_name => "RankingVote" end

    class Request < ActiveRecord::Base
    has_many :votes, :as => :votable, :class_name => "RequestVote" end

    Votes

    class Vote
    belongs_to :poll, :polymorpic => true end

    class RatingVote < Vote
    end

    class RankingVote < Vote
    end

    class RequestVote < Vote
    belongs_to :votable, :polymorphic => :true, :class_name => "Request" end

    Each of these votes and votables has unique behavior (not included here). STI is necessary for the Ranking/Rating polls, but Request is too different and has its own table.

    I want to be able to write:

    poll = RankingPoll.new
    vote = poll.votes.build

    It's possible to make it work by replacing "belong_to :votable, :polymorphic => true"
    with "belongs_to :votable, :foreign_key => ..., :class_name => ..."
    (and that is what I am doing right now instead of the above code)

    However, everything would be simpler if votable_type was always set to the correct 'Poll' subclass.

    Also, I don't ever imagine wanting to switch RatingPoll to RankingPoll by changing it's type - they are pretty different.

    November 25th, 2009 @ 09:50 AM

  7. juozasgaigalas (at gmail)
    juozasgaigalas (at gmail)

    Same code as above but with formatting.

    Votables

    class Poll < ActiveRecord::Base
      has_many :possibles 
    end
    
    class RankingPoll < Poll
      has_many :votes, :as => :votable, :class_name => "RankingVote" 
    end
    
    class RatingPoll < Poll
      has_many :votes, :as => :votable, :class_name => "RankingVote" 
    end
    
    class Request < ActiveRecord::Base
      has_many :votes, :as => :votable, :class_name => "RequestVote" 
    end
    

    Votes

    class Vote
      belongs_to :poll, :polymorpic => true 
    end
    
    class RatingVote < Vote
    end
    
    class RankingVote < Vote
    end
    
    class RequestVote < Vote
      belongs_to :votable, :polymorphic => :true, :class_name => "Request" 
    end
    

    November 25th, 2009 @ 09:53 AM

  8. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.x to 3.x

    May 4th, 2010 @ 06:48 PM

  9. Santiago Pastorino
    Santiago Pastorino
    • State changed from new to open
    • Importance changed from to

    This issue has been automatically marked as stale because it has not been commented on for at least three months.

    The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

    Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

    February 2nd, 2011 @ 04:39 PM

  10. Santiago Pastorino
    Santiago Pastorino
    • State changed from open to stale

    February 2nd, 2011 @ 04:39 PM