Associated records are validated when passing context

[shouichi]

Steps to reproduce

# 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.0.0"
  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 :posts, force: true do |t|
  end

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

class Post < ActiveRecord::Base
  has_many :comments

  after_create :create_invalid_comment!

  def create_invalid_comment!
    c = comments.new
    c.save!(validate: false)
  end
end

class Comment < ActiveRecord::Base
  belongs_to :post

  validates :body, presence: true
end

class BugTest < Minitest::Test
  def test_save
    post = Post.create
    assert post.save
  end

  def test_save_with_context
    post = Post.create
    assert post.save(context: :whatever)
  end

  def test_save_with_context_reloaded
    post = Post.create.reload
    assert post.save(context: :whatever)
  end
end

Expected behavior

Associated records are not validated.

Actual behavior

Associated records are validated.

System configuration

Rails version: Rails 7.0.3

Ruby version: ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [x86_64-linux]

[martinjaimem]

If I’m not mistaken, validations are run for the associated record if the associated record is loaded and either: the associated record changed, or there is a custom validation context.

code

So in this case when we are reloading, the validations in the associated record are not run given that the association is no longer cached.

As an example, these two tests pass:

def test_only_validations_of_matching_custom_validation_context_fire_without_reload
    prisoner = Prisoner.new
    prisoner.build_ship

    assert prisoner.save(validate: false)
    assert_predicate prisoner.ship, :persisted?
    assert prisoner.association_cached?(:ship) # ship is loaded
    assert_not prisoner.save(context: :non_existent_context) # validations over ship are run
  end

  def test_only_validations_of_matching_custom_validation_context_fire_with_reload
    prisoner = Prisoner.new
    prisoner.build_ship

    assert prisoner.save(validate: false)
    assert_predicate prisoner.ship, :persisted?
    assert prisoner.association_cached?(:ship) # ship is loaded
    
    prisoner.reload

    assert_not prisoner.association_cached?(:ship) # ship is not loaded
    assert prisoner.save(context: :non_existent_context) # validations over ship are not run
  end

Maybe we could reword the caveats:

  # === Caveats
  #
  # Note that autosave will only trigger for already-persisted association records
  # if the records themselves have been changed. This is to protect against
  # <tt>SystemStackError</tt> caused by circular association validations. The one
  # exception is if a custom validation context is used, in which case the validations
  # will always fire on the associated record.

to:

# will always fire on the loaded associated record.

But I'm not quite sure it is the best rewording.

[shouichi]

Thanks for the detailed explanation! If the current behavior is intentional (though it's somewhat counter-intuitive IMO), we can improve the doc as you pointed out :+1:

[rails-bot[bot]]

This issue has been automatically marked as stale because it has not been commented on for at least three months. The resources of the Rails team are limited, and so we are asking for your help. If you can still reproduce this error on the 7-0-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open. Thank you for all your contributions.

[shouichi]

Opened a PR that adds an example a while ago https://github.com/rails/rails/pull/45583.