Merge pull request rails#46694 from ghiculescu/strict-loading-on-assoc-writers

Fix: `StrictLoadingViolationError` when concatenating or setting association when non-persisted owner has primary key

Author: eileencodes

activerecord/lib/active_record/associations/association.rb

@@ -45,6 +45,8 @@ def initialize(owner, reflection)
 
         reset
         reset_scope
+
+        @skip_strict_loading = nil
       end
 
       # Resets the \loaded flag to +false+ and sets the \target to +nil+.
@@ -216,7 +218,7 @@ def ensure_klass_exists!
         end
 
         def find_target
-          if violates_strict_loading? && owner.validation_context.nil?
+          if violates_strict_loading?
             Base.strict_loading_violation!(owner: owner.class, reflection: reflection)
           end
 
@@ -239,7 +241,19 @@ def find_target
           end
         end
 
+        def skip_strict_loading(&block)
+          skip_strict_loading_was = @skip_strict_loading
+          @skip_strict_loading = true
+          yield
+        ensure
+          @skip_strict_loading = skip_strict_loading_was
+        end
+
         def violates_strict_loading?
+          return if @skip_strict_loading
+
+          return unless owner.validation_context.nil?
+
           return reflection.strict_loading? if reflection.options.key?(:strict_loading)
 
           owner.strict_loading? && !owner.strict_loading_n_plus_one_only?

activerecord/lib/active_record/associations/collection_association.rb

@@ -119,7 +119,7 @@ def build(attributes = nil, &block)
       def concat(*records)
         records = records.flatten
         if owner.new_record?
-          load_target
+          skip_strict_loading { load_target }
           concat_records(records)
         else
           transaction { concat_records(records) }
@@ -233,7 +233,7 @@ def empty?
       # and delete/add only records that have changed.
       def replace(other_array)
         other_array.each { |val| raise_on_type_mismatch!(val) }
-        original_target = load_target.dup
+        original_target = skip_strict_loading { load_target }.dup
 
         if owner.new_record?
           replace_records(other_array, original_target)

activerecord/test/cases/strict_loading_test.rb

@@ -162,6 +162,60 @@ def test_strict_loading_with_reflection_is_ignored_in_validation_context
     end
   end
 
+  def test_strict_loading_on_concat_is_ignored
+    developer = Developer.first
+    developer.strict_loading!
+
+    assert_nothing_raised do
+      developer.audit_logs << AuditLog.new(message: "message")
+    end
+  end
+
+  def test_strict_loading_on_build_is_ignored
+    developer = Developer.first
+    developer.strict_loading!
+
+    assert_nothing_raised do
+      developer.audit_logs.build(message: message)
+    end
+  end
+
+  def test_strict_loading_on_writer_is_ignored
+    developer = Developer.first
+    developer.strict_loading!
+
+    assert_nothing_raised do
+      developer.audit_logs = [AuditLog.new(message: "message")]
+    end
+  end
+
+  def test_strict_loading_with_new_record_on_concat_is_ignored
+    developer = Developer.new(id: Developer.first.id)
+    developer.strict_loading!
+
+    assert_nothing_raised do
+      developer.audit_logs << AuditLog.new(message: "message")
+    end
+  end
+
+  def test_strict_loading_with_new_record_on_build_is_ignored
+    developer = Developer.new(id: Developer.first.id)
+    developer.strict_loading!
+
+    assert_nothing_raised do
+      developer.audit_logs.build(message: "message")
+    end
+  end
+
+  def test_strict_loading_with_new_record_on_writer_is_ignored
+    developer = Developer.new(id: Developer.first.id)
+    developer.strict_loading!
+
+    assert_nothing_raised do
+      developer.audit_logs = [AuditLog.new(message: "message")]
+    end
+  end
+
   def test_strict_loading_has_one_reload
     with_strict_loading_by_default(Developer) do
       ship = Ship.create!(developer: Developer.first, name: "The Great Ship")