Skip to content

Commit

Permalink
Fix strict loading on validations
Browse files Browse the repository at this point in the history
We don't want strict loading to throw an error on validations since
those need to load the record in order to validate it.

This change check the owners's `validation_context`. If it's `nil` then
we know we're not currently validating an object in create/update. If
it's set then we're validating and want to skip raising for strict
loading.

Fixes: #40767
  • Loading branch information
eileencodes committed Dec 9, 2020
1 parent ff0b9a4 commit bda9bc0
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 2 deletions.
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/associations/association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,11 @@ def create!(attributes = nil, &block)

private
def find_target
if owner.strict_loading?
if owner.strict_loading? && owner.validation_context.nil?
Base.strict_loading_violation!(owner: owner.class, association: klass)
end

if reflection.strict_loading?
if reflection.strict_loading? && owner.validation_context.nil?
Base.strict_loading_violation!(owner: owner.class, association: reflection.name)
end

Expand Down
21 changes: 21 additions & 0 deletions activerecord/test/cases/strict_loading_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,27 @@ def test_raises_if_strict_loading_by_default_and_lazy_loading
end
end

def test_strict_loading_is_ignored_in_validation_context
with_strict_loading_by_default(Developer) do
developer = Developer.first
assert_predicate developer, :strict_loading?

assert_nothing_raised do
AuditLogRequired.create! developer_id: developer.id, message: "i am a message"
end
end
end

def test_strict_loading_with_reflection_is_ignored_in_validation_context
with_strict_loading_by_default(Developer) do
developer = Developer.first
assert_predicate developer, :strict_loading?

developer.required_audit_logs.build(message: "I am message")
developer.save!
end
end

def test_preload_audit_logs_are_strict_loading_because_parent_is_strict_loading
developer = Developer.first

Expand Down
6 changes: 6 additions & 0 deletions activerecord/test/models/developer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ def find_least_recent
class_name: "SpecialProject"

has_many :audit_logs
has_many :required_audit_logs, class_name: "AuditLogRequired"
has_many :strict_loading_audit_logs, -> { strict_loading }, class_name: "AuditLog"
has_many :strict_loading_opt_audit_logs, strict_loading: true, class_name: "AuditLog"
has_many :contracts
Expand Down Expand Up @@ -125,6 +126,11 @@ class AuditLog < ActiveRecord::Base
belongs_to :unvalidated_developer, class_name: "Developer"
end

class AuditLogRequired < ActiveRecord::Base
self.table_name = "audit_logs"
belongs_to :developer, required: true
end

class DeveloperWithBeforeDestroyRaise < ActiveRecord::Base
self.table_name = "developers"
has_and_belongs_to_many :projects, join_table: "developers_projects", foreign_key: "developer_id"
Expand Down

0 comments on commit bda9bc0

Please sign in to comment.