Assocation saving backward incompatibility in Rails 1.2.4

Today I ran across test failures in my app when upgrading from Rails 1.2.3 to 1.2.4. I determined the issue is caused by r7076 (http:// dev.rubyonrails.org/changeset/7076) which tries to save associated records if and only if the association is already loaded. Here is a simple test case that passes in Rails 1.2.3 and fails in Rails 1.2.4:

class Computer < ActiveRecord::Base   has_many :change_logs

  before_save do |computer|     computer.change_logs.create(:message => "Computer created!") if computer.new_record?   end end

class ChangeLog < ActiveRecord::Base end

def test_save_on_parent_saves_child_even_when_not_loaded   computer = Computer.create!   assert_equal "Computer created!", computer.change_logs.first.message end

Upon further investigation I discovered r7511 (http:// dev.rubyonrails.org/changeset/7511) which adds a specific check and exception when a user attempts to use the behavior that r7076 eliminated. That changeset was never backported to the 1.2 branch.

I see three options for resolving this:

1. Backport r7511 to the 1.2 branch. This would give users appropriate feedback if they trip over this behavior, but still break backwards compatibility for AR.

2. Revert r7076 to preserve full backwards compatibility in the 1.2 branch.

3. Attempt to find a solution that addresses the concern in ticket #8713 (http://dev.rubyonrails.org/ticket/8713) while still maintaining backward compatibility with Rails 1.2.3. To this end, I have been toying with the following patch which passes all existing AR tests:

Index: lib/active_record/associations.rb

Today I ran across test failures in my app when upgrading from Rails 1.2.3 to 1.2.4. I determined the issue is caused by r7076 (http:// dev.rubyonrails.org/changeset/7076) which tries to save associated records if and only if the association is already loaded. Here is a simple test case that passes in Rails 1.2.3 and fails in Rails 1.2.4:

class Computer < ActiveRecord::Base   has_many :change_logs

  before_save do |computer|     computer.change_logs.create(:message => "Computer created!") if computer.new_record?   end end

class ChangeLog < ActiveRecord::Base end

def test_save_on_parent_saves_child_even_when_not_loaded   computer = Computer.create!   assert_equal "Computer created!", computer.change_logs.first.message end

Upon further investigation I discovered r7511 (http:// dev.rubyonrails.org/changeset/7511) which adds a specific check and exception when a user attempts to use the behavior that r7076 eliminated. That changeset was never backported to the 1.2 branch.

I see three options for resolving this:

1. Backport r7511 to the 1.2 branch. This would give users appropriate feedback if they trip over this behavior, but still break backwards compatibility for AR.

I'd prefer not to do this.

2. Revert r7076 to preserve full backwards compatibility in the 1.2 branch.

The performance implications can be considerable, so lets try and avoid this too.

3. Attempt to find a solution that addresses the concern in ticket #8713 (http://dev.rubyonrails.org/ticket/8713) while still maintaining backward compatibility with Rails 1.2.3. To this end, I have been toying with the following patch which passes all existing AR tests:

This approach seems most promising

Depending on the preference expressed here, I can work up an appropriate ticket/patch to execute one of those options. My preference at the moment is for either #2 or #3. (Of course I have a vested interest in a Rails 1.2 app that depends on this behavior :wink: )

What do you guys think?

A patch for the 3rd option would be the best bet, assuming the current behaviour makes sense.

Can you whip up a patch for stable which has a failing test?