Belongs_to association - save can reset association id to old value #5563

Closed
calamitas opened this Issue Mar 23, 2012 · 6 comments

Comments

Projects
None yet
5 participants

Rails 3.2.2, ruby 1.9.2p290 (2011-07-09 revision 32553) [x86_64-darwin11.2.0]

class User < ActiveRecord::Base
end

class Post < ActiveRecord::Base
  belongs_to :user
end

user1 = User.create!(:name => "User1")
user2 = User.create!(:name => "User2")
post = Post.new(:content => "Post")
post.user = user1
post.user_id = user2.id
p post.user_id == user2.id # => true
post.save
p post.user_id == user2.id # => false, expected to be true

After the save, the value of post.user_id is user1.id instead of user2.id.

If I insert a reference to post.user just before the save, the last statement prints true.

Owner

rafaelfranca commented Mar 23, 2012

I think it is intentional but I'm not sure. What do you think @jonleighton? If is not I can try to fix it.

Member

jonleighton commented Mar 23, 2012

No, I don't think this is intentional. user and user_id should reflect the most recent change to either of them.

Contributor

kennyj commented Mar 29, 2012

Hi! @jonleighton @rafaelfranca
I've tried to fix this issue, and I guess I fixed this.
Please review my fix.

I think the cause of this problem is load_target method ignores stale state
BTW: blank check was necessary for through association.

Contributor

kennyj commented Mar 30, 2012

Hi guys!
I'm going to send PR. thanks :-)

@kennyj kennyj added a commit to kennyj/rails that referenced this issue Apr 3, 2012

@kennyj kennyj Added a testcase for #5563 7c7f2fb

@kennyj kennyj added a commit to kennyj/rails that referenced this issue Apr 3, 2012

@kennyj kennyj Fix #5563. Should reflect the most recent change to either of associa…
…tion / id.
0929650

Test in the proposed fix does not reflect the bug, and I believe that test would pass with the old code.

Note that the issue has nothing to do with saving (at least my issue in #5744). Here is a test that should reflect that issue, which I believe is the same as this one.

def test_reflect_the_most_recent_change
    author1, author2 = Author.limit(2)
    post = Post.new(:title => "foo", :body=> "bar")
    post.author    = author1

    post.author_id = author2.id

    assert_equal post.author2.id, author2.id
end

Sorry, actually it appears that my may be different.

kennyj closed this in 365b8b6 Apr 25, 2012

@romanvbabenko romanvbabenko added a commit to romanvbabenko/rails that referenced this issue May 2, 2012

@kennyj @romanvbabenko kennyj + romanvbabenko Fix #5563. Should reflect the most recent change to either of associa…
…tion / id.
01f8893
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment