Skip to content
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

Hide deprecated and duplicate attributes #20664

Merged
merged 2 commits into from
Dec 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
2 changes: 2 additions & 0 deletions app/models/ext_management_system.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ class ExtManagementSystem < ApplicationRecord
include ExternalUrlMixin
include VerifyCredentialsMixin

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)
where(:tenant_id => tenant.ancestor_ids + [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
24 changes: 24 additions & 0 deletions lib/extensions/ar_visible_attribute.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
module ArVisibleAttribute
extend ActiveSupport::Concern

included do
class_attribute :hidden_attribute_names, :default => []
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
#
# NOTE: only use in class definitions, or child classes will be broken
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
attribute_names - hidden_attribute_names
kbrock marked this conversation as resolved.
Show resolved Hide resolved
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
97 changes: 97 additions & 0 deletions spec/lib/extensions/ar_visible_attribute_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
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" } }
let(:child_klass) { Class.new(klass) }

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

it "shows virtual attributes" do
klass.virtual_attribute :superb, :string
expect(klass.visible_attribute_names).to include("superb")
end

it "hides hidden virtual attributes" do
klass.virtual_attribute :superb, :string
klass.hide_attribute :superb
expect(klass.visible_attribute_names).not_to include("superb")
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

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

it "hides attributes that are hidden in parent class" do
klass.hide_attribute :type
expect(child_klass.visible_attribute_names).not_to include("type")
end

it "hides virtual attributes that are hidden in the parent class" do
klass.virtual_attribute :superb, :string
klass.hide_attribute :superb
expect(child_klass.visible_attribute_names).not_to include("superb")
end

it "hides attributes that are hidden in class and parent class" do
klass.hide_attribute :type
child_klass.hide_attribute :name
expect(child_klass.visible_attribute_names).not_to include("type")
expect(child_klass.visible_attribute_names).not_to include("name")
end

it "hides attribute only for class and below" do
child_klass.hide_attribute :name
expect(klass.visible_attribute_names).to include("name")
expect(child_klass.visible_attribute_names).not_to include("name")
end
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
kbrock marked this conversation as resolved.
Show resolved Hide resolved

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

it "hides attributes that are hidden in parent class" do
klass.hide_attribute :type
expect(child_klass.hidden_attribute_names).to include("type")
end

it "hides attributes that are hidden in parent class" do
klass.hide_attribute :type
child_klass.hide_attribute :name
expect(child_klass.hidden_attribute_names).to include("type")
expect(child_klass.hidden_attribute_names).to include("name")
end
end
end
end
7 changes: 7 additions & 0 deletions spec/models/mixins/deprecation_mixin_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,11 @@
expect(Host.arel_attribute(: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
expect(Host.visible_attribute_names).not_to include("address")
end
end
end