aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--activerecord/CHANGELOG4
-rwxr-xr-xactiverecord/lib/active_record/associations.rb11
-rw-r--r--activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb12
-rwxr-xr-xactiverecord/lib/active_record/base.rb2
-rwxr-xr-xactiverecord/test/associations_test.rb22
5 files changed, 40 insertions, 11 deletions
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
# * <tt>collection.push_with_attributes(object, join_attributes)</tt> - adds one to the collection by creating an association in the join table that
# also holds the attributes from <tt>join_attributes</tt> (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.
# * <tt>collection.delete(object, ...)</tt> - removes one or more objects from the collection by removing their associations from the join table.
# This does not destroy the objects.
# * <tt>collection=objects</tt> - 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