Skip to content

Commit

Permalink
Hide deprecated and duplicate attributes
Browse files Browse the repository at this point in the history
Some virtual_attributes are not needed in reporting nor the api.
Some deprecated attributes should not be advertised for customers to start using.

Now, the attributes are still accessible. So nothing will break. But
these attributes are no longer advertised on the public apis (i.e.: report builder and rest api).

This allows us to control which parts of our models are exposed.

This fixes ManageIQ#18130
This enables ManageIQ#20532

Hide deprecated and duplicate attributes

Some virtual_attributes are not needed in reporting nor the api.
Some deprecated attributes should not be advertised for customers to use.

After this change, the attributes are still accessible. So nothing will break, but
these attributes are no longer advertised in the public apis (i.e.: report builder and rest api).

This allows us to control which parts of our models are exposed.

This fixes ManageIQ#18130
This enables ManageIQ#20532
  • Loading branch information
kbrock committed Nov 2, 2020
1 parent 2aff38e commit 67b6ce4
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 1 deletion.
1 change: 1 addition & 0 deletions app/models/application_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class ApplicationRecord < ActiveRecord::Base
include ArNestedCountBy
include ArHrefSlug
include ToModelHash
include ArVisibleAttribute

extend ArTableLock
extend ArReferences
Expand Down
3 changes: 3 additions & 0 deletions app/models/ext_management_system.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ class ExtManagementSystem < ApplicationRecord
include SupportsFeatureMixin
include ExternalUrlMixin
include VerifyCredentialsMixin
include DeprecationMixin

hide_attribute "aggregate_memory" # better to use total_memory (coin toss - they're similar)

def self.with_tenant(tenant_id)
tenant = Tenant.find(tenant_id)
Expand Down
1 change: 1 addition & 0 deletions app/models/mixins/deprecation_mixin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ def deprecate_belongs_to(old_belongs_to, new_belongs_to)
def deprecate_attribute(old_attribute, new_attribute)
deprecate_attribute_methods(old_attribute, new_attribute)
virtual_attribute(old_attribute, -> { type_for_attribute(new_attribute.to_s) })
hide_attribute(old_attribute)
end

def deprecate_attribute_methods(old_attribute, new_attribute)
Expand Down
1 change: 1 addition & 0 deletions lib/acts_as_ar_model.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
class ActsAsArModel
include Vmdb::Logging
include ArVisibleAttribute

def self.connection
ActiveRecord::Base.connection
Expand Down
23 changes: 23 additions & 0 deletions lib/extensions/ar_visible_attribute.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
module ArVisibleAttribute
extend ActiveSupport::Concern

included do
class_attribute :hidden_attribute_names
self.hidden_attribute_names = []
end

class_methods do
# @param [String|Symbol] attribute name of attribute to be hidden from the api and reporting
# this attribute is accessible to all ruby methods. But it is not advertised.
# we do this when deprecating an attribute or when introducing an internal attribute
def hide_attribute(attribute)
self.hidden_attribute_names += [attribute.to_s]
end

# @return Array[String] attribute names that can be advertised in the api and reporting
# Other attributes are accessible, they are just no longer in our public api (or never were)
def visible_attribute_names
self.attribute_names - self.hidden_attribute_names
end
end
end
2 changes: 1 addition & 1 deletion lib/miq_expression.rb
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,7 @@ def self.build_relats(model, parent = {}, seen = [])
parent[:class_path] ||= model.name
parent[:assoc_path] ||= model.name
parent[:root] ||= model.name
result = {:columns => model.attribute_names, :parent => parent}
result = {:columns => model.visible_attribute_names, :parent => parent}
result[:reflections] = {}

model.reflections_with_virtual.each do |assoc, ref|
Expand Down
37 changes: 37 additions & 0 deletions spec/lib/extensions/ar_visible_attribute_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
RSpec.describe ArVisibleAttribute do
# NOTE: ApplicationRecord already included ArVisibleAttribute
let(:klass) { Class.new(ApplicationRecord) { self.table_name = "vms" } }
let(:other_klass) { Class.new(ApplicationRecord) { self.table_name = "vms" } }

context ".visible_attribute_names" do
it "shows regular attributes" do
expect(klass.visible_attribute_names).to include("type")
end

it "hides hidden attributes" do
klass.hide_attribute :type
expect(klass.visible_attribute_names).not_to include("type")
end

it "only hides for specified class" do
klass.hide_attribute :type
expect(other_klass.visible_attribute_names).to include("type")
end
end

context ".hidden_attribute_names" do
it "starts with no hidden attributes" do
expect(klass.hidden_attribute_names).to be_empty
end

it "hides hidden attributes" do
klass.hide_attribute :type
expect(klass.hidden_attribute_names).to include("type")
end

it "only hides for specified class" do
klass.hide_attribute :type
expect(other_klass.hidden_attribute_names).not_to include("type")
end
end
end

0 comments on commit 67b6ce4

Please sign in to comment.