This project is archived and is in readonly mode.

#2435 ✓stale
Esteban

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

Reported by Esteban | April 6th, 2009 @ 11:44 PM | in 3.x

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. "

Comments and changes to this ticket

  • Matt Jones

    Matt Jones June 8th, 2009 @ 09:28 PM

    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.

  • Mike Wyatt

    Mike Wyatt October 21st, 2009 @ 05:52 AM

    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?

  • Mike Wyatt

    Mike Wyatt October 21st, 2009 @ 06:14 AM

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

  • Matt Jones

    Matt Jones October 21st, 2009 @ 07:00 AM

    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.

  • Mike Wyatt

    Mike Wyatt October 21st, 2009 @ 08:10 AM

    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

  • juozasgaigalas (at gmail)

    juozasgaigalas (at gmail) November 25th, 2009 @ 09:50 AM

    • 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.

  • juozasgaigalas (at gmail)

    juozasgaigalas (at gmail) November 25th, 2009 @ 09:53 AM

    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
    
  • Jeremy Kemper

    Jeremy Kemper May 4th, 2010 @ 06:48 PM

    • Milestone changed from 2.x to 3.x
  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:39 PM

    • 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.

  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:39 PM

    • State changed from “open” to “stale”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>