I noticed that 4 katello tables have a 'type' for field for STI.
katello_content_view_filters
class ContentViewErratumFilter < ContentViewFilter
class ContentViewPackageFilter < ContentViewFilter
class ContentViewPackageGroupFilter < ContentViewFilter
katello_products
class MarketingProduct < Product
katello_task_statuses
class PulpTaskStatus < TaskStatus
katello_systems
type db field not used
It doesn't appear that we need the 'type' field in katello_systems for STI, so I am renaming it to 'system_type'. https://github.com/Katello/katello/pull/4421
However, I don't understand the apipie docs in katello/app/controllers/katello/api/v2/systems_controller.rb
def_param_group :system do
param :type, String, :desc => N_("Type of the content host, it should always be 'content host'"), :required => true, :action_aware => true
but in the POST and PUT actions, it says
param :type, String, :desc => N_("Type of the content host, it should always be 'system'"), :required => true, :action_aware => true
However, in the db, type is 'Katello::System'.
I don't think we want to tell the API uses to pass 'content host' or 'system' as the type. If so, it breaks on my dev environment.
On the STI topic, I think it would be cleaner to have katello models inherit from ActiveRecord::Base rather than Katello::Model which is only used to turn on/off strong_parameters. Are there any objections to re-factoring this as a mixin or extension of Active::Model rather than using inheritance.
module Katello
class Model < ActiveRecord::Base
self.abstract_class = true
# remove once foreman has strong_parameters or Rails 4
include Katello::ForbiddenAttributesProtection
def destroy!
unless destroy
fail self.errors.full_messages.join('; ')
end
end
end
end
Regards,
Joseph