@iamradioactive this PR introduces a new bug, which causes nested association to not be saved.. apparently it causes reset on the association because of "stale_target"

[3600] doing reset stale_state=, stale_state=1,
/Users/yosi/.rvm/gems/ruby-3.2.2/bundler/gems/rails-6d89b876f012/activerecord/lib/active_record/associations/belongs_to_association.rb:45:in `reset'
/Users/yosi/.rvm/gems/ruby-3.2.2/bundler/gems/rails-6d89b876f012/activerecord/lib/active_record/associations/association.rb:72:in `reload'
/Users/yosi/.rvm/gems/ruby-3.2.2/bundler/gems/rails-6d89b876f012/activerecord/lib/active_record/associations/singular_association.rb:11:in `reader'
/Users/yosi/.rvm/gems/ruby-3.2.2/bundler/gems/rails-6d89b876f012/activerecord/lib/active_record/associations/builder/association.rb:104:in `author'

Reproduction script:

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  # gem "activerecord", "~> 7.1.0"
  gem "rails", github: "iamradioactive/rails", branch: "handle_composite_foreign_keys"
  gem "pg"
end

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

ActiveRecord::Base.establish_connection(
  adapter: "postgresql",
  database: "test",
  encoding: "unicode",
  host: "localhost",
  port: "5432",
  password: "12345",
  username: "test")

ActiveRecord::Schema.define do
  create_table :tenants, force: true do |t|
    t.string :name
  end

  create_table :comments, force: true do |t|
    t.integer :tenant_id
    t.integer :post_id
    t.integer :author_id
    t.string :body
    t.integer :position
  end

  create_table :posts, force: true do |t|
    t.integer :tenant_id
  end

  create_table :author, force: true do |t|
    t.integer :tenant_id
  end
end

class Tenant < ActiveRecord::Base
end

class Post < ActiveRecord::Base
  query_constraints :tenant_id, :id

  has_many :comments
  belongs_to :tenant
end

class Author < ActiveRecord::Base
  query_constraints :tenant_id, :id
  belongs_to :tenant
end

class Comment < ActiveRecord::Base
  query_constraints :tenant_id, :id

  before_save :touch_author

  belongs_to :tenant
  belongs_to :post
  belongs_to :author

  def touch_author
    puts "author: #{self.author}"
  end
end

class BugTest < Minitest::Test
  def test_assoc
    Author.destroy_all
    Post.destroy_all
    Comment.destroy_all
    Tenant.destroy_all

    tenant = Tenant.create!

    post = Post.create!(tenant: tenant)
    post.comments << Comment.new(body: "hello", tenant: tenant, author: Author.new(tenant: tenant))
    post.save!

    assert_equal 1, Post.count, "single post"
    assert_equal 1, Comment.count, "2 comments"
    assert_equal 1, Author.count, "1 author"
  end
end

This test passes on rails 7.1, but not on your branch.

@yosiat , Interestingly, It seems to be another bug uncovered by these changes. Thank you for testing it out. I have updated the PR.

On Rails 7.1 stable, if you verify the value of tenant_id before calling save.

With Query Constraints

• comment.tenant id is nil.
• comment.tenant points to tenant with id 1.

Without Query Constraints

• comment.tenant id is 1
• comment.tenant points to tenant with id 1.

Since, Stale state is not working correctly on main branch as of now, reload is not getting triggered when the tenant id changes and there is no loss of unsaved associations. After this update, stale_state method started to detect changes and state was reset as soon as the tenant_id is changed.

If we are pivoting the fix from stale_state towards properly deriving the foreign key, could we change the PR title & description?

Hi @iamradioactive

Thanks for your work and detailed investigation!

I am bit confused, what this PR is going to fix? there were two issues:

• Original one: accessing the comment.author and it's not loaded
• The nested save

I saw this comment about inversing (#50256 (comment)) which issue does it the address? the first one?

@nvasilevski , @yosiat

These are two independent and valid bugs. I'm gonna open a separate PR to derive the foreign key using the commit and mark the new PR as a dependency for this one.

There are two bugs to be fixed

• State state for composite foreign keys. (Original one)
• Derived/Inferred foreign key for models with Query constraints. (Nested save)

I will update the PRs and drop a comment here.

I have updated the PR, this one contains only the changes related to the fix for stale_state. However it has a dependency on #50347 and is to be merged once those changes are approved and merged.

cc: @nvasilevski @yosiat

Can someone please rebase this branch as the branch suffers from #49597 and I think it makes noise in the verification/fix for this bug.

Can someone please rebase this branch as the branch suffers from #49597 and I think it makes noise in the verification/fix for this bug.

@jarl-dk , I have rebased the branch with latest code from main.

@nvasilevski , Can you please review this again? The dependency #50347 is merged now