Fixes theforeman#7230, #12021 - Upgrade to Rails 4.1.5

This commits upgrades Rails to Rails 4.1.5. See a description of the
changes included here, and go to the pull request in GitHub to see more
detailed explanations:

* Update gems to a Rails 4 compatible version, including dependencies
* Fix counter cache columns
* Remove conditions, order, limit from has_many relations
* Remove test runner
    On minitest 5, the runner API was deprecated, so our custom test
    runner is no longer working.

* Remove useless add_index on lookup_values match
    An index is added previously on lookup_values :priority, and on Rails 4
    rename_column changes 'priority' to 'match' already changes the name of
    the index.

* Alias assert_include to assert_includes.
* Expire topbar cache
* Subclass ApplicationMailer in test to avoid 'missing template' error
* Fixes #12021 - Use .to_param to find parent object in LookupKeys controller
    On Rails 4, .find will not default to .friendly.find so find_hostgroup,
    find_environment and find_host will fail for non numeric IDs. However, we can
    use from_param to find these objects. This is something we can do both on
    Rails 3 and Rails 4, whereas using .friendly isn't an option until friendly_id
     5.0, which depends on Rails 4.

* Use explicit friendly ID search.
* find_common finder uses from_param -> friendly -> find
    The finder needed to be refactored because with the new Friendly ID we
    have to use .friendly explicitely. It currently follows the strategy of
    searching like this:
      - from_param -> .friendly -> regular find

* Remove RecordNotFound errors from parameterizable in rails 4
* Protect attributes using attr_accessible and protected_attributes
* url_for doesn't append port when protocol is specified
    Some of our tests are checking if port 80 or 443 is included in the URL
    generated by lib/foreman/renderer.rb. However url_for has changed in
    Rails 4 and now it does not append the port even if explicitly added.

    If the protocol is specified, as it is in this case, then the url
    generated will just have the protocol (http or https) but not the port
    (80 or 443)

* Validate object instead of _id column in join tables
    On Rails 4 such validations will fail when we try to create objects
    without explicitely assigning the id.
    Puppetclass.new(:config_group => config_group) would fail even for a
    valid config group, :config_group_id => config_group.id should be used
    instead.

    To avoid that, we validate the object, not the ID column

* Rails 4 non backwards compatible syntax changes
    Changes that have to deal with how some of the internal Rails objects
    need a new syntax, like generating routes, exceptions, form builders.
    None of these are Rails 3 compatible.

* Fixes #12199 - rails 4 migration errors
    There are actually two parts to the issue of not being able to migrate
    with this branch. The first is
    https://gist.github.com/eLobato/0f5db50b5c93cc6c277c and can be
    temporarily fixed with rails/sass-rails#136 (comment) I am not sure of a
    long term fix for that atm.

    Once that is patched, this error
    https://gist.github.com/johnpmitsch/96e5ba3629890931193a happens on a
    migration. This will fix that error by changing the Migrator class
    initializer arguments which have changed from rails 3 to rails 4

* Refactor fact value test to not modify user_roles directly
* Return external usergroupsas an array
* Use eager_load to preload associations to be used in where

Fixes theforeman#7230, #12021 - Upgrade to Rails 4.1.5

This commits upgrades Rails to Rails 4.1.5. See a description of the
changes included here, and go to the pull request in GitHub to see more
detailed explanations:

* Update gems to a Rails 4 compatible version, including dependencies
* Fix counter cache columns
* Remove conditions, order, limit from has_many relations
* Remove test runner
    On minitest 5, the runner API was deprecated, so our custom test
    runner is no longer working.

* Remove useless add_index on lookup_values match
    An index is added previously on lookup_values :priority, and on Rails 4
    rename_column changes 'priority' to 'match' already changes the name of
    the index.

