This project is archived and is in readonly mode.

#1928 ✓invalid
Gaspard Bucher

[PATCH] accepts_nested_attributes_for should not redefine the attribute writer

Reported by Gaspard Bucher | February 10th, 2009 @ 11:42 AM | in 2.x

Use case:


class Node < ActiveRecord::Base
  acts_as_multiversioned # defines has_one :redaction
                         # defines redaction_attributes=
  accepts_nested_attributes_for :redaction
                         # should not overwrite 'redaction_attributes='
end

Moving 'accepts_nested_attributes_for' before 'acts_as_multiversioned' does not work since it relies on 'has_one' being called in acts_as_multiversioned.

I think not redefining the writer method is the expected behaviour because it produces the same result as when the writer is defined below the call to accepts_nested_attributes_for.

Comments and changes to this ticket

  • Eloy Duran

    Eloy Duran February 10th, 2009 @ 12:52 PM

    I'm not sure I understand what the use would be. It makes it so you can use accepts_nested_attributes_for :redaction', yet it won't create the writer method, so what good is it?

  • Gaspard Bucher

    Gaspard Bucher February 10th, 2009 @ 01:13 PM

    I should have read through the whole nested_attributes.rb file a little slower.

    My case can be solved with a simple :autosave => true on the association instead of using accepts_nested_attributes_for.

    Sorry. Forget about my patch.

    Just a warning on the tests: they are order dependent:

    if "test_should_disable_allow_destroy_by_default" is run before "test_should_be_possible_to_destroy_the_associated_model" it will fail since the first test modifies the Pirate class.

    It might be a good idea to keep the SpecialPirate class from the patch for this issue.

  • Eloy Duran

    Eloy Duran February 10th, 2009 @ 01:29 PM

    Ah you wanted the :autosave behaviour :) You can checkout autosave_association.rb for its source.

    That's weird, the teardown should fix that:

    
  def teardown
    Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true
  end

    I'll have a look later on though, thanks for the heads up.

  • Gaspard Bucher

    Gaspard Bucher February 10th, 2009 @ 01:44 PM

    Eloy... forget about me.

    I bumped on this and am not at all used to class changes in the teardown so I didn't look there first. I just had strange side effects while testing the patch but there's no need to investigate more.

    Thanks for the responses. Closed for me.

  • Eloy Duran

    Eloy Duran February 10th, 2009 @ 02:06 PM

    Ah ok, no problem :)

    Unfortunately I can't close any tickets, or have tickets assigned to me for that matter…

  • Eloy Duran

    Eloy Duran February 17th, 2009 @ 08:37 AM

    • State changed from “new” to “invalid”
    • Assigned user changed from “Michael Koziarski” to “Eloy Duran”

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>

People watching this ticket

Attachments

Tags

Pages