Merge pull request #49019 from adrianna-chang-shopify/ac-deprecate-read-attribute-id-pk

Deprecate `read_attribute(:id)` returning the primary key

activerecord/CHANGELOG.md

@@ -1,3 +1,11 @@
+*   Deprecate `read_attribute(:id)` returning the primary key if the primary key is not `:id`.
+
+    Starting in Rails 7.2, `read_attribute(:id)` will return the value of the id column, regardless of the model's
+    primary key. To retrieve the value of the primary key, use `#id` instead. `read_attribute(:id)` for composite
+    primary key models will now return the value of the id column.
+
+    *Adrianna Chang*
+
 *   Fix `change_table` setting datetime precision for 6.1 Migrations
 
     *Hartley McGuire*

activerecord/lib/active_record/associations/collection_association.rb

@@ -333,11 +333,7 @@ def merge_target_lists(persisted, memory)
             if mem_record = memory.delete(record)
 
               ((record.attribute_names & mem_record.attribute_names) - mem_record.changed_attribute_names_to_save - mem_record.class._attr_readonly).each do |name|
-                if name == "id" && mem_record.class.composite_primary_key?
-                  mem_record.class.primary_key.zip(record[name]) { |attr, value| mem_record._write_attribute(attr, value) }
-                else
-                  mem_record._write_attribute(name, record[name])
-                end
+                mem_record._write_attribute(name, record[name])
               end
 
               mem_record

activerecord/lib/active_record/attribute_methods/read.rb

@@ -33,8 +33,14 @@ def read_attribute(attr_name, &block)
         return @attributes.fetch_value(name, &block) unless name == "id" && @primary_key
 
         if self.class.composite_primary_key?
-          @primary_key.map { |col| @attributes.fetch_value(col, &block) }
+          @attributes.fetch_value("id", &block)
         else
+          if @primary_key != "id"
+            ActiveRecord.deprecator.warn(<<-MSG.squish)
+              Using read_attribute(:id) to read the primary key value is deprecated.
+              Use #id instead.
+            MSG
+          end
           @attributes.fetch_value(@primary_key, &block)
         end
       end

activerecord/test/cases/primary_keys_test.rb

@@ -36,22 +36,32 @@ def test_to_key_with_composite_primary_key
     assert_equal [1, 2], order.to_key
   end
 
+  def test_read_attribute_id
+    topic = Topic.find(1)
+    id = assert_not_deprecated(ActiveRecord.deprecator) do
+      topic.read_attribute(:id)
+    end
+
+    assert_equal 1, id
+  end
+
   def test_read_attribute_with_custom_primary_key
     keyboard = Keyboard.create!
-    assert_equal keyboard.key_number, keyboard.read_attribute(:id)
+    msg = "Using read_attribute(:id) to read the primary key value is deprecated. Use #id instead."
+    id = assert_deprecated(msg, ActiveRecord.deprecator) do
+      keyboard.read_attribute(:id)
+    end
+
+    assert_equal keyboard.key_number, id
   end
 
   def test_read_attribute_with_composite_primary_key
     book = Cpk::Book.new(id: [1, 2])
-    assert_equal [1, 2], book.read_attribute(:id)
-  end
+    id = assert_not_deprecated(ActiveRecord.deprecator) do
+      book.read_attribute(:id)
+    end
 
-  def test_read_attribute_with_composite_primary_key_and_column_named_id
-    order = Cpk::Order.new
-    order.id = [1, 2]
-
-    assert_equal [1, 2], order.read_attribute(:id)
-    assert_equal 2, order.attributes["id"]
+    assert_equal 2, id
   end
 
   def test_to_key_with_primary_key_after_destroy