Skip to content

Commit

Permalink
Revert "Add :accessible option to Associations for allowing mass assi…
Browse files Browse the repository at this point in the history
…gnments using hash. [#474 state:resolved]"

This reverts commit e0750d6.

Conflicts:

	activerecord/CHANGELOG
	activerecord/lib/active_record/associations.rb
	activerecord/lib/active_record/associations/association_collection.rb
  • Loading branch information
lifo committed Sep 10, 2008
1 parent b518b6c commit 9994f0d
Show file tree
Hide file tree
Showing 6 changed files with 5 additions and 157 deletions.
18 changes: 0 additions & 18 deletions activerecord/CHANGELOG
Expand Up @@ -20,24 +20,6 @@

* Fixed that create database statements would always include "DEFAULT NULL" (Nick Sieger) [#334]

* Add :accessible option to associations for allowing (opt-in) mass assignment. #474. [David Dollar] Example :

class Post < ActiveRecord::Base
belongs_to :author, :accessible => true
has_many :comments, :accessible => true
end

post = Post.create({
:title => 'Accessible Attributes',
:author => { :name => 'David Dollar' },
:comments => [
{ :body => 'First Post!' },
{ :body => 'Nested Hashes are great!' }
]
})

post.comments << { :body => 'Another Comment' }

* Add :tokenizer option to validates_length_of to specify how to split up the attribute string. #507. [David Lowenfels] Example :

# Ensure essay contains at least 100 words.
Expand Down
22 changes: 4 additions & 18 deletions activerecord/lib/active_record/associations.rb
Expand Up @@ -739,9 +739,6 @@ module ClassMethods
# If true, all the associated objects are readonly through the association.
# [:validate]
# If false, don't validate the associated objects when saving the parent object. true by default.
# [:accessible]
# Mass assignment is allowed for this assocation (similar to <tt>ActiveRecord::Base#attr_accessible</tt>).

# Option examples:
# has_many :comments, :order => "posted_on"
# has_many :comments, :include => :author
Expand Down Expand Up @@ -855,8 +852,6 @@ def has_many(association_id, options = {}, &extension)
# If true, the associated object is readonly through the association.
# [:validate]
# If false, don't validate the associated object when saving the parent object. +false+ by default.
# [:accessible]
# Mass assignment is allowed for this assocation (similar to <tt>ActiveRecord::Base#attr_accessible</tt>).
#
# Option examples:
# has_one :credit_card, :dependent => :destroy # destroys the associated credit card
Expand Down Expand Up @@ -973,8 +968,6 @@ def has_one(association_id, options = {})
# If true, the associated object is readonly through the association.
# [:validate]
# If false, don't validate the associated objects when saving the parent object. +false+ by default.
# [:accessible]
# Mass assignment is allowed for this assocation (similar to <tt>ActiveRecord::Base#attr_accessible</tt>).
#
# Option examples:
# belongs_to :firm, :foreign_key => "client_of"
Expand Down Expand Up @@ -1190,8 +1183,6 @@ def belongs_to(association_id, options = {})
# If true, all the associated objects are readonly through the association.
# [:validate]
# If false, don't validate the associated objects when saving the parent object. +true+ by default.
# [:accessible<]
# Mass assignment is allowed for this assocation (similar to <tt>ActiveRecord::Base#attr_accessible</tt>).
#
# Option examples:
# has_and_belongs_to_many :projects
Expand Down Expand Up @@ -1266,8 +1257,6 @@ def association_accessor_methods(reflection, association_proxy_class)
association = association_proxy_class.new(self, reflection)
end

new_value = reflection.build_association(new_value) if reflection.options[:accessible] && new_value.is_a?(Hash)

if association_proxy_class == HasOneThroughAssociation
association.create_through_record(new_value)
self.send(reflection.name, new_value)
Expand Down Expand Up @@ -1519,12 +1508,11 @@ def configure_dependency_for_belongs_to(reflection)
:finder_sql, :counter_sql,
:before_add, :after_add, :before_remove, :after_remove,
:extend, :readonly,
:validate, :accessible
:validate
]

def create_has_many_reflection(association_id, options, &extension)
options.assert_valid_keys(valid_keys_for_has_many_association)

options[:extend] = create_extension_modules(association_id, extension, options[:extend])

create_reflection(:has_many, association_id, options, self)
Expand All @@ -1534,12 +1522,11 @@ def create_has_many_reflection(association_id, options, &extension)
@@valid_keys_for_has_one_association = [
:class_name, :foreign_key, :remote, :select, :conditions, :order,
:include, :dependent, :counter_cache, :extend, :as, :readonly,
:validate, :primary_key, :accessible
:validate, :primary_key
]

def create_has_one_reflection(association_id, options)
options.assert_valid_keys(valid_keys_for_has_one_association)

create_reflection(:has_one, association_id, options, self)
end

Expand All @@ -1554,12 +1541,11 @@ def create_has_one_through_reflection(association_id, options)
@@valid_keys_for_belongs_to_association = [
:class_name, :foreign_key, :foreign_type, :remote, :select, :conditions,
:include, :dependent, :counter_cache, :extend, :polymorphic, :readonly,
:validate, :accessible
:validate
]

def create_belongs_to_reflection(association_id, options)
options.assert_valid_keys(valid_keys_for_belongs_to_association)

reflection = create_reflection(:belongs_to, association_id, options, self)

if options[:polymorphic]
Expand All @@ -1577,7 +1563,7 @@ def create_has_and_belongs_to_many_reflection(association_id, options, &extensio
:finder_sql, :delete_sql, :insert_sql,
:before_add, :after_add, :before_remove, :after_remove,
:extend, :readonly,
:validate, :accessible
:validate
)

options[:extend] = create_extension_modules(association_id, extension, options[:extend])
Expand Down
Expand Up @@ -110,8 +110,6 @@ def <<(*records)

@owner.transaction do
flatten_deeper(records).each do |record|
record = @reflection.build_association(record) if @reflection.options[:accessible] && record.is_a?(Hash)

raise_on_type_mismatch(record)
add_record_to_target_with_callbacks(record) do |r|
result &&= insert_record(record) unless @owner.new_record?
Expand Down Expand Up @@ -286,10 +284,6 @@ def uniq(collection = self)
# Replace this collection with +other_array+
# This will perform a diff and delete/add only records that have changed.
def replace(other_array)
other_array.map! do |val|
val.is_a?(Hash) ? @reflection.build_association(val) : val
end if @reflection.options[:accessible]

other_array.each { |val| raise_on_type_mismatch(val) }

load_target
Expand Down
108 changes: 0 additions & 108 deletions activerecord/test/cases/associations_test.rb
Expand Up @@ -189,114 +189,6 @@ def test_reload_returns_assocition
end
end

def test_belongs_to_mass_assignment
post_attributes = { :title => 'Associations', :body => 'Are They Accessible?' }
author_attributes = { :name => 'David Dollar' }

assert_no_difference 'Author.count' do
assert_raise(ActiveRecord::AssociationTypeMismatch) do
Post.create(post_attributes.merge({:author => author_attributes}))
end
end

assert_difference 'Author.count' do
post = Post.create(post_attributes.merge({:creatable_author => author_attributes}))
assert_equal post.creatable_author.name, author_attributes[:name]
end
end

def test_has_one_mass_assignment
post_attributes = { :title => 'Associations', :body => 'Are They Accessible?' }
comment_attributes = { :body => 'Setter Takes Hash' }

assert_no_difference 'Comment.count' do
assert_raise(ActiveRecord::AssociationTypeMismatch) do
Post.create(post_attributes.merge({:uncreatable_comment => comment_attributes}))
end
end

assert_difference 'Comment.count' do
post = Post.create(post_attributes.merge({:creatable_comment => comment_attributes}))
assert_equal post.creatable_comment.body, comment_attributes[:body]
end
end

def test_has_many_mass_assignment
post = posts(:welcome)
post_attributes = { :title => 'Associations', :body => 'Are They Accessible?' }
comment_attributes = { :body => 'Setter Takes Hash' }

assert_no_difference 'Comment.count' do
assert_raise(ActiveRecord::AssociationTypeMismatch) do
Post.create(post_attributes.merge({:comments => [comment_attributes]}))
end
assert_raise(ActiveRecord::AssociationTypeMismatch) do
post.comments << comment_attributes
end
end

assert_difference 'Comment.count' do
post = Post.create(post_attributes.merge({:creatable_comments => [comment_attributes]}))
assert_equal post.creatable_comments.last.body, comment_attributes[:body]
end

assert_difference 'Comment.count' do
post.creatable_comments << comment_attributes
assert_equal post.comments.last.body, comment_attributes[:body]
end

post.creatable_comments = [comment_attributes, comment_attributes]
assert_equal post.creatable_comments.count, 2
end

def test_has_and_belongs_to_many_mass_assignment
post = posts(:welcome)
post_attributes = { :title => 'Associations', :body => 'Are They Accessible?' }
category_attributes = { :name => 'Accessible Association', :type => 'Category' }

assert_no_difference 'Category.count' do
assert_raise(ActiveRecord::AssociationTypeMismatch) do
Post.create(post_attributes.merge({:categories => [category_attributes]}))
end
assert_raise(ActiveRecord::AssociationTypeMismatch) do
post.categories << category_attributes
end
end

assert_difference 'Category.count' do
post = Post.create(post_attributes.merge({:creatable_categories => [category_attributes]}))
assert_equal post.creatable_categories.last.name, category_attributes[:name]
end

assert_difference 'Category.count' do
post.creatable_categories << category_attributes
assert_equal post.creatable_categories.last.name, category_attributes[:name]
end

post.creatable_categories = [category_attributes, category_attributes]
assert_equal post.creatable_categories.count, 2
end

def test_association_proxy_setter_can_take_hash
special_comment_attributes = { :body => 'Setter Takes Hash' }

post = posts(:welcome)
post.creatable_comment = { :body => 'Setter Takes Hash' }

assert_equal post.creatable_comment.body, special_comment_attributes[:body]
end

def test_association_collection_can_take_hash
post_attributes = { :title => 'Setter Takes', :body => 'Hash' }
david = authors(:david)

post = (david.posts << post_attributes).last
assert_equal post.title, post_attributes[:title]

david.posts = [post_attributes, post_attributes]
assert_equal david.posts.count, 2
end

def setup_dangling_association
josh = Author.create(:name => "Josh")
p = Post.create(:title => "New on Edge", :body => "More cool stuff!", :author => josh)
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/models/author.rb
@@ -1,5 +1,5 @@
class Author < ActiveRecord::Base
has_many :posts, :accessible => true
has_many :posts
has_many :posts_with_comments, :include => :comments, :class_name => "Post"
has_many :posts_with_comments_sorted_by_comment_id, :include => :comments, :class_name => "Post", :order => 'comments.id'
has_many :posts_sorted_by_id_limited, :class_name => "Post", :order => 'posts.id', :limit => 1
Expand Down
6 changes: 0 additions & 6 deletions activerecord/test/models/post.rb
Expand Up @@ -33,12 +33,6 @@ def find_most_recent
has_and_belongs_to_many :categories
has_and_belongs_to_many :special_categories, :join_table => "categories_posts", :association_foreign_key => 'category_id'

belongs_to :creatable_author, :class_name => 'Author', :accessible => true
has_one :uncreatable_comment, :class_name => 'Comment', :accessible => false, :order => 'id desc'
has_one :creatable_comment, :class_name => 'Comment', :accessible => true, :order => 'id desc'
has_many :creatable_comments, :class_name => 'Comment', :accessible => true, :dependent => :destroy
has_and_belongs_to_many :creatable_categories, :class_name => 'Category', :accessible => true

has_many :taggings, :as => :taggable
has_many :tags, :through => :taggings do
def add_joins_and_select
Expand Down

2 comments on commit 9994f0d

@haberbyte
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this reverted? :(

@NZKoz
Copy link
Member

@NZKoz NZKoz commented on 9994f0d Sep 11, 2008

Choose a reason for hiding this comment

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

This was discussed on the core list:

http://groups.google.com/group/rubyonrails-core/browse_thread/thread/3c61e00916c365e5

It’s coming back, just post 2.2, it’s not yet ready for a stable release.

Please sign in to comment.