mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
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. The reason that we chose to do this breaking change is because `#details` should be considered a low level object like `#messages` is. Fix #26499. [Rafael Mendonça França + Marcus Vieira]
This commit is contained in:
parent
c66121722b
commit
d406014b03
3 changed files with 32 additions and 15 deletions
|
@ -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.
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in a new issue