Rails 7.1 friendly attribute changes #23337
Conversation
|
@miq-bot cross-repo-tests /all |
From Pull Request: ManageIQ/manageiq#23337
| # 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.
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.
| @@ -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.
handled by alias_attribute on line 61 👇
| 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.
handles non-attribute calls to deprecate_attribute
| @@ -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.
handled by alias_attribute on line 23 ☝️
Fixes deprecation of this variety: Foo model aliases `x`, but `x` is not an attribute. Starting in Rails 7.2, alias_attribute with non-attribute targets will raise. Use `alias_method :y, :x` or define the method manually. This also handles aliases on association methods We use runtime method delegation instead of class load time alias_method. In Rails 7.1, the attribute/association methods aren't defined at class load time so we can't alias them. Note, we can remove virtual_column/attribute on actual attributes with alias_attribute due to: ManageIQ#23324 ManageIQ/manageiq-automation_engine#565 ManageIQ/manageiq-api#1278 ManageIQ#23321
Note, rails 7.1 lazily load attribute methods so we can't use alias_method here.
Rails 7.1 lazily defines methods. These are sometimes called with associations, not attributes, so we can't use alias_attribute. Fixes the following error with Rails 7.1: Association named 'provisioning_manager' was not found on CustomizationScript
jrafanie
force-pushed
the
rails71-friendly-attribute-changes
branch
from
2a8b3e1 to
f34896c
Compare
|
bumping after ManageIQ/manageiq-ui-classic#9351 merge |
|
@kbrock this is green now along along with the cross repo |
| alias_attribute :miq_provision_request, :miq_request # Legacy provisioning support | ||
| alias_attribute :provision_type, :request_type # Legacy provisioning support |
There was a problem hiding this comment.
do we want to keep these comments?
There was a problem hiding this comment.
I can add them to each method if it makes sense.
There was a problem hiding this comment.
It was one method that lost it: #23338
…attribute-changes Revive a comment accidentally dropped in #23337
(made possible by introduce all_attribute_names #23324, Expose attribute_aliases as attributes manageiq-automation_engine#565, Expose attribute aliases in list of attributes manageiq-api#1278, deprecate attribute no longer uses virtual attribute #23321)
Extracted from #23225 to make it easier to review both this and the rails 7.1 PR separately.