Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autosave happens every time on has_one :through #35680

Closed
Kukunin opened this issue Mar 20, 2019 · 0 comments
Closed

Autosave happens every time on has_one :through #35680

Kukunin opened this issue Mar 20, 2019 · 0 comments

Comments

@Kukunin
Copy link
Contributor

Kukunin commented Mar 20, 2019

Steps to reproduce

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"
  gem "rails", github: 'rails/rails'
  gem "sqlite3"
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :users do |t|
    t.references :representer, null: false
    t.timestamps
  end

  create_table :representers do |t|
    t.timestamps
  end

  create_table :posts do |t|
    t.references :representer, null: false
    t.timestamps
  end

  create_table :logs do |t|
    t.timestamps
  end
end

class Log < ActiveRecord::Base; end

class User < ActiveRecord::Base
  belongs_to :representer

  after_commit -> { Log.create! }
end

class Representer < ActiveRecord::Base
  has_one :user

  after_commit -> { Log.create! }
end

class Post < ActiveRecord::Base
  belongs_to :representer
  has_one :user, through: :representer
end

class BugTest < Minitest::Test
  def setup
    # It's false positive when post_id == user_id
    # make user_id different from post_id
    User.create!(representer: Representer.create!)

    @representer = Representer.create!
    @user = User.create!(representer: @representer)
    @post = Post.create!(representer: @representer)
  end

  def test_autosave_for_a_user
    log_count = Log.count
    @post.tap(&:user).save!
    assert_equal(log_count, Log.count)
  end

  def test_autosave_for_a_representer
    log_count = Log.count
    @post.tap(&:representer).save!
    assert_equal(log_count, Log.count)
  end
end

Expected behavior

after_commit should not be called on User on Post saving

Actual behavior

after_commit is called on User despite it was not changed

System configuration

Rails version:
git master 7c63430

Ruby version:
2.5.3p105

The problem happens here:

https://github.com/rails/rails/blob/master/activerecord/lib/active_record/autosave_association.rb#L460

Because User has foreign_key for the representer, and it's representer_id, not post_id. When both IDs don't match, it thinks that the association was changed, and triggers commit for User too

@Kukunin Kukunin changed the title Autosave happens every time on has_one :throught Autosave happens every time on has_one :through Mar 20, 2019
Kukunin added a commit to Kukunin/rails that referenced this issue Mar 23, 2019
Fixes rails#35680

The problem occurred, when a `has one through` association contains
a foreign key (it belongs to the intermediate association).

For example, Comment belongs to Post, Post belongs to Author, and Author
`has_one :first_comment, through: :first_post`.

In this case, the value of the foreign key is comparing with the original
record, and since they are likely different, the association is marked
as changed. So it updates every time when the origin record updates.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant