From 53aa8da1a73bc283bc8b2e1469be9e0fe5855ab7 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sat, 1 Apr 2006 20:03:10 +0000 Subject: Fixed that records returned from has_and_belongs_to_many associations with additional attributes should be marked as read only (fixes #4512) [DHH] DEPRECATED: Using additional attributes on has_and_belongs_to_many associations. Instead upgrade your association to be a real join model [DHH] git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4123 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- activerecord/CHANGELOG | 4 ++++ activerecord/lib/active_record/associations.rb | 11 +++++------ .../has_and_belongs_to_many_association.rb | 12 +++++++++--- activerecord/lib/active_record/base.rb | 2 +- activerecord/test/associations_test.rb | 22 +++++++++++++++++++++- 5 files changed, 40 insertions(+), 11 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 5fea56f628..01d98d13c8 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,9 @@ *SVN* +* DEPRECATED: Using additional attributes on has_and_belongs_to_many associations. Instead upgrade your association to be a real join model [DHH] + +* Fixed that records returned from has_and_belongs_to_many associations with additional attributes should be marked as read only (fixes #4512) [DHH] + * Do not implicitly mark recordss of has_many :through as readonly but do mark habtm records as readonly (eventually only on join tables without rich attributes). [Marcel Mollina Jr.] * Fixed broken OCIAdapter #4457 [schoenm@earthlink.net] diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 7c60ab8fd0..3ad0eeecc7 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -698,11 +698,10 @@ module ActiveRecord # an option, it is guessed using the lexical order of the class names. So a join between Developer and Project # will give the default join table name of "developers_projects" because "D" outranks "P". # - # Any additional fields added to the join table will be placed as attributes when pulling records out through - # has_and_belongs_to_many associations. This is helpful when have information about the association itself - # that you want available on retrieval. Note that any fields in the join table will override matching field names - # in the two joined tables. As a consequence, having an "id" field in the join table usually has the undesirable - # result of clobbering the "id" fields in either of the other two tables. + # Deprecated: Any additional fields added to the join table will be placed as attributes when pulling records out through + # has_and_belongs_to_many associations. Records returned from join tables with additional attributes will be marked as + # ReadOnly (because we can't save changes to the additional attrbutes). It's strongly recommended that you upgrade any + # associations with attributes to a real join model (see introduction). # # Adds the following methods for retrieval and query. # +collection+ is replaced with the symbol passed as the first argument, so @@ -714,7 +713,7 @@ module ActiveRecord # * collection.push_with_attributes(object, join_attributes) - adds one to the collection by creating an association in the join table that # also holds the attributes from join_attributes (should be a hash with the column names as keys). This can be used to have additional # attributes on the join, which will be injected into the associated objects when they are retrieved through the collection. - # (collection.concat_with_attributes is an alias to this method). + # (collection.concat_with_attributes is an alias to this method). This method is now deprecated. # * collection.delete(object, ...) - removes one or more objects from the collection by removing their associations from the join table. # This does not destroy the objects. # * collection=objects - replaces the collections content by deleting and adding objects as appropriate. diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index 3f6fdb6bac..cd866d2cdd 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -40,8 +40,8 @@ module ActiveRecord end options[:conditions] = conditions - options[:joins] = @join_sql - options[:readonly] ||= !options[:joins].nil? + options[:joins] = @join_sql + options[:readonly] = finding_with_ambigious_select?(options[:select]) if options[:order] && @reflection.options[:order] options[:order] = "#{options[:order]}, #{@reflection.options[:order]}" @@ -157,7 +157,13 @@ module ActiveRecord @join_sql = "INNER JOIN #{@reflection.options[:join_table]} ON #{@reflection.klass.table_name}.#{@reflection.klass.primary_key} = #{@reflection.options[:join_table]}.#{@reflection.association_foreign_key}" end - + + # Join tables with additional columns on top of the two foreign keys must be considered ambigious unless a select + # clause has been explicitly defined. Otherwise you can get broken records back, if, say, the join column also has + # and id column, which will then overwrite the id column of the records coming back. + def finding_with_ambigious_select?(select_clause) + !select_clause && @owner.connection.columns(@reflection.options[:join_table], "Join Table Columns").size != 2 + end end end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 1013251b0a..8227cbe89d 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -850,7 +850,7 @@ module ActiveRecord #:nodoc: (hash[method].keys + params.keys).uniq.each do |key| merge = hash[method][key] && params[key] # merge if both scopes have the same key if key == :conditions && merge - hash[method][key] = [params[key], hash[method][key]].collect{|sql| "( %s )" % sanitize_sql(sql)}.join(" AND ") + hash[method][key] = [params[key], hash[method][key]].collect{ |sql| "( %s )" % sanitize_sql(sql) }.join(" AND ") elsif key == :include && merge hash[method][key] = merge_includes(hash[method][key], params[key]).uniq else diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb index 6b5b8a8d70..b87089b8ec 100755 --- a/activerecord/test/associations_test.rb +++ b/activerecord/test/associations_test.rb @@ -7,6 +7,7 @@ require 'fixtures/reply' require 'fixtures/computer' require 'fixtures/customer' require 'fixtures/order' +require 'fixtures/category' require 'fixtures/post' require 'fixtures/author' @@ -1080,7 +1081,7 @@ end class HasAndBelongsToManyAssociationsTest < Test::Unit::TestCase - fixtures :accounts, :companies, :developers, :projects, :developers_projects + fixtures :accounts, :companies, :categories, :posts, :categories_posts, :developers, :projects, :developers_projects def test_has_and_belongs_to_many david = Developer.find(1) @@ -1469,6 +1470,25 @@ class HasAndBelongsToManyAssociationsTest < Test::Unit::TestCase AND developer_id = #{developer.id} end_sql end + + def test_updating_attributes_on_non_rich_associations + welcome = categories(:technology).posts.first + welcome.title = "Something else" + assert welcome.save! + end + + def test_updating_attributes_on_rich_associations + david = projects(:action_controller).developers.first + david.name = "DHH" + assert_raises(ActiveRecord::ReadOnlyRecord) { david.save! } + end + + + def test_updating_attributes_on_rich_associations_with_limited_find + david = projects(:action_controller).developers.find(:all, :select => "developers.*").first + david.name = "DHH" + assert david.save! + end def test_join_table_alias assert_equal 3, Developer.find(:all, :include => {:projects => :developers}, :conditions => 'developers_projects_join.joined_on IS NOT NULL').size -- cgit v1.2.3