* Alias assert_include to assert_includes.
* Expire topbar cache
* Subclass ApplicationMailer in test to avoid 'missing template' error
* Fixes #12021 - Use .to_param to find parent object in LookupKeys controller
    On Rails 4, .find will not default to .friendly.find so find_hostgroup,
    find_environment and find_host will fail for non numeric IDs. However, we can
    use from_param to find these objects. This is something we can do both on
    Rails 3 and Rails 4, whereas using .friendly isn't an option until friendly_id
     5.0, which depends on Rails 4.

* Use explicit friendly ID search.
* find_common finder uses from_param -> friendly -> find
    The finder needed to be refactored because with the new Friendly ID we
    have to use .friendly explicitely. It currently follows the strategy of
    searching like this:
      - from_param -> .friendly -> regular find

* Remove RecordNotFound errors from parameterizable in rails 4
* Protect attributes using attr_accessible and protected_attributes
* url_for doesn't append port when protocol is specified
    Some of our tests are checking if port 80 or 443 is included in the URL
    generated by lib/foreman/renderer.rb. However url_for has changed in
    Rails 4 and now it does not append the port even if explicitly added.

    If the protocol is specified, as it is in this case, then the url
    generated will just have the protocol (http or https) but not the port
    (80 or 443)

* Validate object instead of _id column in join tables
    On Rails 4 such validations will fail when we try to create objects
    without explicitely assigning the id.
    Puppetclass.new(:config_group => config_group) would fail even for a
    valid config group, :config_group_id => config_group.id should be used
    instead.

    To avoid that, we validate the object, not the ID column

* Rails 4 non backwards compatible syntax changes
    Changes that have to deal with how some of the internal Rails objects
    need a new syntax, like generating routes, exceptions, form builders.
    None of these are Rails 3 compatible.

* Fixes #12199 - rails 4 migration errors
    There are actually two parts to the issue of not being able to migrate
    with this branch. The first is
    https://gist.github.com/eLobato/0f5db50b5c93cc6c277c and can be
    temporarily fixed with rails/sass-rails#136 (comment) I am not sure of a
    long term fix for that atm.

    Once that is patched, this error
    https://gist.github.com/johnpmitsch/96e5ba3629890931193a happens on a
    migration. This will fix that error by changing the Migrator class
    initializer arguments which have changed from rails 3 to rails 4

* Refactor fact value test to not modify user_roles directly
* Return external usergroupsas an array
* Use eager_load to preload associations to be used in where
* Auto detect jenkins rake task in application.rb and set test RAILS_ENV
  in that case

Gemfile

@@ -4,29 +4,28 @@ require File.expand_path('../lib/regexp_extensions', FOREMAN_GEMFILE)
 
 source 'https://rubygems.org'
 
-gem 'rails', '3.2.22'
-gem 'rack-cache', '< 1.3.0'
+gem 'rails', '4.1.5'
 gem 'json', '~> 1.5'
 gem 'rest-client', '~> 1.6.0', :require => 'rest_client'
-gem 'audited-activerecord', '3.0.0'
+gem 'audited-activerecord', '~> 4.0'
 gem 'will_paginate', '~> 3.0'
 gem 'ancestry', '~> 2.0'
-gem 'scoped_search', '~> 3.0'
+gem 'scoped_search', '>= 3.2.2', '< 4'
 gem 'ldap_fluff', '>= 0.3.5', '< 1.0'
 gem 'net-ldap', '>= 0.8.0'
-gem 'apipie-rails', '~> 0.2.5'
+gem 'apipie-rails', '~> 0.3.4'
 gem 'rabl', '~> 0.11'
 gem 'oauth', '~> 0.4'
 gem 'deep_cloneable', '~> 2.0'
 gem 'foreigner', '~> 1.4'
 gem 'validates_lengths_from_database',  '~> 0.2'
-gem 'friendly_id', '~> 4.0'
+gem 'friendly_id', '~> 5.0'
 gem 'secure_headers', '~> 1.3'
 gem 'safemode', '~> 1.2'
 gem 'fast_gettext', '~> 0.8'
 gem 'gettext_i18n_rails', '~> 1.0'
 gem 'i18n', '~> 0.6.4'
