This project is archived and is in readonly mode.

#974 ✓stale
Brad Gessler

attribute_type is not set to null if :dependent => :nullify is specified for a polymorphic belongs_to association

Reported by Brad Gessler | September 5th, 2008 @ 05:56 AM | in 3.x

Given the code


class Person
  has_one :thingy, :as => :thingable, :dependent => :nullify
end

class Thing
  belongs_to :thingable, :polymorphic => true
end

If I do the following


p = Person.first
p.thingy = Thing.first
p.save!  #=> true
p.thingy = nil
p.save! # =>

Rails will set the attributes on thingy in the database as such:

thingable_id = nil; thingable_type = Person

It is expected that :nullify would set the database column, thingable_type, to nil as well:

thingable_id = nil; thingable_type = nil

Starting at line 1451 in associations.rb on edge (http://github.com/rails/rails/tr...)


when :nullify
  module_eval "before_destroy { |record| #{reflection.class_name}.update_all(%(#{reflection.primary_key_name} = NULL),  %(#{dependent_conditions})) }"

It should be something like:


when :nullify
  if reflection.options[:as]
    module_eval "before_destroy { |record| #{reflection.class_name}.update_all(%(#{reflection.options[:as]}_id = NULL, #{reflection.options[:as]}_type = NULL ),  %(#{dependent_conditions})) }"
  else
    module_eval "before_destroy { |record| #{reflection.class_name}.update_all(%(#{reflection.primary_key_name} = NULL),  %(#{dependent_conditions})) }"
  end

Comments and changes to this ticket

  • Pratik

    Pratik December 20th, 2008 @ 04:00 PM

    • Assigned user changed from “Jeremy Kemper” to “Pratik”
    • State changed from “new” to “incomplete”

    Can you please upload a patch with a test case - http://rails.lighthouseapp.com/p... ?

    Thanks.

  • Ryan Bigg

    Ryan Bigg April 10th, 2010 @ 11:18 PM

    • Tag changed from 2.1, activerecord, association, bug, nullify, polymorphic to 3.0.0.beta, activerecord, association, bug, nullify, polymorphic

    This is still broken in Rails 3.0.0 beta 2.

  • Ryan Bigg

    Ryan Bigg April 10th, 2010 @ 11:22 PM

    • Tag changed from 3.0.0.beta, activerecord, association, bug, nullify, polymorphic to 3.0, activerecord, association, bug, nullify, polymorphic
  • Tanel Suurhans

    Tanel Suurhans April 30th, 2010 @ 02:08 PM

    In my opinion this is the expected behavior, as :dependent => :nullify should only null the thingy when person is destroyed.
    The bug itself seems to exist on destroying records also, just that the original authors example is a bit wrong.

    person = Person.first
    person.thingy = Thing.first
    person.save
    person.destroy
    
    Thing.first.thingable_id # => nil
    Thing.first.thingable_type # => Person
    

    So it behaves like the OP described: nulls the id field but not the type field, but does so when destroying the record defining the :dependent => :nullify condition.

    I have attached a patch with failing tests and a proposed fix for this issue for master (rails 3).

    Tho clarify why i opted to use association.update_attribute(reflection.options[:as], nil) instead of directly setting the id and type column values to nil is due to the association accessor handling the in memory state of the has_one object (setting the assoc to nil) in addition to handling the database fields.

  • Tanel Suurhans

    Tanel Suurhans April 30th, 2010 @ 02:17 PM

    Also attached a patch to fix the :nullify behavior on destruction for 2-3-stable branch with tests.

  • Tanel Suurhans

    Tanel Suurhans April 30th, 2010 @ 04:17 PM

    Attached proposed fix for the original problem described in the ticket. Managed to solve the issue with relation_type field not set to nil.

    Previous code didn't handle the foreign_type column

      @target[@reflection.primary_key_name] = nil
    

    This handles both relevant columns, although there might be a better way to handle it

     @target.send("#{@reflection.options.fetch(:as, @reflection.primary_key_name)}=", nil)
    

    What i didn't manage is the in-memory state of the object on the other end of the has_one association.
    Seems that the reflection target and the original object are not the same, and the in-memory state stays in limbo until its explicitly reloaded. Perhaps anyone has some ideas how to handle that, as or the usage it seems a bit inconsistent with lets say a regular belongs_to relation being set to nil with both ends of the association knowing about it right away.

    I will provide a patch for this issue for the 2-3-stable too if it looks okay.

  • Jeremy Kemper

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

    • Milestone changed from 2.x to 3.x
  • rails

    rails April 5th, 2011 @ 01:00 AM

    • Tag changed from 3.0, activerecord, association, bug, nullify, polymorphic to 30, activerecord, association, bug, nullify, polymorphic
    • State changed from “incomplete” to “open”

    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.

  • rails

    rails April 5th, 2011 @ 01:00 AM

    • State changed from “open” to “stale”
  • joson

    joson April 29th, 2011 @ 09:14 AM

    A Replica Breitling is a timepiece of high quality and functionality and you will have all the class, prestige and luxury of a wealthy, successful individual. By choosing from our selection of luxury Replica Watches , you can improve your self-esteem and feel confident to enter new circles of business associates and friends. These Rolex Replicas will surely enhance your style and only you will know the watch you wear did not cost you $1,000's of dollars.

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>

Pages