#2064 [AutosaveAssociation] should not validate associated model if marked_for_destruction - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#2064 ✓resolved

[AutosaveAssociation] should not validate associated model if marked_for_destruction

Reported by Dmitry Polushkin | February 24th, 2009 @ 11:33 PM | in 2.3.6

While coding an application based on 2.3RC1 I've stuck with this behaviour:


  def test_should_skip_validation_and_destroy_the_associated_model
    @pirate.ship.update_attribute(:name, '')

    assert !@pirate.valid?
    assert @pirate.reload.valid?

    assert_difference('Ship.count', -1) do
      @pirate.update_attributes({ :ship_attributes => { :id => @ship.id,  :_delete => true } })
    end
    assert @pirate.valid?
  end
end

Should not validate associated model if it were marked_for_destruction.

Comments and changes to this ticket

You flagged this item as spam.
Dmitry Polushkin February 24th, 2009 @ 11:35 PM
- Tag changed from “2.3-rc1, accepts_nested_attributes_for, validation” to “2.3-rc1, accepts_nested_attributes_for, arked_for_destruction, validation”
- Title changed from “nested attributes” to “[nested attributes] should not validate associated model marked_for_destruction”
Eloy Duran February 24th, 2009 @ 11:56 PM
- Title changed from “[nested attributes] should not validate associated model marked_for_destruction” to “[AutosaveAssociation] should not validate associated model if marked_for_destruction”
Good catch!
DHH February 25th, 2009 @ 06:24 PM
- Milestone cleared.
Eloy Duran February 26th, 2009 @ 11:00 AM
Fixed. This patch relies on the patch from #1930: http://rails.lighthouseapp.com:8...
Eloy Duran February 26th, 2009 @ 11:01 AM
And the patch.
- do_not_validate_records_marked_for_destruction.diff 4.5 KB
Eloy Duran February 26th, 2009 @ 12:47 PM
- Assigned user changed from “Eloy Duran” to “Michael Koziarski”
You flagged this item as spam.
Dmitry Polushkin February 27th, 2009 @ 12:16 AM
Just applied both patches, works great! Thanks.
Repository February 27th, 2009 @ 12:57 PM
(from [e2b925fa68203607fb4f9a9aa457c0435c969f9a]) Don't validate records of an :autosave association if they're marked for destruction. [#2064 status:resolved]

Signed-off-by: David Heinemeier Hansson david@loudthinking.com http://github.com/rails/rails/co...
DHH February 27th, 2009 @ 12:58 PM
- State changed from “new” to “resolved”

Dmitry Polushkin March 23rd, 2009 @ 02:23 AM

Assigned user changed from “Michael Koziarski” to “Eloy Duran”

Oh, found another bug related to this topic. When Model marked_for_destruction? and has custom validation, then it's enters to the validation method, but it shouldn't:


  class ::Ship
    validate :validate_name

    private

    def validate_name
     if name && 'mustnt-go-here' == name
       errors.add(:name, :name_invalid)
       throw :mustnt_validate_when_marked_for_destruction
     end
    end
  end

  def test_should_skip_validation_and_destroy_the_associated_model
    @pirate.ship.update_attribute(:name, 'mustnt-go-here')
    @pirate.ship.mark_for_destruction

    assert_nothing_thrown do
      @pirate.update_attributes({ :ship_attributes => { :id => @ship.id,  :_delete => true } })
    end
  end

Thanks!

Eloy Duran April 22nd, 2009 @ 09:31 AM
- Tag changed from “2.3-rc1, accepts_nested_attributes_for, arked_for_destruction, validation” to “2.3.2, accepts_nested_attributes_for, marked_for_destruction, validation”
- State changed from “resolved” to “open”
@Dmitry: Sorry that I missed your new comment. Next time please create a new ticket or change the state to open again.

About the problem; have you investigated already where the problem lies? And/or maybe a naive patch or something?
You flagged this item as spam.
Dmitry Polushkin April 22nd, 2009 @ 09:48 AM
@Eloy: no, I've tried, but didn't found where is the problem lies.

Dmitry Polushkin April 23rd, 2009 @ 03:37 PM

The problem is in:


    def association_valid?(reflection, association)
      unless valid = association.valid?
        if reflection.options[:autosave]
          unless association.marked_for_destruction?

condition unless association.marked_for_destruction? must be before the valid? method invoked.

Eloy Duran April 23rd, 2009 @ 03:43 PM
That seems obvious yes, good catch again! Are you gonna try to write a patch for it? If not, that's ok then I'll fix it later on.
You flagged this item as spam.
Dmitry Polushkin April 23rd, 2009 @ 03:54 PM
Not so good in git, actually. :)
You flagged this item as spam.
Dmitry Polushkin June 2nd, 2009 @ 08:29 PM
Patch with tests.
- skip_validation_on_marked_for_destruction.diff 3.8 KB
Michael Koziarski June 9th, 2009 @ 08:39 AM
- Milestone set to “2.3.4”
You flagged this item as spam.
Dmitry Polushkin July 3rd, 2009 @ 02:49 PM
New patch is here. Please, if it is possible, push it before 2.3.4 will come out. Thanks.
- autosave_skip_validation_fix_patch.diff 2.8 KB
Dmitry Polushkin July 3rd, 2009 @ 02:51 PM
- no changes were found...
- autosave_skip_validation_fix_patch.diff 2.8 KB
José Valim July 12th, 2009 @ 12:22 PM
+1, applied and it works as expected.
Eloy Duran July 12th, 2009 @ 12:35 PM
- State changed from “open” to “verified”
I have verified and applied this patch on my branch of 2-3-stable, which Micheal will look at and merge in before 2.3.4.

http://github.com/alloy/rails/commit/dbc56d67b644944dcff09afbcb2f7d...

Thanks!
Jeremy Kemper September 11th, 2009 @ 11:04 PM
- Milestone changed from “2.3.4” to “2.3.6”
[milestone:id#50064 bulk edit command]
Repository September 13th, 2009 @ 02:33 AM
- State changed from “verified” to “resolved”
(from [c52a50ec51f7466411c032d676590be67fd6a956]) Fix autosave association to skip validation if it is marked for destruction. [#2064 state:resolved] Signed-off-by: Eloy Duran eloy.de.enige@gmail.com
http://github.com/rails/rails/commit/c52a50ec51f7466411c032d676590b...
Repository September 13th, 2009 @ 02:33 AM
(from [845f62f4730fb9ab8847033f9ab7435c40006662]) Fix autosave association to skip validation if it is marked for destruction. [#2064 state:resolved] Signed-off-by: Eloy Duran eloy.de.enige@gmail.com
http://github.com/rails/rails/commit/845f62f4730fb9ab8847033f9ab743...
Ryan Bigg October 9th, 2010 @ 10:01 PM
- Tag cleared.
- Importance changed from “” to “Low”
Automatic cleanup of spam.
You flagged this item as spam.
bingbing March 26th, 2011 @ 02:04 AM
louis vuitton watches