Skip to content

Commit

Permalink
Merge pull request #9860 from wangjohn/update_attributes_throws_error…
Browse files Browse the repository at this point in the history
…_with_nil

Raising an error when nil is passed to update_attributes.

Conflicts:
	activerecord/CHANGELOG.md
  • Loading branch information
rafaelfranca committed Sep 24, 2013
2 parents e029e02 + 926c4b9 commit 16c7873
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 6 deletions.
10 changes: 10 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
* Calling `update_attributes` will now throw an `ArgumentError` whenever it
gets a `nil` argument. More specifically, it will throw an error if the
argument that it gets passed does not respond to to `stringify_keys`.

Example:

@my_comment.update_attributes(nil) # => raises ArgumentError

*John Wang*

* Deprecate `quoted_locking_column` method, which isn't used anywhere.

*kennyj*
Expand Down
4 changes: 3 additions & 1 deletion activerecord/lib/active_record/attribute_assignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ module AttributeAssignment
# of this method is +false+ an <tt>ActiveModel::ForbiddenAttributesError</tt>
# exception is raised.
def assign_attributes(new_attributes)
return if new_attributes.blank?
if !new_attributes.respond_to?(:stringify_keys)
raise ArgumentError, "When assigning attributes, you must pass a hash as an argument."
end

attributes = new_attributes.stringify_keys
multi_parameter_attributes = []
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/attribute_methods_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def test_set_attributes

def test_set_attributes_without_hash
topic = Topic.new
assert_nothing_raised { topic.attributes = '' }
assert_raise(ArgumentError) { topic.attributes = '' }
end

def test_integers_as_nil
Expand Down
15 changes: 11 additions & 4 deletions activerecord/test/cases/persistence_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -419,10 +419,6 @@ def test_update_attribute
assert !Topic.find(1).approved?
end

def test_update_attribute_does_not_choke_on_nil
assert Topic.find(1).update(nil)
end

def test_update_attribute_for_readonly_attribute
minivan = Minivan.find('m1')
assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_attribute(:color, 'black') }
Expand Down Expand Up @@ -701,6 +697,17 @@ def test_update_attributes
assert_equal topic.title, Topic.find(1234).title
end

def test_update_attributes_parameters
topic = Topic.find(1)
assert_nothing_raised do
topic.update_attributes({})
end

assert_raises(ArgumentError) do
topic.update_attributes(nil)
end
end

def test_update!
Reply.validates_presence_of(:title)
reply = Reply.find(2)
Expand Down

0 comments on commit 16c7873

Please sign in to comment.