Merge pull request #188 from kbrock/deprecate_type

Deprecate message when virtual_delegate is not passed type

Author: Fryguy

CHANGELOG.md

@@ -4,6 +4,10 @@ The versioning of this gem follows ActiveRecord versioning, and does not follow
 
 ## [Unreleased]
 
+## [7.1.1] - 2025-06-18
+
+* Deprecate virtual_delegate without a type [#188](https://github.com/ManageIQ/activerecord-virtual_attributes/pull/188)
+
 ## [7.1.0] - 2025-02-19
 
 * Use TableAlias for table aliasing [#168](https://github.com/ManageIQ/activerecord-virtual_attributes/pull/168)
@@ -111,7 +115,8 @@ The versioning of this gem follows ActiveRecord versioning, and does not follow
 * Initial Release
 * Extracted from ManageIQ/manageiq
 
-[Unreleased]: https://github.com/ManageIQ/activerecord-virtual_attributes/compare/v7.1.0...HEAD
+[Unreleased]: https://github.com/ManageIQ/activerecord-virtual_attributes/compare/v7.1.1...HEAD
+[7.1.1]: https://github.com/ManageIQ/activerecord-virtual_attributes/compare/v7.1.0...v7.1.1
 [7.1.0]: https://github.com/ManageIQ/activerecord-virtual_attributes/compare/v7.0.0...v7.1.0
 [7.0.0]: https://github.com/ManageIQ/activerecord-virtual_attributes/compare/v6.1.2...v7.0.0
 [6.1.2]: https://github.com/ManageIQ/activerecord-virtual_attributes/compare/v6.1.1...v6.1.2

lib/active_record/virtual_attributes.rb

@@ -1,6 +1,7 @@
 require "active_support/concern"
 require "active_record"
 
+require "active_record/virtual_attributes/version"
 require "active_record/virtual_attributes/virtual_includes"
 require "active_record/virtual_attributes/virtual_arel"
 require "active_record/virtual_attributes/virtual_delegates"
@@ -37,6 +38,10 @@ def type
     ActiveRecord::Type.register(:string_set, Type::StringSet)
     ActiveRecord::Type.register(:symbol, Type::Symbol)
 
+    def self.deprecator
+      @deprecator ||= ActiveSupport::Deprecation.new(ActiveRecord::VirtualAttributes::VERSION, "virtual_attributes")
+    end
+
     included do
       class_attribute :virtual_attributes_to_define, :instance_accessor => false, :default => {}
     end

lib/active_record/virtual_attributes/version.rb

@@ -1,5 +1,5 @@
 module ActiveRecord
   module VirtualAttributes
-    VERSION = "7.1.0".freeze
+    VERSION = "7.1.1".freeze
   end
 end

lib/active_record/virtual_attributes/virtual_delegates.rb

@@ -24,6 +24,10 @@ def virtual_delegate(*methods)
             raise ArgumentError, 'Delegation needs an association. Supply an options hash with a :to key as the last argument (e.g. delegate :hello, to: :greeter).'
           end
 
+          unless options[:type]
+            ActiveRecord::VirtualAttributes.deprecator.warn("Calling virtual_delegate without :type is now deprecated", caller)
+          end
+
           to = to.to_s
           if to.include?(".") && methods.size > 1
             raise ArgumentError, 'Delegation only supports specifying a method name when defining a single virtual method'

spec/virtual_attributes_spec.rb

@@ -482,7 +482,7 @@ def col2
       end
 
       it "supports delegates" do
-        TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1
+        TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1, :type => :integer
 
         expect(TestClass.attribute_supported_by_sql?(:parent_col1)).to be_truthy
       end
@@ -530,7 +530,7 @@ def col2
       end
 
       it "supports delegates" do
-        TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1
+        TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1, :type => :integer
 
         expect(TestClass.attribute_supported_by_sql?(:parent_col1)).to be_truthy
       end
@@ -598,42 +598,42 @@ def self.connection
       let(:parent) { TestClass.create(:col1 => 4) }
 
       it "delegates to child" do
-        TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1
+        TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1, :type => :integer
         tc = TestClass.new(:ref1 => parent)
         expect(tc.parent_col1).to eq(4)
       end
 
       it "delegates to nil child" do
-        TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1, :allow_nil => true
+        TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1, :allow_nil => true, :type => :integer
         tc = TestClass.new
         expect(tc.parent_col1).to be_nil
       end
 
       it "defines virtual attribute" do
-        TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1
+        TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1, :type => :integer
         expect(TestClass.virtual_attribute_names).to include("parent_col1")
       end
 
       it "defines with a new name" do
-        TestClass.virtual_delegate 'funky_name', :to => "ref1.col1"
+        TestClass.virtual_delegate 'funky_name', :to => "ref1.col1", :type => :integer
         tc = TestClass.new(:ref1 => parent)
         expect(tc.funky_name).to eq(4)
       end
 
       it "defaults for to nil child (array)" do
-        TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1, :allow_nil => true, :default => []
+        TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1, :allow_nil => true, :default => [], :type => :integer
         tc = TestClass.new
         expect(tc.parent_col1).to eq([])
       end
 
       it "defaults for to nil child (integer)" do
-        TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1, :allow_nil => true, :default => 0
+        TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1, :allow_nil => true, :default => 0, :type => :integer
         tc = TestClass.new
         expect(tc.parent_col1).to eq(0)
       end
 
       it "defaults for to nil child (string)" do
-        TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1, :allow_nil => true, :default => "def"
+        TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1, :allow_nil => true, :default => "def", :type => :integer
         tc = TestClass.new
         expect(tc.parent_col1).to eq("def")
       end

spec/virtual_delegates_spec.rb

@@ -3,24 +3,24 @@
   let(:parent) { TestClass.create(:col1 => 4) }
 
   it "delegates to parent" do
-    TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1
+    TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1, :type => :integer
     tc = TestClass.new(:ref1 => parent)
     expect(tc.parent_col1).to eq(4)
   end
 
   it "delegates to nil parent" do
-    TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1, :allow_nil => true
+    TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1, :allow_nil => true, :type => :integer
     tc = TestClass.new
     expect(tc.parent_col1).to be_nil
   end
 
   it "defines parent virtual attribute" do
-    TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1
+    TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1, :type => :integer
     expect(TestClass.virtual_attribute_names).to include("parent_col1")
   end
 
   it "delegates to parent (sql)" do
-    TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1
+    TestClass.virtual_delegate :col1, :prefix => 'parent', :to => :ref1, :type => :integer
     TestClass.create(:ref1 => parent)
     tcs = TestClass.select(:id, :col1, TestClass.arel_table[:parent_col1].as("x"))
     expect(tcs.map(&:x)).to match_array([nil, 4])
@@ -44,21 +44,27 @@
       end.to raise_error(ArgumentError, /needs an association/)
     end
 
+    it "expects a ':type' for delegation" do
+      expect(ActiveRecord::VirtualAttributes.deprecator).to receive(:warn).with(/type/, anything())
+      TestClass.virtual_delegate :col1, :to => :ref1
+      TestClass.new
+    end
+
     it "only allows 1 method when delegating to a specific method" do
       expect do
-        TestClass.virtual_delegate :col1, :col2, :to => "ref1.method"
+        TestClass.virtual_delegate :col1, :col2, :to => "ref1.method", :type => :string
       end.to raise_error(ArgumentError, /single virtual method/)
     end
 
     it "only allows 1 level deep delegation" do
       expect do
-        TestClass.virtual_delegate :col1, :to => "ref1.method.method2"
+        TestClass.virtual_delegate :col1, :to => "ref1.method.method2", :type => :string
       end.to raise_error(ArgumentError, /single association/)
     end
 
     it "detects invalid destination" do
       expect do
-        TestClass.virtual_delegate :col1, :to => "bogus_ref.method"
+        TestClass.virtual_delegate :col1, :to => "bogus_ref.method", :type => :string
         TestClass.new
       end.to raise_error(ArgumentError, /needs an association/)
     end
@@ -72,24 +78,24 @@
     let(:child) { TestClass.create }
 
     it "delegates to child" do
-      TestClass.virtual_delegate :col1, :prefix => 'child', :to => :ref2
+      TestClass.virtual_delegate :col1, :prefix => 'child', :to => :ref2, :type => :integer
       tc = TestClass.create(:ref2 => child)
       expect(tc.child_col1).to eq(tc.id)
     end
 
     it "delegates to nil child" do
-      TestClass.virtual_delegate :col1, :prefix => 'child', :to => :ref2, :allow_nil => true
+      TestClass.virtual_delegate :col1, :prefix => 'child', :to => :ref2, :allow_nil => true, :type => :integer
       tc = TestClass.new
       expect(tc.child_col1).to be_nil
     end
 
     it "defines child virtual attribute" do
-      TestClass.virtual_delegate :col1, :prefix => 'child', :to => :ref2
+      TestClass.virtual_delegate :col1, :prefix => 'child', :to => :ref2, :type => :integer
       expect(TestClass.virtual_attribute_names).to include("child_col1")
     end
 
     it "delegates to child (sql)" do
-      TestClass.virtual_delegate :col1, :prefix => 'child', :to => :ref2
+      TestClass.virtual_delegate :col1, :prefix => 'child', :to => :ref2, :type => :integer
       tc = TestClass.create(:ref2 => child)
       tcs = TestClass.select(:id, :col1, :child_col1).to_a
       expect { expect(tcs.map(&:child_col1)).to match_array([nil, tc.id]) }.to_not make_database_queries
@@ -98,7 +104,7 @@
     # this may fail in the future as our way of building queries may change
     # just want to make sure it changed due to intentional changes
     it "uses table alias for subquery" do
-      TestClass.virtual_delegate :col1, :prefix => 'child', :to => :ref2
+      TestClass.virtual_delegate :col1, :prefix => 'child', :to => :ref2, :type => :integer
       sql = TestClass.select(:id, :col1, :child_col1).to_sql
       expect(sql).to match(/["`]test_classes_[^"`]*["`][.]["`]col1["`]/i)
     end
@@ -114,7 +120,7 @@
     # ensure virtual attribute referencing a relation with a select()
     # does not throw an exception due to multi-column select
     it "properly generates sub select" do
-      TestClass.virtual_delegate :col1, :prefix => 'child', :to => :ref2
+      TestClass.virtual_delegate :col1, :prefix => 'child', :to => :ref2, :type => :integer
       TestClass.create(:ref2 => child)
       expect { TestClass.select(:id, :child_col1).to_a }.to_not raise_error
     end
@@ -131,7 +137,7 @@
     # ensure virtual attribute referencing a relation with a select()
     # does not throw an exception due to multi-column select
     it "properly generates sub select" do
-      TestClass.virtual_delegate :col1, :prefix => 'child', :to => :ref2
+      TestClass.virtual_delegate :col1, :prefix => 'child', :to => :ref2, :type => :integer
       TestClass.create(:ref2 => child)
       expect { TestClass.select(:id, :child_col1).to_a }.to_not raise_error
     end
@@ -150,7 +156,7 @@ def self.connection
       end
       # TODO: -> { order(:col1) }
       TestClass.has_one :child, :class_name => 'TestOtherClass', :foreign_key => :ocol1
-      TestClass.virtual_delegate :child_str, :to => "child.ostr"
+      TestClass.virtual_delegate :child_str, :to => "child.ostr", :type => :string
     end
 
     after do
@@ -185,7 +191,7 @@ def self.connection
     end
 
     it "delegates to another table" do
-      TestOtherClass.virtual_delegate :col1, :to => :oref1
+      TestOtherClass.virtual_delegate :col1, :to => :oref1, :type => :integer
       TestOtherClass.create(:oref1 => TestClass.create)
       TestOtherClass.create(:oref1 => TestClass.create(:col1 => 99))
       tcs = TestOtherClass.select(:id, :ocol1, TestOtherClass.arel_table[:col1].as("x"))
@@ -198,7 +204,7 @@ def self.connection
     # this may fail in the future as our way of building queries may change
     # just want to make sure it changed due to intentional changes
     it "delegates to another table without alias" do
-      TestOtherClass.virtual_delegate :col1, :to => :oref1
+      TestOtherClass.virtual_delegate :col1, :to => :oref1, :type => :integer
       sql = TestOtherClass.select(:id, :ocol1, TestOtherClass.arel_table[:col1].as("x")).to_sql
       expect(sql).to match(/["`]test_classes["`].["`]col1["`]/i)
     end