Skip to content

Commit

Permalink
Handle deprecated classes in arel
Browse files Browse the repository at this point in the history
Attribute deprecation is based upon aliases and virtual attributes.
Ensure both concepts are working correctly together.

Bug: The virtual attribute was generating bogus arel, when it should
have been deferring to the attribute_alias mechanism built into rails

So the new code prioritizes attribute_alias over virtual_attribute

https://bugzilla.redhat.com/show_bug.cgi?id=1507047
  • Loading branch information
kbrock committed Nov 30, 2017
1 parent 026070f commit a17dc23
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 4 deletions.
2 changes: 1 addition & 1 deletion lib/extensions/ar_virtual.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ module VirtualArel
module ClassMethods
def arel_attribute(column_name, arel_table = self.arel_table)
load_schema
if virtual_attribute?(column_name)
if virtual_attribute?(column_name) && !attribute_alias?(column_name)
col = _virtual_arel[column_name.to_s]
col.call(arel_table) if col
else
Expand Down
51 changes: 48 additions & 3 deletions spec/lib/extensions/ar_virtual_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,18 +103,22 @@ def self.connection
end

context ".virtual_columns=" do
it "" do
it "can have multiple virtual columns" do
TestClass.virtual_column :existing_vcol, :type => :string

{
:vcol1 => {:type => :string},
"vcol2" => {:type => :string},
}.each do |name, options|
TestClass.virtual_column name, options
end

expect(TestClass.virtual_attribute_names).to match_array(["vcol1", "vcol2"])
expect(TestClass.virtual_attribute_names).to match_array(%w(existing_vcol vcol1 vcol2))
end
end

it "with existing virtual columns" do
context ".virtual_attribute_names" do
it "has virtual attributes" do
TestClass.virtual_column :existing_vcol, :type => :string

{
Expand All @@ -126,6 +130,20 @@ def self.connection

expect(TestClass.virtual_attribute_names).to match_array(["existing_vcol", "vcol1", "vcol2"])
end

it "does not have aliases" do
TestClass.virtual_attribute :existing_vcol, :string
TestClass.alias_attribute :col2, :col1
expect(TestClass.virtual_attribute_names).to match_array(["existing_vcol"])
end

it "supports virtual_column aliases" do
TestClass.virtual_attribute :existing_vcol, :string
TestClass.alias_attribute :col2, :col1
TestClass.virtual_attribute :col2, :integer

expect(TestClass.virtual_attribute_names).to match_array(%w(existing_vcol col2))
end
end

shared_examples_for "TestClass with virtual columns" do
Expand All @@ -148,6 +166,12 @@ def self.connection
it("as string") { expect(TestClass.virtual_attribute?("col1")).not_to be_truthy }
it("as symbol") { expect(TestClass.virtual_attribute?(:col1)).not_to be_truthy }
end

context "with alias" do
before { TestClass.alias_attribute :col2, :col1 }
it("as string") { expect(TestClass.virtual_attribute?("col1")).not_to be_truthy }
it("as symbol") { expect(TestClass.virtual_attribute?(:col1)).not_to be_truthy }
end
end

it ".remove_virtual_fields" do
Expand Down Expand Up @@ -661,6 +685,27 @@ def col2
end
end

describe ".arel_attribute" do
it "supports aliases" do
TestClass.alias_attribute :col2, :col1

arel_attr = TestClass.arel_attribute(:col2)
expect(arel_attr).to_not be_nil
expect(arel_attr.name).to eq("col1") # typically this is a symbol. not perfect but it works
end

# NOTE: should not need to add a virtual attribute to an alias
# TODO: change code for reports and automate to expose aliases like it does with attributes/virtual attributes.
it "supports aliases marked as a virtual_attribute" do
TestClass.alias_attribute :col2, :col1
TestClass.virtual_attribute :col2, :integer

arel_attr = TestClass.arel_attribute(:col2)
expect(arel_attr).to_not be_nil
expect(arel_attr.name).to eq("col1") # typically this is a symbol. not perfect but it works
end
end

describe "#select" do
it "supports virtual attributes" do
class TestClass
Expand Down
8 changes: 8 additions & 0 deletions spec/models/mixins/deprecation_mixin_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
describe DeprecationMixin do
# Host.deprecate_attribute :address, :hostname
it "works with arel attributes" do
expect(Host.attribute_supported_by_sql?(:address)).to eq(true)
expect(Host.arel_attribute(:address)).to_not be_nil
expect(Host.arel_attribute(:address).name).to eq("hostname") # typically this is a symbol. not perfect but it works
end
end

0 comments on commit a17dc23

Please sign in to comment.