Merge pull request #26552 from rafaelfranca/fix-errors-details-storage

Always store errors details information with symbols

3 files changed, 32 insertions, 15 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 786420f51e..6cc667b754 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,18 @@
+*   Always store errors details information with symbols.
+
+    When the association is autosaved we were storing the details with
+    string keys. This was creating inconsistency with other details that are
+    added using the `Errors#add` method. It was also inconsistent with the
+    `Errors#messages` storage.
+
+    To fix this inconsistency we are always storing with symbols. This will
+    cause a small breaking change because in those cases the details could
+    be accessed as strings keys but now it can not.
+
+    Fix #26499.
+
+    *Rafael Mendonça França*, *Marcus Vieira*
+
 *   Calling `touch` on a model using optimistic locking will now leave the model
     in a non-dirty state with no attribute changes.
 
diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index db84876b0a..d3e0dee731 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -329,26 +329,20 @@ module ActiveRecord
         return true if record.destroyed? || (reflection.options[:autosave] && record.marked_for_destruction?)
 
         validation_context = self.validation_context unless [:create, :update].include?(self.validation_context)
+
         unless valid = record.valid?(validation_context)
           if reflection.options[:autosave]
             indexed_attribute = !index.nil? && (reflection.options[:index_errors] || ActiveRecord::Base.index_nested_attribute_errors)
 
             record.errors.each do |attribute, message|
-              if indexed_attribute
-                attribute = "#{reflection.name}[#{index}].#{attribute}"
-              else
-                attribute = "#{reflection.name}.#{attribute}"
-              end
+              attribute = normalize_reflection_attribute(indexed_attribute, reflection, index, attribute)
               errors[attribute] << message
               errors[attribute].uniq!
             end
 
             record.errors.details.each_key do |attribute|
-              if indexed_attribute
-                reflection_attribute = "#{reflection.name}[#{index}].#{attribute}"
-              else
-                reflection_attribute = "#{reflection.name}.#{attribute}"
-              end
+              reflection_attribute =
+                normalize_reflection_attribute(indexed_attribute, reflection, index, attribute).to_sym
 
               record.errors.details[attribute].each do |error|
                 errors.details[reflection_attribute] << error
@@ -362,6 +356,14 @@ module ActiveRecord
         valid
       end
 
+      def normalize_reflection_attribute(indexed_attribute, reflection, index, attribute)
+        if indexed_attribute
+          "#{reflection.name}[#{index}].#{attribute}"
+        else
+          "#{reflection.name}.#{attribute}"
+        end
+      end
+
       # Is used as a before_save callback to check while saving a collection
       # association whether or not the parent was a new record before saving.
       def before_save_collection_association
diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index c6e983a106..c24d7b8835 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -443,7 +443,7 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociationWithAcceptsNestedAttrib
     assert_not invalid_electron.valid?
     assert valid_electron.valid?
     assert_not molecule.valid?
-    assert_equal [{ error: :blank }], molecule.errors.details["electrons.name"]
+    assert_equal [{ error: :blank }], molecule.errors.details[:"electrons.name"]
   end
 
   def test_errors_details_should_be_indexed_when_passed_as_array
@@ -457,8 +457,8 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociationWithAcceptsNestedAttrib
     assert_not tuning_peg_invalid.valid?
     assert tuning_peg_valid.valid?
     assert_not guitar.valid?
-    assert_equal [{ error: :not_a_number, value: nil }] , guitar.errors.details["tuning_pegs[1].pitch"]
-    assert_equal [], guitar.errors.details["tuning_pegs.pitch"]
+    assert_equal [{ error: :not_a_number, value: nil }], guitar.errors.details[:"tuning_pegs[1].pitch"]
+    assert_equal [], guitar.errors.details[:"tuning_pegs.pitch"]
   end
 
   def test_errors_details_should_be_indexed_when_global_flag_is_set
@@ -474,8 +474,8 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociationWithAcceptsNestedAttrib
     assert_not invalid_electron.valid?
     assert valid_electron.valid?
     assert_not molecule.valid?
-    assert_equal [{ error: :blank }], molecule.errors.details["electrons[1].name"]
-    assert_equal [], molecule.errors.details["electrons.name"]
+    assert_equal [{ error: :blank }], molecule.errors.details[:"electrons[1].name"]
+    assert_equal [], molecule.errors.details[:"electrons.name"]
   ensure
     ActiveRecord::Base.index_nested_attribute_errors = old_attribute_config
   end