Removed support for accessing attributes on a has_and_belongs_to_many join table. This has been documented as deprecated behaviour since April 2006. Please use has_many :through instead. A deprecation warning will be added to the 3-0-stable branch for the 3.0.4 release.

5 files changed, 11 insertions, 161 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 1d2f814d32..43272160bd 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,9 @@
 *Rails 3.1.0 (unreleased)*
 
+* Removed support for accessing attributes on a has_and_belongs_to_many join table. This has been
+  documented as deprecated behaviour since April 2006. Please use has_many :through instead.
+  [Jon Leighton]
+
 * Migration files generated from model and constructive migration generators
   (for example, add_name_to_users) use the reversible migration's `change`
   method instead of the ordinary `up` and `down` methods. [Prem Sichanugrist]
@@ -26,7 +30,7 @@ along with attribute conditionals as a replacement.
     User.find_by_name("david").try(:authenticate, "mUc3m00RsqyRe") # => user
 
 
-* When a model is generated add_index is added by default for belongs_to or references columns 
+* When a model is generated add_index is added by default for belongs_to or references columns
 
   rails g model post user:belongs_to will generate the following:
 
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index e539e4968b..891ac52f8a 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1301,12 +1301,6 @@ module ActiveRecord
       #     end
       #   end
       #
-      # 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 attributes). 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(force_reload = false)]
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 bc7894173d..b28554dce1 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
@@ -10,18 +10,6 @@ module ActiveRecord
         super
       end
 
-      def columns
-        @reflection.columns(@join_table_name, "#{@join_table_name} Columns")
-      end
-
-      def reset_column_information
-        @reflection.reset_column_information
-      end
-
-      def has_primary_key?
-        @has_primary_key ||= @owner.connection.supports_primary_key? && @owner.connection.primary_key(@join_table_name)
-      end
-
       protected
 
         def count_records
@@ -36,26 +24,11 @@ module ActiveRecord
           if @reflection.options[:insert_sql]
             @owner.connection.insert(interpolate_sql(@reflection.options[:insert_sql], record))
           else
-            relation   = join_table
-            timestamps = record_timestamp_columns(record)
-            timezone   = record.send(:current_time_from_proper_timezone) if timestamps.any?
-
-            attributes = columns.map do |column|
-              name = column.name
-              value = case name.to_s
-                when @reflection.foreign_key.to_s
-                  @owner.id
-                when @reflection.association_foreign_key.to_s
-                  record.id
-                when *timestamps
-                  timezone
-                else
-                  @owner.send(:quote_value, record[name], column) if record.has_attribute?(name)
-              end
-              [relation[name], value] unless value.nil?
-            end
+            stmt = join_table.compile_insert(
+              join_table[@reflection.foreign_key]             => @owner.id,
+              join_table[@reflection.association_foreign_key] => record.id
+            )
 
-            stmt = relation.compile_insert Hash[attributes]
             @owner.connection.insert stmt.to_sql
           end
 
@@ -89,46 +62,17 @@ module ActiveRecord
         end
 
         def association_scope
-          scope = super.joins(construct_joins)
-          scope = scope.readonly if ambiguous_select?(@reflection.options[:select])
-          scope
+          super.joins(construct_joins)
         end
 
         def select_value
-          super || [@reflection.klass.arel_table[Arel.star], join_table[Arel.star]]
-        end
-
-        # Join tables with additional columns on top of the two foreign keys must be considered
-        # ambiguous unless a select clause has been explicitly defined. Otherwise you can get
-        # broken records back, if, for example, the join column also has an id column. This will
-        # then overwrite the id column of the records coming back.
-        def ambiguous_select?(select)
-          extra_join_columns? && select.nil?
-        end
-
-        def extra_join_columns?
-          columns.size > 2
+          super || @reflection.klass.arel_table[Arel.star]
         end
 
       private
-        def record_timestamp_columns(record)
-          if record.record_timestamps
-            record.send(:all_timestamp_attributes).map { |x| x.to_s }
-          else
-            []
-          end
-        end
-
         def invertible_for?(record)
           false
         end
-
-        def find_by_sql(*args)
-          options   = args.extract_options!
-          ambiguous = ambiguous_select?(@reflection.options[:select] || options[:select])
-
-          scoped.readonly(ambiguous).find(*(args << options))
-        end
     end
   end
 end
diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
index 705550216c..30730c7094 100644
--- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -101,38 +101,6 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     assert_equal 't1', record[1]
   end
 
