This project is archived and is in readonly mode.

#5041 ✓resolved
Aaron Lifton

Use of marked_for_destruction? in validation callback breaks autosave association behavior

Reported by Aaron Lifton | July 4th, 2010 @ 04:42 AM

I have a debate model, which should have at least 2 sides.
Here's some code:

has_many :sides, :dependent => :destroy
accepts_nested_attributes_for :sides, :allow_destroy => true
validate :has_enough_sides
# ...
def has_enough_sides
    unmarked_sides = sides.reject { |side| side.marked_for_destruction? }
    errors[:sides] << "A debate must have at least 2 sides." if unmarked_sides.size < 2
end

When I comment out "validate :has_enough_sides", changes are made to the nested sides when the debate model is updated with a nested form, as a normal nested form should. When I don't use the "marked_for_destruction?" function it also works fine.

However, when I include that validation, with the use of "marked_for_destruction?", all changes made to the nested models are ignored and no SQL is actually run, and the debate still saves and validates.

Oddly enough, if a side is invalid, the form is rendered again with the attributes of the original sides as if no changes were made.

I've looked through the source code and I'm pretty sure the problem is in here, but I cannot trace it: http://github.com/rails/rails/blob/76f024ac8db82490a99c71d0d8951d67...

Comments and changes to this ticket

  • Neeraj Singh

    Neeraj Singh July 7th, 2010 @ 07:38 PM

    • Importance changed from “” to “Low”

    ticket #4642 should have solved this problem.

  • Neeraj Singh

    Neeraj Singh July 7th, 2010 @ 09:54 PM

    I am not able to reproduce your case with rails edge. Here is my setup.

    ActiveRecord::Schema.define(:version => 20100707175023) do
    
      create_table "questions", :force => true do |t|
        t.text    "name"
        t.integer "survey_id"
      end
    
      create_table "surveys", :force => true do |t|
        t.string "name"
      end
    
    end
    class Question < ActiveRecord::Base
      belongs_to :survey
    end
    class Survey < ActiveRecord::Base
    
      has_many :questions, :dependent => :destroy
    
      accepts_nested_attributes_for :questions, :allow_destroy => true
    
      validate :has_enough_questions
    
      def self.lab
        Survey.delete_all
        Question.delete_all
        survey = Survey.new(:name => 'cool_survey')
        q1 = survey.questions.build(:name => 'q1')
        q2 = survey.questions.build(:name => 'q2')
        survey.save!
        
        survey = Survey.first
        survey.update_attributes({:name => 'super_cool_survery', :questions_attributes => { '0' => {:id => q1.id.to_s, :name => 'q11'},
        '1' => {:id => q2.id.to_s, :name => 'q22'}}})
        puts survey.questions(true).inspect
      end
    
      private
    
      def has_enough_questions
        unmarked_questions = questions.reject { |q| q.marked_for_destruction? }
        errors[:questions] << "A survery must have at least 2 questions." if unmarked_questions.size < 2
      end
    
    end
    

    Just run Survey.lab and you will see the updated result. You can also see the sql statements for the update.

  • Aaron Lifton

    Aaron Lifton July 8th, 2010 @ 11:06 PM

    Great, thanks a lot for helping and trying to reproduce the error.

  • Neeraj Singh

    Neeraj Singh July 8th, 2010 @ 11:09 PM

    @Aaron Is your problem gone or are you still able to reproduce this on rails edge?

  • Aaron Lifton

    Aaron Lifton July 11th, 2010 @ 05:20 PM

    Well I was on Rails 3.0.0.beta4, I'll install rails edge and let you know.

  • Subba

    Subba July 16th, 2010 @ 09:29 PM

    • Assigned user set to “Neeraj Singh”

    Neeraj,
    i tried the above case it seems working for me also.
    as you pointed out 4642 fixed this.

    
    
    class Car < ActiveRecord::Base
      has_many :brakes
      accepts_nested_attributes_for :brakes, :allow_destroy => true
      validate :has_enough_brakes
    
      def has_enough_brakes
        unmarked_sides = brakes.reject { |side| side.marked_for_destruction? }
        if unmarked_sides.size < 2
          errors[:sides] << "A debate must have at least 2 sides."
        end
      end
    
      def self.lab
        car   = Car.new(:name => 'Toyota')
        car.brakes.build(:name => "middle")
        car.brakes.build(:name => "disc")
        car.save
        left  = car.brakes.create(:name => "left")
        car.update_attributes({
          :name => 'Toyota',
          :brakes_attributes => {
            "0" => {
              "name" => "right"
            },
            "1" =>{
              "id" => left.id.to_s,
              "_destroy" => "1"
            }
          }
        })
      end
    
    end
    
  • Neeraj Singh

    Neeraj Singh July 16th, 2010 @ 09:33 PM

    • State changed from “new” to “resolved”

    marking this ticket as resolved. If it is still an issue please report it and it can be reopened.

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