Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fixes #16172 - remove base_class override in template subclasses
On Rails 5.0, the base_class is used during model instantiation to
validate STI relationships as it now casts similarly to Foreman::STI,
causing failures when base_class returns the subclass rather than parent
class.

Removing the overriding of `base_class` in Template subclasses
necessitates changing polymorphic associations to store the base class
in the table's type field rather than the subclass name because the
has_many associations will search by `base_class`. This is recommended
by ActiveRecord when using STI with polymorphic associations:
https://github.com/rails/rails/blob/v5.0.0/activerecord/lib/active_record/associations.rb#L780
  • Loading branch information
domcleal authored and ares committed Aug 22, 2016
1 parent 9a1dfb9 commit 5908bf4
Show file tree
Hide file tree
Showing 10 changed files with 46 additions and 26 deletions.
2 changes: 1 addition & 1 deletion app/controllers/templates_controller.rb
Expand Up @@ -119,7 +119,7 @@ def set_locked(locked)

def load_history
return unless @template
@history = Audit.descending.where(:auditable_id => @template.id, :auditable_type => @template.class, :action => 'update')
@history = Audit.descending.where(:auditable_id => @template.id, :auditable_type => @template.class.base_class, :action => 'update')
end

def action_permission
Expand Down
8 changes: 0 additions & 8 deletions app/models/provisioning_template.rb
Expand Up @@ -58,14 +58,6 @@ def self.template_ids_for(hosts)
ids.uniq
end

# we have to override the base_class because polymorphic associations does not detect it correctly, more details at
# http://apidock.com/rails/ActiveRecord/Associations/ClassMethods/has_many#1010-Polymorphic-has-many-within-inherited-class-gotcha
def self.base_class
self
end
# not sure why but this changes table_name so we set it explicitly
self.table_name = 'templates'

def self.template_includes
super + [:template_kind, :template_combinations => [:hostgroup, :environment]]
end
Expand Down
12 changes: 4 additions & 8 deletions app/models/ptable.rb
Expand Up @@ -41,19 +41,15 @@ class Ptable < Template
end
}

# we have to override the base_class because polymorphic associations does not detect it correctly, more details at
# http://apidock.com/rails/ActiveRecord/Associations/ClassMethods/has_many#1010-Polymorphic-has-many-within-inherited-class-gotcha
def self.base_class
self
end
# this changes table_name so we set it explicitly
self.table_name = 'templates'

def self.template_includes
super + [:operatingsystems]
end

def preview_host_collection
super.where(:managed => true)
end

def taxonomy_foreign_conditions
{ :ptable_id => id }
end
end
6 changes: 6 additions & 0 deletions app/models/taxable_taxonomy.rb
Expand Up @@ -11,4 +11,10 @@ class TaxableTaxonomy < ActiveRecord::Base
where(["taxable_taxonomies.taxable_type NOT IN (?)",types])
end
}