-  def test_should_record_timestamp_for_join_table
-    setup_data_for_habtm_case
-
-    con = ActiveRecord::Base.connection
-    sql = 'select * from countries_treaties'
-    record = con.select_rows(sql).last
-    assert_not_nil record[2]
-    assert_not_nil record[3]
-    if current_adapter?(:Mysql2Adapter, :OracleAdapter)
-      assert_match %r{\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}}, record[2].to_s(:db)
-      assert_match %r{\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}}, record[3].to_s(:db)
-    else
-      assert_match %r{\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}}, record[2]
-      assert_match %r{\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}}, record[3]
-    end
-  end
-
-  def test_should_record_timestamp_for_join_table_only_if_timestamp_should_be_recorded
-    begin
-      Treaty.record_timestamps = false
-      setup_data_for_habtm_case
-
-      con = ActiveRecord::Base.connection
-      sql = 'select * from countries_treaties'
-      record = con.select_rows(sql).last
-      assert_nil record[2]
-      assert_nil record[3]
-    ensure
-      Treaty.record_timestamps = true
-    end
-  end
-
   def test_has_and_belongs_to_many
     david = Developer.find(1)
 
@@ -218,34 +186,6 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     assert_equal 2, aredridel.projects(true).size
   end
 
-  def test_adding_uses_default_values_on_join_table
-    ac = projects(:action_controller)
-    assert !developers(:jamis).projects.include?(ac)
-    developers(:jamis).projects << ac
-
-    assert developers(:jamis, :reload).projects.include?(ac)
-    project = developers(:jamis).projects.detect { |p| p == ac }
-    assert_equal 1, project.access_level.to_i
-  end
-
-  def test_habtm_attribute_access_and_respond_to
-    project = developers(:jamis).projects[0]
-    assert project.has_attribute?("name")
-    assert project.has_attribute?("joined_on")
-    assert project.has_attribute?("access_level")
-    assert project.respond_to?("name")
-    assert project.respond_to?("name=")
-    assert project.respond_to?("name?")
-    assert project.respond_to?("joined_on")
-    # given that the 'join attribute' won't be persisted, I don't
-    # think we should define the mutators
-    #assert project.respond_to?("joined_on=")
-    assert project.respond_to?("joined_on?")
-    assert project.respond_to?("access_level")
-    #assert project.respond_to?("access_level=")
-    assert project.respond_to?("access_level?")
-  end
-
   def test_habtm_adding_before_save
     no_of_devels = Developer.count
     no_of_projects = Project.count
@@ -430,10 +370,6 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     assert DeveloperWithBeforeDestroyRaise.connection.select_all("SELECT * FROM developers_projects WHERE developer_id = 1").empty?
   end
 
-  def test_additional_columns_from_join_table
-    assert_date_from_db Date.new(2004, 10, 10), Developer.find(1).projects.first.joined_on.to_date
-  end
-
   def test_destroying
     david = Developer.find(1)
     active_record = Project.find(1)
@@ -675,25 +611,6 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     assert_respond_to categories(:technology).select_testing_posts.find(:first), :correctness_marker
   end
 
-  def test_updating_attributes_on_rich_associations
-    david = projects(:action_controller).developers.first
-    david.name = "DHH"
-    assert_raise(ActiveRecord::ReadOnlyRecord) { david.save! }
-  end
-
-  def test_updating_attributes_on_rich_associations_with_limited_find_from_reflection
-    david = projects(:action_controller).selected_developers.first
-    david.name = "DHH"
-    assert_nothing_raised { 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
   end
diff --git a/activerecord/test/cases/readonly_test.rb b/activerecord/test/cases/readonly_test.rb
index 8d694900ff..e21109baae 100644
--- a/activerecord/test/cases/readonly_test.rb
+++ b/activerecord/test/cases/readonly_test.rb
@@ -44,15 +44,6 @@ class ReadOnlyTest < ActiveRecord::TestCase
     Developer.joins(', projects').readonly(false).each { |d| assert !d.readonly? }
   end
 
-
-  def test_habtm_find_readonly
-    dev = Developer.find(1)
-    assert !dev.projects.empty?
-    assert dev.projects.all?(&:readonly?)
-    assert dev.projects.find(:all).all?(&:readonly?)
-    assert dev.projects.readonly(true).all?(&:readonly?)
-  end
-
   def test_has_many_find_readonly
     post = Post.find(1)
     assert !post.comments.empty?