-gem 'rails-i18n', '~> 3.0.0'
+gem 'rails-i18n', '~> 4.0.0'
 gem 'turbolinks', '~> 2.5'
 gem 'logging', '>= 1.8.0', '< 3.0.0'
 gem 'fog-core', '1.34.0'
@@ -36,6 +35,9 @@ if RUBY_VERSION.start_with? '1.9.'
 else
   gem 'net-ssh'
 end
+gem 'activerecord-session_store', '~> 0.1.1'
+gem 'rails-observers', '~> 0.1'
+gem 'protected_attributes', '~> 1.1.1'
 
 Dir["#{File.dirname(FOREMAN_GEMFILE)}/bundler.d/*.rb"].each do |bundle|
   self.instance_eval(Bundler.read_file(bundle))

app/assets/javascripts/topbar.js

@@ -50,7 +50,7 @@ $(document).on('mouseleave','.loc-submenu', function(){
 function mark_active_menu() {
   var menus = $('.menu_tab_dropdown'),
       path = window.location.pathname + window.location.search,
-      link = $("[href='%s'".replace('%s', path));
+      link = $("[href='%s']".replace('%s', path));
 
   menus.removeClass('active');
   $("[class^='menu_tab_']").removeClass('active');

app/controllers/api/v1/reports_controller.rb

@@ -36,7 +36,7 @@ def destroy
       param :id, :identifier, :required => true
 
       def last
-        conditions = { :host_id => Host.authorized(:view_hosts).find(params[:host_id]).try(:id) } if params[:host_id].present?
+        conditions = { :host_id => Host.authorized(:view_hosts).friendly.find(params[:host_id]).try(:id) } if params[:host_id].present?
         max_id = resource_scope.where(conditions).maximum(:id)
         @report = resource_scope.includes(:logs => [:message, :source]).find(max_id)
         render :show

app/controllers/api/v2/config_reports_controller.rb

@@ -54,7 +54,9 @@ def destroy
       param :id, :identifier, :required => true
 
       def last
-        conditions = { :host_id => Host.authorized(:view_hosts).find(params[:host_id]).try(:id) } if params[:host_id].present?
+        if params[:host_id].present?
+          conditions = { :host_id => resource_finder(Host.authorized(:view_hosts), params[:host_id]).try(:id) }
+        end
         max_id = resource_scope.where(conditions).maximum(:id)
         @config_report = resource_scope.includes(:logs => [:message, :source]).find(max_id)
         render :show

app/controllers/api/v2/host_classes_controller.rb

@@ -35,8 +35,10 @@ def destroy
       # overwrite resource_name so it's host and and not host_class, since we want to return @host
       def find_host
         not_found and return false if params[:host_id].blank?
-        @host = Host.find(params[:host_id]) if Host::Managed.respond_to?(:authorized) &&
-                                               Host::Managed.authorized("view_host", Host::Managed)
+        if Host::Managed.respond_to?(:authorized) &&
+            Host::Managed.authorized("view_host", Host::Managed)
+          @host = resource_finder(Host.authorized(:view_hosts), params[:host_id])
+        end
       end
     end
   end

app/controllers/api/v2/models_controller.rb

@@ -47,6 +47,12 @@ def update
       def destroy
         process_response @model.destroy
       end
+
+      private
+
+      def find_resource
+        @model = Model.friendly.find(params[:id]) || super
+      end
     end
   end
 end

app/controllers/api/v2/parameters_controller.rb

@@ -165,8 +165,9 @@ def allowed_nested_id
       def find_parameter
         # nested_obj is required, so no need to check here
         @parameters  = nested_obj.send(parameters_method)
-        @parameter   = @parameters.find(params[:id])
-        return @parameter if @parameter
+        @parameter = @parameters.from_param(params[:id])
+        @parameter ||= @parameters.friendly.find(params[:id])
+        return @parameter if @parameter.present?
         not_found
       end
     end

app/controllers/api/v2/reports_controller.rb

@@ -54,7 +54,7 @@ def destroy
       param :id, :identifier, :required => true
 
       def last
-        conditions = { :host_id => Host.authorized(:view_hosts).find(params[:host_id]).try(:id) } if params[:host_id].present?
+        conditions = { :host_id => resource_finder(Host.authorized(:view_hosts), params[:host_id]).try(:id) } if params[:host_id].present?
         max_id = resource_scope.where(conditions).maximum(:id)
         @report = resource_scope.includes(:logs => [:message, :source]).find(max_id)
         render :show

app/controllers/concerns/api/v2/lookup_keys_common_controller.rb

@@ -19,22 +19,6 @@ module Api::V2::LookupKeysCommonController
     before_filter :return_if_smart_mismatch, :only => [:show, :update, :destroy]
   end
 
-  def puppetclass_id?
-    params.keys.include?('puppetclass_id')
-  end
-
-  def environment_id?
-    params.keys.include?('environment_id')
-  end
-
-  def host_id?
-    params.keys.include?('host_id')
-  end
-
-  def hostgroup_id?
-    params.keys.include?('hostgroup_id')
-  end
-
   def smart_variable_id?
     params.keys.include?('smart_variable_id') || controller_name.match(/smart_variables/)
   end
@@ -43,28 +27,22 @@ def smart_class_parameter_id?
     params.keys.include?('smart_class_parameter_id') || controller_name.match(/smart_class_parameters/)
   end
 
-  def find_puppetclass
-    @puppetclass = Puppetclass.authorized(:view_puppetclasses).find(params['puppetclass_id'])
-  rescue ActiveRecord::RecordNotFound
-    not_found({ :error => { :message => (_("Puppet class with id '%{id}' was not found") % { :id => params['puppetclass_id'] }) } })
-  end
+  [Puppetclass, Environment, Host::Base, Hostgroup].each do |model|
+    model_string = model.to_s.split('::').first.downcase
 
-  def find_environment
-    @environment = Environment.authorized(:view_environments).find(params['environment_id'])
-  rescue ActiveRecord::RecordNotFound
-    not_found({ :error => { :message => (_("Environment with id '%{id}' was not found") % { :id => params['environment_id'] }) } })
-  end
-
-  def find_host
-    @host = Host::Base.authorized(:view_hosts).find(params['host_id'])
-  rescue ActiveRecord::RecordNotFound
-    not_found({ :error => { :message => (_("Host with id '%{id}' was not found") % { :id => params['host_id'] }) } })
-  end
+    define_method("#{model_string}_id?") do
+      params.keys.include?("#{model_string}_id")
+    end
 
-  def find_hostgroup
-    @hostgroup = Hostgroup.authorized(:view_hostgroups).find(params['hostgroup_id'])
-  rescue ActiveRecord::RecordNotFound
-    not_found({ :error => { :message => (_("Hostgroup with id '%{id}' was not found") % { :id => params['hostgroup_id'] }) } })
+    define_method("find_#{model_string}") do
+      scope = model.authorized(:"view_#{model_string.pluralize}")
+      begin
+        instance_variable_set("@#{model_string}",
+                              resource_finder(scope, params["#{model_string}_id"]))
+      rescue ActiveRecord::RecordNotFound
+        model_not_found(model_string)
+      end
+    end
   end
 
   def find_smart_variable
@@ -144,4 +122,13 @@ def return_if_smart_mismatch
       not_found "#{obj} not found by id '#{id}'"
     end
   end
+
+  private
+
+  def model_not_found(model)
+    error_message = (
+      _("%{model} with id '%{id}' was not found") %
+      { :id => params["#{model}_id"], :model => model.capitalize } )
+    not_found(:error => { :message => error_message } )
+  end
 end

app/controllers/concerns/find_common.rb

@@ -1,12 +1,18 @@
-# this mixin is used by both ApplicationController and Api::BaseController
-# searches for an object based on its id, name, label, etc and assign it to an instance variable
+# This mixin is used by both ApplicationController and Api::BaseController
+# Searches for an object based on its id, name, label, etc and assign it to an instance variable
 # friendly_id performs the logic if params[:id] is 'id' or 'id-name' or 'name'
 
 module FindCommon
-  # example: @host = Host.find(params[:id])
   def find_resource
-    not_found and return if params[:id].blank?
-    instance_variable_set("@#{resource_name}", resource_scope.find(params[:id]))
+    instance_variable_set("@#{resource_name}",
+                          resource_finder(resource_scope, params[:id]))
+  end
+
+  def resource_finder(scope, id)
+    raise ActiveRecord::RecordNotFound if scope.empty?
+    result = scope.from_param(id) if scope.respond_to?(:from_param)
+    result ||= scope.friendly.find(id) if scope.respond_to?(:friendly)
+    result || scope.find(id)
   end
 
   def resource_name(resource = controller_name)

app/controllers/config_reports_controller.rb

@@ -10,7 +10,7 @@ def index
   def show
     # are we searching for the last report?
     if params[:id] == "last"
-      conditions = { :host_id => Host.authorized(:view_hosts).find(params[:host_id]).try(:id) } if params[:host_id].present?
+      conditions = { :host_id => resource_finder(Host.authorized(:view_hosts), params[:host_id]).try(:id) } if params[:host_id].present?
       params[:id] = resource_base.where(conditions).maximum(:id)
     end
 

app/controllers/hosts_controller.rb

@@ -169,7 +169,7 @@ def puppetclass_parameters
   def externalNodes
     certname = params[:name]
     @host ||= resource_base.find_by_certname certname
-    @host ||= resource_base.find certname
+    @host ||= resource_base.friendly.find certname
     not_found and return unless @host
 
     begin
@@ -352,7 +352,7 @@ def update_multiple_parameters
       skipped = []
       params[:name].each do |name, value|
         next if value.empty?
-        if (host_param = host.host_parameters.find(name))
+        if (host_param = host.host_parameters.friendly.find(name))
           counter += 1 if host_param.update_attribute(:value, value)
         else
           skipped << name
@@ -698,7 +698,7 @@ def taxonomy_scope
   # overwrite application_controller
   def find_resource
     not_found and return false if (id = params[:id]).blank?
-    @host   = resource_base.find(id)
+    @host   = resource_base.friendly.find(id)
     @host ||= resource_base.find_by_mac params[:host][:mac] if params[:host] && params[:host][:mac]
 
     not_found and return(false) unless @host

app/controllers/settings_controller.rb

@@ -13,7 +13,7 @@ def index
   end
 
   def update
-    @setting = Setting.find(params[:id])
+    @setting = Setting.friendly.find(params[:id])
     if @setting.parse_string_value(params[:setting][:value]) && @setting.save
       render :json => @setting
     else

app/controllers/unattended_controller.rb

@@ -44,8 +44,8 @@ def built
   def template
     return head(:not_found) unless (params.has_key?("id") and params.has_key?(:hostgroup))
 
-    template = ProvisioningTemplate.find(params['id'])
-    @host = Hostgroup.find(params['hostgroup'])
+    template = ProvisioningTemplate.find_by_name(params['id'])
+    @host = Hostgroup.find_by_name(params['hostgroup'])
 
     return head(:not_found) unless template and @host
 
@@ -107,7 +107,7 @@ def get_host_details
 
   def find_host_by_spoof
     host = Nic::Base.primary.find_by_ip(params.delete('spoof')).try(:host) if params['spoof'].present?
-    host ||= Host.find(params.delete('hostname')) if params['hostname'].present?
+    host ||= Host.find_by_name(params.delete('hostname')) if params['hostname'].present?
     @spoof = host.present?
     host
   end

app/helpers/application_helper.rb

@@ -32,6 +32,14 @@ def link_to(*args, &block)
     end
   end
 
+  def link_to_function(name, function, html_options = {})
+    onclick_tag = "#{html_options[:onclick]}; " if html_options[:onclick]
+    onclick = "#{onclick_tag}#{function}; return false;"
+    href = html_options[:href] || '#'
+
+    content_tag(:a, name, html_options.merge(:href => href, :onclick => onclick))
+  end
+
   protected
 
   def contract(model)

app/models/bookmark.rb

@@ -7,7 +7,7 @@ class Bookmark < ActiveRecord::Base
   validates_lengths_from_database
 
   belongs_to :owner, :polymorphic => true
-  attr_accessible :name, :controller, :query, :public
+  attr_accessible :name, :query, :public, :controller
   audited :allow_mass_assignment => true
 
   validates :name, :uniqueness => {:scope => :controller}, :unless => Proc.new{|b| Bookmark.my_bookmarks.where(:name => b.name).empty?}

app/models/compute_profile.rb

@@ -6,6 +6,7 @@ class ComputeProfile < ActiveRecord::Base
 
   validates_lengths_from_database
   attr_accessible :name
+
   audited
   has_associated_audits
 

app/models/concerns/audit_extensions.rb

@@ -5,7 +5,8 @@ module AuditExtensions
   included do
     belongs_to :user, :class_name => 'User'
     belongs_to :search_users, :class_name => 'User', :foreign_key => :user_id
-    belongs_to :search_hosts, :class_name => 'Host', :foreign_key => :auditable_id, :conditions => { :audits => { :auditable_type => 'Host' } }
+    belongs_to :search_hosts, -> { where(:audits => { :auditable_type => 'Host' }) },
+      :class_name => 'Host', :foreign_key => :auditable_id
     belongs_to :search_hostgroups, :class_name => 'Hostgroup', :foreign_key => :auditable_id
     belongs_to :search_parameters, :class_name => 'Parameter', :foreign_key => :auditable_id
     belongs_to :search_templates, :class_name => 'ProvisioningTemplate', :foreign_key => :auditable_id

app/models/concerns/has_many_common.rb

@@ -41,8 +41,9 @@ class << self; self end
     end
 
     #### has_many ####
-    def has_many(association, options = {})
-      has_many_names_for(association, options)
+    def has_many(*args)
+      options = args.last.is_a?(Hash) ? args.last : {}
+      has_many_names_for(args.first, options)
       super
     end
 
@@ -69,8 +70,9 @@ def has_many_names_for(association, options)
     end
 
     #### belongs_to ####
-    def belongs_to(association, options = {})
-      belongs_to_name_for(association, options)
+    def belongs_to(*args)
+      options = args.last.is_a?(Hash) ? args.last : {}
+      belongs_to_name_for(args.first, options)
       super
     end
 

app/models/concerns/host_common.rb

@@ -209,7 +209,7 @@ def individual_puppetclasses
   end
 
   def available_puppetclasses
-    return Puppetclass.where(nil) if environment_id.blank?
+    return Puppetclass.where(nil) if environment.blank?
     environment.puppetclasses - parent_classes
   end
 

app/models/concerns/host_params.rb

@@ -7,6 +7,7 @@ module HostParams
     accepts_nested_attributes_for :host_parameters, :allow_destroy => true
     include ParameterValidators
     attr_reader :cached_host_params
+    attr_accessible :host_parameters_attributes
 
     def params
       host_params.update(lookup_keys_params)
@@ -58,7 +59,7 @@ def host_inherited_params_objects
 
     def host_params_objects
       # Host parameters should always be first for the uniq order
-      (host_parameters + host_inherited_params_objects.reverse!).uniq {|param| param.name}
+      (host_parameters + host_inherited_params_objects.to_a.reverse!).uniq {|param| param.name}
     end
   end
 end