# Always store the base type when associated to an STI class as the has_many scope on the target
# class will always default to searching for its base_class.
def taxable_type=(class_name)
super(class_name.constantize.base_class.to_s)
end
end
8 changes: 4 additions & 4 deletions app/models/taxonomy.rb
Expand Up @@ -14,8 +14,8 @@ class Taxonomy < ActiveRecord::Base
has_many :smart_proxies, :through => :taxable_taxonomies, :source => :taxable, :source_type => 'SmartProxy'
has_many :compute_resources, :through => :taxable_taxonomies, :source => :taxable, :source_type => 'ComputeResource'
has_many :media, :through => :taxable_taxonomies, :source => :taxable, :source_type => 'Medium'
has_many :provisioning_templates, :through => :taxable_taxonomies, :source => :taxable, :source_type => 'ProvisioningTemplate'
has_many :ptables, :through => :taxable_taxonomies, :source => :taxable, :source_type => 'Ptable'
has_many :provisioning_templates, -> { where(:type => 'ProvisioningTemplate') }, :through => :taxable_taxonomies, :source => :taxable, :source_type => 'Template'
has_many :ptables, -> { where(:type => 'Ptable') }, :through => :taxable_taxonomies, :source => :taxable, :source_type => 'Template'
has_many :domains, :through => :taxable_taxonomies, :source => :taxable, :source_type => 'Domain'
has_many :realms, :through => :taxable_taxonomies, :source => :taxable, :source_type => 'Realm'
has_many :hostgroups, :through => :taxable_taxonomies, :source => :taxable, :source_type => 'Hostgroup'
Expand Down Expand Up @@ -145,15 +145,15 @@ def dup
# if ignore?("Domain")
# Domain.pluck(:id)
# else
# self.taxable_taxonomies.where(:taxable_type => "Domain").pluck(:taxable_id)
# super() # self.domain_ids
# end
define_method(key) do
klass = hash_key_to_class(key)
if ignore?(klass)
return User.unscoped.except_admin.except_hidden.map(&:id) if klass == "User"
return klass.constantize.pluck(:id)
else
taxable_taxonomies.where(:taxable_type => klass).pluck(:taxable_id)
super()
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions app/services/tax_host.rb
Expand Up @@ -26,8 +26,8 @@ def selected_ids
return @selected_ids if @selected_ids
ids = default_ids_hash
#types NOT ignored - get ids that are selected
(taxonomy.taxable_taxonomies - taxonomy.ignore_types).group_by { |d| d[:taxable_type] }.map do |k, v|
ids["#{k.tableize.singularize}_ids"] = v.map { |i| i[:taxable_id] }
hash_keys.each do |col|
ids[col] = Array(taxonomy.send(col))
end
#types that ARE ignored - get ALL ids for object
Array(taxonomy.ignore_types).each do |taxonomy_type|
Expand Down
@@ -0,0 +1,12 @@
class ChangeTemplateTaxableTaxonomiesType < ActiveRecord::Migration
def up
known_types = Template.descendants.map(&:to_s)
TaxableTaxonomy.where(:taxable_type => known_types).update_all(:taxable_type => 'Template')
end

def down
Template.descendants.each do |type|
TaxableTaxonomy.where(:taxable_type => 'Template', :taxable_id => type.pluck(:id)).update_all(:taxable_type => type.to_s)
end
end
end
14 changes: 14 additions & 0 deletions db/migrate/20160719100624_change_template_audits_type.rb
@@ -0,0 +1,14 @@
class ChangeTemplateAuditsType < ActiveRecord::Migration
def up
known_types = Template.descendants.map(&:to_s)
Audit.where(:auditable_type => known_types).update_all(:auditable_type => 'Template')
Audit.where(:associated_type => known_types).update_all(:associated_type => 'Template')
end

def down
Template.descendants.each do |type|
Audit.where(:auditable_type => 'Template', :auditable_id => type.pluck(:id)).update_all(:auditable_type => type.to_s)
Audit.where(:associated_type => 'Template', :associated_id => type.pluck(:id)).update_all(:associated_type => type.to_s)
end
end
end
2 changes: 1 addition & 1 deletion db/seeds.rb
Expand Up @@ -16,7 +16,7 @@ def format_errors(model = nil)
# additional attributes may be specified for narrowing the scope but note
# that it can be slow if there's high number of audits for the specified type
def audit_modified?(type, name, attributes = {})
au = Audit.where(:auditable_type => type, :auditable_name => name)
au = Audit.where(:auditable_type => type.base_class, :auditable_name => name)

if attributes.present?
interesting_au = au.select do |audit|
Expand Down
4 changes: 2 additions & 2 deletions test/fixtures/taxable_taxonomies.yml
Expand Up @@ -67,12 +67,12 @@ twelve:
thirteen:
taxonomy: location1
taxable: mystring2
taxable_type: "ProvisioningTemplate"
taxable_type: "Template"

fourteen:
taxonomy: organization1
taxable: mystring2
taxable_type: "ProvisioningTemplate"
taxable_type: "Template"

fifteen:
taxonomy: location1
Expand Down

0 comments on commit 5908bf4

Please sign in to comment.