Skip to content

Commit

Permalink
Allow multiparameter assigned attributes to be used with text_field
Browse files Browse the repository at this point in the history
Between 4.2 and 5.0 the behavior of how multiparameter attributes
interact with `_before_type_cast` changed. In 4.2 it returns the
post-type-cast value. After 5.0, it returns the hash that gets sent to
the type. This behavior is correct, but will cause an issue if you then
tried to render that value in an input like `text_field` or
`hidden_field`.

In this case, we want those fields to use the post-type-cast form,
instead of the `_before_type_cast` (the main reason it uses
`_before_type_cast` at all is to avoid losing data when casting a
non-numeric string to integer).

I've opted to modify `came_from_user?` rather than introduce a new
method for this as I want to avoid complicating that contract further,
and technically the multiparameter hash didn't come from assignment, it
was constructed internally by AR.

Close #27888.
  • Loading branch information
sgrif committed Jul 17, 2017
1 parent 828b05f commit 1519e97
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 1 deletion.
Expand Up @@ -21,6 +21,10 @@ def initialize(defaults: {})
end
end

define_method(:value_constructed_by_mass_assignment?) do |value|
value.is_a?(Hash)
end

define_method(:value_from_multiparameter_assignment) do |values_hash|
defaults.each do |k, v|
values_hash[k] ||= v
Expand Down
4 changes: 4 additions & 0 deletions activemodel/lib/active_model/type/value.rb
Expand Up @@ -86,6 +86,10 @@ def changed_in_place?(raw_old_value, new_value)
false
end

def value_constructed_by_mass_assignment?(_value) # :nodoc:
false
end

def map(value) # :nodoc:
yield value
end
Expand Down
5 changes: 5 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,3 +1,8 @@
* Values constructed using multi-parameter assignment will now use the
post-type-cast value for rendering in single-field form inputs.

*Sean Griffin*

* `Relation#joins` is no longer affected by the target model's
`current_scope`, with the exception of `unscoped`.

Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/attribute.rb
Expand Up @@ -174,7 +174,7 @@ def type_cast(value)
end

def came_from_user?
true
!type.value_constructed_by_mass_assignment?(value_before_type_cast)
end
end

Expand Down
11 changes: 11 additions & 0 deletions activerecord/test/cases/multiparameter_attributes_test.rb
Expand Up @@ -383,4 +383,15 @@ def test_multiparameter_assignment_of_aggregation_with_large_index

assert_equal("address", ex.errors[0].attribute)
end

def test_multiparameter_assigned_attributes_did_not_come_from_user
topic = Topic.new(
"written_on(1i)" => "1952",
"written_on(2i)" => "3",
"written_on(3i)" => "11",
"written_on(4i)" => "13",
"written_on(5i)" => "55",
)
refute_predicate topic, :written_on_came_from_user?
end
end

1 comment on commit 1519e97

@namusyaka
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sgrif Could you backport this into 5-1-stable and 5-0-stable?

Please sign in to comment.