From b7f6d7c052c3562e3fb74babf7bbada023071f1e Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Sat, 30 Jul 2022 21:11:35 +0200 Subject: [PATCH 1/7] Fixes #35300 - Pass keyword arguments correctly In Ruby 3.0 keyword arguments are handled separate from positional arguments[1]. This uses kwargs and passes it along. [1]: https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/ --- app/helpers/application_helper.rb | 2 +- app/models/concerns/foreman/sti.rb | 2 +- app/models/concerns/has_many_common.rb | 22 +++++++++---------- app/services/csv_exporter.rb | 2 +- .../foreman/renderer/scope/template.rb | 2 +- 5 files changed, 14 insertions(+), 16 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 6cd1e472907..01dd29ae09e 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -161,7 +161,7 @@ def auto_complete_controller_name def sort(field, permitted: [], **kwargs) kwargs[:url_options] ||= current_url_params(permitted: permitted) - super(field, kwargs) + super(field, **kwargs) end def help_button diff --git a/app/models/concerns/foreman/sti.rb b/app/models/concerns/foreman/sti.rb index 8cc5965ae53..79a59f418f5 100644 --- a/app/models/concerns/foreman/sti.rb +++ b/app/models/concerns/foreman/sti.rb @@ -20,7 +20,7 @@ def new(*attributes, &block) end end - def save(*args) + def save(*args, **kwargs) type_changed = type_changed? self.class.instance_variable_set("@finder_needs_type_condition", :false) if type_changed super diff --git a/app/models/concerns/has_many_common.rb b/app/models/concerns/has_many_common.rb index dc4721de1af..b2d1cc1260c 100644 --- a/app/models/concerns/has_many_common.rb +++ b/app/models/concerns/has_many_common.rb @@ -42,18 +42,17 @@ class << self; self end end #### has_many #### - def has_many(*args) - options = args.last.is_a?(Hash) ? args.last : {} - has_many_names_for(args.first, options) + def has_many(name, scope = nil, **kwargs, &extension) + has_many_names_for(name) super end - def has_and_belongs_to_many(association, options = {}) - has_many_names_for(association, options) + def has_and_belongs_to_many(name, scope = nil, **kwargs, &extension) + has_many_names_for(name) super end - def has_many_names_for(association, options) + def has_many_names_for(association) assoc = association.to_s.tableize.singularize # SETTER _names= method @@ -71,15 +70,14 @@ def has_many_names_for(association, options) end #### belongs_to #### - def belongs_to(*args) - options = args.last.is_a?(Hash) ? args.last : {} - belongs_to_name_for(args.first, options) - super + def belongs_to(name, scope = nil, name_accessor: nil, **kwargs) + belongs_to_name_for(name, name_accessor) + super(name, scope, **kwargs) end - def belongs_to_name_for(association, options) + def belongs_to_name_for(association, name_accessor) assoc = association.to_s.tableize.singularize - assoc_name = options.delete(:name_accessor) || "#{assoc}_name" + assoc_name = name_accessor || "#{assoc}_name" # SETTER _name= method define_method "#{assoc_name}=" do |name_value| diff --git a/app/services/csv_exporter.rb b/app/services/csv_exporter.rb index 611956cea4c..598aa12fb70 100644 --- a/app/services/csv_exporter.rb +++ b/app/services/csv_exporter.rb @@ -8,7 +8,7 @@ def self.export(resources, columns, header = nil) context = Foreman::ThreadSession::Context.get Enumerator.new do |csv| - Foreman::ThreadSession::Context.set(context) + Foreman::ThreadSession::Context.set(**context) csv << CSV.generate_line(header) cols = columns.map { |c| c.to_s.split('.').map(&:to_sym) } resources.uncached do diff --git a/app/services/foreman/renderer/scope/template.rb b/app/services/foreman/renderer/scope/template.rb index 1c4c2f70174..099664e1fdc 100644 --- a/app/services/foreman/renderer/scope/template.rb +++ b/app/services/foreman/renderer/scope/template.rb @@ -6,7 +6,7 @@ class Template < Foreman::Renderer::Scope::Base attr_reader :template_input_values def initialize(template_input_values: {}, **params) - super(params) + super(**params) @template_input_values = template_input_values end end From 14e409a2d35369552a1b1025df023ae58b2d970a Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Thu, 25 Aug 2022 14:11:45 +0200 Subject: [PATCH 2/7] Refs #35432 - Use Rails 5.0 defaults Rails has introduced config.load_defaults[1] to load the defaults of a certain version. Currently this is not called at all, which means using Rails < 5.0 defaults. Changing this brings Foreman more in line with the Rails recommended defaults. [1]: https://guides.rubyonrails.org/configuring.html#versioned-default-values --- config/application.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/config/application.rb b/config/application.rb index cfb1fd9d169..a778d8204a5 100644 --- a/config/application.rb +++ b/config/application.rb @@ -100,6 +100,11 @@ class Foreman::Consoletie < Rails::Railtie module Foreman class Application < Rails::Application + config.load_defaults 5.0 + + # Rails 5.0 changed this to true, but a lot of code depends on this + config.active_record.belongs_to_required_by_default = false + # Setup additional routes by loading all routes file from routes directory Dir["#{Rails.root}/config/routes/**/*.rb"].each do |route_file| config.paths['config/routes.rb'] << route_file From 35124b1239b62b746563156bc00f4e933ddd24b5 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Thu, 25 Aug 2022 14:11:45 +0200 Subject: [PATCH 3/7] Refs #35432 - Use Rails 5.1 defaults --- config/application.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index a778d8204a5..8a73997fe6b 100644 --- a/config/application.rb +++ b/config/application.rb @@ -100,7 +100,7 @@ class Foreman::Consoletie < Rails::Railtie module Foreman class Application < Rails::Application - config.load_defaults 5.0 + config.load_defaults 5.1 # Rails 5.0 changed this to true, but a lot of code depends on this config.active_record.belongs_to_required_by_default = false From 69079b5668f40cf8c6dc2f15b5a304189df4d860 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Thu, 25 Aug 2022 14:11:45 +0200 Subject: [PATCH 4/7] Refs #35432 - Use Rails 5.2 defaults --- config/application.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index 8a73997fe6b..88725049a9f 100644 --- a/config/application.rb +++ b/config/application.rb @@ -100,7 +100,7 @@ class Foreman::Consoletie < Rails::Railtie module Foreman class Application < Rails::Application - config.load_defaults 5.1 + config.load_defaults 5.2 # Rails 5.0 changed this to true, but a lot of code depends on this config.active_record.belongs_to_required_by_default = false From 01be6cf897a61385fec4fd2241780585319d199c Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Thu, 25 Aug 2022 14:11:45 +0200 Subject: [PATCH 5/7] Refs #35432 - Use Rails 6.0 defaults --- config/application.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index 88725049a9f..cd476d6c784 100644 --- a/config/application.rb +++ b/config/application.rb @@ -100,11 +100,14 @@ class Foreman::Consoletie < Rails::Railtie module Foreman class Application < Rails::Application - config.load_defaults 5.2 + config.load_defaults 6.0 # Rails 5.0 changed this to true, but a lot of code depends on this config.active_record.belongs_to_required_by_default = false + # Rails 6.0 changed this to :zeitwerk + config.autoloader = :classic + # Setup additional routes by loading all routes file from routes directory Dir["#{Rails.root}/config/routes/**/*.rb"].each do |route_file| config.paths['config/routes.rb'] << route_file From 468aa81cb39f663f7021d62f4f97c3abc0a06a11 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Thu, 25 Aug 2022 14:11:45 +0200 Subject: [PATCH 6/7] Fixes #35432 - Use Rails 6.1 defaults Rails has introduced config.load_defaults[1] to load the defaults of a certain version. Currently this is not called at all, which means using Rails < 5.0 defaults. Changing this brings Foreman more in line with the Rails recommended defaults. [1]: https://guides.rubyonrails.org/configuring.html#versioned-default-values --- config/application.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index cd476d6c784..41bd63141f0 100644 --- a/config/application.rb +++ b/config/application.rb @@ -100,7 +100,7 @@ class Foreman::Consoletie < Rails::Railtie module Foreman class Application < Rails::Application - config.load_defaults 6.0 + config.load_defaults 6.1 # Rails 5.0 changed this to true, but a lot of code depends on this config.active_record.belongs_to_required_by_default = false From 90d5a96098d0476c83b53fae9902b5f8af3fcea7 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Thu, 25 Aug 2022 15:21:24 +0200 Subject: [PATCH 7/7] WIP --- test/models/external_usergroup_test.rb | 2 +- test/models/host_test.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/models/external_usergroup_test.rb b/test/models/external_usergroup_test.rb index e43f258e742..44ff9210c18 100644 --- a/test/models/external_usergroup_test.rb +++ b/test/models/external_usergroup_test.rb @@ -18,7 +18,7 @@ class ExternalUsergroupTest < ActiveSupport::TestCase test 'update_usergroups matches LDAP gids with external user groups case insensitively' do setup_ldap_stubs - @auth_source_ldap.expects(:valid_group?).with('IPAUSERS').returns(true) + @auth_source_ldap.expects(:valid_group?).with('IPAUSERS').returns(true).twice external = FactoryBot.create(:external_usergroup, :auth_source => @auth_source_ldap, :name => 'IPAUSERS') ldap_user = FactoryBot.create(:user, :login => 'JohnSmith', :mail => 'a@b.com', :auth_source => @auth_source_ldap) AuthSourceLdap.any_instance.expects(:users_in_group).with('IPAUSERS').returns(['JohnSmith']) diff --git a/test/models/host_test.rb b/test/models/host_test.rb index d3ef5f484ec..772a8be0ff2 100644 --- a/test/models/host_test.rb +++ b/test/models/host_test.rb @@ -2500,8 +2500,8 @@ def to_managed! refute_nil host.provision_interface primary = host.primary_interface - nic = FactoryBot.build_stubbed(:nic_managed, :host => host, :mac => '00:00:00:AA:BB:CC', :ip => '192.168.0.1', - :name => 'host2', :domain => host.domain) + nic = FactoryBot.build(:nic_managed, :host => host, :mac => '00:00:00:AA:BB:CC', :ip => '192.168.0.1', + :name => 'host2', :domain => host.domain) nic.primary = true nic.provision = true