-
Notifications
You must be signed in to change notification settings - Fork 900
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rails 7.1 friendly attribute changes #23337
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,7 +51,6 @@ class MiqRequest < ApplicationRecord | |
virtual_column :v_workflow_class, :type => :string, :uses => :workflow | ||
virtual_column :request_type_display, :type => :string | ||
virtual_column :resource_type, :type => :string | ||
virtual_column :state, :type => :string | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. handled by alias_attribute on line 23 ☝️ |
||
|
||
delegate :allowed_tags, :to => :workflow, :prefix => :v, :allow_nil => true | ||
delegate :class, :to => :workflow, :prefix => :v_workflow | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,9 @@ def deprecate_attribute(old_attribute, new_attribute, type:) | |
end | ||
|
||
def deprecate_attribute_methods(old_attribute, new_attribute) | ||
alias_attribute old_attribute, new_attribute | ||
define_method(old_attribute) { self.send(new_attribute) } | ||
define_method("#{old_attribute}=") { |object| self.send("#{new_attribute}=", object) } | ||
define_method("#{old_attribute}?") { self.send("#{new_attribute}?") } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. handles non-attribute calls to deprecate_attribute |
||
["", "=", "?"].each { |suffix| Vmdb::Deprecation.deprecate_methods(self, "#{old_attribute}#{suffix}" => "#{new_attribute}#{suffix}") } | ||
end | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,7 +52,6 @@ class TenantQuota < ApplicationRecord | |
scope :templates_allocated, -> { where(:name => :templates_allocated) } | ||
|
||
virtual_column :name, :type => :string | ||
virtual_column :total, :type => :integer | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. handled by alias_attribute on line 61 👇 |
||
virtual_column :used, :type => :float | ||
virtual_column :allocated, :type => :float | ||
virtual_column :available, :type => :float | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,4 @@ | ||
RSpec.describe DeprecationMixin do | ||
# Host.deprecate_attribute :address, :hostname | ||
context ".arel_table" do | ||
# this is defining an alias | ||
# it is not typical for aliases to work through arel_table | ||
# may need to get rid of this in the future | ||
it "works for deprecate_attribute columns" do | ||
expect(Host.attribute_supported_by_sql?(:address)).to eq(true) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This arel_table context was testing the internals. We're no longer using virtual columns or alias_attribute as someone could use deprecate_attribute on associations or attributes. The former won't work in rails 7.1 without warnings and will not work with virtual columns. It's much simpler to handle attributes or associations through runtime delegations although there is some performance implications we're accepting for this flexibility. We can bring that back in a different way if we need it. |
||
expect(Host.arel_table[:address]).to_not be_nil | ||
expect(Host.arel_table[:address].name).to eq("hostname") # typically this is a symbol. not perfect but it works | ||
end | ||
end | ||
|
||
# Host.deprecate_attribute :address, :hostname | ||
context ".visible_attribute_names" do | ||
it "hides deprecate_attribute columns" do | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to keep these comments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add them to each method if it makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was one method that lost it: #23338