Merge pull request rails#52145 from Shopify/alias-attribute-override-inherited-methods

Fix `alias_attribute` to ignore methods defined in parent classes

Author: byroot

activemodel/lib/active_model/attribute_methods.rb

@@ -312,26 +312,33 @@ def define_attribute_method(attr_name, _owner: generated_attribute_methods, as:
         end
       end
 
-      def define_attribute_method_pattern(pattern, attr_name, owner:, as:) # :nodoc:
+      def define_attribute_method_pattern(pattern, attr_name, owner:, as:, override: false) # :nodoc:
         canonical_method_name = pattern.method_name(attr_name)
         public_method_name = pattern.method_name(as)
 
-        unless instance_method_already_implemented?(public_method_name)
-          generate_method = "define_method_#{pattern.proxy_target}"
+        # If defining a regular attribute method, we don't override methods that are explictly
+        # defined in parrent classes.
+        if instance_method_already_implemented?(public_method_name)
+          # However, for `alias_attribute`, we always define the method.
+          # We check for override second because `instance_method_already_implemented?`
+          # also check for dangerous methods.
+          return unless override
+        end
 
-          if respond_to?(generate_method, true)
-            send(generate_method, attr_name.to_s, owner: owner, as: as)
-          else
-            define_proxy_call(
-              owner,
-              canonical_method_name,
-              pattern.proxy_target,
-              pattern.parameters,
-              attr_name.to_s,
-              namespace: :active_model_proxy,
-              as: public_method_name
-            )
-          end
+        generate_method = "define_method_#{pattern.proxy_target}"
+
+        if respond_to?(generate_method, true)
+          send(generate_method, attr_name.to_s, owner: owner, as: as)
+        else
+          define_proxy_call(
+            owner,
+            canonical_method_name,
+            pattern.proxy_target,
+            pattern.parameters,
+            attr_name.to_s,
+            namespace: :active_model_proxy,
+            as: public_method_name
+          )
         end
       end
 

activerecord/lib/active_record/attribute_methods.rb

@@ -91,7 +91,7 @@ def alias_attribute_method_definition(code_generator, pattern, new_name, old_nam
           raise ArgumentError, "#{self.name} model aliases `#{old_name}`, but `#{old_name}` is not an attribute. " \
             "Use `alias_method :#{new_name}, :#{old_name}` or define the method manually."
         else
-          define_attribute_method_pattern(pattern, old_name, owner: code_generator, as: new_name)
+          define_attribute_method_pattern(pattern, old_name, owner: code_generator, as: new_name, override: true)
         end
       end
 

activerecord/test/cases/attribute_methods_test.rb

@@ -1217,6 +1217,25 @@ def some_method_that_is_not_on_super
     alias_attribute :subject, :title
   end
 
+  test "#alias_attribute override methods defined in parent models" do
+    parent_model = Class.new(ActiveRecord::Base) do
+      self.abstract_class = true
+
+      def subject
+        "Abstract Subject"
+      end
+    end
+
+    subclass = Class.new(parent_model) do
+      self.table_name = "topics"
+      alias_attribute :subject, :title
+    end
+
+    obj = subclass.new
+    obj.title = "hey"
+    assert_equal("hey", obj.subject)
+  end
+
   test "aliases to the same attribute name do not conflict with each other" do
     first_model_object = ToBeLoadedFirst.new(author_name: "author 1")
     assert_equal("author 1", first_model_object.subject)